All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
@ 2016-10-10 20:30 Paulo Zanoni
  2016-10-10 20:30 ` [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced Paulo Zanoni
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Paulo Zanoni @ 2016-10-10 20:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, stable, Mahesh Kumar, Lyude, Dhinakaran Pandiyan

Mahesh Kumar is already working on a proper implementation for the
workaround, but while we still don't have it, let's just
unconditionally apply the workaround for everybody and we hope we can
close all those numerous bugzilla tickets. Also, I'm not sure how easy
it will be to backport the final implementation to the stable Kernels,
and this patch here is probably easier to backport.

At the present moment I still don't have confirmation that this patch
fixes any of the bugs listed below, but we should definitely try
testing all of them again.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
Cc: stable@vger.kernel.org
Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Lyude <cpaul@redhat.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 62d730d..159831d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane *plane)
 	}
 }
 
+/*
+ * 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 intel_needs_memory_bw_wa(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+
+	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
+	    IS_KABYLAKE(dev_priv))
+		return true;
+
+	return false;
+}
+
 static bool
 intel_has_sagv(struct drm_i915_private *dev_priv)
 {
@@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	struct drm_crtc *crtc;
+	struct intel_crtc *crtc;
+	struct intel_plane *plane;
 	enum pipe pipe;
-	int level, plane;
+	int level, id, latency;
 
 	if (!intel_has_sagv(dev_priv))
 		return false;
@@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 
 	/* Since we're now guaranteed to only have one active CRTC... */
 	pipe = ffs(intel_state->active_crtcs) - 1;
-	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 
-	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
+	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
 		return false;
 
-	for_each_plane(dev_priv, pipe, plane) {
+	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+		id = skl_wm_plane_id(plane);
+
 		/* Skip this plane if it's not enabled */
-		if (intel_state->wm_results.plane[pipe][plane][0] == 0)
+		if (intel_state->wm_results.plane[pipe][id][0] == 0)
 			continue;
 
 		/* Find the highest enabled wm level for this plane */
 		for (level = ilk_wm_max_level(dev);
-		     intel_state->wm_results.plane[pipe][plane][level] == 0; --level)
+		     intel_state->wm_results.plane[pipe][id][level] == 0; --level)
 		     { }
 
+		latency = dev_priv->wm.skl_latency[level];
+
+		if (intel_needs_memory_bw_wa(intel_state) &&
+		    plane->base.state->fb->modifier[0] ==
+		    I915_FORMAT_MOD_X_TILED)
+			latency += 15;
+
 		/*
 		 * If any of the planes on this pipe don't enable wm levels
 		 * that incur memory latencies higher then 30µs we can't enable
 		 * the SAGV
 		 */
-		if (dev_priv->wm.skl_latency[level] < SKL_SAGV_BLOCK_TIME)
+		if (latency < SKL_SAGV_BLOCK_TIME)
 			return false;
 	}
 
@@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	uint32_t width = 0, height = 0;
 	uint32_t plane_pixel_rate;
 	uint32_t y_tile_minimum, y_min_scanlines;
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(cstate->base.state);
+	bool apply_memory_bw_wa = intel_needs_memory_bw_wa(state);
 
 	if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
 		*enabled = false;
 		return 0;
 	}
 
+	if (apply_memory_bw_wa && fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
+		latency += 15;
+
 	width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	height = drm_rect_height(&intel_pstate->base.src) >> 16;
 
@@ -3607,6 +3638,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				 plane_blocks_per_line);
 
 	y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
+	if (apply_memory_bw_wa)
+		y_tile_minimum *= 2;
 
 	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced
  2016-10-10 20:30 [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Paulo Zanoni
@ 2016-10-10 20:30 ` Paulo Zanoni
  2016-10-10 20:55   ` Lyude Paul
  2016-10-10 20:34 ` [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Lyude Paul
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Paulo Zanoni @ 2016-10-10 20:30 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

We want to look at the mode that we're actually going to set. All the
other display checks for interlaced flags also look at adjusted_mode.

Cc: Lyude <cpaul@redhat.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@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 159831d..4b7de7a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3037,7 +3037,7 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 	pipe = ffs(intel_state->active_crtcs) - 1;
 	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 
-	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
+	if (crtc->base.state->adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
 		return false;
 
 	for_each_intel_plane_on_crtc(dev, crtc, plane) {
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-10 20:30 [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Paulo Zanoni
  2016-10-10 20:30 ` [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced Paulo Zanoni
@ 2016-10-10 20:34 ` Lyude Paul
  2016-10-10 20:46   ` Zanoni, Paulo R
  2016-10-10 21:19 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Lyude Paul @ 2016-10-10 20:34 UTC (permalink / raw)
  To: Paulo Zanoni, intel-gfx; +Cc: stable, Mahesh Kumar, Dhinakaran Pandiyan

Was there a new workaround added recently? Or was this something I just
missed when writing the original code for this

On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote:
> Mahesh Kumar is already working on a proper implementation for the
> workaround, but while we still don't have it, let's just
> unconditionally apply the workaround for everybody and we hope we can
> close all those numerous bugzilla tickets. Also, I'm not sure how
> easy
> it will be to backport the final implementation to the stable
> Kernels,
> and this patch here is probably easier to backport.
> 
> At the present moment I still don't have confirmation that this patch
> fixes any of the bugs listed below, but we should definitely try
> testing all of them again.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
> Cc: stable@vger.kernel.org
> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 49
> ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 62d730d..159831d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane
> *plane)
>  	}
>  }
>  
> +/*
> + * 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 intel_needs_memory_bw_wa(struct intel_atomic_state
> *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state-
> >base.dev);
> +
> +	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> +	    IS_KABYLAKE(dev_priv))
> +		return true;
> +
> +	return false;
> +}
> +
>  static bool
>  intel_has_sagv(struct drm_i915_private *dev_priv)
>  {
> @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct
> drm_atomic_state *state)
>  	struct drm_device *dev = state->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> -	struct drm_crtc *crtc;
> +	struct intel_crtc *crtc;
> +	struct intel_plane *plane;
>  	enum pipe pipe;
> -	int level, plane;
> +	int level, id, latency;
>  
>  	if (!intel_has_sagv(dev_priv))
>  		return false;
> @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct
> drm_atomic_state *state)
>  
>  	/* Since we're now guaranteed to only have one active
> CRTC... */
>  	pipe = ffs(intel_state->active_crtcs) - 1;
> -	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>  
> -	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
> +	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		return false;
>  
> -	for_each_plane(dev_priv, pipe, plane) {
> +	for_each_intel_plane_on_crtc(dev, crtc, plane) {
> +		id = skl_wm_plane_id(plane);
> +
>  		/* Skip this plane if it's not enabled */
> -		if (intel_state->wm_results.plane[pipe][plane][0] ==
> 0)
> +		if (intel_state->wm_results.plane[pipe][id][0] == 0)
>  			continue;
>  
>  		/* Find the highest enabled wm level for this plane
> */
>  		for (level = ilk_wm_max_level(dev);
> -		     intel_state-
> >wm_results.plane[pipe][plane][level] == 0; --level)
> +		     intel_state->wm_results.plane[pipe][id][level]
> == 0; --level)
>  		     { }
>  
> +		latency = dev_priv->wm.skl_latency[level];
> +
> +		if (intel_needs_memory_bw_wa(intel_state) &&
> +		    plane->base.state->fb->modifier[0] ==
> +		    I915_FORMAT_MOD_X_TILED)
> +			latency += 15;
> +
>  		/*
>  		 * If any of the planes on this pipe don't enable wm
> levels
>  		 * that incur memory latencies higher then 30µs we
> can't enable
>  		 * the SAGV
>  		 */
> -		if (dev_priv->wm.skl_latency[level] <
> SKL_SAGV_BLOCK_TIME)
> +		if (latency < SKL_SAGV_BLOCK_TIME)
>  			return false;
>  	}
>  
> @@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>  	uint32_t width = 0, height = 0;
>  	uint32_t plane_pixel_rate;
>  	uint32_t y_tile_minimum, y_min_scanlines;
> +	struct intel_atomic_state *state =
> +		to_intel_atomic_state(cstate->base.state);
> +	bool apply_memory_bw_wa = intel_needs_memory_bw_wa(state);
>  
>  	if (latency == 0 || !cstate->base.active || !intel_pstate-
> >base.visible) {
>  		*enabled = false;
>  		return 0;
>  	}
>  
> +	if (apply_memory_bw_wa && fb->modifier[0] ==
> I915_FORMAT_MOD_X_TILED)
> +		latency += 15;
> +
>  	width = drm_rect_width(&intel_pstate->base.src) >> 16;
>  	height = drm_rect_height(&intel_pstate->base.src) >> 16;
>  
> @@ -3607,6 +3638,8 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>  				 plane_blocks_per_line);
>  
>  	y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
> +	if (apply_memory_bw_wa)
> +		y_tile_minimum *= 2;
>  
>  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-10 20:34 ` [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Lyude Paul
@ 2016-10-10 20:46   ` Zanoni, Paulo R
  2016-10-10 20:55       ` Lyude Paul
  0 siblings, 1 reply; 19+ messages in thread
From: Zanoni, Paulo R @ 2016-10-10 20:46 UTC (permalink / raw)
  To: cpaul, intel-gfx; +Cc: Kumar, Mahesh1, stable, Pandiyan, Dhinakaran

Em Seg, 2016-10-10 às 16:34 -0400, Lyude Paul escreveu:
> Was there a new workaround added recently? Or was this something I
> just
> missed when writing the original code for this

It's listed on the public spec:
https://01.org/sites/default/files/docum
entation/intel-gfx-prm-osrc-skl-vol12-display.pdf

Pages 210 - 211.

> 
> On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote:
> > 
> > Mahesh Kumar is already working on a proper implementation for the
> > workaround, but while we still don't have it, let's just
> > unconditionally apply the workaround for everybody and we hope we
> > can
> > close all those numerous bugzilla tickets. Also, I'm not sure how
> > easy
> > it will be to backport the final implementation to the stable
> > Kernels,
> > and this patch here is probably easier to backport.
> > 
> > At the present moment I still don't have confirmation that this
> > patch
> > fixes any of the bugs listed below, but we should definitely try
> > testing all of them again.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
> > Cc: stable@vger.kernel.org
> > Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> > Cc: Lyude <cpaul@redhat.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 49
> > ++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 41 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 62d730d..159831d 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane
> > *plane)
> >  	}
> >  }
> >  
> > +/*
> > + * 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 intel_needs_memory_bw_wa(struct intel_atomic_state
> > *state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(state-
> > > 
> > > base.dev);
> > +
> > +	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> > +	    IS_KABYLAKE(dev_priv))
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  static bool
> >  intel_has_sagv(struct drm_i915_private *dev_priv)
> >  {
> > @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct
> > drm_atomic_state *state)
> >  	struct drm_device *dev = state->dev;
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct intel_atomic_state *intel_state =
> > to_intel_atomic_state(state);
> > -	struct drm_crtc *crtc;
> > +	struct intel_crtc *crtc;
> > +	struct intel_plane *plane;
> >  	enum pipe pipe;
> > -	int level, plane;
> > +	int level, id, latency;
> >  
> >  	if (!intel_has_sagv(dev_priv))
> >  		return false;
> > @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct
> > drm_atomic_state *state)
> >  
> >  	/* Since we're now guaranteed to only have one active
> > CRTC... */
> >  	pipe = ffs(intel_state->active_crtcs) - 1;
> > -	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> > +	crtc = to_intel_crtc(dev_priv-
> > >pipe_to_crtc_mapping[pipe]);
> >  
> > -	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
> > +	if (crtc->base.state->mode.flags &
> > DRM_MODE_FLAG_INTERLACE)
> >  		return false;
> >  
> > -	for_each_plane(dev_priv, pipe, plane) {
> > +	for_each_intel_plane_on_crtc(dev, crtc, plane) {
> > +		id = skl_wm_plane_id(plane);
> > +
> >  		/* Skip this plane if it's not enabled */
> > -		if (intel_state->wm_results.plane[pipe][plane][0]
> > ==
> > 0)
> > +		if (intel_state->wm_results.plane[pipe][id][0] ==
> > 0)
> >  			continue;
> >  
> >  		/* Find the highest enabled wm level for this
> > plane
> > */
> >  		for (level = ilk_wm_max_level(dev);
> > -		     intel_state-
> > > 
> > > wm_results.plane[pipe][plane][level] == 0; --level)
> > +		     intel_state-
> > >wm_results.plane[pipe][id][level]
> > == 0; --level)
> >  		     { }
> >  
> > +		latency = dev_priv->wm.skl_latency[level];
> > +
> > +		if (intel_needs_memory_bw_wa(intel_state) &&
> > +		    plane->base.state->fb->modifier[0] ==
> > +		    I915_FORMAT_MOD_X_TILED)
> > +			latency += 15;
> > +
> >  		/*
> >  		 * If any of the planes on this pipe don't enable
> > wm
> > levels
> >  		 * that incur memory latencies higher then 30µs we
> > can't enable
> >  		 * the SAGV
> >  		 */
> > -		if (dev_priv->wm.skl_latency[level] <
> > SKL_SAGV_BLOCK_TIME)
> > +		if (latency < SKL_SAGV_BLOCK_TIME)
> >  			return false;
> >  	}
> >  
> > @@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const
> > struct
> > drm_i915_private *dev_priv,
> >  	uint32_t width = 0, height = 0;
> >  	uint32_t plane_pixel_rate;
> >  	uint32_t y_tile_minimum, y_min_scanlines;
> > +	struct intel_atomic_state *state =
> > +		to_intel_atomic_state(cstate->base.state);
> > +	bool apply_memory_bw_wa = intel_needs_memory_bw_wa(state);
> >  
> >  	if (latency == 0 || !cstate->base.active || !intel_pstate-
> > > 
> > > base.visible) {
> >  		*enabled = false;
> >  		return 0;
> >  	}
> >  
> > +	if (apply_memory_bw_wa && fb->modifier[0] ==
> > I915_FORMAT_MOD_X_TILED)
> > +		latency += 15;
> > +
> >  	width = drm_rect_width(&intel_pstate->base.src) >> 16;
> >  	height = drm_rect_height(&intel_pstate->base.src) >> 16;
> >  
> > @@ -3607,6 +3638,8 @@ static int skl_compute_plane_wm(const struct
> > drm_i915_private *dev_priv,
> >  				 plane_blocks_per_line);
> >  
> >  	y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
> > +	if (apply_memory_bw_wa)
> > +		y_tile_minimum *= 2;
> >  
> >  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> >  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-10 20:46   ` Zanoni, Paulo R
@ 2016-10-10 20:55       ` Lyude Paul
  0 siblings, 0 replies; 19+ messages in thread
From: Lyude Paul @ 2016-10-10 20:55 UTC (permalink / raw)
  To: Zanoni, Paulo R, intel-gfx; +Cc: Kumar, Mahesh1, stable, Pandiyan, Dhinakaran

I feel like the PRMs should be a little less confusing so it doesn't
take four of us reading it to actually get the full spec into code…

Anyway, while this is going to be running on more then just skylake I
feel like intel_needs_memory_bw_wa() is a little confusing since I
would expect eventually we're going to have to apply other unrelated
memory bandwidth workarounds on other platforms. How about
skl_needs_memory_bw_wa() or gen9_needs_memory_bw_wa()?

On Mon, 2016-10-10 at 20:46 +0000, Zanoni, Paulo R wrote:
> Em Seg, 2016-10-10 às 16:34 -0400, Lyude Paul escreveu:
> > 
> > Was there a new workaround added recently? Or was this something I
> > just
> > missed when writing the original code for this
> 
> It's listed on the public spec:
> https://01.org/sites/default/files/docum
> entation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> 
> Pages 210 - 211.
> 
> > 
> > 
> > On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote:
> > > 
> > > 
> > > Mahesh Kumar is already working on a proper implementation for
> > > the
> > > workaround, but while we still don't have it, let's just
> > > unconditionally apply the workaround for everybody and we hope we
> > > can
> > > close all those numerous bugzilla tickets. Also, I'm not sure how
> > > easy
> > > it will be to backport the final implementation to the stable
> > > Kernels,
> > > and this patch here is probably easier to backport.
> > > 
> > > At the present moment I still don't have confirmation that this
> > > patch
> > > fixes any of the bugs listed below, but we should definitely try
> > > testing all of them again.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
> > > Cc: stable@vger.kernel.org
> > > Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> > > Cc: Lyude <cpaul@redhat.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 49
> > > ++++++++++++++++++++++++++++++++++-------
> > >  1 file changed, 41 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 62d730d..159831d 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane
> > > *plane)
> > >  	}
> > >  }
> > >  
> > > +/*
> > > + * 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 intel_needs_memory_bw_wa(struct intel_atomic_state
> > > *state)
> > > +{
> > > +	struct drm_i915_private *dev_priv = to_i915(state-
> > > > 
> > > > 
> > > > base.dev);
> > > +
> > > +	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> > > +	    IS_KABYLAKE(dev_priv))
> > > +		return true;
> > > +
> > > +	return false;
> > > +}
> > > +
> > >  static bool
> > >  intel_has_sagv(struct drm_i915_private *dev_priv)
> > >  {
> > > @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct
> > > drm_atomic_state *state)
> > >  	struct drm_device *dev = state->dev;
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > >  	struct intel_atomic_state *intel_state =
> > > to_intel_atomic_state(state);
> > > -	struct drm_crtc *crtc;
> > > +	struct intel_crtc *crtc;
> > > +	struct intel_plane *plane;
> > >  	enum pipe pipe;
> > > -	int level, plane;
> > > +	int level, id, latency;
> > >  
> > >  	if (!intel_has_sagv(dev_priv))
> > >  		return false;
> > > @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct
> > > drm_atomic_state *state)
> > >  
> > >  	/* Since we're now guaranteed to only have one active
> > > CRTC... */
> > >  	pipe = ffs(intel_state->active_crtcs) - 1;
> > > -	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> > > +	crtc = to_intel_crtc(dev_priv-
> > > > 
> > > > pipe_to_crtc_mapping[pipe]);
> > >  
> > > -	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
> > > +	if (crtc->base.state->mode.flags &
> > > DRM_MODE_FLAG_INTERLACE)
> > >  		return false;
> > >  
> > > -	for_each_plane(dev_priv, pipe, plane) {
> > > +	for_each_intel_plane_on_crtc(dev, crtc, plane) {
> > > +		id = skl_wm_plane_id(plane);
> > > +
> > >  		/* Skip this plane if it's not enabled */
> > > -		if (intel_state-
> > > >wm_results.plane[pipe][plane][0]
> > > ==
> > > 0)
> > > +		if (intel_state->wm_results.plane[pipe][id][0]
> > > ==
> > > 0)
> > >  			continue;
> > >  
> > >  		/* Find the highest enabled wm level for this
> > > plane
> > > */
> > >  		for (level = ilk_wm_max_level(dev);
> > > -		     intel_state-
> > > > 
> > > > 
> > > > wm_results.plane[pipe][plane][level] == 0; --level)
> > > +		     intel_state-
> > > > 
> > > > wm_results.plane[pipe][id][level]
> > > == 0; --level)
> > >  		     { }
> > >  
> > > +		latency = dev_priv->wm.skl_latency[level];
> > > +
> > > +		if (intel_needs_memory_bw_wa(intel_state) &&
> > > +		    plane->base.state->fb->modifier[0] ==
> > > +		    I915_FORMAT_MOD_X_TILED)
> > > +			latency += 15;
> > > +
> > >  		/*
> > >  		 * If any of the planes on this pipe don't
> > > enable
> > > wm
> > > levels
> > >  		 * that incur memory latencies higher then 30µs
> > > we
> > > can't enable
> > >  		 * the SAGV
> > >  		 */
> > > -		if (dev_priv->wm.skl_latency[level] <
> > > SKL_SAGV_BLOCK_TIME)
> > > +		if (latency < SKL_SAGV_BLOCK_TIME)
> > >  			return false;
> > >  	}
> > >  
> > > @@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const
> > > struct
> > > drm_i915_private *dev_priv,
> > >  	uint32_t width = 0, height = 0;
> > >  	uint32_t plane_pixel_rate;
> > >  	uint32_t y_tile_minimum, y_min_scanlines;
> > > +	struct intel_atomic_state *state =
> > > +		to_intel_atomic_state(cstate->base.state);
> > > +	bool apply_memory_bw_wa =
> > > intel_needs_memory_bw_wa(state);
> > >  
> > >  	if (latency == 0 || !cstate->base.active ||
> > > !intel_pstate-
> > > > 
> > > > 
> > > > base.visible) {
> > >  		*enabled = false;
> > >  		return 0;
> > >  	}
> > >  
> > > +	if (apply_memory_bw_wa && fb->modifier[0] ==
> > > I915_FORMAT_MOD_X_TILED)
> > > +		latency += 15;
> > > +
> > >  	width = drm_rect_width(&intel_pstate->base.src) >> 16;
> > >  	height = drm_rect_height(&intel_pstate->base.src) >> 16;
> > >  
> > > @@ -3607,6 +3638,8 @@ static int skl_compute_plane_wm(const
> > > struct
> > > drm_i915_private *dev_priv,
> > >  				 plane_blocks_per_line);
> > >  
> > >  	y_tile_minimum = plane_blocks_per_line *
> > > y_min_scanlines;
> > > +	if (apply_memory_bw_wa)
> > > +		y_tile_minimum *= 2;
> > >  
> > >  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> > >  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
@ 2016-10-10 20:55       ` Lyude Paul
  0 siblings, 0 replies; 19+ messages in thread
From: Lyude Paul @ 2016-10-10 20:55 UTC (permalink / raw)
  To: Zanoni, Paulo R, intel-gfx; +Cc: Pandiyan, Dhinakaran, stable

I feel like the PRMs should be a little less confusing so it doesn't
take four of us reading it to actually get the full spec into code…

Anyway, while this is going to be running on more then just skylake I
feel like intel_needs_memory_bw_wa() is a little confusing since I
would expect eventually we're going to have to apply other unrelated
memory bandwidth workarounds on other platforms. How about
skl_needs_memory_bw_wa() or gen9_needs_memory_bw_wa()?

On Mon, 2016-10-10 at 20:46 +0000, Zanoni, Paulo R wrote:
> Em Seg, 2016-10-10 às 16:34 -0400, Lyude Paul escreveu:
> > 
> > Was there a new workaround added recently? Or was this something I
> > just
> > missed when writing the original code for this
> 
> It's listed on the public spec:
> https://01.org/sites/default/files/docum
> entation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> 
> Pages 210 - 211.
> 
> > 
> > 
> > On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote:
> > > 
> > > 
> > > Mahesh Kumar is already working on a proper implementation for
> > > the
> > > workaround, but while we still don't have it, let's just
> > > unconditionally apply the workaround for everybody and we hope we
> > > can
> > > close all those numerous bugzilla tickets. Also, I'm not sure how
> > > easy
> > > it will be to backport the final implementation to the stable
> > > Kernels,
> > > and this patch here is probably easier to backport.
> > > 
> > > At the present moment I still don't have confirmation that this
> > > patch
> > > fixes any of the bugs listed below, but we should definitely try
> > > testing all of them again.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
> > > Cc: stable@vger.kernel.org
> > > Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> > > Cc: Lyude <cpaul@redhat.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 49
> > > ++++++++++++++++++++++++++++++++++-------
> > >  1 file changed, 41 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 62d730d..159831d 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane
> > > *plane)
> > >  	}
> > >  }
> > >  
> > > +/*
> > > + * 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 intel_needs_memory_bw_wa(struct intel_atomic_state
> > > *state)
> > > +{
> > > +	struct drm_i915_private *dev_priv = to_i915(state-
> > > > 
> > > > 
> > > > base.dev);
> > > +
> > > +	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> > > +	    IS_KABYLAKE(dev_priv))
> > > +		return true;
> > > +
> > > +	return false;
> > > +}
> > > +
> > >  static bool
> > >  intel_has_sagv(struct drm_i915_private *dev_priv)
> > >  {
> > > @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct
> > > drm_atomic_state *state)
> > >  	struct drm_device *dev = state->dev;
> > >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > >  	struct intel_atomic_state *intel_state =
> > > to_intel_atomic_state(state);
> > > -	struct drm_crtc *crtc;
> > > +	struct intel_crtc *crtc;
> > > +	struct intel_plane *plane;
> > >  	enum pipe pipe;
> > > -	int level, plane;
> > > +	int level, id, latency;
> > >  
> > >  	if (!intel_has_sagv(dev_priv))
> > >  		return false;
> > > @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct
> > > drm_atomic_state *state)
> > >  
> > >  	/* Since we're now guaranteed to only have one active
> > > CRTC... */
> > >  	pipe = ffs(intel_state->active_crtcs) - 1;
> > > -	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> > > +	crtc = to_intel_crtc(dev_priv-
> > > > 
> > > > pipe_to_crtc_mapping[pipe]);
> > >  
> > > -	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
> > > +	if (crtc->base.state->mode.flags &
> > > DRM_MODE_FLAG_INTERLACE)
> > >  		return false;
> > >  
> > > -	for_each_plane(dev_priv, pipe, plane) {
> > > +	for_each_intel_plane_on_crtc(dev, crtc, plane) {
> > > +		id = skl_wm_plane_id(plane);
> > > +
> > >  		/* Skip this plane if it's not enabled */
> > > -		if (intel_state-
> > > >wm_results.plane[pipe][plane][0]
> > > ==
> > > 0)
> > > +		if (intel_state->wm_results.plane[pipe][id][0]
> > > ==
> > > 0)
> > >  			continue;
> > >  
> > >  		/* Find the highest enabled wm level for this
> > > plane
> > > */
> > >  		for (level = ilk_wm_max_level(dev);
> > > -		     intel_state-
> > > > 
> > > > 
> > > > wm_results.plane[pipe][plane][level] == 0; --level)
> > > +		     intel_state-
> > > > 
> > > > wm_results.plane[pipe][id][level]
> > > == 0; --level)
> > >  		     { }
> > >  
> > > +		latency = dev_priv->wm.skl_latency[level];
> > > +
> > > +		if (intel_needs_memory_bw_wa(intel_state) &&
> > > +		    plane->base.state->fb->modifier[0] ==
> > > +		    I915_FORMAT_MOD_X_TILED)
> > > +			latency += 15;
> > > +
> > >  		/*
> > >  		 * If any of the planes on this pipe don't
> > > enable
> > > wm
> > > levels
> > >  		 * that incur memory latencies higher then 30µs
> > > we
> > > can't enable
> > >  		 * the SAGV
> > >  		 */
> > > -		if (dev_priv->wm.skl_latency[level] <
> > > SKL_SAGV_BLOCK_TIME)
> > > +		if (latency < SKL_SAGV_BLOCK_TIME)
> > >  			return false;
> > >  	}
> > >  
> > > @@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const
> > > struct
> > > drm_i915_private *dev_priv,
> > >  	uint32_t width = 0, height = 0;
> > >  	uint32_t plane_pixel_rate;
> > >  	uint32_t y_tile_minimum, y_min_scanlines;
> > > +	struct intel_atomic_state *state =
> > > +		to_intel_atomic_state(cstate->base.state);
> > > +	bool apply_memory_bw_wa =
> > > intel_needs_memory_bw_wa(state);
> > >  
> > >  	if (latency == 0 || !cstate->base.active ||
> > > !intel_pstate-
> > > > 
> > > > 
> > > > base.visible) {
> > >  		*enabled = false;
> > >  		return 0;
> > >  	}
> > >  
> > > +	if (apply_memory_bw_wa && fb->modifier[0] ==
> > > I915_FORMAT_MOD_X_TILED)
> > > +		latency += 15;
> > > +
> > >  	width = drm_rect_width(&intel_pstate->base.src) >> 16;
> > >  	height = drm_rect_height(&intel_pstate->base.src) >> 16;
> > >  
> > > @@ -3607,6 +3638,8 @@ static int skl_compute_plane_wm(const
> > > struct
> > > drm_i915_private *dev_priv,
> > >  				 plane_blocks_per_line);
> > >  
> > >  	y_tile_minimum = plane_blocks_per_line *
> > > y_min_scanlines;
> > > +	if (apply_memory_bw_wa)
> > > +		y_tile_minimum *= 2;
> > >  
> > >  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> > >  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced
  2016-10-10 20:30 ` [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced Paulo Zanoni
@ 2016-10-10 20:55   ` Lyude Paul
  0 siblings, 0 replies; 19+ messages in thread
From: Lyude Paul @ 2016-10-10 20:55 UTC (permalink / raw)
  To: Paulo Zanoni, intel-gfx

Reviewed-by: Lyude <cpaul@redhat.com>

On Mon, 2016-10-10 at 17:30 -0300, Paulo Zanoni wrote:
> We want to look at the mode that we're actually going to set. All the
> other display checks for interlaced flags also look at adjusted_mode.
> 
> Cc: Lyude <cpaul@redhat.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@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 159831d..4b7de7a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3037,7 +3037,7 @@ bool intel_can_enable_sagv(struct
> drm_atomic_state *state)
>  	pipe = ffs(intel_state->active_crtcs) - 1;
>  	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>  
> -	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
> +	if (crtc->base.state->adjusted_mode.flags &
> DRM_MODE_FLAG_INTERLACE)
>  		return false;
>  
>  	for_each_intel_plane_on_crtc(dev, crtc, plane) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-10 20:30 [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Paulo Zanoni
  2016-10-10 20:30 ` [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced Paulo Zanoni
  2016-10-10 20:34 ` [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Lyude Paul
@ 2016-10-10 21:19 ` Patchwork
  2016-10-11  7:34   ` Jani Nikula
  2016-10-11 19:19 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA (rev2) Patchwork
  4 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2016-10-10 21:19 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
URL   : https://patchwork.freedesktop.org/series/13548/
State : warning

== Summary ==

Series 13548v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/13548/revisions/1/mbox/

Test drv_module_reload_basic:
                pass       -> SKIP       (fi-ivb-3520m)
Test kms_busy:
        Subgroup basic-flip-default-a:
                pass       -> DMESG-WARN (fi-ilk-650)
Test vgem_basic:
        Subgroup unload:
                pass       -> SKIP       (fi-ilk-650)

fi-bdw-5557u     total:248  pass:231  dwarn:0   dfail:0   fail:0   skip:17 
fi-bsw-n3050     total:248  pass:204  dwarn:0   dfail:0   fail:0   skip:44 
fi-bxt-t5700     total:248  pass:217  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-j1900     total:248  pass:212  dwarn:0   dfail:0   fail:3   skip:33 
fi-byt-n2820     total:248  pass:211  dwarn:0   dfail:0   fail:1   skip:36 
fi-hsw-4770      total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-hsw-4770r     total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-ilk-650       total:248  pass:183  dwarn:1   dfail:0   fail:2   skip:62 
fi-ivb-3520m     total:248  pass:220  dwarn:0   dfail:0   fail:0   skip:28 
fi-ivb-3770      total:248  pass:207  dwarn:0   dfail:0   fail:0   skip:41 
fi-kbl-7200u     total:248  pass:222  dwarn:0   dfail:0   fail:0   skip:26 
fi-skl-6260u     total:248  pass:232  dwarn:0   dfail:0   fail:0   skip:16 
fi-skl-6700hq    total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6700k     total:248  pass:221  dwarn:1   dfail:0   fail:0   skip:26 
fi-skl-6770hq    total:248  pass:231  dwarn:1   dfail:0   fail:1   skip:15 
fi-snb-2520m     total:248  pass:211  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600      total:248  pass:209  dwarn:0   dfail:0   fail:0   skip:39 

Results at /archive/results/CI_IGT_test/Patchwork_2665/

e37a15c8d775e79dddc8345a0f6afdcfe1f607d9 drm-intel-nightly: 2016y-10m-10d-14h-33m-29s UTC integration manifest
6f2eff9 drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced
01581cd drm/i915/gen9: unconditionally apply the memory bandwidth WA

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the   memory bandwidth WA
  2016-10-10 20:30 [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Paulo Zanoni
@ 2016-10-11  7:34   ` Jani Nikula
  2016-10-10 20:34 ` [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Lyude Paul
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2016-10-11  7:34 UTC (permalink / raw)
  To: Paulo Zanoni, intel-gfx; +Cc: Paulo Zanoni, stable, Dhinakaran Pandiyan

On Mon, 10 Oct 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Mahesh Kumar is already working on a proper implementation for the
> workaround, but while we still don't have it, let's just
> unconditionally apply the workaround for everybody and we hope we can
> close all those numerous bugzilla tickets. Also, I'm not sure how easy
> it will be to backport the final implementation to the stable Kernels,
> and this patch here is probably easier to backport.
>
> At the present moment I still don't have confirmation that this patch
> fixes any of the bugs listed below, but we should definitely try
> testing all of them again.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
> Cc: stable@vger.kernel.org

The patch is a bit on the large side for stable. 100 lines with context
is the rule.

BR,
Jani.


> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 62d730d..159831d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane *plane)
>  	}
>  }
>  
> +/*
> + * 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 intel_needs_memory_bw_wa(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +
> +	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> +	    IS_KABYLAKE(dev_priv))
> +		return true;
> +
> +	return false;
> +}
> +
>  static bool
>  intel_has_sagv(struct drm_i915_private *dev_priv)
>  {
> @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>  	struct drm_device *dev = state->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> -	struct drm_crtc *crtc;
> +	struct intel_crtc *crtc;
> +	struct intel_plane *plane;
>  	enum pipe pipe;
> -	int level, plane;
> +	int level, id, latency;
>  
>  	if (!intel_has_sagv(dev_priv))
>  		return false;
> @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>  
>  	/* Since we're now guaranteed to only have one active CRTC... */
>  	pipe = ffs(intel_state->active_crtcs) - 1;
> -	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>  
> -	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
> +	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		return false;
>  
> -	for_each_plane(dev_priv, pipe, plane) {
> +	for_each_intel_plane_on_crtc(dev, crtc, plane) {
> +		id = skl_wm_plane_id(plane);
> +
>  		/* Skip this plane if it's not enabled */
> -		if (intel_state->wm_results.plane[pipe][plane][0] == 0)
> +		if (intel_state->wm_results.plane[pipe][id][0] == 0)
>  			continue;
>  
>  		/* Find the highest enabled wm level for this plane */
>  		for (level = ilk_wm_max_level(dev);
> -		     intel_state->wm_results.plane[pipe][plane][level] == 0; --level)
> +		     intel_state->wm_results.plane[pipe][id][level] == 0; --level)
>  		     { }
>  
> +		latency = dev_priv->wm.skl_latency[level];
> +
> +		if (intel_needs_memory_bw_wa(intel_state) &&
> +		    plane->base.state->fb->modifier[0] ==
> +		    I915_FORMAT_MOD_X_TILED)
> +			latency += 15;
> +
>  		/*
>  		 * If any of the planes on this pipe don't enable wm levels
>  		 * that incur memory latencies higher then 30µs we can't enable
>  		 * the SAGV
>  		 */
> -		if (dev_priv->wm.skl_latency[level] < SKL_SAGV_BLOCK_TIME)
> +		if (latency < SKL_SAGV_BLOCK_TIME)
>  			return false;
>  	}
>  
> @@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	uint32_t width = 0, height = 0;
>  	uint32_t plane_pixel_rate;
>  	uint32_t y_tile_minimum, y_min_scanlines;
> +	struct intel_atomic_state *state =
> +		to_intel_atomic_state(cstate->base.state);
> +	bool apply_memory_bw_wa = intel_needs_memory_bw_wa(state);
>  
>  	if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
>  		*enabled = false;
>  		return 0;
>  	}
>  
> +	if (apply_memory_bw_wa && fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
> +		latency += 15;
> +
>  	width = drm_rect_width(&intel_pstate->base.src) >> 16;
>  	height = drm_rect_height(&intel_pstate->base.src) >> 16;
>  
> @@ -3607,6 +3638,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  				 plane_blocks_per_line);
>  
>  	y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
> +	if (apply_memory_bw_wa)
> +		y_tile_minimum *= 2;
>  
>  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the   memory bandwidth WA
@ 2016-10-11  7:34   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2016-10-11  7:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, stable, Dhinakaran Pandiyan

On Mon, 10 Oct 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> Mahesh Kumar is already working on a proper implementation for the
> workaround, but while we still don't have it, let's just
> unconditionally apply the workaround for everybody and we hope we can
> close all those numerous bugzilla tickets. Also, I'm not sure how easy
> it will be to backport the final implementation to the stable Kernels,
> and this patch here is probably easier to backport.
>
> At the present moment I still don't have confirmation that this patch
> fixes any of the bugs listed below, but we should definitely try
> testing all of them again.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
> Cc: stable@vger.kernel.org

The patch is a bit on the large side for stable. 100 lines with context
is the rule.

BR,
Jani.


> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 62d730d..159831d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane *plane)
>  	}
>  }
>  
> +/*
> + * 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 intel_needs_memory_bw_wa(struct intel_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +
> +	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> +	    IS_KABYLAKE(dev_priv))
> +		return true;
> +
> +	return false;
> +}
> +
>  static bool
>  intel_has_sagv(struct drm_i915_private *dev_priv)
>  {
> @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>  	struct drm_device *dev = state->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> -	struct drm_crtc *crtc;
> +	struct intel_crtc *crtc;
> +	struct intel_plane *plane;
>  	enum pipe pipe;
> -	int level, plane;
> +	int level, id, latency;
>  
>  	if (!intel_has_sagv(dev_priv))
>  		return false;
> @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
>  
>  	/* Since we're now guaranteed to only have one active CRTC... */
>  	pipe = ffs(intel_state->active_crtcs) - 1;
> -	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>  
> -	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
> +	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		return false;
>  
> -	for_each_plane(dev_priv, pipe, plane) {
> +	for_each_intel_plane_on_crtc(dev, crtc, plane) {
> +		id = skl_wm_plane_id(plane);
> +
>  		/* Skip this plane if it's not enabled */
> -		if (intel_state->wm_results.plane[pipe][plane][0] == 0)
> +		if (intel_state->wm_results.plane[pipe][id][0] == 0)
>  			continue;
>  
>  		/* Find the highest enabled wm level for this plane */
>  		for (level = ilk_wm_max_level(dev);
> -		     intel_state->wm_results.plane[pipe][plane][level] == 0; --level)
> +		     intel_state->wm_results.plane[pipe][id][level] == 0; --level)
>  		     { }
>  
> +		latency = dev_priv->wm.skl_latency[level];
> +
> +		if (intel_needs_memory_bw_wa(intel_state) &&
> +		    plane->base.state->fb->modifier[0] ==
> +		    I915_FORMAT_MOD_X_TILED)
> +			latency += 15;
> +
>  		/*
>  		 * If any of the planes on this pipe don't enable wm levels
>  		 * that incur memory latencies higher then 30µs we can't enable
>  		 * the SAGV
>  		 */
> -		if (dev_priv->wm.skl_latency[level] < SKL_SAGV_BLOCK_TIME)
> +		if (latency < SKL_SAGV_BLOCK_TIME)
>  			return false;
>  	}
>  
> @@ -3549,12 +3574,18 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	uint32_t width = 0, height = 0;
>  	uint32_t plane_pixel_rate;
>  	uint32_t y_tile_minimum, y_min_scanlines;
> +	struct intel_atomic_state *state =
> +		to_intel_atomic_state(cstate->base.state);
> +	bool apply_memory_bw_wa = intel_needs_memory_bw_wa(state);
>  
>  	if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
>  		*enabled = false;
>  		return 0;
>  	}
>  
> +	if (apply_memory_bw_wa && fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
> +		latency += 15;
> +
>  	width = drm_rect_width(&intel_pstate->base.src) >> 16;
>  	height = drm_rect_height(&intel_pstate->base.src) >> 16;
>  
> @@ -3607,6 +3638,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  				 plane_blocks_per_line);
>  
>  	y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
> +	if (apply_memory_bw_wa)
> +		y_tile_minimum *= 2;
>  
>  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-11  7:34   ` Jani Nikula
  (?)
@ 2016-10-11 10:31   ` Greg KH
  2016-10-11 10:54       ` Jani Nikula
  -1 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2016-10-11 10:31 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Paulo Zanoni, intel-gfx, stable, Dhinakaran Pandiyan

On Tue, Oct 11, 2016 at 10:34:14AM +0300, Jani Nikula wrote:
> On Mon, 10 Oct 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> > Mahesh Kumar is already working on a proper implementation for the
> > workaround, but while we still don't have it, let's just
> > unconditionally apply the workaround for everybody and we hope we can
> > close all those numerous bugzilla tickets. Also, I'm not sure how easy
> > it will be to backport the final implementation to the stable Kernels,
> > and this patch here is probably easier to backport.
> >
> > At the present moment I still don't have confirmation that this patch
> > fixes any of the bugs listed below, but we should definitely try
> > testing all of them again.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
> > Cc: stable@vger.kernel.org
> 
> The patch is a bit on the large side for stable. 100 lines with context
> is the rule.

Huh?  It's only 49 line of changes:

> >  drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 41 insertions(+), 8 deletions(-)

That's fine for stable, we take i915 stable patches much bigger than
that all the time :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-11 10:31   ` Greg KH
@ 2016-10-11 10:54       ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2016-10-11 10:54 UTC (permalink / raw)
  To: Greg KH; +Cc: Paulo Zanoni, intel-gfx, stable, Dhinakaran Pandiyan

On Tue, 11 Oct 2016, Greg KH <greg@kroah.com> wrote:
> On Tue, Oct 11, 2016 at 10:34:14AM +0300, Jani Nikula wrote:
>> On Mon, 10 Oct 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
>> The patch is a bit on the large side for stable. 100 lines with context
>> is the rule.
>
> Huh?  It's only 49 line of changes:
>
>> >  drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
>> >  1 file changed, 41 insertions(+), 8 deletions(-)
>
> That's fine for stable, we take i915 stable patches much bigger than
> that all the time :)

Oh, I thought the rule was "100 lines, with context", but I certainly
won't argue! Never mind! ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
@ 2016-10-11 10:54       ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2016-10-11 10:54 UTC (permalink / raw)
  To: Greg KH; +Cc: intel-gfx, Paulo Zanoni, stable, Dhinakaran Pandiyan

On Tue, 11 Oct 2016, Greg KH <greg@kroah.com> wrote:
> On Tue, Oct 11, 2016 at 10:34:14AM +0300, Jani Nikula wrote:
>> On Mon, 10 Oct 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
>> The patch is a bit on the large side for stable. 100 lines with context
>> is the rule.
>
> Huh?  It's only 49 line of changes:
>
>> >  drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
>> >  1 file changed, 41 insertions(+), 8 deletions(-)
>
> That's fine for stable, we take i915 stable patches much bigger than
> that all the time :)

Oh, I thought the rule was "100 lines, with context", but I certainly
won't argue! Never mind! ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-11 10:54       ` Jani Nikula
@ 2016-10-11 11:10         ` Greg KH
  -1 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2016-10-11 11:10 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Paulo Zanoni, intel-gfx, stable, Dhinakaran Pandiyan

On Tue, Oct 11, 2016 at 01:54:09PM +0300, Jani Nikula wrote:
> On Tue, 11 Oct 2016, Greg KH <greg@kroah.com> wrote:
> > On Tue, Oct 11, 2016 at 10:34:14AM +0300, Jani Nikula wrote:
> >> On Mon, 10 Oct 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> >> The patch is a bit on the large side for stable. 100 lines with context
> >> is the rule.
> >
> > Huh?  It's only 49 line of changes:
> >
> >> >  drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
> >> >  1 file changed, 41 insertions(+), 8 deletions(-)
> >
> > That's fine for stable, we take i915 stable patches much bigger than
> > that all the time :)
> 
> Oh, I thought the rule was "100 lines, with context", but I certainly
> won't argue! Never mind! ;)

It's the "official" rule, yes, but really, context of the patch itself
(i.e. what it does), is the key thing.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
@ 2016-10-11 11:10         ` Greg KH
  0 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2016-10-11 11:10 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Paulo Zanoni, stable, Dhinakaran Pandiyan

On Tue, Oct 11, 2016 at 01:54:09PM +0300, Jani Nikula wrote:
> On Tue, 11 Oct 2016, Greg KH <greg@kroah.com> wrote:
> > On Tue, Oct 11, 2016 at 10:34:14AM +0300, Jani Nikula wrote:
> >> On Mon, 10 Oct 2016, Paulo Zanoni <paulo.r.zanoni@intel.com> wrote:
> >> The patch is a bit on the large side for stable. 100 lines with context
> >> is the rule.
> >
> > Huh?  It's only 49 line of changes:
> >
> >> >  drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
> >> >  1 file changed, 41 insertions(+), 8 deletions(-)
> >
> > That's fine for stable, we take i915 stable patches much bigger than
> > that all the time :)
> 
> Oh, I thought the rule was "100 lines, with context", but I certainly
> won't argue! Never mind! ;)

It's the "official" rule, yes, but really, context of the patch itself
(i.e. what it does), is the key thing.

thanks,

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-10 20:55       ` Lyude Paul
@ 2016-10-11 18:25         ` Paulo Zanoni
  -1 siblings, 0 replies; 19+ messages in thread
From: Paulo Zanoni @ 2016-10-11 18:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, stable, Mahesh Kumar, Lyude, Dhinakaran Pandiyan

Mahesh Kumar is already working on a proper implementation for the
workaround, but while we still don't have it, let's just
unconditionally apply the workaround for everybody and we hope we can
close all those numerous bugzilla tickets. Also, I'm not sure how easy
it will be to backport the final implementation to the stable Kernels,
and this patch here is probably easier to backport.

At the present moment I still don't have confirmation that this patch
fixes any of the bugs listed below, but we should definitely try
testing all of them again.

v2: s/intel_needs_memory_bw_wa/skl_needs_memory_bw_wa/ (Lyude).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
Cc: stable@vger.kernel.org
Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Lyude <cpaul@redhat.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fe6c1c6..13bd974 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane *plane)
 	}
 }
 
+/*
+ * 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);
+
+	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
+	    IS_KABYLAKE(dev_priv))
+		return true;
+
+	return false;
+}
+
 static bool
 intel_has_sagv(struct drm_i915_private *dev_priv)
 {
@@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	struct drm_crtc *crtc;
+	struct intel_crtc *crtc;
+	struct intel_plane *plane;
 	enum pipe pipe;
-	int level, plane;
+	int level, id, latency;
 
 	if (!intel_has_sagv(dev_priv))
 		return false;
@@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 
 	/* Since we're now guaranteed to only have one active CRTC... */
 	pipe = ffs(intel_state->active_crtcs) - 1;
-	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 
-	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
+	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
 		return false;
 
-	for_each_plane(dev_priv, pipe, plane) {
+	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+		id = skl_wm_plane_id(plane);
+
 		/* Skip this plane if it's not enabled */
-		if (intel_state->wm_results.plane[pipe][plane][0] == 0)
+		if (intel_state->wm_results.plane[pipe][id][0] == 0)
 			continue;
 
 		/* Find the highest enabled wm level for this plane */
 		for (level = ilk_wm_max_level(dev);
-		     intel_state->wm_results.plane[pipe][plane][level] == 0; --level)
+		     intel_state->wm_results.plane[pipe][id][level] == 0; --level)
 		     { }
 
+		latency = dev_priv->wm.skl_latency[level];
+
+		if (skl_needs_memory_bw_wa(intel_state) &&
+		    plane->base.state->fb->modifier[0] ==
+		    I915_FORMAT_MOD_X_TILED)
+			latency += 15;
+
 		/*
 		 * If any of the planes on this pipe don't enable wm levels
 		 * that incur memory latencies higher then 30µs we can't enable
 		 * the SAGV
 		 */
-		if (dev_priv->wm.skl_latency[level] < SKL_SAGV_BLOCK_TIME)
+		if (latency < SKL_SAGV_BLOCK_TIME)
 			return false;
 	}
 
@@ -3555,12 +3580,18 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	uint32_t width = 0, height = 0;
 	uint32_t plane_pixel_rate;
 	uint32_t y_tile_minimum, 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);
 
 	if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
 		*enabled = false;
 		return 0;
 	}
 
+	if (apply_memory_bw_wa && fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
+		latency += 15;
+
 	width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	height = drm_rect_height(&intel_pstate->base.src) >> 16;
 
@@ -3613,6 +3644,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				 plane_blocks_per_line);
 
 	y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
+	if (apply_memory_bw_wa)
+		y_tile_minimum *= 2;
 
 	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
@ 2016-10-11 18:25         ` Paulo Zanoni
  0 siblings, 0 replies; 19+ messages in thread
From: Paulo Zanoni @ 2016-10-11 18:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, stable, Dhinakaran Pandiyan

Mahesh Kumar is already working on a proper implementation for the
workaround, but while we still don't have it, let's just
unconditionally apply the workaround for everybody and we hope we can
close all those numerous bugzilla tickets. Also, I'm not sure how easy
it will be to backport the final implementation to the stable Kernels,
and this patch here is probably easier to backport.

At the present moment I still don't have confirmation that this patch
fixes any of the bugs listed below, but we should definitely try
testing all of them again.

v2: s/intel_needs_memory_bw_wa/skl_needs_memory_bw_wa/ (Lyude).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
Cc: stable@vger.kernel.org
Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: Lyude <cpaul@redhat.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 49 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fe6c1c6..13bd974 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane *plane)
 	}
 }
 
+/*
+ * 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);
+
+	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
+	    IS_KABYLAKE(dev_priv))
+		return true;
+
+	return false;
+}
+
 static bool
 intel_has_sagv(struct drm_i915_private *dev_priv)
 {
@@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	struct drm_crtc *crtc;
+	struct intel_crtc *crtc;
+	struct intel_plane *plane;
 	enum pipe pipe;
-	int level, plane;
+	int level, id, latency;
 
 	if (!intel_has_sagv(dev_priv))
 		return false;
@@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct drm_atomic_state *state)
 
 	/* Since we're now guaranteed to only have one active CRTC... */
 	pipe = ffs(intel_state->active_crtcs) - 1;
-	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 
-	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
+	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
 		return false;
 
-	for_each_plane(dev_priv, pipe, plane) {
+	for_each_intel_plane_on_crtc(dev, crtc, plane) {
+		id = skl_wm_plane_id(plane);
+
 		/* Skip this plane if it's not enabled */
-		if (intel_state->wm_results.plane[pipe][plane][0] == 0)
+		if (intel_state->wm_results.plane[pipe][id][0] == 0)
 			continue;
 
 		/* Find the highest enabled wm level for this plane */
 		for (level = ilk_wm_max_level(dev);
-		     intel_state->wm_results.plane[pipe][plane][level] == 0; --level)
+		     intel_state->wm_results.plane[pipe][id][level] == 0; --level)
 		     { }
 
+		latency = dev_priv->wm.skl_latency[level];
+
+		if (skl_needs_memory_bw_wa(intel_state) &&
+		    plane->base.state->fb->modifier[0] ==
+		    I915_FORMAT_MOD_X_TILED)
+			latency += 15;
+
 		/*
 		 * If any of the planes on this pipe don't enable wm levels
 		 * that incur memory latencies higher then 30µs we can't enable
 		 * the SAGV
 		 */
-		if (dev_priv->wm.skl_latency[level] < SKL_SAGV_BLOCK_TIME)
+		if (latency < SKL_SAGV_BLOCK_TIME)
 			return false;
 	}
 
@@ -3555,12 +3580,18 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 	uint32_t width = 0, height = 0;
 	uint32_t plane_pixel_rate;
 	uint32_t y_tile_minimum, 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);
 
 	if (latency == 0 || !cstate->base.active || !intel_pstate->base.visible) {
 		*enabled = false;
 		return 0;
 	}
 
+	if (apply_memory_bw_wa && fb->modifier[0] == I915_FORMAT_MOD_X_TILED)
+		latency += 15;
+
 	width = drm_rect_width(&intel_pstate->base.src) >> 16;
 	height = drm_rect_height(&intel_pstate->base.src) >> 16;
 
@@ -3613,6 +3644,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
 				 plane_blocks_per_line);
 
 	y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
+	if (apply_memory_bw_wa)
+		y_tile_minimum *= 2;
 
 	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA (rev2)
  2016-10-10 20:30 [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Paulo Zanoni
                   ` (3 preceding siblings ...)
  2016-10-11  7:34   ` Jani Nikula
@ 2016-10-11 19:19 ` Patchwork
  4 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2016-10-11 19:19 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA (rev2)
URL   : https://patchwork.freedesktop.org/series/13548/
State : warning

== Summary ==

Series 13548v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/13548/revisions/2/mbox/

Test kms_psr_sink_crc:
        Subgroup psr_basic:
                pass       -> DMESG-WARN (fi-skl-6700hq)
Test vgem_basic:
        Subgroup unload:
                pass       -> SKIP       (fi-ilk-650)
                skip       -> PASS       (fi-skl-6700k)

fi-bdw-5557u     total:248  pass:231  dwarn:0   dfail:0   fail:0   skip:17 
fi-bsw-n3050     total:248  pass:204  dwarn:0   dfail:0   fail:0   skip:44 
fi-bxt-t5700     total:248  pass:217  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-j1900     total:248  pass:214  dwarn:1   dfail:0   fail:1   skip:32 
fi-byt-n2820     total:248  pass:211  dwarn:0   dfail:0   fail:1   skip:36 
fi-hsw-4770      total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-hsw-4770r     total:248  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-ilk-650       total:248  pass:184  dwarn:0   dfail:0   fail:2   skip:62 
fi-ivb-3520m     total:248  pass:221  dwarn:0   dfail:0   fail:0   skip:27 
fi-ivb-3770      total:248  pass:222  dwarn:0   dfail:0   fail:0   skip:26 
fi-kbl-7200u     total:248  pass:222  dwarn:0   dfail:0   fail:0   skip:26 
fi-skl-6700hq    total:248  pass:223  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6700k     total:248  pass:222  dwarn:1   dfail:0   fail:0   skip:25 
fi-snb-2520m     total:248  pass:211  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600      total:248  pass:209  dwarn:0   dfail:0   fail:0   skip:39 

Results at /archive/results/CI_IGT_test/Patchwork_2677/

41409515b7b3365bcb2c2e9239fdfaa286a51333 drm-intel-nightly: 2016y-10m-11d-17h-55m-34s UTC integration manifest
85ad389 drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced
604d41f drm/i915/gen9: unconditionally apply the memory bandwidth WA

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA
  2016-10-11 18:25         ` Paulo Zanoni
  (?)
@ 2016-10-14 21:45         ` Lyude
  -1 siblings, 0 replies; 19+ messages in thread
From: Lyude @ 2016-10-14 21:45 UTC (permalink / raw)
  To: Paulo Zanoni, intel-gfx; +Cc: stable, Mahesh Kumar, Dhinakaran Pandiyan

Reviewed-by: Lyude <cpaul@redhat.com>

On Tue, 2016-10-11 at 15:25 -0300, Paulo Zanoni wrote:
> Mahesh Kumar is already working on a proper implementation for the
> workaround, but while we still don't have it, let's just
> unconditionally apply the workaround for everybody and we hope we can
> close all those numerous bugzilla tickets. Also, I'm not sure how
> easy
> it will be to backport the final implementation to the stable
> Kernels,
> and this patch here is probably easier to backport.
> 
> At the present moment I still don't have confirmation that this patch
> fixes any of the bugs listed below, but we should definitely try
> testing all of them again.
> 
> v2: s/intel_needs_memory_bw_wa/skl_needs_memory_bw_wa/ (Lyude).
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94337
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94884
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95010
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96226
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96828
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97450
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97830
> Cc: stable@vger.kernel.org
> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 49
> ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 41 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index fe6c1c6..13bd974 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2879,6 +2879,21 @@ skl_wm_plane_id(const struct intel_plane
> *plane)
>  	}
>  }
>  
> +/*
> + * 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);
> +
> +	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv) ||
> +	    IS_KABYLAKE(dev_priv))
> +		return true;
> +
> +	return false;
> +}
> +
>  static bool
>  intel_has_sagv(struct drm_i915_private *dev_priv)
>  {
> @@ -2999,9 +3014,10 @@ bool intel_can_enable_sagv(struct
> drm_atomic_state *state)
>  	struct drm_device *dev = state->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_atomic_state *intel_state =
> to_intel_atomic_state(state);
> -	struct drm_crtc *crtc;
> +	struct intel_crtc *crtc;
> +	struct intel_plane *plane;
>  	enum pipe pipe;
> -	int level, plane;
> +	int level, id, latency;
>  
>  	if (!intel_has_sagv(dev_priv))
>  		return false;
> @@ -3019,27 +3035,36 @@ bool intel_can_enable_sagv(struct
> drm_atomic_state *state)
>  
>  	/* Since we're now guaranteed to only have one active
> CRTC... */
>  	pipe = ffs(intel_state->active_crtcs) - 1;
> -	crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +	crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>  
> -	if (crtc->state->mode.flags & DRM_MODE_FLAG_INTERLACE)
> +	if (crtc->base.state->mode.flags & DRM_MODE_FLAG_INTERLACE)
>  		return false;
>  
> -	for_each_plane(dev_priv, pipe, plane) {
> +	for_each_intel_plane_on_crtc(dev, crtc, plane) {
> +		id = skl_wm_plane_id(plane);
> +
>  		/* Skip this plane if it's not enabled */
> -		if (intel_state->wm_results.plane[pipe][plane][0] ==
> 0)
> +		if (intel_state->wm_results.plane[pipe][id][0] == 0)
>  			continue;
>  
>  		/* Find the highest enabled wm level for this plane
> */
>  		for (level = ilk_wm_max_level(dev);
> -		     intel_state-
> >wm_results.plane[pipe][plane][level] == 0; --level)
> +		     intel_state->wm_results.plane[pipe][id][level]
> == 0; --level)
>  		     { }
>  
> +		latency = dev_priv->wm.skl_latency[level];
> +
> +		if (skl_needs_memory_bw_wa(intel_state) &&
> +		    plane->base.state->fb->modifier[0] ==
> +		    I915_FORMAT_MOD_X_TILED)
> +			latency += 15;
> +
>  		/*
>  		 * If any of the planes on this pipe don't enable wm
> levels
>  		 * that incur memory latencies higher then 30µs we
> can't enable
>  		 * the SAGV
>  		 */
> -		if (dev_priv->wm.skl_latency[level] <
> SKL_SAGV_BLOCK_TIME)
> +		if (latency < SKL_SAGV_BLOCK_TIME)
>  			return false;
>  	}
>  
> @@ -3555,12 +3580,18 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>  	uint32_t width = 0, height = 0;
>  	uint32_t plane_pixel_rate;
>  	uint32_t y_tile_minimum, 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);
>  
>  	if (latency == 0 || !cstate->base.active || !intel_pstate-
> >base.visible) {
>  		*enabled = false;
>  		return 0;
>  	}
>  
> +	if (apply_memory_bw_wa && fb->modifier[0] ==
> I915_FORMAT_MOD_X_TILED)
> +		latency += 15;
> +
>  	width = drm_rect_width(&intel_pstate->base.src) >> 16;
>  	height = drm_rect_height(&intel_pstate->base.src) >> 16;
>  
> @@ -3613,6 +3644,8 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>  				 plane_blocks_per_line);
>  
>  	y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
> +	if (apply_memory_bw_wa)
> +		y_tile_minimum *= 2;
>  
>  	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>  	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
-- 
Cheers,
	Lyude

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-10-14 21:45 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-10 20:30 [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Paulo Zanoni
2016-10-10 20:30 ` [PATCH 2/2] drm/i915/gen9: look for adjusted_mode in the SAGV check for interlaced Paulo Zanoni
2016-10-10 20:55   ` Lyude Paul
2016-10-10 20:34 ` [PATCH 1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA Lyude Paul
2016-10-10 20:46   ` Zanoni, Paulo R
2016-10-10 20:55     ` Lyude Paul
2016-10-10 20:55       ` Lyude Paul
2016-10-11 18:25       ` Paulo Zanoni
2016-10-11 18:25         ` Paulo Zanoni
2016-10-14 21:45         ` Lyude
2016-10-10 21:19 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
2016-10-11  7:34 ` [Intel-gfx] [PATCH 1/2] " Jani Nikula
2016-10-11  7:34   ` Jani Nikula
2016-10-11 10:31   ` Greg KH
2016-10-11 10:54     ` Jani Nikula
2016-10-11 10:54       ` Jani Nikula
2016-10-11 11:10       ` [Intel-gfx] " Greg KH
2016-10-11 11:10         ` Greg KH
2016-10-11 19:19 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915/gen9: unconditionally apply the memory bandwidth WA (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.