On 12.08.2026 11:14, Boris Brezillon wrote:
> On Tue, 11 Aug 2026 22:42:15 +0100
> Adrián Larumbe <[email protected]> wrote:
> 
> > Because the device must be in a position to accept jobs between the time
> > drm_dev_register() is called and autosuspend first kicks in, there's a very
> > narrow window inbetween during which jobs targeting the tiler buffer
> > object would time out, since the device's PM status is 'Active', but no MMU
> > interrupts were enabled at device initialisation time.
> 
> Reviewed-by: Boris Brezillon <[email protected]>
> 
> It's probably good to have the IRQ regs initialized in the init()
> function, though I'd like to be sure this is enough. If we assume
> that a reset is what it takes to have a functional device, shouldn't we
> do this reset in the device_init() path, before drm_dev_register() is
> called?

Do you mean doing it right before returning from panfrost_device_init()?
I thought about this, that maybe all device interrupts (gpu, mmu, jm) should be
enabled only when all the other subsystems are initialised, perhaps in a sort of
panfrost_device_init_initerrupts() function at the very end of 
panfrost_device_init().

Some of the pre-existing issues detected by Sashiko were about potential nasty 
interactions
of the IRQ handler when triggered by a spurious interrupt at the time some of 
the driver
structures aren't fully initialised, so this might be good. What do you think?

> > 
> > Signed-off-by: Adrián Larumbe <[email protected]>
> > Fixes: 73e467f60acd ("drm/panfrost: Consolidate reset handling")
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_mmu.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c 
> > b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > index 3fe37dd12360..a8ce5d65825c 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> > @@ -336,6 +336,12 @@ void panfrost_mmu_as_put(struct panfrost_device 
> > *pfdev, struct panfrost_mmu *mmu
> >     WARN_ON(atomic_read(&mmu->as_count) < 0);
> >  }
> >  
> > +static void panfrost_mmu_enable_interrupts(struct panfrost_device *pfdev)
> > +{
> > +   mmu_write(pfdev, MMU_INT_CLEAR, ~0);
> > +   mmu_write(pfdev, MMU_INT_MASK, ~0);
> > +}
> > +
> >  void panfrost_mmu_reset(struct panfrost_device *pfdev)
> >  {
> >     struct panfrost_mmu *mmu, *mmu_tmp;
> > @@ -355,8 +361,7 @@ void panfrost_mmu_reset(struct panfrost_device *pfdev)
> >  
> >     spin_unlock(&pfdev->as_lock);
> >  
> > -   mmu_write(pfdev, MMU_INT_CLEAR, ~0);
> > -   mmu_write(pfdev, MMU_INT_MASK, ~0);
> > +   panfrost_mmu_enable_interrupts(pfdev);
> >  }
> >  
> >  static size_t get_pgsize(u64 addr, size_t size, size_t *count)
> > @@ -970,6 +975,8 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
> >             return err;
> >     }
> >  
> > +   panfrost_mmu_enable_interrupts(pfdev);
> > +
> >     return 0;
> >  }
> >  
> > 

Reply via email to