Closed Bug 905784 Opened 6 years ago Closed 6 years ago

/system/build.prop do not have 'debug.sf.hw=1' on moz built ROM for hamachi(buri) and leo

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:leo+, b2g18 verified)

RESOLVED FIXED
blocking-b2g leo+
Tracking Status
b2g18 --- verified

People

(Reporter: sotaro, Assigned: sotaro)

References

Details

(Whiteboard: [NPOVB])

Attachments

(2 files, 5 obsolete files)

+++ This bug was initially created as a clone of Bug #900029 +++

From Bug 900029 comment #27, it become clear that "debug.sf.hw=1" property is necessary.
Component: Graphics: Layers → General
Product: Core → Boot2Gecko
Duplicate of this bug: 905787
"debug.sf.hw=1" is defined in system.prop. But there is no such file at /device/qcom/hamachi.
Assignee: nobody → sotaro.ikeda.g
Attached file system.prop for hamachi (obsolete) —
In source code tree, the file should be at /device/qcom/hamachi.
Sushil, is attachment 790984 [details] enough for the b2g device? Is it necessary to include everything from '/device/qcom/msm7627a/system.prop' ?
Flags: needinfo?(sushilchauhan)
Blocks: 900029
Nominate to leo+. ROM does not have correct "system property". This causes incorrect gralloc buffer allocation and rendering flicker.
blocking-b2g: - → leo?
Summary: /system/build.prop do not have 'debug.sf.hw=1' on hamachi(buri) → /system/build.prop do not have 'debug.sf.hw=1' on hamachi(buri) and leo
Sotaro,

Let's slow down a bit. I believe making this change means we can no longer fall back on system heap when we run out of PMEM [1] since canFallback() fails on COMPOSITION_TYPE_MDP [2]. In other words, we go out-of-memory.

IMO it's more sensible to instead fallback to GPU composition when HWC tries to render a system memory buffer. If this isn't happening correctly, maybe this is the bug we should address.

[1] https://www.codeaurora.org/cgit/external/gigabyte/platform/hardware/qcom/display/tree/libgralloc/alloc_controller.cpp?h=caf/b2g/ics_strawberry_v1#n304
[2] https://www.codeaurora.org/cgit/external/gigabyte/platform/hardware/qcom/display/tree/libgralloc/alloc_controller.cpp?h=caf/b2g/ics_strawberry_v1#n66
Diego, thanks for the advice! I quickly looked the gecko code. There seems the problematic code. gecko calls mHwc->prepare() only within OpenGL rendering. Function calling path is following. If rendering is done only by HwComposer, mHwc->prepare() is not called.

LayerManagerOGL::EndTransaction()
  ->LayerManagerOGL::Render()//OpenGL render.
    ->GLContextEGL::SwapBuffers()
      ->HWComposer::swapBuffers()
        ->mHwc->prepare(mHwc, NULL);
        ->mHwc->set(mHwc, dpy, surf, 0);
(In reply to Sotaro Ikeda [:sotaro] from comment #7)
>         ->mHwc->prepare(mHwc, NULL);
>         ->mHwc->set(mHwc, dpy, surf, 0);

And the prepare()'s argument is NULL.
prepare()'s problem is a different problem. I will create a bug for it.
Depends on: 905882
(In reply to Diego Wilson [:diego] from comment #6)
>
> IMO it's more sensible to instead fallback to GPU composition when HWC tries
> to render a system memory buffer. If this isn't happening correctly, maybe
> this is the bug we should address.
> 
I created Bug 905882 for the above.
I confirmed that Leo's partner built rom has 'debug.sf.hw=1' in 'build.prop'.
Confirmed that hamach's partner built rom also has 'debug.sf.hw=1' in 'build.prop'.
From comment #12 and comment #13, partner's ROMs have 'debug.sf.hw=1'. So, it is a problem only happens on moz built rom. This bug does not block partners, but this bug seriously block mozilla's development of B2G.
Summary: /system/build.prop do not have 'debug.sf.hw=1' on hamachi(buri) and leo → /system/build.prop do not have 'debug.sf.hw=1' on moz built ROM for hamachi(buri) and leo
Compared /device/qcom/msm7627a/system.prop with hamachi and leo's build.prop.
- hamachi: default msm7627a's system.prop is used for hamachi build.prop.
- leo: almost default msm7627a's system.prop is used for hamachi build.prop.
       only one line is changed.

From the above, it seems better to use default msm7627a's system.prop for hamachi and leo device.
blocking-b2g: leo? → leo+
Whiteboard: [NPOVB]
No longer depends on: 905882
Comment on attachment 790984 [details]
system.prop for hamachi

From comment #15, msm7627a's default 'system.prop' file seems OK for hamachi(buri) and leo.
Attachment #790984 - Attachment is obsolete: true
Attached file Pull request to android-device-hamachi (obsolete) —
Pull request of adding system.prop to hamachi.
Attached file Pull request to device-leo (obsolete) —
Pull request of adding system.prop to leo.
Comment on attachment 791599 [details]
Pull request to android-device-hamachi

mwu, can you review the patch? Already confirmed on v1.1 hamachi.
Attachment #791599 - Flags: review?(mwu)
Comment on attachment 791600 [details]
Pull request to device-leo

mwu, can you review the patch? Already confirmed on v1.1 leo.
Attachment #791600 - Flags: review?(mwu)
How do these patches work? Why not just add an appropriate entry to https://github.com/mozilla-b2g/device-leo/blob/master/full_leo.mk#L13 (PRODUCT_PROPERTY_OVERRIDE)?
I just found the magic which makes this work. If we want to switch to the default msm7627a system props, we should just point to them rather than making a copy. Does a symlink work?
(In reply to Michael Wu [:mwu] from comment #21)
> How do these patches work?

system.prop is used to generate build.prop.
https://github.com/mozilla-b2g/platform_build/blob/master/core/Makefile#L146

> Why not just add an appropriate entry to
> https://github.com/mozilla-b2g/device-leo/blob/master/full_leo.mk#L13
> (PRODUCT_PROPERTY_OVERRIDE)?

I feel it is saner to use deault system.prop just come from msm7627a.
(In reply to Michael Wu [:mwu] from comment #22)
> I just found the magic which makes this work. If we want to switch to the
> default msm7627a system props, we should just point to them rather than
> making a copy. Does a symlink work?

I am going to check if this way works.
Attachment #791599 - Attachment is obsolete: true
Attachment #791599 - Flags: review?(mwu)
Attachment #791600 - Attachment is obsolete: true
Attachment #791600 - Flags: review?(mwu)
Attached file Pull request to device-leo (obsolete) —
Create a symlink to msm7627a's system.prop during build.
Attached file Pull request to android-device-hamachi (obsolete) —
Attachment #794704 - Flags: review?(mwu)
Attachment #794705 - Flags: review?(mwu)
Why don't we just commit the symlink itself instead of generating one during build? Is there an issue with relative paths in symlinks?
Thanks for the comment. It is better!
Attachment #794705 - Attachment is obsolete: true
Attachment #794705 - Flags: review?(mwu)
Attachment #794704 - Attachment is obsolete: true
Attachment #794704 - Flags: review?(mwu)
Attachment #796859 - Flags: review?(mwu)
Attachment #796862 - Flags: review?(mwu)
Flags: needinfo?(sushilchauhan)
Comment on attachment 796859 [details]
Pull request to android-device-hamachi

Awesome.

BTW b2g-inbound is closed at the moment. I'm going to hold off merging until it's back open again.
Attachment #796859 - Flags: review?(mwu) → review+
Attachment #796862 - Flags: review?(mwu) → review+
Both PRs have been merged. We'll probably need an uplift.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
jhford, can you help to update the leo.xml and hamachi.xml in v1-train b2g-manifest to include the change in Comment 33?
Flags: needinfo?(jhford)
(In reply to Sotaro Ikeda [:sotaro] from comment #34)
> jhford, can you help to update the leo.xml and hamachi.xml in v1-train
> b2g-manifest to include the change in Comment 33?

Per the leo manifest, we don't need to change anything and this change has gone live for leo:

<project path="device/qcom/leo" name="device-leo" remote="b2g" revision="v    1-train"/>

Same for Hamachi:

<project path="device/qcom/hamachi" name="android-device-hamachi" remote="    b2g" revision="v1-train"/>

Let me know if you have any further concerns, otherwise, I think we can mark this as fixed on v1-train.
Flags: needinfo?(jhford)
Oh, sorry I did not checked it.
Duplicate of this bug: 900029
The issue is fixed on Leo and Buri MOZ RIL
/system/build.prop has 'debug.sf.hw=1'

Environmental  Variables:
Build ID: 20130903041201
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/9da3739ce7d7
Gaia: 8dc90703f4151d6f2a0decede0ee702f425a3a38
Platform Version: 18.1

Environmental  Variables:
Build ID: 20130904041204
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/307824edadd7
Gaia: d0a415bbf23e5d01c2b287d9fca708e167cfe70d
Platform Version: 18.1
Do we need to get new vendor firmware for our buri devices if we're using the latest gecko now?
(In reply to Ben Kelly [:bkelly] from comment #39)
> Do we need to get new vendor firmware for our buri devices if we're using
> the latest gecko now?

What is the intent of your question? IIRC, I do not know such information. Bug 905882 is going to be fixed soon. It is going to change hal area, but it is not a firmware.
Flags: needinfo?(bkelly)
Well, it seemed that the memory fallback strategy in gecko has to be aligned with the flags in gonk.  Perhaps I am confused and this is not an issue.

In very recent gecko I have been seeing copybit errors in the log again and I thought it was because my gonk firmware was out of date.
Flags: needinfo?(bkelly)
Although now I see that this bug is for moz-specific gonk and not the vendor firmware.  So if I am running vendor firmware then this bug should not come into play.  Sorry for the noise.
(In reply to Ben Kelly [:bkelly] from comment #41)
> In very recent gecko I have been seeing copybit errors in the log again and
> I thought it was because my gonk firmware was out of date.

'copybit errors' is not related to a firmware's version. It is related to gonk hal source's version and system properties values.
(In reply to Sotaro Ikeda [:sotaro] from comment #43)
> 'copybit errors' is not related to a firmware's version. It is related to
> gonk hal source's version and system properties values.

Recently, on hamachi and leo, 'Update just gecko and gaia" is recommended. So, in this case, vendor ROM's ones are used for gonk hal and system properties values.
You need to log in before you can comment on or make changes to this bug.