| 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
| ||
|
Message-ID: <20251205-courageous-tortoise-of-abracadabra-2efeee@houat>
Date: Fri, 5 Dec 2025 10:07:00 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: 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>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
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
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@...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.
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
Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)
Powered by blists - more mailing lists