Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/display: Enable PICA power before AUX
@ 2025-09-24 11:54 Gustavo Sousa
  2025-09-24 12:14 ` ✗ CI.checkpatch: warning for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Gustavo Sousa @ 2025-09-24 11:54 UTC (permalink / raw)
  To: intel-gfx, intel-xe; +Cc: Gustavo Sousa

According to Bspec, before enabling AUX power, we need to have the
"power well containing Aux logic powered up". Starting with Xe2_LPD,
such power well is the "PICA" power well, which is managed by the driver
on demand.

While we did add the mapping of AUX power domains to the PICA power
well, we ended up placing its power well descriptor after the
descriptor for AUX power. As a result, when enabling power wells for one
of the aux power domains, the driver will enable AUX power before PICA
power, going against the order specified in Bspec.

It appears that issue did not become apparent to us mainly because,
luckily, AUX power is brought up after we assert PICA power, even if
done in the wrong order; and in enough time for the first AUX
transaction to succeed.

Furthermore, I have also realized that, in some cases, like driver
initialization, PICA power is already up when we need to acquire AUX
power.

One case where we can observe the incorrect ordering is when the driver
is resuming from runtime PM suspend. Here is an excerpt of a dmesg with
some extra debug logs extracted from a LNL machine to illustrate the
issue:

    [  +0.000156] xe 0000:00:02.0: [drm:intel_power_well_enable [xe]] enabling AUX_TC1
    [  +0.001312] xe 0000:00:02.0: [drm:xelpdp_aux_power_well_enable [xe]] DBG: AUX_CH_USBC1 power status: 0
    [  +0.000127] xe 0000:00:02.0: [drm:intel_power_well_enable [xe]] enabling PICA_TC
    [  +0.001072] xe 0000:00:02.0: [drm:xe2lpd_pica_power_well_enable [xe]] DBG: AUX_CH_USBC1 power status: 1
    [  +0.000102] xe 0000:00:02.0: [drm:xe2lpd_pica_power_well_enable [xe]] DBG: AUX_CH_USBC2 power status: 0
    [  +0.000090] xe 0000:00:02.0: [drm:xe2lpd_pica_power_well_enable [xe]] DBG: AUX_CH_USBC3 power status: 0
    [  +0.000092] xe 0000:00:02.0: [drm:xe2lpd_pica_power_well_enable [xe]] DBG: AUX_CH_USBC4 power status: 0

The first "DBG: ..." line shows that AUX power for TC1 is off after we
assert and wait. The remaining lines show that AUX power for TC1 was on
after we enabled PICA power and waited for AUX power.

It is important that we stay compliant with the spec, so let's fix this
by listing the power wells in an order that matches the requirements
from Bspec. (As a side note, it would be nice if we could define those
dependencies explicitly.)

After this change, we have:

    [  +0.000146] xe 0000:00:02.0: [drm:intel_power_well_enable [xe]] enabling PICA_TC
    [  +0.001417] xe 0000:00:02.0: [drm:xe2lpd_pica_power_well_enable [xe]] DBG: AUX_CH_USBC1 power status: 0
    [  +0.000116] xe 0000:00:02.0: [drm:xe2lpd_pica_power_well_enable [xe]] DBG: AUX_CH_USBC2 power status: 0
    [  +0.000096] xe 0000:00:02.0: [drm:xe2lpd_pica_power_well_enable [xe]] DBG: AUX_CH_USBC3 power status: 0
    [  +0.000094] xe 0000:00:02.0: [drm:xe2lpd_pica_power_well_enable [xe]] DBG: AUX_CH_USBC4 power status: 0
    [  +0.000095] xe 0000:00:02.0: [drm:intel_power_well_enable [xe]] enabling AUX_TC1
    [  +0.000915] xe 0000:00:02.0: [drm:xelpdp_aux_power_well_enable [xe]] DBG: AUX_CH_USBC1 power status: 1

Bspec: 68967, 68886, 72519
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_power_map.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
index 39b71fffa2cd..d057bbde42c2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
@@ -1582,8 +1582,8 @@ static const struct i915_power_well_desc_list xe2lpd_power_wells[] = {
 	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
 	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
 	I915_PW_DESCRIPTORS(xe2lpd_power_wells_dcoff),
-	I915_PW_DESCRIPTORS(xelpdp_power_wells_main),
 	I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
+	I915_PW_DESCRIPTORS(xelpdp_power_wells_main),
 };
 
 /*
@@ -1713,8 +1713,8 @@ static const struct i915_power_well_desc_list xe3lpd_power_wells[] = {
 	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
 	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
 	I915_PW_DESCRIPTORS(xe3lpd_power_wells_dcoff),
-	I915_PW_DESCRIPTORS(xe3lpd_power_wells_main),
 	I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
+	I915_PW_DESCRIPTORS(xe3lpd_power_wells_main),
 };
 
 static const struct i915_power_well_desc wcl_power_wells_main[] = {
@@ -1766,8 +1766,8 @@ static const struct i915_power_well_desc_list wcl_power_wells[] = {
 	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
 	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
 	I915_PW_DESCRIPTORS(xe3lpd_power_wells_dcoff),
-	I915_PW_DESCRIPTORS(wcl_power_wells_main),
 	I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
+	I915_PW_DESCRIPTORS(wcl_power_wells_main),
 };
 
 static void init_power_well_domains(const struct i915_power_well_instance *inst,

---
base-commit: 308a05859081aae4125b58d186d582b814c6deb2
change-id: 20250923-pica-power-before-aux-70009ccd1b7b

Best regards,
--  
Gustavo Sousa <gustavo.sousa@intel.com>


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-24 15:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-24 11:54 [PATCH] drm/i915/display: Enable PICA power before AUX Gustavo Sousa
2025-09-24 12:14 ` ✗ CI.checkpatch: warning for " Patchwork
2025-09-24 12:16 ` ✓ CI.KUnit: success " Patchwork
2025-09-24 12:50 ` ✓ Xe.CI.BAT: " Patchwork
2025-09-24 14:03 ` [PATCH] " Imre Deak
2025-09-24 15:56 ` ✗ Xe.CI.Full: failure for " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox