linux-kernel - Re: [PATCH v3 09/13] drm/bridge: samsung-dsim: increase timeout value for PLL

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <b732d2588112932ab399df47fb58e3f5@disroot.org>
Date: Tue, 29 Jul 2025 15:44:03 +0000
From: Kaustabh Chakraborty <kauschluss@...root.org>
To: Inki Dae <daeinki@...il.com>
Cc: Jagan Teki <jagan@...rulasolutions.com>, Marek Szyprowski
 <m.szyprowski@...sung.com>, Andrzej Hajda <andrzej.hajda@...el.com>, Neil
 Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
 Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman
 <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
 <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
 <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Seung-Woo Kim <sw0312.kim@...sung.com>, Kyungmin Park
 <kyungmin.park@...sung.com>, Krzysztof Kozlowski <krzk@...nel.org>, Alim
 Akhtar <alim.akhtar@...sung.com>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org
Subject: Re: [PATCH v3 09/13] drm/bridge: samsung-dsim: increase timeout value
 for PLL_STABLE

On 2025-07-29 05:44, Inki Dae wrote:
> 2025년 7월 7일 (월) 오전 3:28, Kaustabh Chakraborty 
> <kauschluss@...root.org>님이 작성:
>> 
>> Exynos7870's DSIM requires more time to stabilize its PLL. The current
>> timeout value, 1000, doesn't suffice. Increase the value to 3000, 
>> which
>> is just about enough as observed experimentally.
>> 
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@...root.org>
>> ---
>>  drivers/gpu/drm/bridge/samsung-dsim.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
>> b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index 
>> fb2cb09cfd5a4f2fb50f802dc434c0956107b4e9..4b49707730db76aa8fd3ab973b02507436750889 
>> 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -755,7 +755,7 @@ static unsigned long samsung_dsim_set_pll(struct 
>> samsung_dsim *dsi,
>> 
>>         samsung_dsim_write(dsi, DSIM_PLLCTRL_REG, reg);
>> 
>> -       timeout = 1000;
>> +       timeout = 3000;
> 
> Relying on an implicit loop to wait for PLL stabilization is not an
> ideal solution.
> According to the datasheet, this can be addressed more explicitly by
> using the DSIM_PLLTMR (PLL timer) register instead.
> 
> By configuring the pll timer field in DSIM_PLLTMR appropriately, we
> can avoid arbitrary loops.
> For example according to data sheet:
> If the APB clock is 80 MHz and the desired delay is 20 µs,
> the pll timer field should be set to:
> delay_time * apb_clock = 20 * 80 = 1600 (0x3E80)
> 
> Once this value is set and the MskPllStable field in the DSIM_INTMSK
> register is unmasked,
> the pll_stable field in the DSIM_INTSRC register will be set after the
> specified delay (20 µs in this example).
> We can then check this field to determine whether the PLL has 
> stabilized.
> 
> While the current patch relies on an implicit method, I’m fine with
> merging it as-is for now.
> However, since this patch series likely has sufficient time to
> mainline, I believe this is a good opportunity to improve the related
> logic.
> 
> Would you be open to trying the approach described above?

Interesting. I will try implementing it sometime in the near future.
(can't commit to it at the moment, preoccupied)

> 
> Thanks,
> Inki Dae
> 
>>         do {
>>                 if (timeout-- == 0) {
>>                         dev_err(dsi->dev, "PLL failed to 
>> stabilize\n");
>> 
>> --
>> 2.49.0
>> 
>>