| Message ID | 20250227170012.124768-5-marex@denx.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: imx95: Add support for Mali G310 GPU | expand |
Hi Marek, Am Donnerstag, 27. Februar 2025, 17:58:04 CET schrieb Marek Vasut: > The instance of the GPU populated in Freescale i.MX95 does require > release from reset by writing into a single GPUMIX block controller > GPURESET register bit 0. Implement support for one optional reset. > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Boris Brezillon <boris.brezillon@collabora.com> > Cc: Conor Dooley <conor+dt@kernel.org> > Cc: David Airlie <airlied@gmail.com> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Krzysztof Kozlowski <krzk+dt@kernel.org> > Cc: Liviu Dudau <liviu.dudau@arm.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Cc: Rob Herring <robh@kernel.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Sebastian Reichel <sre@kernel.org> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Simona Vetter <simona@ffwll.ch> > Cc: Steven Price <steven.price@arm.com> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: devicetree@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: imx@lists.linux.dev > Cc: linux-arm-kernel@lists.infradead.org > --- > drivers/gpu/drm/panthor/Kconfig | 1 + > drivers/gpu/drm/panthor/panthor_device.c | 23 +++++++++++++++++++++++ > drivers/gpu/drm/panthor/panthor_device.h | 3 +++ > 3 files changed, 27 insertions(+) > > diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig > index 55b40ad07f3b0..ab62bd6a0750f 100644 > --- a/drivers/gpu/drm/panthor/Kconfig > +++ b/drivers/gpu/drm/panthor/Kconfig > @@ -14,6 +14,7 @@ config DRM_PANTHOR > select IOMMU_IO_PGTABLE_LPAE > select IOMMU_SUPPORT > select PM_DEVFREQ > + select RESET_SIMPLE if SOC_IMX9 > help > DRM driver for ARM Mali CSF-based GPUs. > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > index a9da1d1eeb707..51ee9cae94504 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -64,6 +64,17 @@ static int panthor_clk_init(struct panthor_device *ptdev) > return 0; > } > > +static int panthor_reset_init(struct panthor_device *ptdev) > +{ > + ptdev->resets = devm_reset_control_get_optional_exclusive_deasserted(ptdev->base.dev, NULL); If the description as a write-once register is true, wouldn't this already write to it? > + if (IS_ERR(ptdev->resets)) > + return dev_err_probe(ptdev->base.dev, > + PTR_ERR(ptdev->resets), > + "get reset failed"); > + > + return 0; > +} > + > void panthor_device_unplug(struct panthor_device *ptdev) > { > /* This function can be called from two different path: the reset work > @@ -217,6 +228,10 @@ int panthor_device_init(struct panthor_device *ptdev) > if (ret) > return ret; > > + ret = panthor_reset_init(ptdev); > + if (ret) > + return ret; > + > ret = panthor_devfreq_init(ptdev); > if (ret) > return ret; > @@ -470,6 +485,10 @@ int panthor_device_resume(struct device *dev) > if (ret) > goto err_disable_stacks_clk; > > + ret = reset_control_deassert(ptdev->resets); > + if (ret) > + goto err_disable_coregroup_clk; > + This wouldn't work at all on a write-once register, no? Same for resume. Best regards Alexander > panthor_devfreq_resume(ptdev); > > if (panthor_device_is_initialized(ptdev) && > @@ -512,6 +531,9 @@ int panthor_device_resume(struct device *dev) > > err_suspend_devfreq: > panthor_devfreq_suspend(ptdev); > + reset_control_assert(ptdev->resets); > + > +err_disable_coregroup_clk: > clk_disable_unprepare(ptdev->clks.coregroup); > > err_disable_stacks_clk: > @@ -563,6 +585,7 @@ int panthor_device_suspend(struct device *dev) > > panthor_devfreq_suspend(ptdev); > > + reset_control_assert(ptdev->resets); > clk_disable_unprepare(ptdev->clks.coregroup); > clk_disable_unprepare(ptdev->clks.stacks); > clk_disable_unprepare(ptdev->clks.core); > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h > index da6574021664b..fea3a05778e2e 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.h > +++ b/drivers/gpu/drm/panthor/panthor_device.h > @@ -111,6 +111,9 @@ struct panthor_device { > struct clk *coregroup; > } clks; > > + /** @resets: GPU reset. */ > + struct reset_control *resets; Your commit message says "one optional reset", so I would name this just reset. > + > /** @coherent: True if the CPU/GPU are memory coherent. */ > bool coherent; > >
On 2/28/25 11:06 AM, Alexander Stein wrote: Hi, >> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c >> index a9da1d1eeb707..51ee9cae94504 100644 >> --- a/drivers/gpu/drm/panthor/panthor_device.c >> +++ b/drivers/gpu/drm/panthor/panthor_device.c >> @@ -64,6 +64,17 @@ static int panthor_clk_init(struct panthor_device *ptdev) >> return 0; >> } >> >> +static int panthor_reset_init(struct panthor_device *ptdev) >> +{ >> + ptdev->resets = devm_reset_control_get_optional_exclusive_deasserted(ptdev->base.dev, NULL); > > If the description as a write-once register is true, wouldn't this > already write to it? It would. I believe the reset handling in the GPU driver should be generic. The reset has to be deasserted here to access the GPU registers. The question is, whether the GPUMIX GPURESET bit should be modeled as simple reset, even if it really behaves as a set-and-never-can-be-cleared latched bit. >> + if (IS_ERR(ptdev->resets)) >> + return dev_err_probe(ptdev->base.dev, >> + PTR_ERR(ptdev->resets), >> + "get reset failed"); >> + >> + return 0; >> +} >> + >> void panthor_device_unplug(struct panthor_device *ptdev) >> { >> /* This function can be called from two different path: the reset work >> @@ -217,6 +228,10 @@ int panthor_device_init(struct panthor_device *ptdev) >> if (ret) >> return ret; >> >> + ret = panthor_reset_init(ptdev); >> + if (ret) >> + return ret; >> + >> ret = panthor_devfreq_init(ptdev); >> if (ret) >> return ret; >> @@ -470,6 +485,10 @@ int panthor_device_resume(struct device *dev) >> if (ret) >> goto err_disable_stacks_clk; >> >> + ret = reset_control_deassert(ptdev->resets); >> + if (ret) >> + goto err_disable_coregroup_clk; >> + > > This wouldn't work at all on a write-once register, no? Same for resume. See above and also my reply to 2/9 .
diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig index 55b40ad07f3b0..ab62bd6a0750f 100644 --- a/drivers/gpu/drm/panthor/Kconfig +++ b/drivers/gpu/drm/panthor/Kconfig @@ -14,6 +14,7 @@ config DRM_PANTHOR select IOMMU_IO_PGTABLE_LPAE select IOMMU_SUPPORT select PM_DEVFREQ + select RESET_SIMPLE if SOC_IMX9 help DRM driver for ARM Mali CSF-based GPUs. diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c index a9da1d1eeb707..51ee9cae94504 100644 --- a/drivers/gpu/drm/panthor/panthor_device.c +++ b/drivers/gpu/drm/panthor/panthor_device.c @@ -64,6 +64,17 @@ static int panthor_clk_init(struct panthor_device *ptdev) return 0; } +static int panthor_reset_init(struct panthor_device *ptdev) +{ + ptdev->resets = devm_reset_control_get_optional_exclusive_deasserted(ptdev->base.dev, NULL); + if (IS_ERR(ptdev->resets)) + return dev_err_probe(ptdev->base.dev, + PTR_ERR(ptdev->resets), + "get reset failed"); + + return 0; +} + void panthor_device_unplug(struct panthor_device *ptdev) { /* This function can be called from two different path: the reset work @@ -217,6 +228,10 @@ int panthor_device_init(struct panthor_device *ptdev) if (ret) return ret; + ret = panthor_reset_init(ptdev); + if (ret) + return ret; + ret = panthor_devfreq_init(ptdev); if (ret) return ret; @@ -470,6 +485,10 @@ int panthor_device_resume(struct device *dev) if (ret) goto err_disable_stacks_clk; + ret = reset_control_deassert(ptdev->resets); + if (ret) + goto err_disable_coregroup_clk; + panthor_devfreq_resume(ptdev); if (panthor_device_is_initialized(ptdev) && @@ -512,6 +531,9 @@ int panthor_device_resume(struct device *dev) err_suspend_devfreq: panthor_devfreq_suspend(ptdev); + reset_control_assert(ptdev->resets); + +err_disable_coregroup_clk: clk_disable_unprepare(ptdev->clks.coregroup); err_disable_stacks_clk: @@ -563,6 +585,7 @@ int panthor_device_suspend(struct device *dev) panthor_devfreq_suspend(ptdev); + reset_control_assert(ptdev->resets); clk_disable_unprepare(ptdev->clks.coregroup); clk_disable_unprepare(ptdev->clks.stacks); clk_disable_unprepare(ptdev->clks.core); diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h index da6574021664b..fea3a05778e2e 100644 --- a/drivers/gpu/drm/panthor/panthor_device.h +++ b/drivers/gpu/drm/panthor/panthor_device.h @@ -111,6 +111,9 @@ struct panthor_device { struct clk *coregroup; } clks; + /** @resets: GPU reset. */ + struct reset_control *resets; + /** @coherent: True if the CPU/GPU are memory coherent. */ bool coherent;
The instance of the GPU populated in Freescale i.MX95 does require release from reset by writing into a single GPUMIX block controller GPURESET register bit 0. Implement support for one optional reset. Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Boris Brezillon <boris.brezillon@collabora.com> Cc: Conor Dooley <conor+dt@kernel.org> Cc: David Airlie <airlied@gmail.com> Cc: Fabio Estevam <festevam@gmail.com> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org> Cc: Liviu Dudau <liviu.dudau@arm.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> Cc: Philipp Zabel <p.zabel@pengutronix.de> Cc: Rob Herring <robh@kernel.org> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Sebastian Reichel <sre@kernel.org> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Simona Vetter <simona@ffwll.ch> Cc: Steven Price <steven.price@arm.com> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: devicetree@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: imx@lists.linux.dev Cc: linux-arm-kernel@lists.infradead.org --- drivers/gpu/drm/panthor/Kconfig | 1 + drivers/gpu/drm/panthor/panthor_device.c | 23 +++++++++++++++++++++++ drivers/gpu/drm/panthor/panthor_device.h | 3 +++ 3 files changed, 27 insertions(+)