From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
commit a2b6e99d8a623544f3bdccd28ee35b9c1b00daa5 upstream.
Keeping DC states enabled is incompatible with the _noarm()/_arm()
split we use for writing pipe/plane registers. When DC5 and PSR
are enabled, all pipe/plane registers effectively become self-arming
on account of DC5 exit arming the update, and PSR exit latching it.
What probably saves us most of the time is that (with PIPE_MISC[21]=0)
all pipe register writes themselves trigger PSR exit, and then
we don't re-enter PSR until the idle frame count has elapsed.
So it may be that the PSR exit happens already before we've
updated the state too much.
Also the PSR1 panel (at least on this KBL) seems to discard the first
frame we trasmit, presumably still scanning out from its internal
framebuffer at that point. So only the second frame we transmit is
actually visible. But I suppose that could also be panel specific
behaviour. I haven't checked out how other PSR panels behave, nor
did I bother to check what the eDP spec has to say about this.
And since this really is all about DC states, let's switch from
the MODESET domain to the DC_OFF domain. Functionally they are
100% identical. We should probably remove the MODESET domain...
And for good measure let's toss in an assert to the place where
we do the _noarm() register writes to make sure DC states are
in fact off.
v2: Just use intel_display_power_is_enabled() (Imre)
Cc: <stable@xxxxxxxxxxxxxxx> #v5.17+
Cc: Manasi Navare <navaremanasi@xxxxxxxxxx>
Cc: Drew Davenport <ddavenport@xxxxxxxxxxxx>
Cc: Jouni Högander <jouni.hogander@xxxxxxxxx>
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
Fixes: d13dde449580 ("drm/i915: Split pipe+output CSC programming to noarm+arm pair")
Fixes: f8a005eb8972 ("drm/i915: Optimize icl+ universal plane programming")
Fixes: 890b6ec4a522 ("drm/i915: Split skl+ plane update into noarm+arm pair")
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Link: https://patchwork.freedesktop.org/patch/msgid/20230320183532.17727-1-ville.syrjala@xxxxxxxxxxxxxxx
(cherry picked from commit 41b4c7fe72b6105a4b49395eea9aa40cef94288d)
Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_display.c | 28 ++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7124,6 +7124,8 @@ static void intel_update_crtc(struct int
intel_fbc_update(state, crtc);
+ drm_WARN_ON(&i915->drm, !intel_display_power_is_enabled(i915, POWER_DOMAIN_DC_OFF));
+
if (!modeset &&
(new_crtc_state->uapi.color_mgmt_changed ||
new_crtc_state->update_pipe))
@@ -7500,8 +7502,28 @@ static void intel_atomic_commit_tail(str
drm_atomic_helper_wait_for_dependencies(&state->base);
drm_dp_mst_atomic_wait_for_dependencies(&state->base);
- if (state->modeset)
- wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
+ /*
+ * During full modesets we write a lot of registers, wait
+ * for PLLs, etc. Doing that while DC states are enabled
+ * is not a good idea.
+ *
+ * During fastsets and other updates we also need to
+ * disable DC states due to the following scenario:
+ * 1. DC5 exit and PSR exit happen
+ * 2. Some or all _noarm() registers are written
+ * 3. Due to some long delay PSR is re-entered
+ * 4. DC5 entry -> DMC saves the already written new
+ * _noarm() registers and the old not yet written
+ * _arm() registers
+ * 5. DC5 exit -> DMC restores a mixture of old and
+ * new register values and arms the update
+ * 6. PSR exit -> hardware latches a mixture of old and
+ * new register values -> corrupted frame, or worse
+ * 7. New _arm() registers are finally written
+ * 8. Hardware finally latches a complete set of new
+ * register values, and subsequent frames will be OK again
+ */
+ wakeref = intel_display_power_get(dev_priv, POWER_DOMAIN_DC_OFF);
intel_atomic_prepare_plane_clear_colors(state);
@@ -7640,8 +7662,8 @@ static void intel_atomic_commit_tail(str
* the culprit.
*/
intel_uncore_arm_unclaimed_mmio_detection(&dev_priv->uncore);
- intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET, wakeref);
}
+ intel_display_power_put(dev_priv, POWER_DOMAIN_DC_OFF, wakeref);
intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
/*
