Re: [PATCH v2 2/2] drm/tegra: gr2d/gr3d: Contain PM in the gr*d_probe/gr*d_remove

[Mikko Perttunen]

On Monday, May 4, 2026 3:36 AM Svyatoslav Ryhel wrote:
> нд, 3 трав. 2026 р. о 19:38 Svyatoslav Ryhel <clamor95@xxxxxxxxx> пише:
> >
> > From: Ion Agorria <ion@xxxxxxxxxxx>
> >
> > The current power management configuration causes GR2G/GR3D to malfunction
> > after resume. Reconfigure all PM actions to be handled within the GR*D
> > probe and remove operations to address this.
> >
> > Fixes: 62fa0a985e2c ("drm/tegra: Enable runtime PM during probe")
> > Acked-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
> > Signed-off-by: Ion Agorria <ion@xxxxxxxxxxx>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/tegra/gr2d.c | 11 ++++-------
> >  drivers/gpu/drm/tegra/gr3d.c | 11 ++++-------
> >  2 files changed, 8 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c
> > index e4148b034af7..ffcd076b5831 100644
> > --- a/drivers/gpu/drm/tegra/gr2d.c
> > +++ b/drivers/gpu/drm/tegra/gr2d.c
> > @@ -100,9 +100,6 @@ static int gr2d_exit(struct host1x_client *client)
> >         if (err < 0)
> >                 return err;
> >
> > -       pm_runtime_dont_use_autosuspend(client->dev);
> > -       pm_runtime_force_suspend(client->dev);
> > -
> >         host1x_client_iommu_detach(client);
> >         host1x_syncpt_put(client->syncpts[0]);
> >         host1x_channel_put(gr2d->channel);
> > @@ -286,6 +283,10 @@ static int gr2d_probe(struct platform_device *pdev)
> >                 return err;
> >         }
> >
> > +       pm_runtime_enable(dev);
> > +       pm_runtime_use_autosuspend(dev);
> > +       pm_runtime_set_autosuspend_delay(dev, 500);
> > +
> 
> Hello Mikko!

Hello!

> 
> I have used same setup as in VIC. May you please take a look to sashiko's check
> https://sashiko.dev/#/patchset/20260502124055.22475-1-clamor95%40gmail.com
> 
> I do agree with statement that pm_runtime_enable should be before
> host1x_client_register since this same approach is widely used in the
> media subsystem too.

Yep, I agree as well.

> 
> But I am more interested in your thoughts regarding sashiko's
> gr2d_exit situation reasoning.

It sounds accurate to me, there is probably a real issue. I expect
there to be in general some driver unbind related issues, since it
(particularly when the device is in use) isn't really being tested.

FWIW, in an ideal world, it would probably make more sense to do the
IOMMU handling on the platform device side, since the device's memory
accesses are not related to host1x. But we can't really do that right
now.

For the time being, I don't think fixing this is particularly critical
since other things will go wrong as well if root causes .exit to happen
while the engine is executing. I can't say offhand what the proper full
solution would be.

Thanks
Mikko

> 
> Thank you!
> 
> >         return 0;
> >  }
> >
> > @@ -367,10 +368,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 47b0c6c56bfd..cd5554e2117f 100644
> > --- a/drivers/gpu/drm/tegra/gr3d.c
> > +++ b/drivers/gpu/drm/tegra/gr3d.c
> > @@ -109,9 +109,6 @@ static int gr3d_exit(struct host1x_client *client)
> >         if (err < 0)
> >                 return err;
> >
> > -       pm_runtime_dont_use_autosuspend(client->dev);
> > -       pm_runtime_force_suspend(client->dev);
> > -
> >         host1x_client_iommu_detach(client);
> >         host1x_syncpt_put(client->syncpts[0]);
> >         host1x_channel_put(gr3d->channel);
> > @@ -517,6 +514,10 @@ static int gr3d_probe(struct platform_device *pdev)
> >                 return err;
> >         }
> >
> > +       pm_runtime_enable(&pdev->dev);
> > +       pm_runtime_use_autosuspend(&pdev->dev);
> > +       pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
> > +
> >         return 0;
> >  }
> >
> > @@ -578,10 +579,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
> >