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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <172649666078.2630.10319073128429169176@gjsousa-mobl2>
Date: Mon, 16 Sep 2024 11:24:20 -0300
From: Gustavo Sousa <gustavo.sousa@...el.com>
To: Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@...il.com>,
	<dri-devel@...ts.freedesktop.org>
CC: <jani.nikula@...ux.intel.com>, <rodrigo.vivi@...el.com>,
	<joonas.lahtinen@...ux.intel.com>, <tursulin@...ulin.net>,
	<airlied@...il.com>, <daniel@...ll.ch>, <skhan@...uxfoundation.org>,
	<intel-gfx@...ts.freedesktop.org>, <intel-xe@...ts.freedesktop.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/i915/cx0: Use one lane to set power state to ready in DP alt mode

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@...il.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
>