All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
@ 2017-08-17 12:37 Chris Wilson
  2017-08-17 13:11 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Chris Wilson @ 2017-08-17 12:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

If we miss the current vblank because the gpu was busy, that may cause a
jitter as the frame rate temporarily drops. We try to limit the impact
of this by then boosting the GPU clock to deliver the frame as quickly
as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
frequency if we detect outstanding pageflips") but was never forward
ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
Rip out legacy page_flip completion/irq handling").

References: https://bugs.freedesktop.org/show_bug.cgi?id=102199
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 drivers/gpu/drm/i915/intel_pm.c      | 42 ++-----------------------
 3 files changed, 62 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0e93ec201fe3..7d5b19553637 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12636,6 +12636,55 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_crc_source = intel_crtc_set_crc_source,
 };
 
+struct wait_rps_boost {
+	struct wait_queue_entry wait;
+
+	struct drm_crtc *crtc;
+	struct drm_i915_gem_request *request;
+};
+
+static int do_rps_boost(struct wait_queue_entry *_wait,
+			unsigned mode, int sync, void *key)
+{
+	struct wait_rps_boost *wait = container_of(_wait, typeof(*wait), wait);
+	struct drm_i915_gem_request *rq = wait->request;
+
+	gen6_rps_boost(rq, NULL);
+	i915_gem_request_put(rq);
+
+	drm_crtc_vblank_put(wait->crtc);
+
+	list_del(&wait->wait.entry);
+	kfree(wait);
+	return 1;
+}
+
+static void add_rps_boost_after_vblank(struct drm_crtc *crtc,
+				       struct dma_fence *fence)
+{
+	struct wait_rps_boost *wait;
+
+	if (!dma_fence_is_i915(fence))
+		return;
+
+	if (drm_crtc_vblank_get(crtc))
+		return;
+
+	wait = kmalloc(sizeof(*wait), GFP_KERNEL);
+	if (!wait) {
+		drm_crtc_vblank_put(crtc);
+		return;
+	}
+
+	wait->request = to_request(dma_fence_get(fence));
+	wait->crtc = crtc;
+
+	wait->wait.func = do_rps_boost;
+	wait->wait.flags = 0;
+
+	add_wait_queue(drm_crtc_vblank_waitqueue(crtc), &wait->wait);
+}
+
 /**
  * intel_prepare_plane_fb - Prepare fb for usage on plane
  * @plane: drm plane to prepare for
@@ -12733,12 +12782,22 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 		return ret;
 
 	if (!new_state->fence) { /* implicit fencing */
+		struct dma_fence *fence;
+
 		ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
 						      obj->resv, NULL,
 						      false, I915_FENCE_TIMEOUT,
 						      GFP_KERNEL);
 		if (ret < 0)
 			return ret;
+
+		fence = reservation_object_get_excl_rcu(obj->resv);
+		if (fence) {
+			add_rps_boost_after_vblank(new_state->crtc, fence);
+			dma_fence_put(fence);
+		}
+	} else {
+		add_rps_boost_after_vblank(new_state->crtc, new_state->fence);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fa47285918f4..e092354b4d63 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1844,7 +1844,6 @@ void gen6_rps_reset_ei(struct drm_i915_private *dev_priv);
 void gen6_rps_idle(struct drm_i915_private *dev_priv);
 void gen6_rps_boost(struct drm_i915_gem_request *rq,
 		    struct intel_rps_client *rps);
-void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req);
 void g4x_wm_get_hw_state(struct drm_device *dev);
 void vlv_wm_get_hw_state(struct drm_device *dev);
 void ilk_wm_get_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ed662937ec3c..c9fa2eb1903c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6169,6 +6169,7 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
 		    struct intel_rps_client *rps)
 {
 	struct drm_i915_private *i915 = rq->i915;
+	unsigned long flags;
 	bool boost;
 
 	/* This is intentionally racy! We peek at the state here, then
@@ -6178,13 +6179,13 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
 		return;
 
 	boost = false;
-	spin_lock_irq(&rq->lock);
+	spin_lock_irqsave(&rq->lock, flags);
 	if (!rq->waitboost && !i915_gem_request_completed(rq)) {
 		atomic_inc(&i915->rps.num_waiters);
 		rq->waitboost = true;
 		boost = true;
 	}
-	spin_unlock_irq(&rq->lock);
+	spin_unlock_irqrestore(&rq->lock, flags);
 	if (!boost)
 		return;
 
@@ -9132,43 +9133,6 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val)
 		return DIV_ROUND_CLOSEST(val, GT_FREQUENCY_MULTIPLIER);
 }
 
-struct request_boost {
-	struct work_struct work;
-	struct drm_i915_gem_request *req;
-};
-
-static void __intel_rps_boost_work(struct work_struct *work)
-{
-	struct request_boost *boost = container_of(work, struct request_boost, work);
-	struct drm_i915_gem_request *req = boost->req;
-
-	if (!i915_gem_request_completed(req))
-		gen6_rps_boost(req, NULL);
-
-	i915_gem_request_put(req);
-	kfree(boost);
-}
-
-void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req)
-{
-	struct request_boost *boost;
-
-	if (req == NULL || INTEL_GEN(req->i915) < 6)
-		return;
-
-	if (i915_gem_request_completed(req))
-		return;
-
-	boost = kmalloc(sizeof(*boost), GFP_ATOMIC);
-	if (boost == NULL)
-		return;
-
-	boost->req = i915_gem_request_get(req);
-
-	INIT_WORK(&boost->work, __intel_rps_boost_work);
-	queue_work(req->i915->wq, &boost->work);
-}
-
 void intel_pm_setup(struct drm_i915_private *dev_priv)
 {
 	mutex_init(&dev_priv->rps.hw_lock);
-- 
2.14.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-17 12:37 [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank Chris Wilson
@ 2017-08-17 13:11 ` Patchwork
  2017-08-18  7:54 ` [PATCH] " Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-08-17 13:11 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Boost GPU clocks if we miss the pageflip's vblank
URL   : https://patchwork.freedesktop.org/series/28921/
State : failure

== Summary ==

Series 28921v1 drm/i915: Boost GPU clocks if we miss the pageflip's vblank
https://patchwork.freedesktop.org/api/1.0/series/28921/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-skl-6260u)

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:456s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:361s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:548s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:524s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:524s
fi-byt-n2820     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:516s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:608s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:446s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:422s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:422s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:511s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:478s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:596s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:596s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:532s
fi-skl-6260u     total:237  pass:229  dwarn:0   dfail:0   fail:0   skip:7  
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:478s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:483s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:444s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:484s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:551s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:411s

ada53b43f81fe618f3f0f1dfbd3dd776bb277323 drm-tip: 2017y-08m-16d-15h-18m-56s UTC integration manifest
87e8bf68f70d drm/i915: Boost GPU clocks if we miss the pageflip's vblank

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5427/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-17 12:37 [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank Chris Wilson
  2017-08-17 13:11 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2017-08-18  7:54 ` Chris Wilson
  2017-08-18  8:50   ` Szwichtenberg, Radoslaw
  2017-08-18  9:30 ` Chris Wilson
  2017-08-21 15:54 ` Chris Wilson
  3 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2017-08-18  7:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Maarten

Quoting Chris Wilson (2017-08-17 13:37:06)
> If we miss the current vblank because the gpu was busy, that may cause a
> jitter as the frame rate temporarily drops. We try to limit the impact
> of this by then boosting the GPU clock to deliver the frame as quickly
> as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
> frequency if we detect outstanding pageflips") but was never forward
> ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
> Rip out legacy page_flip completion/irq handling").
 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102199
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
Tested-by: Lyude Paul <lyude@redhat.com>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-18  7:54 ` [PATCH] " Chris Wilson
@ 2017-08-18  8:50   ` Szwichtenberg, Radoslaw
  2017-08-22 16:11     ` Chris Wilson
  0 siblings, 1 reply; 10+ messages in thread
From: Szwichtenberg, Radoslaw @ 2017-08-18  8:50 UTC (permalink / raw)
  To: intel-gfx, chris; +Cc: Maarten

On Fri, 2017-08-18 at 08:54 +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2017-08-17 13:37:06)
> > If we miss the current vblank because the gpu was busy, that may cause a
> > jitter as the frame rate temporarily drops. We try to limit the impact
> > of this by then boosting the GPU clock to deliver the frame as quickly
> > as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
> > frequency if we detect outstanding pageflips") but was never forward
> > ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
> > Rip out legacy page_flip completion/irq handling").
> 
>  
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> 
> Tested-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-17 12:37 [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank Chris Wilson
  2017-08-17 13:11 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2017-08-18  7:54 ` [PATCH] " Chris Wilson
@ 2017-08-18  9:30 ` Chris Wilson
  2017-08-18 14:05   ` Chris Wilson
  2017-08-21 15:54 ` Chris Wilson
  3 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2017-08-18  9:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: Maarten

Quoting Chris Wilson (2017-08-17 13:37:06)
> If we miss the current vblank because the gpu was busy, that may cause a
> jitter as the frame rate temporarily drops. We try to limit the impact
> of this by then boosting the GPU clock to deliver the frame as quickly
> as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
> frequency if we detect outstanding pageflips") but was never forward
> ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
> Rip out legacy page_flip completion/irq handling").

+One of the most typical use-cases for this is a mostly idle desktop.
Rendering one frame of the desktop's frontbuffer can easily be
accomplished by the GPU running at low frequency, but often exceeds
the time budget of the desktop compositor. The result is that animations
such as opening the menu, doing a fullscreen switch, or even just trying
to move a window around are slow and jerky. We need to respond within a
frame to give the best impression of a smooth UX, as a compromise we
instead respond if that first frame misses its goal. The result should
be a near-imperceivable initial delay and a smooth animation even
starting from idle. The cost, as ever, is that we spend more power than
is strictly necessary as we overestimate the required GPU frequency and
then try to ramp down.

> References: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-18  9:30 ` Chris Wilson
@ 2017-08-18 14:05   ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2017-08-18 14:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

Quoting Chris Wilson (2017-08-18 10:30:47)
> Quoting Chris Wilson (2017-08-17 13:37:06)
> > If we miss the current vblank because the gpu was busy, that may cause a
> > jitter as the frame rate temporarily drops. We try to limit the impact
> > of this by then boosting the GPU clock to deliver the frame as quickly
> > as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
> > frequency if we detect outstanding pageflips") but was never forward
> > ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
> > Rip out legacy page_flip completion/irq handling").
> 
> +One of the most typical use-cases for this is a mostly idle desktop.
> Rendering one frame of the desktop's frontbuffer can easily be
> accomplished by the GPU running at low frequency, but often exceeds
> the time budget of the desktop compositor. The result is that animations
> such as opening the menu, doing a fullscreen switch, or even just trying
> to move a window around are slow and jerky. We need to respond within a
> frame to give the best impression of a smooth UX, as a compromise we
> instead respond if that first frame misses its goal. The result should
> be a near-imperceivable initial delay and a smooth animation even
> starting from idle. The cost, as ever, is that we spend more power than
> is strictly necessary as we overestimate the required GPU frequency and
> then try to ramp down.

Of course the first thing the display wants it be able to preemptively
boost! Now given that this basically tieing prio=DISPLAY to an always
boosted context, should we extend that grant to prio=MAX_USER. That is
all batches sent by the highest user context are executed at high GPU
clocks?

EGL only offsets low/normal/high priority contexts, so we would have to
give that grant to all high priority GL contexts -- the restriction
being that to get a high priority context at all, you have to be
CAP_SYS_NICE.

Vk is more flexible in that regard and will offer more priority values.

CL I'm not sure about.

A slightly softer idea is that it is a context-param in addition to
priority (to boost or not to boost that is the boolean), but still
always set by GL for its high priority contexts. Or we add a new EGL
extension.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-17 12:37 [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank Chris Wilson
                   ` (2 preceding siblings ...)
  2017-08-18  9:30 ` Chris Wilson
@ 2017-08-21 15:54 ` Chris Wilson
  2017-08-22 17:02   ` Ville Syrjälä
  3 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2017-08-21 15:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Maarten

Quoting Chris Wilson (2017-08-17 13:37:06)
> If we miss the current vblank because the gpu was busy, that may cause a
> jitter as the frame rate temporarily drops. We try to limit the impact
> of this by then boosting the GPU clock to deliver the frame as quickly
> as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
> frequency if we detect outstanding pageflips") but was never forward
> ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
> Rip out legacy page_flip completion/irq handling").
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>

Either of you like to ack the return of this code to the display
subsystem? It's still reactionary and will one day be replace by a pony,
or perhaps supplemented by one.
-Chris

> ---
>  drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  drivers/gpu/drm/i915/intel_pm.c      | 42 ++-----------------------
>  3 files changed, 62 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0e93ec201fe3..7d5b19553637 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12636,6 +12636,55 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
>         .set_crc_source = intel_crtc_set_crc_source,
>  };
>  
> +struct wait_rps_boost {
> +       struct wait_queue_entry wait;
> +
> +       struct drm_crtc *crtc;
> +       struct drm_i915_gem_request *request;
> +};
> +
> +static int do_rps_boost(struct wait_queue_entry *_wait,
> +                       unsigned mode, int sync, void *key)
> +{
> +       struct wait_rps_boost *wait = container_of(_wait, typeof(*wait), wait);
> +       struct drm_i915_gem_request *rq = wait->request;
> +
> +       gen6_rps_boost(rq, NULL);
> +       i915_gem_request_put(rq);
> +
> +       drm_crtc_vblank_put(wait->crtc);
> +
> +       list_del(&wait->wait.entry);
> +       kfree(wait);
> +       return 1;
> +}
> +
> +static void add_rps_boost_after_vblank(struct drm_crtc *crtc,
> +                                      struct dma_fence *fence)
> +{
> +       struct wait_rps_boost *wait;
> +
> +       if (!dma_fence_is_i915(fence))
> +               return;
> +
> +       if (drm_crtc_vblank_get(crtc))
> +               return;
> +
> +       wait = kmalloc(sizeof(*wait), GFP_KERNEL);
> +       if (!wait) {
> +               drm_crtc_vblank_put(crtc);
> +               return;
> +       }
> +
> +       wait->request = to_request(dma_fence_get(fence));
> +       wait->crtc = crtc;
> +
> +       wait->wait.func = do_rps_boost;
> +       wait->wait.flags = 0;
> +
> +       add_wait_queue(drm_crtc_vblank_waitqueue(crtc), &wait->wait);
> +}
> +
>  /**
>   * intel_prepare_plane_fb - Prepare fb for usage on plane
>   * @plane: drm plane to prepare for
> @@ -12733,12 +12782,22 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>                 return ret;
>  
>         if (!new_state->fence) { /* implicit fencing */
> +               struct dma_fence *fence;
> +
>                 ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
>                                                       obj->resv, NULL,
>                                                       false, I915_FENCE_TIMEOUT,
>                                                       GFP_KERNEL);
>                 if (ret < 0)
>                         return ret;
> +
> +               fence = reservation_object_get_excl_rcu(obj->resv);
> +               if (fence) {
> +                       add_rps_boost_after_vblank(new_state->crtc, fence);
> +                       dma_fence_put(fence);
> +               }
> +       } else {
> +               add_rps_boost_after_vblank(new_state->crtc, new_state->fence);
>         }
>  
>         return 0;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index fa47285918f4..e092354b4d63 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1844,7 +1844,6 @@ void gen6_rps_reset_ei(struct drm_i915_private *dev_priv);
>  void gen6_rps_idle(struct drm_i915_private *dev_priv);
>  void gen6_rps_boost(struct drm_i915_gem_request *rq,
>                     struct intel_rps_client *rps);
> -void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req);
>  void g4x_wm_get_hw_state(struct drm_device *dev);
>  void vlv_wm_get_hw_state(struct drm_device *dev);
>  void ilk_wm_get_hw_state(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ed662937ec3c..c9fa2eb1903c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6169,6 +6169,7 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
>                     struct intel_rps_client *rps)
>  {
>         struct drm_i915_private *i915 = rq->i915;
> +       unsigned long flags;
>         bool boost;
>  
>         /* This is intentionally racy! We peek at the state here, then
> @@ -6178,13 +6179,13 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
>                 return;
>  
>         boost = false;
> -       spin_lock_irq(&rq->lock);
> +       spin_lock_irqsave(&rq->lock, flags);
>         if (!rq->waitboost && !i915_gem_request_completed(rq)) {
>                 atomic_inc(&i915->rps.num_waiters);
>                 rq->waitboost = true;
>                 boost = true;
>         }
> -       spin_unlock_irq(&rq->lock);
> +       spin_unlock_irqrestore(&rq->lock, flags);
>         if (!boost)
>                 return;
>  
> @@ -9132,43 +9133,6 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val)
>                 return DIV_ROUND_CLOSEST(val, GT_FREQUENCY_MULTIPLIER);
>  }
>  
> -struct request_boost {
> -       struct work_struct work;
> -       struct drm_i915_gem_request *req;
> -};
> -
> -static void __intel_rps_boost_work(struct work_struct *work)
> -{
> -       struct request_boost *boost = container_of(work, struct request_boost, work);
> -       struct drm_i915_gem_request *req = boost->req;
> -
> -       if (!i915_gem_request_completed(req))
> -               gen6_rps_boost(req, NULL);
> -
> -       i915_gem_request_put(req);
> -       kfree(boost);
> -}
> -
> -void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req)
> -{
> -       struct request_boost *boost;
> -
> -       if (req == NULL || INTEL_GEN(req->i915) < 6)
> -               return;
> -
> -       if (i915_gem_request_completed(req))
> -               return;
> -
> -       boost = kmalloc(sizeof(*boost), GFP_ATOMIC);
> -       if (boost == NULL)
> -               return;
> -
> -       boost->req = i915_gem_request_get(req);
> -
> -       INIT_WORK(&boost->work, __intel_rps_boost_work);
> -       queue_work(req->i915->wq, &boost->work);
> -}
> -
>  void intel_pm_setup(struct drm_i915_private *dev_priv)
>  {
>         mutex_init(&dev_priv->rps.hw_lock);
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-18  8:50   ` Szwichtenberg, Radoslaw
@ 2017-08-22 16:11     ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2017-08-22 16:11 UTC (permalink / raw)
  To: Szwichtenberg, Radoslaw, intel-gfx; +Cc: Maarten

Quoting Szwichtenberg, Radoslaw (2017-08-18 09:50:35)
> On Fri, 2017-08-18 at 08:54 +0100, Chris Wilson wrote:
> > Quoting Chris Wilson (2017-08-17 13:37:06)
> > > If we miss the current vblank because the gpu was busy, that may cause a
> > > jitter as the frame rate temporarily drops. We try to limit the impact
> > > of this by then boosting the GPU clock to deliver the frame as quickly
> > > as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
> > > frequency if we detect outstanding pageflips") but was never forward
> > > ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
> > > Rip out legacy page_flip completion/irq handling").
> > 
> >  
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > 
> > Tested-by: Lyude Paul <lyude@redhat.com>
> Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>

Added the blurb to explain itself better and pushed to paper over the
miserable UX performance.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-21 15:54 ` Chris Wilson
@ 2017-08-22 17:02   ` Ville Syrjälä
  2017-08-22 17:12     ` Chris Wilson
  0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2017-08-22 17:02 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Maarten

On Mon, Aug 21, 2017 at 04:54:21PM +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2017-08-17 13:37:06)
> > If we miss the current vblank because the gpu was busy, that may cause a
> > jitter as the frame rate temporarily drops. We try to limit the impact
> > of this by then boosting the GPU clock to deliver the frame as quickly
> > as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
> > frequency if we detect outstanding pageflips") but was never forward
> > ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
> > Rip out legacy page_flip completion/irq handling").
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> 
> Either of you like to ack the return of this code to the display
> subsystem? It's still reactionary and will one day be replace by a pony,
> or perhaps supplemented by one.

It looks reasonable enough to me.

For the pony part I was wondering if a blind donkey would be enough.
Something like "boost to rpe as soon as a flip is queued" is what
I was thinking. But I suppose it ought to be likely that we're
already >= rpe if we have something running on the gpu. So maybe
rpe just isn't fast enough for these cases?

> -Chris
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 -
> >  drivers/gpu/drm/i915/intel_pm.c      | 42 ++-----------------------
> >  3 files changed, 62 insertions(+), 40 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 0e93ec201fe3..7d5b19553637 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12636,6 +12636,55 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
> >         .set_crc_source = intel_crtc_set_crc_source,
> >  };
> >  
> > +struct wait_rps_boost {
> > +       struct wait_queue_entry wait;
> > +
> > +       struct drm_crtc *crtc;
> > +       struct drm_i915_gem_request *request;
> > +};
> > +
> > +static int do_rps_boost(struct wait_queue_entry *_wait,
> > +                       unsigned mode, int sync, void *key)
> > +{
> > +       struct wait_rps_boost *wait = container_of(_wait, typeof(*wait), wait);
> > +       struct drm_i915_gem_request *rq = wait->request;
> > +
> > +       gen6_rps_boost(rq, NULL);
> > +       i915_gem_request_put(rq);
> > +
> > +       drm_crtc_vblank_put(wait->crtc);
> > +
> > +       list_del(&wait->wait.entry);
> > +       kfree(wait);
> > +       return 1;
> > +}
> > +
> > +static void add_rps_boost_after_vblank(struct drm_crtc *crtc,
> > +                                      struct dma_fence *fence)
> > +{
> > +       struct wait_rps_boost *wait;
> > +
> > +       if (!dma_fence_is_i915(fence))
> > +               return;
> > +
> > +       if (drm_crtc_vblank_get(crtc))
> > +               return;
> > +
> > +       wait = kmalloc(sizeof(*wait), GFP_KERNEL);
> > +       if (!wait) {
> > +               drm_crtc_vblank_put(crtc);
> > +               return;
> > +       }
> > +
> > +       wait->request = to_request(dma_fence_get(fence));
> > +       wait->crtc = crtc;
> > +
> > +       wait->wait.func = do_rps_boost;
> > +       wait->wait.flags = 0;
> > +
> > +       add_wait_queue(drm_crtc_vblank_waitqueue(crtc), &wait->wait);
> > +}
> > +
> >  /**
> >   * intel_prepare_plane_fb - Prepare fb for usage on plane
> >   * @plane: drm plane to prepare for
> > @@ -12733,12 +12782,22 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> >                 return ret;
> >  
> >         if (!new_state->fence) { /* implicit fencing */
> > +               struct dma_fence *fence;
> > +
> >                 ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
> >                                                       obj->resv, NULL,
> >                                                       false, I915_FENCE_TIMEOUT,
> >                                                       GFP_KERNEL);
> >                 if (ret < 0)
> >                         return ret;
> > +
> > +               fence = reservation_object_get_excl_rcu(obj->resv);
> > +               if (fence) {
> > +                       add_rps_boost_after_vblank(new_state->crtc, fence);
> > +                       dma_fence_put(fence);
> > +               }
> > +       } else {
> > +               add_rps_boost_after_vblank(new_state->crtc, new_state->fence);
> >         }
> >  
> >         return 0;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index fa47285918f4..e092354b4d63 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1844,7 +1844,6 @@ void gen6_rps_reset_ei(struct drm_i915_private *dev_priv);
> >  void gen6_rps_idle(struct drm_i915_private *dev_priv);
> >  void gen6_rps_boost(struct drm_i915_gem_request *rq,
> >                     struct intel_rps_client *rps);
> > -void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req);
> >  void g4x_wm_get_hw_state(struct drm_device *dev);
> >  void vlv_wm_get_hw_state(struct drm_device *dev);
> >  void ilk_wm_get_hw_state(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index ed662937ec3c..c9fa2eb1903c 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6169,6 +6169,7 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
> >                     struct intel_rps_client *rps)
> >  {
> >         struct drm_i915_private *i915 = rq->i915;
> > +       unsigned long flags;
> >         bool boost;
> >  
> >         /* This is intentionally racy! We peek at the state here, then
> > @@ -6178,13 +6179,13 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
> >                 return;
> >  
> >         boost = false;
> > -       spin_lock_irq(&rq->lock);
> > +       spin_lock_irqsave(&rq->lock, flags);
> >         if (!rq->waitboost && !i915_gem_request_completed(rq)) {
> >                 atomic_inc(&i915->rps.num_waiters);
> >                 rq->waitboost = true;
> >                 boost = true;
> >         }
> > -       spin_unlock_irq(&rq->lock);
> > +       spin_unlock_irqrestore(&rq->lock, flags);
> >         if (!boost)
> >                 return;
> >  
> > @@ -9132,43 +9133,6 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val)
> >                 return DIV_ROUND_CLOSEST(val, GT_FREQUENCY_MULTIPLIER);
> >  }
> >  
> > -struct request_boost {
> > -       struct work_struct work;
> > -       struct drm_i915_gem_request *req;
> > -};
> > -
> > -static void __intel_rps_boost_work(struct work_struct *work)
> > -{
> > -       struct request_boost *boost = container_of(work, struct request_boost, work);
> > -       struct drm_i915_gem_request *req = boost->req;
> > -
> > -       if (!i915_gem_request_completed(req))
> > -               gen6_rps_boost(req, NULL);
> > -
> > -       i915_gem_request_put(req);
> > -       kfree(boost);
> > -}
> > -
> > -void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req)
> > -{
> > -       struct request_boost *boost;
> > -
> > -       if (req == NULL || INTEL_GEN(req->i915) < 6)
> > -               return;
> > -
> > -       if (i915_gem_request_completed(req))
> > -               return;
> > -
> > -       boost = kmalloc(sizeof(*boost), GFP_ATOMIC);
> > -       if (boost == NULL)
> > -               return;
> > -
> > -       boost->req = i915_gem_request_get(req);
> > -
> > -       INIT_WORK(&boost->work, __intel_rps_boost_work);
> > -       queue_work(req->i915->wq, &boost->work);
> > -}
> > -
> >  void intel_pm_setup(struct drm_i915_private *dev_priv)
> >  {
> >         mutex_init(&dev_priv->rps.hw_lock);
> > -- 
> > 2.14.1
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank
  2017-08-22 17:02   ` Ville Syrjälä
@ 2017-08-22 17:12     ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2017-08-22 17:12 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Maarten

Quoting Ville Syrjälä (2017-08-22 18:02:04)
> On Mon, Aug 21, 2017 at 04:54:21PM +0100, Chris Wilson wrote:
> > Quoting Chris Wilson (2017-08-17 13:37:06)
> > > If we miss the current vblank because the gpu was busy, that may cause a
> > > jitter as the frame rate temporarily drops. We try to limit the impact
> > > of this by then boosting the GPU clock to deliver the frame as quickly
> > > as possible. Originally done in commit 6ad790c0f5ac ("drm/i915: Boost GPU
> > > frequency if we detect outstanding pageflips") but was never forward
> > > ported to atomic and finally dropped in commit fd3a40242e87 ("drm/i915:
> > > Rip out legacy page_flip completion/irq handling").
> > > 
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102199
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > 
> > Either of you like to ack the return of this code to the display
> > subsystem? It's still reactionary and will one day be replace by a pony,
> > or perhaps supplemented by one.
> 
> It looks reasonable enough to me.
> 
> For the pony part I was wondering if a blind donkey would be enough.
> Something like "boost to rpe as soon as a flip is queued" is what
> I was thinking. But I suppose it ought to be likely that we're
> already >= rpe if we have something running on the gpu. So maybe
> rpe just isn't fast enough for these cases?

The counterpoint is that even byt can decode a 1080p mp4 and show it at
near minimal clocks. So I feel any arbitrary boosting will run afoul of
power efficient hw (or at least fixed purpose doing just that). For the
interactivity detection, Ray was suggesting we listen to input events,
but at least we should push that coupling to userspace. My current
favourite remains granting boost privileges to a context so that when
such an interactive workload comes in, we boost (or we generalize that
with "desired clocks" on a context). We are not far then from having a
budget + deadline and the building blocks of a singing and dancing pony.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-08-22 17:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-17 12:37 [PATCH] drm/i915: Boost GPU clocks if we miss the pageflip's vblank Chris Wilson
2017-08-17 13:11 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-08-18  7:54 ` [PATCH] " Chris Wilson
2017-08-18  8:50   ` Szwichtenberg, Radoslaw
2017-08-22 16:11     ` Chris Wilson
2017-08-18  9:30 ` Chris Wilson
2017-08-18 14:05   ` Chris Wilson
2017-08-21 15:54 ` Chris Wilson
2017-08-22 17:02   ` Ville Syrjälä
2017-08-22 17:12     ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.