Hi Badal,

Just a small query below..

On Fri, Jun 06, 2025 at 11:27:02PM +0530, Badal Nilawar wrote:
Load late binding firmware

v2:
- s/EAGAIN/EBUSY/
- Flush worker in suspend and driver unload (Daniele)

Signed-off-by: Badal Nilawar <badal.nila...@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c       | 121 ++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw.h       |   1 +
drivers/gpu/drm/xe/xe_late_bind_fw_types.h |   5 +
3 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c 
b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 0231f3dcfc18..7fe304c54374 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -16,6 +16,16 @@
#include "xe_late_bind_fw.h"
#include "xe_pcode.h"
#include "xe_pcode_api.h"
+#include "xe_pm.h"
+
+/*
+ * The component should load quite quickly in most cases, but it could take
+ * a bit. Using a very big timeout just to cover the worst case scenario
+ */
+#define LB_INIT_TIMEOUT_MS 20000
+
+#define LB_FW_LOAD_RETRY_MAXCOUNT 40
+#define LB_FW_LOAD_RETRY_PAUSE_MS 50

static const char * const fw_type_to_name[] = {
                [CSC_LATE_BINDING_TYPE_FAN_CONTROL] = "fan_control",
@@ -39,6 +49,95 @@ static int late_bind_fw_num_fans(struct xe_late_bind 
*late_bind)
                return 0;
}

+static void xe_late_bind_wait_for_worker_completion(struct xe_late_bind 
*late_bind)
+{
+       struct xe_device *xe = late_bind_to_xe(late_bind);
+       struct xe_late_bind_fw *lbfw;
+
+       lbfw = &late_bind->late_bind_fw;
+       if (lbfw->valid && late_bind->wq) {
+               drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
+                       fw_type_to_name[lbfw->type]);
+                       flush_work(&lbfw->work);
+       }
+}
+
+static void late_bind_work(struct work_struct *work)

Why is this a worker? Do we let the kmd probe go ahead while this worker is trying to do a push_config? Wondering why this logic is not directly called from the bind here.

Thanks,
Umesh

+{
+       struct xe_late_bind_fw *lbfw = container_of(work, struct 
xe_late_bind_fw, work);
+       struct xe_late_bind *late_bind = container_of(lbfw, struct xe_late_bind,
+                                                     late_bind_fw);
+       struct xe_device *xe = late_bind_to_xe(late_bind);
+       int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
+       int ret;
+       int slept;
+
+       if (!late_bind->component_added)
+               return;
+
+       if (!lbfw->valid)
+               return;
+
+       /* we can queue this before the component is bound */
+       for (slept = 0; slept < LB_INIT_TIMEOUT_MS; slept += 100) {
+               if (late_bind->component.ops)
+                       break;
+               msleep(100);
+       }
+
+       xe_pm_runtime_get(xe);
+       mutex_lock(&late_bind->mutex);
+
+       if (!late_bind->component.ops) {
+               drm_err(&xe->drm, "Late bind component not bound\n");
+               goto out;
+       }
+
+       drm_dbg(&xe->drm, "Load %s firmware\n", fw_type_to_name[lbfw->type]);
+
+       do {
+               ret = 
late_bind->component.ops->push_config(late_bind->component.mei_dev,
+                                                           lbfw->type, 
lbfw->flags,
+                                                           lbfw->payload, 
lbfw->payload_size);
+               if (!ret)
+                       break;
+               msleep(LB_FW_LOAD_RETRY_PAUSE_MS);
+       } while (--retry && ret == -EBUSY);
+
+       if (ret)
+               drm_err(&xe->drm, "Load %s firmware failed with err %d\n",
+                       fw_type_to_name[lbfw->type], ret);
+       else
+               drm_dbg(&xe->drm, "Load %s firmware successful\n",
+                       fw_type_to_name[lbfw->type]);
+out:
+       mutex_unlock(&late_bind->mutex);
+       xe_pm_runtime_put(xe);
+}
+
+int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
+{
+       struct xe_device *xe = late_bind_to_xe(late_bind);
+       struct xe_late_bind_fw *lbfw;
+
+       if (!late_bind->component_added)
+               return -EINVAL;
+
+       lbfw = &late_bind->late_bind_fw;
+       if (lbfw->valid) {
+               drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
+                       fw_type_to_name[lbfw->type]);
+                       queue_work(late_bind->wq, &lbfw->work);
+       }
+
+       return 0;
+}
+
+/**
+ * late_bind_fw_init() - initialize late bind firmware
+ *
+ * Return: 0 if the initialization was successful, a negative errno otherwise.
+ */
static int late_bind_fw_init(struct xe_late_bind *late_bind, u32 type)
{
        struct xe_device *xe = late_bind_to_xe(late_bind);
@@ -87,6 +186,7 @@ static int late_bind_fw_init(struct xe_late_bind *late_bind, 
u32 type)

        memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
        release_firmware(fw);
+       INIT_WORK(&lb_fw->work, late_bind_work);
        lb_fw->valid = true;

        return 0;
@@ -99,7 +199,17 @@ static int late_bind_fw_init(struct xe_late_bind 
*late_bind, u32 type)
 */
int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
{
-       return late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
+       int ret;
+
+       late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
+       if (!late_bind->wq)
+               return -ENOMEM;
+
+       ret = late_bind_fw_init(late_bind, CSC_LATE_BINDING_TYPE_FAN_CONTROL);
+       if (ret)
+               return ret;
+
+       return xe_late_bind_fw_load(late_bind);
}

static int xe_late_bind_component_bind(struct device *xe_kdev,
@@ -122,6 +232,8 @@ static void xe_late_bind_component_unbind(struct device 
*xe_kdev,
        struct xe_device *xe = kdev_to_xe_device(xe_kdev);
        struct xe_late_bind *late_bind = &xe->late_bind;

+       xe_late_bind_wait_for_worker_completion(late_bind);
+
        mutex_lock(&late_bind->mutex);
        late_bind->component.ops = NULL;
        mutex_unlock(&late_bind->mutex);
@@ -140,9 +252,16 @@ static void xe_late_bind_remove(void *arg)
        if (!late_bind->component_added)
                return;

+       xe_late_bind_wait_for_worker_completion(late_bind);
+
        component_del(xe->drm.dev, &xe_late_bind_component_ops);
        late_bind->component_added = false;
+       if (late_bind->wq) {
+               destroy_workqueue(late_bind->wq);
+               late_bind->wq = NULL;
+       }
        mutex_destroy(&late_bind->mutex);
+
}

/**
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h 
b/drivers/gpu/drm/xe/xe_late_bind_fw.h
index a8b47523b203..166957e84c2f 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
@@ -12,5 +12,6 @@ struct xe_late_bind;

int xe_late_bind_init(struct xe_late_bind *late_bind);
int xe_late_bind_fw_init(struct xe_late_bind *late_bind);
+int xe_late_bind_fw_load(struct xe_late_bind *late_bind);

#endif
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h 
b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
index c427e141c685..690680e8ff22 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -9,6 +9,7 @@
#include <linux/iosys-map.h>
#include <linux/mutex.h>
#include <linux/types.h>
+#include <linux/workqueue.h>

#define MAX_PAYLOAD_SIZE (1024 * 4)

@@ -43,6 +44,8 @@ struct xe_late_bind_fw {
        u8  payload[MAX_PAYLOAD_SIZE];
        /** @late_bind_fw.payload_size: late binding blob payload_size */
        size_t payload_size;
+       /** @late_bind_fw.work: worker to upload latebind blob */
+       struct work_struct work;
};

/**
@@ -71,6 +74,8 @@ struct xe_late_bind {
        struct mutex mutex;
        /** @late_bind.late_bind_fw: late binding firmware */
        struct xe_late_bind_fw late_bind_fw;
+       /** @late_bind.wq: workqueue to submit request to download late bind 
blob */
+       struct workqueue_struct *wq;
};

#endif
--
2.34.1

Reply via email to