Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/i915/hdmi: pipe_bpp corruption in intel_hdmi_compute_frl_clock restricts fallback --
commit e2a7d5ac5410d9fb50137634f445228551f6d758 Author: Ankit Nautiyal <[email protected]> Subject: drm/i915/hdmi: Add FRL clock computation based on DFM This commit computes the FRL port clock for a mode using the DFM helper. It starts with the lowest FRL rate meeting the DFM requirements and checks if FRL can support the mode. > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c > b/drivers/gpu/drm/i915/display/intel_hdmi.c > index 8e742165a50ad..0391597c9847f 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c [ ... ] > +static int > +intel_hdmi_compute_frl_clock(struct intel_encoder *encoder, > + struct intel_crtc_state *crtc_state) > +{ > + struct intel_hdmi *hdmi = enc_to_intel_hdmi(encoder); > + int max_bpc = max(crtc_state->pipe_bpp / 3, 8); > + int port_clock; > + int bpc; > + > + for (bpc = max_bpc; bpc >= 8; bpc -= 2) { > + int ret; > + > + if (!hdmi_bpc_possible(crtc_state, bpc)) > + continue; > + > + crtc_state->pipe_bpp = bpc * 3; > + > + ret = intel_hdmi_compute_frl_config(encoder, crtc_state); > + if (ret) > + continue; > + > + /* Port clock for FRL rates is bitrate in 10Kbps */ > + port_clock = FRL_GBPS_TO_10KBPS(crtc_state->frl.required_rate); > + > + if (hdmi_port_frl_clock_valid(hdmi, port_clock) == MODE_OK) { > + crtc_state->port_clock = port_clock; > + return 0; > + } > + } > + > + return -EINVAL; [Severity: Medium] Does returning -EINVAL here leave crtc_state->pipe_bpp corrupted? Because pipe_bpp is modified in-place within the loop, if all FRL configurations fail, the function exits with pipe_bpp retaining the last tested value (typically 24). When this happens, the caller intel_hdmi_compute_formats() will reuse the exact same crtc_state object for a YCbCr420 fallback attempt: intel_hdmi_compute_formats() { ret = intel_hdmi_compute_output_format(..., INTEL_OUTPUT_FORMAT_RGB, ...); if (ret && drm_mode_is_420_also(...)) ret = intel_hdmi_compute_output_format(..., INTEL_OUTPUT_FORMAT_YCBCR420, ...); } Since max_bpc is derived from crtc_state->pipe_bpp, the fallback is then artificially restricted to only testing 8 bpc, missing valid 10/12 bpc capabilities. Should pipe_bpp be saved before the loop and restored to its original value before returning -EINVAL? > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=38
