| Message ID | 20230304012705.70003-3-ashutosh.dixit@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/pmu: Freq sampling: Fix requested freq fallback | expand |
On 04/03/2023 01:27, Ashutosh Dixit wrote: > SLPC does not use 'struct intel_rps'. Use UNSLICE_RATIO bits from Would it be more accurate to say 'SLPC does not use rps->cur_freq' rather than it not using struct intel_rps? Fixes: / stable ? CI chances of catching this? > GEN6_RPNSWREQ for SLPC. See intel_rps_get_requested_frequency. > > Bspec: 52745 > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > --- > drivers/gpu/drm/i915/i915_pmu.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index f0a1e36915b8..5ee836610801 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -394,8 +394,13 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns) > * frequency. Fortunately, the read should rarely fail! > */ > val = intel_rps_get_cagf(rps, intel_rps_read_rpstat_fw(rps)); > - if (!val) > - val = rps->cur_freq; > + if (!val) { > + if (intel_uc_uses_guc_slpc(>->uc)) > + val = intel_rps_read_punit_req(rps) >> > + GEN9_SW_REQ_UNSLICE_RATIO_SHIFT; > + else > + val = rps->cur_freq; > + } That's a bunch of duplication from intel_rps.c so perhaps the appropriate helpers should be exported (some way) from there. Regards, Tvrtko > > add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT], > intel_gpu_freq(rps, val), period_ns / 1000);
On Mon, 06 Mar 2023 03:10:24 -0800, Tvrtko Ursulin wrote: > Hi Tvrtko, > On 04/03/2023 01:27, Ashutosh Dixit wrote: > > SLPC does not use 'struct intel_rps'. Use UNSLICE_RATIO bits from > > Would it be more accurate to say 'SLPC does not use rps->cur_freq' rather > than it not using struct intel_rps? No actually SLPC maintains a separate 'struct intel_guc_slpc' and does not use 'struct intel_rps' at all so all of 'struct intel_rps' is 0. > Fixes: / stable ? CI chances of catching this? Same issue as Patch 1, I have answered this there. > > GEN6_RPNSWREQ for SLPC. See intel_rps_get_requested_frequency. > > > > Bspec: 52745 > > > > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > > --- > > drivers/gpu/drm/i915/i915_pmu.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > > index f0a1e36915b8..5ee836610801 100644 > > --- a/drivers/gpu/drm/i915/i915_pmu.c > > +++ b/drivers/gpu/drm/i915/i915_pmu.c > > @@ -394,8 +394,13 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns) > > * frequency. Fortunately, the read should rarely fail! > > */ > > val = intel_rps_get_cagf(rps, intel_rps_read_rpstat_fw(rps)); > > - if (!val) > > - val = rps->cur_freq; > > + if (!val) { > > + if (intel_uc_uses_guc_slpc(>->uc)) > > + val = intel_rps_read_punit_req(rps) >> > > + GEN9_SW_REQ_UNSLICE_RATIO_SHIFT; > > + else > > + val = rps->cur_freq; > > + } > > That's a bunch of duplication from intel_rps.c so perhaps the appropriate > helpers should be exported (some way) from there. This is also addressed in the new series: https://patchwork.freedesktop.org/series/114814/ > > add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT], > > intel_gpu_freq(rps, val), period_ns / 1000); Thanks. -- Ashutosh
On 08/03/2023 05:36, Dixit, Ashutosh wrote: > On Mon, 06 Mar 2023 03:10:24 -0800, Tvrtko Ursulin wrote: >> > > Hi Tvrtko, > >> On 04/03/2023 01:27, Ashutosh Dixit wrote: >>> SLPC does not use 'struct intel_rps'. Use UNSLICE_RATIO bits from >> >> Would it be more accurate to say 'SLPC does not use rps->cur_freq' rather >> than it not using struct intel_rps? > > No actually SLPC maintains a separate 'struct intel_guc_slpc' and does not > use 'struct intel_rps' at all so all of 'struct intel_rps' is 0. I keep forgetting how there is zero code / data sharing with all this.. :( >> Fixes: / stable ? CI chances of catching this? > > Same issue as Patch 1, I have answered this there. Okay lets see it clarified there. Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index f0a1e36915b8..5ee836610801 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -394,8 +394,13 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns) * frequency. Fortunately, the read should rarely fail! */ val = intel_rps_get_cagf(rps, intel_rps_read_rpstat_fw(rps)); - if (!val) - val = rps->cur_freq; + if (!val) { + if (intel_uc_uses_guc_slpc(>->uc)) + val = intel_rps_read_punit_req(rps) >> + GEN9_SW_REQ_UNSLICE_RATIO_SHIFT; + else + val = rps->cur_freq; + } add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT], intel_gpu_freq(rps, val), period_ns / 1000);
SLPC does not use 'struct intel_rps'. Use UNSLICE_RATIO bits from GEN6_RPNSWREQ for SLPC. See intel_rps_get_requested_frequency. Bspec: 52745 Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> --- drivers/gpu/drm/i915/i915_pmu.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)