Re: [PATCH v4 07/10] drm/i915/display: Add bits for Wa_14021768792 for linkm/n ratio > 10

[sashiko-bot@kernel.org]

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 <ankit.k.nautiyal@intel.com>

    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/20260917050649.871960-1-suraj.kandpal@intel.com?part=7