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