[Intel-gfx] [PATCH] drm/i915: Queue page flip work with high priority

[Tvrtko Ursulin]

On 12/09/16 15:09, Imre Deak wrote:
> While user space has control over the scheduling priority of its page
> flipping thread, the corresponding work the driver schedules for MMIO
> flips always runs with normal scheduling priority. This would hinder an
> application that wants more stringent guarantees over flip timing (to
> avoid missing a flip at the next frame count).
>
> Fix this by scheduling the work with high priority, meaning normal
> scheduling policy with -20 nice level.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97775
> Testcase: igt/kms_cursor_legacy
> CC: Chris Wilson <chris at chris-wilson.co.uk>
> CC: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.c      | 7 +++++++
>   drivers/gpu/drm/i915/i915_drv.h      | 4 ++++
>   drivers/gpu/drm/i915/intel_display.c | 2 +-
>   3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 02c34d6..381ef23 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -756,8 +756,14 @@ static int i915_workqueues_init(struct drm_i915_private *dev_priv)
>   	if (dev_priv->hotplug.dp_wq == NULL)
>   		goto out_free_wq;
>
> +	dev_priv->flip_wq = alloc_workqueue("i915-flip", WQ_HIGHPRI, 0);
> +	if (dev_priv->flip_wq == NULL)
> +		goto out_free_dp_wq;
> +
>   	return 0;
>
> +out_free_dp_wq:
> +	destroy_workqueue(dev_priv->hotplug.dp_wq);
>   out_free_wq:
>   	destroy_workqueue(dev_priv->wq);
>   out_err:
> @@ -768,6 +774,7 @@ out_err:
>
>   static void i915_workqueues_cleanup(struct drm_i915_private *dev_priv)
>   {
> +	destroy_workqueue(dev_priv->flip_wq);
>   	destroy_workqueue(dev_priv->hotplug.dp_wq);
>   	destroy_workqueue(dev_priv->wq);
>   }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f499fa5..3653ce4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1844,6 +1844,10 @@ struct drm_i915_private {
>   	 * result in deadlocks.
>   	 */
>   	struct workqueue_struct *wq;
> +	/**
> +	 * flip_wq - High priority flip workqueue.
> +	 */
> +	struct workqueue_struct *flip_wq;
>
>   	/* Display functions */
>   	struct drm_i915_display_funcs display;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3c367d0..48433e1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12278,7 +12278,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>
>   		work->flip_queued_req = i915_gem_active_get(&obj->last_write,
>   							    &obj->base.dev->struct_mutex);
> -		schedule_work(&work->mmio_work);
> +		queue_work(dev_priv->flip_wq, &work->mmio_work);
>   	} else {
>   		request = i915_gem_request_alloc(engine, engine->last_context);
>   		if (IS_ERR(request)) {
>

I am curious if just a dedicated wq would be enough, or you have found 
that it has to be a high-prio one?

Otherwise patch looks fine to me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko