Re: [PATCH v1 1/1] drm/tegra: gr2d/gr3d: Move pm_runtime_enable to gr*d_probe

[Svyatoslav Ryhel]

вт, 28 квіт. 2026 р. о 04:53 Mikko Perttunen <mperttunen@xxxxxxxxxx> пише:
>
> On Monday, April 27, 2026 4:58 PM Svyatoslav Ryhel wrote:
> > From: Ion Agorria <ion@xxxxxxxxxxx>
> >
> > The gr*d_remove() has pm_runtime_disable, this indicates it should be
> > paired with pm_runtime_enable in the probe instead of being inside
> > gr*d_runtime_resume().
> >
> > Signed-off-by: Ion Agorria <ion@xxxxxxxxxxx>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/tegra/gr2d.c | 8 ++++----
> >  drivers/gpu/drm/tegra/gr3d.c | 8 ++++----
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
> > index 21f4dd0fa6af..71f092d59d65 100644
> > --- a/drivers/gpu/drm/tegra/gr2d.c
> > +++ b/drivers/gpu/drm/tegra/gr2d.c
> > @@ -286,6 +286,10 @@ static int gr2d_probe(struct platform_device *pdev)
> >       for (i = 0; i < ARRAY_SIZE(gr2d_addr_regs); i++)
> >               set_bit(gr2d_addr_regs[i], gr2d->addr_regs);
> >
> > +     pm_runtime_enable(dev);
> > +     pm_runtime_use_autosuspend(dev);
> > +     pm_runtime_set_autosuspend_delay(dev, 500);
> > +
> >       return 0;
> >  }
> >
> > @@ -367,10 +371,6 @@ static int __maybe_unused gr2d_runtime_resume(struct device *dev)
> >               goto disable_clk;
> >       }
> >
> > -     pm_runtime_enable(dev);
> > -     pm_runtime_use_autosuspend(dev);
> > -     pm_runtime_set_autosuspend_delay(dev, 500);
> > -
> >       return 0;
> >
> >  disable_clk:
> > diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
> > index 42e9656ab80c..33e88ca4d4c5 100644
> > --- a/drivers/gpu/drm/tegra/gr3d.c
> > +++ b/drivers/gpu/drm/tegra/gr3d.c
> > @@ -517,6 +517,10 @@ static int gr3d_probe(struct platform_device *pdev)
> >       for (i = 0; i < ARRAY_SIZE(gr3d_addr_regs); i++)
> >               set_bit(gr3d_addr_regs[i], gr3d->addr_regs);
> >
> > +     pm_runtime_enable(&pdev->dev);
> > +     pm_runtime_use_autosuspend(&pdev->dev);
> > +     pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
> > +
> >       return 0;
> >  }
> >
> > @@ -578,10 +582,6 @@ static int __maybe_unused gr3d_runtime_resume(struct device *dev)
> >               goto disable_clk;
> >       }
> >
> > -     pm_runtime_enable(dev);
> > -     pm_runtime_use_autosuspend(dev);
> > -     pm_runtime_set_autosuspend_delay(dev, 500);
> > -
> >       return 0;
> >
> >  disable_clk:
> > --
> > 2.51.0
> >
> >
>
> Oof, looks like I had managed to really bungle this with my earlier
> patch. Thanks for fixing it!
>

Hello Mikko!

Thank you for taking time and looking into this patch. Don't be so
harsh to yourself, PM is easy to mess and hard to set properly. This
patch does fix gr*d access but it does not resolve the issue itself.
PM should be set in the init/exit rather then probe/remove. I have v2
which fixes this and one more minor issue and I will send them later
on.

So for now this patch should not be picked.

Best regards,
Svyatoslav R.

> FWIW, I've been working on adding some nightly testing for Host1x/
> TegraDRM, so hopefully we'll be able to catch such things easier
> in the future.
>
> Fixes: 62fa0a985e2c ("drm/tegra: Enable runtime PM during probe")
> Acked-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
>
>
>