On Wed, Jan 15, 2020 at 01:16:33PM +0100, Thomas Zimmermann wrote:
> VBLANK interrupts can be disabled immediately or with a delay, where the
> latter is the default. The former option can be selected by setting
> get_vblank_timestamp, and enabling vblank_disable_immediate in struct
> drm_device.
> 
> The setup is only evaluated once when DRM initializes VBLANKs. Evaluating
> the settings on each use of vblank_disable_immediate will allow for easy
> integration of CRTC VBLANK functions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
> ---
>  drivers/gpu/drm/drm_vblank.c | 31 ++++++++++++++-----------------
>  1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 3f1dd54cc8bb..abb085c67d82 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -481,19 +481,6 @@ int drm_vblank_init(struct drm_device *dev, unsigned int 
> num_crtcs)
>  
>       DRM_INFO("Supports vblank timestamp caching Rev 2 (21.10.2013).\n");
>  
> -     /* Driver specific high-precision vblank timestamping supported? */
> -     if (dev->driver->get_vblank_timestamp)
> -             DRM_INFO("Driver supports precise vblank timestamp query.\n");
> -     else
> -             DRM_INFO("No driver support for vblank timestamp query.\n");
> -
> -     /* Must have precise timestamping for reliable vblank instant disable */
> -     if (dev->vblank_disable_immediate && 
> !dev->driver->get_vblank_timestamp) {
> -             dev->vblank_disable_immediate = false;
> -             DRM_INFO("Setting vblank_disable_immediate to false because "
> -                      "get_vblank_timestamp == NULL\n");
> -     }

Which drivers are so broken they set vblank_disable_immediate to true
without having the vfunc specified? IMO this code should just go away
(or converted to a WARN).

> -
>       return 0;
>  
>  err:
> @@ -1070,6 +1057,15 @@ int drm_crtc_vblank_get(struct drm_crtc *crtc)
>  }
>  EXPORT_SYMBOL(drm_crtc_vblank_get);
>  
> +static bool __vblank_disable_immediate(struct drm_device *dev, unsigned int 
> pipe)
> +{
> +     if (!dev->vblank_disable_immediate)
> +             return false;
> +     if (!dev->driver->get_vblank_timestamp)
> +             return false;
> +     return true;
> +}
> +
>  static void drm_vblank_put(struct drm_device *dev, unsigned int pipe)
>  {
>       struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> @@ -1086,7 +1082,7 @@ static void drm_vblank_put(struct drm_device *dev, 
> unsigned int pipe)
>                       return;
>               else if (drm_vblank_offdelay < 0)
>                       vblank_disable_fn(&vblank->disable_timer);
> -             else if (!dev->vblank_disable_immediate)
> +             else if (__vblank_disable_immediate(dev, pipe))
>                       mod_timer(&vblank->disable_timer,
>                                 jiffies + ((drm_vblank_offdelay * HZ)/1000));
>       }
> @@ -1663,7 +1659,7 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void 
> *data,
>       /* If the counter is currently enabled and accurate, short-circuit
>        * queries to return the cached timestamp of the last vblank.
>        */
> -     if (dev->vblank_disable_immediate &&
> +     if (__vblank_disable_immediate(dev, pipe) &&
>           drm_wait_vblank_is_query(vblwait) &&
>           READ_ONCE(vblank->enabled)) {
>               drm_wait_vblank_reply(dev, pipe, &vblwait->reply);
> @@ -1820,7 +1816,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned 
> int pipe)
>        * been signaled. The disable has to be last (after
>        * drm_handle_vblank_events) so that the timestamp is always accurate.
>        */
> -     disable_irq = (dev->vblank_disable_immediate &&
> +     disable_irq = (__vblank_disable_immediate(dev, pipe) &&
>                      drm_vblank_offdelay > 0 &&
>                      !atomic_read(&vblank->refcount));
>  
> @@ -1893,7 +1889,8 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, 
> void *data,
>       pipe = drm_crtc_index(crtc);
>  
>       vblank = &dev->vblank[pipe];
> -     vblank_enabled = dev->vblank_disable_immediate && 
> READ_ONCE(vblank->enabled);
> +     vblank_enabled = __vblank_disable_immediate(dev, pipe) &&
> +                      READ_ONCE(vblank->enabled);
>  
>       if (!vblank_enabled) {
>               ret = drm_crtc_vblank_get(crtc);
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to