Re: [PATCH][RESEND] drm/bridge: ti-sn65dsi83: Check link status register after enabling the bridge

[Marek Vasut <marex@denx.de>]

On 4/5/22 16:20, Dave Stevenson wrote:

Hi,

>>> If we can initialise the DSI host before the bridge for the
>>> pre_enable, then all the configuration moves to the atomic_pre_enable
>>> and there should be no need to have the delay.
>>>
>>> I can't 100% guarantee that, but one of the folks on the Pi forums is
>>> using [1] which does that, and is reporting it working well. (He's
>>> also using the DSI85 to take 2 DSI links and drive 2 LVDS single link
>>> panels)
>>
>> It seems to me that checking whether the bridge got correctly
>> initialized is orthogonal to the aforementioned patchset though ?
> 
> It's the delay that is ugly.

You do need to wait a little after the initialization and before 
checking the status, so that delay is not going away no matter how you 
frob with the DSI bridge.

> Put the check in atomic_enable which will be slightly later than
> configuration in pre_enable? Check that sufficient jiffies have passed
> if you needed.
> Or wire up the IRQ line from the SN65DSI83 rather than polling the IRQ
> Status register. Delayed workqueue if the IRQ isn't wired up.

Are you able to do such deferred non-atomic operations in atomic_enable 
callback ?

> If I read it right your log message is triggered by any bit being set
> in REG_IRQ_STAT. So an inconveniently timed correctable DSI error will
> set bit 4 and log the error even though it's been corrected. Likewise
> bit 7 / CHA_SYNCH_ERR could get triggered by an H or V sync packet
> being received in that 10-12ms window (we're in atomic_enable, so
> video is already running).

There should be no bits set in the IRQ_STAT register if everything works 
as it should.

> If it's the PLL being unlocked that is the issue then it should only
> be checking bit 0. Or possibly reading the actual PLL lock status from
> REG_RC_LVDS_PLL_PLL_EN_STAT. Although as we've already checked that
> the PLL is locked via REG_RC_LVDS_PLL_PLL_EN_STAT earlier in the
> atomic_enable, I'm now a little confused as to the condition you are
> actually wanting to detect.

Any outstanding errors which are currently hidden and only show up 
sporadically at the worst possible moment.

[...]