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

[Adrián Larumbe]

On 2026-10-02 16:10:43+01:00, Steven Price wrote:
> 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>

I looked into the available debugfs attribute definition macros and none of them provide
a wrapper that avoid passing the input value when the attribute is to be understood as
an action rather than a device property. My understanding is that because debugfs doesn't
become part of the device's uAPI, we can do pretty much whatever we want with it.
However, on a second thought, maybe in the future we'll want to extend the knob so that
it performs resets in different ways, and would want to keep backwards compatibility with
UM binaries that expect certain input values to stand for specific actions.

I'll change it so that all value other than '1' are returned with -EINVAL.