"Luca Ceresoli" <[email protected]> writes:
> 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.
Yes. I will add tags.
>> 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.
I will split it into two patches.
>> 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);
Fixing, thanks.
/Esben
