Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/i915/display: TGDDA workaround state leaks to HDMI modesets --
commit 7ca764c5b58c7f2901c9b7de8005ba2561ae1426 Author: Ankit Nautiyal <[email protected]> drm/i915/display: Add bits for Wa_14021768792 for linkm/n ratio > 10 To support a Link M/N ratio between 10.0 and 15.0, for some BMG ultrajoiner cases we need Wa_14021768792. To bypass the hardware limitation within the Timing Generator DDA (TGDDA), we need to program the LINKM and LINKN registers as defined in the WA. Along with this we also need the relevant bits in HDMI_EMP_DATA and CHICKEN_TRANS. > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > index 6d2025e109956..1e9d8943626a7 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c [ ... ] > @@ -2738,6 +2749,26 @@ bool intel_cpu_transcoder_has_m2_n2(struct > intel_display *display, > return IS_DISPLAY_VER(display, 5, 7) || display->platform.cherryview; > } > > +static void bmg_enhanced_tgdda_write(struct intel_crtc *crtc, > + enum transcoder transcoder, > + const struct intel_link_m_n *m_n) > +{ > + struct intel_display *display = to_intel_display(crtc); > + bool enable = m_n->bypass_m_n_ratio_limit && m_n->link_n; > + u32 link_m_frac = enable ? m_n->link_m % m_n->link_n : 0; > + > + if (!intel_display_wa(display, INTEL_DISPLAY_WA_14021768792)) > + return; > + > + intel_de_rmw(display, HDMI_EMP_DATA(display, transcoder), > + HDMI_EMP_DATA_LINK_M_FRAC_MASK, > + REG_FIELD_PREP(HDMI_EMP_DATA_LINK_M_FRAC_MASK, > link_m_frac)); > + > + intel_de_rmw(display, CHICKEN_TRANS(display, transcoder), > + DP_ENHANCED_TGDDA_ENABLE, > + enable ? DP_ENHANCED_TGDDA_ENABLE : 0); > +} > + > void intel_cpu_transcoder_set_m1_n1(struct intel_crtc *crtc, > enum transcoder transcoder, > const struct intel_link_m_n *m_n) > @@ -2745,6 +2776,8 @@ void intel_cpu_transcoder_set_m1_n1(struct intel_crtc > *crtc, > struct intel_display *display = to_intel_display(crtc); > enum pipe pipe = crtc->pipe; > > + bmg_enhanced_tgdda_write(crtc, transcoder, m_n); > + > if (DISPLAY_VER(display) >= 5) [Severity: Medium] The commit message states that CHICKEN_TRANS and HDMI_EMP_DATA "are explicitly cleared when the mechanism is not in use, so that state cannot leak from a previous modeset." Does this clearing logic work if the transcoder is repurposed for HDMI? The clearing logic is placed in bmg_enhanced_tgdda_write(), which is called exclusively from intel_cpu_transcoder_set_m1_n1(). Looking at hsw_configure_cpu_transcoder(), intel_cpu_transcoder_set_m1_n1() is only called for DP and FDI encoders: } else if (intel_crtc_has_dp_encoder(crtc_state)) { intel_cpu_transcoder_set_m1_n1(crtc, cpu_transcoder, &crtc_state->dp_m_n); intel_cpu_transcoder_set_m2_n2(crtc, cpu_transcoder, &crtc_state->dp_m2_n2); } If a transcoder is first used for DisplayPort (triggering the workaround and setting DP_ENHANCED_TGDDA_ENABLE) and subsequently repurposed for HDMI, intel_cpu_transcoder_set_m1_n1() is completely skipped. Will the Transcoder's Timing Generator DDA remain in the enhanced DP workaround mode and potentially corrupt HDMI timings? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
