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

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

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.

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