[PATCH v4 0/3] drm/xe/i2c: alerts and controller enabling modifications

[PATCH v4 0/3] drm/xe/i2c: alerts and controller enabling modifications

[Heikki Krogerus]

Hi,

The hardware challenges that these patches address are so severe that I'm
marking both of them as fixes. In both cases the GPU may silently end up in
unresponsive state (or worse). The second patch has been refactored so that it
includes the direct AMC alert handling in Xe instead of the normal alert handler
registration. The subject lines were also changed to highlight the fact that
these are fixes. Ramesh helped me with the testing and with the implementation
for the AMC alert handling.

Changed since v2:
- Added Fixes tag to both patches.
- i2c-designware is no longer supplied with an interrupt so it will be in
  polling mode (ACCESS_POLLING will be enabled). The IRQ path in hardware can't
  handle the amount of interrupts the i2c controller generates. Only the
  interrupts from the SMBus Alert line are left enabled.
- The registration of the default smbus alert handler is dropped.
- The AMC alerts are handled directly in Xe. All the alerts will cause the
  device to be declared as wedged at least for now.
- Cleanups proposed by Raag.

v2: https://lore.kernel.org/lkml/20260625125939.429078-1-heikki.krogerus@linux.intel.com/

Changed since v1:
- Global header for the DesignWare I2C registers which meant a bit of
  patch refactoring.
- Selecting CONFIG_SMBUS in CONFIG_XE and handling smbus in xe_i2c.c instead of
  separate file.
- Storing the alert device to the client array and providing enum for the
  clients.
- Allowing other fields in the IC_ENABLE register to be updated except the
  Enable bit.
- Can't sleep in xe_i2c_disable() so using udelay().

v1: https://lore.kernel.org/lkml/20260622114759.3464047-1-heikki.krogerus@linux.intel.com/

This includes support for the SMBus alerts, and special handling for the
IC_ENABLE register.

Thanks,

Heikki Krogerus (3):
  i2c: designware: Global register definitions
  drm/xe/i2c: Fix the interrupt handling
  drm/xe/i2c: Keep the i2c controller always enabled

 MAINTAINERS                                |   1 +
 drivers/gpu/drm/xe/Makefile                |   4 +-
 drivers/gpu/drm/xe/regs/xe_i2c_regs.h      |   2 +
 drivers/gpu/drm/xe/xe_amc.c                | 173 +++++++++++++++++++++
 drivers/gpu/drm/xe/xe_amc.h                |  25 +++
 drivers/gpu/drm/xe/xe_i2c.c                | 136 +++++++++-------
 drivers/gpu/drm/xe/xe_i2c.h                |  14 +-
 drivers/i2c/busses/i2c-designware-common.c |   2 +
 drivers/i2c/busses/i2c-designware-core.h   |  85 +---------
 drivers/i2c/busses/i2c-designware-master.c |   2 +
 drivers/i2c/busses/i2c-designware-slave.c  |   2 +
 include/linux/designware_i2c.h             | 107 +++++++++++++
 12 files changed, 405 insertions(+), 148 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/xe_amc.c
 create mode 100644 drivers/gpu/drm/xe/xe_amc.h
 create mode 100644 include/linux/designware_i2c.h

-- 
2.50.1

[PATCH v4 1/3] i2c: designware: Global register definitions

[Heikki Krogerus]

Moving the register definitions to a global header file
include/linux/designware_i2c.h. That removes the need to
duplicate them in the adaptation layers for this driver
outside of drivers/i2c/busses/. There is at least one of
those in drivers/gpu/drm/xe/xe_i2c.c.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Suggested-by: Raag Jadav <raag.jadav@intel.com>
Reviewed-by: Raag Jadav <raag.jadav@intel.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 MAINTAINERS                                |   1 +
 drivers/i2c/busses/i2c-designware-common.c |   2 +
 drivers/i2c/busses/i2c-designware-core.h   |  85 +---------------
 drivers/i2c/busses/i2c-designware-master.c |   2 +
 drivers/i2c/busses/i2c-designware-slave.c  |   2 +
 include/linux/designware_i2c.h             | 107 +++++++++++++++++++++
 6 files changed, 116 insertions(+), 83 deletions(-)
 create mode 100644 include/linux/designware_i2c.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 42ed870d55f94..ed1b75e9ecfe1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26245,6 +26245,7 @@ R:	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
 L:	linux-i2c@vger.kernel.org
 S:	Supported
 F:	drivers/i2c/busses/i2c-designware-*
+F:	include/linux/designware_i2c.h
 
 SYNOPSYS DESIGNWARE I2C DRIVER - AMDISP
 M:	Nirujogi Pratap <pratap.nirujogi@amd.com>
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e4dfa2ec58bb7..a1eca6cd4b75e 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -33,6 +33,8 @@
 #include <linux/types.h>
 #include <linux/units.h>
 
+#include <linux/designware_i2c.h>
+
 #include "i2c-designware-core.h"
 
 #define DW_IC_DEFAULT_BUS_CAPACITANCE_pF	100
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index c71aa2dd368d5..2c929a6e8da2a 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -18,6 +18,8 @@
 #include <linux/regmap.h>
 #include <linux/types.h>
 
+#include <linux/designware_i2c.h>
+
 #define DW_IC_DEFAULT_FUNCTIONALITY		(I2C_FUNC_I2C | \
 						 I2C_FUNC_SMBUS_BYTE | \
 						 I2C_FUNC_SMBUS_BYTE_DATA | \
@@ -25,23 +27,6 @@
 						 I2C_FUNC_SMBUS_BLOCK_DATA | \
 						 I2C_FUNC_SMBUS_I2C_BLOCK)
 
-#define DW_IC_CON_MASTER			BIT(0)
-#define DW_IC_CON_SPEED_STD			(1 << 1)
-#define DW_IC_CON_SPEED_FAST			(2 << 1)
-#define DW_IC_CON_SPEED_HIGH			(3 << 1)
-#define DW_IC_CON_SPEED_MASK			GENMASK(2, 1)
-#define DW_IC_CON_10BITADDR_SLAVE		BIT(3)
-#define DW_IC_CON_10BITADDR_MASTER		BIT(4)
-#define DW_IC_CON_RESTART_EN			BIT(5)
-#define DW_IC_CON_SLAVE_DISABLE			BIT(6)
-#define DW_IC_CON_STOP_DET_IFADDRESSED		BIT(7)
-#define DW_IC_CON_TX_EMPTY_CTRL			BIT(8)
-#define DW_IC_CON_RX_FIFO_FULL_HLD_CTRL		BIT(9)
-#define DW_IC_CON_BUS_CLEAR_CTRL		BIT(11)
-
-#define DW_IC_DATA_CMD_DAT			GENMASK(7, 0)
-#define DW_IC_DATA_CMD_FIRST_DATA_BYTE		BIT(11)
-
 /*
  * Register access parameters
  */
@@ -55,65 +40,9 @@
 #define DW_IC_FIFO_RX_FIELD			GENMASK(15, 8)
 #define DW_IC_FIFO_MIN_DEPTH			2
 
-/*
- * Registers offset
- */
-#define DW_IC_CON				0x00
-#define DW_IC_TAR				0x04
-#define DW_IC_SAR				0x08
-#define DW_IC_DATA_CMD				0x10
-#define DW_IC_SS_SCL_HCNT			0x14
-#define DW_IC_SS_SCL_LCNT			0x18
-#define DW_IC_FS_SCL_HCNT			0x1c
-#define DW_IC_FS_SCL_LCNT			0x20
-#define DW_IC_HS_SCL_HCNT			0x24
-#define DW_IC_HS_SCL_LCNT			0x28
-#define DW_IC_INTR_STAT				0x2c
-#define DW_IC_INTR_MASK				0x30
-#define DW_IC_RAW_INTR_STAT			0x34
-#define DW_IC_RX_TL				0x38
-#define DW_IC_TX_TL				0x3c
-#define DW_IC_CLR_INTR				0x40
-#define DW_IC_CLR_RX_UNDER			0x44
-#define DW_IC_CLR_RX_OVER			0x48
-#define DW_IC_CLR_TX_OVER			0x4c
-#define DW_IC_CLR_RD_REQ			0x50
-#define DW_IC_CLR_TX_ABRT			0x54
-#define DW_IC_CLR_RX_DONE			0x58
-#define DW_IC_CLR_ACTIVITY			0x5c
-#define DW_IC_CLR_STOP_DET			0x60
-#define DW_IC_CLR_START_DET			0x64
-#define DW_IC_CLR_GEN_CALL			0x68
-#define DW_IC_ENABLE				0x6c
-#define DW_IC_STATUS				0x70
-#define DW_IC_TXFLR				0x74
-#define DW_IC_RXFLR				0x78
-#define DW_IC_SDA_HOLD				0x7c
-#define DW_IC_TX_ABRT_SOURCE			0x80
-#define DW_IC_ENABLE_STATUS			0x9c
-#define DW_IC_CLR_RESTART_DET			0xa8
-#define DW_IC_SMBUS_INTR_MASK			0xcc
-#define DW_IC_COMP_PARAM_1			0xf4
-#define DW_IC_COMP_VERSION			0xf8
 #define DW_IC_SDA_HOLD_MIN_VERS			0x3131312A /* "111*" == v1.11* */
-#define DW_IC_COMP_TYPE				0xfc
 #define DW_IC_COMP_TYPE_VALUE			0x44570140 /* "DW" + 0x0140 */
 
-#define DW_IC_INTR_RX_UNDER			BIT(0)
-#define DW_IC_INTR_RX_OVER			BIT(1)
-#define DW_IC_INTR_RX_FULL			BIT(2)
-#define DW_IC_INTR_TX_OVER			BIT(3)
-#define DW_IC_INTR_TX_EMPTY			BIT(4)
-#define DW_IC_INTR_RD_REQ			BIT(5)
-#define DW_IC_INTR_TX_ABRT			BIT(6)
-#define DW_IC_INTR_RX_DONE			BIT(7)
-#define DW_IC_INTR_ACTIVITY			BIT(8)
-#define DW_IC_INTR_STOP_DET			BIT(9)
-#define DW_IC_INTR_START_DET			BIT(10)
-#define DW_IC_INTR_GEN_CALL			BIT(11)
-#define DW_IC_INTR_RESTART_DET			BIT(12)
-#define DW_IC_INTR_MST_ON_HOLD			BIT(13)
-
 #define DW_IC_INTR_DEFAULT_MASK			(DW_IC_INTR_RX_FULL | \
 						 DW_IC_INTR_TX_ABRT | \
 						 DW_IC_INTR_STOP_DET)
@@ -123,16 +52,6 @@
 						 DW_IC_INTR_RX_UNDER | \
 						 DW_IC_INTR_RD_REQ)
 
-#define DW_IC_ENABLE_ENABLE			BIT(0)
-#define DW_IC_ENABLE_ABORT			BIT(1)
-
-#define DW_IC_STATUS_ACTIVITY			BIT(0)
-#define DW_IC_STATUS_TFE			BIT(2)
-#define DW_IC_STATUS_RFNE			BIT(3)
-#define DW_IC_STATUS_MASTER_ACTIVITY		BIT(5)
-#define DW_IC_STATUS_SLAVE_ACTIVITY		BIT(6)
-#define DW_IC_STATUS_MASTER_HOLD_TX_FIFO_EMPTY	BIT(7)
-
 #define DW_IC_SDA_HOLD_RX_SHIFT			16
 #define DW_IC_SDA_HOLD_RX_MASK			GENMASK(23, 16)
 
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 7a301c8b604ef..a1bcc3797e4ff 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -25,6 +25,8 @@
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
+#include <linux/designware_i2c.h>
+
 #include "i2c-designware-core.h"
 
 #define AMD_TIMEOUT_MIN_US	25
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index ad0d5fbfa6d5e..0abcc7757b231 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -19,6 +19,8 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
+#include <linux/designware_i2c.h>
+
 #include "i2c-designware-core.h"
 
 int i2c_dw_reg_slave(struct i2c_client *slave)
diff --git a/include/linux/designware_i2c.h b/include/linux/designware_i2c.h
new file mode 100644
index 0000000000000..53f37f18a7229
--- /dev/null
+++ b/include/linux/designware_i2c.h
@@ -0,0 +1,107 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Synopsys DesignWare I2C register definitions
+ *
+ * Copyright (C) 2026, Intel Corporation
+ */
+
+#ifndef __LINUX_DESIGNWARE_I2C_H
+#define __LINUX_DESIGNWARE_I2C_H
+
+#include <linux/bits.h>
+
+/*
+ * Registers offset
+ */
+#define DW_IC_CON				0x00
+#define DW_IC_TAR				0x04
+#define DW_IC_SAR				0x08
+#define DW_IC_DATA_CMD				0x10
+#define DW_IC_SS_SCL_HCNT			0x14
+#define DW_IC_SS_SCL_LCNT			0x18
+#define DW_IC_FS_SCL_HCNT			0x1c
+#define DW_IC_FS_SCL_LCNT			0x20
+#define DW_IC_HS_SCL_HCNT			0x24
+#define DW_IC_HS_SCL_LCNT			0x28
+#define DW_IC_INTR_STAT				0x2c
+#define DW_IC_INTR_MASK				0x30
+#define DW_IC_RAW_INTR_STAT			0x34
+#define DW_IC_RX_TL				0x38
+#define DW_IC_TX_TL				0x3c
+#define DW_IC_CLR_INTR				0x40
+#define DW_IC_CLR_RX_UNDER			0x44
+#define DW_IC_CLR_RX_OVER			0x48
+#define DW_IC_CLR_TX_OVER			0x4c
+#define DW_IC_CLR_RD_REQ			0x50
+#define DW_IC_CLR_TX_ABRT			0x54
+#define DW_IC_CLR_RX_DONE			0x58
+#define DW_IC_CLR_ACTIVITY			0x5c
+#define DW_IC_CLR_STOP_DET			0x60
+#define DW_IC_CLR_START_DET			0x64
+#define DW_IC_CLR_GEN_CALL			0x68
+#define DW_IC_ENABLE				0x6c
+#define DW_IC_STATUS				0x70
+#define DW_IC_TXFLR				0x74
+#define DW_IC_RXFLR				0x78
+#define DW_IC_SDA_HOLD				0x7c
+#define DW_IC_TX_ABRT_SOURCE			0x80
+#define DW_IC_ENABLE_STATUS			0x9c
+#define DW_IC_CLR_RESTART_DET			0xa8
+#define DW_IC_SMBUS_INTR_STAT			0xc8
+#define DW_IC_SMBUS_INTR_MASK			0xcc
+#define DW_IC_CLR_SMBUS_INTR			0xd4
+#define DW_IC_COMP_PARAM_1			0xf4
+#define DW_IC_COMP_VERSION			0xf8
+#define DW_IC_COMP_TYPE				0xfc
+
+/* DW_IC_CON bits */
+#define DW_IC_CON_MASTER			BIT(0)
+#define DW_IC_CON_SPEED_STD			(1 << 1)
+#define DW_IC_CON_SPEED_FAST			(2 << 1)
+#define DW_IC_CON_SPEED_HIGH			(3 << 1)
+#define DW_IC_CON_SPEED_MASK			GENMASK(2, 1)
+#define DW_IC_CON_10BITADDR_SLAVE		BIT(3)
+#define DW_IC_CON_10BITADDR_MASTER		BIT(4)
+#define DW_IC_CON_RESTART_EN			BIT(5)
+#define DW_IC_CON_SLAVE_DISABLE			BIT(6)
+#define DW_IC_CON_STOP_DET_IFADDRESSED		BIT(7)
+#define DW_IC_CON_TX_EMPTY_CTRL			BIT(8)
+#define DW_IC_CON_RX_FIFO_FULL_HLD_CTRL		BIT(9)
+#define DW_IC_CON_BUS_CLEAR_CTRL		BIT(11)
+
+/* DW_IC_DATA_CMD bits */
+#define DW_IC_DATA_CMD_DAT			GENMASK(7, 0)
+#define DW_IC_DATA_CMD_FIRST_DATA_BYTE		BIT(11)
+
+/* DW_IC_INTR_* bits */
+#define DW_IC_INTR_RX_UNDER			BIT(0)
+#define DW_IC_INTR_RX_OVER			BIT(1)
+#define DW_IC_INTR_RX_FULL			BIT(2)
+#define DW_IC_INTR_TX_OVER			BIT(3)
+#define DW_IC_INTR_TX_EMPTY			BIT(4)
+#define DW_IC_INTR_RD_REQ			BIT(5)
+#define DW_IC_INTR_TX_ABRT			BIT(6)
+#define DW_IC_INTR_RX_DONE			BIT(7)
+#define DW_IC_INTR_ACTIVITY			BIT(8)
+#define DW_IC_INTR_STOP_DET			BIT(9)
+#define DW_IC_INTR_START_DET			BIT(10)
+#define DW_IC_INTR_GEN_CALL			BIT(11)
+#define DW_IC_INTR_RESTART_DET			BIT(12)
+#define DW_IC_INTR_MST_ON_HOLD			BIT(13)
+
+/* DW_IC_ENABLE bits */
+#define DW_IC_ENABLE_ENABLE			BIT(0)
+#define DW_IC_ENABLE_ABORT			BIT(1)
+
+/* DW_IC_STATUS bits */
+#define DW_IC_STATUS_ACTIVITY			BIT(0)
+#define DW_IC_STATUS_TFE			BIT(2)
+#define DW_IC_STATUS_RFNE			BIT(3)
+#define DW_IC_STATUS_MASTER_ACTIVITY		BIT(5)
+#define DW_IC_STATUS_SLAVE_ACTIVITY		BIT(6)
+#define DW_IC_STATUS_MASTER_HOLD_TX_FIFO_EMPTY	BIT(7)
+
+/* DW_IC_SMBUS_INTR_* bits */
+#define DW_IC_SMBUS_INTR_ALERT			BIT(10)
+
+#endif /* __LINUX_DESIGNWARE_I2C_H */
-- 
2.50.1

[PATCH v4 2/3] drm/xe/i2c: Fix the interrupt handling

[Heikki Krogerus]

The platforms that support the interrupt from the I2C
adapter can not handle the amount of interrupts the adapter
generates because of the way the IRQ is routed in the
hardware. The I2C controller driver has to be kept in
polling mode because of that.

The AMC MCU can still generate critical alerts that have to
be handled. The interrupt from SMBus Alert is left enabled
and handled separately in the Xe. The alerts from the AMC
will cause the device to be declared wedged for now.

Fixes: f0e53aadd702 ("drm/xe: Support for I2C attached MCUs")
Cc: stable@vger.kernel.org
Co-developed-by: Ramesh Babu B <ramesh.babu.b@intel.com>
Signed-off-by: Ramesh Babu B <ramesh.babu.b@intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/xe/Makefile           |   4 +-
 drivers/gpu/drm/xe/regs/xe_i2c_regs.h |   2 +
 drivers/gpu/drm/xe/xe_amc.c           | 173 ++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_amc.h           |  25 ++++
 drivers/gpu/drm/xe/xe_i2c.c           |  93 +++++---------
 drivers/gpu/drm/xe/xe_i2c.h           |  13 +-
 6 files changed, 241 insertions(+), 69 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/xe_amc.c
 create mode 100644 drivers/gpu/drm/xe/xe_amc.h

diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 67ada1d6c2fb9..c92468cb9b894 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -153,7 +153,9 @@ xe-y += xe_bb.o \
 	xe_wait_user_fence.o \
 	xe_wopcm.o
 
-xe-$(CONFIG_I2C)	+= xe_i2c.o
+xe-$(CONFIG_I2C)	+= xe_i2c.o \
+			   xe_amc.o
+
 xe-$(CONFIG_DRM_XE_GPUSVM) += xe_svm.o
 xe-$(CONFIG_DRM_GPUSVM) += xe_userptr.o
 
diff --git a/drivers/gpu/drm/xe/regs/xe_i2c_regs.h b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
index f2e455e2bfe45..37550e4a20f80 100644
--- a/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
+++ b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
@@ -20,4 +20,6 @@
 #define I2C_CONFIG_CMD			XE_REG(I2C_CONFIG_SPACE_OFFSET + PCI_COMMAND)
 #define I2C_CONFIG_PMCSR		XE_REG(I2C_CONFIG_SPACE_OFFSET + 0x84)
 
+#define I2C_REG(reg)			XE_REG((reg) + I2C_MEM_SPACE_OFFSET)
+
 #endif /* _XE_I2C_REGS_H_ */
diff --git a/drivers/gpu/drm/xe/xe_amc.c b/drivers/gpu/drm/xe/xe_amc.c
new file mode 100644
index 0000000000000..60ac4936a5f89
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_amc.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2026 Intel Corporation.
+ */
+
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/pci_ids.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/workqueue.h>
+
+#include "xe_amc.h"
+#include "xe_device.h"
+#include "xe_i2c.h"
+
+/**
+ * DOC: Add-In Management Controller (AMC)
+ *
+ * Handler for the SMBus Alerts from the AMC. All the alerts from AMC will cause
+ * the device to be declared wedged.
+ */
+
+#define AMC_COMMAND		0x0f
+#define AMC_GPU_I2C_ADDR	0x8f
+#define AMC_VERSION_V1		0x01
+#define AMC_DESTINATION_ID	12
+#define AMC_SOURCE_ID		8
+#define AMC_FLAGS		0xc8
+
+#define AMC_MSG_TYPE		0x7e
+#define AMC_GET_ALERT_REASON	0x01
+
+enum xe_amc_alert {
+	AMC_ALERT_UNKNOWN,
+	AMC_ALERT_FW_DOWNLOAD,
+	AMC_ALERT_THERMAL_TRIP,
+	AMC_ALERT_OOB_REQUEST,
+	AMC_ALERT_OOB_RESET,
+	AMC_ALERT_CATERR,
+};
+
+struct xe_amc {
+	struct xe_i2c *i2c;
+	struct work_struct work;
+};
+
+struct amc_header {
+	u8 command;
+	u8 len;
+	u8 address;
+	u8 version;
+	u8 destination;
+	u8 source;
+	u8 flags;
+};
+
+struct amc_message {
+	u8 type;
+	u16 vendor;
+	u8 command;
+} __packed;
+
+struct amc_request {
+	struct amc_header header;
+	struct amc_message message;
+	u32 reserved;
+} __packed;
+
+struct amc_response {
+	struct amc_header header;
+	struct amc_message message;
+	u8 error;
+	u8 value;
+} __packed;
+
+static const struct amc_request amc_get_alert_reason = {
+	.header = {
+		.command	= AMC_COMMAND,
+		.len		= sizeof(struct amc_request) - 2,
+		.address	= AMC_GPU_I2C_ADDR,
+		.version	= AMC_VERSION_V1,
+		.destination	= AMC_DESTINATION_ID,
+		.source		= AMC_SOURCE_ID,
+		.flags		= AMC_FLAGS,
+	},
+	.message = {
+		.type		= AMC_MSG_TYPE,
+		.vendor		= htons(PCI_VENDOR_ID_INTEL),
+		.command	= AMC_GET_ALERT_REASON,
+	},
+};
+
+static void xe_amc_work(struct work_struct *work)
+{
+	struct xe_amc *amc = from_work(amc, work, work);
+	struct i2c_client *client = amc->i2c->client[XE_I2C_CLIENT_AMC];
+	const struct amc_request *request = &amc_get_alert_reason;
+	struct amc_response response;
+	int ret;
+
+	ret = i2c_master_send(client, (u8 *)request, sizeof(*request));
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to send request (%d)\n", ret);
+		return;
+	}
+
+	fsleep(20 * USEC_PER_MSEC);
+
+	ret = i2c_master_recv(client, (u8 *)&response, sizeof(response));
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to read response (%d)\n", ret);
+		return;
+	}
+
+	if (response.header.len == 0) {
+		dev_err(&client->dev, "empty response from AMC\n");
+		return;
+	}
+
+	if (response.header.command != request->header.command ||
+	    memcmp(&response.message, &request->message, sizeof(struct amc_message))) {
+		dev_err(&client->dev, "response does not match the request\n");
+		return;
+	}
+
+	if (response.error) {
+		dev_err(&client->dev, "AMC error 0x%02x\n", response.error);
+		return;
+	}
+
+	dev_dbg(&client->dev, "%s: Alert reason: %d\n", __func__, response.value);
+
+	switch (response.value) {
+	case AMC_ALERT_FW_DOWNLOAD:
+	case AMC_ALERT_THERMAL_TRIP:
+	case AMC_ALERT_OOB_REQUEST:
+	case AMC_ALERT_OOB_RESET:
+	case AMC_ALERT_CATERR:
+		xe_device_declare_wedged(i2c_client_to_xe_device(client));
+		break;
+	default:
+		break;
+	}
+}
+
+void xe_amc_handle_alert(struct xe_i2c *i2c)
+{
+	if (i2c->client[XE_I2C_CLIENT_AMC])
+		queue_work(system_long_wq, &i2c->amc->work);
+}
+
+int xe_amc_init(struct xe_i2c *i2c)
+{
+	struct xe_amc *amc;
+
+	amc = kzalloc(sizeof(*amc), GFP_KERNEL);
+	if (!amc)
+		return -ENOMEM;
+
+	INIT_WORK(&amc->work, xe_amc_work);
+	i2c->amc = amc;
+	amc->i2c = i2c;
+
+	return 0;
+}
+
+void xe_amc_exit(struct xe_i2c *i2c)
+{
+	cancel_work_sync(&i2c->amc->work);
+	kfree(i2c->amc);
+}
diff --git a/drivers/gpu/drm/xe/xe_amc.h b/drivers/gpu/drm/xe/xe_amc.h
new file mode 100644
index 0000000000000..b1d5311fee536
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_amc.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _XE_AMC_H_
+#define _XE_AMC_H_
+
+#include <linux/i2c.h>
+
+#include "xe_device.h"
+
+struct xe_i2c;
+
+static inline struct xe_device *i2c_adapter_to_xe_device(struct i2c_adapter *adapter)
+{
+	return kdev_to_xe_device(adapter->dev.parent->parent);
+}
+
+static inline struct xe_device *i2c_client_to_xe_device(struct i2c_client *client)
+{
+	return i2c_adapter_to_xe_device(client->adapter);
+}
+
+int xe_amc_init(struct xe_i2c *i2c);
+void xe_amc_exit(struct xe_i2c *i2c);
+void xe_amc_handle_alert(struct xe_i2c *i2c);
+
+#endif /* _XE_AMC_H_ */
diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index bd956776b10be..7fa1b16598ee6 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -12,8 +12,6 @@
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/ioport.h>
-#include <linux/irq.h>
-#include <linux/irqdomain.h>
 #include <linux/notifier.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
@@ -24,9 +22,12 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
+#include <linux/designware_i2c.h>
+
 #include "regs/xe_i2c_regs.h"
 #include "regs/xe_irq_regs.h"
 
+#include "xe_amc.h"
 #include "xe_device.h"
 #include "xe_i2c.h"
 #include "xe_mmio.h"
@@ -61,6 +62,20 @@ static inline void xe_i2c_read_endpoint(struct xe_mmio *mmio, void *ep)
 	val[1] = xe_mmio_read32(mmio, REG_SG_REMAP_ADDR_POSTFIX);
 }
 
+static void xe_i2c_handle_smbus_alert(struct xe_i2c *i2c)
+{
+	u32 stat;
+
+	stat = xe_mmio_read32(i2c->mmio, I2C_REG(DW_IC_SMBUS_INTR_STAT));
+	if (!stat)
+		return;
+
+	xe_mmio_write32(i2c->mmio, I2C_REG(DW_IC_CLR_SMBUS_INTR), stat);
+
+	if (stat & DW_IC_SMBUS_INTR_ALERT)
+		xe_amc_handle_alert(i2c);
+}
+
 static void xe_i2c_client_work(struct work_struct *work)
 {
 	struct xe_i2c *i2c = container_of(work, struct xe_i2c, work);
@@ -70,7 +85,7 @@ static void xe_i2c_client_work(struct work_struct *work)
 		.addr	= i2c->ep.addr[1],
 	};
 
-	i2c->client[0] = i2c_new_client_device(i2c->adapter, &info);
+	i2c->client[XE_I2C_CLIENT_AMC] = i2c_new_client_device(i2c->adapter, &info);
 }
 
 static int xe_i2c_notifier(struct notifier_block *nb, unsigned long action, void *data)
@@ -112,16 +127,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
 		goto err_fwnode_remove;
 	}
 
-	if (i2c->adapter_irq) {
-		struct resource res;
-
-		res = DEFINE_RES_IRQ_NAMED(i2c->adapter_irq, "xe_i2c");
-
-		ret = platform_device_add_resources(pdev, &res, 1);
-		if (ret)
-			goto err_pdev_put;
-	}
-
 	pdev->dev.parent = i2c->drm_dev;
 	pdev->dev.fwnode = fwnode;
 	i2c->adapter_node = fwnode;
@@ -163,7 +168,8 @@ bool xe_i2c_present(struct xe_device *xe)
 
 static bool xe_i2c_irq_present(struct xe_device *xe)
 {
-	return xe->i2c && xe->i2c->adapter_irq;
+	return xe->i2c && xe->i2c->ep.capabilities & XE_I2C_EP_CAP_IRQ &&
+		!xe_survivability_mode_is_boot_enabled(xe);
 }
 
 /**
@@ -181,8 +187,7 @@ void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
 	if (!(master_ctl & I2C_IRQ) || !xe_i2c_irq_present(xe))
 		return;
 
-	/* Forward interrupt to I2C adapter */
-	generic_handle_irq_safe(xe->i2c->adapter_irq);
+	xe_i2c_handle_smbus_alert(xe->i2c);
 
 	/* Deassert after I2C adapter clears the interrupt */
 	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
@@ -212,45 +217,6 @@ void xe_i2c_irq_postinstall(struct xe_device *xe)
 	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);
 }
 
-static int xe_i2c_irq_map(struct irq_domain *h, unsigned int virq,
-			  irq_hw_number_t hw_irq_num)
-{
-	irq_set_chip_and_handler(virq, &dummy_irq_chip, handle_simple_irq);
-	return 0;
-}
-
-static const struct irq_domain_ops xe_i2c_irq_ops = {
-	.map = xe_i2c_irq_map,
-};
-
-static int xe_i2c_create_irq(struct xe_device *xe)
-{
-	struct xe_i2c *i2c = xe->i2c;
-	struct irq_domain *domain;
-
-	if (!(i2c->ep.capabilities & XE_I2C_EP_CAP_IRQ) ||
-	    xe_survivability_mode_is_boot_enabled(xe))
-		return 0;
-
-	domain = irq_domain_create_linear(dev_fwnode(i2c->drm_dev), 1, &xe_i2c_irq_ops, NULL);
-	if (!domain)
-		return -ENOMEM;
-
-	i2c->adapter_irq = irq_create_mapping(domain, 0);
-	i2c->irqdomain = domain;
-
-	return 0;
-}
-
-static void xe_i2c_remove_irq(struct xe_i2c *i2c)
-{
-	if (!i2c->irqdomain)
-		return;
-
-	irq_dispose_mapping(i2c->adapter_irq);
-	irq_domain_remove(i2c->irqdomain);
-}
-
 static int xe_i2c_read(void *context, unsigned int reg, unsigned int *val)
 {
 	struct xe_i2c *i2c = context;
@@ -307,12 +273,15 @@ static void xe_i2c_remove(void *data)
 	struct xe_i2c *i2c = data;
 	unsigned int i;
 
-	for (i = 0; i < XE_I2C_MAX_CLIENTS; i++)
+	xe_amc_exit(i2c);
+
+	for (i = 0; i < XE_I2C_MAX_CLIENTS; i++) {
 		i2c_unregister_device(i2c->client[i]);
+		i2c->client[i] = NULL;
+	}
 
 	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
 	xe_i2c_unregister_adapter(i2c);
-	xe_i2c_remove_irq(i2c);
 }
 
 /**
@@ -360,19 +329,19 @@ int xe_i2c_probe(struct xe_device *xe)
 	if (ret)
 		return ret;
 
-	ret = xe_i2c_create_irq(xe);
+	ret = xe_i2c_register_adapter(i2c);
 	if (ret)
 		goto err_unregister_notifier;
 
-	ret = xe_i2c_register_adapter(i2c);
+	ret = xe_amc_init(i2c);
 	if (ret)
-		goto err_remove_irq;
+		goto err_remove_adapter;
 
 	xe_i2c_irq_postinstall(xe);
 	return devm_add_action_or_reset(drm_dev, xe_i2c_remove, i2c);
 
-err_remove_irq:
-	xe_i2c_remove_irq(i2c);
+err_remove_adapter:
+	xe_i2c_remove(i2c);
 
 err_unregister_notifier:
 	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
index 425d8160835f4..c95f98c2053d5 100644
--- a/drivers/gpu/drm/xe/xe_i2c.h
+++ b/drivers/gpu/drm/xe/xe_i2c.h
@@ -11,18 +11,21 @@ struct device;
 struct fwnode_handle;
 struct i2c_adapter;
 struct i2c_client;
-struct irq_domain;
 struct platform_device;
+struct xe_amc;
 struct xe_device;
 struct xe_mmio;
 
-#define XE_I2C_MAX_CLIENTS		3
-
 #define XE_I2C_EP_COOKIE_DEVICE		0xde
 
 /* Endpoint Capabilities */
 #define XE_I2C_EP_CAP_IRQ		BIT(0)
 
+enum XE_I2C_CLIENT {
+	XE_I2C_CLIENT_AMC,
+	XE_I2C_MAX_CLIENTS = 3,
+};
+
 struct xe_i2c_endpoint {
 	u8 cookie;
 	u8 capabilities;
@@ -38,13 +41,11 @@ struct xe_i2c {
 	struct notifier_block bus_notifier;
 	struct work_struct work;
 
-	struct irq_domain *irqdomain;
-	int adapter_irq;
-
 	struct xe_i2c_endpoint ep;
 	struct device *drm_dev;
 
 	struct xe_mmio *mmio;
+	struct xe_amc *amc;
 };
 
 #if IS_ENABLED(CONFIG_I2C)
-- 
2.50.1

Re: [PATCH v4 2/3] drm/xe/i2c: Fix the interrupt handling

[Raag Jadav]

On Mon, Jul 13, 2026 at 05:56:00PM +0200, Heikki Krogerus wrote:
> The platforms that support the interrupt from the I2C
> adapter can not handle the amount of interrupts the adapter
> generates because of the way the IRQ is routed in the
> hardware. The I2C controller driver has to be kept in
> polling mode because of that.
> 
> The AMC MCU can still generate critical alerts that have to
> be handled. The interrupt from SMBus Alert is left enabled
> and handled separately in the Xe. The alerts from the AMC
> will cause the device to be declared wedged for now.

...

> +static void xe_amc_work(struct work_struct *work)
> +{
> +	struct xe_amc *amc = from_work(amc, work, work);
> +	struct i2c_client *client = amc->i2c->client[XE_I2C_CLIENT_AMC];
> +	const struct amc_request *request = &amc_get_alert_reason;
> +	struct amc_response response;
> +	int ret;
> +
> +	ret = i2c_master_send(client, (u8 *)request, sizeof(*request));
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to send request (%d)\n", ret);
> +		return;
> +	}
> +
> +	fsleep(20 * USEC_PER_MSEC);

Nit: Probably worth an explanation.

> +	ret = i2c_master_recv(client, (u8 *)&response, sizeof(response));
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to read response (%d)\n", ret);
> +		return;
> +	}
> +
> +	if (response.header.len == 0) {

Nit: Perhaps !response.header.len?

> +		dev_err(&client->dev, "empty response from AMC\n");
> +		return;
> +	}
> +
> +	if (response.header.command != request->header.command ||

Curious, what about the rest of the header? Would it be any different?

> +	    memcmp(&response.message, &request->message, sizeof(struct amc_message))) {
> +		dev_err(&client->dev, "response does not match the request\n");
> +		return;
> +	}
> +
> +	if (response.error) {
> +		dev_err(&client->dev, "AMC error 0x%02x\n", response.error);
> +		return;
> +	}
> +
> +	dev_dbg(&client->dev, "%s: Alert reason: %d\n", __func__, response.value);

See below [1].

> +	switch (response.value) {
> +	case AMC_ALERT_FW_DOWNLOAD:
> +	case AMC_ALERT_THERMAL_TRIP:
> +	case AMC_ALERT_OOB_REQUEST:
> +	case AMC_ALERT_OOB_RESET:
> +	case AMC_ALERT_CATERR:
> +		xe_device_declare_wedged(i2c_client_to_xe_device(client));
> +		break;
> +	default:
> +		break;
> +	}
> +}

...

> @@ -181,8 +187,7 @@ void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
>  	if (!(master_ctl & I2C_IRQ) || !xe_i2c_irq_present(xe))
>  		return;
>  
> -	/* Forward interrupt to I2C adapter */
> -	generic_handle_irq_safe(xe->i2c->adapter_irq);
> +	xe_i2c_handle_smbus_alert(xe->i2c);

[1] Can we move the below re-assert code to wq now? Or do you suspect any
side-effects?

>  	/* Deassert after I2C adapter clears the interrupt */
>  	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
> @@ -212,45 +217,6 @@ void xe_i2c_irq_postinstall(struct xe_device *xe)
>  	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);
>  }

...

> -#define XE_I2C_MAX_CLIENTS		3
> -
>  #define XE_I2C_EP_COOKIE_DEVICE		0xde
>  
>  /* Endpoint Capabilities */
>  #define XE_I2C_EP_CAP_IRQ		BIT(0)
>  
> +enum XE_I2C_CLIENT {
> +	XE_I2C_CLIENT_AMC,
> +	XE_I2C_MAX_CLIENTS = 3,

I know it was already like this but I probably missed why do we have 3
(atleast from driver POV)?

Raag

> +};

Re: [PATCH v4 2/3] drm/xe/i2c: Fix the interrupt handling

[Heikki Krogerus]

On Wed, Jul 15, 2026 at 08:26:22AM +0200, Raag Jadav wrote:
> On Mon, Jul 13, 2026 at 05:56:00PM +0200, Heikki Krogerus wrote:
> > The platforms that support the interrupt from the I2C
> > adapter can not handle the amount of interrupts the adapter
> > generates because of the way the IRQ is routed in the
> > hardware. The I2C controller driver has to be kept in
> > polling mode because of that.
> > 
> > The AMC MCU can still generate critical alerts that have to
> > be handled. The interrupt from SMBus Alert is left enabled
> > and handled separately in the Xe. The alerts from the AMC
> > will cause the device to be declared wedged for now.
> 
> ...
> 
> > +static void xe_amc_work(struct work_struct *work)
> > +{
> > +	struct xe_amc *amc = from_work(amc, work, work);
> > +	struct i2c_client *client = amc->i2c->client[XE_I2C_CLIENT_AMC];
> > +	const struct amc_request *request = &amc_get_alert_reason;
> > +	struct amc_response response;
> > +	int ret;
> > +
> > +	ret = i2c_master_send(client, (u8 *)request, sizeof(*request));
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "failed to send request (%d)\n", ret);
> > +		return;
> > +	}
> > +
> > +	fsleep(20 * USEC_PER_MSEC);
> 
> Nit: Probably worth an explanation.

Sure.

> > +	ret = i2c_master_recv(client, (u8 *)&response, sizeof(response));
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "failed to read response (%d)\n", ret);
> > +		return;
> > +	}
> > +
> > +	if (response.header.len == 0) {
> 
> Nit: Perhaps !response.header.len?

OK.

> > +		dev_err(&client->dev, "empty response from AMC\n");
> > +		return;
> > +	}
> > +
> > +	if (response.header.command != request->header.command ||
> 
> Curious, what about the rest of the header? Would it be any different?

The command is the only field that matches. But that check is not
needed. The command is always the same. I'll drop that line.

> > +	    memcmp(&response.message, &request->message, sizeof(struct amc_message))) {
> > +		dev_err(&client->dev, "response does not match the request\n");
> > +		return;
> > +	}
> > +
> > +	if (response.error) {
> > +		dev_err(&client->dev, "AMC error 0x%02x\n", response.error);
> > +		return;
> > +	}
> > +
> > +	dev_dbg(&client->dev, "%s: Alert reason: %d\n", __func__, response.value);
> 
> See below [1].
> 
> > +	switch (response.value) {
> > +	case AMC_ALERT_FW_DOWNLOAD:
> > +	case AMC_ALERT_THERMAL_TRIP:
> > +	case AMC_ALERT_OOB_REQUEST:
> > +	case AMC_ALERT_OOB_RESET:
> > +	case AMC_ALERT_CATERR:
> > +		xe_device_declare_wedged(i2c_client_to_xe_device(client));
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +}
> 
> ...
> 
> > @@ -181,8 +187,7 @@ void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
> >  	if (!(master_ctl & I2C_IRQ) || !xe_i2c_irq_present(xe))
> >  		return;
> >  
> > -	/* Forward interrupt to I2C adapter */
> > -	generic_handle_irq_safe(xe->i2c->adapter_irq);
> > +	xe_i2c_handle_smbus_alert(xe->i2c);
> 
> [1] Can we move the below re-assert code to wq now? Or do you suspect any
> side-effects?

I think that you know this better than I do. But at this point
interrupt is cleared, so why should we wait for the wq?

To play it safe, can we change this as a followup if necessary?

> >  	/* Deassert after I2C adapter clears the interrupt */
> >  	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
> > @@ -212,45 +217,6 @@ void xe_i2c_irq_postinstall(struct xe_device *xe)
> >  	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);
> >  }
> 
> ...
> 
> > -#define XE_I2C_MAX_CLIENTS		3
> > -
> >  #define XE_I2C_EP_COOKIE_DEVICE		0xde
> >  
> >  /* Endpoint Capabilities */
> >  #define XE_I2C_EP_CAP_IRQ		BIT(0)
> >  
> > +enum XE_I2C_CLIENT {
> > +	XE_I2C_CLIENT_AMC,
> > +	XE_I2C_MAX_CLIENTS = 3,
> 
> I know it was already like this but I probably missed why do we have 3
> (atleast from driver POV)?

That is the maximum number of clients these platforms can support.
The AMC address is actually at a fixed offset 1. I'll change this so
that XE_I2C_CLIENT_AMC matches the offset:

enum XE_I2C_CLIENT {
	XE_I2C_CLIENT_AMC = 1,
	XE_I2C_MAX_CLIENTS = 3,
};

That probable makes this a bit more clear (right?).

Thanks,

-- 
heikki

Re: [PATCH v4 2/3] drm/xe/i2c: Fix the interrupt handling

[Raag Jadav]

On Wed, Jul 15, 2026 at 12:53:16PM +0300, Heikki Krogerus wrote:
> On Wed, Jul 15, 2026 at 08:26:22AM +0200, Raag Jadav wrote:
> > On Mon, Jul 13, 2026 at 05:56:00PM +0200, Heikki Krogerus wrote:
> > > The platforms that support the interrupt from the I2C
> > > adapter can not handle the amount of interrupts the adapter
> > > generates because of the way the IRQ is routed in the
> > > hardware. The I2C controller driver has to be kept in
> > > polling mode because of that.
> > > 
> > > The AMC MCU can still generate critical alerts that have to
> > > be handled. The interrupt from SMBus Alert is left enabled
> > > and handled separately in the Xe. The alerts from the AMC
> > > will cause the device to be declared wedged for now.

...

> > > +	    memcmp(&response.message, &request->message, sizeof(struct amc_message))) {
> > > +		dev_err(&client->dev, "response does not match the request\n");

Forgot add, use drm_*() variants where possible.

> > > +		return;
> > > +	}
> > > +
> > > +	if (response.error) {
> > > +		dev_err(&client->dev, "AMC error 0x%02x\n", response.error);

Ditto.

> > > +		return;
> > > +	}
> > > +
> > > +	dev_dbg(&client->dev, "%s: Alert reason: %d\n", __func__, response.value);

Ditto.

> > See below [1].
> > 
> > > +	switch (response.value) {
> > > +	case AMC_ALERT_FW_DOWNLOAD:
> > > +	case AMC_ALERT_THERMAL_TRIP:
> > > +	case AMC_ALERT_OOB_REQUEST:
> > > +	case AMC_ALERT_OOB_RESET:
> > > +	case AMC_ALERT_CATERR:
> > > +		xe_device_declare_wedged(i2c_client_to_xe_device(client));
> > > +		break;
> > > +	default:
> > > +		break;
> > > +	}
> > > +}
> > 
> > ...
> > 
> > > @@ -181,8 +187,7 @@ void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
> > >  	if (!(master_ctl & I2C_IRQ) || !xe_i2c_irq_present(xe))
> > >  		return;
> > >  
> > > -	/* Forward interrupt to I2C adapter */
> > > -	generic_handle_irq_safe(xe->i2c->adapter_irq);
> > > +	xe_i2c_handle_smbus_alert(xe->i2c);
> > 
> > [1] Can we move the below re-assert code to wq now? Or do you suspect any
> > side-effects?
> 
> I think that you know this better than I do. But at this point
> interrupt is cleared, so why should we wait for the wq?

When does AMC clear the alert signal? Is it when you query from the wq?
If the answer is yes, there's a possibility we might end up with an
interrupt storm here.

> To play it safe, can we change this as a followup if necessary?

Sure, I'll leave it to you.

> > >  	/* Deassert after I2C adapter clears the interrupt */
> > >  	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
> > > @@ -212,45 +217,6 @@ void xe_i2c_irq_postinstall(struct xe_device *xe)
> > >  	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);
> > >  }
> > 
> > ...
> > 
> > > -#define XE_I2C_MAX_CLIENTS		3
> > > -
> > >  #define XE_I2C_EP_COOKIE_DEVICE		0xde
> > >  
> > >  /* Endpoint Capabilities */
> > >  #define XE_I2C_EP_CAP_IRQ		BIT(0)
> > >  
> > > +enum XE_I2C_CLIENT {
> > > +	XE_I2C_CLIENT_AMC,
> > > +	XE_I2C_MAX_CLIENTS = 3,
> > 
> > I know it was already like this but I probably missed why do we have 3
> > (atleast from driver POV)?
> 
> That is the maximum number of clients these platforms can support.
> The AMC address is actually at a fixed offset 1. I'll change this so
> that XE_I2C_CLIENT_AMC matches the offset:
> 
> enum XE_I2C_CLIENT {
> 	XE_I2C_CLIENT_AMC = 1,
> 	XE_I2C_MAX_CLIENTS = 3,
> };
> 
> That probable makes this a bit more clear (right?).

With that I think these can be used as ep.addr[] indexes as well, but
I'll leave them at your mercy.

Raag

Re: [PATCH v4 2/3] drm/xe/i2c: Fix the interrupt handling

[Heikki Krogerus]

Hi Raag,

On Wed, Jul 15, 2026 at 01:55:08PM +0200, Raag Jadav wrote:
> On Wed, Jul 15, 2026 at 12:53:16PM +0300, Heikki Krogerus wrote:
> > On Wed, Jul 15, 2026 at 08:26:22AM +0200, Raag Jadav wrote:
> > > On Mon, Jul 13, 2026 at 05:56:00PM +0200, Heikki Krogerus wrote:
> > > > The platforms that support the interrupt from the I2C
> > > > adapter can not handle the amount of interrupts the adapter
> > > > generates because of the way the IRQ is routed in the
> > > > hardware. The I2C controller driver has to be kept in
> > > > polling mode because of that.
> > > > 
> > > > The AMC MCU can still generate critical alerts that have to
> > > > be handled. The interrupt from SMBus Alert is left enabled
> > > > and handled separately in the Xe. The alerts from the AMC
> > > > will cause the device to be declared wedged for now.
> 
> ...
> 
> > > > +	    memcmp(&response.message, &request->message, sizeof(struct amc_message))) {
> > > > +		dev_err(&client->dev, "response does not match the request\n");
> 
> Forgot add, use drm_*() variants where possible.

I used deliberately the dev_*() here so I can use the i2c client
device, which to me feels more appropriate. With drm_*() I would need
to use the drm device, right?

But I can change this if you still prefer the drm_*(). Let me know.

> > > > +		return;
> > > > +	}
> > > > +
> > > > +	if (response.error) {
> > > > +		dev_err(&client->dev, "AMC error 0x%02x\n", response.error);
> 
> Ditto.
> 
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	dev_dbg(&client->dev, "%s: Alert reason: %d\n", __func__, response.value);
> 
> Ditto.
> 
> > > See below [1].
> > > 
> > > > +	switch (response.value) {
> > > > +	case AMC_ALERT_FW_DOWNLOAD:
> > > > +	case AMC_ALERT_THERMAL_TRIP:
> > > > +	case AMC_ALERT_OOB_REQUEST:
> > > > +	case AMC_ALERT_OOB_RESET:
> > > > +	case AMC_ALERT_CATERR:
> > > > +		xe_device_declare_wedged(i2c_client_to_xe_device(client));
> > > > +		break;
> > > > +	default:
> > > > +		break;
> > > > +	}
> > > > +}
> > > 
> > > ...
> > > 
> > > > @@ -181,8 +187,7 @@ void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
> > > >  	if (!(master_ctl & I2C_IRQ) || !xe_i2c_irq_present(xe))
> > > >  		return;
> > > >  
> > > > -	/* Forward interrupt to I2C adapter */
> > > > -	generic_handle_irq_safe(xe->i2c->adapter_irq);
> > > > +	xe_i2c_handle_smbus_alert(xe->i2c);
> > > 
> > > [1] Can we move the below re-assert code to wq now? Or do you suspect any
> > > side-effects?
> > 
> > I think that you know this better than I do. But at this point
> > interrupt is cleared, so why should we wait for the wq?
> 
> When does AMC clear the alert signal? Is it when you query from the wq?
> If the answer is yes, there's a possibility we might end up with an
> interrupt storm here.
> 
> > To play it safe, can we change this as a followup if necessary?
> 
> Sure, I'll leave it to you.

I'll move it to the wq. There are no side-effects.

> > > >  	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
> > > >  	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);

Thanks,

-- 
heikki

Re: [PATCH v4 2/3] drm/xe/i2c: Fix the interrupt handling

[Heikki Krogerus]

Hi Raag,

> > > > > -	/* Forward interrupt to I2C adapter */
> > > > > -	generic_handle_irq_safe(xe->i2c->adapter_irq);
> > > > > +	xe_i2c_handle_smbus_alert(xe->i2c);
> > > > 
> > > > [1] Can we move the below re-assert code to wq now? Or do you suspect any
> > > > side-effects?
> > > 
> > > I think that you know this better than I do. But at this point
> > > interrupt is cleared, so why should we wait for the wq?
> > 
> > When does AMC clear the alert signal? Is it when you query from the wq?
> > If the answer is yes, there's a possibility we might end up with an
> > interrupt storm here.
> > 
> > > To play it safe, can we change this as a followup if necessary?
> > 
> > Sure, I'll leave it to you.
> 
> I'll move it to the wq. There are no side-effects.

Can you check does this work:

diff --git a/drivers/gpu/drm/xe/xe_amc.c b/drivers/gpu/drm/xe/xe_amc.c
index b44d5765f2ed5..1a517b79d0bb3 100644
--- a/drivers/gpu/drm/xe/xe_amc.c
+++ b/drivers/gpu/drm/xe/xe_amc.c
@@ -11,9 +11,12 @@
 #include <linux/string.h>
 #include <linux/workqueue.h>
 
+#include "regs/xe_i2c_regs.h"
+
 #include "xe_amc.h"
 #include "xe_device.h"
 #include "xe_i2c.h"
+#include "xe_mmio.h"
 
 /**
  * DOC: Add-In Management Controller (AMC)
@@ -103,7 +106,7 @@ static void xe_amc_work(struct work_struct *work)
        ret = i2c_master_send(client, (u8 *)request, sizeof(*request));
        if (ret < 0) {
                dev_err(&client->dev, "failed to send request (%d)\n", ret);
-               return;
+               goto out_reassert_interrupt;
        }
 
        /* AMC needs 20ms to generate the response. */
@@ -112,22 +115,22 @@ static void xe_amc_work(struct work_struct *work)
        ret = i2c_master_recv(client, (u8 *)&response, sizeof(response));
        if (ret < 0) {
                dev_err(&client->dev, "failed to read response (%d)\n", ret);
-               return;
+               goto out_reassert_interrupt;
        }
 
        if (!response.header.len) {
                dev_err(&client->dev, "empty response from AMC\n");
-               return;
+               goto out_reassert_interrupt;
        }
 
        if (memcmp(&response.message, &request->message, sizeof(struct amc_message))) {
                dev_err(&client->dev, "response does not match the request\n");
-               return;
+               goto out_reassert_interrupt;
        }
 
        if (response.error) {
                dev_err(&client->dev, "AMC error 0x%02x\n", response.error);
-               return;
+               goto out_reassert_interrupt;
        }
 
        dev_dbg(&client->dev, "%s: Alert reason: %d\n", __func__, response.value);
@@ -143,12 +146,19 @@ static void xe_amc_work(struct work_struct *work)
        default:
                break;
        }
+
+out_reassert_interrupt:
+       xe_mmio_rmw32(amc->i2c->mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);
 }
 
 void xe_amc_handle_alert(struct xe_i2c *i2c)
 {
+       xe_mmio_rmw32(i2c->mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
+
        if (i2c->client[XE_I2C_CLIENT_AMC])
                queue_work(system_long_wq, &i2c->amc->work);
+       else
+               xe_mmio_rmw32(i2c->mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);
 }
 
 int xe_amc_init(struct xe_i2c *i2c)
diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index 8a978526e2b66..3859d4a9f75e4 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -87,6 +87,8 @@ static void xe_i2c_client_work(struct work_struct *work)
        };
 
        i2c->client[XE_I2C_CLIENT_AMC] = i2c_new_client_device(i2c->adapter, &info);
+
+       xe_i2c_irq_postinstall(i2c_adapter_to_xe_device(i2c->adapter));
 }
 
 static int xe_i2c_notifier(struct notifier_block *nb, unsigned long action, void *data)
@@ -183,17 +185,10 @@ static bool xe_i2c_irq_present(struct xe_device *xe)
  */
 void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
 {
-       struct xe_mmio *mmio = xe_root_tile_mmio(xe);
-
        if (!(master_ctl & I2C_IRQ) || !xe_i2c_irq_present(xe))
                return;
 
        xe_i2c_handle_smbus_alert(xe->i2c);
-
-       /* Deassert after I2C adapter clears the interrupt */
-       xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE);
-       /* Reassert to allow subsequent interrupt generation */
-       xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);
 }
 
 void xe_i2c_irq_reset(struct xe_device *xe)
@@ -392,6 +387,5 @@ int xe_i2c_probe(struct xe_device *xe)
                return ret;
        }
 
-       xe_i2c_irq_postinstall(xe);
        return devm_add_action_or_reset(drm_dev, xe_i2c_remove, i2c);
 }

Thanks,

-- 
heikki

Re: [PATCH v4 2/3] drm/xe/i2c: Fix the interrupt handling

[Heikki Krogerus]

> Hi Raag,
> 
> > > > > > -	/* Forward interrupt to I2C adapter */
> > > > > > -	generic_handle_irq_safe(xe->i2c->adapter_irq);
> > > > > > +	xe_i2c_handle_smbus_alert(xe->i2c);
> > > > > 
> > > > > [1] Can we move the below re-assert code to wq now? Or do you suspect any
> > > > > side-effects?
> > > > 
> > > > I think that you know this better than I do. But at this point
> > > > interrupt is cleared, so why should we wait for the wq?
> > > 
> > > When does AMC clear the alert signal? Is it when you query from the wq?
> > > If the answer is yes, there's a possibility we might end up with an
> > > interrupt storm here.
> > > 
> > > > To play it safe, can we change this as a followup if necessary?
> > > 
> > > Sure, I'll leave it to you.
> > 
> > I'll move it to the wq. There are no side-effects.
> 
> Can you check does this work:

Nevermind. I'll send the next version.

Thanks,

-- 
heikki

[PATCH v4 3/3] drm/xe/i2c: Keep the i2c controller always enabled

[Heikki Krogerus]

Some platforms make an assumption that the i2c controller's
enabled state indicates also the power state of the
controller. This can create a problem when the controller is
in disabled state, because the hardware may assume
incorrectly that it is then also in low-power state.

To fix this, the controller is kept enabled by taking over
the IC_ENABLE register. The controller has to be disabled
when the configuration is updated and when the target
address or the slave address are assigned, so disabling it
when IC_CON, IC_TAR or IC_SAR registers are programmed, and
then re-enabling it again.

Fixes: f0e53aadd702 ("drm/xe: Support for I2C attached MCUs")
Cc: stable@vger.kernel.org
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/xe/xe_i2c.c | 55 +++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/xe/xe_i2c.h |  1 +
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index 7fa1b16598ee6..956a50dc4ef31 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -8,6 +8,7 @@
 #include <drm/drm_print.h>
 #include <linux/array_size.h>
 #include <linux/container_of.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
@@ -217,11 +218,40 @@ void xe_i2c_irq_postinstall(struct xe_device *xe)
 	xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, PCI_COMMAND_INTX_DISABLE, 0);
 }
 
+/* See "Disabling DW_apb_i2c" in the DesignWare DW_abp_i2c databook. */
+static void xe_i2c_disable(struct xe_i2c *i2c)
+{
+	int timeout = 100;
+	u32 status;
+
+	xe_mmio_rmw32(i2c->mmio, I2C_REG(DW_IC_ENABLE), 1, 0);
+
+	do {
+		status = xe_mmio_read32(i2c->mmio, I2C_REG(DW_IC_ENABLE_STATUS));
+		if (!(status & 1))
+			return;
+		/* Can't sleep here. */
+		udelay(25);
+	} while (timeout--);
+
+	dev_warn(&i2c->adapter->dev, "timeout in disabling adapter\n");
+}
+
 static int xe_i2c_read(void *context, unsigned int reg, unsigned int *val)
 {
 	struct xe_i2c *i2c = context;
 
-	*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
+	*val = xe_mmio_read32(i2c->mmio, I2C_REG(reg));
+
+	switch (reg) {
+	case DW_IC_ENABLE:
+	case DW_IC_ENABLE_STATUS:
+		FIELD_MODIFY(DW_IC_ENABLE_ENABLE, val,
+			     i2c->ic_enable & DW_IC_ENABLE_ENABLE);
+		break;
+	default:
+		break;
+	}
 
 	return 0;
 }
@@ -230,7 +260,28 @@ static int xe_i2c_write(void *context, unsigned int reg, unsigned int val)
 {
 	struct xe_i2c *i2c = context;
 
-	xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
+	switch (reg) {
+	case DW_IC_CON:
+	case DW_IC_TAR:
+	case DW_IC_SAR:
+		/* Disable the controller. */
+		xe_i2c_disable(i2c);
+
+		/* Write the register. */
+		xe_mmio_write32(i2c->mmio, I2C_REG(reg), val);
+
+		/* Enable the controller. */
+		xe_mmio_rmw32(i2c->mmio, I2C_REG(DW_IC_ENABLE), 0, 1);
+		break;
+	case DW_IC_ENABLE:
+		i2c->ic_enable = val;
+		/* Other fields can be updated except the enable bit. */
+		val |= DW_IC_ENABLE_ENABLE;
+		fallthrough;
+	default:
+		xe_mmio_write32(i2c->mmio, I2C_REG(reg), val);
+		break;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
index c95f98c2053d5..2cd17b5726289 100644
--- a/drivers/gpu/drm/xe/xe_i2c.h
+++ b/drivers/gpu/drm/xe/xe_i2c.h
@@ -37,6 +37,7 @@ struct xe_i2c {
 	struct platform_device *pdev;
 	struct i2c_adapter *adapter;
 	struct i2c_client *client[XE_I2C_MAX_CLIENTS];
+	unsigned int ic_enable;
 
 	struct notifier_block bus_notifier;
 	struct work_struct work;
-- 
2.50.1

Re: [PATCH v4 3/3] drm/xe/i2c: Keep the i2c controller always enabled

[Raag Jadav]

On Mon, Jul 13, 2026 at 05:56:01PM +0200, Heikki Krogerus wrote:
> Some platforms make an assumption that the i2c controller's
> enabled state indicates also the power state of the
> controller. This can create a problem when the controller is
> in disabled state, because the hardware may assume
> incorrectly that it is then also in low-power state.
> 
> To fix this, the controller is kept enabled by taking over
> the IC_ENABLE register. The controller has to be disabled
> when the configuration is updated and when the target
> address or the slave address are assigned, so disabling it
> when IC_CON, IC_TAR or IC_SAR registers are programmed, and
> then re-enabling it again.

...

> +/* See "Disabling DW_apb_i2c" in the DesignWare DW_abp_i2c databook. */
> +static void xe_i2c_disable(struct xe_i2c *i2c)
> +{
> +	int timeout = 100;
> +	u32 status;
> +
> +	xe_mmio_rmw32(i2c->mmio, I2C_REG(DW_IC_ENABLE), 1, 0);

Can we use DW_IC_ENABLE_* defines?

> +	do {
> +		status = xe_mmio_read32(i2c->mmio, I2C_REG(DW_IC_ENABLE_STATUS));
> +		if (!(status & 1))

Ditto for DW_IC_STATUS_*.

> +			return;
> +		/* Can't sleep here. */
> +		udelay(25);
> +	} while (timeout--);
> +
> +	dev_warn(&i2c->adapter->dev, "timeout in disabling adapter\n");
> +}

...

> @@ -230,7 +260,28 @@ static int xe_i2c_write(void *context, unsigned int reg, unsigned int val)
>  {
>  	struct xe_i2c *i2c = context;
>  
> -	xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
> +	switch (reg) {
> +	case DW_IC_CON:
> +	case DW_IC_TAR:
> +	case DW_IC_SAR:
> +		/* Disable the controller. */
> +		xe_i2c_disable(i2c);
> +
> +		/* Write the register. */
> +		xe_mmio_write32(i2c->mmio, I2C_REG(reg), val);
> +
> +		/* Enable the controller. */
> +		xe_mmio_rmw32(i2c->mmio, I2C_REG(DW_IC_ENABLE), 0, 1);

Ditto.

Raag

> +		break;
> +	case DW_IC_ENABLE:
> +		i2c->ic_enable = val;
> +		/* Other fields can be updated except the enable bit. */
> +		val |= DW_IC_ENABLE_ENABLE;
> +		fallthrough;
> +	default:
> +		xe_mmio_write32(i2c->mmio, I2C_REG(reg), val);
> +		break;
> +	}
>  
>  	return 0;
>  }

Re: [PATCH v4 3/3] drm/xe/i2c: Keep the i2c controller always enabled

[Heikki Krogerus]

On Wed, Jul 15, 2026 at 08:33:31AM +0200, Raag Jadav wrote:
> > +/* See "Disabling DW_apb_i2c" in the DesignWare DW_abp_i2c databook. */
> > +static void xe_i2c_disable(struct xe_i2c *i2c)
> > +{
> > +	int timeout = 100;
> > +	u32 status;
> > +
> > +	xe_mmio_rmw32(i2c->mmio, I2C_REG(DW_IC_ENABLE), 1, 0);
> 
> Can we use DW_IC_ENABLE_* defines?
> 
> > +	do {
> > +		status = xe_mmio_read32(i2c->mmio, I2C_REG(DW_IC_ENABLE_STATUS));
> > +		if (!(status & 1))
> 
> Ditto for DW_IC_STATUS_*.
> 
> > +			return;
> > +		/* Can't sleep here. */
> > +		udelay(25);
> > +	} while (timeout--);
> > +
> > +	dev_warn(&i2c->adapter->dev, "timeout in disabling adapter\n");
> > +}
> 
> ...
> 
> > @@ -230,7 +260,28 @@ static int xe_i2c_write(void *context, unsigned int reg, unsigned int val)
> >  {
> >  	struct xe_i2c *i2c = context;
> >  
> > -	xe_mmio_write32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET), val);
> > +	switch (reg) {
> > +	case DW_IC_CON:
> > +	case DW_IC_TAR:
> > +	case DW_IC_SAR:
> > +		/* Disable the controller. */
> > +		xe_i2c_disable(i2c);
> > +
> > +		/* Write the register. */
> > +		xe_mmio_write32(i2c->mmio, I2C_REG(reg), val);
> > +
> > +		/* Enable the controller. */
> > +		xe_mmio_rmw32(i2c->mmio, I2C_REG(DW_IC_ENABLE), 0, 1);
> 
> Ditto.

Yes to all three. I'll fix these.

Thanks,

-- 
heikki

Re: [PATCH v4 0/3] drm/xe/i2c: alerts and controller enabling modifications

[Rodrigo Vivi]

On Mon, Jul 13, 2026 at 05:55:58PM +0200, Heikki Krogerus wrote:
> Hi,
> 
> The hardware challenges that these patches address are so severe that I'm
> marking both of them as fixes. In both cases the GPU may silently end up in
> unresponsive state (or worse). The second patch has been refactored so that it
> includes the direct AMC alert handling in Xe instead of the normal alert handler
> registration. The subject lines were also changed to highlight the fact that
> these are fixes. Ramesh helped me with the testing and with the implementation
> for the AMC alert handling.
> 
> Changed since v2:
> - Added Fixes tag to both patches.
> - i2c-designware is no longer supplied with an interrupt so it will be in
>   polling mode (ACCESS_POLLING will be enabled). The IRQ path in hardware can't
>   handle the amount of interrupts the i2c controller generates. Only the
>   interrupts from the SMBus Alert line are left enabled.
> - The registration of the default smbus alert handler is dropped.
> - The AMC alerts are handled directly in Xe. All the alerts will cause the
>   device to be declared as wedged at least for now.
> - Cleanups proposed by Raag.
> 
> v2: https://lore.kernel.org/lkml/20260625125939.429078-1-heikki.krogerus@linux.intel.com/
> 
> Changed since v1:
> - Global header for the DesignWare I2C registers which meant a bit of
>   patch refactoring.
> - Selecting CONFIG_SMBUS in CONFIG_XE and handling smbus in xe_i2c.c instead of
>   separate file.
> - Storing the alert device to the client array and providing enum for the
>   clients.
> - Allowing other fields in the IC_ENABLE register to be updated except the
>   Enable bit.
> - Can't sleep in xe_i2c_disable() so using udelay().
> 
> v1: https://lore.kernel.org/lkml/20260622114759.3464047-1-heikki.krogerus@linux.intel.com/
> 
> This includes support for the SMBus alerts, and special handling for the
> IC_ENABLE register.
> 
> Thanks,


Please take a look to Shashiko review and let us know in case of false positives:
https://sashiko.dev/#/patchset/20260713155601.711389-1-heikki.krogerus%40linux.intel.com

> 
> Heikki Krogerus (3):
>   i2c: designware: Global register definitions
>   drm/xe/i2c: Fix the interrupt handling
>   drm/xe/i2c: Keep the i2c controller always enabled
> 
>  MAINTAINERS                                |   1 +
>  drivers/gpu/drm/xe/Makefile                |   4 +-
>  drivers/gpu/drm/xe/regs/xe_i2c_regs.h      |   2 +
>  drivers/gpu/drm/xe/xe_amc.c                | 173 +++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_amc.h                |  25 +++
>  drivers/gpu/drm/xe/xe_i2c.c                | 136 +++++++++-------
>  drivers/gpu/drm/xe/xe_i2c.h                |  14 +-
>  drivers/i2c/busses/i2c-designware-common.c |   2 +
>  drivers/i2c/busses/i2c-designware-core.h   |  85 +---------
>  drivers/i2c/busses/i2c-designware-master.c |   2 +
>  drivers/i2c/busses/i2c-designware-slave.c  |   2 +
>  include/linux/designware_i2c.h             | 107 +++++++++++++
>  12 files changed, 405 insertions(+), 148 deletions(-)
>  create mode 100644 drivers/gpu/drm/xe/xe_amc.c
>  create mode 100644 drivers/gpu/drm/xe/xe_amc.h
>  create mode 100644 include/linux/designware_i2c.h
> 
> -- 
> 2.50.1
> 

Re: [PATCH v4 0/3] drm/xe/i2c: alerts and controller enabling modifications

[Heikki Krogerus]

Hi Rodrigo,

On Tue, Jul 14, 2026 at 11:54:16AM -0400, Rodrigo Vivi kirjoitti:
> On Mon, Jul 13, 2026 at 05:55:58PM +0200, Heikki Krogerus wrote:
> > Hi,
> > 
> > The hardware challenges that these patches address are so severe that I'm
> > marking both of them as fixes. In both cases the GPU may silently end up in
> > unresponsive state (or worse). The second patch has been refactored so that it
> > includes the direct AMC alert handling in Xe instead of the normal alert handler
> > registration. The subject lines were also changed to highlight the fact that
> > these are fixes. Ramesh helped me with the testing and with the implementation
> > for the AMC alert handling.
> > 
> > Changed since v2:
> > - Added Fixes tag to both patches.
> > - i2c-designware is no longer supplied with an interrupt so it will be in
> >   polling mode (ACCESS_POLLING will be enabled). The IRQ path in hardware can't
> >   handle the amount of interrupts the i2c controller generates. Only the
> >   interrupts from the SMBus Alert line are left enabled.
> > - The registration of the default smbus alert handler is dropped.
> > - The AMC alerts are handled directly in Xe. All the alerts will cause the
> >   device to be declared as wedged at least for now.
> > - Cleanups proposed by Raag.
> > 
> > v2: https://lore.kernel.org/lkml/20260625125939.429078-1-heikki.krogerus@linux.intel.com/
> > 
> > Changed since v1:
> > - Global header for the DesignWare I2C registers which meant a bit of
> >   patch refactoring.
> > - Selecting CONFIG_SMBUS in CONFIG_XE and handling smbus in xe_i2c.c instead of
> >   separate file.
> > - Storing the alert device to the client array and providing enum for the
> >   clients.
> > - Allowing other fields in the IC_ENABLE register to be updated except the
> >   Enable bit.
> > - Can't sleep in xe_i2c_disable() so using udelay().
> > 
> > v1: https://lore.kernel.org/lkml/20260622114759.3464047-1-heikki.krogerus@linux.intel.com/
> > 
> > This includes support for the SMBus alerts, and special handling for the
> > IC_ENABLE register.
> > 
> > Thanks,
> 
> 
> Please take a look to Shashiko review and let us know in case of false positives:
> https://sashiko.dev/#/patchset/20260713155601.711389-1-heikki.krogerus%40linux.intel.com

The high ones are false positive.

- The SMBus interrupts are not cleared the same way as the other
  interrupts.
- i2c-designware driver does not modify the timings unless it is
  supplied a clock device (struct clk).

The rest I'll check and fix as needed together with the modifications
proposed by Raag.

Thanks,

-- 
heikki