diff mbox

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

Message ID 1473872835-11882-1-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak Sept. 14, 2016, 5:07 p.m. UTC 
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(-)

Comments

Maarten Lankhorst Sept. 15, 2016, 8:44 a.m. UTC | #1 
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
Imre Deak Sept. 15, 2016, 11:30 a.m. UTC | #2 
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
diff mbox

Patch

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)) {