[PATCH] drm/i915/vrr: Configure VRR timings after enabling TRANS_DDI_FUNC_CTL

[PATCH] drm/i915/vrr: Configure VRR timings after enabling TRANS_DDI_FUNC_CTL

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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: stable@vger.kernel.org
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reported-by: Benjamin Tissoires <bentiss@kernel.org>
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15777
Tested-by: Benjamin Tissoires <bentiss@kernel.org>
Fixes: dda7dcd9da73 ("drm/i915/vrr: Use fixed timings for platforms that support VRR")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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;
 
-- 
2.52.0

Re: [PATCH] drm/i915/vrr: Configure VRR timings after enabling TRANS_DDI_FUNC_CTL

[Nautiyal, Ankit K]

On 3/3/2026 3:24 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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: stable@vger.kernel.org
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Reported-by: Benjamin Tissoires <bentiss@kernel.org>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15777
> Tested-by: Benjamin Tissoires <bentiss@kernel.org>
> Fixes: dda7dcd9da73 ("drm/i915/vrr: Use fixed timings for platforms that support VRR")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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


> ---
>   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;
>   

Re: [PATCH] drm/i915/vrr: Configure VRR timings after enabling TRANS_DDI_FUNC_CTL

[Ville Syrjälä]

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ä <ville.syrjala@linux.intel.com>
> >
> > 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: stable@vger.kernel.org
> > Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > Reported-by: Benjamin Tissoires <bentiss@kernel.org>
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15777
> > Tested-by: Benjamin Tissoires <bentiss@kernel.org>
> > Fixes: dda7dcd9da73 ("drm/i915/vrr: Use fixed timings for platforms that support VRR")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 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 <ankit.k.nautiyal@intel.com>

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