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

Dragan Simic
2024-07-17T08:45:41Z

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