Re: [2/2] drm/bridge: tc358767: Reset chip again on attach

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi,

Am Montag, 13. Mai 2024, 04:16:28 CEST schrieb Marek Vasut:
> In case the chip is released from reset using the RESX signal while the
> DSI lanes are in non-LP11 mode, the chip may enter some sort of debug
> mode, where its internal clock run at 1/6th of expected clock rate. In
> this mode, the AUX channel also operates at 1/6th of the 10 MHz mandated
> by DP specification, which breaks DPCD communication.
> 
> There is no known software way of bringing the chip out of this state
> once the chip enters it, except for toggling the RESX signal and
> performing full reset.
> 
> The chip may enter this mode when the chip was released from reset in
> probe(), because at that point the DSI lane mode is undefined.
> 
> When the .attach callback is called, the DSI link is surely in LP11 mode.
> Toggle the RESX signal here and reconfigure the AUX channel. That way,
> the AUX channel communication from this point on does surely run at
> 10 MHz as it should.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

This does the trick on my hardware as well.
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Cc: Adam Ford <aford173@gmail.com>
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Frieder Schrempf <frieder.schrempf@kontron.de>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Jonas Karlman <jonas@kwiboo.se>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Michael Walle <mwalle@kernel.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Robert Foss <rfoss@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: kernel@dh-electronics.com
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 50 +++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index fe2b93546eaef..9b01dc885973c 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1749,10 +1749,30 @@ static const struct drm_connector_funcs tc_connector_funcs = {
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
>  
> +static void tc_bridge_reset(struct tc_data *tc)
> +{
> +	if (!tc->reset_gpio)
> +		return;
> +
> +	gpiod_set_value_cansleep(tc->reset_gpio, 0);
> +	usleep_range(10000, 11000);
> +	gpiod_set_value_cansleep(tc->reset_gpio, 1);
> +	usleep_range(5000, 10000);
> +}
> +
>  static int tc_dpi_bridge_attach(struct drm_bridge *bridge,
>  				enum drm_bridge_attach_flags flags)
>  {
>  	struct tc_data *tc = bridge_to_tc(bridge);
> +	int ret;
> +
> +	if (tc->reset_gpio) {
> +		tc_bridge_reset(tc);
> +
> +		ret = tc_set_syspllparam(tc);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	if (!tc->panel_bridge)
>  		return 0;
> @@ -1769,6 +1789,36 @@ static int tc_edp_bridge_attach(struct drm_bridge *bridge,
>  	struct drm_device *drm = bridge->dev;
>  	int ret;
>  
> +	if (tc->reset_gpio) {
> +		/*
> +		 * In case the chip is released from reset using the RESX
> +		 * signal while the DSI lanes are in non-LP11 mode, the chip
> +		 * may enter some sort of debug mode, where its internal
> +		 * clock run at 1/6th of expected clock rate. In this mode,
> +		 * the AUX channel also operates at 1/6th of the 10 MHz
> +		 * mandated by DP specification, which breaks DPCD
> +		 * communication.
> +		 *
> +		 * There is no known software way of bringing the chip out of
> +		 * this state once the chip enters it, except for toggling
> +		 * the RESX signal and performing full reset.
> +		 *
> +		 * The chip may enter this mode when the chip was released
> +		 * from reset in probe(), because at that point the DSI lane
> +		 * mode is undefined.
> +		 *
> +		 * At this point, the DSI link is surely in LP11 mode. Toggle
> +		 * the RESX signal here and reconfigure the AUX channel. That
> +		 * way, the AUX channel communication from this point on does
> +		 * surely run at 10 MHz as it should.
> +		 */
> +		tc_bridge_reset(tc);
> +
> +		ret = tc_aux_link_setup(tc);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	if (tc->panel_bridge) {
>  		/* If a connector is required then this driver shall create it */
>  		ret = drm_bridge_attach(tc->bridge.encoder, tc->panel_bridge,
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/