> Subject: Re: [PATCH v2 04/15] drm/i915/lt_phy: Drop LT PHY crtc_state for port
> calculation
>
> 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.
>
LGTM,
Reviewed-by: Suraj Kandpal <[email protected]>
> > Regards,
> > Suraj Kandpal
> >
> > > intel_lt_phy_transaction_end(encoder, wakeref); }
