Re: [PATCH 2/2] drm/xe/i2c: Expose AMC Alert reason sysfs

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 25-08-2026 00:56, Rodrigo Vivi wrote:
> On Sun, Aug 23, 2026 at 04:05:26PM +0530, Badal Nilawar wrote:
>> AMC raises an SMBUS alert before performing a power removal or
>> power-cycle operation. The xe driver then places the device into
>> vendor-specific wedge mode until the recovery is performed.
>>
>> Expose a read-only xe_amc_alert_reason sysfs attribute to help users
>> identify the required recovery action.
>>
>> Assisted-by: Claude:claude-opus-4.8
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>>   .../ABI/testing/sysfs-driver-intel-xe-amc     | 21 ++++++
>>   drivers/gpu/drm/xe/xe_amc.c                   | 65 ++++++++++++++++++-
>>   2 files changed, 84 insertions(+), 2 deletions(-)
>>   create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-xe-amc
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-xe-amc b/Documentation/ABI/testing/sysfs-driver-intel-xe-amc
>> new file mode 100644
>> index 000000000000..b3de933efe11
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-xe-amc
>> @@ -0,0 +1,21 @@
>> +What:		/sys/bus/pci/drivers/xe/.../xe_amc_alert_reason
> Should we do <bdf> instead of ... ?!
Sure
>
>> +Date:		August 2026
>> +KernelVersion:	7.3
> this will be 7.4
I will fix this.
>
>> +Contact:	intel-xe@lists.freedesktop.org
>> +Description:
>> +		This file exposes the reason for the most recent Add-In
>> +		Management Controller (AMC) alert on Intel Xe platforms.
>> +
>> +		An AMC alert is delivered via an SMBUS interrupt and causes the
>> +		device to be wedged, requiring vendor-specific recovery. This
>> +		attribute is created when such an alert is handled and is
>> +		available to all users as read-only.
>> +
>> +		Read returns a single line containing one of the following
>> +		alert reasons:
>> +
>> +			Firmware Download
>> +			Thermal Trip
>> +			OOB Request
>> +			OOB Reset
>> +			Catastrophic
>> diff --git a/drivers/gpu/drm/xe/xe_amc.c b/drivers/gpu/drm/xe/xe_amc.c
>> index 8ecadee6eea3..ceb2c4d618fe 100644
>> --- a/drivers/gpu/drm/xe/xe_amc.c
>> +++ b/drivers/gpu/drm/xe/xe_amc.c
>> @@ -18,6 +18,7 @@
>>   #include "xe_device.h"
>>   #include "xe_i2c.h"
>>   #include "xe_mmio.h"
>> +#include "xe_printk.h"
>>   
>>   /**
>>    * DOC: Add-In Management Controller (AMC)
>> @@ -56,6 +57,8 @@ static const char * const amc_alert[] = {
>>   struct xe_amc {
>>   	struct xe_i2c *i2c;
>>   	struct work_struct work;
>> +	u8 alert_reason;
>> +	bool sysfs_created;
>>   };
>>   
>>   struct amc_header {
>> @@ -104,6 +107,54 @@ static const struct amc_request amc_get_alert_reason = {
>>   	},
>>   };
>>   
>> +/**
>> + * DOC: AMC Alert Reason
>> + *
>> + * On Intel Xe platforms, AMC sends an alert notification via an SMBUS interrupt
>> + * to notify events such as firmware download, thermal trip or a
>> + * catastrophic error. See enum xe_amc_alert for the full list of reasons.
>> + * Upon an AMC alert the device is wedged and requires vendor-specific recovery.
>> + *
>> + * The alert reason is exposed through
>> + * /sys/bus/pci/drivers/xe/.../xe_amc_alert_reason
> ditto
>
>> + *
>> + * See Documentation/ABI/testing/sysfs-driver-intel-xe-amc for the ABI
>> + * specification.
>> + */
>> +
>> +static ssize_t xe_amc_alert_reason_show(struct device *dev,
>> +					struct device_attribute *attr, char *buff)
>> +{
>> +	struct xe_device *xe = pdev_to_xe_device(to_pci_dev(dev));
>> +	struct xe_amc *amc = xe->i2c->amc;
>> +
>> +	return sysfs_emit(buff, "%s\n", amc_alert[amc->alert_reason]);
>> +}
>> +static DEVICE_ATTR_RO(xe_amc_alert_reason);
>> +
>> +static void xe_remove_amc_alert_sysfs(struct xe_device *xe)
>> +{
>> +	if (xe->i2c->amc->sysfs_created)
>> +		device_remove_file(xe->drm.dev, &dev_attr_xe_amc_alert_reason);
>> +}
>> +
>> +static void xe_create_amc_alert_sysfs(struct xe_device *xe)
>> +{
>> +	struct device *dev = xe->drm.dev;
>> +	int ret;
>> +
>> +	if (xe->i2c->amc->sysfs_created)
>> +		return;
> Why do you need to track this?!
> if this can be called from multiple places shoulnd't you protect with lock?

This will not be called from multiple places. Will drop this check.

Thanks,
Badal

>
> But I prefer that you ensure this is absolutely called only once and
> remove this check.
>
>> +
>> +	ret = device_create_file(dev, &dev_attr_xe_amc_alert_reason);
>> +	if (ret)
>> +		goto failed;
>> +
>> +	xe->i2c->amc->sysfs_created = true;
>> +failed:
>> +	dev_err(dev, "Failed to create sysfs file for amc alert reason\n");
>> +}
>> +
>>   static void xe_amc_work(struct work_struct *work)
>>   {
>>   	const struct amc_request *request = &amc_get_alert_reason;
>> @@ -158,10 +209,16 @@ static void xe_amc_work(struct work_struct *work)
>>   	case AMC_ALERT_THERMAL_TRIP:
>>   	case AMC_ALERT_OOB_REQUEST:
>>   	case AMC_ALERT_OOB_RESET:
>> -	case AMC_ALERT_CATERR:
>> +	case AMC_ALERT_CATERR: {
>> +		struct xe_device *xe = i2c_client_to_xe_device(client);
>> +
>>   		dev_warn(amc->i2c->drm_dev, "AMC Alert: %s\n", amc_alert[alert_reason]);
>> -		xe_device_declare_wedged(i2c_client_to_xe_device(client));
>> +		amc->alert_reason = alert_reason;
>> +		xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_VENDOR);
>> +		xe_device_declare_wedged(xe);
>> +		xe_create_amc_alert_sysfs(xe);
>>   		break;
>> +	}
>>   	default:
>>   		dev_warn(amc->i2c->drm_dev, "unknown AMC alert: %d\n", alert_reason);
>>   		break;
>> @@ -190,8 +247,12 @@ int xe_amc_init(struct xe_i2c *i2c)
>>   
>>   void xe_amc_exit(struct xe_i2c *i2c)
>>   {
>> +	struct xe_device *xe;
>> +
>>   	if (i2c->amc) {
>>   		cancel_work_sync(&i2c->amc->work);
>> +		xe = i2c_client_to_xe_device(i2c->client[XE_I2C_CLIENT_AMC]);
>> +		xe_remove_amc_alert_sysfs(xe);
>>   		kfree(i2c->amc);
>>   	}
>>   }
>> -- 
>> 2.54.0
>>