Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/i915: FRL mode valid misses 3D frame packing clock doubling --
commit 357e437906e7e9f540a08210f7b85cf2f3e109aa Author: Ankit Nautiyal <[email protected]> drm/i915/hdmi: Validate FRL modes in mode_valid using the FRL DFM This commit adds a fallback FRL capability check to intel_hdmi_mode_valid_format() using the non-DSC FRL DFM parameters to validate modes that exceed TMDS limits. It also introduces intel_hdmi_can_support_frl() as a placeholder returning false until FRL support is completed. > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c > b/drivers/gpu/drm/i915/display/intel_hdmi.c > index 67228b538e81a..b82c636b46f93 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c [ ... ] > @@ -2021,6 +2022,51 @@ intel_hdmi_tmds_mode_clock_valid(struct drm_connector > *_connector, int clock, > return status; > } > [ ... ] > +static enum drm_mode_status > +intel_hdmi_frl_mode_clock_valid(struct intel_connector *connector, > + const struct drm_display_mode *mode, > + enum intel_output_format output_format) > +{ > + struct intel_hdmi *hdmi = intel_attached_hdmi(connector); > + struct intel_hdmi_frl_dfm frl_dfm = {}; > + int max_rate = hdmi->max_frl_rate; > + int lanes = max_rate < 24 ? 3 : 4; > + > + frl_dfm.config.pixel_clock_nominal_khz = mode->clock; [Severity: High] Does this code miss the 3D frame packing clock doubling? In intel_hdmi_mode_valid(), the clock variable is doubled for 3D Frame Packing modes: if ((mode->flags & DRM_MODE_FLAG_3D_MASK) == DRM_MODE_FLAG_3D_FRAME_PACKING) clock *= 2; This adjusted clock is passed to intel_hdmi_mode_valid_format() and used by the TMDS validation path. However, intel_hdmi_frl_mode_clock_valid() directly reads mode->clock, which means it uses the unadjusted clock. Could this cause the FRL Data Flow Management (DFM) capacity check to underestimate the required bandwidth by a factor of 2 for 3D Frame Packing modes, erroneously accepting modes that exceed the physical capacity of the FRL link? > + frl_dfm.config.hactive = mode->hdisplay; > + frl_dfm.config.hblank = mode->htotal - mode->hdisplay; > + [ ... ] > @@ -2052,6 +2105,7 @@ intel_hdmi_mode_valid_format(struct intel_connector > *connector, > enum intel_output_format sink_format) > { > struct intel_display *display = to_intel_display(connector); > + struct intel_hdmi *hdmi = intel_attached_hdmi(connector); > enum drm_mode_status status; > > status = intel_hdmi_sink_format_valid(connector, mode, > @@ -2063,7 +2117,12 @@ intel_hdmi_mode_valid_format(struct intel_connector > *connector, > if (status != MODE_OK) > return status; > > - return intel_hdmi_tmds_mode_clock_valid(&connector->base, clock, > has_hdmi_sink, sink_format); > + status = intel_hdmi_tmds_mode_clock_valid(&connector->base, clock, > has_hdmi_sink, sink_format); > + > + if (status != MODE_OK && intel_hdmi_can_support_frl(hdmi)) > + status = intel_hdmi_frl_mode_clock_valid(connector, mode, > sink_format); [Severity: High] Since intel_hdmi_frl_mode_clock_valid() is called without the adjusted clock parameter, it will use the unadjusted mode->clock. Should we pass the adjusted clock variable to intel_hdmi_frl_mode_clock_valid() similar to how it is passed to intel_hdmi_tmds_mode_clock_valid()? > + > + return status; > } > > static enum drm_mode_status -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=36
