[v1] Introducing firmware late binding

[PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw

Posted by Rodrigo Vivi 2 months, 4 weeks ago

From: Badal Nilawar <badal.nilawar@intel.com>

Introducing xe_late_bind_fw to enable firmware loading for the devices,
such as the fan controller, during the driver probe. Typically,
firmware for such devices are part of IFWI flash image but can be
replaced at probe after OEM tuning.
This patch binds mei late binding component to enable firmware loading.

v2:
 - Add devm_add_action_or_reset to remove the component (Daniele)
 - Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
v3:
 - Fail driver probe if late bind initialization fails,
   add has_late_bind flag (Daniele)
v4:
 - %S/I915_COMPONENT_LATE_BIND/INTEL_COMPONENT_LATE_BIND/
v6:
 - rebased

Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/xe/Makefile                |  1 +
 drivers/gpu/drm/xe/xe_device.c             |  5 ++
 drivers/gpu/drm/xe/xe_device_types.h       |  6 ++
 drivers/gpu/drm/xe/xe_late_bind_fw.c       | 84 ++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_late_bind_fw.h       | 15 ++++
 drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 33 +++++++++
 drivers/gpu/drm/xe/xe_pci.c                |  2 +
 drivers/gpu/drm/xe/xe_pci_types.h          |  1 +
 8 files changed, 147 insertions(+)
 create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.c
 create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.h
 create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw_types.h

diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 1d97e5b63f4e..97e2b1368a6e 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -76,6 +76,7 @@ xe-y += xe_bb.o \
 	xe_hw_fence.o \
 	xe_irq.o \
 	xe_lrc.o \
+	xe_late_bind_fw.o \
 	xe_migrate.o \
 	xe_mmio.o \
 	xe_mocs.o \
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 0b73cb72bad1..cb595bae5f55 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -44,6 +44,7 @@
 #include "xe_hw_engine_group.h"
 #include "xe_hwmon.h"
 #include "xe_irq.h"
+#include "xe_late_bind_fw.h"
 #include "xe_mmio.h"
 #include "xe_module.h"
 #include "xe_nvm.h"
@@ -866,6 +867,10 @@ int xe_device_probe(struct xe_device *xe)
 	if (err)
 		return err;
 
+	err = xe_late_bind_init(&xe->late_bind);
+	if (err && err != -ENODEV)
+		return err;
+
 	err = xe_oa_init(xe);
 	if (err)
 		return err;
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 78c4acafd268..a8891833f980 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -16,6 +16,7 @@
 #include "xe_devcoredump_types.h"
 #include "xe_heci_gsc.h"
 #include "xe_lmtt_types.h"
+#include "xe_late_bind_fw_types.h"
 #include "xe_memirq_types.h"
 #include "xe_oa_types.h"
 #include "xe_platform_types.h"
@@ -325,6 +326,8 @@ struct xe_device {
 		u8 has_heci_cscfi:1;
 		/** @info.has_heci_gscfi: device has heci gscfi */
 		u8 has_heci_gscfi:1;
+		/** @info.has_late_bind: Device has firmware late binding support */
+		u8 has_late_bind:1;
 		/** @info.has_llc: Device has a shared CPU+GPU last level cache */
 		u8 has_llc:1;
 		/** @info.has_mbx_power_limits: Device has support to manage power limits using
@@ -557,6 +560,9 @@ struct xe_device {
 	/** @nvm: discrete graphics non-volatile memory */
 	struct intel_dg_nvm_dev *nvm;
 
+	/** @late_bind: xe mei late bind interface */
+	struct xe_late_bind late_bind;
+
 	/** @oa: oa observation subsystem */
 	struct xe_oa oa;
 
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
new file mode 100644
index 000000000000..152f3e58de94
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#include <linux/component.h>
+#include <linux/delay.h>
+
+#include <drm/drm_managed.h>
+#include <drm/intel/i915_component.h>
+#include <drm/intel/intel_lb_mei_interface.h>
+#include <drm/drm_print.h>
+
+#include "xe_device.h"
+#include "xe_late_bind_fw.h"
+
+static struct xe_device *
+late_bind_to_xe(struct xe_late_bind *late_bind)
+{
+	return container_of(late_bind, struct xe_device, late_bind);
+}
+
+static int xe_late_bind_component_bind(struct device *xe_kdev,
+				       struct device *mei_kdev, void *data)
+{
+	struct xe_device *xe = kdev_to_xe_device(xe_kdev);
+	struct xe_late_bind *late_bind = &xe->late_bind;
+
+	late_bind->component.ops = data;
+	late_bind->component.mei_dev = mei_kdev;
+
+	return 0;
+}
+
+static void xe_late_bind_component_unbind(struct device *xe_kdev,
+					  struct device *mei_kdev, void *data)
+{
+	struct xe_device *xe = kdev_to_xe_device(xe_kdev);
+	struct xe_late_bind *late_bind = &xe->late_bind;
+
+	late_bind->component.ops = NULL;
+}
+
+static const struct component_ops xe_late_bind_component_ops = {
+	.bind   = xe_late_bind_component_bind,
+	.unbind = xe_late_bind_component_unbind,
+};
+
+static void xe_late_bind_remove(void *arg)
+{
+	struct xe_late_bind *late_bind = arg;
+	struct xe_device *xe = late_bind_to_xe(late_bind);
+
+	component_del(xe->drm.dev, &xe_late_bind_component_ops);
+}
+
+/**
+ * xe_late_bind_init() - add xe mei late binding component
+ * @late_bind: pointer to late bind structure.
+ *
+ * Return: 0 if the initialization was successful, a negative errno otherwise.
+ */
+int xe_late_bind_init(struct xe_late_bind *late_bind)
+{
+	struct xe_device *xe = late_bind_to_xe(late_bind);
+	int err;
+
+	if (!xe->info.has_late_bind)
+		return 0;
+
+	if (!IS_ENABLED(CONFIG_INTEL_MEI_LATE_BIND) || !IS_ENABLED(CONFIG_INTEL_MEI_GSC)) {
+		drm_info(&xe->drm, "Can't init xe mei late bind missing mei component\n");
+		return -ENODEV;
+	}
+
+	err = component_add_typed(xe->drm.dev, &xe_late_bind_component_ops,
+				  INTEL_COMPONENT_LB);
+	if (err < 0) {
+		drm_info(&xe->drm, "Failed to add mei late bind component (%pe)\n", ERR_PTR(err));
+		return err;
+	}
+
+	return devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
+}
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
new file mode 100644
index 000000000000..4c73571c3e62
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#ifndef _XE_LATE_BIND_FW_H_
+#define _XE_LATE_BIND_FW_H_
+
+#include <linux/types.h>
+
+struct xe_late_bind;
+
+int xe_late_bind_init(struct xe_late_bind *late_bind);
+
+#endif
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
new file mode 100644
index 000000000000..f79e5aefed94
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#ifndef _XE_LATE_BIND_TYPES_H_
+#define _XE_LATE_BIND_TYPES_H_
+
+#include <linux/iosys-map.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+/**
+ * struct xe_late_bind_component - Late Binding services component
+ * @mei_dev: device that provide Late Binding service.
+ * @ops: Ops implemented by Late Binding driver, used by Xe driver.
+ *
+ * Communication between Xe and MEI drivers for Late Binding services
+ */
+struct xe_late_bind_component {
+	struct device *mei_dev;
+	const struct late_bind_component_ops *ops;
+};
+
+/**
+ * struct xe_late_bind
+ */
+struct xe_late_bind {
+	/** @component: struct for communication with mei component */
+	struct xe_late_bind_component component;
+};
+
+#endif
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index ffd6ad569b7c..69b8ead9ca59 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -332,6 +332,7 @@ static const struct xe_device_desc bmg_desc = {
 	.has_gsc_nvm = 1,
 	.has_heci_cscfi = 1,
 	.max_gt_per_tile = 2,
+	.has_late_bind = true,
 	.needs_scratch = true,
 };
 
@@ -578,6 +579,7 @@ static int xe_info_init_early(struct xe_device *xe,
 	xe->info.has_gsc_nvm = desc->has_gsc_nvm;
 	xe->info.has_heci_gscfi = desc->has_heci_gscfi;
 	xe->info.has_heci_cscfi = desc->has_heci_cscfi;
+	xe->info.has_late_bind = desc->has_late_bind;
 	xe->info.has_llc = desc->has_llc;
 	xe->info.has_pxp = desc->has_pxp;
 	xe->info.has_sriov = desc->has_sriov;
diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
index 4de6f69ed975..51a607d323fb 100644
--- a/drivers/gpu/drm/xe/xe_pci_types.h
+++ b/drivers/gpu/drm/xe/xe_pci_types.h
@@ -39,6 +39,7 @@ struct xe_device_desc {
 	u8 has_gsc_nvm:1;
 	u8 has_heci_gscfi:1;
 	u8 has_heci_cscfi:1;
+	u8 has_late_bind:1;
 	u8 has_llc:1;
 	u8 has_mbx_power_limits:1;
 	u8 has_pxp:1;
-- 
2.49.0

Re: [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw

Posted by Lucas De Marchi 2 months, 1 week ago

On Thu, Jul 10, 2025 at 11:08:34AM -0400, Rodrigo Vivi wrote:
>From: Badal Nilawar <badal.nilawar@intel.com>
>
>Introducing xe_late_bind_fw to enable firmware loading for the devices,

here and in the subject: no gerund, please.

>such as the fan controller, during the driver probe. Typically,
>firmware for such devices are part of IFWI flash image but can be
>replaced at probe after OEM tuning.
>This patch binds mei late binding component to enable firmware loading.
>
>v2:
> - Add devm_add_action_or_reset to remove the component (Daniele)
> - Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
>v3:
> - Fail driver probe if late bind initialization fails,
>   add has_late_bind flag (Daniele)
>v4:
> - %S/I915_COMPONENT_LATE_BIND/INTEL_COMPONENT_LATE_BIND/
>v6:
> - rebased
>
>Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/xe/Makefile                |  1 +
> drivers/gpu/drm/xe/xe_device.c             |  5 ++
> drivers/gpu/drm/xe/xe_device_types.h       |  6 ++
> drivers/gpu/drm/xe/xe_late_bind_fw.c       | 84 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_late_bind_fw.h       | 15 ++++
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 33 +++++++++
> drivers/gpu/drm/xe/xe_pci.c                |  2 +
> drivers/gpu/drm/xe/xe_pci_types.h          |  1 +
> 8 files changed, 147 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.c
> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.h
> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>
>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>index 1d97e5b63f4e..97e2b1368a6e 100644
>--- a/drivers/gpu/drm/xe/Makefile
>+++ b/drivers/gpu/drm/xe/Makefile
>@@ -76,6 +76,7 @@ xe-y += xe_bb.o \
> 	xe_hw_fence.o \
> 	xe_irq.o \
> 	xe_lrc.o \
>+	xe_late_bind_fw.o \

almost there, but still not sorted as it should be

> 	xe_migrate.o \
> 	xe_mmio.o \
> 	xe_mocs.o \
>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>index 0b73cb72bad1..cb595bae5f55 100644
>--- a/drivers/gpu/drm/xe/xe_device.c
>+++ b/drivers/gpu/drm/xe/xe_device.c
>@@ -44,6 +44,7 @@
> #include "xe_hw_engine_group.h"
> #include "xe_hwmon.h"
> #include "xe_irq.h"
>+#include "xe_late_bind_fw.h"
> #include "xe_mmio.h"
> #include "xe_module.h"
> #include "xe_nvm.h"
>@@ -866,6 +867,10 @@ int xe_device_probe(struct xe_device *xe)
> 	if (err)
> 		return err;
>
>+	err = xe_late_bind_init(&xe->late_bind);
>+	if (err && err != -ENODEV)

let's not be creative in xe_device_probe(). Boring is good.

All the other parts in this function handle ENODEV internally. It's up
to them to decide if ENODEV -> return 0, not here.


>+		return err;
>+
> 	err = xe_oa_init(xe);
> 	if (err)
> 		return err;
>diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>index 78c4acafd268..a8891833f980 100644
>--- a/drivers/gpu/drm/xe/xe_device_types.h
>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>@@ -16,6 +16,7 @@
> #include "xe_devcoredump_types.h"
> #include "xe_heci_gsc.h"
> #include "xe_lmtt_types.h"
>+#include "xe_late_bind_fw_types.h"

should be sorted

> #include "xe_memirq_types.h"
> #include "xe_oa_types.h"
> #include "xe_platform_types.h"
>@@ -325,6 +326,8 @@ struct xe_device {
> 		u8 has_heci_cscfi:1;
> 		/** @info.has_heci_gscfi: device has heci gscfi */
> 		u8 has_heci_gscfi:1;
>+		/** @info.has_late_bind: Device has firmware late binding support */
>+		u8 has_late_bind:1;
> 		/** @info.has_llc: Device has a shared CPU+GPU last level cache */
> 		u8 has_llc:1;
> 		/** @info.has_mbx_power_limits: Device has support to manage power limits using
>@@ -557,6 +560,9 @@ struct xe_device {
> 	/** @nvm: discrete graphics non-volatile memory */
> 	struct intel_dg_nvm_dev *nvm;
>
>+	/** @late_bind: xe mei late bind interface */
>+	struct xe_late_bind late_bind;
>+
> 	/** @oa: oa observation subsystem */
> 	struct xe_oa oa;
>
>diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>new file mode 100644
>index 000000000000..152f3e58de94
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>@@ -0,0 +1,84 @@
>+// SPDX-License-Identifier: MIT
>+/*
>+ * Copyright © 2025 Intel Corporation
>+ */
>+
>+#include <linux/component.h>
>+#include <linux/delay.h>
>+
>+#include <drm/drm_managed.h>
>+#include <drm/intel/i915_component.h>
>+#include <drm/intel/intel_lb_mei_interface.h>
>+#include <drm/drm_print.h>
>+
>+#include "xe_device.h"
>+#include "xe_late_bind_fw.h"
>+
>+static struct xe_device *
>+late_bind_to_xe(struct xe_late_bind *late_bind)
>+{
>+	return container_of(late_bind, struct xe_device, late_bind);
>+}
>+
>+static int xe_late_bind_component_bind(struct device *xe_kdev,
>+				       struct device *mei_kdev, void *data)
>+{
>+	struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>+	struct xe_late_bind *late_bind = &xe->late_bind;
>+
>+	late_bind->component.ops = data;
>+	late_bind->component.mei_dev = mei_kdev;
>+
>+	return 0;
>+}
>+
>+static void xe_late_bind_component_unbind(struct device *xe_kdev,
>+					  struct device *mei_kdev, void *data)
>+{
>+	struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>+	struct xe_late_bind *late_bind = &xe->late_bind;
>+
>+	late_bind->component.ops = NULL;
>+}
>+
>+static const struct component_ops xe_late_bind_component_ops = {
>+	.bind   = xe_late_bind_component_bind,
>+	.unbind = xe_late_bind_component_unbind,
>+};
>+
>+static void xe_late_bind_remove(void *arg)
>+{
>+	struct xe_late_bind *late_bind = arg;
>+	struct xe_device *xe = late_bind_to_xe(late_bind);
>+
>+	component_del(xe->drm.dev, &xe_late_bind_component_ops);
>+}
>+
>+/**
>+ * xe_late_bind_init() - add xe mei late binding component
>+ * @late_bind: pointer to late bind structure.
>+ *
>+ * Return: 0 if the initialization was successful, a negative errno otherwise.
>+ */
>+int xe_late_bind_init(struct xe_late_bind *late_bind)
>+{
>+	struct xe_device *xe = late_bind_to_xe(late_bind);
>+	int err;
>+
>+	if (!xe->info.has_late_bind)
>+		return 0;
>+
>+	if (!IS_ENABLED(CONFIG_INTEL_MEI_LATE_BIND) || !IS_ENABLED(CONFIG_INTEL_MEI_GSC)) {
>+		drm_info(&xe->drm, "Can't init xe mei late bind missing mei component\n");
>+		return -ENODEV;
>+	}
>+
>+	err = component_add_typed(xe->drm.dev, &xe_late_bind_component_ops,
>+				  INTEL_COMPONENT_LB);
>+	if (err < 0) {
>+		drm_info(&xe->drm, "Failed to add mei late bind component (%pe)\n", ERR_PTR(err));
>+		return err;

if you are going to return an error to stop the probe, then make this a
drm_err()

>+	}
>+
>+	return devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
>+}
>diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>new file mode 100644
>index 000000000000..4c73571c3e62
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>@@ -0,0 +1,15 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2025 Intel Corporation
>+ */
>+
>+#ifndef _XE_LATE_BIND_FW_H_
>+#define _XE_LATE_BIND_FW_H_
>+
>+#include <linux/types.h>
>+
>+struct xe_late_bind;
>+
>+int xe_late_bind_init(struct xe_late_bind *late_bind);
>+
>+#endif
>diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>new file mode 100644
>index 000000000000..f79e5aefed94
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>@@ -0,0 +1,33 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2025 Intel Corporation
>+ */
>+
>+#ifndef _XE_LATE_BIND_TYPES_H_
>+#define _XE_LATE_BIND_TYPES_H_
>+
>+#include <linux/iosys-map.h>
>+#include <linux/mutex.h>
>+#include <linux/types.h>
>+
>+/**
>+ * struct xe_late_bind_component - Late Binding services component
>+ * @mei_dev: device that provide Late Binding service.
>+ * @ops: Ops implemented by Late Binding driver, used by Xe driver.
>+ *
>+ * Communication between Xe and MEI drivers for Late Binding services
>+ */
>+struct xe_late_bind_component {
>+	struct device *mei_dev;
>+	const struct late_bind_component_ops *ops;
>+};
>+
>+/**
>+ * struct xe_late_bind
>+ */
>+struct xe_late_bind {
>+	/** @component: struct for communication with mei component */
>+	struct xe_late_bind_component component;
>+};
>+
>+#endif
>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>index ffd6ad569b7c..69b8ead9ca59 100644
>--- a/drivers/gpu/drm/xe/xe_pci.c
>+++ b/drivers/gpu/drm/xe/xe_pci.c
>@@ -332,6 +332,7 @@ static const struct xe_device_desc bmg_desc = {
> 	.has_gsc_nvm = 1,
> 	.has_heci_cscfi = 1,
> 	.max_gt_per_tile = 2,
>+	.has_late_bind = true,

again, boring is good: all the has_xxxx flags should be sorted
together.

Lucas De Marchi

> 	.needs_scratch = true,
> };
>
>@@ -578,6 +579,7 @@ static int xe_info_init_early(struct xe_device *xe,
> 	xe->info.has_gsc_nvm = desc->has_gsc_nvm;
> 	xe->info.has_heci_gscfi = desc->has_heci_gscfi;
> 	xe->info.has_heci_cscfi = desc->has_heci_cscfi;
>+	xe->info.has_late_bind = desc->has_late_bind;
> 	xe->info.has_llc = desc->has_llc;
> 	xe->info.has_pxp = desc->has_pxp;
> 	xe->info.has_sriov = desc->has_sriov;
>diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
>index 4de6f69ed975..51a607d323fb 100644
>--- a/drivers/gpu/drm/xe/xe_pci_types.h
>+++ b/drivers/gpu/drm/xe/xe_pci_types.h
>@@ -39,6 +39,7 @@ struct xe_device_desc {
> 	u8 has_gsc_nvm:1;
> 	u8 has_heci_gscfi:1;
> 	u8 has_heci_cscfi:1;
>+	u8 has_late_bind:1;
> 	u8 has_llc:1;
> 	u8 has_mbx_power_limits:1;
> 	u8 has_pxp:1;
>-- 
>2.49.0
>

Re: [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw

Posted by Nilawar, Badal 2 months, 1 week ago

On 26-07-2025 03:08, Lucas De Marchi wrote:
> On Thu, Jul 10, 2025 at 11:08:34AM -0400, Rodrigo Vivi wrote:
>> From: Badal Nilawar <badal.nilawar@intel.com>
>>
>> Introducing xe_late_bind_fw to enable firmware loading for the devices,
>
> here and in the subject: no gerund, please.
Sure.
>
>> such as the fan controller, during the driver probe. Typically,
>> firmware for such devices are part of IFWI flash image but can be
>> replaced at probe after OEM tuning.
>> This patch binds mei late binding component to enable firmware loading.
>>
>> v2:
>> - Add devm_add_action_or_reset to remove the component (Daniele)
>> - Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
>> v3:
>> - Fail driver probe if late bind initialization fails,
>>   add has_late_bind flag (Daniele)
>> v4:
>> - %S/I915_COMPONENT_LATE_BIND/INTEL_COMPONENT_LATE_BIND/
>> v6:
>> - rebased
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> drivers/gpu/drm/xe/Makefile                |  1 +
>> drivers/gpu/drm/xe/xe_device.c             |  5 ++
>> drivers/gpu/drm/xe/xe_device_types.h       |  6 ++
>> drivers/gpu/drm/xe/xe_late_bind_fw.c       | 84 ++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_late_bind_fw.h       | 15 ++++
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 33 +++++++++
>> drivers/gpu/drm/xe/xe_pci.c                |  2 +
>> drivers/gpu/drm/xe/xe_pci_types.h          |  1 +
>> 8 files changed, 147 insertions(+)
>> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.c
>> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.h
>> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index 1d97e5b63f4e..97e2b1368a6e 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -76,6 +76,7 @@ xe-y += xe_bb.o \
>>     xe_hw_fence.o \
>>     xe_irq.o \
>>     xe_lrc.o \
>> +    xe_late_bind_fw.o \
>
> almost there, but still not sorted as it should be
It seems the rebase messed up the order—it was correctly placed in the 
previous revision. I’ll fix it.
>
>>     xe_migrate.o \
>>     xe_mmio.o \
>>     xe_mocs.o \
>> diff --git a/drivers/gpu/drm/xe/xe_device.c 
>> b/drivers/gpu/drm/xe/xe_device.c
>> index 0b73cb72bad1..cb595bae5f55 100644
>> --- a/drivers/gpu/drm/xe/xe_device.c
>> +++ b/drivers/gpu/drm/xe/xe_device.c
>> @@ -44,6 +44,7 @@
>> #include "xe_hw_engine_group.h"
>> #include "xe_hwmon.h"
>> #include "xe_irq.h"
>> +#include "xe_late_bind_fw.h"
>> #include "xe_mmio.h"
>> #include "xe_module.h"
>> #include "xe_nvm.h"
>> @@ -866,6 +867,10 @@ int xe_device_probe(struct xe_device *xe)
>>     if (err)
>>         return err;
>>
>> +    err = xe_late_bind_init(&xe->late_bind);
>> +    if (err && err != -ENODEV)
>
> let's not be creative in xe_device_probe(). Boring is good.
>
> All the other parts in this function handle ENODEV internally. It's up
> to them to decide if ENODEV -> return 0, not here.
sure, will update accordingly.
>
>
>> +        return err;
>> +
>>     err = xe_oa_init(xe);
>>     if (err)
>>         return err;
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h 
>> b/drivers/gpu/drm/xe/xe_device_types.h
>> index 78c4acafd268..a8891833f980 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -16,6 +16,7 @@
>> #include "xe_devcoredump_types.h"
>> #include "xe_heci_gsc.h"
>> #include "xe_lmtt_types.h"
>> +#include "xe_late_bind_fw_types.h"
>
> should be sorted
Sure.
>
>> #include "xe_memirq_types.h"
>> #include "xe_oa_types.h"
>> #include "xe_platform_types.h"
>> @@ -325,6 +326,8 @@ struct xe_device {
>>         u8 has_heci_cscfi:1;
>>         /** @info.has_heci_gscfi: device has heci gscfi */
>>         u8 has_heci_gscfi:1;
>> +        /** @info.has_late_bind: Device has firmware late binding 
>> support */
>> +        u8 has_late_bind:1;
>>         /** @info.has_llc: Device has a shared CPU+GPU last level 
>> cache */
>>         u8 has_llc:1;
>>         /** @info.has_mbx_power_limits: Device has support to manage 
>> power limits using
>> @@ -557,6 +560,9 @@ struct xe_device {
>>     /** @nvm: discrete graphics non-volatile memory */
>>     struct intel_dg_nvm_dev *nvm;
>>
>> +    /** @late_bind: xe mei late bind interface */
>> +    struct xe_late_bind late_bind;
>> +
>>     /** @oa: oa observation subsystem */
>>     struct xe_oa oa;
>>
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c 
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> new file mode 100644
>> index 000000000000..152f3e58de94
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -0,0 +1,84 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2025 Intel Corporation
>> + */
>> +
>> +#include <linux/component.h>
>> +#include <linux/delay.h>
>> +
>> +#include <drm/drm_managed.h>
>> +#include <drm/intel/i915_component.h>
>> +#include <drm/intel/intel_lb_mei_interface.h>
>> +#include <drm/drm_print.h>
>> +
>> +#include "xe_device.h"
>> +#include "xe_late_bind_fw.h"
>> +
>> +static struct xe_device *
>> +late_bind_to_xe(struct xe_late_bind *late_bind)
>> +{
>> +    return container_of(late_bind, struct xe_device, late_bind);
>> +}
>> +
>> +static int xe_late_bind_component_bind(struct device *xe_kdev,
>> +                       struct device *mei_kdev, void *data)
>> +{
>> +    struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>> +    struct xe_late_bind *late_bind = &xe->late_bind;
>> +
>> +    late_bind->component.ops = data;
>> +    late_bind->component.mei_dev = mei_kdev;
>> +
>> +    return 0;
>> +}
>> +
>> +static void xe_late_bind_component_unbind(struct device *xe_kdev,
>> +                      struct device *mei_kdev, void *data)
>> +{
>> +    struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>> +    struct xe_late_bind *late_bind = &xe->late_bind;
>> +
>> +    late_bind->component.ops = NULL;
>> +}
>> +
>> +static const struct component_ops xe_late_bind_component_ops = {
>> +    .bind   = xe_late_bind_component_bind,
>> +    .unbind = xe_late_bind_component_unbind,
>> +};
>> +
>> +static void xe_late_bind_remove(void *arg)
>> +{
>> +    struct xe_late_bind *late_bind = arg;
>> +    struct xe_device *xe = late_bind_to_xe(late_bind);
>> +
>> +    component_del(xe->drm.dev, &xe_late_bind_component_ops);
>> +}
>> +
>> +/**
>> + * xe_late_bind_init() - add xe mei late binding component
>> + * @late_bind: pointer to late bind structure.
>> + *
>> + * Return: 0 if the initialization was successful, a negative errno 
>> otherwise.
>> + */
>> +int xe_late_bind_init(struct xe_late_bind *late_bind)
>> +{
>> +    struct xe_device *xe = late_bind_to_xe(late_bind);
>> +    int err;
>> +
>> +    if (!xe->info.has_late_bind)
>> +        return 0;
>> +
>> +    if (!IS_ENABLED(CONFIG_INTEL_MEI_LATE_BIND) || 
>> !IS_ENABLED(CONFIG_INTEL_MEI_GSC)) {
>> +        drm_info(&xe->drm, "Can't init xe mei late bind missing mei 
>> component\n");
>> +        return -ENODEV;
>> +    }
>> +
>> +    err = component_add_typed(xe->drm.dev, &xe_late_bind_component_ops,
>> +                  INTEL_COMPONENT_LB);
>> +    if (err < 0) {
>> +        drm_info(&xe->drm, "Failed to add mei late bind component 
>> (%pe)\n", ERR_PTR(err));
>> +        return err;
>
> if you are going to return an error to stop the probe, then make this a
> drm_err()
Sure.
>
>> +    }
>> +
>> +    return devm_add_action_or_reset(xe->drm.dev, 
>> xe_late_bind_remove, late_bind);
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h 
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> new file mode 100644
>> index 000000000000..4c73571c3e62
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2025 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_LATE_BIND_FW_H_
>> +#define _XE_LATE_BIND_FW_H_
>> +
>> +#include <linux/types.h>
>> +
>> +struct xe_late_bind;
>> +
>> +int xe_late_bind_init(struct xe_late_bind *late_bind);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h 
>> b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> new file mode 100644
>> index 000000000000..f79e5aefed94
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> @@ -0,0 +1,33 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2025 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_LATE_BIND_TYPES_H_
>> +#define _XE_LATE_BIND_TYPES_H_
>> +
>> +#include <linux/iosys-map.h>
>> +#include <linux/mutex.h>
>> +#include <linux/types.h>
>> +
>> +/**
>> + * struct xe_late_bind_component - Late Binding services component
>> + * @mei_dev: device that provide Late Binding service.
>> + * @ops: Ops implemented by Late Binding driver, used by Xe driver.
>> + *
>> + * Communication between Xe and MEI drivers for Late Binding services
>> + */
>> +struct xe_late_bind_component {
>> +    struct device *mei_dev;
>> +    const struct late_bind_component_ops *ops;
>> +};
>> +
>> +/**
>> + * struct xe_late_bind
>> + */
>> +struct xe_late_bind {
>> +    /** @component: struct for communication with mei component */
>> +    struct xe_late_bind_component component;
>> +};
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>> index ffd6ad569b7c..69b8ead9ca59 100644
>> --- a/drivers/gpu/drm/xe/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>> @@ -332,6 +332,7 @@ static const struct xe_device_desc bmg_desc = {
>>     .has_gsc_nvm = 1,
>>     .has_heci_cscfi = 1,
>>     .max_gt_per_tile = 2,
>> +    .has_late_bind = true,
>
> again, boring is good: all the has_xxxx flags should be sorted
> together.

Looks like the re-base affected this part as well. It was correctly 
grouped in the previous revision — I’ll fix this.

Thanks,
Badal

>
> Lucas De Marchi
>
>>     .needs_scratch = true,
>> };
>>
>> @@ -578,6 +579,7 @@ static int xe_info_init_early(struct xe_device *xe,
>>     xe->info.has_gsc_nvm = desc->has_gsc_nvm;
>>     xe->info.has_heci_gscfi = desc->has_heci_gscfi;
>>     xe->info.has_heci_cscfi = desc->has_heci_cscfi;
>> +    xe->info.has_late_bind = desc->has_late_bind;
>>     xe->info.has_llc = desc->has_llc;
>>     xe->info.has_pxp = desc->has_pxp;
>>     xe->info.has_sriov = desc->has_sriov;
>> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h 
>> b/drivers/gpu/drm/xe/xe_pci_types.h
>> index 4de6f69ed975..51a607d323fb 100644
>> --- a/drivers/gpu/drm/xe/xe_pci_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
>> @@ -39,6 +39,7 @@ struct xe_device_desc {
>>     u8 has_gsc_nvm:1;
>>     u8 has_heci_gscfi:1;
>>     u8 has_heci_cscfi:1;
>> +    u8 has_late_bind:1;
>>     u8 has_llc:1;
>>     u8 has_mbx_power_limits:1;
>>     u8 has_pxp:1;
>> -- 
>> 2.49.0
>>