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

From: Marek Vasut <marex@denx.de>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	Jagan Teki <jagan@amarulasolutions.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Robert Foss <robert.foss@linaro.org>,
	Sam Ravnborg <sam@ravnborg.org>
Subject: Re: [PATCH] drm/bridge: ti-sn65dsi83: Check link status register after enabling the bridge
Date: Wed, 8 Sep 2021 17:26:16 +0200	[thread overview]
Message-ID: <13ecc7df-37b9-a686-9314-04a26359fc0d@denx.de> (raw)
In-Reply-To: <CAPY8ntBVdvHSofXcd7nU5Z4uCMUzmiMF3GmJn=VpLDVoe6xL2g@mail.gmail.com>

On 9/8/21 1:11 PM, Dave Stevenson wrote:
> Hi Marek and Andrzej

Hello Dave,

skipping the protocol discussion, which I hope Andrej will pick up.

[...]

>>> Usually video transmission starts in crtc->enable (CRTC->Encoder), and
>>> in encoder->enable (encoder->bridge), so in bridges->enable it would be
>>> too late for LP11 state - transmission can be already in progress.
>>>
>>> It shows well that this order of calls does not fit well to DSI, and
>>> probably many other protocols.
>>>
>>> Maybe moving most of the bridge->enable code to bridge->pre_enable would
>>> help, but I am not sur if it will not pose another issues.
>>
>> Yep, that won't work e.g. with the exynos DSIM, because
>> exynos_dsi_set_display_mode() sets the data lanes to LP11.
> 
> Isn't the bigger question for SN65DSI8[3|4|5] whether the clock lane
> is running or not in pre_enable?

I think the bigger question really is -- how do we cater for all the 
different bridges with different init-time requirements.

>>> This is quick analysis, so please fix me if I am wrong.
>>
>> I pretty much agree that the current state of things does not fit with
>> DSI too well.
> 
> That was why I was questioning how DSI was meant to be implemented in
> https://lore.kernel.org/dri-devel/CAPY8ntBUKRkSam59Y+72dW_6XOeKVswPWffzPj3uvgE6pV4ZGQ@mail.gmail.com/
> 
> The need to have the DSI host in a defined idle state (often LP-11,
> but varying whether the clock lane is in HS) before powering up the
> panel/bridge is incredibly common, but currently undefined in DRM.
> 
> Taking the SN65DSI83 as an example, the datasheet [1] section 7.4.2
> states that the clock lane must be in HS mode, and data lanes in LP-11
> when coming out of reset. That means that we can't be "enable" as that
> will have the data lanes in HS mode and sending video, and as we can't
> be in "pre_enable" as the DSI PHY will be powered down and so we won't
> have the clock lanes in HS mode.
> 
> I've hit a similar one with the Toshiba TC358762 where it seems to get
> upset if it is receiving video data when it gets configured.
> panel-raspberrypi-touchscreen[2] which drives that chip is
> intermittent when using panel enable, whereas panel prepare is
> significantly more reliable but relies on the DSI host being
> initialised to LP-11 by breaking the chain.

Right

To make it worse, not initing the DSI bridge exactly per spec leads to 
intermittent failures, not consistently occuring ones.

>    Dave
> 
> [1] https://www.ti.com/lit/ds/symlink/sn65dsi83.pdf
> [2] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c

Unrelated to this discussion -- there is a tc358762 driver, driver for 
that attiny88 regulator, and driver for the touchscreen chip, on that 
rpi 7" display, in upstream. You can use those to replace the composite 
panel driver (it works at least against stm32mp1 DSI host with the rpi 
7" panel). Sadly there is little documentation for that attiny88 
protocol or firmware, that's what I don't like about that panel.