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

[Marek Vasut]

On 6/25/24 4:37 PM, Alexander Stein wrote:
> Hi Marek,

Hi,

> Am Dienstag, 25. Juni 2024, 14:26:10 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
>> ---
>> V2: Handle case where mode is not set yet
>> ---
>>   drivers/gpu/drm/bridge/samsung-dsim.c | 32 ++++++++++++++++++++-------
>>   1 file changed, 24 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index e7e53a9e42afb..22d3bbd866d97 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -699,20 +699,24 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
>>   
>>   static int samsung_dsim_enable_clock(struct samsung_dsim *dsi)
>>   {
>> -	unsigned long hs_clk, byte_clk, esc_clk, pix_clk;
>> +	unsigned long hs_clk, byte_clk, esc_clk;
>>   	unsigned long esc_div;
>>   	u32 reg;
>>   	struct drm_display_mode *m = &dsi->mode;
>>   	int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>>   
>> -	/* m->clock is in KHz */
>> -	pix_clk = m->clock * 1000;
>> -
>> -	/* Use burst_clk_rate if available, otherwise use the pix_clk */
>> +	/*
>> +	 * Use burst_clk_rate if available, otherwise use the mode clock
>> +	 * if mode is already set and available, otherwise fall back to
>> +	 * PLL input clock and operate in 1:1 lowest frequency mode until
>> +	 * a mode is set.
>> +	 */
>>   	if (dsi->burst_clk_rate)
>>   		hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
>> +	else if (m)	/* m->clock is in KHz */
>> +		hs_clk = samsung_dsim_set_pll(dsi, DIV_ROUND_UP(m->clock * 1000 * bpp, dsi->lanes));
>>   	else
>> -		hs_clk = samsung_dsim_set_pll(dsi, DIV_ROUND_UP(pix_clk * bpp, dsi->lanes));
>> +		hs_clk = dsi->pll_clk_rate;
>>   
> 
> I can't reproduce that mentioned corner case and presumably this problem
> does not exist otherwise. If samsung,burst-clock-frequency is unset I get
> a sluggish image on the monitor.
> 
> It seems the calculation is using a adjusted clock frequency:
> samsung-dsim 32e60000.dsi: Using pixel clock for HS clock frequency
> samsung-dsim 32e60000.dsi: [drm:samsung_dsim_host_attach [samsung_dsim]] Attached tc358767 device (lanes:4 bpp:24 mode-flags:0xc03)
> samsung-dsim 32e60000.dsi: PLL ref clock freq 24000000
> samsung-dsim 32e60000.dsi: failed to find PLL PMS for requested frequency
> samsung-dsim 32e60000.dsi: failed to configure DSI PLL
> samsung-dsim 32e60000.dsi: PLL ref clock freq 24000000
> samsung-dsim 32e60000.dsi: PLL freq 883636363, (p 11, m 810, s 1)
> samsung-dsim 32e60000.dsi: hs_clk = 883636363, byte_clk = 110454545, esc_clk = 9204545
> samsung-dsim 32e60000.dsi: calculated hfp: 60, hbp: 105, hsa: 27
> samsung-dsim 32e60000.dsi: LCD size = 1920x1080
> 
> 891 MHz is the nominal one for 148.5 MHz pixelclock. But even setting
> samsung,burst-clock-frequency to 891 MHz results in a sluggish image.
> Maybe this usecase is nothing I need to consider while using DSI-DP
> with EDID timings available.
> 
> As the burst clock needs to provide more bandwidth than actually needed,
> I consider this a different usecase not providing
> samsung,burst-clock-frequency for DSI->DP usage.
> 
> But the initialization upon attach is working intended, so:
> Reviewed-by: Alexander Stein <alexander.stein at ew.tq-group.com>

Thank you for testing and keeping up with this. I will wait for more 
feedback if there is any (Frieder? Lucas? Michael?). If there are no 
objections, then I can merge it in a week or two ?