Re: [PATCH 02/25] drm/i915: Add vblank notify mechanism

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jun 18, 2014 at 08:58:35PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add a vblank notify mechanism where you can ask for a callback when a
> specific frame counter value has been passed.
> 
> This could be used for various things like FBC, IPS, watermarks,
> and updating single buffered display registers from the interrupt
> handler (eg. gamma).
> 
> As gen2 doesn't have a hardware frame counter we use the software vblank
> counter drm core procvides. This is rather racy, but for something like
> FBC it doesn't matter too much. For gen2 we could just scheudle the FBC
> enable happen a frame later than on other gens. That should paper over
> the races sufficiently.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Chris submitted a very similar patch for vblank work items, same review
still applies: This should be moved into drm_irq.c
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c      |   8 +++
>  drivers/gpu/drm/i915/intel_display.c | 132 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  16 +++++
>  3 files changed, 156 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 218f011..a908a55 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1736,7 +1736,15 @@ static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
>  	if (!drm_handle_vblank(dev, pipe))
>  		return false;
>  
> +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +		return true;
> +
>  	crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> +
> +	spin_lock(&crtc->lock);
> +	intel_vblank_notify_check(crtc);
> +	spin_unlock(&crtc->lock);
> +
>  	wake_up(&crtc->vbl_wait);
>  
>  	return true;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5e8e711..be3ee69 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11499,6 +11499,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	init_waitqueue_head(&intel_crtc->vbl_wait);
>  
> +	spin_lock_init(&intel_crtc->lock);
> +	INIT_LIST_HEAD(&intel_crtc->vblank_notify_list);
> +
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
> @@ -13051,3 +13054,132 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
>  		err_printf(m, "  VSYNC: %08x\n", error->transcoder[i].vsync);
>  	}
>  }
> +
> +/* is a after b? */
> +static bool vbl_count_after_eq(struct drm_device *dev, u32 a, u32 b)
> +{
> +	u32 mask = dev->max_vblank_count;
> +
> +	/* now hardware counter on gen2 */
> +	if (mask == 0)
> +		mask = -1;
> +
> +	mask &= (mask >> 1);
> +
> +	return !((a - b) & mask);
> +}
> +
> +static void intel_vblank_notify_complete(struct intel_vblank_notify *notify)
> +{
> +	struct intel_crtc *crtc = notify->crtc;
> +	struct drm_device *dev = crtc->base.dev;
> +
> +	assert_spin_locked(&crtc->lock);
> +
> +	drm_vblank_put(dev, crtc->pipe);
> +	list_del(&notify->list);
> +	notify->crtc = NULL;
> +}
> +
> +void intel_vblank_notify_check(struct intel_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_vblank_crtc *vblank =
> +		&dev->vblank[drm_crtc_index(&crtc->base)];
> +	struct intel_vblank_notify *notify, *next;
> +	u32 vbl_count;
> +
> +	assert_spin_locked(&crtc->lock);
> +
> +	if (list_empty(&crtc->vblank_notify_list))
> +		return;
> +
> +	/* no hardware frame counter on gen2 */
> +	if (dev->max_vblank_count == 0)
> +		vbl_count = atomic_read(&vblank->count);
> +	else
> +		vbl_count = dev->driver->get_vblank_counter(dev, crtc->pipe);
> +
> +	list_for_each_entry_safe(notify, next, &crtc->vblank_notify_list, list) {
> +		if (!vbl_count_after_eq(dev, vbl_count, notify->vbl_count))
> +			continue;
> +
> +		intel_vblank_notify_complete(notify);
> +		notify->notify(notify);
> +	}
> +}
> +
> +int intel_vblank_notify_add(struct intel_crtc *crtc,
> +			    struct intel_vblank_notify *notify)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	unsigned long irqflags;
> +	u32 vbl_count;
> +	int ret;
> +
> +	if (WARN_ON(notify->crtc))
> +		return -EINVAL;
> +
> +	ret = drm_vblank_get(dev, crtc->pipe);
> +	if (ret)
> +		return ret;
> +
> +	spin_lock_irqsave(&crtc->lock, irqflags);
> +
> +	notify->crtc = crtc;
> +	list_add(&notify->list, &crtc->vblank_notify_list);
> +
> +	/* no hardware frame counter on gen2 */
> +	if (dev->max_vblank_count == 0) {
> +		struct drm_vblank_crtc *vblank =
> +			&dev->vblank[drm_crtc_index(&crtc->base)];
> +
> +		vbl_count = atomic_read(&vblank->count);
> +	} else {
> +		vbl_count = dev->driver->get_vblank_counter(dev, crtc->pipe);
> +	}
> +
> +	if (vbl_count_after_eq(dev, vbl_count, notify->vbl_count)) {
> +		intel_vblank_notify_complete(notify);
> +		notify->notify(notify);
> +	}
> +
> +	spin_unlock_irqrestore(&crtc->lock, irqflags);
> +
> +	return 0;
> +}
> +
> +bool intel_vblank_notify_pending(struct intel_vblank_notify *notify)
> +{
> +	return notify->crtc != NULL;
> +}
> +
> +void intel_vblank_notify_cancel(struct intel_vblank_notify *notify)
> +{
> +	struct intel_crtc *crtc = ACCESS_ONCE(notify->crtc);
> +	unsigned long irqflags;
> +
> +	if (!crtc)
> +		return;
> +
> +	spin_lock_irqsave(&crtc->lock, irqflags);
> +	if (notify->crtc)
> +		intel_vblank_notify_complete(notify);
> +	spin_unlock_irqrestore(&crtc->lock, irqflags);
> +}
> +
> +u32 intel_crtc_vbl_count_rel_to_abs(struct intel_crtc *crtc, u32 rel)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +
> +	/* now hardware counter on gen2 */
> +	if (dev->max_vblank_count == 0) {
> +		struct drm_vblank_crtc *vblank =
> +			&dev->vblank[drm_crtc_index(&crtc->base)];
> +
> +		return atomic_read(&vblank->count) + rel;
> +	}
> +
> +	return (dev->driver->get_vblank_counter(dev, crtc->pipe) + rel) &
> +		dev->max_vblank_count;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ab5962b..c93626b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -358,6 +358,13 @@ struct intel_pipe_wm {
>  	bool sprites_scaled;
>  };
>  
> +struct intel_vblank_notify {
> +	void (*notify)(struct intel_vblank_notify *notify);
> +	struct intel_crtc *crtc;
> +	struct list_head list;
> +	u32 vbl_count;
> +};
> +
>  struct intel_mmio_flip {
>  	u32 seqno;
>  	u32 ring_id;
> @@ -417,6 +424,9 @@ struct intel_crtc {
>  
>  	int scanline_offset;
>  	struct intel_mmio_flip mmio_flip;
> +
> +	struct list_head vblank_notify_list;
> +	spinlock_t lock;
>  };
>  
>  struct intel_plane_wm_parameters {
> @@ -811,6 +821,12 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  				 struct intel_crtc_config *pipe_config);
>  int intel_format_to_fourcc(int format);
>  void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
> +int intel_vblank_notify_add(struct intel_crtc *crtc,
> +			    struct intel_vblank_notify *notify);
> +void intel_vblank_notify_cancel(struct intel_vblank_notify *notify);
> +bool intel_vblank_notify_pending(struct intel_vblank_notify *notify);
> +void intel_vblank_notify_check(struct intel_crtc *crtc);
> +u32 intel_crtc_vbl_count_rel_to_abs(struct intel_crtc *crtc, u32 rel);
>  
>  
>  /* intel_dp.c */
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch