public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915: Activate DRRS after state readout
@ 2022-10-20 12:07 Ville Syrjala
  2022-10-20 16:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Ville Syrjala @ 2022-10-20 12:07 UTC (permalink / raw)
  To: intel-gfx

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

On BDW+ we have just the one set of DP M/N registers. The
values we write into said registers depends on whether we
want DRRS to be in high or low gear. This causes issues
for the state checker which currently has to assume either
set of M/N (high or low refresh rate) values may appear there.
That sort of works for M/N itself, but all other values
derived from the M/N (dotclock, pixel rate) are not handled
correctly, leading to potential for state checker mismatches.

Let's avoid all those problems by simply keeping DRRS in
high gear until the state checker has done its hardware
state readout.

Note that hitting this issue presumable became very hard
after commit 1b333c679a0f ("drm/i915: Do DRRS disable/enable
during pre/post_plane_update()") since the state check would
have to laze about for one full second (delay used by
intel_drrs_schedule_work()) to see the low refresh rate.
But it is still theoretically possible.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 43 ++++----------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 606f9140d024..906a5ad2bbfa 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1261,8 +1261,6 @@ static void intel_post_plane_update(struct intel_atomic_state *state,
 	if (needs_cursorclk_wa(old_crtc_state) &&
 	    !needs_cursorclk_wa(new_crtc_state))
 		icl_wa_cursorclkgating(dev_priv, pipe, false);
-
-	intel_drrs_activate(new_crtc_state);
 }
 
 static void intel_crtc_enable_flip_done(struct intel_atomic_state *state,
@@ -5646,39 +5644,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	PIPE_CONF_CHECK_I(name.y2); \
 } while (0)
 
-/* This is required for BDW+ where there is only one set of registers for
- * switching between high and low RR.
- * This macro can be used whenever a comparison has to be made between one
- * hw state and multiple sw state variables.
- */
-#define PIPE_CONF_CHECK_M_N_ALT(name, alt_name) do { \
-	if (!intel_compare_link_m_n(&current_config->name, \
-				    &pipe_config->name) && \
-	    !intel_compare_link_m_n(&current_config->alt_name, \
-				    &pipe_config->name)) { \
-		pipe_config_mismatch(fastset, crtc, __stringify(name), \
-				     "(expected tu %i data %i/%i link %i/%i, " \
-				     "or tu %i data %i/%i link %i/%i, " \
-				     "found tu %i, data %i/%i link %i/%i)", \
-				     current_config->name.tu, \
-				     current_config->name.data_m, \
-				     current_config->name.data_n, \
-				     current_config->name.link_m, \
-				     current_config->name.link_n, \
-				     current_config->alt_name.tu, \
-				     current_config->alt_name.data_m, \
-				     current_config->alt_name.data_n, \
-				     current_config->alt_name.link_m, \
-				     current_config->alt_name.link_n, \
-				     pipe_config->name.tu, \
-				     pipe_config->name.data_m, \
-				     pipe_config->name.data_n, \
-				     pipe_config->name.link_m, \
-				     pipe_config->name.link_n); \
-		ret = false; \
-	} \
-} while (0)
-
 #define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
 	if ((current_config->name ^ pipe_config->name) & (mask)) { \
 		pipe_config_mismatch(fastset, crtc, __stringify(name), \
@@ -5747,7 +5712,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 	if (HAS_DOUBLE_BUFFERED_M_N(dev_priv)) {
 		if (!fastset || !pipe_config->seamless_m_n)
-			PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
+			PIPE_CONF_CHECK_M_N(dp_m_n);
 	} else {
 		PIPE_CONF_CHECK_M_N(dp_m_n);
 		PIPE_CONF_CHECK_M_N(dp_m2_n2);
@@ -7615,6 +7580,12 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 
 		intel_modeset_verify_crtc(crtc, state, old_crtc_state, new_crtc_state);
 
+		/*
+		 * Activate DRRS after state readout to avoid
+		 * dp_m_n vs. dp_m2_n2 confusion on BDW+.
+		 */
+		intel_drrs_activate(new_crtc_state);
+
 		/*
 		 * DSB cleanup is done in cleanup_work aligning with framebuffer
 		 * cleanup. So copy and reset the dsb structure to sync with
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-21 21:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-20 12:07 [Intel-gfx] [PATCH] drm/i915: Activate DRRS after state readout Ville Syrjala
2022-10-20 16:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-10-20 19:34 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-10-21  9:06 ` [Intel-gfx] [PATCH] " Jani Nikula
2022-10-21 13:23 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Activate DRRS after state readout (rev2) Patchwork
2022-10-21 21:39 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox