[PATCH v4 04/10] drm/xe/xe_late_bind_fw: Initialize late binding firmware
Nilawar, Badal
badal.nilawar at intel.com
Fri Jun 27 12:48:51 UTC 2025
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 at intel.com>
>> ---
>> 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
>
More information about the Intel-xe
mailing list