> -----Original Message-----
> From: Kandpal, Suraj <[email protected]>
> Sent: Tuesday, 18 November 2025 6.21
> To: Kahola, Mika <[email protected]>; [email protected]; 
> [email protected]
> Subject: RE: [PATCH v2 21/32] drm/i915/cx0: Add MTL+ .update_active_dpll hook
> 
> > Subject: [PATCH v2 21/32] drm/i915/cx0: Add MTL+ .update_active_dpll
> > hook
> >
> > Add .update_active_dpll function pointer to support dpll framework.
> > Reuse ICL function pointer.
> >
> > v2: Add check for !HAS_LT_PHY (Suraj)
> 
> I did not comment asking for this change also brings some questions in my 
> mind here

Yes that's right. Your comment was for the last patch of the series but I 
believe that we should add this check on this patch as this patch adds 
.update_active_dpll hook. However, this check caused a regression on mtl so 
this patch is about to be updated. I hold your r-b for version 1 but the 
upcoming third version will need a review.

-Mika-

> 
> >
> > Signed-off-by: Mika Kahola <[email protected]>
> > Reviewed-by: Suraj Kandpal <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c      | 3 +++
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 1 +
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 002ccd47856d..6b43d326e50c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -3671,6 +3671,9 @@ void intel_ddi_update_active_dpll(struct
> > intel_atomic_state *state,
> >     if (DISPLAY_VER(display) >= 14 || !intel_encoder_is_tc(encoder))
> 
> So this check here will never let you call dpll_mgr->update_active_dpll hook 
> So do you not what to remove it.
> If the answer is you want to keep this check then you do not need 
> (!HAS_LT_PHY) If the answer is you need to get this
> removed only then does it make sense to have this check And the check should 
> be return if (HAS_LT_PHY())
> 
> Regards,
> Suraj Kandpal
> 
> >             return;
> >
> > +   if (!HAS_LT_PHY(display))
> > +           return;
> > +
> >     for_each_intel_crtc_in_pipe_mask(display->drm, pipe_crtc,
> >
> > intel_crtc_joined_pipe_mask(crtc_state))
> >             intel_dpll_update_active(state, pipe_crtc, encoder); diff --git
> > a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index c45f18201ee8..e6dd6f1123d6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -4449,6 +4449,7 @@ static const struct intel_dpll_mgr mtl_pll_mgr = {
> >     .compute_dplls = mtl_compute_dplls,
> >     .get_dplls = mtl_get_dplls,
> >     .put_dplls = icl_put_dplls,
> > +   .update_active_dpll = icl_update_active_dpll,
> >  };
> >
> >  /**
> > --
> > 2.34.1

Reply via email to