From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.hugovil.com (mail.hugovil.com [162.243.120.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 414E1EAC7; Tue, 17 Mar 2026 20:54:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=162.243.120.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773780885; cv=none; b=doq8PZCjVEpkIQmddIo2sUtrlM8quKsTOTge87JGTRTTgth6m/2RVudWg7s6U4oyZbUjBlKYKXBR3u+MC6C1ANyByjAGU+8j+K+UO3ukwhnJ+5VCS9uQ3APBtf3uct8nwrhY0zoPtAPkoD4IVRnwgAk4Rc4oC813VWrTPIF8Urc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773780885; c=relaxed/simple; bh=V30PQIUPTTUWfrKyCBsXqtxDpRC/oumixEaWvJOmA6k=; h=Date:From:To:Cc:Subject:Message-Id:In-Reply-To:References: Mime-Version:Content-Type; b=DEezVBAoqxrb9IaV2oEi09nBJ0Vy4qpgy3HEPVVVCelEFoPoyxxuVvc8CI4Bnx2XJ7+gUDq6V2yOijR0GtaXKkyTI0crwGvo/xGDdiY/FsbLI/fNzuKOXob5Tl4gpvnFet2Dlnr5/8U0Da7iTTMkJIWRgkEGw3UOb6b8YcBNHZc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=hugovil.com; spf=pass smtp.mailfrom=hugovil.com; dkim=pass (1024-bit key) header.d=hugovil.com header.i=@hugovil.com header.b=SuSWHbR4; arc=none smtp.client-ip=162.243.120.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=hugovil.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=hugovil.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=hugovil.com header.i=@hugovil.com header.b="SuSWHbR4" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=hugovil.com ; s=default; h=Content-Transfer-Encoding:Mime-Version:Message-Id:Subject:Cc: To:From:Date:subject:date:message-id:reply-to; bh=OXAoW1xbrM4n6AOheI3eiA1P3NihltU5wytjjG7W9Vo=; b=SuSWHbR46wfDfQbeXl9rwVFRLm JFlwSSicH8S2015d0g8MXQfeuLX6/mJp3DZHGBywbS5nPeY8+VDXVObFuyyMGBIl3AjAhQy1viemh 1wZtjJmHvLdvBTu2SjxFFfU36Oag4umKyDVfAKM4wGJFgU3OSk8KX6poNDZCHh+SyunU=; Received: from modemcable061.19-161-184.mc.videotron.ca ([184.161.19.61] helo=debian-lenovo) by mail.hugovil.com with esmtpa (Exim 4.98.2) (envelope-from ) id 1w2bQx-000000006Gd-3B4a; Tue, 17 Mar 2026 16:54:36 -0400 Date: Tue, 17 Mar 2026 16:54:34 -0400 From: Hugo Villeneuve To: Biju Das Cc: biju.das.au , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Philipp Zabel , "dri-devel@lists.freedesktop.org" , "linux-renesas-soc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Geert Uytterhoeven , Chris Brandt , Prabhakar Mahadev Lad Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the power-on sequence Message-Id: <20260317165434.ac87d685c844df896492c384@hugovil.com> In-Reply-To: References: <20260317123610.329630-1-biju.das.jz@bp.renesas.com> <20260317123610.329630-3-biju.das.jz@bp.renesas.com> <20260317110112.338259354289bb60a57fc259@hugovil.com> <20260317112052.3dc4b56b42b906381df80e94@hugovil.com> <20260317121232.39b1728e4d53d7243856a403@hugovil.com> <20260317145226.b84c29335c5badfdd257d0df@hugovil.com> <20260317154951.2670d458fae59adcffa0d497@hugovil.com> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam_score: -2.0 X-Spam_bar: -- Hi Biju, On Tue, 17 Mar 2026 20:10:31 +0000 Biju Das wrote: > Hi Hugo, > > > -----Original Message----- > > From: dri-devel On Behalf Of Hugo Villeneuve > > Sent: 17 March 2026 19:50 > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the power-on sequence > > > > Hi Biju, > > > > On Tue, 17 Mar 2026 19:37:35 +0000 > > Biju Das wrote: > > > > > Hi Hugo, > > > > > > > -----Original Message----- > > > > From: dri-devel On Behalf > > > > Of Hugo Villeneuve > > > > Sent: 17 March 2026 18:52 > > > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the > > > > power-on sequence > > > > > > > > Hi Biju, > > > > > > > > On Tue, 17 Mar 2026 16:36:05 +0000 > > > > Biju Das wrote: > > > > > > > > > Hi Hugo, > > > > > > > > > > > -----Original Message----- > > > > > > From: dri-devel On > > > > > > Behalf Of Hugo Villeneuve > > > > > > Sent: 17 March 2026 16:13 > > > > > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix the > > > > > > power-on sequence > > > > > > > > > > > > Hi Biju, > > > > > > > > > > > > On Tue, 17 Mar 2026 15:45:29 +0000 Biju Das > > > > > > wrote: > > > > > > > > > > > > > Hi Hugo, > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: dri-devel On > > > > > > > > Behalf Of Hugo Villeneuve > > > > > > > > Sent: 17 March 2026 15:21 > > > > > > > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: Fix > > > > > > > > the power-on sequence > > > > > > > > > > > > > > > > Hi Biju, > > > > > > > > > > > > > > > > On Tue, 17 Mar 2026 15:13:07 +0000 Biju Das > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi Hugo, > > > > > > > > > > > > > > > > > > Thanks for the feedback. > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > From: dri-devel > > > > > > > > > > On Behalf Of > > > > > > > > > > Hugo Villeneuve > > > > > > > > > > Sent: 17 March 2026 15:01 > > > > > > > > > > Subject: Re: [PATCH 2/2] drm: renesas: rzg2l_mipi_dsi: > > > > > > > > > > Fix the power-on sequence > > > > > > > > > > > > > > > > > > > > Hi Biju, > > > > > > > > > > > > > > > > > > > > On Tue, 17 Mar 2026 12:36:01 +0000 Biju > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > From: Biju Das > > > > > > > > > > > > > > > > > > > > > > Move reset_control_deassert() and > > > > > > > > > > > reset_control_assert() from > > > > > > > > > > > rzg2l_mipi_dsi_dphy_init()/rzg2l_mipi_dsi_dphy_exit() > > > > > > > > > > > to > > > > > > > > > > > atomic_pre_enable() and atomic_post_disable() > > > > > > > > > > > respectively, and move > > > > > > > > > > > rzg2l_mipi_dsi_set_display_timing() from > > > > > > > > > > > atomic_pre_enable() to atomic_enable(), to align with > > > > > > > > > > > the power-on sequence described in Figure 34.5 of > > > > > > > > > > > section > > > > > > > > > > > "34.4.2.1 Reset" of the RZ/G2L hardware manual > > > > > > > > > > > Rev.1.50 May 2025. > > > > > > > > > > > > > > > > > > > > > > According to the hardware manual, LINK registers must > > > > > > > > > > > be written before deasserting CMN_RSTB, and the 1ms > > > > > > > > > > > delay is retained in > > > > > > > > > > > atomic_pre_enable() after the deassert. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Biju Das > > > > > > > > > > > > > > > > > > > > Seems to me like this should be backported to stable > > > > > > > > > > branches (missing Fixes / Cc: stable > > > > > > tags)? > > > > > > > > > > > > > > > > > > OK, will add fixes/stable tags. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > .../gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 27 +++++++++++-------- > > > > > > > > > > > 1 file changed, 16 insertions(+), 11 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git > > > > > > > > > > > a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > > > > > > > > > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > > > > > > > > > > index e53b48e4de56..9053ce037b75 100644 > > > > > > > > > > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > > > > > > > > > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > > > > > > > > > > @@ -484,7 +484,6 @@ static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi, > > > > > > > > > > > u32 dphytim1; > > > > > > > > > > > u32 dphytim2; > > > > > > > > > > > u32 dphytim3; > > > > > > > > > > > - int ret; > > > > > > > > > > > > > > > > > > > > > > /* All DSI global operation timings are set with recommended setting */ > > > > > > > > > > > for (i = 0; i < > > > > > > > > > > > ARRAY_SIZE(rzg2l_mipi_dsi_global_timings); > > > > > > > > > > > ++i) { @@ > > > > > > > > > > > -524,12 +523,6 @@ static int rzg2l_mipi_dsi_dphy_init(struct rzg2l_mipi_dsi *dsi, > > > > > > > > > > > rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYTIM2, dphytim2); > > > > > > > > > > > rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYTIM3, > > > > > > > > > > > dphytim3); > > > > > > > > > > > > > > > > > > > > > > - ret = reset_control_deassert(dsi->rstc); > > > > > > > > > > > - if (ret < 0) > > > > > > > > > > > - return ret; > > > > > > > > > > > - > > > > > > > > > > > - fsleep(1000); > > > > > > > > > > > - > > > > > > > > > > > return 0; > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > @@ -541,8 +534,6 @@ static void > > > > > > > > > > > rzg2l_mipi_dsi_dphy_exit(struct rzg2l_mipi_dsi *dsi) > > > > > > > > > > > > > > > > > > > > > > dphyctrl0 &= ~(DSIDPHYCTRL0_EN_LDO1200 | DSIDPHYCTRL0_EN_BGR); > > > > > > > > > > > rzg2l_mipi_dsi_phy_write(dsi, DSIDPHYCTRL0, > > > > > > > > > > > dphyctrl0); > > > > > > > > > > > - > > > > > > > > > > > - reset_control_assert(dsi->rstc); > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > static int rzg2l_dphy_conf_clks(struct rzg2l_mipi_dsi > > > > > > > > > > > *dsi, unsigned long mode_freq, @@ -1030,24 +1021,37 @@ > > > > > > > > > > > static void rzg2l_mipi_dsi_atomic_pre_enable(struct > > > > > > > > > > drm_bridge *bridge, > > > > > > > > > > > connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder); > > > > > > > > > > > crtc = drm_atomic_get_new_connector_state(state, connector)->crtc; > > > > > > > > > > > mode = &drm_atomic_get_new_crtc_state(state, > > > > > > > > > > > crtc)->adjusted_mode; > > > > > > > > > > > - > > > > > > > > > > > > > > > > > > > > This is not related to your commit message (coding style change). > > > > > > > > > > > > > > > > > > Ack. Will restore it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ret = rzg2l_mipi_dsi_startup(dsi, mode); > > > > > > > > > > > if (ret < 0) > > > > > > > > > > > return; > > > > > > > > > > > > > > > > > > > > > > - rzg2l_mipi_dsi_set_display_timing(dsi, mode); > > > > > > > > > > > + ret = reset_control_deassert(dsi->rstc); > > > > > > > > > > > + if (ret < 0) > > > > > > > > > > > + return; > > > > > > > > > > > + > > > > > > > > > > > + if (dsi->rstc) > > > > > > > > > > > > > > > > > > > > This seems new and not documented in the commit message? Is this a fix? > > > > > > > > > > > > > > > > > > RZ/V2H does not need this as it uses different IP. > > > > > > > > > Previously > > > > > > > > > fsleep() is in RZ/G2L specific function. I will update commit description for this > > change. > > > > > > > > > > > > > > > > Suggestion: maybe move this to a separate patch, to facilitate review/understanding... > > > > > > > > > > > > > > The only way is to introduce a new callback to handle it for RZ/G2L SoC. > > > > > > > Then we won't be able to apply fixes tag as it is not fixing anything. > > > > > > > > > > > > I am not sure what you mean by that callback? How a callback is > > > > > > needed only if you split the > > > > patch? > > > > > > > > > > You cannot split the patch. > > > > > > > > > > Before: > > > > > atomic_pre_enable(): > > > > > startup() > > > > > dphy_init() > > > > > write DSIDPHYTIMx (F) PHY timing regs > > > > > reset_control_deassert() (G) deassert CMN_RSTB > > > > > udelay(1) (H) > > > > > setting below link registers > > > > > − TXSETR > > > > > − ULPSSETR > > > > > − DSISETR > > > > > − CLSTPTSETR > > > > > − LPTRNSTSETR > > > > > > > > > > Current patch: > > > > > > > > > > atomic_pre_enable(): > > > > > startup() > > > > > dphy_init() > > > > > write DSIDPHYTIMx (F) PHY timing regs > > > > > setting below link registers > > > > > − TXSETR > > > > > − ULPSSETR > > > > > − DSISETR > > > > > − CLSTPTSETR > > > > > − LPTRNSTSETR > > > > > > > > > > reset_control_deassert() (G) deassert CMN_RSTB > > > > > fsleep(1000) (H) > > > > > > > > > > > > > > > > > In this original patch you test for the validity of dsi->rstc to > > > > > > determine if you apply the delay or not. So in the case of RZ/V2H, I understand that it is > > NULL? > > > > > > > > > > Yes, that is correct. > > > > > > > > > > > > > > > > > > Currently this is optional reset, and it is no-op for RZ/V2H. > > > > > > > > > > > > Does this means that the call to reset_control_deassert(dsi->rstc) should not occur for > > RZ/V2H? > > > > > > > > > > reset_control_deassert(dsi->rstc) will return immediately as it is null. > > > > > > > > > > or > > > > > > > > > > We could add this check instead > > > > > > > > > > if (dsi->rstc) { > > > > > ret = reset_control_deassert(dsi->rstc); > > > > > if (ret < 0) > > > > > return; > > > > > > > > > > fsleep(1000); > > > > > } > > > > > > > > Yes, like Tommaso suggested. > > > > > > > > But I don't see why you cannot simply implement (split) this change > > > > as a separate commit just after commit #1, or after commit #2? > > > > > > > > This seems like an optimization for RZ/V2H, so I think it doesnt > > > > really matter if it does not go to stable branches? > > > > > > Previously RZ/V2H do not call reset_control_deassert(dsi->rstc) as it > > > is called from SoC-specific function. > > > > Ok, so this change could be split, if you want, as commit #3. This would make commit #2 easier to > > understand IMHO. > > > atomic_pre_enable(): > startup() > dphy_init() -> SoC specific > write DSIDPHYTIMx (F) PHY timing regs > > The below calls are in common path. So, no way you can split. > setting below link registers > − TXSETR > − ULPSSETR > − DSISETR > − CLSTPTSETR > − LPTRNSTSETR > reset_control_deassert() (G) deassert CMN_RSTB > fsleep(1000) (H) I do understand now, with your previous explanations, why you add the reset guard for RZ/V2H). I do not understand why you say "no way you can split", because my suggestion was merely to split the change in two commits instead of a single one, and the final source code would be 100% identical... > Patch#1 1usec to 1 msec > Patch#2 Move rzg2l_mipi_dsi_set_display_timing() to rzg2l_mipi_dsi_atomic_enable() > after starting hs clock. > Patch#3 Move reset assert/deassert from rzg2l_mipi_dsi_dphy_{init, exit} to > rzg2l_mipi_dsi_atomic_pre_enable and rzg2l_mipi_dsi_atomic_post_disable, > with a guard for RZ/V2H by checking (dsi->rstc), as it is in the common path. > Like previous case assert/deassert won't be called. > > Are you OK? My suggestion was only... a suggestion :) Feel free to do what you think is best, as long as it is very well documented. Hugo Villeneuve