Commit 4c35e5d1 authored by Ville Syrjälä's avatar Ville Syrjälä

drm/i915: Activate DRRS after state readout

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 1b333c67 ("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: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221020120706.25728-1-ville.syrjala@linux.intel.comReviewed-by: default avatarJani Nikula <jani.nikula@intel.com>
parent 2bd0db4b
...@@ -1261,8 +1261,6 @@ static void intel_post_plane_update(struct intel_atomic_state *state, ...@@ -1261,8 +1261,6 @@ static void intel_post_plane_update(struct intel_atomic_state *state,
if (needs_cursorclk_wa(old_crtc_state) && if (needs_cursorclk_wa(old_crtc_state) &&
!needs_cursorclk_wa(new_crtc_state)) !needs_cursorclk_wa(new_crtc_state))
icl_wa_cursorclkgating(dev_priv, pipe, false); 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, 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, ...@@ -5646,39 +5644,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
PIPE_CONF_CHECK_I(name.y2); \ PIPE_CONF_CHECK_I(name.y2); \
} while (0) } 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 { \ #define PIPE_CONF_CHECK_FLAGS(name, mask) do { \
if ((current_config->name ^ pipe_config->name) & (mask)) { \ if ((current_config->name ^ pipe_config->name) & (mask)) { \
pipe_config_mismatch(fastset, crtc, __stringify(name), \ pipe_config_mismatch(fastset, crtc, __stringify(name), \
...@@ -5747,7 +5712,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, ...@@ -5747,7 +5712,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
if (HAS_DOUBLE_BUFFERED_M_N(dev_priv)) { if (HAS_DOUBLE_BUFFERED_M_N(dev_priv)) {
if (!fastset || !pipe_config->seamless_m_n) 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 { } else {
PIPE_CONF_CHECK_M_N(dp_m_n); PIPE_CONF_CHECK_M_N(dp_m_n);
PIPE_CONF_CHECK_M_N(dp_m2_n2); PIPE_CONF_CHECK_M_N(dp_m2_n2);
...@@ -7615,6 +7580,12 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state) ...@@ -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); 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 * DSB cleanup is done in cleanup_work aligning with framebuffer
* cleanup. So copy and reset the dsb structure to sync with * cleanup. So copy and reset the dsb structure to sync with
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment