* [PATCH v2 01/10] mei: bus: add mei_cldev_mtu interface
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
@ 2025-06-06 17:56 ` Badal Nilawar
2025-06-15 18:19 ` Umesh Nerlige Ramappa
2025-06-06 17:56 ` [PATCH v2 02/10] mei: late_bind: add late binding component driver Badal Nilawar
` (8 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:56 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
From: Alexander Usyskin <alexander.usyskin@intel.com>
Allow to bus client to obtain client mtu.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/misc/mei/bus.c | 13 +++++++++++++
include/linux/mei_cl_bus.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 67176caf5416..f860b1b6eda0 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -614,6 +614,19 @@ u8 mei_cldev_ver(const struct mei_cl_device *cldev)
}
EXPORT_SYMBOL_GPL(mei_cldev_ver);
+/**
+ * mei_cldev_mtu - max message that client can send and receive
+ *
+ * @cldev: mei client device
+ *
+ * Return: mtu or 0 if client is not connected
+ */
+size_t mei_cldev_mtu(const struct mei_cl_device *cldev)
+{
+ return mei_cl_mtu(cldev->cl);
+}
+EXPORT_SYMBOL_GPL(mei_cldev_mtu);
+
/**
* mei_cldev_enabled - check whether the device is enabled
*
diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
index 725fd7727422..a82755e1fc40 100644
--- a/include/linux/mei_cl_bus.h
+++ b/include/linux/mei_cl_bus.h
@@ -113,6 +113,7 @@ int mei_cldev_register_notif_cb(struct mei_cl_device *cldev,
mei_cldev_cb_t notif_cb);
u8 mei_cldev_ver(const struct mei_cl_device *cldev);
+size_t mei_cldev_mtu(const struct mei_cl_device *cldev);
void *mei_cldev_get_drvdata(const struct mei_cl_device *cldev);
void mei_cldev_set_drvdata(struct mei_cl_device *cldev, void *data);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 01/10] mei: bus: add mei_cldev_mtu interface
2025-06-06 17:56 ` [PATCH v2 01/10] mei: bus: add mei_cldev_mtu interface Badal Nilawar
@ 2025-06-15 18:19 ` Umesh Nerlige Ramappa
0 siblings, 0 replies; 28+ messages in thread
From: Umesh Nerlige Ramappa @ 2025-06-15 18:19 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-xe, dri-devel, anshuman.gupta, rodrigo.vivi,
alexander.usyskin, gregkh, daniele.ceraolospurio, jgg
On Fri, Jun 06, 2025 at 11:26:58PM +0530, Badal Nilawar wrote:
>From: Alexander Usyskin <alexander.usyskin@intel.com>
>
>Allow to bus client to obtain client mtu.
>
>Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
>Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
LGTM,
Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>---
> drivers/misc/mei/bus.c | 13 +++++++++++++
> include/linux/mei_cl_bus.h | 1 +
> 2 files changed, 14 insertions(+)
>
>diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
>index 67176caf5416..f860b1b6eda0 100644
>--- a/drivers/misc/mei/bus.c
>+++ b/drivers/misc/mei/bus.c
>@@ -614,6 +614,19 @@ u8 mei_cldev_ver(const struct mei_cl_device *cldev)
> }
> EXPORT_SYMBOL_GPL(mei_cldev_ver);
>
>+/**
>+ * mei_cldev_mtu - max message that client can send and receive
>+ *
>+ * @cldev: mei client device
>+ *
>+ * Return: mtu or 0 if client is not connected
>+ */
>+size_t mei_cldev_mtu(const struct mei_cl_device *cldev)
>+{
>+ return mei_cl_mtu(cldev->cl);
>+}
>+EXPORT_SYMBOL_GPL(mei_cldev_mtu);
>+
> /**
> * mei_cldev_enabled - check whether the device is enabled
> *
>diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
>index 725fd7727422..a82755e1fc40 100644
>--- a/include/linux/mei_cl_bus.h
>+++ b/include/linux/mei_cl_bus.h
>@@ -113,6 +113,7 @@ int mei_cldev_register_notif_cb(struct mei_cl_device *cldev,
> mei_cldev_cb_t notif_cb);
>
> u8 mei_cldev_ver(const struct mei_cl_device *cldev);
>+size_t mei_cldev_mtu(const struct mei_cl_device *cldev);
>
> void *mei_cldev_get_drvdata(const struct mei_cl_device *cldev);
> void mei_cldev_set_drvdata(struct mei_cl_device *cldev, void *data);
>--
>2.34.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 02/10] mei: late_bind: add late binding component driver
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
2025-06-06 17:56 ` [PATCH v2 01/10] mei: bus: add mei_cldev_mtu interface Badal Nilawar
@ 2025-06-06 17:56 ` Badal Nilawar
2025-06-14 8:02 ` Gupta, Anshuman
2025-06-06 17:57 ` [PATCH v2 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Badal Nilawar
` (7 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:56 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
From: Alexander Usyskin <alexander.usyskin@intel.com>
Add late binding component driver.
It allows pushing the late binding configuration from, for example,
the Xe graphics driver to the Intel discrete graphics card's CSE device.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
v2:
- Use generic naming (Jani)
- Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha)
---
drivers/misc/mei/Kconfig | 1 +
drivers/misc/mei/Makefile | 1 +
drivers/misc/mei/late_bind/Kconfig | 12 +
drivers/misc/mei/late_bind/Makefile | 9 +
drivers/misc/mei/late_bind/mei_late_bind.c | 261 ++++++++++++++++++++
include/drm/intel/i915_component.h | 1 +
include/drm/intel/late_bind_mei_interface.h | 37 +++
7 files changed, 322 insertions(+)
create mode 100644 drivers/misc/mei/late_bind/Kconfig
create mode 100644 drivers/misc/mei/late_bind/Makefile
create mode 100644 drivers/misc/mei/late_bind/mei_late_bind.c
create mode 100644 include/drm/intel/late_bind_mei_interface.h
diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index 7575fee96cc6..771becc68095 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -84,5 +84,6 @@ config INTEL_MEI_VSC
source "drivers/misc/mei/hdcp/Kconfig"
source "drivers/misc/mei/pxp/Kconfig"
source "drivers/misc/mei/gsc_proxy/Kconfig"
+source "drivers/misc/mei/late_bind/Kconfig"
endif
diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
index 6f9fdbf1a495..84bfde888d81 100644
--- a/drivers/misc/mei/Makefile
+++ b/drivers/misc/mei/Makefile
@@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src)
obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/
obj-$(CONFIG_INTEL_MEI_PXP) += pxp/
obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
+obj-$(CONFIG_INTEL_MEI_LATE_BIND) += late_bind/
obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o
mei-vsc-hw-y := vsc-tp.o
diff --git a/drivers/misc/mei/late_bind/Kconfig b/drivers/misc/mei/late_bind/Kconfig
new file mode 100644
index 000000000000..c5302303e5af
--- /dev/null
+++ b/drivers/misc/mei/late_bind/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025, Intel Corporation. All rights reserved.
+#
+config INTEL_MEI_LATE_BIND
+ tristate "Intel late binding support on ME Interface"
+ select INTEL_MEI_ME
+ depends on DRM_XE
+ help
+ MEI Support for Late Binding for Intel graphics card.
+
+ Enables the ME FW interfaces for Late Binding for
+ Xe display driver of Intel.
diff --git a/drivers/misc/mei/late_bind/Makefile b/drivers/misc/mei/late_bind/Makefile
new file mode 100644
index 000000000000..a0aeda5853f0
--- /dev/null
+++ b/drivers/misc/mei/late_bind/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2025, Intel Corporation. All rights reserved.
+#
+# Makefile - Late Binding client driver for Intel MEI Bus Driver.
+
+subdir-ccflags-y += -I$(srctree)/drivers/misc/mei/
+
+obj-$(CONFIG_INTEL_MEI_LATE_BIND) += mei_late_bind.o
diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c b/drivers/misc/mei/late_bind/mei_late_bind.c
new file mode 100644
index 000000000000..964757a9b33a
--- /dev/null
+++ b/drivers/misc/mei/late_bind/mei_late_bind.c
@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Intel Corporation
+ */
+#include <drm/drm_connector.h>
+#include <drm/intel/i915_component.h>
+#include <drm/intel/late_bind_mei_interface.h>
+#include <linux/component.h>
+#include <linux/pci.h>
+#include <linux/mei_cl_bus.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/slab.h>
+#include <linux/uuid.h>
+
+#include "mkhi.h"
+
+#define GFX_SRV_MKHI_LATE_BINDING_CMD 0x12
+#define GFX_SRV_MKHI_LATE_BINDING_RSP (GFX_SRV_MKHI_LATE_BINDING_CMD | 0x80)
+
+#define LATE_BIND_SEND_TIMEOUT_MSEC 3000
+#define LATE_BIND_RECV_TIMEOUT_MSEC 3000
+
+/**
+ * struct csc_heci_late_bind_req - late binding request
+ * @header: @ref mkhi_msg_hdr
+ * @type: type of the late binding payload
+ * @flags: flags to be passed to the firmware
+ * @reserved: reserved field
+ * @payload_size: size of the payload data in bytes
+ * @payload: data to be sent to the firmware
+ */
+struct csc_heci_late_bind_req {
+ struct mkhi_msg_hdr header;
+ u32 type;
+ u32 flags;
+ u32 reserved[2];
+ u32 payload_size;
+ u8 payload[] __counted_by(payload_size);
+} __packed;
+
+/**
+ * struct csc_heci_late_bind_rsp - late binding response
+ * @header: @ref mkhi_msg_hdr
+ * @type: type of the late binding payload
+ * @reserved: reserved field
+ * @status: status of the late binding command execution by firmware
+ */
+struct csc_heci_late_bind_rsp {
+ struct mkhi_msg_hdr header;
+ u32 type;
+ u32 reserved[2];
+ u32 status;
+} __packed;
+
+static int mei_late_bind_check_response(const struct device *dev, const struct mkhi_msg_hdr *hdr)
+{
+ if (hdr->group_id != MKHI_GROUP_ID_GFX) {
+ dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n",
+ hdr->group_id, MKHI_GROUP_ID_GFX);
+ return -EINVAL;
+ }
+
+ if (hdr->command != GFX_SRV_MKHI_LATE_BINDING_RSP) {
+ dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n",
+ hdr->command, GFX_SRV_MKHI_LATE_BINDING_RSP);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/**
+ * mei_late_bind_push_config - Sends a config to the firmware.
+ * @dev: device struct corresponding to the mei device
+ * @type: payload type
+ * @flags: payload flags
+ * @payload: payload buffer
+ * @payload_size: payload buffer size
+ *
+ * Return: 0 success, negative errno value on transport failure,
+ * positive status returned by FW
+ */
+static int mei_late_bind_push_config(struct device *dev, u32 type, u32 flags,
+ const void *payload, size_t payload_size)
+{
+ struct mei_cl_device *cldev;
+ struct csc_heci_late_bind_req *req = NULL;
+ struct csc_heci_late_bind_rsp rsp;
+ size_t req_size;
+ int ret;
+
+ if (!dev || !payload || !payload_size)
+ return -EINVAL;
+
+ cldev = to_mei_cl_device(dev);
+
+ ret = mei_cldev_enable(cldev);
+ if (ret < 0) {
+ dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret);
+ return ret;
+ }
+
+ req_size = struct_size(req, payload, payload_size);
+ if (req_size > mei_cldev_mtu(cldev)) {
+ dev_err(dev, "Payload is too big %zu\n", payload_size);
+ ret = -EMSGSIZE;
+ goto end;
+ }
+
+ req = kmalloc(req_size, GFP_KERNEL);
+ if (!req) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ req->header.group_id = MKHI_GROUP_ID_GFX;
+ req->header.command = GFX_SRV_MKHI_LATE_BINDING_CMD;
+ req->type = type;
+ req->flags = flags;
+ req->reserved[0] = 0;
+ req->reserved[1] = 0;
+ req->payload_size = payload_size;
+ memcpy(req->payload, payload, payload_size);
+
+ ret = mei_cldev_send_timeout(cldev, (void *)req, req_size, LATE_BIND_SEND_TIMEOUT_MSEC);
+ if (ret < 0) {
+ dev_err(dev, "mei_cldev_send failed. %d\n", ret);
+ goto end;
+ }
+ ret = mei_cldev_recv_timeout(cldev, (void *)&rsp, sizeof(rsp), LATE_BIND_RECV_TIMEOUT_MSEC);
+ if (ret < 0) {
+ dev_err(dev, "mei_cldev_recv failed. %d\n", ret);
+ goto end;
+ }
+ ret = mei_late_bind_check_response(dev, &rsp.header);
+ if (ret) {
+ dev_err(dev, "bad result response from the firmware: 0x%x\n",
+ *(uint32_t *)&rsp.header);
+ goto end;
+ }
+ ret = (int)rsp.status;
+ dev_dbg(dev, "mei_late_bind_push_config status = %d\n", ret);
+
+end:
+ mei_cldev_disable(cldev);
+ kfree(req);
+ return ret;
+}
+
+static const struct late_bind_component_ops mei_late_bind_ops = {
+ .owner = THIS_MODULE,
+ .push_config = mei_late_bind_push_config,
+};
+
+static int mei_component_master_bind(struct device *dev)
+{
+ return component_bind_all(dev, (void *)&mei_late_bind_ops);
+}
+
+static void mei_component_master_unbind(struct device *dev)
+{
+ component_unbind_all(dev, (void *)&mei_late_bind_ops);
+}
+
+static const struct component_master_ops mei_component_master_ops = {
+ .bind = mei_component_master_bind,
+ .unbind = mei_component_master_unbind,
+};
+
+/**
+ * mei_late_bind_component_match - compare function for matching mei late bind.
+ *
+ * The function checks if requested is Intel VGA device
+ * and the parent of requester and the grand parent of mei_if are the same
+ * device.
+ *
+ * @dev: master device
+ * @subcomponent: subcomponent to match (I915_COMPONENT_LATE_BIND)
+ * @data: compare data (mei late-bind bus device)
+ *
+ * Return:
+ * * 1 - if components match
+ * * 0 - otherwise
+ */
+static int mei_late_bind_component_match(struct device *dev, int subcomponent,
+ void *data)
+{
+ struct device *base = data;
+ struct pci_dev *pdev;
+
+ if (!dev)
+ return 0;
+
+ if (!dev_is_pci(dev))
+ return 0;
+
+ pdev = to_pci_dev(dev);
+
+ if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) ||
+ pdev->vendor != PCI_VENDOR_ID_INTEL)
+ return 0;
+
+ if (subcomponent != I915_COMPONENT_LATE_BIND)
+ return 0;
+
+ base = base->parent;
+ if (!base) /* mei device */
+ return 0;
+
+ base = base->parent; /* pci device */
+
+ return !!base && dev == base;
+}
+
+static int mei_late_bind_probe(struct mei_cl_device *cldev,
+ const struct mei_cl_device_id *id)
+{
+ struct component_match *master_match = NULL;
+ int ret;
+
+ component_match_add_typed(&cldev->dev, &master_match,
+ mei_late_bind_component_match, &cldev->dev);
+ if (IS_ERR_OR_NULL(master_match))
+ return -ENOMEM;
+
+ ret = component_master_add_with_match(&cldev->dev,
+ &mei_component_master_ops,
+ master_match);
+ if (ret < 0)
+ dev_err(&cldev->dev, "Master comp add failed %d\n", ret);
+
+ return ret;
+}
+
+static void mei_late_bind_remove(struct mei_cl_device *cldev)
+{
+ component_master_del(&cldev->dev, &mei_component_master_ops);
+}
+
+#define MEI_GUID_MKHI UUID_LE(0xe2c2afa2, 0x3817, 0x4d19, \
+ 0x9d, 0x95, 0x6, 0xb1, 0x6b, 0x58, 0x8a, 0x5d)
+
+static struct mei_cl_device_id mei_late_bind_tbl[] = {
+ { .uuid = MEI_GUID_MKHI, .version = MEI_CL_VERSION_ANY },
+ { }
+};
+MODULE_DEVICE_TABLE(mei, mei_late_bind_tbl);
+
+static struct mei_cl_driver mei_late_bind_driver = {
+ .id_table = mei_late_bind_tbl,
+ .name = KBUILD_MODNAME,
+ .probe = mei_late_bind_probe,
+ .remove = mei_late_bind_remove,
+};
+
+module_mei_cl_driver(mei_late_bind_driver);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MEI Late Binding");
diff --git a/include/drm/intel/i915_component.h b/include/drm/intel/i915_component.h
index 4ea3b17aa143..4945044d41e6 100644
--- a/include/drm/intel/i915_component.h
+++ b/include/drm/intel/i915_component.h
@@ -31,6 +31,7 @@ enum i915_component_type {
I915_COMPONENT_HDCP,
I915_COMPONENT_PXP,
I915_COMPONENT_GSC_PROXY,
+ I915_COMPONENT_LATE_BIND,
};
/* MAX_PORT is the number of port
diff --git a/include/drm/intel/late_bind_mei_interface.h b/include/drm/intel/late_bind_mei_interface.h
new file mode 100644
index 000000000000..6b54b8cec5ae
--- /dev/null
+++ b/include/drm/intel/late_bind_mei_interface.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (c) 2025 Intel Corporation
+ */
+
+#ifndef _LATE_BIND_MEI_INTERFACE_H_
+#define _LATE_BIND_MEI_INTERFACE_H_
+
+#include <linux/types.h>
+
+struct device;
+struct module;
+
+/**
+ * struct late_bind_component_ops - ops for Late Binding services.
+ * @owner: Module providing the ops
+ * @push_config: Sends a config to FW.
+ */
+struct late_bind_component_ops {
+ struct module *owner;
+
+ /**
+ * @push_config: Sends a config to FW.
+ * @dev: device struct corresponding to the mei device
+ * @type: payload type
+ * @flags: payload flags
+ * @payload: payload buffer
+ * @payload_size: payload buffer size
+ *
+ * Return: 0 success, negative errno value on transport failure,
+ * positive status returned by FW
+ */
+ int (*push_config)(struct device *dev, u32 type, u32 flags,
+ const void *payload, size_t payload_size);
+};
+
+#endif /* _LATE_BIND_MEI_INTERFACE_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* RE: [PATCH v2 02/10] mei: late_bind: add late binding component driver
2025-06-06 17:56 ` [PATCH v2 02/10] mei: late_bind: add late binding component driver Badal Nilawar
@ 2025-06-14 8:02 ` Gupta, Anshuman
2025-06-16 15:13 ` Nilawar, Badal
0 siblings, 1 reply; 28+ messages in thread
From: Gupta, Anshuman @ 2025-06-14 8:02 UTC (permalink / raw)
To: Nilawar, Badal, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, Usyskin, Alexander
Cc: Vivi, Rodrigo, gregkh@linuxfoundation.org,
Ceraolo Spurio, Daniele, jgg@nvidia.com
> -----Original Message-----
> From: Nilawar, Badal <badal.nilawar@intel.com>
> Sent: Friday, June 6, 2025 11:27 PM
> To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; Usyskin, Alexander <alexander.usyskin@intel.com>;
> gregkh@linuxfoundation.org; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@intel.com>; jgg@nvidia.com
> Subject: [PATCH v2 02/10] mei: late_bind: add late binding component driver
>
> From: Alexander Usyskin <alexander.usyskin@intel.com>
>
> Add late binding component driver.
> It allows pushing the late binding configuration from, for example, the Xe
> graphics driver to the Intel discrete graphics card's CSE device.
>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> v2:
> - Use generic naming (Jani)
> - Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha)
> ---
> drivers/misc/mei/Kconfig | 1 +
> drivers/misc/mei/Makefile | 1 +
> drivers/misc/mei/late_bind/Kconfig | 12 +
> drivers/misc/mei/late_bind/Makefile | 9 +
> drivers/misc/mei/late_bind/mei_late_bind.c | 261 ++++++++++++++++++++
> include/drm/intel/i915_component.h | 1 +
> include/drm/intel/late_bind_mei_interface.h | 37 +++
> 7 files changed, 322 insertions(+)
> create mode 100644 drivers/misc/mei/late_bind/Kconfig
> create mode 100644 drivers/misc/mei/late_bind/Makefile
> create mode 100644 drivers/misc/mei/late_bind/mei_late_bind.c
> create mode 100644 include/drm/intel/late_bind_mei_interface.h
>
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
> 7575fee96cc6..771becc68095 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -84,5 +84,6 @@ config INTEL_MEI_VSC
> source "drivers/misc/mei/hdcp/Kconfig"
> source "drivers/misc/mei/pxp/Kconfig"
> source "drivers/misc/mei/gsc_proxy/Kconfig"
> +source "drivers/misc/mei/late_bind/Kconfig"
>
> endif
> diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile index
> 6f9fdbf1a495..84bfde888d81 100644
> --- a/drivers/misc/mei/Makefile
> +++ b/drivers/misc/mei/Makefile
> @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src)
> obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/
> obj-$(CONFIG_INTEL_MEI_PXP) += pxp/
> obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
> +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += late_bind/
>
> obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o mei-vsc-hw-y := vsc-
> tp.o diff --git a/drivers/misc/mei/late_bind/Kconfig
> b/drivers/misc/mei/late_bind/Kconfig
> new file mode 100644
> index 000000000000..c5302303e5af
> --- /dev/null
> +++ b/drivers/misc/mei/late_bind/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2025, Intel Corporation. All rights reserved.
> +#
> +config INTEL_MEI_LATE_BIND
> + tristate "Intel late binding support on ME Interface"
> + select INTEL_MEI_ME
> + depends on DRM_XE
> + help
> + MEI Support for Late Binding for Intel graphics card.
> +
> + Enables the ME FW interfaces for Late Binding for
> + Xe display driver of Intel.
It is needed even for headless cards as well.
> diff --git a/drivers/misc/mei/late_bind/Makefile
> b/drivers/misc/mei/late_bind/Makefile
> new file mode 100644
> index 000000000000..a0aeda5853f0
> --- /dev/null
> +++ b/drivers/misc/mei/late_bind/Makefile
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (c) 2025, Intel Corporation. All rights reserved.
> +#
> +# Makefile - Late Binding client driver for Intel MEI Bus Driver.
> +
> +subdir-ccflags-y += -I$(srctree)/drivers/misc/mei/
> +
> +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += mei_late_bind.o
> diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c
> b/drivers/misc/mei/late_bind/mei_late_bind.c
> new file mode 100644
> index 000000000000..964757a9b33a
> --- /dev/null
> +++ b/drivers/misc/mei/late_bind/mei_late_bind.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 Intel Corporation */ #include
> +<drm/drm_connector.h> #include <drm/intel/i915_component.h> #include
Why do we need drm_connector header ?
Remove unnecessary headers.
> +<drm/intel/late_bind_mei_interface.h>
> +#include <linux/component.h>
> +#include <linux/pci.h>
> +#include <linux/mei_cl_bus.h>
> +#include <linux/module.h>
> +#include <linux/overflow.h>
> +#include <linux/slab.h>
> +#include <linux/uuid.h>
> +
> +#include "mkhi.h"
> +
> +#define GFX_SRV_MKHI_LATE_BINDING_CMD 0x12 #define
Why it is not prefixed with DGFX. This is unique to DGFX cards ?
@alexander any comment on above ?
> +GFX_SRV_MKHI_LATE_BINDING_RSP (GFX_SRV_MKHI_LATE_BINDING_CMD
> | 0x80)
> +
> +#define LATE_BIND_SEND_TIMEOUT_MSEC 3000 #define
> +LATE_BIND_RECV_TIMEOUT_MSEC 3000
From where we got these delay numbers, from any specs or it is arbitrary delay specific to let bind ?
Add a code comment in that case.
> +
> +/**
> + * struct csc_heci_late_bind_req - late binding request
> + * @header: @ref mkhi_msg_hdr
> + * @type: type of the late binding payload
> + * @flags: flags to be passed to the firmware
> + * @reserved: reserved field
> + * @payload_size: size of the payload data in bytes
> + * @payload: data to be sent to the firmware */ struct
> +csc_heci_late_bind_req {
> + struct mkhi_msg_hdr header;
> + u32 type;
> + u32 flags;
> + u32 reserved[2];
> + u32 payload_size;
> + u8 payload[] __counted_by(payload_size); } __packed;
> +
> +/**
> + * struct csc_heci_late_bind_rsp - late binding response
> + * @header: @ref mkhi_msg_hdr
> + * @type: type of the late binding payload
> + * @reserved: reserved field
> + * @status: status of the late binding command execution by firmware
> +*/ struct csc_heci_late_bind_rsp {
> + struct mkhi_msg_hdr header;
> + u32 type;
> + u32 reserved[2];
> + u32 status;
> +} __packed;
> +
> +static int mei_late_bind_check_response(const struct device *dev, const
> +struct mkhi_msg_hdr *hdr) {
> + if (hdr->group_id != MKHI_GROUP_ID_GFX) {
> + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n",
> + hdr->group_id, MKHI_GROUP_ID_GFX);
> + return -EINVAL;
> + }
> +
> + if (hdr->command != GFX_SRV_MKHI_LATE_BINDING_RSP) {
> + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n",
> + hdr->command,
> GFX_SRV_MKHI_LATE_BINDING_RSP);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * mei_late_bind_push_config - Sends a config to the firmware.
> + * @dev: device struct corresponding to the mei device
> + * @type: payload type
> + * @flags: payload flags
> + * @payload: payload buffer
> + * @payload_size: payload buffer size
> + *
> + * Return: 0 success, negative errno value on transport failure,
> + * positive status returned by FW
> + */
> +static int mei_late_bind_push_config(struct device *dev, u32 type, u32 flags,
> + const void *payload, size_t payload_size) {
> + struct mei_cl_device *cldev;
> + struct csc_heci_late_bind_req *req = NULL;
> + struct csc_heci_late_bind_rsp rsp;
> + size_t req_size;
> + int ret;
> +
> + if (!dev || !payload || !payload_size)
> + return -EINVAL;
> +
> + cldev = to_mei_cl_device(dev);
> +
> + ret = mei_cldev_enable(cldev);
> + if (ret < 0) {
> + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret);
> + return ret;
> + }
> +
> + req_size = struct_size(req, payload, payload_size);
> + if (req_size > mei_cldev_mtu(cldev)) {
> + dev_err(dev, "Payload is too big %zu\n", payload_size);
> + ret = -EMSGSIZE;
> + goto end;
> + }
> +
> + req = kmalloc(req_size, GFP_KERNEL);
> + if (!req) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + req->header.group_id = MKHI_GROUP_ID_GFX;
> + req->header.command = GFX_SRV_MKHI_LATE_BINDING_CMD;
> + req->type = type;
> + req->flags = flags;
> + req->reserved[0] = 0;
> + req->reserved[1] = 0;
> + req->payload_size = payload_size;
> + memcpy(req->payload, payload, payload_size);
> +
> + ret = mei_cldev_send_timeout(cldev, (void *)req, req_size,
> LATE_BIND_SEND_TIMEOUT_MSEC);
> + if (ret < 0) {
> + dev_err(dev, "mei_cldev_send failed. %d\n", ret);
> + goto end;
> + }
> + ret = mei_cldev_recv_timeout(cldev, (void *)&rsp, sizeof(rsp),
> LATE_BIND_RECV_TIMEOUT_MSEC);
> + if (ret < 0) {
> + dev_err(dev, "mei_cldev_recv failed. %d\n", ret);
> + goto end;
> + }
> + ret = mei_late_bind_check_response(dev, &rsp.header);
> + if (ret) {
> + dev_err(dev, "bad result response from the firmware:
> 0x%x\n",
> + *(uint32_t *)&rsp.header);
> + goto end;
> + }
> + ret = (int)rsp.status;
> + dev_dbg(dev, "mei_late_bind_push_config status = %d\n", ret);
> +
> +end:
> + mei_cldev_disable(cldev);
> + kfree(req);
> + return ret;
> +}
> +
> +static const struct late_bind_component_ops mei_late_bind_ops = {
> + .owner = THIS_MODULE,
> + .push_config = mei_late_bind_push_config, };
> +
> +static int mei_component_master_bind(struct device *dev) {
> + return component_bind_all(dev, (void *)&mei_late_bind_ops); }
> +
> +static void mei_component_master_unbind(struct device *dev) {
> + component_unbind_all(dev, (void *)&mei_late_bind_ops); }
> +
> +static const struct component_master_ops mei_component_master_ops = {
> + .bind = mei_component_master_bind,
> + .unbind = mei_component_master_unbind, };
> +
> +/**
> + * mei_late_bind_component_match - compare function for matching mei
> late bind.
> + *
> + * The function checks if requested is Intel VGA device
> + * and the parent of requester and the grand parent of mei_if are the same
> + * device.
This might be broken for headless card, headless care need to necessarily be VGA card.
> + *
> + * @dev: master device
> + * @subcomponent: subcomponent to match
> (I915_COMPONENT_LATE_BIND)
> + * @data: compare data (mei late-bind bus device)
> + *
> + * Return:
> + * * 1 - if components match
> + * * 0 - otherwise
> + */
> +static int mei_late_bind_component_match(struct device *dev, int
> subcomponent,
> + void *data)
> +{
> + struct device *base = data;
> + struct pci_dev *pdev;
> +
> + if (!dev)
> + return 0;
> +
> + if (!dev_is_pci(dev))
> + return 0;
> +
> + pdev = to_pci_dev(dev);
> +
> + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) ||
> + pdev->vendor != PCI_VENDOR_ID_INTEL)
> + return 0;
> +
> + if (subcomponent != I915_COMPONENT_LATE_BIND)
> + return 0;
Why I915_COMPONENT_LATE_BIND ?
Isn't this should be XE_ COMPONENT_LATE_BIND.
> +
> + base = base->parent;
> + if (!base) /* mei device */
> + return 0;
> +
> + base = base->parent; /* pci device */
Is it sgunit pci endpoint ?
If yes then good to mention in comment.
> +
> + return !!base && dev == base;
> +}
> +
> +static int mei_late_bind_probe(struct mei_cl_device *cldev,
> + const struct mei_cl_device_id *id) {
> + struct component_match *master_match = NULL;
> + int ret;
> +
> + component_match_add_typed(&cldev->dev, &master_match,
> + mei_late_bind_component_match, &cldev-
> >dev);
> + if (IS_ERR_OR_NULL(master_match))
> + return -ENOMEM;
> +
> + ret = component_master_add_with_match(&cldev->dev,
> + &mei_component_master_ops,
> + master_match);
> + if (ret < 0)
> + dev_err(&cldev->dev, "Master comp add failed %d\n", ret);
> +
> + return ret;
> +}
> +
> +static void mei_late_bind_remove(struct mei_cl_device *cldev) {
> + component_master_del(&cldev->dev,
> &mei_component_master_ops); }
> +
> +#define MEI_GUID_MKHI UUID_LE(0xe2c2afa2, 0x3817, 0x4d19, \
> + 0x9d, 0x95, 0x6, 0xb1, 0x6b, 0x58, 0x8a, 0x5d)
> +
> +static struct mei_cl_device_id mei_late_bind_tbl[] = {
> + { .uuid = MEI_GUID_MKHI, .version = MEI_CL_VERSION_ANY },
> + { }
> +};
> +MODULE_DEVICE_TABLE(mei, mei_late_bind_tbl);
> +
> +static struct mei_cl_driver mei_late_bind_driver = {
> + .id_table = mei_late_bind_tbl,
> + .name = KBUILD_MODNAME,
> + .probe = mei_late_bind_probe,
> + .remove = mei_late_bind_remove,
> +};
> +
> +module_mei_cl_driver(mei_late_bind_driver);
> +
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("MEI Late Binding");
> diff --git a/include/drm/intel/i915_component.h
> b/include/drm/intel/i915_component.h
> index 4ea3b17aa143..4945044d41e6 100644
> --- a/include/drm/intel/i915_component.h
> +++ b/include/drm/intel/i915_component.h
> @@ -31,6 +31,7 @@ enum i915_component_type {
> I915_COMPONENT_HDCP,
> I915_COMPONENT_PXP,
> I915_COMPONENT_GSC_PROXY,
> + I915_COMPONENT_LATE_BIND,
We probably need a change here to make a xe specific component.
Thanks,
Anshuman Gupta.
> };
>
> /* MAX_PORT is the number of port
> diff --git a/include/drm/intel/late_bind_mei_interface.h
> b/include/drm/intel/late_bind_mei_interface.h
> new file mode 100644
> index 000000000000..6b54b8cec5ae
> --- /dev/null
> +++ b/include/drm/intel/late_bind_mei_interface.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright (c) 2025 Intel Corporation */
> +
> +#ifndef _LATE_BIND_MEI_INTERFACE_H_
> +#define _LATE_BIND_MEI_INTERFACE_H_
> +
> +#include <linux/types.h>
> +
> +struct device;
> +struct module;
> +
> +/**
> + * struct late_bind_component_ops - ops for Late Binding services.
> + * @owner: Module providing the ops
> + * @push_config: Sends a config to FW.
> + */
> +struct late_bind_component_ops {
> + struct module *owner;
> +
> + /**
> + * @push_config: Sends a config to FW.
> + * @dev: device struct corresponding to the mei device
> + * @type: payload type
> + * @flags: payload flags
> + * @payload: payload buffer
> + * @payload_size: payload buffer size
> + *
> + * Return: 0 success, negative errno value on transport failure,
> + * positive status returned by FW
> + */
> + int (*push_config)(struct device *dev, u32 type, u32 flags,
> + const void *payload, size_t payload_size); };
> +
> +#endif /* _LATE_BIND_MEI_INTERFACE_H_ */
> --
> 2.34.1
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 02/10] mei: late_bind: add late binding component driver
2025-06-14 8:02 ` Gupta, Anshuman
@ 2025-06-16 15:13 ` Nilawar, Badal
0 siblings, 0 replies; 28+ messages in thread
From: Nilawar, Badal @ 2025-06-16 15:13 UTC (permalink / raw)
To: Gupta, Anshuman, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, Usyskin, Alexander
Cc: Vivi, Rodrigo, gregkh@linuxfoundation.org,
Ceraolo Spurio, Daniele, jgg@nvidia.com
[-- Attachment #1: Type: text/plain, Size: 14838 bytes --]
On 14-06-2025 13:32, Gupta, Anshuman wrote:
>
>> -----Original Message-----
>> From: Nilawar, Badal<badal.nilawar@intel.com>
>> Sent: Friday, June 6, 2025 11:27 PM
>> To:intel-xe@lists.freedesktop.org;dri-devel@lists.freedesktop.org
>> Cc: Gupta, Anshuman<anshuman.gupta@intel.com>; Vivi, Rodrigo
>> <rodrigo.vivi@intel.com>; Usyskin, Alexander<alexander.usyskin@intel.com>;
>> gregkh@linuxfoundation.org; Ceraolo Spurio, Daniele
>> <daniele.ceraolospurio@intel.com>;jgg@nvidia.com
>> Subject: [PATCH v2 02/10] mei: late_bind: add late binding component driver
>>
>> From: Alexander Usyskin<alexander.usyskin@intel.com>
>>
>> Add late binding component driver.
>> It allows pushing the late binding configuration from, for example, the Xe
>> graphics driver to the Intel discrete graphics card's CSE device.
>>
>> Signed-off-by: Alexander Usyskin<alexander.usyskin@intel.com>
>> Signed-off-by: Badal Nilawar<badal.nilawar@intel.com>
>> ---
>> v2:
>> - Use generic naming (Jani)
>> - Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha)
>> ---
>> drivers/misc/mei/Kconfig | 1 +
>> drivers/misc/mei/Makefile | 1 +
>> drivers/misc/mei/late_bind/Kconfig | 12 +
>> drivers/misc/mei/late_bind/Makefile | 9 +
>> drivers/misc/mei/late_bind/mei_late_bind.c | 261 ++++++++++++++++++++
>> include/drm/intel/i915_component.h | 1 +
>> include/drm/intel/late_bind_mei_interface.h | 37 +++
>> 7 files changed, 322 insertions(+)
>> create mode 100644 drivers/misc/mei/late_bind/Kconfig
>> create mode 100644 drivers/misc/mei/late_bind/Makefile
>> create mode 100644 drivers/misc/mei/late_bind/mei_late_bind.c
>> create mode 100644 include/drm/intel/late_bind_mei_interface.h
>>
>> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
>> 7575fee96cc6..771becc68095 100644
>> --- a/drivers/misc/mei/Kconfig
>> +++ b/drivers/misc/mei/Kconfig
>> @@ -84,5 +84,6 @@ config INTEL_MEI_VSC
>> source "drivers/misc/mei/hdcp/Kconfig"
>> source "drivers/misc/mei/pxp/Kconfig"
>> source "drivers/misc/mei/gsc_proxy/Kconfig"
>> +source "drivers/misc/mei/late_bind/Kconfig"
>>
>> endif
>> diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile index
>> 6f9fdbf1a495..84bfde888d81 100644
>> --- a/drivers/misc/mei/Makefile
>> +++ b/drivers/misc/mei/Makefile
>> @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src)
>> obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/
>> obj-$(CONFIG_INTEL_MEI_PXP) += pxp/
>> obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
>> +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += late_bind/
>>
>> obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o mei-vsc-hw-y := vsc-
>> tp.o diff --git a/drivers/misc/mei/late_bind/Kconfig
>> b/drivers/misc/mei/late_bind/Kconfig
>> new file mode 100644
>> index 000000000000..c5302303e5af
>> --- /dev/null
>> +++ b/drivers/misc/mei/late_bind/Kconfig
>> @@ -0,0 +1,12 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2025, Intel Corporation. All rights reserved.
>> +#
>> +config INTEL_MEI_LATE_BIND
>> + tristate "Intel late binding support on ME Interface"
>> + select INTEL_MEI_ME
>> + depends on DRM_XE
>> + help
>> + MEI Support for Late Binding for Intel graphics card.
>> +
>> + Enables the ME FW interfaces for Late Binding for
>> + Xe display driver of Intel.
> It is needed even for headless cards as well.
Will change the description.
>> diff --git a/drivers/misc/mei/late_bind/Makefile
>> b/drivers/misc/mei/late_bind/Makefile
>> new file mode 100644
>> index 000000000000..a0aeda5853f0
>> --- /dev/null
>> +++ b/drivers/misc/mei/late_bind/Makefile
>> @@ -0,0 +1,9 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Copyright (c) 2025, Intel Corporation. All rights reserved.
>> +#
>> +# Makefile - Late Binding client driver for Intel MEI Bus Driver.
>> +
>> +subdir-ccflags-y += -I$(srctree)/drivers/misc/mei/
>> +
>> +obj-$(CONFIG_INTEL_MEI_LATE_BIND) += mei_late_bind.o
>> diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c
>> b/drivers/misc/mei/late_bind/mei_late_bind.c
>> new file mode 100644
>> index 000000000000..964757a9b33a
>> --- /dev/null
>> +++ b/drivers/misc/mei/late_bind/mei_late_bind.c
>> @@ -0,0 +1,261 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2025 Intel Corporation */ #include
>> +<drm/drm_connector.h> #include <drm/intel/i915_component.h> #include
> Why do we need drm_connector header ?
> Remove unnecessary headers.
Sure
>> +<drm/intel/late_bind_mei_interface.h>
>> +#include <linux/component.h>
>> +#include <linux/pci.h>
>> +#include <linux/mei_cl_bus.h>
>> +#include <linux/module.h>
>> +#include <linux/overflow.h>
>> +#include <linux/slab.h>
>> +#include <linux/uuid.h>
>> +
>> +#include "mkhi.h"
>> +
>> +#define GFX_SRV_MKHI_LATE_BINDING_CMD 0x12 #define
> Why it is not prefixed with DGFX. This is unique to DGFX cards ?
> @alexander any comment on above ?
>> +GFX_SRV_MKHI_LATE_BINDING_RSP (GFX_SRV_MKHI_LATE_BINDING_CMD
>> | 0x80)
>> +
>> +#define LATE_BIND_SEND_TIMEOUT_MSEC 3000 #define
>> +LATE_BIND_RECV_TIMEOUT_MSEC 3000
> From where we got these delay numbers, from any specs or it is arbitrary delay specific to let bind ?
> Add a code comment in that case.
>> +
>> +/**
>> + * struct csc_heci_late_bind_req - late binding request
>> + * @header: @ref mkhi_msg_hdr
>> + * @type: type of the late binding payload
>> + * @flags: flags to be passed to the firmware
>> + * @reserved: reserved field
>> + * @payload_size: size of the payload data in bytes
>> + * @payload: data to be sent to the firmware */ struct
>> +csc_heci_late_bind_req {
>> + struct mkhi_msg_hdr header;
>> + u32 type;
>> + u32 flags;
>> + u32 reserved[2];
>> + u32 payload_size;
>> + u8 payload[] __counted_by(payload_size); } __packed;
>> +
>> +/**
>> + * struct csc_heci_late_bind_rsp - late binding response
>> + * @header: @ref mkhi_msg_hdr
>> + * @type: type of the late binding payload
>> + * @reserved: reserved field
>> + * @status: status of the late binding command execution by firmware
>> +*/ struct csc_heci_late_bind_rsp {
>> + struct mkhi_msg_hdr header;
>> + u32 type;
>> + u32 reserved[2];
>> + u32 status;
>> +} __packed;
>> +
>> +static int mei_late_bind_check_response(const struct device *dev, const
>> +struct mkhi_msg_hdr *hdr) {
>> + if (hdr->group_id != MKHI_GROUP_ID_GFX) {
>> + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n",
>> + hdr->group_id, MKHI_GROUP_ID_GFX);
>> + return -EINVAL;
>> + }
>> +
>> + if (hdr->command != GFX_SRV_MKHI_LATE_BINDING_RSP) {
>> + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n",
>> + hdr->command,
>> GFX_SRV_MKHI_LATE_BINDING_RSP);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * mei_late_bind_push_config - Sends a config to the firmware.
>> + * @dev: device struct corresponding to the mei device
>> + * @type: payload type
>> + * @flags: payload flags
>> + * @payload: payload buffer
>> + * @payload_size: payload buffer size
>> + *
>> + * Return: 0 success, negative errno value on transport failure,
>> + * positive status returned by FW
>> + */
>> +static int mei_late_bind_push_config(struct device *dev, u32 type, u32 flags,
>> + const void *payload, size_t payload_size) {
>> + struct mei_cl_device *cldev;
>> + struct csc_heci_late_bind_req *req = NULL;
>> + struct csc_heci_late_bind_rsp rsp;
>> + size_t req_size;
>> + int ret;
>> +
>> + if (!dev || !payload || !payload_size)
>> + return -EINVAL;
>> +
>> + cldev = to_mei_cl_device(dev);
>> +
>> + ret = mei_cldev_enable(cldev);
>> + if (ret < 0) {
>> + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret);
>> + return ret;
>> + }
>> +
>> + req_size = struct_size(req, payload, payload_size);
>> + if (req_size > mei_cldev_mtu(cldev)) {
>> + dev_err(dev, "Payload is too big %zu\n", payload_size);
>> + ret = -EMSGSIZE;
>> + goto end;
>> + }
>> +
>> + req = kmalloc(req_size, GFP_KERNEL);
>> + if (!req) {
>> + ret = -ENOMEM;
>> + goto end;
>> + }
>> +
>> + req->header.group_id = MKHI_GROUP_ID_GFX;
>> + req->header.command = GFX_SRV_MKHI_LATE_BINDING_CMD;
>> + req->type = type;
>> + req->flags = flags;
>> + req->reserved[0] = 0;
>> + req->reserved[1] = 0;
>> + req->payload_size = payload_size;
>> + memcpy(req->payload, payload, payload_size);
>> +
>> + ret = mei_cldev_send_timeout(cldev, (void *)req, req_size,
>> LATE_BIND_SEND_TIMEOUT_MSEC);
>> + if (ret < 0) {
>> + dev_err(dev, "mei_cldev_send failed. %d\n", ret);
>> + goto end;
>> + }
>> + ret = mei_cldev_recv_timeout(cldev, (void *)&rsp, sizeof(rsp),
>> LATE_BIND_RECV_TIMEOUT_MSEC);
>> + if (ret < 0) {
>> + dev_err(dev, "mei_cldev_recv failed. %d\n", ret);
>> + goto end;
>> + }
>> + ret = mei_late_bind_check_response(dev, &rsp.header);
>> + if (ret) {
>> + dev_err(dev, "bad result response from the firmware:
>> 0x%x\n",
>> + *(uint32_t *)&rsp.header);
>> + goto end;
>> + }
>> + ret = (int)rsp.status;
>> + dev_dbg(dev, "mei_late_bind_push_config status = %d\n", ret);
>> +
>> +end:
>> + mei_cldev_disable(cldev);
>> + kfree(req);
>> + return ret;
>> +}
>> +
>> +static const struct late_bind_component_ops mei_late_bind_ops = {
>> + .owner = THIS_MODULE,
>> + .push_config = mei_late_bind_push_config, };
>> +
>> +static int mei_component_master_bind(struct device *dev) {
>> + return component_bind_all(dev, (void *)&mei_late_bind_ops); }
>> +
>> +static void mei_component_master_unbind(struct device *dev) {
>> + component_unbind_all(dev, (void *)&mei_late_bind_ops); }
>> +
>> +static const struct component_master_ops mei_component_master_ops = {
>> + .bind = mei_component_master_bind,
>> + .unbind = mei_component_master_unbind, };
>> +
>> +/**
>> + * mei_late_bind_component_match - compare function for matching mei
>> late bind.
>> + *
>> + * The function checks if requested is Intel VGA device
>> + * and the parent of requester and the grand parent of mei_if are the same
>> + * device.
> This might be broken for headless card, headless care need to necessarily be VGA card.
Will do %s/PCI_CLASS_DISPLAY_VGA/PCI_CLASS_DISPLAY to cover headless cards.
>> + *
>> + * @dev: master device
>> + * @subcomponent: subcomponent to match
>> (I915_COMPONENT_LATE_BIND)
>> + * @data: compare data (mei late-bind bus device)
>> + *
>> + * Return:
>> + * * 1 - if components match
>> + * * 0 - otherwise
>> + */
>> +static int mei_late_bind_component_match(struct device *dev, int
>> subcomponent,
>> + void *data)
>> +{
>> + struct device *base = data;
>> + struct pci_dev *pdev;
>> +
>> + if (!dev)
>> + return 0;
>> +
>> + if (!dev_is_pci(dev))
>> + return 0;
>> +
>> + pdev = to_pci_dev(dev);
>> +
>> + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) ||
>> + pdev->vendor != PCI_VENDOR_ID_INTEL)
>> + return 0;
>> +
>> + if (subcomponent != I915_COMPONENT_LATE_BIND)
>> + return 0;
> Why I915_COMPONENT_LATE_BIND ?
> Isn't this should be XE_ COMPONENT_LATE_BIND.
Suggestion is not to use XE_ prefix in generic code. I915_ prefix is due
to historical reasons, its being used for other components as well which
are common in i915/xe. |
If everyone agree then I will use INTEL_ prefix here.
>> +
>> + base = base->parent;
>> + if (!base) /* mei device */
>> + return 0;
>> +
>> + base = base->parent; /* pci device */
> Is it sgunit pci endpoint ?
> If yes then good to mention in comment.
>> +
>> + return !!base && dev == base;
>> +}
>> +
>> +static int mei_late_bind_probe(struct mei_cl_device *cldev,
>> + const struct mei_cl_device_id *id) {
>> + struct component_match *master_match = NULL;
>> + int ret;
>> +
>> + component_match_add_typed(&cldev->dev, &master_match,
>> + mei_late_bind_component_match, &cldev-
>>> dev);
>> + if (IS_ERR_OR_NULL(master_match))
>> + return -ENOMEM;
>> +
>> + ret = component_master_add_with_match(&cldev->dev,
>> + &mei_component_master_ops,
>> + master_match);
>> + if (ret < 0)
>> + dev_err(&cldev->dev, "Master comp add failed %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static void mei_late_bind_remove(struct mei_cl_device *cldev) {
>> + component_master_del(&cldev->dev,
>> &mei_component_master_ops); }
>> +
>> +#define MEI_GUID_MKHI UUID_LE(0xe2c2afa2, 0x3817, 0x4d19, \
>> + 0x9d, 0x95, 0x6, 0xb1, 0x6b, 0x58, 0x8a, 0x5d)
>> +
>> +static struct mei_cl_device_id mei_late_bind_tbl[] = {
>> + { .uuid = MEI_GUID_MKHI, .version = MEI_CL_VERSION_ANY },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(mei, mei_late_bind_tbl);
>> +
>> +static struct mei_cl_driver mei_late_bind_driver = {
>> + .id_table = mei_late_bind_tbl,
>> + .name = KBUILD_MODNAME,
>> + .probe = mei_late_bind_probe,
>> + .remove = mei_late_bind_remove,
>> +};
>> +
>> +module_mei_cl_driver(mei_late_bind_driver);
>> +
>> +MODULE_AUTHOR("Intel Corporation");
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("MEI Late Binding");
>> diff --git a/include/drm/intel/i915_component.h
>> b/include/drm/intel/i915_component.h
>> index 4ea3b17aa143..4945044d41e6 100644
>> --- a/include/drm/intel/i915_component.h
>> +++ b/include/drm/intel/i915_component.h
>> @@ -31,6 +31,7 @@ enum i915_component_type {
>> I915_COMPONENT_HDCP,
>> I915_COMPONENT_PXP,
>> I915_COMPONENT_GSC_PROXY,
>> + I915_COMPONENT_LATE_BIND,
> We probably need a change here to make a xe specific component.
Yes as mentioned in response of previous comment this can be
INTEL_COMPONENT_LATE_BIND if everyone agree.
Regards,
Badal
>
> Thanks,
> Anshuman Gupta.
>> };
>>
>> /* MAX_PORT is the number of port
>> diff --git a/include/drm/intel/late_bind_mei_interface.h
>> b/include/drm/intel/late_bind_mei_interface.h
>> new file mode 100644
>> index 000000000000..6b54b8cec5ae
>> --- /dev/null
>> +++ b/include/drm/intel/late_bind_mei_interface.h
>> @@ -0,0 +1,37 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright (c) 2025 Intel Corporation */
>> +
>> +#ifndef _LATE_BIND_MEI_INTERFACE_H_
>> +#define _LATE_BIND_MEI_INTERFACE_H_
>> +
>> +#include <linux/types.h>
>> +
>> +struct device;
>> +struct module;
>> +
>> +/**
>> + * struct late_bind_component_ops - ops for Late Binding services.
>> + * @owner: Module providing the ops
>> + * @push_config: Sends a config to FW.
>> + */
>> +struct late_bind_component_ops {
>> + struct module *owner;
>> +
>> + /**
>> + * @push_config: Sends a config to FW.
>> + * @dev: device struct corresponding to the mei device
>> + * @type: payload type
>> + * @flags: payload flags
>> + * @payload: payload buffer
>> + * @payload_size: payload buffer size
>> + *
>> + * Return: 0 success, negative errno value on transport failure,
>> + * positive status returned by FW
>> + */
>> + int (*push_config)(struct device *dev, u32 type, u32 flags,
>> + const void *payload, size_t payload_size); };
>> +
>> +#endif /* _LATE_BIND_MEI_INTERFACE_H_ */
>> --
>> 2.34.1
[-- Attachment #2: Type: text/html, Size: 17886 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
2025-06-06 17:56 ` [PATCH v2 01/10] mei: bus: add mei_cldev_mtu interface Badal Nilawar
2025-06-06 17:56 ` [PATCH v2 02/10] mei: late_bind: add late binding component driver Badal Nilawar
@ 2025-06-06 17:57 ` Badal Nilawar
2025-06-10 21:47 ` Daniele Ceraolo Spurio
2025-06-14 9:57 ` Gupta, Anshuman
2025-06-06 17:57 ` [PATCH v2 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware Badal Nilawar
` (6 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:57 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
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 late binding component to enable firmware loading
through CSE.
v2:
- Add devm_add_action_or_reset to remove the component (Daniele)
- Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device.c | 3 +
drivers/gpu/drm/xe/xe_device_types.h | 4 +
drivers/gpu/drm/xe/xe_late_bind_fw.c | 96 ++++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw.h | 15 ++++
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 39 +++++++++
6 files changed, 158 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 01d231777901..134eee21c75e 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 d4b6e623aa48..e062ddaa83bb 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -43,6 +43,7 @@
#include "xe_hw_engine_group.h"
#include "xe_hwmon.h"
#include "xe_irq.h"
+#include "xe_late_bind_fw.h"
#include "xe_memirq.h"
#include "xe_mmio.h"
#include "xe_module.h"
@@ -888,6 +889,8 @@ int xe_device_probe(struct xe_device *xe)
if (err)
return err;
+ xe_late_bind_init(&xe->late_bind);
+
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 043515f8c068..3fda450a0774 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"
@@ -549,6 +550,9 @@ struct xe_device {
/** @heci_gsc: graphics security controller */
struct xe_heci_gsc heci_gsc;
+ /** @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..22eb9b51b4ee
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -0,0 +1,96 @@
+// 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/late_bind_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;
+
+ mutex_lock(&late_bind->mutex);
+ late_bind->component.ops = data;
+ late_bind->component.mei_dev = mei_kdev;
+ mutex_unlock(&late_bind->mutex);
+
+ 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;
+
+ mutex_lock(&late_bind->mutex);
+ late_bind->component.ops = NULL;
+ mutex_unlock(&late_bind->mutex);
+}
+
+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);
+
+ if (!late_bind->component_added)
+ return;
+
+ component_del(xe->drm.dev, &xe_late_bind_component_ops);
+ late_bind->component_added = false;
+ mutex_destroy(&late_bind->mutex);
+}
+
+/**
+ * xe_late_bind_init() - add xe mei late binding component
+ *
+ * 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.platform != XE_BATTLEMAGE)
+ return 0;
+
+ mutex_init(&late_bind->mutex);
+
+ 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,
+ I915_COMPONENT_LATE_BIND);
+ if (err < 0) {
+ drm_info(&xe->drm, "Failed to add mei late bind component (%pe)\n", ERR_PTR(err));
+ return err;
+ }
+
+ late_bind->component_added = true;
+
+ 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..afa1917b5f51
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#ifndef _XE_MEI_LATE_BIND_TYPES_H_
+#define _XE_MEI_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 {
+ /** @late_bind_component.mei_dev: mei device */
+ struct device *mei_dev;
+ /** @late_bind_component.ops: late binding ops */
+ const struct late_bind_component_ops *ops;
+};
+
+/**
+ * struct xe_late_bind
+ */
+struct xe_late_bind {
+ /** @late_bind.component: struct for communication with mei component */
+ struct xe_late_bind_component component;
+ /** @late_bind.component_added: whether the component has been added */
+ bool component_added;
+ /** @late_bind.mutex: protects the component binding and usage */
+ struct mutex mutex;
+};
+
+#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
2025-06-06 17:57 ` [PATCH v2 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Badal Nilawar
@ 2025-06-10 21:47 ` Daniele Ceraolo Spurio
2025-06-14 9:57 ` Gupta, Anshuman
1 sibling, 0 replies; 28+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-10 21:47 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/6/2025 10:57 AM, Badal Nilawar wrote:
> 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 late binding component to enable firmware loading
> through CSE.
>
> v2:
> - Add devm_add_action_or_reset to remove the component (Daniele)
> - Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device.c | 3 +
> drivers/gpu/drm/xe/xe_device_types.h | 4 +
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 96 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_late_bind_fw.h | 15 ++++
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 39 +++++++++
> 6 files changed, 158 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 01d231777901..134eee21c75e 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 d4b6e623aa48..e062ddaa83bb 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -43,6 +43,7 @@
> #include "xe_hw_engine_group.h"
> #include "xe_hwmon.h"
> #include "xe_irq.h"
> +#include "xe_late_bind_fw.h"
> #include "xe_memirq.h"
> #include "xe_mmio.h"
> #include "xe_module.h"
> @@ -888,6 +889,8 @@ int xe_device_probe(struct xe_device *xe)
> if (err)
> return err;
>
> + xe_late_bind_init(&xe->late_bind);
I believe that the xe approach is to always fail the probe if something
goes wrong, even if it is not fatal,s oyou should check the error here.
However, make sure that the probe is not aborted in the missing mei
component case (e.g., only fail if "err && err != -ENODEV")
> +
> 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 043515f8c068..3fda450a0774 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"
> @@ -549,6 +550,9 @@ struct xe_device {
> /** @heci_gsc: graphics security controller */
> struct xe_heci_gsc heci_gsc;
>
> + /** @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..22eb9b51b4ee
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -0,0 +1,96 @@
> +// 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/late_bind_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;
> +
> + mutex_lock(&late_bind->mutex);
> + late_bind->component.ops = data;
> + late_bind->component.mei_dev = mei_kdev;
> + mutex_unlock(&late_bind->mutex);
> +
> + 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;
> +
> + mutex_lock(&late_bind->mutex);
> + late_bind->component.ops = NULL;
> + mutex_unlock(&late_bind->mutex);
> +}
> +
> +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)
nit: no need for the xe_* prefix for static functions
> +{
> + struct xe_late_bind *late_bind = arg;
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> +
> + if (!late_bind->component_added)
> + return;
This check against late_bind->component_added doesn't seem necessary,
because late_bind_remove() is only added via devm if the component was
successfully added.
Other components might still have these checks leftover from when we
didn't use devm for them.
> +
> + component_del(xe->drm.dev, &xe_late_bind_component_ops);
> + late_bind->component_added = false;
> + mutex_destroy(&late_bind->mutex);
> +}
> +
> +/**
> + * xe_late_bind_init() - add xe mei late binding component
> + *
> + * 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.platform != XE_BATTLEMAGE)
Would this be better as a "has_late_bind" flag in the info? that way we
won't need to keep a list of platforms here if new ones are added in the
future.
> + return 0;
> +
> + mutex_init(&late_bind->mutex);
> +
> + 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,
> + I915_COMPONENT_LATE_BIND);
> + if (err < 0) {
> + drm_info(&xe->drm, "Failed to add mei late bind component (%pe)\n", ERR_PTR(err));
> + return err;
> + }
> +
> + late_bind->component_added = true;
> +
> + 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..afa1917b5f51
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef _XE_MEI_LATE_BIND_TYPES_H_
> +#define _XE_MEI_LATE_BIND_TYPES_H_
I think the "MEI" should be dropped from this define.
> +
> +#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 {
> + /** @late_bind_component.mei_dev: mei device */
> + struct device *mei_dev;
> + /** @late_bind_component.ops: late binding ops */
> + const struct late_bind_component_ops *ops;
> +};
It's a bit weird to have those 2 variables in their own struct, since
the other 2 variables in struct xe_late_bind are also component-related.
I understand we do have these separate for other components, but that's
because the MEI driver requires it.
Not a blocker.
Daniele
> +
> +/**
> + * struct xe_late_bind
> + */
> +struct xe_late_bind {
> + /** @late_bind.component: struct for communication with mei component */
> + struct xe_late_bind_component component;
> + /** @late_bind.component_added: whether the component has been added */
> + bool component_added;
> + /** @late_bind.mutex: protects the component binding and usage */
> + struct mutex mutex;
> +};
> +
> +#endif
^ permalink raw reply [flat|nested] 28+ messages in thread* RE: [PATCH v2 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
2025-06-06 17:57 ` [PATCH v2 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Badal Nilawar
2025-06-10 21:47 ` Daniele Ceraolo Spurio
@ 2025-06-14 9:57 ` Gupta, Anshuman
1 sibling, 0 replies; 28+ messages in thread
From: Gupta, Anshuman @ 2025-06-14 9:57 UTC (permalink / raw)
To: Nilawar, Badal, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org
Cc: Vivi, Rodrigo, Usyskin, Alexander, gregkh@linuxfoundation.org,
Ceraolo Spurio, Daniele, jgg@nvidia.com
> -----Original Message-----
> From: Nilawar, Badal <badal.nilawar@intel.com>
> Sent: Friday, June 6, 2025 11:27 PM
> To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; Usyskin, Alexander <alexander.usyskin@intel.com>;
> gregkh@linuxfoundation.org; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@intel.com>; jgg@nvidia.com
> Subject: [PATCH v2 03/10] drm/xe/xe_late_bind_fw: Introducing
> xe_late_bind_fw
>
> 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 late binding component to enable firmware loading through
> CSE.
>
> v2:
> - Add devm_add_action_or_reset to remove the component (Daniele)
> - Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device.c | 3 +
> drivers/gpu/drm/xe/xe_device_types.h | 4 +
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 96 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_late_bind_fw.h | 15 ++++
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 39 +++++++++
> 6 files changed, 158 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
> 01d231777901..134eee21c75e 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 d4b6e623aa48..e062ddaa83bb 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -43,6 +43,7 @@
> #include "xe_hw_engine_group.h"
> #include "xe_hwmon.h"
> #include "xe_irq.h"
> +#include "xe_late_bind_fw.h"
> #include "xe_memirq.h"
> #include "xe_mmio.h"
> #include "xe_module.h"
> @@ -888,6 +889,8 @@ int xe_device_probe(struct xe_device *xe)
> if (err)
> return err;
>
> + xe_late_bind_init(&xe->late_bind);
> +
> 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 043515f8c068..3fda450a0774 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"
> @@ -549,6 +550,9 @@ struct xe_device {
> /** @heci_gsc: graphics security controller */
> struct xe_heci_gsc heci_gsc;
>
> + /** @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..22eb9b51b4ee
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -0,0 +1,96 @@
> +// 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/late_bind_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;
> +
> + mutex_lock(&late_bind->mutex);
> + late_bind->component.ops = data;
> + late_bind->component.mei_dev = mei_kdev;
> + mutex_unlock(&late_bind->mutex);
> +
> + 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;
> +
> + mutex_lock(&late_bind->mutex);
> + late_bind->component.ops = NULL;
> + mutex_unlock(&late_bind->mutex);
> +}
> +
> +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);
> +
> + if (!late_bind->component_added)
> + return;
> +
> + component_del(xe->drm.dev, &xe_late_bind_component_ops);
> + late_bind->component_added = false;
> + mutex_destroy(&late_bind->mutex);
> +}
> +
> +/**
> + * xe_late_bind_init() - add xe mei late binding component
> + *
> + * 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.platform != XE_BATTLEMAGE)
> + return 0;
Change the condition which can be scalable for future discrete platforms.
If (DG2 || !DGFX)
return 0
Thanks,
Anshuman
> +
> + mutex_init(&late_bind->mutex);
> +
> + 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,
> + I915_COMPONENT_LATE_BIND);
> + if (err < 0) {
> + drm_info(&xe->drm, "Failed to add mei late bind component
> (%pe)\n", ERR_PTR(err));
> + return err;
> + }
> +
> + late_bind->component_added = true;
> +
> + 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..afa1917b5f51
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef _XE_MEI_LATE_BIND_TYPES_H_
> +#define _XE_MEI_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 {
> + /** @late_bind_component.mei_dev: mei device */
> + struct device *mei_dev;
> + /** @late_bind_component.ops: late binding ops */
> + const struct late_bind_component_ops *ops; };
> +
> +/**
> + * struct xe_late_bind
> + */
> +struct xe_late_bind {
> + /** @late_bind.component: struct for communication with mei
> component */
> + struct xe_late_bind_component component;
> + /** @late_bind.component_added: whether the component has been
> added */
> + bool component_added;
> + /** @late_bind.mutex: protects the component binding and usage */
> + struct mutex mutex;
> +};
> +
> +#endif
> --
> 2.34.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
` (2 preceding siblings ...)
2025-06-06 17:57 ` [PATCH v2 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Badal Nilawar
@ 2025-06-06 17:57 ` Badal Nilawar
2025-06-11 0:06 ` Daniele Ceraolo Spurio
2025-06-06 17:57 ` [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load " Badal Nilawar
` (5 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:57 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Search for late binding firmware binaries and populate the meta data of
firmware structures.
v2:
- drm_err if firmware size is more than max pay load size (Daniele)
- s/request_firmware/firmware_request_nowarn/ as firmware will
not be available for all possible cards (Daniele)
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 2 +
drivers/gpu/drm/xe/xe_late_bind_fw.c | 86 +++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 37 ++++++++++
4 files changed, 124 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index e062ddaa83bb..df4b0e038a6d 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -891,6 +891,8 @@ int xe_device_probe(struct xe_device *xe)
xe_late_bind_init(&xe->late_bind);
+ xe_late_bind_fw_init(&xe->late_bind);
+
err = xe_oa_init(xe);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 22eb9b51b4ee..0231f3dcfc18 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -5,6 +5,7 @@
#include <linux/component.h>
#include <linux/delay.h>
+#include <linux/firmware.h>
#include <drm/drm_managed.h>
#include <drm/intel/i915_component.h>
@@ -13,13 +14,94 @@
#include "xe_device.h"
#include "xe_late_bind_fw.h"
+#include "xe_pcode.h"
+#include "xe_pcode_api.h"
-static struct xe_device *
-late_bind_to_xe(struct xe_late_bind *late_bind)
+static const char * const fw_type_to_name[] = {
+ [CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
+ };
+
+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 late_bind_fw_num_fans(struct xe_late_bind *late_bind)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ struct xe_tile *root_tile = xe_device_get_root_tile(xe);
+ u32 uval;
+
+ if (!xe_pcode_read(root_tile,
+ PCODE_MBOX(FAN_SPEED_CONTROL, FSC_READ_NUM_FANS, 0), &uval, NULL))
+ return uval;
+ else
+ return 0;
+}
+
+static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
+ struct xe_late_bind_fw *lb_fw;
+ const struct firmware *fw;
+ u32 num_fans;
+ int ret;
+
+ if (!late_bind->component_added)
+ return 0;
+
+ lb_fw = &late_bind->late_bind_fw;
+
+ lb_fw->type = type;
+ lb_fw->valid = false;
+
+ if (lb_fw->type == CSC_LATE_BINDING_TYPE_FAN_CONTROL) {
+ num_fans = late_bind_fw_num_fans(late_bind);
+ drm_dbg(&xe->drm, "Number of Fans: %d\n", num_fans);
+ if (!num_fans)
+ return 0;
+ }
+
+ lb_fw->flags = CSC_LATE_BINDING_FLAGS_IS_PERSISTENT;
+
+ snprintf(lb_fw->blob_path, sizeof(lb_fw->blob_path), "xe/%s_8086_%04x_%04x_%04x.bin",
+ fw_type_to_name[type], pdev->device,
+ pdev->subsystem_vendor, pdev->subsystem_device);
+
+ drm_dbg(&xe->drm, "Request late binding firmware %s\n", lb_fw->blob_path);
+ ret = firmware_request_nowarn(&fw, lb_fw->blob_path, xe->drm.dev);
+ if (ret) {
+ drm_dbg(&xe->drm, "Failed to request %s\n", lb_fw->blob_path);
+ return 0;
+ }
+
+ if (fw->size > MAX_PAYLOAD_SIZE) {
+ lb_fw->payload_size = MAX_PAYLOAD_SIZE;
+ drm_err(&xe->drm, "Firmware %s size %zu is larger than max pay load size %u\n",
+ lb_fw->blob_path, fw->size, MAX_PAYLOAD_SIZE);
+ return 0;
+ }
+
+ lb_fw->payload_size = fw->size;
+
+ memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
+ release_firmware(fw);
+ lb_fw->valid = true;
+
+ return 0;
+}
+
+/**
+ * xe_mei_late_bind_fw_init() - Initialize late bind firmware
+ *
+ * Return: 0 if the initialization was successful, a negative errno otherwise.
+ */
+int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
+{
+ return late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
+}
+
static int xe_late_bind_component_bind(struct device *xe_kdev,
struct device *mei_kdev, void *data)
{
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
index 4c73571c3e62..a8b47523b203 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
@@ -11,5 +11,6 @@
struct xe_late_bind;
int xe_late_bind_init(struct xe_late_bind *late_bind);
+int xe_late_bind_fw_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
index afa1917b5f51..c427e141c685 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -10,6 +10,41 @@
#include <linux/mutex.h>
#include <linux/types.h>
+#define MAX_PAYLOAD_SIZE (1024 * 4)
+
+/**
+ * xe_late_bind_fw_type - enum to determine late binding fw type
+ */
+enum xe_late_bind_type {
+ CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1,
+};
+
+/**
+ * Late Binding flags
+ */
+enum csc_late_binding_flags {
+ /** Persistent across warm reset */
+ CSC_LATE_BINDING_FLAGS_IS_PERSISTENT = 0x1
+};
+
+/**
+ * struct xe_late_bind_fw
+ */
+struct xe_late_bind_fw {
+ /** @late_bind_fw.valid */
+ bool valid;
+ /** @late_bind_fw.blob_path: late binding fw blob path */
+ char blob_path[PATH_MAX];
+ /** @late_bind_fw.type */
+ u32 type;
+ /** @late_bind_fw.flags */
+ u32 flags;
+ /** @late_bind_fw.payload: to store the late binding blob */
+ u8 payload[MAX_PAYLOAD_SIZE];
+ /** @late_bind_fw.payload_size: late binding blob payload_size */
+ size_t payload_size;
+};
+
/**
* struct xe_late_bind_component - Late Binding services component
* @mei_dev: device that provide Late Binding service.
@@ -34,6 +69,8 @@ struct xe_late_bind {
bool component_added;
/** @late_bind.mutex: protects the component binding and usage */
struct mutex mutex;
+ /** @late_bind.late_bind_fw: late binding firmware */
+ struct xe_late_bind_fw late_bind_fw;
};
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
2025-06-06 17:57 ` [PATCH v2 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware Badal Nilawar
@ 2025-06-11 0:06 ` Daniele Ceraolo Spurio
2025-06-12 10:35 ` Nilawar, Badal
0 siblings, 1 reply; 28+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-11 0:06 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/6/2025 10:57 AM, Badal Nilawar wrote:
> Search for late binding firmware binaries and populate the meta data of
> firmware structures.
>
> v2:
> - drm_err if firmware size is more than max pay load size (Daniele)
> - s/request_firmware/firmware_request_nowarn/ as firmware will
> not be available for all possible cards (Daniele)
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/xe_device.c | 2 +
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 86 +++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 37 ++++++++++
> 4 files changed, 124 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index e062ddaa83bb..df4b0e038a6d 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -891,6 +891,8 @@ int xe_device_probe(struct xe_device *xe)
>
> xe_late_bind_init(&xe->late_bind);
>
> + xe_late_bind_fw_init(&xe->late_bind);
I still think this should be called from xe_late_bind_init(). You also
need to check the return code.
> +
> err = xe_oa_init(xe);
> if (err)
> return err;
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> index 22eb9b51b4ee..0231f3dcfc18 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -5,6 +5,7 @@
>
> #include <linux/component.h>
> #include <linux/delay.h>
> +#include <linux/firmware.h>
>
> #include <drm/drm_managed.h>
> #include <drm/intel/i915_component.h>
> @@ -13,13 +14,94 @@
>
> #include "xe_device.h"
> #include "xe_late_bind_fw.h"
> +#include "xe_pcode.h"
> +#include "xe_pcode_api.h"
>
> -static struct xe_device *
> -late_bind_to_xe(struct xe_late_bind *late_bind)
nit: might be worth modifying the previous patch to have this introduced
in 1 line instead of modifying it here, to keep the diff cleaner.
> +static const char * const fw_type_to_name[] = {
> + [CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
> + };
> +
> +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 late_bind_fw_num_fans(struct xe_late_bind *late_bind)
> +{
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> + struct xe_tile *root_tile = xe_device_get_root_tile(xe);
> + u32 uval;
> +
> + if (!xe_pcode_read(root_tile,
> + PCODE_MBOX(FAN_SPEED_CONTROL, FSC_READ_NUM_FANS, 0), &uval, NULL))
> + return uval;
> + else
> + return 0;
> +}
> +
> +static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
> +{
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> + struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> + struct xe_late_bind_fw *lb_fw;
> + const struct firmware *fw;
> + u32 num_fans;
> + int ret;
> +
> + if (!late_bind->component_added)
> + return 0;
> +
> + lb_fw = &late_bind->late_bind_fw;
> +
> + lb_fw->type = type;
Should we validate that "type" is sane?
> + lb_fw->valid = false;
> +
> + if (lb_fw->type == CSC_LATE_BINDING_TYPE_FAN_CONTROL) {
> + num_fans = late_bind_fw_num_fans(late_bind);
> + drm_dbg(&xe->drm, "Number of Fans: %d\n", num_fans);
> + if (!num_fans)
> + return 0;
> + }
> +
> + lb_fw->flags = CSC_LATE_BINDING_FLAGS_IS_PERSISTENT;
> +
> + snprintf(lb_fw->blob_path, sizeof(lb_fw->blob_path), "xe/%s_8086_%04x_%04x_%04x.bin",
> + fw_type_to_name[type], pdev->device,
> + pdev->subsystem_vendor, pdev->subsystem_device);
> +
> + drm_dbg(&xe->drm, "Request late binding firmware %s\n", lb_fw->blob_path);
> + ret = firmware_request_nowarn(&fw, lb_fw->blob_path, xe->drm.dev);
> + if (ret) {
> + drm_dbg(&xe->drm, "Failed to request %s\n", lb_fw->blob_path);
This log still seems like an error when it's actually an ok outcome.
Maybe change it to something like:
drm_dbg("%s late binding fw not available for current device",
fw_type_to_name[type])
> + return 0;
> + }
> +
> + if (fw->size > MAX_PAYLOAD_SIZE) {
> + lb_fw->payload_size = MAX_PAYLOAD_SIZE;
Why are we even setting payload_size here?
> + drm_err(&xe->drm, "Firmware %s size %zu is larger than max pay load size %u\n",
> + lb_fw->blob_path, fw->size, MAX_PAYLOAD_SIZE);
> + return 0;
Maybe add a comment to explain why we don't return an error here (if
this was indeed on purpose).
Also, you're not calling release_firmware() when exiting from here.
> + }
> +
> + lb_fw->payload_size = fw->size;
> +
> + memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
> + release_firmware(fw);
> + lb_fw->valid = true;
> +
> + return 0;
This function seems to return 0 from all return calls. Is that
intentional? if so, just switch to void.
> +}
> +
> +/**
> + * xe_mei_late_bind_fw_init() - Initialize late bind firmware
> + *
> + * Return: 0 if the initialization was successful, a negative errno otherwise.
> + */
> +int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
> +{
> + return late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
> +}
> +
> static int xe_late_bind_component_bind(struct device *xe_kdev,
> struct device *mei_kdev, void *data)
> {
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
> index 4c73571c3e62..a8b47523b203 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
> @@ -11,5 +11,6 @@
> struct xe_late_bind;
>
> int xe_late_bind_init(struct xe_late_bind *late_bind);
> +int xe_late_bind_fw_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
> index afa1917b5f51..c427e141c685 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> @@ -10,6 +10,41 @@
> #include <linux/mutex.h>
> #include <linux/types.h>
>
> +#define MAX_PAYLOAD_SIZE (1024 * 4)
> +
> +/**
> + * xe_late_bind_fw_type - enum to determine late binding fw type
> + */
> +enum xe_late_bind_type {
> + CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1,
> +};
> +
> +/**
> + * Late Binding flags
> + */
> +enum csc_late_binding_flags {
> + /** Persistent across warm reset */
> + CSC_LATE_BINDING_FLAGS_IS_PERSISTENT = 0x1
Should this be a define, since it is part of a bitmask of flags?
> +};
AFAIU those 2 enums are not internal to Xe and are defined as part of
CSC interface instead, but that's not clear here. IMO better to either
move them to a file in the abi folder, or at least make it extremely
clear that those values are CSC-defined. Moving then to
late_bind_mei_interface.h could also be an option since they're values
for that interface.
> +
> +/**
> + * struct xe_late_bind_fw
> + */
> +struct xe_late_bind_fw {
> + /** @late_bind_fw.valid */
> + bool valid;
> + /** @late_bind_fw.blob_path: late binding fw blob path */
> + char blob_path[PATH_MAX];
> + /** @late_bind_fw.type */
> + u32 type;
> + /** @late_bind_fw.flags */
Missing descriptions for these 3 vars
> + u32 flags;
> + /** @late_bind_fw.payload: to store the late binding blob */
> + u8 payload[MAX_PAYLOAD_SIZE];
> + /** @late_bind_fw.payload_size: late binding blob payload_size */
if you only set payload_size when the fw init is successful you can use
it being non-zero as a valid check instead of having a dedicated
variable for that. not a blocker.
> + size_t payload_size;
> +};
> +
> /**
> * struct xe_late_bind_component - Late Binding services component
> * @mei_dev: device that provide Late Binding service.
> @@ -34,6 +69,8 @@ struct xe_late_bind {
> bool component_added;
> /** @late_bind.mutex: protects the component binding and usage */
> struct mutex mutex;
> + /** @late_bind.late_bind_fw: late binding firmware */
> + struct xe_late_bind_fw late_bind_fw;
The code seems to be designed from multiple type of late binding FW in
mind (hence the type), but here there is only one. Maybe switch this to
an array, even if it only has one entry for now. That way if we need to
extend it later we just increase the array size.
Daniele
> };
>
> #endif
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
2025-06-11 0:06 ` Daniele Ceraolo Spurio
@ 2025-06-12 10:35 ` Nilawar, Badal
2025-06-12 15:11 ` Nilawar, Badal
0 siblings, 1 reply; 28+ messages in thread
From: Nilawar, Badal @ 2025-06-12 10:35 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 11-06-2025 05:36, Daniele Ceraolo Spurio wrote:
>
>
> On 6/6/2025 10:57 AM, Badal Nilawar wrote:
>> Search for late binding firmware binaries and populate the meta data of
>> firmware structures.
>>
>> v2:
>> - drm_err if firmware size is more than max pay load size (Daniele)
>> - s/request_firmware/firmware_request_nowarn/ as firmware will
>> not be available for all possible cards (Daniele)
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_device.c | 2 +
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 86 +++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 37 ++++++++++
>> 4 files changed, 124 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_device.c
>> b/drivers/gpu/drm/xe/xe_device.c
>> index e062ddaa83bb..df4b0e038a6d 100644
>> --- a/drivers/gpu/drm/xe/xe_device.c
>> +++ b/drivers/gpu/drm/xe/xe_device.c
>> @@ -891,6 +891,8 @@ int xe_device_probe(struct xe_device *xe)
>> xe_late_bind_init(&xe->late_bind);
>> + xe_late_bind_fw_init(&xe->late_bind);
>
> I still think this should be called from xe_late_bind_init(). You also
> need to check the return code.
Sure, I will move this to xe_late_bind_init.
>
>> +
>> err = xe_oa_init(xe);
>> if (err)
>> return err;
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> index 22eb9b51b4ee..0231f3dcfc18 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -5,6 +5,7 @@
>> #include <linux/component.h>
>> #include <linux/delay.h>
>> +#include <linux/firmware.h>
>> #include <drm/drm_managed.h>
>> #include <drm/intel/i915_component.h>
>> @@ -13,13 +14,94 @@
>> #include "xe_device.h"
>> #include "xe_late_bind_fw.h"
>> +#include "xe_pcode.h"
>> +#include "xe_pcode_api.h"
>> -static struct xe_device *
>> -late_bind_to_xe(struct xe_late_bind *late_bind)
>
> nit: might be worth modifying the previous patch to have this
> introduced in 1 line instead of modifying it here, to keep the diff
> cleaner.
Ok,
>
>> +static const char * const fw_type_to_name[] = {
>> + [CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
>> + };
>> +
>> +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 late_bind_fw_num_fans(struct xe_late_bind *late_bind)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + struct xe_tile *root_tile = xe_device_get_root_tile(xe);
>> + u32 uval;
>> +
>> + if (!xe_pcode_read(root_tile,
>> + PCODE_MBOX(FAN_SPEED_CONTROL, FSC_READ_NUM_FANS, 0),
>> &uval, NULL))
>> + return uval;
>> + else
>> + return 0;
>> +}
>> +
>> +static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>> + struct xe_late_bind_fw *lb_fw;
>> + const struct firmware *fw;
>> + u32 num_fans;
>> + int ret;
>> +
>> + if (!late_bind->component_added)
>> + return 0;
>> +
>> + lb_fw = &late_bind->late_bind_fw;
>> +
>> + lb_fw->type = type;
>
> Should we validate that "type" is sane?
You mean if type is not max supported type. Sure, I will add.
>
>> + lb_fw->valid = false;
>> +
>> + if (lb_fw->type == CSC_LATE_BINDING_TYPE_FAN_CONTROL) {
>> + num_fans = late_bind_fw_num_fans(late_bind);
>> + drm_dbg(&xe->drm, "Number of Fans: %d\n", num_fans);
>> + if (!num_fans)
>> + return 0;
>> + }
>> +
>> + lb_fw->flags = CSC_LATE_BINDING_FLAGS_IS_PERSISTENT;
>> +
>> + snprintf(lb_fw->blob_path, sizeof(lb_fw->blob_path),
>> "xe/%s_8086_%04x_%04x_%04x.bin",
>> + fw_type_to_name[type], pdev->device,
>> + pdev->subsystem_vendor, pdev->subsystem_device);
>> +
>> + drm_dbg(&xe->drm, "Request late binding firmware %s\n",
>> lb_fw->blob_path);
>> + ret = firmware_request_nowarn(&fw, lb_fw->blob_path, xe->drm.dev);
>> + if (ret) {
>> + drm_dbg(&xe->drm, "Failed to request %s\n", lb_fw->blob_path);
>
> This log still seems like an error when it's actually an ok outcome.
> Maybe change it to something like:
>
> drm_dbg("%s late binding fw not available for current device",
> fw_type_to_name[type])
Ok,
>
>> + return 0;
>> + }
>> +
>> + if (fw->size > MAX_PAYLOAD_SIZE) {
>> + lb_fw->payload_size = MAX_PAYLOAD_SIZE;
>
> Why are we even setting payload_size here?
>
>> + drm_err(&xe->drm, "Firmware %s size %zu is larger than max
>> pay load size %u\n",
>> + lb_fw->blob_path, fw->size, MAX_PAYLOAD_SIZE);
>> + return 0;
>
> Maybe add a comment to explain why we don't return an error here (if
> this was indeed on purpose).
> Also, you're not calling release_firmware() when exiting from here.
I will fix this. In fact I will return error here, as firmware is found
but is of incorrect size.
>
>> + }
>> +
>> + lb_fw->payload_size = fw->size;
>> +
>> + memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
>> + release_firmware(fw);
>> + lb_fw->valid = true;
>> +
>> + return 0;
>
> This function seems to return 0 from all return calls. Is that
> intentional? if so, just switch to void.
>> +}
>> +
>> +/**
>> + * xe_mei_late_bind_fw_init() - Initialize late bind firmware
>> + *
>> + * Return: 0 if the initialization was successful, a negative errno
>> otherwise.
>> + */
>> +int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
>> +{
>> + return late_bind_fw_init(late_bind,
>> CSC_LATE_BINDING_TYPE_FAN_CONTROL);
>> +}
>> +
>> static int xe_late_bind_component_bind(struct device *xe_kdev,
>> struct device *mei_kdev, void *data)
>> {
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> index 4c73571c3e62..a8b47523b203 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> @@ -11,5 +11,6 @@
>> struct xe_late_bind;
>> int xe_late_bind_init(struct xe_late_bind *late_bind);
>> +int xe_late_bind_fw_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
>> index afa1917b5f51..c427e141c685 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> @@ -10,6 +10,41 @@
>> #include <linux/mutex.h>
>> #include <linux/types.h>
>> +#define MAX_PAYLOAD_SIZE (1024 * 4)
>> +
>> +/**
>> + * xe_late_bind_fw_type - enum to determine late binding fw type
>> + */
>> +enum xe_late_bind_type {
>> + CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1,
>> +};
>> +
>> +/**
>> + * Late Binding flags
>> + */
>> +enum csc_late_binding_flags {
>> + /** Persistent across warm reset */
>> + CSC_LATE_BINDING_FLAGS_IS_PERSISTENT = 0x1
>
> Should this be a define, since it is part of a bitmask of flags?
>> +};
>
> AFAIU those 2 enums are not internal to Xe and are defined as part of
> CSC interface instead, but that's not clear here. IMO better to either
> move them to a file in the abi folder, or at least make it extremely
> clear that those values are CSC-defined. Moving then to
> late_bind_mei_interface.h could also be an option since they're values
> for that interface.
Fine I will move this to late_bind_mei_interface.h
>
>> +
>> +/**
>> + * struct xe_late_bind_fw
>> + */
>> +struct xe_late_bind_fw {
>> + /** @late_bind_fw.valid */
>> + bool valid;
>> + /** @late_bind_fw.blob_path: late binding fw blob path */
>> + char blob_path[PATH_MAX];
>> + /** @late_bind_fw.type */
>> + u32 type;
>> + /** @late_bind_fw.flags */
>
> Missing descriptions for these 3 vars
I will fix this.
>
>> + u32 flags;
>> + /** @late_bind_fw.payload: to store the late binding blob */
>> + u8 payload[MAX_PAYLOAD_SIZE];
>> + /** @late_bind_fw.payload_size: late binding blob payload_size */
>
> if you only set payload_size when the fw init is successful you can
> use it being non-zero as a valid check instead of having a dedicated
> variable for that. not a blocker.
>
>> + size_t payload_size;
>> +};
>> +
>> /**
>> * struct xe_late_bind_component - Late Binding services component
>> * @mei_dev: device that provide Late Binding service.
>> @@ -34,6 +69,8 @@ struct xe_late_bind {
>> bool component_added;
>> /** @late_bind.mutex: protects the component binding and usage */
>> struct mutex mutex;
>> + /** @late_bind.late_bind_fw: late binding firmware */
>> + struct xe_late_bind_fw late_bind_fw;
>
> The code seems to be designed from multiple type of late binding FW in
> mind (hence the type), but here there is only one. Maybe switch this
> to an array, even if it only has one entry for now. That way if we
> need to extend it later we just increase the array size.
Yes, it is designed to support multiple types. In fact in previous
implementation this was array only. If ok I will move back to array
based implementation and just Populate FAN type.
Regards,
Badal
>
> Daniele
>
>> };
>> #endif
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
2025-06-12 10:35 ` Nilawar, Badal
@ 2025-06-12 15:11 ` Nilawar, Badal
0 siblings, 0 replies; 28+ messages in thread
From: Nilawar, Badal @ 2025-06-12 15:11 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
[-- Attachment #1: Type: text/plain, Size: 745 bytes --]
On 12-06-2025 16:05, Nilawar, Badal wrote:
>>> +static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
>>> +{
>>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>>> + struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>> + struct xe_late_bind_fw *lb_fw;
>>> + const struct firmware *fw;
>>> + u32 num_fans;
>>> + int ret;
>>> +
>>> + if (!late_bind->component_added)
>>> + return 0;
>>> +
>>> + lb_fw = &late_bind->late_bind_fw;
>>> +
>>> + lb_fw->type = type;
>>
>> Should we validate that "type" is sane?
> You mean if type is not max supported type. Sure, I will add.
IMO we don't need to validate type, CSC will return -4 i.e. invalid
parameter error.
[-- Attachment #2: Type: text/html, Size: 1728 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
` (3 preceding siblings ...)
2025-06-06 17:57 ` [PATCH v2 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware Badal Nilawar
@ 2025-06-06 17:57 ` Badal Nilawar
2025-06-11 0:17 ` Daniele Ceraolo Spurio
2025-06-15 18:26 ` Umesh Nerlige Ramappa
2025-06-06 17:57 ` [PATCH v2 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume Badal Nilawar
` (4 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:57 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Load late binding firmware
v2:
- s/EAGAIN/EBUSY/
- Flush worker in suspend and driver unload (Daniele)
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 121 ++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
3 files changed, 126 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 0231f3dcfc18..7fe304c54374 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -16,6 +16,16 @@
#include "xe_late_bind_fw.h"
#include "xe_pcode.h"
#include "xe_pcode_api.h"
+#include "xe_pm.h"
+
+/*
+ * The component should load quite quickly in most cases, but it could take
+ * a bit. Using a very big timeout just to cover the worst case scenario
+ */
+#define LB_INIT_TIMEOUT_MS 20000
+
+#define LB_FW_LOAD_RETRY_MAXCOUNT 40
+#define LB_FW_LOAD_RETRY_PAUSE_MS 50
static const char * const fw_type_to_name[] = {
[CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
@@ -39,6 +49,95 @@ static int late_bind_fw_num_fans(struct xe_late_bind *late_bind)
return 0;
}
+static void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ struct xe_late_bind_fw *lbfw;
+
+ lbfw = &late_bind->late_bind_fw;
+ if (lbfw->valid && late_bind->wq) {
+ drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
+ fw_type_to_name[lbfw->type]);
+ flush_work(&lbfw->work);
+ }
+}
+
+static void late_bind_work(struct work_struct *work)
+{
+ struct xe_late_bind_fw *lbfw = container_of(work, struct xe_late_bind_fw, work);
+ struct xe_late_bind *late_bind = container_of(lbfw, struct xe_late_bind,
+ late_bind_fw);
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
+ int ret;
+ int slept;
+
+ if (!late_bind->component_added)
+ return;
+
+ if (!lbfw->valid)
+ return;
+
+ /* we can queue this before the component is bound */
+ for (slept = 0; slept < LB_INIT_TIMEOUT_MS; slept += 100) {
+ if (late_bind->component.ops)
+ break;
+ msleep(100);
+ }
+
+ xe_pm_runtime_get(xe);
+ mutex_lock(&late_bind->mutex);
+
+ if (!late_bind->component.ops) {
+ drm_err(&xe->drm, "Late bind component not bound\n");
+ goto out;
+ }
+
+ drm_dbg(&xe->drm, "Load %s firmware\n", fw_type_to_name[lbfw->type]);
+
+ do {
+ ret = late_bind->component.ops->push_config(late_bind->component.mei_dev,
+ lbfw->type, lbfw->flags,
+ lbfw->payload, lbfw->payload_size);
+ if (!ret)
+ break;
+ msleep(LB_FW_LOAD_RETRY_PAUSE_MS);
+ } while (--retry && ret == -EBUSY);
+
+ if (ret)
+ drm_err(&xe->drm, "Load %s firmware failed with err %d\n",
+ fw_type_to_name[lbfw->type], ret);
+ else
+ drm_dbg(&xe->drm, "Load %s firmware successful\n",
+ fw_type_to_name[lbfw->type]);
+out:
+ mutex_unlock(&late_bind->mutex);
+ xe_pm_runtime_put(xe);
+}
+
+int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ struct xe_late_bind_fw *lbfw;
+
+ if (!late_bind->component_added)
+ return -EINVAL;
+
+ lbfw = &late_bind->late_bind_fw;
+ if (lbfw->valid) {
+ drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
+ fw_type_to_name[lbfw->type]);
+ queue_work(late_bind->wq, &lbfw->work);
+ }
+
+ return 0;
+}
+
+/**
+ * late_bind_fw_init() - initialize late bind firmware
+ *
+ * Return: 0 if the initialization was successful, a negative errno otherwise.
+ */
static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
{
struct xe_device *xe = late_bind_to_xe(late_bind);
@@ -87,6 +186,7 @@ static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
release_firmware(fw);
+ INIT_WORK(&lb_fw->work, late_bind_work);
lb_fw->valid = true;
return 0;
@@ -99,7 +199,17 @@ static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
*/
int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
{
- return late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
+ int ret;
+
+ late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
+ if (!late_bind->wq)
+ return -ENOMEM;
+
+ ret = late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
+ if (ret)
+ return ret;
+
+ return xe_late_bind_fw_load(late_bind);
}
static int xe_late_bind_component_bind(struct device *xe_kdev,
@@ -122,6 +232,8 @@ static void xe_late_bind_component_unbind(struct device *xe_kdev,
struct xe_device *xe = kdev_to_xe_device(xe_kdev);
struct xe_late_bind *late_bind = &xe->late_bind;
+ xe_late_bind_wait_for_worker_completion(late_bind);
+
mutex_lock(&late_bind->mutex);
late_bind->component.ops = NULL;
mutex_unlock(&late_bind->mutex);
@@ -140,9 +252,16 @@ static void xe_late_bind_remove(void *arg)
if (!late_bind->component_added)
return;
+ xe_late_bind_wait_for_worker_completion(late_bind);
+
component_del(xe->drm.dev, &xe_late_bind_component_ops);
late_bind->component_added = false;
+ if (late_bind->wq) {
+ destroy_workqueue(late_bind->wq);
+ late_bind->wq = NULL;
+ }
mutex_destroy(&late_bind->mutex);
+
}
/**
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
index a8b47523b203..166957e84c2f 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
@@ -12,5 +12,6 @@ struct xe_late_bind;
int xe_late_bind_init(struct xe_late_bind *late_bind);
int xe_late_bind_fw_init(struct xe_late_bind *late_bind);
+int xe_late_bind_fw_load(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
index c427e141c685..690680e8ff22 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -9,6 +9,7 @@
#include <linux/iosys-map.h>
#include <linux/mutex.h>
#include <linux/types.h>
+#include <linux/workqueue.h>
#define MAX_PAYLOAD_SIZE (1024 * 4)
@@ -43,6 +44,8 @@ struct xe_late_bind_fw {
u8 payload[MAX_PAYLOAD_SIZE];
/** @late_bind_fw.payload_size: late binding blob payload_size */
size_t payload_size;
+ /** @late_bind_fw.work: worker to upload latebind blob */
+ struct work_struct work;
};
/**
@@ -71,6 +74,8 @@ struct xe_late_bind {
struct mutex mutex;
/** @late_bind.late_bind_fw: late binding firmware */
struct xe_late_bind_fw late_bind_fw;
+ /** @late_bind.wq: workqueue to submit request to download late bind blob */
+ struct workqueue_struct *wq;
};
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-06 17:57 ` [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load " Badal Nilawar
@ 2025-06-11 0:17 ` Daniele Ceraolo Spurio
2025-06-12 11:54 ` Usyskin, Alexander
2025-06-12 13:31 ` Nilawar, Badal
2025-06-15 18:26 ` Umesh Nerlige Ramappa
1 sibling, 2 replies; 28+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-11 0:17 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/6/2025 10:57 AM, Badal Nilawar wrote:
> Load late binding firmware
>
> v2:
> - s/EAGAIN/EBUSY/
> - Flush worker in suspend and driver unload (Daniele)
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 121 ++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
> 3 files changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> index 0231f3dcfc18..7fe304c54374 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -16,6 +16,16 @@
> #include "xe_late_bind_fw.h"
> #include "xe_pcode.h"
> #include "xe_pcode_api.h"
> +#include "xe_pm.h"
> +
> +/*
> + * The component should load quite quickly in most cases, but it could take
> + * a bit. Using a very big timeout just to cover the worst case scenario
> + */
> +#define LB_INIT_TIMEOUT_MS 20000
> +
> +#define LB_FW_LOAD_RETRY_MAXCOUNT 40
> +#define LB_FW_LOAD_RETRY_PAUSE_MS 50
Are those retry values spec'd anywhere? For GSC we use those because the
GSC specs say to retry in 50ms intervals for up to 2 secs to give time
for the GSC to do proxy handling. Does it make sense to do the same in
this case, given that there is no proxy involved?
>
> static const char * const fw_type_to_name[] = {
> [CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
> @@ -39,6 +49,95 @@ static int late_bind_fw_num_fans(struct xe_late_bind *late_bind)
> return 0;
> }
>
> +static void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
> +{
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> + struct xe_late_bind_fw *lbfw;
> +
> + lbfw = &late_bind->late_bind_fw;
> + if (lbfw->valid && late_bind->wq) {
> + drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
> + fw_type_to_name[lbfw->type]);
> + flush_work(&lbfw->work);
> + }
> +}
> +
> +static void late_bind_work(struct work_struct *work)
> +{
> + struct xe_late_bind_fw *lbfw = container_of(work, struct xe_late_bind_fw, work);
> + struct xe_late_bind *late_bind = container_of(lbfw, struct xe_late_bind,
> + late_bind_fw);
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> + int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
> + int ret;
> + int slept;
> +
> + if (!late_bind->component_added)
> + return;
> +
> + if (!lbfw->valid)
> + return;
The first check is redundant because lbfw->valid can't be true if
late_bind->component_added is false with the current code.
> +
> + /* we can queue this before the component is bound */
> + for (slept = 0; slept < LB_INIT_TIMEOUT_MS; slept += 100) {
> + if (late_bind->component.ops)
> + break;
> + msleep(100);
> + }
> +
> + xe_pm_runtime_get(xe);
> + mutex_lock(&late_bind->mutex);
> +
> + if (!late_bind->component.ops) {
> + drm_err(&xe->drm, "Late bind component not bound\n");
> + goto out;
> + }
> +
> + drm_dbg(&xe->drm, "Load %s firmware\n", fw_type_to_name[lbfw->type]);
> +
> + do {
> + ret = late_bind->component.ops->push_config(late_bind->component.mei_dev,
> + lbfw->type, lbfw->flags,
> + lbfw->payload, lbfw->payload_size);
> + if (!ret)
> + break;
> + msleep(LB_FW_LOAD_RETRY_PAUSE_MS);
> + } while (--retry && ret == -EBUSY);
> +
> + if (ret)
> + drm_err(&xe->drm, "Load %s firmware failed with err %d\n",
> + fw_type_to_name[lbfw->type], ret);
> + else
> + drm_dbg(&xe->drm, "Load %s firmware successful\n",
> + fw_type_to_name[lbfw->type]);
> +out:
> + mutex_unlock(&late_bind->mutex);
> + xe_pm_runtime_put(xe);
> +}
> +
> +int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
> +{
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> + struct xe_late_bind_fw *lbfw;
> +
> + if (!late_bind->component_added)
> + return -EINVAL;
> +
> + lbfw = &late_bind->late_bind_fw;
> + if (lbfw->valid) {
> + drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
> + fw_type_to_name[lbfw->type]);
This log seems a bit too specific, also given that you also have logs
inside the work
Daniele
> + queue_work(late_bind->wq, &lbfw->work);
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * late_bind_fw_init() - initialize late bind firmware
> + *
> + * Return: 0 if the initialization was successful, a negative errno otherwise.
> + */
> static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
> {
> struct xe_device *xe = late_bind_to_xe(late_bind);
> @@ -87,6 +186,7 @@ static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
>
> memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
> release_firmware(fw);
> + INIT_WORK(&lb_fw->work, late_bind_work);
> lb_fw->valid = true;
>
> return 0;
> @@ -99,7 +199,17 @@ static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
> */
> int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
> {
> - return late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
> + int ret;
> +
> + late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
> + if (!late_bind->wq)
> + return -ENOMEM;
> +
> + ret = late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
> + if (ret)
> + return ret;
> +
> + return xe_late_bind_fw_load(late_bind);
> }
>
> static int xe_late_bind_component_bind(struct device *xe_kdev,
> @@ -122,6 +232,8 @@ static void xe_late_bind_component_unbind(struct device *xe_kdev,
> struct xe_device *xe = kdev_to_xe_device(xe_kdev);
> struct xe_late_bind *late_bind = &xe->late_bind;
>
> + xe_late_bind_wait_for_worker_completion(late_bind);
> +
> mutex_lock(&late_bind->mutex);
> late_bind->component.ops = NULL;
> mutex_unlock(&late_bind->mutex);
> @@ -140,9 +252,16 @@ static void xe_late_bind_remove(void *arg)
> if (!late_bind->component_added)
> return;
>
> + xe_late_bind_wait_for_worker_completion(late_bind);
> +
> component_del(xe->drm.dev, &xe_late_bind_component_ops);
> late_bind->component_added = false;
> + if (late_bind->wq) {
> + destroy_workqueue(late_bind->wq);
> + late_bind->wq = NULL;
> + }
> mutex_destroy(&late_bind->mutex);
> +
> }
>
> /**
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
> index a8b47523b203..166957e84c2f 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
> @@ -12,5 +12,6 @@ struct xe_late_bind;
>
> int xe_late_bind_init(struct xe_late_bind *late_bind);
> int xe_late_bind_fw_init(struct xe_late_bind *late_bind);
> +int xe_late_bind_fw_load(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
> index c427e141c685..690680e8ff22 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> @@ -9,6 +9,7 @@
> #include <linux/iosys-map.h>
> #include <linux/mutex.h>
> #include <linux/types.h>
> +#include <linux/workqueue.h>
>
> #define MAX_PAYLOAD_SIZE (1024 * 4)
>
> @@ -43,6 +44,8 @@ struct xe_late_bind_fw {
> u8 payload[MAX_PAYLOAD_SIZE];
> /** @late_bind_fw.payload_size: late binding blob payload_size */
> size_t payload_size;
> + /** @late_bind_fw.work: worker to upload latebind blob */
> + struct work_struct work;
> };
>
> /**
> @@ -71,6 +74,8 @@ struct xe_late_bind {
> struct mutex mutex;
> /** @late_bind.late_bind_fw: late binding firmware */
> struct xe_late_bind_fw late_bind_fw;
> + /** @late_bind.wq: workqueue to submit request to download late bind blob */
> + struct workqueue_struct *wq;
> };
>
> #endif
^ permalink raw reply [flat|nested] 28+ messages in thread* RE: [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-11 0:17 ` Daniele Ceraolo Spurio
@ 2025-06-12 11:54 ` Usyskin, Alexander
2025-06-12 13:26 ` Nilawar, Badal
2025-06-12 13:31 ` Nilawar, Badal
1 sibling, 1 reply; 28+ messages in thread
From: Usyskin, Alexander @ 2025-06-12 11:54 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, Nilawar, Badal,
intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: Gupta, Anshuman, Vivi, Rodrigo, gregkh@linuxfoundation.org,
jgg@nvidia.com
> Subject: Re: [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding
> firmware
>
>
>
> On 6/6/2025 10:57 AM, Badal Nilawar wrote:
> > Load late binding firmware
> >
> > v2:
> > - s/EAGAIN/EBUSY/
> > - Flush worker in suspend and driver unload (Daniele)
> >
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_late_bind_fw.c | 121
> ++++++++++++++++++++-
> > drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
> > drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
> > 3 files changed, 126 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > index 0231f3dcfc18..7fe304c54374 100644
> > --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > @@ -16,6 +16,16 @@
> > #include "xe_late_bind_fw.h"
> > #include "xe_pcode.h"
> > #include "xe_pcode_api.h"
> > +#include "xe_pm.h"
> > +
> > +/*
> > + * The component should load quite quickly in most cases, but it could take
> > + * a bit. Using a very big timeout just to cover the worst case scenario
> > + */
> > +#define LB_INIT_TIMEOUT_MS 20000
> > +
> > +#define LB_FW_LOAD_RETRY_MAXCOUNT 40
> > +#define LB_FW_LOAD_RETRY_PAUSE_MS 50
>
> Are those retry values spec'd anywhere? For GSC we use those because the
> GSC specs say to retry in 50ms intervals for up to 2 secs to give time
> for the GSC to do proxy handling. Does it make sense to do the same in
> this case, given that there is no proxy involved?
>
Here 50ms is too small, we are waiting for other OS components to release handle.
We usually have 3 times 2 sec in user-space, but it is too big for kernel,
let's do 200ms step up to 6 sec.
- -
Thanks,
Sasha
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-12 11:54 ` Usyskin, Alexander
@ 2025-06-12 13:26 ` Nilawar, Badal
0 siblings, 0 replies; 28+ messages in thread
From: Nilawar, Badal @ 2025-06-12 13:26 UTC (permalink / raw)
To: Usyskin, Alexander, Ceraolo Spurio, Daniele,
intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: Gupta, Anshuman, Vivi, Rodrigo, gregkh@linuxfoundation.org,
jgg@nvidia.com
On 12-06-2025 17:24, Usyskin, Alexander wrote:
>> Subject: Re: [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding
>> firmware
>>
>>
>>
>> On 6/6/2025 10:57 AM, Badal Nilawar wrote:
>>> Load late binding firmware
>>>
>>> v2:
>>> - s/EAGAIN/EBUSY/
>>> - Flush worker in suspend and driver unload (Daniele)
>>>
>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>> ---
>>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 121
>> ++++++++++++++++++++-
>>> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
>>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
>>> 3 files changed, 126 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>> index 0231f3dcfc18..7fe304c54374 100644
>>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>> @@ -16,6 +16,16 @@
>>> #include "xe_late_bind_fw.h"
>>> #include "xe_pcode.h"
>>> #include "xe_pcode_api.h"
>>> +#include "xe_pm.h"
>>> +
>>> +/*
>>> + * The component should load quite quickly in most cases, but it could take
>>> + * a bit. Using a very big timeout just to cover the worst case scenario
>>> + */
>>> +#define LB_INIT_TIMEOUT_MS 20000
>>> +
>>> +#define LB_FW_LOAD_RETRY_MAXCOUNT 40
>>> +#define LB_FW_LOAD_RETRY_PAUSE_MS 50
>> Are those retry values spec'd anywhere? For GSC we use those because the
>> GSC specs say to retry in 50ms intervals for up to 2 secs to give time
>> for the GSC to do proxy handling. Does it make sense to do the same in
>> this case, given that there is no proxy involved?
>>
> Here 50ms is too small, we are waiting for other OS components to release handle.
> We usually have 3 times 2 sec in user-space, but it is too big for kernel,
> let's do 200ms step up to 6 sec.
Sure I will change the intervals.
Regards,
Badal
>
> - -
> Thanks,
> Sasha
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-11 0:17 ` Daniele Ceraolo Spurio
2025-06-12 11:54 ` Usyskin, Alexander
@ 2025-06-12 13:31 ` Nilawar, Badal
1 sibling, 0 replies; 28+ messages in thread
From: Nilawar, Badal @ 2025-06-12 13:31 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 11-06-2025 05:47, Daniele Ceraolo Spurio wrote:
>
>
> On 6/6/2025 10:57 AM, Badal Nilawar wrote:
>> Load late binding firmware
>>
>> v2:
>> - s/EAGAIN/EBUSY/
>> - Flush worker in suspend and driver unload (Daniele)
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 121 ++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
>> 3 files changed, 126 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> index 0231f3dcfc18..7fe304c54374 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -16,6 +16,16 @@
>> #include "xe_late_bind_fw.h"
>> #include "xe_pcode.h"
>> #include "xe_pcode_api.h"
>> +#include "xe_pm.h"
>> +
>> +/*
>> + * The component should load quite quickly in most cases, but it
>> could take
>> + * a bit. Using a very big timeout just to cover the worst case
>> scenario
>> + */
>> +#define LB_INIT_TIMEOUT_MS 20000
>> +
>> +#define LB_FW_LOAD_RETRY_MAXCOUNT 40
>> +#define LB_FW_LOAD_RETRY_PAUSE_MS 50
>
> Are those retry values spec'd anywhere? For GSC we use those because
> the GSC specs say to retry in 50ms intervals for up to 2 secs to give
> time for the GSC to do proxy handling. Does it make sense to do the
> same in this case, given that there is no proxy involved?
>
>> static const char * const fw_type_to_name[] = {
>> [CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
>> @@ -39,6 +49,95 @@ static int late_bind_fw_num_fans(struct
>> xe_late_bind *late_bind)
>> return 0;
>> }
>> +static void xe_late_bind_wait_for_worker_completion(struct
>> xe_late_bind *late_bind)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + struct xe_late_bind_fw *lbfw;
>> +
>> + lbfw = &late_bind->late_bind_fw;
>> + if (lbfw->valid && late_bind->wq) {
>> + drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
>> + fw_type_to_name[lbfw->type]);
>> + flush_work(&lbfw->work);
>> + }
>> +}
>> +
>> +static void late_bind_work(struct work_struct *work)
>> +{
>> + struct xe_late_bind_fw *lbfw = container_of(work, struct
>> xe_late_bind_fw, work);
>> + struct xe_late_bind *late_bind = container_of(lbfw, struct
>> xe_late_bind,
>> + late_bind_fw);
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
>> + int ret;
>> + int slept;
>> +
>> + if (!late_bind->component_added)
>> + return;
>> +
>> + if (!lbfw->valid)
>> + return;
>
> The first check is redundant because lbfw->valid can't be true if
> late_bind->component_added is false with the current code.
I will remove this change, while scheduling work already lbfw->valid is
being checked. Shall I even remove check for late_bind->component_added)
as this is also being checked while before scheduling work.
>
>> +
>> + /* we can queue this before the component is bound */
>> + for (slept = 0; slept < LB_INIT_TIMEOUT_MS; slept += 100) {
>> + if (late_bind->component.ops)
>> + break;
>> + msleep(100);
>> + }
>> +
>> + xe_pm_runtime_get(xe);
>> + mutex_lock(&late_bind->mutex);
>> +
>> + if (!late_bind->component.ops) {
>> + drm_err(&xe->drm, "Late bind component not bound\n");
>> + goto out;
>> + }
>> +
>> + drm_dbg(&xe->drm, "Load %s firmware\n",
>> fw_type_to_name[lbfw->type]);
>> +
>> + do {
>> + ret =
>> late_bind->component.ops->push_config(late_bind->component.mei_dev,
>> + lbfw->type, lbfw->flags,
>> + lbfw->payload, lbfw->payload_size);
>> + if (!ret)
>> + break;
>> + msleep(LB_FW_LOAD_RETRY_PAUSE_MS);
>> + } while (--retry && ret == -EBUSY);
>> +
>> + if (ret)
>> + drm_err(&xe->drm, "Load %s firmware failed with err %d\n",
>> + fw_type_to_name[lbfw->type], ret);
>> + else
>> + drm_dbg(&xe->drm, "Load %s firmware successful\n",
>> + fw_type_to_name[lbfw->type]);
>> +out:
>> + mutex_unlock(&late_bind->mutex);
>> + xe_pm_runtime_put(xe);
>> +}
>> +
>> +int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + struct xe_late_bind_fw *lbfw;
>> +
>> + if (!late_bind->component_added)
>> + return -EINVAL;
>> +
>> + lbfw = &late_bind->late_bind_fw;
>> + if (lbfw->valid) {
>> + drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
>> + fw_type_to_name[lbfw->type]);
>
> This log seems a bit too specific, also given that you also have logs
> inside the work
Will remove this log.
Thanks,
Badal
>
> Daniele
>
>> + queue_work(late_bind->wq, &lbfw->work);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * late_bind_fw_init() - initialize late bind firmware
>> + *
>> + * Return: 0 if the initialization was successful, a negative errno
>> otherwise.
>> + */
>> static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
>> {
>> struct xe_device *xe = late_bind_to_xe(late_bind);
>> @@ -87,6 +186,7 @@ static int late_bind_fw_init(struct xe_late_bind
>> *late_bind, u32 type)
>> memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
>> release_firmware(fw);
>> + INIT_WORK(&lb_fw->work, late_bind_work);
>> lb_fw->valid = true;
>> return 0;
>> @@ -99,7 +199,17 @@ static int late_bind_fw_init(struct xe_late_bind
>> *late_bind, u32 type)
>> */
>> int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
>> {
>> - return late_bind_fw_init(late_bind,
>> CSC_LATE_BINDING_TYPE_FAN_CONTROL);
>> + int ret;
>> +
>> + late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
>> + if (!late_bind->wq)
>> + return -ENOMEM;
>> +
>> + ret = late_bind_fw_init(late_bind,
>> CSC_LATE_BINDING_TYPE_FAN_CONTROL);
>> + if (ret)
>> + return ret;
>> +
>> + return xe_late_bind_fw_load(late_bind);
>> }
>> static int xe_late_bind_component_bind(struct device *xe_kdev,
>> @@ -122,6 +232,8 @@ static void xe_late_bind_component_unbind(struct
>> device *xe_kdev,
>> struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>> struct xe_late_bind *late_bind = &xe->late_bind;
>> + xe_late_bind_wait_for_worker_completion(late_bind);
>> +
>> mutex_lock(&late_bind->mutex);
>> late_bind->component.ops = NULL;
>> mutex_unlock(&late_bind->mutex);
>> @@ -140,9 +252,16 @@ static void xe_late_bind_remove(void *arg)
>> if (!late_bind->component_added)
>> return;
>> + xe_late_bind_wait_for_worker_completion(late_bind);
>> +
>> component_del(xe->drm.dev, &xe_late_bind_component_ops);
>> late_bind->component_added = false;
>> + if (late_bind->wq) {
>> + destroy_workqueue(late_bind->wq);
>> + late_bind->wq = NULL;
>> + }
>> mutex_destroy(&late_bind->mutex);
>> +
>> }
>> /**
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> index a8b47523b203..166957e84c2f 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> @@ -12,5 +12,6 @@ struct xe_late_bind;
>> int xe_late_bind_init(struct xe_late_bind *late_bind);
>> int xe_late_bind_fw_init(struct xe_late_bind *late_bind);
>> +int xe_late_bind_fw_load(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
>> index c427e141c685..690680e8ff22 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> @@ -9,6 +9,7 @@
>> #include <linux/iosys-map.h>
>> #include <linux/mutex.h>
>> #include <linux/types.h>
>> +#include <linux/workqueue.h>
>> #define MAX_PAYLOAD_SIZE (1024 * 4)
>> @@ -43,6 +44,8 @@ struct xe_late_bind_fw {
>> u8 payload[MAX_PAYLOAD_SIZE];
>> /** @late_bind_fw.payload_size: late binding blob payload_size */
>> size_t payload_size;
>> + /** @late_bind_fw.work: worker to upload latebind blob */
>> + struct work_struct work;
>> };
>> /**
>> @@ -71,6 +74,8 @@ struct xe_late_bind {
>> struct mutex mutex;
>> /** @late_bind.late_bind_fw: late binding firmware */
>> struct xe_late_bind_fw late_bind_fw;
>> + /** @late_bind.wq: workqueue to submit request to download late
>> bind blob */
>> + struct workqueue_struct *wq;
>> };
>> #endif
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-06 17:57 ` [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load " Badal Nilawar
2025-06-11 0:17 ` Daniele Ceraolo Spurio
@ 2025-06-15 18:26 ` Umesh Nerlige Ramappa
2025-06-17 9:00 ` Nilawar, Badal
1 sibling, 1 reply; 28+ messages in thread
From: Umesh Nerlige Ramappa @ 2025-06-15 18:26 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-xe, dri-devel, anshuman.gupta, rodrigo.vivi,
alexander.usyskin, gregkh, daniele.ceraolospurio, jgg
Hi Badal,
Just a small query below..
On Fri, Jun 06, 2025 at 11:27:02PM +0530, Badal Nilawar wrote:
>Load late binding firmware
>
>v2:
> - s/EAGAIN/EBUSY/
> - Flush worker in suspend and driver unload (Daniele)
>
>Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>---
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 121 ++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
> 3 files changed, 126 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>index 0231f3dcfc18..7fe304c54374 100644
>--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>@@ -16,6 +16,16 @@
> #include "xe_late_bind_fw.h"
> #include "xe_pcode.h"
> #include "xe_pcode_api.h"
>+#include "xe_pm.h"
>+
>+/*
>+ * The component should load quite quickly in most cases, but it could take
>+ * a bit. Using a very big timeout just to cover the worst case scenario
>+ */
>+#define LB_INIT_TIMEOUT_MS 20000
>+
>+#define LB_FW_LOAD_RETRY_MAXCOUNT 40
>+#define LB_FW_LOAD_RETRY_PAUSE_MS 50
>
> static const char * const fw_type_to_name[] = {
> [CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
>@@ -39,6 +49,95 @@ static int late_bind_fw_num_fans(struct xe_late_bind *late_bind)
> return 0;
> }
>
>+static void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
>+{
>+ struct xe_device *xe = late_bind_to_xe(late_bind);
>+ struct xe_late_bind_fw *lbfw;
>+
>+ lbfw = &late_bind->late_bind_fw;
>+ if (lbfw->valid && late_bind->wq) {
>+ drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
>+ fw_type_to_name[lbfw->type]);
>+ flush_work(&lbfw->work);
>+ }
>+}
>+
>+static void late_bind_work(struct work_struct *work)
Why is this a worker? Do we let the kmd probe go ahead while this worker
is trying to do a push_config? Wondering why this logic is not directly
called from the bind here.
Thanks,
Umesh
>+{
>+ struct xe_late_bind_fw *lbfw = container_of(work, struct xe_late_bind_fw, work);
>+ struct xe_late_bind *late_bind = container_of(lbfw, struct xe_late_bind,
>+ late_bind_fw);
>+ struct xe_device *xe = late_bind_to_xe(late_bind);
>+ int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
>+ int ret;
>+ int slept;
>+
>+ if (!late_bind->component_added)
>+ return;
>+
>+ if (!lbfw->valid)
>+ return;
>+
>+ /* we can queue this before the component is bound */
>+ for (slept = 0; slept < LB_INIT_TIMEOUT_MS; slept += 100) {
>+ if (late_bind->component.ops)
>+ break;
>+ msleep(100);
>+ }
>+
>+ xe_pm_runtime_get(xe);
>+ mutex_lock(&late_bind->mutex);
>+
>+ if (!late_bind->component.ops) {
>+ drm_err(&xe->drm, "Late bind component not bound\n");
>+ goto out;
>+ }
>+
>+ drm_dbg(&xe->drm, "Load %s firmware\n", fw_type_to_name[lbfw->type]);
>+
>+ do {
>+ ret = late_bind->component.ops->push_config(late_bind->component.mei_dev,
>+ lbfw->type, lbfw->flags,
>+ lbfw->payload, lbfw->payload_size);
>+ if (!ret)
>+ break;
>+ msleep(LB_FW_LOAD_RETRY_PAUSE_MS);
>+ } while (--retry && ret == -EBUSY);
>+
>+ if (ret)
>+ drm_err(&xe->drm, "Load %s firmware failed with err %d\n",
>+ fw_type_to_name[lbfw->type], ret);
>+ else
>+ drm_dbg(&xe->drm, "Load %s firmware successful\n",
>+ fw_type_to_name[lbfw->type]);
>+out:
>+ mutex_unlock(&late_bind->mutex);
>+ xe_pm_runtime_put(xe);
>+}
>+
>+int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
>+{
>+ struct xe_device *xe = late_bind_to_xe(late_bind);
>+ struct xe_late_bind_fw *lbfw;
>+
>+ if (!late_bind->component_added)
>+ return -EINVAL;
>+
>+ lbfw = &late_bind->late_bind_fw;
>+ if (lbfw->valid) {
>+ drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
>+ fw_type_to_name[lbfw->type]);
>+ queue_work(late_bind->wq, &lbfw->work);
>+ }
>+
>+ return 0;
>+}
>+
>+/**
>+ * late_bind_fw_init() - initialize late bind firmware
>+ *
>+ * Return: 0 if the initialization was successful, a negative errno otherwise.
>+ */
> static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
> {
> struct xe_device *xe = late_bind_to_xe(late_bind);
>@@ -87,6 +186,7 @@ static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
>
> memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
> release_firmware(fw);
>+ INIT_WORK(&lb_fw->work, late_bind_work);
> lb_fw->valid = true;
>
> return 0;
>@@ -99,7 +199,17 @@ static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
> */
> int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
> {
>- return late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
>+ int ret;
>+
>+ late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
>+ if (!late_bind->wq)
>+ return -ENOMEM;
>+
>+ ret = late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
>+ if (ret)
>+ return ret;
>+
>+ return xe_late_bind_fw_load(late_bind);
> }
>
> static int xe_late_bind_component_bind(struct device *xe_kdev,
>@@ -122,6 +232,8 @@ static void xe_late_bind_component_unbind(struct device *xe_kdev,
> struct xe_device *xe = kdev_to_xe_device(xe_kdev);
> struct xe_late_bind *late_bind = &xe->late_bind;
>
>+ xe_late_bind_wait_for_worker_completion(late_bind);
>+
> mutex_lock(&late_bind->mutex);
> late_bind->component.ops = NULL;
> mutex_unlock(&late_bind->mutex);
>@@ -140,9 +252,16 @@ static void xe_late_bind_remove(void *arg)
> if (!late_bind->component_added)
> return;
>
>+ xe_late_bind_wait_for_worker_completion(late_bind);
>+
> component_del(xe->drm.dev, &xe_late_bind_component_ops);
> late_bind->component_added = false;
>+ if (late_bind->wq) {
>+ destroy_workqueue(late_bind->wq);
>+ late_bind->wq = NULL;
>+ }
> mutex_destroy(&late_bind->mutex);
>+
> }
>
> /**
>diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>index a8b47523b203..166957e84c2f 100644
>--- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>@@ -12,5 +12,6 @@ struct xe_late_bind;
>
> int xe_late_bind_init(struct xe_late_bind *late_bind);
> int xe_late_bind_fw_init(struct xe_late_bind *late_bind);
>+int xe_late_bind_fw_load(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
>index c427e141c685..690680e8ff22 100644
>--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>@@ -9,6 +9,7 @@
> #include <linux/iosys-map.h>
> #include <linux/mutex.h>
> #include <linux/types.h>
>+#include <linux/workqueue.h>
>
> #define MAX_PAYLOAD_SIZE (1024 * 4)
>
>@@ -43,6 +44,8 @@ struct xe_late_bind_fw {
> u8 payload[MAX_PAYLOAD_SIZE];
> /** @late_bind_fw.payload_size: late binding blob payload_size */
> size_t payload_size;
>+ /** @late_bind_fw.work: worker to upload latebind blob */
>+ struct work_struct work;
> };
>
> /**
>@@ -71,6 +74,8 @@ struct xe_late_bind {
> struct mutex mutex;
> /** @late_bind.late_bind_fw: late binding firmware */
> struct xe_late_bind_fw late_bind_fw;
>+ /** @late_bind.wq: workqueue to submit request to download late bind blob */
>+ struct workqueue_struct *wq;
> };
>
> #endif
>--
>2.34.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-15 18:26 ` Umesh Nerlige Ramappa
@ 2025-06-17 9:00 ` Nilawar, Badal
0 siblings, 0 replies; 28+ messages in thread
From: Nilawar, Badal @ 2025-06-17 9:00 UTC (permalink / raw)
To: Umesh Nerlige Ramappa
Cc: intel-xe, dri-devel, anshuman.gupta, rodrigo.vivi,
alexander.usyskin, gregkh, daniele.ceraolospurio, jgg
Hi Umesh,
On 15-06-2025 23:56, Umesh Nerlige Ramappa wrote:
> Hi Badal,
>
> Just a small query below..
>
> On Fri, Jun 06, 2025 at 11:27:02PM +0530, Badal Nilawar wrote:
>> Load late binding firmware
>>
>> v2:
>> - s/EAGAIN/EBUSY/
>> - Flush worker in suspend and driver unload (Daniele)
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 121 ++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
>> 3 files changed, 126 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> index 0231f3dcfc18..7fe304c54374 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -16,6 +16,16 @@
>> #include "xe_late_bind_fw.h"
>> #include "xe_pcode.h"
>> #include "xe_pcode_api.h"
>> +#include "xe_pm.h"
>> +
>> +/*
>> + * The component should load quite quickly in most cases, but it
>> could take
>> + * a bit. Using a very big timeout just to cover the worst case
>> scenario
>> + */
>> +#define LB_INIT_TIMEOUT_MS 20000
>> +
>> +#define LB_FW_LOAD_RETRY_MAXCOUNT 40
>> +#define LB_FW_LOAD_RETRY_PAUSE_MS 50
>>
>> static const char * const fw_type_to_name[] = {
>> [CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
>> @@ -39,6 +49,95 @@ static int late_bind_fw_num_fans(struct
>> xe_late_bind *late_bind)
>> return 0;
>> }
>>
>> +static void xe_late_bind_wait_for_worker_completion(struct
>> xe_late_bind *late_bind)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + struct xe_late_bind_fw *lbfw;
>> +
>> + lbfw = &late_bind->late_bind_fw;
>> + if (lbfw->valid && late_bind->wq) {
>> + drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
>> + fw_type_to_name[lbfw->type]);
>> + flush_work(&lbfw->work);
>> + }
>> +}
>> +
>> +static void late_bind_work(struct work_struct *work)
>
> Why is this a worker? Do we let the kmd probe go ahead while this
> worker is trying to do a push_config? Wondering why this logic is not
> directly called from the bind here.
Component load may take time and doesn't happen in sequence, so
push_config should be done through worker. Also push_config may take
time to connect with mei, which is handled with retry on busy.
[ 38.014850] xe 0000:03:00.0: [drm:skl_compute_wm [xe]] [CRTC:88:pipe
A] dbuf slices 0x1 -> 0xf, ddb (0 - 682) -> (0 - 4096), active pipes 0x1
-> 0x1
[ 38.014934] xe 0000:03:00.0: [drm:skl_compute_wm [xe]]
[PLANE:33:plane 1A] ddb ( 0 - 682) -> ( 0 - 4037), size 682 -> 4037
[ 38.014941] mei_late_bind
xe.mei-gscfi.768-e2c2afa2-3817-4d19-9d95-06b16b588a5d: bound
0000:03:00.0 (ops xe_late_bind_component_ops [xe])
[ 38.015012] xe 0000:03:00.0: [drm:skl_compute_wm [xe]]
[PLANE:83:cursor A] ddb ( 0 - 0) -> (4037 - 4096), size 0 -> 59
[ 38.015088] xe 0000:03:00.0: [drm:skl_compute_wm [xe]]
[PLANE:33:plane 1A] level *wm0, wm1, wm2, wm3, wm4, wm5, wm6, wm7,
twm,*swm, stwm -> *wm0,*wm1,*wm2,*wm3,*wm4,*wm5, wm6, wm7, twm,*swm, stwm
Worker can be scheduled from bind call, but in s2idle resume flow
interface rebounds. Which will eventually schedule the worker to reload
the lb binary. Which we don't want.
Regards,
Badal
>
> Thanks,
> Umesh
>
>> +{
>> + struct xe_late_bind_fw *lbfw = container_of(work, struct
>> xe_late_bind_fw, work);
>> + struct xe_late_bind *late_bind = container_of(lbfw, struct
>> xe_late_bind,
>> + late_bind_fw);
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
>> + int ret;
>> + int slept;
>> +
>> + if (!late_bind->component_added)
>> + return;
>> +
>> + if (!lbfw->valid)
>> + return;
>> +
>> + /* we can queue this before the component is bound */
>> + for (slept = 0; slept < LB_INIT_TIMEOUT_MS; slept += 100) {
>> + if (late_bind->component.ops)
>> + break;
>> + msleep(100);
>> + }
>> +
>> + xe_pm_runtime_get(xe);
>> + mutex_lock(&late_bind->mutex);
>> +
>> + if (!late_bind->component.ops) {
>> + drm_err(&xe->drm, "Late bind component not bound\n");
>> + goto out;
>> + }
>> +
>> + drm_dbg(&xe->drm, "Load %s firmware\n",
>> fw_type_to_name[lbfw->type]);
>> +
>> + do {
>> + ret =
>> late_bind->component.ops->push_config(late_bind->component.mei_dev,
>> + lbfw->type, lbfw->flags,
>> + lbfw->payload, lbfw->payload_size);
>> + if (!ret)
>> + break;
>> + msleep(LB_FW_LOAD_RETRY_PAUSE_MS);
>> + } while (--retry && ret == -EBUSY);
>> +
>> + if (ret)
>> + drm_err(&xe->drm, "Load %s firmware failed with err %d\n",
>> + fw_type_to_name[lbfw->type], ret);
>> + else
>> + drm_dbg(&xe->drm, "Load %s firmware successful\n",
>> + fw_type_to_name[lbfw->type]);
>> +out:
>> + mutex_unlock(&late_bind->mutex);
>> + xe_pm_runtime_put(xe);
>> +}
>> +
>> +int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + struct xe_late_bind_fw *lbfw;
>> +
>> + if (!late_bind->component_added)
>> + return -EINVAL;
>> +
>> + lbfw = &late_bind->late_bind_fw;
>> + if (lbfw->valid) {
>> + drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
>> + fw_type_to_name[lbfw->type]);
>> + queue_work(late_bind->wq, &lbfw->work);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * late_bind_fw_init() - initialize late bind firmware
>> + *
>> + * Return: 0 if the initialization was successful, a negative errno
>> otherwise.
>> + */
>> static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
>> {
>> struct xe_device *xe = late_bind_to_xe(late_bind);
>> @@ -87,6 +186,7 @@ static int late_bind_fw_init(struct xe_late_bind
>> *late_bind, u32 type)
>>
>> memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
>> release_firmware(fw);
>> + INIT_WORK(&lb_fw->work, late_bind_work);
>> lb_fw->valid = true;
>>
>> return 0;
>> @@ -99,7 +199,17 @@ static int late_bind_fw_init(struct xe_late_bind
>> *late_bind, u32 type)
>> */
>> int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
>> {
>> - return late_bind_fw_init(late_bind,
>> CSC_LATE_BINDING_TYPE_FAN_CONTROL);
>> + int ret;
>> +
>> + late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
>> + if (!late_bind->wq)
>> + return -ENOMEM;
>> +
>> + ret = late_bind_fw_init(late_bind,
>> CSC_LATE_BINDING_TYPE_FAN_CONTROL);
>> + if (ret)
>> + return ret;
>> +
>> + return xe_late_bind_fw_load(late_bind);
>> }
>>
>> static int xe_late_bind_component_bind(struct device *xe_kdev,
>> @@ -122,6 +232,8 @@ static void xe_late_bind_component_unbind(struct
>> device *xe_kdev,
>> struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>> struct xe_late_bind *late_bind = &xe->late_bind;
>>
>> + xe_late_bind_wait_for_worker_completion(late_bind);
>> +
>> mutex_lock(&late_bind->mutex);
>> late_bind->component.ops = NULL;
>> mutex_unlock(&late_bind->mutex);
>> @@ -140,9 +252,16 @@ static void xe_late_bind_remove(void *arg)
>> if (!late_bind->component_added)
>> return;
>>
>> + xe_late_bind_wait_for_worker_completion(late_bind);
>> +
>> component_del(xe->drm.dev, &xe_late_bind_component_ops);
>> late_bind->component_added = false;
>> + if (late_bind->wq) {
>> + destroy_workqueue(late_bind->wq);
>> + late_bind->wq = NULL;
>> + }
>> mutex_destroy(&late_bind->mutex);
>> +
>> }
>>
>> /**
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> index a8b47523b203..166957e84c2f 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> @@ -12,5 +12,6 @@ struct xe_late_bind;
>>
>> int xe_late_bind_init(struct xe_late_bind *late_bind);
>> int xe_late_bind_fw_init(struct xe_late_bind *late_bind);
>> +int xe_late_bind_fw_load(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
>> index c427e141c685..690680e8ff22 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> @@ -9,6 +9,7 @@
>> #include <linux/iosys-map.h>
>> #include <linux/mutex.h>
>> #include <linux/types.h>
>> +#include <linux/workqueue.h>
>>
>> #define MAX_PAYLOAD_SIZE (1024 * 4)
>>
>> @@ -43,6 +44,8 @@ struct xe_late_bind_fw {
>> u8 payload[MAX_PAYLOAD_SIZE];
>> /** @late_bind_fw.payload_size: late binding blob payload_size */
>> size_t payload_size;
>> + /** @late_bind_fw.work: worker to upload latebind blob */
>> + struct work_struct work;
>> };
>>
>> /**
>> @@ -71,6 +74,8 @@ struct xe_late_bind {
>> struct mutex mutex;
>> /** @late_bind.late_bind_fw: late binding firmware */
>> struct xe_late_bind_fw late_bind_fw;
>> + /** @late_bind.wq: workqueue to submit request to download late
>> bind blob */
>> + struct workqueue_struct *wq;
>> };
>>
>> #endif
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
` (4 preceding siblings ...)
2025-06-06 17:57 ` [PATCH v2 05/10] drm/xe/xe_late_bind_fw: Load " Badal Nilawar
@ 2025-06-06 17:57 ` Badal Nilawar
2025-06-06 17:57 ` [PATCH v2 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume Badal Nilawar
` (3 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:57 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Reload late binding fw during runtime resume.
v2: Flush worker during runtime suspend
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 2 +-
drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
drivers/gpu/drm/xe/xe_pm.c | 6 ++++++
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 7fe304c54374..d69f950bddd2 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -49,7 +49,7 @@ static int late_bind_fw_num_fans(struct xe_late_bind *late_bind)
return 0;
}
-static void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
+void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
{
struct xe_device *xe = late_bind_to_xe(late_bind);
struct xe_late_bind_fw *lbfw;
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
index 166957e84c2f..5fb724c5c863 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
@@ -13,5 +13,6 @@ struct xe_late_bind;
int xe_late_bind_init(struct xe_late_bind *late_bind);
int xe_late_bind_fw_init(struct xe_late_bind *late_bind);
int xe_late_bind_fw_load(struct xe_late_bind *late_bind);
+void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind);
#endif
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 693866def183..af6f6e63ca9c 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -20,6 +20,7 @@
#include "xe_gt.h"
#include "xe_guc.h"
#include "xe_irq.h"
+#include "xe_late_bind_fw.h"
#include "xe_pcode.h"
#include "xe_pxp.h"
#include "xe_trace.h"
@@ -460,6 +461,8 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
if (err)
goto out;
+ xe_late_bind_wait_for_worker_completion(&xe->late_bind);
+
/*
* Applying lock for entire list op as xe_ttm_bo_destroy and xe_bo_move_notify
* also checks and deletes bo entry from user fault list.
@@ -550,6 +553,9 @@ int xe_pm_runtime_resume(struct xe_device *xe)
xe_pxp_pm_resume(xe->pxp);
+ if (xe->d3cold.allowed)
+ xe_late_bind_fw_load(&xe->late_bind);
+
out:
xe_rpm_lockmap_release(xe);
xe_pm_write_callback_task(xe, NULL);
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
` (5 preceding siblings ...)
2025-06-06 17:57 ` [PATCH v2 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume Badal Nilawar
@ 2025-06-06 17:57 ` Badal Nilawar
2025-06-06 17:57 ` [PATCH v2 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Badal Nilawar
` (2 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:57 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Reload late binding fw during S2Idle/S3 resume.
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_pm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index af6f6e63ca9c..2fa3d4f1b192 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -205,6 +205,9 @@ int xe_pm_resume(struct xe_device *xe)
xe_pxp_pm_resume(xe->pxp);
+ if (xe->d3cold.allowed)
+ xe_late_bind_fw_load(&xe->late_bind);
+
drm_dbg(&xe->drm, "Device resumed\n");
return 0;
err:
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
` (6 preceding siblings ...)
2025-06-06 17:57 ` [PATCH v2 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume Badal Nilawar
@ 2025-06-06 17:57 ` Badal Nilawar
2025-06-12 21:28 ` Daniele Ceraolo Spurio
2025-06-06 17:57 ` [PATCH v2 09/10] {fwctl,drm}/xe/pcode: Introduce xe_pcode_fwctl Badal Nilawar
2025-06-06 17:57 ` [PATCH v2 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI Do not review Badal Nilawar
9 siblings, 1 reply; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:57 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Introduce a debug filesystem node to disable late binding fw reload
during the system or runtime resume. This is intended for situations
where the late binding fw needs to be loaded from user mode.
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_debugfs.c | 42 ++++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw.c | 5 ++-
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
index d83cd6ed3fa8..1c145089d177 100644
--- a/drivers/gpu/drm/xe/xe_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_debugfs.c
@@ -226,6 +226,45 @@ static const struct file_operations atomic_svm_timeslice_ms_fops = {
.write = atomic_svm_timeslice_ms_set,
};
+static ssize_t disable_late_binding_show(struct file *f, char __user *ubuf,
+ size_t size, loff_t *pos)
+{
+ struct xe_device *xe = file_inode(f)->i_private;
+ struct xe_late_bind *late_bind = &xe->late_bind;
+ char buf[32];
+ int len;
+
+ len = scnprintf(buf, sizeof(buf), "%d\n", late_bind->disable);
+
+ return simple_read_from_buffer(ubuf, size, pos, buf, len);
+}
+
+static ssize_t disable_late_binding_set(struct file *f, const char __user *ubuf,
+ size_t size, loff_t *pos)
+{
+ struct xe_device *xe = file_inode(f)->i_private;
+ struct xe_late_bind *late_bind = &xe->late_bind;
+ u32 uval;
+ ssize_t ret;
+
+ ret = kstrtouint_from_user(ubuf, size, sizeof(uval), &uval);
+ if (ret)
+ return ret;
+
+ if (uval > 1)
+ return -EINVAL;
+
+ late_bind->disable = (uval == 1) ? true : false;
+
+ return size;
+}
+
+static const struct file_operations disable_late_binding_fops = {
+ .owner = THIS_MODULE,
+ .read = disable_late_binding_show,
+ .write = disable_late_binding_set,
+};
+
void xe_debugfs_register(struct xe_device *xe)
{
struct ttm_device *bdev = &xe->ttm;
@@ -249,6 +288,9 @@ void xe_debugfs_register(struct xe_device *xe)
debugfs_create_file("atomic_svm_timeslice_ms", 0600, root, xe,
&atomic_svm_timeslice_ms_fops);
+ debugfs_create_file("disable_late_binding", 0600, root, xe,
+ &disable_late_binding_fops);
+
for (mem_type = XE_PL_VRAM0; mem_type <= XE_PL_VRAM1; ++mem_type) {
man = ttm_manager_type(bdev, mem_type);
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index d69f950bddd2..f9d3d0f341f2 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -123,11 +123,14 @@ int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
if (!late_bind->component_added)
return -EINVAL;
+ if (late_bind->disable)
+ return 0;
+
lbfw = &late_bind->late_bind_fw;
if (lbfw->valid) {
drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
fw_type_to_name[lbfw->type]);
- queue_work(late_bind->wq, &lbfw->work);
+ queue_work(late_bind->wq, &lbfw->work);
}
return 0;
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
index 690680e8ff22..6f4a945ca236 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -76,6 +76,9 @@ struct xe_late_bind {
struct xe_late_bind_fw late_bind_fw;
/** @late_bind.wq: workqueue to submit request to download late bind blob */
struct workqueue_struct *wq;
+
+ /** @late_bind.disable to block late binding reload during pm resume flow*/
+ bool disable;
};
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-06-06 17:57 ` [PATCH v2 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Badal Nilawar
@ 2025-06-12 21:28 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 28+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-12 21:28 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/6/2025 10:57 AM, Badal Nilawar wrote:
> Introduce a debug filesystem node to disable late binding fw reload
> during the system or runtime resume. This is intended for situations
> where the late binding fw needs to be loaded from user mode.
Can you clarify a bit what the use-case here is? Because this
description makes it sound like this is something a user might want to
do, but the debugfs interface is for testing only. If we only need this
for testing, please specify so.
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/xe_debugfs.c | 42 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 5 ++-
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++
> 3 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index d83cd6ed3fa8..1c145089d177 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -226,6 +226,45 @@ static const struct file_operations atomic_svm_timeslice_ms_fops = {
> .write = atomic_svm_timeslice_ms_set,
> };
>
> +static ssize_t disable_late_binding_show(struct file *f, char __user *ubuf,
> + size_t size, loff_t *pos)
> +{
> + struct xe_device *xe = file_inode(f)->i_private;
> + struct xe_late_bind *late_bind = &xe->late_bind;
> + char buf[32];
> + int len;
> +
> + len = scnprintf(buf, sizeof(buf), "%d\n", late_bind->disable);
> +
> + return simple_read_from_buffer(ubuf, size, pos, buf, len);
> +}
> +
> +static ssize_t disable_late_binding_set(struct file *f, const char __user *ubuf,
> + size_t size, loff_t *pos)
> +{
> + struct xe_device *xe = file_inode(f)->i_private;
> + struct xe_late_bind *late_bind = &xe->late_bind;
> + u32 uval;
> + ssize_t ret;
> +
> + ret = kstrtouint_from_user(ubuf, size, sizeof(uval), &uval);
> + if (ret)
> + return ret;
> +
> + if (uval > 1)
> + return -EINVAL;
> +
> + late_bind->disable = (uval == 1) ? true : false;
nit: could do
late_bind->disable = !!uval;
> +
> + return size;
> +}
> +
> +static const struct file_operations disable_late_binding_fops = {
> + .owner = THIS_MODULE,
> + .read = disable_late_binding_show,
> + .write = disable_late_binding_set,
> +};
> +
> void xe_debugfs_register(struct xe_device *xe)
> {
> struct ttm_device *bdev = &xe->ttm;
> @@ -249,6 +288,9 @@ void xe_debugfs_register(struct xe_device *xe)
> debugfs_create_file("atomic_svm_timeslice_ms", 0600, root, xe,
> &atomic_svm_timeslice_ms_fops);
>
> + debugfs_create_file("disable_late_binding", 0600, root, xe,
> + &disable_late_binding_fops);
> +
> for (mem_type = XE_PL_VRAM0; mem_type <= XE_PL_VRAM1; ++mem_type) {
> man = ttm_manager_type(bdev, mem_type);
>
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> index d69f950bddd2..f9d3d0f341f2 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -123,11 +123,14 @@ int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
> if (!late_bind->component_added)
> return -EINVAL;
>
> + if (late_bind->disable)
> + return 0;
> +
> lbfw = &late_bind->late_bind_fw;
> if (lbfw->valid) {
> drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
> fw_type_to_name[lbfw->type]);
> - queue_work(late_bind->wq, &lbfw->work);
> + queue_work(late_bind->wq, &lbfw->work);
This seems to be a fix for a broken indent in a previous patch in the
series, so it should be moved to that patch.
Daniele
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> index 690680e8ff22..6f4a945ca236 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> @@ -76,6 +76,9 @@ struct xe_late_bind {
> struct xe_late_bind_fw late_bind_fw;
> /** @late_bind.wq: workqueue to submit request to download late bind blob */
> struct workqueue_struct *wq;
> +
> + /** @late_bind.disable to block late binding reload during pm resume flow*/
> + bool disable;
> };
>
> #endif
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 09/10] {fwctl,drm}/xe/pcode: Introduce xe_pcode_fwctl
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
` (7 preceding siblings ...)
2025-06-06 17:57 ` [PATCH v2 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Badal Nilawar
@ 2025-06-06 17:57 ` Badal Nilawar
2025-06-16 14:42 ` Rodrigo Vivi
2025-06-06 17:57 ` [PATCH v2 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI Do not review Badal Nilawar
9 siblings, 1 reply; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:57 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
Xe PCODE FWCTL implements the generic FWCTL IOCLTs to allow limited
access from user space (as admin) to some very specific PCODE
Mailboxes only related to hardware configuration.
PCODE is a Firmware in Intel GPUs which is the main responsible
component for power and thermal aspects of the Intel GPUs.
Each different Intel GPU came with different PCODE versions with
different mailboxes and different needs. In the lack of an unified
interface, the per platform sysfs entries at the device level is
trending to grow, to allow admins to control different aspects of
the Hardware.
In this first experiment, xe_pcode_fwctl only adds support for the
Battlemage late-binding firmware information.
Late-binding is the name given to 2 other new auxiliary firmware
blobs that for now lives in the Flash like PCODE, but that soon
it is coming to linux-firmware.git: Fan-controller and
Voltage-regulator. Then, PCODE provides some mailboxes where the
status of both late-binding firmware can be queried as specified
in the documentation that is added along with the new uAPI here.
RFC IMPORTANT NOTE:
===================
Admins will need to query this information. This code here aims
to be used by Level0-Sysman and/or Intel XPU Manager directly
from user space. But following the drm upstream rules, the
userspace code will need to be ready before we can consider
getting this patch merged!
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
v2:
- Resolve review comments (Jason Gunthorpe)
- Use explicit padding in structure fwctl_rpc_xe_pcode (Jason Gunthorpe)
- Add xe_pcode_fwctl.rst
---
Documentation/userspace-api/fwctl/index.rst | 1 +
.../userspace-api/fwctl/xe_pcode_fwctl.rst | 17 ++
drivers/gpu/drm/xe/Kconfig | 1 +
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_pci.c | 5 +
drivers/gpu/drm/xe/xe_pcode_fwctl.c | 212 ++++++++++++++++++
drivers/gpu/drm/xe/xe_pcode_fwctl.h | 13 ++
include/uapi/fwctl/fwctl.h | 1 +
include/uapi/fwctl/xe_pcode.h | 82 +++++++
9 files changed, 333 insertions(+)
create mode 100644 Documentation/userspace-api/fwctl/xe_pcode_fwctl.rst
create mode 100644 drivers/gpu/drm/xe/xe_pcode_fwctl.c
create mode 100644 drivers/gpu/drm/xe/xe_pcode_fwctl.h
create mode 100644 include/uapi/fwctl/xe_pcode.h
diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
index 316ac456ad3b..186f8cf17583 100644
--- a/Documentation/userspace-api/fwctl/index.rst
+++ b/Documentation/userspace-api/fwctl/index.rst
@@ -12,3 +12,4 @@ to securely construct and execute RPCs inside device firmware.
fwctl
fwctl-cxl
pds_fwctl
+ xe_pcode_fwctl
diff --git a/Documentation/userspace-api/fwctl/xe_pcode_fwctl.rst b/Documentation/userspace-api/fwctl/xe_pcode_fwctl.rst
new file mode 100644
index 000000000000..c16d0675a485
--- /dev/null
+++ b/Documentation/userspace-api/fwctl/xe_pcode_fwctl.rst
@@ -0,0 +1,17 @@
+.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+==================
+fwctl drm/xe pcode
+==================
+
+.. kernel-doc:: drivers/gpu/drm/xe/xe_pcode_fwctl.c
+ :doc: XE PCODE FWCTL
+
+uAPI
+====
+
+.. kernel-doc:: include/uapi/fwctl/xe_pcode.h
+ :internal:
+
+.. kernel-doc:: include/uapi/fwctl/xe_pcode.h
+ :doc: Late Binding Commands
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 98b46c534278..27a06d7359c7 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -45,6 +45,7 @@ config DRM_XE
select WANT_DEV_COREDUMP
select AUXILIARY_BUS
select HMM_MIRROR
+ select FWCTL
help
Experimental driver for Intel Xe series GPUs
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 134eee21c75e..eecf5b1680a6 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -86,6 +86,7 @@ xe-y += xe_bb.o \
xe_pat.o \
xe_pci.o \
xe_pcode.o \
+ xe_pcode_fwctl.o \
xe_pm.o \
xe_preempt_fence.o \
xe_pt.o \
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index ac4beaed58ff..d0c428adb4c2 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -27,6 +27,7 @@
#include "xe_module.h"
#include "xe_pci_sriov.h"
#include "xe_pci_types.h"
+#include "xe_pcode_fwctl.h"
#include "xe_pm.h"
#include "xe_sriov.h"
#include "xe_step.h"
@@ -875,6 +876,10 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (err)
goto err_driver_cleanup;
+ err = xe_pcode_fwctl_init(xe);
+ if (err)
+ goto err_driver_cleanup;
+
drm_dbg(&xe->drm, "d3cold: capable=%s\n",
str_yes_no(xe->d3cold.capable));
diff --git a/drivers/gpu/drm/xe/xe_pcode_fwctl.c b/drivers/gpu/drm/xe/xe_pcode_fwctl.c
new file mode 100644
index 000000000000..849bf3fa4d30
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_pcode_fwctl.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#include "xe_pcode_fwctl.h"
+
+#include <linux/fwctl.h>
+#include <uapi/fwctl/xe_pcode.h>
+
+#include "xe_device.h"
+#include "xe_pcode_api.h"
+#include "xe_pcode.h"
+#include "xe_pm.h"
+
+/**
+ * DOC: XE PCODE FWCTL
+ *
+ * Xe PCODE FWCTL implements the generic FWCTL IOCLTs to allow limited access
+ * from user space (as admin) to some very specific PCODE Mailboxes.
+ *
+ * User space first needs to issue the ```FWCTL_INFO``` ioctl and check for the
+ * capability flag, which will indicate which group of Mailboxes commands are
+ * supported on that current running firmware.
+ *
+ * After verifying the availability of the desired Mailbox command,
+ * ```FWCTL_RPC``` needs to be issued with in and out parameter both using
+ * pointers to a ```struct fwctl_rpc_xe_pcode``` allocated by userspace.
+ * In and out length needs to be sizeof(struct fwctl_rpc_xe_pcode).
+ *
+ * Any command that is not listed in the include/uapi/fwctl/xe_pcode.h or not
+ * supported by the running firmware, will return ERR_PTR(-EBADMSG).
+ *
+ * Example:
+ *
+ * .. code-block:: C
+ *
+ * struct fwctl_info_xe_pcode xe_pcode_info;
+ *
+ * struct fwctl_info info = {
+ * .size = sizeof(struct fwctl_info),
+ * .flags = 0,
+ * .out_device_type = 0,
+ * .device_data_len = sizeof(struct fwctl_info_xe_pcode),
+ * .out_device_data = (__aligned_u64) &xe_pcode_info,
+ * };
+ *
+ * fd = open("/dev/fwctl/fwctl0", O_RDWR);
+ * if (fd < 0) {
+ * perror("Failed to open /dev/fwctl/fwctl0");
+ * return -1;
+ * }
+ *
+ * if (ioctl(fd, FWCTL_INFO, &info)) {
+ * perror("ioctl(FWCTL_INFO) failed");
+ * close(fd);
+ * return -1;
+ * }
+ *
+ * if (xe_pcode_info.uctx_caps & FWCTL_XE_PCODE_LATEBINDING) {
+ * struct fwctl_rpc_xe_pcode rpc_in = {
+ * .command = PCODE_CMD_LATE_BINDING,
+ * .param1 = PARAM1_GET_CAPABILITY_STATUS,
+ * };
+ *
+ * struct fwctl_rpc_xe_pcode rpc_out = {0};
+ *
+ * struct fwctl_rpc rpc = {
+ * .size = sizeof(struct fwctl_rpc),
+ * .scope = FWCTL_RPC_CONFIGURATION,
+ * .in_len = sizeof(struct fwctl_rpc_xe_pcode),
+ * .out_len = sizeof(struct fwctl_rpc_xe_pcode),
+ * .in = (__aligned_u64) &rpc_in,
+ * .out = (__aligned_u64) &rpc_out,
+ * };
+ *
+ * if (ioctl(fd, FWCTL_RPC, &rpc)) {
+ * perror("ioctl(FWCTL_RPC) failed");
+ * close(fd);
+ * return -1;
+ * }
+ *
+ */
+
+struct xe_pcode_fwctl_dev {
+ struct fwctl_device fwctl;
+ struct xe_device *xe;
+};
+
+DEFINE_FREE(xe_pcode_fwctl, struct xe_pcode_fwctl_dev *, if (_T) fwctl_put(&_T->fwctl))
+
+static int xe_pcode_fwctl_uctx_open(struct fwctl_uctx *uctx)
+{
+ return 0;
+}
+
+static void xe_pcode_fwctl_uctx_close(struct fwctl_uctx *uctx)
+{
+}
+
+static void *xe_pcode_fwctl_info(struct fwctl_uctx *uctx, size_t *length)
+{
+ struct xe_pcode_fwctl_dev *fwctl_dev =
+ container_of(uctx->fwctl, struct xe_pcode_fwctl_dev, fwctl);
+ struct xe_device *xe = fwctl_dev->xe;
+ struct fwctl_info_xe_pcode *info;
+
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return ERR_PTR(-ENOMEM);
+
+ if (xe->info.platform == XE_BATTLEMAGE)
+ info->uctx_caps = FWCTL_XE_PCODE_LATEBINDING;
+
+ *length = sizeof(*info);
+
+ return info;
+}
+
+static bool xe_pcode_fwctl_rpc_validate(struct fwctl_rpc_xe_pcode *rpc,
+ enum fwctl_rpc_scope scope)
+{
+ u32 mbox = PCODE_MBOX(rpc->command, rpc->param1, rpc->param2);
+
+ if (mbox == PCODE_MBOX(PCODE_CMD_LATE_BINDING,
+ PARAM1_GET_CAPABILITY_STATUS, 0))
+ return scope == FWCTL_RPC_CONFIGURATION;
+
+ if (mbox == PCODE_MBOX(PCODE_CMD_LATE_BINDING,
+ PARAM1_GET_VERSION_LOW, 0))
+ return (rpc->data0 == DATA0_TYPE_FAN_CONTROLLER ||
+ rpc->data0 == DATA0_TYPE_VOLTAGE_REGULATOR) &&
+ scope == FWCTL_RPC_CONFIGURATION;
+
+ return false;
+}
+
+static void *xe_pcode_fwctl_rpc(struct fwctl_uctx *uctx,
+ enum fwctl_rpc_scope scope,
+ void *in, size_t in_len, size_t *out_len)
+{
+ struct xe_pcode_fwctl_dev *fwctl_dev =
+ container_of(uctx->fwctl, struct xe_pcode_fwctl_dev, fwctl);
+ struct xe_tile *root_tile = xe_device_get_root_tile(fwctl_dev->xe);
+ struct fwctl_rpc_xe_pcode *rpc = in;
+ int err;
+
+ if (in_len != sizeof(struct fwctl_rpc_xe_pcode) ||
+ *out_len != sizeof(struct fwctl_rpc_xe_pcode))
+ return ERR_PTR(-EMSGSIZE);
+
+ if (!xe_pcode_fwctl_rpc_validate(rpc, scope))
+ return ERR_PTR(-EPERM);
+
+ xe_pm_runtime_get(fwctl_dev->xe);
+
+ err = xe_pcode_read(root_tile, PCODE_MBOX(rpc->command,
+ rpc->param1,
+ rpc->param2),
+ &rpc->data0,
+ &rpc->data1);
+
+ xe_pm_runtime_put(fwctl_dev->xe);
+
+ if (err)
+ return ERR_PTR(err);
+
+ return rpc;
+}
+
+static const struct fwctl_ops xe_pcode_fwctl_ops = {
+ .device_type = FWCTL_DEVICE_TYPE_XE_PCODE,
+ .uctx_size = sizeof(struct fwctl_uctx),
+ .open_uctx = xe_pcode_fwctl_uctx_open,
+ .close_uctx = xe_pcode_fwctl_uctx_close,
+ .info = xe_pcode_fwctl_info,
+ .fw_rpc = xe_pcode_fwctl_rpc,
+};
+
+static void xe_pcode_fwctl_fini(void *dev)
+{
+ struct fwctl_device *fwctl = dev;
+
+ fwctl_unregister(fwctl);
+ fwctl_put(fwctl);
+}
+
+int xe_pcode_fwctl_init(struct xe_device *xe)
+{
+ struct xe_pcode_fwctl_dev *fwctl_dev __free(xe_pcode_fwctl) =
+ fwctl_alloc_device(xe->drm.dev, &xe_pcode_fwctl_ops,
+ struct xe_pcode_fwctl_dev, fwctl);
+ int err;
+
+ /* For now xe_pcode_fwctl supports only Late-Binding commands on BMG */
+ if (xe->info.platform != XE_BATTLEMAGE)
+ return -ENODEV;
+
+ if (!fwctl_dev)
+ return -ENOMEM;
+
+ fwctl_dev->xe = xe;
+
+ err = fwctl_register(&fwctl_dev->fwctl);
+ if (err)
+ return err;
+
+ return devm_add_action_or_reset(xe->drm.dev, xe_pcode_fwctl_fini,
+ &fwctl_dev->fwctl);
+}
+
+MODULE_IMPORT_NS("FWCTL");
diff --git a/drivers/gpu/drm/xe/xe_pcode_fwctl.h b/drivers/gpu/drm/xe/xe_pcode_fwctl.h
new file mode 100644
index 000000000000..67386d7bf2ea
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_pcode_fwctl.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#ifndef _XE_PCODE_FWCTL_H_
+#define _XE_PCODE_FWCTL_H_
+
+struct xe_device;
+
+int xe_pcode_fwctl_init(struct xe_device *xe);
+
+#endif
diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
index 716ac0eee42d..9e7e84aef791 100644
--- a/include/uapi/fwctl/fwctl.h
+++ b/include/uapi/fwctl/fwctl.h
@@ -45,6 +45,7 @@ enum fwctl_device_type {
FWCTL_DEVICE_TYPE_MLX5 = 1,
FWCTL_DEVICE_TYPE_CXL = 2,
FWCTL_DEVICE_TYPE_PDS = 4,
+ FWCTL_DEVICE_TYPE_XE_PCODE = 5,
};
/**
diff --git a/include/uapi/fwctl/xe_pcode.h b/include/uapi/fwctl/xe_pcode.h
new file mode 100644
index 000000000000..87544e99faa1
--- /dev/null
+++ b/include/uapi/fwctl/xe_pcode.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#ifndef _UAPI_FWCTL_XE_PCODE_H_
+#define _UAPI_FWCTL_XE_PCODE_H_
+
+#include <linux/types.h>
+
+/**
+ * struct fwctl_info_xe_pcode - FWCTL Information struct for Xe PCODE
+ *
+ * @uctx_caps: bitmap of available capabilities:
+ * - %FWCTL_XE_PCODE_LATEBINDING - Command to configure Late Bind FW such as
+ * Fan Controller and Voltage Regulator
+ * @rsvd: Reserved for future usage or flags
+ */
+struct fwctl_info_xe_pcode {
+ __u32 uctx_caps;
+ __u32 rsvd[3];
+};
+
+#define FWCTL_XE_PCODE_LATEBINDING (1 << 0)
+
+/**
+ * struct fwctl_rpc_xe_pcode - FWCTL Remote Procedure Calls for Xe PCODE
+ */
+struct fwctl_rpc_xe_pcode {
+ /** @command: The main Mailbox command */
+ __u8 command;
+ /** @pad: Padding the uAPI struct - Must be 0. Not sent to firmware */
+ __u8 pad[3];
+ /** @param1: A subcommand or a parameter of the main command */
+ __u16 param1;
+ /** @param2: A parameter of a subcommand or a subsubcommand */
+ __u16 param2;
+ /** @data0: The first 32 bits of data. In general data-in as param */
+ __u32 data0;
+ /** @data1: The other 32 bits of data. In general data-out */
+ __u32 data1;
+ /** @rsvd: Reserved for future use */
+ __u32 rsvd[2];
+};
+
+/**
+ * DOC: Late Binding Commands
+ *
+ * FWCTL info.uctx_caps: FWCTL_XE_PCODE_LATEBINDING
+ * FWCTL rpc.scope: FWCTL_RPC_CONFIGURATION
+ *
+ * Command 0x5C - LATE_BINDING
+ * Param1 0x0 - GET_CAPABILITY_STATUS
+ * Param2 0
+ * Data in None
+ * Data out:
+ *
+ * - Bit0: Late binding for V1 Fan Tables is supported.
+ * - Bit3: Late binding for VR parameters.
+ * - Bit16: Late binding done for V1 Fan tables
+ * - Bit17: Late binding done for power co-efficients.
+ * - Bit18: Late binding done for V2 Fan tables
+ * - Bit19: Late binding done for VR Parameters
+ *
+ * Command 0x5C - LATE_BINDING
+ * Param1 0x1 - GET_VERSION_LOW
+ * Param2 0
+ * Data in - conveys the Type of the Late Binding Configuration:
+ *
+ * - FAN_CONTROLLER = 1
+ * - VOLTAGE_REGULATOR = 2
+ *
+ * Data out - Lower 32 bits of Version Number for Late Binding configuration
+ * that has been applied successfully.
+ */
+#define PCODE_CMD_LATE_BINDING 0x5C
+#define PARAM1_GET_CAPABILITY_STATUS 0x0
+#define PARAM1_GET_VERSION_LOW 0x1
+#define DATA0_TYPE_FAN_CONTROLLER 1
+#define DATA0_TYPE_VOLTAGE_REGULATOR 2
+
+#endif /* _UAPI_FWCTL_XE_PCODE_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 09/10] {fwctl,drm}/xe/pcode: Introduce xe_pcode_fwctl
2025-06-06 17:57 ` [PATCH v2 09/10] {fwctl,drm}/xe/pcode: Introduce xe_pcode_fwctl Badal Nilawar
@ 2025-06-16 14:42 ` Rodrigo Vivi
0 siblings, 0 replies; 28+ messages in thread
From: Rodrigo Vivi @ 2025-06-16 14:42 UTC (permalink / raw)
To: Badal Nilawar, Simona Vetter, Dave Airlie
Cc: intel-xe, dri-devel, anshuman.gupta, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
On Fri, Jun 06, 2025 at 11:27:06PM +0530, Badal Nilawar wrote:
> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Xe PCODE FWCTL implements the generic FWCTL IOCLTs to allow limited
> access from user space (as admin) to some very specific PCODE
> Mailboxes only related to hardware configuration.
>
> PCODE is a Firmware in Intel GPUs which is the main responsible
> component for power and thermal aspects of the Intel GPUs.
>
> Each different Intel GPU came with different PCODE versions with
> different mailboxes and different needs. In the lack of an unified
> interface, the per platform sysfs entries at the device level is
> trending to grow, to allow admins to control different aspects of
> the Hardware.
>
> In this first experiment, xe_pcode_fwctl only adds support for the
> Battlemage late-binding firmware information.
>
> Late-binding is the name given to 2 other new auxiliary firmware
> blobs that for now lives in the Flash like PCODE, but that soon
> it is coming to linux-firmware.git: Fan-controller and
> Voltage-regulator. Then, PCODE provides some mailboxes where the
> status of both late-binding firmware can be queried as specified
> in the documentation that is added along with the new uAPI here.
>
> RFC IMPORTANT NOTE:
>
> ===================
> Admins will need to query this information. This code here aims
> to be used by Level0-Sysman and/or Intel XPU Manager directly
> from user space. But following the drm upstream rules, the
> userspace code will need to be ready before we can consider
> getting this patch merged!
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> v2:
> - Resolve review comments (Jason Gunthorpe)
I'm afraid it didn't resolve all of the comments as this code is still under drm
and not moved under fwctl.
Sima, Dave, what's your take on this? Should we move this to under fwctl
or should we step back for fwctl for now and only go with a couple extra sysfs
for this case?
Thanks,
Rodrigo.
> - Use explicit padding in structure fwctl_rpc_xe_pcode (Jason Gunthorpe)
> - Add xe_pcode_fwctl.rst
> ---
> Documentation/userspace-api/fwctl/index.rst | 1 +
> .../userspace-api/fwctl/xe_pcode_fwctl.rst | 17 ++
> drivers/gpu/drm/xe/Kconfig | 1 +
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_pci.c | 5 +
> drivers/gpu/drm/xe/xe_pcode_fwctl.c | 212 ++++++++++++++++++
> drivers/gpu/drm/xe/xe_pcode_fwctl.h | 13 ++
> include/uapi/fwctl/fwctl.h | 1 +
> include/uapi/fwctl/xe_pcode.h | 82 +++++++
> 9 files changed, 333 insertions(+)
> create mode 100644 Documentation/userspace-api/fwctl/xe_pcode_fwctl.rst
> create mode 100644 drivers/gpu/drm/xe/xe_pcode_fwctl.c
> create mode 100644 drivers/gpu/drm/xe/xe_pcode_fwctl.h
> create mode 100644 include/uapi/fwctl/xe_pcode.h
>
> diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
> index 316ac456ad3b..186f8cf17583 100644
> --- a/Documentation/userspace-api/fwctl/index.rst
> +++ b/Documentation/userspace-api/fwctl/index.rst
> @@ -12,3 +12,4 @@ to securely construct and execute RPCs inside device firmware.
> fwctl
> fwctl-cxl
> pds_fwctl
> + xe_pcode_fwctl
> diff --git a/Documentation/userspace-api/fwctl/xe_pcode_fwctl.rst b/Documentation/userspace-api/fwctl/xe_pcode_fwctl.rst
> new file mode 100644
> index 000000000000..c16d0675a485
> --- /dev/null
> +++ b/Documentation/userspace-api/fwctl/xe_pcode_fwctl.rst
> @@ -0,0 +1,17 @@
> +.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +==================
> +fwctl drm/xe pcode
> +==================
> +
> +.. kernel-doc:: drivers/gpu/drm/xe/xe_pcode_fwctl.c
> + :doc: XE PCODE FWCTL
> +
> +uAPI
> +====
> +
> +.. kernel-doc:: include/uapi/fwctl/xe_pcode.h
> + :internal:
> +
> +.. kernel-doc:: include/uapi/fwctl/xe_pcode.h
> + :doc: Late Binding Commands
> diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
> index 98b46c534278..27a06d7359c7 100644
> --- a/drivers/gpu/drm/xe/Kconfig
> +++ b/drivers/gpu/drm/xe/Kconfig
> @@ -45,6 +45,7 @@ config DRM_XE
> select WANT_DEV_COREDUMP
> select AUXILIARY_BUS
> select HMM_MIRROR
> + select FWCTL
> help
> Experimental driver for Intel Xe series GPUs
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 134eee21c75e..eecf5b1680a6 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -86,6 +86,7 @@ xe-y += xe_bb.o \
> xe_pat.o \
> xe_pci.o \
> xe_pcode.o \
> + xe_pcode_fwctl.o \
> xe_pm.o \
> xe_preempt_fence.o \
> xe_pt.o \
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index ac4beaed58ff..d0c428adb4c2 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -27,6 +27,7 @@
> #include "xe_module.h"
> #include "xe_pci_sriov.h"
> #include "xe_pci_types.h"
> +#include "xe_pcode_fwctl.h"
> #include "xe_pm.h"
> #include "xe_sriov.h"
> #include "xe_step.h"
> @@ -875,6 +876,10 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (err)
> goto err_driver_cleanup;
>
> + err = xe_pcode_fwctl_init(xe);
> + if (err)
> + goto err_driver_cleanup;
> +
> drm_dbg(&xe->drm, "d3cold: capable=%s\n",
> str_yes_no(xe->d3cold.capable));
>
> diff --git a/drivers/gpu/drm/xe/xe_pcode_fwctl.c b/drivers/gpu/drm/xe/xe_pcode_fwctl.c
> new file mode 100644
> index 000000000000..849bf3fa4d30
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pcode_fwctl.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#include "xe_pcode_fwctl.h"
> +
> +#include <linux/fwctl.h>
> +#include <uapi/fwctl/xe_pcode.h>
> +
> +#include "xe_device.h"
> +#include "xe_pcode_api.h"
> +#include "xe_pcode.h"
> +#include "xe_pm.h"
> +
> +/**
> + * DOC: XE PCODE FWCTL
> + *
> + * Xe PCODE FWCTL implements the generic FWCTL IOCLTs to allow limited access
> + * from user space (as admin) to some very specific PCODE Mailboxes.
> + *
> + * User space first needs to issue the ```FWCTL_INFO``` ioctl and check for the
> + * capability flag, which will indicate which group of Mailboxes commands are
> + * supported on that current running firmware.
> + *
> + * After verifying the availability of the desired Mailbox command,
> + * ```FWCTL_RPC``` needs to be issued with in and out parameter both using
> + * pointers to a ```struct fwctl_rpc_xe_pcode``` allocated by userspace.
> + * In and out length needs to be sizeof(struct fwctl_rpc_xe_pcode).
> + *
> + * Any command that is not listed in the include/uapi/fwctl/xe_pcode.h or not
> + * supported by the running firmware, will return ERR_PTR(-EBADMSG).
> + *
> + * Example:
> + *
> + * .. code-block:: C
> + *
> + * struct fwctl_info_xe_pcode xe_pcode_info;
> + *
> + * struct fwctl_info info = {
> + * .size = sizeof(struct fwctl_info),
> + * .flags = 0,
> + * .out_device_type = 0,
> + * .device_data_len = sizeof(struct fwctl_info_xe_pcode),
> + * .out_device_data = (__aligned_u64) &xe_pcode_info,
> + * };
> + *
> + * fd = open("/dev/fwctl/fwctl0", O_RDWR);
> + * if (fd < 0) {
> + * perror("Failed to open /dev/fwctl/fwctl0");
> + * return -1;
> + * }
> + *
> + * if (ioctl(fd, FWCTL_INFO, &info)) {
> + * perror("ioctl(FWCTL_INFO) failed");
> + * close(fd);
> + * return -1;
> + * }
> + *
> + * if (xe_pcode_info.uctx_caps & FWCTL_XE_PCODE_LATEBINDING) {
> + * struct fwctl_rpc_xe_pcode rpc_in = {
> + * .command = PCODE_CMD_LATE_BINDING,
> + * .param1 = PARAM1_GET_CAPABILITY_STATUS,
> + * };
> + *
> + * struct fwctl_rpc_xe_pcode rpc_out = {0};
> + *
> + * struct fwctl_rpc rpc = {
> + * .size = sizeof(struct fwctl_rpc),
> + * .scope = FWCTL_RPC_CONFIGURATION,
> + * .in_len = sizeof(struct fwctl_rpc_xe_pcode),
> + * .out_len = sizeof(struct fwctl_rpc_xe_pcode),
> + * .in = (__aligned_u64) &rpc_in,
> + * .out = (__aligned_u64) &rpc_out,
> + * };
> + *
> + * if (ioctl(fd, FWCTL_RPC, &rpc)) {
> + * perror("ioctl(FWCTL_RPC) failed");
> + * close(fd);
> + * return -1;
> + * }
> + *
> + */
> +
> +struct xe_pcode_fwctl_dev {
> + struct fwctl_device fwctl;
> + struct xe_device *xe;
> +};
> +
> +DEFINE_FREE(xe_pcode_fwctl, struct xe_pcode_fwctl_dev *, if (_T) fwctl_put(&_T->fwctl))
> +
> +static int xe_pcode_fwctl_uctx_open(struct fwctl_uctx *uctx)
> +{
> + return 0;
> +}
> +
> +static void xe_pcode_fwctl_uctx_close(struct fwctl_uctx *uctx)
> +{
> +}
> +
> +static void *xe_pcode_fwctl_info(struct fwctl_uctx *uctx, size_t *length)
> +{
> + struct xe_pcode_fwctl_dev *fwctl_dev =
> + container_of(uctx->fwctl, struct xe_pcode_fwctl_dev, fwctl);
> + struct xe_device *xe = fwctl_dev->xe;
> + struct fwctl_info_xe_pcode *info;
> +
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return ERR_PTR(-ENOMEM);
> +
> + if (xe->info.platform == XE_BATTLEMAGE)
> + info->uctx_caps = FWCTL_XE_PCODE_LATEBINDING;
> +
> + *length = sizeof(*info);
> +
> + return info;
> +}
> +
> +static bool xe_pcode_fwctl_rpc_validate(struct fwctl_rpc_xe_pcode *rpc,
> + enum fwctl_rpc_scope scope)
> +{
> + u32 mbox = PCODE_MBOX(rpc->command, rpc->param1, rpc->param2);
> +
> + if (mbox == PCODE_MBOX(PCODE_CMD_LATE_BINDING,
> + PARAM1_GET_CAPABILITY_STATUS, 0))
> + return scope == FWCTL_RPC_CONFIGURATION;
> +
> + if (mbox == PCODE_MBOX(PCODE_CMD_LATE_BINDING,
> + PARAM1_GET_VERSION_LOW, 0))
> + return (rpc->data0 == DATA0_TYPE_FAN_CONTROLLER ||
> + rpc->data0 == DATA0_TYPE_VOLTAGE_REGULATOR) &&
> + scope == FWCTL_RPC_CONFIGURATION;
> +
> + return false;
> +}
> +
> +static void *xe_pcode_fwctl_rpc(struct fwctl_uctx *uctx,
> + enum fwctl_rpc_scope scope,
> + void *in, size_t in_len, size_t *out_len)
> +{
> + struct xe_pcode_fwctl_dev *fwctl_dev =
> + container_of(uctx->fwctl, struct xe_pcode_fwctl_dev, fwctl);
> + struct xe_tile *root_tile = xe_device_get_root_tile(fwctl_dev->xe);
> + struct fwctl_rpc_xe_pcode *rpc = in;
> + int err;
> +
> + if (in_len != sizeof(struct fwctl_rpc_xe_pcode) ||
> + *out_len != sizeof(struct fwctl_rpc_xe_pcode))
> + return ERR_PTR(-EMSGSIZE);
> +
> + if (!xe_pcode_fwctl_rpc_validate(rpc, scope))
> + return ERR_PTR(-EPERM);
> +
> + xe_pm_runtime_get(fwctl_dev->xe);
> +
> + err = xe_pcode_read(root_tile, PCODE_MBOX(rpc->command,
> + rpc->param1,
> + rpc->param2),
> + &rpc->data0,
> + &rpc->data1);
> +
> + xe_pm_runtime_put(fwctl_dev->xe);
> +
> + if (err)
> + return ERR_PTR(err);
> +
> + return rpc;
> +}
> +
> +static const struct fwctl_ops xe_pcode_fwctl_ops = {
> + .device_type = FWCTL_DEVICE_TYPE_XE_PCODE,
> + .uctx_size = sizeof(struct fwctl_uctx),
> + .open_uctx = xe_pcode_fwctl_uctx_open,
> + .close_uctx = xe_pcode_fwctl_uctx_close,
> + .info = xe_pcode_fwctl_info,
> + .fw_rpc = xe_pcode_fwctl_rpc,
> +};
> +
> +static void xe_pcode_fwctl_fini(void *dev)
> +{
> + struct fwctl_device *fwctl = dev;
> +
> + fwctl_unregister(fwctl);
> + fwctl_put(fwctl);
> +}
> +
> +int xe_pcode_fwctl_init(struct xe_device *xe)
> +{
> + struct xe_pcode_fwctl_dev *fwctl_dev __free(xe_pcode_fwctl) =
> + fwctl_alloc_device(xe->drm.dev, &xe_pcode_fwctl_ops,
> + struct xe_pcode_fwctl_dev, fwctl);
> + int err;
> +
> + /* For now xe_pcode_fwctl supports only Late-Binding commands on BMG */
> + if (xe->info.platform != XE_BATTLEMAGE)
> + return -ENODEV;
> +
> + if (!fwctl_dev)
> + return -ENOMEM;
> +
> + fwctl_dev->xe = xe;
> +
> + err = fwctl_register(&fwctl_dev->fwctl);
> + if (err)
> + return err;
> +
> + return devm_add_action_or_reset(xe->drm.dev, xe_pcode_fwctl_fini,
> + &fwctl_dev->fwctl);
> +}
> +
> +MODULE_IMPORT_NS("FWCTL");
> diff --git a/drivers/gpu/drm/xe/xe_pcode_fwctl.h b/drivers/gpu/drm/xe/xe_pcode_fwctl.h
> new file mode 100644
> index 000000000000..67386d7bf2ea
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pcode_fwctl.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef _XE_PCODE_FWCTL_H_
> +#define _XE_PCODE_FWCTL_H_
> +
> +struct xe_device;
> +
> +int xe_pcode_fwctl_init(struct xe_device *xe);
> +
> +#endif
> diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
> index 716ac0eee42d..9e7e84aef791 100644
> --- a/include/uapi/fwctl/fwctl.h
> +++ b/include/uapi/fwctl/fwctl.h
> @@ -45,6 +45,7 @@ enum fwctl_device_type {
> FWCTL_DEVICE_TYPE_MLX5 = 1,
> FWCTL_DEVICE_TYPE_CXL = 2,
> FWCTL_DEVICE_TYPE_PDS = 4,
> + FWCTL_DEVICE_TYPE_XE_PCODE = 5,
> };
>
> /**
> diff --git a/include/uapi/fwctl/xe_pcode.h b/include/uapi/fwctl/xe_pcode.h
> new file mode 100644
> index 000000000000..87544e99faa1
> --- /dev/null
> +++ b/include/uapi/fwctl/xe_pcode.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef _UAPI_FWCTL_XE_PCODE_H_
> +#define _UAPI_FWCTL_XE_PCODE_H_
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct fwctl_info_xe_pcode - FWCTL Information struct for Xe PCODE
> + *
> + * @uctx_caps: bitmap of available capabilities:
> + * - %FWCTL_XE_PCODE_LATEBINDING - Command to configure Late Bind FW such as
> + * Fan Controller and Voltage Regulator
> + * @rsvd: Reserved for future usage or flags
> + */
> +struct fwctl_info_xe_pcode {
> + __u32 uctx_caps;
> + __u32 rsvd[3];
> +};
> +
> +#define FWCTL_XE_PCODE_LATEBINDING (1 << 0)
> +
> +/**
> + * struct fwctl_rpc_xe_pcode - FWCTL Remote Procedure Calls for Xe PCODE
> + */
> +struct fwctl_rpc_xe_pcode {
> + /** @command: The main Mailbox command */
> + __u8 command;
> + /** @pad: Padding the uAPI struct - Must be 0. Not sent to firmware */
> + __u8 pad[3];
> + /** @param1: A subcommand or a parameter of the main command */
> + __u16 param1;
> + /** @param2: A parameter of a subcommand or a subsubcommand */
> + __u16 param2;
> + /** @data0: The first 32 bits of data. In general data-in as param */
> + __u32 data0;
> + /** @data1: The other 32 bits of data. In general data-out */
> + __u32 data1;
> + /** @rsvd: Reserved for future use */
> + __u32 rsvd[2];
> +};
> +
> +/**
> + * DOC: Late Binding Commands
> + *
> + * FWCTL info.uctx_caps: FWCTL_XE_PCODE_LATEBINDING
> + * FWCTL rpc.scope: FWCTL_RPC_CONFIGURATION
> + *
> + * Command 0x5C - LATE_BINDING
> + * Param1 0x0 - GET_CAPABILITY_STATUS
> + * Param2 0
> + * Data in None
> + * Data out:
> + *
> + * - Bit0: Late binding for V1 Fan Tables is supported.
> + * - Bit3: Late binding for VR parameters.
> + * - Bit16: Late binding done for V1 Fan tables
> + * - Bit17: Late binding done for power co-efficients.
> + * - Bit18: Late binding done for V2 Fan tables
> + * - Bit19: Late binding done for VR Parameters
> + *
> + * Command 0x5C - LATE_BINDING
> + * Param1 0x1 - GET_VERSION_LOW
> + * Param2 0
> + * Data in - conveys the Type of the Late Binding Configuration:
> + *
> + * - FAN_CONTROLLER = 1
> + * - VOLTAGE_REGULATOR = 2
> + *
> + * Data out - Lower 32 bits of Version Number for Late Binding configuration
> + * that has been applied successfully.
> + */
> +#define PCODE_CMD_LATE_BINDING 0x5C
> +#define PARAM1_GET_CAPABILITY_STATUS 0x0
> +#define PARAM1_GET_VERSION_LOW 0x1
> +#define DATA0_TYPE_FAN_CONTROLLER 1
> +#define DATA0_TYPE_VOLTAGE_REGULATOR 2
> +
> +#endif /* _UAPI_FWCTL_XE_PCODE_H_ */
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI Do not review
2025-06-06 17:56 [PATCH v2 00/10] Introducing firmware late binding Badal Nilawar
` (8 preceding siblings ...)
2025-06-06 17:57 ` [PATCH v2 09/10] {fwctl,drm}/xe/pcode: Introduce xe_pcode_fwctl Badal Nilawar
@ 2025-06-06 17:57 ` Badal Nilawar
2025-06-07 8:02 ` kernel test robot
9 siblings, 1 reply; 28+ messages in thread
From: Badal Nilawar @ 2025-06-06 17:57 UTC (permalink / raw)
To: intel-xe, dri-devel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 27a06d7359c7..2211365827d7 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -45,6 +45,7 @@ config DRM_XE
select WANT_DEV_COREDUMP
select AUXILIARY_BUS
select HMM_MIRROR
+ select INTEL_MEI_LATE_BIND
select FWCTL
help
Experimental driver for Intel Xe series GPUs
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI Do not review
2025-06-06 17:57 ` [PATCH v2 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI Do not review Badal Nilawar
@ 2025-06-07 8:02 ` kernel test robot
0 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2025-06-07 8:02 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel
Cc: Paul Gazzillo, Necip Fazil Yildiran, oe-kbuild-all,
anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Hi Badal,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-xe/drm-xe-next]
[also build test WARNING on next-20250606]
[cannot apply to char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus drm-i915/for-linux-next drm-i915/for-linux-next-fixes linus/master v6.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Badal-Nilawar/mei-bus-add-mei_cldev_mtu-interface/20250607-015525
base: https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next
patch link: https://lore.kernel.org/r/20250606175707.1403384-11-badal.nilawar%40intel.com
patch subject: [PATCH v2 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI Do not review
config: alpha-kismet-CONFIG_INTEL_MEI_LATE_BIND-CONFIG_DRM_XE-0-0 (https://download.01.org/0day-ci/archive/20250607/202506071532.WlVvupfu-lkp@intel.com/config)
reproduce: (https://download.01.org/0day-ci/archive/20250607/202506071532.WlVvupfu-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506071532.WlVvupfu-lkp@intel.com/
kismet warnings: (new ones prefixed by >>)
>> kismet: WARNING: unmet direct dependencies detected for INTEL_MEI_LATE_BIND when selected by DRM_XE
WARNING: unmet direct dependencies detected for INTEL_MEI_LATE_BIND
Depends on [n]: INTEL_MEI [=n] && DRM_XE [=y]
Selected by [y]:
- DRM_XE [=y] && HAS_IOMEM [=y] && DRM [=y] && PCI [=y] && MMU [=y] && (m [=m] && MODULES [=y] || KUNIT [=y]=y [=y]) && (INTEL_VSEC [=n] || !INTEL_VSEC [=n]) && (X86_PLATFORM_DEVICES [=n] || !X86 || !ACPI)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 28+ messages in thread