Re: [Intel-gfx] [PATCH 07/11] drm/i915/skl: Updated the gen9_enable_rps function
On Wed, Mar 11, 2015 at 12:27:59PM -0700, Jesse Barnes wrote: On 03/05/2015 09:37 PM, akash.g...@intel.com wrote: + /* Leaning on the below call to gen6_set_rps to program/setup the +* Up/Down EI threshold registers, as well as the RP_CONTROL, +* RP_INTERRUPT_LIMITS RPNSWREQ registers */ + dev_priv-rps.power = HIGH_POWER; /* force a reset */ Are you also sure that dev_priv-rps.cur_freq != min_freq_softlimit at this point? That's the condition for calling into the threshold update function (maybe gen6_set_rps should check both variables though). It's a good point, but Akash has inherited that bug from me. What I think we want is removing the actual intel_set_rps() calls here (and the rest of the *_enable_rps()) and do an intel_set_rps_idle() call from the common point in the caller, where we can put all the dancing required to force the RPS initialisation. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/11] drm/i915/skl: Updated the gen9_enable_rps function
On 03/11/2015 12:46 PM, Chris Wilson wrote: On Wed, Mar 11, 2015 at 12:27:59PM -0700, Jesse Barnes wrote: On 03/05/2015 09:37 PM, akash.g...@intel.com wrote: + /* Leaning on the below call to gen6_set_rps to program/setup the +* Up/Down EI threshold registers, as well as the RP_CONTROL, +* RP_INTERRUPT_LIMITS RPNSWREQ registers */ + dev_priv-rps.power = HIGH_POWER; /* force a reset */ Are you also sure that dev_priv-rps.cur_freq != min_freq_softlimit at this point? That's the condition for calling into the threshold update function (maybe gen6_set_rps should check both variables though). It's a good point, but Akash has inherited that bug from me. What I think we want is removing the actual intel_set_rps() calls here (and the rest of the *_enable_rps()) and do an intel_set_rps_idle() call from the common point in the caller, where we can put all the dancing required to force the RPS initialisation. Yeah, that would make things a little clearer. I'd be fine with that as a patch on top, fixing up the other functions as well. Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/11] drm/i915/skl: Updated the gen9_enable_rps function
On Wed, Mar 11, 2015 at 12:27:59PM -0700, Jesse Barnes wrote: On 03/05/2015 09:37 PM, akash.g...@intel.com wrote: From: Akash Goel akash.g...@intel.com On SKL, GT frequency is programmed in units of 16.66 MHZ units compared to 50 MHZ for older platforms. Also the time value specified for Up/Down EI Up/Down thresholds are expressed in units of 1.33 us, compared to 1.28 us for older platforms. So updated the gen9_enable_rps function as per that. v2: Updated to use new macro GT_INTERVAL_FROM_US v3: Removed the initial setup of certain registers, from gen9_enable_rps, which gets overridden later from gen6_set_rps (Damien) v4: Removed the enabling of rps interrupts, from gen9_enable_rps. To be done from intel_gen6_powersave_work only, as done for other platforms also. Signed-off-by: Akash Goel akash.g...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c49950f..6273c282 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4132,23 +4132,21 @@ static void gen9_enable_rps(struct drm_device *dev) gen6_init_rps_frequencies(dev); - I915_WRITE(GEN6_RPNSWREQ, 0xc80); - I915_WRITE(GEN6_RC_VIDEO_FREQ, 0xc80); - - I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 0xf4240); - I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, 0x1206); - I915_WRITE(GEN6_RP_UP_THRESHOLD, 0xe808); - I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 0x3bd08); - I915_WRITE(GEN6_RP_UP_EI, 0x101d0); - I915_WRITE(GEN6_RP_DOWN_EI, 0x55730); + /* Program defaults and thresholds for RPS*/ + I915_WRITE(GEN6_RC_VIDEO_FREQ, + GEN9_FREQUENCY(dev_priv-rps.rp1_freq)); + + /* 1 second timeout*/ + I915_WRITE(GEN6_RP_DOWN_TIMEOUT, + GT_INTERVAL_FROM_US(dev_priv, 100)); + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 0xa); - I915_WRITE(GEN6_PMINTRMSK, 0x6); - I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO | - GEN6_RP_MEDIA_HW_MODE | GEN6_RP_MEDIA_IS_GFX | - GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG | - GEN6_RP_DOWN_IDLE_AVG); - gen6_enable_rps_interrupts(dev); + /* Leaning on the below call to gen6_set_rps to program/setup the +* Up/Down EI threshold registers, as well as the RP_CONTROL, +* RP_INTERRUPT_LIMITS RPNSWREQ registers */ + dev_priv-rps.power = HIGH_POWER; /* force a reset */ Are you also sure that dev_priv-rps.cur_freq != min_freq_softlimit at this point? That's the condition for calling into the threshold update function (maybe gen6_set_rps should check both variables though). gen6 uses the same trick, so I hope it's safe. And indeed a bit there's a call in both functions to gen6_init_rps_frequences which clears cur_freq to 0. Not the clearest code though, maybe we should move that right to above the call to gen6_set_rps. There's the added confusion that vlv/chv works different. Anyway that's material for a different patch, if at all. + gen6_set_rps(dev_priv-dev, dev_priv-rps.min_freq_softlimit); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); } I'm assuming these match the latest SKL PM bits, but either way can be updated later based on tuning. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org Merged up to this patch, still waiting for some final review on the remaining ones. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/11] drm/i915/skl: Updated the gen9_enable_rps function
On 03/05/2015 09:37 PM, akash.g...@intel.com wrote: From: Akash Goel akash.g...@intel.com On SKL, GT frequency is programmed in units of 16.66 MHZ units compared to 50 MHZ for older platforms. Also the time value specified for Up/Down EI Up/Down thresholds are expressed in units of 1.33 us, compared to 1.28 us for older platforms. So updated the gen9_enable_rps function as per that. v2: Updated to use new macro GT_INTERVAL_FROM_US v3: Removed the initial setup of certain registers, from gen9_enable_rps, which gets overridden later from gen6_set_rps (Damien) v4: Removed the enabling of rps interrupts, from gen9_enable_rps. To be done from intel_gen6_powersave_work only, as done for other platforms also. Signed-off-by: Akash Goel akash.g...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c49950f..6273c282 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4132,23 +4132,21 @@ static void gen9_enable_rps(struct drm_device *dev) gen6_init_rps_frequencies(dev); - I915_WRITE(GEN6_RPNSWREQ, 0xc80); - I915_WRITE(GEN6_RC_VIDEO_FREQ, 0xc80); - - I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 0xf4240); - I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, 0x1206); - I915_WRITE(GEN6_RP_UP_THRESHOLD, 0xe808); - I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 0x3bd08); - I915_WRITE(GEN6_RP_UP_EI, 0x101d0); - I915_WRITE(GEN6_RP_DOWN_EI, 0x55730); + /* Program defaults and thresholds for RPS*/ + I915_WRITE(GEN6_RC_VIDEO_FREQ, + GEN9_FREQUENCY(dev_priv-rps.rp1_freq)); + + /* 1 second timeout*/ + I915_WRITE(GEN6_RP_DOWN_TIMEOUT, + GT_INTERVAL_FROM_US(dev_priv, 100)); + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 0xa); - I915_WRITE(GEN6_PMINTRMSK, 0x6); - I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO | -GEN6_RP_MEDIA_HW_MODE | GEN6_RP_MEDIA_IS_GFX | -GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG | -GEN6_RP_DOWN_IDLE_AVG); - gen6_enable_rps_interrupts(dev); + /* Leaning on the below call to gen6_set_rps to program/setup the + * Up/Down EI threshold registers, as well as the RP_CONTROL, + * RP_INTERRUPT_LIMITS RPNSWREQ registers */ + dev_priv-rps.power = HIGH_POWER; /* force a reset */ Are you also sure that dev_priv-rps.cur_freq != min_freq_softlimit at this point? That's the condition for calling into the threshold update function (maybe gen6_set_rps should check both variables though). + gen6_set_rps(dev_priv-dev, dev_priv-rps.min_freq_softlimit); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); } I'm assuming these match the latest SKL PM bits, but either way can be updated later based on tuning. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/11] drm/i915/skl: Updated the gen9_enable_rps function
From: Akash Goel akash.g...@intel.com On SKL, GT frequency is programmed in units of 16.66 MHZ units compared to 50 MHZ for older platforms. Also the time value specified for Up/Down EI Up/Down thresholds are expressed in units of 1.33 us, compared to 1.28 us for older platforms. So updated the gen9_enable_rps function as per that. v2: Updated to use new macro GT_INTERVAL_FROM_US v3: Removed the initial setup of certain registers, from gen9_enable_rps, which gets overridden later from gen6_set_rps (Damien) v4: Removed the enabling of rps interrupts, from gen9_enable_rps. To be done from intel_gen6_powersave_work only, as done for other platforms also. Signed-off-by: Akash Goel akash.g...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c49950f..6273c282 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4132,23 +4132,21 @@ static void gen9_enable_rps(struct drm_device *dev) gen6_init_rps_frequencies(dev); - I915_WRITE(GEN6_RPNSWREQ, 0xc80); - I915_WRITE(GEN6_RC_VIDEO_FREQ, 0xc80); - - I915_WRITE(GEN6_RP_DOWN_TIMEOUT, 0xf4240); - I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, 0x1206); - I915_WRITE(GEN6_RP_UP_THRESHOLD, 0xe808); - I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 0x3bd08); - I915_WRITE(GEN6_RP_UP_EI, 0x101d0); - I915_WRITE(GEN6_RP_DOWN_EI, 0x55730); + /* Program defaults and thresholds for RPS*/ + I915_WRITE(GEN6_RC_VIDEO_FREQ, + GEN9_FREQUENCY(dev_priv-rps.rp1_freq)); + + /* 1 second timeout*/ + I915_WRITE(GEN6_RP_DOWN_TIMEOUT, + GT_INTERVAL_FROM_US(dev_priv, 100)); + I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 0xa); - I915_WRITE(GEN6_PMINTRMSK, 0x6); - I915_WRITE(GEN6_RP_CONTROL, GEN6_RP_MEDIA_TURBO | - GEN6_RP_MEDIA_HW_MODE | GEN6_RP_MEDIA_IS_GFX | - GEN6_RP_ENABLE | GEN6_RP_UP_BUSY_AVG | - GEN6_RP_DOWN_IDLE_AVG); - gen6_enable_rps_interrupts(dev); + /* Leaning on the below call to gen6_set_rps to program/setup the +* Up/Down EI threshold registers, as well as the RP_CONTROL, +* RP_INTERRUPT_LIMITS RPNSWREQ registers */ + dev_priv-rps.power = HIGH_POWER; /* force a reset */ + gen6_set_rps(dev_priv-dev, dev_priv-rps.min_freq_softlimit); intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); } -- 1.9.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
