Re: [PATCH 4/5] drm/xe/xe_ras: Add structures and commands for get counter

["Tauro, Riana" <riana.tauro@intel.com>]

On 4/1/2026 1:28 PM, Raag Jadav wrote:
> On Fri, Mar 20, 2026 at 03:55:59PM +0530, Riana Tauro wrote:
>> Add request and response structures for get counter command.
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>>   drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h |   9 ++
>>   drivers/gpu/drm/xe/xe_ras_types.h       | 151 ++++++++++++++++++++++++
>>   2 files changed, 160 insertions(+)
>>   create mode 100644 drivers/gpu/drm/xe/xe_ras_types.h
>>
>> diff --git a/drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h b/drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h
>> index 4cbde267ac44..fd4248b82f69 100644
>> --- a/drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h
>> +++ b/drivers/gpu/drm/xe/abi/xe_sysctrl_abi.h
>> @@ -40,6 +40,15 @@
>>    * between driver and System Controller firmware.
>>    */
>>   
>> +/**
>> + * enum xe_sysctrl_group - System Controller command groups
>> + *
>> + * @XE_SYSCTRL_GROUP_GFSP: GFSP group for RAS commands
>> + */
>> +enum xe_sysctrl_group {
>> +	XE_SYSCTRL_GROUP_GFSP = 0x01
> I thought we can add this to xe_sysctrl_mailbox.h but it seems I was
> mistaken.
My bad.  Will align with other patches
>> +};
>> +
>>   /**
>>    * struct xe_sysctrl_app_msg_hdr - Application layer message header
>>    * @data: 32-bit header data
>> diff --git a/drivers/gpu/drm/xe/xe_ras_types.h b/drivers/gpu/drm/xe/xe_ras_types.h
>> new file mode 100644
>> index 000000000000..b3256a28221e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_ras_types.h
>> @@ -0,0 +1,151 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2026 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_RAS_TYPES_H_
>> +#define _XE_RAS_TYPES_H_
>> +
>> +#include <linux/types.h>
>> +
>> +#define XE_RAS_INFO_QUEUE_MAX_CHUNK_SIZE	200
>> +
>> +/**
>> + * enum xe_ras_command - RAS Command IDs for GFSP group
>> + *
>> + * @XE_RAS_CMD_GET_COUNTER: Get RAS counter information
>> + */
>> +enum xe_ras_command {
>> +	XE_RAS_CMD_GET_COUNTER = 0x03
> We have commands like GET_PENDING_EVENT that are likely to be used for
> wider usecases, so not if this is a suitable place for it.
Sure will move it to sysctrl
>> +};
>> +
>> +/**
>> + * struct xe_ras_error_common - Common RAS error class
>> + *
>> + * This structure contains error severity and component information
>> + * across all products
>> + */
>> +struct xe_ras_error_common {
>> +	/** @severity: Error Severity */
>> +	u8 severity;
>> +	/** @component: IP where the error originated */
>> +	u8 component;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_error_unit - Error unit information
>> + */
>> +struct xe_ras_error_unit {
>> +	/** @tile: Tile identifier */
>> +	u8 tile;
>> +	/** @instance: Instance identifier within a component */
>> +	u32 instance;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_error_cause - Error cause information
>> + */
>> +struct xe_ras_error_cause {
>> +	/** @cause: Cause */
>> +	u32 cause;
>> +	/** @reserved: For future use */
>> +	u8 reserved;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_error_product - Error fields that are specific to the product
>> + */
>> +struct xe_ras_error_product {
>> +	/** @unit: Unit within IP block */
>> +	struct xe_ras_error_unit unit;
>> +	/** @error_cause: Cause/checker */
>> +	struct xe_ras_error_cause error_cause;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_error_class - Complete RAS Error Class
>> + *
>> + * This structure provides the complete error classification by combining
>> + * the common error class with the product-specific error class.
>> + */
>> +struct xe_ras_error_class {
>> +	/** @common: Common error severity and component */
>> +	struct xe_ras_error_common common;
>> +	/** @product: Product-specific unit and cause */
>> +	struct xe_ras_error_product product;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_info_queue_header - Info queue header
>> + *
>> + * This structure provides metadata about large info queue data
>> + */
>> +struct xe_ras_info_queue_header {
>> +	/** @total_size: Total size of complete info queue data (bytes) */
>> +	u32 total_size;
>> +	/** @chunk_offset: Offset of this chunk within total data (bytes) */
>> +	u32 chunk_offset;
>> +	/** @chunk_size: Size of data in this chunk (bytes) */
>> +	u32 chunk_size;
>> +	/** @sequence_number: Sequence number of this chunk (Starts at 0) */
>> +	u32 sequence_number;
>> +	/** @flags: Info queue control flags */
>> +	u32 flags:8;
> Why not u8?

flags, compression type and reserved are part of u32.

It's in-line with the firmware definition.

Also it is not recommended to mix the types if the bit fields belong to 
the same type.

Thanks
Riana

>> +	/** @compression_type: Compression type used for this chunk */
>> +	u32 compression_type:4;
> Ditto.
>
>> +	/** @reserved: Reserved for future use */
>> +	u32 reserved:20;
>> +	/** @checksum: Checksum of the chunk data */
>> +	u32 checksum;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_info_queue_response - Info queue response
>> + *
>> + * This structure provides the response for commands with info queue
>> + */
>> +struct xe_ras_info_queue_response {
>> +	/** @queue_header: Info queue metadata */
>> +	struct xe_ras_info_queue_header queue_header;
>> +	/** @queue_data: Info queue data chunk */
>> +	u8 queue_data[XE_RAS_INFO_QUEUE_MAX_CHUNK_SIZE];
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_get_counter_request - Request for XE_RAS_CMD_GET_COUNTER
>> + *
>> + * This structure defines the request format for getting RAS counter values
>> + */
>> +struct xe_ras_get_counter_request {
>> +	/** @counter_class: RAS error class */
>> +	struct xe_ras_error_class counter_class;
>> +	/** @reserved: Reserved for future use */
>> +	u32 reserved;
>> +} __packed;
>> +
>> +/**
>> + * struct xe_ras_get_counter_response - Response for XE_RAS_CMD_GET_COUNTER
>> + *
>> + * This structure defines the response format for getting RAS counter values
>> + */
>> +struct xe_ras_get_counter_response {
>> +	/** @counter_class: RAS error class */
>> +	struct xe_ras_error_class counter_class;
>> +	/** @counter_value: Current counter value */
>> +	u32 counter_value;
>> +	/** @timestamp: Timestamp of the counter value */
>> +	u64 timestamp;
>> +	/** @threshold_value: Threshold value for the counter */
>> +	u32 threshold_value;
>> +	/** @counter_status: Status of the counter */
>> +	u32 counter_status:8;
> Ditto.
>
>> +	/** @reserved: Reserved for future use */
>> +	u32 reserved:1;
> Ditto.
>
>> +	/** @has_info_queue: Indicates if info queue is present */
>> +	u32 has_info_queue:1;
> Ditto.
>
> Raag
>
>> +	/** @reserved1: Reserved for future use */
>> +	u32 reserved1:22;
>> +	/** @info_queue: Info queue data */
>> +	struct xe_ras_info_queue_response info_queue;
>> +} __packed;
>> +#endif
>> -- 
>> 2.47.1
>>