[PATCH v2 0/2] drm/tegra: gr2d/gr3d: Move pm_runtime_enable to gr*d_probe

[PATCH v2 0/2] drm/tegra: gr2d/gr3d: Move pm_runtime_enable to gr*d_probe

[Svyatoslav Ryhel]

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().

---
Changes in v2:
- remove pm from gr2d_exit along with pm from gr2d_runtime_resume
- move register map initialize before host1x_client_register
---

Ion Agorria (1):
  drm/tegra: gr2d/gr3d: Contain PM in the gr*d_probe/gr*d_remove

Svyatoslav Ryhel (1):
  drm/tegra: gr2d/gr3d: Initialize address register map before HOST1X
    client is registered

 drivers/gpu/drm/tegra/gr2d.c | 17 +++++++----------
 drivers/gpu/drm/tegra/gr3d.c | 17 +++++++----------
 2 files changed, 14 insertions(+), 20 deletions(-)

-- 
2.51.0

[PATCH v2 1/2] drm/tegra: gr2d/gr3d: Initialize address register map before HOST1X client is registered

[Svyatoslav Ryhel]

The host1x_client_register() function is called just prior to register map
initialization loop, making the device available to userspace. This may
result in userspace attempting to submits a job before the register map is
initialized. Address this by moving register initialization before host1x
client registration.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 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..e4148b034af7 100644
--- a/drivers/gpu/drm/tegra/gr2d.c
+++ b/drivers/gpu/drm/tegra/gr2d.c
@@ -276,16 +276,16 @@ static int gr2d_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	/* initialize address register map */
+	for (i = 0; i < ARRAY_SIZE(gr2d_addr_regs); i++)
+		set_bit(gr2d_addr_regs[i], gr2d->addr_regs);
+
 	err = host1x_client_register(&gr2d->client.base);
 	if (err < 0) {
 		dev_err(dev, "failed to register host1x client: %d\n", err);
 		return err;
 	}
 
-	/* initialize address register map */
-	for (i = 0; i < ARRAY_SIZE(gr2d_addr_regs); i++)
-		set_bit(gr2d_addr_regs[i], gr2d->addr_regs);
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index 42e9656ab80c..47b0c6c56bfd 100644
--- a/drivers/gpu/drm/tegra/gr3d.c
+++ b/drivers/gpu/drm/tegra/gr3d.c
@@ -506,6 +506,10 @@ static int gr3d_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	/* initialize address register map */
+	for (i = 0; i < ARRAY_SIZE(gr3d_addr_regs); i++)
+		set_bit(gr3d_addr_regs[i], gr3d->addr_regs);
+
 	err = host1x_client_register(&gr3d->client.base);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
@@ -513,10 +517,6 @@ static int gr3d_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	/* initialize address register map */
-	for (i = 0; i < ARRAY_SIZE(gr3d_addr_regs); i++)
-		set_bit(gr3d_addr_regs[i], gr3d->addr_regs);
-
 	return 0;
 }
 
-- 
2.51.0

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

[Svyatoslav Ryhel]

From: Ion Agorria <ion@agorria.com>

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@nvidia.com>
Signed-off-by: Ion Agorria <ion@agorria.com>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 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);
+
 	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

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

[Svyatoslav Ryhel]

нд, 3 трав. 2026 р. о 19:38 Svyatoslav Ryhel <clamor95@gmail.com> пише:
>
> From: Ion Agorria <ion@agorria.com>
>
> 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@nvidia.com>
> Signed-off-by: Ion Agorria <ion@agorria.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  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!

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.

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

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
>

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@gmail.com> пише:
> >
> > From: Ion Agorria <ion@agorria.com>
> >
> > 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@nvidia.com>
> > Signed-off-by: Ion Agorria <ion@agorria.com>
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  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
> >