Re: [PATCH v9 01/16] drm/panfrost: Move shrinker initialization and unplug one level down
From: Boris Brezillon
Date: Wed Sep 23 2026 - 03:02:18 EST
On Tue, 22 Sep 2026 20:48:49 +0100
Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx> wrote:
> On 14.09.2026 10:36, Boris Brezillon wrote:
> > On Sat, 12 Sep 2026 00:28:02 +0100
> > Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx> wrote:
> >
> > > Since the moment we call drm_dev_register() the device should be in a
> > > position to accept jobs, so it's best if the shrinker is already
> > > initialized by then.
> > >
> > > On top of that, make shrinker functions take an panfrost_device pointer
> > > like other functions in the same sequence and rename them accordingly.
> > >
> > > Essentially mimic the init/fini behaviour in Panthor.
> > >
> > > Signed-off-by: Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx>
> >
> > Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> >
> > One remark below.
> >
> > > ---
> > > drivers/gpu/drm/panfrost/panfrost_device.c | 8 +++++++-
> > > drivers/gpu/drm/panfrost/panfrost_drv.c | 6 ------
> > > drivers/gpu/drm/panfrost/panfrost_gem.c | 26 ++++++++++++++----------
> > > drivers/gpu/drm/panfrost/panfrost_gem.h | 7 ++++---
> > > drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 8 ++------
> > > 5 files changed, 28 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> > > index 485349faf251..05c40d5a20b5 100644
> > > --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> > > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > > @@ -280,9 +280,14 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> > > if (err)
> > > goto out_job;
> > >
> > > - panfrost_gem_init(pfdev);
> > > + err = panfrost_gem_init(pfdev);
> >
> > It feels weird to have the GEM subsystem initialized last when you
> > consider the fact other subsystems might want to allocate GEMs in their
> > _init() function. I know it's where the panfrost_gem_init() is right
> > now, and that ultimately it doesn't prevent anyone from allocating
> > GEMs, but I think it would make sense have this called before any of
> > the other subsystem init functions, still.
>
> I think we discussed having panfrost_gem_init() be called before the other
> subsystem init functions, but then I checked panthor and saw it's also being
> called right after all the others there too. I think you're right that it's
> best to initialise gem first, because I wonder whether other subsystem init
> functions creating GEMs when the mount point for huge page-backed objects
> hasn't been created yet could lead to some sort of trouble.
>
> However, given how much this patch series has already grown, I believe it'd
> be best to leave this change for a later one.
Sounds good to me.