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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <287caa145232a310c0e40a8b761bcd9a@manjaro.org>
Date: Wed, 17 Jul 2024 09:41:50 +0200
From: Dragan Simic <dsimic@...jaro.org>
To: Dragan Simic <dsimic@...jaro.org>
Cc: linux-rockchip@...ts.infradead.org, dri-devel@...ts.freedesktop.org,
 heiko@...ech.de, hjc@...k-chips.com, andy.yan@...k-chips.com,
 maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
 airlied@...il.com, daniel@...ll.ch, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/rockchip: dsi: Reset ISP1 DPHY before powering it on

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@....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@....cz>
>> > Signed-off-by: Dragan Simic <dsimic@...jaro.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