* [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors
@ 2025-11-27 8:42 Luca Ceresoli
2025-12-02 10:56 ` Emanuele Ghidoli
2025-12-02 11:19 ` Emanuele Ghidoli
0 siblings, 2 replies; 6+ messages in thread
From: Luca Ceresoli @ 2025-11-27 8:42 UTC (permalink / raw)
To: João Paulo Gonçalves, Francesco Dolcini,
Emanuele Ghidoli, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter
Cc: Philippe Schenker, Hui Pu, Thomas Petazzoni, dri-devel,
linux-kernel, stable, Hervé Codina, Luca Ceresoli
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@vger.kernel.org # 6.15+
Co-developed-by: Hervé Codina <herve.codina@bootlin.com>
Signed-off-by: Hervé Codina <herve.codina@bootlin.com>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.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@gmail.com>
Cc: Francesco Dolcini <francesco@dolcini.it>
Cc: Emanuele Ghidoli <ghidoliemanuele@gmail.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,
--
Luca Ceresoli <luca.ceresoli@bootlin.com>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors
2025-11-27 8:42 [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors Luca Ceresoli
@ 2025-12-02 10:56 ` Emanuele Ghidoli
2025-12-02 11:19 ` Emanuele Ghidoli
1 sibling, 0 replies; 6+ messages in thread
From: Emanuele Ghidoli @ 2025-12-02 10:56 UTC (permalink / raw)
To: Luca Ceresoli, João Paulo Gonçalves, Francesco Dolcini,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Philippe Schenker, Hui Pu, Thomas Petazzoni, dri-devel,
linux-kernel, stable, Hervé Codina
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@vger.kernel.org # 6.15+
> Co-developed-by: Hervé Codina <herve.codina@bootlin.com>
> Signed-off-by: Hervé Codina <herve.codina@bootlin.com>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.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@gmail.com>
> Cc: Francesco Dolcini <francesco@dolcini.it>
> Cc: Emanuele Ghidoli <ghidoliemanuele@gmail.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,
Hi Luca,
the display works correctly with this patch, thanks!
Kind regards.
Tested-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors
2025-11-27 8:42 [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors Luca Ceresoli
2025-12-02 10:56 ` Emanuele Ghidoli
@ 2025-12-02 11:19 ` Emanuele Ghidoli
2025-12-03 17:32 ` Luca Ceresoli
1 sibling, 1 reply; 6+ messages in thread
From: Emanuele Ghidoli @ 2025-12-02 11:19 UTC (permalink / raw)
To: Luca Ceresoli, João Paulo Gonçalves, Francesco Dolcini,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Philippe Schenker, Hui Pu, Thomas Petazzoni, dri-devel,
linux-kernel, stable, Hervé Codina
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@vger.kernel.org # 6.15+
> Co-developed-by: Hervé Codina <herve.codina@bootlin.com>
> Signed-off-by: Hervé Codina <herve.codina@bootlin.com>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.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@gmail.com>
> Cc: Francesco Dolcini <francesco@dolcini.it>
> Cc: Emanuele Ghidoli <ghidoliemanuele@gmail.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,
Well,
I would suggest a couple of tags, thanks.
Emanuele
Fixes: ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery mechanism")
Reported-by: João Paulo Gonçalves <joao.goncalves@toradex.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors
2025-12-02 11:19 ` Emanuele Ghidoli
@ 2025-12-03 17:32 ` Luca Ceresoli
2025-12-05 9:07 ` Maxime Ripard
0 siblings, 1 reply; 6+ messages in thread
From: Luca Ceresoli @ 2025-12-03 17:32 UTC (permalink / raw)
To: Emanuele Ghidoli, João Paulo Gonçalves,
Francesco Dolcini, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter
Cc: Philippe Schenker, Hui Pu, Thomas Petazzoni, dri-devel,
linux-kernel, stable, Hervé Codina
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@vger.kernel.org # 6.15+
>> Co-developed-by: Hervé Codina <herve.codina@bootlin.com>
>> Signed-off-by: Hervé Codina <herve.codina@bootlin.com>
>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.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@gmail.com>
>> Cc: Francesco Dolcini <francesco@dolcini.it>
>> Cc: Emanuele Ghidoli <ghidoliemanuele@gmail.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@toradex.com>
Absolutely! Sorry I forgot to add this.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors
2025-12-03 17:32 ` Luca Ceresoli
@ 2025-12-05 9:07 ` Maxime Ripard
2025-12-09 10:29 ` Luca Ceresoli
0 siblings, 1 reply; 6+ messages in thread
From: Maxime Ripard @ 2025-12-05 9:07 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Emanuele Ghidoli, João Paulo Gonçalves,
Francesco Dolcini, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Philippe Schenker, Hui Pu, Thomas Petazzoni, dri-devel,
linux-kernel, stable, Hervé Codina
[-- Attachment #1: Type: text/plain, Size: 5788 bytes --]
On Wed, Dec 03, 2025 at 06:32:47PM +0100, Luca Ceresoli wrote:
> 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@vger.kernel.org # 6.15+
> >> Co-developed-by: Hervé Codina <herve.codina@bootlin.com>
> >> Signed-off-by: Hervé Codina <herve.codina@bootlin.com>
> >> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.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@gmail.com>
> >> Cc: Francesco Dolcini <francesco@dolcini.it>
> >> Cc: Emanuele Ghidoli <ghidoliemanuele@gmail.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.
Sigh. We had that discussion already. Wouldn't you consider "the display
doesn't work" a bug on any platform? A real world device wasn't behaving
the way the driver expected it to be. The root cause of it doesn't
really matter: it was a bug.
And whether it's technically correct or not isn't relevant: we only care
about what actually happens, not what the datasheet is saying.
>
> 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.
It's not about blaming someone, it's about tracking that there was a
regression, and it got fixed. Who's to blame is not relevant either, and
I don't think anyone blamed anyone in that thread.
Anyway, patch applied.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors
2025-12-05 9:07 ` Maxime Ripard
@ 2025-12-09 10:29 ` Luca Ceresoli
0 siblings, 0 replies; 6+ messages in thread
From: Luca Ceresoli @ 2025-12-09 10:29 UTC (permalink / raw)
To: Maxime Ripard
Cc: Emanuele Ghidoli, João Paulo Gonçalves,
Francesco Dolcini, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Philippe Schenker, Hui Pu, Thomas Petazzoni, dri-devel,
linux-kernel, stable, Hervé Codina
Hello,
On Fri Dec 5, 2025 at 10:07 AM CET, Maxime Ripard wrote:
> On Wed, Dec 03, 2025 at 06:32:47PM +0100, Luca Ceresoli wrote:
>> 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@vger.kernel.org # 6.15+
>> >> Co-developed-by: Hervé Codina <herve.codina@bootlin.com>
>> >> Signed-off-by: Hervé Codina <herve.codina@bootlin.com>
>> >> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.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@gmail.com>
>> >> Cc: Francesco Dolcini <francesco@dolcini.it>
>> >> Cc: Emanuele Ghidoli <ghidoliemanuele@gmail.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.
>
> Sigh. We had that discussion already. Wouldn't you consider "the display
> doesn't work" a bug on any platform? A real world device wasn't behaving
> the way the driver expected it to be. The root cause of it doesn't
> really matter: it was a bug.
>
> And whether it's technically correct or not isn't relevant: we only care
> about what actually happens, not what the datasheet is saying.
>>
>> 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.
>
> It's not about blaming someone, it's about tracking that there was a
> regression, and it got fixed. Who's to blame is not relevant either, and
> I don't think anyone blamed anyone in that thread.
>
> Anyway, patch applied.
I'm glad we agree about the code and this patch got applied. Thanks! I
think it is the least invasive solution in the short term.
Now, looking at the long term, it's time to find out how to support both
Toradex hardware and the full error recovery on boards where PLL_UNLOCK
behaves as expected (it triggers only in visual error cases).
Emanuele, Francesco, did you gather more information on the issue? Such as
which clock combinations trigger the PLL_UNLOCK with a visually working
panel?
Also, can you report how the PLL_UNLOCK bit behaves over time on your
hardware? It stays asserted all the time, it goes on and off regularly,
irregularly, it disapperas after some time...?
Best regards,
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-12-09 10:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-27 8:42 [PATCH] drm/bridge: ti-sn65dsi83: ignore PLL_UNLOCK errors Luca Ceresoli
2025-12-02 10:56 ` Emanuele Ghidoli
2025-12-02 11:19 ` Emanuele Ghidoli
2025-12-03 17:32 ` Luca Ceresoli
2025-12-05 9:07 ` Maxime Ripard
2025-12-09 10:29 ` Luca Ceresoli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).