[Intel-gfx] [PATCH v10 10/11] drm/i915: Ensure correct master/slave enable/disable sequence
Ville Syrjälä
ville.syrjala at linux.intel.com
Mon Oct 19 16:26:19 UTC 2020
On Thu, Oct 08, 2020 at 02:45:34PM -0700, Manasi Navare wrote:
> From: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>
> Enabling is done in a special sequence and so should plane updates
> be. Ideally the end user never notices the second pipe is used.
>
> This way ideally everything will be tear free, and updates are
> really atomic as userspace expects it.
>
> This uses generic modeset_enables() calls like trans port sync
> but still has special handling for disable since for slave we
> should not disable things like encoder, plls that are not enabled
> for slave.
>
> v3:
> * Fixes in enable and disable sequence from testing (Manasi)
> v2:
> * Manual Rebase (Manasi)
> * Refactoring on intel_update_crtc and enable_crtc and removing
> special trans_port_sync_update (Manasi)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 55 ++++++++++++++------
> drivers/gpu/drm/i915/display/intel_sprite.c | 5 +-
> 2 files changed, 43 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 357cc2bce300..101ddd0b48ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15878,6 +15878,9 @@ static void intel_enable_crtc(struct intel_atomic_state *state,
>
> dev_priv->display.crtc_enable(state, crtc);
>
> + if (new_crtc_state->bigjoiner_slave)
> + return;
> +
> /* vblanks work again, re-enable pipe CRC. */
> intel_crtc_enable_pipe_crc(crtc);
> }
> @@ -15914,9 +15917,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>
> commit_pipe_config(state, crtc);
>
> - if (new_crtc_state->bigjoiner) {
> - /* Not supported yet */
> - } else if (INTEL_GEN(dev_priv) >= 9)
> + if (INTEL_GEN(dev_priv) >= 9)
> skl_update_planes_on_crtc(state, crtc);
> else
> i9xx_update_planes_on_crtc(state, crtc);
> @@ -15945,9 +15946,17 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> drm_WARN_ON(&dev_priv->drm, old_crtc_state->bigjoiner_slave);
>
> intel_crtc_disable_planes(state, crtc);
> - if (old_crtc_state->bigjoiner)
> +
> + /*
> + * We still need special handling for disabling bigjoiner master
> + * and slaves since for slave we do not have encoder or plls
> + * so we dont need to disable those.
> + */
> + if (old_crtc_state->bigjoiner) {
> intel_crtc_disable_planes(state,
> old_crtc_state->bigjoiner_linked_crtc);
> + old_crtc_state->bigjoiner_linked_crtc->active = false;
> + }
>
> /*
> * We need to disable pipe CRC before disabling the pipe,
> @@ -15977,7 +15986,7 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
> /* Only disable port sync and MST slaves */
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> - if (!needs_modeset(new_crtc_state) || old_crtc_state->bigjoiner_slave)
> + if (!needs_modeset(new_crtc_state) || old_crtc_state->bigjoiner)
> continue;
>
> if (!old_crtc_state->hw.active)
> @@ -16040,6 +16049,7 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> struct intel_crtc *crtc;
> struct intel_crtc_state *old_crtc_state, *new_crtc_state;
> struct skl_ddb_entry entries[I915_MAX_PIPES] = {};
> + struct skl_ddb_entry new_entries[I915_MAX_PIPES] = {};
> u8 update_pipes = 0, modeset_pipes = 0;
> int i;
>
> @@ -16056,6 +16066,14 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> } else {
> modeset_pipes |= BIT(pipe);
> }
> +
> + new_entries[i] = new_crtc_state->wm.skl.ddb;
> +
> + /* ignore allocations for crtc's that have been turned off during modeset. */
> + if (needs_modeset(new_crtc_state))
> + continue;
> +
> + entries[i] = old_crtc_state->wm.skl.ddb;
> }
>
> /*
> @@ -16071,28 +16089,28 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> enum pipe pipe = crtc->pipe;
> + bool ddb_changed;
>
> if ((update_pipes & BIT(pipe)) == 0)
> continue;
>
> - if (skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> + if (skl_ddb_allocation_overlaps(&new_entries[pipe],
> entries, I915_MAX_PIPES, pipe))
> continue;
>
> - entries[pipe] = new_crtc_state->wm.skl.ddb;
> + ddb_changed = !skl_ddb_entry_equal(&new_entries[pipe], &entries[pipe]);
> + entries[pipe] = new_entries[pipe];
> update_pipes &= ~BIT(pipe);
>
> - intel_update_crtc(state, crtc);
> -
> /*
> * If this is an already active pipe, it's DDB changed,
> * and this isn't the last pipe that needs updating
> * then we need to wait for a vblank to pass for the
> * new ddb allocation to take effect.
> */
> - if (!skl_ddb_entry_equal(&new_crtc_state->wm.skl.ddb,
> - &old_crtc_state->wm.skl.ddb) &&
> - (update_pipes | modeset_pipes))
> + intel_update_crtc(state, crtc);
> +
> + if (ddb_changed && (update_pipes | modeset_pipes))
> intel_wait_for_vblank(dev_priv, pipe);
What are these ddb changes trying to achieve?
> }
> }
> @@ -16110,7 +16128,8 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> continue;
>
> if (intel_dp_mst_is_slave_trans(new_crtc_state) ||
> - is_trans_port_sync_master(new_crtc_state))
> + is_trans_port_sync_master(new_crtc_state) ||
> + (new_crtc_state->bigjoiner && !new_crtc_state->bigjoiner_slave))
> continue;
>
> modeset_pipes &= ~BIT(pipe);
> @@ -16120,7 +16139,7 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
>
> /*
> * Then we enable all remaining pipes that depend on other
> - * pipes: MST slaves and port sync masters.
> + * pipes: MST slaves and port sync masters, big joiner master
> */
> for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
> enum pipe pipe = crtc->pipe;
> @@ -16128,6 +16147,10 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> if ((modeset_pipes & BIT(pipe)) == 0)
> continue;
>
> + WARN_ON(skl_ddb_allocation_overlaps(&new_entries[pipe],
> + entries, I915_MAX_PIPES, pipe));
> +
> + entries[pipe] = new_entries[pipe];
> modeset_pipes &= ~BIT(pipe);
>
> intel_enable_crtc(state, crtc);
> @@ -16142,10 +16165,10 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> if ((update_pipes & BIT(pipe)) == 0)
> continue;
>
> - drm_WARN_ON(&dev_priv->drm, skl_ddb_allocation_overlaps(&new_crtc_state->wm.skl.ddb,
> + drm_WARN_ON(&dev_priv->drm, skl_ddb_allocation_overlaps(&new_entries[pipe],
> entries, I915_MAX_PIPES, pipe));
>
> - entries[pipe] = new_crtc_state->wm.skl.ddb;
> + entries[pipe] = new_entries[pipe];
> update_pipes &= ~BIT(pipe);
>
> intel_update_crtc(state, crtc);
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 9e235210adc7..1c740a22a8d7 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -103,6 +103,8 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
>
> /* FIXME needs to be calibrated sensibly */
> min = vblank_start - intel_usecs_to_scanlines(adjusted_mode,
> + new_crtc_state->bigjoiner ?
> + 2 * VBLANK_EVASION_TIME_US :
> VBLANK_EVASION_TIME_US);
> max = vblank_start - 1;
>
> @@ -227,7 +229,8 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
>
> spin_lock(&crtc->base.dev->event_lock);
> drm_crtc_arm_vblank_event(&crtc->base,
> - new_crtc_state->uapi.event);
> + new_crtc_state->uapi.event);
> +
Spurious whitespace change.
> spin_unlock(&crtc->base.dev->event_lock);
>
> new_crtc_state->uapi.event = NULL;
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
More information about the Intel-gfx
mailing list