Re: [PATCH v4 10/18] drm/imagination: Add reset controller support for GPU initialization
From: Matt Coster <Matt.Coster@imgtec.com>
Date: 2025-01-31 15:59:49
Also in:
dri-devel, linux-clk, linux-pm, linux-riscv, lkml
On 28/01/2025 19:48, Michal Wilczynski wrote:
Certain platforms, such as the T-Head TH1520 and Banana Pi BPI-F3, require a controlled GPU reset sequence during the power-up procedure to ensure proper initialization. Without this reset, the GPU may remain in an undefined state, potentially leading to stability or performance issues.
Can you reword this to clarify that _all_ IMG Rogue GPUs have a reset line that participates in the power-up sequence but some SoCs handle this in silicon and/or firmware without exposing the reset line directly (as the currently supported TI SoC does).
quoted hunk ↗ jump to hunk
This commit integrates a dedicated reset controller within the drm/imagination driver. By doing so, the driver can coordinate the necessary reset operations as part of the normal GPU bring-up process, improving reliability and ensuring that the hardware is ready for operation. Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- drivers/gpu/drm/imagination/pvr_device.c | 21 +++++++++++++++++++++ drivers/gpu/drm/imagination/pvr_device.h | 9 +++++++++ drivers/gpu/drm/imagination/pvr_power.c | 12 +++++++++++- 3 files changed, 41 insertions(+), 1 deletion(-)diff --git a/drivers/gpu/drm/imagination/pvr_device.c b/drivers/gpu/drm/imagination/pvr_device.c index 1704c0268589..ef73e95157ee 100644 --- a/drivers/gpu/drm/imagination/pvr_device.c +++ b/drivers/gpu/drm/imagination/pvr_device.c@@ -25,6 +25,7 @@ #include <linux/interrupt.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/reset.h> #include <linux/slab.h> #include <linux/stddef.h> #include <linux/types.h>@@ -120,6 +121,21 @@ static int pvr_device_clk_init(struct pvr_device *pvr_dev) return 0; } +static int pvr_device_reset_init(struct pvr_device *pvr_dev) +{ + struct drm_device *drm_dev = from_pvr_device(pvr_dev); + struct reset_control *reset; + + reset = devm_reset_control_get_optional_exclusive(drm_dev->dev, NULL); + if (IS_ERR(reset)) + return dev_err_probe(drm_dev->dev, PTR_ERR(reset), + "failed to get gpu reset line\n"); + + pvr_dev->reset = reset; + + return 0; +} + /** * pvr_device_process_active_queues() - Process all queue related events. * @pvr_dev: PowerVR device to check@@ -509,6 +525,11 @@ pvr_device_init(struct pvr_device *pvr_dev) if (err) return err; + /* Get the reset line for the GPU */ + err = pvr_device_reset_init(pvr_dev); + if (err) + return err; + /* Explicitly power the GPU so we can access control registers before the FW is booted. */ err = pm_runtime_resume_and_get(dev); if (err)diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h index 6d0dfacb677b..f6576c08111c 100644 --- a/drivers/gpu/drm/imagination/pvr_device.h +++ b/drivers/gpu/drm/imagination/pvr_device.h@@ -131,6 +131,15 @@ struct pvr_device { */ struct clk *mem_clk; + /** + * @reset: Optional reset line. + * + * This may be used on some platforms to provide a reset line that needs to be de-asserted + * after power-up procedure. It would also need to be asserted after the power-down + * procedure. + */ + struct reset_control *reset; + /** @irq: IRQ number. */ int irq;diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c index ba7816fd28ec..e39460d594bd 100644 --- a/drivers/gpu/drm/imagination/pvr_power.c +++ b/drivers/gpu/drm/imagination/pvr_power.c@@ -15,6 +15,7 @@ #include <linux/mutex.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/reset.h> #include <linux/timer.h> #include <linux/types.h> #include <linux/workqueue.h>@@ -252,6 +253,8 @@ pvr_power_device_suspend(struct device *dev) clk_disable_unprepare(pvr_dev->sys_clk); clk_disable_unprepare(pvr_dev->core_clk); + err = reset_control_assert(pvr_dev->reset); + err_drm_dev_exit: drm_dev_exit(idx);@@ -282,16 +285,23 @@ pvr_power_device_resume(struct device *dev) if (err) goto err_sys_clk_disable;
This is where I'd expect to see the 32 cycle delay that's currently in
P9 ("reset: thead: Add TH1520 reset controller driver").
If it turns out that delay is required in the reset driver, would you be
opposed to adding it here as well? It's a very small amount of time and
would make this codepath more versatile to future reset controllers.
Cheers,
Matt
+ err = reset_control_deassert(pvr_dev->reset);
+ if (err)
+ goto err_mem_clk_disable;
+
if (pvr_dev->fw_dev.booted) {
err = pvr_power_fw_enable(pvr_dev);
if (err)
- goto err_mem_clk_disable;
+ goto err_reset_assert;
}
drm_dev_exit(idx);
return 0;
+err_reset_assert:
+ reset_control_assert(pvr_dev->reset);
+
err_mem_clk_disable:
clk_disable_unprepare(pvr_dev->mem_clk);
-- Matt Coster E: matt.coster@imgtec.com
Attachments
- OpenPGP_signature.asc [application/pgp-signature] 236 bytes