Re: [PATCH v1] Revert "drm: bridge: ti-sn65dsi83: Add error recovery mechanism"

[Maxime Ripard]

Hi,

On Thu, Nov 27, 2025 at 09:46:07AM +0100, Luca Ceresoli wrote:
> Hello Francesco, all,
> 
> On Tue Nov 25, 2025 at 11:38 AM CET, Francesco Dolcini wrote:
> > From: Francesco Dolcini <francesco.dolcini@xxxxxxxxxxx>
> >
> > This reverts commit ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error
> > recovery mechanism").
> >
> > The reverted commit introduces a regression on Verdin AM62, and
> > potentially on more devices, not being able to generate a clock
> > that the TI SN65DSI83 PLL can lock to, with the display periodically
> > blinking.
> >
> > Verdin AM62 SoM has a Toshiba TC358778 DPI to DSI bridge, that can be
> > connected to an LVDS display over a TI SN65DSI83 bridge. Before this
> > change despite the TI SN65DSI83 reporting with a debug print a PLL
> > locking error the display was working fine with no visible glitches.
> >
> > The reasons for this issue was investigated without getting to a final
> > conclusion:
> >
> >  - the DPI clock was measure and it is stable/accurate
> >  - the DSI clock was not possible to measure, but this setup is used
> >    with other display/bridges with no known issues
> >  - the DSI clock is configured in continuous mode
> >  - the actual DSI clock generated from the TC358778 is generate with a
> >    PLL from a 25MHz reference clock
> >  - it's not clear why some frequencies are working and some are not, for
> >    example 50000000, 68750000, 72750000, 75000000 frequencies are fine,
> >    while 69750000, 71100000, 72500000 are not
> >
> > Given that the safest approach is to just revert the commit, till a
> > proper solution for error recovery that is not introducing regression
> > is figured out.
> >
> > Reported-by: João Paulo Gonçalves <jpaulo.silvagoncalves@xxxxxxxxx>
> > Closes: https://lore.kernel.org/all/bhkn6hley4xrol5o3ytn343h4unkwsr26p6s6ltcwexnrsjsdx@mgkdf6ztow42/
> > Fixes: ad5c6ecef27e ("drm: bridge: ti-sn65dsi83: Add error recovery mechanism")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@xxxxxxxxxxx>
> 
> Thanks for having sent this revert patch.
> 
> However after evaluating the overall situation I decided to send a
> different patch to address this issue in the short term. The idea is to
> just ignore the PLL_UNLOCK error, keeping the existing
> structure. Rationale:
> 
>  * this sloves the issue for Toradex, based on João's initial report
>  * there is no evidence of any bugs in the recovery mechanism, it's
>    just exposing a pre-existing problem that was only producing a
>    non-fatal dev_err() before
>  * a full revert would remove error checking for all errors, including
>    those not creating any issue, thus removing a useful feature
>  * a full revert would require rewriting patches such as [0] (not a big
>    deal per se, but see next bullet)
>  * after patches such as [0] are applied, re-adding the error recovery
>    mechanism would require another rework, so more work for authors,
>    reviewers, testers and maintainers

Were are we on this? Both patches work for me, but we need to take a decision.

Maxime

Attachment: signature.asc
Description: PGP signature