drm/i915/display: Refactor intel_commit_modeset_disables()

Commit 9c722e17 ("drm/i915: Disable pipes in reverse order")
reverted the order that pipes gets disabled because of TGL
master/slave relationship between transcoders in MST mode.

But as stated in a comment in skl_commit_modeset_enables() the
enabling order is not always crescent, possibly causing previously
selected slave transcoder being enabled before master so another
approach will be needed to select a transcoder to master in MST mode.
It will be similar to the approach taken in port sync.

But instead of implement something like
intel_trans_port_sync_modeset_disables() to MST lets simply it and
iterate over all pipes 2 times, the first one disabling any slave and
then disabling everything else.
The MST bits will be added in another patch.

v2:
Not using crtc->active as it is deprecated

v3:
Removing is_trans_port_sync_mode() check, just check for
is_trans_port_sync_master() is enough

v4:
Adding and using is_trans_port_sync_slave(), otherwise non-port sync
pipes will be disabled in the first loop, what is not wrong but is
not what patch description promises

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (v2)
Signed-off-by: default avatarJosé Roberto de Souza <jose.souza@intel.com>
Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191205210350.96795-3-jose.souza@intel.com
parent 3ca8f191
...@@ -561,6 +561,12 @@ is_trans_port_sync_master(const struct intel_crtc_state *crtc_state) ...@@ -561,6 +561,12 @@ is_trans_port_sync_master(const struct intel_crtc_state *crtc_state)
crtc_state->sync_mode_slaves_mask); crtc_state->sync_mode_slaves_mask);
} }
static bool
is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state)
{
return crtc_state->master_transcoder != INVALID_TRANSCODER;
}
/* /*
* Platform specific helpers to calculate the port PLL loopback- (clock.m), * Platform specific helpers to calculate the port PLL loopback- (clock.m),
* and post-divider (clock.p) values, pre- (clock.vco) and post-divided fast * and post-divider (clock.p) values, pre- (clock.vco) and post-divided fast
...@@ -14393,77 +14399,47 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state, ...@@ -14393,77 +14399,47 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
dev_priv->display.initial_watermarks(state, crtc); dev_priv->display.initial_watermarks(state, crtc);
} }
static void intel_trans_port_sync_modeset_disables(struct intel_atomic_state *state,
struct intel_crtc *crtc,
struct intel_crtc_state *old_crtc_state,
struct intel_crtc_state *new_crtc_state)
{
struct intel_crtc *slave_crtc = intel_get_slave_crtc(new_crtc_state);
struct intel_crtc_state *new_slave_crtc_state =
intel_atomic_get_new_crtc_state(state, slave_crtc);
struct intel_crtc_state *old_slave_crtc_state =
intel_atomic_get_old_crtc_state(state, slave_crtc);
WARN_ON(!slave_crtc || !new_slave_crtc_state ||
!old_slave_crtc_state);
/* Disable Slave first */
intel_pre_plane_update(state, slave_crtc);
if (old_slave_crtc_state->hw.active)
intel_old_crtc_state_disables(state,
old_slave_crtc_state,
new_slave_crtc_state,
slave_crtc);
/* Disable Master */
intel_pre_plane_update(state, crtc);
if (old_crtc_state->hw.active)
intel_old_crtc_state_disables(state,
old_crtc_state,
new_crtc_state,
crtc);
}
static void intel_commit_modeset_disables(struct intel_atomic_state *state) static void intel_commit_modeset_disables(struct intel_atomic_state *state)
{ {
struct intel_crtc_state *new_crtc_state, *old_crtc_state; struct intel_crtc_state *new_crtc_state, *old_crtc_state;
struct intel_crtc *crtc; struct intel_crtc *crtc;
u32 handled = 0;
int i; int i;
/* /* Only disable port sync slaves */
* Disable CRTC/pipes in reverse order because some features(MST in for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
* TGL+) requires master and slave relationship between pipes, so it
* should always pick the lowest pipe as master as it will be enabled
* first and disable in the reverse order so the master will be the
* last one to be disabled.
*/
for_each_oldnew_intel_crtc_in_state_reverse(state, crtc, old_crtc_state,
new_crtc_state, i) { new_crtc_state, i) {
if (!needs_modeset(new_crtc_state)) if (!needs_modeset(new_crtc_state))
continue; continue;
if (!old_crtc_state->hw.active)
continue;
/* In case of Transcoder port Sync master slave CRTCs can be /* In case of Transcoder port Sync master slave CRTCs can be
* assigned in any order and we need to make sure that * assigned in any order and we need to make sure that
* slave CRTCs are disabled first and then master CRTC since * slave CRTCs are disabled first and then master CRTC since
* Slave vblanks are masked till Master Vblanks. * Slave vblanks are masked till Master Vblanks.
*/ */
if (is_trans_port_sync_mode(old_crtc_state)) { if (!is_trans_port_sync_slave(old_crtc_state))
if (is_trans_port_sync_master(old_crtc_state))
intel_trans_port_sync_modeset_disables(state,
crtc,
old_crtc_state,
new_crtc_state);
else
continue; continue;
} else {
intel_pre_plane_update(state, crtc); intel_pre_plane_update(state, crtc);
intel_old_crtc_state_disables(state, old_crtc_state,
new_crtc_state, crtc);
handled |= BIT(crtc->pipe);
}
/* Disable everything else left on */
for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
if (!needs_modeset(new_crtc_state) ||
(handled & BIT(crtc->pipe)))
continue;
intel_pre_plane_update(state, crtc);
if (old_crtc_state->hw.active) if (old_crtc_state->hw.active)
intel_old_crtc_state_disables(state, intel_old_crtc_state_disables(state, old_crtc_state,
old_crtc_state, new_crtc_state, crtc);
new_crtc_state,
crtc);
}
} }
} }
......
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