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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Sotaro Ikeda [:sotaro]]

+++ 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.

Comment 2

[Sotaro Ikeda [:sotaro]]

"debug.sf.hw=1" is defined in system.prop. But there is no such file at /device/qcom/hamachi.

Comment 3

[Sotaro Ikeda [:sotaro]]

Attachment: system.prop for hamachi

In source code tree, the file should be at /device/qcom/hamachi.

Comment 4

[Sotaro Ikeda [:sotaro]]

Sushil, is attachment 790984 [details] enough for the b2g device? Is it necessary to include everything from '/device/qcom/msm7627a/system.prop' ?

Comment 5

[Sotaro Ikeda [:sotaro]]

Nominate to leo+. ROM does not have correct "system property". This causes incorrect gralloc buffer allocation and rendering flicker.

Comment 6

[Diego Wilson [:diego]]

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

Comment 7

[Sotaro Ikeda [:sotaro]]

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);

Comment 8

[Sotaro Ikeda [:sotaro]]

Following seems non-contiguous memory check. It called under mHwc->prepare().
 
https://www.codeaurora.org/cgit/external/gigabyte/platform/hardware/qcom/display/tree/libhwcomposer/a-family/hwcomposer.cpp?h=caf/b2g/ics_strawberry_v1#n417

Comment 9

[Sotaro Ikeda [:sotaro]]

(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.

Comment 10

[Sotaro Ikeda [:sotaro]]

prepare()'s problem is a different problem. I will create a bug for it.

Comment 11

[Sotaro Ikeda [:sotaro]]

(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.

Comment 12

[Sotaro Ikeda [:sotaro]]

I confirmed that Leo's partner built rom has 'debug.sf.hw=1' in 'build.prop'.

Comment 13

[Sotaro Ikeda [:sotaro]]

Confirmed that hamach's partner built rom also has 'debug.sf.hw=1' in 'build.prop'.

Comment 14

[Sotaro Ikeda [:sotaro]]

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.

Comment 15

[Sotaro Ikeda [:sotaro]]

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.

Comment 16

[Sotaro Ikeda [:sotaro]]

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.

Comment 17

[Sotaro Ikeda [:sotaro]]

Attachment: Pull request to android-device-hamachi

Pull request of adding system.prop to hamachi.

Comment 18

[Sotaro Ikeda [:sotaro]]

Attachment: Pull request to device-leo

Pull request of adding system.prop to leo.

Comment 19

[Sotaro Ikeda [:sotaro]]

Comment on attachment 791599 [details]
Pull request to android-device-hamachi

mwu, can you review the patch? Already confirmed on v1.1 hamachi.

Comment 20

[Sotaro Ikeda [:sotaro]]

Comment on attachment 791600 [details]
Pull request to device-leo

mwu, can you review the patch? Already confirmed on v1.1 leo.

Comment 21

[Michael Wu [: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)?

Comment 22

[Michael Wu [:mwu]]

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?

Comment 23

[Sotaro Ikeda [:sotaro]]

(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.

Comment 24

[Sotaro Ikeda [:sotaro]]

(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.

Comment 25

[Sotaro Ikeda [:sotaro]]

Attachment: Pull request to device-leo

Create a symlink to msm7627a's system.prop during build.

Comment 26

[Sotaro Ikeda [:sotaro]]

Attachment: Pull request to android-device-hamachi

Comment 27

[Michael Wu [: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?

Comment 28

[Sotaro Ikeda [:sotaro]]

Thanks for the comment. It is better!

Comment 29

[Sotaro Ikeda [:sotaro]]

Attachment: Pull request to android-device-hamachi

Comment 30

[Sotaro Ikeda [:sotaro]]

Attachment: Pull request to device-leo

Comment 31

[Michael Wu [:mwu]]

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.

Comment 32

[Michael Wu [:mwu]]

Both PRs have been merged. We'll probably need an uplift.

Comment 33

[Sotaro Ikeda [:sotaro]]

up lift to v1-train
https://github.com/mozilla-b2g/device-leo/commit/1e7c35d0ec0daaeb284b505943afa00fbbcdbd2a

https://github.com/mozilla-b2g/android-device-hamachi/commit/38a8de608ee0a6672d70e8861ec50370bdf301a7

Comment 34

[Sotaro Ikeda [:sotaro]]

jhford, can you help to update the leo.xml and hamachi.xml in v1-train b2g-manifest to include the change in Comment 33?

Comment 35

[John Ford [:jhford] CET/CEST Berlin Time]

(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.

Comment 36

[Sotaro Ikeda [:sotaro]]

Oh, sorry I did not checked it.

Comment 38

[sarsenyev]

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

Comment 39

[Ben Kelly [:bkelly, not reviewing]]

Do we need to get new vendor firmware for our buri devices if we're using the latest gecko now?

Comment 40

[Sotaro Ikeda [:sotaro]]

(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.

Comment 41

[Ben Kelly [:bkelly, not reviewing]]

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.

Comment 42

[Ben Kelly [:bkelly, not reviewing]]

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.

Comment 43

[Sotaro Ikeda [:sotaro]]

(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.

Comment 44

[Sotaro Ikeda [:sotaro]]

(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.