Re: [PATCH v5 4/6] drm/xe/xe_drm_ras: Wire get-error-counter and clear-error-counter support for CRI

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

On 5/11/2026 9:04 PM, Raag Jadav wrote:
> On Mon, May 04, 2026 at 12:26:19PM +0530, Riana Tauro wrote:
>> Hook CRI get-error-counter and clear-error-counter support to
>> xe_drm_ras to allow userspace to query and clear counters if supported.
>> When userspace requests for a drm_ras error counter, query the system
>> controller to get/clear the value.
>>
>> Integrate this with xe_drm_ras.
>>
>> Usage :
>>
>> Query all error counter value using ynl
>>
>> $ sudo ynl --family drm_ras --dump get-error-counter --json \
>> '{"node-id":0}'
>> [{'error-id': 1, 'error-name': 'core-compute', 'error-value': 0},
>>   {'error-id': 2, 'error-name': 'soc-internal', 'error-value': 0},
>>   {'error-id': 3, 'error-name': 'device-memory', 'error-value': 0},
>>   {'error-id': 4, 'error-name': 'pcie', 'error-value': 0},
>>   {'error-id': 5, 'error-name': 'fabric', 'error-value': 0}]
>>
>> Query single error counter value using ynl
>>
>> $ sudo ynl --family drm_ras  --do get-error-counter --json \
>> '{"node-id":1, "error-id":1}'
>> {'error-id': 1, 'error-name': 'core-compute', 'error-value': 2}
>>
>> Clear counter using ynl
>>
>> $ sudo ynl --family drm_ras  --do clear-error-counter --json '\
>> {"node-id":1, "error-id":1}'
>> None
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>> v2: split patches (Raag)
>>
>> v3: fix early return
>>      align spacing in commit message (Raag)
>>      integrate clear counter with drm_ras
>>
>> v4: rebase
>> ---
>>   drivers/gpu/drm/xe/xe_drm_ras.c | 39 +++++++++++++++++++++------------
>>   1 file changed, 25 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_drm_ras.c b/drivers/gpu/drm/xe/xe_drm_ras.c
>> index c21c8b428de6..e7bd3f09a762 100644
>> --- a/drivers/gpu/drm/xe/xe_drm_ras.c
>> +++ b/drivers/gpu/drm/xe/xe_drm_ras.c
>> @@ -11,27 +11,46 @@
>>   
>>   #include "xe_device_types.h"
>>   #include "xe_drm_ras.h"
>> +#include "xe_ras.h"
>>   
>>   static const char * const error_components[] = DRM_XE_RAS_ERROR_COMPONENT_NAMES;
>>   static const char * const error_severity[] = DRM_XE_RAS_ERROR_SEVERITY_NAMES;
>>   
>> -static int hw_query_error_counter(struct xe_drm_ras_counter *info,
>> +static int hw_query_error_counter(struct xe_device *xe,
>> +				  enum drm_xe_ras_error_severity severity,
>>   				  u32 error_id, const char **name, u32 *val)
>>   {
>> +	struct xe_drm_ras *ras = &xe->ras;
>> +	struct xe_drm_ras_counter *info = ras->info[severity];
>> +
>>   	if (!info || !info[error_id].name)
>>   		return -ENOENT;
>>   
>>   	*name = info[error_id].name;
>> +
>> +	/* Fetch counter from system controller if supported */
>> +	if (xe->info.has_sysctrl)
> Curious. Should we check this inside xe_ras_get_counter()? We'll probably
> end up doing it at some point anyway.

We also have PVC counters. If moved, the hw_error counters also have to 
be moved under
the ras function. Let's keep the hw_error and ras seperated.

if we need to move sometime in future, then we could have a generic check
For now,  we can retain the same.

Thanks
Riana

>
>> +		return xe_ras_get_counter(xe, severity, error_id, val);
>> +
>>   	*val = atomic_read(&info[error_id].counter);
>>   
>>   	return 0;
>>   }
>>   
>> -static int hw_clear_error_counter(struct xe_drm_ras_counter *info, u32 error_id)
>> +static int hw_clear_error_counter(struct xe_device *xe,
>> +				  enum drm_xe_ras_error_severity severity,
>> +				  u32 error_id)
>>   {
>> +	struct xe_drm_ras *ras = &xe->ras;
>> +	struct xe_drm_ras_counter *info = ras->info[severity];
>> +
>>   	if (!info || !info[error_id].name)
>>   		return -ENOENT;
>>   
>> +	/* Clear counter from system controller if supported */
>> +	if (xe->info.has_sysctrl)
> Ditto.
>
> Raag
>
>> +		return xe_ras_clear_counter(xe, severity, error_id);
>> +
>>   	atomic_set(&info[error_id].counter, 0);
>>   
>>   	return 0;
>> @@ -41,38 +60,30 @@ static int query_uncorrectable_error_counter(struct drm_ras_node *ep, u32 error_
>>   					     const char **name, u32 *val)
>>   {
>>   	struct xe_device *xe = ep->priv;
>> -	struct xe_drm_ras *ras = &xe->ras;
>> -	struct xe_drm_ras_counter *info = ras->info[DRM_XE_RAS_ERR_SEV_UNCORRECTABLE];
>>   
>> -	return hw_query_error_counter(info, error_id, name, val);
>> +	return hw_query_error_counter(xe, DRM_XE_RAS_ERR_SEV_UNCORRECTABLE, error_id, name, val);
>>   }
>>   
>>   static int clear_uncorrectable_error_counter(struct drm_ras_node *node, u32 error_id)
>>   {
>>   	struct xe_device *xe = node->priv;
>> -	struct xe_drm_ras *ras = &xe->ras;
>> -	struct xe_drm_ras_counter *info = ras->info[DRM_XE_RAS_ERR_SEV_UNCORRECTABLE];
>>   
>> -	return hw_clear_error_counter(info, error_id);
>> +	return hw_clear_error_counter(xe, DRM_XE_RAS_ERR_SEV_UNCORRECTABLE, error_id);
>>   }
>>   
>>   static int query_correctable_error_counter(struct drm_ras_node *ep, u32 error_id,
>>   					   const char **name, u32 *val)
>>   {
>>   	struct xe_device *xe = ep->priv;
>> -	struct xe_drm_ras *ras = &xe->ras;
>> -	struct xe_drm_ras_counter *info = ras->info[DRM_XE_RAS_ERR_SEV_CORRECTABLE];
>>   
>> -	return hw_query_error_counter(info, error_id, name, val);
>> +	return hw_query_error_counter(xe, DRM_XE_RAS_ERR_SEV_CORRECTABLE, error_id, name, val);
>>   }
>>   
>>   static int clear_correctable_error_counter(struct drm_ras_node *node, u32 error_id)
>>   {
>>   	struct xe_device *xe = node->priv;
>> -	struct xe_drm_ras *ras = &xe->ras;
>> -	struct xe_drm_ras_counter *info = ras->info[DRM_XE_RAS_ERR_SEV_CORRECTABLE];
>>   
>> -	return hw_clear_error_counter(info, error_id);
>> +	return hw_clear_error_counter(xe, DRM_XE_RAS_ERR_SEV_CORRECTABLE, error_id);
>>   }
>>   
>>   static struct xe_drm_ras_counter *allocate_and_copy_counters(struct xe_device *xe)
>> -- 
>> 2.47.1
>>