[PATCH 1/6] drm/i915: Limit FBC flush to post batch flush

[PATCH 1/6] drm/i915: Limit FBC flush to post batch flush

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Don't issue the FBC nuke/cache clean command when invalidate_domains!=0.
That would indicate that we're not being called for the post-batch
flush.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e32c08a..752f208 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -354,7 +354,7 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
-	if (flush_domains)
+	if (!invalidate_domains && flush_domains)
 		return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
 
 	return 0;
@@ -1837,7 +1837,7 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring,
 	}
 	intel_ring_advance(ring);
 
-	if (IS_GEN7(dev) && flush)
+	if (IS_GEN7(dev) && !invalidate && flush)
 		return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
 
 	return 0;
-- 
1.8.1.5

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

[PATCH 2/6] drm/i915: Emit SRM after the MSG_FBC_REND_STATE LRI

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The spec tells us that we need to emit an SRM after the LRI
to MSG_FBC_REND_STATE.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         | 1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0719c8b..7a4d3e1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -235,6 +235,7 @@
  */
 #define MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*x-1)
 #define MI_STORE_REGISTER_MEM(x) MI_INSTR(0x24, 2*x-1)
+#define  MI_SRM_LRM_GLOBAL_GTT		(1<<22)
 #define MI_FLUSH_DW		MI_INSTR(0x26, 1) /* for GEN6 */
 #define   MI_FLUSH_DW_STORE_INDEX	(1<<21)
 #define   MI_INVALIDATE_TLB		(1<<18)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 752f208..4649bf5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -285,14 +285,16 @@ static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value)
 	if (!ring->fbc_dirty)
 		return 0;
 
-	ret = intel_ring_begin(ring, 4);
+	ret = intel_ring_begin(ring, 6);
 	if (ret)
 		return ret;
-	intel_ring_emit(ring, MI_NOOP);
 	/* WaFbcNukeOn3DBlt:ivb/hsw */
 	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
 	intel_ring_emit(ring, MSG_FBC_REND_STATE);
 	intel_ring_emit(ring, value);
+	intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) | MI_SRM_LRM_GLOBAL_GTT);
+	intel_ring_emit(ring, MSG_FBC_REND_STATE);
+	intel_ring_emit(ring, ring->scratch.gtt_offset + 256);
 	intel_ring_advance(ring);
 
 	ring->fbc_dirty = false;
-- 
1.8.1.5

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

[PATCH 3/6] drm/i915: Implement LRI based FBC tracking

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

As per the SNB and HSW PM guides, we should enable FBC render/blitter
tracking only during batches targetting the front buffer.

On SNB we must also update the FBC render tracking address whenever it
changes. And since the register in question is stored in the context,
we need to make sure we reload it with correct data after context
switches.

On IVB/HSW we use the render nuke mechanism, so no render tracking
address updates are needed. Hoever on the blitter side we need to
enable the blitter tracking like on SNB, and in addition we need
to issue the cache clean messages, which we already did.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c    |  7 ++++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31 ++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c       | 28 +++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h           |  3 ++
 drivers/gpu/drm/i915/intel_pm.c            |  2 --
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 58 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  2 ++
 7 files changed, 126 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 72a3df3..d438ea1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -404,6 +404,13 @@ mi_set_context(struct intel_ring_buffer *ring,
 
 	intel_ring_advance(ring);
 
+	/*
+	 * FBC RT address is stored in the context, so we may have just
+	 * restored it to an old value. Make sure we emit a new LRI
+	 * to update the address.
+	 */
+	ring->fbc_address_dirty = true;
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 885d595..a8f8634 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -886,6 +886,35 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec,
 }
 
 static void
+i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring,
+				   struct list_head *vmas)
+{
+	struct i915_vma *vma;
+	struct drm_i915_gem_object *fbc_obj = NULL;
+	u32 fbc_address = -1;
+
+	list_for_each_entry(vma, vmas, exec_list) {
+		struct drm_i915_gem_object *obj = vma->obj;
+
+		if (obj->base.pending_write_domain == 0)
+			continue;
+
+		if (obj->pin_count) /* check for potential scanout */
+			intel_mark_fbc_dirty(obj, ring, &fbc_obj);
+	}
+
+	if (fbc_obj)
+		fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
+
+	/* need to nuke/cache_clean on IVB+? */
+	ring->fbc_dirty = fbc_obj != NULL;
+
+	/* need to update FBC tracking? */
+	ring->fbc_address_dirty = fbc_address != ring->fbc_address;
+	ring->fbc_address = fbc_address;
+}
+
+static void
 i915_gem_execbuffer_move_to_active(struct list_head *vmas,
 				   struct intel_ring_buffer *ring)
 {
@@ -1150,6 +1179,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping)
 		i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level);
 
+	i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas);
+
 	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
 	if (ret)
 		goto err;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 12cf362..49c40aa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8064,6 +8064,32 @@ void intel_mark_idle(struct drm_device *dev)
 		gen6_rps_idle(dev->dev_private);
 }
 
+void intel_mark_fbc_dirty(struct drm_i915_gem_object *obj,
+			  struct intel_ring_buffer *ring,
+			  struct drm_i915_gem_object **fbc_obj)
+{
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+
+	if (!i915_powersave)
+		return;
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		if (!crtc->fb)
+			continue;
+
+		if (dev_priv->fbc.plane != to_intel_crtc(crtc)->plane)
+			continue;
+
+		if (to_intel_framebuffer(crtc->fb)->obj != obj)
+			continue;
+
+		WARN_ON(*fbc_obj && *fbc_obj != obj);
+		*fbc_obj = obj;
+	}
+}
+
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			struct intel_ring_buffer *ring)
 {
@@ -8081,8 +8107,6 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			continue;
 
 		intel_increase_pllclock(crtc);
-		if (ring && intel_fbc_enabled(dev))
-			ring->fbc_dirty = true;
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6d701e7..5841f60 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -614,6 +614,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 /* intel_display.c */
 int intel_pch_rawclk(struct drm_device *dev);
 void intel_mark_busy(struct drm_device *dev);
+void intel_mark_fbc_dirty(struct drm_i915_gem_object *obj,
+			  struct intel_ring_buffer *ring,
+			  struct drm_i915_gem_object **fbc_obj);
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			struct intel_ring_buffer *ring);
 void intel_mark_idle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index dc306e1..bfec348 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -193,8 +193,6 @@ static void sandybridge_blit_fbc_update(struct drm_device *dev)
 	/* Make sure blitter notifies FBC of writes */
 	gen6_gt_force_wake_get(dev_priv);
 	blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
-	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
-		GEN6_BLITTER_LOCK_SHIFT;
 	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
 	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
 	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4649bf5..43f7eab 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -207,6 +207,57 @@ intel_emit_post_sync_nonzero_flush(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static int gen6_blt_fbc_tracking(struct intel_ring_buffer *ring)
+{
+	int ret;
+
+	if (!ring->fbc_address_dirty)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, GEN6_BLITTER_ECOSKPD);
+	if (ring->fbc_address != -1)
+		intel_ring_emit(ring, _MASKED_BIT_ENABLE(GEN6_BLITTER_FBC_NOTIFY));
+	else
+		intel_ring_emit(ring, _MASKED_BIT_DISABLE(GEN6_BLITTER_FBC_NOTIFY));
+	intel_ring_advance(ring);
+
+	ring->fbc_address_dirty = true;
+
+	return 0;
+}
+
+static int gen6_render_fbc_tracking(struct intel_ring_buffer *ring)
+{
+	int ret;
+
+	if (!ring->fbc_address_dirty)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, ILK_FBC_RT_BASE);
+	if (ring->fbc_address != -1)
+		intel_ring_emit(ring, ring->fbc_address |
+				SNB_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID);
+	else
+		intel_ring_emit(ring, 0);
+	intel_ring_advance(ring);
+
+	ring->fbc_address_dirty = true;
+
+	return 0;
+}
+
 static int
 gen6_render_ring_flush(struct intel_ring_buffer *ring,
                          u32 invalidate_domains, u32 flush_domains)
@@ -256,6 +307,9 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (invalidate_domains)
+		return gen6_render_fbc_tracking(ring);
+
 	return 0;
 }
 
@@ -1839,7 +1893,9 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring,
 	}
 	intel_ring_advance(ring);
 
-	if (IS_GEN7(dev) && !invalidate && flush)
+	if (invalidate)
+		return gen6_blt_fbc_tracking(ring);
+	else if (flush && IS_GEN7(dev))
 		return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 71a73f4..1e5bbd6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -143,8 +143,10 @@ struct  intel_ring_buffer {
 	 */
 	struct drm_i915_gem_request *preallocated_lazy_request;
 	u32 outstanding_lazy_seqno;
+	u32 fbc_address;
 	bool gpu_caches_dirty;
 	bool fbc_dirty;
+	bool fbc_address_dirty;
 
 	wait_queue_head_t irq_queue;
 
-- 
1.8.1.5

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

Re: [PATCH 3/6] drm/i915: Implement LRI based FBC tracking

[Chris Wilson]

On Wed, Nov 06, 2013 at 05:39:02PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> As per the SNB and HSW PM guides, we should enable FBC render/blitter
> tracking only during batches targetting the front buffer.
> 
> On SNB we must also update the FBC render tracking address whenever it
> changes. And since the register in question is stored in the context,
> we need to make sure we reload it with correct data after context
> switches.
> 
> On IVB/HSW we use the render nuke mechanism, so no render tracking
> address updates are needed. Hoever on the blitter side we need to
> enable the blitter tracking like on SNB, and in addition we need
> to issue the cache clean messages, which we already did.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c    |  7 ++++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31 ++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c       | 28 +++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h           |  3 ++
>  drivers/gpu/drm/i915/intel_pm.c            |  2 --
>  drivers/gpu/drm/i915/intel_ringbuffer.c    | 58 +++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |  2 ++
>  7 files changed, 126 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 72a3df3..d438ea1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -404,6 +404,13 @@ mi_set_context(struct intel_ring_buffer *ring,
>  
>  	intel_ring_advance(ring);
>  
> +	/*
> +	 * FBC RT address is stored in the context, so we may have just
> +	 * restored it to an old value. Make sure we emit a new LRI
> +	 * to update the address.
> +	 */
> +	ring->fbc_address_dirty = true;
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 885d595..a8f8634 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -886,6 +886,35 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec,
>  }
>  
>  static void
> +i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring,
> +				   struct list_head *vmas)
> +{
> +	struct i915_vma *vma;
> +	struct drm_i915_gem_object *fbc_obj = NULL;
> +	u32 fbc_address = -1;
> +
> +	list_for_each_entry(vma, vmas, exec_list) {
> +		struct drm_i915_gem_object *obj = vma->obj;
> +
> +		if (obj->base.pending_write_domain == 0)
> +			continue;
> +
> +		if (obj->pin_count) /* check for potential scanout */
> +			intel_mark_fbc_dirty(obj, ring, &fbc_obj);

if (obj->pin_display && intel_fbc_active(obj))
	fbc_address = i915_gem_obj_ggtt_offset(obj);

> +	}
> +
> +	if (fbc_obj)
> +		fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
> +
> +	/* need to nuke/cache_clean on IVB+? */
> +	ring->fbc_dirty = fbc_obj != NULL;
> +
> +	/* need to update FBC tracking? */
> +	ring->fbc_address_dirty = fbc_address != ring->fbc_address;

I'm not going to mention the unused bits in fbc_address....

> +	ring->fbc_address = fbc_address;
> +}
> +
> +static void
>  i915_gem_execbuffer_move_to_active(struct list_head *vmas,
>  				   struct intel_ring_buffer *ring)
>  {
> @@ -1150,6 +1179,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  	if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping)
>  		i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level);
>  
> +	i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas);
> +
>  	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
>  	if (ret)
>  		goto err;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 12cf362..49c40aa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8064,6 +8064,32 @@ void intel_mark_idle(struct drm_device *dev)
>  		gen6_rps_idle(dev->dev_private);
>  }
>  
> +void intel_mark_fbc_dirty(struct drm_i915_gem_object *obj,
> +			  struct intel_ring_buffer *ring,
> +			  struct drm_i915_gem_object **fbc_obj)
> +{
> +	struct drm_device *dev = obj->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +
> +	if (!i915_powersave)
> +		return;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		if (!crtc->fb)
> +			continue;
> +
> +		if (dev_priv->fbc.plane != to_intel_crtc(crtc)->plane)
> +			continue;
> +
> +		if (to_intel_framebuffer(crtc->fb)->obj != obj)
> +			continue;
> +
> +		WARN_ON(*fbc_obj && *fbc_obj != obj);
> +		*fbc_obj = obj;
> +	}
> +}
> +
>  void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
>  			struct intel_ring_buffer *ring)
>  {
> @@ -8081,8 +8107,6 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
>  			continue;
>  
>  		intel_increase_pllclock(crtc);
> -		if (ring && intel_fbc_enabled(dev))
> -			ring->fbc_dirty = true;
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6d701e7..5841f60 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -614,6 +614,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  /* intel_display.c */
>  int intel_pch_rawclk(struct drm_device *dev);
>  void intel_mark_busy(struct drm_device *dev);
> +void intel_mark_fbc_dirty(struct drm_i915_gem_object *obj,
> +			  struct intel_ring_buffer *ring,
> +			  struct drm_i915_gem_object **fbc_obj);
>  void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
>  			struct intel_ring_buffer *ring);
>  void intel_mark_idle(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index dc306e1..bfec348 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -193,8 +193,6 @@ static void sandybridge_blit_fbc_update(struct drm_device *dev)
>  	/* Make sure blitter notifies FBC of writes */
>  	gen6_gt_force_wake_get(dev_priv);
>  	blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
> -	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
> -		GEN6_BLITTER_LOCK_SHIFT;
>  	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);

You can kill this write here, right?


>  	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
>  	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 4649bf5..43f7eab 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -207,6 +207,57 @@ intel_emit_post_sync_nonzero_flush(struct intel_ring_buffer *ring)
>  	return 0;
>  }
>  
> +static int gen6_blt_fbc_tracking(struct intel_ring_buffer *ring)
> +{
> +	int ret;
> +
> +	if (!ring->fbc_address_dirty)
> +		return 0;
> +
> +	ret = intel_ring_begin(ring, 4);
> +	if (ret)
> +		return ret;
> +
> +	intel_ring_emit(ring, MI_NOOP);
> +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> +	intel_ring_emit(ring, GEN6_BLITTER_ECOSKPD);
> +	if (ring->fbc_address != -1)
> +		intel_ring_emit(ring, _MASKED_BIT_ENABLE(GEN6_BLITTER_FBC_NOTIFY));
> +	else
> +		intel_ring_emit(ring, _MASKED_BIT_DISABLE(GEN6_BLITTER_FBC_NOTIFY));
> +	intel_ring_advance(ring);
> +
> +	ring->fbc_address_dirty = true;

false, surely?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/6] drm/i915: Implement LRI based FBC tracking

[Ville Syrjälä]

On Wed, Nov 06, 2013 at 04:06:29PM +0000, Chris Wilson wrote:
> On Wed, Nov 06, 2013 at 05:39:02PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > As per the SNB and HSW PM guides, we should enable FBC render/blitter
> > tracking only during batches targetting the front buffer.
> > 
> > On SNB we must also update the FBC render tracking address whenever it
> > changes. And since the register in question is stored in the context,
> > we need to make sure we reload it with correct data after context
> > switches.
> > 
> > On IVB/HSW we use the render nuke mechanism, so no render tracking
> > address updates are needed. Hoever on the blitter side we need to
> > enable the blitter tracking like on SNB, and in addition we need
> > to issue the cache clean messages, which we already did.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_context.c    |  7 ++++
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31 ++++++++++++++++
> >  drivers/gpu/drm/i915/intel_display.c       | 28 +++++++++++++--
> >  drivers/gpu/drm/i915/intel_drv.h           |  3 ++
> >  drivers/gpu/drm/i915/intel_pm.c            |  2 --
> >  drivers/gpu/drm/i915/intel_ringbuffer.c    | 58 +++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/intel_ringbuffer.h    |  2 ++
> >  7 files changed, 126 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 72a3df3..d438ea1 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -404,6 +404,13 @@ mi_set_context(struct intel_ring_buffer *ring,
> >  
> >  	intel_ring_advance(ring);
> >  
> > +	/*
> > +	 * FBC RT address is stored in the context, so we may have just
> > +	 * restored it to an old value. Make sure we emit a new LRI
> > +	 * to update the address.
> > +	 */
> > +	ring->fbc_address_dirty = true;
> > +
> >  	return ret;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 885d595..a8f8634 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -886,6 +886,35 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec,
> >  }
> >  
> >  static void
> > +i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring,
> > +				   struct list_head *vmas)
> > +{
> > +	struct i915_vma *vma;
> > +	struct drm_i915_gem_object *fbc_obj = NULL;
> > +	u32 fbc_address = -1;
> > +
> > +	list_for_each_entry(vma, vmas, exec_list) {
> > +		struct drm_i915_gem_object *obj = vma->obj;
> > +
> > +		if (obj->base.pending_write_domain == 0)
> > +			continue;
> > +
> > +		if (obj->pin_count) /* check for potential scanout */
> > +			intel_mark_fbc_dirty(obj, ring, &fbc_obj);
> 
> if (obj->pin_display && intel_fbc_active(obj))
> 	fbc_address = i915_gem_obj_ggtt_offset(obj);

Ah yeah, pin_display is good. Not sure I like intel_fbc_active() much.
But it needs to get replaced w/ some pipe config stuff or something
anyway eventually. I do already check fbc.plane == crtc->plane in
intel_mark_fbc_dirty() which should catch the case when fbc is disabled
anyway.

> 
> > +	}
> > +
> > +	if (fbc_obj)
> > +		fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
> > +
> > +	/* need to nuke/cache_clean on IVB+? */
> > +	ring->fbc_dirty = fbc_obj != NULL;
> > +
> > +	/* need to update FBC tracking? */
> > +	ring->fbc_address_dirty = fbc_address != ring->fbc_address;
> 
> I'm not going to mention the unused bits in fbc_address....

Just preparing myself for full 32bit addres space :)

> 
> > +	ring->fbc_address = fbc_address;
> > +}
> > +
> > +static void
> >  i915_gem_execbuffer_move_to_active(struct list_head *vmas,
> >  				   struct intel_ring_buffer *ring)
> >  {
> > @@ -1150,6 +1179,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> >  	if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping)
> >  		i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level);
> >  
> > +	i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas);
> > +
> >  	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
> >  	if (ret)
> >  		goto err;
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 12cf362..49c40aa 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8064,6 +8064,32 @@ void intel_mark_idle(struct drm_device *dev)
> >  		gen6_rps_idle(dev->dev_private);
> >  }
> >  
> > +void intel_mark_fbc_dirty(struct drm_i915_gem_object *obj,
> > +			  struct intel_ring_buffer *ring,
> > +			  struct drm_i915_gem_object **fbc_obj)
> > +{
> > +	struct drm_device *dev = obj->base.dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_crtc *crtc;
> > +
> > +	if (!i915_powersave)
> > +		return;
> > +
> > +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > +		if (!crtc->fb)
> > +			continue;
> > +
> > +		if (dev_priv->fbc.plane != to_intel_crtc(crtc)->plane)
> > +			continue;
> > +
> > +		if (to_intel_framebuffer(crtc->fb)->obj != obj)
> > +			continue;
> > +
> > +		WARN_ON(*fbc_obj && *fbc_obj != obj);
> > +		*fbc_obj = obj;
> > +	}
> > +}
> > +
> >  void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
> >  			struct intel_ring_buffer *ring)
> >  {
> > @@ -8081,8 +8107,6 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
> >  			continue;
> >  
> >  		intel_increase_pllclock(crtc);
> > -		if (ring && intel_fbc_enabled(dev))
> > -			ring->fbc_dirty = true;
> >  	}
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 6d701e7..5841f60 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -614,6 +614,9 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  /* intel_display.c */
> >  int intel_pch_rawclk(struct drm_device *dev);
> >  void intel_mark_busy(struct drm_device *dev);
> > +void intel_mark_fbc_dirty(struct drm_i915_gem_object *obj,
> > +			  struct intel_ring_buffer *ring,
> > +			  struct drm_i915_gem_object **fbc_obj);
> >  void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
> >  			struct intel_ring_buffer *ring);
> >  void intel_mark_idle(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index dc306e1..bfec348 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -193,8 +193,6 @@ static void sandybridge_blit_fbc_update(struct drm_device *dev)
> >  	/* Make sure blitter notifies FBC of writes */
> >  	gen6_gt_force_wake_get(dev_priv);
> >  	blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
> > -	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
> > -		GEN6_BLITTER_LOCK_SHIFT;
> >  	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
> 
> You can kill this write here, right?

I'm killing the whole function later. This was a squash accident since
I had Ben's earlier patches on the bottom originally. I'll drop this
hunk.

> 
> 
> >  	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
> >  	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 4649bf5..43f7eab 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -207,6 +207,57 @@ intel_emit_post_sync_nonzero_flush(struct intel_ring_buffer *ring)
> >  	return 0;
> >  }
> >  
> > +static int gen6_blt_fbc_tracking(struct intel_ring_buffer *ring)
> > +{
> > +	int ret;
> > +
> > +	if (!ring->fbc_address_dirty)
> > +		return 0;
> > +
> > +	ret = intel_ring_begin(ring, 4);
> > +	if (ret)
> > +		return ret;
> > +
> > +	intel_ring_emit(ring, MI_NOOP);
> > +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> > +	intel_ring_emit(ring, GEN6_BLITTER_ECOSKPD);
> > +	if (ring->fbc_address != -1)
> > +		intel_ring_emit(ring, _MASKED_BIT_ENABLE(GEN6_BLITTER_FBC_NOTIFY));
> > +	else
> > +		intel_ring_emit(ring, _MASKED_BIT_DISABLE(GEN6_BLITTER_FBC_NOTIFY));
> > +	intel_ring_advance(ring);
> > +
> > +	ring->fbc_address_dirty = true;
> 
> false, surely?

Oops, yeah. And now I need to go and re-rest to make sure that didn't
make it look like it works even when it doesn't. But it shouldn't
since we recompute it from scratch for every execbuffer.

-- 
Ville Syrjälä
Intel OTC

[PATCH v2 3/6] drm/i915: Implement LRI based FBC tracking

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

As per the SNB and HSW PM guides, we should enable FBC render/blitter
tracking only during batches targetting the front buffer.

On SNB we must also update the FBC render tracking address whenever it
changes. And since the register in question is stored in the context,
we need to make sure we reload it with correct data after context
switches.

On IVB/HSW we use the render nuke mechanism, so no render tracking
address updates are needed. Hoever on the blitter side we need to
enable the blitter tracking like on SNB, and in addition we need
to issue the cache clean messages, which we already did.

v2: Introduce intel_fb_obj_has_fbc()
    Fix crtc locking around crtc->fb access
    Drop a hunk that was included by accident in v1
    Set fbc_address_dirty=false not true after emitting the LRI

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c    |  7 ++++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 31 ++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c       | 29 +++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h           |  1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 58 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  2 ++
 6 files changed, 125 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 72a3df3..d438ea1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -404,6 +404,13 @@ mi_set_context(struct intel_ring_buffer *ring,
 
 	intel_ring_advance(ring);
 
+	/*
+	 * FBC RT address is stored in the context, so we may have just
+	 * restored it to an old value. Make sure we emit a new LRI
+	 * to update the address.
+	 */
+	ring->fbc_address_dirty = true;
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 885d595..db25158 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -886,6 +886,35 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec,
 }
 
 static void
+i915_gem_execbuffer_mark_fbc_dirty(struct intel_ring_buffer *ring,
+				   struct list_head *vmas)
+{
+	struct i915_vma *vma;
+	struct drm_i915_gem_object *fbc_obj = NULL;
+	u32 fbc_address = -1;
+
+	list_for_each_entry(vma, vmas, exec_list) {
+		struct drm_i915_gem_object *obj = vma->obj;
+
+		if (obj->base.pending_write_domain &&
+		    intel_fb_obj_has_fbc(obj)) {
+			WARN_ON(fbc_obj && fbc_obj != obj);
+			fbc_obj = obj;
+		}
+	}
+
+	if (fbc_obj)
+		fbc_address = i915_gem_obj_ggtt_offset(fbc_obj);
+
+	/* need to nuke/cache_clean on IVB+? */
+	ring->fbc_dirty = fbc_obj != NULL;
+
+	/* need to update FBC tracking? */
+	ring->fbc_address_dirty = fbc_address != ring->fbc_address;
+	ring->fbc_address = fbc_address;
+}
+
+static void
 i915_gem_execbuffer_move_to_active(struct list_head *vmas,
 				   struct intel_ring_buffer *ring)
 {
@@ -1150,6 +1179,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping)
 		i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level);
 
+	i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas);
+
 	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
 	if (ret)
 		goto err;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 12cf362..fb080b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8064,6 +8064,33 @@ void intel_mark_idle(struct drm_device *dev)
 		gen6_rps_idle(dev->dev_private);
 }
 
+bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj)
+{
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+
+	/* check for potential scanout */
+	if (!obj->pin_display)
+		return false;
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		bool has_fbc;
+
+		if (dev_priv->fbc.plane != to_intel_crtc(crtc)->plane)
+			continue;
+
+		mutex_lock(&crtc->mutex);
+		has_fbc = crtc->fb && to_intel_framebuffer(crtc->fb)->obj == obj;
+		mutex_unlock(&crtc->mutex);
+
+		if (has_fbc)
+			return true;
+	}
+
+	return false;
+}
+
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			struct intel_ring_buffer *ring)
 {
@@ -8081,8 +8108,6 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			continue;
 
 		intel_increase_pllclock(crtc);
-		if (ring && intel_fbc_enabled(dev))
-			ring->fbc_dirty = true;
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6d701e7..5c7e8b4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -614,6 +614,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 /* intel_display.c */
 int intel_pch_rawclk(struct drm_device *dev);
 void intel_mark_busy(struct drm_device *dev);
+bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj);
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			struct intel_ring_buffer *ring);
 void intel_mark_idle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4649bf5..64fbab5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -207,6 +207,57 @@ intel_emit_post_sync_nonzero_flush(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static int gen6_blt_fbc_tracking(struct intel_ring_buffer *ring)
+{
+	int ret;
+
+	if (!ring->fbc_address_dirty)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, GEN6_BLITTER_ECOSKPD);
+	if (ring->fbc_address != -1)
+		intel_ring_emit(ring, _MASKED_BIT_ENABLE(GEN6_BLITTER_FBC_NOTIFY));
+	else
+		intel_ring_emit(ring, _MASKED_BIT_DISABLE(GEN6_BLITTER_FBC_NOTIFY));
+	intel_ring_advance(ring);
+
+	ring->fbc_address_dirty = false;
+
+	return 0;
+}
+
+static int gen6_render_fbc_tracking(struct intel_ring_buffer *ring)
+{
+	int ret;
+
+	if (!ring->fbc_address_dirty)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, ILK_FBC_RT_BASE);
+	if (ring->fbc_address != -1)
+		intel_ring_emit(ring, ring->fbc_address |
+				SNB_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID);
+	else
+		intel_ring_emit(ring, 0);
+	intel_ring_advance(ring);
+
+	ring->fbc_address_dirty = false;
+
+	return 0;
+}
+
 static int
 gen6_render_ring_flush(struct intel_ring_buffer *ring,
                          u32 invalidate_domains, u32 flush_domains)
@@ -256,6 +307,9 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (invalidate_domains)
+		return gen6_render_fbc_tracking(ring);
+
 	return 0;
 }
 
@@ -1839,7 +1893,9 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring,
 	}
 	intel_ring_advance(ring);
 
-	if (IS_GEN7(dev) && !invalidate && flush)
+	if (invalidate)
+		return gen6_blt_fbc_tracking(ring);
+	else if (flush && IS_GEN7(dev))
 		return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 71a73f4..1e5bbd6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -143,8 +143,10 @@ struct  intel_ring_buffer {
 	 */
 	struct drm_i915_gem_request *preallocated_lazy_request;
 	u32 outstanding_lazy_seqno;
+	u32 fbc_address;
 	bool gpu_caches_dirty;
 	bool fbc_dirty;
+	bool fbc_address_dirty;
 
 	wait_queue_head_t irq_queue;
 
-- 
1.8.1.5

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

Re: [PATCH v2 3/6] drm/i915: Implement LRI based FBC tracking

[Chris Wilson]

On Wed, Nov 06, 2013 at 07:23:25PM +0200, ville.syrjala@linux.intel.com wrote:
> +bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj)
> +{
> +	struct drm_device *dev = obj->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc;
> +
> +	/* check for potential scanout */
> +	if (!obj->pin_display)
> +		return false;

	if (dev_oriv->fbc.plane == -1)
		return false;

	crtc = dev_priv->plane_to_crtc[dev_priv->fbc.plane];

	mutex_lock(&crtc->mutex);
	has_fbc = crtc->fb && to_intel_framebuffer(crtc->fb)->obj == obj;
	mutex_unlock(&crtc->mutex);

	return has_fbc
 }

Perhaps?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH v2 3/6] drm/i915: Implement LRI based FBC tracking

[Ville Syrjälä]

On Wed, Nov 06, 2013 at 05:36:01PM +0000, Chris Wilson wrote:
> On Wed, Nov 06, 2013 at 07:23:25PM +0200, ville.syrjala@linux.intel.com wrote:
> > +bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj)
> > +{
> > +	struct drm_device *dev = obj->base.dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_crtc *crtc;
> > +
> > +	/* check for potential scanout */
> > +	if (!obj->pin_display)
> > +		return false;
> 
> 	if (dev_oriv->fbc.plane == -1)
> 		return false;
> 
> 	crtc = dev_priv->plane_to_crtc[dev_priv->fbc.plane];
> 
> 	mutex_lock(&crtc->mutex);
> 	has_fbc = crtc->fb && to_intel_framebuffer(crtc->fb)->obj == obj;
> 	mutex_unlock(&crtc->mutex);
> 
> 	return has_fbc
>  }
> 
> Perhaps?

Oh yeah, much nicer. Except now the locking I came up here sucks. Need
to rethink that part somehow. Maybe I'll just slap an
ACCESS_ONCE(crtc->fb) there. Hmm, but I guess I'd also need to grab a
reference to the fb to avoid it disappearing while were looking at it.

This is starting to be a bit hairy. So maybe I should just stick our
current fbc scanout object to dev_priv->fbc.obj, and update it when
doing page flips/modeset. I really didn't want to start messing about
with fbc locking but looks like there's no way around it.

-- 
Ville Syrjälä
Intel OTC

[PATCH 4/6] drm/i915: Kill sandybridge_blit_fbc_update()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

No longer needed since the LRIs do the work.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bfec348..4b016d8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -185,24 +185,6 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
 	return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
 }
 
-static void sandybridge_blit_fbc_update(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 blt_ecoskpd;
-
-	/* Make sure blitter notifies FBC of writes */
-	gen6_gt_force_wake_get(dev_priv);
-	blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
-	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
-	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
-	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
-	blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
-			 GEN6_BLITTER_LOCK_SHIFT);
-	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
-	POSTING_READ(GEN6_BLITTER_ECOSKPD);
-	gen6_gt_force_wake_put(dev_priv);
-}
-
 static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 {
 	struct drm_device *dev = crtc->dev;
@@ -235,7 +217,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		I915_WRITE(SNB_DPFC_CTL_SA,
 			   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
 		I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
-		sandybridge_blit_fbc_update(dev);
 	}
 
 	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
@@ -291,8 +272,6 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
 	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
 
-	sandybridge_blit_fbc_update(dev);
-
 	DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
 }
 
-- 
1.8.1.5

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

[PATCH v2 4/6] drm/i915: Kill sandybridge_blit_fbc_update()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

No longer needed since the LRIs do the work.

v2: Rebased due to hunk getting dropped from an ealier patch

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index dc306e1..4b016d8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -185,26 +185,6 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
 	return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
 }
 
-static void sandybridge_blit_fbc_update(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 blt_ecoskpd;
-
-	/* Make sure blitter notifies FBC of writes */
-	gen6_gt_force_wake_get(dev_priv);
-	blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
-	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
-		GEN6_BLITTER_LOCK_SHIFT;
-	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
-	blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
-	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
-	blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
-			 GEN6_BLITTER_LOCK_SHIFT);
-	I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
-	POSTING_READ(GEN6_BLITTER_ECOSKPD);
-	gen6_gt_force_wake_put(dev_priv);
-}
-
 static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 {
 	struct drm_device *dev = crtc->dev;
@@ -237,7 +217,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		I915_WRITE(SNB_DPFC_CTL_SA,
 			   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
 		I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
-		sandybridge_blit_fbc_update(dev);
 	}
 
 	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
@@ -293,8 +272,6 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
 	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
 
-	sandybridge_blit_fbc_update(dev);
-
 	DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
 }
 
-- 
1.8.1.5

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

[PATCH 5/6] drm/i915: Don't write ILK_FBC_RT_BASE directly on SNB

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We use LRIs to enable/disable the render tracking as needed. So leave
ILK_FBC_RT_BASE alone when enabling FBC.

TODO: Make ILK use the LRI mechanism too?

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4b016d8..f58f476 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -209,7 +209,8 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
 		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
-	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
+	if (IS_GEN5(dev))
+		I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
-- 
1.8.1.5

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

[PATCH v2 5/6] drm/i915: Don't write ILK/IVB_FBC_RT_BASE directly

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We use LRIs to enable/disable the render tracking as needed. So leave
ILK_FBC_RT_BASE alone when enabling FBC on SNB.

While at it, kill the IVB_FBC_RT_BASE completely since we don't use
render tracking on IVB+.

TODO: Make ILK use the LRI mechanism too?

v2: Drop the IVB_FBC_RT_BASE write too

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4b016d8..471217a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -209,7 +209,8 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
 		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
-	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
+	if (IS_GEN5(dev))
+		I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
@@ -253,8 +254,6 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	I915_WRITE(IVB_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj));
-
 	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
 		   IVB_DPFC_CTL_FENCE_EN |
 		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
-- 
1.8.1.5

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

[PATCH 6/6] drm/i915: Set has_fbc=true for all SNB+, except VLV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

At least since SNB (perhaps even earlier) even the desktop parts
should have FBC.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c0ab5d4..9b8c9bf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -265,6 +265,7 @@ static const struct intel_device_info intel_ironlake_m_info = {
 static const struct intel_device_info intel_sandybridge_d_info = {
 	.gen = 6, .num_pipes = 2,
 	.need_gfx_hws = 1, .has_hotplug = 1,
+	.has_fbc = 1,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING,
 	.has_llc = 1,
 };
@@ -280,6 +281,7 @@ static const struct intel_device_info intel_sandybridge_m_info = {
 #define GEN7_FEATURES  \
 	.gen = 7, .num_pipes = 3, \
 	.need_gfx_hws = 1, .has_hotplug = 1, \
+	.has_fbc = 1, \
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING, \
 	.has_llc = 1
 
@@ -292,7 +294,6 @@ static const struct intel_device_info intel_ivybridge_m_info = {
 	GEN7_FEATURES,
 	.is_ivybridge = 1,
 	.is_mobile = 1,
-	.has_fbc = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_q_info = {
@@ -307,6 +308,7 @@ static const struct intel_device_info intel_valleyview_m_info = {
 	.num_pipes = 2,
 	.is_valleyview = 1,
 	.display_mmio_offset = VLV_DISPLAY_BASE,
+	.has_fbc = 0, /* legal, last one wins */
 	.has_llc = 0, /* legal, last one wins */
 };
 
@@ -315,6 +317,7 @@ static const struct intel_device_info intel_valleyview_d_info = {
 	.num_pipes = 2,
 	.is_valleyview = 1,
 	.display_mmio_offset = VLV_DISPLAY_BASE,
+	.has_fbc = 0, /* legal, last one wins */
 	.has_llc = 0, /* legal, last one wins */
 };
 
@@ -332,7 +335,6 @@ static const struct intel_device_info intel_haswell_m_info = {
 	.is_mobile = 1,
 	.has_ddi = 1,
 	.has_fpga_dbg = 1,
-	.has_fbc = 1,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING,
 };
 
-- 
1.8.1.5

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