[PATCH 01/13] drm/i915: Consolidate write_tail vfunc initializer

[PATCH 01/13] drm/i915: Consolidate write_tail vfunc initializer

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Introduce a function which initializes vfuncs mostly common
across engines and move write_tail initialization in it since
only one engine overrides the default.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 04a2d141e690..b715707947d8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2873,6 +2873,12 @@ static int gen6_ring_flush(struct drm_i915_gem_request *req,
 	return 0;
 }
 
+static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
+				      struct intel_engine_cs *engine)
+{
+	engine->write_tail = ring_write_tail;
+}
+
 int intel_init_render_ring_buffer(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2886,6 +2892,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	engine->hw_id = 0;
 	engine->mmio_base = RENDER_RING_BASE;
 
+	intel_ring_default_vfuncs(dev_priv, engine);
+
 	if (INTEL_GEN(dev_priv) >= 8) {
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			obj = i915_gem_object_create(dev, 4096);
@@ -2977,7 +2985,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		}
 		engine->irq_enable_mask = I915_USER_INTERRUPT;
 	}
-	engine->write_tail = ring_write_tail;
 
 	if (IS_HASWELL(dev_priv))
 		engine->dispatch_execbuffer = hsw_ring_dispatch_execbuffer;
@@ -3036,7 +3043,8 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 	engine->exec_id = I915_EXEC_BSD;
 	engine->hw_id = 1;
 
-	engine->write_tail = ring_write_tail;
+	intel_ring_default_vfuncs(dev_priv, engine);
+
 	if (INTEL_GEN(dev_priv) >= 6) {
 		engine->mmio_base = GEN6_BSD_RING_BASE;
 		/* gen6 bsd needs a special wa for tail updates */
@@ -3114,9 +3122,10 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	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;
+
+	intel_ring_default_vfuncs(dev_priv, engine);
+
 	engine->flush = gen6_bsd_ring_flush;
 	engine->add_request = gen6_add_request;
 	engine->irq_seqno_barrier = gen6_seqno_barrier;
@@ -3147,9 +3156,10 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	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;
+
+	intel_ring_default_vfuncs(dev_priv, engine);
+
 	engine->flush = gen6_ring_flush;
 	engine->add_request = gen6_add_request;
 	engine->irq_seqno_barrier = gen6_seqno_barrier;
@@ -3207,9 +3217,10 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	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;
+
+	intel_ring_default_vfuncs(dev_priv, engine);
+
 	engine->flush = gen6_ring_flush;
 	engine->add_request = gen6_add_request;
 	engine->irq_seqno_barrier = gen6_seqno_barrier;
-- 
1.9.1

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

[PATCH 02/13] drm/i915: Consolidate add_request vfunc

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

All engines apart from render select this based on Gen.

Move it to the common helper as well.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b715707947d8..d82eb12ed6b6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2877,6 +2877,11 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 				      struct intel_engine_cs *engine)
 {
 	engine->write_tail = ring_write_tail;
+
+	if (INTEL_GEN(dev_priv) >= 6)
+		engine->add_request = gen6_add_request;
+	else
+		engine->add_request = i9xx_add_request;
 }
 
 int intel_init_render_ring_buffer(struct drm_device *dev)
@@ -2928,7 +2933,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		}
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		engine->init_context = intel_rcs_ctx_init;
-		engine->add_request = gen6_add_request;
 		engine->flush = gen7_render_ring_flush;
 		if (IS_GEN6(dev_priv))
 			engine->flush = gen6_render_ring_flush;
@@ -2969,7 +2973,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT |
 					GT_RENDER_PIPECTL_NOTIFY_INTERRUPT;
 	} else {
-		engine->add_request = i9xx_add_request;
 		if (INTEL_GEN(dev_priv) < 4)
 			engine->flush = gen2_render_ring_flush;
 		else
@@ -3051,7 +3054,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		if (IS_GEN6(dev_priv))
 			engine->write_tail = gen6_bsd_ring_write_tail;
 		engine->flush = gen6_bsd_ring_flush;
-		engine->add_request = gen6_add_request;
 		engine->irq_seqno_barrier = gen6_seqno_barrier;
 		engine->get_seqno = ring_get_seqno;
 		engine->set_seqno = ring_set_seqno;
@@ -3091,7 +3093,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 	} else {
 		engine->mmio_base = BSD_RING_BASE;
 		engine->flush = bsd_ring_flush;
-		engine->add_request = i9xx_add_request;
 		engine->get_seqno = ring_get_seqno;
 		engine->set_seqno = ring_set_seqno;
 		if (IS_GEN5(dev_priv)) {
@@ -3127,7 +3128,6 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_bsd_ring_flush;
-	engine->add_request = gen6_add_request;
 	engine->irq_seqno_barrier = gen6_seqno_barrier;
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
@@ -3161,7 +3161,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_ring_flush;
-	engine->add_request = gen6_add_request;
 	engine->irq_seqno_barrier = gen6_seqno_barrier;
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
@@ -3222,7 +3221,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_ring_flush;
-	engine->add_request = gen6_add_request;
 	engine->irq_seqno_barrier = gen6_seqno_barrier;
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
-- 
1.9.1

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

[PATCH 03/13] drm/i915: Consolidate seqno_barrier vfunc

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d82eb12ed6b6..a4391cbbb2b6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2878,10 +2878,12 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 {
 	engine->write_tail = ring_write_tail;
 
-	if (INTEL_GEN(dev_priv) >= 6)
+	if (INTEL_GEN(dev_priv) >= 6) {
 		engine->add_request = gen6_add_request;
-	else
+		engine->irq_seqno_barrier = gen6_seqno_barrier;
+	} else {
 		engine->add_request = i9xx_add_request;
+	}
 }
 
 int intel_init_render_ring_buffer(struct drm_device *dev)
@@ -2939,7 +2941,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->irq_get = gen6_ring_get_irq;
 		engine->irq_put = gen6_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_priv)) {
@@ -3054,7 +3055,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		if (IS_GEN6(dev_priv))
 			engine->write_tail = gen6_bsd_ring_write_tail;
 		engine->flush = gen6_bsd_ring_flush;
-		engine->irq_seqno_barrier = gen6_seqno_barrier;
 		engine->get_seqno = ring_get_seqno;
 		engine->set_seqno = ring_set_seqno;
 		if (INTEL_GEN(dev_priv) >= 8) {
@@ -3128,7 +3128,6 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_bsd_ring_flush;
-	engine->irq_seqno_barrier = gen6_seqno_barrier;
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
 	engine->irq_enable_mask =
@@ -3161,7 +3160,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_ring_flush;
-	engine->irq_seqno_barrier = gen6_seqno_barrier;
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
 	if (INTEL_GEN(dev_priv) >= 8) {
@@ -3221,7 +3219,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_ring_flush;
-	engine->irq_seqno_barrier = gen6_seqno_barrier;
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
 
-- 
1.9.1

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

[PATCH 04/13] drm/i915: Consolidate get and put irq vfuncs

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

v2: Consistent INTEL_GEN vs IS_GEN usage. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 46 ++++++++++++---------------------
 1 file changed, 17 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a4391cbbb2b6..8d9e2e24f67d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2884,6 +2884,23 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 	} else {
 		engine->add_request = i9xx_add_request;
 	}
+
+	if (INTEL_GEN(dev_priv) >= 8) {
+		engine->irq_get = gen8_ring_get_irq;
+		engine->irq_put = gen8_ring_put_irq;
+	} else if (INTEL_GEN(dev_priv) >= 6) {
+		engine->irq_get = gen6_ring_get_irq;
+		engine->irq_put = gen6_ring_put_irq;
+	} else if (INTEL_GEN(dev_priv) >= 5) {
+		engine->irq_get = gen5_ring_get_irq;
+		engine->irq_put = gen5_ring_put_irq;
+	} else if (INTEL_GEN(dev_priv) >= 3) {
+		engine->irq_get = i9xx_ring_get_irq;
+		engine->irq_put = i9xx_ring_put_irq;
+	} else {
+		engine->irq_get = i8xx_ring_get_irq;
+		engine->irq_put = i8xx_ring_put_irq;
+	}
 }
 
 int intel_init_render_ring_buffer(struct drm_device *dev)
@@ -2922,8 +2939,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->init_context = intel_rcs_ctx_init;
 		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->get_seqno = ring_get_seqno;
 		engine->set_seqno = ring_set_seqno;
@@ -2938,8 +2953,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->flush = gen7_render_ring_flush;
 		if (IS_GEN6(dev_priv))
 			engine->flush = gen6_render_ring_flush;
-		engine->irq_get = gen6_ring_get_irq;
-		engine->irq_put = gen6_ring_put_irq;
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
 		engine->get_seqno = ring_get_seqno;
 		engine->set_seqno = ring_set_seqno;
@@ -2969,8 +2982,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->flush = gen4_render_ring_flush;
 		engine->get_seqno = pc_render_get_seqno;
 		engine->set_seqno = pc_render_set_seqno;
-		engine->irq_get = gen5_ring_get_irq;
-		engine->irq_put = gen5_ring_put_irq;
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT |
 					GT_RENDER_PIPECTL_NOTIFY_INTERRUPT;
 	} else {
@@ -2980,13 +2991,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 			engine->flush = gen4_render_ring_flush;
 		engine->get_seqno = ring_get_seqno;
 		engine->set_seqno = ring_set_seqno;
-		if (IS_GEN2(dev_priv)) {
-			engine->irq_get = i8xx_ring_get_irq;
-			engine->irq_put = i8xx_ring_put_irq;
-		} else {
-			engine->irq_get = i9xx_ring_get_irq;
-			engine->irq_put = i9xx_ring_put_irq;
-		}
 		engine->irq_enable_mask = I915_USER_INTERRUPT;
 	}
 
@@ -3060,8 +3064,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		if (INTEL_GEN(dev_priv) >= 8) {
 			engine->irq_enable_mask =
 				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
-			engine->irq_get = gen8_ring_get_irq;
-			engine->irq_put = gen8_ring_put_irq;
 			engine->dispatch_execbuffer =
 				gen8_ring_dispatch_execbuffer;
 			if (i915_semaphore_is_enabled(dev_priv)) {
@@ -3071,8 +3073,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 			}
 		} else {
 			engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
-			engine->irq_get = gen6_ring_get_irq;
-			engine->irq_put = gen6_ring_put_irq;
 			engine->dispatch_execbuffer =
 				gen6_ring_dispatch_execbuffer;
 			if (i915_semaphore_is_enabled(dev_priv)) {
@@ -3097,12 +3097,8 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		engine->set_seqno = ring_set_seqno;
 		if (IS_GEN5(dev_priv)) {
 			engine->irq_enable_mask = ILK_BSD_USER_INTERRUPT;
-			engine->irq_get = gen5_ring_get_irq;
-			engine->irq_put = gen5_ring_put_irq;
 		} else {
 			engine->irq_enable_mask = I915_BSD_USER_INTERRUPT;
-			engine->irq_get = i9xx_ring_get_irq;
-			engine->irq_put = i9xx_ring_put_irq;
 		}
 		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
 	}
@@ -3132,8 +3128,6 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	engine->set_seqno = ring_set_seqno;
 	engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
-	engine->irq_get = gen8_ring_get_irq;
-	engine->irq_put = gen8_ring_put_irq;
 	engine->dispatch_execbuffer =
 			gen8_ring_dispatch_execbuffer;
 	if (i915_semaphore_is_enabled(dev_priv)) {
@@ -3165,8 +3159,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
-		engine->irq_get = gen8_ring_get_irq;
-		engine->irq_put = gen8_ring_put_irq;
 		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen8_ring_sync;
@@ -3175,8 +3167,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 		}
 	} else {
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
-		engine->irq_get = gen6_ring_get_irq;
-		engine->irq_put = gen6_ring_put_irq;
 		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.signal = gen6_signal;
@@ -3225,8 +3215,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT;
-		engine->irq_get = gen8_ring_get_irq;
-		engine->irq_put = gen8_ring_put_irq;
 		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen8_ring_sync;
-- 
1.9.1

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

[PATCH 05/13] drm/i915: Consolidate get/set_seqno

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8d9e2e24f67d..e0e90b99bbca 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2877,6 +2877,8 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 				      struct intel_engine_cs *engine)
 {
 	engine->write_tail = ring_write_tail;
+	engine->get_seqno = ring_get_seqno;
+	engine->set_seqno = ring_set_seqno;
 
 	if (INTEL_GEN(dev_priv) >= 6) {
 		engine->add_request = gen6_add_request;
@@ -2940,8 +2942,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->add_request = gen8_render_add_request;
 		engine->flush = gen8_render_ring_flush;
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
-		engine->get_seqno = ring_get_seqno;
-		engine->set_seqno = ring_set_seqno;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			WARN_ON(!dev_priv->semaphore_obj);
 			engine->semaphore.sync_to = gen8_ring_sync;
@@ -2954,8 +2954,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		if (IS_GEN6(dev_priv))
 			engine->flush = gen6_render_ring_flush;
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
-		engine->get_seqno = ring_get_seqno;
-		engine->set_seqno = ring_set_seqno;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen6_ring_sync;
 			engine->semaphore.signal = gen6_signal;
@@ -2989,8 +2987,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 			engine->flush = gen2_render_ring_flush;
 		else
 			engine->flush = gen4_render_ring_flush;
-		engine->get_seqno = ring_get_seqno;
-		engine->set_seqno = ring_set_seqno;
 		engine->irq_enable_mask = I915_USER_INTERRUPT;
 	}
 
@@ -3059,8 +3055,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		if (IS_GEN6(dev_priv))
 			engine->write_tail = gen6_bsd_ring_write_tail;
 		engine->flush = gen6_bsd_ring_flush;
-		engine->get_seqno = ring_get_seqno;
-		engine->set_seqno = ring_set_seqno;
 		if (INTEL_GEN(dev_priv) >= 8) {
 			engine->irq_enable_mask =
 				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
@@ -3093,8 +3087,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 	} else {
 		engine->mmio_base = BSD_RING_BASE;
 		engine->flush = bsd_ring_flush;
-		engine->get_seqno = ring_get_seqno;
-		engine->set_seqno = ring_set_seqno;
 		if (IS_GEN5(dev_priv)) {
 			engine->irq_enable_mask = ILK_BSD_USER_INTERRUPT;
 		} else {
@@ -3124,8 +3116,6 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_bsd_ring_flush;
-	engine->get_seqno = ring_get_seqno;
-	engine->set_seqno = ring_set_seqno;
 	engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
 	engine->dispatch_execbuffer =
@@ -3154,8 +3144,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_ring_flush;
-	engine->get_seqno = ring_get_seqno;
-	engine->set_seqno = ring_set_seqno;
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
@@ -3209,8 +3197,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_ring_flush;
-	engine->get_seqno = ring_get_seqno;
-	engine->set_seqno = ring_set_seqno;
 
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
-- 
1.9.1

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

[PATCH 06/13] drm/i915: Consolidate init_hw vfunc

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e0e90b99bbca..dcacf17c525f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2876,6 +2876,7 @@ static int gen6_ring_flush(struct drm_i915_gem_request *req,
 static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 				      struct intel_engine_cs *engine)
 {
+	engine->init_hw = init_ring_common;
 	engine->write_tail = ring_write_tail;
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
@@ -3094,7 +3095,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		}
 		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
 	}
-	engine->init_hw = init_ring_common;
 
 	return intel_init_ring_buffer(dev, engine);
 }
@@ -3125,7 +3125,6 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 		engine->semaphore.signal = gen8_xcs_signal;
 		GEN8_RING_SEMAPHORE_INIT(engine);
 	}
-	engine->init_hw = init_ring_common;
 
 	return intel_init_ring_buffer(dev, engine);
 }
@@ -3178,7 +3177,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 			engine->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
 		}
 	}
-	engine->init_hw = init_ring_common;
 
 	return intel_init_ring_buffer(dev, engine);
 }
@@ -3227,7 +3225,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 			engine->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
 		}
 	}
-	engine->init_hw = init_ring_common;
 
 	return intel_init_ring_buffer(dev, engine);
 }
-- 
1.9.1

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

[PATCH 07/13] drm/i915: Consolidate dispatch_execbuffer vfunc

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

v2: Put dispatch_execbuffer before add_request. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index dcacf17c525f..a961b095680b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2881,10 +2881,14 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
 
-	if (INTEL_GEN(dev_priv) >= 6) {
+	if (INTEL_GEN(dev_priv) >= 8) {
+		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
+	} else if (INTEL_GEN(dev_priv) >= 6) {
+		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		engine->add_request = gen6_add_request;
 		engine->irq_seqno_barrier = gen6_seqno_barrier;
 	} else {
+		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
 		engine->add_request = i9xx_add_request;
 	}
 
@@ -2993,15 +2997,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 
 	if (IS_HASWELL(dev_priv))
 		engine->dispatch_execbuffer = hsw_ring_dispatch_execbuffer;
-	else if (IS_GEN8(dev_priv))
-		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
-	else if (INTEL_GEN(dev_priv) >= 6)
-		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
-	else if (INTEL_GEN(dev_priv) >= 4)
-		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
 	else if (IS_I830(dev_priv) || IS_845G(dev_priv))
 		engine->dispatch_execbuffer = i830_dispatch_execbuffer;
-	else
+	else if (INTEL_GEN(dev_priv) <= 3)
 		engine->dispatch_execbuffer = i915_dispatch_execbuffer;
 	engine->init_hw = init_render_ring;
 	engine->cleanup = render_ring_cleanup;
@@ -3059,8 +3057,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		if (INTEL_GEN(dev_priv) >= 8) {
 			engine->irq_enable_mask =
 				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
-			engine->dispatch_execbuffer =
-				gen8_ring_dispatch_execbuffer;
 			if (i915_semaphore_is_enabled(dev_priv)) {
 				engine->semaphore.sync_to = gen8_ring_sync;
 				engine->semaphore.signal = gen8_xcs_signal;
@@ -3068,8 +3064,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 			}
 		} else {
 			engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
-			engine->dispatch_execbuffer =
-				gen6_ring_dispatch_execbuffer;
 			if (i915_semaphore_is_enabled(dev_priv)) {
 				engine->semaphore.sync_to = gen6_ring_sync;
 				engine->semaphore.signal = gen6_signal;
@@ -3093,7 +3087,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		} else {
 			engine->irq_enable_mask = I915_BSD_USER_INTERRUPT;
 		}
-		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
 	}
 
 	return intel_init_ring_buffer(dev, engine);
@@ -3118,8 +3111,6 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	engine->flush = gen6_bsd_ring_flush;
 	engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
-	engine->dispatch_execbuffer =
-			gen8_ring_dispatch_execbuffer;
 	if (i915_semaphore_is_enabled(dev_priv)) {
 		engine->semaphore.sync_to = gen8_ring_sync;
 		engine->semaphore.signal = gen8_xcs_signal;
@@ -3146,7 +3137,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
-		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen8_ring_sync;
 			engine->semaphore.signal = gen8_xcs_signal;
@@ -3154,7 +3144,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 		}
 	} else {
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
-		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.signal = gen6_signal;
 			engine->semaphore.sync_to = gen6_ring_sync;
@@ -3199,7 +3188,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT;
-		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen8_ring_sync;
 			engine->semaphore.signal = gen8_xcs_signal;
@@ -3209,7 +3197,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 		engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
 		engine->irq_get = hsw_vebox_get_irq;
 		engine->irq_put = hsw_vebox_put_irq;
-		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen6_ring_sync;
 			engine->semaphore.signal = gen6_signal;
-- 
1.9.1

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

[PATCH 08/13] drm/i915: Consolidate semaphore vfuncs init

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 48 +++++++++++++--------------------
 1 file changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a961b095680b..d0401bb800a6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2873,6 +2873,22 @@ static int gen6_ring_flush(struct drm_i915_gem_request *req,
 	return 0;
 }
 
+static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
+				       struct intel_engine_cs *engine)
+{
+	if (!i915_semaphore_is_enabled(dev_priv))
+		return;
+
+	if (INTEL_GEN(dev_priv) >= 8) {
+		engine->semaphore.sync_to = gen8_ring_sync;
+		engine->semaphore.signal = gen8_xcs_signal;
+		GEN8_RING_SEMAPHORE_INIT(engine);
+	} else if (INTEL_GEN(dev_priv) >= 6) {
+		engine->semaphore.sync_to = gen6_ring_sync;
+		engine->semaphore.signal = gen6_signal;
+	}
+}
+
 static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 				      struct intel_engine_cs *engine)
 {
@@ -2908,6 +2924,8 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 		engine->irq_get = i8xx_ring_get_irq;
 		engine->irq_put = i8xx_ring_put_irq;
 	}
+
+	intel_ring_init_semaphores(dev_priv, engine);
 }
 
 int intel_init_render_ring_buffer(struct drm_device *dev)
@@ -2949,9 +2967,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			WARN_ON(!dev_priv->semaphore_obj);
-			engine->semaphore.sync_to = gen8_ring_sync;
 			engine->semaphore.signal = gen8_rcs_signal;
-			GEN8_RING_SEMAPHORE_INIT(engine);
 		}
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		engine->init_context = intel_rcs_ctx_init;
@@ -2960,8 +2976,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 			engine->flush = gen6_render_ring_flush;
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
 		if (i915_semaphore_is_enabled(dev_priv)) {
-			engine->semaphore.sync_to = gen6_ring_sync;
-			engine->semaphore.signal = gen6_signal;
 			/*
 			 * The current semaphore is only applied on pre-gen8
 			 * platform.  And there is no VCS2 ring on the pre-gen8
@@ -3057,16 +3071,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		if (INTEL_GEN(dev_priv) >= 8) {
 			engine->irq_enable_mask =
 				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
-			if (i915_semaphore_is_enabled(dev_priv)) {
-				engine->semaphore.sync_to = gen8_ring_sync;
-				engine->semaphore.signal = gen8_xcs_signal;
-				GEN8_RING_SEMAPHORE_INIT(engine);
-			}
 		} else {
 			engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
 			if (i915_semaphore_is_enabled(dev_priv)) {
-				engine->semaphore.sync_to = gen6_ring_sync;
-				engine->semaphore.signal = gen6_signal;
 				engine->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VR;
 				engine->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_INVALID;
 				engine->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_VB;
@@ -3111,11 +3118,6 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	engine->flush = gen6_bsd_ring_flush;
 	engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
-	if (i915_semaphore_is_enabled(dev_priv)) {
-		engine->semaphore.sync_to = gen8_ring_sync;
-		engine->semaphore.signal = gen8_xcs_signal;
-		GEN8_RING_SEMAPHORE_INIT(engine);
-	}
 
 	return intel_init_ring_buffer(dev, engine);
 }
@@ -3137,16 +3139,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
-		if (i915_semaphore_is_enabled(dev_priv)) {
-			engine->semaphore.sync_to = gen8_ring_sync;
-			engine->semaphore.signal = gen8_xcs_signal;
-			GEN8_RING_SEMAPHORE_INIT(engine);
-		}
 	} else {
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
 		if (i915_semaphore_is_enabled(dev_priv)) {
-			engine->semaphore.signal = gen6_signal;
-			engine->semaphore.sync_to = gen6_ring_sync;
 			/*
 			 * The current semaphore is only applied on pre-gen8
 			 * platform.  And there is no VCS2 ring on the pre-gen8
@@ -3188,18 +3183,11 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT;
-		if (i915_semaphore_is_enabled(dev_priv)) {
-			engine->semaphore.sync_to = gen8_ring_sync;
-			engine->semaphore.signal = gen8_xcs_signal;
-			GEN8_RING_SEMAPHORE_INIT(engine);
-		}
 	} else {
 		engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
 		engine->irq_get = hsw_vebox_get_irq;
 		engine->irq_put = hsw_vebox_put_irq;
 		if (i915_semaphore_is_enabled(dev_priv)) {
-			engine->semaphore.sync_to = gen6_ring_sync;
-			engine->semaphore.signal = gen6_signal;
 			engine->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VER;
 			engine->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_VEV;
 			engine->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_VEB;
-- 
1.9.1

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

[PATCH 09/13] drm/i915: Move semaphore object creation into intel_ring_init_semaphores

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

The object needs to be created before semaphores can be initialized
on any ring and it makes sense to pull it out to this semaphore
dedicated helper.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 45 ++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d0401bb800a6..77d663fcdff1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2876,6 +2876,30 @@ static int gen6_ring_flush(struct drm_i915_gem_request *req,
 static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 				       struct intel_engine_cs *engine)
 {
+	struct drm_i915_gem_object *obj;
+	int ret;
+
+	if (!i915_semaphore_is_enabled(dev_priv))
+		return;
+
+	if (INTEL_GEN(dev_priv) >= 8 && !dev_priv->semaphore_obj) {
+		obj = i915_gem_object_create(dev_priv->dev, 4096);
+		if (IS_ERR(obj)) {
+			DRM_ERROR("Failed to allocate semaphore bo. Disabling semaphores\n");
+			i915.semaphores = 0;
+		} else {
+			i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
+			ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_NONBLOCK);
+			if (ret != 0) {
+				drm_gem_object_unreference(&obj->base);
+				DRM_ERROR("Failed to pin semaphore bo. Disabling semaphores\n");
+				i915.semaphores = 0;
+			} else {
+				dev_priv->semaphore_obj = obj;
+			}
+		}
+	}
+
 	if (!i915_semaphore_is_enabled(dev_priv))
 		return;
 
@@ -2944,31 +2968,12 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	if (INTEL_GEN(dev_priv) >= 8) {
-		if (i915_semaphore_is_enabled(dev_priv)) {
-			obj = i915_gem_object_create(dev, 4096);
-			if (IS_ERR(obj)) {
-				DRM_ERROR("Failed to allocate semaphore bo. Disabling semaphores\n");
-				i915.semaphores = 0;
-			} else {
-				i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
-				ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_NONBLOCK);
-				if (ret != 0) {
-					drm_gem_object_unreference(&obj->base);
-					DRM_ERROR("Failed to pin semaphore bo. Disabling semaphores\n");
-					i915.semaphores = 0;
-				} else
-					dev_priv->semaphore_obj = obj;
-			}
-		}
-
 		engine->init_context = intel_rcs_ctx_init;
 		engine->add_request = gen8_render_add_request;
 		engine->flush = gen8_render_ring_flush;
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
-		if (i915_semaphore_is_enabled(dev_priv)) {
-			WARN_ON(!dev_priv->semaphore_obj);
+		if (i915_semaphore_is_enabled(dev_priv))
 			engine->semaphore.signal = gen8_rcs_signal;
-		}
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		engine->init_context = intel_rcs_ctx_init;
 		engine->flush = gen7_render_ring_flush;
-- 
1.9.1

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

[PATCH 10/13] drm/i915: Compact Gen8 semaphore initialization

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Replace the macro initializer with a programatic loop which
results in smaller code and hopefully just as clear.

v2: Rebase.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 16 ++++++++++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h | 12 ------------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 77d663fcdff1..14218d893d7b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2877,7 +2877,7 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 				       struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_object *obj;
-	int ret;
+	int ret, i;
 
 	if (!i915_semaphore_is_enabled(dev_priv))
 		return;
@@ -2904,9 +2904,21 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 		return;
 
 	if (INTEL_GEN(dev_priv) >= 8) {
+		u64 offset = i915_gem_obj_ggtt_offset(dev_priv->semaphore_obj);
+
 		engine->semaphore.sync_to = gen8_ring_sync;
 		engine->semaphore.signal = gen8_xcs_signal;
-		GEN8_RING_SEMAPHORE_INIT(engine);
+
+		for (i = 0; i < I915_NUM_ENGINES; i++) {
+			u64 ring_offset;
+
+			if (i != engine->id)
+				ring_offset = offset + GEN8_SEMAPHORE_OFFSET(engine->id, i);
+			else
+				ring_offset = MI_SEMAPHORE_SYNC_INVALID;
+
+			engine->semaphore.signal_ggtt[i] = ring_offset;
+		}
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		engine->semaphore.sync_to = gen6_ring_sync;
 		engine->semaphore.signal = gen6_signal;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b33c876fed20..113d5230a6de 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -62,18 +62,6 @@ struct  intel_hw_status_page {
 	(i915_gem_obj_ggtt_offset(dev_priv->semaphore_obj) + \
 	 GEN8_SEMAPHORE_OFFSET(from, (__ring)->id))
 
-#define GEN8_RING_SEMAPHORE_INIT(e) do { \
-	if (!dev_priv->semaphore_obj) { \
-		break; \
-	} \
-	(e)->semaphore.signal_ggtt[RCS] = GEN8_SIGNAL_OFFSET((e), RCS); \
-	(e)->semaphore.signal_ggtt[VCS] = GEN8_SIGNAL_OFFSET((e), VCS); \
-	(e)->semaphore.signal_ggtt[BCS] = GEN8_SIGNAL_OFFSET((e), BCS); \
-	(e)->semaphore.signal_ggtt[VECS] = GEN8_SIGNAL_OFFSET((e), VECS); \
-	(e)->semaphore.signal_ggtt[VCS2] = GEN8_SIGNAL_OFFSET((e), VCS2); \
-	(e)->semaphore.signal_ggtt[(e)->id] = MI_SEMAPHORE_SYNC_INVALID; \
-	} while(0)
-
 enum intel_ring_hangcheck_action {
 	HANGCHECK_IDLE = 0,
 	HANGCHECK_WAIT,
-- 
1.9.1

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

[PATCH 11/13] drm/i915: Compact gen8_ring_sync

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Store the semaphore offset in a temporary variable to avoid
having to get the VMA offset twice.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 14218d893d7b..648ddee60c24 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1542,6 +1542,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_req->i915;
+	u64 offset = GEN8_WAIT_OFFSET(waiter, signaller->id);
 	struct i915_hw_ppgtt *ppgtt;
 	int ret;
 
@@ -1553,10 +1554,8 @@ gen8_ring_sync(struct drm_i915_gem_request *waiter_req,
 				MI_SEMAPHORE_GLOBAL_GTT |
 				MI_SEMAPHORE_SAD_GTE_SDD);
 	intel_ring_emit(waiter, seqno);
-	intel_ring_emit(waiter,
-			lower_32_bits(GEN8_WAIT_OFFSET(waiter, signaller->id)));
-	intel_ring_emit(waiter,
-			upper_32_bits(GEN8_WAIT_OFFSET(waiter, signaller->id)));
+	intel_ring_emit(waiter, lower_32_bits(offset));
+	intel_ring_emit(waiter, upper_32_bits(offset));
 	intel_ring_advance(waiter);
 
 	/* When the !RCS engines idle waiting upon a semaphore, they lose their
-- 
1.9.1

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

[PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Replace per-engine initialization with a common half-programatic,
half-data driven code for ease of maintenance and compactness.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 110 ++++++++++++++------------------
 1 file changed, 48 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 648ddee60c24..7bbc59eef267 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2921,6 +2921,54 @@ static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 	} else if (INTEL_GEN(dev_priv) >= 6) {
 		engine->semaphore.sync_to = gen6_ring_sync;
 		engine->semaphore.signal = gen6_signal;
+
+		/*
+		 * The current semaphore is only applied on pre-gen8
+		 * platform.  And there is no VCS2 ring on the pre-gen8
+		 * platform. So the semaphore between RCS and VCS2 is
+		 * initialized as INVALID.  Gen8 will initialize the
+		 * sema between VCS2 and RCS later.
+		 */
+		for (i = 0; i < I915_NUM_ENGINES; i++) {
+			static const struct {
+				u32 wait_mbox;
+				i915_reg_t mbox_reg;
+			} sem_data[I915_NUM_ENGINES][I915_NUM_ENGINES] = {
+				[RCS] = {
+					[VCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_RV,  .mbox_reg = GEN6_VRSYNC },
+					[BCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_RB,  .mbox_reg = GEN6_BRSYNC },
+					[VECS] = { .wait_mbox = MI_SEMAPHORE_SYNC_RVE, .mbox_reg = GEN6_VERSYNC },
+				},
+				[VCS] = {
+					[RCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_VR,  .mbox_reg = GEN6_RVSYNC },
+					[BCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_VB,  .mbox_reg = GEN6_BVSYNC },
+					[VECS] = { .wait_mbox = MI_SEMAPHORE_SYNC_VVE, .mbox_reg = GEN6_VEVSYNC },
+				},
+				[BCS] = {
+					[RCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_BR,  .mbox_reg = GEN6_RBSYNC },
+					[VCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_BV,  .mbox_reg = GEN6_VBSYNC },
+					[VECS] = { .wait_mbox = MI_SEMAPHORE_SYNC_BVE, .mbox_reg = GEN6_VEBSYNC },
+				},
+				[VECS] = {
+					[RCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_VER, .mbox_reg = GEN6_RVESYNC },
+					[VCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_VEV, .mbox_reg = GEN6_VVESYNC },
+					[BCS] =  { .wait_mbox = MI_SEMAPHORE_SYNC_VEB, .mbox_reg = GEN6_BVESYNC },
+				},
+			};
+			u32 wait_mbox;
+			i915_reg_t mbox_reg;
+
+			if (i == engine->id || i == VCS2) {
+				wait_mbox = MI_SEMAPHORE_SYNC_INVALID;
+				mbox_reg = GEN6_NOSYNC;
+			} else {
+				wait_mbox = sem_data[engine->id][i].wait_mbox;
+				mbox_reg = sem_data[engine->id][i].mbox_reg;
+			}
+
+			engine->semaphore.mbox.wait[i] = wait_mbox;
+			engine->semaphore.mbox.signal[i] = mbox_reg;
+		}
 	}
 }
 
@@ -2991,25 +3039,6 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		if (IS_GEN6(dev_priv))
 			engine->flush = gen6_render_ring_flush;
 		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
-		if (i915_semaphore_is_enabled(dev_priv)) {
-			/*
-			 * The current semaphore is only applied on pre-gen8
-			 * platform.  And there is no VCS2 ring on the pre-gen8
-			 * platform. So the semaphore between RCS and VCS2 is
-			 * initialized as INVALID.  Gen8 will initialize the
-			 * sema between VCS2 and RCS later.
-			 */
-			engine->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_INVALID;
-			engine->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_RV;
-			engine->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_RB;
-			engine->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_RVE;
-			engine->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
-			engine->semaphore.mbox.signal[RCS] = GEN6_NOSYNC;
-			engine->semaphore.mbox.signal[VCS] = GEN6_VRSYNC;
-			engine->semaphore.mbox.signal[BCS] = GEN6_BRSYNC;
-			engine->semaphore.mbox.signal[VECS] = GEN6_VERSYNC;
-			engine->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
-		}
 	} else if (IS_GEN5(dev_priv)) {
 		engine->add_request = pc_render_add_request;
 		engine->flush = gen4_render_ring_flush;
@@ -3089,18 +3118,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
 		} else {
 			engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
-			if (i915_semaphore_is_enabled(dev_priv)) {
-				engine->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VR;
-				engine->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_INVALID;
-				engine->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_VB;
-				engine->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_VVE;
-				engine->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
-				engine->semaphore.mbox.signal[RCS] = GEN6_RVSYNC;
-				engine->semaphore.mbox.signal[VCS] = GEN6_NOSYNC;
-				engine->semaphore.mbox.signal[BCS] = GEN6_BVSYNC;
-				engine->semaphore.mbox.signal[VECS] = GEN6_VEVSYNC;
-				engine->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
-			}
 		}
 	} else {
 		engine->mmio_base = BSD_RING_BASE;
@@ -3157,25 +3174,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
 	} else {
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
-		if (i915_semaphore_is_enabled(dev_priv)) {
-			/*
-			 * The current semaphore is only applied on pre-gen8
-			 * platform.  And there is no VCS2 ring on the pre-gen8
-			 * platform. So the semaphore between BCS and VCS2 is
-			 * initialized as INVALID.  Gen8 will initialize the
-			 * sema between BCS and VCS2 later.
-			 */
-			engine->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_BR;
-			engine->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_BV;
-			engine->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_INVALID;
-			engine->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_BVE;
-			engine->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
-			engine->semaphore.mbox.signal[RCS] = GEN6_RBSYNC;
-			engine->semaphore.mbox.signal[VCS] = GEN6_VBSYNC;
-			engine->semaphore.mbox.signal[BCS] = GEN6_NOSYNC;
-			engine->semaphore.mbox.signal[VECS] = GEN6_VEBSYNC;
-			engine->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
-		}
 	}
 
 	return intel_init_ring_buffer(dev, engine);
@@ -3203,18 +3201,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 		engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
 		engine->irq_get = hsw_vebox_get_irq;
 		engine->irq_put = hsw_vebox_put_irq;
-		if (i915_semaphore_is_enabled(dev_priv)) {
-			engine->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VER;
-			engine->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_VEV;
-			engine->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_VEB;
-			engine->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_INVALID;
-			engine->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
-			engine->semaphore.mbox.signal[RCS] = GEN6_RVESYNC;
-			engine->semaphore.mbox.signal[VCS] = GEN6_VVESYNC;
-			engine->semaphore.mbox.signal[BCS] = GEN6_BVESYNC;
-			engine->semaphore.mbox.signal[VECS] = GEN6_NOSYNC;
-			engine->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
-		}
 	}
 
 	return intel_init_ring_buffer(dev, engine);
-- 
1.9.1

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

[PATCH 13/13] drm/i915: Trim some if-else braces

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Just a bit of cleanup after the previous refactoring.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 7bbc59eef267..0df7a13c0992 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -3113,20 +3113,18 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		if (IS_GEN6(dev_priv))
 			engine->write_tail = gen6_bsd_ring_write_tail;
 		engine->flush = gen6_bsd_ring_flush;
-		if (INTEL_GEN(dev_priv) >= 8) {
+		if (INTEL_GEN(dev_priv) >= 8)
 			engine->irq_enable_mask =
 				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
-		} else {
+		else
 			engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
-		}
 	} else {
 		engine->mmio_base = BSD_RING_BASE;
 		engine->flush = bsd_ring_flush;
-		if (IS_GEN5(dev_priv)) {
+		if (IS_GEN5(dev_priv))
 			engine->irq_enable_mask = ILK_BSD_USER_INTERRUPT;
-		} else {
+		else
 			engine->irq_enable_mask = I915_BSD_USER_INTERRUPT;
-		}
 	}
 
 	return intel_init_ring_buffer(dev, engine);
@@ -3169,12 +3167,11 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	intel_ring_default_vfuncs(dev_priv, engine);
 
 	engine->flush = gen6_ring_flush;
-	if (INTEL_GEN(dev_priv) >= 8) {
+	if (INTEL_GEN(dev_priv) >= 8)
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
-	} else {
+	else
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
-	}
 
 	return intel_init_ring_buffer(dev, engine);
 }
-- 
1.9.1

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

Re: [PATCH 09/13] drm/i915: Move semaphore object creation into intel_ring_init_semaphores

[Chris Wilson]

On Wed, Jun 29, 2016 at 04:09:28PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> The object needs to be created before semaphores can be initialized
> on any ring and it makes sense to pull it out to this semaphore
> dedicated helper.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 45 ++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index d0401bb800a6..77d663fcdff1 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2876,6 +2876,30 @@ static int gen6_ring_flush(struct drm_i915_gem_request *req,
>  static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
>  				       struct intel_engine_cs *engine)
>  {
> +	struct drm_i915_gem_object *obj;
> +	int ret;
> +
> +	if (!i915_semaphore_is_enabled(dev_priv))
> +		return;
> +
> +	if (INTEL_GEN(dev_priv) >= 8 && !dev_priv->semaphore_obj) {

struct drm_i915_gem_object *obj;

Probably best to scope this object locally since we don't carry it
forward into the next loop.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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 10/13] drm/i915: Compact Gen8 semaphore initialization

[Chris Wilson]

On Wed, Jun 29, 2016 at 04:09:29PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Replace the macro initializer with a programatic loop which
> results in smaller code and hopefully just as clear.
> 
> v2: Rebase.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

We could just trivially compute these in the semaphore vfuncs!
-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 11/13] drm/i915: Compact gen8_ring_sync

[Chris Wilson]

On Wed, Jun 29, 2016 at 04:09:30PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Store the semaphore offset in a temporary variable to avoid
> having to get the VMA offset twice.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

The lookup will be gone very soon, and interesting, the offset can only
be 32bits (since we know this code will not run on any future gen that
may have more than 4GiB GGTT).
-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 12/13] drm/i915: Consolidate legacy semaphore initialization

[Chris Wilson]

On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Replace per-engine initialization with a common half-programatic,
> half-data driven code for ease of maintenance and compactness.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This is the biggest pill to swallow (since our 5x5 table is only
sparsely populated), but it looks correct, and more importantly easier to
read.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-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 13/13] drm/i915: Trim some if-else braces

[Chris Wilson]

On Wed, Jun 29, 2016 at 04:09:32PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Just a bit of cleanup after the previous refactoring.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

And for any earlier patches I may have skipped.
-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 12/13] drm/i915: Consolidate legacy semaphore initialization

[Tvrtko Ursulin]

On 29/06/16 16:34, Chris Wilson wrote:
> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Replace per-engine initialization with a common half-programatic,
>> half-data driven code for ease of maintenance and compactness.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This is the biggest pill to swallow (since our 5x5 table is only
> sparsely populated), but it looks correct, and more importantly easier to
> read.

Yeah I was out of ideas on how to improve it. Fresh mind needed to try 
and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map to bits 
and registers respectively, and write it as a function.

> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks!

Regards,

Tvrtko

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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Chris Wilson]

On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
> 
> On 29/06/16 16:34, Chris Wilson wrote:
> >On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>Replace per-engine initialization with a common half-programatic,
> >>half-data driven code for ease of maintenance and compactness.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >
> >This is the biggest pill to swallow (since our 5x5 table is only
> >sparsely populated), but it looks correct, and more importantly easier to
> >read.
> 
> Yeah I was out of ideas on how to improve it. Fresh mind needed to
> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
> to bits and registers respectively, and write it as a function.

It's actually a very simple cyclic function based on register
offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.

(The only real challenge is picking the direction.)

commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
Author: Ben Widawsky <ben@bwidawsk.net>
Date:   Wed Sep 14 20:32:47 2011 -0700

    drm/i915: Dumb down the semaphore logic
    
    While I think the previous code is correct, it was hard to follow and
    hard to debug. Since we already have a ring abstraction, might as well
    use it to handle the semaphore updates and compares.

-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

✗ Ro.CI.BAT: failure for series starting with [01/13] drm/i915: Consolidate write_tail vfunc initializer

[Patchwork]

== Series Details ==

Series: series starting with [01/13] drm/i915: Consolidate write_tail vfunc initializer
URL   : https://patchwork.freedesktop.org/series/9279/
State : failure

== Summary ==

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

Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (ro-bdw-i7-5600u)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                pass       -> FAIL       (ro-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-bdw-i5-5250u)

fi-hsw-i7-4770k  total:229  pass:194  dwarn:0   dfail:0   fail:2   skip:33 
fi-kbl-qkkr      total:229  pass:160  dwarn:29  dfail:0   fail:0   skip:40 
fi-skl-i5-6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2   skip:25 
fi-skl-i7-6700k  total:103  pass:79   dwarn:0   dfail:0   fail:0   skip:23 
fi-snb-i7-2600   total:229  pass:174  dwarn:0   dfail:0   fail:2   skip:53 
ro-bdw-i5-5250u  total:229  pass:201  dwarn:1   dfail:1   fail:3   skip:23 
ro-bdw-i7-5557U  total:229  pass:202  dwarn:1   dfail:1   fail:2   skip:23 
ro-bdw-i7-5600u  total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  
ro-byt-n2820     total:229  pass:178  dwarn:0   dfail:1   fail:5   skip:45 
ro-hsw-i3-4010u  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-hsw-i7-4770r  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-ilk-i7-620lm  total:229  pass:155  dwarn:0   dfail:1   fail:3   skip:70 
ro-ilk1-i5-650   total:224  pass:155  dwarn:0   dfail:1   fail:3   skip:65 
ro-ivb-i7-3770   total:229  pass:186  dwarn:0   dfail:1   fail:2   skip:40 
ro-ivb2-i7-3770  total:229  pass:190  dwarn:0   dfail:1   fail:2   skip:36 
ro-skl3-i5-6260u total:229  pass:206  dwarn:1   dfail:1   fail:2   skip:19 
ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 
ro-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1332/

63f6b6c drm-intel-nightly: 2016y-06m-29d-14h-53m-39s UTC integration manifest
571fe9c drm/i915: Trim some if-else braces
eda8e1b drm/i915: Consolidate legacy semaphore initialization
27fd9b9 drm/i915: Compact gen8_ring_sync
8382a5e drm/i915: Compact Gen8 semaphore initialization
e6df407 drm/i915: Move semaphore object creation into intel_ring_init_semaphores
93df019 drm/i915: Consolidate semaphore vfuncs init
57af58e drm/i915: Consolidate dispatch_execbuffer vfunc
4d2069f drm/i915: Consolidate init_hw vfunc
638e9cf drm/i915: Consolidate get/set_seqno
96f9f94 drm/i915: Consolidate get and put irq vfuncs
403479a drm/i915: Consolidate seqno_barrier vfunc
f791cd5 drm/i915: Consolidate add_request vfunc
9056f6d drm/i915: Consolidate write_tail vfunc initializer

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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Tvrtko Ursulin]

On 29/06/16 17:00, Chris Wilson wrote:
> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>
>> On 29/06/16 16:34, Chris Wilson wrote:
>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Replace per-engine initialization with a common half-programatic,
>>>> half-data driven code for ease of maintenance and compactness.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> This is the biggest pill to swallow (since our 5x5 table is only
>>> sparsely populated), but it looks correct, and more importantly easier to
>>> read.
>>
>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>> to bits and registers respectively, and write it as a function.
>
> It's actually a very simple cyclic function based on register
> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>
> (The only real challenge is picking the direction.)
>
> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Wed Sep 14 20:32:47 2011 -0700
>
>      drm/i915: Dumb down the semaphore logic
>
>      While I think the previous code is correct, it was hard to follow and
>      hard to debug. Since we already have a ring abstraction, might as well
>      use it to handle the semaphore updates and compares.

Should I try to go back to that then? Since I am not too happy with the 
sparse table...

This has passed CI so we could merge some of it if that would help your 
series, or wait until I rework this patch.

Regards,

Tvrtko

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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Chris Wilson]

On Wed, Jun 29, 2016 at 05:14:11PM +0100, Tvrtko Ursulin wrote:
> 
> On 29/06/16 17:00, Chris Wilson wrote:
> >On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 29/06/16 16:34, Chris Wilson wrote:
> >>>On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
> >>>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>>Replace per-engine initialization with a common half-programatic,
> >>>>half-data driven code for ease of maintenance and compactness.
> >>>>
> >>>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>
> >>>This is the biggest pill to swallow (since our 5x5 table is only
> >>>sparsely populated), but it looks correct, and more importantly easier to
> >>>read.
> >>
> >>Yeah I was out of ideas on how to improve it. Fresh mind needed to
> >>try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
> >>to bits and registers respectively, and write it as a function.
> >
> >It's actually a very simple cyclic function based on register
> >offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
> >
> >(The only real challenge is picking the direction.)
> >
> >commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
> >Author: Ben Widawsky <ben@bwidawsk.net>
> >Date:   Wed Sep 14 20:32:47 2011 -0700
> >
> >     drm/i915: Dumb down the semaphore logic
> >
> >     While I think the previous code is correct, it was hard to follow and
> >     hard to debug. Since we already have a ring abstraction, might as well
> >     use it to handle the semaphore updates and compares.
> 
> Should I try to go back to that then? Since I am not too happy with
> the sparse table...
> 
> This has passed CI so we could merge some of it if that would help
> your series, or wait until I rework this patch.

The rule of thumb is incremental improvements tell a better story and
should be easier to find a misstep. (My personal experience says the
longer I play with a patch the larger it gets...)

In short, you've already consolidated a lot of duplication in the vfuncs
that will make my life easier (after some rebasing joy). Anything more
is just icing on the cake. :)
-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 12/13] drm/i915: Consolidate legacy semaphore initialization

[Tvrtko Ursulin]

On 29/06/16 17:24, Chris Wilson wrote:
> On Wed, Jun 29, 2016 at 05:14:11PM +0100, Tvrtko Ursulin wrote:
>>
>> On 29/06/16 17:00, Chris Wilson wrote:
>>> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>>>
>>>> On 29/06/16 16:34, Chris Wilson wrote:
>>>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> Replace per-engine initialization with a common half-programatic,
>>>>>> half-data driven code for ease of maintenance and compactness.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> This is the biggest pill to swallow (since our 5x5 table is only
>>>>> sparsely populated), but it looks correct, and more importantly easier to
>>>>> read.
>>>>
>>>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>>>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>>>> to bits and registers respectively, and write it as a function.
>>>
>>> It's actually a very simple cyclic function based on register
>>> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>>>
>>> (The only real challenge is picking the direction.)
>>>
>>> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
>>> Author: Ben Widawsky <ben@bwidawsk.net>
>>> Date:   Wed Sep 14 20:32:47 2011 -0700
>>>
>>>      drm/i915: Dumb down the semaphore logic
>>>
>>>      While I think the previous code is correct, it was hard to follow and
>>>      hard to debug. Since we already have a ring abstraction, might as well
>>>      use it to handle the semaphore updates and compares.
>>
>> Should I try to go back to that then? Since I am not too happy with
>> the sparse table...
>>
>> This has passed CI so we could merge some of it if that would help
>> your series, or wait until I rework this patch.
>
> The rule of thumb is incremental improvements tell a better story and
> should be easier to find a misstep. (My personal experience says the
> longer I play with a patch the larger it gets...)
>
> In short, you've already consolidated a lot of duplication in the vfuncs
> that will make my life easier (after some rebasing joy). Anything more
> is just icing on the cake. :)

It also looks like I have broke something, wonder how CI did not catch 
it or I am imagining things. "drm/i915: Consolidate dispatch_execbuffer 
vfunc" looks wrong wrt add_request and dispatch_execbuffer for gen8+. 
Leaving it for tomorrow.

Regards,

Tvrtko


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

[PATCH v3] drm/i915: Consolidate dispatch_execbuffer vfunc

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

v2: Put dispatch_execbuffer before add_request. (Chris Wilson)
v3: Fix add_request and irq_seqno_barrier for gen8+.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index dcacf17c525f..15f8ded6325d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2881,10 +2881,16 @@ static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
 	engine->get_seqno = ring_get_seqno;
 	engine->set_seqno = ring_set_seqno;
 
-	if (INTEL_GEN(dev_priv) >= 6) {
+	if (INTEL_GEN(dev_priv) >= 8) {
+		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
+		engine->add_request = gen6_add_request;
+		engine->irq_seqno_barrier = gen6_seqno_barrier;
+	} else if (INTEL_GEN(dev_priv) >= 6) {
+		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		engine->add_request = gen6_add_request;
 		engine->irq_seqno_barrier = gen6_seqno_barrier;
 	} else {
+		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
 		engine->add_request = i9xx_add_request;
 	}
 
@@ -2993,15 +2999,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 
 	if (IS_HASWELL(dev_priv))
 		engine->dispatch_execbuffer = hsw_ring_dispatch_execbuffer;
-	else if (IS_GEN8(dev_priv))
-		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
-	else if (INTEL_GEN(dev_priv) >= 6)
-		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
-	else if (INTEL_GEN(dev_priv) >= 4)
-		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
 	else if (IS_I830(dev_priv) || IS_845G(dev_priv))
 		engine->dispatch_execbuffer = i830_dispatch_execbuffer;
-	else
+	else if (INTEL_GEN(dev_priv) <= 3)
 		engine->dispatch_execbuffer = i915_dispatch_execbuffer;
 	engine->init_hw = init_render_ring;
 	engine->cleanup = render_ring_cleanup;
@@ -3059,8 +3059,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		if (INTEL_GEN(dev_priv) >= 8) {
 			engine->irq_enable_mask =
 				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
-			engine->dispatch_execbuffer =
-				gen8_ring_dispatch_execbuffer;
 			if (i915_semaphore_is_enabled(dev_priv)) {
 				engine->semaphore.sync_to = gen8_ring_sync;
 				engine->semaphore.signal = gen8_xcs_signal;
@@ -3068,8 +3066,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 			}
 		} else {
 			engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
-			engine->dispatch_execbuffer =
-				gen6_ring_dispatch_execbuffer;
 			if (i915_semaphore_is_enabled(dev_priv)) {
 				engine->semaphore.sync_to = gen6_ring_sync;
 				engine->semaphore.signal = gen6_signal;
@@ -3093,7 +3089,6 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		} else {
 			engine->irq_enable_mask = I915_BSD_USER_INTERRUPT;
 		}
-		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
 	}
 
 	return intel_init_ring_buffer(dev, engine);
@@ -3118,8 +3113,6 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 	engine->flush = gen6_bsd_ring_flush;
 	engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
-	engine->dispatch_execbuffer =
-			gen8_ring_dispatch_execbuffer;
 	if (i915_semaphore_is_enabled(dev_priv)) {
 		engine->semaphore.sync_to = gen8_ring_sync;
 		engine->semaphore.signal = gen8_xcs_signal;
@@ -3146,7 +3139,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
-		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen8_ring_sync;
 			engine->semaphore.signal = gen8_xcs_signal;
@@ -3154,7 +3146,6 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 		}
 	} else {
 		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
-		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.signal = gen6_signal;
 			engine->semaphore.sync_to = gen6_ring_sync;
@@ -3199,7 +3190,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	if (INTEL_GEN(dev_priv) >= 8) {
 		engine->irq_enable_mask =
 			GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT;
-		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen8_ring_sync;
 			engine->semaphore.signal = gen8_xcs_signal;
@@ -3209,7 +3199,6 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 		engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
 		engine->irq_get = hsw_vebox_get_irq;
 		engine->irq_put = hsw_vebox_put_irq;
-		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		if (i915_semaphore_is_enabled(dev_priv)) {
 			engine->semaphore.sync_to = gen6_ring_sync;
 			engine->semaphore.signal = gen6_signal;
-- 
1.9.1

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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Chris Wilson]

On Wed, Jun 29, 2016 at 05:34:27PM +0100, Tvrtko Ursulin wrote:
> It also looks like I have broke something, wonder how CI did not
> catch it or I am imagining things. "drm/i915: Consolidate
> dispatch_execbuffer vfunc" looks wrong wrt add_request and
> dispatch_execbuffer for gen8+. Leaving it for tomorrow.

Ah, should also include i915.enable_execlists=0 when sending to trybot.
-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

✗ Ro.CI.BAT: warning for series starting with [01/13] drm/i915: Consolidate write_tail vfunc initializer (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [01/13] drm/i915: Consolidate write_tail vfunc initializer (rev2)
URL   : https://patchwork.freedesktop.org/series/9279/
State : warning

== Summary ==

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

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (ro-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ro-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-skl-i5-6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2   skip:25 
fi-snb-i7-2600   total:229  pass:174  dwarn:0   dfail:0   fail:2   skip:53 
ro-bdw-i5-5250u  total:229  pass:202  dwarn:1   dfail:1   fail:2   skip:23 
ro-bdw-i7-5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38 
ro-bsw-n3050     total:229  pass:176  dwarn:1   dfail:1   fail:2   skip:49 
ro-byt-n2820     total:229  pass:178  dwarn:1   dfail:1   fail:4   skip:45 
ro-hsw-i3-4010u  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-hsw-i7-4770r  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-ilk-i7-620lm  total:229  pass:155  dwarn:0   dfail:1   fail:3   skip:70 
ro-ilk1-i5-650   total:224  pass:155  dwarn:0   dfail:1   fail:3   skip:65 
ro-ivb-i7-3770   total:229  pass:186  dwarn:0   dfail:1   fail:2   skip:40 
ro-ivb2-i7-3770  total:229  pass:190  dwarn:0   dfail:1   fail:2   skip:36 
ro-skl3-i5-6260u total:229  pass:206  dwarn:1   dfail:1   fail:2   skip:19 
ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 
fi-kbl-qkkr failed to connect after reboot
fi-skl-i7-6700k failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1335/

8a6521c drm-intel-nightly: 2016y-06m-29d-16h-08m-16s UTC integration manifest
5a0b3b6 drm/i915: Trim some if-else braces
6546565 drm/i915: Consolidate legacy semaphore initialization
7cd391c drm/i915: Compact gen8_ring_sync
c36607f drm/i915: Compact Gen8 semaphore initialization
e862990 drm/i915: Move semaphore object creation into intel_ring_init_semaphores
2168bca drm/i915: Consolidate semaphore vfuncs init
af5b4cc drm/i915: Consolidate dispatch_execbuffer vfunc
0c72cfa drm/i915: Consolidate init_hw vfunc
2cb2fab drm/i915: Consolidate get/set_seqno
6e3bbdc drm/i915: Consolidate get and put irq vfuncs
24b7851 drm/i915: Consolidate seqno_barrier vfunc
0bd03a1 drm/i915: Consolidate add_request vfunc
42e13b2 drm/i915: Consolidate write_tail vfunc initializer

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

Re: ✗ Ro.CI.BAT: warning for series starting with [01/13] drm/i915: Consolidate write_tail vfunc initializer (rev2)

[Tvrtko Ursulin]

On 30/06/16 06:20, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [01/13] drm/i915: Consolidate write_tail vfunc initializer (rev2)
> URL   : https://patchwork.freedesktop.org/series/9279/
> State : warning
>
> == Summary ==
>
> Series 9279v2 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/9279/revisions/2/mbox
>
> Test gem_exec_flush:
>          Subgroup basic-batch-kernel-default-cmd:
>                  fail       -> PASS       (ro-byt-n2820)
> Test kms_flip:
>          Subgroup basic-flip-vs-dpms:
>                  pass       -> DMESG-WARN (ro-byt-n2820)

Looks like https://bugs.freedesktop.org/show_bug.cgi?id=95125

> Test kms_pipe_crc_basic:
>          Subgroup suspend-read-crc-pipe-b:
>                  dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
>          Subgroup suspend-read-crc-pipe-c:
>                  dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
>
> fi-skl-i5-6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2   skip:25
> fi-snb-i7-2600   total:229  pass:174  dwarn:0   dfail:0   fail:2   skip:53
> ro-bdw-i5-5250u  total:229  pass:202  dwarn:1   dfail:1   fail:2   skip:23
> ro-bdw-i7-5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38
> ro-bsw-n3050     total:229  pass:176  dwarn:1   dfail:1   fail:2   skip:49
> ro-byt-n2820     total:229  pass:178  dwarn:1   dfail:1   fail:4   skip:45
> ro-hsw-i3-4010u  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31
> ro-hsw-i7-4770r  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31
> ro-ilk-i7-620lm  total:229  pass:155  dwarn:0   dfail:1   fail:3   skip:70
> ro-ilk1-i5-650   total:224  pass:155  dwarn:0   dfail:1   fail:3   skip:65
> ro-ivb-i7-3770   total:229  pass:186  dwarn:0   dfail:1   fail:2   skip:40
> ro-ivb2-i7-3770  total:229  pass:190  dwarn:0   dfail:1   fail:2   skip:36
> ro-skl3-i5-6260u total:229  pass:206  dwarn:1   dfail:1   fail:2   skip:19
> ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48
> fi-kbl-qkkr failed to connect after reboot
> fi-skl-i7-6700k failed to connect after reboot
> ro-bdw-i7-5557U failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1335/
>
> 8a6521c drm-intel-nightly: 2016y-06m-29d-16h-08m-16s UTC integration manifest
> 5a0b3b6 drm/i915: Trim some if-else braces
> 6546565 drm/i915: Consolidate legacy semaphore initialization
> 7cd391c drm/i915: Compact gen8_ring_sync
> c36607f drm/i915: Compact Gen8 semaphore initialization
> e862990 drm/i915: Move semaphore object creation into intel_ring_init_semaphores
> 2168bca drm/i915: Consolidate semaphore vfuncs init
> af5b4cc drm/i915: Consolidate dispatch_execbuffer vfunc
> 0c72cfa drm/i915: Consolidate init_hw vfunc
> 2cb2fab drm/i915: Consolidate get/set_seqno
> 6e3bbdc drm/i915: Consolidate get and put irq vfuncs
> 24b7851 drm/i915: Consolidate seqno_barrier vfunc
> 0bd03a1 drm/i915: Consolidate add_request vfunc
> 42e13b2 drm/i915: Consolidate write_tail vfunc initializer

Looks good for merge. Just one r-b missing on v3 7/13.

Regards,

Tvrtko


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

Re: [PATCH v3] drm/i915: Consolidate dispatch_execbuffer vfunc

[Chris Wilson]

On Wed, Jun 29, 2016 at 05:40:26PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> v2: Put dispatch_execbuffer before add_request. (Chris Wilson)
> v3: Fix add_request and irq_seqno_barrier for gen8+.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

The functions match up, so

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I would have put i915_dispatch_execbuffer and i8xx_dispatch_execbuffer
into the same tree as they follow the standard form.
-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 12/13] drm/i915: Consolidate legacy semaphore initialization

[Tvrtko Ursulin]

On 29/06/16 17:00, Chris Wilson wrote:
> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>
>> On 29/06/16 16:34, Chris Wilson wrote:
>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Replace per-engine initialization with a common half-programatic,
>>>> half-data driven code for ease of maintenance and compactness.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> This is the biggest pill to swallow (since our 5x5 table is only
>>> sparsely populated), but it looks correct, and more importantly easier to
>>> read.
>>
>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>> to bits and registers respectively, and write it as a function.
>
> It's actually a very simple cyclic function based on register
> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>
> (The only real challenge is picking the direction.)
>
> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Wed Sep 14 20:32:47 2011 -0700
>
>      drm/i915: Dumb down the semaphore logic
>
>      While I think the previous code is correct, it was hard to follow and
>      hard to debug. Since we already have a ring abstraction, might as well
>      use it to handle the semaphore updates and compares.

Doesn't seem to fit, or I just can't figure it out. Needs two functions 
to get rid of the table:

f1(0, 1) = 2
f1(0, 2) = 0
f1(0, 3) = 2
f1(1, 0) = 0
f1(1, 2) = 2
f1(1, 3) = 1
f1(2, 0) = 2
f1(2, 1) = 0
f1(2, 3) = 0
f1(3, 0) = 1
f1(3, 1) = 1
f1(3, 2) = 1

and:

f2(0, 1) = 1
f2(0, 2) = 0
f2(0, 3) = 1
f2(1, 0) = 0
f2(1, 2) = 1
f2(1, 3) = 2
f2(2, 0) = 1
f2(2, 1) = 0
f2(2, 3) = 0
f2(3, 0) = 2
f2(3, 1) = 2
f2(3, 2) = 2

A weekend math puzzle for someone? :)

Regards,

Tvrtko

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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Dave Gordon]

On 15/07/16 14:13, Tvrtko Ursulin wrote:
>
> On 29/06/16 17:00, Chris Wilson wrote:
>> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>>
>>> On 29/06/16 16:34, Chris Wilson wrote:
>>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> Replace per-engine initialization with a common half-programatic,
>>>>> half-data driven code for ease of maintenance and compactness.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> This is the biggest pill to swallow (since our 5x5 table is only
>>>> sparsely populated), but it looks correct, and more importantly
>>>> easier to
>>>> read.
>>>
>>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>>> to bits and registers respectively, and write it as a function.
>>
>> It's actually a very simple cyclic function based on register
>> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>>
>> (The only real challenge is picking the direction.)
>>
>> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
>> Author: Ben Widawsky <ben@bwidawsk.net>
>> Date:   Wed Sep 14 20:32:47 2011 -0700
>>
>>      drm/i915: Dumb down the semaphore logic
>>
>>      While I think the previous code is correct, it was hard to follow
>> and
>>      hard to debug. Since we already have a ring abstraction, might as
>> well
>>      use it to handle the semaphore updates and compares.
>
> Doesn't seem to fit, or I just can't figure it out. Needs two functions
> to get rid of the table:
>
> f1(0, 1) = 2
> f1(0, 2) = 0
> f1(0, 3) = 2
> f1(1, 0) = 0
> f1(1, 2) = 2
> f1(1, 3) = 1
> f1(2, 0) = 2
> f1(2, 1) = 0
> f1(2, 3) = 0
> f1(3, 0) = 1
> f1(3, 1) = 1
> f1(3, 2) = 1
>
> and:
>
> f2(0, 1) = 1
> f2(0, 2) = 0
> f2(0, 3) = 1
> f2(1, 0) = 0
> f2(1, 2) = 1
> f2(1, 3) = 2
> f2(2, 0) = 1
> f2(2, 1) = 0
> f2(2, 3) = 0
> f2(3, 0) = 2
> f2(3, 1) = 2
> f2(3, 2) = 2
>
> A weekend math puzzle for someone? :)
>
> Regards,
> Tvrtko

Here's the APL expression for (the transpose of) f2, with -1's filled in 
along the leading diagonal (you need ⎕io←0 so the ⍳-vectors are in origin 0)

       {¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4

┌────────┬────────┬────────┬────────┐
│¯1 0 1 2│1 ¯1 0 2│0 1 ¯1 2│1 2 0 ¯1│
└────────┴────────┴────────┴────────┘

or transposed back so that the first argument is the row index and the 
second is the column index:

       ⍉↑{¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4

¯1  1  0  1
  0 ¯1  1  2
  1  0 ¯1  0
  2  2  2 ¯1

http://tryapl.org/?a=%u2349%u2191%7B%AF1+%28%u2375%u2260%u23734%29%u2340%282%7C%u2375%29%u233D%28%u233D%u2363%281%3D%u2375%29%291+%u23733%7D%A8%u23734&run

f1 is trivially derived from this by the observation that f1 is just f2 
with the 1's and 2's interchanged.

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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Tvrtko Ursulin]

On 19/07/16 19:38, Dave Gordon wrote:
> On 15/07/16 14:13, Tvrtko Ursulin wrote:
>>
>> On 29/06/16 17:00, Chris Wilson wrote:
>>> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>>>
>>>> On 29/06/16 16:34, Chris Wilson wrote:
>>>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> Replace per-engine initialization with a common half-programatic,
>>>>>> half-data driven code for ease of maintenance and compactness.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> This is the biggest pill to swallow (since our 5x5 table is only
>>>>> sparsely populated), but it looks correct, and more importantly
>>>>> easier to
>>>>> read.
>>>>
>>>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>>>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>>>> to bits and registers respectively, and write it as a function.
>>>
>>> It's actually a very simple cyclic function based on register
>>> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>>>
>>> (The only real challenge is picking the direction.)
>>>
>>> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
>>> Author: Ben Widawsky <ben@bwidawsk.net>
>>> Date:   Wed Sep 14 20:32:47 2011 -0700
>>>
>>>      drm/i915: Dumb down the semaphore logic
>>>
>>>      While I think the previous code is correct, it was hard to follow
>>> and
>>>      hard to debug. Since we already have a ring abstraction, might as
>>> well
>>>      use it to handle the semaphore updates and compares.
>>
>> Doesn't seem to fit, or I just can't figure it out. Needs two functions
>> to get rid of the table:
>>
>> f1(0, 1) = 2
>> f1(0, 2) = 0
>> f1(0, 3) = 2
>> f1(1, 0) = 0
>> f1(1, 2) = 2
>> f1(1, 3) = 1
>> f1(2, 0) = 2
>> f1(2, 1) = 0
>> f1(2, 3) = 0
>> f1(3, 0) = 1
>> f1(3, 1) = 1
>> f1(3, 2) = 1
>>
>> and:
>>
>> f2(0, 1) = 1
>> f2(0, 2) = 0
>> f2(0, 3) = 1
>> f2(1, 0) = 0
>> f2(1, 2) = 1
>> f2(1, 3) = 2
>> f2(2, 0) = 1
>> f2(2, 1) = 0
>> f2(2, 3) = 0
>> f2(3, 0) = 2
>> f2(3, 1) = 2
>> f2(3, 2) = 2
>>
>> A weekend math puzzle for someone? :)
>>
>> Regards,
>> Tvrtko
>
> Here's the APL expression for (the transpose of) f2, with -1's filled in
> along the leading diagonal (you need ⎕io←0 so the ⍳-vectors are in
> origin 0)
>
>        {¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>
> ┌────────┬────────┬────────┬────────┐
> │¯1 0 1 2│1 ¯1 0 2│0 1 ¯1 2│1 2 0 ¯1│
> └────────┴────────┴────────┴────────┘
>
> or transposed back so that the first argument is the row index and the
> second is the column index:
>
>        ⍉↑{¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>
> ¯1  1  0  1
>   0 ¯1  1  2
>   1  0 ¯1  0
>   2  2  2 ¯1
>
> http://tryapl.org/?a=%u2349%u2191%7B%AF1+%28%u2375%u2260%u23734%29%u2340%282%7C%u2375%29%u233D%28%u233D%u2363%281%3D%u2375%29%291+%u23733%7D%A8%u23734&run

  :-C ! How to convert that to C ? :)

> f1 is trivially derived from this by the observation that f1 is just f2
> with the 1's and 2's interchanged.

Ah yes, nicely spotted.

Regards,

Tvrtko


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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Dave Gordon]

On 20/07/16 10:54, Tvrtko Ursulin wrote:
>
> On 19/07/16 19:38, Dave Gordon wrote:
>> On 15/07/16 14:13, Tvrtko Ursulin wrote:
>>>
>>> On 29/06/16 17:00, Chris Wilson wrote:
>>>> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 29/06/16 16:34, Chris Wilson wrote:
>>>>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> Replace per-engine initialization with a common half-programatic,
>>>>>>> half-data driven code for ease of maintenance and compactness.
>>>>>>>
>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>
>>>>>> This is the biggest pill to swallow (since our 5x5 table is only
>>>>>> sparsely populated), but it looks correct, and more importantly
>>>>>> easier to
>>>>>> read.
>>>>>
>>>>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>>>>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>>>>> to bits and registers respectively, and write it as a function.
>>>>
>>>> It's actually a very simple cyclic function based on register
>>>> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>>>>
>>>> (The only real challenge is picking the direction.)
>>>>
>>>> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
>>>> Author: Ben Widawsky <ben@bwidawsk.net>
>>>> Date:   Wed Sep 14 20:32:47 2011 -0700
>>>>
>>>>      drm/i915: Dumb down the semaphore logic
>>>>
>>>>      While I think the previous code is correct, it was hard to follow
>>>> and
>>>>      hard to debug. Since we already have a ring abstraction, might as
>>>> well
>>>>      use it to handle the semaphore updates and compares.
>>>
>>> Doesn't seem to fit, or I just can't figure it out. Needs two functions
>>> to get rid of the table:
>>>
>>> f1(0, 1) = 2
>>> f1(0, 2) = 0
>>> f1(0, 3) = 2
>>> f1(1, 0) = 0
>>> f1(1, 2) = 2
>>> f1(1, 3) = 1
>>> f1(2, 0) = 2
>>> f1(2, 1) = 0
>>> f1(2, 3) = 0
>>> f1(3, 0) = 1
>>> f1(3, 1) = 1
>>> f1(3, 2) = 1
>>>
>>> and:
>>>
>>> f2(0, 1) = 1
>>> f2(0, 2) = 0
>>> f2(0, 3) = 1
>>> f2(1, 0) = 0
>>> f2(1, 2) = 1
>>> f2(1, 3) = 2
>>> f2(2, 0) = 1
>>> f2(2, 1) = 0
>>> f2(2, 3) = 0
>>> f2(3, 0) = 2
>>> f2(3, 1) = 2
>>> f2(3, 2) = 2
>>>
>>> A weekend math puzzle for someone? :)
>>>
>>> Regards,
>>> Tvrtko
>>
>> Here's the APL expression for (the transpose of) f2, with -1's filled in
>> along the leading diagonal (you need ⎕io←0 so the ⍳-vectors are in
>> origin 0)
>>
>>        {¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>>
>> ┌────────┬────────┬────────┬────────┐
>> │¯1 0 1 2│1 ¯1 0 2│0 1 ¯1 2│1 2 0 ¯1│
>> └────────┴────────┴────────┴────────┘
>>
>> or transposed back so that the first argument is the row index and the
>> second is the column index:
>>
>>        ⍉↑{¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>>
>>  ¯1  1  0  1
>>   0 ¯1  1  2
>>   1  0 ¯1  0
>>   2  2  2 ¯1
>>
>> http://tryapl.org/?a=%u2349%u2191%7B%AF1+%28%u2375%u2260%u23734%29%u2340%282%7C%u2375%29%u233D%28%u233D%u2363%281%3D%u2375%29%291+%u23733%7D%A8%u23734&run
>>
>
>   :-C ! How to convert that to C ? :)
>
>> f1 is trivially derived from this by the observation that f1 is just f2
>> with the 1's and 2's interchanged.
>
> Ah yes, nicely spotted.
>
> Regards,
> Tvrtko

Assuming you don't care about the leading diagonal (x == y), then

  (⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))

translates into:

int f2(unsigned int x, unsigned int y)
{
     x -= x >= y;
     if (y == 1)
         x = 3 - x;
     x += y & 1;
     return x % 3;
}

y:x 0 1 2 3
0:  0 0 1 2
1:  1 1 0 2
2:  0 1 1 2
3:  1 2 0 0

Each line of C corresponds quite closely to one operation in the APL :)
Although, in APL we tend to leave the data unchanged while shuffling it 
around into new shapes, whereas the C below does the equivalent things 
by changing the data (noting that it's all modulo-3 arithmetic).

  (⍵≠⍳4)⍀  inserts the leading diagonal, corresponding to the subtraction
           of x >= y (which removes the leading diagonal).

  ⌽⍣(1=⍵)  reverses the sequence if y==1; in C, that's the 3-x

  (2|⍵)⌽   rotates the sequence by 1 if y is odd; that's the +=

and the final % ensures that the result is 0-2.

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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Tvrtko Ursulin]

On 20/07/16 13:50, Dave Gordon wrote:
> On 20/07/16 10:54, Tvrtko Ursulin wrote:
>>
>> On 19/07/16 19:38, Dave Gordon wrote:
>>> On 15/07/16 14:13, Tvrtko Ursulin wrote:
>>>>
>>>> On 29/06/16 17:00, Chris Wilson wrote:
>>>>> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> On 29/06/16 16:34, Chris Wilson wrote:
>>>>>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>>
>>>>>>>> Replace per-engine initialization with a common half-programatic,
>>>>>>>> half-data driven code for ease of maintenance and compactness.
>>>>>>>>
>>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> This is the biggest pill to swallow (since our 5x5 table is only
>>>>>>> sparsely populated), but it looks correct, and more importantly
>>>>>>> easier to
>>>>>>> read.
>>>>>>
>>>>>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>>>>>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>>>>>> to bits and registers respectively, and write it as a function.
>>>>>
>>>>> It's actually a very simple cyclic function based on register
>>>>> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>>>>>
>>>>> (The only real challenge is picking the direction.)
>>>>>
>>>>> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
>>>>> Author: Ben Widawsky <ben@bwidawsk.net>
>>>>> Date:   Wed Sep 14 20:32:47 2011 -0700
>>>>>
>>>>>      drm/i915: Dumb down the semaphore logic
>>>>>
>>>>>      While I think the previous code is correct, it was hard to follow
>>>>> and
>>>>>      hard to debug. Since we already have a ring abstraction, might as
>>>>> well
>>>>>      use it to handle the semaphore updates and compares.
>>>>
>>>> Doesn't seem to fit, or I just can't figure it out. Needs two functions
>>>> to get rid of the table:
>>>>
>>>> f1(0, 1) = 2
>>>> f1(0, 2) = 0
>>>> f1(0, 3) = 2
>>>> f1(1, 0) = 0
>>>> f1(1, 2) = 2
>>>> f1(1, 3) = 1
>>>> f1(2, 0) = 2
>>>> f1(2, 1) = 0
>>>> f1(2, 3) = 0
>>>> f1(3, 0) = 1
>>>> f1(3, 1) = 1
>>>> f1(3, 2) = 1
>>>>
>>>> and:
>>>>
>>>> f2(0, 1) = 1
>>>> f2(0, 2) = 0
>>>> f2(0, 3) = 1
>>>> f2(1, 0) = 0
>>>> f2(1, 2) = 1
>>>> f2(1, 3) = 2
>>>> f2(2, 0) = 1
>>>> f2(2, 1) = 0
>>>> f2(2, 3) = 0
>>>> f2(3, 0) = 2
>>>> f2(3, 1) = 2
>>>> f2(3, 2) = 2
>>>>
>>>> A weekend math puzzle for someone? :)
>>>>
>>>> Regards,
>>>> Tvrtko
>>>
>>> Here's the APL expression for (the transpose of) f2, with -1's filled in
>>> along the leading diagonal (you need ⎕io←0 so the ⍳-vectors are in
>>> origin 0)
>>>
>>>        {¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>>>
>>> ┌────────┬────────┬────────┬────────┐
>>> │¯1 0 1 2│1 ¯1 0 2│0 1 ¯1 2│1 2 0 ¯1│
>>> └────────┴────────┴────────┴────────┘
>>>
>>> or transposed back so that the first argument is the row index and the
>>> second is the column index:
>>>
>>>        ⍉↑{¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>>>
>>>  ¯1  1  0  1
>>>   0 ¯1  1  2
>>>   1  0 ¯1  0
>>>   2  2  2 ¯1
>>>
>>> http://tryapl.org/?a=%u2349%u2191%7B%AF1+%28%u2375%u2260%u23734%29%u2340%282%7C%u2375%29%u233D%28%u233D%u2363%281%3D%u2375%29%291+%u23733%7D%A8%u23734&run
>>>
>>>
>>
>>   :-C ! How to convert that to C ? :)
>>
>>> f1 is trivially derived from this by the observation that f1 is just f2
>>> with the 1's and 2's interchanged.
>>
>> Ah yes, nicely spotted.
>>
>> Regards,
>> Tvrtko
>
> Assuming you don't care about the leading diagonal (x == y), then
>
>   (⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))
>
> translates into:
>
> int f2(unsigned int x, unsigned int y)
> {
>      x -= x >= y;
>      if (y == 1)
>          x = 3 - x;
>      x += y & 1;
>      return x % 3;
> }
>
> y:x 0 1 2 3
> 0:  0 0 1 2
> 1:  1 1 0 2
> 2:  0 1 1 2
> 3:  1 2 0 0
>
> Each line of C corresponds quite closely to one operation in the APL :)
> Although, in APL we tend to leave the data unchanged while shuffling it
> around into new shapes, whereas the C below does the equivalent things
> by changing the data (noting that it's all modulo-3 arithmetic).
>
>   (⍵≠⍳4)⍀  inserts the leading diagonal, corresponding to the subtraction
>            of x >= y (which removes the leading diagonal).
>
>   ⌽⍣(1=⍵)  reverses the sequence if y==1; in C, that's the 3-x
>
>   (2|⍵)⌽   rotates the sequence by 1 if y is odd; that's the +=
>
> and the final % ensures that the result is 0-2.

I was hoping for a solution which does not include conditionals, someone 
led me to believe it is possible! :)

But thanks, your transformation really works. I've sent a patch 
implementing it to trybot for now.

Regards,

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

Re: [PATCH 12/13] drm/i915: Consolidate legacy semaphore initialization

[Dave Gordon]

On 20/07/16 17:07, Tvrtko Ursulin wrote:
>
> On 20/07/16 13:50, Dave Gordon wrote:
>> On 20/07/16 10:54, Tvrtko Ursulin wrote:
>>>
>>> On 19/07/16 19:38, Dave Gordon wrote:
>>>> On 15/07/16 14:13, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 29/06/16 17:00, Chris Wilson wrote:
>>>>>> On Wed, Jun 29, 2016 at 04:41:58PM +0100, Tvrtko Ursulin wrote:
>>>>>>>
>>>>>>> On 29/06/16 16:34, Chris Wilson wrote:
>>>>>>>> On Wed, Jun 29, 2016 at 04:09:31PM +0100, Tvrtko Ursulin wrote:
>>>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>>>
>>>>>>>>> Replace per-engine initialization with a common half-programatic,
>>>>>>>>> half-data driven code for ease of maintenance and compactness.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>>
>>>>>>>> This is the biggest pill to swallow (since our 5x5 table is only
>>>>>>>> sparsely populated), but it looks correct, and more importantly
>>>>>>>> easier to
>>>>>>>> read.
>>>>>>>
>>>>>>> Yeah I was out of ideas on how to improve it. Fresh mind needed to
>>>>>>> try and spot a pattern in how MI_SEMAPHORE_SYNC_* and GEN6_*SYNC map
>>>>>>> to bits and registers respectively, and write it as a function.
>>>>>>
>>>>>> It's actually a very simple cyclic function based on register
>>>>>> offset = base + (signaler hw_id - waiter hw_id - 1) % num_rings.
>>>>>>
>>>>>> (The only real challenge is picking the direction.)
>>>>>>
>>>>>> commit c8c99b0f0dea1ced5d0e10cdb9143356cc16b484
>>>>>> Author: Ben Widawsky <ben@bwidawsk.net>
>>>>>> Date:   Wed Sep 14 20:32:47 2011 -0700
>>>>>>
>>>>>>      drm/i915: Dumb down the semaphore logic
>>>>>>
>>>>>>      While I think the previous code is correct, it was hard to
>>>>>> follow
>>>>>> and
>>>>>>      hard to debug. Since we already have a ring abstraction,
>>>>>> might as
>>>>>> well
>>>>>>      use it to handle the semaphore updates and compares.
>>>>>
>>>>> Doesn't seem to fit, or I just can't figure it out. Needs two
>>>>> functions
>>>>> to get rid of the table:
>>>>>
>>>>> f1(0, 1) = 2
>>>>> f1(0, 2) = 0
>>>>> f1(0, 3) = 2
>>>>> f1(1, 0) = 0
>>>>> f1(1, 2) = 2
>>>>> f1(1, 3) = 1
>>>>> f1(2, 0) = 2
>>>>> f1(2, 1) = 0
>>>>> f1(2, 3) = 0
>>>>> f1(3, 0) = 1
>>>>> f1(3, 1) = 1
>>>>> f1(3, 2) = 1
>>>>>
>>>>> and:
>>>>>
>>>>> f2(0, 1) = 1
>>>>> f2(0, 2) = 0
>>>>> f2(0, 3) = 1
>>>>> f2(1, 0) = 0
>>>>> f2(1, 2) = 1
>>>>> f2(1, 3) = 2
>>>>> f2(2, 0) = 1
>>>>> f2(2, 1) = 0
>>>>> f2(2, 3) = 0
>>>>> f2(3, 0) = 2
>>>>> f2(3, 1) = 2
>>>>> f2(3, 2) = 2
>>>>>
>>>>> A weekend math puzzle for someone? :)
>>>>>
>>>>> Regards,
>>>>> Tvrtko
>>>>
>>>> Here's the APL expression for (the transpose of) f2, with -1's
>>>> filled in
>>>> along the leading diagonal (you need ⎕io←0 so the ⍳-vectors are in
>>>> origin 0)
>>>>
>>>>        {¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>>>>
>>>> ┌────────┬────────┬────────┬────────┐
>>>> │¯1 0 1 2│1 ¯1 0 2│0 1 ¯1 2│1 2 0 ¯1│
>>>> └────────┴────────┴────────┴────────┘
>>>>
>>>> or transposed back so that the first argument is the row index and the
>>>> second is the column index:
>>>>
>>>>        ⍉↑{¯1+(⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))1+⍳3}¨⍳4
>>>>
>>>>  ¯1  1  0  1
>>>>   0 ¯1  1  2
>>>>   1  0 ¯1  0
>>>>   2  2  2 ¯1
>>>>
>>>> http://tryapl.org/?a=%u2349%u2191%7B%AF1+%28%u2375%u2260%u23734%29%u2340%282%7C%u2375%29%u233D%28%u233D%u2363%281%3D%u2375%29%291+%u23733%7D%A8%u23734&run
>>>>
>>>>
>>>>
>>>
>>>   :-C ! How to convert that to C ? :)
>>>
>>>> f1 is trivially derived from this by the observation that f1 is just f2
>>>> with the 1's and 2's interchanged.
>>>
>>> Ah yes, nicely spotted.
>>>
>>> Regards,
>>> Tvrtko
>>
>> Assuming you don't care about the leading diagonal (x == y), then
>>
>>   (⍵≠⍳4)⍀(2|⍵)⌽(⌽⍣(1=⍵))
>>
>> translates into:
>>
>> int f2(unsigned int x, unsigned int y)
>> {
>>      x -= x >= y;
>>      if (y == 1)
>>          x = 3 - x;
>>      x += y & 1;
>>      return x % 3;
>> }
>>
>> y:x 0 1 2 3
>> 0:  0 0 1 2
>> 1:  1 1 0 2
>> 2:  0 1 1 2
>> 3:  1 2 0 0
>>
>> Each line of C corresponds quite closely to one operation in the APL :)
>> Although, in APL we tend to leave the data unchanged while shuffling it
>> around into new shapes, whereas the C below does the equivalent things
>> by changing the data (noting that it's all modulo-3 arithmetic).
>>
>>   (⍵≠⍳4)⍀  inserts the leading diagonal, corresponding to the subtraction
>>            of x >= y (which removes the leading diagonal).
>>
>>   ⌽⍣(1=⍵)  reverses the sequence if y==1; in C, that's the 3-x
>>
>>   (2|⍵)⌽   rotates the sequence by 1 if y is odd; that's the +=
>>
>> and the final % ensures that the result is 0-2.
>
> I was hoping for a solution which does not include conditionals, someone
> led me to believe it is possible! :)
>
> But thanks, your transformation really works. I've sent a patch
> implementing it to trybot for now.
>
> Regards,
> Tvrtko

You can write it like this if you don't want any visible conditionals :)

unsigned int f2(unsigned int x, unsigned int y)
{
	x -= x >= y;
	x += y & 1;
	x ^= y & x >> y;	/* WTF?	*/
	return x % 3;
}

But I think that's even more obscure.

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