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

Hi Umesh,

> From: Umesh Nerlige Ramappa <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Ashutosh Dixit <[email protected]>
> ---
>  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

Reply via email to