Re: [PATCH v2 1/4] drm/bridge: nwl-dsi: Limit LCDIF specific sync override

[Luca Ceresoli]

On Mon, 03 Aug 2026 16:56:19 +0200, Esben Haabendal <esben@xxxxxxxxxx> wrote:

Hi Esben,

> When using DCSS with NWL, overriding the mode flags to enforce active high
> sync is preventing the use of active low with downstream bridges, such as
> ti-sn65dsi83, which will not see such mode flags set by the panel.
> 
> Signed-off-by: Esben Haabendal <esben@xxxxxxxxxx>

Is this a fix? If so, it should have Fixes: + Cc: stable.

>
>
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
> index 09992529f3d8..0fb10b686d1f 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -821,10 +823,13 @@ static int nwl_dsi_bridge_atomic_check(struct drm_bridge *bridge,
>  				       struct drm_connector_state *conn_state)
>  {
>  	struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
> +	struct nwl_dsi *dsi = bridge_to_dsi(bridge);
>  
>  	/* At least LCDIF + NWL needs active high sync */
> -	adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
> -	adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
> +	if (dsi->endpoint == NWL_DSI_ENDPOINT_LCDIF) {
> +		adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
> +		adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
> +	}

In some sense, this patch is doing two things. One is the hunk above, doing
what the commit message says...

> @@ -1103,11 +1107,12 @@ static int nwl_dsi_select_input(struct nwl_dsi *dsi)
>  				      "No valid input endpoint found\n");
>  			return -EINVAL;
>  		}
> +		dsi->endpoint = NWL_DSI_ENDPOINT_DCSS;
>  	}

...and this is a somewhat orthogonal change, changing the way the endpoint
selection is stored: either as a bool (false = LCDIF, true = DCSS) or as an
index (0 = LCDIF, 1 = DCSS). No problem with this change, I think it makes
code more readable. Just I'd rather keep it as a separate cleanup patch.

>  
>  	DRM_DEV_INFO(dsi->dev, "Using %s as input source\n",
> -		     (use_dcss) ? "DCSS" : "LCDIF");
> -	ret = mux_control_try_select(dsi->mux, use_dcss);
> +		     (dsi->endpoint == NWL_DSI_ENDPOINT_DCSS) ? "DCSS" : "LCDIF");
> +	ret = mux_control_try_select(dsi->mux, dsi->endpoint == NWL_DSI_ENDPOINT_DCSS);

The code here is not formally correct. mux_control_try_select() takes a
state argument, which is an integer, but you are passing a bool. It would
still work just because bools are mapped to 0/1. Looks like should simply
become:

	ret = mux_control_try_select(dsi->mux, dsi->endpoint);

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com