]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
drm/i915/display: Enable PICA power before AUX
authorGustavo Sousa <gustavo.sousa@intel.com>
Wed, 1 Oct 2025 16:04:49 +0000 (13:04 -0300)
committerGustavo Sousa <gustavo.sousa@intel.com>
Thu, 2 Oct 2025 21:28:36 +0000 (18:28 -0300)
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
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://lore.kernel.org/r/20251001-pica-power-before-aux-v2-2-6308df4de5a8@intel.com
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
drivers/gpu/drm/i915/display/intel_display_power_map.c

index e89f18b7037f23e848732124d6b64a98ad59d2ee..9b49952994ceafbcea848a70fc6f192061e868d8 100644 (file)
@@ -1588,8 +1588,8 @@ static const struct i915_power_well_desc_list xe2lpd_power_wells[] = {
        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(xelpdp_power_wells_aux),
        I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
+       I915_PW_DESCRIPTORS(xelpdp_power_wells_aux),
 };
 
 /*
@@ -1710,8 +1710,8 @@ static const struct i915_power_well_desc_list xe3lpd_power_wells[] = {
        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(xelpdp_power_wells_aux),
        I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
+       I915_PW_DESCRIPTORS(xelpdp_power_wells_aux),
 };
 
 static const struct i915_power_well_desc wcl_power_wells_main[] = {
@@ -1768,8 +1768,8 @@ static const struct i915_power_well_desc_list wcl_power_wells[] = {
        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(wcl_power_wells_aux),
        I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
+       I915_PW_DESCRIPTORS(wcl_power_wells_aux),
 };
 
 static void init_power_well_domains(const struct i915_power_well_instance *inst,