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/[email protected]/
> >> [2]
> >> https://lore.kernel.org/all/[email protected]/
> >>
> >> Closes:
> >> https://lore.kernel.org/r/bhkn6hley4xrol5o3ytn343h4unkwsr26p6s6ltcwexnrsjsdx@mgkdf6ztow42
> >> Cc: [email protected] # 6.15+
> >> Co-developed-by: Hervé Codina <[email protected]>
> >> Signed-off-by: Hervé Codina <[email protected]>
> >> Signed-off-by: Luca Ceresoli <[email protected]>
> >> ---
> >> 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 <[email protected]>
> >> Cc: Francesco Dolcini <[email protected]>
> >> Cc: Emanuele Ghidoli <[email protected]>
> >> ---
> >> 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
signature.asc
Description: PGP signature
