Re: [PATCH i-g-t v2] tools/xe-perf-recorder: Wait for mmio-trigger-report

[Shekhar Chauhan <shekhar.chauhan@intel.com>]

On 9/5/2026 6:50, Ashutosh Dixit wrote:
> After mmio trigger is issued, we should wait till we receive a report with
> the context id issued in the mmio trigger.
>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>   tools/xe-perf/xe_perf_recorder.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/tools/xe-perf/xe_perf_recorder.c b/tools/xe-perf/xe_perf_recorder.c
> index e13eef5190..cb7ca67bdb 100644
> --- a/tools/xe-perf/xe_perf_recorder.c
> +++ b/tools/xe-perf/xe_perf_recorder.c
> @@ -370,6 +370,7 @@ struct recording_context {
>   	uint32_t vm;
>   	uint32_t exec_queue;
>   	struct intel_bb *ibb;
> +	uint32_t mmio_trg_val;
>   };
>   
>   static uint32_t oa_unit_mmio_trigger_reg(struct recording_context *ctx)
> @@ -642,13 +643,15 @@ check_mmio_trigger_report(struct recording_context *ctx, const void *report)
>   	const uint32_t *report_32 = report;
>   	uint64_t value;
>   
> -	if (report_reason(report_32))
> +	if (!ctx->mmio_trg_val || report_reason(report_32))
>   		return;
>   
>   	value = (fmt->header == HDR_64_BIT) ? ((const uint64_t *)report)[2] : report_32[2];
>   
> -	if (value == 0xc0ffee01 || value == 0xc0ffee02)
> +	if (value == ctx->mmio_trg_val) {
>   		fprintf(stdout, "Received trigger report with value 0x%" PRIx64 "\n", value);
> +		ctx->mmio_trg_val = 0;
> +	}
>   }
>   
>   static bool write_stream_data(struct recording_context *ctx,
> @@ -712,6 +715,18 @@ static bool write_perf_data(FILE *output, struct recording_context *ctx)
>   	return false;
>   }
>   
> +static void write_mmio_trigger_data(struct recording_context *ctx)
> +{
> +	struct pollfd pollfd = { .fd = ctx->perf_fd, .events = POLLIN };
> +
> +	while (ctx->mmio_trg_val) {
> +		poll(&pollfd, 1, -1);
> +		assert(pollfd.revents & POLLIN);
> +
> +		write_perf_data(ctx->output_stream, ctx);
> +	}
> +}

On a write failure, write_stream_data() bails out in the middle, and 
some reports are dropped without being checked for the trigger value.  
If the trigger is one of them, this while loop spins forever.  I think 
we should use a return value (maybe a bool value) and bail out when 
write_perf_data() fails so the error reaches main() instead of looping 
forever.  Something like v1.

Rest looks fine to me.

-shekhar

> +
>   static clock_t correlation_clock_id = CLOCK_MONOTONIC;
>   
>   static const char *
> @@ -797,6 +812,9 @@ static void emit_oa_trigger(struct recording_context *ctx, uint32_t value)
>   	intel_bb_flush_render(ctx->ibb);
>   	intel_bb_sync(ctx->ibb);
>   	intel_bb_reset(ctx->ibb, false);
> +
> +	ctx->mmio_trg_val = value;
> +	write_mmio_trigger_data(ctx);
>   }
>   
>   static void
> @@ -815,7 +833,6 @@ read_command_file(struct recording_context *ctx)
>   		FILE *file;
>   
>   		emit_oa_trigger(ctx, 0xc0ffee02);
> -		write_perf_data(ctx->output_stream, ctx);
>   
>   		while (offset < len &&
>   		       ((ret = read(ctx->command_fifo_fd,

-- 
Shekhar Chauhan
Linux Graphics Software Engineer
Intel Corporation