[PATCH v1 0/2] drm/xe: i2c support

[PATCH v1 0/2] drm/xe: i2c support

[Heikki Krogerus]

Hi,

Some of the future "data center GPUs" will provide access to the
on-board Synopsys DesignWare I2C host adapter. The i2c is used to
connect various microcontrollers. The initially supported
microcontroller unit is called Add-In Management Controller (AMC).

Thanks,

Heikki Krogerus (2):
  i2c: designware: Add quirk for Intel Xe
  drm/xe: Support for I2C attached MCUs

 drivers/gpu/drm/xe/Makefile                 |   1 +
 drivers/gpu/drm/xe/regs/xe_i2c_regs.h       |  16 ++
 drivers/gpu/drm/xe/regs/xe_irq_regs.h       |   1 +
 drivers/gpu/drm/xe/xe_device.c              |   3 +
 drivers/gpu/drm/xe/xe_device_types.h        |   4 +
 drivers/gpu/drm/xe/xe_i2c.c                 | 270 ++++++++++++++++++++
 drivers/gpu/drm/xe/xe_i2c.h                 |  60 +++++
 drivers/gpu/drm/xe/xe_irq.c                 |   2 +
 drivers/i2c/busses/i2c-designware-platdrv.c |  18 +-
 9 files changed, 369 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/regs/xe_i2c_regs.h
 create mode 100644 drivers/gpu/drm/xe/xe_i2c.c
 create mode 100644 drivers/gpu/drm/xe/xe_i2c.h

-- 
2.47.2

[PATCH v1 1/2] i2c: designware: Add quirk for Intel Xe

[Heikki Krogerus]

The regmap is coming from the parent also in case of Xe
GPUs. Reusing the Wangxun quirk for that.

Originally-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 879719e91df2..a417e571197a 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -101,7 +101,7 @@ static int bt1_i2c_request_regs(struct dw_i2c_dev *dev)
 }
 #endif
 
-static int txgbe_i2c_request_regs(struct dw_i2c_dev *dev)
+static int dw_i2c_get_parent_regmap(struct dw_i2c_dev *dev)
 {
 	dev->map = dev_get_regmap(dev->dev->parent, NULL);
 	if (!dev->map)
@@ -123,12 +123,15 @@ static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev)
 	struct platform_device *pdev = to_platform_device(dev->dev);
 	int ret;
 
+	if (device_is_compatible(dev->dev, "intel,xe-mcu-i2c"))
+		return dw_i2c_get_parent_regmap(dev);
+
 	switch (dev->flags & MODEL_MASK) {
 	case MODEL_BAIKAL_BT1:
 		ret = bt1_i2c_request_regs(dev);
 		break;
 	case MODEL_WANGXUN_SP:
-		ret = txgbe_i2c_request_regs(dev);
+		ret = dw_i2c_get_parent_regmap(dev);
 		break;
 	default:
 		dev->base = devm_platform_ioremap_resource(pdev, 0);
@@ -205,25 +208,28 @@ static void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
 
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
+	u32 flags = (uintptr_t)device_get_match_data(&pdev->dev);
 	struct device *device = &pdev->dev;
 	struct i2c_adapter *adap;
 	struct dw_i2c_dev *dev;
 	int irq, ret;
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
+	irq = platform_get_irq_optional(pdev, 0);
+	if (irq == -ENXIO)
+		flags |= ACCESS_POLLING;
+	else if (irq < 0)
 		return irq;
 
 	dev = devm_kzalloc(device, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
 
-	dev->flags = (uintptr_t)device_get_match_data(device);
 	if (device_property_present(device, "wx,i2c-snps-model"))
-		dev->flags = MODEL_WANGXUN_SP | ACCESS_POLLING;
+		flags = MODEL_WANGXUN_SP | ACCESS_POLLING;
 
 	dev->dev = device;
 	dev->irq = irq;
+	dev->flags = flags;
 	platform_set_drvdata(pdev, dev);
 
 	ret = dw_i2c_plat_request_regs(dev);
-- 
2.47.2

[PATCH v1 2/2] drm/xe: Support for I2C attached MCUs

[Heikki Krogerus]

Adding adaption/glue layer where the I2C host adapter
(Synopsys DesignWare I2C adapter) and the I2C clients (the
microcontroller units) are enumerated.

The microcontroller units (MCU) that are attached to the GPU
depend on the OEM. The initially supported MCU will be the
Add-In Management Controller (AMC).

Originally-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/gpu/drm/xe/Makefile           |   1 +
 drivers/gpu/drm/xe/regs/xe_i2c_regs.h |  16 ++
 drivers/gpu/drm/xe/regs/xe_irq_regs.h |   1 +
 drivers/gpu/drm/xe/xe_device.c        |   3 +
 drivers/gpu/drm/xe/xe_device_types.h  |   4 +
 drivers/gpu/drm/xe/xe_i2c.c           | 270 ++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_i2c.h           |  60 ++++++
 drivers/gpu/drm/xe/xe_irq.c           |   2 +
 8 files changed, 357 insertions(+)
 create mode 100644 drivers/gpu/drm/xe/regs/xe_i2c_regs.h
 create mode 100644 drivers/gpu/drm/xe/xe_i2c.c
 create mode 100644 drivers/gpu/drm/xe/xe_i2c.h

diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index e4bf484d4121..3f581e2bb564 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -124,6 +124,7 @@ xe-y += xe_bb.o \
 	xe_wait_user_fence.o \
 	xe_wopcm.o
 
+xe-$(CONFIG_I2C)	+= xe_i2c.o
 xe-$(CONFIG_HMM_MIRROR) += xe_hmm.o
 xe-$(CONFIG_DRM_XE_GPUSVM) += xe_svm.o
 
diff --git a/drivers/gpu/drm/xe/regs/xe_i2c_regs.h b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
new file mode 100644
index 000000000000..2acb55eeef0d
--- /dev/null
+++ b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _XE_I2C_REGS_H_
+#define _XE_I2C_REGS_H_
+
+#include "xe_reg_defs.h"
+
+#define SOC_BASE			0x280000
+
+#define I2C_CONFIG_SPACE_OFFSET		(SOC_BASE + 0xf6000)
+#define I2C_MEM_SPACE_OFFSET		(SOC_BASE + 0xf7400)
+#define I2C_BRIDGE_OFFSET		(SOC_BASE + 0xd9000)
+
+#define CLIENT_DISC_COOKIE		XE_REG(SOC_BASE + 0x0164)
+#define CLIENT_DISC_ADDRESS		XE_REG(SOC_BASE + 0x0168)
+
+#endif /* _XE_I2C_REGS_H_ */
diff --git a/drivers/gpu/drm/xe/regs/xe_irq_regs.h b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
index f0ecfcac4003..13635e4331d4 100644
--- a/drivers/gpu/drm/xe/regs/xe_irq_regs.h
+++ b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
@@ -19,6 +19,7 @@
 #define   MASTER_IRQ				REG_BIT(31)
 #define   GU_MISC_IRQ				REG_BIT(29)
 #define   DISPLAY_IRQ				REG_BIT(16)
+#define   I2C_IRQ				REG_BIT(12)
 #define   GT_DW_IRQ(x)				REG_BIT(x)
 
 /*
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index d4b6e623aa48..dd635101420b 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -42,6 +42,7 @@
 #include "xe_guc.h"
 #include "xe_hw_engine_group.h"
 #include "xe_hwmon.h"
+#include "xe_i2c.h"
 #include "xe_irq.h"
 #include "xe_memirq.h"
 #include "xe_mmio.h"
@@ -929,6 +930,8 @@ int xe_device_probe(struct xe_device *xe)
 
 	xe_vsec_init(xe);
 
+	xe_i2c_probe(xe);
+
 	return devm_add_action_or_reset(xe->drm.dev, xe_device_sanitize, xe);
 
 err_unregister_display:
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 50b2bfa682ac..7011a078b9c7 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -36,6 +36,7 @@
 #endif
 
 struct xe_ggtt;
+struct xe_i2c;
 struct xe_pat_ops;
 struct xe_pxp;
 
@@ -572,6 +573,9 @@ struct xe_device {
 	/** @pmu: performance monitoring unit */
 	struct xe_pmu pmu;
 
+	/** @i2c: I2C host controller */
+	struct xe_i2c *i2c;
+
 	/** @atomic_svm_timeslice_ms: Atomic SVM fault timeslice MS */
 	u32 atomic_svm_timeslice_ms;
 
diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
new file mode 100644
index 000000000000..cc231368789f
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel Xe I2C attached Microcontroller Units (MCU)
+ *
+ * Copyright (C) 2025 Intel Corporation.
+ */
+
+#include <linux/array_size.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#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>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/sprintf.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+#include "regs/xe_i2c_regs.h"
+#include "regs/xe_irq_regs.h"
+
+#include "xe_device.h"
+#include "xe_device_types.h"
+#include "xe_i2c.h"
+#include "xe_mmio.h"
+#include "xe_platform_types.h"
+
+/* Synopsys DesignWare I2C Host Adapter */
+static const char adapter_name[] = "i2c_designware";
+
+static const struct property_entry xe_i2c_adapter_properties[] = {
+	PROPERTY_ENTRY_STRING("compatible", "intel,xe-mcu-i2c"),
+	PROPERTY_ENTRY_U32("clock-frequency", I2C_MAX_FAST_MODE_PLUS_FREQ),
+	{ }
+};
+
+static inline void xe_i2c_read_endpoint(struct xe_mmio *mmio, void *ep)
+{
+	u32 *val = ep;
+
+	val[0] = xe_mmio_read32(mmio, CLIENT_DISC_COOKIE);
+	val[1] = xe_mmio_read32(mmio, CLIENT_DISC_ADDRESS);
+}
+
+static void xe_i2c_client_work(struct work_struct *work)
+{
+	struct xe_i2c *i2c = container_of(work, struct xe_i2c, work);
+	struct i2c_board_info info = {
+		.type	= "amc",
+		.flags	= I2C_CLIENT_HOST_NOTIFY,
+		.addr	= i2c->ep.addr[1],
+	};
+
+	i2c->client[0] = i2c_new_client_device(i2c->adapter, &info);
+}
+
+static int xe_i2c_notifier(struct notifier_block *nb, unsigned long action, void *data)
+{
+	struct xe_i2c *i2c = container_of(nb, struct xe_i2c, bus_notifier);
+	struct i2c_adapter *adapter = i2c_verify_adapter(data);
+	struct device *dev = data;
+
+	if (action == BUS_NOTIFY_ADD_DEVICE &&
+	    adapter && dev->parent == &i2c->pdev->dev) {
+		i2c->adapter = adapter;
+		schedule_work(&i2c->work);
+		return NOTIFY_OK;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static int xe_i2c_register_adapter(struct xe_i2c *i2c)
+{
+	struct pci_dev *pci = to_pci_dev(i2c->drm_dev);
+	struct platform_device *pdev;
+	struct fwnode_handle *fwnode;
+	int ret;
+
+	fwnode = fwnode_create_software_node(xe_i2c_adapter_properties, NULL);
+	if (!fwnode)
+		return -ENOMEM;
+
+	/*
+	 * Not using platform_device_register_full() here because we don't have
+	 * a handle to the platform_device before it returns. xe_i2c_notifier()
+	 * uses that handle, but it may be called before
+	 * platform_device_register_full() is done.
+	 */
+	pdev = platform_device_alloc(adapter_name, pci_dev_id(pci));
+	if (!pdev) {
+		ret = -ENOMEM;
+		goto err_fwnode_remove;
+	}
+
+	if (i2c->adapter_irq) {
+		struct resource	res = { };
+
+		res.start = i2c->adapter_irq;
+		res.name = "xe_i2c";
+		res.flags = IORESOURCE_IRQ;
+
+		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;
+	i2c->pdev = pdev;
+
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto err_pdev_put;
+
+	return 0;
+
+err_pdev_put:
+	platform_device_put(pdev);
+err_fwnode_remove:
+	fwnode_remove_software_node(fwnode);
+
+	return ret;
+}
+
+static void xe_i2c_unregister_adapter(struct xe_i2c *i2c)
+{
+	platform_device_unregister(i2c->pdev);
+	fwnode_remove_software_node(i2c->adapter_node);
+}
+
+void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
+{
+	if (!xe->i2c || !xe->i2c->adapter_irq)
+		return;
+
+	if (master_ctl & I2C_IRQ)
+		generic_handle_irq_safe(xe->i2c->adapter_irq);
+}
+
+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_i2c *i2c)
+{
+	struct irq_domain *domain;
+
+	if (!(i2c->ep.capabilities & XE_I2C_EP_CAP_IRQ))
+		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) {
+		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;
+
+	*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
+
+	return 0;
+}
+
+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);
+
+	return 0;
+}
+
+static const struct regmap_config i2c_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_read = xe_i2c_read,
+	.reg_write = xe_i2c_write,
+	.fast_io = true,
+};
+
+static void xe_i2c_remove(void *data)
+{
+	struct xe_i2c *i2c = data;
+	int i;
+
+	for (i = 0; i < XE_I2C_MAX_CLIENTS; i++)
+		i2c_unregister_device(i2c->client[i]);
+
+	xe_i2c_unregister_adapter(i2c);
+	xe_i2c_remove_irq(i2c);
+	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
+}
+
+int xe_i2c_probe(struct xe_device *xe)
+{
+	struct xe_i2c_endpoint ep;
+	struct regmap *regmap;
+	struct xe_i2c *i2c;
+	int ret;
+
+	if (xe->info.platform != XE_BATTLEMAGE)
+		return 0;
+
+	xe_i2c_read_endpoint(xe_root_tile_mmio(xe), &ep);
+	if (ep.cookie != XE_I2C_EP_COOKIE_DEVICE)
+		return 0;
+
+	i2c = devm_kzalloc(xe->drm.dev, sizeof(*i2c), GFP_KERNEL);
+	if (!i2c)
+		return -ENOMEM;
+
+	INIT_WORK(&i2c->work, xe_i2c_client_work);
+	i2c->mmio = xe_root_tile_mmio(xe);
+	i2c->drm_dev = xe->drm.dev;
+	i2c->ep = ep;
+
+	regmap = devm_regmap_init(i2c->drm_dev, NULL, i2c, &i2c_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	i2c->bus_notifier.notifier_call = xe_i2c_notifier;
+	ret = bus_register_notifier(&i2c_bus_type, &i2c->bus_notifier);
+	if (ret)
+		return ret;
+
+	ret = xe_i2c_create_irq(i2c);
+	if (ret)
+		goto err_unregister_notifier;
+
+	ret = xe_i2c_register_adapter(i2c);
+	if (ret)
+		goto err_unregister_notifier;
+
+	return devm_add_action_or_reset(i2c->drm_dev, xe_i2c_remove, i2c);
+
+err_unregister_notifier:
+	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
new file mode 100644
index 000000000000..ab8e21630838
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_i2c.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _XE_I2C_H_
+#define _XE_I2C_H_
+
+#include <linux/bits.h>
+#include <linux/notifier.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+struct device;
+struct fwnode_handle;
+struct i2c_adapter;
+struct i2c_client;
+struct irq_domain;
+struct platform_device;
+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)
+#define XE_I2C_EP_CAP_MULTI_MCU		BIT(1)
+#define XE_I2C_EP_CAP_SMBUS		BIT(2)
+
+struct xe_i2c_endpoint {
+	u8 cookie;
+	u8 capabilities;
+	u16 addr[XE_I2C_MAX_CLIENTS];
+};
+
+struct xe_i2c {
+	struct fwnode_handle *adapter_node;
+	struct platform_device *pdev;
+	struct i2c_adapter *adapter;
+	struct i2c_client *client[XE_I2C_MAX_CLIENTS];
+
+	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;
+};
+
+#if IS_ENABLED(CONFIG_I2C)
+int xe_i2c_probe(struct xe_device *xe);
+void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl);
+#else
+static inline int xe_i2c_probe(struct xe_device *xe) { return 0; }
+static inline void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl) { }
+#endif
+
+#endif
diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
index 5362d3174b06..c43e62dc692e 100644
--- a/drivers/gpu/drm/xe/xe_irq.c
+++ b/drivers/gpu/drm/xe/xe_irq.c
@@ -18,6 +18,7 @@
 #include "xe_gt.h"
 #include "xe_guc.h"
 #include "xe_hw_engine.h"
+#include "xe_i2c.h"
 #include "xe_memirq.h"
 #include "xe_mmio.h"
 #include "xe_pxp.h"
@@ -476,6 +477,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
 			if (xe->info.has_heci_cscfi)
 				xe_heci_csc_irq_handler(xe, master_ctl);
 			xe_display_irq_handler(xe, master_ctl);
+			xe_i2c_irq_handler(xe, master_ctl);
 			gu_misc_iir = gu_misc_irq_ack(xe, master_ctl);
 		}
 	}
-- 
2.47.2

Re: [PATCH v1 2/2] drm/xe: Support for I2C attached MCUs

[Raag Jadav]

On Fri, May 30, 2025 at 05:17:44PM +0300, Heikki Krogerus wrote:
> Adding adaption/glue layer where the I2C host adapter
> (Synopsys DesignWare I2C adapter) and the I2C clients (the
> microcontroller units) are enumerated.
> 
> The microcontroller units (MCU) that are attached to the GPU
> depend on the OEM. The initially supported MCU will be the
> Add-In Management Controller (AMC).

...

> diff --git a/drivers/gpu/drm/xe/regs/xe_i2c_regs.h b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
> new file mode 100644
> index 000000000000..2acb55eeef0d
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _XE_I2C_REGS_H_
> +#define _XE_I2C_REGS_H_
> +
> +#include "xe_reg_defs.h"
> +
> +#define SOC_BASE			0x280000

I just noticed we already have this in xe_pmt.h, so let's not duplicate
it. Perhaps move it to a common header (xe_regs.h) and reuse it in both
places?

> +#define I2C_CONFIG_SPACE_OFFSET		(SOC_BASE + 0xf6000)
> +#define I2C_MEM_SPACE_OFFSET		(SOC_BASE + 0xf7400)
> +#define I2C_BRIDGE_OFFSET		(SOC_BASE + 0xd9000)
> +
> +#define CLIENT_DISC_COOKIE		XE_REG(SOC_BASE + 0x0164)
> +#define CLIENT_DISC_ADDRESS		XE_REG(SOC_BASE + 0x0168)
> +
> +#endif /* _XE_I2C_REGS_H_ */

Raag

Re: [PATCH v1 2/2] drm/xe: Support for I2C attached MCUs

[Heikki Krogerus]

On Fri, May 30, 2025 at 07:02:23PM +0300, Raag Jadav wrote:
> On Fri, May 30, 2025 at 05:17:44PM +0300, Heikki Krogerus wrote:
> > Adding adaption/glue layer where the I2C host adapter
> > (Synopsys DesignWare I2C adapter) and the I2C clients (the
> > microcontroller units) are enumerated.
> > 
> > The microcontroller units (MCU) that are attached to the GPU
> > depend on the OEM. The initially supported MCU will be the
> > Add-In Management Controller (AMC).
> 
> ...
> 
> > diff --git a/drivers/gpu/drm/xe/regs/xe_i2c_regs.h b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
> > new file mode 100644
> > index 000000000000..2acb55eeef0d
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _XE_I2C_REGS_H_
> > +#define _XE_I2C_REGS_H_
> > +
> > +#include "xe_reg_defs.h"
> > +
> > +#define SOC_BASE			0x280000
> 
> I just noticed we already have this in xe_pmt.h, so let's not duplicate
> it. Perhaps move it to a common header (xe_regs.h) and reuse it in both
> places?

Makes sense to me. If no objections, I'll move it there in v2.

thanks,

> > +#define I2C_CONFIG_SPACE_OFFSET		(SOC_BASE + 0xf6000)
> > +#define I2C_MEM_SPACE_OFFSET		(SOC_BASE + 0xf7400)
> > +#define I2C_BRIDGE_OFFSET		(SOC_BASE + 0xd9000)
> > +
> > +#define CLIENT_DISC_COOKIE		XE_REG(SOC_BASE + 0x0164)
> > +#define CLIENT_DISC_ADDRESS		XE_REG(SOC_BASE + 0x0168)
> > +
> > +#endif /* _XE_I2C_REGS_H_ */

-- 
heikki

Re: [PATCH v1 2/2] drm/xe: Support for I2C attached MCUs

[Lucas De Marchi]

On Fri, May 30, 2025 at 05:17:44PM +0300, Heikki Krogerus wrote:
>Adding adaption/glue layer where the I2C host adapter
>(Synopsys DesignWare I2C adapter) and the I2C clients (the
>microcontroller units) are enumerated.
>
>The microcontroller units (MCU) that are attached to the GPU
>depend on the OEM. The initially supported MCU will be the
>Add-In Management Controller (AMC).
>
>Originally-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Please use drm-tip as base for the patch series, otherwise we can't run
CI to make sure it doesn't regress:
https://drm.pages.freedesktop.org/maintainer-tools/repositories/drm-tip.html

We will probably also need new tests in igt.

>---
> drivers/gpu/drm/xe/Makefile           |   1 +
> drivers/gpu/drm/xe/regs/xe_i2c_regs.h |  16 ++
> drivers/gpu/drm/xe/regs/xe_irq_regs.h |   1 +
> drivers/gpu/drm/xe/xe_device.c        |   3 +
> drivers/gpu/drm/xe/xe_device_types.h  |   4 +
> drivers/gpu/drm/xe/xe_i2c.c           | 270 ++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_i2c.h           |  60 ++++++
> drivers/gpu/drm/xe/xe_irq.c           |   2 +
> 8 files changed, 357 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/regs/xe_i2c_regs.h
> create mode 100644 drivers/gpu/drm/xe/xe_i2c.c
> create mode 100644 drivers/gpu/drm/xe/xe_i2c.h
>
>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>index e4bf484d4121..3f581e2bb564 100644
>--- a/drivers/gpu/drm/xe/Makefile
>+++ b/drivers/gpu/drm/xe/Makefile
>@@ -124,6 +124,7 @@ xe-y += xe_bb.o \
> 	xe_wait_user_fence.o \
> 	xe_wopcm.o
>
>+xe-$(CONFIG_I2C)	+= xe_i2c.o
> xe-$(CONFIG_HMM_MIRROR) += xe_hmm.o
> xe-$(CONFIG_DRM_XE_GPUSVM) += xe_svm.o
>
>diff --git a/drivers/gpu/drm/xe/regs/xe_i2c_regs.h b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
>new file mode 100644
>index 000000000000..2acb55eeef0d
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
>@@ -0,0 +1,16 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+#ifndef _XE_I2C_REGS_H_
>+#define _XE_I2C_REGS_H_
>+
>+#include "xe_reg_defs.h"
>+
>+#define SOC_BASE			0x280000
>+
>+#define I2C_CONFIG_SPACE_OFFSET		(SOC_BASE + 0xf6000)
>+#define I2C_MEM_SPACE_OFFSET		(SOC_BASE + 0xf7400)
>+#define I2C_BRIDGE_OFFSET		(SOC_BASE + 0xd9000)
>+
>+#define CLIENT_DISC_COOKIE		XE_REG(SOC_BASE + 0x0164)
>+#define CLIENT_DISC_ADDRESS		XE_REG(SOC_BASE + 0x0168)
>+
>+#endif /* _XE_I2C_REGS_H_ */
>diff --git a/drivers/gpu/drm/xe/regs/xe_irq_regs.h b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
>index f0ecfcac4003..13635e4331d4 100644
>--- a/drivers/gpu/drm/xe/regs/xe_irq_regs.h
>+++ b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
>@@ -19,6 +19,7 @@
> #define   MASTER_IRQ				REG_BIT(31)
> #define   GU_MISC_IRQ				REG_BIT(29)
> #define   DISPLAY_IRQ				REG_BIT(16)
>+#define   I2C_IRQ				REG_BIT(12)
> #define   GT_DW_IRQ(x)				REG_BIT(x)
>
> /*
>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>index d4b6e623aa48..dd635101420b 100644
>--- a/drivers/gpu/drm/xe/xe_device.c
>+++ b/drivers/gpu/drm/xe/xe_device.c
>@@ -42,6 +42,7 @@
> #include "xe_guc.h"
> #include "xe_hw_engine_group.h"
> #include "xe_hwmon.h"
>+#include "xe_i2c.h"
> #include "xe_irq.h"
> #include "xe_memirq.h"
> #include "xe_mmio.h"
>@@ -929,6 +930,8 @@ int xe_device_probe(struct xe_device *xe)
>
> 	xe_vsec_init(xe);
>
>+	xe_i2c_probe(xe);

please don't ignore errors on probe, it's painful to keep fixing this up
later. See e.g.

	35359c36356a drm/xe: Stop ignoring errors from xe_ttm_sys_mgr_init()
	292b1a8a5054 drm/xe: Stop ignoring errors from xe_heci_gsc_init()
	d40f275d96e8 drm/xe: Move survivability entirely to xe_pci
	62fbc75b28a7 drm/xe/hwmon: Stop ignoring errors on probe
	6b5506158f90 drm/xe/pmu: Fail probe if xe_pmu_register() fails
	960d71044eee drm/xe/oa: Handle errors in xe_oa_register()

if it's failing we want to catch it in our CI rather than realizing much
later a subsystem integration was not really working and the driver is
in a semi-working state.

>+
> 	return devm_add_action_or_reset(xe->drm.dev, xe_device_sanitize, xe);
>
> err_unregister_display:
>diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>index 50b2bfa682ac..7011a078b9c7 100644
>--- a/drivers/gpu/drm/xe/xe_device_types.h
>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>@@ -36,6 +36,7 @@
> #endif
>
> struct xe_ggtt;
>+struct xe_i2c;
> struct xe_pat_ops;
> struct xe_pxp;
>
>@@ -572,6 +573,9 @@ struct xe_device {
> 	/** @pmu: performance monitoring unit */
> 	struct xe_pmu pmu;
>
>+	/** @i2c: I2C host controller */
>+	struct xe_i2c *i2c;
>+
> 	/** @atomic_svm_timeslice_ms: Atomic SVM fault timeslice MS */
> 	u32 atomic_svm_timeslice_ms;
>
>diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
>new file mode 100644
>index 000000000000..cc231368789f
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_i2c.c
>@@ -0,0 +1,270 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * Intel Xe I2C attached Microcontroller Units (MCU)
>+ *
>+ * Copyright (C) 2025 Intel Corporation.
>+ */
>+
>+#include <linux/array_size.h>
>+#include <linux/container_of.h>
>+#include <linux/device.h>
>+#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>
>+#include <linux/property.h>
>+#include <linux/regmap.h>
>+#include <linux/sprintf.h>
>+#include <linux/string.h>
>+#include <linux/types.h>
>+#include <linux/workqueue.h>
>+
>+#include "regs/xe_i2c_regs.h"
>+#include "regs/xe_irq_regs.h"
>+
>+#include "xe_device.h"
>+#include "xe_device_types.h"
>+#include "xe_i2c.h"
>+#include "xe_mmio.h"
>+#include "xe_platform_types.h"
>+
>+/* Synopsys DesignWare I2C Host Adapter */
>+static const char adapter_name[] = "i2c_designware";
>+
>+static const struct property_entry xe_i2c_adapter_properties[] = {
>+	PROPERTY_ENTRY_STRING("compatible", "intel,xe-mcu-i2c"),
>+	PROPERTY_ENTRY_U32("clock-frequency", I2C_MAX_FAST_MODE_PLUS_FREQ),
>+	{ }
>+};
>+
>+static inline void xe_i2c_read_endpoint(struct xe_mmio *mmio, void *ep)
>+{
>+	u32 *val = ep;
>+
>+	val[0] = xe_mmio_read32(mmio, CLIENT_DISC_COOKIE);
>+	val[1] = xe_mmio_read32(mmio, CLIENT_DISC_ADDRESS);
>+}
>+
>+static void xe_i2c_client_work(struct work_struct *work)
>+{
>+	struct xe_i2c *i2c = container_of(work, struct xe_i2c, work);
>+	struct i2c_board_info info = {
>+		.type	= "amc",
>+		.flags	= I2C_CLIENT_HOST_NOTIFY,
>+		.addr	= i2c->ep.addr[1],
>+	};

so... AMC is not a new thing. We also have it in PVC for example.
However the way it is exposed to the driver seems to be different
and what you did here here wouldn't match.

>+
>+	i2c->client[0] = i2c_new_client_device(i2c->adapter, &info);
>+}
>+
>+static int xe_i2c_notifier(struct notifier_block *nb, unsigned long action, void *data)
>+{
>+	struct xe_i2c *i2c = container_of(nb, struct xe_i2c, bus_notifier);
>+	struct i2c_adapter *adapter = i2c_verify_adapter(data);
>+	struct device *dev = data;
>+
>+	if (action == BUS_NOTIFY_ADD_DEVICE &&
>+	    adapter && dev->parent == &i2c->pdev->dev) {
>+		i2c->adapter = adapter;
>+		schedule_work(&i2c->work);
>+		return NOTIFY_OK;
>+	}
>+
>+	return NOTIFY_DONE;
>+}
>+
>+static int xe_i2c_register_adapter(struct xe_i2c *i2c)
>+{
>+	struct pci_dev *pci = to_pci_dev(i2c->drm_dev);
>+	struct platform_device *pdev;
>+	struct fwnode_handle *fwnode;
>+	int ret;
>+
>+	fwnode = fwnode_create_software_node(xe_i2c_adapter_properties, NULL);
>+	if (!fwnode)
>+		return -ENOMEM;
>+
>+	/*
>+	 * Not using platform_device_register_full() here because we don't have
>+	 * a handle to the platform_device before it returns. xe_i2c_notifier()
>+	 * uses that handle, but it may be called before
>+	 * platform_device_register_full() is done.
>+	 */
>+	pdev = platform_device_alloc(adapter_name, pci_dev_id(pci));
>+	if (!pdev) {
>+		ret = -ENOMEM;
>+		goto err_fwnode_remove;
>+	}
>+
>+	if (i2c->adapter_irq) {
>+		struct resource	res = { };
>+
>+		res.start = i2c->adapter_irq;
>+		res.name = "xe_i2c";
>+		res.flags = IORESOURCE_IRQ;
>+
>+		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;
>+	i2c->pdev = pdev;
>+
>+	ret = platform_device_add(pdev);
>+	if (ret)
>+		goto err_pdev_put;
>+
>+	return 0;
>+
>+err_pdev_put:
>+	platform_device_put(pdev);
>+err_fwnode_remove:
>+	fwnode_remove_software_node(fwnode);
>+
>+	return ret;
>+}
>+
>+static void xe_i2c_unregister_adapter(struct xe_i2c *i2c)
>+{
>+	platform_device_unregister(i2c->pdev);
>+	fwnode_remove_software_node(i2c->adapter_node);
>+}
>+
>+void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
>+{
>+	if (!xe->i2c || !xe->i2c->adapter_irq)
>+		return;
>+
>+	if (master_ctl & I2C_IRQ)
>+		generic_handle_irq_safe(xe->i2c->adapter_irq);
>+}
>+
>+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_i2c *i2c)
>+{
>+	struct irq_domain *domain;
>+
>+	if (!(i2c->ep.capabilities & XE_I2C_EP_CAP_IRQ))
>+		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) {
>+		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;
>+
>+	*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
>+
>+	return 0;
>+}
>+
>+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);
>+
>+	return 0;
>+}
>+
>+static const struct regmap_config i2c_regmap_config = {
>+	.reg_bits = 32,
>+	.val_bits = 32,
>+	.reg_read = xe_i2c_read,
>+	.reg_write = xe_i2c_write,
>+	.fast_io = true,
>+};
>+
>+static void xe_i2c_remove(void *data)
>+{
>+	struct xe_i2c *i2c = data;
>+	int i;
>+
>+	for (i = 0; i < XE_I2C_MAX_CLIENTS; i++)
>+		i2c_unregister_device(i2c->client[i]);
>+
>+	xe_i2c_unregister_adapter(i2c);
>+	xe_i2c_remove_irq(i2c);
>+	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);

this looks racy. Shouldn't you unregister the notifier before the other
2 calls?

>+}
>+
>+int xe_i2c_probe(struct xe_device *xe)
>+{
>+	struct xe_i2c_endpoint ep;
>+	struct regmap *regmap;
>+	struct xe_i2c *i2c;
>+	int ret;
>+
>+	if (xe->info.platform != XE_BATTLEMAGE)
>+		return 0;

we prefer checking for IP versions but don't have a good way for the SOC
versioning. However bspec 54028 seems to disagree here. It
would probably be better to check on the GT IP + is_dgfx. Or if it's
safe to rely on the cookie below, just check for is_dgfx.

>+
>+	xe_i2c_read_endpoint(xe_root_tile_mmio(xe), &ep);
>+	if (ep.cookie != XE_I2C_EP_COOKIE_DEVICE)
>+		return 0;
>+
>+	i2c = devm_kzalloc(xe->drm.dev, sizeof(*i2c), GFP_KERNEL);
>+	if (!i2c)
>+		return -ENOMEM;
>+
>+	INIT_WORK(&i2c->work, xe_i2c_client_work);
>+	i2c->mmio = xe_root_tile_mmio(xe);
>+	i2c->drm_dev = xe->drm.dev;
>+	i2c->ep = ep;
>+
>+	regmap = devm_regmap_init(i2c->drm_dev, NULL, i2c, &i2c_regmap_config);
>+	if (IS_ERR(regmap))
>+		return PTR_ERR(regmap);
>+
>+	i2c->bus_notifier.notifier_call = xe_i2c_notifier;
>+	ret = bus_register_notifier(&i2c_bus_type, &i2c->bus_notifier);
>+	if (ret)
>+		return ret;
>+
>+	ret = xe_i2c_create_irq(i2c);
>+	if (ret)
>+		goto err_unregister_notifier;
>+
>+	ret = xe_i2c_register_adapter(i2c);
>+	if (ret)
>+		goto err_unregister_notifier;
>+
>+	return devm_add_action_or_reset(i2c->drm_dev, xe_i2c_remove, i2c);
>+
>+err_unregister_notifier:
>+	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
>+
>+	return ret;
>+}
>diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
>new file mode 100644
>index 000000000000..ab8e21630838
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_i2c.h
>@@ -0,0 +1,60 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+#ifndef _XE_I2C_H_
>+#define _XE_I2C_H_
>+
>+#include <linux/bits.h>
>+#include <linux/notifier.h>
>+#include <linux/types.h>
>+#include <linux/workqueue.h>
>+
>+struct device;
>+struct fwnode_handle;
>+struct i2c_adapter;
>+struct i2c_client;
>+struct irq_domain;
>+struct platform_device;
>+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)
>+#define XE_I2C_EP_CAP_MULTI_MCU		BIT(1)
>+#define XE_I2C_EP_CAP_SMBUS		BIT(2)

these 2 are unused?

>+
>+struct xe_i2c_endpoint {
>+	u8 cookie;
>+	u8 capabilities;
>+	u16 addr[XE_I2C_MAX_CLIENTS];
>+};
>+
>+struct xe_i2c {
>+	struct fwnode_handle *adapter_node;
>+	struct platform_device *pdev;
>+	struct i2c_adapter *adapter;
>+	struct i2c_client *client[XE_I2C_MAX_CLIENTS];
>+
>+	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;
>+};
>+
>+#if IS_ENABLED(CONFIG_I2C)
>+int xe_i2c_probe(struct xe_device *xe);
>+void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl);
>+#else
>+static inline int xe_i2c_probe(struct xe_device *xe) { return 0; }
>+static inline void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl) { }
>+#endif
>+
>+#endif
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index 5362d3174b06..c43e62dc692e 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -18,6 +18,7 @@
> #include "xe_gt.h"
> #include "xe_guc.h"
> #include "xe_hw_engine.h"
>+#include "xe_i2c.h"
> #include "xe_memirq.h"
> #include "xe_mmio.h"
> #include "xe_pxp.h"
>@@ -476,6 +477,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
> 			if (xe->info.has_heci_cscfi)
> 				xe_heci_csc_irq_handler(xe, master_ctl);
> 			xe_display_irq_handler(xe, master_ctl);
>+			xe_i2c_irq_handler(xe, master_ctl);
> 			gu_misc_iir = gu_misc_irq_ack(xe, master_ctl);
> 		}
> 	}
>-- 
>2.47.2
>

Re: [PATCH v1 2/2] drm/xe: Support for I2C attached MCUs

[Heikki Krogerus]

Hi Lucas,

Thank you for the review.

On Mon, Jun 02, 2025 at 09:48:21AM -0500, Lucas De Marchi wrote:
> On Fri, May 30, 2025 at 05:17:44PM +0300, Heikki Krogerus wrote:
> > Adding adaption/glue layer where the I2C host adapter
> > (Synopsys DesignWare I2C adapter) and the I2C clients (the
> > microcontroller units) are enumerated.
> > 
> > The microcontroller units (MCU) that are attached to the GPU
> > depend on the OEM. The initially supported MCU will be the
> > Add-In Management Controller (AMC).
> > 
> > Originally-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> Please use drm-tip as base for the patch series, otherwise we can't run
> CI to make sure it doesn't regress:
> https://drm.pages.freedesktop.org/maintainer-tools/repositories/drm-tip.html

Got it.

> We will probably also need new tests in igt.
> 
> > ---
> > drivers/gpu/drm/xe/Makefile           |   1 +
> > drivers/gpu/drm/xe/regs/xe_i2c_regs.h |  16 ++
> > drivers/gpu/drm/xe/regs/xe_irq_regs.h |   1 +
> > drivers/gpu/drm/xe/xe_device.c        |   3 +
> > drivers/gpu/drm/xe/xe_device_types.h  |   4 +
> > drivers/gpu/drm/xe/xe_i2c.c           | 270 ++++++++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_i2c.h           |  60 ++++++
> > drivers/gpu/drm/xe/xe_irq.c           |   2 +
> > 8 files changed, 357 insertions(+)
> > create mode 100644 drivers/gpu/drm/xe/regs/xe_i2c_regs.h
> > create mode 100644 drivers/gpu/drm/xe/xe_i2c.c
> > create mode 100644 drivers/gpu/drm/xe/xe_i2c.h
> > 
> > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> > index e4bf484d4121..3f581e2bb564 100644
> > --- a/drivers/gpu/drm/xe/Makefile
> > +++ b/drivers/gpu/drm/xe/Makefile
> > @@ -124,6 +124,7 @@ xe-y += xe_bb.o \
> > 	xe_wait_user_fence.o \
> > 	xe_wopcm.o
> > 
> > +xe-$(CONFIG_I2C)	+= xe_i2c.o
> > xe-$(CONFIG_HMM_MIRROR) += xe_hmm.o
> > xe-$(CONFIG_DRM_XE_GPUSVM) += xe_svm.o
> > 
> > diff --git a/drivers/gpu/drm/xe/regs/xe_i2c_regs.h b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
> > new file mode 100644
> > index 000000000000..2acb55eeef0d
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/regs/xe_i2c_regs.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _XE_I2C_REGS_H_
> > +#define _XE_I2C_REGS_H_
> > +
> > +#include "xe_reg_defs.h"
> > +
> > +#define SOC_BASE			0x280000
> > +
> > +#define I2C_CONFIG_SPACE_OFFSET		(SOC_BASE + 0xf6000)
> > +#define I2C_MEM_SPACE_OFFSET		(SOC_BASE + 0xf7400)
> > +#define I2C_BRIDGE_OFFSET		(SOC_BASE + 0xd9000)
> > +
> > +#define CLIENT_DISC_COOKIE		XE_REG(SOC_BASE + 0x0164)
> > +#define CLIENT_DISC_ADDRESS		XE_REG(SOC_BASE + 0x0168)
> > +
> > +#endif /* _XE_I2C_REGS_H_ */
> > diff --git a/drivers/gpu/drm/xe/regs/xe_irq_regs.h b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> > index f0ecfcac4003..13635e4331d4 100644
> > --- a/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> > +++ b/drivers/gpu/drm/xe/regs/xe_irq_regs.h
> > @@ -19,6 +19,7 @@
> > #define   MASTER_IRQ				REG_BIT(31)
> > #define   GU_MISC_IRQ				REG_BIT(29)
> > #define   DISPLAY_IRQ				REG_BIT(16)
> > +#define   I2C_IRQ				REG_BIT(12)
> > #define   GT_DW_IRQ(x)				REG_BIT(x)
> > 
> > /*
> > diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> > index d4b6e623aa48..dd635101420b 100644
> > --- a/drivers/gpu/drm/xe/xe_device.c
> > +++ b/drivers/gpu/drm/xe/xe_device.c
> > @@ -42,6 +42,7 @@
> > #include "xe_guc.h"
> > #include "xe_hw_engine_group.h"
> > #include "xe_hwmon.h"
> > +#include "xe_i2c.h"
> > #include "xe_irq.h"
> > #include "xe_memirq.h"
> > #include "xe_mmio.h"
> > @@ -929,6 +930,8 @@ int xe_device_probe(struct xe_device *xe)
> > 
> > 	xe_vsec_init(xe);
> > 
> > +	xe_i2c_probe(xe);
> 
> please don't ignore errors on probe, it's painful to keep fixing this up
> later. See e.g.
> 
> 	35359c36356a drm/xe: Stop ignoring errors from xe_ttm_sys_mgr_init()
> 	292b1a8a5054 drm/xe: Stop ignoring errors from xe_heci_gsc_init()
> 	d40f275d96e8 drm/xe: Move survivability entirely to xe_pci
> 	62fbc75b28a7 drm/xe/hwmon: Stop ignoring errors on probe
> 	6b5506158f90 drm/xe/pmu: Fail probe if xe_pmu_register() fails
> 	960d71044eee drm/xe/oa: Handle errors in xe_oa_register()
> 
> if it's failing we want to catch it in our CI rather than realizing much
> later a subsystem integration was not really working and the driver is
> in a semi-working state.

Sure thing. I'll fix this.

> > +
> > 	return devm_add_action_or_reset(xe->drm.dev, xe_device_sanitize, xe);
> > 
> > err_unregister_display:
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> > index 50b2bfa682ac..7011a078b9c7 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -36,6 +36,7 @@
> > #endif
> > 
> > struct xe_ggtt;
> > +struct xe_i2c;
> > struct xe_pat_ops;
> > struct xe_pxp;
> > 
> > @@ -572,6 +573,9 @@ struct xe_device {
> > 	/** @pmu: performance monitoring unit */
> > 	struct xe_pmu pmu;
> > 
> > +	/** @i2c: I2C host controller */
> > +	struct xe_i2c *i2c;
> > +
> > 	/** @atomic_svm_timeslice_ms: Atomic SVM fault timeslice MS */
> > 	u32 atomic_svm_timeslice_ms;
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
> > new file mode 100644
> > index 000000000000..cc231368789f
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_i2c.c
> > @@ -0,0 +1,270 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Intel Xe I2C attached Microcontroller Units (MCU)
> > + *
> > + * Copyright (C) 2025 Intel Corporation.
> > + */
> > +
> > +#include <linux/array_size.h>
> > +#include <linux/container_of.h>
> > +#include <linux/device.h>
> > +#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>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +#include <linux/sprintf.h>
> > +#include <linux/string.h>
> > +#include <linux/types.h>
> > +#include <linux/workqueue.h>
> > +
> > +#include "regs/xe_i2c_regs.h"
> > +#include "regs/xe_irq_regs.h"
> > +
> > +#include "xe_device.h"
> > +#include "xe_device_types.h"
> > +#include "xe_i2c.h"
> > +#include "xe_mmio.h"
> > +#include "xe_platform_types.h"
> > +
> > +/* Synopsys DesignWare I2C Host Adapter */
> > +static const char adapter_name[] = "i2c_designware";
> > +
> > +static const struct property_entry xe_i2c_adapter_properties[] = {
> > +	PROPERTY_ENTRY_STRING("compatible", "intel,xe-mcu-i2c"),
> > +	PROPERTY_ENTRY_U32("clock-frequency", I2C_MAX_FAST_MODE_PLUS_FREQ),
> > +	{ }
> > +};
> > +
> > +static inline void xe_i2c_read_endpoint(struct xe_mmio *mmio, void *ep)
> > +{
> > +	u32 *val = ep;
> > +
> > +	val[0] = xe_mmio_read32(mmio, CLIENT_DISC_COOKIE);
> > +	val[1] = xe_mmio_read32(mmio, CLIENT_DISC_ADDRESS);
> > +}
> > +
> > +static void xe_i2c_client_work(struct work_struct *work)
> > +{
> > +	struct xe_i2c *i2c = container_of(work, struct xe_i2c, work);
> > +	struct i2c_board_info info = {
> > +		.type	= "amc",
> > +		.flags	= I2C_CLIENT_HOST_NOTIFY,
> > +		.addr	= i2c->ep.addr[1],
> > +	};
> 
> so... AMC is not a new thing. We also have it in PVC for example.
> However the way it is exposed to the driver seems to be different
> and what you did here here wouldn't match.

The I2C was most likely hidden and taken care of in some firmware
blob (or maybe the AMC was attached to some completely different
interconnect), but that's no longer the case. The I2C is no longer
taken care of by the GPU itself, but instead the I2C host adapter
registers are now mapped to the GPU MMIO, and the operating system is
now expected to take care of it.

The AMC uses the Intelligent Platform Management Interface (IPMI), or
at least used in the past [1] (I think IPMI is what you are referring
to here - please let me know if I misunderstood). It should not be a
problem to bind this I2C client device to a driver that then registers
the IPMI (or what ever interface is used).

However, right now this I2C client device is used only to let the user
space (the XPU manager) know that the system has the AMC attached to
it. The actual communication with the AMC happens with the i2c-dev for
now. The client device is nevertheless already needed.

The i2c-dev interface really can't be the final solution, so this
device really has to be bind to a driver sooner or later. But this is
the first step.

[1] https://github.com/intel/xpumanager

> > +
> > +	i2c->client[0] = i2c_new_client_device(i2c->adapter, &info);
> > +}
> > +
> > +static int xe_i2c_notifier(struct notifier_block *nb, unsigned long action, void *data)
> > +{
> > +	struct xe_i2c *i2c = container_of(nb, struct xe_i2c, bus_notifier);
> > +	struct i2c_adapter *adapter = i2c_verify_adapter(data);
> > +	struct device *dev = data;
> > +
> > +	if (action == BUS_NOTIFY_ADD_DEVICE &&
> > +	    adapter && dev->parent == &i2c->pdev->dev) {
> > +		i2c->adapter = adapter;
> > +		schedule_work(&i2c->work);
> > +		return NOTIFY_OK;
> > +	}
> > +
> > +	return NOTIFY_DONE;
> > +}
> > +
> > +static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> > +{
> > +	struct pci_dev *pci = to_pci_dev(i2c->drm_dev);
> > +	struct platform_device *pdev;
> > +	struct fwnode_handle *fwnode;
> > +	int ret;
> > +
> > +	fwnode = fwnode_create_software_node(xe_i2c_adapter_properties, NULL);
> > +	if (!fwnode)
> > +		return -ENOMEM;
> > +
> > +	/*
> > +	 * Not using platform_device_register_full() here because we don't have
> > +	 * a handle to the platform_device before it returns. xe_i2c_notifier()
> > +	 * uses that handle, but it may be called before
> > +	 * platform_device_register_full() is done.
> > +	 */
> > +	pdev = platform_device_alloc(adapter_name, pci_dev_id(pci));
> > +	if (!pdev) {
> > +		ret = -ENOMEM;
> > +		goto err_fwnode_remove;
> > +	}
> > +
> > +	if (i2c->adapter_irq) {
> > +		struct resource	res = { };
> > +
> > +		res.start = i2c->adapter_irq;
> > +		res.name = "xe_i2c";
> > +		res.flags = IORESOURCE_IRQ;
> > +
> > +		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;
> > +	i2c->pdev = pdev;
> > +
> > +	ret = platform_device_add(pdev);
> > +	if (ret)
> > +		goto err_pdev_put;
> > +
> > +	return 0;
> > +
> > +err_pdev_put:
> > +	platform_device_put(pdev);
> > +err_fwnode_remove:
> > +	fwnode_remove_software_node(fwnode);
> > +
> > +	return ret;
> > +}
> > +
> > +static void xe_i2c_unregister_adapter(struct xe_i2c *i2c)
> > +{
> > +	platform_device_unregister(i2c->pdev);
> > +	fwnode_remove_software_node(i2c->adapter_node);
> > +}
> > +
> > +void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl)
> > +{
> > +	if (!xe->i2c || !xe->i2c->adapter_irq)
> > +		return;
> > +
> > +	if (master_ctl & I2C_IRQ)
> > +		generic_handle_irq_safe(xe->i2c->adapter_irq);
> > +}
> > +
> > +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_i2c *i2c)
> > +{
> > +	struct irq_domain *domain;
> > +
> > +	if (!(i2c->ep.capabilities & XE_I2C_EP_CAP_IRQ))
> > +		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) {
> > +		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;
> > +
> > +	*val = xe_mmio_read32(i2c->mmio, XE_REG(reg + I2C_MEM_SPACE_OFFSET));
> > +
> > +	return 0;
> > +}
> > +
> > +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);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct regmap_config i2c_regmap_config = {
> > +	.reg_bits = 32,
> > +	.val_bits = 32,
> > +	.reg_read = xe_i2c_read,
> > +	.reg_write = xe_i2c_write,
> > +	.fast_io = true,
> > +};
> > +
> > +static void xe_i2c_remove(void *data)
> > +{
> > +	struct xe_i2c *i2c = data;
> > +	int i;
> > +
> > +	for (i = 0; i < XE_I2C_MAX_CLIENTS; i++)
> > +		i2c_unregister_device(i2c->client[i]);
> > +
> > +	xe_i2c_unregister_adapter(i2c);
> > +	xe_i2c_remove_irq(i2c);
> > +	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
> 
> this looks racy. Shouldn't you unregister the notifier before the other
> 2 calls?

OK, I'll move it up.

> > +}
> > +
> > +int xe_i2c_probe(struct xe_device *xe)
> > +{
> > +	struct xe_i2c_endpoint ep;
> > +	struct regmap *regmap;
> > +	struct xe_i2c *i2c;
> > +	int ret;
> > +
> > +	if (xe->info.platform != XE_BATTLEMAGE)
> > +		return 0;
> 
> we prefer checking for IP versions but don't have a good way for the SOC
> versioning. However bspec 54028 seems to disagree here. It
> would probably be better to check on the GT IP + is_dgfx. Or if it's
> safe to rely on the cookie below, just check for is_dgfx.

I'll drop this. I quite certain that the cookie should be sufficient
for this.

> > +
> > +	xe_i2c_read_endpoint(xe_root_tile_mmio(xe), &ep);
> > +	if (ep.cookie != XE_I2C_EP_COOKIE_DEVICE)
> > +		return 0;
> > +
> > +	i2c = devm_kzalloc(xe->drm.dev, sizeof(*i2c), GFP_KERNEL);
> > +	if (!i2c)
> > +		return -ENOMEM;
> > +
> > +	INIT_WORK(&i2c->work, xe_i2c_client_work);
> > +	i2c->mmio = xe_root_tile_mmio(xe);
> > +	i2c->drm_dev = xe->drm.dev;
> > +	i2c->ep = ep;
> > +
> > +	regmap = devm_regmap_init(i2c->drm_dev, NULL, i2c, &i2c_regmap_config);
> > +	if (IS_ERR(regmap))
> > +		return PTR_ERR(regmap);
> > +
> > +	i2c->bus_notifier.notifier_call = xe_i2c_notifier;
> > +	ret = bus_register_notifier(&i2c_bus_type, &i2c->bus_notifier);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = xe_i2c_create_irq(i2c);
> > +	if (ret)
> > +		goto err_unregister_notifier;
> > +
> > +	ret = xe_i2c_register_adapter(i2c);
> > +	if (ret)
> > +		goto err_unregister_notifier;
> > +
> > +	return devm_add_action_or_reset(i2c->drm_dev, xe_i2c_remove, i2c);
> > +
> > +err_unregister_notifier:
> > +	bus_unregister_notifier(&i2c_bus_type, &i2c->bus_notifier);
> > +
> > +	return ret;
> > +}
> > diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
> > new file mode 100644
> > index 000000000000..ab8e21630838
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_i2c.h
> > @@ -0,0 +1,60 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _XE_I2C_H_
> > +#define _XE_I2C_H_
> > +
> > +#include <linux/bits.h>
> > +#include <linux/notifier.h>
> > +#include <linux/types.h>
> > +#include <linux/workqueue.h>
> > +
> > +struct device;
> > +struct fwnode_handle;
> > +struct i2c_adapter;
> > +struct i2c_client;
> > +struct irq_domain;
> > +struct platform_device;
> > +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)
> > +#define XE_I2C_EP_CAP_MULTI_MCU		BIT(1)
> > +#define XE_I2C_EP_CAP_SMBUS		BIT(2)
> 
> these 2 are unused?

True, I'll drop them.

thanks,

-- 
heikki