Quoting Tvrtko Ursulin (2019-11-28 16:10:51) > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > We used to report the minimum possible frequency as both requested and > active while GPU was in sleep state. This was a consequence of sampling > the value from the "current frequency" field in our software tracking. > > This was strictly speaking wrong, but given that until recently the > current frequency in sleeping state used to be equal to minimum, it did > not stand out sufficiently to be noticed as such. > > After some recent changes have made the current frequency be reported > as last active before GPU went to sleep, meaning both requested and active > frequencies could end up being reported at their maximum values for the > duration of the GPU idle state, it became much more obvious that this does > not make sense. > > To fix this we will now sample the frequency counters only when the GPU is > awake. As a consequence reported frequencies could be reported as below > the GPU reported minimum but that should be much less confusing that the > current situation. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Hmm. 0/0 while off, that will be a bit of a shock. Idea/codewise Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> I'll dogfood it a bit to see how much getting used to it will take :) > --- > drivers/gpu/drm/i915/i915_pmu.c | 44 +++++++++++++++++---------------- > 1 file changed, 23 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index 95e824a78d4d..b576a2be9f81 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -378,32 +378,32 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns) > struct i915_pmu *pmu = &i915->pmu; > struct intel_rps *rps = >->rps; > > + if (!(pmu->enable & > + (config_enabled_mask(I915_PMU_ACTUAL_FREQUENCY) | > + config_enabled_mask(I915_PMU_REQUESTED_FREQUENCY))) || > + !intel_gt_pm_get_if_awake(gt)) > + return; Fwiw, I'd put this as two if()s since the multiline flags makes it harder to read. Any chance we could do a if (!frequency_sample_enabled(pmu)) return; /* Report 0/0 (actual/requested) frequency while parked */ if (!intel_gt_pm_get_if_awake(gt)) return; -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
