> -----Original Message----- > From: Deak, Imre <imre.deak@xxxxxxxxx> > Sent: Friday, May 3, 2024 2:00 PM > To: Kahola, Mika <mika.kahola@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] drm/i915/display: Calculate crtc clock rate based on PLL parameters > > On Thu, May 02, 2024 at 04:17:16PM +0300, Mika Kahola wrote: > > With HDMI monitors we bumped up a case where the crtc clock rate > > caused a mismatch on state verification. This was due to assumption > > that the SW clock rate from PLL structure would match the calculated > > counterpart from HW. This is not necessarily always the case and > > therefore we would actually need to recalculate the clock rate from SW > > PLL parameters. Then these SW and HW crtc clock rates can be compared > > with each other. > > > > The patch recalculates the crtc clock rate for SW state based on SW > > PLL parameters and compares the crtc clock rate calculated from the > > parameters found from the HW. > > > > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > index 8e3b13884bb8..89a195917179 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > > @@ -3078,9 +3078,10 @@ static void intel_c20pll_state_verify(const struct intel_crtc_state *state, > > const struct intel_c20pll_state *mpll_sw_state = &state->dpll_hw_state.cx0pll.c20; > > bool sw_use_mpllb = intel_c20phy_use_mpllb(mpll_sw_state); > > bool hw_use_mpllb = intel_c20phy_use_mpllb(mpll_hw_state); > > + int clock = intel_c20pll_calc_port_clock(encoder, mpll_sw_state); > > int i; > > > > - I915_STATE_WARN(i915, mpll_hw_state->clock != mpll_sw_state->clock, > > + I915_STATE_WARN(i915, mpll_hw_state->clock != clock, > > There is a corresponding check already in the encoder state checker, which is more approriate, since it compares the calculated > PLL clock against the - adjusted - crtc port clock and I think that's the only place all other platforms check this. In any case the > above check looks correct: > > Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx> Patch pushed. Thanks for the review! -Mika- > > > "[CRTC:%d:%s] mismatch in C20: Register CLOCK (expected %d, found %d)", > > crtc->base.base.id, crtc->base.name, > > mpll_sw_state->clock, mpll_hw_state->clock); > > -- > > 2.34.1 > >
