On 08/17/2016 09:11 AM, Sean Paul wrote:
> A few things that need tidying up, no functional changes.
>
> Signed-off-by: Sean Paul <seanpaul at chromium.org>
Reviewed-by: Yakir Yang <ykk at rock-chips.com>
> ---
>
> Changes in v2:
> - Introduced
>
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index 5bd54f2..c6ac5d0 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -62,27 +62,25 @@ static void psr_set_state_locked(struct psr_drv *psr,
> enum psr_state state)
> * Allowed finite state machine:
> *
> * PSR_ENABLE < = = = = = > PSR_FLUSH
> - * | ^ |
> - * | | |
> - * v | |
> + * | ^ |
> + * | | |
> + * v | |
> * PSR_DISABLE < - - - - - - - - -
> */
> -
> - /* Forbid no state change */
> if (state == psr->state)
> return;
>
> - /* Forbid DISABLE change to FLUSH */
> + /* Requesting a flush when disabled is a noop */
> if (state == PSR_FLUSH && psr->state == PSR_DISABLE)
> return;
>
> psr->state = state;
>
> - /* Allow but no need hardware change, just need assign the state */
> + /* Already disabled in flush, change the state, but not the hardware */
> if (state == PSR_DISABLE && psr->state == PSR_FLUSH)
> return;
>
> - /* Refact to hardware state change */
> + /* Actually commit the state change to hardware */
> switch (psr->state) {
> case PSR_ENABLE:
> psr->set(psr->encoder, true);
> @@ -109,10 +107,7 @@ static void psr_flush_handler(unsigned long data)
> struct psr_drv *psr = (struct psr_drv *)data;
> unsigned long flags;
>
> - if (!psr)
> - return;
> -
> - /* State changed between flush time, then keep it */
> + /* If the state has changed since we initiated the flush, do nothing */
> spin_lock_irqsave(&psr->lock, flags);
> if (psr->state == PSR_FLUSH)
> psr_set_state_locked(psr, PSR_ENABLE);
