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

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 unsuppor=
ted 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 <[email protected]>
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=20
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/br=
idge/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_fmt=
s(struct drm_bridge *bridge,
>  	return output_fmts;
>  }
> =20
> +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 =3D=3D 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() ma=
kes
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 scree=
n)
when dw_dp_bridge_atomic_check() checks if the negotiated format is actually
supported:

dw_dp_bridge_atomic_check() {
        ...
        fmt =3D 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., verif=
ying
it exists in dw_dp_output_formats) before propagating it, to allow the brid=
ge
chain to properly negotiate a fallback format?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803-synopsys-d=
[email protected]?part=3D10