Hi Marek,

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 <ma...@denx.de>
> ---
> Cc: Adam Ford <aford...@gmail.com>
> Cc: Alexander Stein <alexander.st...@ew.tq-group.com>
> Cc: Andrzej Hajda <andrzej.ha...@intel.com>
> Cc: Daniel Vetter <dan...@ffwll.ch>
> Cc: David Airlie <airl...@gmail.com>
> Cc: Frieder Schrempf <frieder.schre...@kontron.de>
> Cc: Inki Dae <inki....@samsung.com>
> Cc: Jagan Teki <ja...@amarulasolutions.com>
> Cc: Jernej Skrabec <jernej.skra...@gmail.com>
> Cc: Jonas Karlman <jo...@kwiboo.se>
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Cc: Lucas Stach <l.st...@pengutronix.de>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Marek Szyprowski <m.szyprow...@samsung.com>
> Cc: Maxime Ripard <mrip...@kernel.org>
> Cc: Michael Walle <mwa...@kernel.org>
> Cc: Neil Armstrong <neil.armstr...@linaro.org>
> Cc: Robert Foss <rf...@kernel.org>
> Cc: Thomas Zimmermann <tzimmerm...@suse.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: ker...@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.st...@ew.tq-group.com>

>       if (!hs_clk) {
>               dev_err(dsi->dev, "failed to configure DSI PLL\n");
> @@ -1643,9 +1647,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;
>  
> -     return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge,
> -                              flags);
> +     ret = pm_runtime_resume_and_get(dsi->dev);
> +     if (ret < 0)
> +             return ret;
> +
> +     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 = {
> 


-- 
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/


Reply via email to