[PATCH i-g-t v3 1/2] tests/intel/xe_oa: Clean up buffer-fill test

[Umesh Nerlige Ramappa]

On Thu, Aug 28, 2025 at 03:56:28PM -0700, Dixit, Ashutosh wrote:
>On Thu, 28 Aug 2025 14:49:06 -0700, Umesh Nerlige Ramappa wrote:
>>
>> Checking for overflow status before enabling the OA buffer is always
>> going to return false. Drop the logic.
>>
>> Move out core buffer overflow check to a helper for reuse by other tests
>> when needed.
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
>> ---
>>  tests/intel/xe_oa.c | 36 ++++++++++++++++++++----------------
>>  1 file changed, 20 insertions(+), 16 deletions(-)
>>
>> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
>> index 1826623ab864..3b3cdf9314c2 100644
>> --- a/tests/intel/xe_oa.c
>> +++ b/tests/intel/xe_oa.c
>> @@ -2488,6 +2488,25 @@ test_oa_tlb_invalidate(const struct drm_xe_engine_class_instance *hwe)
>>	igt_assert(num_reports2 > 0.95 * num_expected_reports);
>>  }
>>
>> +/*
>> + * This is a blocking read trying to read 0 bytes, so it will return ENOSPC
>> + * until there is data available. Once the OA buffer overflows, we should see an
>
>What does it mean "until there is data available"?

I mean - if there is no data available, the blocking read will not 
return. We get ENOSPC only when data is available and the buffer 
provided is not enough. Probably will drop the comment.

>Won't reading 0 bytes
>always return ENOSPC? Or you mean until it returns EIO instead of ENOSPC?
>
>> + * EIO with the oevrflow status set.
>
>s/oevrflow/overflow/
>
>> + */
>> +static void
>> +wait_for_oa_buffer_overflow(int fd, int poll_period_us)
>> +{
>> +	char buf;
>> +
>> +	while (-1 == read(fd, &buf, 0)) {
>> +		if (errno == EIO &&
>> +		    get_stream_status(fd) & DRM_XE_OASTATUS_BUFFER_OVERFLOW)
>> +			return;
>> +
>> +		usleep(poll_period_us);
>> +	}
>> +}
>> +
>>  /**
>>   * SUBTEST: buffer-fill
>>   * Description: Test filling and overflow of OA buffer
>> @@ -2526,24 +2545,9 @@ test_buffer_fill(const struct drm_xe_engine_class_instance *hwe)
>>	stream_fd = __perf_open(drm_fd, &param, true /* prevent_pm */);
>>          set_fd_flags(stream_fd, O_CLOEXEC);
>>
>> -	/* OA buffer is disabled, we do not expect any error status */
>> -	oa_status = get_stream_status(stream_fd);
>> -	overflow_seen = oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW;
>> -	igt_assert_eq(overflow_seen, 0);
>> -
>>	do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0);
>>
>> -	/* Read 0 bytes repeatedly until you see an EIO */
>> -	while (-1 == read(stream_fd, buf, 0)) {
>> -		if (errno == EIO) {
>> -			oa_status = get_stream_status(stream_fd);
>> -			overflow_seen = oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW;
>> -			if (overflow_seen)
>> -				break;
>> -		}
>> -		usleep(100);
>> -	}
>> -	igt_assert(overflow_seen);
>> +	wait_for_oa_buffer_overflow(stream_fd, 100);
>>
>>	/* Make sure the buffer overflow is cleared */
>>	read(stream_fd, buf, 0);
>> --
>> 2.43.0
>>
>
>Also the ftrace patch is not merged so maybe add that to the series too.

I thought I will merge it already since it has your R-b.

These are the only 2 patches remaining.

>
>Otherwise this is:
>
>Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>

Thanks,
Umesh