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 <ch...@chris-wilson.co.uk>
> >CC: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> >Signed-off-by: Imre Deak <imre.d...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to