[PATCH v4 06/11] drm/bridge: cdns-dsi: Wait for Clk and Data Lanes to be ready

[Tomi Valkeinen]

On 22/06/2024 14:09, Aradhya Bhatia wrote:
> Once the DSI Link and DSI Phy are initialized, the code needs to wait
> for Clk and Data Lanes to be ready, before continuing configuration.
> This is in accordance with the DSI Start-up procedure, found in the
> Technical Reference Manual of Texas Instrument's J721E SoC[0] which
> houses this DSI TX controller.
> 
> If the previous bridge (or crtc/encoder) are configured pre-maturely,
> the input signal FIFO gets corrupt. This introduces a color-shift on the
> display.
> 
> Allow the driver to wait for the clk and data lanes to get ready during
> DSI enable.
> 
> [0]: See section 12.6.5.7.3 "Start-up Procedure" in J721E SoC TRM
>       TRM Link: http://www.ti.com/lit/pdf/spruil1
> 
> Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver")
> Tested-by: Dominik Haller <d.haller at phytec.de>
> Signed-off-by: Aradhya Bhatia <a-bhatia1 at ti.com>
> ---
>   drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> index 426f77092341..126e4bccd868 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> @@ -764,7 +764,7 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge)
>   	struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy;
>   	unsigned long tx_byte_period;
>   	struct cdns_dsi_cfg dsi_cfg;
> -	u32 tmp, reg_wakeup, div;
> +	u32 tmp, reg_wakeup, div, status;
>   	int nlanes;
>   
>   	if (WARN_ON(pm_runtime_get_sync(dsi->base.dev) < 0))
> @@ -781,6 +781,17 @@ static void cdns_dsi_bridge_enable(struct drm_bridge *bridge)
>   	cdns_dsi_init_link(dsi);
>   	cdns_dsi_hs_init(dsi);
>   
> +	/*
> +	 * Now that the DSI Link and DSI Phy are initialized,
> +	 * wait for the CLK and Data Lanes to be ready.
> +	 */
> +	tmp = CLK_LANE_RDY;
> +	for (int i = 0; i < nlanes; i++)
> +		tmp |= DATA_LANE_RDY(i);
> +
> +	WARN_ON_ONCE(readl_poll_timeout(dsi->regs + MCTL_MAIN_STS, status,
> +					status & tmp, 100, 0));

I think an error print is more suitable than WARN_ON_ONCE(). Other than 
that:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>

  Tomi

> +
>   	writel(HBP_LEN(dsi_cfg.hbp) | HSA_LEN(dsi_cfg.hsa),
>   	       dsi->regs + VID_HSIZE1);
>   	writel(HFP_LEN(dsi_cfg.hfp) | HACT_LEN(dsi_cfg.hact),