| Message ID | 20170220094713.22874-6-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2017-02-20 at 09:47 +0000, Chris Wilson wrote: > During initialisation, we set different flags for different > architectures - these should be preserved when we reload the RPS > thresholds. If we use a mmio read, it will first ensure that the > threshold registers are written before we apply the latch in RP_CONTROL. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: stable@vger.kernel.org Reviewed-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
Chris Wilson <chris@chris-wilson.co.uk> writes: > During initialisation, we set different flags for different > architectures - these should be preserved when we reload the RPS > thresholds. If we use a mmio read, it will first ensure that the > threshold registers are written before we apply the latch in RP_CONTROL. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: stable@vger.kernel.org This will change how the valleyview will do the DOWN_IDLE, due to readback you will get a GEN6_RP_DOWN_IDLE_CONT. I can't think of why we would like to keep that behaviour as the IDLE_CONT setup is a twart in my opinion. If you agree with the above, substitute the IDLE_CONT in valleview setup and you can add, Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 3041cd4988a6..d37e95b3525d 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4884,13 +4884,9 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) > GT_INTERVAL_FROM_US(dev_priv, > ei_down * threshold_down / 100)); > > + /* Restart RPS to reload the thresholds */ > I915_WRITE_FW(GEN6_RP_CONTROL, > - GEN6_RP_MEDIA_TURBO | > - GEN6_RP_MEDIA_HW_NORMAL_MODE | > - GEN6_RP_MEDIA_IS_GFX | > - GEN6_RP_ENABLE | > - GEN6_RP_UP_BUSY_AVG | > - GEN6_RP_DOWN_IDLE_AVG); > + I915_READ_FW(GEN6_RP_CONTROL) | GEN6_RP_ENABLE); > > intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL); > spin_unlock_irq(&dev_priv->uncore.lock); > -- > 2.11.0
On Mon, Feb 20, 2017 at 04:40:47PM +0200, Mika Kuoppala wrote: > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > During initialisation, we set different flags for different > > architectures - these should be preserved when we reload the RPS > > thresholds. If we use a mmio read, it will first ensure that the > > threshold registers are written before we apply the latch in RP_CONTROL. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Cc: stable@vger.kernel.org > > This will change how the valleyview will do the DOWN_IDLE, > due to readback you will get a GEN6_RP_DOWN_IDLE_CONT. No change, since we don't use it on byt - we only use the EI intervals as we manually calculate the up/down signals based on the C0 counters. > I can't think of why we would like to keep that behaviour > as the IDLE_CONT setup is a twart in my opinion. > > If you agree with the above, substitute the IDLE_CONT in > valleview setup and you can add, It is a mistake in the setup, but that change has to be seperate to exclude it as being part of the magic that avoids the hang. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > On Mon, Feb 20, 2017 at 04:40:47PM +0200, Mika Kuoppala wrote: >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > During initialisation, we set different flags for different >> > architectures - these should be preserved when we reload the RPS >> > thresholds. If we use a mmio read, it will first ensure that the >> > threshold registers are written before we apply the latch in RP_CONTROL. >> > >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> > Cc: stable@vger.kernel.org >> >> This will change how the valleyview will do the DOWN_IDLE, >> due to readback you will get a GEN6_RP_DOWN_IDLE_CONT. > > No change, since we don't use it on byt - we only use the EI intervals > as we manually calculate the up/down signals based on the C0 counters. > >> I can't think of why we would like to keep that behaviour >> as the IDLE_CONT setup is a twart in my opinion. >> >> If you agree with the above, substitute the IDLE_CONT in >> valleview setup and you can add, > > It is a mistake in the setup, but that change has to be seperate to > exclude it as being part of the magic that avoids the hang. Ok, so let me rephrase. As the skipping of the IDLE_CONT had an effect to the way byt behaves, we intentionally want to keep that with byt. So this is no accident, and yes it makes sense Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3041cd4988a6..d37e95b3525d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4884,13 +4884,9 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) GT_INTERVAL_FROM_US(dev_priv, ei_down * threshold_down / 100)); + /* Restart RPS to reload the thresholds */ I915_WRITE_FW(GEN6_RP_CONTROL, - GEN6_RP_MEDIA_TURBO | - GEN6_RP_MEDIA_HW_NORMAL_MODE | - GEN6_RP_MEDIA_IS_GFX | - GEN6_RP_ENABLE | - GEN6_RP_UP_BUSY_AVG | - GEN6_RP_DOWN_IDLE_AVG); + I915_READ_FW(GEN6_RP_CONTROL) | GEN6_RP_ENABLE); intel_uncore_forcewake_put__locked(dev_priv, FORCEWAKE_ALL); spin_unlock_irq(&dev_priv->uncore.lock);
During initialisation, we set different flags for different architectures - these should be preserved when we reload the RPS thresholds. If we use a mmio read, it will first ensure that the threshold registers are written before we apply the latch in RP_CONTROL. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: stable@vger.kernel.org --- drivers/gpu/drm/i915/intel_pm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)