> -----Original Message-----
> From: Karas, Krzysztof <[email protected]>
> Sent: Monday, 8 December 2025 9.56
> To: Kahola, Mika <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH] drm/i915/cx0: Unify naming for C20 pll tables
> 
> Hi,
> 
> nice job noticing the discrepancy in naming. I think to get the naming issue 
> fully resolved, you'd need to also include changes
> to some other names in that file:
> 
> :~/kernel/drm-tip$ rg intel_c10_pll_
> drivers/gpu/drm/i915/display/intel_cx0_phy.c
> 2288:static void intel_c10_pll_program(struct intel_display *display,
> 3261:           intel_c10_pll_program(display, encoder, &pll_state->c10);
> 
> :~/kernel/drm-tip$ rg intel_c20_pll_
> drivers/gpu/drm/i915/display/intel_cx0_phy.c
> 2459:intel_c20_pll_tables_get(const struct intel_crtc_state *crtc_state, 
> 2627:intel_c20_pll_find_table(const struct
> intel_crtc_state *crtc_state,
> 2633:   tables = intel_c20_pll_tables_get(crtc_state, encoder);
> 2650:   table = intel_c20_pll_find_table(crtc_state, encoder);
> 2881:static void intel_c20_pll_program(struct intel_display *display,
> 3263:           intel_c20_pll_program(display, encoder, &pll_state->c20);
> 
> otherwise you'd be left with intel_c20pll_tables_get() and 
> intel_c20_pll_find_table(), so the names with *_c20_pll_* would
> be mismatched.

I didn't include these c10_pll* and c20_pll* fixes here as these both cases are 
named similarly. However, you're are right that we should follow one style what 
comes to naming.

I will follow up on this and drop the underscore from *_pll*.

Thanks for the review!

-Mika-

> 
> On 2025-12-05 at 13:52:03 +0200, Mika Kahola wrote:
> > To fetch pll divider tables a function name for C20 is
> > intel_c20_pll_tables_get() but for C10 the similar function is called
> > intel_c10pll_tables_get(). Rename
> > intel_c20_pll_tables_get() to intel_c20pll_tables_get() to be more
> > consistent.
> >
> > Signed-off-by: Mika Kahola <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index 27be2a490297..ef69e8762b90 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -2456,8 +2456,8 @@ static int intel_c20_compute_hdmi_tmds_pll(const
> > struct intel_crtc_state *crtc_s  }
> >
> >  static const struct intel_c20pll_state * const *
> > -intel_c20_pll_tables_get(const struct intel_crtc_state *crtc_state,
> > -                    struct intel_encoder *encoder)
> > +intel_c20pll_tables_get(const struct intel_crtc_state *crtc_state,
> > +                   struct intel_encoder *encoder)
> >  {
> >     struct intel_display *display = to_intel_display(crtc_state);
> >
> > @@ -2630,7 +2630,7 @@ intel_c20_pll_find_table(const struct 
> > intel_crtc_state *crtc_state,
> >     const struct intel_c20pll_state * const *tables;
> >     int i;
> >
> > -   tables = intel_c20_pll_tables_get(crtc_state, encoder);
> > +   tables = intel_c20pll_tables_get(crtc_state, encoder);
> >     if (!tables)
> >             return NULL;
> >
> > --
> > 2.34.1
> >
> 
> --
> Best Regards,
> Krzysztof

Reply via email to