Re: [Intel-gfx] [PATCH 2/8] drm/i915: Use literal representation of cdclk tables
On Sat, Sep 07, 2019 at 09:05:00PM -0700, Matt Roper wrote:
> The bspec lays out legal cdclk frequencies, PLL ratios, and CD2X
> dividers in an easy-to-read table for most recent platforms. We've been
> translating the data from that table into platform-specific code logic,
> but it's easy to overlook an area we need to update when adding new
> cdclk values or enabling new platforms. Let's just add a form of the
> bspec table to the code and then adjust our functions to pull what they
> need directly out of the table.
>
> v2: Fix comparison when finding best cdclk.
>
> v3: Another logic fix for calc_cdclk.
>
> Cc: Ville Syrjälä
> Signed-off-by: Matt Roper
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 318 -
> drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +
> drivers/gpu/drm/i915/i915_drv.h| 4 +
> 3 files changed, 125 insertions(+), 205 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index e07de3b84cec..ca64143b6d7e 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1161,28 +1161,96 @@ static void skl_uninit_cdclk(struct drm_i915_private
> *dev_priv)
> skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> -static int bxt_calc_cdclk(int min_cdclk)
> -{
> - if (min_cdclk > 576000)
> - return 624000;
> - else if (min_cdclk > 384000)
> - return 576000;
> - else if (min_cdclk > 288000)
> - return 384000;
> - else if (min_cdclk > 144000)
> - return 288000;
> - else
> - return 144000;
> +static const struct intel_cdclk_vals bxt_cdclk_table[] = {
> + { 144000, 8, 60 },
> + { 288000, 4, 60 },
> + { 384000, 3, 60 },
> + { 576000, 2, 60 },
> + { 624000, 2, 65 },
Named initializers might make this number soup digestible.
> +};
> +
> +static const struct intel_cdclk_vals glk_cdclk_table[] = {
> + { 79200, 8, 33 },
> + { 158400, 4, 33 },
> + { 316800, 2, 33 },
> +};
> +
> +static const struct intel_cdclk_vals cnl_cdclk_table[] = {
> + { 168000, 4, 35, 28 },
> + { 336000, 2, 35, 28 },
> + { 528000, 2, 55, 44 },
> +};
> +
> +static const struct intel_cdclk_vals icl_cdclk_table[] = {
> + { 172800, 2, 18, 0, 9 },
> + { 18, 2, 0, 15, 0 },
> + { 192000, 2, 20, 16, 10 },
> + { 307200, 2, 32, 0, 16 },
> + { 312000, 2, 0, 26, 0 },
> + { 324000, 4, 0, 54, 0 },
> + { 326400, 4, 68, 0, 34 },
> + { 552000, 2, 0, 46, 0 },
> + { 556800, 2, 58, 0, 29 },
> + { 648000, 2, 0, 54, 0 },
> + { 652800, 2, 68, 0, 34 },
> +};
> +
> +static int calc_cdclk(struct drm_i915_private *dev_priv,
> + int min_cdclk)
> +{
> + const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
> + unsigned int ref = dev_priv->cdclk.hw.ref;
> + int best_cdclk = 0;
> + int i;
> +
> + if (WARN_ON(ref != 19200 && ref != 24000 && ref != 38400))
> + ref = 19200;
Feels a bit like dead code.
> +
> + for (i = 0; i < dev_priv->cdclk.table_size; i++) {
> + if ((ref == 19200 && table[i].ratio_19 == 0) ||
> + (ref == 24000 && table[i].ratio_24 == 0) ||
> + (ref == 38400 && table[i].ratio_38 == 0))
> + continue;
> +
> + best_cdclk = table[i].cdclk;
> + if (table[i].cdclk >= min_cdclk)
> + return best_cdclk;
> + }
> +
> + WARN(1, "Cannot satisfy minimum cdclk %d\n", min_cdclk);
> + return best_cdclk;
Same for the best_cdclk thing. I'd probably just MISSING_CASE()
and return 0, or something.
> }
>
> -static int glk_calc_cdclk(int min_cdclk)
> +static int calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
We still use these things for bxt+ only, so would be nice if
the names kept reflecting that fact.
Though I suppose we should be able to extend this more
declarative approach backwards to older platforms as well.
> {
> - if (min_cdclk > 158400)
> - return 316800;
> - else if (min_cdclk > 79200)
> - return 158400;
> - else
> - return 79200;
> + const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
> + int ratio, best_ratio, i;
> +
> + if (cdclk == dev_priv->cdclk.hw.bypass)
> + return 0;
> +
> + for (i = 0; i < dev_priv->cdclk.table_size; i++) {
> + if (dev_priv->cdclk.hw.ref == 19200)
> + ratio = table[i].ratio_19;
> + else if (dev_priv->cdclk.hw.ref == 24000)
> + ratio = table[i].ratio_24;
> + else
> + ratio = table[i].ratio_38;
> +
> + if (ratio == 0)
> + continue;
> + else
> + best_ratio = ratio;
> +
> + if (table[i].cdclk == cdclk ||
[Intel-gfx] [PATCH 2/8] drm/i915: Use literal representation of cdclk tables
The bspec lays out legal cdclk frequencies, PLL ratios, and CD2X
dividers in an easy-to-read table for most recent platforms. We've been
translating the data from that table into platform-specific code logic,
but it's easy to overlook an area we need to update when adding new
cdclk values or enabling new platforms. Let's just add a form of the
bspec table to the code and then adjust our functions to pull what they
need directly out of the table.
v2: Fix comparison when finding best cdclk.
v3: Another logic fix for calc_cdclk.
Cc: Ville Syrjälä
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 318 -
drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +
drivers/gpu/drm/i915/i915_drv.h| 4 +
3 files changed, 125 insertions(+), 205 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index e07de3b84cec..ca64143b6d7e 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1161,28 +1161,96 @@ static void skl_uninit_cdclk(struct drm_i915_private
*dev_priv)
skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int bxt_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 576000)
- return 624000;
- else if (min_cdclk > 384000)
- return 576000;
- else if (min_cdclk > 288000)
- return 384000;
- else if (min_cdclk > 144000)
- return 288000;
- else
- return 144000;
+static const struct intel_cdclk_vals bxt_cdclk_table[] = {
+ { 144000, 8, 60 },
+ { 288000, 4, 60 },
+ { 384000, 3, 60 },
+ { 576000, 2, 60 },
+ { 624000, 2, 65 },
+};
+
+static const struct intel_cdclk_vals glk_cdclk_table[] = {
+ { 79200, 8, 33 },
+ { 158400, 4, 33 },
+ { 316800, 2, 33 },
+};
+
+static const struct intel_cdclk_vals cnl_cdclk_table[] = {
+ { 168000, 4, 35, 28 },
+ { 336000, 2, 35, 28 },
+ { 528000, 2, 55, 44 },
+};
+
+static const struct intel_cdclk_vals icl_cdclk_table[] = {
+ { 172800, 2, 18, 0, 9 },
+ { 18, 2, 0, 15, 0 },
+ { 192000, 2, 20, 16, 10 },
+ { 307200, 2, 32, 0, 16 },
+ { 312000, 2, 0, 26, 0 },
+ { 324000, 4, 0, 54, 0 },
+ { 326400, 4, 68, 0, 34 },
+ { 552000, 2, 0, 46, 0 },
+ { 556800, 2, 58, 0, 29 },
+ { 648000, 2, 0, 54, 0 },
+ { 652800, 2, 68, 0, 34 },
+};
+
+static int calc_cdclk(struct drm_i915_private *dev_priv,
+ int min_cdclk)
+{
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ unsigned int ref = dev_priv->cdclk.hw.ref;
+ int best_cdclk = 0;
+ int i;
+
+ if (WARN_ON(ref != 19200 && ref != 24000 && ref != 38400))
+ ref = 19200;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if ((ref == 19200 && table[i].ratio_19 == 0) ||
+ (ref == 24000 && table[i].ratio_24 == 0) ||
+ (ref == 38400 && table[i].ratio_38 == 0))
+ continue;
+
+ best_cdclk = table[i].cdclk;
+ if (table[i].cdclk >= min_cdclk)
+ return best_cdclk;
+ }
+
+ WARN(1, "Cannot satisfy minimum cdclk %d\n", min_cdclk);
+ return best_cdclk;
}
-static int glk_calc_cdclk(int min_cdclk)
+static int calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
{
- if (min_cdclk > 158400)
- return 316800;
- else if (min_cdclk > 79200)
- return 158400;
- else
- return 79200;
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ int ratio, best_ratio, i;
+
+ if (cdclk == dev_priv->cdclk.hw.bypass)
+ return 0;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (dev_priv->cdclk.hw.ref == 19200)
+ ratio = table[i].ratio_19;
+ else if (dev_priv->cdclk.hw.ref == 24000)
+ ratio = table[i].ratio_24;
+ else
+ ratio = table[i].ratio_38;
+
+ if (ratio == 0)
+ continue;
+ else
+ best_ratio = ratio;
+
+ if (table[i].cdclk == cdclk ||
+ WARN_ON(table[i].cdclk > cdclk))
+ return dev_priv->cdclk.hw.ref * ratio;
+ }
+
+ WARN(1, "cdclk %d not valid for refclk %d\n",
+cdclk, dev_priv->cdclk.hw.ref);
+
+ return dev_priv->cdclk.hw.ref * best_ratio;
}
static u8 bxt_calc_voltage_level(int cdclk)
@@ -1220,52 +1288,6 @@ static u8 ehl_calc_voltage_level(int cdclk)
return 0;
}
-static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
-
[Intel-gfx] [PATCH 2/8] drm/i915: Use literal representation of cdclk tables
The bspec lays out legal cdclk frequencies, PLL ratios, and CD2X
dividers in an easy-to-read table for most recent platforms. We've been
translating the data from that table into platform-specific code logic,
but it's easy to overlook an area we need to update when adding new
cdclk values or enabling new platforms. Let's just add a form of the
bspec table to the code and then adjust our functions to pull what they
need directly out of the table.
v2: Fix comparison when finding best cdclk.
Cc: Ville Syrjälä
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 318 -
drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +
drivers/gpu/drm/i915/i915_drv.h| 4 +
3 files changed, 125 insertions(+), 205 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index e07de3b84cec..90efc82b8a94 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1161,28 +1161,96 @@ static void skl_uninit_cdclk(struct drm_i915_private
*dev_priv)
skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int bxt_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 576000)
- return 624000;
- else if (min_cdclk > 384000)
- return 576000;
- else if (min_cdclk > 288000)
- return 384000;
- else if (min_cdclk > 144000)
- return 288000;
- else
- return 144000;
+static const struct intel_cdclk_vals bxt_cdclk_table[] = {
+ { 144000, 8, 60 },
+ { 288000, 4, 60 },
+ { 384000, 3, 60 },
+ { 576000, 2, 60 },
+ { 624000, 2, 65 },
+};
+
+static const struct intel_cdclk_vals glk_cdclk_table[] = {
+ { 79200, 8, 33 },
+ { 158400, 4, 33 },
+ { 316800, 2, 33 },
+};
+
+static const struct intel_cdclk_vals cnl_cdclk_table[] = {
+ { 168000, 4, 35, 28 },
+ { 336000, 2, 35, 28 },
+ { 528000, 2, 55, 44 },
+};
+
+static const struct intel_cdclk_vals icl_cdclk_table[] = {
+ { 172800, 2, 18, 0, 9 },
+ { 18, 2, 0, 15, 0 },
+ { 192000, 2, 20, 16, 10 },
+ { 307200, 2, 32, 0, 16 },
+ { 312000, 2, 0, 26, 0 },
+ { 324000, 4, 0, 54, 0 },
+ { 326400, 4, 68, 0, 34 },
+ { 552000, 2, 0, 46, 0 },
+ { 556800, 2, 58, 0, 29 },
+ { 648000, 2, 0, 54, 0 },
+ { 652800, 2, 68, 0, 34 },
+};
+
+static int calc_cdclk(struct drm_i915_private *dev_priv,
+ int min_cdclk)
+{
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ unsigned int ref = dev_priv->cdclk.hw.ref;
+ int best_cdclk = 0;
+ int i;
+
+ if (WARN_ON(ref != 19200 && ref != 24000 && ref != 38400))
+ ref = 19200;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (ref == 19200 && table[i].ratio_19 == 0 ||
+ ref == 24000 && table[i].ratio_24 == 0 ||
+ ref == 38400 && table[i].ratio_38 == 0)
+ continue;
+
+ best_cdclk = table[i].cdclk;
+ if (table[i].cdclk >= min_cdclk)
+ return best_cdclk;
+ }
+
+ WARN(1, "Cannot satisfy minimum cdclk %d\n", min_cdclk);
+ return best_cdclk;
}
-static int glk_calc_cdclk(int min_cdclk)
+static int calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
{
- if (min_cdclk > 158400)
- return 316800;
- else if (min_cdclk > 79200)
- return 158400;
- else
- return 79200;
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ int ratio, best_ratio, i;
+
+ if (cdclk == dev_priv->cdclk.hw.bypass)
+ return 0;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (dev_priv->cdclk.hw.ref == 19200)
+ ratio = table[i].ratio_19;
+ else if (dev_priv->cdclk.hw.ref == 24000)
+ ratio = table[i].ratio_24;
+ else
+ ratio = table[i].ratio_38;
+
+ if (ratio == 0)
+ continue;
+ else
+ best_ratio = ratio;
+
+ if (table[i].cdclk == cdclk ||
+ WARN_ON(table[i].cdclk > cdclk))
+ return dev_priv->cdclk.hw.ref * ratio;
+ }
+
+ WARN(1, "cdclk %d not valid for refclk %d\n",
+cdclk, dev_priv->cdclk.hw.ref);
+
+ return dev_priv->cdclk.hw.ref * best_ratio;
}
static u8 bxt_calc_voltage_level(int cdclk)
@@ -1220,52 +1288,6 @@ static u8 ehl_calc_voltage_level(int cdclk)
return 0;
}
-static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdc
[Intel-gfx] [PATCH 2/8] drm/i915: Use literal representation of cdclk tables
The bspec lays out legal cdclk frequencies, PLL ratios, and CD2X
dividers in an easy-to-read table for most recent platforms. We've been
translating the data from that table into platform-specific code logic,
but it's easy to overlook an area we need to update when adding new
cdclk values or enabling new platforms. Let's just add a form of the
bspec table to the code and then adjust our functions to pull what they
need directly out of the table.
Cc: Ville Syrjälä
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 319 -
drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +
drivers/gpu/drm/i915/i915_drv.h| 4 +
3 files changed, 126 insertions(+), 205 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
b/drivers/gpu/drm/i915/display/intel_cdclk.c
index e07de3b84cec..8ac31f8775f0 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1161,28 +1161,97 @@ static void skl_uninit_cdclk(struct drm_i915_private
*dev_priv)
skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int bxt_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 576000)
- return 624000;
- else if (min_cdclk > 384000)
- return 576000;
- else if (min_cdclk > 288000)
- return 384000;
- else if (min_cdclk > 144000)
- return 288000;
- else
- return 144000;
+static const struct intel_cdclk_vals bxt_cdclk_table[] = {
+ { 144000, 8, 60 },
+ { 288000, 4, 60 },
+ { 384000, 3, 60 },
+ { 576000, 2, 60 },
+ { 624000, 2, 65 },
+};
+
+static const struct intel_cdclk_vals glk_cdclk_table[] = {
+ { 79200, 8, 33 },
+ { 158400, 4, 33 },
+ { 316800, 2, 33 },
+};
+
+static const struct intel_cdclk_vals cnl_cdclk_table[] = {
+ { 168000, 4, 35, 28 },
+ { 336000, 2, 35, 28 },
+ { 528000, 2, 55, 44 },
+};
+
+static const struct intel_cdclk_vals icl_cdclk_table[] = {
+ { 172800, 2, 18, 0, 9 },
+ { 18, 2, 0, 15, 0 },
+ { 192000, 2, 20, 16, 10 },
+ { 307200, 2, 32, 0, 16 },
+ { 312000, 2, 0, 26, 0 },
+ { 324000, 4, 0, 54, 0 },
+ { 326400, 4, 68, 0, 34 },
+ { 552000, 2, 0, 46, 0 },
+ { 556800, 2, 58, 0, 29 },
+ { 648000, 2, 0, 54, 0 },
+ { 652800, 2, 68, 0, 34 },
+};
+
+static int calc_cdclk(struct drm_i915_private *dev_priv,
+ int min_cdclk)
+{
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ unsigned int ref = dev_priv->cdclk.hw.ref;
+ int best_cdclk = 0;
+ int i;
+
+ if (WARN_ON(ref != 19200 && ref != 24000 && ref != 38400))
+ ref = 19200;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (ref == 19200 && table[i].ratio_19 != 0)
+ best_cdclk = table[i].cdclk;
+ else if (ref == 24000 && table[i].ratio_24 != 0)
+ best_cdclk = table[i].cdclk;
+ else if (ref == 38400 && table[i].ratio_38 != 0)
+ best_cdclk = table[i].cdclk;
+
+ if (table[i].cdclk < min_cdclk)
+ return best_cdclk;
+ }
+
+ WARN(1, "Cannot satisfy minimum cdclk %d\n", min_cdclk);
+ return best_cdclk;
}
-static int glk_calc_cdclk(int min_cdclk)
+static int calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
{
- if (min_cdclk > 158400)
- return 316800;
- else if (min_cdclk > 79200)
- return 158400;
- else
- return 79200;
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ int ratio, best_ratio, i;
+
+ if (cdclk == dev_priv->cdclk.hw.bypass)
+ return 0;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (dev_priv->cdclk.hw.ref == 19200)
+ ratio = table[i].ratio_19;
+ else if (dev_priv->cdclk.hw.ref == 24000)
+ ratio = table[i].ratio_24;
+ else
+ ratio = table[i].ratio_38;
+
+ if (ratio == 0)
+ continue;
+ else
+ best_ratio = ratio;
+
+ if (table[i].cdclk == cdclk ||
+ WARN_ON(table[i].cdclk > cdclk))
+ return dev_priv->cdclk.hw.ref * ratio;
+ }
+
+ WARN(1, "cdclk %d not valid for refclk %d\n",
+cdclk, dev_priv->cdclk.hw.ref);
+
+ return dev_priv->cdclk.hw.ref * best_ratio;
}
static u8 bxt_calc_voltage_level(int cdclk)
@@ -1220,52 +1289,6 @@ static u8 ehl_calc_voltage_level(int cdclk)
return 0;
}
-static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
-
