| Message ID | 6e0ce232acfe952970e9b37402fe08a3678aa43a.1721196758.git.dsimic@manjaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/rockchip: dsi: Reset ISP1 DPHY before powering it on | expand |
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. [1] https://megous.com/git/linux/commit/?h=orange-pi-6.9&id=ed7992f668a1e529719ee6847ca114f9b67efacb
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 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;
Hello Ondrej, On 2024-07-17 09:32, Ondřej Jirman wrote: > 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. Great, thanks for the attached patch. I assume that you already have a patch that performs the other required operations on suspend and resume, i.e. stops the stream and restarts it? How about dropping my "handled" variant of your patch and having you submit the patch you sent as attachment, and the additional patch you described as also needed? >> [1] >> https://megous.com/git/linux/commit/?h=orange-pi-6.9&id=ed7992f668a1e529719ee6847ca114f9b67efacb
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);