Re: [PATCH] drm/xe/oa: Call xe_oa_emit_oa_config() with new config when updating config

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 19 Jun 2024 12:28:54 -0700, José Roberto de Souza wrote:
>

Hi Jose,

> When UMD ask config to be updated, xe_oa_config_locked() was calling
> xe_oa_emit_oa_config() that would use stream->oa_config but that is
> only changed to the next oa_config after xe_oa_emit_oa_config() finish.
> So it was setting the same config for all DRM_XE_PERF_IOCTL_CONFIG
> calls.
>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_oa.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index 34206e0b6a08e..a2da54d49f1ce 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -887,9 +887,8 @@ __xe_oa_alloc_config_buffer(struct xe_oa_stream *stream, struct xe_oa_config *oa
>	return ERR_CAST(bb);
>  }
>
> -static struct xe_oa_config_bo *xe_oa_alloc_config_buffer(struct xe_oa_stream *stream)
> +static struct xe_oa_config_bo *xe_oa_alloc_config_buffer(struct xe_oa_stream *stream, struct xe_oa_config *oa_config)

Nice catch and thanks for the patch.

Just fix the reported checkpatch warning here ("line length of 117 exceeds
100 columns"). With that this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

I think no need for "Fixes" tag(s) because this will get into the same
kernel version. Thanks.


>  {
> -	struct xe_oa_config *oa_config = stream->oa_config;
>	struct xe_oa_config_bo *oa_bo;
>
>	/* Look for the buffer in the already allocated BOs attached to the stream */
> @@ -905,13 +904,13 @@ static struct xe_oa_config_bo *xe_oa_alloc_config_buffer(struct xe_oa_stream *st
>	return oa_bo;
>  }
>
> -static int xe_oa_emit_oa_config(struct xe_oa_stream *stream)
> +static int xe_oa_emit_oa_config(struct xe_oa_stream *stream, struct xe_oa_config *config)
>  {
>  #define NOA_PROGRAM_ADDITIONAL_DELAY_US 500
>	struct xe_oa_config_bo *oa_bo;
>	int err, us = NOA_PROGRAM_ADDITIONAL_DELAY_US;
>
> -	oa_bo = xe_oa_alloc_config_buffer(stream);
> +	oa_bo = xe_oa_alloc_config_buffer(stream, config);
>	if (IS_ERR(oa_bo)) {
>		err = PTR_ERR(oa_bo);
>		goto exit;
> @@ -989,7 +988,7 @@ static int xe_oa_enable_metric_set(struct xe_oa_stream *stream)
>			return ret;
>	}
>
> -	return xe_oa_emit_oa_config(stream);
> +	return xe_oa_emit_oa_config(stream, stream->oa_config);
>  }
>
>  static void xe_oa_stream_enable(struct xe_oa_stream *stream)
> @@ -1054,7 +1053,7 @@ static long xe_oa_config_locked(struct xe_oa_stream *stream, u64 arg)
>		return -ENODEV;
>
>	if (config != stream->oa_config) {
> -		err = xe_oa_emit_oa_config(stream);
> +		err = xe_oa_emit_oa_config(stream, config);
>		if (!err)
>			config = xchg(&stream->oa_config, config);
>		else
> --
> 2.45.2
>