[PATCH] drm/i915/cx0: Use one lane to set power state to ready in DP alt mode

[Gustavo Sousa]

Hi, Vamsi.

Thanks for your patch. Please, see my feedback below.

Quoting Vamsi Krishna Brahmajosyula (2024-09-06 14:46:01-03:00)
>In DP alt mode one lane is owned by display and the other by usb
>intel_cx0pll_enable currently performs a power cycle ready on both
>the lanes in all cases.
>
>Address the todo to perfom power state ready only on the display lane
>when DP alt mode is enabled.
>
>Tested on Meteor Lake-P [Intel Arc Graphics] with DP alt mode.
>
>Signed-off-by: Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula at gmail.com>
>---
> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>index 4a6c3040ca15..47aa0418379c 100644
>--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>@@ -2949,9 +2949,13 @@ static void intel_cx0pll_enable(struct intel_encoder *encoder,
> 
>         /*
>          * 3. Change Phy power state to Ready.
>-         * TODO: For DP alt mode use only one lane.
>+         * For DP alt mode use only one lane.
>          */
>-        intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
>+        if (intel_tc_port_in_dp_alt_mode(dig_port))

The TODO description is a bit incomplete. Actually, we should do the PHY
power state update for *owned* PHY lanes. Both lanes could still be
owned in DP-Alt mode, depending on the pin assignment. In particular, a
single PHY lane is owned in DP-Alt mode when using pin assignment D.

Thus, I suggest doing an unconditional call to
intel_cx0_powerdown_change_sequence() and use the value returned by
intel_cx0_get_owned_lane_mask() as the argument for lane_mask.

See https://patchwork.freedesktop.org/series/121334/ for some reference.

--
Gustavo Sousa

>+                intel_cx0_powerdown_change_sequence(encoder, maxpclk_lane,
>+                                            CX0_P2_STATE_READY);
>+        else
>+                intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
>                                             CX0_P2_STATE_READY);
> 
>         /*
>
>base-commit: b831f83e40a24f07c8dcba5be408d93beedc820f
>-- 
>2.46.0
>