diff mbox series

[13/13] drm/i915: Remove the fragile array index -> link rate mapping

Message ID 20190207173230.22368-13-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [01/13] drm/i915: Don't pass crtc to intel_find_shared_dpll() | expand

Commit Message

Ville Syrjälä Feb. 7, 2019, 5:32 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than try to maintain some magic relationship between the link
rates and the index into the wrpll params array let's just store
the link rate in the array itself. Much less fragile.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 136 +++++++++++++-------------
 1 file changed, 68 insertions(+), 68 deletions(-)

Comments

Lucas De Marchi Feb. 7, 2019, 11:11 p.m. UTC | #1
On Thu, Feb 7, 2019 at 9:33 AM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rather than try to maintain some magic relationship between the link
> rates and the index into the wrpll params array let's just store
> the link rate in the array itself. Much less fragile.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 136 +++++++++++++-------------
>  1 file changed, 68 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 78dd28aa123c..9aacd19aede1 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2417,47 +2417,69 @@ static const struct intel_dpll_mgr cnl_pll_mgr = {
>         .dump_hw_state = cnl_dump_hw_state,
>  };
>
> +struct icl_combo_pll_params {
> +       int clock;

isn't clock confusing here since we have the reference clock and here
is the link rate?

> +       struct skl_wrpll_params wrpll;

humn... since we will look for the clock to lookup the entry, we could
go one step further and
join the 2 tables:

struct icl_combo_pll_params {
       int clock;
       struct skl_wrpll_params wrpll[2];
};

> +};
> +
>  /*
>   * These values alrea already adjusted: they're the bits we write to the
>   * registers, not the logical values.
>   */
> -static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [0]: 5.4 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [1]: 2.7 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [2]: 1.62 */
> -         .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [3]: 3.24 */
> -         .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x168, .dco_fraction = 0x0000,         /* [4]: 2.16 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
> -       { .dco_integer = 0x168, .dco_fraction = 0x0000,         /* [5]: 4.32 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x195, .dco_fraction = 0x0000,         /* [6]: 6.48 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [7]: 8.1 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> +static const struct icl_combo_pll_params icl_dp_combo_pll_24MHz_values[] = {
> +       { 540000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [0]: 5.4 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },

I'm almost always for designated initializer, but in this table that
comes directly from spec, wouldn't
be easier  to read by skipping the initializer and just aligning the
numbers? IMO the lines below seem
easier to read:

 static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
         /* clock, dco_int,  dco_frac, kdiv, qdiv_mode, qdiv_ratio */
         { 5400,  {  0x151,    0x4000,    1,        0,          0 } },
...
}

Lucas De Marchi

> +       { 270000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [1]: 2.7 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 162000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [2]: 1.62 */
> +           .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 324000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [3]: 3.24 */
> +           .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 216000,
> +         { .dco_integer = 0x168, .dco_fraction = 0x0000,               /* [4]: 2.16 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> +       { 432000,
> +         { .dco_integer = 0x168, .dco_fraction = 0x0000,               /* [5]: 4.32 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 648000,
> +         { .dco_integer = 0x195, .dco_fraction = 0x0000,               /* [6]: 6.48 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 810000,
> +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [7]: 8.1 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
>  };
>
> +
>  /* Also used for 38.4 MHz values. */
> -static const struct skl_wrpll_params icl_dp_combo_pll_19_2MHz_values[] = {
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [0]: 5.4 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [1]: 2.7 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [2]: 1.62 */
> -         .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [3]: 3.24 */
> -         .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1C2, .dco_fraction = 0x0000,         /* [4]: 2.16 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
> -       { .dco_integer = 0x1C2, .dco_fraction = 0x0000,         /* [5]: 4.32 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1FA, .dco_fraction = 0x2000,         /* [6]: 6.48 */
> -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [7]: 8.1 */
> -         .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> +static const struct icl_combo_pll_params icl_dp_combo_pll_19_2MHz_values[] = {
> +       { 540000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [0]: 5.4 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 270000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [1]: 2.7 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 162000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [2]: 1.62 */
> +           .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 324000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [3]: 3.24 */
> +           .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 216000,
> +         { .dco_integer = 0x1C2, .dco_fraction = 0x0000,               /* [4]: 2.16 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> +       { 432000,
> +         { .dco_integer = 0x1C2, .dco_fraction = 0x0000,               /* [5]: 4.32 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 648000,
> +         { .dco_integer = 0x1FA, .dco_fraction = 0x2000,               /* [6]: 6.48 */
> +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> +       { 810000,
> +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [7]: 8.1 */
> +           .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
>  };
>
>  static const struct skl_wrpll_params icl_tbt_pll_24MHz_values = {
> @@ -2474,44 +2496,22 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
>                                   struct skl_wrpll_params *pll_params)
>  {
>         struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> -       const struct skl_wrpll_params *params;
> +       const struct icl_combo_pll_params *params =
> +               dev_priv->cdclk.hw.ref == 24000 ?
> +               icl_dp_combo_pll_24MHz_values :
> +               icl_dp_combo_pll_19_2MHz_values;
>         int clock = crtc_state->port_clock;
> +       int i;
>
> -       params = dev_priv->cdclk.hw.ref == 24000 ?
> -                       icl_dp_combo_pll_24MHz_values :
> -                       icl_dp_combo_pll_19_2MHz_values;
> -
> -       switch (clock) {
> -       case 540000:
> -               *pll_params = params[0];
> -               break;
> -       case 270000:
> -               *pll_params = params[1];
> -               break;
> -       case 162000:
> -               *pll_params = params[2];
> -               break;
> -       case 324000:
> -               *pll_params = params[3];
> -               break;
> -       case 216000:
> -               *pll_params = params[4];
> -               break;
> -       case 432000:
> -               *pll_params = params[5];
> -               break;
> -       case 648000:
> -               *pll_params = params[6];
> -               break;
> -       case 810000:
> -               *pll_params = params[7];
> -               break;
> -       default:
> -               MISSING_CASE(clock);
> -               return false;
> +       for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
> +               if (clock == params[i].clock) {
> +                       *pll_params = params[i].wrpll;
> +                       return true;
> +               }
>         }
>
> -       return true;
> +       MISSING_CASE(clock);
> +       return false;
>  }
>
>  static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
> --
> 2.19.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Feb. 8, 2019, 12:24 p.m. UTC | #2
On Thu, Feb 07, 2019 at 03:11:59PM -0800, Lucas De Marchi wrote:
> On Thu, Feb 7, 2019 at 9:33 AM Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Rather than try to maintain some magic relationship between the link
> > rates and the index into the wrpll params array let's just store
> > the link rate in the array itself. Much less fragile.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 136 +++++++++++++-------------
> >  1 file changed, 68 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index 78dd28aa123c..9aacd19aede1 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -2417,47 +2417,69 @@ static const struct intel_dpll_mgr cnl_pll_mgr = {
> >         .dump_hw_state = cnl_dump_hw_state,
> >  };
> >
> > +struct icl_combo_pll_params {
> > +       int clock;
> 
> isn't clock confusing here since we have the reference clock and here
> is the link rate?

This is PLL code. There is no such thing as link rate in a PLL.

> 
> > +       struct skl_wrpll_params wrpll;
> 
> humn... since we will look for the clock to lookup the entry, we could
> go one step further and
> join the 2 tables:
> 
> struct icl_combo_pll_params {
>        int clock;
>        struct skl_wrpll_params wrpll[2];
> };

That would add another magic array index. Which wouldn't be consistent
with the tbt pll params. We'd have to add some kind of enum for the
index and the convert all of these. I certainly don't care enough to
do that myself.

> 
> > +};
> > +
> >  /*
> >   * These values alrea already adjusted: they're the bits we write to the
> >   * registers, not the logical values.
> >   */
> > -static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [0]: 5.4 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [1]: 2.7 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [2]: 1.62 */
> > -         .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [3]: 3.24 */
> > -         .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x168, .dco_fraction = 0x0000,         /* [4]: 2.16 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
> > -       { .dco_integer = 0x168, .dco_fraction = 0x0000,         /* [5]: 4.32 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x195, .dco_fraction = 0x0000,         /* [6]: 6.48 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x151, .dco_fraction = 0x4000,         /* [7]: 8.1 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > +static const struct icl_combo_pll_params icl_dp_combo_pll_24MHz_values[] = {
> > +       { 540000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [0]: 5.4 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> 
> I'm almost always for designated initializer, but in this table that
> comes directly from spec, wouldn't
> be easier  to read by skipping the initializer and just aligning the
> numbers? IMO the lines below seem
> easier to read:
> 
>  static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
>          /* clock, dco_int,  dco_frac, kdiv, qdiv_mode, qdiv_ratio */
>          { 5400,  {  0x151,    0x4000,    1,        0,          0 } },
> ...
> }

With the comment it might be OK. But it'll certainly cause cscope to
miss stuff if you look for eg. qdiv_mode by name. So not sure if it's
better or worse tbh.

> 
> Lucas De Marchi
> 
> > +       { 270000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [1]: 2.7 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 162000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [2]: 1.62 */
> > +           .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 324000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [3]: 3.24 */
> > +           .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 216000,
> > +         { .dco_integer = 0x168, .dco_fraction = 0x0000,               /* [4]: 2.16 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> > +       { 432000,
> > +         { .dco_integer = 0x168, .dco_fraction = 0x0000,               /* [5]: 4.32 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 648000,
> > +         { .dco_integer = 0x195, .dco_fraction = 0x0000,               /* [6]: 6.48 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 810000,
> > +         { .dco_integer = 0x151, .dco_fraction = 0x4000,               /* [7]: 8.1 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> >  };
> >
> > +
> >  /* Also used for 38.4 MHz values. */
> > -static const struct skl_wrpll_params icl_dp_combo_pll_19_2MHz_values[] = {
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [0]: 5.4 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [1]: 2.7 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [2]: 1.62 */
> > -         .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [3]: 3.24 */
> > -         .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1C2, .dco_fraction = 0x0000,         /* [4]: 2.16 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
> > -       { .dco_integer = 0x1C2, .dco_fraction = 0x0000,         /* [5]: 4.32 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1FA, .dco_fraction = 0x2000,         /* [6]: 6.48 */
> > -         .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > -       { .dco_integer = 0x1A5, .dco_fraction = 0x7000,         /* [7]: 8.1 */
> > -         .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
> > +static const struct icl_combo_pll_params icl_dp_combo_pll_19_2MHz_values[] = {
> > +       { 540000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [0]: 5.4 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 270000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [1]: 2.7 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 162000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [2]: 1.62 */
> > +           .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 324000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [3]: 3.24 */
> > +           .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 216000,
> > +         { .dco_integer = 0x1C2, .dco_fraction = 0x0000,               /* [4]: 2.16 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
> > +       { 432000,
> > +         { .dco_integer = 0x1C2, .dco_fraction = 0x0000,               /* [5]: 4.32 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 648000,
> > +         { .dco_integer = 0x1FA, .dco_fraction = 0x2000,               /* [6]: 6.48 */
> > +           .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> > +       { 810000,
> > +         { .dco_integer = 0x1A5, .dco_fraction = 0x7000,               /* [7]: 8.1 */
> > +           .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
> >  };
> >
> >  static const struct skl_wrpll_params icl_tbt_pll_24MHz_values = {
> > @@ -2474,44 +2496,22 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
> >                                   struct skl_wrpll_params *pll_params)
> >  {
> >         struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > -       const struct skl_wrpll_params *params;
> > +       const struct icl_combo_pll_params *params =
> > +               dev_priv->cdclk.hw.ref == 24000 ?
> > +               icl_dp_combo_pll_24MHz_values :
> > +               icl_dp_combo_pll_19_2MHz_values;
> >         int clock = crtc_state->port_clock;
> > +       int i;
> >
> > -       params = dev_priv->cdclk.hw.ref == 24000 ?
> > -                       icl_dp_combo_pll_24MHz_values :
> > -                       icl_dp_combo_pll_19_2MHz_values;
> > -
> > -       switch (clock) {
> > -       case 540000:
> > -               *pll_params = params[0];
> > -               break;
> > -       case 270000:
> > -               *pll_params = params[1];
> > -               break;
> > -       case 162000:
> > -               *pll_params = params[2];
> > -               break;
> > -       case 324000:
> > -               *pll_params = params[3];
> > -               break;
> > -       case 216000:
> > -               *pll_params = params[4];
> > -               break;
> > -       case 432000:
> > -               *pll_params = params[5];
> > -               break;
> > -       case 648000:
> > -               *pll_params = params[6];
> > -               break;
> > -       case 810000:
> > -               *pll_params = params[7];
> > -               break;
> > -       default:
> > -               MISSING_CASE(clock);
> > -               return false;
> > +       for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
> > +               if (clock == params[i].clock) {
> > +                       *pll_params = params[i].wrpll;
> > +                       return true;
> > +               }
> >         }
> >
> > -       return true;
> > +       MISSING_CASE(clock);
> > +       return false;
> >  }
> >
> >  static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
> > --
> > 2.19.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Lucas De Marchi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 78dd28aa123c..9aacd19aede1 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2417,47 +2417,69 @@  static const struct intel_dpll_mgr cnl_pll_mgr = {
 	.dump_hw_state = cnl_dump_hw_state,
 };
 
+struct icl_combo_pll_params {
+	int clock;
+	struct skl_wrpll_params wrpll;
+};
+
 /*
  * These values alrea already adjusted: they're the bits we write to the
  * registers, not the logical values.
  */
-static const struct skl_wrpll_params icl_dp_combo_pll_24MHz_values[] = {
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [0]: 5.4 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [1]: 2.7 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [2]: 1.62 */
-	  .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [3]: 3.24 */
-	  .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x168, .dco_fraction = 0x0000,		/* [4]: 2.16 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
-	{ .dco_integer = 0x168, .dco_fraction = 0x0000,		/* [5]: 4.32 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x195, .dco_fraction = 0x0000,		/* [6]: 6.48 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [7]: 8.1 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
+static const struct icl_combo_pll_params icl_dp_combo_pll_24MHz_values[] = {
+	{ 540000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [0]: 5.4 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 270000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [1]: 2.7 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 162000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [2]: 1.62 */
+	    .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 324000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [3]: 3.24 */
+	    .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 216000,
+	  { .dco_integer = 0x168, .dco_fraction = 0x0000,		/* [4]: 2.16 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
+	{ 432000,
+	  { .dco_integer = 0x168, .dco_fraction = 0x0000,		/* [5]: 4.32 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 648000,
+	  { .dco_integer = 0x195, .dco_fraction = 0x0000,		/* [6]: 6.48 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 810000,
+	  { .dco_integer = 0x151, .dco_fraction = 0x4000,		/* [7]: 8.1 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
 };
 
+
 /* Also used for 38.4 MHz values. */
-static const struct skl_wrpll_params icl_dp_combo_pll_19_2MHz_values[] = {
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [0]: 5.4 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [1]: 2.7 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [2]: 1.62 */
-	  .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [3]: 3.24 */
-	  .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1C2, .dco_fraction = 0x0000,		/* [4]: 2.16 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2},
-	{ .dco_integer = 0x1C2, .dco_fraction = 0x0000,		/* [5]: 4.32 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1FA, .dco_fraction = 0x2000,		/* [6]: 6.48 */
-	  .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
-	{ .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [7]: 8.1 */
-	  .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0},
+static const struct icl_combo_pll_params icl_dp_combo_pll_19_2MHz_values[] = {
+	{ 540000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [0]: 5.4 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 270000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [1]: 2.7 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 162000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [2]: 1.62 */
+	    .pdiv = 0x4 /* 5 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 324000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [3]: 3.24 */
+	    .pdiv = 0x4 /* 5 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 216000,
+	  { .dco_integer = 0x1C2, .dco_fraction = 0x0000,		/* [4]: 2.16 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 1, .qdiv_ratio = 2, }, },
+	{ 432000,
+	  { .dco_integer = 0x1C2, .dco_fraction = 0x0000,		/* [5]: 4.32 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 2, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 648000,
+	  { .dco_integer = 0x1FA, .dco_fraction = 0x2000,		/* [6]: 6.48 */
+	    .pdiv = 0x2 /* 3 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
+	{ 810000,
+	  { .dco_integer = 0x1A5, .dco_fraction = 0x7000,		/* [7]: 8.1 */
+	    .pdiv = 0x1 /* 2 */, .kdiv = 1, .qdiv_mode = 0, .qdiv_ratio = 0, }, },
 };
 
 static const struct skl_wrpll_params icl_tbt_pll_24MHz_values = {
@@ -2474,44 +2496,22 @@  static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
 				  struct skl_wrpll_params *pll_params)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
-	const struct skl_wrpll_params *params;
+	const struct icl_combo_pll_params *params =
+		dev_priv->cdclk.hw.ref == 24000 ?
+		icl_dp_combo_pll_24MHz_values :
+		icl_dp_combo_pll_19_2MHz_values;
 	int clock = crtc_state->port_clock;
+	int i;
 
-	params = dev_priv->cdclk.hw.ref == 24000 ?
-			icl_dp_combo_pll_24MHz_values :
-			icl_dp_combo_pll_19_2MHz_values;
-
-	switch (clock) {
-	case 540000:
-		*pll_params = params[0];
-		break;
-	case 270000:
-		*pll_params = params[1];
-		break;
-	case 162000:
-		*pll_params = params[2];
-		break;
-	case 324000:
-		*pll_params = params[3];
-		break;
-	case 216000:
-		*pll_params = params[4];
-		break;
-	case 432000:
-		*pll_params = params[5];
-		break;
-	case 648000:
-		*pll_params = params[6];
-		break;
-	case 810000:
-		*pll_params = params[7];
-		break;
-	default:
-		MISSING_CASE(clock);
-		return false;
+	for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
+		if (clock == params[i].clock) {
+			*pll_params = params[i].wrpll;
+			return true;
+		}
 	}
 
-	return true;
+	MISSING_CASE(clock);
+	return false;
 }
 
 static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,