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: dda7dcd9da ("drm/i915/vrr: Use fixed timings for platforms that support VRR")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260303095414.4331-1-ville.syrjala@linux.intel.com
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
(cherry picked from commit 93f3a267c3dd4d811b224bb9e179a10d81456a74)
Signed-off-by: Tvrtko Ursulin <tursulin@ursulin.net>
Write DC Balance parameters to hw registers.
--v2:
- Update commit header.
- Separate crtc_state params from this patch. (Ankit)
--v3:
- Write registers at compute config.
- Update condition for write.
--v4:
- Address issue with state checker.
--v5:
- Initialise some more dc balance register while enabling VRR.
--v6:
- FLIPLINE_CFG need to be configure at last, as it is double buffer
arming point.
--v7:
- Initialise and reset live value of vmax and vmin as well.
--v8:
- Add separate functions while writing hw registers. (Ankit)
--v9:
- Add DC Balance counter enable bit to this patch. (Ankit)
--v10:
- Add rigister writes to vrr_enable/disable. (Ankit)
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Link: https://patch.msgid.link/20251223104542.2688548-12-mitulkumar.ajitkumar.golani@intel.com
Track dc balance flip count with params per crtc. Increment
DC Balance Flip count before every flip to indicate DMC
firmware about new flip occurrence which needs to be adjusted
for dc balancing. This is tracked separately from legacy
FLIP_COUNT register also Reset DC balance flip count value
while disabling VRR adaptive mode, this is to start with
fresh counts when VRR adaptive refresh mode is triggered again.
--v2:
- Call during intel_update_crtc.(Ankit)
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Link: https://patch.msgid.link/20251223104542.2688548-11-mitulkumar.ajitkumar.golani@intel.com
Compute DC Balance parameters and tunable params based on
experiments.
--v2:
- Document tunable params. (Ankit)
--v3:
- Add line spaces to compute config. (Ankit)
- Remove redundancy checks.
--v4:
- Separate out conpute config to separate function.
- As all the valuse are being computed in scanlines, and slope
is still in usec, convert and store it to scanlines.
--v5:
- Update and add comments for slope calculation. (Ankit)
- Update early return conditions for dc balance compute. (Ankit)
--v6:
- Early return condition simplified for dc balance compute config. (Ankit)
- Make use of pipe restrictions to this patch. (Ankit)
--v7:
- Separate out PIPE_A and PIPE_B restrictions to other patch.(Ankit)
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Link: https://patch.msgid.link/20251223104542.2688548-8-mitulkumar.ajitkumar.golani@intel.com
Add DC Balance params to crtc_state, also add state checker
params for related properties.
--v3:
- Seggregate crtc_state params with this patch. (Ankit)
--v4:
- Update commit message and header. (Ankit)
- Add +1 to VMIN and VMAX only when it is non-zero. (Ankit)
--v5:
- Add headers in sorted order. (Jani Nikula)
--v6:
- Add a separate function to get and check dc_balance params.
- Avoid repeatative use of MMIO read. (Ankit)
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Link: https://patch.msgid.link/20251223104542.2688548-6-mitulkumar.ajitkumar.golani@intel.com
intel_de_wait*() take the timeout in milliseconds. Include
that information in the function name to make life less
confusing. I'll also be introducing microsecond variants
of these later.
Done with cocci:
@@
@@
(
static int
- intel_de_wait
+ intel_de_wait_ms
(...)
{
...
}
|
static int
- intel_de_wait_fw
+ intel_de_wait_fw_ms
(...)
{
...
}
|
static int
- intel_de_wait_for_set
+ intel_de_wait_for_set_ms
(...)
{
...
}
|
static int
- intel_de_wait_for_clear
+ intel_de_wait_for_clear_ms
(...)
{
...
}
)
@@
@@
(
- intel_de_wait
+ intel_de_wait_ms
|
- intel_de_wait_fw
+ intel_de_wait_fw_ms
|
- intel_de_wait_for_set
+ intel_de_wait_for_set_ms
|
- intel_de_wait_for_clear
+ intel_de_wait_for_clear_ms
)
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20251110172756.2132-4-ville.syrjala@linux.intel.com
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Currently, depending on vrr.enable, we may write TRANS_VRR_CTL from
both intel_vrr_set_transcoder_timings() and intel_vrr_transcoder_enable()
on !always_use_vrr_tg() platforms. Streamline this so that we just
always write it from intel_vrr_set_transcoder_timings(), and
never from intel_vrr_transcoder_enable().
The main benefit is that intel_vrr_transcoder_enable() becomes symmetric
to intel_vrr_transcoder_disable().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20251020185038.4272-16-ville.syrjala@linux.intel.com
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Currently intel_vrr_disable() writes TRANS_VRR_CTL() with
trans_vrr_ctl(), whereas intel_vrr_transcoder_disable() always
writes just a plain 0. Write trans_vrr_ctl() in both places to
unify the code, allowing for more shared code in the future.
Since the VRR timing generator will be disabled by the
TRANS_VRR_CTL write it doesn't really matter what we write to
the register (other than VRR_CTL_VRR_ENABLE that is).
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20251020185038.4272-12-ville.syrjala@linux.intel.com
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
The way intel_vrr_compute_*_timings() works is rather confusing.
First intel_vrr_compute_config() assigns the computed vmin/vmax
into crtc_state->vrr.{vmin,vmax}, and then either
intel_vrr_compute_vrr_timings() leaves them untouched or
intel_vrr_compute_{cmrr,fixed_rr}_timings() overwrite them with
something else.
Clean this up by moving all crtc_state->vrr.{vmin,vmax} assignments
into intel_vrr_compute_*_timings().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20251020185038.4272-7-ville.syrjala@linux.intel.com
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
In the current VRR implementation, vrr.vmin and vrr.guardband are set such
that they do not need to change when switching from fixed refresh rate to
variable refresh rate. Specifically, vrr.guardband is always set to match
the vblank length. This approach works for most cases, but not for LRR,
where the guardband would need to change while the VRR timing generator is
still active.
With the VRR TG always active, live updates to guardband are unsafe and not
recommended. To ensure hardware safety, guardband was moved out of the
!fastset block, meaning any change now requires a full modeset.
This breaks seamless LRR switching, which was previously supported.
Since the problem arises from guardband being matched to the vblank length,
solution is to use a minimal, sufficient static value, instead. So we use a
static guardband defined during mode-set that fits within the smallest
expected vblank and remains unchanged in case of features like LRR where
vtotal changes. To compute this minimum guardband we take into account
latencies/delays due to different features as mentioned in the Bspec.
Introduce a helper to compute the minimal sufficient guardband.
On platforms where the VRR timing generator is always ON, we optimize the
guardband regardless of whether the display is operating in fixed or
variable refresh rate mode.
v2:
- Use max of sagv latency and skl_wm_latency(1) for PM delay
computation. (Ville)
- Avoid guardband optimization for HDMI for now. (Ville)
- Allow guardband optimization only for platforms with
intel_vrr_always_use_vrr_tg = true. (Ville)
- Add comments for PM delay and a #TODO note for HDMI.
v3: Drop the variable prefill_min_guardband. (Ville)
Bspec: 70151
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://lore.kernel.org/r/20251017123504.2247954-5-ankit.k.nautiyal@intel.com
As we move towards using a shorter, optimized guardband, we need to adjust
how the delayed vblank start is computed.
Adjust the crtc_vblank_start using Vmin Vtotal - guardband only when
intel_vrr_always_use_vrr_tg() is true. Also update the
pipe_mode->crtc_vblank_start which is derived from
adjusted_mode->crtc_vblank_start in intel_crtc_compute_pipe_mode().
To maintain consistency between the computed and readout paths, update
the readout logic in intel_vrr_get_config() to overwrite crtc_vblank_start
with the same value (vtotal - guardband) on platforms with always-on
VRR TG.
This also paves way for guardband optimization, by handling the movement of
the crtc_vblank_start for platforms that have VRR TG always active.
v2: Drop the helper and add the adjustment directly to
intel_vrr_compute_guardband(). (Ville)
v3: Use adjusted_mode.crtc_vtotal instead of vmin and include the readout
logic to keep the compute and readout paths in sync. (Ville)
v4: Also set pipe_mode->crtc_vblank_start as its derived from
adjusted_mode. (Ville)
v5: Add a comment about rationale behind updating
pipe_mode->crtc_vblank_start. (Ville)
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://lore.kernel.org/r/20251016055415.2101347-10-ankit.k.nautiyal@intel.com
The helper intel_vrr_compute_config_late() practically just computes the
guardband. Rename intel_vrr_compute_config_late() to
intel_vrr_compute_guardband().
Since we are going to compute the guardband and then move the
vblank_start for optmizing guardband move it to
intel_crtc_compute_config() which handles such changes.
v2: Move the function at the last after clocks, pipe_mode etc. are all
set. (Ville)
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://lore.kernel.org/r/20251016055415.2101347-4-ankit.k.nautiyal@intel.com
The helper intel_vrr_vblank_delay() was used to keep track of the SCL
lines + the extra vblank delay required for ICL/TGL.
This was used to wait for sufficient lines for:
-push send bit to clear for VRR case
-evasion to delay the commit.
For first case we are using safe window scanline wait and with that we
just need to wait for SCL lines, we do not need to wait for the extra
vblank delay required for ICL/TGL. For the second case, we actually
do not need to wait for extra lines before the undelayed vblank, if we
are already in the safe window.
To sum up, SCL lines is sufficient for both cases.
So drop the helper intel_vrr_vblank_delay and just use
crtc_state->set_context_latency instead.
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://lore.kernel.org/r/20250924141542.3122126-10-ankit.k.nautiyal@intel.com
The maximum guardband value is constrained by two factors:
- The actual vblank length minus set context latency (SCL)
- The hardware register field width:
- 8 bits for ICL/TGL (VRR_CTL_PIPELINE_FULL_MASK -> max 255)
- 16 bits for ADL+ (XELPD_VRR_CTL_VRR_GUARDBAND_MASK -> max 65535)
Remove the #FIXME and clamp the guardband to the maximum allowed value.
v2:
- Use REG_FIELD_MAX(). (Ville)
- Separate out functions for intel_vrr_max_guardband(),
intel_vrr_max_vblank_guardband(). (Ville)
v3:
- Fix Typo: Add the missing adjusted_mode->crtc_vdisplay in guardband
computation. (Ville)
- Refactor intel_vrr_max_hw_guardband() and use else for consistency.
(Ville)
v4:
- Drop max_guardband from intel_vrr_max_hw_guardband(). (Ville)
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (#v2)
Link: https://lore.kernel.org/r/20250924141542.3122126-9-ankit.k.nautiyal@intel.com
Until LNL, intel_dsb_wait_vblanks() used to wait for the undelayed vblank
start. However, from PTL onwards, it waits for the start of the
safe-window defined by the number of lines programmed in the register
TRANS_SET_CONTEXT_LATENCY. This change was introduced to move the SCL
window out of the vblank region, supporting modes with higher refresh
rates and smaller vblanks. This change introduces a "safe window" a
scanline range from (undelayed vblank - SCL) to (delayed vblank - SCL).
As a result, on PTL+ platforms, the DSB wait for vblank completes exactly
SCL lines earlier than the undelayed vblank start (safe window start).
If the flip occurs in the active region and the push happens before the
vmin decision boundary, the DSB wait fires early, and the push is sent
inside this safe window. In such cases, the push bit is cleared at the
delayed vblank, but our wait logic does not account for the early trigger,
leading to DSB poll errors.
To fix this, we add an explicit wait for the end of the safe window i.e.,
the scanline range from (undelayed vblank - SCL) to (delayed vblank - SCL).
Once past this window, we are exactly SCL lines away from the delayed
vblank, and our existing wait logic works as intended.
This additional wait is only effective if the push occurs before the vmin
decision boundary. If the push happens after the boundary, the hardware
already guarantees we're SCL lines away from the delayed vblank, and the
extra wait becomes a no-op.
v2:
- Use helpers for safe window start/end. (Ville)
- Move the extra wait inside the helper to wait for delayed vblank. (Ville)
- Update the commit message.
v3:
- Add more documentation for explanation for the wait. (Ville)
- Rename intel_vrr_vmin_safe_window_start/end as this is vmin safe
window. (Ville)
- Minor refactoring to align with the code. (Ville)
- Update the commit message for more clarity.
v4:
- Retain name for intel_vrr_safe_window_start as it doesn't change with
vmin/vmax etc. (Ville)
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://lore.kernel.org/r/20250924141542.3122126-7-ankit.k.nautiyal@intel.com
For now guardband is equal to the vblank length so ideally it should be
computed as difference between the vmin vtotal and vactive. However
since we are having few lines as SCL, we need to account for this while
computing the guardband.
Since the vblank start is moved by SCL lines from the vactive, the delta
between the vmin vtotal and new vblank start was used to account for this.
Now that SCL is explicitly tracked using the `set_context_latency` member,
use it directly in the guardband calculation.
In the future, when the guardband is shortened or optimized, we may need
to factor in both the change in the vblank start and SCL lines. For now,
explicitly accounting for SCL is sufficient.
v2: Fix typo: replace adjusted_mode->vdisplay with
adjusted_mode->crtc_vdisplay. (Ville)
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://lore.kernel.org/r/20250924141542.3122126-5-ankit.k.nautiyal@intel.com
ICL/TGL VRR hardware won't allow us to program flipline==vmin. If we do
that the actual effect will be the same as if we had programmed
flipline=vmin+1, which would make the minimum vtotal one scanline taller
than expected.
To compensate for this we reduce vmin by one, and then program
flipline=vmin+1. So we end up with a flipline value that matches
the expected minimum vtotal. Currently this adjustment happens
in intel_vrr_compute_config() which means that crtc_state->vrr.vmin
will no longer be directly usable for the remainder of the high
level VRR code. That is annoying at best, fragile at worst.
Hide the adjustment in low level code instead. This will allow most
of the higher level VRR code to remain blissfully ignorant about this
fact. Afterwards crtc_state->vrr.{vmin,flipline} will be equal
and match the minimum vtotal, exactly how things already work
on ADL+.
The only slight downside is that the actual register value will no
longer match crtc_state->vrr.vmin on ICL/TGL, but that may already
be the case on TGL because the register value will also have been
adjusted by the SCL.
Note that we must change the guardband calculation to account
for intel_vrr_extra_vblank_delay() explicitly. Previously that
was accidentally handled by the earlier vmin reduction by
intel_vrr_flipline_offset().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250918232226.25295-2-ville.syrjala@linux.intel.com
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Currently our crtc_state->vrr.{vmin.vmax,flipline} are mangled on
TGL to account for the SCL delay (the hardware requires this mangling
or the actual vtotals will become incorrect). Unfortunately this
means that one can't simply use these values directly in many places,
and instead we always have to go through functions that undo the
damage first. This is all rather fragile.
Simplify our lives a bit by hiding this mangling deeper inside
the low level VRR code, leaving the number stored in the crtc
state actually something that humans can use.
This does introduce a dependdency as intel_vrr_get_config()
will now need to know the SCL value, which is read out in
intel_get_transcoder_timings(). I suppose we could simply
duplicate the SCL readout in both places should this become
a real hinderance. For now just leave a note around the
intel_get_transcoder_timings() call to remind us.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250917203446.14374-6-ville.syrjala@linux.intel.com
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
While ICL/TGL VRR hardware doesn't have a register for the guardband
value, our lives will be simpler if we pretend that it does. Start
by computing the guardband the same as on ADL+ and storing it in
the state, and only then we convert it into the corresponding
pipeline_full value that the hardware can consume. During readout we
do the opposite.
I was debating whether to completely remove pipeline_full from the
crtc state, but decided to keep it for now. Mainly because we check
it in vrr_params_changed() and simply checking the guardband instead
isn't 100% equivalent; Theoretically, framestart_delay may have
changed in the opposite direction to pipeline_full, keeping the
derived guardband value unchaged. One solution would be to also check
framestart_delay, but that feels a bit leaky abstraction wise.
Also note that we don't currently handle the maximum limit of 255
scanlines for the pipeline_full in a very nice way. The actual
position of the delayed vblank will move because of that clamping,
and so some of our code may get confused. But fixing this shall
wait a for now.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250917203446.14374-4-ville.syrjala@linux.intel.com
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
