Re: [PATCH v11 09/21] 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] The DW DP bridge accepts explicitly negotiated output bus formats without verifying if the resulting bit-per-pixel (BPP) exceeds the DP link bandwidth.
--

--- Patch [9]: [PATCH v11 09/21] drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED ---
commit 8f5ee9653fefa35534059addf0b8fba75d680cad
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED

This commit adds support for MEDIA_BUS_FMT_FIXED for USB-C DP chains where
the final bridge does not implement atomic_get_output_bus_fmts(). It updates
atomic_check to resolve the output format from the negotiated input format,
and adds a custom .atomic_get_input_bus_fmts hook to advertise all
bandwidth-validated formats when FIXED is used.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index d35dcd4a413f7..b9864a17c01d0 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c

[ ... ]

> @@ -1830,6 +1834,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 does propagating the downstream format
unconditionally bypass bandwidth checks?

When output_fmt is not MEDIA_BUS_FMT_FIXED, it is propagated directly. Then
later in dw_dp_bridge_atomic_check(), the corresponding BPP is assigned
to the state:

    fmt = dw_dp_get_output_format(out_bus_format);
    if (!fmt)
        return -EINVAL;
    
    ...
    state->bpp = fmt->bpp;

It appears the format's BPP is assigned without validating it against the DP
link capabilities. If a downstream display requests an explicit, high-BPP
format (e.g., 36-bpp RGB) that exceeds the link's maximum bandwidth for the
selected resolution, could this cause the driver to accept an unsupported
configuration?

If the atomic check succeeds but the hardware lacks the bandwidth, would this
result in a failure to transmit the video stream, causing a blank screen or
link training failure? 

Should the explicitly negotiated format be validated against link bandwidth
limits here or in atomic_check?

> +}
> +
>  static struct drm_bridge_state *dw_dp_bridge_atomic_duplicate_state(struct drm_bridge *bridge)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-synopsys-dw-dp-improvements-v11-0-0d508505f383@collabora.com?part=9