Re: [PATCH v6 2/6] drm/xe/xe_ras: Add support to get error counter in CRI

[Raag Jadav <raag.jadav@intel.com>]

On Thu, May 14, 2026 at 10:52:08AM +0530, Riana Tauro wrote:
> Add request/response structures and helper functions to query system
> controller to get error counter value.

...

> +	xe_dbg(xe, "[RAS]: get counter value %u for %s %s\n", response.counter_value,
> +	       comp_to_str(response.counter.common.component),
> +	       sev_to_str(response.counter.common.severity));

A bit heavy handed with nesting :D
You can use a local counter which will also help remove the wrapping[1].

[1] https://lore.kernel.org/intel-xe/20260512191610.1817578-7-raag.jadav@intel.com/

...

> +/**
> + * struct xe_ras_get_counter_request - Request for get error counter
> + */
> +struct xe_ras_get_counter_request {
> +	/** @counter: Error counter to be queried */
> +	struct xe_ras_error_class counter;
> +	/** @reserved: Reserved for future use */
> +	u32 reserved;
> +} __packed;
> +
> +/**
> + * struct xe_ras_get_counter_response - Response for get error counter
> + */
> +struct xe_ras_get_counter_response {
> +	/** @counter: Error counter that was queried */
> +	struct xe_ras_error_class counter;
> +	/** @counter_value: Current counter value */
> +	u32 counter_value;

Nit: It's already 'get_counter', so perhaps just 'value'?

> +	/** @timestamp: Timestamp when counter was last updated */
> +	u64 timestamp;
> +	/** @threshold_value: Threshold value for the counter */
> +	u32 threshold_value;

Ditto, 'threshold'?

> +	/** @counter_status: Status of the counter */
> +	u32 counter_status:8;

'status'?

We should probably make these consistent across series', so let me know
which ones you prefer.

> +	/** @reserved: Reserved for future use */
> +	u32 reserved:24;
> +	/** @reserved1: Reserved for future use */
> +	u32 reserved1[56];
> +} __packed;
> +
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> index 3caa9f15875f..b7c4d8c37819 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> @@ -293,6 +293,34 @@ static int sysctrl_send_command(struct xe_sysctrl *sc,
>  	return 0;
>  }
>  
> +/**
> + * xe_sysctrl_prepare_command() - Prepare System controller command structure
> + * @command: Sysctrl command structure
> + * @group: Command group ID
> + * @cmd_id: Command code
> + * @request: Pointer to request buffer (can be NULL)
> + * @request_len: Size of request buffer
> + * @response: Pointer to response buffer
> + * @response_len: Size of response buffer
> + *
> + * Helper function to prepare sysctrl command to be sent via xe_sysctrl_send_command()
> + */
> +void xe_sysctrl_prepare_command(struct xe_sysctrl_mailbox_command *command, u8 group, u8 cmd_id,

There's already a sysctrl_prepare_command() in place, so let's make this
a bit distinguishable.

PS: I've used xe_sysctrl_populate_command() in my series but I'll leave
it to you.

Raag

> +				void *request, size_t request_len, void *response,
> +				size_t response_len)
> +{
> +	struct xe_sysctrl_app_msg_hdr header = {0};
> +
> +	header.data = FIELD_PREP(APP_HDR_GROUP_ID_MASK, group) |
> +		      FIELD_PREP(APP_HDR_COMMAND_MASK, cmd_id);
> +
> +	command->header = header;
> +	command->data_in = request;
> +	command->data_in_len = request_len;
> +	command->data_out = response;
> +	command->data_out_len = response_len;
> +}