From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 13/13] drm/i915/dsb: Use DSB for plane/color management updates
Date: Mon, 2 Sep 2024 16:53:42 +0300 [thread overview]
Message-ID: <20240902135342.1050-14-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20240902135342.1050-1-ville.syrjala@linux.intel.com>
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Push regular plane/color management updates to the DSB,
if other constraints allow it.
The first part of the sequence will go as follows:
- CPU will kick off DSB0 immediately
- DSB0 writes double bufferd non-arming registers
- DSB0 evades the vblank
- DSB0 writes double buffered arming registers
If no color management updates is needed we follow that up with:
- DSB0 waits for the undelayed vblank
- DSB0 waits for the delayed vblank (usec wait)
- DSB0 emits an interrupt which will cause the CPU to complete the commit
If color management update is needed:
- DSB0 will start DSB1 with wait for undelayed vblank
- DSB0 will in parallel perform the force DEwake tricks
- DSB1 writes single buffered LUT registers
- DSB1 waits for the delayed vblank (usec wait)
- DSB1 emits an interrupt which will cause the CPU to complete the commit
With this sequence we don't need to increase the vblank delay
to make room for register programming during vblank, which is
a good thing for high refresh rate display. But I'll need to
still think of some way to eliminate VRR commit completion
related races under this scheme.
Stuff that isn't ready for DSB yet:
- modesets (potentially we could do
at least the plane enabling via DSB)
- fastsets
- VRR
- PSR
- scalers
- async flips
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/display/intel_atomic.c | 5 +-
drivers/gpu/drm/i915/display/intel_color.c | 25 +---
drivers/gpu/drm/i915/display/intel_crtc.c | 5 +-
drivers/gpu/drm/i915/display/intel_display.c | 128 ++++++++++++++++--
.../drm/i915/display/intel_display_types.h | 5 +-
5 files changed, 128 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index 12d6ed940751..52a055032c68 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -277,7 +277,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->fb_bits = 0;
crtc_state->update_planes = 0;
crtc_state->dsb_color_vblank = NULL;
- crtc_state->dsb_color_commit = NULL;
+ crtc_state->dsb_commit = NULL;
+ crtc_state->use_dsb = false;
return &crtc_state->uapi;
}
@@ -312,7 +313,7 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
drm_WARN_ON(crtc->dev, crtc_state->dsb_color_vblank);
- drm_WARN_ON(crtc->dev, crtc_state->dsb_color_commit);
+ drm_WARN_ON(crtc->dev, crtc_state->dsb_commit);
__drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi);
intel_crtc_free_hw_state(crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 685da9f84b0a..ac4d4f5349a4 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1913,9 +1913,6 @@ void intel_color_commit_arm(struct intel_dsb *dsb,
struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
i915->display.funcs.color->color_commit_arm(dsb, crtc_state);
-
- if (crtc_state->dsb_color_commit)
- intel_dsb_commit(crtc_state->dsb_color_commit, false);
}
void intel_color_post_update(const struct intel_crtc_state *crtc_state)
@@ -1949,28 +1946,14 @@ void intel_color_prepare_commit(struct intel_atomic_state *state,
i915->display.funcs.color->load_luts(crtc_state);
+ intel_dsb_wait_vblank_delay(state, crtc_state->dsb_color_vblank);
+ intel_dsb_interrupt(crtc_state->dsb_color_vblank);
+
intel_dsb_finish(crtc_state->dsb_color_vblank);
-
- crtc_state->dsb_color_commit = intel_dsb_prepare(state, crtc, INTEL_DSB_0, 16);
- if (!crtc_state->dsb_color_commit) {
- intel_dsb_cleanup(crtc_state->dsb_color_vblank);
- crtc_state->dsb_color_vblank = NULL;
- return;
- }
-
- intel_dsb_chain(state, crtc_state->dsb_color_commit,
- crtc_state->dsb_color_vblank, true);
-
- intel_dsb_finish(crtc_state->dsb_color_commit);
}
void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state)
{
- if (crtc_state->dsb_color_commit) {
- intel_dsb_cleanup(crtc_state->dsb_color_commit);
- crtc_state->dsb_color_commit = NULL;
- }
-
if (crtc_state->dsb_color_vblank) {
intel_dsb_cleanup(crtc_state->dsb_color_vblank);
crtc_state->dsb_color_vblank = NULL;
@@ -1979,8 +1962,6 @@ void intel_color_cleanup_commit(struct intel_crtc_state *crtc_state)
void intel_color_wait_commit(const struct intel_crtc_state *crtc_state)
{
- if (crtc_state->dsb_color_commit)
- intel_dsb_wait(crtc_state->dsb_color_commit);
if (crtc_state->dsb_color_vblank)
intel_dsb_wait(crtc_state->dsb_color_vblank);
}
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 30616101b4f2..f643430c1881 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -390,10 +390,11 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
static bool intel_crtc_needs_vblank_work(const struct intel_crtc_state *crtc_state)
{
return crtc_state->hw.active &&
- !intel_crtc_needs_modeset(crtc_state) &&
!crtc_state->preload_luts &&
+ !intel_crtc_needs_modeset(crtc_state) &&
intel_crtc_needs_color_update(crtc_state) &&
- !intel_color_uses_dsb(crtc_state);
+ !intel_color_uses_dsb(crtc_state) &&
+ !crtc_state->use_dsb;
}
static void intel_crtc_vblank_work(struct kthread_work *base)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7d157fc64a3b..a1724e3a9794 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6821,7 +6821,7 @@ static void commit_pipe_pre_planes(struct intel_atomic_state *state,
* During modesets pipe configuration was programmed as the
* CRTC was enabled.
*/
- if (!modeset) {
+ if (!modeset && !new_crtc_state->use_dsb) {
if (intel_crtc_needs_color_update(new_crtc_state))
intel_color_commit_arm(NULL, new_crtc_state);
@@ -6923,10 +6923,12 @@ static void intel_pre_update_crtc(struct intel_atomic_state *state,
drm_WARN_ON(&i915->drm, !intel_display_power_is_enabled(i915, POWER_DOMAIN_DC_OFF));
if (!modeset &&
- intel_crtc_needs_color_update(new_crtc_state))
+ intel_crtc_needs_color_update(new_crtc_state) &&
+ !new_crtc_state->use_dsb)
intel_color_commit_noarm(NULL, new_crtc_state);
- intel_crtc_planes_update_noarm(NULL, state, crtc);
+ if (!new_crtc_state->use_dsb)
+ intel_crtc_planes_update_noarm(NULL, state, crtc);
}
static void intel_update_crtc(struct intel_atomic_state *state,
@@ -6937,16 +6939,25 @@ static void intel_update_crtc(struct intel_atomic_state *state,
struct intel_crtc_state *new_crtc_state =
intel_atomic_get_new_crtc_state(state, crtc);
- /* Perform vblank evasion around commit operation */
- intel_pipe_update_start(state, crtc);
+ if (new_crtc_state->use_dsb) {
+ intel_crtc_prepare_vblank_event(new_crtc_state, &crtc->dsb_event);
- commit_pipe_pre_planes(state, crtc);
+ intel_dsb_commit(new_crtc_state->dsb_commit, false);
+ } else {
+ /* Perform vblank evasion around commit operation */
+ intel_pipe_update_start(state, crtc);
- intel_crtc_planes_update_arm(NULL, state, crtc);
+ if (new_crtc_state->dsb_commit)
+ intel_dsb_commit(new_crtc_state->dsb_commit, false);
- commit_pipe_post_planes(state, crtc);
+ commit_pipe_pre_planes(state, crtc);
- intel_pipe_update_end(state, crtc);
+ intel_crtc_planes_update_arm(NULL, state, crtc);
+
+ commit_pipe_post_planes(state, crtc);
+
+ intel_pipe_update_end(state, crtc);
+ }
/*
* VRR/Seamless M/N update may need to update frame timings.
@@ -7271,6 +7282,24 @@ static void intel_atomic_commit_fence_wait(struct intel_atomic_state *intel_stat
}
}
+static void intel_atomic_dsb_wait_commit(struct intel_crtc_state *crtc_state)
+{
+ if (crtc_state->dsb_commit)
+ intel_dsb_wait(crtc_state->dsb_commit);
+
+ intel_color_wait_commit(crtc_state);
+}
+
+static void intel_atomic_dsb_cleanup(struct intel_crtc_state *crtc_state)
+{
+ if (crtc_state->dsb_commit) {
+ intel_dsb_cleanup(crtc_state->dsb_commit);
+ crtc_state->dsb_commit = NULL;
+ }
+
+ intel_color_cleanup_commit(crtc_state);
+}
+
static void intel_atomic_cleanup_work(struct work_struct *work)
{
struct intel_atomic_state *state =
@@ -7281,7 +7310,7 @@ static void intel_atomic_cleanup_work(struct work_struct *work)
int i;
for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i)
- intel_color_cleanup_commit(old_crtc_state);
+ intel_atomic_dsb_cleanup(old_crtc_state);
drm_atomic_helper_cleanup_planes(&i915->drm, &state->base);
drm_atomic_helper_commit_cleanup_done(&state->base);
@@ -7337,6 +7366,78 @@ static void intel_atomic_dsb_prepare(struct intel_atomic_state *state,
intel_color_prepare_commit(state, crtc);
}
+static void intel_atomic_dsb_finish(struct intel_atomic_state *state,
+ struct intel_crtc *crtc)
+{
+ const struct intel_crtc_state *old_crtc_state =
+ intel_atomic_get_old_crtc_state(state, crtc);
+ struct intel_crtc_state *new_crtc_state =
+ intel_atomic_get_new_crtc_state(state, crtc);
+
+ if (!new_crtc_state->hw.active)
+ return;
+
+ if (state->base.legacy_cursor_update)
+ return;
+
+ /* FIXME deal with everything */
+ new_crtc_state->use_dsb =
+ new_crtc_state->update_planes &&
+ !new_crtc_state->vrr.enable &&
+ !new_crtc_state->do_async_flip &&
+ !new_crtc_state->has_psr &&
+ !new_crtc_state->scaler_state.scaler_users &&
+ !old_crtc_state->scaler_state.scaler_users &&
+ !intel_crtc_needs_modeset(new_crtc_state) &&
+ !intel_crtc_needs_fastset(new_crtc_state);
+
+ if (!new_crtc_state->use_dsb && !new_crtc_state->dsb_color_vblank)
+ return;
+
+ /*
+ * Rough estimate:
+ * ~64 registers per each plane * 8 planes = 512
+ * Double that for pipe stuff and other overhead.
+ */
+ new_crtc_state->dsb_commit = intel_dsb_prepare(state, crtc, INTEL_DSB_0,
+ new_crtc_state->use_dsb ? 1024 : 16);
+ if (!new_crtc_state->dsb_commit) {
+ new_crtc_state->use_dsb = false;
+ intel_color_cleanup_commit(new_crtc_state);
+ return;
+ }
+
+ if (new_crtc_state->use_dsb) {
+ if (intel_crtc_needs_color_update(new_crtc_state))
+ intel_color_commit_noarm(new_crtc_state->dsb_commit,
+ new_crtc_state);
+ intel_crtc_planes_update_noarm(new_crtc_state->dsb_commit,
+ state, crtc);
+
+ intel_dsb_vblank_evade(state, new_crtc_state->dsb_commit);
+
+ if (intel_crtc_needs_color_update(new_crtc_state))
+ intel_color_commit_arm(new_crtc_state->dsb_commit,
+ new_crtc_state);
+ bdw_set_pipe_misc(new_crtc_state->dsb_commit,
+ new_crtc_state);
+ intel_crtc_planes_update_arm(new_crtc_state->dsb_commit,
+ state, crtc);
+
+ if (!new_crtc_state->dsb_color_vblank) {
+ intel_dsb_wait_vblanks(new_crtc_state->dsb_commit, 1);
+ intel_dsb_wait_vblank_delay(state, new_crtc_state->dsb_commit);
+ intel_dsb_interrupt(new_crtc_state->dsb_commit);
+ }
+ }
+
+ if (new_crtc_state->dsb_color_vblank)
+ intel_dsb_chain(state, new_crtc_state->dsb_commit,
+ new_crtc_state->dsb_color_vblank, true);
+
+ intel_dsb_finish(new_crtc_state->dsb_commit);
+}
+
static void intel_atomic_commit_tail(struct intel_atomic_state *state)
{
struct drm_device *dev = state->base.dev;
@@ -7356,6 +7457,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
intel_atomic_prepare_plane_clear_colors(state);
+ for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
+ intel_atomic_dsb_finish(state, crtc);
+
drm_atomic_helper_wait_for_dependencies(&state->base);
drm_dp_mst_atomic_wait_for_dependencies(&state->base);
intel_atomic_global_state_wait_for_dependencies(state);
@@ -7469,7 +7573,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
if (new_crtc_state->do_async_flip)
intel_crtc_disable_flip_done(state, crtc);
- intel_color_wait_commit(new_crtc_state);
+ intel_atomic_dsb_wait_commit(new_crtc_state);
}
/*
@@ -7521,7 +7625,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
* FIXME get rid of this funny new->old swapping
*/
old_crtc_state->dsb_color_vblank = fetch_and_zero(&new_crtc_state->dsb_color_vblank);
- old_crtc_state->dsb_color_commit = fetch_and_zero(&new_crtc_state->dsb_color_commit);
+ old_crtc_state->dsb_commit = fetch_and_zero(&new_crtc_state->dsb_commit);
}
/* Underruns don't always raise interrupts, so check manually */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 36f52f2d0104..7719f3fc2721 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1400,8 +1400,9 @@ struct intel_crtc_state {
/* Only valid on TGL+ */
enum transcoder mst_master_transcoder;
- /* For DSB based color LUT updates */
- struct intel_dsb *dsb_color_vblank, *dsb_color_commit;
+ /* For DSB based pipe updates */
+ struct intel_dsb *dsb_color_vblank, *dsb_commit;
+ bool use_dsb;
u32 psr2_man_track_ctl;
--
2.44.2
next prev parent reply other threads:[~2024-09-02 13:55 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-02 13:53 [PATCH 00/13] drm/i915: Use DSB for plane/color management commits Ville Syrjala
2024-09-02 13:53 ` [PATCH 01/13] drm/i915/dsb: Avoid reads of the DSB buffer for indexed register writes Ville Syrjala
2024-09-02 13:53 ` [PATCH 02/13] drm/i915: Prepare clear color before wait_for_dependencies() Ville Syrjala
2024-09-02 13:53 ` [PATCH 03/13] drm/i915/dsb: Generate the DSB buffer in commit_tail() Ville Syrjala
2024-09-02 13:53 ` [PATCH 04/13] drm/i915/dsb: Enable programmable DSB interrupt Ville Syrjala
2024-09-02 13:53 ` [PATCH 05/13] drm/i915/dsb: Introduce intel_dsb_vblank_evade() Ville Syrjala
2024-09-02 13:53 ` [PATCH 06/13] drm/i915/dsb: Introduce intel_dsb_wait_usec() Ville Syrjala
2024-09-02 13:53 ` [PATCH 07/13] drm/i915/dsb: Introduce intel_dsb_wait_vblanks() Ville Syrjala
2024-09-02 13:53 ` [PATCH 08/13] drm/i915: Introduce intel_scanlines_to_usecs() Ville Syrjala
2024-09-02 13:53 ` [PATCH 09/13] drm/i915/dsb: Introduce intel_dsb_wait_vblank_delay() Ville Syrjala
2024-09-02 13:53 ` [PATCH 10/13] drm/i915: Extract intel_crtc_prepare_vblank_event() Ville Syrjala
2024-09-02 13:53 ` [PATCH 11/13] drm/i915: Plumb 'dsb' all way to the plane hooks Ville Syrjala
2024-09-02 13:53 ` [PATCH 12/13] drm/i915: Plumb 'dsb' all way to the color commit hooks Ville Syrjala
2024-09-02 13:53 ` Ville Syrjala [this message]
2024-09-02 14:45 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Use DSB for plane/color management commits Patchwork
2024-09-02 14:45 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-09-02 15:05 ` ✓ Fi.CI.BAT: success " Patchwork
2024-09-03 7:56 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-09-17 7:00 ` [PATCH 00/13] " Manna, Animesh
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=20240902135342.1050-14-ville.syrjala@linux.intel.com \
--to=ville.syrjala@linux.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