Re: [PATCH v2 04/15] drm/i915/lt_phy: Drop LT PHY crtc_state for port calculation

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

On Tue, Jan 06, 2026 at 05:49:15AM +0000, Kandpal, Suraj wrote:
> > Subject: [PATCH v2 04/15] drm/i915/lt_phy: Drop LT PHY crtc_state for port calculation
> > ...
> >
> > @@ -1748,12 +1746,10 @@ intel_lt_phy_calc_hdmi_port_clock(const struct intel_crtc_state *crtc_state)
> > }
> > 
> >  int
> > -intel_lt_phy_calc_port_clock(struct intel_encoder *encoder,
> > -			     const struct intel_crtc_state *crtc_state)
> > +intel_lt_phy_calc_port_clock(struct intel_display *display,
> > +			     const struct intel_lt_phy_pll_state *lt_state)
> >  {
> >  	int clk;
> > -	const struct intel_lt_phy_pll_state *lt_state =
> > -		&crtc_state->dpll_hw_state.ltpll;
> >  	u8 mode, rate;
> > 
> >  	mode = REG_FIELD_GET8(LT_PHY_VDR_MODE_ENCODING_MASK,
> > @@ -1769,7 +1765,7 @@ intel_lt_phy_calc_port_clock(struct intel_encoder *encoder,
> >  				      lt_state->config[0]);
> >  		clk = intel_lt_phy_get_dp_clock(rate);
> >  	} else {
> > -		clk = intel_lt_phy_calc_hdmi_port_clock(crtc_state);
> > +		clk = intel_lt_phy_calc_hdmi_port_clock(display, lt_state);
> >  	}
> > 
> >  	return clk;
> > @@ -2220,6 +2216,7 @@ void intel_lt_phy_pll_readout_hw_state(struct intel_encoder *encoder,
> >  				       const struct intel_crtc_state *crtc_state,
> >  				       struct intel_lt_phy_pll_state *pll_state)
> > {
> > +	struct intel_display *display = to_intel_display(encoder);
> >  	u8 owned_lane_mask;
> >  	u8 lane;
> >  	struct ref_tracker *wakeref;
> > @@ -2245,7 +2242,7 @@ void intel_lt_phy_pll_readout_hw_state(struct intel_encoder *encoder,
> >  	}
> > 
> >  	pll_state->clock =
> > -		intel_lt_phy_calc_port_clock(encoder, crtc_state);
> > +		intel_lt_phy_calc_port_clock(display,
> > +&crtc_state->dpll_hw_state.ltpll);
> 
> Readout_hw_state already has pll_state maybe you can directly pass
> that instead of what's inside crtc_state Since by this point we would
> have read and dumped everything inside pll_state anyways.

This is actually a fix of the existing code: crtc_state is the new state
of CTRC computed by the commit when intel_lt_phy_pll_readout_hw_state()
is called from intel_lt_phy_pll_state_verify(). That new CRTC state and
within that the new PLL state is what supposed to be verified, so
nothing from crtc_state should be used to derive the read-out pll_state.

In detail, for the verification intel_lt_phy_pll_readout_hw_state()
reads out the PLL state from the HW into pll_state passed to it, also
computing the corresponding PLL clock via
intel_lt_phy_calc_port_clock(). intel_lt_phy_pll_state_verify() verifies
then if the read-out PLL state in pll_state matches the state in
crtc_state->dpll_hw_state.ltpll. So computing pll_state->clock based on
crtc_state->dpll_hw_state.ltpll is incorrect based on the above (in the
existing code before this patchset) and as such the fix for it should be
a separate patch.

> Regards,
> Suraj Kandpal
> 
> >  	intel_lt_phy_transaction_end(encoder, wakeref);
> > }