Re: [PATCH v2 08/10] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 6/6/2025 10:57 AM, Badal Nilawar wrote:
> Introduce a debug filesystem node to disable late binding fw reload
> during the system or runtime resume. This is intended for situations
> where the late binding fw needs to be loaded from user mode.

Can you clarify a bit what the use-case here is? Because this 
description makes it sound like this is something a user might want to 
do, but the debugfs interface is for testing only. If we only need this 
for testing, please specify so.

>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_debugfs.c            | 42 ++++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_late_bind_fw.c       |  5 ++-
>   drivers/gpu/drm/xe/xe_late_bind_fw_types.h |  3 ++
>   3 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index d83cd6ed3fa8..1c145089d177 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -226,6 +226,45 @@ static const struct file_operations atomic_svm_timeslice_ms_fops = {
>   	.write = atomic_svm_timeslice_ms_set,
>   };
>   
> +static ssize_t disable_late_binding_show(struct file *f, char __user *ubuf,
> +					 size_t size, loff_t *pos)
> +{
> +	struct xe_device *xe = file_inode(f)->i_private;
> +	struct xe_late_bind *late_bind = &xe->late_bind;
> +	char buf[32];
> +	int len;
> +
> +	len = scnprintf(buf, sizeof(buf), "%d\n", late_bind->disable);
> +
> +	return simple_read_from_buffer(ubuf, size, pos, buf, len);
> +}
> +
> +static ssize_t disable_late_binding_set(struct file *f, const char __user *ubuf,
> +					size_t size, loff_t *pos)
> +{
> +	struct xe_device *xe = file_inode(f)->i_private;
> +	struct xe_late_bind *late_bind = &xe->late_bind;
> +	u32 uval;
> +	ssize_t ret;
> +
> +	ret = kstrtouint_from_user(ubuf, size, sizeof(uval), &uval);
> +	if (ret)
> +		return ret;
> +
> +	if (uval > 1)
> +		return -EINVAL;
> +
> +	late_bind->disable = (uval == 1) ? true : false;

nit: could do

late_bind->disable = !!uval;

> +
> +	return size;
> +}
> +
> +static const struct file_operations disable_late_binding_fops = {
> +	.owner = THIS_MODULE,
> +	.read = disable_late_binding_show,
> +	.write = disable_late_binding_set,
> +};
> +
>   void xe_debugfs_register(struct xe_device *xe)
>   {
>   	struct ttm_device *bdev = &xe->ttm;
> @@ -249,6 +288,9 @@ void xe_debugfs_register(struct xe_device *xe)
>   	debugfs_create_file("atomic_svm_timeslice_ms", 0600, root, xe,
>   			    &atomic_svm_timeslice_ms_fops);
>   
> +	debugfs_create_file("disable_late_binding", 0600, root, xe,
> +			    &disable_late_binding_fops);
> +
>   	for (mem_type = XE_PL_VRAM0; mem_type <= XE_PL_VRAM1; ++mem_type) {
>   		man = ttm_manager_type(bdev, mem_type);
>   
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> index d69f950bddd2..f9d3d0f341f2 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -123,11 +123,14 @@ int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
>   	if (!late_bind->component_added)
>   		return -EINVAL;
>   
> +	if (late_bind->disable)
> +		return 0;
> +
>   	lbfw = &late_bind->late_bind_fw;
>   	if (lbfw->valid) {
>   		drm_dbg(&xe->drm, "Queue work: to load %s firmware\n",
>   			fw_type_to_name[lbfw->type]);
> -			queue_work(late_bind->wq, &lbfw->work);
> +		queue_work(late_bind->wq, &lbfw->work);

This seems to be a fix for a broken indent in a previous patch in the 
series, so it should be moved to that patch.

Daniele

>   	}
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> index 690680e8ff22..6f4a945ca236 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> @@ -76,6 +76,9 @@ struct xe_late_bind {
>   	struct xe_late_bind_fw late_bind_fw;
>   	/** @late_bind.wq: workqueue to submit request to download late bind blob */
>   	struct workqueue_struct *wq;
> +
> +	/** @late_bind.disable to block late binding reload during pm resume flow*/
> +	bool disable;
>   };
>   
>   #endif