Re: [PATCH] drm/rockchip: dsi: Reset ISP1 DPHY before powering it on

Ondřej Jirman
2024-07-17T07:32:57Z

[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]

On Wed, Jul 17, 2024 at 08:48:29AM GMT, Dragan Simic wrote:
> Hello all,
>
> On 2024-07-17 08:29, Dragan Simic wrote:
> > From: Ondrej Jirman <megi@xff.cz>
> >
> > After a suspend and resume cycle, ISP1 stops receiving data, as observed
> > on the Pine64 PinePhone Pro, which is based on the Rockchip RK3399 SoC.
> > Re-initializing DPHY during the PHY power-on, if the SoC variant
> > supports
> > initialization, fixes this issue.
> >
> > [ dsimic: Added more details to the commit summary and description ]
> >
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> > ---
> > drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > index 4cc8ed8f4fbd..9ad48c6dfac3 100644
> > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > @@ -1240,6 +1240,14 @@ static int dw_mipi_dsi_dphy_power_on(struct phy
> > *phy)
> > goto err_phy_cfg_clk;
> > }
> >
> > + if (dsi->cdata->dphy_rx_init) {
> > + ret = dsi->cdata->dphy_rx_init(phy);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dsi->dev, "hardware-specific phy init failed: %d\n",
> > ret);
> > + goto err_pwr_on;
> > + }
> > + }
> > +
> > /* do soc-variant specific init */
> > if (dsi->cdata->dphy_rx_power_on) {
> > ret = dsi->cdata->dphy_rx_power_on(phy);
>
> After thinking a bit more about this patch in its original form [1]
> that's preserved above, I think it would be better to move the
> additional DPHY initialization to dw_mipi_dsi_rockchip_resume(),
> because that function seems to be the right place for such fixes.
>
> Please, let me know your thoughts.

That also works (see attachment) to fix the original issue in the commit
message, but if you keep the stream on across suspend/resume it does halt so
it's not a complete solution either.

Kind regards,
o.

> [1] https://megous.com/git/linux/commit/?h=orange-pi-6.9&id=ed7992f668a1e529719ee6847ca114f9b67efacb

[-- Attachment #2: 0001-drm-rockchip-dw-mipi-dsi-rockchip-Restore-DPHY-confi.patch --]
[-- Type: text/plain, Size: 2657 bytes --]

From 4db7a9a913bb4b78094d7845295d2c3306513c56 Mon Sep 17 00:00:00 2001
From: Ondrej Jirman <megi@xff.cz>
Date: Wed, 17 Jul 2024 09:30:39 +0200
Subject: [PATCH] drm: rockchip: dw-mipi-dsi-rockchip: Restore DPHY config on
resume

After a suspend and resume cycle, ISP1 stops receiving data, as observed on the
Pine64 PinePhone Pro, which is based on the Rockchip RK3399 SoC. Re-initializing
DPHY during resume, if the SoC variant supports initialization, fixes this
issue.

Signed-off-by: Ondrej Jirman <megi@xff.cz>
---
.../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 51 ++++++++++++-------
1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index 364b738b6935..c2b58e545080 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -1118,6 +1118,31 @@ static const struct component_ops dw_mipi_dsi_rockchip_dphy_ops = {
.unbind = dw_mipi_dsi_rockchip_dphy_unbind,
};

+static int dw_mipi_dsi_dphy_rx_init(struct dw_mipi_dsi_rockchip *dsi)
+{
+ int ret;
+
+ if (dsi->cdata->dphy_rx_init) {
+ ret = clk_prepare_enable(dsi->pclk);
+ if (ret < 0)
+ return ret;
+
+ ret = clk_prepare_enable(dsi->grf_clk);
+ if (ret) {
+ clk_disable_unprepare(dsi->pclk);
+ return ret;
+ }
+
+ ret = dsi->cdata->dphy_rx_init(dsi->dphy);
+ clk_disable_unprepare(dsi->grf_clk);
+ clk_disable_unprepare(dsi->pclk);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
static int dw_mipi_dsi_dphy_init(struct phy *phy)
{
struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
@@ -1138,23 +1163,9 @@ static int dw_mipi_dsi_dphy_init(struct phy *phy)
if (ret < 0)
goto err_graph;

- if (dsi->cdata->dphy_rx_init) {
- ret = clk_prepare_enable(dsi->pclk);
- if (ret < 0)
- goto err_init;
-
- ret = clk_prepare_enable(dsi->grf_clk);
- if (ret) {
- clk_disable_unprepare(dsi->pclk);
- goto err_init;
- }
-
- ret = dsi->cdata->dphy_rx_init(phy);
- clk_disable_unprepare(dsi->grf_clk);
- clk_disable_unprepare(dsi->pclk);
- if (ret < 0)
- goto err_init;
- }
+ ret = dw_mipi_dsi_dphy_rx_init(dsi);
+ if (ret < 0)
+ goto err_init;

return 0;

@@ -1337,6 +1348,12 @@ static int __maybe_unused dw_mipi_dsi_rockchip_resume(struct device *dev)
dw_mipi_dsi_rockchip_config(dsi->slave);

clk_disable_unprepare(dsi->grf_clk);
+ } else if (dsi->usage_mode == DW_DSI_USAGE_PHY) {
+ ret = dw_mipi_dsi_dphy_rx_init(dsi);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dsi->dev, "hardware-specific phy init failed: %d\n", ret);
+ return ret;
+ }
}

return 0;
--
2.45.2

[-- Attachment #3: Type: text/plain, Size: 170 bytes --]