diff mbox series

drm/i915/rps: Flip interpretation of ips fmin/fmax to max rps

Message ID 20191026200917.1780-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series drm/i915/rps: Flip interpretation of ips fmin/fmax to max rps | expand

Commit Message

Chris Wilson Oct. 26, 2019, 8:09 p.m. UTC
ips uses clock delays as opposed to rps frequency bins. To fit the
delays into the same rps calculations, we need to invert the ips delays.

Fixes: 3e7abf814193 ("drm/i915: Extract GT render power state management")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andi Shyti <andi.shyti@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_rps.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Andi Shyti Oct. 27, 2019, 7:01 p.m. UTC | #1
Hi Chris,

> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 30f56c786468..032a0c6389f9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -180,8 +180,8 @@ static void gen5_rps_init(struct intel_rps *rps)
>  	DRM_DEBUG_DRIVER("fmax: %d, fmin: %d, fstart: %d\n",
>  			 fmax, fmin, fstart);
>  
> -	rps->min_freq = -fstart;

and we don't need fstart anymore... can we remove it?

Reviewed-by: Andi Shyti <andi.shyti@intel.com>

Andi

> -	rps->max_freq = -fmin;
> +	rps->min_freq = fmax;
> +	rps->max_freq = fmin;
>  
>  	rps->idle_freq = rps->min_freq;
>  	rps->cur_freq = rps->idle_freq;
Chris Wilson Oct. 27, 2019, 7:11 p.m. UTC | #2
Quoting Andi Shyti (2019-10-27 19:01:33)
> Hi Chris,
> 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> > index 30f56c786468..032a0c6389f9 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > @@ -180,8 +180,8 @@ static void gen5_rps_init(struct intel_rps *rps)
> >       DRM_DEBUG_DRIVER("fmax: %d, fmin: %d, fstart: %d\n",
> >                        fmax, fmin, fstart);
> >  
> > -     rps->min_freq = -fstart;
> 
> and we don't need fstart anymore... can we remove it?

I've not worked out how to best to use it, other than log it. The choice
is whether to restore to fstart on returning control to the BIOS (i.e.
module unload / intel_rps_disable) which is what we've previously tried.
But what's the point -- we use idle frequency currently, and so is
fstart a better choice for idle frequency (and would that be?) I have
more questions than answers, so left it alone.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 30f56c786468..032a0c6389f9 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -180,8 +180,8 @@  static void gen5_rps_init(struct intel_rps *rps)
 	DRM_DEBUG_DRIVER("fmax: %d, fmin: %d, fstart: %d\n",
 			 fmax, fmin, fstart);
 
-	rps->min_freq = -fstart;
-	rps->max_freq = -fmin;
+	rps->min_freq = fmax;
+	rps->max_freq = fmin;
 
 	rps->idle_freq = rps->min_freq;
 	rps->cur_freq = rps->idle_freq;
@@ -307,7 +307,9 @@  static bool gen5_rps_set(struct intel_rps *rps, u8 val)
 		return false; /* still busy with another command */
 	}
 
-	val = -val;
+	/* Invert the frequency bin into an ips delay */
+	val = rps->max_freq - val;
+	val = rps->min_freq + val;
 
 	rgvswctl =
 		(MEMCTL_CMD_CHFREQ << MEMCTL_CMD_SHIFT) |