On Mon, 03 Aug 2026 16:56:19 +0200, Esben Haabendal <[email protected]> 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 <[email protected]> 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
