Re: [igt-dev] [PATCH i-g-t] tests/perf: Add test to verify non-power-of-2 OA reports

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

On 14/09/2019 01:57, Umesh Nerlige Ramappa wrote:
> Add test to verify perf report size that is not a power of 2. Verify
> that the report that spans the boundary of the OA buffer is read
> correctly.
>
> v2: (Lionel)
> - Call sanity_check_reports to verify report contents.
> - Move format definition to the end of the enum to avoid breaking API
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>   include/drm-uapi/i915_drm.h |   1 +
>   tests/perf.c                | 115 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 116 insertions(+)
>
> diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
> index 761517f1..befede6f 100644
> --- a/include/drm-uapi/i915_drm.h
> +++ b/include/drm-uapi/i915_drm.h
> @@ -1834,6 +1834,7 @@ enum drm_i915_oa_format {
>   	I915_OA_FORMAT_A12,
>   	I915_OA_FORMAT_A12_B8_C8,
>   	I915_OA_FORMAT_A32u40_A4u32_B8_C8,
> +	I915_OA_FORMAT_A29_B8_C8,
>   
>   	I915_OA_FORMAT_MAX	    /* non-ABI */
>   };
> diff --git a/tests/perf.c b/tests/perf.c
> index 5ad8b2db..00c81b6d 100644
> --- a/tests/perf.c
> +++ b/tests/perf.c
> @@ -136,6 +136,11 @@ static struct oa_format hsw_oa_formats[I915_OA_FORMAT_MAX] = {
>   		"C4_B8", .size = 64,
>   		.c_off = 16, .n_c = 4,
>   		.b_off = 28, .n_b = 8 },
> +	[I915_OA_FORMAT_A29_B8_C8] = { /* HSW only */
> +		"A29_B8_C8", .size = 192,
> +		.a_off = 12,  .n_a = 29,
> +		.b_off = 128, .n_b = 8,
> +		.c_off = 160, .n_c = 8, },
>   };
>   
>   static struct oa_format gen8_oa_formats[I915_OA_FORMAT_MAX] = {
> @@ -1191,6 +1196,70 @@ test_missing_sample_flags(void)
>   	do_ioctl_err(drm_fd, DRM_IOCTL_I915_PERF_OPEN, &param, EINVAL);
>   }
>   
> +static void
> +read_oa_reports(int format_id,
> +		int exponent,
> +		int max_reports)
> +{
> +	size_t format_size = get_oa_format(format_id).size;
> +	size_t sample_size = (sizeof(struct drm_i915_perf_record_header) +
> +			      format_size);
> +	const struct drm_i915_perf_record_header *header;
> +	uint32_t exponent_mask = (1 << (exponent + 1)) - 1;
> +
> +	/* Allocate twice max buffer size to verify odd report size support and
> +	 * wrap around
> +	 */
> +	int buf_size = sample_size * max_reports * 1.5;
> +	uint8_t *buf = malloc(buf_size);
> +	uint8_t *rbuf = buf;
> +	int num_reports = 0;
> +	uint32_t *oa_report0;
> +	ssize_t len = 0;
> +
> +	while (num_reports < max_reports) {
> +		buf += len;
> +		while ((len = read(stream_fd, buf, buf_size)) < 0 &&
> +		       errno == EINTR)
> +			;
> +
> +		igt_assert(len > 0);
> +		igt_debug("read %d bytes\n", (int)len);
> +
> +		for (size_t offset = 0; (offset < len) && (num_reports < max_reports); offset += header->size) {
> +			uint32_t *oa_report1;
> +
> +			header = (void *)(buf + offset);
> +			igt_debug("header = %p\n", header);
> +
> +			igt_assert_eq(header->pad, 0);
> +			igt_assert_neq(header->type, DRM_I915_PERF_RECORD_OA_BUFFER_LOST);
> +			if (header->type == DRM_I915_PERF_RECORD_OA_REPORT_LOST) {
> +				igt_debug("read restart: OA trigger collision / report lost\n");
> +				num_reports = 0;
> +				break;
> +			}
> +
> +			igt_assert_eq(header->type, DRM_I915_PERF_RECORD_SAMPLE);
> +			igt_assert_eq(header->size, sample_size);
> +			oa_report1 = (void *)(header + 1);
> +			igt_debug("read report: reason = %x, timestamp = %x, exponent mask=%x\n",
> +				  oa_report1[0], oa_report1[1], exponent_mask);
> +			igt_assert_neq(oa_report1[1], 0);
> +
> +			num_reports++;
> +			if (num_reports >= 2) {
> +				sanity_check_reports(oa_report0, oa_report1,
> +						     format_id);
> +			}
> +			oa_report0 = oa_report1;
> +		}
> +	}
> +
> +	igt_debug("num_reports %d\n", num_reports);
> +	free(rbuf);
> +}
> +
>   static void
>   read_2_oa_reports(int format_id,
>   		  int exponent,
> @@ -1304,6 +1373,34 @@ read_2_oa_reports(int format_id,
>   	igt_assert(!"reached");
>   }
>   
> +static void
> +open_and_read_oa_reports(int format_id,
> +			 int exponent,
> +			 int num_reports)
> +{
> +	uint64_t properties[] = {
> +		/* Include OA reports in samples */
> +		DRM_I915_PERF_PROP_SAMPLE_OA, true,
> +
> +		/* OA unit configuration */
> +		DRM_I915_PERF_PROP_OA_METRICS_SET, test_metric_set_id,
> +		DRM_I915_PERF_PROP_OA_FORMAT, format_id,
> +		DRM_I915_PERF_PROP_OA_EXPONENT, exponent,
> +
> +	};
> +	struct drm_i915_perf_open_param param = {
> +		.flags = I915_PERF_FLAG_FD_CLOEXEC,
> +		.num_properties = sizeof(properties) / 16,
> +		.properties_ptr = to_user_pointer(properties),
> +	};
> +
> +	stream_fd = __perf_open(drm_fd, &param, false);
> +
> +	read_oa_reports(format_id, exponent, num_reports);
> +
> +	__perf_close(stream_fd);
> +}
> +
>   static void
>   open_and_read_2_oa_reports(int format_id,
>   			   int exponent,
> @@ -1493,6 +1590,19 @@ print_report(uint32_t *report, int fmt)
>   }
>   #endif
>   
> +static void
> +test_non_power_of_2_oa_formats(void)
> +{
> +	struct oa_format format = get_oa_format(I915_OA_FORMAT_A29_B8_C8);
> +	int num_reports = ((MAX_OA_BUF_SIZE / format.size) << 1) + 100;
> +
> +	igt_assert(format.name);
> +	igt_debug("Reading %d reports for OA format %s\n", num_reports, format.name);
> +	open_and_read_oa_reports(I915_OA_FORMAT_A29_B8_C8,
> +				 oa_exp_1_millisec,
> +				 num_reports);
> +}
> +
>   static void
>   test_oa_formats(void)
>   {
> @@ -4124,6 +4234,11 @@ igt_main
>   	igt_subtest("oa-formats")
>   		test_oa_formats();
>   
> +	igt_subtest("non-power-of-2-oa-formats") {
> +		igt_require(IS_HASWELL(devid));
> +		test_non_power_of_2_oa_formats();


Like I mentioned on the kernel series, the kernel change associated with 
this test should be versioned and this test only be run if the perf 
version is appropriate.

That way it won't fail on older kernels.


-Lionel


> +	}
> +
>   	igt_subtest("invalid-oa-exponent")
>   		test_invalid_oa_exponent();
>   	igt_subtest("low-oa-exponent-permissions")


_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev