* [PATCH v3 01/10] mei: bus: add mei_cldev_mtu interface
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
@ 2025-06-18 18:59 ` Badal Nilawar
2025-06-18 18:59 ` [PATCH v3 02/10] mei: late_bind: add late binding component driver Badal Nilawar
` (8 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 18:59 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
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>
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 related [flat|nested] 33+ messages in thread* [PATCH v3 02/10] mei: late_bind: add late binding component driver
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
2025-06-18 18:59 ` [PATCH v3 01/10] mei: bus: add mei_cldev_mtu interface Badal Nilawar
@ 2025-06-18 18:59 ` Badal Nilawar
2025-06-19 7:32 ` Gupta, Anshuman
2025-06-18 19:00 ` [PATCH v3 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Badal Nilawar
` (7 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 18:59 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
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)
v3:
- Updated kconfig description
- Move CSC late binding specific flags/defines to late_bind_mei_interface.h (Daniele)
v4:
- Add match for PCI_CLASS_DISPLAY_OTHER to support headless cards (Anshuman)
---
drivers/misc/mei/Kconfig | 1 +
drivers/misc/mei/Makefile | 1 +
drivers/misc/mei/late_bind/Kconfig | 13 +
drivers/misc/mei/late_bind/Makefile | 9 +
drivers/misc/mei/late_bind/mei_late_bind.c | 264 ++++++++++++++++++++
include/drm/intel/i915_component.h | 1 +
include/drm/intel/late_bind_mei_interface.h | 50 ++++
7 files changed, 339 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..65c7180c5678
--- /dev/null
+++ b/drivers/misc/mei/late_bind/Kconfig
@@ -0,0 +1,13 @@
+# 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 feature,
+ allowing loading of firmware for the devices like Fan
+ Controller during by Intel Xe driver.
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..cb985f32309e
--- /dev/null
+++ b/drivers/misc/mei/late_bind/mei_late_bind.c
@@ -0,0 +1,264 @@
+// 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, "%s status = %d\n", __func__, 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->vendor != PCI_VENDOR_ID_INTEL)
+ return 0;
+
+ if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) ||
+ pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
+ 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..2c53657ce91b
--- /dev/null
+++ b/include/drm/intel/late_bind_mei_interface.h
@@ -0,0 +1,50 @@
+/* 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;
+
+/**
+ * Late Binding flags
+ * Persistent across warm reset
+ */
+#define CSC_LATE_BINDING_FLAGS_IS_PERSISTENT BIT(0)
+
+/**
+ * xe_late_bind_fw_type - enum to determine late binding fw type
+ */
+enum late_bind_type {
+ CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1,
+};
+
+/**
+ * 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] 33+ messages in thread* RE: [PATCH v3 02/10] mei: late_bind: add late binding component driver
2025-06-18 18:59 ` [PATCH v3 02/10] mei: late_bind: add late binding component driver Badal Nilawar
@ 2025-06-19 7:32 ` Gupta, Anshuman
2025-06-19 8:11 ` Jani Nikula
0 siblings, 1 reply; 33+ messages in thread
From: Gupta, Anshuman @ 2025-06-19 7:32 UTC (permalink / raw)
To: Vivi, Rodrigo, Nilawar, Badal, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Jani Nikula, Usyskin, Alexander
Cc: gregkh@linuxfoundation.org, Ceraolo Spurio, Daniele,
jgg@nvidia.com
> -----Original Message-----
> From: Nilawar, Badal <badal.nilawar@intel.com>
> Sent: Thursday, June 19, 2025 12:30 AM
> To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.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 v3 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)
This patch still wrong naming I915_COMPONENT_LATE_BIND.
LATE_BIND will never be supported by i915, it is a wrong prefix.
@Nikula, Jani @Vivi, Rodrigo is it ok use the i915 naming prefix here ?
We can use INTEL_COMPONENT_LATE_BIND here ?
This header include/drm/intel/i915_component.h is used by both XE and i915.
May be a separate series later requires refactoring this header.
> - Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha)
> v3:
> - Updated kconfig description
> - Move CSC late binding specific flags/defines to late_bind_mei_interface.h
> (Daniele)
> v4:
> - Add match for PCI_CLASS_DISPLAY_OTHER to support headless cards
> (Anshuman)
> ---
> drivers/misc/mei/Kconfig | 1 +
> drivers/misc/mei/Makefile | 1 +
> drivers/misc/mei/late_bind/Kconfig | 13 +
> drivers/misc/mei/late_bind/Makefile | 9 +
> drivers/misc/mei/late_bind/mei_late_bind.c | 264 ++++++++++++++++++++
> include/drm/intel/i915_component.h | 1 +
> include/drm/intel/late_bind_mei_interface.h | 50 ++++
> 7 files changed, 339 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..65c7180c5678
> --- /dev/null
> +++ b/drivers/misc/mei/late_bind/Kconfig
> @@ -0,0 +1,13 @@
> +# 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 feature,
> + allowing loading of firmware for the devices like Fan
> + Controller during by Intel Xe driver.
> 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..cb985f32309e
> --- /dev/null
> +++ b/drivers/misc/mei/late_bind/mei_late_bind.c
> @@ -0,0 +1,264 @@
> +// 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
I commented earlier in V2 series as well, is this timeout specific only to LATE BINDING ?
If this is generic timeout for mei_cldev_{send,recv}_timeout(),
then this marco should be part of standard MEI headers not late binding.
Other consumers of mei_cldev_{send,recv}_timeout() send the timeout input by component-ops callback .
@Shahsa could you please explained that.
> +
> +/**
> + * 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;
> + }
Why are we not checking mkhi_msg_hdr hdr->result here ?
> +
> + 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;
> + }
Use Kzalloc here, to make sure reserved filed of header is zeroed.
> +
> + 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, "%s status = %d\n", __func__, ret);
AFAIU It would be useful to add the status enum in late_bind_mei_interface.h.
> +
> +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
Please modify the Kenel Doc comment here, as per the function.
> + * and the parent of requester and the grand parent of mei_if are the same
We are matching against the requester not parent of requester.
Modify the Kernel Doc comment properly.
> + * device.
> + *
> + * @dev: master device
> + * @subcomponent: subcomponent to match (I915_COMPONENT_LATE_BIND)
> + * @data: compare data (mei late-bind bus device)
AFAIK It is mei client device not mei 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->vendor != PCI_VENDOR_ID_INTEL)
> + return 0;
> +
> + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) ||
> + pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
> + return 0;
This condition should be like below, if I am not missing anything.
if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) &&
pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
Thanks,
Anshuman.
> +
> + 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..2c53657ce91b
> --- /dev/null
> +++ b/include/drm/intel/late_bind_mei_interface.h
> @@ -0,0 +1,50 @@
> +/* 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;
> +
> +/**
> + * Late Binding flags
> + * Persistent across warm reset
> + */
> +#define CSC_LATE_BINDING_FLAGS_IS_PERSISTENT BIT(0)
> +
> +/**
> + * xe_late_bind_fw_type - enum to determine late binding fw type */
> +enum late_bind_type {
> + CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1, };
> +
> +/**
> + * 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] 33+ messages in thread* RE: [PATCH v3 02/10] mei: late_bind: add late binding component driver
2025-06-19 7:32 ` Gupta, Anshuman
@ 2025-06-19 8:11 ` Jani Nikula
0 siblings, 0 replies; 33+ messages in thread
From: Jani Nikula @ 2025-06-19 8:11 UTC (permalink / raw)
To: Gupta, Anshuman, Vivi, Rodrigo, Nilawar, Badal,
intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, Usyskin, Alexander
Cc: gregkh@linuxfoundation.org, Ceraolo Spurio, Daniele,
jgg@nvidia.com
On Thu, 19 Jun 2025, "Gupta, Anshuman" <anshuman.gupta@intel.com> wrote:
>> -----Original Message-----
>> From: Nilawar, Badal <badal.nilawar@intel.com>
>> Sent: Thursday, June 19, 2025 12:30 AM
>> To: intel-xe@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
>> kernel@vger.kernel.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 v3 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)
> This patch still wrong naming I915_COMPONENT_LATE_BIND.
> LATE_BIND will never be supported by i915, it is a wrong prefix.
> @Nikula, Jani @Vivi, Rodrigo is it ok use the i915 naming prefix here ?
> We can use INTEL_COMPONENT_LATE_BIND here ?
>
> This header include/drm/intel/i915_component.h is used by both XE and i915.
> May be a separate series later requires refactoring this header.
Yeah the goal is that everything under include/drm/intel would be
independent of xe and i915, both in naming and implementation.
BR,
Jani.
>
>
>> - Drop xe_late_bind_component struct to move to xe code (Daniele/Sasha)
>> v3:
>> - Updated kconfig description
>> - Move CSC late binding specific flags/defines to late_bind_mei_interface.h
>> (Daniele)
>> v4:
>> - Add match for PCI_CLASS_DISPLAY_OTHER to support headless cards
>> (Anshuman)
>> ---
>> drivers/misc/mei/Kconfig | 1 +
>> drivers/misc/mei/Makefile | 1 +
>> drivers/misc/mei/late_bind/Kconfig | 13 +
>> drivers/misc/mei/late_bind/Makefile | 9 +
>> drivers/misc/mei/late_bind/mei_late_bind.c | 264 ++++++++++++++++++++
>> include/drm/intel/i915_component.h | 1 +
>> include/drm/intel/late_bind_mei_interface.h | 50 ++++
>> 7 files changed, 339 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..65c7180c5678
>> --- /dev/null
>> +++ b/drivers/misc/mei/late_bind/Kconfig
>> @@ -0,0 +1,13 @@
>> +# 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 feature,
>> + allowing loading of firmware for the devices like Fan
>> + Controller during by Intel Xe driver.
>> 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..cb985f32309e
>> --- /dev/null
>> +++ b/drivers/misc/mei/late_bind/mei_late_bind.c
>> @@ -0,0 +1,264 @@
>> +// 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
> I commented earlier in V2 series as well, is this timeout specific only to LATE BINDING ?
> If this is generic timeout for mei_cldev_{send,recv}_timeout(),
> then this marco should be part of standard MEI headers not late binding.
> Other consumers of mei_cldev_{send,recv}_timeout() send the timeout input by component-ops callback .
>
> @Shahsa could you please explained that.
>> +
>> +/**
>> + * 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;
>> + }
> Why are we not checking mkhi_msg_hdr hdr->result here ?
>> +
>> + 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;
>> + }
> Use Kzalloc here, to make sure reserved filed of header is zeroed.
>> +
>> + 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, "%s status = %d\n", __func__, ret);
> AFAIU It would be useful to add the status enum in late_bind_mei_interface.h.
>> +
>> +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
> Please modify the Kenel Doc comment here, as per the function.
>> + * and the parent of requester and the grand parent of mei_if are the same
> We are matching against the requester not parent of requester.
> Modify the Kernel Doc comment properly.
>> + * device.
>> + *
>> + * @dev: master device
>> + * @subcomponent: subcomponent to match (I915_COMPONENT_LATE_BIND)
>> + * @data: compare data (mei late-bind bus device)
> AFAIK It is mei client device not mei 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->vendor != PCI_VENDOR_ID_INTEL)
>> + return 0;
>> +
>> + if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) ||
>> + pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
>> + return 0;
> This condition should be like below, if I am not missing anything.
> if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) &&
> pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
>
> Thanks,
> Anshuman.
>> +
>> + 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..2c53657ce91b
>> --- /dev/null
>> +++ b/include/drm/intel/late_bind_mei_interface.h
>> @@ -0,0 +1,50 @@
>> +/* 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;
>> +
>> +/**
>> + * Late Binding flags
>> + * Persistent across warm reset
>> + */
>> +#define CSC_LATE_BINDING_FLAGS_IS_PERSISTENT BIT(0)
>> +
>> +/**
>> + * xe_late_bind_fw_type - enum to determine late binding fw type */
>> +enum late_bind_type {
>> + CSC_LATE_BINDING_TYPE_FAN_CONTROL = 1, };
>> +
>> +/**
>> + * 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
>
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
2025-06-18 18:59 ` [PATCH v3 01/10] mei: bus: add mei_cldev_mtu interface Badal Nilawar
2025-06-18 18:59 ` [PATCH v3 02/10] mei: late_bind: add late binding component driver Badal Nilawar
@ 2025-06-18 19:00 ` Badal Nilawar
2025-06-18 20:16 ` Daniele Ceraolo Spurio
2025-06-18 19:00 ` [PATCH v3 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware Badal Nilawar
` (6 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 19:00 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
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 mei late binding component to enable firmware loading.
v2:
- Add devm_add_action_or_reset to remove the component (Daniele)
- Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
v3:
- Fail driver probe if late bind initialization fails,
add has_late_bind flag (Daniele)
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device.c | 5 ++
drivers/gpu/drm/xe/xe_device_types.h | 6 ++
drivers/gpu/drm/xe/xe_late_bind_fw.c | 93 ++++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw.h | 15 ++++
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 39 +++++++++
drivers/gpu/drm/xe/xe_pci.c | 3 +
7 files changed, 162 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 f5f5775acdc0..001384ca7357 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 7d9a31868ea9..13cf5f90d09e 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"
@@ -889,6 +890,10 @@ int xe_device_probe(struct xe_device *xe)
if (err)
return err;
+ err = xe_late_bind_init(&xe->late_bind);
+ if (err && err != -ENODEV)
+ return err;
+
err = xe_oa_init(xe);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index a504b8ea6f3f..facafe2eea5c 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"
@@ -323,6 +324,8 @@ struct xe_device {
u8 has_heci_cscfi:1;
/** @info.has_heci_gscfi: device has heci gscfi */
u8 has_heci_gscfi:1;
+ /** @info.has_late_bind: Device has firmware late binding support */
+ u8 has_late_bind:1;
/** @info.has_llc: Device has a shared CPU+GPU last level cache */
u8 has_llc:1;
/** @info.has_mbx_power_limits: Device has support to manage power limits using
@@ -552,6 +555,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..52cb295b7df6
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -0,0 +1,93 @@
+// 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);
+
+ 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.has_late_bind)
+ 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..ef0a9723bee4
--- /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_LATE_BIND_TYPES_H_
+#define _XE_LATE_BIND_TYPES_H_
+
+#include <linux/iosys-map.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+/**
+ * struct xe_late_bind_component - Late Binding services component
+ * @mei_dev: device that provide Late Binding service.
+ * @ops: Ops implemented by Late Binding driver, used by Xe driver.
+ *
+ * Communication between Xe and MEI drivers for Late Binding services
+ */
+struct xe_late_bind_component {
+ /** @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
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 89814b32e585..d54d1d84e240 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -65,6 +65,7 @@ struct xe_device_desc {
u8 has_fan_control:1;
u8 has_heci_gscfi:1;
u8 has_heci_cscfi:1;
+ u8 has_late_bind:1;
u8 has_llc:1;
u8 has_mbx_power_limits:1;
u8 has_pxp:1;
@@ -348,6 +349,7 @@ static const struct xe_device_desc bmg_desc = {
.has_fan_control = true,
.has_mbx_power_limits = true,
.has_heci_cscfi = 1,
+ .has_late_bind = true,
.needs_scratch = true,
};
@@ -592,6 +594,7 @@ static int xe_info_init_early(struct xe_device *xe,
xe->info.has_mbx_power_limits = desc->has_mbx_power_limits;
xe->info.has_heci_gscfi = desc->has_heci_gscfi;
xe->info.has_heci_cscfi = desc->has_heci_cscfi;
+ xe->info.has_late_bind = desc->has_late_bind;
xe->info.has_llc = desc->has_llc;
xe->info.has_pxp = desc->has_pxp;
xe->info.has_sriov = desc->has_sriov;
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
2025-06-18 19:00 ` [PATCH v3 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Badal Nilawar
@ 2025-06-18 20:16 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-18 20:16 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/18/2025 12:00 PM, 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 mei late binding component to enable firmware loading.
>
> v2:
> - Add devm_add_action_or_reset to remove the component (Daniele)
> - Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
> v3:
> - Fail driver probe if late bind initialization fails,
> add has_late_bind flag (Daniele)
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device.c | 5 ++
> drivers/gpu/drm/xe/xe_device_types.h | 6 ++
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 93 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_late_bind_fw.h | 15 ++++
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 39 +++++++++
> drivers/gpu/drm/xe/xe_pci.c | 3 +
> 7 files changed, 162 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 f5f5775acdc0..001384ca7357 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 7d9a31868ea9..13cf5f90d09e 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"
> @@ -889,6 +890,10 @@ int xe_device_probe(struct xe_device *xe)
> if (err)
> return err;
>
> + err = xe_late_bind_init(&xe->late_bind);
> + if (err && err != -ENODEV)
> + return err;
> +
> err = xe_oa_init(xe);
> if (err)
> return err;
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index a504b8ea6f3f..facafe2eea5c 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"
> @@ -323,6 +324,8 @@ struct xe_device {
> u8 has_heci_cscfi:1;
> /** @info.has_heci_gscfi: device has heci gscfi */
> u8 has_heci_gscfi:1;
> + /** @info.has_late_bind: Device has firmware late binding support */
> + u8 has_late_bind:1;
> /** @info.has_llc: Device has a shared CPU+GPU last level cache */
> u8 has_llc:1;
> /** @info.has_mbx_power_limits: Device has support to manage power limits using
> @@ -552,6 +555,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..52cb295b7df6
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -0,0 +1,93 @@
> +// 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);
> +
> + 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.has_late_bind)
> + 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;
late_bind->component_added is unused in this patch, so it might be worth
moving its addition to the patch that actually makes use of it.
With that:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Daniele
> +
> + 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..ef0a9723bee4
> --- /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_LATE_BIND_TYPES_H_
> +#define _XE_LATE_BIND_TYPES_H_
> +
> +#include <linux/iosys-map.h>
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +
> +/**
> + * struct xe_late_bind_component - Late Binding services component
> + * @mei_dev: device that provide Late Binding service.
> + * @ops: Ops implemented by Late Binding driver, used by Xe driver.
> + *
> + * Communication between Xe and MEI drivers for Late Binding services
> + */
> +struct xe_late_bind_component {
> + /** @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
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 89814b32e585..d54d1d84e240 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -65,6 +65,7 @@ struct xe_device_desc {
> u8 has_fan_control:1;
> u8 has_heci_gscfi:1;
> u8 has_heci_cscfi:1;
> + u8 has_late_bind:1;
> u8 has_llc:1;
> u8 has_mbx_power_limits:1;
> u8 has_pxp:1;
> @@ -348,6 +349,7 @@ static const struct xe_device_desc bmg_desc = {
> .has_fan_control = true,
> .has_mbx_power_limits = true,
> .has_heci_cscfi = 1,
> + .has_late_bind = true,
> .needs_scratch = true,
> };
>
> @@ -592,6 +594,7 @@ static int xe_info_init_early(struct xe_device *xe,
> xe->info.has_mbx_power_limits = desc->has_mbx_power_limits;
> xe->info.has_heci_gscfi = desc->has_heci_gscfi;
> xe->info.has_heci_cscfi = desc->has_heci_cscfi;
> + xe->info.has_late_bind = desc->has_late_bind;
> xe->info.has_llc = desc->has_llc;
> xe->info.has_pxp = desc->has_pxp;
> xe->info.has_sriov = desc->has_sriov;
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
` (2 preceding siblings ...)
2025-06-18 19:00 ` [PATCH v3 03/10] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Badal Nilawar
@ 2025-06-18 19:00 ` Badal Nilawar
2025-06-18 20:46 ` Daniele Ceraolo Spurio
2025-06-18 19:00 ` [PATCH v3 05/10] drm/xe/xe_late_bind_fw: Load " Badal Nilawar
` (5 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 19:00 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
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 (Daniele):
- drm_err if firmware size is more than max pay load size
- s/request_firmware/firmware_request_nowarn/ as firmware will
not be available for all possible cards
v3 (Daniele):
- init firmware from within xe_late_bind_init, propagate error
- switch late_bind_fw to array to handle multiple firmware types
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 97 +++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 32 +++++++
drivers/misc/mei/late_bind/mei_late_bind.c | 1 -
3 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 52cb295b7df6..d416d6cc1fa2 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,6 +14,16 @@
#include "xe_device.h"
#include "xe_late_bind_fw.h"
+#include "xe_pcode.h"
+#include "xe_pcode_api.h"
+
+static const u32 fw_id_to_type[] = {
+ [FAN_CONTROL_FW_ID] = CSC_LATE_BINDING_TYPE_FAN_CONTROL,
+ };
+
+static const char * const fw_id_to_name[] = {
+ [FAN_CONTROL_FW_ID] = "fan_control",
+ };
static struct xe_device *
late_bind_to_xe(struct xe_late_bind *late_bind)
@@ -20,6 +31,86 @@ 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_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 __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
+{
+ 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 (fw_id >= MAX_FW_ID)
+ return -EINVAL;
+
+ lb_fw = &late_bind->late_bind_fw[fw_id];
+
+ lb_fw->valid = false;
+ lb_fw->id = fw_id;
+ lb_fw->type = fw_id_to_type[lb_fw->id];
+ lb_fw->flags &= ~CSC_LATE_BINDING_FLAGS_IS_PERSISTENT;
+
+ if (lb_fw->type == CSC_LATE_BINDING_TYPE_FAN_CONTROL) {
+ num_fans = xe_late_bind_fw_num_fans(late_bind);
+ drm_dbg(&xe->drm, "Number of Fans: %d\n", num_fans);
+ if (!num_fans)
+ return 0;
+ }
+
+ snprintf(lb_fw->blob_path, sizeof(lb_fw->blob_path), "xe/%s_8086_%04x_%04x_%04x.bin",
+ fw_id_to_name[lb_fw->id], 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, "%s late binding fw not available for current device",
+ fw_id_to_name[lb_fw->id]);
+ return 0;
+ }
+
+ if (fw->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);
+ release_firmware(fw);
+ return -ENODATA;
+ }
+
+ 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;
+}
+
+static int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
+{
+ int ret;
+ int fw_id;
+
+ for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
+ ret = __xe_late_bind_fw_init(late_bind, fw_id);
+ if (ret)
+ return ret;
+ }
+ return ret;
+}
+
static int xe_late_bind_component_bind(struct device *xe_kdev,
struct device *mei_kdev, void *data)
{
@@ -89,5 +180,9 @@ int xe_late_bind_init(struct xe_late_bind *late_bind)
late_bind->component_added = true;
- return devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
+ err = devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
+ if (err)
+ return err;
+
+ return xe_late_bind_fw_init(late_bind);
}
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 ef0a9723bee4..c6fd33fd5484 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,36 @@
#include <linux/mutex.h>
#include <linux/types.h>
+#define MAX_PAYLOAD_SIZE (1024 * 4)
+
+/**
+ * xe_late_bind_fw_id - enum to determine late binding fw index
+ */
+enum xe_late_bind_fw_id {
+ FAN_CONTROL_FW_ID = 0,
+ MAX_FW_ID
+};
+
+/**
+ * struct xe_late_bind_fw
+ */
+struct xe_late_bind_fw {
+ /** @late_bind_fw.valid: to check if fw is valid */
+ bool valid;
+ /** @late_bind_fw.id: firmware index */
+ u32 id;
+ /** @late_bind_fw.blob_path: firmware binary path */
+ char blob_path[PATH_MAX];
+ /** @late_bind_fw.type: firmware type */
+ u32 type;
+ /** @late_bind_fw.flags: firmware 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 +64,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 array */
+ struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
};
#endif
diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c b/drivers/misc/mei/late_bind/mei_late_bind.c
index cb985f32309e..6ea64c44bb94 100644
--- a/drivers/misc/mei/late_bind/mei_late_bind.c
+++ b/drivers/misc/mei/late_bind/mei_late_bind.c
@@ -2,7 +2,6 @@
/*
* 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>
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
2025-06-18 19:00 ` [PATCH v3 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware Badal Nilawar
@ 2025-06-18 20:46 ` Daniele Ceraolo Spurio
2025-06-19 4:57 ` Nilawar, Badal
0 siblings, 1 reply; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-18 20:46 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/18/2025 12:00 PM, Badal Nilawar wrote:
> Search for late binding firmware binaries and populate the meta data of
> firmware structures.
>
> v2 (Daniele):
> - drm_err if firmware size is more than max pay load size
> - s/request_firmware/firmware_request_nowarn/ as firmware will
> not be available for all possible cards
> v3 (Daniele):
> - init firmware from within xe_late_bind_init, propagate error
> - switch late_bind_fw to array to handle multiple firmware types
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 97 +++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 32 +++++++
> drivers/misc/mei/late_bind/mei_late_bind.c | 1 -
> 3 files changed, 128 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> index 52cb295b7df6..d416d6cc1fa2 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,6 +14,16 @@
>
> #include "xe_device.h"
> #include "xe_late_bind_fw.h"
> +#include "xe_pcode.h"
> +#include "xe_pcode_api.h"
> +
> +static const u32 fw_id_to_type[] = {
> + [FAN_CONTROL_FW_ID] = CSC_LATE_BINDING_TYPE_FAN_CONTROL,
> + };
> +
> +static const char * const fw_id_to_name[] = {
> + [FAN_CONTROL_FW_ID] = "fan_control",
> + };
>
> static struct xe_device *
> late_bind_to_xe(struct xe_late_bind *late_bind)
> @@ -20,6 +31,86 @@ 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_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 __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
> +{
> + 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 (fw_id >= MAX_FW_ID)
> + return -EINVAL;
> +
> + lb_fw = &late_bind->late_bind_fw[fw_id];
> +
> + lb_fw->valid = false;
> + lb_fw->id = fw_id;
> + lb_fw->type = fw_id_to_type[lb_fw->id];
> + lb_fw->flags &= ~CSC_LATE_BINDING_FLAGS_IS_PERSISTENT;
nit: lb_fw->flags should already be zero here, so no need to make sure
that flag is not set. Also, that flag is ignored on BMG, so there is no
need to make sure it is not set anyway.
> +
> + if (lb_fw->type == CSC_LATE_BINDING_TYPE_FAN_CONTROL) {
> + num_fans = xe_late_bind_fw_num_fans(late_bind);
> + drm_dbg(&xe->drm, "Number of Fans: %d\n", num_fans);
> + if (!num_fans)
> + return 0;
> + }
> +
> + snprintf(lb_fw->blob_path, sizeof(lb_fw->blob_path), "xe/%s_8086_%04x_%04x_%04x.bin",
> + fw_id_to_name[lb_fw->id], 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, "%s late binding fw not available for current device",
> + fw_id_to_name[lb_fw->id]);
> + return 0;
> + }
> +
> + if (fw->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);
> + release_firmware(fw);
> + return -ENODATA;
> + }
> +
> + 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;
> +}
> +
> +static int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
> +{
> + int ret;
> + int fw_id;
> +
> + for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
> + ret = __xe_late_bind_fw_init(late_bind, fw_id);
> + if (ret)
> + return ret;
> + }
> + return ret;
nit: this could be a return 0, since if ret != 0 we return earlier
> +}
> +
> static int xe_late_bind_component_bind(struct device *xe_kdev,
> struct device *mei_kdev, void *data)
> {
> @@ -89,5 +180,9 @@ int xe_late_bind_init(struct xe_late_bind *late_bind)
>
> late_bind->component_added = true;
>
> - return devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
> + err = devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
> + if (err)
> + return err;
> +
> + return xe_late_bind_fw_init(late_bind);
> }
> 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 ef0a9723bee4..c6fd33fd5484 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,36 @@
> #include <linux/mutex.h>
> #include <linux/types.h>
>
> +#define MAX_PAYLOAD_SIZE (1024 * 4)
nit: could use SZ_4K instead of 1024 * 4
> +
> +/**
> + * xe_late_bind_fw_id - enum to determine late binding fw index
> + */
> +enum xe_late_bind_fw_id {
> + FAN_CONTROL_FW_ID = 0,
> + MAX_FW_ID
nit: Maybe use a less generic name here, to avoid clashes? something like:
enum xe_late_bind_fw_id {
XE_LB_FW_FAN_CONTROL = 0,
XE_LB_FW_MAX_ID
}
> +};
> +
> +/**
> + * struct xe_late_bind_fw
> + */
> +struct xe_late_bind_fw {
> + /** @late_bind_fw.valid: to check if fw is valid */
> + bool valid;
> + /** @late_bind_fw.id: firmware index */
> + u32 id;
> + /** @late_bind_fw.blob_path: firmware binary path */
> + char blob_path[PATH_MAX];
> + /** @late_bind_fw.type: firmware type */
> + u32 type;
> + /** @late_bind_fw.flags: firmware flags */
> + u32 flags;
> + /** @late_bind_fw.payload: to store the late binding blob */
> + u8 payload[MAX_PAYLOAD_SIZE];
Sorry for the late comment on this, just realized that this is a 4K
allocation, should we alloc this dynamically only if we need it?
> + /** @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 +64,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 array */
> + struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
> };
>
> #endif
> diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c b/drivers/misc/mei/late_bind/mei_late_bind.c
> index cb985f32309e..6ea64c44bb94 100644
> --- a/drivers/misc/mei/late_bind/mei_late_bind.c
> +++ b/drivers/misc/mei/late_bind/mei_late_bind.c
> @@ -2,7 +2,6 @@
> /*
> * Copyright (C) 2025 Intel Corporation
> */
> -#include <drm/drm_connector.h>
This change shouldn't be in this patch. If this header is not needed
just modify the patch that adds it to not do so.
All nits are non blocking.
Daniele
> #include <drm/intel/i915_component.h>
> #include <drm/intel/late_bind_mei_interface.h>
> #include <linux/component.h>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
2025-06-18 20:46 ` Daniele Ceraolo Spurio
@ 2025-06-19 4:57 ` Nilawar, Badal
0 siblings, 0 replies; 33+ messages in thread
From: Nilawar, Badal @ 2025-06-19 4:57 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 19-06-2025 02:16, Daniele Ceraolo Spurio wrote:
>
>
> On 6/18/2025 12:00 PM, Badal Nilawar wrote:
>> Search for late binding firmware binaries and populate the meta data of
>> firmware structures.
>>
>> v2 (Daniele):
>> - drm_err if firmware size is more than max pay load size
>> - s/request_firmware/firmware_request_nowarn/ as firmware will
>> not be available for all possible cards
>> v3 (Daniele):
>> - init firmware from within xe_late_bind_init, propagate error
>> - switch late_bind_fw to array to handle multiple firmware types
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 97 +++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 32 +++++++
>> drivers/misc/mei/late_bind/mei_late_bind.c | 1 -
>> 3 files changed, 128 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> index 52cb295b7df6..d416d6cc1fa2 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,6 +14,16 @@
>> #include "xe_device.h"
>> #include "xe_late_bind_fw.h"
>> +#include "xe_pcode.h"
>> +#include "xe_pcode_api.h"
>> +
>> +static const u32 fw_id_to_type[] = {
>> + [FAN_CONTROL_FW_ID] = CSC_LATE_BINDING_TYPE_FAN_CONTROL,
>> + };
>> +
>> +static const char * const fw_id_to_name[] = {
>> + [FAN_CONTROL_FW_ID] = "fan_control",
>> + };
>> static struct xe_device *
>> late_bind_to_xe(struct xe_late_bind *late_bind)
>> @@ -20,6 +31,86 @@ 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_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 __xe_late_bind_fw_init(struct xe_late_bind *late_bind,
>> u32 fw_id)
>> +{
>> + 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 (fw_id >= MAX_FW_ID)
>> + return -EINVAL;
>> +
>> + lb_fw = &late_bind->late_bind_fw[fw_id];
>> +
>> + lb_fw->valid = false;
>> + lb_fw->id = fw_id;
>> + lb_fw->type = fw_id_to_type[lb_fw->id];
>> + lb_fw->flags &= ~CSC_LATE_BINDING_FLAGS_IS_PERSISTENT;
>
> nit: lb_fw->flags should already be zero here, so no need to make sure
> that flag is not set. Also, that flag is ignored on BMG, so there is
> no need to make sure it is not set anyway.
If I discard this, I will need to remove the definition of
CSC_LATE_BINDING_FLAGS_IS_PERSISTENT. I kept this for future use. I
would prefer this way.
I will fix rest of the nits and comments in next rev.
Thanks,
Badal
>> +
>> + if (lb_fw->type == CSC_LATE_BINDING_TYPE_FAN_CONTROL) {
>> + num_fans = xe_late_bind_fw_num_fans(late_bind);
>> + drm_dbg(&xe->drm, "Number of Fans: %d\n", num_fans);
>> + if (!num_fans)
>> + return 0;
>> + }
>> +
>> + snprintf(lb_fw->blob_path, sizeof(lb_fw->blob_path),
>> "xe/%s_8086_%04x_%04x_%04x.bin",
>> + fw_id_to_name[lb_fw->id], 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, "%s late binding fw not available for
>> current device",
>> + fw_id_to_name[lb_fw->id]);
>> + return 0;
>> + }
>> +
>> + if (fw->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);
>> + release_firmware(fw);
>> + return -ENODATA;
>> + }
>> +
>> + 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;
>> +}
>> +
>> +static int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
>> +{
>> + int ret;
>> + int fw_id;
>> +
>> + for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
>> + ret = __xe_late_bind_fw_init(late_bind, fw_id);
>> + if (ret)
>> + return ret;
>> + }
>> + return ret;
>
> nit: this could be a return 0, since if ret != 0 we return earlier
>
>> +}
>> +
>> static int xe_late_bind_component_bind(struct device *xe_kdev,
>> struct device *mei_kdev, void *data)
>> {
>> @@ -89,5 +180,9 @@ int xe_late_bind_init(struct xe_late_bind *late_bind)
>> late_bind->component_added = true;
>> - return devm_add_action_or_reset(xe->drm.dev,
>> xe_late_bind_remove, late_bind);
>> + err = devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove,
>> late_bind);
>> + if (err)
>> + return err;
>> +
>> + return xe_late_bind_fw_init(late_bind);
>> }
>> 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 ef0a9723bee4..c6fd33fd5484 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,36 @@
>> #include <linux/mutex.h>
>> #include <linux/types.h>
>> +#define MAX_PAYLOAD_SIZE (1024 * 4)
>
> nit: could use SZ_4K instead of 1024 * 4
>
>> +
>> +/**
>> + * xe_late_bind_fw_id - enum to determine late binding fw index
>> + */
>> +enum xe_late_bind_fw_id {
>> + FAN_CONTROL_FW_ID = 0,
>> + MAX_FW_ID
>
> nit: Maybe use a less generic name here, to avoid clashes? something
> like:
>
> enum xe_late_bind_fw_id {
> XE_LB_FW_FAN_CONTROL = 0,
> XE_LB_FW_MAX_ID
> }
>
>> +};
>> +
>> +/**
>> + * struct xe_late_bind_fw
>> + */
>> +struct xe_late_bind_fw {
>> + /** @late_bind_fw.valid: to check if fw is valid */
>> + bool valid;
>> + /** @late_bind_fw.id: firmware index */
>> + u32 id;
>> + /** @late_bind_fw.blob_path: firmware binary path */
>> + char blob_path[PATH_MAX];
>> + /** @late_bind_fw.type: firmware type */
>> + u32 type;
>> + /** @late_bind_fw.flags: firmware flags */
>> + u32 flags;
>> + /** @late_bind_fw.payload: to store the late binding blob */
>> + u8 payload[MAX_PAYLOAD_SIZE];
>
> Sorry for the late comment on this, just realized that this is a 4K
> allocation, should we alloc this dynamically only if we need it?
>
>> + /** @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 +64,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 array */
>> + struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
>> };
>> #endif
>> diff --git a/drivers/misc/mei/late_bind/mei_late_bind.c
>> b/drivers/misc/mei/late_bind/mei_late_bind.c
>> index cb985f32309e..6ea64c44bb94 100644
>> --- a/drivers/misc/mei/late_bind/mei_late_bind.c
>> +++ b/drivers/misc/mei/late_bind/mei_late_bind.c
>> @@ -2,7 +2,6 @@
>> /*
>> * Copyright (C) 2025 Intel Corporation
>> */
>> -#include <drm/drm_connector.h>
>
> This change shouldn't be in this patch. If this header is not needed
> just modify the patch that adds it to not do so.
>
> All nits are non blocking.
> Daniele
>
>> #include <drm/intel/i915_component.h>
>> #include <drm/intel/late_bind_mei_interface.h>
>> #include <linux/component.h>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
` (3 preceding siblings ...)
2025-06-18 19:00 ` [PATCH v3 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware Badal Nilawar
@ 2025-06-18 19:00 ` Badal Nilawar
2025-06-18 20:57 ` Daniele Ceraolo Spurio
2025-06-18 19:00 ` [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume Badal Nilawar
` (4 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 19:00 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
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)
v3:
- Use retry interval of 6s, in steps of 200ms, to allow
other OS components release MEI CL handle (Sasha)
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 113 ++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
3 files changed, 118 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 d416d6cc1fa2..54aa08c6bdfd 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -16,6 +16,20 @@
#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
+
+/*
+ * Retry interval set to 6 seconds, in steps of 200 ms, to allow time for
+ * other OS components to release the MEI CL handle
+ */
+#define LB_FW_LOAD_RETRY_MAXCOUNT 30
+#define LB_FW_LOAD_RETRY_PAUSE_MS 200
static const u32 fw_id_to_type[] = {
[FAN_CONTROL_FW_ID] = CSC_LATE_BINDING_TYPE_FAN_CONTROL,
@@ -44,6 +58,85 @@ static int xe_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;
+ int fw_id;
+
+ for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
+ lbfw = &late_bind->late_bind_fw[fw_id];
+ if (lbfw->valid && late_bind->wq) {
+ drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
+ fw_id_to_name[lbfw->id]);
+ 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[lbfw->id]);
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
+ int ret;
+ int slept;
+
+ /* 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_id_to_name[lbfw->id]);
+
+ 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_id_to_name[lbfw->id], ret);
+ else
+ drm_dbg(&xe->drm, "Load %s firmware successful\n",
+ fw_id_to_name[lbfw->id]);
+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_late_bind_fw *lbfw;
+ int fw_id;
+
+ if (!late_bind->component_added)
+ return -EINVAL;
+
+ for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
+ lbfw = &late_bind->late_bind_fw[fw_id];
+ if (lbfw->valid)
+ queue_work(late_bind->wq, &lbfw->work);
+ }
+ return 0;
+}
+
static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
{
struct xe_device *xe = late_bind_to_xe(late_bind);
@@ -93,6 +186,7 @@ static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
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;
@@ -103,11 +197,16 @@ static int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
int ret;
int fw_id;
+ late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
+ if (!late_bind->wq)
+ return -ENOMEM;
+
for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
ret = __xe_late_bind_fw_init(late_bind, fw_id);
if (ret)
return ret;
}
+
return ret;
}
@@ -131,6 +230,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);
@@ -146,8 +247,14 @@ 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);
+ 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);
}
@@ -184,5 +291,9 @@ int xe_late_bind_init(struct xe_late_bind *late_bind)
if (err)
return err;
- return xe_late_bind_fw_init(late_bind);
+ err = xe_late_bind_fw_init(late_bind);
+ if (err)
+ return err;
+
+ return xe_late_bind_fw_load(late_bind);
}
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
index 4c73571c3e62..28d56ed2bfdc 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_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 c6fd33fd5484..d256f53d59e6 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)
@@ -38,6 +39,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;
};
/**
@@ -66,6 +69,8 @@ struct xe_late_bind {
struct mutex mutex;
/** @late_bind.late_bind_fw: late binding firmware array */
struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
+ /** @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] 33+ messages in thread* Re: [PATCH v3 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-18 19:00 ` [PATCH v3 05/10] drm/xe/xe_late_bind_fw: Load " Badal Nilawar
@ 2025-06-18 20:57 ` Daniele Ceraolo Spurio
2025-06-19 5:54 ` Nilawar, Badal
0 siblings, 1 reply; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-18 20:57 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/18/2025 12:00 PM, Badal Nilawar wrote:
> Load late binding firmware
>
> v2:
> - s/EAGAIN/EBUSY/
> - Flush worker in suspend and driver unload (Daniele)
> v3:
> - Use retry interval of 6s, in steps of 200ms, to allow
> other OS components release MEI CL handle (Sasha)
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 113 ++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
> 3 files changed, 118 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 d416d6cc1fa2..54aa08c6bdfd 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -16,6 +16,20 @@
> #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
> +
> +/*
> + * Retry interval set to 6 seconds, in steps of 200 ms, to allow time for
> + * other OS components to release the MEI CL handle
> + */
> +#define LB_FW_LOAD_RETRY_MAXCOUNT 30
> +#define LB_FW_LOAD_RETRY_PAUSE_MS 200
>
> static const u32 fw_id_to_type[] = {
> [FAN_CONTROL_FW_ID] = CSC_LATE_BINDING_TYPE_FAN_CONTROL,
> @@ -44,6 +58,85 @@ static int xe_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;
> + int fw_id;
> +
> + for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
> + lbfw = &late_bind->late_bind_fw[fw_id];
> + if (lbfw->valid && late_bind->wq) {
> + drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
> + fw_id_to_name[lbfw->id]);
> + flush_work(&lbfw->work);
incorrect indent here for flush_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[lbfw->id]);
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> + int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
> + int ret;
> + int slept;
> +
> + /* 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_id_to_name[lbfw->id]);
> +
> + 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_id_to_name[lbfw->id], ret);
> + else
> + drm_dbg(&xe->drm, "Load %s firmware successful\n",
> + fw_id_to_name[lbfw->id]);
> +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_late_bind_fw *lbfw;
> + int fw_id;
> +
> + if (!late_bind->component_added)
> + return -EINVAL;
-ENODEV instead? This should only happen if we don't support late_bind
or if the component was not built.
> +
> + for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
> + lbfw = &late_bind->late_bind_fw[fw_id];
> + if (lbfw->valid)
> + queue_work(late_bind->wq, &lbfw->work);
> + }
> + return 0;
> +}
> +
> static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
> {
> struct xe_device *xe = late_bind_to_xe(late_bind);
> @@ -93,6 +186,7 @@ static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
>
> 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;
> @@ -103,11 +197,16 @@ static int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
> int ret;
> int fw_id;
>
> + late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
> + if (!late_bind->wq)
> + return -ENOMEM;
> +
> for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
> ret = __xe_late_bind_fw_init(late_bind, fw_id);
> if (ret)
> return ret;
> }
> +
nit: this newline could be moved to the patch that adds this code.
> return ret;
> }
>
> @@ -131,6 +230,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);
I don't see this called for full suspend (not rpm), even if follow up
patches. Am I just not seeing it, or is it missing?
Daniele
> +
> mutex_lock(&late_bind->mutex);
> late_bind->component.ops = NULL;
> mutex_unlock(&late_bind->mutex);
> @@ -146,8 +247,14 @@ 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);
>
> + 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);
> }
>
> @@ -184,5 +291,9 @@ int xe_late_bind_init(struct xe_late_bind *late_bind)
> if (err)
> return err;
>
> - return xe_late_bind_fw_init(late_bind);
> + err = xe_late_bind_fw_init(late_bind);
> + if (err)
> + return err;
> +
> + return xe_late_bind_fw_load(late_bind);
> }
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
> index 4c73571c3e62..28d56ed2bfdc 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_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 c6fd33fd5484..d256f53d59e6 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)
>
> @@ -38,6 +39,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;
> };
>
> /**
> @@ -66,6 +69,8 @@ struct xe_late_bind {
> struct mutex mutex;
> /** @late_bind.late_bind_fw: late binding firmware array */
> struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
> + /** @late_bind.wq: workqueue to submit request to download late bind blob */
> + struct workqueue_struct *wq;
> };
>
> #endif
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 05/10] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-06-18 20:57 ` Daniele Ceraolo Spurio
@ 2025-06-19 5:54 ` Nilawar, Badal
0 siblings, 0 replies; 33+ messages in thread
From: Nilawar, Badal @ 2025-06-19 5:54 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 19-06-2025 02:27, Daniele Ceraolo Spurio wrote:
>
>
> On 6/18/2025 12:00 PM, Badal Nilawar wrote:
>> Load late binding firmware
>>
>> v2:
>> - s/EAGAIN/EBUSY/
>> - Flush worker in suspend and driver unload (Daniele)
>> v3:
>> - Use retry interval of 6s, in steps of 200ms, to allow
>> other OS components release MEI CL handle (Sasha)
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 113 ++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 5 +
>> 3 files changed, 118 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 d416d6cc1fa2..54aa08c6bdfd 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -16,6 +16,20 @@
>> #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
>> +
>> +/*
>> + * Retry interval set to 6 seconds, in steps of 200 ms, to allow
>> time for
>> + * other OS components to release the MEI CL handle
>> + */
>> +#define LB_FW_LOAD_RETRY_MAXCOUNT 30
>> +#define LB_FW_LOAD_RETRY_PAUSE_MS 200
>> static const u32 fw_id_to_type[] = {
>> [FAN_CONTROL_FW_ID] = CSC_LATE_BINDING_TYPE_FAN_CONTROL,
>> @@ -44,6 +58,85 @@ static int xe_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;
>> + int fw_id;
>> +
>> + for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
>> + lbfw = &late_bind->late_bind_fw[fw_id];
>> + if (lbfw->valid && late_bind->wq) {
>> + drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
>> + fw_id_to_name[lbfw->id]);
>> + flush_work(&lbfw->work);
>
> incorrect indent here for flush_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[lbfw->id]);
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
>> + int ret;
>> + int slept;
>> +
>> + /* 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_id_to_name[lbfw->id]);
>> +
>> + 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_id_to_name[lbfw->id], ret);
>> + else
>> + drm_dbg(&xe->drm, "Load %s firmware successful\n",
>> + fw_id_to_name[lbfw->id]);
>> +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_late_bind_fw *lbfw;
>> + int fw_id;
>> +
>> + if (!late_bind->component_added)
>> + return -EINVAL;
>
> -ENODEV instead? This should only happen if we don't support late_bind
> or if the component was not built.
Sure.
>
>> +
>> + for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
>> + lbfw = &late_bind->late_bind_fw[fw_id];
>> + if (lbfw->valid)
>> + queue_work(late_bind->wq, &lbfw->work);
>> + }
>> + return 0;
>> +}
>> +
>> static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind,
>> u32 fw_id)
>> {
>> struct xe_device *xe = late_bind_to_xe(late_bind);
>> @@ -93,6 +186,7 @@ static int __xe_late_bind_fw_init(struct
>> xe_late_bind *late_bind, u32 fw_id)
>> 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;
>> @@ -103,11 +197,16 @@ static int xe_late_bind_fw_init(struct
>> xe_late_bind *late_bind)
>> int ret;
>> int fw_id;
>> + late_bind->wq =
>> alloc_ordered_workqueue("late-bind-ordered-wq", 0);
>> + if (!late_bind->wq)
>> + return -ENOMEM;
>> +
>> for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
>> ret = __xe_late_bind_fw_init(late_bind, fw_id);
>> if (ret)
>> return ret;
>> }
>> +
>
> nit: this newline could be moved to the patch that adds this code.
>
>> return ret;
>> }
>> @@ -131,6 +230,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);
>
> I don't see this called for full suspend (not rpm), even if follow up
> patches. Am I just not seeing it, or is it missing?
I missed this, I will address in follow up patch.
Badal
>
> Daniele
>
>> +
>> mutex_lock(&late_bind->mutex);
>> late_bind->component.ops = NULL;
>> mutex_unlock(&late_bind->mutex);
>> @@ -146,8 +247,14 @@ 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);
>> + 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);
>> }
>> @@ -184,5 +291,9 @@ int xe_late_bind_init(struct xe_late_bind
>> *late_bind)
>> if (err)
>> return err;
>> - return xe_late_bind_fw_init(late_bind);
>> + err = xe_late_bind_fw_init(late_bind);
>> + if (err)
>> + return err;
>> +
>> + return xe_late_bind_fw_load(late_bind);
>> }
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> index 4c73571c3e62..28d56ed2bfdc 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_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 c6fd33fd5484..d256f53d59e6 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)
>> @@ -38,6 +39,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;
>> };
>> /**
>> @@ -66,6 +69,8 @@ struct xe_late_bind {
>> struct mutex mutex;
>> /** @late_bind.late_bind_fw: late binding firmware array */
>> struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
>> + /** @late_bind.wq: workqueue to submit request to download late
>> bind blob */
>> + struct workqueue_struct *wq;
>> };
>> #endif
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
` (4 preceding siblings ...)
2025-06-18 19:00 ` [PATCH v3 05/10] drm/xe/xe_late_bind_fw: Load " Badal Nilawar
@ 2025-06-18 19:00 ` Badal Nilawar
2025-06-18 21:05 ` Daniele Ceraolo Spurio
2025-06-18 19:00 ` [PATCH v3 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume Badal Nilawar
` (3 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 19:00 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
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 54aa08c6bdfd..c0be9611c73b 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -58,7 +58,7 @@ static int xe_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 28d56ed2bfdc..07e437390539 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_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 ff749edc005b..91923fd4af80 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] 33+ messages in thread* Re: [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
2025-06-18 19:00 ` [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume Badal Nilawar
@ 2025-06-18 21:05 ` Daniele Ceraolo Spurio
2025-06-19 5:52 ` Nilawar, Badal
0 siblings, 1 reply; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-18 21:05 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/18/2025 12:00 PM, Badal Nilawar wrote:
> 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 54aa08c6bdfd..c0be9611c73b 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -58,7 +58,7 @@ static int xe_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 28d56ed2bfdc..07e437390539 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_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 ff749edc005b..91923fd4af80 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);
I thing this can deadlock, because you do an rpm_put from within the
worker and if that's the last put it'll end up here and wait for the
worker to complete.
We could probably just skip this wait, because the worker can handle rpm
itself. What we might want to be careful about is to nor re-queue it
(from xe_late_bind_fw_load below) if it's currently being executed; we
could also just let the fw be loaded twice if we hit that race
condition, that shouldn't be an issue apart from doing something not needed.
Daniele
> +
> /*
> * 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);
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
2025-06-18 21:05 ` Daniele Ceraolo Spurio
@ 2025-06-19 5:52 ` Nilawar, Badal
2025-06-23 15:26 ` Daniele Ceraolo Spurio
0 siblings, 1 reply; 33+ messages in thread
From: Nilawar, Badal @ 2025-06-19 5:52 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 19-06-2025 02:35, Daniele Ceraolo Spurio wrote:
>
>
> On 6/18/2025 12:00 PM, Badal Nilawar wrote:
>> 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 54aa08c6bdfd..c0be9611c73b 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -58,7 +58,7 @@ static int xe_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 28d56ed2bfdc..07e437390539 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_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 ff749edc005b..91923fd4af80 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);
>
> I thing this can deadlock, because you do an rpm_put from within the
> worker and if that's the last put it'll end up here and wait for the
> worker to complete.
> We could probably just skip this wait, because the worker can handle
> rpm itself. What we might want to be careful about is to nor re-queue
> it (from xe_late_bind_fw_load below) if it's currently being executed;
> we could also just let the fw be loaded twice if we hit that race
> condition, that shouldn't be an issue apart from doing something not
> needed.
In xe_pm_runtime_get/_put, deadlocks are avoided by verifying the
condition (xe_pm_read_callback_task(xe) == current).
Badal
>
> Daniele
>
>> +
>> /*
>> * 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);
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
2025-06-19 5:52 ` Nilawar, Badal
@ 2025-06-23 15:26 ` Daniele Ceraolo Spurio
2025-06-23 16:11 ` Nilawar, Badal
0 siblings, 1 reply; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-23 15:26 UTC (permalink / raw)
To: Nilawar, Badal, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/18/2025 10:52 PM, Nilawar, Badal wrote:
>
> On 19-06-2025 02:35, Daniele Ceraolo Spurio wrote:
>>
>>
>> On 6/18/2025 12:00 PM, Badal Nilawar wrote:
>>> 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 54aa08c6bdfd..c0be9611c73b 100644
>>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>> @@ -58,7 +58,7 @@ static int xe_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 28d56ed2bfdc..07e437390539 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_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 ff749edc005b..91923fd4af80 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);
>>
>> I thing this can deadlock, because you do an rpm_put from within the
>> worker and if that's the last put it'll end up here and wait for the
>> worker to complete.
>> We could probably just skip this wait, because the worker can handle
>> rpm itself. What we might want to be careful about is to nor re-queue
>> it (from xe_late_bind_fw_load below) if it's currently being
>> executed; we could also just let the fw be loaded twice if we hit
>> that race condition, that shouldn't be an issue apart from doing
>> something not needed.
>
> In xe_pm_runtime_get/_put, deadlocks are avoided by verifying the
> condition (xe_pm_read_callback_task(xe) == current).
Isn't that for calls to rpm_get/put done from within the
rpm_suspend/resume code? This is not the case here, we're not
deadlocking on the rpm lock, we're deadlocking on the worker.
The error flow as I see it here would be as follow:
rpm refcount is 1, owned by thread X
worker starts
worker takes rpm [rpm refcount now 2]
thread X releases rpm [rpm refcount now 1]
worker releases rpm [rpm refcount now 0]
rpm_suspend is called from within the worker
xe_pm_write_callback_task is called
flush_work is called -> deadlock
I don't see how the callback_task() code can block the flush_work from
deadlocking here.
Also, what happens if when the worker starts the rpm refcount is 0?
Assuming the deadlock issue is not there.
worker starts
worker takes rpm [rpm refcount now 1]
rpm_resume is called
worker is re-queued
worker releases rpm [rpm refcount now 0]
worker exits
worker re-starts -> go back to beginning
This second issue should be easily fixed by using pm_get_if_in_use from
the worker, to not load the late_bind table if we're rpm_suspended since
we'll do it when someone else resumes the device.
Daniele
>
> Badal
>
>>
>> Daniele
>>
>>> +
>>> /*
>>> * 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);
>>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
2025-06-23 15:26 ` Daniele Ceraolo Spurio
@ 2025-06-23 16:11 ` Nilawar, Badal
2025-06-23 16:42 ` Daniele Ceraolo Spurio
0 siblings, 1 reply; 33+ messages in thread
From: Nilawar, Badal @ 2025-06-23 16:11 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 23-06-2025 20:56, Daniele Ceraolo Spurio wrote:
>
>
> On 6/18/2025 10:52 PM, Nilawar, Badal wrote:
>>
>> On 19-06-2025 02:35, Daniele Ceraolo Spurio wrote:
>>>
>>>
>>> On 6/18/2025 12:00 PM, Badal Nilawar wrote:
>>>> 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 54aa08c6bdfd..c0be9611c73b 100644
>>>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>>> @@ -58,7 +58,7 @@ static int xe_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 28d56ed2bfdc..07e437390539 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_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 ff749edc005b..91923fd4af80 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);
>>>
>>> I thing this can deadlock, because you do an rpm_put from within the
>>> worker and if that's the last put it'll end up here and wait for the
>>> worker to complete.
>>> We could probably just skip this wait, because the worker can handle
>>> rpm itself. What we might want to be careful about is to nor
>>> re-queue it (from xe_late_bind_fw_load below) if it's currently
>>> being executed; we could also just let the fw be loaded twice if we
>>> hit that race condition, that shouldn't be an issue apart from doing
>>> something not needed.
>>
>> In xe_pm_runtime_get/_put, deadlocks are avoided by verifying the
>> condition (xe_pm_read_callback_task(xe) == current).
>
> Isn't that for calls to rpm_get/put done from within the
> rpm_suspend/resume code? This is not the case here, we're not
> deadlocking on the rpm lock, we're deadlocking on the worker.
>
> The error flow as I see it here would be as follow:
>
> rpm refcount is 1, owned by thread X
> worker starts
> worker takes rpm [rpm refcount now 2]
> thread X releases rpm [rpm refcount now 1]
> worker releases rpm [rpm refcount now 0]
> rpm_suspend is called from within the worker
rpm_suspend is not called within worker. First device will move to idle
state, then via auto suspend flow it will be runtime suspended, all run
time pm state changes will happen from rpm worker.
> xe_pm_write_callback_task is called
> flush_work is called -> deadlock
>
> I don't see how the callback_task() code can block the flush_work from
> deadlocking here.
flush_work, as per my understanding, will wait for work to finish
executing last queuing instance. It runs the worker from the same thread
it is being flushed. So how deadlock will happen?
>
> Also, what happens if when the worker starts the rpm refcount is 0?
> Assuming the deadlock issue is not there.
>
> worker starts
> worker takes rpm [rpm refcount now 1]
> rpm_resume is called
> worker is re-queued
> worker releases rpm [rpm refcount now 0]
> worker exits
> worker re-starts -> go back to beginning
>
> This second issue should be easily fixed by using pm_get_if_in_use
> from the worker, to not load the late_bind table if we're
> rpm_suspended since we'll do it when someone else resumes the device.
Yes this makes sense, I will take care of this in next revision.
Badal
>
> Daniele
>
>>
>> Badal
>>
>>>
>>> Daniele
>>>
>>>> +
>>>> /*
>>>> * 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);
>>>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
2025-06-23 16:11 ` Nilawar, Badal
@ 2025-06-23 16:42 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-23 16:42 UTC (permalink / raw)
To: Nilawar, Badal, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/23/2025 9:11 AM, Nilawar, Badal wrote:
>
> On 23-06-2025 20:56, Daniele Ceraolo Spurio wrote:
>>
>>
>> On 6/18/2025 10:52 PM, Nilawar, Badal wrote:
>>>
>>> On 19-06-2025 02:35, Daniele Ceraolo Spurio wrote:
>>>>
>>>>
>>>> On 6/18/2025 12:00 PM, Badal Nilawar wrote:
>>>>> 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 54aa08c6bdfd..c0be9611c73b 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>>>> @@ -58,7 +58,7 @@ static int xe_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 28d56ed2bfdc..07e437390539 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_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 ff749edc005b..91923fd4af80 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);
>>>>
>>>> I thing this can deadlock, because you do an rpm_put from within
>>>> the worker and if that's the last put it'll end up here and wait
>>>> for the worker to complete.
>>>> We could probably just skip this wait, because the worker can
>>>> handle rpm itself. What we might want to be careful about is to nor
>>>> re-queue it (from xe_late_bind_fw_load below) if it's currently
>>>> being executed; we could also just let the fw be loaded twice if we
>>>> hit that race condition, that shouldn't be an issue apart from
>>>> doing something not needed.
>>>
>>> In xe_pm_runtime_get/_put, deadlocks are avoided by verifying the
>>> condition (xe_pm_read_callback_task(xe) == current).
>>
>> Isn't that for calls to rpm_get/put done from within the
>> rpm_suspend/resume code? This is not the case here, we're not
>> deadlocking on the rpm lock, we're deadlocking on the worker.
>>
>> The error flow as I see it here would be as follow:
>>
>> rpm refcount is 1, owned by thread X
>> worker starts
>> worker takes rpm [rpm refcount now 2]
>> thread X releases rpm [rpm refcount now 1]
>> worker releases rpm [rpm refcount now 0]
>> rpm_suspend is called from within the worker
> rpm_suspend is not called within worker. First device will move to
> idle state, then via auto suspend flow it will be runtime suspended,
> all run time pm state changes will happen from rpm worker.
>
>> xe_pm_write_callback_task is called
>> flush_work is called -> deadlock
>>
>> I don't see how the callback_task() code can block the flush_work
>> from deadlocking here.
>
> flush_work, as per my understanding, will wait for work to finish
> executing last queuing instance. It runs the worker from the same
> thread it is being flushed. So how deadlock will happen?
My bad, for some reason I was convinced that we were doing a sync
suspend, instead of going through the rpm worker. You're correct, no
deadlock here.
Daniele
>
>>
>> Also, what happens if when the worker starts the rpm refcount is 0?
>> Assuming the deadlock issue is not there.
>>
>> worker starts
>> worker takes rpm [rpm refcount now 1]
>> rpm_resume is called
>> worker is re-queued
>> worker releases rpm [rpm refcount now 0]
>> worker exits
>> worker re-starts -> go back to beginning
>>
>> This second issue should be easily fixed by using pm_get_if_in_use
>> from the worker, to not load the late_bind table if we're
>> rpm_suspended since we'll do it when someone else resumes the device.
>
> Yes this makes sense, I will take care of this in next revision.
>
> Badal
>
>>
>> Daniele
>>
>>>
>>> Badal
>>>
>>>>
>>>> Daniele
>>>>
>>>>> +
>>>>> /*
>>>>> * 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);
>>>>
>>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
` (5 preceding siblings ...)
2025-06-18 19:00 ` [PATCH v3 06/10] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume Badal Nilawar
@ 2025-06-18 19:00 ` Badal Nilawar
2025-06-20 13:49 ` Rodrigo Vivi
2025-06-18 19:00 ` [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Badal Nilawar
` (2 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 19:00 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
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 91923fd4af80..6c44a075a6ab 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] 33+ messages in thread* Re: [PATCH v3 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume
2025-06-18 19:00 ` [PATCH v3 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume Badal Nilawar
@ 2025-06-20 13:49 ` Rodrigo Vivi
2025-06-20 15:14 ` Nilawar, Badal
0 siblings, 1 reply; 33+ messages in thread
From: Rodrigo Vivi @ 2025-06-20 13:49 UTC (permalink / raw)
To: Badal Nilawar
Cc: intel-xe, dri-devel, linux-kernel, anshuman.gupta,
alexander.usyskin, gregkh, daniele.ceraolospurio, jgg
On Thu, Jun 19, 2025 at 12:30:04AM +0530, Badal Nilawar wrote:
> 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 91923fd4af80..6c44a075a6ab 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);
something seems off here... d3cold allowed should only be used
for the runtime pm portion from the cases we can lose power like d3cold.
But we don't use that in the s3 path.
We should probably have 2 different calls here. unconditionally call
in the regular/system suspend path and conditionally call in the runtime one.
> +
> drm_dbg(&xe->drm, "Device resumed\n");
> return 0;
> err:
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume
2025-06-20 13:49 ` Rodrigo Vivi
@ 2025-06-20 15:14 ` Nilawar, Badal
0 siblings, 0 replies; 33+ messages in thread
From: Nilawar, Badal @ 2025-06-20 15:14 UTC (permalink / raw)
To: Rodrigo Vivi
Cc: intel-xe, dri-devel, linux-kernel, anshuman.gupta,
alexander.usyskin, gregkh, daniele.ceraolospurio, jgg
On 20-06-2025 19:19, Rodrigo Vivi wrote:
> On Thu, Jun 19, 2025 at 12:30:04AM +0530, Badal Nilawar wrote:
>> 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 91923fd4af80..6c44a075a6ab 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);
> something seems off here... d3cold allowed should only be used
> for the runtime pm portion from the cases we can lose power like d3cold.
> But we don't use that in the s3 path.
>
> We should probably have 2 different calls here. unconditionally call
> in the regular/system suspend path and conditionally call in the runtime one.
Agree, we should unconditionally reload lb binary in regular
suspend/resume path. I will remove D3Cold check.
Regards,
Badal
>
>> +
>> drm_dbg(&xe->drm, "Device resumed\n");
>> return 0;
>> err:
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
` (6 preceding siblings ...)
2025-06-18 19:00 ` [PATCH v3 07/10] drm/xe/xe_late_bind_fw: Reload late binding fw in S2Idle/S3 resume Badal Nilawar
@ 2025-06-18 19:00 ` Badal Nilawar
2025-06-18 21:19 ` Daniele Ceraolo Spurio
2025-06-18 19:00 ` [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info Badal Nilawar
2025-06-18 19:00 ` [PATCH v3 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI Badal Nilawar
9 siblings, 1 reply; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 19:00 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
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.
v2:
-s/(uval == 1) ? true : false/!!uval/ (Daniele)
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_debugfs.c | 41 ++++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++
3 files changed, 47 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
index d83cd6ed3fa8..d1f6f556efa2 100644
--- a/drivers/gpu/drm/xe/xe_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_debugfs.c
@@ -226,6 +226,44 @@ 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;
+ 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 +287,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 c0be9611c73b..001e526e569a 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -129,6 +129,9 @@ 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;
+
for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
lbfw = &late_bind->late_bind_fw[fw_id];
if (lbfw->valid)
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 d256f53d59e6..f79f0c0b2c4a 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -71,6 +71,9 @@ struct xe_late_bind {
struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
/** @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] 33+ messages in thread* Re: [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-06-18 19:00 ` [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Badal Nilawar
@ 2025-06-18 21:19 ` Daniele Ceraolo Spurio
2025-06-19 6:51 ` Nilawar, Badal
0 siblings, 1 reply; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-18 21:19 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/18/2025 12:00 PM, 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.
You haven't replied to my question on the previous rev in regards to the
expected use-case here.
Is this for testing only, or something an actual user might want to do?
If we only need this for testing, please specify so.
Also, what happens if we suspend with a user-loaded binary? userspace
doesn't have visibility to know that they have to re-load their binary.
Daniele
>
> v2:
> -s/(uval == 1) ? true : false/!!uval/ (Daniele)
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/xe_debugfs.c | 41 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++
> 3 files changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index d83cd6ed3fa8..d1f6f556efa2 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -226,6 +226,44 @@ 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;
> + 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 +287,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 c0be9611c73b..001e526e569a 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -129,6 +129,9 @@ 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;
> +
> for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
> lbfw = &late_bind->late_bind_fw[fw_id];
> if (lbfw->valid)
> 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 d256f53d59e6..f79f0c0b2c4a 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> @@ -71,6 +71,9 @@ struct xe_late_bind {
> struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
> /** @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] 33+ messages in thread* Re: [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-06-18 21:19 ` Daniele Ceraolo Spurio
@ 2025-06-19 6:51 ` Nilawar, Badal
2025-06-20 13:46 ` Rodrigo Vivi
2025-06-23 15:37 ` Daniele Ceraolo Spurio
0 siblings, 2 replies; 33+ messages in thread
From: Nilawar, Badal @ 2025-06-19 6:51 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 19-06-2025 02:49, Daniele Ceraolo Spurio wrote:
>
>
> On 6/18/2025 12:00 PM, 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.
>
> You haven't replied to my question on the previous rev in regards to
> the expected use-case here.
> Is this for testing only, or something an actual user might want to
> do? If we only need this for testing, please specify so.
Apologies for the oversight. Yes, this is only necessary for testing the
binary before releasing it for up-streaming. There is internal
tool which uses IGSC lib to download the binary. To avoid clash between
the binaries, this debug fs node is provided.
>
> Also, what happens if we suspend with a user-loaded binary? userspace
> doesn't have visibility to know that they have to re-load their binary.
If the device enters D3 cold state, the binary needs to be reloaded.
However, the kernel mode driver (KMD) does not have control over
binaries downloaded via the IGSC library.
If needed D3 cold can be disabled from BIOS or by setting up
vram_threshold = 0.
Regards,
Badal
> Daniele
>
>>
>> v2:
>> -s/(uval == 1) ? true : false/!!uval/ (Daniele)
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_debugfs.c | 41 ++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++
>> 3 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
>> b/drivers/gpu/drm/xe/xe_debugfs.c
>> index d83cd6ed3fa8..d1f6f556efa2 100644
>> --- a/drivers/gpu/drm/xe/xe_debugfs.c
>> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
>> @@ -226,6 +226,44 @@ 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;
>> + 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 +287,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 c0be9611c73b..001e526e569a 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -129,6 +129,9 @@ 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;
>> +
>> for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
>> lbfw = &late_bind->late_bind_fw[fw_id];
>> if (lbfw->valid)
>> 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 d256f53d59e6..f79f0c0b2c4a 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> @@ -71,6 +71,9 @@ struct xe_late_bind {
>> struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
>> /** @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] 33+ messages in thread* Re: [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-06-19 6:51 ` Nilawar, Badal
@ 2025-06-20 13:46 ` Rodrigo Vivi
2025-06-23 15:37 ` Daniele Ceraolo Spurio
1 sibling, 0 replies; 33+ messages in thread
From: Rodrigo Vivi @ 2025-06-20 13:46 UTC (permalink / raw)
To: Nilawar, Badal
Cc: Daniele Ceraolo Spurio, intel-xe, dri-devel, linux-kernel,
anshuman.gupta, alexander.usyskin, gregkh, jgg
On Thu, Jun 19, 2025 at 12:21:10PM +0530, Nilawar, Badal wrote:
>
> On 19-06-2025 02:49, Daniele Ceraolo Spurio wrote:
> >
> >
> > On 6/18/2025 12:00 PM, 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.
> >
> > You haven't replied to my question on the previous rev in regards to the
> > expected use-case here.
> > Is this for testing only, or something an actual user might want to do?
> > If we only need this for testing, please specify so.
>
> Apologies for the oversight. Yes, this is only necessary for testing the
> binary before releasing it for up-streaming. There is internal
> tool which uses IGSC lib to download the binary. To avoid clash between the
> binaries, this debug fs node is provided.
>
> >
> > Also, what happens if we suspend with a user-loaded binary? userspace
> > doesn't have visibility to know that they have to re-load their binary.
>
> If the device enters D3 cold state, the binary needs to be reloaded.
> However, the kernel mode driver (KMD) does not have control over binaries
> downloaded via the IGSC library.
> If needed D3 cold can be disabled from BIOS or by setting up vram_threshold
> = 0.
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Regards,
> Badal
>
> > Daniele
> >
> > >
> > > v2:
> > > -s/(uval == 1) ? true : false/!!uval/ (Daniele)
> > >
> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > ---
> > > drivers/gpu/drm/xe/xe_debugfs.c | 41 ++++++++++++++++++++++
> > > drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
> > > drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++
> > > 3 files changed, 47 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
> > > b/drivers/gpu/drm/xe/xe_debugfs.c
> > > index d83cd6ed3fa8..d1f6f556efa2 100644
> > > --- a/drivers/gpu/drm/xe/xe_debugfs.c
> > > +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> > > @@ -226,6 +226,44 @@ 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;
> > > + 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 +287,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 c0be9611c73b..001e526e569a 100644
> > > --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > > +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > > @@ -129,6 +129,9 @@ 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;
> > > +
> > > for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
> > > lbfw = &late_bind->late_bind_fw[fw_id];
> > > if (lbfw->valid)
> > > 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 d256f53d59e6..f79f0c0b2c4a 100644
> > > --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> > > @@ -71,6 +71,9 @@ struct xe_late_bind {
> > > struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
> > > /** @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] 33+ messages in thread* Re: [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-06-19 6:51 ` Nilawar, Badal
2025-06-20 13:46 ` Rodrigo Vivi
@ 2025-06-23 15:37 ` Daniele Ceraolo Spurio
2025-06-24 10:12 ` Nilawar, Badal
1 sibling, 1 reply; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-23 15:37 UTC (permalink / raw)
To: Nilawar, Badal, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/18/2025 11:51 PM, Nilawar, Badal wrote:
>
> On 19-06-2025 02:49, Daniele Ceraolo Spurio wrote:
>>
>>
>> On 6/18/2025 12:00 PM, 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.
>>
>> You haven't replied to my question on the previous rev in regards to
>> the expected use-case here.
>> Is this for testing only, or something an actual user might want to
>> do? If we only need this for testing, please specify so.
>
> Apologies for the oversight. Yes, this is only necessary for testing
> the binary before releasing it for up-streaming. There is internal
> tool which uses IGSC lib to download the binary. To avoid clash
> between the binaries, this debug fs node is provided.
>
>>
>> Also, what happens if we suspend with a user-loaded binary? userspace
>> doesn't have visibility to know that they have to re-load their binary.
>
> If the device enters D3 cold state, the binary needs to be reloaded.
> However, the kernel mode driver (KMD) does not have control over
> binaries downloaded via the IGSC library.
> If needed D3 cold can be disabled from BIOS or by setting up
> vram_threshold = 0.
I'm confused. Whatever the tool writes is lost on d3cold entry, does it
make any difference to the tester if we revert to the default values or
to the kernel-loaded table? It's still not what they've written. It
sounds to me more like what you need is to block D3cold (and potentially
S2/S3) when the UMD tool is used so that the userspace-provided table is
not lost.
Otherwise, we could add a modparam to override the binary like we have
for the other firmwares and test anything new that way.
Daniele
>
> Regards,
> Badal
>
>> Daniele
>>
>>>
>>> v2:
>>> -s/(uval == 1) ? true : false/!!uval/ (Daniele)
>>>
>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>> ---
>>> drivers/gpu/drm/xe/xe_debugfs.c | 41
>>> ++++++++++++++++++++++
>>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
>>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++
>>> 3 files changed, 47 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
>>> b/drivers/gpu/drm/xe/xe_debugfs.c
>>> index d83cd6ed3fa8..d1f6f556efa2 100644
>>> --- a/drivers/gpu/drm/xe/xe_debugfs.c
>>> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
>>> @@ -226,6 +226,44 @@ 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;
>>> + 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 +287,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 c0be9611c73b..001e526e569a 100644
>>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>> @@ -129,6 +129,9 @@ 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;
>>> +
>>> for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
>>> lbfw = &late_bind->late_bind_fw[fw_id];
>>> if (lbfw->valid)
>>> 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 d256f53d59e6..f79f0c0b2c4a 100644
>>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>>> @@ -71,6 +71,9 @@ struct xe_late_bind {
>>> struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
>>> /** @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] 33+ messages in thread* Re: [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-06-23 15:37 ` Daniele Ceraolo Spurio
@ 2025-06-24 10:12 ` Nilawar, Badal
0 siblings, 0 replies; 33+ messages in thread
From: Nilawar, Badal @ 2025-06-24 10:12 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 23-06-2025 21:07, Daniele Ceraolo Spurio wrote:
>
>
> On 6/18/2025 11:51 PM, Nilawar, Badal wrote:
>>
>> On 19-06-2025 02:49, Daniele Ceraolo Spurio wrote:
>>>
>>>
>>> On 6/18/2025 12:00 PM, 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.
>>>
>>> You haven't replied to my question on the previous rev in regards to
>>> the expected use-case here.
>>> Is this for testing only, or something an actual user might want to
>>> do? If we only need this for testing, please specify so.
>>
>> Apologies for the oversight. Yes, this is only necessary for testing
>> the binary before releasing it for up-streaming. There is internal
>> tool which uses IGSC lib to download the binary. To avoid clash
>> between the binaries, this debug fs node is provided.
>>
>>>
>>> Also, what happens if we suspend with a user-loaded binary?
>>> userspace doesn't have visibility to know that they have to re-load
>>> their binary.
>>
>> If the device enters D3 cold state, the binary needs to be reloaded.
>> However, the kernel mode driver (KMD) does not have control over
>> binaries downloaded via the IGSC library.
>> If needed D3 cold can be disabled from BIOS or by setting up
>> vram_threshold = 0.
>
> I'm confused. Whatever the tool writes is lost on d3cold entry, does
> it make any difference to the tester if we revert to the default
> values or to the kernel-loaded table? It's still not what they've
> written. It sounds to me more like what you need is to block D3cold
> (and potentially S2/S3) when the UMD tool is used so that the
> userspace-provided table is not lost.
>
> Otherwise, we could add a modparam to override the binary like we have
> for the other firmwares and test anything new that way.
In user space late binding flow xe kmd doesn't need to participate. In
System suspend flow we will not know whether it will go to D3Cold or
not, so we will simply reload the binary during resume. Meanwhile as
discussed offline I will document that config will be lost on D3Cold entry.
Regards,
Badal
>
> Daniele
>
>>
>> Regards,
>> Badal
>>
>>> Daniele
>>>
>>>>
>>>> v2:
>>>> -s/(uval == 1) ? true : false/!!uval/ (Daniele)
>>>>
>>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>>> ---
>>>> drivers/gpu/drm/xe/xe_debugfs.c | 41
>>>> ++++++++++++++++++++++
>>>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
>>>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 ++
>>>> 3 files changed, 47 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
>>>> b/drivers/gpu/drm/xe/xe_debugfs.c
>>>> index d83cd6ed3fa8..d1f6f556efa2 100644
>>>> --- a/drivers/gpu/drm/xe/xe_debugfs.c
>>>> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
>>>> @@ -226,6 +226,44 @@ 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;
>>>> + 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 +287,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 c0be9611c73b..001e526e569a 100644
>>>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>>>> @@ -129,6 +129,9 @@ 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;
>>>> +
>>>> for (fw_id = 0; fw_id < MAX_FW_ID; fw_id++) {
>>>> lbfw = &late_bind->late_bind_fw[fw_id];
>>>> if (lbfw->valid)
>>>> 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 d256f53d59e6..f79f0c0b2c4a 100644
>>>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>>>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>>>> @@ -71,6 +71,9 @@ struct xe_late_bind {
>>>> struct xe_late_bind_fw late_bind_fw[MAX_FW_ID];
>>>> /** @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] 33+ messages in thread
* [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
` (7 preceding siblings ...)
2025-06-18 19:00 ` [PATCH v3 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Badal Nilawar
@ 2025-06-18 19:00 ` Badal Nilawar
2025-06-18 21:56 ` Daniele Ceraolo Spurio
2025-06-24 13:41 ` Dan Carpenter
2025-06-18 19:00 ` [PATCH v3 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI Badal Nilawar
9 siblings, 2 replies; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 19:00 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Extract and print version info of the late binding binary.
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 132 ++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 +
drivers/gpu/drm/xe/xe_uc_fw_abi.h | 69 +++++++++++
3 files changed, 203 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 001e526e569a..f71d5825ac5b 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -45,6 +45,129 @@ late_bind_to_xe(struct xe_late_bind *late_bind)
return container_of(late_bind, struct xe_device, late_bind);
}
+/* Refer to the "Late Bind based Firmware Layout" documentation entry for details */
+static int parse_cpd_header(struct xe_late_bind *late_bind, u32 fw_id,
+ const void *data, size_t size, const char *manifest_entry)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ const struct gsc_cpd_header_v2 *header = data;
+ const struct gsc_manifest_header *manifest;
+ const struct gsc_cpd_entry *entry;
+ size_t min_size = sizeof(*header);
+ struct xe_late_bind_fw *lb_fw;
+ u32 offset;
+ int i;
+
+ if (fw_id >= MAX_FW_ID)
+ return -EINVAL;
+ lb_fw = &late_bind->late_bind_fw[fw_id];
+
+ /* manifest_entry is mandatory */
+ xe_assert(xe, manifest_entry);
+
+ if (size < min_size || header->header_marker != GSC_CPD_HEADER_MARKER)
+ return -ENOENT;
+
+ if (header->header_length < sizeof(struct gsc_cpd_header_v2)) {
+ drm_err(&xe->drm, "%s late binding fw: Invalid CPD header length %u!\n",
+ fw_id_to_name[lb_fw->id], header->header_length);
+ return -EINVAL;
+ }
+
+ min_size = header->header_length + sizeof(struct gsc_cpd_entry) * header->num_of_entries;
+ if (size < min_size) {
+ drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
+ fw_id_to_name[lb_fw->id], size, min_size);
+ return -ENODATA;
+ }
+
+ /* Look for the manifest first */
+ entry = (void *)header + header->header_length;
+ for (i = 0; i < header->num_of_entries; i++, entry++)
+ if (strcmp(entry->name, manifest_entry) == 0)
+ offset = entry->offset & GSC_CPD_ENTRY_OFFSET_MASK;
+
+ if (!offset) {
+ drm_err(&xe->drm, "%s late binding fw: Failed to find manifest_entry\n",
+ fw_id_to_name[lb_fw->id]);
+ return -ENODATA;
+ }
+
+ min_size = offset + sizeof(struct gsc_manifest_header);
+ if (size < min_size) {
+ drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
+ fw_id_to_name[lb_fw->id], size, min_size);
+ return -ENODATA;
+ }
+
+ manifest = data + offset;
+
+ lb_fw->version.major = manifest->fw_version.major;
+ lb_fw->version.minor = manifest->fw_version.minor;
+ lb_fw->version.hotfix = manifest->fw_version.hotfix;
+ lb_fw->version.build = manifest->fw_version.build;
+
+ return 0;
+}
+
+/* Refer to the "Late Bind based Firmware Layout" documentation entry for details */
+static int parse_lb_layout(struct xe_late_bind *late_bind, u32 fw_id,
+ const void *data, size_t size, const char *fpt_entry)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ const struct csc_fpt_header *header = data;
+ const struct csc_fpt_entry *entry;
+ size_t min_size = sizeof(*header);
+ struct xe_late_bind_fw *lb_fw;
+ u32 offset;
+ int i;
+
+ if (fw_id >= MAX_FW_ID)
+ return -EINVAL;
+
+ lb_fw = &late_bind->late_bind_fw[fw_id];
+
+ /* fpt_entry is mandatory */
+ xe_assert(xe, fpt_entry);
+
+ if (size < min_size || header->header_marker != CSC_FPT_HEADER_MARKER)
+ return -ENOENT;
+
+ if (header->header_length < sizeof(struct csc_fpt_header)) {
+ drm_err(&xe->drm, "%s late binding fw: Invalid FPT header length %u!\n",
+ fw_id_to_name[lb_fw->id], header->header_length);
+ return -EINVAL;
+ }
+
+ min_size = header->header_length + sizeof(struct csc_fpt_entry) * header->num_of_entries;
+ if (size < min_size) {
+ drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
+ fw_id_to_name[lb_fw->id], size, min_size);
+ return -ENODATA;
+ }
+
+ /* Look for the manifest first */
+ entry = (void *)header + header->header_length;
+ for (i = 0; i < header->num_of_entries; i++, entry++)
+ if (strcmp(entry->name, fpt_entry) == 0)
+ offset = entry->offset;
+
+ if (!offset) {
+ drm_err(&xe->drm, "%s late binding fw: Failed to find fpt_entry\n",
+ fw_id_to_name[lb_fw->id]);
+ return -ENODATA;
+ }
+
+ min_size = offset + sizeof(struct gsc_cpd_header_v2);
+ if (size < min_size) {
+ drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
+ fw_id_to_name[lb_fw->id], size, min_size);
+ return -ENODATA;
+ }
+
+ return parse_cpd_header(late_bind, fw_id, data + offset, size - offset, "LTES.man");
+}
+
static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
{
struct xe_device *xe = late_bind_to_xe(late_bind);
@@ -185,8 +308,15 @@ static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
return -ENODATA;
}
- lb_fw->payload_size = fw->size;
+ ret = parse_lb_layout(late_bind, fw_id, fw->data, fw->size, "LTES");
+ if (ret)
+ return ret;
+
+ drm_info(&xe->drm, "Using %s firmware from %s version %d.%d.%d\n",
+ fw_id_to_name[lb_fw->id], lb_fw->blob_path,
+ lb_fw->version.major, lb_fw->version.minor, lb_fw->version.hotfix);
+ lb_fw->payload_size = fw->size;
memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
release_firmware(fw);
INIT_WORK(&lb_fw->work, late_bind_work);
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 f79f0c0b2c4a..3fc4f350c81f 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,7 @@
#include <linux/mutex.h>
#include <linux/types.h>
#include <linux/workqueue.h>
+#include "xe_uc_fw_abi.h"
#define MAX_PAYLOAD_SIZE (1024 * 4)
@@ -41,6 +42,8 @@ struct xe_late_bind_fw {
size_t payload_size;
/** @late_bind_fw.work: worker to upload latebind blob */
struct work_struct work;
+ /** @late_bind_fw.version: late binding blob manifest version */
+ struct gsc_version version;
};
/**
diff --git a/drivers/gpu/drm/xe/xe_uc_fw_abi.h b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
index 87ade41209d0..13da2ca96817 100644
--- a/drivers/gpu/drm/xe/xe_uc_fw_abi.h
+++ b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
@@ -318,4 +318,73 @@ struct gsc_manifest_header {
u32 exponent_size; /* in dwords */
} __packed;
+/**
+ * DOC: Late binding Firmware Layout
+ *
+ * The Late binding binary starts with FPT header, which contains locations
+ * of various partitions of the binary. Here we're interested in finding out
+ * manifest version. To the manifest version, we need to locate CPD header
+ * one of the entry in CPD header points to manifest header. Manifest header
+ * contains the version.
+ *
+ * +================================================+
+ * | FPT Header |
+ * +================================================+
+ * | FPT entries[] |
+ * | entry1 |
+ * | ... |
+ * | entryX |
+ * | "LTES" |
+ * | ... |
+ * | offset >-----------------------------|------o
+ * +================================================+ |
+ * |
+ * +================================================+ |
+ * | CPD Header |<-----o
+ * +================================================+
+ * | CPD entries[] |
+ * | entry1 |
+ * | ... |
+ * | entryX |
+ * | "LTES.man" |
+ * | ... |
+ * | offset >----------------------------|------o
+ * +================================================+ |
+ * |
+ * +================================================+ |
+ * | Manifest Header |<-----o
+ * | ... |
+ * | FW version |
+ * | ... |
+ * +================================================+
+ */
+
+/* FPT Headers */
+struct csc_fpt_header {
+ u32 header_marker;
+#define CSC_FPT_HEADER_MARKER 0x54504624
+ u32 num_of_entries;
+ u8 header_version;
+ u8 entry_version;
+ u8 header_length; /* in bytes */
+ u8 flags;
+ u16 ticks_to_add;
+ u16 tokens_to_add;
+ u32 uma_size;
+ u32 crc32;
+ u16 fitc_major;
+ u16 fitc_minor;
+ u16 fitc_hotfix;
+ u16 fitc_build;
+} __packed;
+
+struct csc_fpt_entry {
+ u8 name[4]; /* partition name */
+ u32 reserved1;
+ u32 offset; /* offset from beginning of CSE region */
+ u32 length; /* partition length in bytes */
+ u32 reserved2[3];
+ u32 partition_flags;
+} __packed;
+
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info
2025-06-18 19:00 ` [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info Badal Nilawar
@ 2025-06-18 21:56 ` Daniele Ceraolo Spurio
2025-06-19 9:32 ` Nilawar, Badal
2025-06-24 13:41 ` Dan Carpenter
1 sibling, 1 reply; 33+ messages in thread
From: Daniele Ceraolo Spurio @ 2025-06-18 21:56 UTC (permalink / raw)
To: Badal Nilawar, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 6/18/2025 12:00 PM, Badal Nilawar wrote:
> Extract and print version info of the late binding binary.
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 132 ++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 +
> drivers/gpu/drm/xe/xe_uc_fw_abi.h | 69 +++++++++++
> 3 files changed, 203 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 001e526e569a..f71d5825ac5b 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -45,6 +45,129 @@ late_bind_to_xe(struct xe_late_bind *late_bind)
> return container_of(late_bind, struct xe_device, late_bind);
> }
>
> +/* Refer to the "Late Bind based Firmware Layout" documentation entry for details */
> +static int parse_cpd_header(struct xe_late_bind *late_bind, u32 fw_id,
> + const void *data, size_t size, const char *manifest_entry)
We'll need to try and make this common between the uc_fw code and this
code to reduce duplication, but we can do that as a follow up.
> +{
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> + const struct gsc_cpd_header_v2 *header = data;
> + const struct gsc_manifest_header *manifest;
> + const struct gsc_cpd_entry *entry;
> + size_t min_size = sizeof(*header);
> + struct xe_late_bind_fw *lb_fw;
> + u32 offset;
> + int i;
> +
> + if (fw_id >= MAX_FW_ID)
> + return -EINVAL;
> + lb_fw = &late_bind->late_bind_fw[fw_id];
> +
> + /* manifest_entry is mandatory */
> + xe_assert(xe, manifest_entry);
> +
> + if (size < min_size || header->header_marker != GSC_CPD_HEADER_MARKER)
> + return -ENOENT;
> +
> + if (header->header_length < sizeof(struct gsc_cpd_header_v2)) {
> + drm_err(&xe->drm, "%s late binding fw: Invalid CPD header length %u!\n",
> + fw_id_to_name[lb_fw->id], header->header_length);
> + return -EINVAL;
> + }
> +
> + min_size = header->header_length + sizeof(struct gsc_cpd_entry) * header->num_of_entries;
> + if (size < min_size) {
> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
> + fw_id_to_name[lb_fw->id], size, min_size);
> + return -ENODATA;
> + }
> +
> + /* Look for the manifest first */
> + entry = (void *)header + header->header_length;
> + for (i = 0; i < header->num_of_entries; i++, entry++)
> + if (strcmp(entry->name, manifest_entry) == 0)
> + offset = entry->offset & GSC_CPD_ENTRY_OFFSET_MASK;
> +
> + if (!offset) {
> + drm_err(&xe->drm, "%s late binding fw: Failed to find manifest_entry\n",
> + fw_id_to_name[lb_fw->id]);
> + return -ENODATA;
> + }
> +
> + min_size = offset + sizeof(struct gsc_manifest_header);
> + if (size < min_size) {
> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
> + fw_id_to_name[lb_fw->id], size, min_size);
> + return -ENODATA;
> + }
> +
> + manifest = data + offset;
> +
> + lb_fw->version.major = manifest->fw_version.major;
> + lb_fw->version.minor = manifest->fw_version.minor;
> + lb_fw->version.hotfix = manifest->fw_version.hotfix;
> + lb_fw->version.build = manifest->fw_version.build;
not: here you can just do:
lb_fw->version = manifest->fw_version;
since both variables are of type struct gsc_version.
> +
> + return 0;
> +}
> +
> +/* Refer to the "Late Bind based Firmware Layout" documentation entry for details */
> +static int parse_lb_layout(struct xe_late_bind *late_bind, u32 fw_id,
IMO it'd be cleaner to just pass xe and xe_late_bind_fw, instead of
xe_late_bind and fw_id.
You should also be able to do a lb_fw_to_xe() call if you want with
something like:
container_of(lb_fw, struct xe_device, late_bind.late_bind_fw[lb_fw->id])
> + const void *data, size_t size, const char *fpt_entry)
> +{
> + struct xe_device *xe = late_bind_to_xe(late_bind);
> + const struct csc_fpt_header *header = data;
> + const struct csc_fpt_entry *entry;
> + size_t min_size = sizeof(*header);
> + struct xe_late_bind_fw *lb_fw;
> + u32 offset;
> + int i;
> +
> + if (fw_id >= MAX_FW_ID)
> + return -EINVAL;
> +
> + lb_fw = &late_bind->late_bind_fw[fw_id];
> +
> + /* fpt_entry is mandatory */
> + xe_assert(xe, fpt_entry);
> +
> + if (size < min_size || header->header_marker != CSC_FPT_HEADER_MARKER)
> + return -ENOENT;
> +
> + if (header->header_length < sizeof(struct csc_fpt_header)) {
> + drm_err(&xe->drm, "%s late binding fw: Invalid FPT header length %u!\n",
> + fw_id_to_name[lb_fw->id], header->header_length);
> + return -EINVAL;
> + }
> +
> + min_size = header->header_length + sizeof(struct csc_fpt_entry) * header->num_of_entries;
> + if (size < min_size) {
> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
> + fw_id_to_name[lb_fw->id], size, min_size);
> + return -ENODATA;
> + }
> +
> + /* Look for the manifest first */
Here you're looking for the cpd header, not the manifest.
> + entry = (void *)header + header->header_length;
> + for (i = 0; i < header->num_of_entries; i++, entry++)
> + if (strcmp(entry->name, fpt_entry) == 0)
> + offset = entry->offset;
> +
> + if (!offset) {
> + drm_err(&xe->drm, "%s late binding fw: Failed to find fpt_entry\n",
> + fw_id_to_name[lb_fw->id]);
> + return -ENODATA;
> + }
> +
> + min_size = offset + sizeof(struct gsc_cpd_header_v2);
> + if (size < min_size) {
> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
> + fw_id_to_name[lb_fw->id], size, min_size);
> + return -ENODATA;
> + }
> +
> + return parse_cpd_header(late_bind, fw_id, data + offset, size - offset, "LTES.man");
> +}
> +
> static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
> {
> struct xe_device *xe = late_bind_to_xe(late_bind);
> @@ -185,8 +308,15 @@ static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
> return -ENODATA;
> }
>
> - lb_fw->payload_size = fw->size;
> + ret = parse_lb_layout(late_bind, fw_id, fw->data, fw->size, "LTES");
> + if (ret)
> + return ret;
> +
> + drm_info(&xe->drm, "Using %s firmware from %s version %d.%d.%d\n",
> + fw_id_to_name[lb_fw->id], lb_fw->blob_path,
> + lb_fw->version.major, lb_fw->version.minor, lb_fw->version.hotfix);
You need to log the build number as well, as that needs to be relevant
for this type of headers (we do log it for GSC for example).
>
> + lb_fw->payload_size = fw->size;
> memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
> release_firmware(fw);
> INIT_WORK(&lb_fw->work, late_bind_work);
> 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 f79f0c0b2c4a..3fc4f350c81f 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,7 @@
> #include <linux/mutex.h>
> #include <linux/types.h>
> #include <linux/workqueue.h>
> +#include "xe_uc_fw_abi.h"
>
> #define MAX_PAYLOAD_SIZE (1024 * 4)
>
> @@ -41,6 +42,8 @@ struct xe_late_bind_fw {
> size_t payload_size;
> /** @late_bind_fw.work: worker to upload latebind blob */
> struct work_struct work;
> + /** @late_bind_fw.version: late binding blob manifest version */
> + struct gsc_version version;
> };
>
> /**
> diff --git a/drivers/gpu/drm/xe/xe_uc_fw_abi.h b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
> index 87ade41209d0..13da2ca96817 100644
> --- a/drivers/gpu/drm/xe/xe_uc_fw_abi.h
> +++ b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
> @@ -318,4 +318,73 @@ struct gsc_manifest_header {
> u32 exponent_size; /* in dwords */
> } __packed;
>
> +/**
> + * DOC: Late binding Firmware Layout
> + *
> + * The Late binding binary starts with FPT header, which contains locations
> + * of various partitions of the binary. Here we're interested in finding out
> + * manifest version. To the manifest version, we need to locate CPD header
> + * one of the entry in CPD header points to manifest header. Manifest header
> + * contains the version.
> + *
> + * +================================================+
> + * | FPT Header |
> + * +================================================+
> + * | FPT entries[] |
> + * | entry1 |
> + * | ... |
> + * | entryX |
> + * | "LTES" |
> + * | ... |
> + * | offset >-----------------------------|------o
> + * +================================================+ |
> + * |
> + * +================================================+ |
> + * | CPD Header |<-----o
> + * +================================================+
> + * | CPD entries[] |
> + * | entry1 |
> + * | ... |
> + * | entryX |
> + * | "LTES.man" |
> + * | ... |
> + * | offset >----------------------------|------o
> + * +================================================+ |
> + * |
> + * +================================================+ |
> + * | Manifest Header |<-----o
> + * | ... |
> + * | FW version |
> + * | ... |
> + * +================================================+
> + */
> +
> +/* FPT Headers */
> +struct csc_fpt_header {
> + u32 header_marker;
> +#define CSC_FPT_HEADER_MARKER 0x54504624
> + u32 num_of_entries;
> + u8 header_version;
> + u8 entry_version;
> + u8 header_length; /* in bytes */
> + u8 flags;
> + u16 ticks_to_add;
> + u16 tokens_to_add;
> + u32 uma_size;
> + u32 crc32;
> + u16 fitc_major;
> + u16 fitc_minor;
> + u16 fitc_hotfix;
> + u16 fitc_build;
For other headers we grouped the version values in a gsc_version struct.
So here instead of the 4 separate versions you could have:
struct gsc_version fitc_version;
Which makes it easier to read as all headers have the same type for the
version. We don't read this one though, so not a blocker.
Daniele
> +} __packed;
> +
> +struct csc_fpt_entry {
> + u8 name[4]; /* partition name */
> + u32 reserved1;
> + u32 offset; /* offset from beginning of CSE region */
> + u32 length; /* partition length in bytes */
> + u32 reserved2[3];
> + u32 partition_flags;
> +} __packed;
> +
> #endif
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info
2025-06-18 21:56 ` Daniele Ceraolo Spurio
@ 2025-06-19 9:32 ` Nilawar, Badal
0 siblings, 0 replies; 33+ messages in thread
From: Nilawar, Badal @ 2025-06-19 9:32 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh, jgg
On 19-06-2025 03:26, Daniele Ceraolo Spurio wrote:
>
>
> On 6/18/2025 12:00 PM, Badal Nilawar wrote:
>> Extract and print version info of the late binding binary.
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 132 ++++++++++++++++++++-
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 +
>> drivers/gpu/drm/xe/xe_uc_fw_abi.h | 69 +++++++++++
>> 3 files changed, 203 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 001e526e569a..f71d5825ac5b 100644
>> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -45,6 +45,129 @@ late_bind_to_xe(struct xe_late_bind *late_bind)
>> return container_of(late_bind, struct xe_device, late_bind);
>> }
>> +/* Refer to the "Late Bind based Firmware Layout" documentation
>> entry for details */
>> +static int parse_cpd_header(struct xe_late_bind *late_bind, u32 fw_id,
>> + const void *data, size_t size, const char
>> *manifest_entry)
>
> We'll need to try and make this common between the uc_fw code and this
> code to reduce duplication, but we can do that as a follow up.
I agree, we should do this as follow up.
>
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + const struct gsc_cpd_header_v2 *header = data;
>> + const struct gsc_manifest_header *manifest;
>> + const struct gsc_cpd_entry *entry;
>> + size_t min_size = sizeof(*header);
>> + struct xe_late_bind_fw *lb_fw;
>> + u32 offset;
>> + int i;
>> +
>> + if (fw_id >= MAX_FW_ID)
>> + return -EINVAL;
>> + lb_fw = &late_bind->late_bind_fw[fw_id];
>> +
>> + /* manifest_entry is mandatory */
>> + xe_assert(xe, manifest_entry);
>> +
>> + if (size < min_size || header->header_marker !=
>> GSC_CPD_HEADER_MARKER)
>> + return -ENOENT;
>> +
>> + if (header->header_length < sizeof(struct gsc_cpd_header_v2)) {
>> + drm_err(&xe->drm, "%s late binding fw: Invalid CPD header
>> length %u!\n",
>> + fw_id_to_name[lb_fw->id], header->header_length);
>> + return -EINVAL;
>> + }
>> +
>> + min_size = header->header_length + sizeof(struct gsc_cpd_entry)
>> * header->num_of_entries;
>> + if (size < min_size) {
>> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> + fw_id_to_name[lb_fw->id], size, min_size);
>> + return -ENODATA;
>> + }
>> +
>> + /* Look for the manifest first */
>> + entry = (void *)header + header->header_length;
>> + for (i = 0; i < header->num_of_entries; i++, entry++)
>> + if (strcmp(entry->name, manifest_entry) == 0)
>> + offset = entry->offset & GSC_CPD_ENTRY_OFFSET_MASK;
>> +
>> + if (!offset) {
>> + drm_err(&xe->drm, "%s late binding fw: Failed to find
>> manifest_entry\n",
>> + fw_id_to_name[lb_fw->id]);
>> + return -ENODATA;
>> + }
>> +
>> + min_size = offset + sizeof(struct gsc_manifest_header);
>> + if (size < min_size) {
>> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> + fw_id_to_name[lb_fw->id], size, min_size);
>> + return -ENODATA;
>> + }
>> +
>> + manifest = data + offset;
>> +
>> + lb_fw->version.major = manifest->fw_version.major;
>> + lb_fw->version.minor = manifest->fw_version.minor;
>> + lb_fw->version.hotfix = manifest->fw_version.hotfix;
>> + lb_fw->version.build = manifest->fw_version.build;
>
> not: here you can just do:
>
> lb_fw->version = manifest->fw_version;
>
> since both variables are of type struct gsc_version.
Ok
>
>> +
>> + return 0;
>> +}
>> +
>> +/* Refer to the "Late Bind based Firmware Layout" documentation
>> entry for details */
>> +static int parse_lb_layout(struct xe_late_bind *late_bind, u32 fw_id,
>
> IMO it'd be cleaner to just pass xe and xe_late_bind_fw, instead of
> xe_late_bind and fw_id.
> You should also be able to do a lb_fw_to_xe() call if you want with
> something like:
>
> container_of(lb_fw, struct xe_device, late_bind.late_bind_fw[lb_fw->id])
Sure.
>
>> + const void *data, size_t size, const char *fpt_entry)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + const struct csc_fpt_header *header = data;
>> + const struct csc_fpt_entry *entry;
>> + size_t min_size = sizeof(*header);
>> + struct xe_late_bind_fw *lb_fw;
>> + u32 offset;
>> + int i;
>> +
>> + if (fw_id >= MAX_FW_ID)
>> + return -EINVAL;
>> +
>> + lb_fw = &late_bind->late_bind_fw[fw_id];
>> +
>> + /* fpt_entry is mandatory */
>> + xe_assert(xe, fpt_entry);
>> +
>> + if (size < min_size || header->header_marker !=
>> CSC_FPT_HEADER_MARKER)
>> + return -ENOENT;
>> +
>> + if (header->header_length < sizeof(struct csc_fpt_header)) {
>> + drm_err(&xe->drm, "%s late binding fw: Invalid FPT header
>> length %u!\n",
>> + fw_id_to_name[lb_fw->id], header->header_length);
>> + return -EINVAL;
>> + }
>> +
>> + min_size = header->header_length + sizeof(struct csc_fpt_entry)
>> * header->num_of_entries;
>> + if (size < min_size) {
>> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> + fw_id_to_name[lb_fw->id], size, min_size);
>> + return -ENODATA;
>> + }
>> +
>> + /* Look for the manifest first */
>
> Here you're looking for the cpd header, not the manifest.
Ok.
>
>> + entry = (void *)header + header->header_length;
>> + for (i = 0; i < header->num_of_entries; i++, entry++)
>> + if (strcmp(entry->name, fpt_entry) == 0)
>> + offset = entry->offset;
>> +
>> + if (!offset) {
>> + drm_err(&xe->drm, "%s late binding fw: Failed to find
>> fpt_entry\n",
>> + fw_id_to_name[lb_fw->id]);
>> + return -ENODATA;
>> + }
>> +
>> + min_size = offset + sizeof(struct gsc_cpd_header_v2);
>> + if (size < min_size) {
>> + drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
>> + fw_id_to_name[lb_fw->id], size, min_size);
>> + return -ENODATA;
>> + }
>> +
>> + return parse_cpd_header(late_bind, fw_id, data + offset, size -
>> offset, "LTES.man");
>> +}
>> +
>> static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
>> {
>> struct xe_device *xe = late_bind_to_xe(late_bind);
>> @@ -185,8 +308,15 @@ static int __xe_late_bind_fw_init(struct
>> xe_late_bind *late_bind, u32 fw_id)
>> return -ENODATA;
>> }
>> - lb_fw->payload_size = fw->size;
>> + ret = parse_lb_layout(late_bind, fw_id, fw->data, fw->size,
>> "LTES");
>> + if (ret)
>> + return ret;
>> +
>> + drm_info(&xe->drm, "Using %s firmware from %s version %d.%d.%d\n",
>> + fw_id_to_name[lb_fw->id], lb_fw->blob_path,
>> + lb_fw->version.major, lb_fw->version.minor,
>> lb_fw->version.hotfix);
>
> You need to log the build number as well, as that needs to be relevant
> for this type of headers (we do log it for GSC for example).
I will log build number too.
>
>> + lb_fw->payload_size = fw->size;
>> memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
>> release_firmware(fw);
>> INIT_WORK(&lb_fw->work, late_bind_work);
>> 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 f79f0c0b2c4a..3fc4f350c81f 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,7 @@
>> #include <linux/mutex.h>
>> #include <linux/types.h>
>> #include <linux/workqueue.h>
>> +#include "xe_uc_fw_abi.h"
>> #define MAX_PAYLOAD_SIZE (1024 * 4)
>> @@ -41,6 +42,8 @@ struct xe_late_bind_fw {
>> size_t payload_size;
>> /** @late_bind_fw.work: worker to upload latebind blob */
>> struct work_struct work;
>> + /** @late_bind_fw.version: late binding blob manifest version */
>> + struct gsc_version version;
>> };
>> /**
>> diff --git a/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> index 87ade41209d0..13da2ca96817 100644
>> --- a/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> +++ b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
>> @@ -318,4 +318,73 @@ struct gsc_manifest_header {
>> u32 exponent_size; /* in dwords */
>> } __packed;
>> +/**
>> + * DOC: Late binding Firmware Layout
>> + *
>> + * The Late binding binary starts with FPT header, which contains
>> locations
>> + * of various partitions of the binary. Here we're interested in
>> finding out
>> + * manifest version. To the manifest version, we need to locate CPD
>> header
>> + * one of the entry in CPD header points to manifest header.
>> Manifest header
>> + * contains the version.
>> + *
>> + * +================================================+
>> + * | FPT Header |
>> + * +================================================+
>> + * | FPT entries[] |
>> + * | entry1 |
>> + * | ... |
>> + * | entryX |
>> + * | "LTES" |
>> + * | ... |
>> + * | offset >-----------------------------|------o
>> + * +================================================+ |
>> + * |
>> + * +================================================+ |
>> + * | CPD Header |<-----o
>> + * +================================================+
>> + * | CPD entries[] |
>> + * | entry1 |
>> + * | ... |
>> + * | entryX |
>> + * | "LTES.man" |
>> + * | ... |
>> + * | offset >----------------------------|------o
>> + * +================================================+ |
>> + * |
>> + * +================================================+ |
>> + * | Manifest Header |<-----o
>> + * | ... |
>> + * | FW version |
>> + * | ... |
>> + * +================================================+
>> + */
>> +
>> +/* FPT Headers */
>> +struct csc_fpt_header {
>> + u32 header_marker;
>> +#define CSC_FPT_HEADER_MARKER 0x54504624
>> + u32 num_of_entries;
>> + u8 header_version;
>> + u8 entry_version;
>> + u8 header_length; /* in bytes */
>> + u8 flags;
>> + u16 ticks_to_add;
>> + u16 tokens_to_add;
>> + u32 uma_size;
>> + u32 crc32;
>> + u16 fitc_major;
>> + u16 fitc_minor;
>> + u16 fitc_hotfix;
>> + u16 fitc_build;
>
> For other headers we grouped the version values in a gsc_version
> struct. So here instead of the 4 separate versions you could have:
>
> struct gsc_version fitc_version;
>
> Which makes it easier to read as all headers have the same type for
> the version. We don't read this one though, so not a blocker.
Fine, I will take care of this.
Badal
>
> Daniele
>
>> +} __packed;
>> +
>> +struct csc_fpt_entry {
>> + u8 name[4]; /* partition name */
>> + u32 reserved1;
>> + u32 offset; /* offset from beginning of CSE region */
>> + u32 length; /* partition length in bytes */
>> + u32 reserved2[3];
>> + u32 partition_flags;
>> +} __packed;
>> +
>> #endif
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info
2025-06-18 19:00 ` [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info Badal Nilawar
2025-06-18 21:56 ` Daniele Ceraolo Spurio
@ 2025-06-24 13:41 ` Dan Carpenter
1 sibling, 0 replies; 33+ messages in thread
From: Dan Carpenter @ 2025-06-24 13:41 UTC (permalink / raw)
To: oe-kbuild, Badal Nilawar, intel-xe, dri-devel, linux-kernel
Cc: lkp, oe-kbuild-all, anshuman.gupta, rodrigo.vivi,
alexander.usyskin, gregkh, daniele.ceraolospurio, jgg
Hi Badal,
kernel test robot noticed the following build warnings:
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/20250619-025825
base: https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next
patch link: https://lore.kernel.org/r/20250618190007.2932322-10-badal.nilawar%40intel.com
patch subject: [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info
config: i386-randconfig-141-20250623 (https://download.01.org/0day-ci/archive/20250624/202506241449.WdiucfJp-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202506241449.WdiucfJp-lkp@intel.com/
New smatch warnings:
drivers/gpu/drm/xe/xe_late_bind_fw.c:90 parse_cpd_header() error: uninitialized symbol 'offset'.
drivers/gpu/drm/xe/xe_late_bind_fw.c:155 parse_lb_layout() error: uninitialized symbol 'offset'.
Old smatch warnings:
drivers/gpu/drm/xe/xe_late_bind_fw.c:195 xe_late_bind_wait_for_worker_completion() warn: inconsistent indenting
vim +/offset +90 drivers/gpu/drm/xe/xe_late_bind_fw.c
f9ea24fb9528adc Badal Nilawar 2025-06-19 49 static int parse_cpd_header(struct xe_late_bind *late_bind, u32 fw_id,
f9ea24fb9528adc Badal Nilawar 2025-06-19 50 const void *data, size_t size, const char *manifest_entry)
f9ea24fb9528adc Badal Nilawar 2025-06-19 51 {
f9ea24fb9528adc Badal Nilawar 2025-06-19 52 struct xe_device *xe = late_bind_to_xe(late_bind);
f9ea24fb9528adc Badal Nilawar 2025-06-19 53 const struct gsc_cpd_header_v2 *header = data;
f9ea24fb9528adc Badal Nilawar 2025-06-19 54 const struct gsc_manifest_header *manifest;
f9ea24fb9528adc Badal Nilawar 2025-06-19 55 const struct gsc_cpd_entry *entry;
f9ea24fb9528adc Badal Nilawar 2025-06-19 56 size_t min_size = sizeof(*header);
f9ea24fb9528adc Badal Nilawar 2025-06-19 57 struct xe_late_bind_fw *lb_fw;
f9ea24fb9528adc Badal Nilawar 2025-06-19 58 u32 offset;
^^^^^^^^^^^
f9ea24fb9528adc Badal Nilawar 2025-06-19 59 int i;
f9ea24fb9528adc Badal Nilawar 2025-06-19 60
f9ea24fb9528adc Badal Nilawar 2025-06-19 61 if (fw_id >= MAX_FW_ID)
f9ea24fb9528adc Badal Nilawar 2025-06-19 62 return -EINVAL;
f9ea24fb9528adc Badal Nilawar 2025-06-19 63 lb_fw = &late_bind->late_bind_fw[fw_id];
f9ea24fb9528adc Badal Nilawar 2025-06-19 64
f9ea24fb9528adc Badal Nilawar 2025-06-19 65 /* manifest_entry is mandatory */
f9ea24fb9528adc Badal Nilawar 2025-06-19 66 xe_assert(xe, manifest_entry);
f9ea24fb9528adc Badal Nilawar 2025-06-19 67
f9ea24fb9528adc Badal Nilawar 2025-06-19 68 if (size < min_size || header->header_marker != GSC_CPD_HEADER_MARKER)
f9ea24fb9528adc Badal Nilawar 2025-06-19 69 return -ENOENT;
f9ea24fb9528adc Badal Nilawar 2025-06-19 70
f9ea24fb9528adc Badal Nilawar 2025-06-19 71 if (header->header_length < sizeof(struct gsc_cpd_header_v2)) {
f9ea24fb9528adc Badal Nilawar 2025-06-19 72 drm_err(&xe->drm, "%s late binding fw: Invalid CPD header length %u!\n",
f9ea24fb9528adc Badal Nilawar 2025-06-19 73 fw_id_to_name[lb_fw->id], header->header_length);
f9ea24fb9528adc Badal Nilawar 2025-06-19 74 return -EINVAL;
f9ea24fb9528adc Badal Nilawar 2025-06-19 75 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 76
f9ea24fb9528adc Badal Nilawar 2025-06-19 77 min_size = header->header_length + sizeof(struct gsc_cpd_entry) * header->num_of_entries;
f9ea24fb9528adc Badal Nilawar 2025-06-19 78 if (size < min_size) {
f9ea24fb9528adc Badal Nilawar 2025-06-19 79 drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
f9ea24fb9528adc Badal Nilawar 2025-06-19 80 fw_id_to_name[lb_fw->id], size, min_size);
f9ea24fb9528adc Badal Nilawar 2025-06-19 81 return -ENODATA;
f9ea24fb9528adc Badal Nilawar 2025-06-19 82 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 83
f9ea24fb9528adc Badal Nilawar 2025-06-19 84 /* Look for the manifest first */
f9ea24fb9528adc Badal Nilawar 2025-06-19 85 entry = (void *)header + header->header_length;
f9ea24fb9528adc Badal Nilawar 2025-06-19 86 for (i = 0; i < header->num_of_entries; i++, entry++)
f9ea24fb9528adc Badal Nilawar 2025-06-19 87 if (strcmp(entry->name, manifest_entry) == 0)
f9ea24fb9528adc Badal Nilawar 2025-06-19 88 offset = entry->offset & GSC_CPD_ENTRY_OFFSET_MASK;
^^^^^^^^
Only initialized if found.
f9ea24fb9528adc Badal Nilawar 2025-06-19 89
f9ea24fb9528adc Badal Nilawar 2025-06-19 @90 if (!offset) {
^^^^^^
Uninitialized.
It's a good idea for developers to set CONFIG_INIT_STACK_ALL_PATTERN=y
in their testing.
f9ea24fb9528adc Badal Nilawar 2025-06-19 91 drm_err(&xe->drm, "%s late binding fw: Failed to find manifest_entry\n",
f9ea24fb9528adc Badal Nilawar 2025-06-19 92 fw_id_to_name[lb_fw->id]);
f9ea24fb9528adc Badal Nilawar 2025-06-19 93 return -ENODATA;
f9ea24fb9528adc Badal Nilawar 2025-06-19 94 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 95
f9ea24fb9528adc Badal Nilawar 2025-06-19 96 min_size = offset + sizeof(struct gsc_manifest_header);
f9ea24fb9528adc Badal Nilawar 2025-06-19 97 if (size < min_size) {
f9ea24fb9528adc Badal Nilawar 2025-06-19 98 drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
f9ea24fb9528adc Badal Nilawar 2025-06-19 99 fw_id_to_name[lb_fw->id], size, min_size);
f9ea24fb9528adc Badal Nilawar 2025-06-19 100 return -ENODATA;
f9ea24fb9528adc Badal Nilawar 2025-06-19 101 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 102
f9ea24fb9528adc Badal Nilawar 2025-06-19 103 manifest = data + offset;
f9ea24fb9528adc Badal Nilawar 2025-06-19 104
f9ea24fb9528adc Badal Nilawar 2025-06-19 105 lb_fw->version.major = manifest->fw_version.major;
f9ea24fb9528adc Badal Nilawar 2025-06-19 106 lb_fw->version.minor = manifest->fw_version.minor;
f9ea24fb9528adc Badal Nilawar 2025-06-19 107 lb_fw->version.hotfix = manifest->fw_version.hotfix;
f9ea24fb9528adc Badal Nilawar 2025-06-19 108 lb_fw->version.build = manifest->fw_version.build;
f9ea24fb9528adc Badal Nilawar 2025-06-19 109
f9ea24fb9528adc Badal Nilawar 2025-06-19 110 return 0;
f9ea24fb9528adc Badal Nilawar 2025-06-19 111 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 112
f9ea24fb9528adc Badal Nilawar 2025-06-19 113 /* Refer to the "Late Bind based Firmware Layout" documentation entry for details */
f9ea24fb9528adc Badal Nilawar 2025-06-19 114 static int parse_lb_layout(struct xe_late_bind *late_bind, u32 fw_id,
f9ea24fb9528adc Badal Nilawar 2025-06-19 115 const void *data, size_t size, const char *fpt_entry)
f9ea24fb9528adc Badal Nilawar 2025-06-19 116 {
f9ea24fb9528adc Badal Nilawar 2025-06-19 117 struct xe_device *xe = late_bind_to_xe(late_bind);
f9ea24fb9528adc Badal Nilawar 2025-06-19 118 const struct csc_fpt_header *header = data;
f9ea24fb9528adc Badal Nilawar 2025-06-19 119 const struct csc_fpt_entry *entry;
f9ea24fb9528adc Badal Nilawar 2025-06-19 120 size_t min_size = sizeof(*header);
f9ea24fb9528adc Badal Nilawar 2025-06-19 121 struct xe_late_bind_fw *lb_fw;
f9ea24fb9528adc Badal Nilawar 2025-06-19 122 u32 offset;
f9ea24fb9528adc Badal Nilawar 2025-06-19 123 int i;
f9ea24fb9528adc Badal Nilawar 2025-06-19 124
f9ea24fb9528adc Badal Nilawar 2025-06-19 125 if (fw_id >= MAX_FW_ID)
f9ea24fb9528adc Badal Nilawar 2025-06-19 126 return -EINVAL;
f9ea24fb9528adc Badal Nilawar 2025-06-19 127
f9ea24fb9528adc Badal Nilawar 2025-06-19 128 lb_fw = &late_bind->late_bind_fw[fw_id];
f9ea24fb9528adc Badal Nilawar 2025-06-19 129
f9ea24fb9528adc Badal Nilawar 2025-06-19 130 /* fpt_entry is mandatory */
f9ea24fb9528adc Badal Nilawar 2025-06-19 131 xe_assert(xe, fpt_entry);
f9ea24fb9528adc Badal Nilawar 2025-06-19 132
f9ea24fb9528adc Badal Nilawar 2025-06-19 133 if (size < min_size || header->header_marker != CSC_FPT_HEADER_MARKER)
f9ea24fb9528adc Badal Nilawar 2025-06-19 134 return -ENOENT;
f9ea24fb9528adc Badal Nilawar 2025-06-19 135
f9ea24fb9528adc Badal Nilawar 2025-06-19 136 if (header->header_length < sizeof(struct csc_fpt_header)) {
f9ea24fb9528adc Badal Nilawar 2025-06-19 137 drm_err(&xe->drm, "%s late binding fw: Invalid FPT header length %u!\n",
f9ea24fb9528adc Badal Nilawar 2025-06-19 138 fw_id_to_name[lb_fw->id], header->header_length);
f9ea24fb9528adc Badal Nilawar 2025-06-19 139 return -EINVAL;
f9ea24fb9528adc Badal Nilawar 2025-06-19 140 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 141
f9ea24fb9528adc Badal Nilawar 2025-06-19 142 min_size = header->header_length + sizeof(struct csc_fpt_entry) * header->num_of_entries;
f9ea24fb9528adc Badal Nilawar 2025-06-19 143 if (size < min_size) {
f9ea24fb9528adc Badal Nilawar 2025-06-19 144 drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
f9ea24fb9528adc Badal Nilawar 2025-06-19 145 fw_id_to_name[lb_fw->id], size, min_size);
f9ea24fb9528adc Badal Nilawar 2025-06-19 146 return -ENODATA;
f9ea24fb9528adc Badal Nilawar 2025-06-19 147 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 148
f9ea24fb9528adc Badal Nilawar 2025-06-19 149 /* Look for the manifest first */
f9ea24fb9528adc Badal Nilawar 2025-06-19 150 entry = (void *)header + header->header_length;
f9ea24fb9528adc Badal Nilawar 2025-06-19 151 for (i = 0; i < header->num_of_entries; i++, entry++)
f9ea24fb9528adc Badal Nilawar 2025-06-19 152 if (strcmp(entry->name, fpt_entry) == 0)
f9ea24fb9528adc Badal Nilawar 2025-06-19 153 offset = entry->offset;
f9ea24fb9528adc Badal Nilawar 2025-06-19 154
f9ea24fb9528adc Badal Nilawar 2025-06-19 @155 if (!offset) {
Same.
f9ea24fb9528adc Badal Nilawar 2025-06-19 156 drm_err(&xe->drm, "%s late binding fw: Failed to find fpt_entry\n",
f9ea24fb9528adc Badal Nilawar 2025-06-19 157 fw_id_to_name[lb_fw->id]);
f9ea24fb9528adc Badal Nilawar 2025-06-19 158 return -ENODATA;
f9ea24fb9528adc Badal Nilawar 2025-06-19 159 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 160
f9ea24fb9528adc Badal Nilawar 2025-06-19 161 min_size = offset + sizeof(struct gsc_cpd_header_v2);
f9ea24fb9528adc Badal Nilawar 2025-06-19 162 if (size < min_size) {
f9ea24fb9528adc Badal Nilawar 2025-06-19 163 drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
f9ea24fb9528adc Badal Nilawar 2025-06-19 164 fw_id_to_name[lb_fw->id], size, min_size);
f9ea24fb9528adc Badal Nilawar 2025-06-19 165 return -ENODATA;
f9ea24fb9528adc Badal Nilawar 2025-06-19 166 }
f9ea24fb9528adc Badal Nilawar 2025-06-19 167
f9ea24fb9528adc Badal Nilawar 2025-06-19 168 return parse_cpd_header(late_bind, fw_id, data + offset, size - offset, "LTES.man");
f9ea24fb9528adc Badal Nilawar 2025-06-19 169 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 10/10] [CI]drm/xe/xe_late_bind_fw: Select INTEL_MEI_LATE_BIND for CI
2025-06-18 18:59 [PATCH v3 00/10] Introducing firmware late binding Badal Nilawar
` (8 preceding siblings ...)
2025-06-18 19:00 ` [PATCH v3 09/10] drm/xe/xe_late_bind_fw: Extract and print version info Badal Nilawar
@ 2025-06-18 19:00 ` Badal Nilawar
9 siblings, 0 replies; 33+ messages in thread
From: Badal Nilawar @ 2025-06-18 19:00 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: anshuman.gupta, rodrigo.vivi, alexander.usyskin, gregkh,
daniele.ceraolospurio, jgg
Do not review
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 30ed74ad29ab..b161e1156c73 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -44,6 +44,7 @@ config DRM_XE
select WANT_DEV_COREDUMP
select AUXILIARY_BUS
select HMM_MIRROR
+ select INTEL_MEI_LATE_BIND
help
Driver for Intel Xe2 series GPUs and later. Experimental support
for Xe series is also available.
--
2.34.1
^ permalink raw reply related [flat|nested] 33+ messages in thread