On Thu, 09 Dec 2021, Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx> wrote:
> i915 has squashing for DG2 and crawling for ADLP.
> Moving the checks to atomic check phase so
> at a later phase we know how the cdclk changes.
Just some high level comments:
- Functions named intel_cdclk_can_foo() must *not* change the state,
that's unexpected and surprising.
- There's a bunch of state handling already for cdclk, please don't just
dump new state in drm_i915_private, outside of the existing states. In
particular, storing yet another copy of cdclk is suspicious.
- Please don't add short stuff like CRAWL and SQUASH to our module
namespace.
BR,
Jani.
>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 49 +++++++++++++---------
> drivers/gpu/drm/i915/i915_drv.h | 11 +++++
> 2 files changed, 41 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 639a64733f61..9382dd24d889 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1707,9 +1707,27 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> return;
> }
>
> - if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->cdclk.hw.vco > 0 && vco > 0) {
> - if (dev_priv->cdclk.hw.vco != vco)
> - adlp_cdclk_pll_crawl(dev_priv, vco);
> + if (DISPLAY_VER(dev_priv) >= 12) {
> + int i = 0;
> + u32 squash_ctl = 0;
> + struct cdclk_steps *cdclk_steps = dev_priv->cdclk.steps;
> +
> + for (i = 0; i < CDCLK_ACTIONS; i++) {
> + switch (cdclk_steps[i].action) {
> + case CRAWL:
> + adlp_cdclk_pll_crawl(dev_priv, vco);
> + break;
> + case SQUASH:
> + waveform = cdclk_squash_waveform(dev_priv, cdclk_steps[i].cdclk);
> + clock = vco / 2;
> + squash_ctl = CDCLK_SQUASH_ENABLE |
> + CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform;
> + intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl);
> + break;
> + default:
> + break;
> + }
> + }
> } else if (DISPLAY_VER(dev_priv) >= 11) {
> if (dev_priv->cdclk.hw.vco != 0 &&
> dev_priv->cdclk.hw.vco != vco)
> @@ -1726,22 +1744,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> bxt_de_pll_enable(dev_priv, vco);
> }
>
> - waveform = cdclk_squash_waveform(dev_priv, cdclk);
> -
> - if (waveform)
> - clock = vco / 2;
> - else
> - clock = cdclk;
> -
> - if (has_cdclk_squasher(dev_priv)) {
> - u32 squash_ctl = 0;
> -
> - if (waveform)
> - squash_ctl = CDCLK_SQUASH_ENABLE |
> - CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform;
> -
> - intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl);
> - }
> + clock = cdclk;
>
> val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) |
> bxt_cdclk_cd2x_pipe(dev_priv, pipe) |
> @@ -1934,6 +1937,7 @@ static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv,
> const struct intel_cdclk_config *a,
> const struct intel_cdclk_config *b)
> {
> + struct cdclk_steps *cdclk_transition = dev_priv->cdclk.steps;
> int a_div, b_div;
>
> if (!HAS_CDCLK_CRAWL(dev_priv))
> @@ -1946,6 +1950,9 @@ static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv,
> a_div = DIV_ROUND_CLOSEST(a->vco, a->cdclk);
> b_div = DIV_ROUND_CLOSEST(b->vco, b->cdclk);
>
> + cdclk_transition[0].action = CRAWL;
> + cdclk_transition[0].cdclk = b->cdclk;
> +
> return a->vco != 0 && b->vco != 0 &&
> a->vco != b->vco &&
> a_div == b_div &&
> @@ -1956,6 +1963,7 @@ static bool intel_cdclk_can_squash(struct drm_i915_private *dev_priv,
> const struct intel_cdclk_config *a,
> const struct intel_cdclk_config *b)
> {
> + struct cdclk_steps *cdclk_transition = dev_priv->cdclk.steps;
> /*
> * FIXME should store a bit more state in intel_cdclk_config
> * to differentiate squasher vs. cd2x divider properly. For
> @@ -1965,6 +1973,9 @@ static bool intel_cdclk_can_squash(struct drm_i915_private *dev_priv,
> if (!has_cdclk_squasher(dev_priv))
> return false;
>
> + cdclk_transition[0].action = SQUASH;
> + cdclk_transition[0].cdclk = b->cdclk;
> +
> return a->cdclk != b->cdclk &&
> a->vco != 0 &&
> a->vco == b->vco &&
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ae7dc7862b5d..c03299253b81 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -117,6 +117,12 @@
>
> struct drm_i915_gem_object;
>
> +enum cdclk_actions {
> + SQUASH = 0,
> + CRAWL,
> + CDCLK_ACTIONS
> +};
> +
> /* Threshold == 5 for long IRQs, 50 for short */
> #define HPD_STORM_DEFAULT_THRESHOLD 50
>
> @@ -782,6 +788,11 @@ struct drm_i915_private {
> const struct intel_cdclk_vals *table;
>
> struct intel_global_obj obj;
> +
> + struct cdclk_steps {
> + enum cdclk_actions action;
> + u32 cdclk;
> + } steps[CDCLK_ACTIONS];
> } cdclk;
>
> struct {
--
Jani Nikula, Intel Open Source Graphics Center
