On Fri, 08 Sep 2023 18:16:26 -0700, Ashutosh Dixit wrote: > Hi Umesh, > From: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx> > > Correct values for OAR counters are still dependent on enabling the > GEN12_OAG_OACONTROL_OA_COUNTER_ENABLE in OAG_OACONTROL. Enabling this > bit means OAG unit will write reports to the OAG buffer, so > initialize the OAG buffer unconditionally for all use cases. > > BSpec: 46822 > > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_perf.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 1347e4ec9dd5a..30cf37d6e79be 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -3032,12 +3032,12 @@ static void gen12_oa_enable(struct i915_perf_stream *stream) > u32 val; > > /* > - * If we don't want OA reports from the OA buffer, then we don't even > - * need to program the OAG unit. > + * BSpec: 46822 > + * Correct values for OAR counters are still dependent on enabling the > + * GEN12_OAG_OACONTROL_OA_COUNTER_ENABLE in OAG_OACONTROL. Enabling this > + * bit means OAG unit will write reports to the OAG buffer, so > + * initialize the OAG buffer correctly. > */ > - if (!(stream->sample_flags & SAMPLE_OA_REPORT)) > - return; > - > gen12_init_oa_buffer(stream); > > regs = __oa_regs(stream); Looks like this should be needed, I can R-b it. However, gen12_test_mi_rpc IGT says: /* OA unit configuration: * DRM_I915_PERF_PROP_SAMPLE_OA is no longer required for Gen12 * because the OAR unit increments counters only for the * relevant context. No other parameters are needed since we do * not rely on the OA buffer anymore to normalize the counter * values. */ So gen12_test_mi_rpc doesn't set DRM_I915_PERF_PROP_SAMPLE_OA and also seems to be passing in CI (don't see it but there seem to be no open bugs). Thoughts? Thanks. -- Ashutosh
