From: Tvrtko Ursulin tvrtko.ursulin@intel.com
We use GT parked status to estimate RC6 while not in use, however if RC6 is not supported to start with that does not work very well and produces a false 100% RC6 readout.
Fix by not advancing the estimated RC6 counter when feature is not supported.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Fixes: 1fe699e30113 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout") Reported-by: Eero T Tamminen eero.t.tamminen@intel.com --- drivers/gpu/drm/i915/i915_pmu.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 41651ac255fa..02fe0d22c470 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -191,7 +191,10 @@ static u64 get_rc6(struct intel_gt *gt) * on top of the last known real value, as the approximated RC6 * counter value. */ - val = ktime_since_raw(pmu->sleep_last); + if (gt->rc6.supported) + val = ktime_since_raw(pmu->sleep_last); + else + val = 0; val += pmu->sample[__I915_SAMPLE_RC6].cur; }
On Tue, Mar 30, 2021 at 04:06:37PM +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
We use GT parked status to estimate RC6 while not in use, however if RC6 is not supported to start with that does not work very well and produces a false 100% RC6 readout.
oh! I had missed this one...
Fix by not advancing the estimated RC6 counter when feature is not supported.
either this or the other proposal, consider both as
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
I prefer this, but I don't have strong opinions on which one. you (or Eero) pick one...
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Fixes: 1fe699e30113 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout") Reported-by: Eero T Tamminen eero.t.tamminen@intel.com
drivers/gpu/drm/i915/i915_pmu.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 41651ac255fa..02fe0d22c470 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -191,7 +191,10 @@ static u64 get_rc6(struct intel_gt *gt) * on top of the last known real value, as the approximated RC6 * counter value. */
val = ktime_since_raw(pmu->sleep_last);
if (gt->rc6.supported)
val = ktime_since_raw(pmu->sleep_last);
else
val += pmu->sample[__I915_SAMPLE_RC6].cur; }val = 0;
-- 2.27.0
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Tvrtko,
We use GT parked status to estimate RC6 while not in use, however if RC6 is not supported to start with that does not work very well and produces a false 100% RC6 readout.
Fix by not advancing the estimated RC6 counter when feature is not supported.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Fixes: 1fe699e30113 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout") Reported-by: Eero T Tamminen eero.t.tamminen@intel.com
drivers/gpu/drm/i915/i915_pmu.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 41651ac255fa..02fe0d22c470 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -191,7 +191,10 @@ static u64 get_rc6(struct intel_gt *gt) * on top of the last known real value, as the approximated RC6 * counter value. */
val = ktime_since_raw(pmu->sleep_last);
if (gt->rc6.supported)
val = ktime_since_raw(pmu->sleep_last);
else
val = 0;
if rc6 is not supported, why are we here?
Did you mean rc6.enabled ?
Andi
On 06/04/2021 12:19, Andi Shyti wrote:
Hi Tvrtko,
We use GT parked status to estimate RC6 while not in use, however if RC6 is not supported to start with that does not work very well and produces a false 100% RC6 readout.
Fix by not advancing the estimated RC6 counter when feature is not supported.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Fixes: 1fe699e30113 ("drm/i915/pmu: Fix sleep under atomic in RC6 readout") Reported-by: Eero T Tamminen eero.t.tamminen@intel.com
drivers/gpu/drm/i915/i915_pmu.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 41651ac255fa..02fe0d22c470 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -191,7 +191,10 @@ static u64 get_rc6(struct intel_gt *gt) * on top of the last known real value, as the approximated RC6 * counter value. */
val = ktime_since_raw(pmu->sleep_last);
if (gt->rc6.supported)
val = ktime_since_raw(pmu->sleep_last);
else
val = 0;
if rc6 is not supported, why are we here?
There is another flavour of this patch which indeed prevents us from getting here if rc6 is not enabled. (By not exposing the counter if not supported.)
Did you mean rc6.enabled ?
Yeah, I did not see that one initially at all! But it doesn't matter since this patch is not going in anyway.
Regards,
Tvrtko
dri-devel@lists.freedesktop.org