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@xxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
