Re: [PATCH v5 05/15] 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] dw_dp_bridge_atomic_check ignores the negotiated input bus format when the output format is MEDIA_BUS_FMT_FIXED, forcing the highest-bandwidth format instead.
- [Low] OOM condition masked as -EINVAL instead of -ENOMEM in dw_dp_bridge_atomic_check.
--

--- Patch [5]: [PATCH v5 05/15] drm/bridge: synopsys: dw-dp: Support MEDIA_BUS_FMT_FIXED ---
commit 22a7a7c58c5ed8233a59740b01dc41f49ab3cdc8
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

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

This patch adds support for MEDIA_BUS_FMT_FIXED to the Synopsys DW-DP bridge.
It updates atomic_check to resolve MEDIA_BUS_FMT_FIXED by selecting the first
bandwidth-filtered output format, and implements a custom
atomic_get_input_bus_fmts hook to advertise all supported formats for upstream
negotiation.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 3dec7777fb285..812c67fdfb6b8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> @@ -1586,7 +1587,25 @@ 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) {
> +		unsigned int num_fmts = 0;
> +		u32 *fmts;
> +
> +		fmts = dw_dp_bridge_atomic_get_output_bus_fmts(bridge,
> +							       bridge_state,
> +							       crtc_state,
> +							       conn_state,
> +							       &num_fmts);
> +		if (!num_fmts) {
> +			kfree(fmts);
> +			return -EINVAL;

[Severity: Low]
Does this mask an out-of-memory condition? 

If dw_dp_bridge_atomic_get_output_bus_fmts() fails to allocate memory via
kcalloc(), it returns NULL and sets num_fmts to 0. Returning -EINVAL here
obscures the actual -ENOMEM error.

> +		}
> +
> +		out_bus_format = fmts[0];

[Severity: High]
Does this unconditionally override the successfully negotiated input format?

If the upstream element negotiated a format other than fmts[0] (which would
be stored in bridge_state->input_bus_cfg.format), won't forcing fmts[0] here
cause a format mismatch between the hardware and the upstream encoder?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-synopsys-dw-dp-improvements-v5-0-9445c2e87441@collabora.com?part=5