In the selftest, we are observing that requests to change frequency are simply not occuring [within a 20ms period]. The assumption was that with an active GPU, these writes would be flush naturally; this appears to be false. Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/gt/intel_rps.c | 33 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c index 4dcfae16a7ce..72142e527eb2 100644 --- a/drivers/gpu/drm/i915/gt/intel_rps.c +++ b/drivers/gpu/drm/i915/gt/intel_rps.c @@ -698,18 +698,32 @@ static int rps_set(struct intel_rps *rps, u8 val, bool update) if (val == rps->last_freq) return 0; + /* + * The punit delays the write of the frequency and voltage until it + * determines the GPU is awake. During normal usage we don't want to + * waste power changing the frequency if the GPU is sleeping (rc6). + * However, the GPU and driver is now idle and we do not want to delay + * switching to minimum voltage (reducing power whilst idle) as we do + * not expect to be woken in the near future and so must flush the + * change by waking the device. + */ + intel_uncore_forcewake_get(rps_to_uncore(rps), FORCEWAKE_ALL); + if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) err = vlv_rps_set(rps, val); else err = gen6_rps_set(rps, val); if (err) - return err; + goto out_fw; if (update) gen6_rps_set_thresholds(rps, val); + rps->last_freq = val; - return 0; +out_fw: + intel_uncore_forcewake_get(rps_to_uncore(rps), FORCEWAKE_ALL); + return err; } void intel_rps_unpark(struct intel_rps *rps) @@ -755,22 +769,7 @@ void intel_rps_park(struct intel_rps *rps) if (rps->last_freq <= rps->idle_freq) return; - /* - * The punit delays the write of the frequency and voltage until it - * determines the GPU is awake. During normal usage we don't want to - * waste power changing the frequency if the GPU is sleeping (rc6). - * However, the GPU and driver is now idle and we do not want to delay - * switching to minimum voltage (reducing power whilst idle) as we do - * not expect to be woken in the near future and so must flush the - * change by waking the device. - * - * We choose to take the media powerwell (either would do to trick the - * punit into committing the voltage change) as that takes a lot less - * power than the render powerwell. - */ - intel_uncore_forcewake_get(rps_to_uncore(rps), FORCEWAKE_MEDIA); rps_set(rps, rps->idle_freq, false); - intel_uncore_forcewake_put(rps_to_uncore(rps), FORCEWAKE_MEDIA); /* * Since we will try and restart from the previously requested -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
