> -----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
>
> 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
