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 <[email protected]> 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-9445c2e87...@collabora.com?part=5
