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