Re: [PATCH v12 10/15] drm/panfrost: Add debugfs knob for manually triggering a GPU reset

[Steven Price]

On 29/09/2026 04:44, Adrián Larumbe wrote:
> This will be of great help when testing potential races between the GPU
> reset sequence and other parts of the code accessing HW registers.
> 
> We must also disable the reset work item rather than simply cancelling
> it, to prevent the knob from triggering another reset when the device
> is being removed.
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 42 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/panfrost/panfrost_job.c    |  2 +-
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 09a5752a3f40..94d2de341838 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -2,6 +2,7 @@
>  /* Copyright 2018 Marty E. Plummer <hanetzer@xxxxxxxxxxxxx> */
>  /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@xxxxxxxxxx> */
>  
> +#include <linux/debugfs.h>
>  #include <linux/clk.h>
>  #include <linux/reset.h>
>  #include <linux/platform_device.h>
> @@ -595,9 +596,50 @@ EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = {
>  };
>  
>  #ifdef CONFIG_DEBUG_FS
> +static int reset_get(void *data, u64 *val)
> +{
> +	struct panfrost_device *pfdev =
> +		container_of(data, struct panfrost_device, base);
> +
> +	*val = atomic_read(&pfdev->reset.pending);
> +	return 0;
> +}
> +
> +static int reset_set(void *data, u64 val)
> +{
> +	struct panfrost_device *pfdev =
> +		container_of(data, struct panfrost_device, base);
> +	int ret = pm_runtime_get_if_active(pfdev->base.dev);
> +
> +	if (!ret)
> +		return 0;
> +
> +	panfrost_device_schedule_reset(pfdev);
> +	flush_work(&pfdev->reset.work);
> +
> +	/* ret < 0 means runtime PM for the device is disabled, so we
> +	 * only need to return the PM reference in the opposite case
> +	 */
> +	if (ret > 0)
> +		pm_runtime_put(pfdev->base.dev);
> +
> +	return 0;
> +}

NIT: If this wasn't debugfs I'd be complaining that you're ignoring
'val' and so this isn't very extensible. But hey, it's debugfs... so:

Reviewed-by: Steven Price <steven.price@xxxxxxx>

> +
> +DEFINE_DEBUGFS_ATTRIBUTE(panfrost_reset_debugfs_fops,
> +			 reset_get, reset_set,
> +			 "0x%08llx\n");
> +
> +static void panfrost_reset_debugfs_init(struct drm_minor *minor)
> +{
> +	debugfs_create_file("reset", 0600, minor->debugfs_root,
> +			    minor->dev, &panfrost_reset_debugfs_fops);
> +}
> +
>  void panfrost_device_debugfs_init(struct drm_minor *minor)
>  {
>  	panfrost_gem_debugfs_init(minor);
>  	panfrost_sched_debugfs_init(minor);
> +	panfrost_reset_debugfs_init(minor);
>  }
>  #endif // CONFIG_DEBUG_FS
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> index 00fb36c577df..ab65786bf63b 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> @@ -932,7 +932,7 @@ void panfrost_jm_fini(struct panfrost_device *pfdev)
>  		drm_sched_fini(&js->queue[j].sched);
>  	}
>  
> -	cancel_work_sync(&pfdev->reset.work);
> +	disable_work_sync(&pfdev->reset.work);
>  	destroy_workqueue(pfdev->reset.wq);
>  }
>  
>