[Intel-gfx] [PATCH] drm/i915: Introduce vblank work function

[Bloomfield, Jon]

What's the status of this patch ? I can't find any subsequent mention of it, but we currently use it in one of our Android development trees. I'm trying to work out whether to retain it or replace it.

Was it killed off, or is it still in the pipeline ?

Thanks.

> -----Original Message-----
> From: intel-gfx-bounces+jon.bloomfield=intel.com at lists.freedesktop.org
> [mailto:intel-gfx-bounces+jon.bloomfield=intel.com at lists.freedesktop.org]
> On Behalf Of Chris Wilson
> Sent: Friday, July 05, 2013 9:49 AM
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function
> 
> Throughout the driver, we have a number of pieces of code that must wait
> for a vblank before we can update some state. Often these could be run
> asynchronously since they are merely freeing a resource no long referenced
> by a double-buffered registered. So we introduce a vblank worker upon
> which we queue various tasks to be run after the next vvblank.
> 
> This will be used in the next patches to avoid unnecessary stalls when
> updating registers and for freeing resources.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 79
> ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  8 +++-
>  2 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b70ce4e..dff3b93 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -795,6 +795,74 @@ void intel_wait_for_vblank(struct drm_device *dev,
> int pipe)
>  		DRM_DEBUG_KMS("vblank wait timed out\n");  }
> 
> +struct intel_crtc_vblank_task {
> +	struct list_head list;
> +	void (*func)(struct intel_crtc *, void *data);
> +	void *data;
> +};
> +
> +static void intel_crtc_vblank_work_fn(struct work_struct *_work) {
> +	struct intel_crtc_vblank_work *work =
> +		container_of(_work, struct intel_crtc_vblank_work, work);
> +	struct intel_crtc *crtc =
> +		container_of(work, struct intel_crtc, vblank_work);
> +	struct list_head tasks;
> +
> +	intel_wait_for_vblank(crtc->base.dev, crtc->pipe);
> +
> +	mutex_lock(&crtc->vblank_work.mutex);
> +	list_replace_init(&work->tasks, &tasks);
> +	mutex_unlock(&crtc->vblank_work.mutex);
> +
> +	while (!list_empty(&tasks)) {
> +		struct intel_crtc_vblank_task *task
> +			= list_first_entry(&tasks,
> +					   struct intel_crtc_vblank_task,
> +					   list);
> +
> +		task->func(crtc, task->data);
> +		list_del(&task->list);
> +		kfree(task);
> +	}
> +}
> +
> +static int intel_crtc_add_vblank_task(struct intel_crtc *crtc,
> +				      bool single,
> +				      void (*func)(struct intel_crtc *,
> +						   void *data),
> +				      void *data)
> +{
> +	struct intel_crtc_vblank_task *task;
> +	struct intel_crtc_vblank_work *work = &crtc->vblank_work;
> +
> +	task = kzalloc(sizeof *task, GFP_KERNEL);
> +	if (task == NULL)
> +		return -ENOMEM;
> +	task->func = func;
> +	task->data = data;
> +
> +	mutex_lock(&work->mutex);
> +	if (list_empty(&work->tasks)) {
> +		schedule_work(&work->work);
> +	} else if (single) {
> +		struct intel_crtc_vblank_task *old;
> +		list_for_each_entry(old, &work->tasks, list) {
> +			if (task->func == func && task->data == data) {
> +				func = NULL;
> +				break;
> +			}
> +		}
> +	}
> +	if (func)
> +		list_add(&task->list, &work->tasks);
> +	else
> +		kfree(task);
> +	mutex_unlock(&work->mutex);
> +
> +	return 0;
> +}
> +
>  /*
>   * intel_wait_for_pipe_off - wait for pipe to turn off
>   * @dev: drm device
> @@ -2835,6 +2903,8 @@ static void intel_crtc_wait_for_pending_flips(struct
> drm_crtc *crtc)
>  	if (crtc->fb == NULL)
>  		return;
> 
> +	flush_work(&to_intel_crtc(crtc)->vblank_work.work);
> +
>  	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
> 
>  	wait_event(dev_priv->pending_flip_queue,
> @@ -9097,6 +9167,10 @@ static void intel_crtc_init(struct drm_device *dev,
> int pipe)
>  	if (intel_crtc == NULL)
>  		return;
> 
> +	mutex_init(&intel_crtc->vblank_work.mutex);
> +	INIT_LIST_HEAD(&intel_crtc->vblank_work.tasks);
> +	INIT_WORK(&intel_crtc->vblank_work.work,
> intel_crtc_vblank_work_fn);
> +
>  	drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
> 
>  	if (drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256)) @@ -
> 10296,11 +10370,16 @@ void intel_modeset_cleanup(struct drm_device
> *dev)
>  	 */
>  	drm_kms_helper_poll_fini(dev);
> 
> +	/* Clear the vblank worker prior to taking any locks */
> +	flush_scheduled_work();
> +
>  	mutex_lock(&dev->struct_mutex);
> 
>  	intel_unregister_dsm_handler();
> 
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		flush_work(&to_intel_crtc(crtc)->vblank_work.work);
> +
>  		/* Skip inactive CRTCs */
>  		if (!crtc->fb)
>  			continue;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 619caf9..19df4ec 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -314,10 +314,16 @@ struct intel_crtc {
>  	bool primary_disabled; /* is the crtc obscured by a plane? */
>  	bool lowfreq_avail;
>  	struct intel_overlay *overlay;
> -	struct intel_unpin_work *unpin_work;
> 
> +	struct intel_unpin_work *unpin_work;
>  	atomic_t unpin_work_count;
> 
> +	struct intel_crtc_vblank_work {
> +		struct work_struct work;
> +		struct mutex mutex;
> +		struct list_head tasks;
> +	} vblank_work;
> +
>  	/* Display surface base address adjustement for pageflips. Note that
> on
>  	 * gen4+ this only adjusts up to a tile, offsets within a tile are
>  	 * handled in the hw itself (with the TILEOFF register). */
> --
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx