Re: [Intel-gfx] [PATCH 13/17] drm/i915/icl: Enable 2nd DBuf slice only when needed

2018-01-25 Thread James Ausmus 
On Tue, Jan 23, 2018 at 05:05:32PM -0200, Paulo Zanoni wrote:
> From: Mahesh Kumar 
> 
> ICL has two slices of DBuf, each slice of size 1024 blocks.
> We should not always enable slice-2. It should be enabled only if
> display total required BW is > 12GBps OR more than 1 pipes are enabled.
> 
> Changes since V1:
>  - typecast total_data_rate to u64 before multiplication to solve any
>possible overflow (Rodrigo)
>  - fix where skl_wm_get_hw_state was memsetting ddb, resulting
>enabled_slices to become zero
>  - Fix the logic of calculating ddb_size
> Changes since V2:
>  - If no-crtc is part of commit required_slices will have value "0",
>don't try to disable DBuf slice.
> 
> Signed-off-by: Mahesh Kumar 
> ---
>  drivers/gpu/drm/i915/intel_display.c| 12 +++
>  drivers/gpu/drm/i915/intel_drv.h|  6 
>  drivers/gpu/drm/i915/intel_pm.c | 64 
> +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 47 +---
>  4 files changed, 110 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index bad3b112ac3e..3eb2359c221b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12117,6 +12117,8 @@ static void skl_update_crtcs(struct drm_atomic_state 
> *state)
>   bool progress;
>   enum pipe pipe;
>   int i;
> + uint8_t hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
> + uint8_t required_slices = intel_state->wm_results.ddb.enabled_slices;
>  
>   const struct skl_ddb_entry *entries[I915_MAX_PIPES] = {};
>  
> @@ -12125,6 +12127,11 @@ static void skl_update_crtcs(struct drm_atomic_state 
> *state)
>   if (new_crtc_state->active)
>   entries[i] = 
> _intel_crtc_state(old_crtc_state)->wm.skl.ddb;
>  
> + /* If 2nd DBuf slice required, enable it here */
> + if (INTEL_GEN(dev_priv) >= 11 && required_slices &&
> +  required_slices > hw_enabled_slices)
> + icl_dbuf_slices_update(dev_priv, required_slices);
> +
>   /*
>* Whenever the number of active pipes changes, we need to make sure we
>* update the pipes in the right order so that their ddb allocations
> @@ -12175,6 +12182,11 @@ static void skl_update_crtcs(struct drm_atomic_state 
> *state)
>   progress = true;
>   }
>   } while (progress);
> +
> + /* If 2nd DBuf slice is no more required disable it */
> + if (INTEL_GEN(dev_priv) >= 11 && required_slices &&
> +  required_slices < hw_enabled_slices)
> + icl_dbuf_slices_update(dev_priv, required_slices);
>  }
>  
>  static void intel_atomic_helper_free_state(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index c5d6092aca41..d4639a161fe3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -140,6 +140,10 @@
>  #define KHz(x) (1000 * (x))
>  #define MHz(x) KHz(1000 * (x))
>  
> +#define KBps(x) (1000 * (x))
> +#define MBps(x) KBps(1000 * (x))
> +#define GBps(x) ((uint64_t) 1000 * MBps((x)))
> +
>  /*
>   * Display related stuff
>   */
> @@ -1890,6 +1894,8 @@ bool intel_display_power_get_if_enabled(struct 
> drm_i915_private *dev_priv,
>   enum intel_display_power_domain domain);
>  void intel_display_power_put(struct drm_i915_private *dev_priv,
>enum intel_display_power_domain domain);
> +void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
> + uint8_t req_slices);
>  
>  static inline void
>  assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e8d98857c208..d4cd631377da 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3767,9 +3767,42 @@ bool intel_can_enable_sagv(struct drm_atomic_state 
> *state)
>   return true;
>  }
>  
> +static unsigned int intel_get_ddb_size(struct drm_i915_private *dev_priv,
> +const struct intel_crtc_state *cstate,
> +const unsigned int total_data_rate,
> +const int num_active,
> +struct skl_ddb_allocation *ddb)
> +{
> + const struct drm_display_mode *adjusted_mode;
> + u64 total_data_bw;
> + u16 ddb_size = INTEL_INFO(dev_priv)->ddb_size;
> +
> + WARN_ON(ddb_size == 0);
> +
> + if (INTEL_GEN(dev_priv) < 11)
> + return ddb_size - 4; /* 4 blocks for bypass path allocation */
> +
> + adjusted_mode = >base.adjusted_mode;
> + total_data_bw = (u64)total_data_rate *

[Intel-gfx] [PATCH 13/17] drm/i915/icl: Enable 2nd DBuf slice only when needed

2018-01-23 Thread Paulo Zanoni 
From: Mahesh Kumar 

ICL has two slices of DBuf, each slice of size 1024 blocks.
We should not always enable slice-2. It should be enabled only if
display total required BW is > 12GBps OR more than 1 pipes are enabled.

Changes since V1:
 - typecast total_data_rate to u64 before multiplication to solve any
   possible overflow (Rodrigo)
 - fix where skl_wm_get_hw_state was memsetting ddb, resulting
   enabled_slices to become zero
 - Fix the logic of calculating ddb_size
Changes since V2:
 - If no-crtc is part of commit required_slices will have value "0",
   don't try to disable DBuf slice.

Signed-off-by: Mahesh Kumar 
---
 drivers/gpu/drm/i915/intel_display.c| 12 +++
 drivers/gpu/drm/i915/intel_drv.h|  6 
 drivers/gpu/drm/i915/intel_pm.c | 64 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 47 +---
 4 files changed, 110 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index bad3b112ac3e..3eb2359c221b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12117,6 +12117,8 @@ static void skl_update_crtcs(struct drm_atomic_state 
*state)
bool progress;
enum pipe pipe;
int i;
+   uint8_t hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices;
+   uint8_t required_slices = intel_state->wm_results.ddb.enabled_slices;
 
const struct skl_ddb_entry *entries[I915_MAX_PIPES] = {};
 
@@ -12125,6 +12127,11 @@ static void skl_update_crtcs(struct drm_atomic_state 
*state)
if (new_crtc_state->active)
entries[i] = 
_intel_crtc_state(old_crtc_state)->wm.skl.ddb;
 
+   /* If 2nd DBuf slice required, enable it here */
+   if (INTEL_GEN(dev_priv) >= 11 && required_slices &&
+required_slices > hw_enabled_slices)
+   icl_dbuf_slices_update(dev_priv, required_slices);
+
/*
 * Whenever the number of active pipes changes, we need to make sure we
 * update the pipes in the right order so that their ddb allocations
@@ -12175,6 +12182,11 @@ static void skl_update_crtcs(struct drm_atomic_state 
*state)
progress = true;
}
} while (progress);
+
+   /* If 2nd DBuf slice is no more required disable it */
+   if (INTEL_GEN(dev_priv) >= 11 && required_slices &&
+required_slices < hw_enabled_slices)
+   icl_dbuf_slices_update(dev_priv, required_slices);
 }
 
 static void intel_atomic_helper_free_state(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c5d6092aca41..d4639a161fe3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -140,6 +140,10 @@
 #define KHz(x) (1000 * (x))
 #define MHz(x) KHz(1000 * (x))
 
+#define KBps(x) (1000 * (x))
+#define MBps(x) KBps(1000 * (x))
+#define GBps(x) ((uint64_t) 1000 * MBps((x)))
+
 /*
  * Display related stuff
  */
@@ -1890,6 +1894,8 @@ bool intel_display_power_get_if_enabled(struct 
drm_i915_private *dev_priv,
enum intel_display_power_domain domain);
 void intel_display_power_put(struct drm_i915_private *dev_priv,
 enum intel_display_power_domain domain);
+void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
+   uint8_t req_slices);
 
 static inline void
 assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e8d98857c208..d4cd631377da 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3767,9 +3767,42 @@ bool intel_can_enable_sagv(struct drm_atomic_state 
*state)
return true;
 }
 
+static unsigned int intel_get_ddb_size(struct drm_i915_private *dev_priv,
+  const struct intel_crtc_state *cstate,
+  const unsigned int total_data_rate,
+  const int num_active,
+  struct skl_ddb_allocation *ddb)
+{
+   const struct drm_display_mode *adjusted_mode;
+   u64 total_data_bw;
+   u16 ddb_size = INTEL_INFO(dev_priv)->ddb_size;
+
+   WARN_ON(ddb_size == 0);
+
+   if (INTEL_GEN(dev_priv) < 11)
+   return ddb_size - 4; /* 4 blocks for bypass path allocation */
+
+   adjusted_mode = >base.adjusted_mode;
+   total_data_bw = (u64)total_data_rate * drm_mode_vrefresh(adjusted_mode);
+
+   /*
+* 12GB/s is maximum BW supported by single DBuf slice.
+*/
+   if (total_data_bw >= GBps(12) || num_active > 1)
+   ddb->enabled_slices = 2;
+   else {
+