[PATCH 1/2] drm: bridge: samsung-dsim: Initialize bridge on attach

[Marek Vasut]

On 5/16/24 8:51 AM, Alexander Stein wrote:
> Hi Marek,
> 
> thanks for the patch.
> 
> Am Montag, 13. Mai 2024, 04:16:27 CEST schrieb Marek Vasut:
>> Initialize the bridge on attach already, to force lanes into LP11
>> state, since attach does trigger attach of downstream bridges which
>> may trigger (e)DP AUX channel mode read.
>>
>> This fixes a corner case where DSIM with TC9595 attached to it fails
>> to operate the DP AUX channel, because the TC9595 enters some debug
>> mode when it is released from reset without lanes in LP11 mode. By
>> ensuring the DSIM lanes are in LP11, the TC9595 (tc358767.c driver)
>> can be reset in its attach callback called from DSIM attach callback,
>> and recovered out of the debug mode just before TC9595 performs first
>> AUX channel access later in its attach callback.
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> ---
>> Cc: Adam Ford <aford173 at gmail.com>
>> Cc: Alexander Stein <alexander.stein at ew.tq-group.com>
>> Cc: Andrzej Hajda <andrzej.hajda at intel.com>
>> Cc: Daniel Vetter <daniel at ffwll.ch>
>> Cc: David Airlie <airlied at gmail.com>
>> Cc: Frieder Schrempf <frieder.schrempf at kontron.de>
>> Cc: Inki Dae <inki.dae at samsung.com>
>> Cc: Jagan Teki <jagan at amarulasolutions.com>
>> Cc: Jernej Skrabec <jernej.skrabec at gmail.com>
>> Cc: Jonas Karlman <jonas at kwiboo.se>
>> Cc: Laurent Pinchart <Laurent.pinchart at ideasonboard.com>
>> Cc: Lucas Stach <l.stach at pengutronix.de>
>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> Cc: Marek Szyprowski <m.szyprowski at samsung.com>
>> Cc: Maxime Ripard <mripard at kernel.org>
>> Cc: Michael Walle <mwalle at kernel.org>
>> Cc: Neil Armstrong <neil.armstrong at linaro.org>
>> Cc: Robert Foss <rfoss at kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: kernel at dh-electronics.com
>> ---
>>   drivers/gpu/drm/bridge/samsung-dsim.c | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index 95fedc68b0ae5..56093fc3d62cc 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -1622,9 +1622,21 @@ static int samsung_dsim_attach(struct drm_bridge *bridge,
>>   			       enum drm_bridge_attach_flags flags)
>>   {
>>   	struct samsung_dsim *dsi = bridge_to_dsi(bridge);
>> +	int ret;
>> +
>> +	ret = pm_runtime_resume_and_get(dsi->dev);
>> +	if (ret < 0)
>> +		return ret;
>>   
>> -	return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
>> -				 flags);
>> +	ret = samsung_dsim_init(dsi);
>> +	if (ret < 0)
>> +		goto err;
>> +
>> +	ret = drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
>> +				flags);
>> +err:
>> +	pm_runtime_put_sync(dsi->dev);
>> +	return ret;
>>   }
>>   
>>   static const struct drm_bridge_funcs samsung_dsim_bridge_funcs = {
>>
> 
> It seems the right thing to do. But if 'samsung,burst-clock-frequency' is
> not specified for DSIM I get a DSI PLL configuration failure. There is no
> mode yet, thus samsung_dsim_enable_clock() tries to configure the PLL for
> 0Hz. There is another reconfiguration while pre_enabling the chain targeting
> the correct clock (891000000Hz for 1920x1080), so this seems fine.
> But I'm not sure if the 1st config error has any negative side effects.

Will be fixed in V2, thanks for pointing this out.