On Wed, Mar 04, 2026 at 11:36:12AM +0530, Nautiyal, Ankit K wrote:
>
> On 3/3/2026 3:24 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä <[email protected]>
> >
> > Apparently ICL may hang with an MCE if we write TRANS_VRR_VMAX/FLIPLINE
> > before enabling TRANS_DDI_FUNC_CTL.
> >
> > Personally I was only able to reproduce a hang (on an Dell XPS 7390
> > 2-in-1) with an external display connected via a dock using a dodgy
> > type-C cable that made the link training fail. After the failed
> > link training the machine would hang. TGL seemed immune to the
> > problem for whatever reason.
> >
> > BSpec does tell us to configure VRR after enabling TRANS_DDI_FUNC_CTL
> > as well. The DMC firmware also does the VRR restore in two stages:
> > - first stage seems to be unconditional and includes TRANS_VRR_CTL
> > and a few other VRR registers, among other things
> > - second stage is conditional on the DDI being enabled,
> > and includes TRANS_DDI_FUNC_CTL and TRANS_VRR_VMAX/VMIN/FLIPLINE,
> > among other things
> >
> > So let's reorder the steps to match to avoid the hang, and
> > toss in an extra WARN to make sure we don't screw this up later.
> >
> > BSpec: 22243
> > Cc: [email protected]
> > Cc: Ankit Nautiyal <[email protected]>
> > Reported-by: Benjamin Tissoires <[email protected]>
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15777
> > Tested-by: Benjamin Tissoires <[email protected]>
> > Fixes: dda7dcd9da73 ("drm/i915/vrr: Use fixed timings for platforms that
> > support VRR")
> > Signed-off-by: Ville Syrjälä <[email protected]>
>
> You are right. VRR timing registers are indeed supposed to be programmed
> after TRANS_DDI_FUNC_CTL.
>
> Thanks for catching this, Ville, and thanks Benjamin for the bisection.
>
> Change looks good to me.
>
> Reviewed-by: Ankit Nautiyal <[email protected]>
Pushed.
Thanks for the review, and the bug report.
>
>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 1 -
> > drivers/gpu/drm/i915/display/intel_vrr.c | 14 ++++++++++++++
> > 2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 27354585ba92..138ee7dd1977 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1637,7 +1637,6 @@ static void hsw_configure_cpu_transcoder(const struct
> > intel_crtc_state *crtc_sta
> > }
> >
> > intel_set_transcoder_timings(crtc_state);
> > - intel_vrr_set_transcoder_timings(crtc_state);
> >
> > if (cpu_transcoder != TRANSCODER_EDP)
> > intel_de_write(display, TRANS_MULT(display, cpu_transcoder),
> > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > index 00ca76dbdd6c..8a957804cb97 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > @@ -599,6 +599,18 @@ void intel_vrr_set_transcoder_timings(const struct
> > intel_crtc_state *crtc_state)
> > if (!HAS_VRR(display))
> > return;
> >
> > + /*
> > + * Bspec says:
> > + * "(note: VRR needs to be programmed after
> > + * TRANS_DDI_FUNC_CTL and before TRANS_CONF)."
> > + *
> > + * In practice it turns out that ICL can hang if
> > + * TRANS_VRR_VMAX/FLIPLINE are written before
> > + * enabling TRANS_DDI_FUNC_CTL.
> > + */
> > + drm_WARN_ON(display->drm,
> > + !(intel_de_read(display, TRANS_DDI_FUNC_CTL(display,
> > cpu_transcoder)) & TRANS_DDI_FUNC_ENABLE));
> > +
> > /*
> > * This bit seems to have two meanings depending on the platform:
> > * TGL: generate VRR "safe window" for DSB vblank waits
> > @@ -961,6 +973,8 @@ void intel_vrr_transcoder_enable(const struct
> > intel_crtc_state *crtc_state)
> > {
> > struct intel_display *display = to_intel_display(crtc_state);
> >
> > + intel_vrr_set_transcoder_timings(crtc_state);
> > +
> > if (!intel_vrr_possible(crtc_state))
> > return;
> >
--
Ville Syrjälä
Intel
