Re: [PATCH v4 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
From: Nilawar, Badal
Date: Fri Jun 27 2025 - 08:49:40 EST
On 27-06-2025 02:36, Daniele Ceraolo Spurio wrote:
On 6/25/2025 10:00 AM, 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
v4 (Daniele):
- Alloc payload dynamically, fix nits
Signed-off-by: Badal Nilawar <badal.nilawar@xxxxxxxxx>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 103 ++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 32 +++++++
2 files changed, 134 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 eaf12cfec848..32d1436e7191 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[] = {
+ [XE_LB_FW_FAN_CONTROL] = CSC_LATE_BINDING_TYPE_FAN_CONTROL,
+ };
+
+static const char * const fw_id_to_name[] = {
+ [XE_LB_FW_FAN_CONTROL] = "fan_control",
+ };
static struct xe_device *
late_bind_to_xe(struct xe_late_bind *late_bind)
@@ -20,6 +31,92 @@ 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 >= XE_LB_FW_MAX_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 = drmm_kzalloc(&xe->drm, lb_fw->payload_size,
GFP_KERNEL);
here you're using lb_fw->payload_size before assigning it.
My bad, I will fix it. But I'm curious why drmm_kzalloc, unlike
kzalloc, doesn't perform a size=0 check. When size=0, kzalloc returns
ZERO_SIZE_POINTER, which is absent in drmm_kzalloc. Even if drmm_kzalloc
had returned ZERO_SIZE_POINTER, the check below wouldn't have caught it.
+ if (!lb_fw->payload) {
+ release_firmware(fw);
+ return -ENOMEM;
+ }
+
+ lb_fw->payload_size = fw->size;
+
+ memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
+ release_firmware(fw);
+ lb_fw->valid = true;
You can now use lb_fw->payload to check if the FW is valid, no need
for a separate variable. not a blocker.
Sure.
+
+ 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 < XE_LB_FW_MAX_ID; fw_id++) {
+ ret = __xe_late_bind_fw_init(late_bind, fw_id);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
static int xe_late_bind_component_bind(struct device *xe_kdev,
struct device *mei_kdev, void *data)
{
@@ -86,5 +183,9 @@ int xe_late_bind_init(struct xe_late_bind *late_bind)
return err;
}
- 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 1156ef94f0d5..93abf4c51789 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 SZ_4K
+
+/**
+ * xe_late_bind_fw_id - enum to determine late binding fw index
+ */
+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;
Why a u8 pointer and not a void one?
It should have been const u8 * as firmware structure has uses const u8 *.
struct firmware {
size_t size;
const u8 *data;
/* firmware loader private fields */
void *priv;
};
Badal
Daniele
+ /** @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.
@@ -32,6 +62,8 @@ struct xe_late_bind {
struct xe_late_bind_component component;
/** @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[XE_LB_FW_MAX_ID];
};
#endif