Re: [PATCH v9 10/22] drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] dw_dp_bridge_atomic_get_input_bus_fmts blindly propagates unsupported formats, breaking bus format negotiation fallback.
--

--- Patch [10]: [PATCH v9 10/22] drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED ---
commit 536f8f5a7df863a273e0062b67fbc6ca7b57d8a8
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED

Add support for MEDIA_BUS_FMT_FIXED, which is requested for USB-C DP
chains when the last bridge does not implement atomic_get_output_bus_fmts().
This resolves the output format by using the negotiated input format in 
atomic_check and implements a custom atomic_get_input_bus_fmts hook.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index bde938b0d116d..e60dab0d3b9dd 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
[ ... ]
> @@ -1823,6 +1827,32 @@ static u32 *dw_dp_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
>  	return output_fmts;
>  }
>  
> +static u32 *
> +dw_dp_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
> +				       struct drm_bridge_state *bridge_state,
> +				       struct drm_crtc_state *crtc_state,
> +				       struct drm_connector_state *conn_state,
> +				       u32 output_fmt,
> +				       unsigned int *num_input_fmts)
> +{
> +	/*
> +	 * MEDIA_BUS_FMT_FIXED means the downstream bridge does not constrain
> +	 * the bus format. In that case, advertise all formats supported by the
> +	 * DP link so the upstream encoder can negotiate the best match.
> +	 */
> +	if (output_fmt == MEDIA_BUS_FMT_FIXED)
> +		return dw_dp_bridge_atomic_get_output_bus_fmts(bridge,
> +							       bridge_state,
> +							       crtc_state,
> +							       conn_state,
> +							       num_input_fmts);
> +
> +	return drm_atomic_helper_bridge_propagate_bus_fmt(bridge, bridge_state,
> +							 crtc_state, conn_state,
> +							 output_fmt,
> +							 num_input_fmts);
> +}

[Severity: High]
This is a pre-existing issue, but since we are adding a custom
dw_dp_bridge_atomic_get_input_bus_fmts() hook here, does this code blindly
propagate downstream formats even if they aren't supported by the dw-dp
hardware?

If a downstream bridge proposes an unsupported preferred bus format,
propagating it directly via drm_atomic_helper_bridge_propagate_bus_fmt() makes
the format negotiation succeed prematurely. It seems this would skip testing
any valid fallback formats.

Later during the modeset, this results in a complete failure (a black screen)
when dw_dp_bridge_atomic_check() checks if the negotiated format is actually
supported:

dw_dp_bridge_atomic_check() {
        ...
        fmt = dw_dp_get_output_format(out_bus_format);
        if (!fmt)
                return -EINVAL;
}

Should this new hook check if output_fmt is supported by dw-dp (e.g., verifying
it exists in dw_dp_output_formats) before propagating it, to allow the bridge
chain to properly negotiate a fallback format?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-synopsys-dw-dp-improvements-v9-0-2e6e46135312@collabora.com?part=10