On Tue, 22 Sep 2026 20:54:11 +0100 Adrián Larumbe <[email protected]> wrote:
> On 14.09.2026 11:39, Boris Brezillon wrote: > > On Sat, 12 Sep 2026 00:28:11 +0100 > > Adrián Larumbe <[email protected]> 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. > > > > > > Signed-off-by: Adrián Larumbe <[email protected]> > > > > Reviewed-by: Boris Brezillon <[email protected]> > > > > One comment below. > > > > > --- > > > drivers/gpu/drm/panfrost/panfrost_device.c | 37 > > > ++++++++++++++++++++++++++++++ > > > 1 file changed, 37 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > > > b/drivers/gpu/drm/panfrost/panfrost_device.c > > > index d75566a45392..0aee1d87a7ab 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> > > > @@ -583,9 +584,45 @@ 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) { > > > + panfrost_device_schedule_reset(pfdev); > > > > It feels weird to schedule a reset if the device is suspended, because > > a suspend/resume cycle already acts as a reset. Any particular reason > > you'd want to do that? If not, I'd suggest going for: > > That's been my goal all along, for the knob to trigger a reset only when > the device isn't suspended. pm_runtime_get_if_active() docs states that: > > """ > Increment the runtime PM usage counter of @dev if its runtime PM status is > %RPM_ACTIVE, in which case it returns 1. If the device is in a different > state, 0 is returned. -EINVAL is returned if runtime PM is disabled for the > device, in which case also the usage_count will remain unmodified. > """ > > So (ret != 0) means the device either doesn't support RPM or else it > was in the ACTIVE state. In both cases, that means the device is powered > so we can proceed with the reset. However, I only decrement the usage > counter when kernel RPM is enabled (ret > 1). Okay, I see. Can we have a comment explaining that? Also, I think I prefer an early bail out over the extra indentation level added here. if (!ret) return 0; Oh, and we should probably make this knob inactive in the device removal path (with a disable_work()?), otherwise -EINVAL will be returned as soon as pm_runtime_disable() is called, and this might cause a reset to be scheduled, which we probably don't want, since it would interfere with our device_fini() logic. > > > if (ret <= 0) > > return ret; > > > > panfrost_device_schedule_reset(pfdev); > > pm_runtime_put(pfdev->base.dev); > > return 0; > > > > > + flush_work(&pfdev->reset.work); > > > + if (ret > 0) > > > + pm_runtime_put(pfdev->base.dev); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +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 > > > > > Adrian Larumbe
