> 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.
Either way
Reviewed-by: Suraj Kandpal <[email protected]>
>
> > something like this.
> >
> > Regards,
> > Suraj Kandpal
> >
> > > +
> > > + return err;
> > > }
> > >
> > > int intel_cx0pll_calc_state(struct intel_crtc_state *crtc_state,
> > > --
> > > 2.34.1
