Re: [PATCH i-g-t 1/2] tests/intel/xe_oa: Fetch OA buffer size from observation IOCTL

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

On Sun, 17 Nov 2024 22:32:22 -0800, Sai Teja Pottumuttu wrote:
>

This one mostly looks good, just a couple of nits below.

Also how are you testing these patches? To test I suggest just run:

	sudo ./build/tests/xe_oa

> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index 33e978a48..4e4d0fabc 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -63,9 +63,6 @@
>  #define PIPE_CONTROL_PPGTT_WRITE	(0 << 2)
>  #define PIPE_CONTROL_GLOBAL_GTT_WRITE   (1 << 2)
>
> -#define MAX_OA_BUF_SIZE (16 * 1024 * 1024)
> -#define OA_BUFFER_SIZE MAX_OA_BUF_SIZE
> -
>  #define RING_FORCE_TO_NONPRIV_ADDRESS_MASK 0x03fffffc
>  /*
>   * Engine specific registers defined as offsets from engine->mmio_base. For
> @@ -303,6 +300,7 @@ struct drm_xe_engine_class_instance default_hwe;
>
>  static struct intel_xe_perf *intel_xe_perf;
>  static uint64_t oa_exp_1_millisec;
> +static uint64_t oa_buffer_size;

s/size_t/uint64_t/

Also, for clarity, let's call this default_oa_buffer_size too. Because
oa_buffer_size later can be changed per stream. So that later when we
introduce different oa buffer sizes etc there won't be any confusion.

>  struct intel_mmio_data mmio_data;
>  static igt_render_copyfunc_t render_copy;
>
> @@ -511,6 +509,30 @@ __perf_open(int fd, struct intel_xe_oa_open_prop *param, bool prevent_pm)
>	return ret;
>  }
>
> +static u64 get_default_oa_buffer_size(int fd)

Let's make the return type size_t here too.

Rest lgtm. I'll take a quick look at the next version and should be able to
R-b it.

Thanks.
--
Ashutosh