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

[Chris Wilson]

On Tue, Sep 13, 2016 at 11:24:52AM +0100, Tvrtko Ursulin wrote:
> 
> 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.

The problem is that this just the tip of the iceberg. mmioflips will be
driven by atomic in the near future. This isn't a viable solution as not
everything in that work is suitable for high priority.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre