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 <[email protected]> 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-0d508505f...@collabora.com?part=9
