| Message ID | 20220928190415.282665-2-anusha.srivatsa@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/display: Do both crawl and squash when changing cdclk | expand |
On Wed, Sep 28, 2022 at 12:04:15PM -0700, Anusha Srivatsa wrote: > As per bSpec MTL has 38.4 MHz Reference clock. > MTL does support squasher like DG2 but only for lower > frequencies. Change the has_cdclk_squasher() > helper to reflect this. > > bxt_get_cdclk() is not properly calculating HW clock for MTL, > because the squash formula is only prepared for DG2. > Apart from adding the cdclk table, align cdclk support with the > new cdclk_crawl_and_squash() introduced in previous patch. > > BSpec: 65243 > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 95 +++++++++++++++++++++- > 1 file changed, 93 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index f7bc1013b149..6271eed0d7cf 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1222,7 +1222,7 @@ static void skl_cdclk_uninit_hw(struct drm_i915_private *dev_priv) > > static bool has_cdclk_squasher(struct drm_i915_private *i915) > { > - return IS_DG2(i915); > + return DISPLAY_VER(i915) >= 14 || IS_DG2(i915); > } > > struct intel_cdclk_vals { > @@ -1350,6 +1350,16 @@ static const struct intel_cdclk_vals dg2_cdclk_table[] = { > {} > }; > > +static const struct intel_cdclk_vals mtl_cdclk_table[] = { > + { .refclk = 38400, .cdclk = 172800, .divider = 2, .ratio = 16, .waveform = 0xad5a }, > + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 16, .waveform = 0xb6b6 }, > + { .refclk = 38400, .cdclk = 307200, .divider = 2, .ratio = 16, .waveform = 0x0000 }, > + { .refclk = 38400, .cdclk = 480000, .divider = 2, .ratio = 25, .waveform = 0x0000 }, > + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29, .waveform = 0x0000 }, > + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34, .waveform = 0x0000 }, > + {} > +}; > + > static int bxt_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk) > { > const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; > @@ -1479,6 +1489,76 @@ static void bxt_de_pll_readout(struct drm_i915_private *dev_priv, > cdclk_config->vco = ratio * cdclk_config->ref; > } > > +static void mtl_get_cdclk(struct drm_i915_private *i915, > + struct intel_cdclk_config *cdclk_config) > +{ > + const struct intel_cdclk_vals *table = i915->display.cdclk.table; > + u32 squash_ctl, divider, waveform; > + int div, i, ratio; > + > + bxt_de_pll_readout(i915, cdclk_config); > + > + cdclk_config->bypass = cdclk_config->ref / 2; > + > + if (cdclk_config->vco == 0) { > + cdclk_config->cdclk = cdclk_config->bypass; > + goto out; > + } > + > + divider = intel_de_read(i915, CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK; > + switch (divider) { > + case BXT_CDCLK_CD2X_DIV_SEL_1: > + div = 2; > + break; > + case BXT_CDCLK_CD2X_DIV_SEL_1_5: > + div = 3; > + break; > + case BXT_CDCLK_CD2X_DIV_SEL_2: > + div = 4; > + break; > + case BXT_CDCLK_CD2X_DIV_SEL_4: > + div = 8; > + break; > + default: > + MISSING_CASE(divider); > + return; > + } > + > + squash_ctl = intel_de_read(i915, CDCLK_SQUASH_CTL); > + if (squash_ctl & CDCLK_SQUASH_ENABLE) > + waveform = squash_ctl & CDCLK_SQUASH_WAVEFORM_MASK; > + else > + waveform = 0; > + > + ratio = cdclk_config->vco / cdclk_config->ref; > + > + for (i = 0, cdclk_config->cdclk = 0; table[i].refclk; i++) { > + if (table[i].refclk != cdclk_config->ref) > + continue; > + > + if (table[i].divider != div) > + continue; > + > + if (table[i].waveform != waveform) > + continue; > + > + if (table[i].ratio != ratio) > + continue; > + > + cdclk_config->cdclk = table[i].cdclk; > + break; > + } NAK. Readout must not depend on these tables. Otherwise it's not proper readout and bugs can slip through. What is the supposed problem with the already existing code? > +out: > + /* > + * Can't read this out :( Let's assume it's > + * at least what the CDCLK frequency requires. > + */ > + cdclk_config->voltage_level = > + intel_cdclk_calc_voltage_level(i915, cdclk_config->cdclk); > +} > + > + > static void bxt_get_cdclk(struct drm_i915_private *dev_priv, > struct intel_cdclk_config *cdclk_config) > { > @@ -3138,6 +3218,13 @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv) > return freq; > } > > +static const struct intel_cdclk_funcs mtl_cdclk_funcs = { > + .get_cdclk = mtl_get_cdclk, > + .set_cdclk = bxt_set_cdclk, > + .modeset_calc_cdclk = bxt_modeset_calc_cdclk, > + .calc_voltage_level = tgl_calc_voltage_level, > +}; > + > static const struct intel_cdclk_funcs tgl_cdclk_funcs = { > .get_cdclk = bxt_get_cdclk, > .set_cdclk = bxt_set_cdclk, > @@ -3273,7 +3360,11 @@ static const struct intel_cdclk_funcs i830_cdclk_funcs = { > */ > void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) > { > - if (IS_DG2(dev_priv)) { > + > + if (IS_METEORLAKE(dev_priv)) { > + dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs; > + dev_priv->display.cdclk.table = mtl_cdclk_table; > + } else if (IS_DG2(dev_priv)) { > dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; > dev_priv->display.cdclk.table = dg2_cdclk_table; > } else if (IS_ALDERLAKE_P(dev_priv)) { > -- > 2.25.1
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Wednesday, September 28, 2022 12:24 PM > To: Srivatsa, Anusha <anusha.srivatsa@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/display: Add CDCLK Support for > MTL > > On Wed, Sep 28, 2022 at 12:04:15PM -0700, Anusha Srivatsa wrote: > > As per bSpec MTL has 38.4 MHz Reference clock. > > MTL does support squasher like DG2 but only for lower frequencies. > > Change the has_cdclk_squasher() helper to reflect this. > > > > bxt_get_cdclk() is not properly calculating HW clock for MTL, because > > the squash formula is only prepared for DG2. > > Apart from adding the cdclk table, align cdclk support with the new > > cdclk_crawl_and_squash() introduced in previous patch. > > > > BSpec: 65243 > > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_cdclk.c | 95 > > +++++++++++++++++++++- > > 1 file changed, 93 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c > > b/drivers/gpu/drm/i915/display/intel_cdclk.c > > index f7bc1013b149..6271eed0d7cf 100644 > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > > @@ -1222,7 +1222,7 @@ static void skl_cdclk_uninit_hw(struct > > drm_i915_private *dev_priv) > > > > static bool has_cdclk_squasher(struct drm_i915_private *i915) { > > - return IS_DG2(i915); > > + return DISPLAY_VER(i915) >= 14 || IS_DG2(i915); > > } > > > > struct intel_cdclk_vals { > > @@ -1350,6 +1350,16 @@ static const struct intel_cdclk_vals > dg2_cdclk_table[] = { > > {} > > }; > > > > +static const struct intel_cdclk_vals mtl_cdclk_table[] = { > > + { .refclk = 38400, .cdclk = 172800, .divider = 2, .ratio = 16, .waveform > = 0xad5a }, > > + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 16, .waveform > = 0xb6b6 }, > > + { .refclk = 38400, .cdclk = 307200, .divider = 2, .ratio = 16, .waveform > = 0x0000 }, > > + { .refclk = 38400, .cdclk = 480000, .divider = 2, .ratio = 25, .waveform > = 0x0000 }, > > + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29, .waveform > = 0x0000 }, > > + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34, .waveform > = 0x0000 }, > > + {} > > +}; > > + > > static int bxt_calc_cdclk(struct drm_i915_private *dev_priv, int > > min_cdclk) { > > const struct intel_cdclk_vals *table = > > dev_priv->display.cdclk.table; @@ -1479,6 +1489,76 @@ static void > bxt_de_pll_readout(struct drm_i915_private *dev_priv, > > cdclk_config->vco = ratio * cdclk_config->ref; } > > > > +static void mtl_get_cdclk(struct drm_i915_private *i915, > > + struct intel_cdclk_config *cdclk_config) { > > + const struct intel_cdclk_vals *table = i915->display.cdclk.table; > > + u32 squash_ctl, divider, waveform; > > + int div, i, ratio; > > + > > + bxt_de_pll_readout(i915, cdclk_config); > > + > > + cdclk_config->bypass = cdclk_config->ref / 2; > > + > > + if (cdclk_config->vco == 0) { > > + cdclk_config->cdclk = cdclk_config->bypass; > > + goto out; > > + } > > + > > + divider = intel_de_read(i915, CDCLK_CTL) & > BXT_CDCLK_CD2X_DIV_SEL_MASK; > > + switch (divider) { > > + case BXT_CDCLK_CD2X_DIV_SEL_1: > > + div = 2; > > + break; > > + case BXT_CDCLK_CD2X_DIV_SEL_1_5: > > + div = 3; > > + break; > > + case BXT_CDCLK_CD2X_DIV_SEL_2: > > + div = 4; > > + break; > > + case BXT_CDCLK_CD2X_DIV_SEL_4: > > + div = 8; > > + break; > > + default: > > + MISSING_CASE(divider); > > + return; > > + } > > + > > + squash_ctl = intel_de_read(i915, CDCLK_SQUASH_CTL); > > + if (squash_ctl & CDCLK_SQUASH_ENABLE) > > + waveform = squash_ctl & > CDCLK_SQUASH_WAVEFORM_MASK; > > + else > > + waveform = 0; > > + > > + ratio = cdclk_config->vco / cdclk_config->ref; > > + > > + for (i = 0, cdclk_config->cdclk = 0; table[i].refclk; i++) { > > + if (table[i].refclk != cdclk_config->ref) > > + continue; > > + > > + if (table[i].divider != div) > > + continue; > > + > > + if (table[i].waveform != waveform) > > + continue; > > + > > + if (table[i].ratio != ratio) > > + continue; > > + > > + cdclk_config->cdclk = table[i].cdclk; > > + break; > > + } > > NAK. Readout must not depend on these tables. Otherwise it's not proper > readout and bugs can slip through. What is the supposed problem with the > already existing code? IIRC we were getting wrong values with the existing mathematical calculations for cdclk. This way we force to pick an entry from the table. Anusha > > +out: > > + /* > > + * Can't read this out :( Let's assume it's > > + * at least what the CDCLK frequency requires. > > + */ > > + cdclk_config->voltage_level = > > + intel_cdclk_calc_voltage_level(i915, cdclk_config->cdclk); } > > + > > + > > static void bxt_get_cdclk(struct drm_i915_private *dev_priv, > > struct intel_cdclk_config *cdclk_config) { @@ - > 3138,6 +3218,13 > > @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv) > > return freq; > > } > > > > +static const struct intel_cdclk_funcs mtl_cdclk_funcs = { > > + .get_cdclk = mtl_get_cdclk, > > + .set_cdclk = bxt_set_cdclk, > > + .modeset_calc_cdclk = bxt_modeset_calc_cdclk, > > + .calc_voltage_level = tgl_calc_voltage_level, }; > > + > > static const struct intel_cdclk_funcs tgl_cdclk_funcs = { > > .get_cdclk = bxt_get_cdclk, > > .set_cdclk = bxt_set_cdclk, > > @@ -3273,7 +3360,11 @@ static const struct intel_cdclk_funcs > i830_cdclk_funcs = { > > */ > > void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) { > > - if (IS_DG2(dev_priv)) { > > + > > + if (IS_METEORLAKE(dev_priv)) { > > + dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs; > > + dev_priv->display.cdclk.table = mtl_cdclk_table; > > + } else if (IS_DG2(dev_priv)) { > > dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; > > dev_priv->display.cdclk.table = dg2_cdclk_table; > > } else if (IS_ALDERLAKE_P(dev_priv)) { > > -- > > 2.25.1 > > -- > Ville Syrjälä > Intel
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index f7bc1013b149..6271eed0d7cf 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -1222,7 +1222,7 @@ static void skl_cdclk_uninit_hw(struct drm_i915_private *dev_priv) static bool has_cdclk_squasher(struct drm_i915_private *i915) { - return IS_DG2(i915); + return DISPLAY_VER(i915) >= 14 || IS_DG2(i915); } struct intel_cdclk_vals { @@ -1350,6 +1350,16 @@ static const struct intel_cdclk_vals dg2_cdclk_table[] = { {} }; +static const struct intel_cdclk_vals mtl_cdclk_table[] = { + { .refclk = 38400, .cdclk = 172800, .divider = 2, .ratio = 16, .waveform = 0xad5a }, + { .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 16, .waveform = 0xb6b6 }, + { .refclk = 38400, .cdclk = 307200, .divider = 2, .ratio = 16, .waveform = 0x0000 }, + { .refclk = 38400, .cdclk = 480000, .divider = 2, .ratio = 25, .waveform = 0x0000 }, + { .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29, .waveform = 0x0000 }, + { .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34, .waveform = 0x0000 }, + {} +}; + static int bxt_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk) { const struct intel_cdclk_vals *table = dev_priv->display.cdclk.table; @@ -1479,6 +1489,76 @@ static void bxt_de_pll_readout(struct drm_i915_private *dev_priv, cdclk_config->vco = ratio * cdclk_config->ref; } +static void mtl_get_cdclk(struct drm_i915_private *i915, + struct intel_cdclk_config *cdclk_config) +{ + const struct intel_cdclk_vals *table = i915->display.cdclk.table; + u32 squash_ctl, divider, waveform; + int div, i, ratio; + + bxt_de_pll_readout(i915, cdclk_config); + + cdclk_config->bypass = cdclk_config->ref / 2; + + if (cdclk_config->vco == 0) { + cdclk_config->cdclk = cdclk_config->bypass; + goto out; + } + + divider = intel_de_read(i915, CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK; + switch (divider) { + case BXT_CDCLK_CD2X_DIV_SEL_1: + div = 2; + break; + case BXT_CDCLK_CD2X_DIV_SEL_1_5: + div = 3; + break; + case BXT_CDCLK_CD2X_DIV_SEL_2: + div = 4; + break; + case BXT_CDCLK_CD2X_DIV_SEL_4: + div = 8; + break; + default: + MISSING_CASE(divider); + return; + } + + squash_ctl = intel_de_read(i915, CDCLK_SQUASH_CTL); + if (squash_ctl & CDCLK_SQUASH_ENABLE) + waveform = squash_ctl & CDCLK_SQUASH_WAVEFORM_MASK; + else + waveform = 0; + + ratio = cdclk_config->vco / cdclk_config->ref; + + for (i = 0, cdclk_config->cdclk = 0; table[i].refclk; i++) { + if (table[i].refclk != cdclk_config->ref) + continue; + + if (table[i].divider != div) + continue; + + if (table[i].waveform != waveform) + continue; + + if (table[i].ratio != ratio) + continue; + + cdclk_config->cdclk = table[i].cdclk; + break; + } + +out: + /* + * Can't read this out :( Let's assume it's + * at least what the CDCLK frequency requires. + */ + cdclk_config->voltage_level = + intel_cdclk_calc_voltage_level(i915, cdclk_config->cdclk); +} + + static void bxt_get_cdclk(struct drm_i915_private *dev_priv, struct intel_cdclk_config *cdclk_config) { @@ -3138,6 +3218,13 @@ u32 intel_read_rawclk(struct drm_i915_private *dev_priv) return freq; } +static const struct intel_cdclk_funcs mtl_cdclk_funcs = { + .get_cdclk = mtl_get_cdclk, + .set_cdclk = bxt_set_cdclk, + .modeset_calc_cdclk = bxt_modeset_calc_cdclk, + .calc_voltage_level = tgl_calc_voltage_level, +}; + static const struct intel_cdclk_funcs tgl_cdclk_funcs = { .get_cdclk = bxt_get_cdclk, .set_cdclk = bxt_set_cdclk, @@ -3273,7 +3360,11 @@ static const struct intel_cdclk_funcs i830_cdclk_funcs = { */ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv) { - if (IS_DG2(dev_priv)) { + + if (IS_METEORLAKE(dev_priv)) { + dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs; + dev_priv->display.cdclk.table = mtl_cdclk_table; + } else if (IS_DG2(dev_priv)) { dev_priv->display.funcs.cdclk = &tgl_cdclk_funcs; dev_priv->display.cdclk.table = dg2_cdclk_table; } else if (IS_ALDERLAKE_P(dev_priv)) {
As per bSpec MTL has 38.4 MHz Reference clock. MTL does support squasher like DG2 but only for lower frequencies. Change the has_cdclk_squasher() helper to reflect this. bxt_get_cdclk() is not properly calculating HW clock for MTL, because the squash formula is only prepared for DG2. Apart from adding the cdclk table, align cdclk support with the new cdclk_crawl_and_squash() introduced in previous patch. BSpec: 65243 Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com> --- drivers/gpu/drm/i915/display/intel_cdclk.c | 95 +++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-)