[PATCH] drm/bridge: ti-sn65dsi86: fix REFCLK setting

[Doug Anderson]

Hi,

On Thu, Jun 12, 2025 at 10:52 AM Doug Anderson <dianders at chromium.org> wrote:
>
> Hi,
>
> On Thu, Jun 12, 2025 at 12:35 AM Jayesh Choudhary <j-choudhary at ti.com> wrote:
> >
> > >> If refclk is described in devicetree node, then I see that
> > >> the driver modifies it in every resume call based solely on the
> > >> clock value in dts.
> > >
> > > Exactly. But that is racy with what the chip itself is doing. I.e.
> > > if you don't have that usleep() above, the chip will win the race
> > > and the refclk frequency setting will be set according to the
> > > external GPIOs (which is poorly described in the datasheet, btw),
> > > regardless what the linux driver is setting (because that I2C write
> > > happens too early).
> >
> > I am a little confused here.
> > Won't it be opposite?
> > If we have this delay here, GPIO will stabilize and set the register
> > accordingly?
> >
> > In the driver, I came across the case when we do not have refclk.
> > (My platform does have a refclk, I am just removing the property from
> > the dts node to check the affect of GPIO[3:1] in question because clock
> > is not a required property for the bridge as per the bindings)
> >
> > In the ti_sn65dsi86_probe(), before we read SN_DEVICE_ID_REGS,
> > when we go to resume(), we do not do enable_comms() that calls
> > ti_sn_bridge_set_refclk_freq() to set SN_DPPLL_SRC_REG.
> > I see that register read for SN_DEVICE_ID_REGS fails in that case.
> >
> > Adding this delay fixes that issue. This made me think that we need
> > the delay for GPIO to stabilize and set the refclk.
>
> FWIW, it's been on my plate for a while to delete the "no refclk"
> support. The chip is really hard to use properly without a refclk and
> I'm not at all convinced that the current code actually works properly
> without a refclk. I'm not aware of any current hardware working this
> way. I know we had some very early prototype hardware ages ago that
> tried it and we got it limping along at one point, but the driver
> looked _very_ different then. I believe someone on the lists once
> mentioned trying to do something without a refclk and it didn't work
> and I strongly encouraged them to add a refclk.

Actually, I may have to eat my words here. I double-checked the dts
and I see there's at least two mainline users
("meson-g12b-bananapi-cm4-mnt-reform2.dts" and
"/imx8mq-mnt-reform2.dts") that don't seem to be specifying a `refclk`
to `ti,sn65dsi86`.

Neil / Lucas: is that correct? ...and it actually works?

-Doug