drm/i915: Add boot paramter to control rps boost at boot time.
Subsystems (2)
PATCH v3 1/1 drm/i915: Add boot paramter to control rps boost at boot time.
From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
v2: Addressed rps default comment (Jani)
v3: Use bool to represent the boot parameter (Ville).
--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 5 +++++ 3 files changed, 7 insertions(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -1945,6 +1945,7 @@ struct i915_params {bool reset;bool disable_display;bool disable_vtd_wa; +bool enable_rps_boost; };extern struct i915_params i915__read_mostly ;
drivers/gpu/drm/i915/i915_gem.c +1 -1
@@ -1049,7 +1049,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, timeout_expire = timeout ? jiffies +timespec_to_jiffies_timeout (timeout) :0 ; -if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv)) { +if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv) && i915.enable_rps_boost) {gen6_rps_boost (dev_priv);if (file_priv) mod_delayed_work(dev_priv->wq,
drivers/gpu/drm/i915/i915_params.c +5 -0
@@ -48,6 +48,7 @@ struct i915_params i915 __read_mostly = { .disable_display = 0, .enable_cmd_parser = 1, .disable_vtd_wa = 0, + .enable_rps_boost = true, };module_param_named (modeset, i915.modeset, int,0400 );
@@ -156,3 +157,7 @@ MODULE_PARM_DESC(disable_vtd_wa, "Disable all VT-d workarounds (default: false)"module_param_named (enable_cmd_parser, i915.enable_cmd_parser, int,0600 );MODULE_PARM_DESC (enable_cmd_parser,"Enable command parsing (1=enabled [default], 0=disabled)" ); + +module_param_named (enable_rps_boost, i915.enable_rps_boost, bool,0600 ); +MODULE_PARM_DESC (enable_rps_boost, +"Enable/Disable boost RPS frequency (default: true)" );
Signature · 6 lines
1.8.5.2 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 35 lines · On Mon, Apr 28, 2014 at 08:17:04PM +0530, [email protected] wrote:
From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
v2: Addressed rps default comment (Jani)
v3: Use bool to represent the boot parameter (Ville).
Signed-off-by Deepak S <[email protected]>Reviewed-by Ville Syrjälä <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_params.c | 5 +++++ 3 files changed, 7 insertions(+), 1 deletion(-)drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -1945,6 +1945,7 @@ struct i915_params {bool reset;bool disable_display;bool disable_vtd_wa; +bool enable_rps_boost; };extern struct i915_params i915__read_mostly ;drivers/gpu/drm/i915/i915_gem.c +0
-0
drivers/gpu/drm/i915/i915_gem.c static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, · line 1052
Quoted 5 lines
drivers/gpu/drm/i915/i915_gem.c +1
-1@@ -1049,7 +1049,7 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno, timeout_expire = timeout ? jiffies +timespec_to_jiffies_timeout (timeout) :0 ; -if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv)) { +if (INTEL_INFO (dev)->gen >=6 &&can_wait_boost (file_priv) && i915.enable_rps_boost) {
The separate INTEL_INFO was because this used to be a neat dev_priv->info.gen dereference (and dev used to not be derivable from file_priv, which itself may be NULL here), but please don't add another predicate that means can_wait_boost().
-Chris
Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
Quoted 6 lines · On Mon, Apr 28, 2014 at 4:47 PM, <[email protected]> wrote:
From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
Quoted 6 lines
v2: Addressed rps default comment (Jani)
v3: Use bool to represent the boot parameter (Ville).
Signed-off-by Deepak S <[email protected]>Reviewed-by Ville Syrjälä <[email protected]>
I'm still unhappy about this since it feels like cheating in benchmarks and it gives me the impression that you guys frob this at runtime on Android ;-)
A few more ideas:
1. "light-boost": We add some hysteris (either time or whether we're still above rpe or something like that) and don't boost if this is the case. I expect that we won't be able to have the full boost benefits without the downside.
2. "eco-boost". We try to boost just enough to not miss the next frame. For that the app needs to tell us (with two new execbuf flag) whether it hit or missed the last deadline. Once an app used those flags for the first time we decrease the boost target freq once per HIT_DEADLINE and until we get the first MISS_DEADLINE. The we only try to sporadically test the limit again. TCP flow control theory might be interesting for copying ideas.
3. "runtime-boost-control". The workloads with very predictable regular loads seem to be known. We can just add a new execbuf NO_BOOST flag which libva uses on all execbufs but the first one (since we don't want to drop the first frame really).
Approach 3 should be the simplest to implement and also the simplest to demonstrate in the open-source libva (since that's always a merge criteria).
Aside: If you really use this at runtime then you essentially create a new ABI with this patch. Which means we need open-source userspace for it anyway.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 36 lines · Daniel Vetter
Quoted 6 lines · On Mon, Apr 28, 2014 at 4:47 PM, <[email protected]> wrote:
From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
Quoted 6 lines
v2: Addressed rps default comment (Jani)
v3: Use bool to represent the boot parameter (Ville).
Signed-off-by Deepak S <[email protected]>Reviewed-by Ville Syrjälä <[email protected]>I'm still unhappy about this since it feels like cheating in benchmarks and it gives me the impression that you guys frob this at runtime on Android ;-)
A few more ideas:
1. "light-boost": We add some hysteris (either time or whether we're still above rpe or something like that) and don't boost if this is the case. I expect that we won't be able to have the full boost benefits without the downside.2. "eco-boost". We try to boost just enough to not miss the next frame. For that the app needs to tell us (with two new execbuf flag) whether it hit or missed the last deadline. Once an app used those flags for the first time we decrease the boost target freq once per
HIT_DEADLINEand until we get the firstMISS_DEADLINE. The we only try to sporadically test the limit again. TCP flow control theory might be interesting for copying ideas.3. "runtime-boost-control". The workloads with very predictable regular loads seem to be known. We can just add a new execbuf
NO_BOOSTflag which libva uses on all execbufs but the first one (since we don't want to drop the first frame really).
Quoted 8 lines
Approach 3 should be the simplest to implement and also the simplest to demonstrate in the open-source libva (since that's always a merge criteria).
Aside: If you really use this at runtime then you essentially create a new ABI with this patch. Which means we need open-source userspace for it anyway.
-Daniel
Thanks for the review.
Agreed. option 3 is better. I will work on this.
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 42 lines · Deepak S
Quoted 40 lines · Daniel Vetter
Quoted 6 lines · On Mon, Apr 28, 2014 at 4:47 PM, <[email protected]> wrote:
From: Deepak S <[email protected]>
We are adding a module paramter to control rps boost. By default, we enable the boost for better performace. Based on the need (perf/power) we can either enable/disable.
Quoted 6 lines
v2: Addressed rps default comment (Jani)
v3: Use bool to represent the boot parameter (Ville).
Signed-off-by Deepak S <[email protected]>Reviewed-by Ville Syrjälä <[email protected]>I'm still unhappy about this since it feels like cheating in benchmarks and it gives me the impression that you guys frob this at runtime on Android ;-)
A few more ideas:
1. "light-boost": We add some hysteris (either time or whether we're still above rpe or something like that) and don't boost if this is the case. I expect that we won't be able to have the full boost benefits without the downside.2. "eco-boost". We try to boost just enough to not miss the next frame. For that the app needs to tell us (with two new execbuf flag) whether it hit or missed the last deadline. Once an app used those flags for the first time we decrease the boost target freq once per
HIT_DEADLINEand until we get the firstMISS_DEADLINE. The we only try to sporadically test the limit again. TCP flow control theory might be interesting for copying ideas.3. "runtime-boost-control". The workloads with very predictable regular loads seem to be known. We can just add a new execbuf
NO_BOOSTflag which libva uses on all execbufs but the first one (since we don't want to drop the first frame really).Approach 3 should be the simplest to implement and also the simplest to demonstrate in the open-source libva (since that's always a merge criteria).
Quoted 7 lines
Quoted 4 lines
Aside: If you really use this at runtime then you essentially create a new ABI with this patch. Which means we need open-source userspace for it anyway.
-DanielThanks for the review.
Agreed. option 3 is better. I will work on this.
Cool. Can you please get in contact with the otc libva guys for the demonstration user? The libva patch should be approved by libva owners for merging.
Wrt testcase I think we only need to update the execbuf flag tests in gem_exec_params. Having a functional test seems less useful, we want to regression test that this works through overall power consumption metrics I guess.
Cheers, Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch