Hi Daniel,

On 06/02/2016 10:19 PM, Daniel Vetter wrote:
> On Wed, Jun 01, 2016 at 10:54:09AM +0800, Yakir Yang wrote:
>> Hi Daniel,
>>
>> On 05/31/2016 10:38 PM, Daniel Vetter wrote:
>>> On Tue, May 31, 2016 at 09:37:36PM +0800, Yakir Yang wrote:
>>>> 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]: https://patchwork.kernel.org/patch/8886041/
>>>> [1]: https://patchwork.kernel.org/patch/9132713/
>>> 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
>> 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.
> 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.

Sounds good, use timer to driver the PSR status, and abandon the vblank
enable/disable event. Okay, I start to do some experiments.

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

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.



> 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

Sorry, I'm not very clear about what you mean, seems you have a reference
code about PSR test ;)

Thanks,
- Yakir