On 3/20/19 6:54 AM, Imre Deak wrote:
From: Ville Syrjälä <ville.syrj...@linux.intel.com>

If we have only a single active pipe and the cdclk change only requires
the cd2x divider to be updated bxt+ can do the update with forcing a full
modeset on the pipe. Try to hook that up.

v2:
- Wait for vblank after an optimized CDCLK change.
- Avoid optimization if the pipe needs a modeset (or was disabled).
- Split CDCLK change to a pre/post plane update step.
v3:
- Use correct version of CDCLK state as old state. (Ville)
- Remove unused intel_cdclk_can_skip_modeset()
v4:
- For consistency call intel_set_cdclk_post_plane_update() only during
   modesets (and not fastsets).

Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.ku...@intel.com>
Tested-by: Abhay Kumar <abhay.ku...@intel.com>
Signed-off-by: Imre Deak <imre.d...@intel.com>
---
  drivers/gpu/drm/i915/i915_drv.h      |   3 +-
  drivers/gpu/drm/i915/i915_reg.h      |   3 +-
  drivers/gpu/drm/i915/intel_cdclk.c   | 142 +++++++++++++++++++++++++++--------
  drivers/gpu/drm/i915/intel_display.c |  42 ++++++++++-
  drivers/gpu/drm/i915/intel_drv.h     |  17 ++++-
  5 files changed, 170 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6b10cee4e77f..d8f91525c94c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -277,7 +277,8 @@ struct drm_i915_display_funcs {
        void (*get_cdclk)(struct drm_i915_private *dev_priv,
                          struct intel_cdclk_state *cdclk_state);
        void (*set_cdclk)(struct drm_i915_private *dev_priv,
-                         const struct intel_cdclk_state *cdclk_state);
+                         const struct intel_cdclk_state *cdclk_state,
+                         enum pipe pipe);
        int (*get_fifo_size)(struct drm_i915_private *dev_priv,
                             enum i9xx_plane_id i9xx_plane);
        int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9b69cec21f7b..12b8170ced96 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9521,7 +9521,8 @@ enum skl_power_gate {
  #define  BXT_CDCLK_CD2X_PIPE(pipe)    ((pipe) << 20)
  #define  CDCLK_DIVMUX_CD_OVERRIDE     (1 << 19)
  #define  BXT_CDCLK_CD2X_PIPE_NONE     BXT_CDCLK_CD2X_PIPE(3)
-#define  ICL_CDCLK_CD2X_PIPE_NONE      (7 << 19)
+#define  ICL_CDCLK_CD2X_PIPE(pipe)     ((pipe) << 19)

Unfortunately bits 21:19 of CDCLK_CTL don't match the pipe enum. This MACRO will not work except for PIPE_A.

Pipe_A is 0x00, PIPE_B is 0x02, PIPE_C is 0x06.

In BXT only bits 21:20 were used for CD2X pipe select and the pipe enum does work.


-Clint



+#define  ICL_CDCLK_CD2X_PIPE_NONE      ICL_CDCLK_CD2X_PIPE(7)
  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE       (1 << 16)
  #define  CDCLK_FREQ_DECIMAL_MASK      (0x7ff)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 5c25626f8cf0..48cc42a7ef4f 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -516,7 +516,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private 
*dev_priv)
  }
static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
-                         const struct intel_cdclk_state *cdclk_state)
+                         const struct intel_cdclk_state *cdclk_state,
+                         enum pipe pipe)
  {
        int cdclk = cdclk_state->cdclk;
        u32 val, cmd = cdclk_state->voltage_level;
@@ -598,7 +599,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
  }
static void chv_set_cdclk(struct drm_i915_private *dev_priv,
-                         const struct intel_cdclk_state *cdclk_state)
+                         const struct intel_cdclk_state *cdclk_state,
+                         enum pipe pipe)
  {
        int cdclk = cdclk_state->cdclk;
        u32 val, cmd = cdclk_state->voltage_level;
@@ -697,7 +699,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
  }
static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
-                         const struct intel_cdclk_state *cdclk_state)
+                         const struct intel_cdclk_state *cdclk_state,
+                         enum pipe pipe)
  {
        int cdclk = cdclk_state->cdclk;
        u32 val;
@@ -987,7 +990,8 @@ static void skl_dpll0_disable(struct drm_i915_private 
*dev_priv)
  }
static void skl_set_cdclk(struct drm_i915_private *dev_priv,
-                         const struct intel_cdclk_state *cdclk_state)
+                         const struct intel_cdclk_state *cdclk_state,
+                         enum pipe pipe)
  {
        int cdclk = cdclk_state->cdclk;
        int vco = cdclk_state->vco;
@@ -1158,7 +1162,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
        cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
        cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
- skl_set_cdclk(dev_priv, &cdclk_state);
+       skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
  }
/**
@@ -1176,7 +1180,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
        cdclk_state.vco = 0;
        cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
- skl_set_cdclk(dev_priv, &cdclk_state);
+       skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
  }
static int bxt_calc_cdclk(int min_cdclk)
@@ -1355,7 +1359,8 @@ static void bxt_de_pll_enable(struct drm_i915_private 
*dev_priv, int vco)
  }
static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
-                         const struct intel_cdclk_state *cdclk_state)
+                         const struct intel_cdclk_state *cdclk_state,
+                         enum pipe pipe)
  {
        int cdclk = cdclk_state->cdclk;
        int vco = cdclk_state->vco;
@@ -1408,11 +1413,10 @@ static void bxt_set_cdclk(struct drm_i915_private 
*dev_priv,
                bxt_de_pll_enable(dev_priv, vco);
val = divider | skl_cdclk_decimal(cdclk);
-       /*
-        * FIXME if only the cd2x divider needs changing, it could be done
-        * without shutting off the pipe (if only one pipe is active).
-        */
-       val |= BXT_CDCLK_CD2X_PIPE_NONE;
+       if (pipe == INVALID_PIPE)
+               val |= BXT_CDCLK_CD2X_PIPE_NONE;
+       else
+               val |= BXT_CDCLK_CD2X_PIPE(pipe);
        /*
         * Disable SSA Precharge when CD clock frequency < 500 MHz,
         * enable otherwise.
@@ -1421,6 +1425,9 @@ static void bxt_set_cdclk(struct drm_i915_private 
*dev_priv,
                val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
        I915_WRITE(CDCLK_CTL, val);
+ if (pipe != INVALID_PIPE)
+               intel_wait_for_vblank(dev_priv, pipe);
+
        mutex_lock(&dev_priv->pcu_lock);
        /*
         * The timeout isn't specified, the 2ms used here is based on
@@ -1525,7 +1532,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
        }
        cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
- bxt_set_cdclk(dev_priv, &cdclk_state);
+       bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
  }
/**
@@ -1543,7 +1550,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
        cdclk_state.vco = 0;
        cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
- bxt_set_cdclk(dev_priv, &cdclk_state);
+       bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
  }
static int cnl_calc_cdclk(int min_cdclk)
@@ -1663,7 +1670,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private 
*dev_priv, int vco)
  }
static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
-                         const struct intel_cdclk_state *cdclk_state)
+                         const struct intel_cdclk_state *cdclk_state,
+                         enum pipe pipe)
  {
        int cdclk = cdclk_state->cdclk;
        int vco = cdclk_state->vco;
@@ -1704,13 +1712,15 @@ static void cnl_set_cdclk(struct drm_i915_private 
*dev_priv,
                cnl_cdclk_pll_enable(dev_priv, vco);
val = divider | skl_cdclk_decimal(cdclk);
-       /*
-        * FIXME if only the cd2x divider needs changing, it could be done
-        * without shutting off the pipe (if only one pipe is active).
-        */
-       val |= BXT_CDCLK_CD2X_PIPE_NONE;
+       if (pipe == INVALID_PIPE)
+               val |= BXT_CDCLK_CD2X_PIPE_NONE;
+       else
+               val |= BXT_CDCLK_CD2X_PIPE(pipe);
        I915_WRITE(CDCLK_CTL, val);
+ if (pipe != INVALID_PIPE)
+               intel_wait_for_vblank(dev_priv, pipe);
+
        /* inform PCU of the change */
        mutex_lock(&dev_priv->pcu_lock);
        sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
@@ -1847,10 +1857,12 @@ static int icl_calc_cdclk_pll_vco(struct 
drm_i915_private *dev_priv, int cdclk)
  }
static void icl_set_cdclk(struct drm_i915_private *dev_priv,
-                         const struct intel_cdclk_state *cdclk_state)
+                         const struct intel_cdclk_state *cdclk_state,
+                         enum pipe pipe)
  {
        unsigned int cdclk = cdclk_state->cdclk;
        unsigned int vco = cdclk_state->vco;
+       u32 val;
        int ret;
mutex_lock(&dev_priv->pcu_lock);
@@ -1872,8 +1884,15 @@ static void icl_set_cdclk(struct drm_i915_private 
*dev_priv,
        if (dev_priv->cdclk.hw.vco != vco)
                cnl_cdclk_pll_enable(dev_priv, vco);
- I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
-                             skl_cdclk_decimal(cdclk));
+       val = skl_cdclk_decimal(cdclk);
+       if (pipe == INVALID_PIPE)
+               val |= ICL_CDCLK_CD2X_PIPE_NONE;
+       else
+               val |= ICL_CDCLK_CD2X_PIPE(pipe);
+       I915_WRITE(CDCLK_CTL, val);
+
+       if (pipe != INVALID_PIPE)
+               intel_wait_for_vblank(dev_priv, pipe);
mutex_lock(&dev_priv->pcu_lock);
        sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
@@ -2002,7 +2021,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
        sanitized_state.voltage_level =
                                icl_calc_voltage_level(sanitized_state.cdclk);
- icl_set_cdclk(dev_priv, &sanitized_state);
+       icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
  }
/**
@@ -2020,7 +2039,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
        cdclk_state.vco = 0;
        cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
- icl_set_cdclk(dev_priv, &cdclk_state);
+       icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
  }
/**
@@ -2048,7 +2067,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
        cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
        cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state);
+       cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
  }
/**
@@ -2066,7 +2085,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
        cdclk_state.vco = 0;
        cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state);
+       cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
  }
/**
@@ -2086,6 +2105,27 @@ bool intel_cdclk_needs_modeset(const struct 
intel_cdclk_state *a,
  }
/**
+ * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a 
cd2x divider update
+ * @a: first CDCLK state
+ * @b: second CDCLK state
+ *
+ * Returns:
+ * True if the CDCLK states require just a cd2x divider update, false if not.
+ */
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+                                  const struct intel_cdclk_state *a,
+                                  const struct intel_cdclk_state *b)
+{
+       /* Older hw doesn't have the capability */
+       if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
+               return false;
+
+       return a->cdclk != b->cdclk &&
+               a->vco == b->vco &&
+               a->ref == b->ref;
+}
+
+/**
   * intel_cdclk_changed - Determine if two CDCLK states are different
   * @a: first CDCLK state
   * @b: second CDCLK state
@@ -2133,12 +2173,14 @@ void intel_dump_cdclk_state(const struct 
intel_cdclk_state *cdclk_state,
   * intel_set_cdclk - Push the CDCLK state to the hardware
   * @dev_priv: i915 device
   * @cdclk_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
   *
   * Program the hardware based on the passed in CDCLK state,
   * if necessary.
   */
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
-                    const struct intel_cdclk_state *cdclk_state)
+static void intel_set_cdclk(struct drm_i915_private *dev_priv,
+                           const struct intel_cdclk_state *cdclk_state,
+                           enum pipe pipe)
  {
        if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
                return;
@@ -2148,7 +2190,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to"); - dev_priv->display.set_cdclk(dev_priv, cdclk_state);
+       dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
                 "cdclk state doesn't match!\n")) {
@@ -2157,6 +2199,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
        }
  }
+/**
+ * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware before updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+                                const struct intel_cdclk_state *old_state,
+                                const struct intel_cdclk_state *new_state,
+                                enum pipe pipe)
+{
+       if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
+               intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
+/**
+ * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware after updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+                                 const struct intel_cdclk_state *old_state,
+                                 const struct intel_cdclk_state *new_state,
+                                 enum pipe pipe)
+{
+       if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
+               intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
  static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
                                     int pixel_rate)
  {
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f7b44773e1e7..bcec03f43d3a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12982,6 +12982,7 @@ static int intel_modeset_checks(struct drm_atomic_state 
*state)
        intel_state->active_crtcs = dev_priv->active_crtcs;
        intel_state->cdclk.logical = dev_priv->cdclk.logical;
        intel_state->cdclk.actual = dev_priv->cdclk.actual;
+       intel_state->cdclk.pipe = INVALID_PIPE;
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
                if (new_crtc_state->active)
@@ -13001,6 +13002,8 @@ static int intel_modeset_checks(struct drm_atomic_state 
*state)
         * adjusted_mode bits in the crtc directly.
         */
        if (dev_priv->display.modeset_calc_cdclk) {
+               enum pipe pipe;
+
                ret = dev_priv->display.modeset_calc_cdclk(state);
                if (ret < 0)
                        return ret;
@@ -13017,12 +13020,36 @@ static int intel_modeset_checks(struct 
drm_atomic_state *state)
                                return ret;
                }
+ if (is_power_of_2(intel_state->active_crtcs)) {
+                       struct drm_crtc *crtc;
+                       struct drm_crtc_state *crtc_state;
+
+                       pipe = ilog2(intel_state->active_crtcs);
+                       crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
+                       crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+                       if (crtc_state && needs_modeset(crtc_state))
+                               pipe = INVALID_PIPE;
+               } else {
+                       pipe = INVALID_PIPE;
+               }
+
                /* All pipes must be switched off while we change the cdclk. */
-               if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
-                                             &intel_state->cdclk.actual)) {
+               if (pipe != INVALID_PIPE &&
+                   intel_cdclk_needs_cd2x_update(dev_priv,
+                                                 &dev_priv->cdclk.actual,
+                                                 &intel_state->cdclk.actual)) {
+                       ret = intel_lock_all_pipes(state);
+                       if (ret < 0)
+                               return ret;
+
+                       intel_state->cdclk.pipe = pipe;
+               } else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
+                                                    
&intel_state->cdclk.actual)) {
                        ret = intel_modeset_all_pipes(state);
                        if (ret < 0)
                                return ret;
+
+                       intel_state->cdclk.pipe = INVALID_PIPE;
                }
DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
@@ -13431,7 +13458,10 @@ static void intel_atomic_commit_tail(struct 
drm_atomic_state *state)
        if (intel_state->modeset) {
                drm_atomic_helper_update_legacy_modeset_state(state->dev, 
state);
- intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
+               intel_set_cdclk_pre_plane_update(dev_priv,
+                                                &intel_state->cdclk.actual,
+                                                &dev_priv->cdclk.actual,
+                                                intel_state->cdclk.pipe);
/*
                 * SKL workaround: bspec recommends we disable the SAGV when we
@@ -13460,6 +13490,12 @@ static void intel_atomic_commit_tail(struct 
drm_atomic_state *state)
        /* Now enable the clocks, plane, pipe, and connectors that we set up. */
        dev_priv->display.update_crtcs(state);
+ if (intel_state->modeset)
+               intel_set_cdclk_post_plane_update(dev_priv,
+                                                 &intel_state->cdclk.actual,
+                                                 &dev_priv->cdclk.actual,
+                                                 intel_state->cdclk.pipe);
+
        /* FIXME: We should call drm_atomic_helper_commit_hw_done() here
         * already, but still need the state for the delayed optimization. To
         * fix this:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 85dd6a9d1e42..d22a0e92e0d4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -559,6 +559,8 @@ struct intel_atomic_state {
int force_min_cdclk;
                bool force_min_cdclk_changed;
+               /* pipe to which cd2x update is synchronized */
+               enum pipe pipe;
        } cdclk;
bool dpll_set, modeset;
@@ -1694,13 +1696,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private 
*dev_priv);
  void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
  void intel_update_cdclk(struct drm_i915_private *dev_priv);
  void intel_update_rawclk(struct drm_i915_private *dev_priv);
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+                                  const struct intel_cdclk_state *a,
+                                  const struct intel_cdclk_state *b);
  bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
                               const struct intel_cdclk_state *b);
  bool intel_cdclk_changed(const struct intel_cdclk_state *a,
                         const struct intel_cdclk_state *b);
  void intel_cdclk_swap_state(struct intel_atomic_state *state);
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
-                    const struct intel_cdclk_state *cdclk_state);
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+                                const struct intel_cdclk_state *old_state,
+                                const struct intel_cdclk_state *new_state,
+                                enum pipe pipe);
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+                                 const struct intel_cdclk_state *old_state,
+                                 const struct intel_cdclk_state *new_state,
+                                 enum pipe pipe);
  void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
                            const char *context);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to