[PATCH 01/10] drm/i915: Apply strongly ordered RCS breadcrumb to gen8/legacy

[PATCH 01/10] drm/i915: Apply strongly ordered RCS breadcrumb to gen8/legacy

[Chris Wilson]

For legacy ringbuffer mode, we need the new ordered breadcrumb emission
tried and tested on execlists.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 37 +++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b5e79ac29ebc..138afed82682 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1425,6 +1425,40 @@ gen6_add_request(struct drm_i915_gem_request *req)
 	return 0;
 }
 
+static inline u32 hws_seqno_address(struct intel_engine_cs *engine)
+{
+	return engine->status_page.gfx_addr + I915_GEM_HWS_INDEX_ADDR;
+}
+
+static int
+gen8_render_add_request(struct drm_i915_gem_request *req)
+{
+	struct intel_engine_cs *engine = req->engine;
+	int ret;
+
+	if (engine->semaphore.signal)
+		ret = engine->semaphore.signal(req, 8);
+	else
+		ret = intel_ring_begin(req, 8);
+	if (ret)
+		return ret;
+
+	intel_ring_emit(engine, GFX_OP_PIPE_CONTROL(6));
+	intel_ring_emit(engine, (PIPE_CONTROL_GLOBAL_GTT_IVB |
+				 PIPE_CONTROL_CS_STALL |
+				 PIPE_CONTROL_QW_WRITE));
+	intel_ring_emit(engine, hws_seqno_address(req->engine));
+	intel_ring_emit(engine, 0);
+	intel_ring_emit(engine, i915_gem_request_get_seqno(req));
+	/* We're thrashing one dword of HWS. */
+	intel_ring_emit(engine, 0);
+	intel_ring_emit(engine, MI_USER_INTERRUPT);
+	intel_ring_emit(engine, MI_NOOP);
+	__intel_ring_advance(engine);
+
+	return 0;
+}
+
 static inline bool i915_gem_has_seqno_wrapped(struct drm_device *dev,
 					      u32 seqno)
 {
@@ -2746,12 +2780,11 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		}
 
 		engine->init_context = intel_rcs_ctx_init;
-		engine->add_request = gen6_add_request;
+		engine->add_request = gen8_render_add_request;
 		engine->flush = gen8_render_ring_flush;
 		engine->irq_get = gen8_ring_get_irq;
 		engine->irq_put = gen8_ring_put_irq;
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
-		engine->irq_seqno_barrier = gen6_seqno_barrier;
 		engine->get_seqno = ring_get_seqno;
 		engine->set_seqno = ring_set_seqno;
 		if (i915_semaphore_is_enabled(dev)) {
-- 
2.8.1

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

[PATCH 02/10] drm/i915: Fix ordering of sanitize ppgtt and sanitize execlists

[Chris Wilson]

The i915.enable_ppgtt option depends upon the state of
i915.enable_execlists option - so we need to sanitize execlists first.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c     | 13 +++++++++++++
 drivers/gpu/drm/i915/i915_drv.h     |  2 ++
 drivers/gpu/drm/i915/i915_gem.c     |  3 ---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 16 +++++-----------
 drivers/gpu/drm/i915/intel_lrc.c    |  2 --
 5 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index f69330cf0118..c91387f1aedd 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -968,6 +968,19 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
 			 info->has_subslice_pg ? "y" : "n");
 	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
 			 info->has_eu_pg ? "y" : "n");
+
+	i915.enable_execlists =
+		intel_sanitize_enable_execlists(dev, i915.enable_execlists);
+
+	/*
+	 * i915.enable_ppgtt is read-only, so do an early pass to validate the
+	 * user's requested state against the hardware/driver capabilities.  We
+	 * do this now so that we can print out any log messages once rather
+	 * than every time we check intel_enable_ppgtt().
+	 */
+	i915.enable_ppgtt =
+		intel_sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
+	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915.enable_ppgtt);
 }
 
 static void intel_init_dpio(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e2abbcc27f2c..17f22009f2b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2753,6 +2753,8 @@ extern int i915_max_ioctl;
 extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
 extern int i915_resume_switcheroo(struct drm_device *dev);
 
+int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt);
+
 /* i915_dma.c */
 void __printf(3, 4)
 __i915_printk(struct drm_i915_private *dev_priv, const char *level,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 96fbc9ab4039..bd6061f177fe 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4901,9 +4901,6 @@ int i915_gem_init(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
-	i915.enable_execlists = intel_sanitize_enable_execlists(dev,
-			i915.enable_execlists);
-
 	mutex_lock(&dev->struct_mutex);
 
 	if (!i915.enable_execlists) {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 59a78f760b6b..364cf8236021 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -110,7 +110,7 @@ const struct i915_ggtt_view i915_ggtt_view_rotated = {
 	.type = I915_GGTT_VIEW_ROTATED,
 };
 
-static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
+int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
 {
 	bool has_aliasing_ppgtt;
 	bool has_full_ppgtt;
@@ -123,12 +123,14 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
 	if (intel_vgpu_active(dev))
 		has_full_ppgtt = false; /* emulation is too hard */
 
+	if (!has_aliasing_ppgtt)
+		return 0;
+
 	/*
 	 * We don't allow disabling PPGTT for gen9+ as it's a requirement for
 	 * execlists, the sole mechanism available to submit work.
 	 */
-	if (INTEL_INFO(dev)->gen < 9 &&
-	    (enable_ppgtt == 0 || !has_aliasing_ppgtt))
+	if (enable_ppgtt == 0 && INTEL_INFO(dev)->gen < 9)
 		return 0;
 
 	if (enable_ppgtt == 1)
@@ -3219,14 +3221,6 @@ int i915_ggtt_init_hw(struct drm_device *dev)
 	if (intel_iommu_gfx_mapped)
 		DRM_INFO("VT-d active for gfx access\n");
 #endif
-	/*
-	 * i915.enable_ppgtt is read-only, so do an early pass to validate the
-	 * user's requested state against the hardware/driver capabilities.  We
-	 * do this now so that we can print out any log messages once rather
-	 * than every time we check intel_enable_ppgtt().
-	 */
-	i915.enable_ppgtt = sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
-	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915.enable_ppgtt);
 
 	return 0;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 874c2515f9d4..4c832f90fe49 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -245,8 +245,6 @@ static int intel_lr_context_pin(struct intel_context *ctx,
  */
 int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists)
 {
-	WARN_ON(i915.enable_ppgtt == -1);
-
 	/* On platforms with execlist available, vGPU will only
 	 * support execlist mode, no ring buffer mode.
 	 */
-- 
2.8.1

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

[PATCH 03/10] drm/i915: Fix gen8 semaphores id for legacy mode

[Chris Wilson]

With the introduction of a distinct engine->id vs the hardware id, we need
to fix up the value we use for selecting the target engine when signaling
a semaphore. Note that these values can be merged with engine->guc_id.

Fixes: de1add360522c876c25ef2bbbbab1c94bdb509ab
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 138afed82682..9761443bcfdf 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1308,7 +1308,7 @@ static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req,
 		intel_ring_emit(signaller, seqno);
 		intel_ring_emit(signaller, 0);
 		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
-					   MI_SEMAPHORE_TARGET(waiter->id));
+					   MI_SEMAPHORE_TARGET(waiter->hw_id));
 		intel_ring_emit(signaller, 0);
 	}
 
@@ -1348,7 +1348,7 @@ static int gen8_xcs_signal(struct drm_i915_gem_request *signaller_req,
 		intel_ring_emit(signaller, upper_32_bits(gtt_offset));
 		intel_ring_emit(signaller, seqno);
 		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
-					   MI_SEMAPHORE_TARGET(waiter->id));
+					   MI_SEMAPHORE_TARGET(waiter->hw_id));
 		intel_ring_emit(signaller, 0);
 	}
 
@@ -2759,6 +2759,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	engine->name = "render ring";
 	engine->id = RCS;
 	engine->exec_id = I915_EXEC_RENDER;
+	engine->hw_id = 0;
 	engine->mmio_base = RENDER_RING_BASE;
 
 	if (INTEL_INFO(dev)->gen >= 8) {
@@ -2909,6 +2910,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 	engine->name = "bsd ring";
 	engine->id = VCS;
 	engine->exec_id = I915_EXEC_BSD;
+	engine->hw_id = 1;
 
 	engine->write_tail = ring_write_tail;
 	if (INTEL_INFO(dev)->gen >= 6) {
@@ -2987,6 +2989,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	engine->name = "bsd2 ring";
 	engine->id = VCS2;
 	engine->exec_id = I915_EXEC_BSD;
+	engine->hw_id = 4;
 
 	engine->write_tail = ring_write_tail;
 	engine->mmio_base = GEN8_BSD2_RING_BASE;
@@ -3019,6 +3022,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	engine->name = "blitter ring";
 	engine->id = BCS;
 	engine->exec_id = I915_EXEC_BLT;
+	engine->hw_id = 2;
 
 	engine->mmio_base = BLT_RING_BASE;
 	engine->write_tail = ring_write_tail;
@@ -3078,6 +3082,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	engine->name = "video enhancement ring";
 	engine->id = VECS;
 	engine->exec_id = I915_EXEC_VEBOX;
+	engine->hw_id = 3;
 
 	engine->mmio_base = VEBOX_RING_BASE;
 	engine->write_tail = ring_write_tail;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 7023e88531b5..2651fd5263eb 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -153,7 +153,8 @@ struct  intel_engine_cs {
 #define I915_NUM_ENGINES 5
 #define _VCS(n) (VCS + (n))
 	unsigned int exec_id;
-	unsigned int guc_id;
+	unsigned int hw_id;
+	unsigned int guc_id; /* XXX same as hw_id? */
 	u32		mmio_base;
 	struct		drm_device *dev;
 	struct intel_ringbuffer *buffer;
-- 
2.8.1

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

[PATCH 04/10] drm/i915: Fix serialisation of pipecontrol write vs semaphore signal

[Chris Wilson]

In order for the MI_SEMAPHORE_SIGNAL command to wait until after the
pipecontrol writing the signal value is complete, we have to pause the
CS inside the PIPE_CONTROL with the CS_STALL bit.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9761443bcfdf..cd0ae687a3cf 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1302,7 +1302,7 @@ static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req,
 		intel_ring_emit(signaller, GFX_OP_PIPE_CONTROL(6));
 		intel_ring_emit(signaller, PIPE_CONTROL_GLOBAL_GTT_IVB |
 					   PIPE_CONTROL_QW_WRITE |
-					   PIPE_CONTROL_FLUSH_ENABLE);
+					   PIPE_CONTROL_CS_STALL);
 		intel_ring_emit(signaller, lower_32_bits(gtt_offset));
 		intel_ring_emit(signaller, upper_32_bits(gtt_offset));
 		intel_ring_emit(signaller, seqno);
@@ -1489,7 +1489,6 @@ gen8_ring_sync(struct drm_i915_gem_request *waiter_req,
 
 	intel_ring_emit(waiter, MI_SEMAPHORE_WAIT |
 				MI_SEMAPHORE_GLOBAL_GTT |
-				MI_SEMAPHORE_POLL |
 				MI_SEMAPHORE_SAD_GTE_SDD);
 	intel_ring_emit(waiter, seqno);
 	intel_ring_emit(waiter,
-- 
2.8.1

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

[PATCH 05/10] drm/i915: Reload PD tables after semaphore wait on gen8

[Chris Wilson]

When the engine idles waiting upon a semaphore, it loses its
pagetables and we must reload them before executing the batch.

v2: Restrict w/a to non-RCS rings (RCS works correctly apparently).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cd0ae687a3cf..8ae70a525847 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1481,6 +1481,7 @@ gen8_ring_sync(struct drm_i915_gem_request *waiter_req,
 {
 	struct intel_engine_cs *waiter = waiter_req->engine;
 	struct drm_i915_private *dev_priv = waiter->dev->dev_private;
+	struct i915_hw_ppgtt *ppgtt;
 	int ret;
 
 	ret = intel_ring_begin(waiter_req, 4);
@@ -1496,6 +1497,15 @@ gen8_ring_sync(struct drm_i915_gem_request *waiter_req,
 	intel_ring_emit(waiter,
 			upper_32_bits(GEN8_WAIT_OFFSET(waiter, signaller->id)));
 	intel_ring_advance(waiter);
+
+	/* When the !RCS engines idle waiting upon a semaphore, they lose their
+	 * pagetables and we must reload them before executing the batch.
+	 * We do this on the i915_switch_context() following the wait and
+	 * before the dispatch.
+	 */
+	ppgtt = waiter_req->ctx->ppgtt;
+	if (ppgtt && waiter_req->engine->id != RCS)
+		ppgtt->pd_dirty_rings |= intel_engine_flag(waiter_req->engine);
 	return 0;
 }
 
-- 
2.8.1

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

[PATCH 06/10] drm/i915: Bump reserved size for legacy gen8 semaphore emission

[Chris Wilson]

With 5 rings and a flush, we need 192 bytes of space to emit the
breadcrumb and semaphores. However, we need some spare room the size of
the single largest packet (36 dwords, 144 bytes) to accommodate
wraparound giving a grand total of 336 bytes

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 2651fd5263eb..dbf64f8a2356 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -484,10 +484,10 @@ static inline u32 intel_ring_get_tail(struct intel_ringbuffer *ringbuf)
 /*
  * Arbitrary size for largest possible 'add request' sequence. The code paths
  * are complex and variable. Empirical measurement shows that the worst case
- * is ILK at 136 words. Reserving too much is better than reserving too little
- * as that allows for corner cases that might have been missed. So the figure
- * has been rounded up to 160 words.
+ * is BDW at 192 bytes (6 + 6 + 36 dwords), then ILK at 136 bytes. However,
+ * we need to allocate double the largest single packet within that emission
+ * to account for tail wraparound (so 6 + 6 + 72 dwords for BDW).
  */
-#define MIN_SPACE_FOR_ADD_REQUEST	160
+#define MIN_SPACE_FOR_ADD_REQUEST 336
 
 #endif /* _INTEL_RINGBUFFER_H_ */
-- 
2.8.1

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

[PATCH 07/10] drm/i915: Trim the flush for the legacy request emission

[Chris Wilson]

At the start of request emission, we flush some space for the request,
estimating the typical size for the request body. The tail is now much
larger than the typical body, so we can shrink the flush slightly.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8ae70a525847..5611d6a449b5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -34,6 +34,11 @@
 #include "i915_trace.h"
 #include "intel_drv.h"
 
+/* Rough estimate of the typical request size, performing a flush,
+ * set-context and then emitting the batch.
+ */
+#define LEGACY_REQUEST_SIZE 200
+
 int __intel_ring_space(int head, int tail, int size)
 {
 	int space = head - tail;
@@ -2388,7 +2393,7 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
 	 * we start building the request - in which case we will just
 	 * have to repeat work.
 	 */
-	request->reserved_space += MIN_SPACE_FOR_ADD_REQUEST;
+	request->reserved_space += LEGACY_REQUEST_SIZE;
 
 	request->ringbuf = request->engine->buffer;
 
@@ -2396,7 +2401,7 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
 	if (ret)
 		return ret;
 
-	request->reserved_space -= MIN_SPACE_FOR_ADD_REQUEST;
+	request->reserved_space -= LEGACY_REQUEST_SIZE;
 	return 0;
 }
 
-- 
2.8.1

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

[PATCH 08/10] drm/i915: Trim the flush for the execlists request emission

[Chris Wilson]

At the start of request emission, we flush some space for the request,
estimating the typical size for the request body. The common tail is now
much larger than the typical body, so we can shrink the flush
substantially.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4c832f90fe49..4735460be1a0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -228,6 +228,9 @@ enum {
 #define GEN8_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT	0x17
 #define GEN9_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT	0x26
 
+/* Typical size of the average request (2 pipecontrols and a MI_BB) */
+#define EXECLISTS_REQUEST_SIZE 64 /* bytes */
+
 static int execlists_context_deferred_alloc(struct intel_context *ctx,
 					    struct intel_engine_cs *engine);
 static int intel_lr_context_pin(struct intel_context *ctx,
@@ -679,7 +682,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
 	 * we start building the request - in which case we will just
 	 * have to repeat work.
 	 */
-	request->reserved_space += MIN_SPACE_FOR_ADD_REQUEST;
+	request->reserved_space += EXECLISTS_REQUEST_SIZE;
 
 	if (request->ctx->engine[engine->id].state == NULL) {
 		ret = execlists_context_deferred_alloc(request->ctx, engine);
@@ -725,7 +728,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
 	 * to cancel/unwind this request now.
 	 */
 
-	request->reserved_space -= MIN_SPACE_FOR_ADD_REQUEST;
+	request->reserved_space -= EXECLISTS_REQUEST_SIZE;
 	return 0;
 
 err_unpin:
-- 
2.8.1

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

[PATCH 09/10] drm/i915: Enable semaphores for legacy submission on gen8

[Chris Wilson]

We have sufficient evidence from igt to support that semaphores are in
a working state. Enabling semaphores now for legacy provides a better
comparison of execlists against legacy ring submission.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d37c0a671eed..9fd221c97275 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -542,10 +542,6 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
 	if (i915.enable_execlists)
 		return false;
 
-	/* Until we get further testing... */
-	if (IS_GEN8(dev))
-		return false;
-
 #ifdef CONFIG_INTEL_IOMMU
 	/* Enable semaphores on SNB when IO remapping is off */
 	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
-- 
2.8.1

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

[PATCH 10/10] drm/i915: Enable legacy/semaphores for CI

[Chris Wilson]

---
 drivers/gpu/drm/i915/intel_lrc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4735460be1a0..8d9849b9b6ed 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -260,10 +260,6 @@ int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists
 	if (enable_execlists == 0)
 		return 0;
 
-	if (HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev) &&
-	    i915.use_mmio_flip >= 0)
-		return 1;
-
 	return 0;
 }
 
-- 
2.8.1

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

✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915: Apply strongly ordered RCS breadcrumb to gen8/legacy

[Patchwork]

== Series Details ==

Series: series starting with [01/10] drm/i915: Apply strongly ordered RCS breadcrumb to gen8/legacy
URL   : https://patchwork.freedesktop.org/series/6490/
State : failure

== Summary ==

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

Test gem_ringfill:
        Subgroup basic-default:
                dmesg-warn -> PASS       (ilk-hp8440p)
        Subgroup basic-default-hang:
                pass       -> INCOMPLETE (snb-dellxps)
        Subgroup basic-default-interruptible:
                dmesg-warn -> PASS       (ilk-hp8440p)

bdw-nuci7-2      total:201  pass:189  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:201  pass:176  dwarn:0   dfail:0   fail:0   skip:25 
bsw-nuc-2        total:200  pass:159  dwarn:0   dfail:0   fail:0   skip:41 
byt-nuc          total:200  pass:159  dwarn:0   dfail:0   fail:0   skip:41 
hsw-brixbox      total:201  pass:175  dwarn:0   dfail:0   fail:0   skip:26 
hsw-gt2          total:201  pass:179  dwarn:0   dfail:0   fail:1   skip:21 
ilk-hp8440p      total:201  pass:140  dwarn:0   dfail:0   fail:0   skip:61 
ivb-t430s        total:201  pass:170  dwarn:0   dfail:0   fail:0   skip:31 
skl-i7k-2        total:201  pass:174  dwarn:0   dfail:0   fail:0   skip:27 
skl-nuci5        total:201  pass:190  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:157  pass:122  dwarn:0   dfail:0   fail:0   skip:34 
snb-x220t        total:201  pass:159  dwarn:0   dfail:0   fail:1   skip:41 

Results at /archive/results/CI_IGT_test/Patchwork_2104/

52ea0b90039f73447d684b98b00285c58ba1d497 drm-intel-nightly: 2016y-04m-28d-14h-10m-04s UTC integration manifest
a5a20d9 drm/i915: Enable legacy/semaphores for CI
543cf9f drm/i915: Enable semaphores for legacy submission on gen8
7769519ad drm/i915: Trim the flush for the execlists request emission
2b560e3 drm/i915: Trim the flush for the legacy request emission
4a8bd90 drm/i915: Bump reserved size for legacy gen8 semaphore emission
c5b340b drm/i915: Reload PD tables after semaphore wait on gen8
baa2570 drm/i915: Fix serialisation of pipecontrol write vs semaphore signal
5ee28aa drm/i915: Fix gen8 semaphores id for legacy mode
da9f523 drm/i915: Fix ordering of sanitize ppgtt and sanitize execlists
0280a66 drm/i915: Apply strongly ordered RCS breadcrumb to gen8/legacy

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

Re: [PATCH 02/10] drm/i915: Fix ordering of sanitize ppgtt and sanitize execlists

[Dave Gordon]

On 28/04/16 17:24, Chris Wilson wrote:
> The i915.enable_ppgtt option depends upon the state of
> i915.enable_execlists option - so we need to sanitize execlists first.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_dma.c     | 13 +++++++++++++
>   drivers/gpu/drm/i915/i915_drv.h     |  2 ++
>   drivers/gpu/drm/i915/i915_gem.c     |  3 ---
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 16 +++++-----------
>   drivers/gpu/drm/i915/intel_lrc.c    |  2 --
>   5 files changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index f69330cf0118..c91387f1aedd 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -968,6 +968,19 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>   			 info->has_subslice_pg ? "y" : "n");
>   	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
>   			 info->has_eu_pg ? "y" : "n");
> +
> +	i915.enable_execlists =
> +		intel_sanitize_enable_execlists(dev, i915.enable_execlists);
> +
> +	/*
> +	 * i915.enable_ppgtt is read-only, so do an early pass to validate the
> +	 * user's requested state against the hardware/driver capabilities.  We
> +	 * do this now so that we can print out any log messages once rather
> +	 * than every time we check intel_enable_ppgtt().
> +	 */
> +	i915.enable_ppgtt =
> +		intel_sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
> +	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915.enable_ppgtt);
>   }
>
>   static void intel_init_dpio(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e2abbcc27f2c..17f22009f2b3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2753,6 +2753,8 @@ extern int i915_max_ioctl;
>   extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
>   extern int i915_resume_switcheroo(struct drm_device *dev);
>
> +int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt);
> +
>   /* i915_dma.c */
>   void __printf(3, 4)
>   __i915_printk(struct drm_i915_private *dev_priv, const char *level,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 96fbc9ab4039..bd6061f177fe 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4901,9 +4901,6 @@ int i915_gem_init(struct drm_device *dev)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	int ret;
>
> -	i915.enable_execlists = intel_sanitize_enable_execlists(dev,
> -			i915.enable_execlists);
> -
>   	mutex_lock(&dev->struct_mutex);
>
>   	if (!i915.enable_execlists) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 59a78f760b6b..364cf8236021 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -110,7 +110,7 @@ const struct i915_ggtt_view i915_ggtt_view_rotated = {
>   	.type = I915_GGTT_VIEW_ROTATED,
>   };
>
> -static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> +int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>   {
>   	bool has_aliasing_ppgtt;
>   	bool has_full_ppgtt;
> @@ -123,12 +123,14 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>   	if (intel_vgpu_active(dev))
>   		has_full_ppgtt = false; /* emulation is too hard */
>
> +	if (!has_aliasing_ppgtt)
> +		return 0;
> +
>   	/*
>   	 * We don't allow disabling PPGTT for gen9+ as it's a requirement for
>   	 * execlists, the sole mechanism available to submit work.
>   	 */

(Not your change, but just to make a note of it):

This comment is a bit misleading, as GuC submission is available on 
Gen9+ and in fact may be the preferred mechanism. Of course GuC 
submission is really ELSP submission behind the scenes, but it still 
looks odd to say "execlists only".

.Dave.

> -	if (INTEL_INFO(dev)->gen < 9 &&
> -	    (enable_ppgtt == 0 || !has_aliasing_ppgtt))
> +	if (enable_ppgtt == 0 && INTEL_INFO(dev)->gen < 9)
>   		return 0;
>
>   	if (enable_ppgtt == 1)
> @@ -3219,14 +3221,6 @@ int i915_ggtt_init_hw(struct drm_device *dev)
>   	if (intel_iommu_gfx_mapped)
>   		DRM_INFO("VT-d active for gfx access\n");
>   #endif
> -	/*
> -	 * i915.enable_ppgtt is read-only, so do an early pass to validate the
> -	 * user's requested state against the hardware/driver capabilities.  We
> -	 * do this now so that we can print out any log messages once rather
> -	 * than every time we check intel_enable_ppgtt().
> -	 */
> -	i915.enable_ppgtt = sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
> -	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915.enable_ppgtt);
>
>   	return 0;
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 874c2515f9d4..4c832f90fe49 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -245,8 +245,6 @@ static int intel_lr_context_pin(struct intel_context *ctx,
>    */
>   int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists)
>   {
> -	WARN_ON(i915.enable_ppgtt == -1);
> -
>   	/* On platforms with execlist available, vGPU will only
>   	 * support execlist mode, no ring buffer mode.
>   	 */
>

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

Re: [PATCH 06/10] drm/i915: Bump reserved size for legacy gen8 semaphore emission

[Joonas Lahtinen]

On to, 2016-04-28 at 17:24 +0100, Chris Wilson wrote:
> With 5 rings and a flush, we need 192 bytes of space to emit the
> breadcrumb and semaphores. However, we need some spare room the size of
> the single largest packet (36 dwords, 144 bytes) to accommodate
> wraparound giving a grand total of 336 bytes
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Based on the IRC discussion, seems reasonable.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 2651fd5263eb..dbf64f8a2356 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -484,10 +484,10 @@ static inline u32 intel_ring_get_tail(struct intel_ringbuffer *ringbuf)
>  /*
>   * Arbitrary size for largest possible 'add request' sequence. The code paths
>   * are complex and variable. Empirical measurement shows that the worst case
> - * is ILK at 136 words. Reserving too much is better than reserving too little
> - * as that allows for corner cases that might have been missed. So the figure
> - * has been rounded up to 160 words.
> + * is BDW at 192 bytes (6 + 6 + 36 dwords), then ILK at 136 bytes. However,
> + * we need to allocate double the largest single packet within that emission
> + * to account for tail wraparound (so 6 + 6 + 72 dwords for BDW).
>   */
> -#define MIN_SPACE_FOR_ADD_REQUEST	160
> +#define MIN_SPACE_FOR_ADD_REQUEST 336
>  
>  #endif /* _INTEL_RINGBUFFER_H_ */
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/10] drm/i915: Trim the flush for the legacy request emission

[Joonas Lahtinen]

On to, 2016-04-28 at 17:24 +0100, Chris Wilson wrote:
> At the start of request emission, we flush some space for the request,
> estimating the typical size for the request body. The tail is now much
> larger than the typical body, so we can shrink the flush slightly.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

This is more than the previous number, so it will not hurt at least.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 8ae70a525847..5611d6a449b5 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -34,6 +34,11 @@
>  #include "i915_trace.h"
>  #include "intel_drv.h"
>  
> +/* Rough estimate of the typical request size, performing a flush,
> + * set-context and then emitting the batch.
> + */
> +#define LEGACY_REQUEST_SIZE 200
> +
>  int __intel_ring_space(int head, int tail, int size)
>  {
>  	int space = head - tail;
> @@ -2388,7 +2393,7 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
>  	 * we start building the request - in which case we will just
>  	 * have to repeat work.
>  	 */
> -	request->reserved_space += MIN_SPACE_FOR_ADD_REQUEST;
> +	request->reserved_space += LEGACY_REQUEST_SIZE;
>  
>  	request->ringbuf = request->engine->buffer;
>  
> @@ -2396,7 +2401,7 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
>  	if (ret)
>  		return ret;
>  
> -	request->reserved_space -= MIN_SPACE_FOR_ADD_REQUEST;
> +	request->reserved_space -= LEGACY_REQUEST_SIZE;
>  	return 0;
>  }
>  
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/10] drm/i915: Trim the flush for the execlists request emission

[Joonas Lahtinen]

On to, 2016-04-28 at 17:24 +0100, Chris Wilson wrote:
> At the start of request emission, we flush some space for the request,
> estimating the typical size for the request body. The common tail is now
> much larger than the typical body, so we can shrink the flush
> substantially.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I assume this was an educated guess to some extent not to cause huge
regressions in performance.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 4c832f90fe49..4735460be1a0 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -228,6 +228,9 @@ enum {
>  #define GEN8_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT	0x17
>  #define GEN9_CTX_RCS_INDIRECT_CTX_OFFSET_DEFAULT	0x26
>  
> +/* Typical size of the average request (2 pipecontrols and a MI_BB) */
> +#define EXECLISTS_REQUEST_SIZE 64 /* bytes */
> +
>  static int execlists_context_deferred_alloc(struct intel_context *ctx,
>  					    struct intel_engine_cs *engine);
>  static int intel_lr_context_pin(struct intel_context *ctx,
> @@ -679,7 +682,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
>  	 * we start building the request - in which case we will just
>  	 * have to repeat work.
>  	 */
> -	request->reserved_space += MIN_SPACE_FOR_ADD_REQUEST;
> +	request->reserved_space += EXECLISTS_REQUEST_SIZE;
>  
>  	if (request->ctx->engine[engine->id].state == NULL) {
>  		ret = execlists_context_deferred_alloc(request->ctx, engine);
> @@ -725,7 +728,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request
>  	 * to cancel/unwind this request now.
>  	 */
>  
> -	request->reserved_space -= MIN_SPACE_FOR_ADD_REQUEST;
> +	request->reserved_space -= EXECLISTS_REQUEST_SIZE;
>  	return 0;
>  
>  err_unpin:
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/10] drm/i915: Apply strongly ordered RCS breadcrumb to gen8/legacy

[Joonas Lahtinen]

On to, 2016-04-28 at 17:24 +0100, Chris Wilson wrote:
> For legacy ringbuffer mode, we need the new ordered breadcrumb emission
> tried and tested on execlists.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 37 +++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index b5e79ac29ebc..138afed82682 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1425,6 +1425,40 @@ gen6_add_request(struct drm_i915_gem_request *req)
>  	return 0;
>  }
>  
> +static inline u32 hws_seqno_address(struct intel_engine_cs *engine)
> +{
> +	return engine->status_page.gfx_addr + I915_GEM_HWS_INDEX_ADDR;
> +}

Duplicate function, duh.

> +
> +static int
> +gen8_render_add_request(struct drm_i915_gem_request *req)
> +{
> +	struct intel_engine_cs *engine = req->engine;
> +	int ret;
> +
> +	if (engine->semaphore.signal)
> +		ret = engine->semaphore.signal(req, 8);
> +	else
> +		ret = intel_ring_begin(req, 8);
> +	if (ret)
> +		return ret;
> +
> +	intel_ring_emit(engine, GFX_OP_PIPE_CONTROL(6));
> +	intel_ring_emit(engine, (PIPE_CONTROL_GLOBAL_GTT_IVB |
> +				 PIPE_CONTROL_CS_STALL |
> +				 PIPE_CONTROL_QW_WRITE));
> +	intel_ring_emit(engine, hws_seqno_address(req->engine));
> +	intel_ring_emit(engine, 0);
> +	intel_ring_emit(engine, i915_gem_request_get_seqno(req));
> +	/* We're thrashing one dword of HWS. */
> +	intel_ring_emit(engine, 0);
> +	intel_ring_emit(engine, MI_USER_INTERRUPT);
> +	intel_ring_emit(engine, MI_NOOP);
> +	__intel_ring_advance(engine);
> +
> +	return 0;
> +}

This seems like a mix of gen6_add_request and gen8_emit_request from
lrc, so with above duplicate function either explained in commit
message or otherwise handled;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> +
>  static inline bool i915_gem_has_seqno_wrapped(struct drm_device *dev,
>  					      u32 seqno)
>  {
> @@ -2746,12 +2780,11 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>  		}
>  
>  		engine->init_context = intel_rcs_ctx_init;
> -		engine->add_request = gen6_add_request;
> +		engine->add_request = gen8_render_add_request;
>  		engine->flush = gen8_render_ring_flush;
>  		engine->irq_get = gen8_ring_get_irq;
>  		engine->irq_put = gen8_ring_put_irq;
>  		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
> -		engine->irq_seqno_barrier = gen6_seqno_barrier;
>  		engine->get_seqno = ring_get_seqno;
>  		engine->set_seqno = ring_set_seqno;
>  		if (i915_semaphore_is_enabled(dev)) {
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Fix gen8 semaphores id for legacy mode

[Tvrtko Ursulin]

On 28/04/16 17:24, Chris Wilson wrote:
> With the introduction of a distinct engine->id vs the hardware id, we need
> to fix up the value we use for selecting the target engine when signaling
> a semaphore. Note that these values can be merged with engine->guc_id.

So I broke something more with the decoupling, did not realize. I 
suppose it was still worth it. This at least wasn't being used.

Regards,

Tvrtko


> Fixes: de1add360522c876c25ef2bbbbab1c94bdb509ab
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++--
>   drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++-
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 138afed82682..9761443bcfdf 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1308,7 +1308,7 @@ static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req,
>   		intel_ring_emit(signaller, seqno);
>   		intel_ring_emit(signaller, 0);
>   		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
> -					   MI_SEMAPHORE_TARGET(waiter->id));
> +					   MI_SEMAPHORE_TARGET(waiter->hw_id));
>   		intel_ring_emit(signaller, 0);
>   	}
>
> @@ -1348,7 +1348,7 @@ static int gen8_xcs_signal(struct drm_i915_gem_request *signaller_req,
>   		intel_ring_emit(signaller, upper_32_bits(gtt_offset));
>   		intel_ring_emit(signaller, seqno);
>   		intel_ring_emit(signaller, MI_SEMAPHORE_SIGNAL |
> -					   MI_SEMAPHORE_TARGET(waiter->id));
> +					   MI_SEMAPHORE_TARGET(waiter->hw_id));
>   		intel_ring_emit(signaller, 0);
>   	}
>
> @@ -2759,6 +2759,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>   	engine->name = "render ring";
>   	engine->id = RCS;
>   	engine->exec_id = I915_EXEC_RENDER;
> +	engine->hw_id = 0;
>   	engine->mmio_base = RENDER_RING_BASE;
>
>   	if (INTEL_INFO(dev)->gen >= 8) {
> @@ -2909,6 +2910,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>   	engine->name = "bsd ring";
>   	engine->id = VCS;
>   	engine->exec_id = I915_EXEC_BSD;
> +	engine->hw_id = 1;
>
>   	engine->write_tail = ring_write_tail;
>   	if (INTEL_INFO(dev)->gen >= 6) {
> @@ -2987,6 +2989,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
>   	engine->name = "bsd2 ring";
>   	engine->id = VCS2;
>   	engine->exec_id = I915_EXEC_BSD;
> +	engine->hw_id = 4;
>
>   	engine->write_tail = ring_write_tail;
>   	engine->mmio_base = GEN8_BSD2_RING_BASE;
> @@ -3019,6 +3022,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
>   	engine->name = "blitter ring";
>   	engine->id = BCS;
>   	engine->exec_id = I915_EXEC_BLT;
> +	engine->hw_id = 2;
>
>   	engine->mmio_base = BLT_RING_BASE;
>   	engine->write_tail = ring_write_tail;
> @@ -3078,6 +3082,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
>   	engine->name = "video enhancement ring";
>   	engine->id = VECS;
>   	engine->exec_id = I915_EXEC_VEBOX;
> +	engine->hw_id = 3;
>
>   	engine->mmio_base = VEBOX_RING_BASE;
>   	engine->write_tail = ring_write_tail;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 7023e88531b5..2651fd5263eb 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -153,7 +153,8 @@ struct  intel_engine_cs {
>   #define I915_NUM_ENGINES 5
>   #define _VCS(n) (VCS + (n))
>   	unsigned int exec_id;
> -	unsigned int guc_id;
> +	unsigned int hw_id;
> +	unsigned int guc_id; /* XXX same as hw_id? */
>   	u32		mmio_base;
>   	struct		drm_device *dev;
>   	struct intel_ringbuffer *buffer;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Fix gen8 semaphores id for legacy mode

[Chris Wilson]

On Fri, Apr 29, 2016 at 09:36:37AM +0100, Tvrtko Ursulin wrote:
> 
> On 28/04/16 17:24, Chris Wilson wrote:
> >With the introduction of a distinct engine->id vs the hardware id, we need
> >to fix up the value we use for selecting the target engine when signaling
> >a semaphore. Note that these values can be merged with engine->guc_id.
> 
> So I broke something more with the decoupling, did not realize. I
> suppose it was still worth it. This at least wasn't being used.

A consolation prize: wean the guc over to a common hw_id :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: Fix ordering of sanitize ppgtt and sanitize execlists

[Joonas Lahtinen]

On to, 2016-04-28 at 17:24 +0100, Chris Wilson wrote:
> The i915.enable_ppgtt option depends upon the state of
> i915.enable_execlists option - so we need to sanitize execlists first.
> 

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_dma.c     | 13 +++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h     |  2 ++
>  drivers/gpu/drm/i915/i915_gem.c     |  3 ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 16 +++++-----------
>  drivers/gpu/drm/i915/intel_lrc.c    |  2 --
>  5 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index f69330cf0118..c91387f1aedd 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -968,6 +968,19 @@ static void intel_device_info_runtime_init(struct drm_device *dev)
>  			 info->has_subslice_pg ? "y" : "n");
>  	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
>  			 info->has_eu_pg ? "y" : "n");
> +
> +	i915.enable_execlists =
> +		intel_sanitize_enable_execlists(dev, i915.enable_execlists);
> +
> +	/*
> +	 * i915.enable_ppgtt is read-only, so do an early pass to validate the
> +	 * user's requested state against the hardware/driver capabilities.  We
> +	 * do this now so that we can print out any log messages once rather
> +	 * than every time we check intel_enable_ppgtt().
> +	 */
> +	i915.enable_ppgtt =
> +		intel_sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
> +	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915.enable_ppgtt);
>  }
>  
>  static void intel_init_dpio(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e2abbcc27f2c..17f22009f2b3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2753,6 +2753,8 @@ extern int i915_max_ioctl;
>  extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
>  extern int i915_resume_switcheroo(struct drm_device *dev);
>  
> +int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt);
> +
>  /* i915_dma.c */
>  void __printf(3, 4)
>  __i915_printk(struct drm_i915_private *dev_priv, const char *level,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 96fbc9ab4039..bd6061f177fe 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4901,9 +4901,6 @@ int i915_gem_init(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
>  
> -	i915.enable_execlists = intel_sanitize_enable_execlists(dev,
> -			i915.enable_execlists);
> -
>  	mutex_lock(&dev->struct_mutex);
>  
>  	if (!i915.enable_execlists) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 59a78f760b6b..364cf8236021 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -110,7 +110,7 @@ const struct i915_ggtt_view i915_ggtt_view_rotated = {
>  	.type = I915_GGTT_VIEW_ROTATED,
>  };
>  
> -static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
> +int intel_sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>  {
>  	bool has_aliasing_ppgtt;
>  	bool has_full_ppgtt;
> @@ -123,12 +123,14 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>  	if (intel_vgpu_active(dev))
>  		has_full_ppgtt = false; /* emulation is too hard */
>  
> +	if (!has_aliasing_ppgtt)
> +		return 0;
> +
>  	/*
>  	 * We don't allow disabling PPGTT for gen9+ as it's a requirement for
>  	 * execlists, the sole mechanism available to submit work.
>  	 */
> -	if (INTEL_INFO(dev)->gen < 9 &&
> -	    (enable_ppgtt == 0 || !has_aliasing_ppgtt))
> +	if (enable_ppgtt == 0 && INTEL_INFO(dev)->gen < 9)
>  		return 0;
>  
>  	if (enable_ppgtt == 1)
> @@ -3219,14 +3221,6 @@ int i915_ggtt_init_hw(struct drm_device *dev)
>  	if (intel_iommu_gfx_mapped)
>  		DRM_INFO("VT-d active for gfx access\n");
>  #endif
> -	/*
> -	 * i915.enable_ppgtt is read-only, so do an early pass to validate the
> -	 * user's requested state against the hardware/driver capabilities.  We
> -	 * do this now so that we can print out any log messages once rather
> -	 * than every time we check intel_enable_ppgtt().
> -	 */
> -	i915.enable_ppgtt = sanitize_enable_ppgtt(dev, i915.enable_ppgtt);
> -	DRM_DEBUG_DRIVER("ppgtt mode: %i\n", i915.enable_ppgtt);
>  
>  	return 0;
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 874c2515f9d4..4c832f90fe49 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -245,8 +245,6 @@ static int intel_lr_context_pin(struct intel_context *ctx,
>   */
>  int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists)
>  {
> -	WARN_ON(i915.enable_ppgtt == -1);
> -
>  	/* On platforms with execlist available, vGPU will only
>  	 * support execlist mode, no ring buffer mode.
>  	 */
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/10] drm/i915: Enable semaphores for legacy submission on gen8

[Daniel Vetter]

On Thu, Apr 28, 2016 at 05:24:31PM +0100, Chris Wilson wrote:
> We have sufficient evidence from igt to support that semaphores are in
> a working state. Enabling semaphores now for legacy provides a better
> comparison of execlists against legacy ring submission.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Somehow my reply didn't make it. Since still disabled by default, and mesa
CI folks confirmed that they stopped disabling execlist on bdw this seems
all safe. And useful for delta-benchmarking execlist.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index d37c0a671eed..9fd221c97275 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -542,10 +542,6 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
>  	if (i915.enable_execlists)
>  		return false;
>  
> -	/* Until we get further testing... */
> -	if (IS_GEN8(dev))
> -		return false;
> -
>  #ifdef CONFIG_INTEL_IOMMU
>  	/* Enable semaphores on SNB when IO remapping is off */
>  	if (INTEL_INFO(dev)->gen == 6 && intel_iommu_gfx_mapped)
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Fix gen8 semaphores id for legacy mode

[Dave Gordon]

[-- Attachment #1.1: Type: text/plain, Size: 666 bytes --]

On 29/04/2016 09:49, Chris Wilson wrote:
> On Fri, Apr 29, 2016 at 09:36:37AM +0100, Tvrtko Ursulin wrote:
>> On 28/04/16 17:24, Chris Wilson wrote:
>>> With the introduction of a distinct engine->id vs the hardware id, we need
>>> to fix up the value we use for selecting the target engine when signaling
>>> a semaphore. Note that these values can be merged with engine->guc_id.
>> So I broke something more with the decoupling, did not realize. I
>> suppose it was still worth it. This at least wasn't being used.
> A consolation prize: wean the guc over to a common hw_id :)
> -Chris
As in, use the GuC's concept of the engine ID for other purposes too?

.Dave.

[-- Attachment #1.2: Type: text/html, Size: 1312 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 03/10] drm/i915: Fix gen8 semaphores id for legacy mode

[Chris Wilson]

On Wed, May 04, 2016 at 12:35:09PM +0100, Dave Gordon wrote:
>    On 29/04/2016 09:49, Chris Wilson wrote:
> 
>  On Fri, Apr 29, 2016 at 09:36:37AM +0100, Tvrtko Ursulin wrote:
> 
>  On 28/04/16 17:24, Chris Wilson wrote:
> 
>  With the introduction of a distinct engine->id vs the hardware id, we need
>  to fix up the value we use for selecting the target engine when signaling
>  a semaphore. Note that these values can be merged with engine->guc_id.
> 
>  So I broke something more with the decoupling, did not realize. I
>  suppose it was still worth it. This at least wasn't being used.
> 
>  A consolation prize: wean the guc over to a common hw_id :)
>  -Chris
> 
> 
>    As in, use the GuC's concept of the engine ID for other purposes too?

Looks like the HW engine ID predates the GuC and so if they continued
to use the same values for the GuC (which for the engines that exist up
to and including gen9 they have), the GuC can use the older id and we
can have one fewer special GuC value.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx