Re: [PATCH i-g-t 1/1] tests/intel/xe_oa: Add test for different OA buffer sizes

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

On Wed, 04 Dec 2024 08:16:06 -0800, Sai Teja Pottumuttu wrote:
>

Hi Sai Teja,

> Add a new test oa-buffer-size to test different OA buffer sizes, its
> allocation and overflow scenarios. The test uses the new property
> DRM_XE_OA_PROPERTY_OA_BUFFER_SIZE to be able to vary the OA buffer
> sizes.
>
> Signed-off-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>
> ---
>  include/drm-uapi/xe_drm.h |  8 +++++
>  tests/intel/xe_oa.c       | 71 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+)
>
> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> index 56163eb91..6d3ea045f 100644
> --- a/include/drm-uapi/xe_drm.h
> +++ b/include/drm-uapi/xe_drm.h
> @@ -1486,6 +1486,7 @@ struct drm_xe_oa_unit {
>	__u64 capabilities;
>  #define DRM_XE_OA_CAPS_BASE		(1 << 0)
>  #define DRM_XE_OA_CAPS_SYNCS		(1 << 1)
> +#define DRM_XE_OA_CAPS_OA_BUFFER_SIZE	(1 << 2)
>
>	/** @oa_timestamp_freq: OA timestamp freq */
>	__u64 oa_timestamp_freq;
> @@ -1651,6 +1652,13 @@ enum drm_xe_oa_property_id {
>	 * to the VM bind case.
>	 */
>	DRM_XE_OA_PROPERTY_SYNCS,
> +
> +	/**
> +	 * @DRM_XE_OA_PROPERTY_OA_BUFFER_SIZE: Specifies the size of OA buffer
> +	 * allocated by the driver in bytes. The default size would be 16MB and the
> +	 * supported sizes are powers of 2 from 128KB to 128MB.
> +	 */
> +	DRM_XE_OA_PROPERTY_OA_BUFFER_SIZE,

OK for now but finally, when we merge this, this uapi change will need to
come in via a separate commit which syncs IGT uapi headers with kernel
headers. You can see previous IGT commits for this file. We can easily take
care of this after the kernel patch is merged. And then we can merge the
remainder of this patch.

>  };
>
>  /**
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index 40f5d3607..abe0d2cc3 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -2504,6 +2504,60 @@ again_1:
>	__perf_close(stream_fd);
>  }
>
> +/**
> + * SUBTEST: oa-buffer-size
> + * Description: Test OA buffer allocation and overflow with different OA buffer sizes.
> + */
> +static void test_oa_buffer_size(size_t oa_buffer_size)
> +{
> +	int oa_exponent = max_oa_exponent_for_period_lte(20000);
> +	uint64_t properties[] = {
> +		DRM_XE_OA_PROPERTY_OA_UNIT_ID, 0,
> +
> +		/* Include OA reports in samples */
> +		DRM_XE_OA_PROPERTY_SAMPLE_OA, true,
> +
> +		/* OA unit configuration */
> +		DRM_XE_OA_PROPERTY_OA_METRIC_SET, default_test_set->perf_oa_metrics_set,
> +		DRM_XE_OA_PROPERTY_OA_FORMAT, __ff(default_test_set->perf_oa_format),
> +		DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, oa_exponent,
> +
> +		DRM_XE_OA_PROPERTY_OA_BUFFER_SIZE, oa_buffer_size
> +	};
> +	struct intel_xe_oa_open_prop param = {
> +		.num_properties = ARRAY_SIZE(properties) / 2,
> +		.properties_ptr = to_user_pointer(properties),
> +	};
> +	uint32_t buf_size = 2 * oa_buffer_size;
> +	uint8_t *buf = malloc(buf_size);
> +	struct drm_xe_oa_stream_info info;
> +	uint32_t total_len = 0;
> +	int len;
> +	u32 oa_status;
> +
> +	igt_assert(buf);
> +
> +	stream_fd = __perf_open(drm_fd, &param, true);
> +
> +	do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_INFO, &info);
> +	igt_assert_eq(oa_buffer_size, info.oa_buf_size);
> +
> +	while (total_len < buf_size &&
> +	       ((len = read(stream_fd, &buf[total_len], buf_size - total_len)) > 0 ||
> +		(len == -1 && (errno == EINTR || errno == EIO)))) {
> +		if (errno == EIO) {
> +			oa_status = get_stream_status(stream_fd);
> +			igt_debug("oa_status %#x\n", oa_status);
> +			igt_assert(!(oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW));
> +		}
> +		if (len > 0)
> +			total_len += len;
> +	}
> +
> +	 __perf_close(stream_fd);
> +	free(buf);

The problem I am seeing here is we are not really checking for
anything. How about adding buffer size to the non-zero-reason subtest and
try to reuse that?

Also,
	$ sudo ./build/tests/xe_oa --r oa-buffer-size
	IGT-Version: 1.29-g724fd0b79 (x86_64) (Linux: 6.12.0-rc4+ x86_64)
	Using IGT_SRANDOM=1733459091 for randomisation
	Opened device: /dev/dri/card0
	Starting subtest: oa-buffer-size
	Starting dynamic subtest: 8MB
	Dynamic subtest 8MB: SUCCESS (0.403s)
	Starting dynamic subtest: 32MB
	Dynamic subtest 32MB: SUCCESS (1.595s)
	Starting dynamic subtest: 128MB
	Dynamic subtest 128MB: SUCCESS (6.326s)
	Subtest oa-buffer-size: SUCCESS (8.324s)

So maybe 128 MB takes too long to run, maybe we could do 8, 16 and 32 MB?
Or even just two.

So if we are able to reuse non-zero-reason subtest without issues it would
become two or three subtests:

	non-zero-reason-8MB
	non-zero-reason-16MB
	non-zero-reason-32MB

Or similar, say using dynamic subtests.

We'll also need to time these and see how long they take to run. We can
even look at 64 and 128 MB times and then decide.

Thanks.
--
Ashutosh