Re: [CI 04/32] drm/i915/display: Sanitize calculating C20 PLL state from tables

[Imre Deak <imre.deak@intel.com>]

On Wed, Nov 12, 2025 at 06:10:36AM +0200, Suraj Kandpal wrote:
> > Subject: Re: [CI 04/32] drm/i915/display: Sanitize calculating C20 PLL state from tables
> > 
> > On Tue, Nov 11, 2025 at 07:36:47AM +0200, Suraj Kandpal wrote:
> > > > + [...]
> > > > +static int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
> > > > +				   struct intel_encoder *encoder) {
> > > > +	int err = -ENOENT;
> > > > +
> > > > +	crtc_state->dpll_hw_state.cx0pll.use_c10 = false;
> > > > +
> > > > +	/* try computed C20 HDMI tables before using consolidated tables */
> > > > +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> > > > +		/* TODO: Update SSC state for HDMI as well */
> > > > +		err = intel_c20_compute_hdmi_tmds_pll(crtc_state);
> > > > +
> > > > +	if (err)
> > > > +		err = intel_c20pll_calc_state_from_table(crtc_state, encoder);
> > >
> > > So this is something I have been meaning to fix we should really be
> > > using the HDMI tables already defined. Computing them ourselves, that
> > > should be reserved for only when we do not have any HDMI table for the
> > > said port clock available.
> > 
> > > Also if we use the computed tables directly that means we never end up
> > > using the defined tables.
> > >
> > > SO the flow here should be
> > >
> > > err = intel_c20pll_calc_state_from_table(crtc_state, encoder);
> > >
> > > if (err && intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)))
> > > 	err = intel_c20_compute_hdmi_tmds_pll(crtc_state);
> > 
> > This patch is not meant to change the logic, it simply wants to make the logic
> > clearer to the reader. What you suggest should be a separate patch
> 
> I am fine with that do you want to add that as a part of this series
> or should I send a separate Patch fixing this.

I think that change is not in the scope of this patchset, so it would be
better if you could follow up with it separately.

> Either way
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

Thanks.

> > 
> > > something like this.
> > >
> > > Regards,
> > > Suraj Kandpal
> > >
> > > > +
> > > > +	return err;
> > > >  }
> > > >
> > > >  int intel_cx0pll_calc_state(struct intel_crtc_state *crtc_state,
> > > > --
> > > > 2.34.1