[PATCH v2 00/17] drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD

[PATCH v2 00/17] drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD

[Gustavo Sousa]

Using the DMC wakelock is the official recommendation for Xe3_LPD. This
series apply fixes to the current DMC wakelock implementation and
enables it by default for Xe3_LPD. The series has been tested with a PTL
machine.

Gustavo Sousa (17):
  drm/i915/dmc_wl: Use i915_mmio_reg_offset() instead of reg.reg
  drm/xe: Mimic i915 behavior for non-sleeping MMIO wait
  drm/i915/dmc_wl: Use non-sleeping variant of MMIO wait
  drm/i915/dmc_wl: Check for non-zero refcount in release work
  drm/i915/dmc_wl: Get wakelock when disabling dynamic DC states
  drm/i915/dmc_wl: Use sentinel item for range tables
  drm/i915/dmc_wl: Extract intel_dmc_wl_reg_in_range()
  drm/i915/dmc_wl: Rename lnl_wl_range to powered_off_ranges
  drm/i915/dmc_wl: Track registers touched by the DMC
  drm/i915/dmc_wl: Allow simpler syntax for single reg in range tables
  drm/i915/dmc_wl: Deal with existing references when disabling
  drm/i915/dmc_wl: Couple enable/disable with dynamic DC states
  drm/i915/dmc_wl: Add and use HAS_DMC_WAKELOCK()
  drm/i915/dmc_wl: Init only after we have runtime device info
  drm/i915/dmc_wl: Use HAS_DMC() in HAS_DMC_WAKELOCK()
  drm/i915/dmc_wl: Sanitize enable_dmc_wl according to hardware support
  drm/i915/xe3lpd: Use DMC wakelock by default

 drivers/gpu/drm/i915/display/intel_de.h       |  10 +
 .../drm/i915/display/intel_display_device.h   |   1 +
 .../drm/i915/display/intel_display_driver.c   |   2 +-
 .../drm/i915/display/intel_display_params.c   |   6 +-
 .../drm/i915/display/intel_display_params.h   |   2 +-
 .../i915/display/intel_display_power_well.c   |  19 +-
 drivers/gpu/drm/i915/display/intel_dmc.c      |   4 -
 drivers/gpu/drm/i915/display/intel_dmc_wl.c   | 304 ++++++++++++++----
 drivers/gpu/drm/i915/display/intel_dmc_wl.h   |  13 +-
 .../drm/xe/compat-i915-headers/intel_uncore.h |  11 +-
 10 files changed, 296 insertions(+), 76 deletions(-)

-- 
2.47.0

[PATCH v2 01/17] drm/i915/dmc_wl: Use i915_mmio_reg_offset() instead of reg.reg

[Gustavo Sousa]

The macro i915_mmio_reg_offset() is the proper interface to get a
register's offset. Use that instead of looking directly at reg.reg.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 5634ff07269d..05892a237d3a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -91,14 +91,15 @@ static void intel_dmc_wl_work(struct work_struct *work)
 	spin_unlock_irqrestore(&wl->lock, flags);
 }
 
-static bool intel_dmc_wl_check_range(u32 address)
+static bool intel_dmc_wl_check_range(i915_reg_t reg)
 {
 	int i;
 	bool wl_needed = false;
+	u32 offset = i915_mmio_reg_offset(reg);
 
 	for (i = 0; i < ARRAY_SIZE(lnl_wl_range); i++) {
-		if (address >= lnl_wl_range[i].start &&
-		    address <= lnl_wl_range[i].end) {
+		if (offset >= lnl_wl_range[i].start &&
+		    offset <= lnl_wl_range[i].end) {
 			wl_needed = true;
 			break;
 		}
@@ -191,7 +192,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
 	if (!__intel_dmc_wl_supported(display))
 		return;
 
-	if (!intel_dmc_wl_check_range(reg.reg))
+	if (!intel_dmc_wl_check_range(reg))
 		return;
 
 	spin_lock_irqsave(&wl->lock, flags);
@@ -239,7 +240,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg)
 	if (!__intel_dmc_wl_supported(display))
 		return;
 
-	if (!intel_dmc_wl_check_range(reg.reg))
+	if (!intel_dmc_wl_check_range(reg))
 		return;
 
 	spin_lock_irqsave(&wl->lock, flags);
-- 
2.47.0

Re: [PATCH v2 01/17] drm/i915/dmc_wl: Use i915_mmio_reg_offset() instead of reg.reg

[Jani Nikula]

On Wed, 06 Nov 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> The macro i915_mmio_reg_offset() is the proper interface to get a
> register's offset. Use that instead of looking directly at reg.reg.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dmc_wl.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> index 5634ff07269d..05892a237d3a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> +++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
> @@ -91,14 +91,15 @@ static void intel_dmc_wl_work(struct work_struct *work)
>  	spin_unlock_irqrestore(&wl->lock, flags);
>  }
>  
> -static bool intel_dmc_wl_check_range(u32 address)
> +static bool intel_dmc_wl_check_range(i915_reg_t reg)
>  {
>  	int i;
>  	bool wl_needed = false;
> +	u32 offset = i915_mmio_reg_offset(reg);
>  
>  	for (i = 0; i < ARRAY_SIZE(lnl_wl_range); i++) {
> -		if (address >= lnl_wl_range[i].start &&
> -		    address <= lnl_wl_range[i].end) {
> +		if (offset >= lnl_wl_range[i].start &&
> +		    offset <= lnl_wl_range[i].end) {
>  			wl_needed = true;
>  			break;
>  		}
> @@ -191,7 +192,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
>  	if (!__intel_dmc_wl_supported(display))
>  		return;
>  
> -	if (!intel_dmc_wl_check_range(reg.reg))
> +	if (!intel_dmc_wl_check_range(reg))
>  		return;
>  
>  	spin_lock_irqsave(&wl->lock, flags);
> @@ -239,7 +240,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg)
>  	if (!__intel_dmc_wl_supported(display))
>  		return;
>  
> -	if (!intel_dmc_wl_check_range(reg.reg))
> +	if (!intel_dmc_wl_check_range(reg))
>  		return;
>  
>  	spin_lock_irqsave(&wl->lock, flags);

-- 
Jani Nikula, Intel

Re: [PATCH v2 01/17] drm/i915/dmc_wl: Use i915_mmio_reg_offset() instead of reg.reg

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> The macro i915_mmio_reg_offset() is the proper interface to get a
> register's offset. Use that instead of looking directly at reg.reg.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 02/17] drm/xe: Mimic i915 behavior for non-sleeping MMIO wait

[Gustavo Sousa]

In upcoming display changes, we will modify the DMC wakelock MMIO
waiting code to choose a non-sleeping variant implementation, because
the wakelock is also taking in atomic context.

While xe provides an explicit parameter (namely "atomic") to prevent
xe_mmio_wait32() from sleeping, i915 does not and implements that
behavior when slow_timeout_ms is zero.

So, for now, let's mimic what i915 does to allow for display to use
non-sleeping MMIO wait. In the future, we should come up with a better
and explicit interface for this behavior in i915, at least while display
code is not an independent entity with proper interfaces between xe and
i915.

v2:
  - Make the tone in comment the comment added in
    __intel_wait_for_register() more explanatory than a FIXME-like text.
    (Luca)

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h b/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
index 0382beb4035b..686c39f320e4 100644
--- a/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
+++ b/drivers/gpu/drm/xe/compat-i915-headers/intel_uncore.h
@@ -117,10 +117,19 @@ __intel_wait_for_register(struct intel_uncore *uncore, i915_reg_t i915_reg,
 			  unsigned int slow_timeout_ms, u32 *out_value)
 {
 	struct xe_reg reg = XE_REG(i915_mmio_reg_offset(i915_reg));
+	bool atomic;
+
+	/*
+	 * Replicate the behavior from i915 here, in which sleep is not
+	 * performed if slow_timeout_ms == 0. This is necessary because
+	 * of some paths in display code where waits are done in atomic
+	 * context.
+	 */
+	atomic = !slow_timeout_ms && fast_timeout_us > 0;
 
 	return xe_mmio_wait32(__compat_uncore_to_mmio(uncore), reg, mask, value,
 			      fast_timeout_us + 1000 * slow_timeout_ms,
-			      out_value, false);
+			      out_value, atomic);
 }
 
 static inline u32 intel_uncore_read_fw(struct intel_uncore *uncore,
-- 
2.47.0

Re: [PATCH v2 02/17] drm/xe: Mimic i915 behavior for non-sleeping MMIO wait

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> In upcoming display changes, we will modify the DMC wakelock MMIO
> waiting code to choose a non-sleeping variant implementation, because
> the wakelock is also taking in atomic context.
> 
> While xe provides an explicit parameter (namely "atomic") to prevent
> xe_mmio_wait32() from sleeping, i915 does not and implements that
> behavior when slow_timeout_ms is zero.
> 
> So, for now, let's mimic what i915 does to allow for display to use
> non-sleeping MMIO wait. In the future, we should come up with a better
> and explicit interface for this behavior in i915, at least while display
> code is not an independent entity with proper interfaces between xe and
> i915.
> 
> v2:
>   - Make the tone in comment the comment added in
>     __intel_wait_for_register() more explanatory than a FIXME-like text.
>     (Luca)
> 
> Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 03/17] drm/i915/dmc_wl: Use non-sleeping variant of MMIO wait

[Gustavo Sousa]

Some display MMIO transactions for offsets in the range that requires
the DMC wakelock happen in atomic context (this has been confirmed
during tests on PTL). That means that we need to use a non-sleeping
variant of MMIO waiting function.

Implement __intel_de_wait_for_register_atomic_nowl() and use it when
waiting for acknowledgment of acquire/release.

v2:
  - No __intel_de_wait_for_register_atomic_nowl() wrapper to convert
    i915 to display. (Jani)
  - Add a quick explanation why DMC_WAKELOCK_CTL_TIMEOUT_US is defined
    in microseconds. (Luca)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_de.h     | 10 +++++++++
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 24 ++++++++++++++-------
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_de.h b/drivers/gpu/drm/i915/display/intel_de.h
index bb51f974e9e2..4561de5d5e10 100644
--- a/drivers/gpu/drm/i915/display/intel_de.h
+++ b/drivers/gpu/drm/i915/display/intel_de.h
@@ -117,6 +117,16 @@ __intel_de_wait_for_register_nowl(struct intel_display *display,
 				       value, timeout);
 }
 
+static inline int
+__intel_de_wait_for_register_atomic_nowl(struct intel_display *display,
+					 i915_reg_t reg,
+					 u32 mask, u32 value,
+					 unsigned int fast_timeout_us)
+{
+	return __intel_wait_for_register(__to_uncore(display), reg, mask,
+					 value, fast_timeout_us, 0, NULL);
+}
+
 static inline int
 intel_de_wait(struct intel_display *display, i915_reg_t reg,
 	      u32 mask, u32 value, unsigned int timeout)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 05892a237d3a..9255505437d5 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -39,7 +39,11 @@
  * potential future use.
  */
 
-#define DMC_WAKELOCK_CTL_TIMEOUT 5
+/*
+ * Define DMC_WAKELOCK_CTL_TIMEOUT_US in microseconds because we use the
+ * atomic variant of waiting MMIO.
+ */
+#define DMC_WAKELOCK_CTL_TIMEOUT_US 5000
 #define DMC_WAKELOCK_HOLD_TIME 50
 
 struct intel_dmc_wl_range {
@@ -78,9 +82,9 @@ static void intel_dmc_wl_work(struct work_struct *work)
 
 	__intel_de_rmw_nowl(display, DMC_WAKELOCK1_CTL, DMC_WAKELOCK_CTL_REQ, 0);
 
-	if (__intel_de_wait_for_register_nowl(display, DMC_WAKELOCK1_CTL,
-					      DMC_WAKELOCK_CTL_ACK, 0,
-					      DMC_WAKELOCK_CTL_TIMEOUT)) {
+	if (__intel_de_wait_for_register_atomic_nowl(display, DMC_WAKELOCK1_CTL,
+						     DMC_WAKELOCK_CTL_ACK, 0,
+						     DMC_WAKELOCK_CTL_TIMEOUT_US)) {
 		WARN_RATELIMIT(1, "DMC wakelock release timed out");
 		goto out_unlock;
 	}
@@ -217,10 +221,14 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
 		__intel_de_rmw_nowl(display, DMC_WAKELOCK1_CTL, 0,
 				    DMC_WAKELOCK_CTL_REQ);
 
-		if (__intel_de_wait_for_register_nowl(display, DMC_WAKELOCK1_CTL,
-						      DMC_WAKELOCK_CTL_ACK,
-						      DMC_WAKELOCK_CTL_ACK,
-						      DMC_WAKELOCK_CTL_TIMEOUT)) {
+		/*
+		 * We need to use the atomic variant of the waiting routine
+		 * because the DMC wakelock is also taken in atomic context.
+		 */
+		if (__intel_de_wait_for_register_atomic_nowl(display, DMC_WAKELOCK1_CTL,
+							     DMC_WAKELOCK_CTL_ACK,
+							     DMC_WAKELOCK_CTL_ACK,
+							     DMC_WAKELOCK_CTL_TIMEOUT_US)) {
 			WARN_RATELIMIT(1, "DMC wakelock ack timed out");
 			goto out_unlock;
 		}
-- 
2.47.0

Re: [PATCH v2 03/17] drm/i915/dmc_wl: Use non-sleeping variant of MMIO wait

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> Some display MMIO transactions for offsets in the range that requires
> the DMC wakelock happen in atomic context (this has been confirmed
> during tests on PTL). That means that we need to use a non-sleeping
> variant of MMIO waiting function.
> 
> Implement __intel_de_wait_for_register_atomic_nowl() and use it when
> waiting for acknowledgment of acquire/release.
> 
> v2:
>   - No __intel_de_wait_for_register_atomic_nowl() wrapper to convert
>     i915 to display. (Jani)
>   - Add a quick explanation why DMC_WAKELOCK_CTL_TIMEOUT_US is defined
>     in microseconds. (Luca)
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Luca Coelho <luciano.coelho@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 04/17] drm/i915/dmc_wl: Check for non-zero refcount in release work

[Gustavo Sousa]

When the DMC wakelock refcount reaches zero, we know that there are no
users and that we can do the actual release operation on the hardware,
which is queued with a delayed work. The idea of the delayed work is to
avoid performing the release if a new lock user appears (i.e. refcount
gets incremented) in a very short period of time.

Based on the above, the release work should bail out if refcount is
non-zero (meaning new lock users appeared in the meantime), but our
current code actually does the opposite: it bails when refcount is zero.
That means that the wakelock is not released when it should be; and
that, when the work is not canceled in time, it ends up being releasing
when it should not.

Fix that by inverting the condition.

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 9255505437d5..a0a060706305 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -76,8 +76,11 @@ static void intel_dmc_wl_work(struct work_struct *work)
 
 	spin_lock_irqsave(&wl->lock, flags);
 
-	/* Bail out if refcount reached zero while waiting for the spinlock */
-	if (!refcount_read(&wl->refcount))
+	/*
+	 * Bail out if refcount became non-zero while waiting for the spinlock,
+	 * meaning that the lock is now taken again.
+	 */
+	if (refcount_read(&wl->refcount))
 		goto out_unlock;
 
 	__intel_de_rmw_nowl(display, DMC_WAKELOCK1_CTL, DMC_WAKELOCK_CTL_REQ, 0);
-- 
2.47.0

[PATCH v2 05/17] drm/i915/dmc_wl: Get wakelock when disabling dynamic DC states

[Gustavo Sousa]

Bspec says that disabling dynamic DC states require taking the DMC
wakelock to cause an DC exit before writing to DC_STATE_EN. Implement
that.

In fact, testing on PTL revealed we end up failing to exit DC5/6 without
this step.

Bspec: 71583
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 .../drm/i915/display/intel_display_power_well.c    | 10 +++++++---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c        | 14 ++++++++++++--
 drivers/gpu/drm/i915/display/intel_dmc_wl.h        |  2 ++
 3 files changed, 21 insertions(+), 5 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 f0131dd853de..0c77b6252969 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -994,10 +994,14 @@ void gen9_disable_dc_states(struct intel_display *display)
 		return;
 	}
 
-	gen9_set_dc_state(display, DC_STATE_DISABLE);
-
-	if (!HAS_DISPLAY(display))
+	if (HAS_DISPLAY(display)) {
+		intel_dmc_wl_get_noreg(display);
+		gen9_set_dc_state(display, DC_STATE_DISABLE);
+		intel_dmc_wl_put_noreg(display);
+	} else {
+		gen9_set_dc_state(display, DC_STATE_DISABLE);
 		return;
+	}
 
 	intel_dmc_wl_disable(display);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index a0a060706305..e837c39491bb 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -199,7 +199,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
 	if (!__intel_dmc_wl_supported(display))
 		return;
 
-	if (!intel_dmc_wl_check_range(reg))
+	if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg))
 		return;
 
 	spin_lock_irqsave(&wl->lock, flags);
@@ -251,7 +251,7 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg)
 	if (!__intel_dmc_wl_supported(display))
 		return;
 
-	if (!intel_dmc_wl_check_range(reg))
+	if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg))
 		return;
 
 	spin_lock_irqsave(&wl->lock, flags);
@@ -272,3 +272,13 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg)
 out_unlock:
 	spin_unlock_irqrestore(&wl->lock, flags);
 }
+
+void intel_dmc_wl_get_noreg(struct intel_display *display)
+{
+	intel_dmc_wl_get(display, INVALID_MMIO_REG);
+}
+
+void intel_dmc_wl_put_noreg(struct intel_display *display)
+{
+	intel_dmc_wl_put(display, INVALID_MMIO_REG);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.h b/drivers/gpu/drm/i915/display/intel_dmc_wl.h
index adab51208d0a..9aa72a4bf153 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.h
@@ -27,5 +27,7 @@ void intel_dmc_wl_enable(struct intel_display *display);
 void intel_dmc_wl_disable(struct intel_display *display);
 void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg);
 void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg);
+void intel_dmc_wl_get_noreg(struct intel_display *display);
+void intel_dmc_wl_put_noreg(struct intel_display *display);
 
 #endif /* __INTEL_WAKELOCK_H__ */
-- 
2.47.0

Re: [PATCH v2 05/17] drm/i915/dmc_wl: Get wakelock when disabling dynamic DC states

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> Bspec says that disabling dynamic DC states require taking the DMC
> wakelock to cause an DC exit before writing to DC_STATE_EN. Implement
> that.
> 
> In fact, testing on PTL revealed we end up failing to exit DC5/6 without
> this step.
> 
> Bspec: 71583
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 06/17] drm/i915/dmc_wl: Use sentinel item for range tables

[Gustavo Sousa]

We are currently using ARRAY_SIZE() to iterate address ranges in
intel_dmc_wl_check_range(). In upcoming changes, we will be using more
than a single table and will extract the range checking logic into a
dedicated function that takes a range table as argument. As we will not
able to use ARRAY_SIZE() then, let's make range tables contain a
sentinel item at the end and use that instead of having to pass the size
as parameter in this future function.

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index e837c39491bb..1753c334f3fd 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -53,6 +53,7 @@ struct intel_dmc_wl_range {
 
 static struct intel_dmc_wl_range lnl_wl_range[] = {
 	{ .start = 0x60000, .end = 0x7ffff },
+	{},
 };
 
 static void __intel_dmc_wl_release(struct intel_display *display)
@@ -104,7 +105,7 @@ static bool intel_dmc_wl_check_range(i915_reg_t reg)
 	bool wl_needed = false;
 	u32 offset = i915_mmio_reg_offset(reg);
 
-	for (i = 0; i < ARRAY_SIZE(lnl_wl_range); i++) {
+	for (i = 0; lnl_wl_range[i].start; i++) {
 		if (offset >= lnl_wl_range[i].start &&
 		    offset <= lnl_wl_range[i].end) {
 			wl_needed = true;
-- 
2.47.0

[PATCH v2 07/17] drm/i915/dmc_wl: Extract intel_dmc_wl_reg_in_range()

[Gustavo Sousa]

We will be using more than one range table in
intel_dmc_wl_check_range(). As such, move the logic to a new function
and name it intel_dmc_wl_reg_in_range().

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 1753c334f3fd..4b958a4c4358 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -99,21 +99,22 @@ static void intel_dmc_wl_work(struct work_struct *work)
 	spin_unlock_irqrestore(&wl->lock, flags);
 }
 
-static bool intel_dmc_wl_check_range(i915_reg_t reg)
+static bool intel_dmc_wl_reg_in_range(i915_reg_t reg,
+				      const struct intel_dmc_wl_range ranges[])
 {
-	int i;
-	bool wl_needed = false;
 	u32 offset = i915_mmio_reg_offset(reg);
 
-	for (i = 0; lnl_wl_range[i].start; i++) {
-		if (offset >= lnl_wl_range[i].start &&
-		    offset <= lnl_wl_range[i].end) {
-			wl_needed = true;
-			break;
-		}
+	for (int i = 0; ranges[i].start; i++) {
+		if (ranges[i].start <= offset && offset <= ranges[i].end)
+			return true;
 	}
 
-	return wl_needed;
+	return false;
+}
+
+static bool intel_dmc_wl_check_range(i915_reg_t reg)
+{
+	return intel_dmc_wl_reg_in_range(reg, lnl_wl_range);
 }
 
 static bool __intel_dmc_wl_supported(struct intel_display *display)
-- 
2.47.0

Re: [PATCH v2 07/17] drm/i915/dmc_wl: Extract intel_dmc_wl_reg_in_range()

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> We will be using more than one range table in
> intel_dmc_wl_check_range(). As such, move the logic to a new function
> and name it intel_dmc_wl_reg_in_range().
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 08/17] drm/i915/dmc_wl: Rename lnl_wl_range to powered_off_ranges

[Gustavo Sousa]

In an upcoming change, we will add extra range tables for registers that
are touched by the DMC during DC states. The range table that we are
currently using is meant for registers that are powered off during DC
states. As such, let's rename the table to powered_off_ranges and also
add a comment regarding its purpose in the function that uses it.

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 4b958a4c4358..1877a89affab 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -51,7 +51,7 @@ struct intel_dmc_wl_range {
 	u32 end;
 };
 
-static struct intel_dmc_wl_range lnl_wl_range[] = {
+static struct intel_dmc_wl_range powered_off_ranges[] = {
 	{ .start = 0x60000, .end = 0x7ffff },
 	{},
 };
@@ -114,7 +114,11 @@ static bool intel_dmc_wl_reg_in_range(i915_reg_t reg,
 
 static bool intel_dmc_wl_check_range(i915_reg_t reg)
 {
-	return intel_dmc_wl_reg_in_range(reg, lnl_wl_range);
+	/*
+	 * Check that the offset is in one of the ranges for which
+	 * registers are powered off during DC states.
+	 */
+	return intel_dmc_wl_reg_in_range(reg, powered_off_ranges);
 }
 
 static bool __intel_dmc_wl_supported(struct intel_display *display)
-- 
2.47.0

Re: [PATCH v2 08/17] drm/i915/dmc_wl: Rename lnl_wl_range to powered_off_ranges

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> In an upcoming change, we will add extra range tables for registers that
> are touched by the DMC during DC states. The range table that we are
> currently using is meant for registers that are powered off during DC
> states. As such, let's rename the table to powered_off_ranges and also
> add a comment regarding its purpose in the function that uses it.
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 09/17] drm/i915/dmc_wl: Track registers touched by the DMC

[Gustavo Sousa]

There are extra registers that require the DMC wakelock when specific
dynamic DC states are in place. Those are registers that are touched by
the DMC and require DC exit for proper access. Add the range tables for
them and use the correct one depending on the enabled DC state.

v2:
  - Do not look into power domains guts (i.e.
    display->power.domains.dc_state). (Jani)
  - Come up with better names for variables containing register ranges.
    (Luca)
  - Keep a copy of dc_state in struct intel_dmc_wl.
  - Update commit message for a clearer explanation for the need of
    these new tables.

Bspec: 71583
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 .../i915/display/intel_display_power_well.c   |   4 +-
 drivers/gpu/drm/i915/display/intel_dmc_wl.c   | 126 ++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_dmc_wl.h   |  11 +-
 3 files changed, 128 insertions(+), 13 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 0c77b6252969..578959ff2d75 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -833,7 +833,7 @@ void gen9_enable_dc5(struct intel_display *display)
 		intel_de_rmw(display, GEN8_CHICKEN_DCPR_1,
 			     0, SKL_SELECT_ALTERNATE_DC_EXIT);
 
-	intel_dmc_wl_enable(display);
+	intel_dmc_wl_enable(display, DC_STATE_EN_UPTO_DC5);
 
 	gen9_set_dc_state(display, DC_STATE_EN_UPTO_DC5);
 }
@@ -866,7 +866,7 @@ void skl_enable_dc6(struct intel_display *display)
 		intel_de_rmw(display, GEN8_CHICKEN_DCPR_1,
 			     0, SKL_SELECT_ALTERNATE_DC_EXIT);
 
-	intel_dmc_wl_enable(display);
+	intel_dmc_wl_enable(display, DC_STATE_EN_UPTO_DC6);
 
 	gen9_set_dc_state(display, DC_STATE_EN_UPTO_DC6);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 1877a89affab..db01b65cb05d 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -5,6 +5,7 @@
 
 #include <linux/kernel.h>
 
+#include "i915_reg.h"
 #include "intel_de.h"
 #include "intel_dmc.h"
 #include "intel_dmc_regs.h"
@@ -56,6 +57,87 @@ static struct intel_dmc_wl_range powered_off_ranges[] = {
 	{},
 };
 
+static struct intel_dmc_wl_range xe3lpd_dc5_dc6_dmc_ranges[] = {
+	{ .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */
+	{ .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */
+	{ .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */
+	{ .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */
+	{ .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */
+
+	/* DBUF_CTL_* */
+	{ .start = 0x44300, .end = 0x44300 },
+	{ .start = 0x44304, .end = 0x44304 },
+	{ .start = 0x44f00, .end = 0x44f00 },
+	{ .start = 0x44f04, .end = 0x44f04 },
+	{ .start = 0x44fe8, .end = 0x44fe8 },
+	{ .start = 0x45008, .end = 0x45008 },
+
+	{ .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */
+	{ .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */
+	{ .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */
+
+	/* TRANS_CMTG_CTL_* */
+	{ .start = 0x6fa88, .end = 0x6fa88 },
+	{ .start = 0x6fb88, .end = 0x6fb88 },
+
+	{ .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */
+	{ .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */
+	{ .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */
+	{ .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */
+	{ .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */
+	{ .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */
+	{ .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */
+
+	{},
+};
+
+static struct intel_dmc_wl_range xe3lpd_dc3co_dmc_ranges[] = {
+	{ .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */
+
+	{ .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */
+
+	/* DBUF_CTL_* */
+	{ .start = 0x44300, .end = 0x44300 },
+	{ .start = 0x44304, .end = 0x44304 },
+	{ .start = 0x44f00, .end = 0x44f00 },
+	{ .start = 0x44f04, .end = 0x44f04 },
+	{ .start = 0x44fe8, .end = 0x44fe8 },
+	{ .start = 0x45008, .end = 0x45008 },
+
+	{ .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */
+	{ .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */
+	{ .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */
+	{ .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */
+
+	/* Scanline registers */
+	{ .start = 0x70000, .end = 0x70000 },
+	{ .start = 0x70004, .end = 0x70004 },
+	{ .start = 0x70014, .end = 0x70014 },
+	{ .start = 0x70018, .end = 0x70018 },
+	{ .start = 0x71000, .end = 0x71000 },
+	{ .start = 0x71004, .end = 0x71004 },
+	{ .start = 0x71014, .end = 0x71014 },
+	{ .start = 0x71018, .end = 0x71018 },
+	{ .start = 0x72000, .end = 0x72000 },
+	{ .start = 0x72004, .end = 0x72004 },
+	{ .start = 0x72014, .end = 0x72014 },
+	{ .start = 0x72018, .end = 0x72018 },
+	{ .start = 0x73000, .end = 0x73000 },
+	{ .start = 0x73004, .end = 0x73004 },
+	{ .start = 0x73014, .end = 0x73014 },
+	{ .start = 0x73018, .end = 0x73018 },
+	{ .start = 0x7b000, .end = 0x7b000 },
+	{ .start = 0x7b004, .end = 0x7b004 },
+	{ .start = 0x7b014, .end = 0x7b014 },
+	{ .start = 0x7b018, .end = 0x7b018 },
+	{ .start = 0x7c000, .end = 0x7c000 },
+	{ .start = 0x7c004, .end = 0x7c004 },
+	{ .start = 0x7c014, .end = 0x7c014 },
+	{ .start = 0x7c018, .end = 0x7c018 },
+
+	{},
+};
+
 static void __intel_dmc_wl_release(struct intel_display *display)
 {
 	struct drm_i915_private *i915 = to_i915(display->drm);
@@ -112,13 +194,37 @@ static bool intel_dmc_wl_reg_in_range(i915_reg_t reg,
 	return false;
 }
 
-static bool intel_dmc_wl_check_range(i915_reg_t reg)
+static bool intel_dmc_wl_check_range(i915_reg_t reg, u32 dc_state)
 {
+	const struct intel_dmc_wl_range *ranges;
+
 	/*
 	 * Check that the offset is in one of the ranges for which
 	 * registers are powered off during DC states.
 	 */
-	return intel_dmc_wl_reg_in_range(reg, powered_off_ranges);
+	if (intel_dmc_wl_reg_in_range(reg, powered_off_ranges))
+		return true;
+
+	/*
+	 * Check that the offset is for a register that is touched by
+	 * the DMC and requires a DC exit for proper access.
+	 */
+	switch (dc_state) {
+	case DC_STATE_EN_DC3CO:
+		ranges = xe3lpd_dc3co_dmc_ranges;
+		break;
+	case DC_STATE_EN_UPTO_DC5:
+	case DC_STATE_EN_UPTO_DC6:
+		ranges = xe3lpd_dc5_dc6_dmc_ranges;
+		break;
+	default:
+		ranges = NULL;
+	}
+
+	if (ranges && intel_dmc_wl_reg_in_range(reg, ranges))
+		return true;
+
+	return false;
 }
 
 static bool __intel_dmc_wl_supported(struct intel_display *display)
@@ -144,7 +250,7 @@ void intel_dmc_wl_init(struct intel_display *display)
 	refcount_set(&wl->refcount, 0);
 }
 
-void intel_dmc_wl_enable(struct intel_display *display)
+void intel_dmc_wl_enable(struct intel_display *display, u32 dc_state)
 {
 	struct intel_dmc_wl *wl = &display->wl;
 	unsigned long flags;
@@ -154,6 +260,8 @@ void intel_dmc_wl_enable(struct intel_display *display)
 
 	spin_lock_irqsave(&wl->lock, flags);
 
+	wl->dc_state = dc_state;
+
 	if (wl->enabled)
 		goto out_unlock;
 
@@ -205,11 +313,11 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
 	if (!__intel_dmc_wl_supported(display))
 		return;
 
-	if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg))
-		return;
-
 	spin_lock_irqsave(&wl->lock, flags);
 
+	if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg, wl->dc_state))
+		goto out_unlock;
+
 	if (!wl->enabled)
 		goto out_unlock;
 
@@ -257,11 +365,11 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg)
 	if (!__intel_dmc_wl_supported(display))
 		return;
 
-	if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg))
-		return;
-
 	spin_lock_irqsave(&wl->lock, flags);
 
+	if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg, wl->dc_state))
+		goto out_unlock;
+
 	if (!wl->enabled)
 		goto out_unlock;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.h b/drivers/gpu/drm/i915/display/intel_dmc_wl.h
index 9aa72a4bf153..147eeb4d8432 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.h
@@ -15,15 +15,22 @@
 struct intel_display;
 
 struct intel_dmc_wl {
-	spinlock_t lock; /* protects enabled, taken  and refcount */
+	spinlock_t lock; /* protects enabled, taken, dc_state and refcount */
 	bool enabled;
 	bool taken;
 	refcount_t refcount;
+	/*
+	 * We are keeping a copy of the enabled DC state because
+	 * intel_display.power.domains is protected by a mutex and we do
+	 * not want call mutex_lock() in atomic context, where some of
+	 * the tracked MMIO operations happen.
+	 */
+	u32 dc_state;
 	struct delayed_work work;
 };
 
 void intel_dmc_wl_init(struct intel_display *display);
-void intel_dmc_wl_enable(struct intel_display *display);
+void intel_dmc_wl_enable(struct intel_display *display, u32 dc_state);
 void intel_dmc_wl_disable(struct intel_display *display);
 void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg);
 void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg);
-- 
2.47.0

Re: [PATCH v2 09/17] drm/i915/dmc_wl: Track registers touched by the DMC

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> There are extra registers that require the DMC wakelock when specific
> dynamic DC states are in place. Those are registers that are touched by
> the DMC and require DC exit for proper access. Add the range tables for
> them and use the correct one depending on the enabled DC state.
> 
> v2:
>   - Do not look into power domains guts (i.e.
>     display->power.domains.dc_state). (Jani)
>   - Come up with better names for variables containing register ranges.
>     (Luca)
>   - Keep a copy of dc_state in struct intel_dmc_wl.
>   - Update commit message for a clearer explanation for the need of
>     these new tables.
> 
> Bspec: 71583
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Luca Coelho <luciano.coelho@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
> 

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 10/17] drm/i915/dmc_wl: Allow simpler syntax for single reg in range tables

[Gustavo Sousa]

Allow simpler syntax for defining entries for single registers in range
tables. That makes them easier to type as well as to read, allowing one
to quickly tell whether a range actually refers to a single register or
a "true range".

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 118 ++++++++++----------
 1 file changed, 60 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index db01b65cb05d..4a182a049374 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -58,82 +58,82 @@ static struct intel_dmc_wl_range powered_off_ranges[] = {
 };
 
 static struct intel_dmc_wl_range xe3lpd_dc5_dc6_dmc_ranges[] = {
-	{ .start = 0x45500, .end = 0x45500 }, /* DC_STATE_SEL */
+	{ .start = 0x45500 }, /* DC_STATE_SEL */
 	{ .start = 0x457a0, .end = 0x457b0 }, /* DC*_RESIDENCY_COUNTER */
-	{ .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */
+	{ .start = 0x45504 }, /* DC_STATE_EN */
 	{ .start = 0x45400, .end = 0x4540c }, /* PWR_WELL_CTL_* */
-	{ .start = 0x454f0, .end = 0x454f0 }, /* RETENTION_CTRL */
+	{ .start = 0x454f0 }, /* RETENTION_CTRL */
 
 	/* DBUF_CTL_* */
-	{ .start = 0x44300, .end = 0x44300 },
-	{ .start = 0x44304, .end = 0x44304 },
-	{ .start = 0x44f00, .end = 0x44f00 },
-	{ .start = 0x44f04, .end = 0x44f04 },
-	{ .start = 0x44fe8, .end = 0x44fe8 },
-	{ .start = 0x45008, .end = 0x45008 },
+	{ .start = 0x44300 },
+	{ .start = 0x44304 },
+	{ .start = 0x44f00 },
+	{ .start = 0x44f04 },
+	{ .start = 0x44fe8 },
+	{ .start = 0x45008 },
 
-	{ .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */
-	{ .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */
-	{ .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */
+	{ .start = 0x46070 }, /* CDCLK_PLL_ENABLE */
+	{ .start = 0x46000 }, /* CDCLK_CTL */
+	{ .start = 0x46008 }, /* CDCLK_SQUASH_CTL */
 
 	/* TRANS_CMTG_CTL_* */
-	{ .start = 0x6fa88, .end = 0x6fa88 },
-	{ .start = 0x6fb88, .end = 0x6fb88 },
-
-	{ .start = 0x46430, .end = 0x46430 }, /* CHICKEN_DCPR_1 */
-	{ .start = 0x46434, .end = 0x46434 }, /* CHICKEN_DCPR_2 */
-	{ .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */
-	{ .start = 0x42084, .end = 0x42084 }, /* CHICKEN_MISC_2 */
-	{ .start = 0x42088, .end = 0x42088 }, /* CHICKEN_MISC_3 */
-	{ .start = 0x46160, .end = 0x46160 }, /* CMTG_CLK_SEL */
+	{ .start = 0x6fa88 },
+	{ .start = 0x6fb88 },
+
+	{ .start = 0x46430 }, /* CHICKEN_DCPR_1 */
+	{ .start = 0x46434 }, /* CHICKEN_DCPR_2 */
+	{ .start = 0x454a0 }, /* CHICKEN_DCPR_4 */
+	{ .start = 0x42084 }, /* CHICKEN_MISC_2 */
+	{ .start = 0x42088 }, /* CHICKEN_MISC_3 */
+	{ .start = 0x46160 }, /* CMTG_CLK_SEL */
 	{ .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */
 
 	{},
 };
 
 static struct intel_dmc_wl_range xe3lpd_dc3co_dmc_ranges[] = {
-	{ .start = 0x454a0, .end = 0x454a0 }, /* CHICKEN_DCPR_4 */
+	{ .start = 0x454a0 }, /* CHICKEN_DCPR_4 */
 
-	{ .start = 0x45504, .end = 0x45504 }, /* DC_STATE_EN */
+	{ .start = 0x45504 }, /* DC_STATE_EN */
 
 	/* DBUF_CTL_* */
-	{ .start = 0x44300, .end = 0x44300 },
-	{ .start = 0x44304, .end = 0x44304 },
-	{ .start = 0x44f00, .end = 0x44f00 },
-	{ .start = 0x44f04, .end = 0x44f04 },
-	{ .start = 0x44fe8, .end = 0x44fe8 },
-	{ .start = 0x45008, .end = 0x45008 },
-
-	{ .start = 0x46070, .end = 0x46070 }, /* CDCLK_PLL_ENABLE */
-	{ .start = 0x46000, .end = 0x46000 }, /* CDCLK_CTL */
-	{ .start = 0x46008, .end = 0x46008 }, /* CDCLK_SQUASH_CTL */
+	{ .start = 0x44300 },
+	{ .start = 0x44304 },
+	{ .start = 0x44f00 },
+	{ .start = 0x44f04 },
+	{ .start = 0x44fe8 },
+	{ .start = 0x45008 },
+
+	{ .start = 0x46070 }, /* CDCLK_PLL_ENABLE */
+	{ .start = 0x46000 }, /* CDCLK_CTL */
+	{ .start = 0x46008 }, /* CDCLK_SQUASH_CTL */
 	{ .start = 0x8f000, .end = 0x8ffff }, /* Main DMC registers */
 
 	/* Scanline registers */
-	{ .start = 0x70000, .end = 0x70000 },
-	{ .start = 0x70004, .end = 0x70004 },
-	{ .start = 0x70014, .end = 0x70014 },
-	{ .start = 0x70018, .end = 0x70018 },
-	{ .start = 0x71000, .end = 0x71000 },
-	{ .start = 0x71004, .end = 0x71004 },
-	{ .start = 0x71014, .end = 0x71014 },
-	{ .start = 0x71018, .end = 0x71018 },
-	{ .start = 0x72000, .end = 0x72000 },
-	{ .start = 0x72004, .end = 0x72004 },
-	{ .start = 0x72014, .end = 0x72014 },
-	{ .start = 0x72018, .end = 0x72018 },
-	{ .start = 0x73000, .end = 0x73000 },
-	{ .start = 0x73004, .end = 0x73004 },
-	{ .start = 0x73014, .end = 0x73014 },
-	{ .start = 0x73018, .end = 0x73018 },
-	{ .start = 0x7b000, .end = 0x7b000 },
-	{ .start = 0x7b004, .end = 0x7b004 },
-	{ .start = 0x7b014, .end = 0x7b014 },
-	{ .start = 0x7b018, .end = 0x7b018 },
-	{ .start = 0x7c000, .end = 0x7c000 },
-	{ .start = 0x7c004, .end = 0x7c004 },
-	{ .start = 0x7c014, .end = 0x7c014 },
-	{ .start = 0x7c018, .end = 0x7c018 },
+	{ .start = 0x70000 },
+	{ .start = 0x70004 },
+	{ .start = 0x70014 },
+	{ .start = 0x70018 },
+	{ .start = 0x71000 },
+	{ .start = 0x71004 },
+	{ .start = 0x71014 },
+	{ .start = 0x71018 },
+	{ .start = 0x72000 },
+	{ .start = 0x72004 },
+	{ .start = 0x72014 },
+	{ .start = 0x72018 },
+	{ .start = 0x73000 },
+	{ .start = 0x73004 },
+	{ .start = 0x73014 },
+	{ .start = 0x73018 },
+	{ .start = 0x7b000 },
+	{ .start = 0x7b004 },
+	{ .start = 0x7b014 },
+	{ .start = 0x7b018 },
+	{ .start = 0x7c000 },
+	{ .start = 0x7c004 },
+	{ .start = 0x7c014 },
+	{ .start = 0x7c018 },
 
 	{},
 };
@@ -187,7 +187,9 @@ static bool intel_dmc_wl_reg_in_range(i915_reg_t reg,
 	u32 offset = i915_mmio_reg_offset(reg);
 
 	for (int i = 0; ranges[i].start; i++) {
-		if (ranges[i].start <= offset && offset <= ranges[i].end)
+		u32 end = ranges[i].end ?: ranges[i].start;
+
+		if (ranges[i].start <= offset && offset <= end)
 			return true;
 	}
 
-- 
2.47.0

Re: [PATCH v2 10/17] drm/i915/dmc_wl: Allow simpler syntax for single reg in range tables

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> Allow simpler syntax for defining entries for single registers in range
> tables. That makes them easier to type as well as to read, allowing one
> to quickly tell whether a range actually refers to a single register or
> a "true range".
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 11/17] drm/i915/dmc_wl: Deal with existing references when disabling

[Gustavo Sousa]

It is possible that there are active wakelock references at the time we
are disabling the DMC wakelock mechanism. We need to deal with that in
two ways:

(A) Implement the missing step from Bspec:

    The Bspec instructs us to clear any existing wakelock request bit
    after disabling the mechanism. That gives a clue that it is okay to
    disable while there are locks held and we do not need to wait for
    them. However, since the spec is not explicit about it, we need
    still to get confirmation with the hardware team. Let's thus
    implement the spec and add a TODO note.

(B) Ensure a consistent driver state:

    The enable/disable logic would be problematic if the following
    sequence of events would happen:

    1. Function A calls intel_dmc_wl_get();
    2. Some function calls intel_dmc_wl_disable();
    3. Some function calls intel_dmc_wl_enable();
    4. Function A is done and calls intel_dmc_wl_put().

    At (2), the refcount becomes zero and then (4) causes an invalid
    decrement to the refcount. That would cause some issues:

        - At the time between (3) and (4), function A would think that
          the hardware lock is held but it could not be really held
          until intel_dmc_wl_get() is called by something else.
        - The call made to (4) could cause the refcount to become zero
          and consequently the hardware lock to be released while there
          could be innocent paths trusting they still have the lock.

    To fix that, we need to keep the refcount correctly in sync with
    intel_dmc_wl_{get,put}() calls and retake the hardware lock when
    enabling the DMC wakelock with a non-zero refcount.

    One missing piece left to be handled here is the following scenario:

    1. Function A calls intel_dmc_wl_get();
    2. Some function calls intel_dmc_wl_disable();
    3. Some function calls intel_dmc_wl_enable();
    4. Concurrently with (3), function A performs the MMIO in between
       setting DMC_WAKELOCK_CFG_ENABLE and asserting the lock with
       __intel_dmc_wl_take().

    I'm mostly sure this would cause issues future display IPs if DMC
    trap implementation was completely removed. We need to check with
    the hardware team whether it would be safe to assert the hardware
    lock before setting DMC_WAKELOCK_CFG_ENABLE to avoid this scenario.
    If not, then we would have to deal with that via software
    synchronization.

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 97 ++++++++++++++-------
 1 file changed, 67 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 4a182a049374..b8887216a684 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -181,6 +181,37 @@ static void intel_dmc_wl_work(struct work_struct *work)
 	spin_unlock_irqrestore(&wl->lock, flags);
 }
 
+static void __intel_dmc_wl_take(struct intel_display *display)
+{
+	struct intel_dmc_wl *wl = &display->wl;
+
+	/*
+	 * Only try to take the wakelock if it's not marked as taken
+	 * yet.  It may be already taken at this point if we have
+	 * already released the last reference, but the work has not
+	 * run yet.
+	 */
+	if (wl->taken)
+		return;
+
+	__intel_de_rmw_nowl(display, DMC_WAKELOCK1_CTL, 0,
+			    DMC_WAKELOCK_CTL_REQ);
+
+	/*
+	 * We need to use the atomic variant of the waiting routine
+	 * because the DMC wakelock is also taken in atomic context.
+	 */
+	if (__intel_de_wait_for_register_atomic_nowl(display, DMC_WAKELOCK1_CTL,
+						     DMC_WAKELOCK_CTL_ACK,
+						     DMC_WAKELOCK_CTL_ACK,
+						     DMC_WAKELOCK_CTL_TIMEOUT_US)) {
+		WARN_RATELIMIT(1, "DMC wakelock ack timed out");
+		return;
+	}
+
+	wl->taken = true;
+}
+
 static bool intel_dmc_wl_reg_in_range(i915_reg_t reg,
 				      const struct intel_dmc_wl_range ranges[])
 {
@@ -275,7 +306,23 @@ void intel_dmc_wl_enable(struct intel_display *display, u32 dc_state)
 	__intel_de_rmw_nowl(display, DMC_WAKELOCK_CFG, 0, DMC_WAKELOCK_CFG_ENABLE);
 
 	wl->enabled = true;
-	wl->taken = false;
+
+	/*
+	 * This would be racy in the following scenario:
+	 *
+	 *   1. Function A calls intel_dmc_wl_get();
+	 *   2. Some function calls intel_dmc_wl_disable();
+	 *   3. Some function calls intel_dmc_wl_enable();
+	 *   4. Concurrently with (3), function A performs the MMIO in between
+	 *      setting DMC_WAKELOCK_CFG_ENABLE and asserting the lock with
+	 *      __intel_dmc_wl_take().
+	 *
+	 * TODO: Check with the hardware team whether it is safe to assert the
+	 * hardware lock before enabling to avoid such a scenario. Otherwise, we
+	 * would need to deal with it via software synchronization.
+	 */
+	if (refcount_read(&wl->refcount))
+		__intel_dmc_wl_take(display);
 
 out_unlock:
 	spin_unlock_irqrestore(&wl->lock, flags);
@@ -299,8 +346,18 @@ void intel_dmc_wl_disable(struct intel_display *display)
 	/* Disable wakelock in DMC */
 	__intel_de_rmw_nowl(display, DMC_WAKELOCK_CFG, DMC_WAKELOCK_CFG_ENABLE, 0);
 
-	refcount_set(&wl->refcount, 0);
 	wl->enabled = false;
+
+	/*
+	 * The spec is not explicit about the expectation of existing
+	 * lock users at the moment of disabling, but it does say that we must
+	 * clear DMC_WAKELOCK_CTL_REQ, which gives us a clue that it is okay to
+	 * disable with existing lock users.
+	 *
+	 * TODO: Get the correct expectation from the hardware team.
+	 */
+	__intel_de_rmw_nowl(display, DMC_WAKELOCK1_CTL, DMC_WAKELOCK_CTL_REQ, 0);
+
 	wl->taken = false;
 
 out_unlock:
@@ -320,8 +377,11 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
 	if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg, wl->dc_state))
 		goto out_unlock;
 
-	if (!wl->enabled)
+	if (!wl->enabled) {
+		if (!refcount_inc_not_zero(&wl->refcount))
+			refcount_set(&wl->refcount, 1);
 		goto out_unlock;
+	}
 
 	cancel_delayed_work(&wl->work);
 
@@ -330,30 +390,7 @@ void intel_dmc_wl_get(struct intel_display *display, i915_reg_t reg)
 
 	refcount_set(&wl->refcount, 1);
 
-	/*
-	 * Only try to take the wakelock if it's not marked as taken
-	 * yet.  It may be already taken at this point if we have
-	 * already released the last reference, but the work has not
-	 * run yet.
-	 */
-	if (!wl->taken) {
-		__intel_de_rmw_nowl(display, DMC_WAKELOCK1_CTL, 0,
-				    DMC_WAKELOCK_CTL_REQ);
-
-		/*
-		 * We need to use the atomic variant of the waiting routine
-		 * because the DMC wakelock is also taken in atomic context.
-		 */
-		if (__intel_de_wait_for_register_atomic_nowl(display, DMC_WAKELOCK1_CTL,
-							     DMC_WAKELOCK_CTL_ACK,
-							     DMC_WAKELOCK_CTL_ACK,
-							     DMC_WAKELOCK_CTL_TIMEOUT_US)) {
-			WARN_RATELIMIT(1, "DMC wakelock ack timed out");
-			goto out_unlock;
-		}
-
-		wl->taken = true;
-	}
+	__intel_dmc_wl_take(display);
 
 out_unlock:
 	spin_unlock_irqrestore(&wl->lock, flags);
@@ -372,14 +409,14 @@ void intel_dmc_wl_put(struct intel_display *display, i915_reg_t reg)
 	if (i915_mmio_reg_valid(reg) && !intel_dmc_wl_check_range(reg, wl->dc_state))
 		goto out_unlock;
 
-	if (!wl->enabled)
-		goto out_unlock;
-
 	if (WARN_RATELIMIT(!refcount_read(&wl->refcount),
 			   "Tried to put wakelock with refcount zero\n"))
 		goto out_unlock;
 
 	if (refcount_dec_and_test(&wl->refcount)) {
+		if (!wl->enabled)
+			goto out_unlock;
+
 		__intel_dmc_wl_release(display);
 
 		goto out_unlock;
-- 
2.47.0

[PATCH v2 12/17] drm/i915/dmc_wl: Couple enable/disable with dynamic DC states

[Gustavo Sousa]

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.

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
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 578959ff2d75..bdf6c690a03b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -988,6 +988,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);
@@ -1003,7 +1004,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 87bdacfd9edf..221d3abda791 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -638,8 +638,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)
@@ -1146,8 +1144,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 b8887216a684..f2d64954916a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -283,6 +283,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, u32 dc_state)
 {
 	struct intel_dmc_wl *wl = &display->wl;
@@ -295,7 +296,7 @@ void intel_dmc_wl_enable(struct intel_display *display, u32 dc_state)
 
 	wl->dc_state = dc_state;
 
-	if (wl->enabled)
+	if (drm_WARN_ON(display->drm, wl->enabled))
 		goto out_unlock;
 
 	/*
@@ -328,6 +329,7 @@ void intel_dmc_wl_enable(struct intel_display *display, u32 dc_state)
 	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;
@@ -340,7 +342,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

[PATCH v2 13/17] drm/i915/dmc_wl: Add and use HAS_DMC_WAKELOCK()

[Gustavo Sousa]

A HAS_DMC_WAKELOCK() macro gives more semantic than openly checking the
display version. Define it and use it where appropriate.

v2:
  - Make this patch contain only the non-functional refactor. Functional
    changes related to including HAS_DMC() in the macro are done in
    upcoming changes. (Jani)

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_device.h | 1 +
 drivers/gpu/drm/i915/display/intel_dmc_wl.c         | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 43144a037f9f..65d17c6cbbb3 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -147,6 +147,7 @@ struct intel_display_platforms {
 #define HAS_DDI(i915)			(DISPLAY_INFO(i915)->has_ddi)
 #define HAS_DISPLAY(i915)		(DISPLAY_RUNTIME_INFO(i915)->pipe_mask != 0)
 #define HAS_DMC(i915)			(DISPLAY_RUNTIME_INFO(i915)->has_dmc)
+#define HAS_DMC_WAKELOCK(i915)		(DISPLAY_VER(i915) >= 20)
 #define HAS_DOUBLE_BUFFERED_M_N(i915)	(DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915))
 #define HAS_DP_MST(i915)		(DISPLAY_INFO(i915)->has_dp_mst)
 #define HAS_DP20(i915)			(IS_DG2(i915) || DISPLAY_VER(i915) >= 14)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index f2d64954916a..4ca2b990ec6a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -262,7 +262,7 @@ static bool intel_dmc_wl_check_range(i915_reg_t reg, u32 dc_state)
 
 static bool __intel_dmc_wl_supported(struct intel_display *display)
 {
-	if (DISPLAY_VER(display) < 20 ||
+	if (!HAS_DMC_WAKELOCK(display) ||
 	    !intel_dmc_has_payload(display) ||
 	    !display->params.enable_dmc_wl)
 		return false;
@@ -275,7 +275,7 @@ void intel_dmc_wl_init(struct intel_display *display)
 	struct intel_dmc_wl *wl = &display->wl;
 
 	/* don't call __intel_dmc_wl_supported(), DMC is not loaded yet */
-	if (DISPLAY_VER(display) < 20 || !display->params.enable_dmc_wl)
+	if (!HAS_DMC_WAKELOCK(display) || !display->params.enable_dmc_wl)
 		return;
 
 	INIT_DELAYED_WORK(&wl->work, intel_dmc_wl_work);
-- 
2.47.0

Re: [PATCH v2 13/17] drm/i915/dmc_wl: Add and use HAS_DMC_WAKELOCK()

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> A HAS_DMC_WAKELOCK() macro gives more semantic than openly checking the
> display version. Define it and use it where appropriate.
> 
> v2:
>   - Make this patch contain only the non-functional refactor. Functional
>     changes related to including HAS_DMC() in the macro are done in
>     upcoming changes. (Jani)
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
> 

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 14/17] drm/i915/dmc_wl: Init only after we have runtime device info

[Gustavo Sousa]

We should be able to use the DMC wakelock only if the display hardware
has support for DMC. We will add a check for that in an upcoming change.

Since info for DMC availability (HAS_DMC()) needs runtime device info,
move the call to intel_dmc_wl_init() to a place where we know we have
the hardware has been probed for such an info (i.e. after
intel_display_device_info_runtime_init()).

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 56b78cf6b854..4257cc380475 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -200,7 +200,6 @@ void intel_display_driver_early_probe(struct drm_i915_private *i915)
 	intel_dpll_init_clock_hook(i915);
 	intel_init_display_hooks(i915);
 	intel_fdi_init_hook(i915);
-	intel_dmc_wl_init(&i915->display);
 }
 
 /* part #1: call before irq install */
@@ -238,6 +237,7 @@ int intel_display_driver_probe_noirq(struct drm_i915_private *i915)
 		return 0;
 
 	intel_dmc_init(display);
+	intel_dmc_wl_init(display);
 
 	i915->display.wq.modeset = alloc_ordered_workqueue("i915_modeset", 0);
 	i915->display.wq.flip = alloc_workqueue("i915_flip", WQ_HIGHPRI |
-- 
2.47.0

Re: [PATCH v2 14/17] drm/i915/dmc_wl: Init only after we have runtime device info

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> We should be able to use the DMC wakelock only if the display hardware
> has support for DMC. We will add a check for that in an upcoming change.
> 
> Since info for DMC availability (HAS_DMC()) needs runtime device info,
> move the call to intel_dmc_wl_init() to a place where we know we have
> the hardware has been probed for such an info (i.e. after
> intel_display_device_info_runtime_init()).
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
> 

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 15/17] drm/i915/dmc_wl: Use HAS_DMC() in HAS_DMC_WAKELOCK()

[Gustavo Sousa]

In order to be able to use the DMC wakelock, we also need to know that
the display hardware has support for DMC. For that, include HAS_DMC() in
the definition of HAS_DMC_WAKELOCK().

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_device.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 65d17c6cbbb3..43aa1b2501d7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -147,7 +147,7 @@ struct intel_display_platforms {
 #define HAS_DDI(i915)			(DISPLAY_INFO(i915)->has_ddi)
 #define HAS_DISPLAY(i915)		(DISPLAY_RUNTIME_INFO(i915)->pipe_mask != 0)
 #define HAS_DMC(i915)			(DISPLAY_RUNTIME_INFO(i915)->has_dmc)
-#define HAS_DMC_WAKELOCK(i915)		(DISPLAY_VER(i915) >= 20)
+#define HAS_DMC_WAKELOCK(i915)		(HAS_DMC(i915) && DISPLAY_VER(i915) >= 20)
 #define HAS_DOUBLE_BUFFERED_M_N(i915)	(DISPLAY_VER(i915) >= 9 || IS_BROADWELL(i915))
 #define HAS_DP_MST(i915)		(DISPLAY_INFO(i915)->has_dp_mst)
 #define HAS_DP20(i915)			(IS_DG2(i915) || DISPLAY_VER(i915) >= 14)
-- 
2.47.0

Re: [PATCH v2 15/17] drm/i915/dmc_wl: Use HAS_DMC() in HAS_DMC_WAKELOCK()

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> In order to be able to use the DMC wakelock, we also need to know that
> the display hardware has support for DMC. For that, include HAS_DMC() in
> the definition of HAS_DMC_WAKELOCK().
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
> 

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

[PATCH v2 16/17] drm/i915/dmc_wl: Sanitize enable_dmc_wl according to hardware support

[Gustavo Sousa]

Instead of checking for HAS_DMC_WAKELOCK() multiple times, let's use it
to sanitize the enable_dmc_wl parameter and use that variable when
necessary.

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc_wl.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index 4ca2b990ec6a..c164ac6e1ada 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -5,6 +5,8 @@
 
 #include <linux/kernel.h>
 
+#include <drm/drm_print.h>
+
 #include "i915_reg.h"
 #include "intel_de.h"
 #include "intel_dmc.h"
@@ -262,20 +264,25 @@ static bool intel_dmc_wl_check_range(i915_reg_t reg, u32 dc_state)
 
 static bool __intel_dmc_wl_supported(struct intel_display *display)
 {
-	if (!HAS_DMC_WAKELOCK(display) ||
-	    !intel_dmc_has_payload(display) ||
-	    !display->params.enable_dmc_wl)
-		return false;
+	return display->params.enable_dmc_wl && intel_dmc_has_payload(display);
+}
 
-	return true;
+static void intel_dmc_wl_sanitize_param(struct intel_display *display)
+{
+	if (!HAS_DMC_WAKELOCK(display))
+		display->params.enable_dmc_wl = false;
+
+	drm_dbg_kms(display->drm, "Sanitized enable_dmc_wl value: %d\n",
+		    display->params.enable_dmc_wl);
 }
 
 void intel_dmc_wl_init(struct intel_display *display)
 {
 	struct intel_dmc_wl *wl = &display->wl;
 
-	/* don't call __intel_dmc_wl_supported(), DMC is not loaded yet */
-	if (!HAS_DMC_WAKELOCK(display) || !display->params.enable_dmc_wl)
+	intel_dmc_wl_sanitize_param(display);
+
+	if (!display->params.enable_dmc_wl)
 		return;
 
 	INIT_DELAYED_WORK(&wl->work, intel_dmc_wl_work);
-- 
2.47.0

[PATCH v2 17/17] drm/i915/xe3lpd: Use DMC wakelock by default

[Gustavo Sousa]

Although Bspec doesn't explicitly mentions that, as of Xe3_LPD, using
DMC wakelock is the officially recommended way of accessing registers
that would be off during DC5/DC6 and the legacy method (where the DMC
intercepts MMIO to wake up the hardware) is to be avoided.

As such, update the driver to use the DMC wakelock by default starting
with Xe3_LPD. Since the feature is somewhat new to the driver, also
allow disabling it via a module parameter for debugging purposes.

For that, make the existing parameter allow values -1 (per-chip
default), 0 (disabled) and 1 (enabled), similarly to what is done for
other parameters.

v2:
  - Describe -1 in the same area where 0 and 1 are described. (Luca)

Cc: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_params.c | 6 +++---
 drivers/gpu/drm/i915/display/intel_display_params.h | 2 +-
 drivers/gpu/drm/i915/display/intel_dmc_wl.c         | 6 +++++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_params.c b/drivers/gpu/drm/i915/display/intel_display_params.c
index 024de8abcb1a..dc666aefa362 100644
--- a/drivers/gpu/drm/i915/display/intel_display_params.c
+++ b/drivers/gpu/drm/i915/display/intel_display_params.c
@@ -123,10 +123,10 @@ intel_display_param_named_unsafe(enable_psr2_sel_fetch, bool, 0400,
 	"(0=disabled, 1=enabled) "
 	"Default: 1");
 
-intel_display_param_named_unsafe(enable_dmc_wl, bool, 0400,
+intel_display_param_named_unsafe(enable_dmc_wl, int, 0400,
 	"Enable DMC wakelock "
-	"(0=disabled, 1=enabled) "
-	"Default: 0");
+	"(-1=use per-chip default, 0=disabled, 1=enabled) "
+	"Default: -1");
 
 __maybe_unused
 static void _param_print_bool(struct drm_printer *p, const char *driver_name,
diff --git a/drivers/gpu/drm/i915/display/intel_display_params.h b/drivers/gpu/drm/i915/display/intel_display_params.h
index dcb6face936a..5317138e6044 100644
--- a/drivers/gpu/drm/i915/display/intel_display_params.h
+++ b/drivers/gpu/drm/i915/display/intel_display_params.h
@@ -47,7 +47,7 @@ struct drm_printer;
 	param(int, enable_psr, -1, 0600) \
 	param(bool, psr_safest_params, false, 0400) \
 	param(bool, enable_psr2_sel_fetch, true, 0400) \
-	param(bool, enable_dmc_wl, false, 0400) \
+	param(int, enable_dmc_wl, -1, 0400) \
 
 #define MEMBER(T, member, ...) T member;
 struct intel_display_params {
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_wl.c b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
index c164ac6e1ada..853d75610489 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_wl.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc_wl.c
@@ -270,7 +270,11 @@ static bool __intel_dmc_wl_supported(struct intel_display *display)
 static void intel_dmc_wl_sanitize_param(struct intel_display *display)
 {
 	if (!HAS_DMC_WAKELOCK(display))
-		display->params.enable_dmc_wl = false;
+		display->params.enable_dmc_wl = 0;
+	else if (display->params.enable_dmc_wl >= 0)
+		display->params.enable_dmc_wl = !!display->params.enable_dmc_wl;
+	else
+		display->params.enable_dmc_wl = DISPLAY_VER(display) >= 30;
 
 	drm_dbg_kms(display->drm, "Sanitized enable_dmc_wl value: %d\n",
 		    display->params.enable_dmc_wl);
-- 
2.47.0

Re: [PATCH v2 17/17] drm/i915/xe3lpd: Use DMC wakelock by default

[Luca Coelho]

On Wed, 2024-11-06 at 18:50 -0300, Gustavo Sousa wrote:
> Although Bspec doesn't explicitly mentions that, as of Xe3_LPD, using
> DMC wakelock is the officially recommended way of accessing registers
> that would be off during DC5/DC6 and the legacy method (where the DMC
> intercepts MMIO to wake up the hardware) is to be avoided.
> 
> As such, update the driver to use the DMC wakelock by default starting
> with Xe3_LPD. Since the feature is somewhat new to the driver, also
> allow disabling it via a module parameter for debugging purposes.
> 
> For that, make the existing parameter allow values -1 (per-chip
> default), 0 (disabled) and 1 (enabled), similarly to what is done for
> other parameters.
> 
> v2:
>   - Describe -1 in the same area where 0 and 1 are described. (Luca)
> 
> Cc: Luca Coelho <luciano.coelho@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
> 

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

✗ Fi.CI.CHECKPATCH: warning for drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD (rev2)
URL   : https://patchwork.freedesktop.org/series/140282/
State : warning

== Summary ==

Error: dim checkpatch failed
d03be180852b drm/i915/dmc_wl: Use i915_mmio_reg_offset() instead of reg.reg
8781d6c39354 drm/xe: Mimic i915 behavior for non-sleeping MMIO wait
b6314b0f43e9 drm/i915/dmc_wl: Use non-sleeping variant of MMIO wait
bb441af11eb3 drm/i915/dmc_wl: Check for non-zero refcount in release work
920986e9ecca drm/i915/dmc_wl: Get wakelock when disabling dynamic DC states
05d898edb8ce drm/i915/dmc_wl: Use sentinel item for range tables
3823a2d2f1d9 drm/i915/dmc_wl: Extract intel_dmc_wl_reg_in_range()
d0d734f8a182 drm/i915/dmc_wl: Rename lnl_wl_range to powered_off_ranges
7e995f91539c drm/i915/dmc_wl: Track registers touched by the DMC
77d8de7056d0 drm/i915/dmc_wl: Allow simpler syntax for single reg in range tables
c3e92a65bc76 drm/i915/dmc_wl: Deal with existing references when disabling
611aa98f714c drm/i915/dmc_wl: Couple enable/disable with dynamic DC states
199b4466ae10 drm/i915/dmc_wl: Add and use HAS_DMC_WAKELOCK()
b7e7c1ccf706 drm/i915/dmc_wl: Init only after we have runtime device info
2719d94e6e7f drm/i915/dmc_wl: Use HAS_DMC() in HAS_DMC_WAKELOCK()
-:22: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'i915' - possible side-effects?
#22: FILE: drivers/gpu/drm/i915/display/intel_display_device.h:150:
+#define HAS_DMC_WAKELOCK(i915)		(HAS_DMC(i915) && DISPLAY_VER(i915) >= 20)

total: 0 errors, 0 warnings, 1 checks, 8 lines checked
ea92bac5bd9a drm/i915/dmc_wl: Sanitize enable_dmc_wl according to hardware support
e886d8c2addc drm/i915/xe3lpd: Use DMC wakelock by default
-:35: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#35: FILE: drivers/gpu/drm/i915/display/intel_display_params.c:127:
+intel_display_param_named_unsafe(enable_dmc_wl, int, 0400,
 	"Enable DMC wakelock "

total: 0 errors, 0 warnings, 1 checks, 33 lines checked

✗ Fi.CI.SPARSE: warning for drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD (rev2)
URL   : https://patchwork.freedesktop.org/series/140282/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4160 bytes --]

== Series Details ==

Series: drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD (rev2)
URL   : https://patchwork.freedesktop.org/series/140282/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15648 -> Patchwork_140282v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_140282v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_140282v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/index.html

Participating hosts (45 -> 43)
------------------------------

  Missing    (2): fi-snb-2520m bat-jsl-3 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_140282v2:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - fi-cfl-8109u:       [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/fi-cfl-8109u/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/fi-cfl-8109u/igt@i915_module_load@load.html

  * igt@i915_selftest@live:
    - bat-twl-1:          [PASS][3] -> [INCOMPLETE][4] +1 other test incomplete
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/bat-twl-1/igt@i915_selftest@live.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/bat-twl-1/igt@i915_selftest@live.html
    - fi-skl-6600u:       [PASS][5] -> [ABORT][6] +1 other test abort
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/fi-skl-6600u/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/fi-skl-6600u/igt@i915_selftest@live.html

  * igt@i915_selftest@live@uncore:
    - fi-glk-j4005:       [PASS][7] -> [ABORT][8] +1 other test abort
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/fi-glk-j4005/igt@i915_selftest@live@uncore.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/fi-glk-j4005/igt@i915_selftest@live@uncore.html

  * igt@kms_busy@basic@flip:
    - fi-cfl-8109u:       [PASS][9] -> [INCOMPLETE][10] +1 other test incomplete
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/fi-cfl-8109u/igt@kms_busy@basic@flip.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/fi-cfl-8109u/igt@kms_busy@basic@flip.html

  
Known issues
------------

  Here are the changes found in Patchwork_140282v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - bat-adlp-6:         [PASS][11] -> [INCOMPLETE][12] ([i915#9413]) +1 other test incomplete
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/bat-adlp-6/igt@i915_selftest@live.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/bat-adlp-6/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-6:         [PASS][13] -> [ABORT][14] ([i915#12061]) +1 other test abort
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/bat-mtlp-6/igt@i915_selftest@live@workarounds.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/bat-mtlp-6/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#9413]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9413


Build changes
-------------

  * Linux: CI_DRM_15648 -> Patchwork_140282v2

  CI-20190529: 20190529
  CI_DRM_15648: 5ce87c5ad2cbfd2b89a0347e4e4f75de2762b7a3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8100: 84e42580f918da926481fd2fb37be01451d6ee9a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_140282v2: 5ce87c5ad2cbfd2b89a0347e4e4f75de2762b7a3 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/index.html

[-- Attachment #2: Type: text/html, Size: 4927 bytes --]

Re: ✗ Fi.CI.BAT: failure for drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD (rev2)

[Gustavo Sousa]

Quoting Patchwork (2024-11-06 19:46:38-03:00)
>== Series Details ==
>
>Series: drm/i915/dmc_wl: Fixes and enablement for Xe3_LPD (rev2)
>URL   : https://patchwork.freedesktop.org/series/140282/
>State : failure
>
>== Summary ==
>
>CI Bug Log - changes from CI_DRM_15648 -> Patchwork_140282v2
>====================================================
>
>Summary
>-------
>
>  **FAILURE**
>
>  Serious unknown changes coming with Patchwork_140282v2 absolutely need to be
>  verified manually.
>  
>  If you think the reported changes have nothing to do with the changes
>  introduced in Patchwork_140282v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
>  to document this new failure mode, which will reduce false positives in CI.
>
>  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/index.html
>
>Participating hosts (45 -> 43)
>------------------------------
>
>  Missing    (2): fi-snb-2520m bat-jsl-3 
>
>Possible new issues
>-------------------
>
>  Here are the unknown changes that may have been introduced in Patchwork_140282v2:
>
>### IGT changes ###
>
>#### Possible regressions ####
>
>  * igt@i915_module_load@load:
>    - fi-cfl-8109u:       [PASS][1] -> [DMESG-WARN][2]
>   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/fi-cfl-8109u/igt@i915_module_load@load.html
>   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/fi-cfl-8109u/igt@i915_module_load@load.html
>
>  * igt@i915_selftest@live:
>    - bat-twl-1:          [PASS][3] -> [INCOMPLETE][4] +1 other test incomplete
>   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/bat-twl-1/igt@i915_selftest@live.html
>   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/bat-twl-1/igt@i915_selftest@live.html
>    - fi-skl-6600u:       [PASS][5] -> [ABORT][6] +1 other test abort
>   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/fi-skl-6600u/igt@i915_selftest@live.html
>   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/fi-skl-6600u/igt@i915_selftest@live.html
>
>  * igt@i915_selftest@live@uncore:
>    - fi-glk-j4005:       [PASS][7] -> [ABORT][8] +1 other test abort
>   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/fi-glk-j4005/igt@i915_selftest@live@uncore.html
>   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/fi-glk-j4005/igt@i915_selftest@live@uncore.html
>
>  * igt@kms_busy@basic@flip:
>    - fi-cfl-8109u:       [PASS][9] -> [INCOMPLETE][10] +1 other test incomplete
>   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15648/fi-cfl-8109u/igt@kms_busy@basic@flip.html
>   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140282v2/fi-cfl-8109u/igt@kms_busy@basic@flip.html


Some of the issues above are due to intel_dmc_wl_get() and
intel_dmc_wl_put() being called by MMIO functions before
intel_dmc_wl_init() was called. I posted a new version of the series to
address that issue.

--
Gustavo Sousa