Re: [PATCH] tools/xe-perf-recorder: expose some OA stream open properties

Couple of things to note:

On 7/28/2026 12:06, Shekhar Chauhan wrote:
> xe-perf-recorder only passes 5 of the OA stream open properties to
> DRM_XE_OBSERVATION_OP_STREAM_OPEN, leaving the rest without an open
> source consumer. Add few of the remaining ones.
1. I missed out on adding "PATCH i-g-t" prefix, I did send it with a 
subject-prefix="PATCH i-g-t" hoping it'll append that, but turns out, it 
didn't, so I think I should explicitly do that while doing a git 
format-patch. I remember being told to add it in the gitconfig, but, I 
didn't wanna do it for all the patches that I send, so I didn't add there.
>
> Signed-off-by: Shekhar Chauhan <[email protected]>
> ---
>   tools/xe-perf/xe_perf_recorder.c | 105 ++++++++++++++++++++++++++++---
>   1 file changed, 98 insertions(+), 7 deletions(-)
>
> diff --git a/tools/xe-perf/xe_perf_recorder.c b/tools/xe-perf/xe_perf_recorder.c
> index c69050b43..06d1016a0 100644
> --- a/tools/xe-perf/xe_perf_recorder.c
> +++ b/tools/xe-perf/xe_perf_recorder.c
> @@ -26,6 +26,7 @@
>   #include <unistd.h>
>   
>   #include "igt_core.h"
> +#include "igt_sizes.h"
>   #include "intel_batchbuffer.h"
>   #include "intel_chipset.h"
>   #include "ioctl_wrappers.h"
> @@ -364,6 +365,16 @@ struct recording_context {
>   	struct drm_xe_oa_unit *oa_unit;
>   	struct drm_xe_engine_class_instance *hwe;
>   
> +	/* Optional OA stream open properties */
> +	bool oa_disabled;
> +	bool no_preempt;
> +	bool exec_queue_id_set;
> +	uint32_t exec_queue_id;
> +	bool oa_engine_instance_set;
> +	uint32_t oa_engine_instance;
> +	uint64_t oa_buffer_size;
> +	uint64_t wait_num_reports;
> +
>   	uint32_t vm;
>   	uint32_t exec_queue;
>   	struct intel_bb *ibb;
> @@ -477,7 +488,7 @@ perf_open(struct recording_context *ctx)
>   {
>   	int stream_fd;
>   
> -	uint64_t properties[] = {
> +	uint64_t properties[32] = {
>   		DRM_XE_OA_PROPERTY_OA_UNIT_ID, ctx->oa_unit->oa_unit_id,
>   
>   		/* Include OA reports in samples */
> @@ -488,10 +499,39 @@ perf_open(struct recording_context *ctx)
>   		DRM_XE_OA_PROPERTY_OA_FORMAT, __ff(ctx->metric_set->perf_oa_format),
>   		DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, ctx->oa_exponent,
>   	};
> -	struct intel_xe_oa_open_prop param = {
> -		.num_properties = ARRAY_SIZE(properties) / 2,
> -		.properties_ptr = to_user_pointer(properties),
> -	};
> +	struct intel_xe_oa_open_prop param = { 0 };
> +	uint32_t n = 10; /* 5 mandatory properties (10 u64 slots) set above */
> +
> +	/* Optional properties, only passed when given on the command line */
> +	if (ctx->oa_disabled) {
> +		properties[n++] = DRM_XE_OA_PROPERTY_OA_DISABLED;
> +		properties[n++] = 1;
> +	}
> +	if (ctx->exec_queue_id_set) {
> +		properties[n++] = DRM_XE_OA_PROPERTY_EXEC_QUEUE_ID;
> +		properties[n++] = ctx->exec_queue_id;
> +	}
> +	if (ctx->oa_engine_instance_set) {
> +		properties[n++] = DRM_XE_OA_PROPERTY_OA_ENGINE_INSTANCE;
> +		properties[n++] = ctx->oa_engine_instance;
> +	}
> +	if (ctx->no_preempt) {
> +		properties[n++] = DRM_XE_OA_PROPERTY_NO_PREEMPT;
> +		properties[n++] = 1;
> +	}
> +	if (ctx->oa_buffer_size) {
> +		properties[n++] = DRM_XE_OA_PROPERTY_OA_BUFFER_SIZE;
> +		properties[n++] = ctx->oa_buffer_size;
> +	}
> +	if (ctx->wait_num_reports) {
> +		properties[n++] = DRM_XE_OA_PROPERTY_WAIT_NUM_REPORTS;
> +		properties[n++] = ctx->wait_num_reports;
> +	}
> +
> +	igt_assert(n <= ARRAY_SIZE(properties));
> +
> +	param.num_properties = n / 2;
> +	param.properties_ptr = to_user_pointer(properties);
>   
>   	stream_fd = intel_xe_perf_ioctl(ctx->drm_fd, DRM_XE_OBSERVATION_OP_STREAM_OPEN, &param);
>   	if (stream_fd < 0) {
> @@ -924,7 +964,14 @@ usage(const char *name)
>   		"     --output,             -o <path>   Output file (default = xe_perf.record)\n"
>   		"     --cpu-clock,          -k <path>   Cpu clock to use for correlations\n"
>   		"                                       Values: boot, mono, mono_raw (default = mono)\n"
> -		"     --oa-unit-id          -u <value>  OA unit id for the capture.\n",
> +		"     --oa-unit-id          -u <value>  OA unit id for the capture.\n"
> +		"     --oa-disabled         -D          Open the OA stream in the disabled state\n"
> +		"                                       (the stream is enabled right after open)\n"
> +		"     --exec-queue-id       -q <value>  Exec queue id for context scoped capture\n"
> +		"     --oa-engine-instance  -e <value>  Engine instance for context scoped capture\n"
> +		"     --no-preempt          -n          Disable preemption (requires --exec-queue-id)\n"
> +		"     --oa-buffer-size      -b <value>  OA buffer size in bytes (pow2, 128K - 128M)\n"
> +		"     --wait-num-reports    -w <value>  Min reports before poll()/read() unblocks\n",
>   		name);
>   }
>   
> @@ -1038,6 +1085,12 @@ main(int argc, char *argv[])
>   		{"command-fifo",	required_argument, 0, 'f'},
>   		{"cpu-clock",		required_argument, 0, 'k'},
>   		{"oa-unit-id",		required_argument, 0, 'u'},
> +		{"oa-disabled",		no_argument,	   0, 'D'},
> +		{"exec-queue-id",	required_argument, 0, 'q'},
> +		{"oa-engine-instance",	required_argument, 0, 'e'},
> +		{"no-preempt",		no_argument,	   0, 'n'},
> +		{"oa-buffer-size",	required_argument, 0, 'b'},
> +		{"wait-num-reports",	required_argument, 0, 'w'},
>   		{0, 0, 0, 0}
>   	};
>   	const struct {
> @@ -1068,7 +1121,7 @@ main(int argc, char *argv[])
>   		.oa_unit_id = 0,
>   	};
>   
> -	while ((opt = getopt_long(argc, argv, "hc:d:p:m:Co:s:f:k:P:u:", long_options, NULL)) != -1) {
> +	while ((opt = getopt_long(argc, argv, "hc:d:p:m:Co:s:f:k:P:u:Dq:e:nb:w:", long_options, NULL)) != -1) {
>   		switch (opt) {
>   		case 'h':
>   			usage(argv[0]);
> @@ -1119,6 +1172,26 @@ main(int argc, char *argv[])
>   		case 'u':
>   			ctx.oa_unit_id = atoi(optarg);
>   			break;
> +		case 'D':
> +			ctx.oa_disabled = true;
> +			break;
> +		case 'q':
> +			ctx.exec_queue_id = strtoul(optarg, NULL, 0);
> +			ctx.exec_queue_id_set = true;
> +			break;
> +		case 'e':
> +			ctx.oa_engine_instance = strtoul(optarg, NULL, 0);
> +			ctx.oa_engine_instance_set = true;
> +			break;
> +		case 'n':
> +			ctx.no_preempt = true;
> +			break;
> +		case 'b':
> +			ctx.oa_buffer_size = strtoull(optarg, NULL, 0);
> +			break;
> +		case 'w':
> +			ctx.wait_num_reports = strtoull(optarg, NULL, 0);
> +			break;
>   		default:
>   			fprintf(stderr, "Internal error: "
>   				"unexpected getopt value: %d\n", opt);
> @@ -1127,6 +1200,17 @@ main(int argc, char *argv[])
>   		}
>   	}
>   
> +	if (ctx.no_preempt && !ctx.exec_queue_id_set) {
> +		fprintf(stderr, "--no-preempt requires --exec-queue-id\n");
> +		return EXIT_FAILURE;
> +	}
> +	if (ctx.oa_buffer_size &&
> +	    ((ctx.oa_buffer_size & (ctx.oa_buffer_size - 1)) ||
> +	     ctx.oa_buffer_size < SZ_128K || ctx.oa_buffer_size > SZ_128M)) {
> +		fprintf(stderr, "--oa-buffer-size must be a power of 2 in [128K, 128M]\n");
> +		return EXIT_FAILURE;
> +	}
> +
>   	if (dev_node_id == -2) {
>   		print_intel_devices();
>   		return EXIT_SUCCESS;
> @@ -1291,6 +1375,13 @@ main(int argc, char *argv[])
>   		goto fail;
>   	}
>   
> +	if (ctx.oa_disabled &&
> +	    perf_ioctl(ctx.perf_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0) < 0) {
> +		fprintf(stderr, "Unable to enable xe oa stream: %s\n",
> +			strerror(errno));
> +		goto fail;
> +	}
> +

The above 3 ctx.no_preempt, ctx.oa_buffer_size and ctx.oa_disabled 
blocks were suggested by AI to improve error handling.

-shekhar

>   	init_mmio_trigger_ctx(&ctx);
>   	emit_oa_trigger(&ctx, 0xc0ffee01);
>   

-- 
Shekhar Chauhan
Linux Graphics Software Engineer
Intel Corporation