Re: [PATCH 5/5] drm/i915/cx0: Read out power-down state of both PHY lanes for reversed lanes

[Imre Deak <imre.deak@intel.com>]

On Fri, Nov 21, 2025 at 05:54:46AM +0200, Suraj Kandpal wrote:
> > Subject: [PATCH 5/5] drm/i915/cx0: Read out power-down state of both PHY
> > lanes for reversed lanes
> > 
> > For a port used with lane reversal enabled the first two TX lanes will be enabled
> > in PHY lane#1 instead of PHY lane#0. At the moment the HW readout will read
> > out the power-down state for these two TX lanes from PHY lane#0 incorrectly.
> > The display HW lane reversal feature (vs. the similar TCSS lane swap) is only
> > used for TypeC legacy mode and for non-TypeC PHYs. Since in both of these
> > cases the display owns both PHY lanes, both of these PHY lanes' state can be
> > read out. Do that to fix cases when lane reversal is used with 1 or 2 active TX
> > lanes.
> > 
> > While at it add an assert to the PLL enable function about the above
> > assumption on when lane reversal can be used.
> > 
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> > Fixes: 230d4c748113 ("drm/i915/cx0: Track the Cx0 PHY enabled lane count in
> > the PLL state")
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 24 ++++++++++++++++++--
> >  1 file changed, 22 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 0d524735dcf95..27be2a490297f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -2197,17 +2197,30 @@ static int readout_enabled_lane_count(struct intel_encoder *encoder)  {
> >  	struct intel_display *display = to_intel_display(encoder);
> >  	u8 enabled_tx_lane_count = 0;
> > -	int max_tx_lane_count;
> > +	int max_tx_lane_count = 4;
> > +	bool lane_reversal;
> >  	int tx_lane;
> > 
> > +	lane_reversal = intel_de_read(display, XELPDP_PORT_BUF_CTL1(display, encoder->port)) &
> > +			XELPDP_PORT_REVERSAL;
> > +
> 
> Can't we just do 
> struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> bool lane_reversal = dig_port->lane_reversal;
> 
> or are there limitations with that now?

The HW/SW state verification should check what was programmed to the HW
(comparing the HW state to the SW state), so the readout here shouldn't
use any SW state. Also the PLL code should be independent, not rely on
anything in intel_encoder (except for port which is used to read out the
PLL/PHY registers).

> Regards.
> Suraj Kandpal