From: "Navare, Manasi" <manasi.d.navare@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v6 10/11] drm/i915: Add intel_update_bigjoiner handling.
Date: Mon, 24 Aug 2020 15:15:58 -0700 [thread overview]
Message-ID: <20200824221558.GA18640@labuser-Z97X-UD5H> (raw)
In-Reply-To: <20200715224222.7557-10-manasi.d.navare@intel.com>
On Wed, Jul 15, 2020 at 03:42:21PM -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,
> so use the vblank evasion to cover both pipes.
>
> This way ideally everything will be tear free, and updates are
> really atomic as userspace expects it.
>
> ****This needs to be checked if it still works since lot of refactoring
> in skl_commit_modeset_enables
>
> 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>
This looks good to me in terms of modeset enable sequence as per Bspec
but would be good to get an ack from Ville/Maarten just to double
check I didnt break anything while rebase
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 120 +++++++++++++++++--
> drivers/gpu/drm/i915/display/intel_sprite.c | 25 +++-
> drivers/gpu/drm/i915/display/intel_sprite.h | 3 +-
> 3 files changed, 129 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a1011414da6d..00b26863ffc6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -15656,7 +15656,7 @@ static void intel_update_crtc(struct intel_atomic_state *state,
> else
> i9xx_update_planes_on_crtc(state, crtc);
>
> - intel_pipe_update_end(new_crtc_state);
> + intel_pipe_update_end(new_crtc_state, NULL);
>
> /*
> * We usually enable FIFO underrun interrupts as part of the
> @@ -15754,6 +15754,52 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
> }
> }
>
> +static void intel_update_bigjoiner(struct intel_crtc *crtc,
> + struct intel_atomic_state *state,
> + struct intel_crtc_state *old_crtc_state,
> + struct intel_crtc_state *new_crtc_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + bool modeset = needs_modeset(new_crtc_state);
> + struct intel_crtc *slave = new_crtc_state->bigjoiner_linked_crtc;
> + struct intel_crtc_state *new_slave_crtc_state =
> + intel_atomic_get_new_crtc_state(state, slave);
> +
> + if (modeset) {
> + /* Enable slave first */
> + intel_crtc_update_active_timings(new_slave_crtc_state);
> + dev_priv->display.crtc_enable(state, slave);
> +
> + /* Then master */
> + intel_crtc_update_active_timings(new_crtc_state);
> + dev_priv->display.crtc_enable(state, crtc);
> +
> + /* vblanks work again, re-enable pipe CRC. */
> + intel_crtc_enable_pipe_crc(crtc);
> +
> + } else {
> + intel_pre_plane_update(state, crtc);
> + intel_pre_plane_update(state, slave);
> +
> + if (new_crtc_state->update_pipe)
> + intel_encoders_update_pipe(state, crtc);
> + }
> +
> + /*
> + * Perform vblank evasion around commit operation, and make sure to
> + * commit both planes simultaneously for best results.
> + */
> + intel_pipe_update_start(new_crtc_state);
> +
> + commit_pipe_config(state, crtc);
> + commit_pipe_config(state, slave);
> +
> + skl_update_planes_on_crtc(state, crtc);
> + skl_update_planes_on_crtc(state, slave);
> +
> + intel_pipe_update_end(new_crtc_state, new_slave_crtc_state);
> +}
> +
> static void intel_commit_modeset_enables(struct intel_atomic_state *state)
> {
> struct intel_crtc_state *new_crtc_state;
> @@ -15772,15 +15818,22 @@ static void intel_commit_modeset_enables(struct intel_atomic_state *state)
> static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> - struct intel_crtc *crtc;
> + struct intel_crtc *crtc, *slave;
> 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;
> + const struct intel_crtc_state *slave_crtc_state;
> int i;
>
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> enum pipe pipe = crtc->pipe;
>
> + if (new_crtc_state->bigjoiner_slave) {
> + /* We're updated from master */
> + continue;
> + }
> +
> if (!new_crtc_state->hw.active)
> continue;
>
> @@ -15791,6 +15844,34 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
> } else {
> modeset_pipes |= BIT(pipe);
> }
> +
> + if (new_crtc_state->bigjoiner) {
> + slave = new_crtc_state->bigjoiner_linked_crtc;
> + slave_crtc_state =
> + intel_atomic_get_new_crtc_state(state,
> + slave);
> +
> + /* put both entries in */
> + new_entries[i].start = new_crtc_state->wm.skl.ddb.start;
> + new_entries[i].end = slave_crtc_state->wm.skl.ddb.end;
> + } else {
> + 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;
> +
> + if (old_crtc_state->bigjoiner) {
> + slave = old_crtc_state->bigjoiner_linked_crtc;
> + slave_crtc_state =
> + intel_atomic_get_old_crtc_state(state, slave);
> +
> + entries[i].start = old_crtc_state->wm.skl.ddb.start;
> + entries[i].end = slave_crtc_state->wm.skl.ddb.end;
> + } else {
> + entries[i] = old_crtc_state->wm.skl.ddb;
> + }
> }
>
> /*
> @@ -15806,28 +15887,34 @@ 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))
> + if (new_crtc_state->bigjoiner) {
> + intel_update_bigjoiner(crtc, state,
> + old_crtc_state,
> + new_crtc_state);
> + } else {
> + intel_update_crtc(state, crtc);
> + }
> +
> + if (ddb_changed && (update_pipes | modeset_pipes))
> intel_wait_for_vblank(dev_priv, pipe);
> }
> }
> @@ -15863,9 +15950,18 @@ 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);
> + if (new_crtc_state->bigjoiner)
> + intel_update_bigjoiner(crtc, state,
> + old_crtc_state,
> + new_crtc_state);
> + else
> + intel_enable_crtc(state, crtc);
> }
>
> /*
> @@ -15877,10 +15973,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 60eeed06a780..eaae5df546fe 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -99,6 +99,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;
>
> @@ -191,7 +193,8 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
> * re-enables interrupts and verifies the update was actually completed
> * before a vblank.
> */
> -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> +void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state,
> + struct intel_crtc_state *slave_crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> enum pipe pipe = crtc->pipe;
> @@ -206,16 +209,26 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> * Would be slightly nice to just grab the vblank count and arm the
> * event outside of the critical section - the spinlock might spin for a
> * while ... */
> - if (new_crtc_state->uapi.event) {
> - drm_WARN_ON(&dev_priv->drm,
> - drm_crtc_vblank_get(&crtc->base) != 0);
> + if (new_crtc_state->uapi.event || (slave_crtc_state && slave_crtc_state->uapi.event)) {
> + if (new_crtc_state->uapi.event)
> + drm_WARN_ON(&dev_priv->drm,
> + drm_crtc_vblank_get(&crtc->base) != 0);
> + if (slave_crtc_state && slave_crtc_state->uapi.event)
> + drm_WARN_ON(&dev_priv->drm,
> + drm_crtc_vblank_get(&crtc->base) != 0);
>
> spin_lock(&crtc->base.dev->event_lock);
> - drm_crtc_arm_vblank_event(&crtc->base,
> - new_crtc_state->uapi.event);
> + if (new_crtc_state->uapi.event)
> + drm_crtc_arm_vblank_event(&crtc->base,
> + new_crtc_state->uapi.event);
> + if (slave_crtc_state && slave_crtc_state->uapi.event)
> + drm_crtc_arm_vblank_event(&crtc->base,
> + slave_crtc_state->uapi.event);
> spin_unlock(&crtc->base.dev->event_lock);
>
> new_crtc_state->uapi.event = NULL;
> + if (slave_crtc_state)
> + slave_crtc_state->uapi.event = NULL;
> }
>
> local_irq_enable();
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.h b/drivers/gpu/drm/i915/display/intel_sprite.h
> index cd2104ba1ca1..15e7c112ec77 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.h
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.h
> @@ -24,7 +24,8 @@ struct intel_plane *intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> int intel_sprite_set_colorkey_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state);
> -void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state);
> +void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state,
> + struct intel_crtc_state *slave_crtc_state);
> int intel_plane_check_stride(const struct intel_plane_state *plane_state);
> int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state);
> int chv_plane_check_rotation(const struct intel_plane_state *plane_state);
> --
> 2.19.1
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-08-24 22:14 UTC|newest]
Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-15 22:42 [Intel-gfx] [PATCH v6 01/11] HAX to make DSC work on the icelake test system Manasi Navare
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode Manasi Navare
2020-08-17 7:26 ` Manna, Animesh
2020-09-03 17:49 ` Ville Syrjälä
2020-09-03 18:04 ` Navare, Manasi
2020-09-03 18:40 ` Ville Syrjälä
2020-09-07 12:35 ` Ville Syrjälä
2020-09-14 18:32 ` Navare, Manasi
2020-09-14 18:52 ` Ville Syrjälä
2020-09-21 21:01 ` Navare, Manasi
2020-09-22 10:19 ` Ville Syrjälä
2020-09-22 18:52 ` Navare, Manasi
2020-09-23 14:54 ` Navare, Manasi
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 03/11] drm/i915: Add hw.pipe_mode to allow bigjoiner pipe/transcoder split Manasi Navare
2020-08-10 12:38 ` Maarten Lankhorst
2020-08-17 7:32 ` Manna, Animesh
2020-09-03 17:54 ` Ville Syrjälä
2020-09-14 18:45 ` Navare, Manasi
2020-09-14 18:48 ` Ville Syrjälä
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 04/11] drm/i915/dp: Allow big joiner modes in intel_dp_mode_valid(), v3 Manasi Navare
2020-08-10 12:40 ` Maarten Lankhorst
2020-08-21 9:41 ` Manna, Animesh
2020-08-21 21:51 ` Navare, Manasi
2020-09-07 11:20 ` Ville Syrjälä
2020-09-14 19:00 ` Navare, Manasi
2020-09-14 19:17 ` Ville Syrjälä
2020-09-14 19:38 ` Navare, Manasi
2020-09-14 19:47 ` Ville Syrjälä
2020-09-15 23:03 ` Navare, Manasi
2020-09-17 12:20 ` Ville Syrjälä
2020-09-23 5:46 ` Navare, Manasi
2020-09-23 9:57 ` Ville Syrjälä
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 05/11] drm/i915: Try to make bigjoiner work in atomic check Manasi Navare
2020-08-21 10:16 ` Manna, Animesh
2020-08-21 18:22 ` Navare, Manasi
2020-09-03 18:38 ` Ville Syrjälä
2020-09-23 22:58 ` Navare, Manasi
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 06/11] drm/i915: Enable big joiner support in enable and disable sequences Manasi Navare
2020-07-16 19:27 ` Manasi Navare
2020-08-10 12:45 ` Maarten Lankhorst
2020-08-10 23:04 ` Navare, Manasi
2020-07-16 21:12 ` [Intel-gfx] [PATCH v7 " Manasi Navare
2020-08-10 23:28 ` [Intel-gfx] [PATCH v8 " Manasi Navare
2020-08-27 23:35 ` Navare, Manasi
2020-08-28 6:26 ` Maarten Lankhorst
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 07/11] drm/i915: Make hardware readout work on i915 Manasi Navare
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 08/11] drm/i915: Link planes in a bigjoiner configuration, v3 Manasi Navare
2020-09-03 19:19 ` Ville Syrjälä
2020-09-14 19:14 ` Navare, Manasi
2020-09-14 19:20 ` Ville Syrjälä
2020-09-14 19:27 ` Navare, Manasi
2020-09-14 19:34 ` Ville Syrjälä
2020-09-14 19:45 ` Navare, Manasi
2020-09-14 20:05 ` Ville Syrjälä
2020-09-15 22:40 ` Navare, Manasi
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 09/11] drm/i915: Add bigjoiner aware plane clipping checks Manasi Navare
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 10/11] drm/i915: Add intel_update_bigjoiner handling Manasi Navare
2020-08-24 22:15 ` Navare, Manasi [this message]
2020-09-03 19:23 ` Ville Syrjälä
2020-09-14 19:21 ` Navare, Manasi
2020-09-21 21:18 ` Navare, Manasi
2020-09-22 10:27 ` Ville Syrjälä
2020-09-22 18:54 ` Navare, Manasi
2020-07-15 22:42 ` [Intel-gfx] [PATCH v6 11/11] drm/i915: Add debugfs dumping for bigjoiner, v3 Manasi Navare
2020-08-10 12:47 ` Maarten Lankhorst
2020-07-15 22:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,01/11] HAX to make DSC work on the icelake test system Patchwork
2020-07-15 22:51 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-07-15 23:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-07-16 5:48 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-07-16 21:53 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,01/11] HAX to make DSC work on the icelake test system (rev2) Patchwork
2020-07-16 21:54 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-07-16 22:14 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-07-17 1:04 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-08-10 23:47 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,01/11] HAX to make DSC work on the icelake test system (rev3) Patchwork
2020-08-10 23:48 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-08-11 0:09 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2020-08-11 18:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v6,01/11] HAX to make DSC work on the icelake test system (rev4) Patchwork
2020-08-11 18:35 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-08-11 18:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-08-11 20:15 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " 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=20200824221558.GA18640@labuser-Z97X-UD5H \
--to=manasi.d.navare@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/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;
as well as URLs for NNTP newsgroup(s).