<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body text="#000000" bgcolor="#FFFFFF"> Hi Daniel, <div class="moz-cite-prefix">On 06/02/2016 10:19 PM, Daniel Vetter wrote: </div> <blockquote cite="mid:20160602141910.GL7231@phenom.ffwll.local" type="cite"> <pre wrap="">On Wed, Jun 01, 2016 at 10:54:09AM +0800, Yakir Yang wrote: </pre> <blockquote type="cite"> <pre wrap="">Hi Daniel, On 05/31/2016 10:38 PM, Daniel Vetter wrote: </pre> <blockquote type="cite"> <pre wrap="">On Tue, May 31, 2016 at 09:37:36PM +0800, Yakir Yang wrote: </pre> <blockquote type="cite"> <pre wrap="">The full name of PSR is Panel Self Refresh, panel device could refresh itself with the hardware framebuffer in panel, this would make a lots of sense to save the power consumption. For example, when desktop haven't change the context for a long time, then we could refresh the data to the hardware framebuffer of panel, and then let panel enter into PSR mode. After that system could poweroff the LCDC controller and eDP controller, just let panel refresh the screen by itself. It's hard to decide when panel should enter into PSR or exit from PSR, in this time I chose to let the drm_vblank enable/disable event driver the PSR. This thread is based on Mark's RK3399 VOP thread[0] and my RK3399 eDP thread[1]. [0]: <a class="moz-txt-link-freetext" href="https://patchwork.kernel.org/patch/8886041/">https://patchwork.kernel.org/patch/8886041/</a> [1]: <a class="moz-txt-link-freetext" href="https://patchwork.kernel.org/patch/9132713/">https://patchwork.kernel.org/patch/9132713/</a> </pre> </blockquote> <pre wrap="">Looks like you didn't wire up the drm_framebuffer->funcs->dirty callback for manual upload of simple clients like bootsplash or fbdev. I think that's needed. At least it's needed for every other manual upload dsi and edp psr implementation. -Daniel </pre> </blockquote> <pre wrap="">That's great, thanks for your remind. Seems like userspace which does frontbuffer rendering must call this ioctl to flush out the changes on manual-update display outputs. It's helpful to hook this callback to notify eDP refresh the eDP RFB(remote frame buffer). But I think this is hard to used on Rockchip eDP controller, Rockchip eDP driver just used two modes, Sink Device PSR_S0 (PSR inactive), and Sink Device PSR_S2 (PSR active, display from RFB). I think the "dirty" callback is only used when Sink device enter into PSR_S3 mode (PSR active, capture and display), need to update the remote frame buffer. But on Rockchip platform the panel would be very easy to lose frame in this PSR mode. I'm confused in this case, so I didn't enable that. If we didn't enable the PSR_S3 mode, then we don't need to update the panel remote frame buffer, so this "->dirty" callback would be unused in this case. </pre> </blockquote> <pre wrap=""> Sorry missed your reply here somehow. Every time you don't scan out the drm_framebuffer directly, but from any kind of intermediate buffer, you _must_ implement the dirty callback. Usually the simplest way is to kick the device out of self-refresh and arm the time to re-enter self-refresh. </pre> </blockquote> Sounds good, use timer to driver the PSR status, and abandon the vblank enable/disable event. Okay, I start to do some experiments. <blockquote cite="mid:20160602141910.GL7231@phenom.ffwll.local" type="cite"> <pre wrap="">I'm not entirely clear where your RFB is, so no idea whether that is scanning out from the drm_framebuffer data directly, or whether that's some on-chip or in-sink framebuffer cache. </pre> </blockquote> RFB is an in-sink framebuffer cache. When Sink enter into PSR_State1 or PSR_State3, then it can capture the eDP timing from source to update the RFB. <img src="cid:part1.01040605.06000003@rock-chips.com" alt=""> <blockquote cite="mid:20160602141910.GL7231@phenom.ffwll.local" type="cite"> <pre wrap="">We have a ridiculously nasty testuite in i-g-t for all these cases (we use it to validate our framebuffer compression code, psr code and soon also dsi manual upload). Unfortunately it's not yet ported over to be a generic testcase. But at least for PSR that would be easy, since every PSR panel has eDP sink crc support. And the testcase does require some kind of display CRC support to validate actual screen contents, as opposed to what's in memory in the drm_framebuffer. -Daniel </pre> </blockquote> Sorry, I'm not very clear about what you mean, seems you have a reference code about PSR test ;) Thanks, - Yakir </body> </html>