Quoting Matt Roper (2025-10-29 17:06:28-03:00)
>On Tue, Oct 21, 2025 at 09:28:35PM -0300, Gustavo Sousa wrote:
>> The LT PHY in Xe3p_LPD allows polling for the AUX channel power status
>> to verify completion of power up and down. As such, let's use that field
>> to have a more precise waiting time instead of a fixed one.
>>
>> Bspec: 68967
>> Signed-off-by: Gustavo Sousa <[email protected]>
>> ---
>> .../drm/i915/display/intel_display_power_well.c | 32
>> +++++++++++++++++-----
>> 1 file changed, 25 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 5e88b930f5aa..ba2552adb58b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> @@ -1858,23 +1858,41 @@ static void xelpdp_aux_power_well_enable(struct
>> intel_display *display,
>> XELPDP_DP_AUX_CH_CTL_POWER_REQUEST,
>> XELPDP_DP_AUX_CH_CTL_POWER_REQUEST);
>>
>> - /*
>> - * The power status flag cannot be used to determine whether aux
>> - * power wells have finished powering up. Instead we're
>> - * expected to just wait a fixed 600us after raising the request
>> - * bit.
>> - */
>> - usleep_range(600, 1200);
>> + if (DISPLAY_VER(display) >= 35) {
>
>Since the bspec specifically calls this out as a flow for LT PHY, it
>seems like it would make more sense to make the condition here a feature
>flag check for LT PHY. As I understand it, the selection of one type of
>PHY over another is more of a business / per-platform decision than an
>IP progression thing, so it's quite possible that some future platforms
>past Xe3p may not have LT PHYs.
Yeah, makes sense.
I know that the LT PHY series[1] does include a HAS_LT_PHY() macro, but
it does the same IP version check. Perhaps we should define that macro
using a platform check.
For this series, I think I'll add a patch to include the HAS_LT_PHY()
macro with platform check and use it here.
In the long term, I was thinking we could have an enum intel_phy_type
and have a function that returns the correct value based on the port.
--
Gustavo Sousa
>
>
>Matt
>
>> + if (intel_de_wait_for_set(display,
>> XELPDP_DP_AUX_CH_CTL(display, aux_ch),
>> +
>> XELPDP_DP_AUX_CH_CTL_POWER_STATUS, 1))
>> + drm_warn(display->drm,
>> + "Timeout waiting for PHY %c AUX channel
>> power to be up\n",
>> + phy_name(phy));
>> + } else {
>> + /*
>> + * The power status flag cannot be used to determine
>> whether aux
>> + * power wells have finished powering up. Instead we're
>> + * expected to just wait a fixed 600us after raising the
>> request
>> + * bit.
>> + */
>> + usleep_range(600, 1200);
>> + }
>> }
>>
>> static void xelpdp_aux_power_well_disable(struct intel_display *display,
>> struct i915_power_well
>> *power_well)
>> {
>> enum aux_ch aux_ch =
>> i915_power_well_instance(power_well)->xelpdp.aux_ch;
>> + enum phy phy = icl_aux_pw_to_phy(display, power_well);
>>
>> intel_de_rmw(display, XELPDP_DP_AUX_CH_CTL(display, aux_ch),
>> XELPDP_DP_AUX_CH_CTL_POWER_REQUEST,
>> 0);
>> +
>> + if (DISPLAY_VER(display) >= 35) {
>> + if (intel_de_wait_for_clear(display,
>> XELPDP_DP_AUX_CH_CTL(display, aux_ch),
>> +
>> XELPDP_DP_AUX_CH_CTL_POWER_STATUS, 1))
>> + drm_warn(display->drm,
>> + "Timeout waiting for PHY %c AUX channel
>> power to be down\n",
>> + phy_name(phy));
>> + }
>> +
>> usleep_range(10, 30);
>> }
>>
>>
>> --
>> 2.51.0
>>
>
>--
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation
