Re: [PATCH v4 02/10] drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The bridge configures itself for RGB888 when MEDIA_BUS_FMT_FIXED is requested, but fails to propagate this requirement to the upstream encoder during bus format negotiation.
- [High] Forcing 24 bpp (RGB888) when MEDIA_BUS_FMT_FIXED is requested bypasses DP link bandwidth validation, allowing display modes that exceed the physical link capacity.
--

--- Patch [2]: [PATCH v4 02/10] drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED ---
commit 98faee8d93ff4e450a6ec04ce4882b2871508172
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 when the downstream
bridge does not implement atomic_get_output_bus_fmts(). This patch coerces
MEDIA_BUS_FMT_FIXED to MEDIA_BUS_FMT_RGB888_1X24 in the atomic check phase.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 368e1f85e6253..aca6816febe67 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> @@ -1538,7 +1539,10 @@ static int dw_dp_bridge_atomic_check(struct drm_bridge *bridge,
>  	state = to_dw_dp_bridge_state(bridge_state);
>  	mode = &state->mode;
>  
> -	fmt = dw_dp_get_output_format(bridge_state->output_bus_cfg.format);
> +	if (out_bus_format == MEDIA_BUS_FMT_FIXED)
> +		out_bus_format = MEDIA_BUS_FMT_RGB888_1X24;

[Severity: High]
Will this coercion fail to propagate upstream? 

The dw-dp driver still uses the generic helper
drm_atomic_helper_bridge_propagate_bus_fmt for its .atomic_get_input_bus_fmts
hook. This helper propagates the originally requested MEDIA_BUS_FMT_FIXED
format to the upstream encoder, rather than the locally overridden RGB888
format. 

If MEDIA_BUS_FMT_FIXED is propagated to an upstream encoder (like Rockchip
VOP), could it choose an incompatible default output format (like YUV422)
since dw-dp operates strictly in RGB888? Should this driver implement a
custom .atomic_get_input_bus_fmts hook to propagate MEDIA_BUS_FMT_RGB888_1X24
instead?

[Severity: High]
Could forcing MEDIA_BUS_FMT_RGB888_1X24 (24 bpp) here bypass DP link
bandwidth validation? 

When the downstream bridge lacks an .atomic_get_output_bus_fmts() hook, the
DRM core skips the output format validation. Since dw_dp_bridge_mode_valid()
only verifies bandwidth against a minimum possible bpp (e.g., 18 bpp), could
we end up allowing display modes that physically fit at 18 bpp but exceed the
DP link capacity at 24 bpp? 

Should this code call dw_dp_bandwidth_ok() after forcing the format to ensure
the hardware link can actually support 24 bpp?

> +
> +	fmt = dw_dp_get_output_format(out_bus_format);
>  	if (!fmt)
>  		return -EINVAL;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-synopsys-dw-dp-improvements-v4-0-f0f4a4ede712@collabora.com?part=2