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)
Tracking
()
Tracking | Status | |
---|---|---|
b2g-master | --- | verified |
People
(Reporter: drs, Assigned: sotaro)
References
Details
Attachments
(3 files, 2 obsolete files)
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.
Reporter | ||
Comment 1•9 years ago
|
||
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)
Comment 2•9 years ago
|
||
Pretty sure this was working well until some weeks ago ...
Assignee | ||
Comment 3•9 years 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)
Assignee | ||
Comment 4•9 years ago
|
||
I reproduced the problem on my aries.
Assignee | ||
Updated•9 years ago
|
Component: GonkIntegration → Graphics
Product: Firefox OS → Core
Assignee | ||
Comment 5•9 years ago
|
||
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
Assignee | ||
Comment 6•9 years ago
|
||
(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.
Assignee | ||
Comment 7•9 years ago
|
||
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.
Assignee | ||
Comment 8•9 years ago
|
||
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.
Assignee | ||
Comment 9•9 years ago
|
||
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.
Assignee | ||
Comment 10•9 years ago
|
||
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.
Assignee | ||
Comment 11•9 years ago
|
||
This simply remove the error returning code. There seems a risk of tearing rendering. But the problem seems to be fixed on aries.
Assignee | ||
Comment 12•9 years ago
|
||
The patch seems to fix the problem on aries.
Assignee | ||
Comment 13•9 years ago
|
||
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].
Assignee | ||
Comment 14•9 years ago
|
||
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)
Assignee | ||
Comment 15•9 years ago
|
||
(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?
Assignee | ||
Comment 17•9 years ago
|
||
(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)
Comment 18•9 years ago
|
||
I don't believe there's official CAF support for FxOS on msm8974.
Flags: needinfo?(dwilson)
Assignee | ||
Comment 19•9 years ago
|
||
(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)
Assignee | ||
Updated•9 years ago
|
Attachment #8624968 -
Attachment is obsolete: true
Comment 20•9 years ago
|
||
(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)
Assignee | ||
Comment 21•9 years ago
|
||
Thanks.
blocking-b2g: spark+ → 2.5+
Assignee | ||
Comment 23•9 years ago
|
||
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.
Assignee | ||
Comment 24•9 years ago
|
||
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
Assignee | ||
Comment 25•9 years ago
|
||
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
Assignee | ||
Comment 26•9 years ago
|
||
(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.
Assignee | ||
Comment 27•9 years ago
|
||
Assignee | ||
Comment 28•9 years ago
|
||
attachment 8624976 [details] [diff] [review] also do same thing to attachment 8634810 [details] [diff] [review]. But attachment 8634810 [details] [diff] [review] is more consistent to Overlay::commit().
Assignee | ||
Updated•9 years ago
|
Attachment #8624976 -
Attachment is obsolete: true
Assignee | ||
Comment 29•9 years ago
|
||
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)
Assignee | ||
Comment 30•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8634857 -
Flags: review?(mwu)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(sushilchauhan)
Comment 31•9 years ago
|
||
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+
Assignee | ||
Comment 32•9 years ago
|
||
(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.
Assignee | ||
Comment 33•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
status-b2g-master:
--- → fixed
Target Milestone: --- → FxOS-S3 (24Jul)
Comment 34•9 years ago
|
||
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)
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
Assignee | ||
Updated•9 years ago
|
Attachment #8634810 -
Flags: feedback?(sushilchauhan)
You need to log in
before you can comment on or make changes to this bug.
Description
•