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

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

[Imre Deak]

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@chris-wilson.co.uk>
CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@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)) {
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Queue page flip work with high priority

[Patchwork]

== Series Details ==

Series: drm/i915: Queue page flip work with high priority
URL   : https://patchwork.freedesktop.org/series/12336/
State : success

== Summary ==

Series 12336v1 drm/i915: Queue page flip work with high priority
https://patchwork.freedesktop.org/api/1.0/series/12336/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-cursor-vs-flip-varying-size:
                fail       -> PASS       (fi-bsw-n3050)

fi-bdw-5557u     total:254  pass:239  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:254  pass:208  dwarn:0   dfail:0   fail:0   skip:46 
fi-byt-n2820     total:254  pass:212  dwarn:0   dfail:0   fail:1   skip:41 
fi-hsw-4770k     total:254  pass:232  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:254  pass:228  dwarn:0   dfail:0   fail:0   skip:26 
fi-ilk-650       total:254  pass:184  dwarn:0   dfail:0   fail:2   skip:68 
fi-ivb-3520m     total:254  pass:223  dwarn:0   dfail:0   fail:0   skip:31 
fi-ivb-3770      total:254  pass:223  dwarn:0   dfail:0   fail:0   skip:31 
fi-skl-6260u     total:254  pass:240  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:254  pass:227  dwarn:0   dfail:0   fail:1   skip:26 
fi-skl-6700k     total:254  pass:225  dwarn:1   dfail:0   fail:0   skip:28 
fi-snb-2520m     total:254  pass:209  dwarn:0   dfail:0   fail:0   skip:45 
fi-snb-2600      total:254  pass:209  dwarn:0   dfail:0   fail:0   skip:45 

Results at /archive/results/CI_IGT_test/Patchwork_2512/

bef9c1f4afe24cfff578d386bde349add65673eb drm-intel-nightly: 2016y-09m-12d-08h-35m-02s UTC integration manifest
1c9476a drm/i915: Queue page flip work with high priority

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@chris-wilson.co.uk>
> CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@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@intel.com>

Regards,

Tvrtko



_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Queue page flip work with high priority

[Imre Deak]

On ti, 2016-09-13 at 11:24 +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@chris-wilson.co.uk>
> > CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@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?

I haven't tried a dedicated normal-prio wq. Right, another work in the
queue could also hold up this one, but the system_wq is unordered, so
that kind of dependency shouldn't be a problem if that's what you
meant.

> Otherwise patch looks fine to me.
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Regards,
> 
> Tvrtko
> 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@chris-wilson.co.uk>
> >CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >Signed-off-by: Imre Deak <imre.deak@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

Re: [PATCH] drm/i915: Queue page flip work with high priority

[Imre Deak]

On ti, 2016-09-13 at 11:32 +0100, Chris Wilson wrote:
> 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@chris-wilson.co.uk>
> > > CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@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.

In what sense isn't it viable? The intention is to complete the work
before the next vblank, so it is by the definition a high-prio work.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Queue page flip work with high priority

[Tvrtko Ursulin]

On 13/09/16 11:31, Imre Deak wrote:
> On ti, 2016-09-13 at 11:24 +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@chris-wilson.co.uk>
>>> CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Imre Deak <imre.deak@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?
>
> I haven't tried a dedicated normal-prio wq. Right, another work in the
> queue could also hold up this one, but the system_wq is unordered, so
> that kind of dependency shouldn't be a problem if that's what you
> meant.

Yes, I've suspicious whether the problem is work start latency and not 
actually the worker priority. Since the flip work item mostly does 
waiting and little CPU activity, I though the former sounded like more 
likely.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Queue page flip work with high priority

[Imre Deak]

On ti, 2016-09-13 at 12:12 +0100, Tvrtko Ursulin wrote:
> On 13/09/16 11:31, Imre Deak wrote:
> > On ti, 2016-09-13 at 11:24 +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@chris-wilson.co.uk>
> > > > CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak@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?
> > 
> > I haven't tried a dedicated normal-prio wq. Right, another work in
> > the
> > queue could also hold up this one, but the system_wq is unordered,
> > so
> > that kind of dependency shouldn't be a problem if that's what you
> > meant.
> 
> Yes, I've suspicious whether the problem is work start latency and
> not 
> actually the worker priority. Since the flip work item mostly does 
> waiting and little CPU activity, I though the former sounded like
> more 
> likely.

Hm, testing it with a WQ_UNBOUND dedicated queue I couldn't reproduce
the problem either. The fact that the system_wq is not WQ_UNBOUND could
explain the extra latency. So I can resend this with that change.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915: Queue page flip work via a low latency, unbound workqueue

[Imre Deak]

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 from the generic system workqueue which has some
scheduling overhead due it being CPU bound. 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 from a dedicated, unbound workqueue
which provides for minimal scheduling latency.

v2:
- Use an unbound workqueue instead of a high-prio one. (Tvrtko, Chris)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97775
Testcase: igt/kms_cursor_legacy
CC: Chris Wilson <chris@chris-wilson.co.uk>
CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v1)
---
 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 7f4e8ad..b5445a1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -755,8 +755,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_UNBOUND, 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:
@@ -767,6 +773,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 d60ade9..cdd835b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1859,6 +1859,10 @@ struct drm_i915_private {
 	 * result in deadlocks.
 	 */
 	struct workqueue_struct *wq;
+	/**
+	 * flip_wq - Low latency, unordered 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 7bb7874..7728c18 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12266,7 +12266,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)) {
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: Queue page flip work with high priority (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Queue page flip work with high priority (rev2)
URL   : https://patchwork.freedesktop.org/series/12336/
State : failure

== Summary ==

Series 12336v2 drm/i915: Queue page flip work with high priority
https://patchwork.freedesktop.org/api/1.0/series/12336/revisions/2/mbox/

Test drv_module_reload_basic:
                dmesg-warn -> PASS       (fi-skl-6770hq)
Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> PASS       (fi-hsw-4770k)
Test kms_busy:
        Subgroup basic-flip-default-a:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-flip-default-b:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-flip-default-c:
                pass       -> DMESG-WARN (fi-skl-6700k)
                skip       -> PASS       (fi-skl-6770hq)
Test kms_cursor_legacy:
        Subgroup basic-flip-after-cursor-legacy:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-flip-after-cursor-varying-size:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-flip-before-cursor-legacy:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-flip-before-cursor-varying-size:
                skip       -> PASS       (fi-skl-6770hq)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-flip-vs-wf_vblank:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-plain-flip:
                skip       -> PASS       (fi-skl-6770hq)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                skip       -> FAIL       (fi-skl-6770hq)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup hang-read-crc-pipe-b:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup hang-read-crc-pipe-c:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup nonblocking-crc-pipe-a:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup nonblocking-crc-pipe-b:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup nonblocking-crc-pipe-c:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup read-crc-pipe-a:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup read-crc-pipe-a-frame-sequence:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup read-crc-pipe-b:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup read-crc-pipe-b-frame-sequence:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup read-crc-pipe-c:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup read-crc-pipe-c-frame-sequence:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup suspend-read-crc-pipe-a:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900)
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> SKIP       (fi-hsw-4770r)
                skip       -> PASS       (fi-skl-6770hq)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                skip       -> PASS       (fi-skl-6770hq)
        Subgroup basic-rte:
                skip       -> PASS       (fi-skl-6770hq)
Test prime_vgem:
        Subgroup basic-fence-flip:
                skip       -> PASS       (fi-skl-6770hq)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-j1900     total:244  pass:211  dwarn:1   dfail:0   fail:1   skip:31 
fi-byt-n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770k     total:244  pass:226  dwarn:0   dfail:0   fail:0   skip:18 
fi-hsw-4770r     total:244  pass:221  dwarn:0   dfail:0   fail:0   skip:23 
fi-ilk-650       total:244  pass:183  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:221  dwarn:0   dfail:0   fail:1   skip:22 
fi-skl-6700k     total:244  pass:218  dwarn:2   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2536/

9aa8c0cdbc076bcc0486d7a31922a0f77c032fe7 drm-intel-nightly: 2016y-09m-14d-09h-19m-25s UTC integration manifest
c04762c drm/i915: Queue page flip work via a low latency, unbound workqueue

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915: Queue page flip work with high priority (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Queue page flip work with high priority (rev2)
URL   : https://patchwork.freedesktop.org/series/12336/
State : warning

== Summary ==

Series 12336v2 drm/i915: Queue page flip work with high priority
https://patchwork.freedesktop.org/api/1.0/series/12336/revisions/2/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> PASS       (fi-hsw-4770k)
Test kms_busy:
        Subgroup basic-flip-default-c:
                pass       -> DMESG-WARN (fi-skl-6700k)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> SKIP       (fi-hsw-4770r)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-j1900     total:244  pass:211  dwarn:1   dfail:0   fail:1   skip:31 
fi-byt-n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770k     total:244  pass:226  dwarn:0   dfail:0   fail:0   skip:18 
fi-hsw-4770r     total:244  pass:221  dwarn:0   dfail:0   fail:0   skip:23 
fi-ilk-650       total:244  pass:183  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:221  dwarn:0   dfail:0   fail:1   skip:22 
fi-skl-6700k     total:244  pass:218  dwarn:2   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2536/

9aa8c0cdbc076bcc0486d7a31922a0f77c032fe7 drm-intel-nightly: 2016y-09m-14d-09h-19m-25s UTC integration manifest
c04762c drm/i915: Queue page flip work via a low latency, unbound workqueue

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Queue page flip work via a low latency, unbound workqueue

[Maarten Lankhorst]

Op 14-09-16 om 19:07 schreef Imre Deak:
> 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 from the generic system workqueue which has some
> scheduling overhead due it being CPU bound. 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 from a dedicated, unbound workqueue
> which provides for minimal scheduling latency.
I think it should use the same wq as intel_atomic_commit, either page flip should use
system_unbound_wq, or atomic_commit should use this one.

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Queue page flip work via a low latency, unbound workqueue

[Imre Deak]

On Thu, 2016-09-15 at 10:44 +0200, Maarten Lankhorst wrote:
> Op 14-09-16 om 19:07 schreef Imre Deak:
> > 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 from the generic system workqueue which has some
> > scheduling overhead due it being CPU bound. 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 from a dedicated, unbound workqueue
> > which provides for minimal scheduling latency.
> I think it should use the same wq as intel_atomic_commit, either page
> flip should use
> system_unbound_wq, or atomic_commit should use this one.

Hm, I didn't notice system_unbound_wq, and I suppose there is no
advantage (smaller latency) in using a dedicated wq. So I can change
this to use system_unbound_wq.

--Imre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3] drm/i915: Queue page flip work via a low latency, unbound workqueue

[Imre Deak]

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 from the generic system workqueue which has some
scheduling overhead due it being CPU bound. 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 from the unbound system workqueue
which provides for minimal scheduling latency.

v2:
- Use an unbound workqueue instead of a high-prio one. (Tvrtko, Chris)
v3:
- Use the system unbound wq instead of a dedicated one. (Maarten)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97775
Testcase: igt/kms_cursor_legacy
CC: Chris Wilson <chris@chris-wilson.co.uk>
CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v1)
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7bb7874..8da9f3c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12266,7 +12266,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(system_unbound_wq, &work->mmio_work);
 	} else {
 		request = i915_gem_request_alloc(engine, engine->last_context);
 		if (IS_ERR(request)) {
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915: Queue page flip work with high priority (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Queue page flip work with high priority (rev3)
URL   : https://patchwork.freedesktop.org/series/12336/
State : warning

== Summary ==

Series 12336v3 drm/i915: Queue page flip work with high priority
https://patchwork.freedesktop.org/api/1.0/series/12336/revisions/3/mbox/

Test gem_exec_gttfill:
        Subgroup basic:
                pass       -> SKIP       (fi-snb-2600)
Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> PASS       (fi-hsw-4770k)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> SKIP       (fi-hsw-4770r)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> SKIP       (fi-hsw-4770r)
        Subgroup suspend-read-crc-pipe-b:
                skip       -> PASS       (fi-hsw-4770r)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                fail       -> DMESG-WARN (fi-skl-6700hq)

fi-bdw-5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770k     total:208  pass:187  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:244  pass:220  dwarn:0   dfail:0   fail:0   skip:24 
fi-ilk-650       total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
fi-skl-6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:244  pass:206  dwarn:0   dfail:0   fail:0   skip:38 

Results at /archive/results/CI_IGT_test/Patchwork_2560/

de934d54f765dcfa82c018ea5d256801cffee7af drm-intel-nightly: 2016y-09m-20d-09h-13m-55s UTC integration manifest
04838a2 drm/i915: Queue page flip work via a low latency, unbound workqueue

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Queue page flip work via a low latency, unbound workqueue

[Chris Wilson]

On Tue, Sep 20, 2016 at 02:58:19PM +0300, 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 from the generic system workqueue which has some
> scheduling overhead due it being CPU bound. 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 from the unbound system workqueue
> which provides for minimal scheduling latency.
> 
> v2:
> - Use an unbound workqueue instead of a high-prio one. (Tvrtko, Chris)
> v3:
> - Use the system unbound wq instead of a dedicated one. (Maarten)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97775
> Testcase: igt/kms_cursor_legacy
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v1)

We violate the unbound_wq rules no worse than the ordinary system_wq,
and this brings mmioflip on a par with nonblocking atomic modesets, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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

Re: [PATCH v3] drm/i915: Queue page flip work via a low latency, unbound workqueue

[Maarten Lankhorst]

Op 20-09-16 om 14:51 schreef Chris Wilson:
> On Tue, Sep 20, 2016 at 02:58:19PM +0300, 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 from the generic system workqueue which has some
>> scheduling overhead due it being CPU bound. 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 from the unbound system workqueue
>> which provides for minimal scheduling latency.
>>
>> v2:
>> - Use an unbound workqueue instead of a high-prio one. (Tvrtko, Chris)
>> v3:
>> - Use the system unbound wq instead of a dedicated one. (Maarten)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97775
>> Testcase: igt/kms_cursor_legacy
>> CC: Chris Wilson <chris@chris-wilson.co.uk>
>> CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> (v1)
> We violate the unbound_wq rules no worse than the ordinary system_wq,
> and this brings mmioflip on a par with nonblocking atomic modesets, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for drm/i915: Queue page flip work with high priority (rev3)

[Imre Deak]

On ti, 2016-09-20 at 12:24 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Queue page flip work with high priority (rev3)
> URL   : https://patchwork.freedesktop.org/series/12336/
> State : warning
> 
> == Summary ==
> 
> Series 12336v3 drm/i915: Queue page flip work with high priority
> https://patchwork.freedesktop.org/api/1.0/series/12336/revisions/3/mb
> ox/
> 
> Test gem_exec_gttfill:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-snb-2600)

"""
Estimated that we need 513 objects and 2053 MiB for the test, but only
have 2013 MiB available (RAM) and a maximum of 383430 objects
Last errno: 22, Invalid argument
Subtest basic: SKIP (0.000s)
"""

It's a pre-existing sporadic issue on multiple platforms.

> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 incomplete -> PASS       (fi-hsw-4770k)
> Test kms_pipe_crc_basic:
>         Subgroup read-crc-pipe-c-frame-sequence:
>                 pass       -> SKIP       (fi-hsw-4770r)

"""
read-crc-pipe-C-frame-sequence: Testing connector HDMI-A-1 using pipe C
Test requirement not met in function test_read_crc, file
kms_pipe_crc_basic.c:178:
Test requirement: valid_connectors
No connector found for pipe 2
Subtest read-crc-pipe-C-frame-sequence: SKIP (0.179s)
"""

>         Subgroup suspend-read-crc-pipe-a:
>                 pass       -> SKIP       (fi-hsw-4770r)

"""
suspend-read-crc-pipe-A: Testing connector HDMI-A-1 using pipe A
Test requirement not met in function test_read_crc, file
kms_pipe_crc_basic.c:178:
Test requirement: valid_connectors
No connector found for pipe 0
"""

In both cases above HDMI detect fails for outputs successfully detected
in earlier subtests, a pre-existing issue on multiple platforms.

>         Subgroup suspend-read-crc-pipe-b:
>                 skip       -> PASS       (fi-hsw-4770r)
> Test kms_psr_sink_crc:
>         Subgroup psr_basic:
>                 fail       -> DMESG-WARN (fi-skl-6700hq)

[drm:intel_pipe_update_start] *ERROR* Potential atomic update failure on pipe A

Pre-existing issue on this machine (and the test succeeded only 
once in CI history).

Pushed to -dinq, thanks for the reviews.

--Imre

> 
> fi-bdw-
> 5557u     total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
> fi-bsw-
> n3050     total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
> fi-byt-
> n2820     total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
> fi-hsw-
> 4770k     total:208  pass:187  dwarn:0   dfail:0   fail:0   skip:20 
> fi-hsw-
> 4770r     total:244  pass:220  dwarn:0   dfail:0   fail:0   skip:24 
> fi-ilk-
> 650       total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
> fi-ivb-
> 3520m     total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
> fi-ivb-
> 3770      total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
> fi-skl-
> 6260u     total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
> fi-skl-
> 6700hq    total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
> fi-skl-
> 6700k     total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
> fi-skl-
> 6770hq    total:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
> fi-snb-
> 2520m     total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
> fi-snb-
> 2600      total:244  pass:206  dwarn:0   dfail:0   fail:0   skip:38 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_2560/
> 
> de934d54f765dcfa82c018ea5d256801cffee7af drm-intel-nightly: 2016y-
> 09m-20d-09h-13m-55s UTC integration manifest
> 04838a2 drm/i915: Queue page flip work via a low latency, unbound
> workqueue
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx