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 <[email protected]>
> > Signed-off-by: Adrián Larumbe <[email protected]>
> > ---
> >  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 <[email protected]> */
> >  /* Copyright 2019 Linaro, Ltd, Rob Herring <[email protected]> */
> >  
> > +#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 <[email protected]>

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.


Reply via email to