Closed Bug 1175463 Opened 9 years ago Closed 9 years ago

[Aries] Gallery pictures of certain dimensions take down to all black

Categories

(Core :: Graphics, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

VERIFIED FIXED
FxOS-S3 (24Jul)
blocking-b2g 2.5+
Tracking Status
b2g-master --- verified

People

(Reporter: drs, Assigned: sotaro)

References

Details

Attachments

(3 files, 2 obsolete files)

Attached image map.png (deleted) —
STR: 1. Copy attached picture to internal storage or SD card. 2. Open it in Gallery app. 3. Pinch to zoom. Expected: Can pan and zoom normally. Actual: The screen flickers before going completely black. The only way to guaranteed restore it is to reboot.
Sotaro, could you investigate this? Since this looks like it may require a Gonk-level fix, any chance for a quick WIP patch for us to pull into a build snapshot?
Flags: needinfo?(sotaro.ikeda.g)
Pretty sure this was working well until some weeks ago ...
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #1) > Sotaro, could you investigate this? Since this looks like it may require a > Gonk-level fix, any chance for a quick WIP patch for us to pull into a build > snapshot? Tomorrow, I am going to investigate it.
Assignee: nobody → sotaro.ikeda.g
Flags: needinfo?(sotaro.ikeda.g)
I reproduced the problem on my aries.
Component: GonkIntegration → Graphics
Product: Firefox OS → Core
When the problem happens, mdss_mdp_smp_mmb_reserve() was failed in kernel. https://github.com/mozilla-b2g/codeaurora_kernel_msm/blob/shinano/drivers/video/msm/mdss/mdss_mdp_pipe.c#L57
(In reply to Sotaro Ikeda [:sotaro] from comment #5) > When the problem happens, mdss_mdp_smp_mmb_reserve() was failed in kernel. > > https://github.com/mozilla-b2g/codeaurora_kernel_msm/blob/shinano/drivers/ > video/msm/mdss/mdss_mdp_pipe.c#L57 When this condition happens, the problem continue until display on/off.
The problem happened in the following situations. - mdss_mdp_smp_mmb_reserve() is reserving smp blocks from mdss_mdp_pipe_smp_map. - The smp_map already allocated smp blocks. In this case, a number of requesting blocks should be same the same number to the already allocated smp blocks. - But they are different. The difference could happen when rendering gralloc buffer size is changed a lot.
A number of sms block is calculated in mdss_mdp_smp_reserve(). The number is determined by gralloc buffer's width. It is calculated like the following. > num_blks = DIV_ROUND_UP(ps.ystride[i] * nlines, SMP_MB_SIZE); https://github.com/mozilla-b2g/codeaurora_kernel_msm/blob/shinano/drivers/video/msm/mdss/mdss_mdp_pipe.c#L319 Just before the problem happened, ystride = 5120 and num_blks = 3. When the problem happned, ystride = 2880 and num_blks = 2. Because of this difference, mdss_mdp_smp_mmb_reserve() was failed.
I checked the Aries mdp driver code. But the code seems not handle correctly this problem. From the following git change log, the following change seems to handle similar problems. But it does not handle this bug's problem. https://github.com/mozilla-b2g/codeaurora_kernel_msm/commit/cc264ed8987beba7f1d5d4fec77f021314647a39 The above change does not address a problem of "allocated smb block vs requesting smb block". If already allocated smb blocks number is different, this handling pipe can not be used until the pipe is released in current mdp driver's code.
There seems 2 ways to address this problem. - [1] mdp driver permit allocated sms block number change. But this might cause a problem as written in https://github.com/mozilla-b2g/codeaurora_kernel_msm/commit/cc264ed8987beba7f1d5d4fec77f021314647a39 - [2] hwc hal recreate GenericPipe when this situation is detected.
This simply remove the error returning code. There seems a risk of tearing rendering. But the problem seems to be fixed on aries.
The patch seems to fix the problem on aries.
With either of attachment 8624968 [details] [diff] [review] or attachment 8624976 [details] [diff] [review], I saw the rendering problem with the following STR. But the problem happened also with disabling hwc. Therefore, it is a different problem. [STR] [1] Store only one image(attachment 8623585 [details] in comment 0) to sdcard. [2] Start Galley app [3] Select the image's tile. [4] Back to tile's view When doing [3] and [4] very quickly, sometimes only the image did not rendered at [3].
Sushil, diego, do you know if there is already a fix for this "allocated sms block number" problem on latest mdp driver code?
Flags: needinfo?(sushilchauhan)
Flags: needinfo?(dwilson)
(In reply to Sotaro Ikeda [:sotaro] from comment #14) > Sushil, diego, do you know if there is already a fix for this "allocated sms > block number" problem on latest mdp driver code? Or latest mdp hwc hal code?
What kind of device is Aries?
Flags: needinfo?(dwilson)
(In reply to Diego Wilson [:diego] from comment #16) > What kind of device is Aries? Aries is xperia Z3C. It uses msm8974. http://www.sonymobile.com/gb/products/phones/xperia-z3-compact/specifications/
Flags: needinfo?(dwilson)
I don't believe there's official CAF support for FxOS on msm8974.
Flags: needinfo?(dwilson)
(In reply to Diego Wilson [:diego] from comment #18) > I don't believe there's official CAF support for FxOS on msm8974. It is not official support. My question is whether the problem is fixed in latest codeaurora code. Because it is general implementation problem. Though, android do not use hwc hal like in this use case. If codeaura code does not handle this problem. moz build source is going to address it locally.
Flags: needinfo?(dwilson)
Attachment #8624968 - Attachment is obsolete: true
(In reply to Sotaro Ikeda [:sotaro] from comment #19) > (In reply to Diego Wilson [:diego] from comment #18) > > I don't believe there's official CAF support for FxOS on msm8974. > > It is not official support. > > My question is whether the problem is fixed in latest codeaurora code. > Because it is general implementation problem. Though, android do not use hwc > hal like in this use case. If codeaura code does not handle this problem. > moz build source is going to address it locally. I don't know if it's fixed. Perhaps Sushil may know.
Flags: needinfo?(dwilson)
Thanks.
blocking-b2g: spark+ → 2.5+
On the latest master aries, I could not reproduce the problem. It seems an effect of changing layout code. Recently, full hwc composition usage seems a bit decreased.
hwc's implementation is very different between KK and L. On L, the problem seems to be addressed. When mPipe->commit() failed in Overlay::commit(), it deallocate pipes for a related display. https://github.com/mozilla-b2g/hardware_qcom_display/blob/b2g-5.1.0_r1/msm8974/liboverlay/overlay.cpp#L136
L Overlay::commit()'s equivalent is split to 2 functions in KK. - Overlay::validateAndSet() - Overlay::commit() commit() implements error handling. But validateAndSet() does not do correct error handling. https://github.com/mozilla-b2g/hardware_qcom_display/blob/shinano/liboverlay/overlay.cpp#L425 https://github.com/mozilla-b2g/hardware_qcom_display/blob/shinano/liboverlay/overlay.cpp#L178
Blocks: 1173213
(In reply to Sotaro Ikeda [:sotaro] from comment #23) > On the latest master aries, I could not reproduce the problem. It seems an > effect of changing layout code. Recently, full hwc composition usage seems a > bit decreased. Correction: I could reproduce the problem. It took long time until reproduce the symptom.
Attachment #8624976 - Attachment is obsolete: true
Comment on attachment 8634810 [details] [diff] [review] patch - Add error handling to Overlay::validateAndSet() Sushil, can you give a feedback to the patch?
Attachment #8634810 - Flags: feedback?(sushilchauhan)
Attachment #8634857 - Flags: review?(mwu)
Flags: needinfo?(sushilchauhan)
Comment on attachment 8634857 [details] https://github.com/mozilla-b2g/hardware_qcom_display/pull/4 Another rubberstamp. Though I do have one question - when the gralloc buffer size changes, is it because the same gralloc buffer is resized, or because there is a different gralloc buffer with a different size?
Attachment #8634857 - Flags: review?(mwu) → review+
(In reply to Michael Wu [:mwu] from comment #31) > Comment on attachment 8634857 [details] > https://github.com/mozilla-b2g/hardware_qcom_display/pull/4 > > Another rubberstamp. > > Though I do have one question - when the gralloc buffer size changes, is it > because the same gralloc buffer is resized, or because there is a different > gralloc buffer with a different size? When it happens, a hwc layer uses a different gfx layer compared to previous hwc composition. Then size could becomes different.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
No longer blocks: 1188877
Target Milestone: --- → FxOS-S3 (24Jul)
This issue is verified fixed on the latest Aries 2.5 Master build. Viewing the attached map.png image in the Gallery app, I was able to pinch zoom in and out, as well as pan the image with no graphical flickering, black tiling, or display issues. Environmental Variables: Device: Aries 2.5 BuildID: 20150918122511 Gaia: 4f22dfecdc046fe5223ee858dd06c11b75884740 Gecko: 37c7812ce0e6d10c7e7182f12e752832835e1d67 Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd Version: 43.0a1 (2.5) Firmware Version: D5803_23.1.A.1.28_NCB.ftf User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
Attachment #8634810 - Flags: feedback?(sushilchauhan)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: