mihusolutions / googlebugs

Automatically exported from code.google.com/p/googlebugs
0 stars 0 forks source link

Cache line artifact for "Recent App" on Android with cache enabled for ION #694

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Open any application
2. push the third button in the navigation bar - a square button 
3. the recent apps list will show and the garbage line will be observed in the 
screen

What is the expected output? What do you see instead?
the recent apps list should looks well, but the garbage line will be shown in 
the scaled screen capture in the list.

What version of the product are you using? On what operating system?
the issue exists on Android 4.4 and later

[The Detailed explanation of the issue]
(1)The different screen capture behavior in surfaceflinger between the version 
after 4.4 and the earlier one.
(a). Screen capture on MR2 (captureScreenImplCpuConsumerLocked):
generate render buffer
create fbo with render buffer (glFramebufferRenderbufferOES)
renderScreenImplLocked (composites layers)
dequeue_buffer_and_wait
lock buffer
glReadPixels into dequeued buffer
unlock buffer
queueBuffer (with -1 fence)

(b). Screen capture on KLP (captureScreenImplLocked):
dequeue_buffer_and_wait
eglCreateImageKHR from the dequeued buffer
bind the EGLImage as a framebuffer object (RenderEngine::BindImageAsFramebuffer)
glEGLImageTargetTexture2DOES
glFrameBufferTexture2D
renderScreenImplLocked (composites layers)
compositionComplete
queueBuffer (with -1 fence)

A major change in behavior, where MR2 used CPU to readpixels from an fbo, while 
KLP use GPU to render directly into an EGLImage bound as FBO.

(2) after the screen capture operation, the system_server process will continue 
use CPU to do some operation based on the result from surfaceflinger

(3) if the user/gralloc allocate the buffer with system heap, A normal physical 
page from kernel always has the possibility in the cache line already. then the 
dirty cache exists if the cache invalid operation is not done.

(4) based on the three points, if Android uses CPU to do the operation just 
after the GPU rendering without cache invalid, the dirty cache may be still 
exists, which will corrupt the screen.

[the expected behavior]
1) allocate ion buffer-> dma_bidirectional
2) gralloc_lock -> to_cpu to invalidate CPU cache
3) gralloc_unlock-> to_dev to clean CPU cache

Android miss the step 2) in current framework

[solution]
we could add the flag through ion_sync_fd to tell to_cpu or to_dev, then we can 
use ion_snyc_fd in gralloc_lock to invalidate CPU cache.

[notes for the performance impact with the solution]
1) only for SW_usage, gralloc will do allocation cachable buffer, other cases 
will still use WB
2) invalidate cpu cache doens't introduce cache flush, will not take much time
3) actually, there are little operation in android framework based on CPU 
anymore, HWUI almost transit all the operation from CPU to GPU
so in conclusion the impact will be very little

Original issue reported on code.google.com by xu.mar...@gmail.com on 20 May 2015 at 3:40

GoogleCodeExporter commented 9 years ago
To supplement described above,
[the expected behavior]
If the process wants using CPU to access the buffer, it should call 
gralloc_lock/gralloc_unlock during the buffer access.
gralloc_lock: should call "to_cpu cache sync" to invalidate CPU cache
gralloc_unlock: should call "to_dev cache sync" to clean CPU cache

[actual behavior]
there is no interface in ion to do "to_cpu" and "to_dev" seperately, then we 
should find a way to resolve it.

[solution]
1. gralloc : add the "to_cpu" cache sync in gralloc_lock, and the "to_dev" 
cache sync in gralloc_unlock.

2. libion : add the flag in the interface ion_sync_fd to tell "to_cpu" or 
"to_dev", which make gralloc call according to its situation.

3. kernel : Modify the implementation of the ioctl ion_sync_fd, and do the 
different cache sync operation according to the flags.

Original comment by xu.mar...@gmail.com on 25 May 2015 at 10:52