From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Manasi Navare <manasi.d.navare@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v10 10/11] drm/i915: Ensure correct master/slave enable/disable sequence
Date: Mon, 19 Oct 2020 19:26:19 +0300 [thread overview]
Message-ID: <20201019162619.GU6112@intel.com> (raw)
In-Reply-To: <20201008214535.22942-10-manasi.d.navare@intel.com>
On Thu, Oct 08, 2020 at 02:45:34PM -0700, Manasi Navare wrote:
> From: Maarten Lankhorst <maarten.lankhorst@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@linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-10-19 16:26 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-08 21:45 [Intel-gfx] [PATCH v10 01/11] HAX to make DSC work on the icelake test system Manasi Navare
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 02/11] drm/i915/display: Rename pipe_timings to transcoder_timings Manasi Navare
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 03/11] drm/i915: Add hw.pipe_mode to allow bigjoiner pipe/transcoder split Manasi Navare
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 04/11] drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3 Manasi Navare
2020-10-14 11:26 ` Ville Syrjälä
2020-10-14 19:04 ` Navare, Manasi
2020-10-15 11:52 ` Ville Syrjälä
2020-10-15 16:26 ` Navare, Manasi
2020-10-19 16:30 ` Ville Syrjälä
2020-10-19 22:51 ` Navare, Manasi
2020-10-20 18:39 ` Ville Syrjälä
2020-10-20 18:53 ` Navare, Manasi
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 05/11] drm/i915: Try to make bigjoiner work in atomic check Manasi Navare
2020-10-14 11:33 ` Ville Syrjälä
2020-10-14 19:09 ` Navare, Manasi
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 06/11] drm/i915: Enable big joiner support in enable and disable sequences Manasi Navare
2020-10-14 11:07 ` Jani Nikula
2020-10-15 13:07 ` Ville Syrjälä
2020-10-15 16:37 ` Navare, Manasi
2020-10-16 16:06 ` Ville Syrjälä
2020-10-16 18:17 ` Navare, Manasi
2020-10-16 18:50 ` Ville Syrjälä
2020-10-16 19:24 ` Navare, Manasi
2020-10-19 11:34 ` Jani Nikula
2020-10-19 22:53 ` Navare, Manasi
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 07/11] drm/i915: Make hardware readout work on i915 Manasi Navare
2020-10-19 16:36 ` Ville Syrjälä
2020-10-19 22:45 ` Navare, Manasi
2020-10-20 18:45 ` Ville Syrjälä
2020-10-20 18:57 ` Navare, Manasi
2020-10-20 21:57 ` Navare, Manasi
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 08/11] drm/i915: Link planes in a bigjoiner configuration, v3 Manasi Navare
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 09/11] drm/i915: Add bigjoiner aware plane clipping checks Manasi Navare
2020-10-19 16:20 ` Ville Syrjälä
2020-10-19 22:56 ` Navare, Manasi
2020-10-20 18:51 ` Ville Syrjälä
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 10/11] drm/i915: Ensure correct master/slave enable/disable sequence Manasi Navare
2020-10-19 16:26 ` Ville Syrjälä [this message]
2020-10-19 23:05 ` Navare, Manasi
2020-10-08 21:45 ` [Intel-gfx] [PATCH v10 11/11] drm/i915: Add debugfs dumping for bigjoiner, v3 Manasi Navare
2020-10-08 22:15 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v10,01/11] HAX to make DSC work on the icelake test system Patchwork
2020-10-08 22:17 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-10-08 22:33 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-10-09 1:32 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2020-10-13 19:19 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v10,01/11] HAX to make DSC work on the icelake test system (rev2) Patchwork
2020-10-13 19:21 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-10-13 19:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-10-14 14:32 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201019162619.GU6112@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=manasi.d.navare@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox