public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: <sunpeng.li@amd.com>
To: <amd-gfx@lists.freedesktop.org>, <dri-devel@lists.freedesktop.org>
Cc: <Harry.Wentland@amd.com>, <simona@ffwll.ch>, <airlied@gmail.com>,
	<jani.nikula@linux.intel.com>, <ville.syrjala@linux.intel.com>,
	<superm1@kernel.org>, Leo Li <sunpeng.li@amd.com>
Subject: [PATCH 4/5] drm/amd/display: Implement deferred vblanks on IPS platforms
Date: Tue, 24 Feb 2026 16:26:38 -0500	[thread overview]
Message-ID: <20260224212639.390768-5-sunpeng.li@amd.com> (raw)
In-Reply-To: <20260224212639.390768-1-sunpeng.li@amd.com>

From: Leo Li <sunpeng.li@amd.com>

[Why]

Newer generations of Display Core Next (DCN) hardware allow for the
entire DCN block to be power-gated in a feature called Idle Power States
(IPS). Once DCN is in IPS, HW register access will timeout. Therefore,
allowing/disallowing IPS requires synchronization with the rest of the
driver through the big dc_lock mutex. It happens that writing vblank
interrupt control and reading counter/scanout position registers
requires IPS disallow. Since that requires the dc_lock, it turns into a
blocking operation.

The immediately obvious (not)solution is to disable IPS. But since it
provides sizable power savings, and it's not going away for future APUs,
it's not a good option.

The other (non)solution is to get rid of the dc_lock or make it not
block. But since all hardware (HW) and Display Micro-controller (DMCU)
access is invalid when DCN is in IPS (because everything is gated), it
needs to be synchronized with all other HW/DMCU access. It happens that
the dc_lock is responsible for that. I don't think replacing it with a
spinlock is a valid solution either. Consider link training: IPS cannot
be allowed while that is happening, yet the timescale of link training
can be in the 100ms. A spinlock would spin too long here.

So let's implement the newly introduced deferred vblank enable/disable
callbacks in DRM vblank core.

[How]

Implement the deferred CRTC callbacks to request the DRM vblank core to
defer vblank enable/disable. Only implement for ASICs that have IPS
support. Otherwise, keep the non-deferred behavior.

To ensure interrupts are enabled before programming flips (otherwise the
page flip interrupt for event delivery may not fire), call
drm_crtc_vblank_wait_deferred_enable() in the commit_planes code when
getting a reference on vblanks.

Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   8 +
 .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c    | 177 +++++++++++++++++-
 2 files changed, 184 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index a8e4e3ab5e402..0405466666e2f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10268,6 +10268,14 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
 				&acrtc_attach->dm_irq_params.vrr_params.adjust);
 			spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
 		}
+
+		/*
+		 * If vblank is being enabled in worker thread, wait for it to
+		 * enable interrupts before programming pflips, otherwise the
+		 * interrupt may not fire.
+		 */
+		drm_crtc_vblank_wait_deferred_enable(pcrtc);
+
 		mutex_lock(&dm->dc_lock);
 		update_planes_and_stream_adapter(dm->dc,
 					 acrtc_state->update_type,
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 3df38f3cb7423..fd84977f55ecd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -454,6 +454,140 @@ static void amdgpu_dm_crtc_disable_vblank(struct drm_crtc *crtc)
 						 acrtc_state, false);
 }
 
+/*
+ * Deferred vblank enable/disable works differently: the drm vblank core manages
+ * the workqueue instead of amdgpu_dm, sandwiching the vblank_enable/disable()
+ * with the crtc pre/post callbacks. Therefore, they need to be sequenced
+ * differently from their non-deferred variants.
+ */
+
+static int amdgpu_dm_crtc_deferred_enable_vblank(struct drm_crtc *crtc)
+{
+	struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+	struct amdgpu_display_manager *dm = &adev->dm;
+	int ret;
+
+	ret = amdgpu_dm_crtc_set_vblank(crtc, true);
+	if (!ret)
+		dm->active_vblank_irq_count++;
+
+	return ret;
+}
+
+static void amdgpu_dm_crtc_deferred_disable_vblank(struct drm_crtc *crtc)
+{
+	struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+	struct amdgpu_display_manager *dm = &adev->dm;
+
+	amdgpu_dm_crtc_set_vblank(crtc, false);
+
+	if (dm->active_vblank_irq_count)
+		dm->active_vblank_irq_count--;
+}
+
+
+static void amdgpu_dm_crtc_pre_enable_vblank(struct drm_crtc *crtc)
+{
+	struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+	struct amdgpu_display_manager *dm = &adev->dm;
+
+	mutex_lock(&dm->dc_lock);
+	dc_allow_idle_optimizations(dm->dc, false);
+}
+
+static void amdgpu_dm_crtc_post_enable_vblank(struct drm_crtc *crtc)
+{
+	struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+	struct amdgpu_display_manager *dm = &adev->dm;
+	struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+	struct dm_crtc_state *acrtc_state = to_dm_crtc_state(crtc->state);
+	struct vblank_control_work vblank_work = { 0 };
+
+	/* If crtc disabled, skip panel optimizations exit */
+	if (!crtc->enabled) {
+		mutex_unlock(&dm->dc_lock);
+		return;
+	}
+
+	vblank_work.dm = dm;
+	vblank_work.acrtc = acrtc;
+	vblank_work.enable = true;
+	if (acrtc_state->stream) {
+		dc_stream_retain(acrtc_state->stream);
+		vblank_work.stream = acrtc_state->stream;
+	}
+
+	/*
+	 * Control PSR based on vblank requirements from OS
+	 *
+	 * If panel supports PSR SU, there's no need to disable PSR when OS is
+	 * submitting fast atomic commits (we infer this by whether the OS
+	 * requests vblank events). Fast atomic commits will simply trigger a
+	 * full-frame-update (FFU); a specific case of selective-update (SU)
+	 * where the SU region is the full hactive*vactive region. See
+	 * fill_dc_dirty_rects().
+	 */
+	if (vblank_work.stream && vblank_work.stream->link && vblank_work.acrtc) {
+		amdgpu_dm_crtc_set_panel_sr_feature(
+			&vblank_work, true,
+			vblank_work.acrtc->dm_irq_params.allow_sr_entry);
+	}
+
+	if (vblank_work.stream)
+		dc_stream_release(vblank_work.stream);
+
+	mutex_unlock(&dm->dc_lock);
+}
+
+static void amdgpu_dm_crtc_pre_disable_vblank(struct drm_crtc *crtc)
+{
+	struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+	struct amdgpu_display_manager *dm = &adev->dm;
+
+	mutex_lock(&dm->dc_lock);
+}
+
+static void amdgpu_dm_crtc_post_disable_vblank(struct drm_crtc *crtc)
+{
+	struct amdgpu_device *adev = drm_to_adev(crtc->dev);
+	struct amdgpu_display_manager *dm = &adev->dm;
+	struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
+	struct dm_crtc_state *acrtc_state = to_dm_crtc_state(crtc->state);
+	struct vblank_control_work vblank_work = { 0 };
+	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
+
+	/* If a vblank_get got ahead of us (can happen when !disable_immediate),
+	 * skip panel optimizations */
+	if (atomic_read(&vblank->refcount) > 0) {
+		mutex_unlock(&dm->dc_lock);
+		return;
+	}
+
+	vblank_work.dm = dm;
+	vblank_work.acrtc = acrtc;
+	vblank_work.enable = false;
+	if (acrtc_state->stream) {
+		dc_stream_retain(acrtc_state->stream);
+		vblank_work.stream = acrtc_state->stream;
+	}
+
+	if (vblank_work.stream && vblank_work.stream->link && vblank_work.acrtc) {
+		amdgpu_dm_crtc_set_panel_sr_feature(
+			&vblank_work, false,
+			vblank_work.acrtc->dm_irq_params.allow_sr_entry);
+	}
+
+	if (vblank_work.stream)
+		dc_stream_release(vblank_work.stream);
+
+	if (dm->active_vblank_irq_count == 0) {
+		dc_post_update_surfaces_to_stream(dm->dc);
+		dc_allow_idle_optimizations(dm->dc, true);
+	}
+
+	mutex_unlock(&dm->dc_lock);
+}
+
 static void amdgpu_dm_crtc_destroy_state(struct drm_crtc *crtc,
 				  struct drm_crtc_state *state)
 {
@@ -623,6 +757,33 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
 #endif
 };
 
+static const struct drm_crtc_funcs amdgpu_dm_crtc_deferred_vblank_funcs = {
+	.reset = amdgpu_dm_crtc_reset_state,
+	.destroy = amdgpu_dm_crtc_destroy,
+	.set_config = drm_atomic_helper_set_config,
+	.page_flip = drm_atomic_helper_page_flip,
+	.atomic_duplicate_state = amdgpu_dm_crtc_duplicate_state,
+	.atomic_destroy_state = amdgpu_dm_crtc_destroy_state,
+	.set_crc_source = amdgpu_dm_crtc_set_crc_source,
+	.verify_crc_source = amdgpu_dm_crtc_verify_crc_source,
+	.get_crc_sources = amdgpu_dm_crtc_get_crc_sources,
+	.get_vblank_counter = amdgpu_get_vblank_counter_kms,
+	.enable_vblank = amdgpu_dm_crtc_deferred_enable_vblank,
+	.disable_vblank = amdgpu_dm_crtc_deferred_disable_vblank,
+	.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
+#if defined(CONFIG_DEBUG_FS)
+	.late_register = amdgpu_dm_crtc_late_register,
+#endif
+#ifdef AMD_PRIVATE_COLOR
+	.atomic_set_property = amdgpu_dm_atomic_crtc_set_property,
+	.atomic_get_property = amdgpu_dm_atomic_crtc_get_property,
+#endif
+	.pre_enable_vblank = amdgpu_dm_crtc_pre_enable_vblank,
+	.post_enable_vblank = amdgpu_dm_crtc_post_enable_vblank,
+	.pre_disable_vblank = amdgpu_dm_crtc_pre_disable_vblank,
+	.post_disable_vblank = amdgpu_dm_crtc_post_disable_vblank,
+};
+
 static void amdgpu_dm_crtc_helper_disable(struct drm_crtc *crtc)
 {
 }
@@ -755,6 +916,7 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
 			       struct drm_plane *plane,
 			       uint32_t crtc_index)
 {
+	const struct drm_crtc_funcs *crtc_funcs;
 	struct amdgpu_crtc *acrtc = NULL;
 	struct drm_plane *cursor_plane;
 	bool has_degamma;
@@ -771,12 +933,25 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
 	if (!acrtc)
 		goto fail;
 
+	/*
+	 * Only enable deferred vblank enable/disable for ASICs with IPS
+	 * support
+	 */
+	if (dm->dc->caps.ips_support) {
+		crtc_funcs = &amdgpu_dm_crtc_deferred_vblank_funcs;
+		drm_dbg_driver(dm->ddev,
+			       "Initializing CRTC %d with deferred vBlank enable/disable\n",
+			       crtc_index);
+	} else {
+		crtc_funcs = &amdgpu_dm_crtc_funcs;
+	}
+
 	res = drm_crtc_init_with_planes(
 			dm->ddev,
 			&acrtc->base,
 			plane,
 			cursor_plane,
-			&amdgpu_dm_crtc_funcs, NULL);
+			crtc_funcs, NULL);
 
 	if (res)
 		goto fail;
-- 
2.52.0


  parent reply	other threads:[~2026-02-24 21:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-24 21:26 [PATCH 0/5] drm/vblank: Deferred Enable and Disable sunpeng.li
2026-02-24 21:26 ` [PATCH 1/5] drm/vblank: Add drm_crtc_vblank_is_off() helper sunpeng.li
2026-02-27  4:40   ` Claude review: " Claude Code Review Bot
2026-02-24 21:26 ` [PATCH 2/5] drm/vblank: Introduce deferred vblank enable/disable sunpeng.li
2026-02-25  4:42   ` kernel test robot
2026-02-27  4:40   ` Claude review: " Claude Code Review Bot
2026-02-24 21:26 ` [PATCH 3/5] drm/amd/display: Refactor amdgpu_dm_crtc_set_vblank sunpeng.li
2026-02-27  4:40   ` Claude review: " Claude Code Review Bot
2026-02-24 21:26 ` sunpeng.li [this message]
2026-02-27  4:40   ` Claude review: drm/amd/display: Implement deferred vblanks on IPS platforms Claude Code Review Bot
2026-02-24 21:26 ` [PATCH 5/5] drm/vblank: Add some debugging trace events sunpeng.li
2026-02-27  4:40   ` Claude review: " Claude Code Review Bot
2026-02-27  4:40 ` Claude review: drm/vblank: Deferred Enable and Disable Claude Code Review Bot

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=20260224212639.390768-5-sunpeng.li@amd.com \
    --to=sunpeng.li@amd.com \
    --cc=Harry.Wentland@amd.com \
    --cc=airlied@gmail.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=simona@ffwll.ch \
    --cc=superm1@kernel.org \
    --cc=ville.syrjala@linux.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