[PATCH i-g-t CI run 07/14] tests/intel/xe_oa: Simplify the buffer-fill test
Dixit, Ashutosh
ashutosh.dixit at intel.com
Mon Feb 24 21:31:01 UTC 2025
On Tue, 18 Feb 2025 12:28:05 -0800, Umesh Nerlige Ramappa wrote:
>
> We only want to test that the BUFFER OVERFLOW status is set when we do
> not read the OA stream data in time. To do so, keeping reading zero
> bytes of data until you hit a buffer overflow.
The approach in this patch is great!
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
> ---
> tests/intel/xe_oa.c | 135 +++++++++-----------------------------------
> 1 file changed, 26 insertions(+), 109 deletions(-)
>
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index 5792ffec2..fc03b74af 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -304,6 +304,7 @@ static size_t default_oa_buffer_size;
> static struct intel_mmio_data mmio_data;
> static igt_render_copyfunc_t render_copy;
> static uint32_t rc_width, rc_height;
> +static uint32_t buffer_fill_size;
I am assuming these globals will be changed for slow platforms?
>
> static struct intel_xe_perf_metric_set *metric_set(const struct drm_xe_engine_class_instance *hwe)
> {
> @@ -1092,6 +1093,7 @@ init_sys_info(void)
>
> rc_width = 1920;
> rc_height = 1080;
> + buffer_fill_size = SZ_16M;
> oa_exponent_default = max_oa_exponent_for_period_lte(1000000);
>
> default_oa_buffer_size = get_default_oa_buffer_size(drm_fd);
> @@ -2370,11 +2372,7 @@ test_oa_tlb_invalidate(const struct drm_xe_engine_class_instance *hwe)
> static void
> test_buffer_fill(const struct drm_xe_engine_class_instance *hwe)
There is this comment above this line:
Description: Test filling, wraparound and overflow of OA buffer
I think we should drop "wraparound", since this test never tested the
wraparound of the OA buffer, it just tested overflow.
To test wraparound I was using non-zero-reason.
> {
> - /* ~5 micro second period */
> - int oa_exponent = max_oa_exponent_for_period_lte(5000);
> - uint64_t oa_period = oa_exponent_to_ns(oa_exponent);
> struct intel_xe_perf_metric_set *test_set = metric_set(hwe);
> - uint64_t fmt = test_set->perf_oa_format;
> uint64_t properties[] = {
> DRM_XE_OA_PROPERTY_OA_UNIT_ID, 0,
>
> @@ -2383,126 +2381,45 @@ test_buffer_fill(const struct drm_xe_engine_class_instance *hwe)
>
> /* OA unit configuration */
> DRM_XE_OA_PROPERTY_OA_METRIC_SET, test_set->perf_oa_metrics_set,
> - DRM_XE_OA_PROPERTY_OA_FORMAT, __ff(fmt),
> - DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, oa_exponent,
> + DRM_XE_OA_PROPERTY_OA_FORMAT, __ff(test_set->perf_oa_format),
> + DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, oa_exponent_default,
> DRM_XE_OA_PROPERTY_OA_ENGINE_INSTANCE, hwe->engine_instance,
> + DRM_XE_OA_PROPERTY_OA_DISABLED, true,
> + DRM_XE_OA_PROPERTY_OA_BUFFER_SIZE, buffer_fill_size,
> };
> struct intel_xe_oa_open_prop param = {
> .num_properties = ARRAY_SIZE(properties) / 2,
> .properties_ptr = to_user_pointer(properties),
> };
> - size_t report_size = get_oa_format(fmt).size;
> - int buf_size = 65536 * report_size;
> - uint8_t *buf = malloc(buf_size);
> - int len;
> - int n_full_oa_reports = default_oa_buffer_size / report_size;
> - uint64_t fill_duration = n_full_oa_reports * oa_period;
> - uint32_t *last_periodic_report = malloc(report_size);
> + uint64_t oa_period = oa_exponent_to_ns(oa_exponent_default);
> + char *buf = malloc(1024);
> + bool overflow_seen;
> u32 oa_status;
> + int len;
>
> - igt_assert(fill_duration < 1000000000);
> -
> + igt_debug("oa_period %s\n", pretty_print_oa_period(oa_period));
> stream_fd = __perf_open(drm_fd, ¶m, true /* prevent_pm */);
> set_fd_flags(stream_fd, O_CLOEXEC);
>
> - for (int i = 0; i < 5; i++) {
> - bool overflow_seen;
> - uint32_t n_periodic_reports;
> - uint32_t first_timestamp = 0, last_timestamp = 0;
> -
> - do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0);
> -
> - nanosleep(&(struct timespec){ .tv_sec = 0,
> - .tv_nsec = fill_duration * 1.25 },
> - NULL);
> -again:
> - oa_status = 0;
> - while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR)
> - ;
> -
> - if (errno == EIO) {
> - oa_status = get_stream_status(stream_fd);
> - igt_debug("oa_status %#x\n", oa_status);
> - overflow_seen = oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW;
> - igt_assert_eq(overflow_seen, true);
> - goto again;
> - }
> - igt_assert_neq(len, -1);
> -
> - do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_DISABLE, 0);
> -
> - igt_debug("fill_duration = %"PRIu64"ns, oa_exponent = %u\n",
> - fill_duration, oa_exponent);
> -
> - do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0);
> -
> - nanosleep(&(struct timespec){ .tv_sec = 0,
> - .tv_nsec = fill_duration / 2 },
> - NULL);
> -
> - n_periodic_reports = 0;
> -
> - /* Because of the race condition between notification of new
> - * reports and reports landing in memory, we need to rely on
> - * timestamps to figure whether we've read enough of them.
> - */
> - while (((last_timestamp - first_timestamp) * oa_period) < (fill_duration / 2)) {
> -
> - igt_debug("dts=%u elapsed=%"PRIu64" duration=%"PRIu64"\n",
> - last_timestamp - first_timestamp,
> - (last_timestamp - first_timestamp) * oa_period,
> - fill_duration / 2);
> -again_1:
> - oa_status = 0;
> - while ((len = read(stream_fd, buf, buf_size)) == -1 && errno == EINTR)
> - ;
> - 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));
> - goto again_1;
> - }
> - igt_assert_neq(len, -1);
> -
> - for (int offset = 0; offset < len; offset += report_size) {
> - uint32_t *report = (void *) (buf + offset);
> -
> - igt_debug(" > report ts=%"PRIu64""
> - " ts_delta_last_periodic=%"PRIu64" is_timer=%i ctx_id=%8x nb_periodic=%u\n",
> - oa_timestamp(report, fmt),
> - n_periodic_reports > 0 ? oa_timestamp_delta(report, last_periodic_report, fmt) : 0,
> - oa_report_is_periodic(report),
> - oa_report_get_ctx_id(report),
> - n_periodic_reports);
> -
> - if (first_timestamp == 0)
> - first_timestamp = oa_timestamp(report, fmt);
> - last_timestamp = oa_timestamp(report, fmt);
> -
> - if (oa_report_is_periodic(report)) {
> - memcpy(last_periodic_report, report, report_size);
> - n_periodic_reports++;
> - }
> - }
> - }
> -
> - do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_DISABLE, 0);
> -
> - igt_debug("first ts = %u, last ts = %u\n", first_timestamp, last_timestamp);
> + /* OA buffer is disable, we do not expect any error status */
disabled
> + oa_status = get_stream_status(stream_fd);
> + overflow_seen = !!(oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW);
Don't need !!
> + igt_assert_eq(overflow_seen, 0);
>
> - igt_debug("%f < %zu < %f\n",
> - report_size * n_full_oa_reports * 0.45,
> - n_periodic_reports * report_size,
> - report_size * n_full_oa_reports * 0.55);
> + do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0);
>
> - igt_assert(n_periodic_reports * report_size >
> - report_size * n_full_oa_reports * 0.45);
> - igt_assert(n_periodic_reports * report_size <
> - report_size * n_full_oa_reports * 0.55);
> + errno = 0;
> + /* Read 0 bytes repeatedly until you see an EIO */
> + while ((len = read(stream_fd, buf, 0)) == -1 && (errno == EINTR || errno == ENOSPC)) {
> + usleep(100);
> }
> + igt_assert_eq(len, -1);
> + igt_assert_eq(errno, EIO);
>
> - free(last_periodic_report);
> - free(buf);
> + /* Ensure buffer overflowed */
> + oa_status = get_stream_status(stream_fd);
> + overflow_seen = !!(oa_status & DRM_XE_OASTATUS_BUFFER_OVERFLOW);
Don't need !!
> + igt_assert(overflow_seen);
Also I am wondering if we disabled the stream now using
DRM_XE_OBSERVATION_IOCTL_DISABLE, would it clear the overflow status? But
anyway the test is good as is.
>
> __perf_close(stream_fd);
> }
Apart from the above nits, this is:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
More information about the igt-dev
mailing list