Re: [PATCH V3 2/2] drm/bridge: ti-sn65dsi83: Add TI SN65DSI83 and SN65DSI84 driver

[Dave Stevenson <dave.stevenson@raspberrypi.com>]

On Sat, 8 May 2021 at 21:26, Marek Vasut <marex@denx.de> wrote:
>
> On 5/7/21 2:48 PM, Dave Stevenson wrote:
>
> [...]
>
> >> +static void sn65dsi83_enable(struct drm_bridge *bridge)
> >> +{
> >> +       struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> >> +       unsigned int pval;
> >> +       u16 val;
> >> +       int ret;
> >> +
> >> +       /* Clear reset, disable PLL */
> >> +       regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
> >> +       regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
> >
> > Sorry, a further thread of discussion coming from the investigations
> > I've been involved with.
> >
> > You've powered up in pre_enable, and are sending the I2C writes in enable.
> >
> >>From the docs for drm_bridge_funcs->enable[1]
> >
> >   * The bridge can assume that the display pipe (i.e. clocks and timing
> >   * signals) feeding it is running when this callback is called. This
> >   * callback must enable the display link feeding the next bridge in the
> >   * chain if there is one.
> >
> > So video is running when enable is called, and the DSI data lanes may
> > be HS. (Someone correct me if that is an incorrect reading of the
> > text).
> >
> > The SN65DSI84 datasheet table 7-2 Initialization Sequence gives init
> > seq 8 as being "Change DSI data lanes to HS state and start DSI video
> > stream", AFTER all the I2C has been completed except reading back
> > registers and checking for errors.
> > With video running you don't fulfil the second part of init seq 2 "the
> > DSI data lanes MUST be driven to LP11 state"
> >
> > My investigations have been over delaying starting the DSI video
> > stream until after enable, but reading the descriptive text for enable
> > I believe the Pi is correct to be sending video at that point.
> > I guess there is some ambiguity as to whether the clock lane is going
> > to be in HS mode during pre_enable. On the Pi the PHY and clocks will
> > be enabled prior to pre_enable to allow for sending DSI commands
> > during pre_enable, but it may not be true on other platforms.
>
> You have to make sure the clock lane is running and in HS mode when
> configuring the DSI83, otherwise the internal DSI83 state machine won't
> be able to operate.

Indeed, but my reading of the documentation says that neither
pre_enable nor enable give you the state that you require.
You need a hook in the middle, an option to ask for clock lanes during
pre_enable or no video during enable, or an amendment to the docs over
the state during enable.

Having the data lanes in HS mode does appear to stop the DSI83
accepting the I2C setup commands.

  Dave