[Intel-gfx] [PATCH 1/5] drm/vblank: Do not update vblank counts if vblanks are already disabled.

[Chris Wilson]

Quoting Dhinakaran Pandiyan (2018-01-03 20:39:57)
> Updating the vblank counts requires register reads and these reads may not
> return meaningful values after the vblank interrupts are disabled as the
> device may go to low power state. An additional change would be to allow
> the driver to save the vblank counts before entering a low power state, but
> that's for the future.
> 
> Also, disable vblanks after reading the HW counter in the case where
> _crtc_vblank_off() is disabling vblanks.
> 
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> ---
>  drivers/gpu/drm/drm_vblank.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 32d9bcf5be7f..7eee82c06ed8 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -347,23 +347,14 @@ void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
>         spin_lock_irqsave(&dev->vblank_time_lock, irqflags);
>  
>         /*
> -        * Only disable vblank interrupts if they're enabled. This avoids
> -        * calling the ->disable_vblank() operation in atomic context with the
> -        * hardware potentially runtime suspended.
> -        */
> -       if (vblank->enabled) {
> -               __disable_vblank(dev, pipe);
> -               vblank->enabled = false;
> -       }
> -
> -       /*
> -        * Always update the count and timestamp to maintain the
> +        * Update the count and timestamp to maintain the
>          * appearance that the counter has been ticking all along until
>          * this time. This makes the count account for the entire time
>          * between drm_crtc_vblank_on() and drm_crtc_vblank_off().
>          */
>         drm_update_vblank_count(dev, pipe, false);
> -
> +       __disable_vblank(dev, pipe);
> +       vblank->enabled = false;
>         spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
>  }
>  
> @@ -1122,8 +1113,12 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
>                       pipe, vblank->enabled, vblank->inmodeset);
>  
>         /* Avoid redundant vblank disables without previous
> -        * drm_crtc_vblank_on(). */
> -       if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
> +        * drm_crtc_vblank_on() and only disable them if they're enabled. This
> +        * avoids calling the ->disable_vblank() operation in atomic context
> +        * with the hardware potentially runtime suspended.
> +        */
> +       if ((drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset) &&
> +           vblank->enabled)

Outside of the spinlock protecting vblank->enabled.
-Chris