[PATCH 1/3] drm/i915: Wa#0893 is defined per platform.

[PATCH 1/3] drm/i915: Wa#0893 is defined per platform.

[Rodrigo Vivi]

No detection will ever be needed. So let's simplify
and clean it up.

Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d2a5749..5abeb3f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2904,6 +2904,10 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
 #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \
 	(IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
 
+/* WA#0893 */
+#define NEEDS_WaIncreaseMemoryBW(dev_priv)	(IS_GEN9_BC(dev_priv) || \
+						 IS_BROXTON(dev_priv))
+
 /*
  * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
  * even when in MSI mode. This results in spurious interrupt warnings if the
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 32b3b3e..919235c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3532,21 +3532,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
 
 #define SKL_SAGV_BLOCK_TIME	30 /* µs */
 
-/*
- * FIXME: We still don't have the proper code detect if we need to apply the WA,
- * so assume we'll always need it in order to avoid underruns.
- */
-static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
-{
-	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-
-	/* WA#0893: GEN9:ALL */
-	if (IS_GEN9(dev_priv))
-		return true;
-
-	return false;
-}
-
 static bool
 intel_has_sagv(struct drm_i915_private *dev_priv)
 {
@@ -3694,7 +3679,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 
 		latency = dev_priv->wm.skl_latency[level];
 
-		if (skl_needs_memory_bw_wa(intel_state) &&
+		if (NEEDS_WaIncreaseMemoryBW(dev_priv) &&
 		    plane->base.state->fb->modifier ==
 		    I915_FORMAT_MOD_X_TILED)
 			latency += 15;
@@ -4265,9 +4250,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	uint32_t plane_pixel_rate;
 	uint_fixed_16_16_t y_tile_minimum;
 	uint32_t y_min_scanlines;
-	struct intel_atomic_state *state =
-		to_intel_atomic_state(cstate->base.state);
-	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
+	bool apply_memory_bw_wa = NEEDS_WaIncreaseMemoryBW(dev_priv);
 	bool y_tiled, x_tiled;
 
 	if (latency == 0 ||
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/glk: WA#0893: Also apply memory bw wa to Geminilake.

[Rodrigo Vivi]

According to spec this WA is needed for every gen9.

Cc:Arthur Runyan <arthur.j.runyan@intel.com>
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5abeb3f..7a5f2e4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2904,9 +2904,8 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
 #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \
 	(IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
 
-/* WA#0893 */
-#define NEEDS_WaIncreaseMemoryBW(dev_priv)	(IS_GEN9_BC(dev_priv) || \
-						 IS_BROXTON(dev_priv))
+/* WA#0893: GEN9:ALL */
+#define NEEDS_WaIncreaseMemoryBW(dev_priv)	(IS_GEN9(dev_priv))
 
 /*
  * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] RFC drm/i915: Don't increase the BW when WA is not required.

[Rodrigo Vivi]

Based on patch submited to intel-gfx:
"drm/i915/cnl: don't apply the GEN9/CNL:A WM WAs to CNL:B+"
and subsequential tests on CNL, it seems that
this part is not required if we are not applying WA#0893.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 919235c..85d9705 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4356,7 +4356,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	res_lines = div_round_up_fixed16(selected_result,
 					 plane_blocks_per_line);
 
-	if (level >= 1 && level <= 7) {
+	if (NEEDS_WaIncreaseMemoryBW(dev_priv) && level >= 1 && level <= 7) {
 		if (y_tiled) {
 			res_blocks += fixed_16_16_to_u32_round_up(y_tile_minimum);
 			res_lines += y_min_scanlines;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Wa#0893 is defined per platform.

[Mahesh Kumar]

Hi,

There are already patches floated to get the total_display_bandwidth 
in-order to implement correct memory bandwidth related WA for GEN9. This 
is the series floated earlier for this:

https://patchwork.freedesktop.org/series/18842/

above series is hitting the performance issue, because we were including 
all CRTC in each flip even if crtc is not part of flip & we don't have 
i915 level global ww_mutex lock.

There was another RFC doc to list down different approaches to implement 
bandwidth related WA, which will be implemented eventually.
http://www.spinics.net/lists/intel-gfx/msg124872.html

unconditional implementation of these WA's restricts us from using more 
planes per CRTC due to limited availability of DDB.

Do we still need this patch? because once above patches comes in, this 
patch will require modification.

-Mahesh


On Saturday 27 May 2017 04:53 AM, Rodrigo Vivi wrote:
> No detection will ever be needed. So let's simplify
> and clean it up.
>
> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h |  4 ++++
>   drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>   2 files changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d2a5749..5abeb3f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2904,6 +2904,10 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
>   #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \
>   	(IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
>   
> +/* WA#0893 */
> +#define NEEDS_WaIncreaseMemoryBW(dev_priv)	(IS_GEN9_BC(dev_priv) || \
> +						 IS_BROXTON(dev_priv))
> +
>   /*
>    * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
>    * even when in MSI mode. This results in spurious interrupt warnings if the
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 32b3b3e..919235c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3532,21 +3532,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
>   
>   #define SKL_SAGV_BLOCK_TIME	30 /* µs */
>   
> -/*
> - * FIXME: We still don't have the proper code detect if we need to apply the WA,
> - * so assume we'll always need it in order to avoid underruns.
> - */
> -static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -
> -	/* WA#0893: GEN9:ALL */
> -	if (IS_GEN9(dev_priv))
> -		return true;
> -
> -	return false;
> -}
> -
>   static bool
>   intel_has_sagv(struct drm_i915_private *dev_priv)
>   {
> @@ -3694,7 +3679,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>   
>   		latency = dev_priv->wm.skl_latency[level];
>   
> -		if (skl_needs_memory_bw_wa(intel_state) &&
> +		if (NEEDS_WaIncreaseMemoryBW(dev_priv) &&
>   		    plane->base.state->fb->modifier ==
>   		    I915_FORMAT_MOD_X_TILED)
>   			latency += 15;
> @@ -4265,9 +4250,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>   	uint32_t plane_pixel_rate;
>   	uint_fixed_16_16_t y_tile_minimum;
>   	uint32_t y_min_scanlines;
> -	struct intel_atomic_state *state =
> -		to_intel_atomic_state(cstate->base.state);
> -	bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state);
> +	bool apply_memory_bw_wa = NEEDS_WaIncreaseMemoryBW(dev_priv);
>   	bool y_tiled, x_tiled;
>   
>   	if (latency == 0 ||

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/glk: WA#0893: Also apply memory bw wa to Geminilake.

[Ander Conselvan De Oliveira]

On Fri, 2017-05-26 at 16:23 -0700, Rodrigo Vivi wrote:
> According to spec this WA is needed for every gen9.

Actually GLK has a gen10 display, so the gen9 workarounds don't apply.

Ander

> 
> Cc:Arthur Runyan <arthur.j.runyan@intel.com>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5abeb3f..7a5f2e4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2904,9 +2904,8 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
>  #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \
>  	(IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
>  
> -/* WA#0893 */
> -#define NEEDS_WaIncreaseMemoryBW(dev_priv)	(IS_GEN9_BC(dev_priv) || \
> -						 IS_BROXTON(dev_priv))
> +/* WA#0893: GEN9:ALL */
> +#define NEEDS_WaIncreaseMemoryBW(dev_priv)	(IS_GEN9(dev_priv))
>  
>  /*
>   * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/glk: WA#0893: Also apply memory bw wa to Geminilake.

[Vivi, Rodrigo]

On Mon, 2017-05-29 at 11:26 +0300, Ander Conselvan De Oliveira wrote:
> On Fri, 2017-05-26 at 16:23 -0700, Rodrigo Vivi wrote:
> > According to spec this WA is needed for every gen9.
> 
> Actually GLK has a gen10 display, so the gen9 workarounds don't apply.

Oh, indeed!

Please ignore this patch.

> 
> Ander
> 
> > 
> > Cc:Arthur Runyan <arthur.j.runyan@intel.com>
> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 5abeb3f..7a5f2e4 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2904,9 +2904,8 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
> >  #define NEEDS_WaRsDisableCoarsePowerGating(dev_priv) \
> >  	(IS_SKL_GT3(dev_priv) || IS_SKL_GT4(dev_priv))
> >  
> > -/* WA#0893 */
> > -#define NEEDS_WaIncreaseMemoryBW(dev_priv)	(IS_GEN9_BC(dev_priv) || \
> > -						 IS_BROXTON(dev_priv))
> > +/* WA#0893: GEN9:ALL */
> > +#define NEEDS_WaIncreaseMemoryBW(dev_priv)	(IS_GEN9(dev_priv))
> >  
> >  /*
> >   * dp aux and gmbus irq on gen4 seems to be able to generate legacy interrupts

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] RFC drm/i915: Don't increase the BW when WA is not required.

[Mahesh Kumar]

Hi,

memory bandwidth WA is not applicable for GEN10 but below WA is needed 
for CNL-A.

-Mahesh

On Saturday 27 May 2017 04:53 AM, Rodrigo Vivi wrote:
> Based on patch submited to intel-gfx:
> "drm/i915/cnl: don't apply the GEN9/CNL:A WM WAs to CNL:B+"
> and subsequential tests on CNL, it seems that
> this part is not required if we are not applying WA#0893.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 919235c..85d9705 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4356,7 +4356,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>   	res_lines = div_round_up_fixed16(selected_result,
>   					 plane_blocks_per_line);
>   
> -	if (level >= 1 && level <= 7) {
> +	if (NEEDS_WaIncreaseMemoryBW(dev_priv) && level >= 1 && level <= 7) {
>   		if (y_tiled) {
>   			res_blocks += fixed_16_16_to_u32_round_up(y_tile_minimum);
>   			res_lines += y_min_scanlines;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx