Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Gustavo Sousa <gustavo.sousa@intel.com>
To: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Cc: Luca Coelho <luciano.coelho@intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: [PATCH 10/13] drm/i915/dmc_wl: Couple enable/disable with dynamic DC states
Date: Mon, 21 Oct 2024 19:27:29 -0300	[thread overview]
Message-ID: <20241021222744.294371-11-gustavo.sousa@intel.com> (raw)
In-Reply-To: <20241021222744.294371-1-gustavo.sousa@intel.com>

Enabling and disabling the DMC wakelock should be done as part of
enabling and disabling of dynamic DC states, respectively. We should not
enable or disable DMC wakelock independently of DC states, otherwise we
would risk ending up with an inconsistent state where dynamic DC states
are enabled and the DMC wakelock is disabled, going against current
recommendations and making MMIO transactions potentially slower. In
future display IPs that could have a worse outcome if DMC trap
implementation is completely removed.

So, let's make things safer by tying stuff together, removing the
independent calls, and also put warnings in place to detect inconsistent
calls.

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power_well.c | 5 ++++-
 drivers/gpu/drm/i915/display/intel_dmc.c                | 4 ----
 drivers/gpu/drm/i915/display/intel_dmc_wl.c             | 6 ++++--
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
index e8946ce86aaa..1a6c93170a5a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -981,6 +981,7 @@ void gen9_disable_dc_states(struct intel_display *display)
 	struct drm_i915_private *dev_priv = to_i915(display->drm);
 	struct i915_power_domains *power_domains = &display->power.domains;
 	struct intel_cdclk_config cdclk_config = {};
+	u32 old_state = power_domains->dc_state;
 
 	if (power_domains->target_dc_state == DC_STATE_EN_DC3CO) {
 		tgl_disable_dc3co(display);
@@ -996,7 +997,9 @@ void gen9_disable_dc_states(struct intel_display *display)
 		return;
 	}
 
-	intel_dmc_wl_disable(display);
+	if (old_state == DC_STATE_EN_UPTO_DC5 ||
+	    old_state == DC_STATE_EN_UPTO_DC6)
+		intel_dmc_wl_disable(display);
 
 	intel_cdclk_get_cdclk(display, &cdclk_config);
 	/* Can't read out voltage_level so can't use intel_cdclk_changed() */
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 48bbbf8f312c..f0b12c609884 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -632,8 +632,6 @@ void intel_dmc_disable_program(struct intel_display *display)
 	pipedmc_clock_gating_wa(display, true);
 	disable_all_event_handlers(display);
 	pipedmc_clock_gating_wa(display, false);
-
-	intel_dmc_wl_disable(display);
 }
 
 void assert_dmc_loaded(struct intel_display *display)
@@ -1140,8 +1138,6 @@ void intel_dmc_suspend(struct intel_display *display)
 	if (dmc)
 		flush_work(&dmc->work);
 
-	intel_dmc_wl_disable(display);
-
 	/* Drop the reference held in case DMC isn't loaded. */
 	if (!intel_dmc_has_payload(display))
 		intel_dmc_runtime_pm_put(display);
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index e27c06b7c42f..8283b607aac4 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -271,6 +271,7 @@ void intel_dmc_wl_init(struct intel_display *display)
 	refcount_set(&wl->refcount, 0);
 }
 
+/* Must only be called as part of enabling dynamic DC states. */
 void intel_dmc_wl_enable(struct intel_display *display)
 {
 	struct intel_dmc_wl *wl = &display->wl;
@@ -281,7 +282,7 @@ void intel_dmc_wl_enable(struct intel_display *display)
 
 	spin_lock_irqsave(&wl->lock, flags);
 
-	if (wl->enabled)
+	if (drm_WARN_ON(display->drm, wl->enabled))
 		goto out_unlock;
 
 	/*
@@ -314,6 +315,7 @@ void intel_dmc_wl_enable(struct intel_display *display)
 	spin_unlock_irqrestore(&wl->lock, flags);
 }
 
+/* Must only be called as part of disabling dynamic DC states. */
 void intel_dmc_wl_disable(struct intel_display *display)
 {
 	struct intel_dmc_wl *wl = &display->wl;
@@ -326,7 +328,7 @@ void intel_dmc_wl_disable(struct intel_display *display)
 
 	spin_lock_irqsave(&wl->lock, flags);
 
-	if (!wl->enabled)
+	if (drm_WARN_ON(display->drm, !wl->enabled))
 		goto out_unlock;
 
 	/* Disable wakelock in DMC */
-- 
2.47.0


  parent reply	other threads:[~2024-10-21 22:28 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-21 22:27 [PATCH 00/13] drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD Gustavo Sousa
2024-10-21 22:27 ` [PATCH 01/13] drm/xe: Mimic i915 behavior for non-sleeping MMIO wait Gustavo Sousa
2024-11-01 10:57   ` Luca Coelho
2024-11-05 12:17     ` Gustavo Sousa
2024-10-21 22:27 ` [PATCH 02/13] drm/i915/dmc_wl: Use non-sleeping variant of " Gustavo Sousa
2024-10-22  9:34   ` Jani Nikula
2024-10-22 10:55     ` Gustavo Sousa
2024-11-01 11:04       ` Luca Coelho
2024-11-01 11:18   ` Luca Coelho
2024-10-21 22:27 ` [PATCH 03/13] drm/i915/dmc_wl: Check for non-zero refcount in release work Gustavo Sousa
2024-11-01 11:48   ` Luca Coelho
2024-10-21 22:27 ` [PATCH 04/13] drm/i915/dmc_wl: Get wakelock when disabling dynamic DC states Gustavo Sousa
2024-11-01 12:24   ` Luca Coelho
2024-11-05 12:44     ` Gustavo Sousa
2024-11-06 11:37       ` Luca Coelho
2024-10-21 22:27 ` [PATCH 05/13] drm/i915/dmc_wl: Use sentinel item for range tables Gustavo Sousa
2024-11-01 12:25   ` Luca Coelho
2024-10-21 22:27 ` [PATCH 06/13] drm/i915/dmc_wl: Extract intel_dmc_wl_addr_in_range() Gustavo Sousa
2024-10-21 22:27 ` [PATCH 07/13] drm/i915/dmc_wl: Check ranges specific to DC states Gustavo Sousa
2024-10-22  8:03   ` Jani Nikula
2024-10-22 11:06     ` Gustavo Sousa
2024-11-05 19:54     ` Gustavo Sousa
2024-10-22  8:03   ` Jani Nikula
2024-10-22 11:10     ` Gustavo Sousa
2024-10-22 11:14   ` Gustavo Sousa
2024-11-01 12:51   ` Luca Coelho
2024-11-05 13:00     ` Gustavo Sousa
2024-11-06 11:47       ` Luca Coelho
2024-11-06 13:56         ` Gustavo Sousa
2024-10-21 22:27 ` [PATCH 08/13] drm/i915/dmc_wl: Allow simpler syntax for single reg in range tables Gustavo Sousa
2024-11-01 12:58   ` Luca Coelho
2024-11-05 13:42     ` Gustavo Sousa
2024-11-06 12:23       ` Luca Coelho
2024-11-06 12:29         ` Gustavo Sousa
2024-11-06 12:35           ` Luca Coelho
2024-10-21 22:27 ` [PATCH 09/13] drm/i915/dmc_wl: Deal with existing references when disabling Gustavo Sousa
2024-11-01 14:17   ` Luca Coelho
2024-10-21 22:27 ` Gustavo Sousa [this message]
2024-11-01 14:19   ` [PATCH 10/13] drm/i915/dmc_wl: Couple enable/disable with dynamic DC states Luca Coelho
2024-10-21 22:27 ` [PATCH 11/13] drm/i915/dmc_wl: Add and use HAS_DMC_WAKELOCK() Gustavo Sousa
2024-10-22  9:37   ` Jani Nikula
2024-10-22 11:03     ` Gustavo Sousa
2024-11-05 13:56       ` Gustavo Sousa
2024-11-06  9:25         ` Jani Nikula
2024-11-06 13:24           ` Gustavo Sousa
2024-10-21 22:27 ` [PATCH 12/13] drm/i915/dmc_wl: Sanitize enable_dmc_wl according to hardware support Gustavo Sousa
2024-11-01 14:25   ` Luca Coelho
2024-10-21 22:27 ` [PATCH 13/13] drm/i915/xe3lpd: Use DMC wakelock by default Gustavo Sousa
2024-11-01 14:27   ` Luca Coelho
2024-11-05 13:46     ` Gustavo Sousa
2024-11-05 21:12       ` Gustavo Sousa
2024-11-06 12:27         ` Luca Coelho
2024-10-21 22:54 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD Patchwork
2024-10-21 22:54 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-10-21 23:44 ` ✗ Fi.CI.BAT: 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=20241021222744.294371-11-gustavo.sousa@intel.com \
    --to=gustavo.sousa@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=luciano.coelho@intel.com \
    --cc=rodrigo.vivi@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