Message-Id: <DEORF6XCJEOG.3BGTKVL2QFQKN@bootlin.com>
Date: Wed, 03 Dec 2025 18:32:47 +0100
From: "Luca Ceresoli" <luca.ceresoli@...tlin.com>
To: "Emanuele Ghidoli" <ghidoliemanuele@...il.com>,
 João Paulo Gonçalves
 <jpaulo.silvagoncalves@...il.com>, "Francesco Dolcini"
 <francesco@...cini.it>, "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>
Cc: "Philippe Schenker" <philippe.schenker@...ulsing.ch>, "Hui Pu"
 <Hui.Pu@...ealthcare.com>, "Thomas Petazzoni"
 <thomas.petazzoni@...tlin.com>, <dri-devel@...ts.freedesktop.org>,
 <linux-kernel@...r.kernel.org>, <stable@...r.kernel.org>,
 Hervé Codina <herve.codina@...tlin.com>
Subject: Re: [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors

Hi Emanuele,

On Tue Dec 2, 2025 at 12:19 PM CET, Emanuele Ghidoli wrote:
>
>
> On 27/11/2025 09:42, Luca Ceresoli wrote:
>> On hardware based on Toradex Verdin AM62 the recovery mechanism added by
>> commit ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery
>> mechanism") has been reported [0] to make the display turn on and off and
>> and the kernel logging "Unexpected link status 0x01".
>>
>> According to the report, the error recovery mechanism is triggered by the
>> PLL_UNLOCK error going active. Analysis suggested the board is unable to
>> provide the correct DSI clock neede by the SN65DSI84, to which the TI
>> SN65DSI84 reacts by raising the PLL_UNLOCK, while the display still works
>> apparently without issues.
>>
>> On other hardware, where all the clocks are within the components
>> specifications, the PLL_UNLOCK bit does not trigger while the display is in
>> normal use. It can trigger for e.g. electromagnetic interference, which is
>> a transient event and exactly the reason why the error recovery mechanism
>> has been implemented.
>>
>> Idelly the PLL_UNLOCK bit could be ignored when working out of
>> specification, but this requires to detect in software whether it triggers
>> because the device is working out of specification but visually correctly
>> for the user or for good reasons (e.g. EMI, or even because working out of
>> specifications but compromising the visual output).
>>
>> The ongoing analysis as of this writing [1][2] has not yet found a way for
>> the driver to discriminate among the two cases. So as a temporary measure
>> mask the PLL_UNLOCK error bit unconditionally.
>>
>> [0] https://lore.kernel.org/r/bhkn6hley4xrol5o3ytn343h4unkwsr26p6s6ltcwexnrsjsdx@mgkdf6ztow42
>> [1] https://lore.kernel.org/all/b71e941c-fc8a-4ac1-9407-0fe7df73b412@gmail.com/
>> [2] https://lore.kernel.org/all/20251125103900.31750-1-francesco@dolcini.it/
>>
>> Closes: https://lore.kernel.org/r/bhkn6hley4xrol5o3ytn343h4unkwsr26p6s6ltcwexnrsjsdx@mgkdf6ztow42
>> Cc: stable@...r.kernel.org # 6.15+
>> Co-developed-by: Hervé Codina <herve.codina@...tlin.com>
>> Signed-off-by: Hervé Codina <herve.codina@...tlin.com>
>> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
>> ---
>> Francesco, Emanuele, João: can you please apply this patch and report
>> whether the display on the affected boards gets back to working as before?
>>
>> Cc: João Paulo Gonçalves <jpaulo.silvagoncalves@...il.com>
>> Cc: Francesco Dolcini <francesco@...cini.it>
>> Cc: Emanuele Ghidoli <ghidoliemanuele@...il.com>
>> ---
>>  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> index 033c44326552..fffb47b62f43 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
>> @@ -429,7 +429,14 @@ static void sn65dsi83_handle_errors(struct sn65dsi83 *ctx)
>>  	 */
>>
>>  	ret = regmap_read(ctx->regmap, REG_IRQ_STAT, &irq_stat);
>> -	if (ret || irq_stat) {
>> +
>> +	/*
>> +	 * Some hardware (Toradex Verdin AM62) is known to report the
>> +	 * PLL_UNLOCK error interrupt while working without visible
>> +	 * problems. In lack of a reliable way to discriminate such cases
>> +	 * from user-visible PLL_UNLOCK cases, ignore that bit entirely.
>> +	 */
>> +	if (ret || irq_stat & ~REG_IRQ_STAT_CHA_PLL_UNLOCK) {
>>  		/*
>>  		 * IRQ acknowledged is not always possible (the bridge can be in
>>  		 * a state where it doesn't answer anymore). To prevent an
>> @@ -654,7 +661,7 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
>>  	if (ctx->irq) {
>>  		/* Enable irq to detect errors */
>>  		regmap_write(ctx->regmap, REG_IRQ_GLOBAL, REG_IRQ_GLOBAL_IRQ_EN);
>> -		regmap_write(ctx->regmap, REG_IRQ_EN, 0xff);
>> +		regmap_write(ctx->regmap, REG_IRQ_EN, 0xff & ~REG_IRQ_EN_CHA_PLL_UNLOCK_EN);
>>  	} else {
>>  		/* Use the polling task */
>>  		sn65dsi83_monitor_start(ctx);
>>
>> ---
>> base-commit: c884ee70b15a8d63184d7c1e02eba99676a6fcf7
>> change-id: 20251126-drm-ti-sn65dsi83-ignore-pll-unlock-4a28aa29eb5c
>>
>> Best regards,

Thanks for testing!

We'll still need a R-by from a maintainer, after that this patch can be applied.

> I would suggest a couple of tags, thanks.
> Emanuele
>
> Fixes: ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery mechanism")

I'm not sure about this one. There is no known bug in that commit, right?
It's rather exposing a pre-existing issue. I thought about adding it for
stable branches pickup, but the 'Cc: stable...v6.15+' line is for that.

So apart from blaming someone I don't see much point.

That said, if there is a valid reason I'm not seeing for the Fixes: line,
I'll be OK in adding it while applying.

> Reported-by: João Paulo Gonçalves <joao.goncalves@...adex.com>

Absolutely! Sorry I forgot to add this.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Powered by blists - more mailing lists