Re: [PATCH v2 2/7] drm: rcar-du: lvds: Add runtime PM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20/01/2023 18:16, Laurent Pinchart wrote:
Hi Tomi,

Thank you for the patch.

On Fri, Jan 20, 2023 at 10:50:04AM +0200, Tomi Valkeinen wrote:
Add simple runtime PM suspend and resume functionality.

I think you need to depend on PM in Kconfig. That's not a compile-time
dependency but a runtime-dependency, with runtime PM support the
suspend/resume handler will never be called.

Yes, LVDS won't work without runtime PM after this patch.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/rcar-du/rcar_lvds.c | 43 +++++++++++++++++++++++++----
  1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 81a060c2fe3f..8e1be51fbee6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -16,6 +16,7 @@
  #include <linux/of_device.h>
  #include <linux/of_graph.h>
  #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
  #include <linux/slab.h>
  #include <linux/sys_soc.h>
@@ -316,8 +317,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq) dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq); - ret = clk_prepare_enable(lvds->clocks.mod);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret)
  		return ret;
__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
@@ -337,7 +338,7 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
rcar_lvds_write(lvds, LVDPLLCR, 0); - clk_disable_unprepare(lvds->clocks.mod);
+	pm_runtime_put(lvds->dev);

Should we use pm_runtime_put_sync() here, to make sure the clock gets
disabled right away ? The DU hardware may depend on the exact sequencing
of events. I would then do the same in rcar_lvds_atomic_disable().

That's perhaps a good idea. I think I saw some of the docs saying that startup sequences must begin with the reset. If we don't use _sync, we could end up not resetting at all.

Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

Thanks!

 Tomi




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux