> Subject: Re: [PATCH 5/5] drm/i915/cx0: Read out power-down state of both
> PHY lanes for reversed lanes
> 
> 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 <[email protected]>
> > > Cc: Suraj Kandpal <[email protected]>
> > > Fixes: 230d4c748113 ("drm/i915/cx0: Track the Cx0 PHY enabled lane
> > > count in the PLL state")
> > > Signed-off-by: Imre Deak <[email protected]>
> > > ---
> > >  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).

Ohkay got it.

Reviewed-by: Suraj Kandpal <[email protected]>

> 
> > Regards.
> > Suraj Kandpal

Reply via email to