> -----Original Message-----
> From: Kandpal, Suraj <[email protected]>
> Sent: Tuesday, 9 June 2026 13.04
> To: Kahola, Mika <[email protected]>; [email protected];
> [email protected]
> Subject: RE: [PATCH] drm/i915/display: Skip generic pipe dpll_hw_state
> compare on LT PHY
>
>
>
> > -----Original Message-----
> > From: Kahola, Mika <[email protected]>
> > Sent: Tuesday, June 9, 2026 2:40 PM
> > To: Kandpal, Suraj <[email protected]>;
> > [email protected];
> > [email protected]
> > Subject: RE: [PATCH] drm/i915/display: Skip generic pipe dpll_hw_state
> > compare on LT PHY
> >
> > > -----Original Message-----
> > > From: Kandpal, Suraj <[email protected]>
> > > Sent: Tuesday, 9 June 2026 11.33
> > > To: Kahola, Mika <[email protected]>;
> > > [email protected]; [email protected]
> > > Subject: RE: [PATCH] drm/i915/display: Skip generic pipe dpll_hw_state
> > > compare on LT PHY
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Kahola, Mika <[email protected]>
> > > > Sent: Tuesday, June 9, 2026 1:38 PM
> > > > To: Kandpal, Suraj <[email protected]>; intel-
> > > > [email protected]; [email protected]
> > > > Subject: RE: [PATCH] drm/i915/display: Skip generic pipe
> > > > dpll_hw_state compare on LT PHY
> > > >
> > > > > -----Original Message-----
> > > > > From: Kandpal, Suraj <[email protected]>
> > > > > Sent: Tuesday, 9 June 2026 10.51
> > > > > To: Kahola, Mika <[email protected]>;
> > > > > [email protected]; [email protected]
> > > > > Cc: Kahola, Mika <[email protected]>
> > > > > Subject: RE: [PATCH] drm/i915/display: Skip generic pipe
> > > > > dpll_hw_state compare on LT PHY
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Kandpal, Suraj
> > > > > > Sent: Tuesday, June 9, 2026 1:13 PM
> > > > > > To: 'Mika Kahola' <[email protected]>;
> > > > > > [email protected]; [email protected]
> > > > > > Cc: Kahola, Mika <[email protected]>
> > > > > > Subject: RE: [PATCH] drm/i915/display: Skip generic pipe
> > > > > > dpll_hw_state compare on LT PHY
> > > > > >
> > > > > > > Subject: [PATCH] drm/i915/display: Skip generic pipe
> > > > > > > dpll_hw_state compare on LT PHY
> > > > > > >
> > > > > > > LT PHY PLL readout is only partially reliable, and the LT PHY
> > > > > > > code already documents that only a subset of the state can be
> > > > > > > read back reliably after power gating.
> > > > > > >
> > > > > > > The generic pipe-state verification compares dpll_hw_state as
> > > > > > > part of intel_pipe_config_compare(), which can trigger
> > > > > > > false-positive "pipe state doesn't match!" warnings on LT PHY
> > platforms.
> > > > > > > DPLL-specific verification already exists via
> > > > > > > intel_dpll_state_verify().
> > > > > > >
> > > > > > > Skip the generic dpll_hw_state pipe-state compare on LT PHY
> > > > > > > platforms and rely on the dedicated DPLL verification path
> > > > > > > instead.
> > > > > > >
> > > > > > > Signed-off-by: Mika Kahola <[email protected]>
> > > > > > > ---
> > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 8 ++++++--
> > > > > > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > index 2fa10f858279..85ad2bc4963d 100644
> > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > @@ -5374,8 +5374,12 @@ intel_pipe_config_compare(const struct
> > > > > > > intel_crtc_state *current_config,
> > > > > > > if (display->dpll.mgr)
> > > > > > > PIPE_CONF_CHECK_P(intel_dpll);
> > > > > > >
> > > > > > > - /* FIXME convert everything over the dpll_mgr */
> > > > > > > - if (display->dpll.mgr || HAS_GMCH(display))
> > > > > > > + /*
> > > > > > > + * LT PHY PLL readout is only partially reliable and the PLL
> > > > > > > state
> > > > > > > + * is already verified via intel_dpll_state_verify(). Avoid
> > > > > > > false
> > > > > > > + * positives from the generic pipe state comparison.
> > > > > > > + */
> > > > > > > + if ((display->dpll.mgr || HAS_GMCH(display)) &&.
> > > > > > > !HAS_LT_PHY(display))
> > > > > > > PIPE_CONF_CHECK_PLL(dpll_hw_state);
> > > > > >
> > > > > > intel_lt_phy_pll_compare_hw_state only checks the reliable state
> > > > > > hence we don’t want to add this here config 0 and config 2 are
> > > > > > expected to be reliable
> > > > > >
> > > > >
> > > > > If you are seeing pipe state mismatch on either of these VDR
> > > > > registers then it’s a issue where PHY is not giving use correct
> > > > > value since these two
> > > > register must absolutely be retained by LT PHY.
> > > >
> > > > This error showed up with TBT monitor which yields empty states for
> > > > found and expected states. Since we check in
> > > > verify_single_dpll_state() function PLL state with .compare_hw_state
> > > > hook I think we wouldn't need to check the PLL state here.
> > >
> > > So .compare_hw_state would call intel_lt_phy_pll_compare_hw_state
> > > Which means for tbt mode it would return early anyways. So we again wont
> > require this check.
> > >
> > > We already have this bit of code as I mentioned above in the function
> > >
> > > if (a->tbt_mode || b->tbt_mode)
> > > return true;
> > >
> > It looks like we may be verifying PLL state twice in
> > intel_modeset_verify_crtc().
> >
> > verify_crtc_state() -> intel_pipe_config_compare() does
> > PIPE_CONF_CHECK_PLL(dpll_hw_state), which is a generic compare and does
> > not seem to account for the LT PHY/TBT special case.
>
> So PIPE_CONF_CHECK_PLL has the following code
> #define PIPE_CONF_CHECK_PLL(name) do { \
> if (!intel_dpll_compare_hw_state(display, ¤t_config->name, \
> &pipe_config->name)) { \
> ...
> } \
> } while (0)
>
> bool intel_dpll_compare_hw_state(struct intel_display *display,
> const struct intel_dpll_hw_state *a,
> const struct intel_dpll_hw_state *b)
> {
> if (display->dpll.mgr)
> return display->dpll.mgr->compare_hw_state(a, b);
> ...
> }
>
> And compare_hw_state calls
>
> static bool xe3plpd_compare_hw_state(const struct intel_dpll_hw_state *_a,
> const struct intel_dpll_hw_state *_b)
> {
> return intel_lt_phy_pll_compare_hw_state(&_a->ltpll, &_b->ltpll);
> }
>
> Hence it is not generic code it gets directed to LT PHY portion of code.
> If you are still seeing a mismatch means someone is not setting the tbt_mode.
> That would be the root cause of any mismatch we see
> which is
> why removing the CONF_CHECK_PLL() isn’t the correct way to go
>
> Regards,
> Suraj Kandpal
>
We end up calling intel_lt_phy_pll_compare_hw_state() from both
verify_crtc_state() and intel_dpll_state_verify(). The only difference that I
spotted was that in verify_single_dpll_state() we readout the HW state which
sets the tbt_mode. Maybe that gives a hint why the bug was fixed by this
proposed patch.
intel_modeset_verify_crtc
├── verify_crtc_state
│ └── intel_pipe_config_compare
│ └── PIPE_CONF_CHECK_PLL(dpll_hw_state)
│ └── intel_dpll_compare_hw_state
│ └── display->dpll.mgr->compare_hw_state
│ └── xe3plpd_compare_hw_state
│ └── intel_lt_phy_pll_compare_hw_state
└── intel_dpll_state_verify
└── verify_single_dpll_state
└── dpll_mgr->compare_hw_state
└── xe3plpd_compare_hw_state
└── intel_lt_phy_pll_compare_hw_state
-Mika-
> >
> > After that, intel_modeset_verify_crtc() also calls
> > intel_dpll_state_verify(), which
> > for LT PHY goes through the .compare_hw_state hook and therefore already
> > applies the platform-specific handling.
> >
> > Because of that, I wonder if the generic PLL state compare is really needed
> > in
> > this case. At the very least, it would seem that PIPE_CONF_CHECK_PLL()
> > should
> > also handle the TBT case explicitly and return early in that case, in the
> > same
> > way as the LT PHY-specific .compare_hw_state path does.
> >
> > -Mika-
> >
> > > Regards,
> > > Suraj Kandpal
> > >
> > > >
> > > > -Mika-
> > > >
> > > > >
> > > > > Regards,
> > > > > Suraj Kandpal
> > > > >
> > > > > > Regards,
> > > > > > Suraj Kandpal
> > > > > >
> > > > > > >
> > > > > > > PIPE_CONF_CHECK_X(dsi_pll.ctrl);
> > > > > > > --
> > > > > > > 2.43.0
