Re: [Intel-gfx] [PATCH 3/3] drm/i915/perf: Initialize gen12 OA buffer unconditionally

Dixit, Ashutosh
2023-09-09T01:24:21Z

On Fri, 08 Sep 2023 18:16:26 -0700, Ashutosh Dixit wrote:
>

Hi Umesh,

> From: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>
> 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@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
> 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