All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library
@ 2018-04-27 10:03 Katarzyna Dec
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 2/5] lib/media_spin: Remove gen8lp_media_spin function Katarzyna Dec
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Katarzyna Dec @ 2018-04-27 10:03 UTC (permalink / raw)
  To: igt-dev

Let's remove duplications introduced by moving media_spin helper
functions to gpu_fill. These were mainly the same functions
as for Gen8 media/gpgpu fill. gen8_render_flush from media_spin
was replaced by gen7_render_flush. The only functions that were
left intact are gen8_emit_media_objects_spin and
gen8_spin_render_flush.

v2: squashed patches 1 and 2 from v1

Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/gpu_fill.c   |  81 +++++++++++++
 lib/gpu_fill.h   |  13 +++
 lib/media_spin.c | 347 +++----------------------------------------------------
 3 files changed, 113 insertions(+), 328 deletions(-)

diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
index f05d4eca..f5fc61bb 100644
--- a/lib/gpu_fill.c
+++ b/lib/gpu_fill.c
@@ -351,6 +351,20 @@ gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
 	OUT_BATCH(0xffffffff);
 }
 
+uint32_t
+gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
+			    uint32_t iters)
+{
+	uint32_t *curbe_buffer;
+	uint32_t offset;
+
+	curbe_buffer = intel_batchbuffer_subdata_alloc(batch, 64, 64);
+	offset = intel_batchbuffer_subdata_offset(batch, curbe_buffer);
+	*curbe_buffer = iters;
+
+	return offset;
+}
+
 uint32_t
 gen8_fill_surface_state(struct intel_batchbuffer *batch,
 			struct igt_buf *buf,
@@ -525,6 +539,30 @@ gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
 	OUT_BATCH(0);
 }
 
+void
+gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch)
+{
+	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
+
+	/* scratch buffer */
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+
+	/* number of threads & urb entries */
+	OUT_BATCH(2 << 8);
+
+	OUT_BATCH(0);
+
+	/* urb entry size & curbe size */
+	OUT_BATCH(2 << 16 |
+		2);
+
+	/* scoreboard */
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+}
+
 void
 gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
 		     unsigned x, unsigned y,
@@ -585,6 +623,49 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
 	OUT_BATCH(0xffffffff);
 }
 
+void
+gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
+{
+	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
+
+	/* interface descriptor offset */
+	OUT_BATCH(0);
+
+	/* without indirect data */
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+
+	/* scoreboard */
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+
+	/* inline data (xoffset, yoffset) */
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+	gen8_emit_media_state_flush(batch);
+}
+
+void
+gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch)
+{
+	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
+
+	/* interface descriptor offset */
+	OUT_BATCH(0);
+
+	/* without indirect data */
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+
+	/* scoreboard */
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+
+	/* inline data (xoffset, yoffset) */
+	OUT_BATCH(0);
+	OUT_BATCH(0);
+}
+
 void
 gen9_emit_state_base_address(struct intel_batchbuffer *batch)
 {
diff --git a/lib/gpu_fill.h b/lib/gpu_fill.h
index 067d4987..5335fe3f 100644
--- a/lib/gpu_fill.h
+++ b/lib/gpu_fill.h
@@ -88,6 +88,10 @@ gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
 		     unsigned x, unsigned y,
 		     unsigned width, unsigned height);
 
+uint32_t
+gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
+			    uint32_t iters);
+
 uint32_t
 gen8_fill_surface_state(struct intel_batchbuffer *batch,
 			struct igt_buf *buf,
@@ -109,11 +113,20 @@ gen8_emit_vfe_state(struct intel_batchbuffer *batch);
 void
 gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
 
+void
+gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch);
+
 void
 gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
 		     unsigned x, unsigned y,
 		     unsigned width, unsigned height);
 
+void
+gen8_emit_media_objects_spin(struct intel_batchbuffer *batch);
+
+void
+gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch);
+
 void
 gen9_emit_state_base_address(struct intel_batchbuffer *batch);
 
diff --git a/lib/media_spin.c b/lib/media_spin.c
index d9e058b1..16ea8483 100644
--- a/lib/media_spin.c
+++ b/lib/media_spin.c
@@ -31,6 +31,7 @@
 #include "intel_batchbuffer.h"
 #include "gen8_media.h"
 #include "media_spin.h"
+#include "gpu_fill.h"
 
 static const uint32_t spin_kernel[][4] = {
 	{ 0x00600001, 0x20800208, 0x008d0000, 0x00000000 }, /* mov (8)r4.0<1>:ud r0.0<8;8;1>:ud */
@@ -45,316 +46,6 @@ static const uint32_t spin_kernel[][4] = {
 	{ 0x07800031, 0x20000a40, 0x0e000e00, 0x82000010 }, /* send.ts (16)null<1> r112<0;1;0>:d 0x82000010 */
 };
 
-static void
-gen8_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end)
-{
-	int ret;
-
-	ret = drm_intel_bo_subdata(batch->bo, 0, 4096, batch->buffer);
-	if (ret == 0)
-		ret = drm_intel_gem_bo_context_exec(batch->bo, NULL,
-						    batch_end, 0);
-	igt_assert_eq(ret, 0);
-}
-
-static uint32_t
-gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
-			    uint32_t iters)
-{
-	uint32_t *curbe_buffer;
-	uint32_t offset;
-
-	curbe_buffer = intel_batchbuffer_subdata_alloc(batch, 64, 64);
-	offset = intel_batchbuffer_subdata_offset(batch, curbe_buffer);
-	*curbe_buffer = iters;
-
-	return offset;
-}
-
-static uint32_t
-gen8_spin_surface_state(struct intel_batchbuffer *batch,
-			struct igt_buf *buf,
-			uint32_t format,
-			int is_dst)
-{
-	struct gen8_surface_state *ss;
-	uint32_t write_domain, read_domain, offset;
-	int ret;
-
-	if (is_dst) {
-		write_domain = read_domain = I915_GEM_DOMAIN_RENDER;
-	} else {
-		write_domain = 0;
-		read_domain = I915_GEM_DOMAIN_SAMPLER;
-	}
-
-	ss = intel_batchbuffer_subdata_alloc(batch, sizeof(*ss), 64);
-	offset = intel_batchbuffer_subdata_offset(batch, ss);
-
-	ss->ss0.surface_type = GEN8_SURFACE_2D;
-	ss->ss0.surface_format = format;
-	ss->ss0.render_cache_read_write = 1;
-	ss->ss0.vertical_alignment = 1; /* align 4 */
-	ss->ss0.horizontal_alignment = 1; /* align 4 */
-
-	if (buf->tiling == I915_TILING_X)
-		ss->ss0.tiled_mode = 2;
-	else if (buf->tiling == I915_TILING_Y)
-		ss->ss0.tiled_mode = 3;
-
-	ss->ss8.base_addr = buf->bo->offset;
-
-	ret = drm_intel_bo_emit_reloc(batch->bo,
-				intel_batchbuffer_subdata_offset(batch, ss) + 8 * 4,
-				buf->bo, 0,
-				read_domain, write_domain);
-	igt_assert_eq(ret, 0);
-
-	ss->ss2.height = igt_buf_height(buf) - 1;
-	ss->ss2.width  = igt_buf_width(buf) - 1;
-	ss->ss3.pitch  = buf->stride - 1;
-
-	ss->ss7.shader_chanel_select_r = 4;
-	ss->ss7.shader_chanel_select_g = 5;
-	ss->ss7.shader_chanel_select_b = 6;
-	ss->ss7.shader_chanel_select_a = 7;
-
-	return offset;
-}
-
-static uint32_t
-gen8_spin_binding_table(struct intel_batchbuffer *batch,
-			struct igt_buf *dst)
-{
-	uint32_t *binding_table, offset;
-
-	binding_table = intel_batchbuffer_subdata_alloc(batch, 32, 64);
-	offset = intel_batchbuffer_subdata_offset(batch, binding_table);
-
-	binding_table[0] = gen8_spin_surface_state(batch, dst,
-					GEN8_SURFACEFORMAT_R8_UNORM, 1);
-
-	return offset;
-}
-
-static uint32_t
-gen8_spin_media_kernel(struct intel_batchbuffer *batch,
-		       const uint32_t kernel[][4],
-		       size_t size)
-{
-	uint32_t offset;
-
-	offset = intel_batchbuffer_copy_data(batch, kernel, size, 64);
-
-	return offset;
-}
-
-static uint32_t
-gen8_spin_interface_descriptor(struct intel_batchbuffer *batch,
-			       struct igt_buf *dst)
-{
-	struct gen8_interface_descriptor_data *idd;
-	uint32_t offset;
-	uint32_t binding_table_offset, kernel_offset;
-
-	binding_table_offset = gen8_spin_binding_table(batch, dst);
-	kernel_offset = gen8_spin_media_kernel(batch, spin_kernel,
-					       sizeof(spin_kernel));
-
-	idd = intel_batchbuffer_subdata_alloc(batch, sizeof(*idd), 64);
-	offset = intel_batchbuffer_subdata_offset(batch, idd);
-
-	idd->desc0.kernel_start_pointer = (kernel_offset >> 6);
-
-	idd->desc2.single_program_flow = 1;
-	idd->desc2.floating_point_mode = GEN8_FLOATING_POINT_IEEE_754;
-
-	idd->desc3.sampler_count = 0;      /* 0 samplers used */
-	idd->desc3.sampler_state_pointer = 0;
-
-	idd->desc4.binding_table_entry_count = 0;
-	idd->desc4.binding_table_pointer = (binding_table_offset >> 5);
-
-	idd->desc5.constant_urb_entry_read_offset = 0;
-	idd->desc5.constant_urb_entry_read_length = 1; /* grf 1 */
-
-	return offset;
-}
-
-static void
-gen8_emit_state_base_address(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN8_STATE_BASE_ADDRESS | (16 - 2));
-
-	/* general */
-	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
-	OUT_BATCH(0);
-
-	/* stateless data port */
-	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
-
-	/* surface */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
-
-	/* dynamic */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
-		0, BASE_ADDRESS_MODIFY);
-
-	/* indirect */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* instruction */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
-
-	/* general state buffer size */
-	OUT_BATCH(0xfffff000 | 1);
-	/* dynamic state buffer size */
-	OUT_BATCH(1 << 12 | 1);
-	/* indirect object buffer size */
-	OUT_BATCH(0xfffff000 | 1);
-	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
-	OUT_BATCH(1 << 12 | 1);
-}
-
-static void
-gen9_emit_state_base_address(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN8_STATE_BASE_ADDRESS | (19 - 2));
-
-	/* general */
-	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
-	OUT_BATCH(0);
-
-	/* stateless data port */
-	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
-
-	/* surface */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
-
-	/* dynamic */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
-		0, BASE_ADDRESS_MODIFY);
-
-	/* indirect */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* instruction */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
-
-	/* general state buffer size */
-	OUT_BATCH(0xfffff000 | 1);
-	/* dynamic state buffer size */
-	OUT_BATCH(1 << 12 | 1);
-	/* indirect object buffer size */
-	OUT_BATCH(0xfffff000 | 1);
-	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
-	OUT_BATCH(1 << 12 | 1);
-
-	/* Bindless surface state base address */
-	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
-	OUT_BATCH(0);
-	OUT_BATCH(0xfffff000);
-}
-
-static void
-gen8_emit_vfe_state(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
-
-	/* scratch buffer */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* number of threads & urb entries */
-	OUT_BATCH(2 << 8);
-
-	OUT_BATCH(0);
-
-	/* urb entry size & curbe size */
-	OUT_BATCH(2 << 16 |
-		2);
-
-	/* scoreboard */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-}
-
-static void
-gen8_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer)
-{
-	OUT_BATCH(GEN8_MEDIA_CURBE_LOAD | (4 - 2));
-	OUT_BATCH(0);
-	/* curbe total data length */
-	OUT_BATCH(64);
-	/* curbe data start address, is relative to the dynamics base address */
-	OUT_BATCH(curbe_buffer);
-}
-
-static void
-gen8_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
-				    uint32_t interface_descriptor)
-{
-	OUT_BATCH(GEN8_MEDIA_INTERFACE_DESCRIPTOR_LOAD | (4 - 2));
-	OUT_BATCH(0);
-	/* interface descriptor data length */
-	OUT_BATCH(sizeof(struct gen8_interface_descriptor_data));
-	/* interface descriptor address, is relative to the dynamics base address */
-	OUT_BATCH(interface_descriptor);
-}
-
-static void
-gen8_emit_media_state_flush(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN8_MEDIA_STATE_FLUSH | (2 - 2));
-	OUT_BATCH(0);
-}
-
-static void
-gen8_emit_media_objects(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
-
-	/* interface descriptor offset */
-	OUT_BATCH(0);
-
-	/* without indirect data */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* scoreboard */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* inline data (xoffset, yoffset) */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-	gen8_emit_media_state_flush(batch);
-}
-
-static void
-gen8lp_emit_media_objects(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
-
-	/* interface descriptor offset */
-	OUT_BATCH(0);
-
-	/* without indirect data */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* scoreboard */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* inline data (xoffset, yoffset) */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-}
-
 /*
  * This sets up the media pipeline,
  *
@@ -390,7 +81,7 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
 	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
 
 	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
-	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
+	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
 	igt_assert(batch->ptr < &batch->buffer[4095]);
 
 	/* media pipeline */
@@ -398,20 +89,20 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
 	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
 	gen8_emit_state_base_address(batch);
 
-	gen8_emit_vfe_state(batch);
+	gen8_emit_vfe_state_spin(batch);
 
-	gen8_emit_curbe_load(batch, curbe_buffer);
+	gen7_emit_curbe_load(batch, curbe_buffer);
 
-	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
+	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
 
-	gen8_emit_media_objects(batch);
+	gen8_emit_media_objects_spin(batch);
 
 	OUT_BATCH(MI_BATCH_BUFFER_END);
 
 	batch_end = intel_batchbuffer_align(batch, 8);
 	igt_assert(batch_end < BATCH_STATE_SPLIT);
 
-	gen8_render_flush(batch, batch_end);
+	gen7_render_flush(batch, batch_end);
 	intel_batchbuffer_reset(batch);
 }
 
@@ -428,7 +119,7 @@ gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
 	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
 
 	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
-	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
+	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
 	igt_assert(batch->ptr < &batch->buffer[4095]);
 
 	/* media pipeline */
@@ -436,20 +127,20 @@ gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
 	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
 	gen8_emit_state_base_address(batch);
 
-	gen8_emit_vfe_state(batch);
+	gen8_emit_vfe_state_spin(batch);
 
-	gen8_emit_curbe_load(batch, curbe_buffer);
+	gen7_emit_curbe_load(batch, curbe_buffer);
 
-	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
+	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
 
-	gen8lp_emit_media_objects(batch);
+	gen8lp_emit_media_objects_spin(batch);
 
 	OUT_BATCH(MI_BATCH_BUFFER_END);
 
 	batch_end = intel_batchbuffer_align(batch, 8);
 	igt_assert(batch_end < BATCH_STATE_SPLIT);
 
-	gen8_render_flush(batch, batch_end);
+	gen7_render_flush(batch, batch_end);
 	intel_batchbuffer_reset(batch);
 }
 
@@ -466,7 +157,7 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
 	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
 
 	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
-	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
+	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
 	igt_assert(batch->ptr < &batch->buffer[4095]);
 
 	/* media pipeline */
@@ -479,13 +170,13 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
 			GEN9_FORCE_MEDIA_AWAKE_MASK);
 	gen9_emit_state_base_address(batch);
 
-	gen8_emit_vfe_state(batch);
+	gen8_emit_vfe_state_spin(batch);
 
-	gen8_emit_curbe_load(batch, curbe_buffer);
+	gen7_emit_curbe_load(batch, curbe_buffer);
 
-	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
+	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
 
-	gen8_emit_media_objects(batch);
+	gen8_emit_media_objects_spin(batch);
 
 	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA |
 			GEN9_FORCE_MEDIA_AWAKE_DISABLE |
@@ -499,6 +190,6 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
 	batch_end = intel_batchbuffer_align(batch, 8);
 	igt_assert(batch_end < BATCH_STATE_SPLIT);
 
-	gen8_render_flush(batch, batch_end);
+	gen7_render_flush(batch, batch_end);
 	intel_batchbuffer_reset(batch);
 }
-- 
2.14.3

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] [PATCH i-g-t v2 2/5] lib/media_spin: Remove gen8lp_media_spin function
  2018-04-27 10:03 [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library Katarzyna Dec
@ 2018-04-27 10:03 ` Katarzyna Dec
  2018-05-02 15:22   ` Daniele Ceraolo Spurio
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 3/5] lib: Adjust media_spin and gpu_fill to our code style Katarzyna Dec
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Katarzyna Dec @ 2018-04-27 10:03 UTC (permalink / raw)
  To: igt-dev

Gen8lp function are duplicates of Gen8 with difference in
emit_media_objects - we cannot perform media_state_flush on CHT.
Similar changes were done previously for gpgpu_fill and media_fill.

v2: replaced IS_BRW and IS_CHT with IS_GEN8

Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/gpu_fill.c          | 24 ++----------------------
 lib/intel_batchbuffer.c |  4 +---
 lib/media_spin.c        | 38 --------------------------------------
 lib/media_spin.h        |  3 ---
 4 files changed, 3 insertions(+), 66 deletions(-)

diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
index f5fc61bb..8dab39df 100644
--- a/lib/gpu_fill.c
+++ b/lib/gpu_fill.c
@@ -642,28 +642,8 @@ gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
 	/* inline data (xoffset, yoffset) */
 	OUT_BATCH(0);
 	OUT_BATCH(0);
-	gen8_emit_media_state_flush(batch);
-}
-
-void
-gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
-
-	/* interface descriptor offset */
-	OUT_BATCH(0);
-
-	/* without indirect data */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* scoreboard */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* inline data (xoffset, yoffset) */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
+	if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
+		gen8_emit_media_state_flush(batch);
 }
 
 void
diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index f87e6bed..cf7da44d 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -906,10 +906,8 @@ igt_media_spinfunc_t igt_get_media_spinfunc(int devid)
 
 	if (IS_GEN9(devid))
 		spin = gen9_media_spinfunc;
-	else if (IS_BROADWELL(devid))
+	else if (IS_GEN8(devid))
 		spin = gen8_media_spinfunc;
-	else if (IS_CHERRYVIEW(devid))
-		spin = gen8lp_media_spinfunc;
 
 	return spin;
 }
diff --git a/lib/media_spin.c b/lib/media_spin.c
index 16ea8483..b4414bee 100644
--- a/lib/media_spin.c
+++ b/lib/media_spin.c
@@ -106,44 +106,6 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
 	intel_batchbuffer_reset(batch);
 }
 
-void
-gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
-		      struct igt_buf *dst, uint32_t spins)
-{
-	uint32_t curbe_buffer, interface_descriptor;
-	uint32_t batch_end;
-
-	intel_batchbuffer_flush_with_context(batch, NULL);
-
-	/* setup states */
-	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
-
-	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
-	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
-	igt_assert(batch->ptr < &batch->buffer[4095]);
-
-	/* media pipeline */
-	batch->ptr = batch->buffer;
-	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
-	gen8_emit_state_base_address(batch);
-
-	gen8_emit_vfe_state_spin(batch);
-
-	gen7_emit_curbe_load(batch, curbe_buffer);
-
-	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
-
-	gen8lp_emit_media_objects_spin(batch);
-
-	OUT_BATCH(MI_BATCH_BUFFER_END);
-
-	batch_end = intel_batchbuffer_align(batch, 8);
-	igt_assert(batch_end < BATCH_STATE_SPLIT);
-
-	gen7_render_flush(batch, batch_end);
-	intel_batchbuffer_reset(batch);
-}
-
 void
 gen9_media_spinfunc(struct intel_batchbuffer *batch,
 		    struct igt_buf *dst, uint32_t spins)
diff --git a/lib/media_spin.h b/lib/media_spin.h
index 8bc48291..57d8c2e2 100644
--- a/lib/media_spin.h
+++ b/lib/media_spin.h
@@ -30,9 +30,6 @@
 void gen8_media_spinfunc(struct intel_batchbuffer *batch,
 			 struct igt_buf *dst, uint32_t spins);
 
-void gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
-			   struct igt_buf *dst, uint32_t spins);
-
 void gen9_media_spinfunc(struct intel_batchbuffer *batch,
 			 struct igt_buf *dst, uint32_t spins);
 
-- 
2.14.3

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] [PATCH i-g-t v2 3/5] lib: Adjust media_spin and gpu_fill to our code style
  2018-04-27 10:03 [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library Katarzyna Dec
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 2/5] lib/media_spin: Remove gen8lp_media_spin function Katarzyna Dec
@ 2018-04-27 10:03 ` Katarzyna Dec
  2018-05-02 15:42   ` Daniele Ceraolo Spurio
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 4/5] lib/gpu_fill: Further code unification in gpu_fill Katarzyna Dec
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Katarzyna Dec @ 2018-04-27 10:03 UTC (permalink / raw)
  To: igt-dev

Let's adjust code to our coding style during refactoring
media_spin code.

v2: fixed minor typos

Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
---
 lib/gpgpu_fill.c | 15 +++++-------
 lib/gpu_fill.c   | 72 +++++++++++++++++++++++++++++++++-----------------------
 lib/gpu_fill.h   | 26 ++++++++++----------
 lib/media_spin.c |  6 +++--
 4 files changed, 66 insertions(+), 53 deletions(-)

diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
index 010dde06..52925a5c 100644
--- a/lib/gpgpu_fill.c
+++ b/lib/gpgpu_fill.c
@@ -112,9 +112,8 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
 
 	/*
-	 * const buffer needs to fill for every thread, but as we have just 1 thread
-	 * per every group, so need only one curbe data.
-	 *
+	 * const buffer needs to fill for every thread, but as we have just 1
+	 * thread per every group, so need only one curbe data.
 	 * For each thread, just use thread group ID for buffer offset.
 	 */
 	curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
@@ -160,9 +159,8 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
 
 	/*
-	 * const buffer needs to fill for every thread, but as we have just 1 thread
-	 * per every group, so need only one curbe data.
-	 *
+	 * const buffer needs to fill for every thread, but as we have just 1
+	 * thread per every group, so need only one curbe data.
 	 * For each thread, just use thread group ID for buffer offset.
 	 */
 	curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
@@ -208,9 +206,8 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
 
 	/*
-	 * const buffer needs to fill for every thread, but as we have just 1 thread
-	 * per every group, so need only one curbe data.
-	 *
+	 * const buffer needs to fill for every thread, but as we have just 1
+	 * thread per every group, so need only one curbe data.
 	 * For each thread, just use thread group ID for buffer offset.
 	 */
 	curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
index 8dab39df..25c0c810 100644
--- a/lib/gpu_fill.c
+++ b/lib/gpu_fill.c
@@ -132,8 +132,8 @@ gen7_fill_kernel(struct intel_batchbuffer *batch,
 }
 
 uint32_t
-gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,
-			       const uint32_t kernel[][4], size_t size)
+gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf
+			       *dst, const uint32_t kernel[][4], size_t size)
 {
 	struct gen7_interface_descriptor_data *idd;
 	uint32_t offset;
@@ -171,16 +171,19 @@ gen7_emit_state_base_address(struct intel_batchbuffer *batch)
 	OUT_BATCH(0);
 
 	/* surface */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
+	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
+		  BASE_ADDRESS_MODIFY);
 
 	/* dynamic */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
+	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
+		  BASE_ADDRESS_MODIFY);
 
 	/* indirect */
 	OUT_BATCH(0);
 
 	/* instruction */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
+	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
+		  BASE_ADDRESS_MODIFY);
 
 	/* general/dynamic/indirect/instruction access Bound */
 	OUT_BATCH(0);
@@ -204,8 +207,8 @@ gen7_emit_vfe_state(struct intel_batchbuffer *batch)
 	OUT_BATCH(0);
 
 	/* urb entry size & curbe size */
-	OUT_BATCH(2 << 16 | 	/* in 256 bits unit */
-		2);		/* in 256 bits unit */
+	OUT_BATCH(2 << 16 |	/* in 256 bits unit */
+		  2);		/* in 256 bits unit */
 
 	/* scoreboard */
 	OUT_BATCH(0);
@@ -229,7 +232,7 @@ gen7_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
 	OUT_BATCH(0);
 
 	/* urb entry size & curbe size */
-	OUT_BATCH(0 << 16 | 	/* URB entry size in 256 bits unit */
+	OUT_BATCH(0 << 16 |	/* URB entry size in 256 bits unit */
 		  1);		/* CURBE entry size in 256 bits unit */
 
 	/* scoreboard */
@@ -250,7 +253,8 @@ gen7_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer)
 }
 
 void
-gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch, uint32_t interface_descriptor)
+gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
+				    uint32_t interface_descriptor)
 {
 	OUT_BATCH(GEN7_MEDIA_INTERFACE_DESCRIPTOR_LOAD | (4 - 2));
 	OUT_BATCH(0);
@@ -259,14 +263,16 @@ gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch, uint32_t in
 		OUT_BATCH(sizeof(struct gen7_interface_descriptor_data));
 	else
 		OUT_BATCH(sizeof(struct gen8_interface_descriptor_data));
-	/* interface descriptor address, is relative to the dynamics base address */
+	/* interface descriptor address, is relative to the dynamics base
+	 * address
+	 */
 	OUT_BATCH(interface_descriptor);
 }
 
 void
 gen7_emit_media_objects(struct intel_batchbuffer *batch,
-			unsigned x, unsigned y,
-			unsigned width, unsigned height)
+			unsigned int x, unsigned int y,
+			unsigned int width, unsigned int height)
 {
 	int i, j;
 
@@ -288,7 +294,8 @@ gen7_emit_media_objects(struct intel_batchbuffer *batch,
 			/* inline data (xoffset, yoffset) */
 			OUT_BATCH(x + i * 16);
 			OUT_BATCH(y + j * 16);
-			if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
+			if (AT_LEAST_GEN(batch->devid, 8) &&
+			    !IS_CHERRYVIEW(batch->devid))
 				gen8_emit_media_state_flush(batch);
 		}
 	}
@@ -296,8 +303,8 @@ gen7_emit_media_objects(struct intel_batchbuffer *batch,
 
 void
 gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
-		     unsigned x, unsigned y,
-		     unsigned width, unsigned height)
+		     unsigned int x, unsigned int y,
+		     unsigned int width, unsigned int height)
 {
 	uint32_t x_dim, y_dim, tmp, right_mask;
 
@@ -399,9 +406,8 @@ gen8_fill_surface_state(struct intel_batchbuffer *batch,
 	ss->ss8.base_addr = buf->bo->offset;
 
 	ret = drm_intel_bo_emit_reloc(batch->bo,
-				intel_batchbuffer_subdata_offset(batch, ss) + 8 * 4,
-				buf->bo, 0,
-				read_domain, write_domain);
+				intel_batchbuffer_subdata_offset(batch, ss) +
+				8 * 4, buf->bo, 0, read_domain, write_domain);
 	igt_assert(ret == 0);
 
 	ss->ss2.height = igt_buf_height(buf) - 1;
@@ -417,7 +423,9 @@ gen8_fill_surface_state(struct intel_batchbuffer *batch,
 }
 
 uint32_t
-gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,  const uint32_t kernel[][4], size_t size)
+gen8_fill_interface_descriptor(struct intel_batchbuffer *batch,
+			       struct igt_buf *dst, const uint32_t kernel[][4],
+			       size_t size)
 {
 	struct gen8_interface_descriptor_data *idd;
 	uint32_t offset;
@@ -464,15 +472,16 @@ gen8_emit_state_base_address(struct intel_batchbuffer *batch)
 	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
 
 	/* dynamic */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
-		0, BASE_ADDRESS_MODIFY);
+	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER |
+		  I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
 
 	/* indirect */
 	OUT_BATCH(0);
 	OUT_BATCH(0);
 
 	/* instruction */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
+	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
+		  BASE_ADDRESS_MODIFY);
 
 	/* general state buffer size */
 	OUT_BATCH(0xfffff000 | 1);
@@ -480,7 +489,9 @@ gen8_emit_state_base_address(struct intel_batchbuffer *batch)
 	OUT_BATCH(1 << 12 | 1);
 	/* indirect object buffer size */
 	OUT_BATCH(0xfffff000 | 1);
-	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
+	/* instruction buffer size, must set modify enable bit, otherwise it may
+	 * result in GPU hang
+	 */
 	OUT_BATCH(1 << 12 | 1);
 }
 
@@ -565,8 +576,8 @@ gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch)
 
 void
 gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
-		     unsigned x, unsigned y,
-		     unsigned width, unsigned height)
+		     unsigned int x, unsigned int y,
+		     unsigned int width, unsigned int height)
 {
 	uint32_t x_dim, y_dim, tmp, right_mask;
 
@@ -662,15 +673,16 @@ gen9_emit_state_base_address(struct intel_batchbuffer *batch)
 	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
 
 	/* dynamic */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
-		0, BASE_ADDRESS_MODIFY);
+	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER |
+		  I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
 
 	/* indirect */
 	OUT_BATCH(0);
 	OUT_BATCH(0);
 
 	/* instruction */
-	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
+	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
+		  BASE_ADDRESS_MODIFY);
 
 	/* general state buffer size */
 	OUT_BATCH(0xfffff000 | 1);
@@ -678,7 +690,9 @@ gen9_emit_state_base_address(struct intel_batchbuffer *batch)
 	OUT_BATCH(1 << 12 | 1);
 	/* indirect object buffer size */
 	OUT_BATCH(0xfffff000 | 1);
-	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
+	/* intruction buffer size, must set modify enable bit, otherwise it may
+	 * result in GPU hang
+	 */
 	OUT_BATCH(1 << 12 | 1);
 
 	/* Bindless surface state base address */
diff --git a/lib/gpu_fill.h b/lib/gpu_fill.h
index 5335fe3f..abb3b4c3 100644
--- a/lib/gpu_fill.h
+++ b/lib/gpu_fill.h
@@ -60,8 +60,8 @@ gen7_fill_kernel(struct intel_batchbuffer *batch,
 		size_t size);
 
 uint32_t
-gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,
-			       const uint32_t kernel[][4], size_t size);
+gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf
+			       *dst, const uint32_t kernel[][4], size_t size);
 
 void
 gen7_emit_state_base_address(struct intel_batchbuffer *batch);
@@ -76,17 +76,18 @@ void
 gen7_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer);
 
 void
-gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch, uint32_t interface_descriptor);
+gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
+				    uint32_t interface_descriptor);
 
 void
 gen7_emit_media_objects(struct intel_batchbuffer *batch,
-			unsigned x, unsigned y,
-			unsigned width, unsigned height);
+			unsigned int x, unsigned int y,
+			unsigned int width, unsigned int height);
 
 void
 gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
-		     unsigned x, unsigned y,
-		     unsigned width, unsigned height);
+		     unsigned int x, unsigned int y,
+		     unsigned int width, unsigned int height);
 
 uint32_t
 gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
@@ -99,7 +100,9 @@ gen8_fill_surface_state(struct intel_batchbuffer *batch,
 			int is_dst);
 
 uint32_t
-gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,  const uint32_t kernel[][4], size_t size);
+gen8_fill_interface_descriptor(struct intel_batchbuffer *batch,
+			       struct igt_buf *dst, const uint32_t kernel[][4],
+			       size_t size);
 
 void
 gen8_emit_state_base_address(struct intel_batchbuffer *batch);
@@ -118,15 +121,12 @@ gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch);
 
 void
 gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
-		     unsigned x, unsigned y,
-		     unsigned width, unsigned height);
+		     unsigned int x, unsigned int y,
+		     unsigned int width, unsigned int height);
 
 void
 gen8_emit_media_objects_spin(struct intel_batchbuffer *batch);
 
-void
-gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch);
-
 void
 gen9_emit_state_base_address(struct intel_batchbuffer *batch);
 
diff --git a/lib/media_spin.c b/lib/media_spin.c
index b4414bee..b323550a 100644
--- a/lib/media_spin.c
+++ b/lib/media_spin.c
@@ -81,7 +81,8 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
 	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
 
 	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
-	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
+	interface_descriptor = gen8_fill_interface_descriptor(batch, dst,
+					      spin_kernel, sizeof(spin_kernel));
 	igt_assert(batch->ptr < &batch->buffer[4095]);
 
 	/* media pipeline */
@@ -119,7 +120,8 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
 	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
 
 	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
-	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
+	interface_descriptor = gen8_fill_interface_descriptor(batch, dst,
+					      spin_kernel, sizeof(spin_kernel));
 	igt_assert(batch->ptr < &batch->buffer[4095]);
 
 	/* media pipeline */
-- 
2.14.3

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] [PATCH i-g-t v2 4/5] lib/gpu_fill: Further code unification in gpu_fill
  2018-04-27 10:03 [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library Katarzyna Dec
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 2/5] lib/media_spin: Remove gen8lp_media_spin function Katarzyna Dec
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 3/5] lib: Adjust media_spin and gpu_fill to our code style Katarzyna Dec
@ 2018-04-27 10:03 ` Katarzyna Dec
  2018-04-27 17:19   ` Daniele Ceraolo Spurio
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 5/5] lib: Rename library from gpu_fill to gpu_helpers Katarzyna Dec
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Katarzyna Dec @ 2018-04-27 10:03 UTC (permalink / raw)
  To: igt-dev

We can unify gen7_emit_vfe_state and gen8_emit_vfe_state
functions for gpgpu/media_fill and media_spin by adding
parameters. gen8_emit_media_object was renamed to gen_*
and extended with additional offset parameters - we can
have one gen7_emit_media_objects for all tests.
I have renamed gen8_emit_media_object to gen_emit_*, because
funtion belongs to all gens and it would be odd to have
all named genX_* and only one without this prefix.

Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/gpgpu_fill.c      |  26 ++++++++--
 lib/gpu_fill.c        | 129 +++++++++-----------------------------------------
 lib/gpu_fill.h        |  20 ++++----
 lib/media_fill_gen7.c |  10 +++-
 lib/media_fill_gen8.c |   8 +++-
 lib/media_fill_gen9.c |   8 +++-
 lib/media_spin.c      |  25 ++++++++--
 7 files changed, 97 insertions(+), 129 deletions(-)

diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
index 52925a5c..3b89dd1b 100644
--- a/lib/gpgpu_fill.c
+++ b/lib/gpgpu_fill.c
@@ -106,6 +106,13 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	uint32_t curbe_buffer, interface_descriptor;
 	uint32_t batch_end;
 
+	/* Parameters for gen7_emit_vfe_state. */
+	uint32_t threads = 1;
+	uint32_t urb_entries = 0;
+	uint32_t urb_size = 0;
+	uint32_t curbe_size = 1;
+	uint32_t mode = 1;
+
 	intel_batchbuffer_flush(batch);
 
 	/* setup states */
@@ -129,7 +136,8 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
 
 	gen7_emit_state_base_address(batch);
-	gen7_emit_vfe_state_gpgpu(batch);
+	gen7_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size,
+			    mode);
 	gen7_emit_curbe_load(batch, curbe_buffer);
 	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
 	gen7_emit_gpgpu_walk(batch, x, y, width, height);
@@ -153,6 +161,12 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	uint32_t curbe_buffer, interface_descriptor;
 	uint32_t batch_end;
 
+	/* Parameters for gen8_emit_vfe_state. */
+	uint32_t threads = 1;
+	uint32_t urb_entries = 1;
+	uint32_t urb_size = 0;
+	uint32_t curbe_size = 1;
+
 	intel_batchbuffer_flush(batch);
 
 	/* setup states */
@@ -176,7 +190,7 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
 
 	gen8_emit_state_base_address(batch);
-	gen8_emit_vfe_state_gpgpu(batch);
+	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
 	gen7_emit_curbe_load(batch, curbe_buffer);
 	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
 	gen8_emit_gpgpu_walk(batch, x, y, width, height);
@@ -200,6 +214,12 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 	uint32_t curbe_buffer, interface_descriptor;
 	uint32_t batch_end;
 
+	/* Parameters for gen9_emit_vfe_state. */
+	uint32_t threads = 1;
+	uint32_t urb_entries = 1;
+	uint32_t urb_size = 0;
+	uint32_t curbe_size = 1;
+
 	intel_batchbuffer_flush(batch);
 
 	/* setup states */
@@ -224,7 +244,7 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
 		  PIPELINE_SELECT_GPGPU);
 
 	gen9_emit_state_base_address(batch);
-	gen8_emit_vfe_state_gpgpu(batch);
+	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
 	gen7_emit_curbe_load(batch, curbe_buffer);
 	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
 	gen8_emit_gpgpu_walk(batch, x, y, width, height);
diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
index 25c0c810..57b95840 100644
--- a/lib/gpu_fill.c
+++ b/lib/gpu_fill.c
@@ -37,8 +37,7 @@ gen7_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end)
 }
 
 uint32_t
-gen7_fill_curbe_buffer_data(struct intel_batchbuffer *batch,
-			uint8_t color)
+gen7_fill_curbe_buffer_data(struct intel_batchbuffer *batch, uint8_t color)
 {
 	uint8_t *curbe_buffer;
 	uint32_t offset;
@@ -193,7 +192,9 @@ gen7_emit_state_base_address(struct intel_batchbuffer *batch)
 }
 
 void
-gen7_emit_vfe_state(struct intel_batchbuffer *batch)
+gen7_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
+		    uint32_t urb_entries, uint32_t urb_size,
+		    uint32_t curbe_size, uint32_t mode)
 {
 	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (8 - 2));
 
@@ -201,39 +202,15 @@ gen7_emit_vfe_state(struct intel_batchbuffer *batch)
 	OUT_BATCH(0);
 
 	/* number of threads & urb entries */
-	OUT_BATCH(1 << 16 |
-		2 << 8);
+	OUT_BATCH(threads << 16 |
+		urb_entries << 8 |
+		mode << 2); /* GPGPU mode */
 
 	OUT_BATCH(0);
 
 	/* urb entry size & curbe size */
-	OUT_BATCH(2 << 16 |	/* in 256 bits unit */
-		  2);		/* in 256 bits unit */
-
-	/* scoreboard */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-}
-
-void
-gen7_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (8 - 2));
-
-	/* scratch buffer */
-	OUT_BATCH(0);
-
-	/* number of threads & urb entries */
-	OUT_BATCH(1 << 16 | /* max num of threads */
-		  0 << 8 | /* num of URB entry */
-		  1 << 2); /* GPGPU mode */
-
-	OUT_BATCH(0);
-
-	/* urb entry size & curbe size */
-	OUT_BATCH(0 << 16 |	/* URB entry size in 256 bits unit */
-		  1);		/* CURBE entry size in 256 bits unit */
+	OUT_BATCH(urb_size << 16 |	/* in 256 bits unit */
+		  curbe_size);		/* in 256 bits unit */
 
 	/* scoreboard */
 	OUT_BATCH(0);
@@ -271,32 +248,14 @@ gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
 
 void
 gen7_emit_media_objects(struct intel_batchbuffer *batch,
-			unsigned int x, unsigned int y,
+			unsigned int xoffset, unsigned int yoffset,
 			unsigned int width, unsigned int height)
 {
 	int i, j;
 
 	for (i = 0; i < width / 16; i++) {
 		for (j = 0; j < height / 16; j++) {
-			OUT_BATCH(GEN7_MEDIA_OBJECT | (8 - 2));
-
-			/* interface descriptor offset */
-			OUT_BATCH(0);
-
-			/* without indirect data */
-			OUT_BATCH(0);
-			OUT_BATCH(0);
-
-			/* scoreboard */
-			OUT_BATCH(0);
-			OUT_BATCH(0);
-
-			/* inline data (xoffset, yoffset) */
-			OUT_BATCH(x + i * 16);
-			OUT_BATCH(y + j * 16);
-			if (AT_LEAST_GEN(batch->devid, 8) &&
-			    !IS_CHERRYVIEW(batch->devid))
-				gen8_emit_media_state_flush(batch);
+			gen_emit_media_object(batch, xoffset, yoffset);
 		}
 	}
 }
@@ -503,32 +462,9 @@ gen8_emit_media_state_flush(struct intel_batchbuffer *batch)
 }
 
 void
-gen8_emit_vfe_state(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (9 - 2));
-
-	/* scratch buffer */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* number of threads & urb entries */
-	OUT_BATCH(1 << 16 |
-		2 << 8);
-
-	OUT_BATCH(0);
-
-	/* urb entry size & curbe size */
-	OUT_BATCH(2 << 16 |
-		2);
-
-	/* scoreboard */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-}
-
-void
-gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
+gen8_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
+		    uint32_t urb_entries, uint32_t urb_size,
+		    uint32_t curbe_size)
 {
 	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (9 - 2));
 
@@ -537,36 +473,14 @@ gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
 	OUT_BATCH(0);
 
 	/* number of threads & urb entries */
-	OUT_BATCH(1 << 16 | 1 << 8);
-
-	OUT_BATCH(0);
-
-	/* urb entry size & curbe size */
-	OUT_BATCH(0 << 16 | 1);
-
-	/* scoreboard */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-}
-
-void
-gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch)
-{
-	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
-
-	/* scratch buffer */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
-
-	/* number of threads & urb entries */
-	OUT_BATCH(2 << 8);
+	OUT_BATCH(threads << 16 |
+		urb_entries << 8);
 
 	OUT_BATCH(0);
 
 	/* urb entry size & curbe size */
-	OUT_BATCH(2 << 16 |
-		2);
+	OUT_BATCH(urb_size << 16 |
+		curbe_size);
 
 	/* scoreboard */
 	OUT_BATCH(0);
@@ -635,7 +549,8 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
 }
 
 void
-gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
+gen_emit_media_object(struct intel_batchbuffer *batch,
+		       unsigned int xoffset, unsigned int yoffset)
 {
 	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
 
@@ -651,8 +566,8 @@ gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
 	OUT_BATCH(0);
 
 	/* inline data (xoffset, yoffset) */
-	OUT_BATCH(0);
-	OUT_BATCH(0);
+	OUT_BATCH(xoffset);
+	OUT_BATCH(yoffset);
 	if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
 		gen8_emit_media_state_flush(batch);
 }
diff --git a/lib/gpu_fill.h b/lib/gpu_fill.h
index abb3b4c3..3a800198 100644
--- a/lib/gpu_fill.h
+++ b/lib/gpu_fill.h
@@ -67,10 +67,9 @@ void
 gen7_emit_state_base_address(struct intel_batchbuffer *batch);
 
 void
-gen7_emit_vfe_state(struct intel_batchbuffer *batch);
-
-void
-gen7_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
+gen7_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
+		    uint32_t urb_entries, uint32_t urb_size,
+		    uint32_t curbe_size, uint32_t mode);
 
 void
 gen7_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer);
@@ -111,13 +110,9 @@ void
 gen8_emit_media_state_flush(struct intel_batchbuffer *batch);
 
 void
-gen8_emit_vfe_state(struct intel_batchbuffer *batch);
-
-void
-gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
-
-void
-gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch);
+gen8_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
+		    uint32_t urb_entries, uint32_t urb_size,
+		    uint32_t curbe_size);
 
 void
 gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
@@ -125,7 +120,8 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
 		     unsigned int width, unsigned int height);
 
 void
-gen8_emit_media_objects_spin(struct intel_batchbuffer *batch);
+gen_emit_media_object(struct intel_batchbuffer *batch, unsigned int xoffset,
+		  unsigned int yoffset);
 
 void
 gen9_emit_state_base_address(struct intel_batchbuffer *batch);
diff --git a/lib/media_fill_gen7.c b/lib/media_fill_gen7.c
index 3dc5617e..02dff04b 100644
--- a/lib/media_fill_gen7.c
+++ b/lib/media_fill_gen7.c
@@ -54,6 +54,13 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch,
 	uint32_t curbe_buffer, interface_descriptor;
 	uint32_t batch_end;
 
+	/* Parameters for gen7_emit_vfe_state. */
+	uint32_t threads = 1;
+	uint32_t urb_entries = 2;
+	uint32_t urb_size = 2;
+	uint32_t curbe_size = 2;
+	uint32_t mode = 0;
+
 	intel_batchbuffer_flush(batch);
 
 	/* setup states */
@@ -69,7 +76,8 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch,
 	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
 	gen7_emit_state_base_address(batch);
 
-	gen7_emit_vfe_state(batch);
+	gen7_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size,
+			    mode);
 
 	gen7_emit_curbe_load(batch, curbe_buffer);
 
diff --git a/lib/media_fill_gen8.c b/lib/media_fill_gen8.c
index 63fe72eb..3103b5e7 100644
--- a/lib/media_fill_gen8.c
+++ b/lib/media_fill_gen8.c
@@ -57,6 +57,12 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch,
 	uint32_t curbe_buffer, interface_descriptor;
 	uint32_t batch_end;
 
+	/* Parameters for gen8_emit_vfe_state. */
+	uint32_t threads = 1;
+	uint32_t urb_entries = 2;
+	uint32_t urb_size = 2;
+	uint32_t curbe_size = 2;
+
 	intel_batchbuffer_flush(batch);
 
 	/* setup states */
@@ -72,7 +78,7 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch,
 	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
 	gen8_emit_state_base_address(batch);
 
-	gen8_emit_vfe_state(batch);
+	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
 
 	gen7_emit_curbe_load(batch, curbe_buffer);
 
diff --git a/lib/media_fill_gen9.c b/lib/media_fill_gen9.c
index 78e892f2..d783d0bd 100644
--- a/lib/media_fill_gen9.c
+++ b/lib/media_fill_gen9.c
@@ -54,6 +54,12 @@ gen9_media_fillfunc(struct intel_batchbuffer *batch,
 	uint32_t curbe_buffer, interface_descriptor;
 	uint32_t batch_end;
 
+	/* Parameters for gen9_emit_vfe_state. */
+	uint32_t threads = 1;
+	uint32_t urb_entries = 2;
+	uint32_t urb_size = 2;
+	uint32_t curbe_size = 2;
+
 	intel_batchbuffer_flush(batch);
 
 	/* setup states */
@@ -74,7 +80,7 @@ gen9_media_fillfunc(struct intel_batchbuffer *batch,
 			GEN9_FORCE_MEDIA_AWAKE_MASK);
 	gen9_emit_state_base_address(batch);
 
-	gen8_emit_vfe_state(batch);
+	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
 
 	gen7_emit_curbe_load(batch, curbe_buffer);
 
diff --git a/lib/media_spin.c b/lib/media_spin.c
index b323550a..2a8a6bdb 100644
--- a/lib/media_spin.c
+++ b/lib/media_spin.c
@@ -68,6 +68,11 @@ static const uint32_t spin_kernel[][4] = {
 
 #define BATCH_STATE_SPLIT 2048
 
+/* Offsets needed in gen_emit_media_object. In media_spin library this  * values do not matter.
+ */
+#define xoffset 0
+#define yoffset 0
+
 void
 gen8_media_spinfunc(struct intel_batchbuffer *batch,
 		    struct igt_buf *dst, uint32_t spins)
@@ -75,6 +80,12 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
 	uint32_t curbe_buffer, interface_descriptor;
 	uint32_t batch_end;
 
+	/* Parameters for gen8_emit_vfe_state. */
+	uint32_t threads = 2;
+	uint32_t urb_entries = 0;
+	uint32_t urb_size = 2;
+	uint32_t curbe_size = 2;
+
 	intel_batchbuffer_flush_with_context(batch, NULL);
 
 	/* setup states */
@@ -90,13 +101,13 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
 	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
 	gen8_emit_state_base_address(batch);
 
-	gen8_emit_vfe_state_spin(batch);
+	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
 
 	gen7_emit_curbe_load(batch, curbe_buffer);
 
 	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
 
-	gen8_emit_media_objects_spin(batch);
+	gen_emit_media_object(batch, xoffset, yoffset);
 
 	OUT_BATCH(MI_BATCH_BUFFER_END);
 
@@ -114,6 +125,12 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
 	uint32_t curbe_buffer, interface_descriptor;
 	uint32_t batch_end;
 
+	/* Parameters for gen9_emit_vfe_state. */
+	uint32_t threads = 1;
+	uint32_t urb_entries = 1;
+	uint32_t urb_size = 0;
+	uint32_t curbe_size = 1;
+
 	intel_batchbuffer_flush_with_context(batch, NULL);
 
 	/* setup states */
@@ -134,13 +151,13 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
 			GEN9_FORCE_MEDIA_AWAKE_MASK);
 	gen9_emit_state_base_address(batch);
 
-	gen8_emit_vfe_state_spin(batch);
+	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
 
 	gen7_emit_curbe_load(batch, curbe_buffer);
 
 	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
 
-	gen8_emit_media_objects_spin(batch);
+	gen_emit_media_object(batch, xoffset, yoffset);
 
 	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA |
 			GEN9_FORCE_MEDIA_AWAKE_DISABLE |
-- 
2.14.3

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] [PATCH i-g-t v2 5/5] lib: Rename library from gpu_fill to gpu_helpers
  2018-04-27 10:03 [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library Katarzyna Dec
                   ` (2 preceding siblings ...)
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 4/5] lib/gpu_fill: Further code unification in gpu_fill Katarzyna Dec
@ 2018-04-27 10:03 ` Katarzyna Dec
  2018-05-02 15:28   ` Daniele Ceraolo Spurio
  2018-04-27 11:30 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/5] lib/media_spin: Move helper functions to gpu_fill library Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Katarzyna Dec @ 2018-04-27 10:03 UTC (permalink / raw)
  To: igt-dev

After refactoring media_spin library - gpu_fill contains helper
functions for render copy, *_fill functions and media_spin.
Let's rename this library to gpu_helpers. This name will be more
general.

Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/Makefile.sources              | 4 ++--
 lib/gpgpu_fill.c                  | 2 +-
 lib/{gpu_fill.c => gpu_helpers.c} | 2 +-
 lib/{gpu_fill.h => gpu_helpers.h} | 6 +++---
 lib/media_fill_gen7.c             | 2 +-
 lib/media_fill_gen8.c             | 2 +-
 lib/media_fill_gen9.c             | 2 +-
 lib/media_spin.c                  | 2 +-
 lib/meson.build                   | 2 +-
 9 files changed, 12 insertions(+), 12 deletions(-)
 rename lib/{gpu_fill.c => gpu_helpers.c} (99%)
 rename lib/{gpu_fill.h => gpu_helpers.h} (98%)

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 9c0150c1..2b642277 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -63,8 +63,8 @@ lib_source_list =	 	\
 	media_spin.c		\
 	gpgpu_fill.h		\
 	gpgpu_fill.c		\
-	gpu_fill.h		\
-	gpu_fill.c		\
+	gpu_helpers.h		\
+	gpu_helpers.c		\
 	gen7_media.h            \
 	gen8_media.h            \
 	rendercopy_i915.c	\
diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
index 3b89dd1b..820fc7d4 100644
--- a/lib/gpgpu_fill.c
+++ b/lib/gpgpu_fill.c
@@ -32,7 +32,7 @@
 #include "drmtest.h"
 
 #include "gpgpu_fill.h"
-#include "gpu_fill.h"
+#include "gpu_helpers.h"
 
 /* shaders/gpgpu/gpgpu_fill.gxa */
 static const uint32_t gen7_gpgpu_kernel[][4] = {
diff --git a/lib/gpu_fill.c b/lib/gpu_helpers.c
similarity index 99%
rename from lib/gpu_fill.c
rename to lib/gpu_helpers.c
index 57b95840..650a3859 100644
--- a/lib/gpu_fill.c
+++ b/lib/gpu_helpers.c
@@ -22,7 +22,7 @@
  *
  */
 
-#include "gpu_fill.h"
+#include "gpu_helpers.h"
 
 void
 gen7_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end)
diff --git a/lib/gpu_fill.h b/lib/gpu_helpers.h
similarity index 98%
rename from lib/gpu_fill.h
rename to lib/gpu_helpers.h
index 3a800198..62320293 100644
--- a/lib/gpu_fill.h
+++ b/lib/gpu_helpers.h
@@ -22,8 +22,8 @@
  *
  */
 
-#ifndef GPU_FILL_H
-#define GPU_FILL_H
+#ifndef GPU_HELPERS_H
+#define GPU_HELPERS_H
 
 #include <intel_bufmgr.h>
 #include <i915_drm.h>
@@ -126,4 +126,4 @@ gen_emit_media_object(struct intel_batchbuffer *batch, unsigned int xoffset,
 void
 gen9_emit_state_base_address(struct intel_batchbuffer *batch);
 
-#endif /* GPU_FILL_H */
+#endif /* GPU_HELPERS_H */
diff --git a/lib/media_fill_gen7.c b/lib/media_fill_gen7.c
index 02dff04b..3c2c218a 100644
--- a/lib/media_fill_gen7.c
+++ b/lib/media_fill_gen7.c
@@ -5,7 +5,7 @@
 #include "gen7_media.h"
 #include "intel_reg.h"
 #include "drmtest.h"
-#include "gpu_fill.h"
+#include "gpu_helpers.h"
 #include <assert.h>
 
 static const uint32_t media_kernel[][4] = {
diff --git a/lib/media_fill_gen8.c b/lib/media_fill_gen8.c
index 3103b5e7..f9bec157 100644
--- a/lib/media_fill_gen8.c
+++ b/lib/media_fill_gen8.c
@@ -5,7 +5,7 @@
 #include "gen8_media.h"
 #include "intel_reg.h"
 #include "drmtest.h"
-#include "gpu_fill.h"
+#include "gpu_helpers.h"
 #include <assert.h>
 
 
diff --git a/lib/media_fill_gen9.c b/lib/media_fill_gen9.c
index d783d0bd..7c24b12b 100644
--- a/lib/media_fill_gen9.c
+++ b/lib/media_fill_gen9.c
@@ -4,7 +4,7 @@
 #include "media_fill.h"
 #include "gen8_media.h"
 #include "intel_reg.h"
-#include "gpu_fill.h"
+#include "gpu_helpers.h"
 #include <assert.h>
 
 static const uint32_t media_kernel[][4] = {
diff --git a/lib/media_spin.c b/lib/media_spin.c
index 2a8a6bdb..2e26088c 100644
--- a/lib/media_spin.c
+++ b/lib/media_spin.c
@@ -31,7 +31,7 @@
 #include "intel_batchbuffer.h"
 #include "gen8_media.h"
 #include "media_spin.h"
-#include "gpu_fill.h"
+#include "gpu_helpers.h"
 
 static const uint32_t spin_kernel[][4] = {
 	{ 0x00600001, 0x20800208, 0x008d0000, 0x00000000 }, /* mov (8)r4.0<1>:ud r0.0<8;8;1>:ud */
diff --git a/lib/meson.build b/lib/meson.build
index 5f2567fb..f5f60340 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -29,7 +29,7 @@ lib_sources = [
 	'media_fill_gen9.c',
 	'media_spin.c',
 	'gpgpu_fill.c',
-	'gpu_fill.c',
+	'gpu_helpers.c',
 	'rendercopy_i915.c',
 	'rendercopy_i830.c',
 	'rendercopy_gen6.c',
-- 
2.14.3

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/5] lib/media_spin: Move helper functions to gpu_fill library
  2018-04-27 10:03 [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library Katarzyna Dec
                   ` (3 preceding siblings ...)
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 5/5] lib: Rename library from gpu_fill to gpu_helpers Katarzyna Dec
@ 2018-04-27 11:30 ` Patchwork
  2018-04-27 14:30 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
  2018-04-27 17:47 ` [igt-dev] [PATCH i-g-t v2 1/5] " Daniele Ceraolo Spurio
  6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-04-27 11:30 UTC (permalink / raw)
  To: Katarzyna Dec; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/5] lib/media_spin: Move helper functions to gpu_fill library
URL   : https://patchwork.freedesktop.org/series/42391/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4107 -> IGTPW_1305 =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1305 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1305, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42391/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1305:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_force_connector_basic@prune-stale-modes:
      fi-snb-2600:        PASS -> SKIP

    
== Known issues ==

  Here are the changes found in IGTPW_1305 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-cnl-y3:          PASS -> FAIL (fdo#103167)
      fi-hsw-4200u:       PASS -> DMESG-FAIL (fdo#106103)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cnl-psr:         PASS -> DMESG-WARN (fdo#104951)

    
    ==== Possible fixes ====

    igt@kms_busy@basic-flip-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-cnl-psr:         FAIL (fdo#100368) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-cnl-psr:         FAIL (fdo#103481) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (39 -> 36) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4450 -> IGTPW_1305

  CI_DRM_4107: f711c0b36f2382983c964bd69d6c477482e604f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1305: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1305/
  IGT_4450: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4450: b57600ba58ae0cdbad86826fd653aa0191212f27 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1305/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* [igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,v2,1/5] lib/media_spin: Move helper functions to gpu_fill library
  2018-04-27 10:03 [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library Katarzyna Dec
                   ` (4 preceding siblings ...)
  2018-04-27 11:30 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/5] lib/media_spin: Move helper functions to gpu_fill library Patchwork
@ 2018-04-27 14:30 ` Patchwork
  2018-04-27 17:47 ` [igt-dev] [PATCH i-g-t v2 1/5] " Daniele Ceraolo Spurio
  6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2018-04-27 14:30 UTC (permalink / raw)
  To: Katarzyna Dec; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/5] lib/media_spin: Move helper functions to gpu_fill library
URL   : https://patchwork.freedesktop.org/series/42391/
State : failure

== Summary ==

= CI Bug Log - changes from IGT_4450_full -> IGTPW_1305_full =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1305_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1305_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42391/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1305_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_media_fill:
      shard-apl:          PASS -> FAIL
      shard-glk:          PASS -> FAIL
      shard-hsw:          PASS -> FAIL

    
    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in IGTPW_1305_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368) +5

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-hsw:          FAIL (fdo#100368) -> PASS +1

    igt@kms_flip@2x-wf_vblank-ts-check:
      shard-hsw:          FAIL (fdo#105312) -> PASS

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          FAIL (fdo#103925) -> PASS

    igt@perf@blocking:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105312 https://bugs.freedesktop.org/show_bug.cgi?id=105312


== Participating hosts (8 -> 4) ==

  Missing    (4): pig-skl-6600 pig-glk-j4005 pig-hsw-4770r shard-kbl 


== Build changes ==

    * IGT: IGT_4450 -> IGTPW_1305
    * Linux: CI_DRM_4105 -> CI_DRM_4107

  CI_DRM_4105: e0ab6277c75e2bdc4a651c6b62b1a75ac5c9b375 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4107: f711c0b36f2382983c964bd69d6c477482e604f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1305: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1305/
  IGT_4450: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4450: b57600ba58ae0cdbad86826fd653aa0191212f27 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1305/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v2 4/5] lib/gpu_fill: Further code unification in gpu_fill
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 4/5] lib/gpu_fill: Further code unification in gpu_fill Katarzyna Dec
@ 2018-04-27 17:19   ` Daniele Ceraolo Spurio
  2018-05-02 11:27     ` Katarzyna Dec
  0 siblings, 1 reply; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-04-27 17:19 UTC (permalink / raw)
  To: Katarzyna Dec, igt-dev



On 27/04/18 03:03, Katarzyna Dec wrote:
> We can unify gen7_emit_vfe_state and gen8_emit_vfe_state
> functions for gpgpu/media_fill and media_spin by adding
> parameters. gen8_emit_media_object was renamed to gen_*
> and extended with additional offset parameters - we can
> have one gen7_emit_media_objects for all tests.
> I have renamed gen8_emit_media_object to gen_emit_*, because
> funtion belongs to all gens and it would be odd to have
> all named genX_* and only one without this prefix.
> 
> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   lib/gpgpu_fill.c      |  26 ++++++++--
>   lib/gpu_fill.c        | 129 +++++++++-----------------------------------------
>   lib/gpu_fill.h        |  20 ++++----
>   lib/media_fill_gen7.c |  10 +++-
>   lib/media_fill_gen8.c |   8 +++-
>   lib/media_fill_gen9.c |   8 +++-
>   lib/media_spin.c      |  25 ++++++++--
>   7 files changed, 97 insertions(+), 129 deletions(-)
> 
> diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> index 52925a5c..3b89dd1b 100644
> --- a/lib/gpgpu_fill.c
> +++ b/lib/gpgpu_fill.c
> @@ -106,6 +106,13 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	uint32_t curbe_buffer, interface_descriptor;
>   	uint32_t batch_end;
>   
> +	/* Parameters for gen7_emit_vfe_state. */
> +	uint32_t threads = 1;
> +	uint32_t urb_entries = 0;
> +	uint32_t urb_size = 0;
> +	uint32_t curbe_size = 1;
> +	uint32_t mode = 1;
> +

Those are mostly the same across the various functions. Maybe we can 
just use global defines with some comments, something like:

/* VFE STATE param */
#define THREADS 1
#define GEN7_URB_ENTRIES 0
#define GEN8_URB_ENTRIES 1
#define URB_SIZE 0   	/* size of 1 entry in 256 bits unit */
#define CURBE_SIZE 1 	/* in 256 bits unit */
#define GEN7_VFE_STATE_GPGPU_MODE 1

>   	intel_batchbuffer_flush(batch);
>   
>   	/* setup states */
> @@ -129,7 +136,8 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
>   
>   	gen7_emit_state_base_address(batch);
> -	gen7_emit_vfe_state_gpgpu(batch);
> +	gen7_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size,
> +			    mode);
>   	gen7_emit_curbe_load(batch, curbe_buffer);
>   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>   	gen7_emit_gpgpu_walk(batch, x, y, width, height);
> @@ -153,6 +161,12 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	uint32_t curbe_buffer, interface_descriptor;
>   	uint32_t batch_end;
>   
> +	/* Parameters for gen8_emit_vfe_state. */
> +	uint32_t threads = 1;
> +	uint32_t urb_entries = 1;
> +	uint32_t urb_size = 0;
> +	uint32_t curbe_size = 1;
> +
>   	intel_batchbuffer_flush(batch);
>   
>   	/* setup states */
> @@ -176,7 +190,7 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
>   
>   	gen8_emit_state_base_address(batch);
> -	gen8_emit_vfe_state_gpgpu(batch);
> +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
>   	gen7_emit_curbe_load(batch, curbe_buffer);
>   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>   	gen8_emit_gpgpu_walk(batch, x, y, width, height);
> @@ -200,6 +214,12 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	uint32_t curbe_buffer, interface_descriptor;
>   	uint32_t batch_end;
>   
> +	/* Parameters for gen9_emit_vfe_state. */
> +	uint32_t threads = 1;
> +	uint32_t urb_entries = 1;
> +	uint32_t urb_size = 0;
> +	uint32_t curbe_size = 1;
> +
>   	intel_batchbuffer_flush(batch);
>   
>   	/* setup states */
> @@ -224,7 +244,7 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   		  PIPELINE_SELECT_GPGPU);
>   
>   	gen9_emit_state_base_address(batch);
> -	gen8_emit_vfe_state_gpgpu(batch);
> +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
>   	gen7_emit_curbe_load(batch, curbe_buffer);
>   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>   	gen8_emit_gpgpu_walk(batch, x, y, width, height);
> diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
> index 25c0c810..57b95840 100644
> --- a/lib/gpu_fill.c
> +++ b/lib/gpu_fill.c
> @@ -37,8 +37,7 @@ gen7_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end)
>   }
>   
>   uint32_t
> -gen7_fill_curbe_buffer_data(struct intel_batchbuffer *batch,
> -			uint8_t color)
> +gen7_fill_curbe_buffer_data(struct intel_batchbuffer *batch, uint8_t color)
>   {
>   	uint8_t *curbe_buffer;
>   	uint32_t offset;
> @@ -193,7 +192,9 @@ gen7_emit_state_base_address(struct intel_batchbuffer *batch)
>   }
>   
>   void
> -gen7_emit_vfe_state(struct intel_batchbuffer *batch)
> +gen7_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
> +		    uint32_t urb_entries, uint32_t urb_size,
> +		    uint32_t curbe_size, uint32_t mode)
>   {
>   	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (8 - 2));
>   
> @@ -201,39 +202,15 @@ gen7_emit_vfe_state(struct intel_batchbuffer *batch)
>   	OUT_BATCH(0);
>   
>   	/* number of threads & urb entries */
> -	OUT_BATCH(1 << 16 |
> -		2 << 8);
> +	OUT_BATCH(threads << 16 |
> +		urb_entries << 8 |
> +		mode << 2); /* GPGPU mode */

Mode is used to select between media and GPGPU, so the comment could be 
updated to something like:

/* GPGPU vs media mode */

>   
>   	OUT_BATCH(0);
>   
>   	/* urb entry size & curbe size */
> -	OUT_BATCH(2 << 16 |	/* in 256 bits unit */
> -		  2);		/* in 256 bits unit */
> -
> -	/* scoreboard */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -}
> -
> -void
> -gen7_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (8 - 2));
> -
> -	/* scratch buffer */
> -	OUT_BATCH(0);
> -
> -	/* number of threads & urb entries */
> -	OUT_BATCH(1 << 16 | /* max num of threads */
> -		  0 << 8 | /* num of URB entry */
> -		  1 << 2); /* GPGPU mode */
> -
> -	OUT_BATCH(0);
> -
> -	/* urb entry size & curbe size */
> -	OUT_BATCH(0 << 16 |	/* URB entry size in 256 bits unit */
> -		  1);		/* CURBE entry size in 256 bits unit */
> +	OUT_BATCH(urb_size << 16 |	/* in 256 bits unit */
> +		  curbe_size);		/* in 256 bits unit */
>   
>   	/* scoreboard */
>   	OUT_BATCH(0);
> @@ -271,32 +248,14 @@ gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
>   
>   void
>   gen7_emit_media_objects(struct intel_batchbuffer *batch,
> -			unsigned int x, unsigned int y,
> +			unsigned int xoffset, unsigned int yoffset,
>   			unsigned int width, unsigned int height)
>   {
>   	int i, j;
>   
>   	for (i = 0; i < width / 16; i++) {
>   		for (j = 0; j < height / 16; j++) {
> -			OUT_BATCH(GEN7_MEDIA_OBJECT | (8 - 2));
> -
> -			/* interface descriptor offset */
> -			OUT_BATCH(0);
> -
> -			/* without indirect data */
> -			OUT_BATCH(0);
> -			OUT_BATCH(0);
> -
> -			/* scoreboard */
> -			OUT_BATCH(0);
> -			OUT_BATCH(0);
> -
> -			/* inline data (xoffset, yoffset) */
> -			OUT_BATCH(x + i * 16);
> -			OUT_BATCH(y + j * 16);
> -			if (AT_LEAST_GEN(batch->devid, 8) &&
> -			    !IS_CHERRYVIEW(batch->devid))
> -				gen8_emit_media_state_flush(batch);
> +			gen_emit_media_object(batch, xoffset, yoffset);

This is wrong, you want to pass in x + i * 16 and y + j * 16 and not x 
and y as they are. Probably the reason why CI is failing.

>   		}
>   	}
>   }
> @@ -503,32 +462,9 @@ gen8_emit_media_state_flush(struct intel_batchbuffer *batch)
>   }
>   
>   void
> -gen8_emit_vfe_state(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (9 - 2));
> -
> -	/* scratch buffer */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* number of threads & urb entries */
> -	OUT_BATCH(1 << 16 |
> -		2 << 8);
> -
> -	OUT_BATCH(0);
> -
> -	/* urb entry size & curbe size */
> -	OUT_BATCH(2 << 16 |
> -		2);
> -
> -	/* scoreboard */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -}
> -
> -void
> -gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
> +gen8_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
> +		    uint32_t urb_entries, uint32_t urb_size,
> +		    uint32_t curbe_size)
>   {
>   	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (9 - 2));
>   
> @@ -537,36 +473,14 @@ gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
>   	OUT_BATCH(0);
>   
>   	/* number of threads & urb entries */
> -	OUT_BATCH(1 << 16 | 1 << 8);
> -
> -	OUT_BATCH(0);
> -
> -	/* urb entry size & curbe size */
> -	OUT_BATCH(0 << 16 | 1);
> -
> -	/* scoreboard */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -}
> -
> -void
> -gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
> -
> -	/* scratch buffer */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* number of threads & urb entries */
> -	OUT_BATCH(2 << 8);
> +	OUT_BATCH(threads << 16 |
> +		urb_entries << 8);
>   
>   	OUT_BATCH(0);
>   
>   	/* urb entry size & curbe size */
> -	OUT_BATCH(2 << 16 |
> -		2);
> +	OUT_BATCH(urb_size << 16 |
> +		curbe_size);
>   
>   	/* scoreboard */
>   	OUT_BATCH(0);
> @@ -635,7 +549,8 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
>   }
>   
>   void
> -gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
> +gen_emit_media_object(struct intel_batchbuffer *batch,
> +		       unsigned int xoffset, unsigned int yoffset)
>   {
>   	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
>   
> @@ -651,8 +566,8 @@ gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
>   	OUT_BATCH(0);
>   
>   	/* inline data (xoffset, yoffset) */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> +	OUT_BATCH(xoffset);
> +	OUT_BATCH(yoffset);
>   	if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
>   		gen8_emit_media_state_flush(batch);
>   }
> diff --git a/lib/gpu_fill.h b/lib/gpu_fill.h
> index abb3b4c3..3a800198 100644
> --- a/lib/gpu_fill.h
> +++ b/lib/gpu_fill.h
> @@ -67,10 +67,9 @@ void
>   gen7_emit_state_base_address(struct intel_batchbuffer *batch);
>   
>   void
> -gen7_emit_vfe_state(struct intel_batchbuffer *batch);
> -
> -void
> -gen7_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
> +gen7_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
> +		    uint32_t urb_entries, uint32_t urb_size,
> +		    uint32_t curbe_size, uint32_t mode);
>   
>   void
>   gen7_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer);
> @@ -111,13 +110,9 @@ void
>   gen8_emit_media_state_flush(struct intel_batchbuffer *batch);
>   
>   void
> -gen8_emit_vfe_state(struct intel_batchbuffer *batch);
> -
> -void
> -gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
> -
> -void
> -gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch);
> +gen8_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
> +		    uint32_t urb_entries, uint32_t urb_size,
> +		    uint32_t curbe_size);
>   
>   void
>   gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> @@ -125,7 +120,8 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
>   		     unsigned int width, unsigned int height);
>   
>   void
> -gen8_emit_media_objects_spin(struct intel_batchbuffer *batch);
> +gen_emit_media_object(struct intel_batchbuffer *batch, unsigned int xoffset,
> +		  unsigned int yoffset);
>   
>   void
>   gen9_emit_state_base_address(struct intel_batchbuffer *batch);
> diff --git a/lib/media_fill_gen7.c b/lib/media_fill_gen7.c
> index 3dc5617e..02dff04b 100644
> --- a/lib/media_fill_gen7.c
> +++ b/lib/media_fill_gen7.c
> @@ -54,6 +54,13 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch,
>   	uint32_t curbe_buffer, interface_descriptor;
>   	uint32_t batch_end;
>   
> +	/* Parameters for gen7_emit_vfe_state. */
> +	uint32_t threads = 1;
> +	uint32_t urb_entries = 2;
> +	uint32_t urb_size = 2;
> +	uint32_t curbe_size = 2;
> +	uint32_t mode = 0;
> +

Same as the above, we can use defines for these (and in this case there 
are no differences in values across gens, apart from gen7 having to 
select the mode).

>   	intel_batchbuffer_flush(batch);
>   
>   	/* setup states */
> @@ -69,7 +76,8 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch,
>   	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
>   	gen7_emit_state_base_address(batch);
>   
> -	gen7_emit_vfe_state(batch);
> +	gen7_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size,
> +			    mode);
>   
>   	gen7_emit_curbe_load(batch, curbe_buffer);
>   
> diff --git a/lib/media_fill_gen8.c b/lib/media_fill_gen8.c
> index 63fe72eb..3103b5e7 100644
> --- a/lib/media_fill_gen8.c
> +++ b/lib/media_fill_gen8.c
> @@ -57,6 +57,12 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch,
>   	uint32_t curbe_buffer, interface_descriptor;
>   	uint32_t batch_end;
>   
> +	/* Parameters for gen8_emit_vfe_state. */
> +	uint32_t threads = 1;
> +	uint32_t urb_entries = 2;
> +	uint32_t urb_size = 2;
> +	uint32_t curbe_size = 2;
> +
>   	intel_batchbuffer_flush(batch);
>   
>   	/* setup states */
> @@ -72,7 +78,7 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch,
>   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
>   	gen8_emit_state_base_address(batch);
>   
> -	gen8_emit_vfe_state(batch);
> +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
>   
>   	gen7_emit_curbe_load(batch, curbe_buffer);
>   
> diff --git a/lib/media_fill_gen9.c b/lib/media_fill_gen9.c
> index 78e892f2..d783d0bd 100644
> --- a/lib/media_fill_gen9.c
> +++ b/lib/media_fill_gen9.c
> @@ -54,6 +54,12 @@ gen9_media_fillfunc(struct intel_batchbuffer *batch,
>   	uint32_t curbe_buffer, interface_descriptor;
>   	uint32_t batch_end;
>   
> +	/* Parameters for gen9_emit_vfe_state. */
> +	uint32_t threads = 1;
> +	uint32_t urb_entries = 2;
> +	uint32_t urb_size = 2;
> +	uint32_t curbe_size = 2;
> +
>   	intel_batchbuffer_flush(batch);
>   
>   	/* setup states */
> @@ -74,7 +80,7 @@ gen9_media_fillfunc(struct intel_batchbuffer *batch,
>   			GEN9_FORCE_MEDIA_AWAKE_MASK);
>   	gen9_emit_state_base_address(batch);
>   
> -	gen8_emit_vfe_state(batch);
> +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
>   
>   	gen7_emit_curbe_load(batch, curbe_buffer);
>   
> diff --git a/lib/media_spin.c b/lib/media_spin.c
> index b323550a..2a8a6bdb 100644
> --- a/lib/media_spin.c
> +++ b/lib/media_spin.c
> @@ -68,6 +68,11 @@ static const uint32_t spin_kernel[][4] = {
>   
>   #define BATCH_STATE_SPLIT 2048
>   
> +/* Offsets needed in gen_emit_media_object. In media_spin library this  * values do not matter.

Missing a newline here

> + */
> +#define xoffset 0
> +#define yoffset 0
> +
>   void
>   gen8_media_spinfunc(struct intel_batchbuffer *batch,
>   		    struct igt_buf *dst, uint32_t spins)
> @@ -75,6 +80,12 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
>   	uint32_t curbe_buffer, interface_descriptor;
>   	uint32_t batch_end;
>   
> +	/* Parameters for gen8_emit_vfe_state. */
> +	uint32_t threads = 2;
> +	uint32_t urb_entries = 0;
> +	uint32_t urb_size = 2;
> +	uint32_t curbe_size = 2;
> +
>   	intel_batchbuffer_flush_with_context(batch, NULL);
>   
>   	/* setup states */
> @@ -90,13 +101,13 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
>   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
>   	gen8_emit_state_base_address(batch);
>   
> -	gen8_emit_vfe_state_spin(batch);
> +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
>   
>   	gen7_emit_curbe_load(batch, curbe_buffer);
>   
>   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>   
> -	gen8_emit_media_objects_spin(batch);
> +	gen_emit_media_object(batch, xoffset, yoffset);
>   
>   	OUT_BATCH(MI_BATCH_BUFFER_END);
>   
> @@ -114,6 +125,12 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
>   	uint32_t curbe_buffer, interface_descriptor;
>   	uint32_t batch_end;
>   
> +	/* Parameters for gen9_emit_vfe_state. */
> +	uint32_t threads = 1;
> +	uint32_t urb_entries = 1;
> +	uint32_t urb_size = 0;
> +	uint32_t curbe_size = 1;
> +

both the gen8 and gen9 media spin called into gen8_emit_vfe_state_spin 
so they should have the same parameters. Any reason to change them for 
gen9? If this was just a cut & paste mistake and the parameters are 
indeed going to be the same I suggest switching to defines like in other 
places.

Daniele

>   	intel_batchbuffer_flush_with_context(batch, NULL);
>   
>   	/* setup states */
> @@ -134,13 +151,13 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
>   			GEN9_FORCE_MEDIA_AWAKE_MASK);
>   	gen9_emit_state_base_address(batch);
>   
> -	gen8_emit_vfe_state_spin(batch);
> +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
>   
>   	gen7_emit_curbe_load(batch, curbe_buffer);
>   
>   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>   
> -	gen8_emit_media_objects_spin(batch);
> +	gen_emit_media_object(batch, xoffset, yoffset);
>   
>   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA |
>   			GEN9_FORCE_MEDIA_AWAKE_DISABLE |
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library
  2018-04-27 10:03 [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library Katarzyna Dec
                   ` (5 preceding siblings ...)
  2018-04-27 14:30 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-04-27 17:47 ` Daniele Ceraolo Spurio
  2018-05-02 11:21   ` Katarzyna Dec
  6 siblings, 1 reply; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-04-27 17:47 UTC (permalink / raw)
  To: Katarzyna Dec, igt-dev



On 27/04/18 03:03, Katarzyna Dec wrote:
> Let's remove duplications introduced by moving media_spin helper
> functions to gpu_fill. These were mainly the same functions
> as for Gen8 media/gpgpu fill. gen8_render_flush from media_spin
> was replaced by gen7_render_flush. The only functions that were
> left intact are gen8_emit_media_objects_spin and
> gen8_spin_render_flush.
> 

gen8_spin_render_flush does not exist in this patch. Also you've left 
gen8_spin_curbe_buffer_data, gen8_emit_vfe_state_spin and 
gen8lp_emit_media_objects_spin as well.

With the commit message fixed:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

> v2: squashed patches 1 and 2 from v1
> 
> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   lib/gpu_fill.c   |  81 +++++++++++++
>   lib/gpu_fill.h   |  13 +++
>   lib/media_spin.c | 347 +++----------------------------------------------------
>   3 files changed, 113 insertions(+), 328 deletions(-)
> 
> diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
> index f05d4eca..f5fc61bb 100644
> --- a/lib/gpu_fill.c
> +++ b/lib/gpu_fill.c
> @@ -351,6 +351,20 @@ gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
>   	OUT_BATCH(0xffffffff);
>   }
>   
> +uint32_t
> +gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
> +			    uint32_t iters)
> +{
> +	uint32_t *curbe_buffer;
> +	uint32_t offset;
> +
> +	curbe_buffer = intel_batchbuffer_subdata_alloc(batch, 64, 64);
> +	offset = intel_batchbuffer_subdata_offset(batch, curbe_buffer);
> +	*curbe_buffer = iters;
> +
> +	return offset;
> +}
> +
>   uint32_t
>   gen8_fill_surface_state(struct intel_batchbuffer *batch,
>   			struct igt_buf *buf,
> @@ -525,6 +539,30 @@ gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
>   	OUT_BATCH(0);
>   }
>   
> +void
> +gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch)
> +{
> +	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
> +
> +	/* scratch buffer */
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +
> +	/* number of threads & urb entries */
> +	OUT_BATCH(2 << 8);
> +
> +	OUT_BATCH(0);
> +
> +	/* urb entry size & curbe size */
> +	OUT_BATCH(2 << 16 |
> +		2);
> +
> +	/* scoreboard */
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +}
> +
>   void
>   gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
>   		     unsigned x, unsigned y,
> @@ -585,6 +623,49 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
>   	OUT_BATCH(0xffffffff);
>   }
>   
> +void
> +gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
> +{
> +	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> +
> +	/* interface descriptor offset */
> +	OUT_BATCH(0);
> +
> +	/* without indirect data */
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +
> +	/* scoreboard */
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +
> +	/* inline data (xoffset, yoffset) */
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +	gen8_emit_media_state_flush(batch);
> +}
> +
> +void
> +gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch)
> +{
> +	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> +
> +	/* interface descriptor offset */
> +	OUT_BATCH(0);
> +
> +	/* without indirect data */
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +
> +	/* scoreboard */
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +
> +	/* inline data (xoffset, yoffset) */
> +	OUT_BATCH(0);
> +	OUT_BATCH(0);
> +}
> +
>   void
>   gen9_emit_state_base_address(struct intel_batchbuffer *batch)
>   {
> diff --git a/lib/gpu_fill.h b/lib/gpu_fill.h
> index 067d4987..5335fe3f 100644
> --- a/lib/gpu_fill.h
> +++ b/lib/gpu_fill.h
> @@ -88,6 +88,10 @@ gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
>   		     unsigned x, unsigned y,
>   		     unsigned width, unsigned height);
>   
> +uint32_t
> +gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
> +			    uint32_t iters);
> +
>   uint32_t
>   gen8_fill_surface_state(struct intel_batchbuffer *batch,
>   			struct igt_buf *buf,
> @@ -109,11 +113,20 @@ gen8_emit_vfe_state(struct intel_batchbuffer *batch);
>   void
>   gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
>   
> +void
> +gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch);
> +
>   void
>   gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
>   		     unsigned x, unsigned y,
>   		     unsigned width, unsigned height);
>   
> +void
> +gen8_emit_media_objects_spin(struct intel_batchbuffer *batch);
> +
> +void
> +gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch);
> +
>   void
>   gen9_emit_state_base_address(struct intel_batchbuffer *batch);
>   
> diff --git a/lib/media_spin.c b/lib/media_spin.c
> index d9e058b1..16ea8483 100644
> --- a/lib/media_spin.c
> +++ b/lib/media_spin.c
> @@ -31,6 +31,7 @@
>   #include "intel_batchbuffer.h"
>   #include "gen8_media.h"
>   #include "media_spin.h"
> +#include "gpu_fill.h"
>   
>   static const uint32_t spin_kernel[][4] = {
>   	{ 0x00600001, 0x20800208, 0x008d0000, 0x00000000 }, /* mov (8)r4.0<1>:ud r0.0<8;8;1>:ud */
> @@ -45,316 +46,6 @@ static const uint32_t spin_kernel[][4] = {
>   	{ 0x07800031, 0x20000a40, 0x0e000e00, 0x82000010 }, /* send.ts (16)null<1> r112<0;1;0>:d 0x82000010 */
>   };
>   
> -static void
> -gen8_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end)
> -{
> -	int ret;
> -
> -	ret = drm_intel_bo_subdata(batch->bo, 0, 4096, batch->buffer);
> -	if (ret == 0)
> -		ret = drm_intel_gem_bo_context_exec(batch->bo, NULL,
> -						    batch_end, 0);
> -	igt_assert_eq(ret, 0);
> -}
> -
> -static uint32_t
> -gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
> -			    uint32_t iters)
> -{
> -	uint32_t *curbe_buffer;
> -	uint32_t offset;
> -
> -	curbe_buffer = intel_batchbuffer_subdata_alloc(batch, 64, 64);
> -	offset = intel_batchbuffer_subdata_offset(batch, curbe_buffer);
> -	*curbe_buffer = iters;
> -
> -	return offset;
> -}
> -
> -static uint32_t
> -gen8_spin_surface_state(struct intel_batchbuffer *batch,
> -			struct igt_buf *buf,
> -			uint32_t format,
> -			int is_dst)
> -{
> -	struct gen8_surface_state *ss;
> -	uint32_t write_domain, read_domain, offset;
> -	int ret;
> -
> -	if (is_dst) {
> -		write_domain = read_domain = I915_GEM_DOMAIN_RENDER;
> -	} else {
> -		write_domain = 0;
> -		read_domain = I915_GEM_DOMAIN_SAMPLER;
> -	}
> -
> -	ss = intel_batchbuffer_subdata_alloc(batch, sizeof(*ss), 64);
> -	offset = intel_batchbuffer_subdata_offset(batch, ss);
> -
> -	ss->ss0.surface_type = GEN8_SURFACE_2D;
> -	ss->ss0.surface_format = format;
> -	ss->ss0.render_cache_read_write = 1;
> -	ss->ss0.vertical_alignment = 1; /* align 4 */
> -	ss->ss0.horizontal_alignment = 1; /* align 4 */
> -
> -	if (buf->tiling == I915_TILING_X)
> -		ss->ss0.tiled_mode = 2;
> -	else if (buf->tiling == I915_TILING_Y)
> -		ss->ss0.tiled_mode = 3;
> -
> -	ss->ss8.base_addr = buf->bo->offset;
> -
> -	ret = drm_intel_bo_emit_reloc(batch->bo,
> -				intel_batchbuffer_subdata_offset(batch, ss) + 8 * 4,
> -				buf->bo, 0,
> -				read_domain, write_domain);
> -	igt_assert_eq(ret, 0);
> -
> -	ss->ss2.height = igt_buf_height(buf) - 1;
> -	ss->ss2.width  = igt_buf_width(buf) - 1;
> -	ss->ss3.pitch  = buf->stride - 1;
> -
> -	ss->ss7.shader_chanel_select_r = 4;
> -	ss->ss7.shader_chanel_select_g = 5;
> -	ss->ss7.shader_chanel_select_b = 6;
> -	ss->ss7.shader_chanel_select_a = 7;
> -
> -	return offset;
> -}
> -
> -static uint32_t
> -gen8_spin_binding_table(struct intel_batchbuffer *batch,
> -			struct igt_buf *dst)
> -{
> -	uint32_t *binding_table, offset;
> -
> -	binding_table = intel_batchbuffer_subdata_alloc(batch, 32, 64);
> -	offset = intel_batchbuffer_subdata_offset(batch, binding_table);
> -
> -	binding_table[0] = gen8_spin_surface_state(batch, dst,
> -					GEN8_SURFACEFORMAT_R8_UNORM, 1);
> -
> -	return offset;
> -}
> -
> -static uint32_t
> -gen8_spin_media_kernel(struct intel_batchbuffer *batch,
> -		       const uint32_t kernel[][4],
> -		       size_t size)
> -{
> -	uint32_t offset;
> -
> -	offset = intel_batchbuffer_copy_data(batch, kernel, size, 64);
> -
> -	return offset;
> -}
> -
> -static uint32_t
> -gen8_spin_interface_descriptor(struct intel_batchbuffer *batch,
> -			       struct igt_buf *dst)
> -{
> -	struct gen8_interface_descriptor_data *idd;
> -	uint32_t offset;
> -	uint32_t binding_table_offset, kernel_offset;
> -
> -	binding_table_offset = gen8_spin_binding_table(batch, dst);
> -	kernel_offset = gen8_spin_media_kernel(batch, spin_kernel,
> -					       sizeof(spin_kernel));
> -
> -	idd = intel_batchbuffer_subdata_alloc(batch, sizeof(*idd), 64);
> -	offset = intel_batchbuffer_subdata_offset(batch, idd);
> -
> -	idd->desc0.kernel_start_pointer = (kernel_offset >> 6);
> -
> -	idd->desc2.single_program_flow = 1;
> -	idd->desc2.floating_point_mode = GEN8_FLOATING_POINT_IEEE_754;
> -
> -	idd->desc3.sampler_count = 0;      /* 0 samplers used */
> -	idd->desc3.sampler_state_pointer = 0;
> -
> -	idd->desc4.binding_table_entry_count = 0;
> -	idd->desc4.binding_table_pointer = (binding_table_offset >> 5);
> -
> -	idd->desc5.constant_urb_entry_read_offset = 0;
> -	idd->desc5.constant_urb_entry_read_length = 1; /* grf 1 */
> -
> -	return offset;
> -}
> -
> -static void
> -gen8_emit_state_base_address(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN8_STATE_BASE_ADDRESS | (16 - 2));
> -
> -	/* general */
> -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> -	OUT_BATCH(0);
> -
> -	/* stateless data port */
> -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> -
> -	/* surface */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
> -
> -	/* dynamic */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
> -		0, BASE_ADDRESS_MODIFY);
> -
> -	/* indirect */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* instruction */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> -
> -	/* general state buffer size */
> -	OUT_BATCH(0xfffff000 | 1);
> -	/* dynamic state buffer size */
> -	OUT_BATCH(1 << 12 | 1);
> -	/* indirect object buffer size */
> -	OUT_BATCH(0xfffff000 | 1);
> -	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
> -	OUT_BATCH(1 << 12 | 1);
> -}
> -
> -static void
> -gen9_emit_state_base_address(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN8_STATE_BASE_ADDRESS | (19 - 2));
> -
> -	/* general */
> -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> -	OUT_BATCH(0);
> -
> -	/* stateless data port */
> -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> -
> -	/* surface */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
> -
> -	/* dynamic */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
> -		0, BASE_ADDRESS_MODIFY);
> -
> -	/* indirect */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* instruction */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> -
> -	/* general state buffer size */
> -	OUT_BATCH(0xfffff000 | 1);
> -	/* dynamic state buffer size */
> -	OUT_BATCH(1 << 12 | 1);
> -	/* indirect object buffer size */
> -	OUT_BATCH(0xfffff000 | 1);
> -	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
> -	OUT_BATCH(1 << 12 | 1);
> -
> -	/* Bindless surface state base address */
> -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> -	OUT_BATCH(0);
> -	OUT_BATCH(0xfffff000);
> -}
> -
> -static void
> -gen8_emit_vfe_state(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
> -
> -	/* scratch buffer */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* number of threads & urb entries */
> -	OUT_BATCH(2 << 8);
> -
> -	OUT_BATCH(0);
> -
> -	/* urb entry size & curbe size */
> -	OUT_BATCH(2 << 16 |
> -		2);
> -
> -	/* scoreboard */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -}
> -
> -static void
> -gen8_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer)
> -{
> -	OUT_BATCH(GEN8_MEDIA_CURBE_LOAD | (4 - 2));
> -	OUT_BATCH(0);
> -	/* curbe total data length */
> -	OUT_BATCH(64);
> -	/* curbe data start address, is relative to the dynamics base address */
> -	OUT_BATCH(curbe_buffer);
> -}
> -
> -static void
> -gen8_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
> -				    uint32_t interface_descriptor)
> -{
> -	OUT_BATCH(GEN8_MEDIA_INTERFACE_DESCRIPTOR_LOAD | (4 - 2));
> -	OUT_BATCH(0);
> -	/* interface descriptor data length */
> -	OUT_BATCH(sizeof(struct gen8_interface_descriptor_data));
> -	/* interface descriptor address, is relative to the dynamics base address */
> -	OUT_BATCH(interface_descriptor);
> -}
> -
> -static void
> -gen8_emit_media_state_flush(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN8_MEDIA_STATE_FLUSH | (2 - 2));
> -	OUT_BATCH(0);
> -}
> -
> -static void
> -gen8_emit_media_objects(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> -
> -	/* interface descriptor offset */
> -	OUT_BATCH(0);
> -
> -	/* without indirect data */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* scoreboard */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* inline data (xoffset, yoffset) */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -	gen8_emit_media_state_flush(batch);
> -}
> -
> -static void
> -gen8lp_emit_media_objects(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> -
> -	/* interface descriptor offset */
> -	OUT_BATCH(0);
> -
> -	/* without indirect data */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* scoreboard */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* inline data (xoffset, yoffset) */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -}
> -
>   /*
>    * This sets up the media pipeline,
>    *
> @@ -390,7 +81,7 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
>   	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> -	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
> +	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
>   	igt_assert(batch->ptr < &batch->buffer[4095]);
>   
>   	/* media pipeline */
> @@ -398,20 +89,20 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
>   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
>   	gen8_emit_state_base_address(batch);
>   
> -	gen8_emit_vfe_state(batch);
> +	gen8_emit_vfe_state_spin(batch);
>   
> -	gen8_emit_curbe_load(batch, curbe_buffer);
> +	gen7_emit_curbe_load(batch, curbe_buffer);
>   
> -	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
> +	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>   
> -	gen8_emit_media_objects(batch);
> +	gen8_emit_media_objects_spin(batch);
>   
>   	OUT_BATCH(MI_BATCH_BUFFER_END);
>   
>   	batch_end = intel_batchbuffer_align(batch, 8);
>   	igt_assert(batch_end < BATCH_STATE_SPLIT);
>   
> -	gen8_render_flush(batch, batch_end);
> +	gen7_render_flush(batch, batch_end);
>   	intel_batchbuffer_reset(batch);
>   }
>   
> @@ -428,7 +119,7 @@ gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
>   	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> -	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
> +	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
>   	igt_assert(batch->ptr < &batch->buffer[4095]);
>   
>   	/* media pipeline */
> @@ -436,20 +127,20 @@ gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
>   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
>   	gen8_emit_state_base_address(batch);
>   
> -	gen8_emit_vfe_state(batch);
> +	gen8_emit_vfe_state_spin(batch);
>   
> -	gen8_emit_curbe_load(batch, curbe_buffer);
> +	gen7_emit_curbe_load(batch, curbe_buffer);
>   
> -	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
> +	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>   
> -	gen8lp_emit_media_objects(batch);
> +	gen8lp_emit_media_objects_spin(batch);
>   
>   	OUT_BATCH(MI_BATCH_BUFFER_END);
>   
>   	batch_end = intel_batchbuffer_align(batch, 8);
>   	igt_assert(batch_end < BATCH_STATE_SPLIT);
>   
> -	gen8_render_flush(batch, batch_end);
> +	gen7_render_flush(batch, batch_end);
>   	intel_batchbuffer_reset(batch);
>   }
>   
> @@ -466,7 +157,7 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
>   	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> -	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
> +	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
>   	igt_assert(batch->ptr < &batch->buffer[4095]);
>   
>   	/* media pipeline */
> @@ -479,13 +170,13 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
>   			GEN9_FORCE_MEDIA_AWAKE_MASK);
>   	gen9_emit_state_base_address(batch);
>   
> -	gen8_emit_vfe_state(batch);
> +	gen8_emit_vfe_state_spin(batch);
>   
> -	gen8_emit_curbe_load(batch, curbe_buffer);
> +	gen7_emit_curbe_load(batch, curbe_buffer);
>   
> -	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
> +	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>   
> -	gen8_emit_media_objects(batch);
> +	gen8_emit_media_objects_spin(batch);
>   
>   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA |
>   			GEN9_FORCE_MEDIA_AWAKE_DISABLE |
> @@ -499,6 +190,6 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
>   	batch_end = intel_batchbuffer_align(batch, 8);
>   	igt_assert(batch_end < BATCH_STATE_SPLIT);
>   
> -	gen8_render_flush(batch, batch_end);
> +	gen7_render_flush(batch, batch_end);
>   	intel_batchbuffer_reset(batch);
>   }
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library
  2018-04-27 17:47 ` [igt-dev] [PATCH i-g-t v2 1/5] " Daniele Ceraolo Spurio
@ 2018-05-02 11:21   ` Katarzyna Dec
  0 siblings, 0 replies; 15+ messages in thread
From: Katarzyna Dec @ 2018-05-02 11:21 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: igt-dev

On Fri, Apr 27, 2018 at 10:47:27AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 27/04/18 03:03, Katarzyna Dec wrote:
> > Let's remove duplications introduced by moving media_spin helper
> > functions to gpu_fill. These were mainly the same functions
> > as for Gen8 media/gpgpu fill. gen8_render_flush from media_spin
> > was replaced by gen7_render_flush. The only functions that were
> > left intact are gen8_emit_media_objects_spin and
> > gen8_spin_render_flush.
> > 
> 
> gen8_spin_render_flush does not exist in this patch. Also you've left
> gen8_spin_curbe_buffer_data, gen8_emit_vfe_state_spin and
> gen8lp_emit_media_objects_spin as well.
> 
> With the commit message fixed:
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Daniele
>
I will submit new version with commit msg fixed.
Thanks for feedback :)
Kasia
> > v2: squashed patches 1 and 2 from v1
> > 
> > Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> > Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> > Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > ---
> >   lib/gpu_fill.c   |  81 +++++++++++++
> >   lib/gpu_fill.h   |  13 +++
> >   lib/media_spin.c | 347 +++----------------------------------------------------
> >   3 files changed, 113 insertions(+), 328 deletions(-)
> > 
> > diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
> > index f05d4eca..f5fc61bb 100644
> > --- a/lib/gpu_fill.c
> > +++ b/lib/gpu_fill.c
> > @@ -351,6 +351,20 @@ gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(0xffffffff);
> >   }
> > +uint32_t
> > +gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
> > +			    uint32_t iters)
> > +{
> > +	uint32_t *curbe_buffer;
> > +	uint32_t offset;
> > +
> > +	curbe_buffer = intel_batchbuffer_subdata_alloc(batch, 64, 64);
> > +	offset = intel_batchbuffer_subdata_offset(batch, curbe_buffer);
> > +	*curbe_buffer = iters;
> > +
> > +	return offset;
> > +}
> > +
> >   uint32_t
> >   gen8_fill_surface_state(struct intel_batchbuffer *batch,
> >   			struct igt_buf *buf,
> > @@ -525,6 +539,30 @@ gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
> >   	OUT_BATCH(0);
> >   }
> > +void
> > +gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch)
> > +{
> > +	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
> > +
> > +	/* scratch buffer */
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +
> > +	/* number of threads & urb entries */
> > +	OUT_BATCH(2 << 8);
> > +
> > +	OUT_BATCH(0);
> > +
> > +	/* urb entry size & curbe size */
> > +	OUT_BATCH(2 << 16 |
> > +		2);
> > +
> > +	/* scoreboard */
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +}
> > +
> >   void
> >   gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> >   		     unsigned x, unsigned y,
> > @@ -585,6 +623,49 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(0xffffffff);
> >   }
> > +void
> > +gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
> > +{
> > +	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> > +
> > +	/* interface descriptor offset */
> > +	OUT_BATCH(0);
> > +
> > +	/* without indirect data */
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +
> > +	/* scoreboard */
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +
> > +	/* inline data (xoffset, yoffset) */
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +	gen8_emit_media_state_flush(batch);
> > +}
> > +
> > +void
> > +gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch)
> > +{
> > +	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> > +
> > +	/* interface descriptor offset */
> > +	OUT_BATCH(0);
> > +
> > +	/* without indirect data */
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +
> > +	/* scoreboard */
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +
> > +	/* inline data (xoffset, yoffset) */
> > +	OUT_BATCH(0);
> > +	OUT_BATCH(0);
> > +}
> > +
> >   void
> >   gen9_emit_state_base_address(struct intel_batchbuffer *batch)
> >   {
> > diff --git a/lib/gpu_fill.h b/lib/gpu_fill.h
> > index 067d4987..5335fe3f 100644
> > --- a/lib/gpu_fill.h
> > +++ b/lib/gpu_fill.h
> > @@ -88,6 +88,10 @@ gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> >   		     unsigned x, unsigned y,
> >   		     unsigned width, unsigned height);
> > +uint32_t
> > +gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
> > +			    uint32_t iters);
> > +
> >   uint32_t
> >   gen8_fill_surface_state(struct intel_batchbuffer *batch,
> >   			struct igt_buf *buf,
> > @@ -109,11 +113,20 @@ gen8_emit_vfe_state(struct intel_batchbuffer *batch);
> >   void
> >   gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
> > +void
> > +gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch);
> > +
> >   void
> >   gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> >   		     unsigned x, unsigned y,
> >   		     unsigned width, unsigned height);
> > +void
> > +gen8_emit_media_objects_spin(struct intel_batchbuffer *batch);
> > +
> > +void
> > +gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch);
> > +
> >   void
> >   gen9_emit_state_base_address(struct intel_batchbuffer *batch);
> > diff --git a/lib/media_spin.c b/lib/media_spin.c
> > index d9e058b1..16ea8483 100644
> > --- a/lib/media_spin.c
> > +++ b/lib/media_spin.c
> > @@ -31,6 +31,7 @@
> >   #include "intel_batchbuffer.h"
> >   #include "gen8_media.h"
> >   #include "media_spin.h"
> > +#include "gpu_fill.h"
> >   static const uint32_t spin_kernel[][4] = {
> >   	{ 0x00600001, 0x20800208, 0x008d0000, 0x00000000 }, /* mov (8)r4.0<1>:ud r0.0<8;8;1>:ud */
> > @@ -45,316 +46,6 @@ static const uint32_t spin_kernel[][4] = {
> >   	{ 0x07800031, 0x20000a40, 0x0e000e00, 0x82000010 }, /* send.ts (16)null<1> r112<0;1;0>:d 0x82000010 */
> >   };
> > -static void
> > -gen8_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end)
> > -{
> > -	int ret;
> > -
> > -	ret = drm_intel_bo_subdata(batch->bo, 0, 4096, batch->buffer);
> > -	if (ret == 0)
> > -		ret = drm_intel_gem_bo_context_exec(batch->bo, NULL,
> > -						    batch_end, 0);
> > -	igt_assert_eq(ret, 0);
> > -}
> > -
> > -static uint32_t
> > -gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
> > -			    uint32_t iters)
> > -{
> > -	uint32_t *curbe_buffer;
> > -	uint32_t offset;
> > -
> > -	curbe_buffer = intel_batchbuffer_subdata_alloc(batch, 64, 64);
> > -	offset = intel_batchbuffer_subdata_offset(batch, curbe_buffer);
> > -	*curbe_buffer = iters;
> > -
> > -	return offset;
> > -}
> > -
> > -static uint32_t
> > -gen8_spin_surface_state(struct intel_batchbuffer *batch,
> > -			struct igt_buf *buf,
> > -			uint32_t format,
> > -			int is_dst)
> > -{
> > -	struct gen8_surface_state *ss;
> > -	uint32_t write_domain, read_domain, offset;
> > -	int ret;
> > -
> > -	if (is_dst) {
> > -		write_domain = read_domain = I915_GEM_DOMAIN_RENDER;
> > -	} else {
> > -		write_domain = 0;
> > -		read_domain = I915_GEM_DOMAIN_SAMPLER;
> > -	}
> > -
> > -	ss = intel_batchbuffer_subdata_alloc(batch, sizeof(*ss), 64);
> > -	offset = intel_batchbuffer_subdata_offset(batch, ss);
> > -
> > -	ss->ss0.surface_type = GEN8_SURFACE_2D;
> > -	ss->ss0.surface_format = format;
> > -	ss->ss0.render_cache_read_write = 1;
> > -	ss->ss0.vertical_alignment = 1; /* align 4 */
> > -	ss->ss0.horizontal_alignment = 1; /* align 4 */
> > -
> > -	if (buf->tiling == I915_TILING_X)
> > -		ss->ss0.tiled_mode = 2;
> > -	else if (buf->tiling == I915_TILING_Y)
> > -		ss->ss0.tiled_mode = 3;
> > -
> > -	ss->ss8.base_addr = buf->bo->offset;
> > -
> > -	ret = drm_intel_bo_emit_reloc(batch->bo,
> > -				intel_batchbuffer_subdata_offset(batch, ss) + 8 * 4,
> > -				buf->bo, 0,
> > -				read_domain, write_domain);
> > -	igt_assert_eq(ret, 0);
> > -
> > -	ss->ss2.height = igt_buf_height(buf) - 1;
> > -	ss->ss2.width  = igt_buf_width(buf) - 1;
> > -	ss->ss3.pitch  = buf->stride - 1;
> > -
> > -	ss->ss7.shader_chanel_select_r = 4;
> > -	ss->ss7.shader_chanel_select_g = 5;
> > -	ss->ss7.shader_chanel_select_b = 6;
> > -	ss->ss7.shader_chanel_select_a = 7;
> > -
> > -	return offset;
> > -}
> > -
> > -static uint32_t
> > -gen8_spin_binding_table(struct intel_batchbuffer *batch,
> > -			struct igt_buf *dst)
> > -{
> > -	uint32_t *binding_table, offset;
> > -
> > -	binding_table = intel_batchbuffer_subdata_alloc(batch, 32, 64);
> > -	offset = intel_batchbuffer_subdata_offset(batch, binding_table);
> > -
> > -	binding_table[0] = gen8_spin_surface_state(batch, dst,
> > -					GEN8_SURFACEFORMAT_R8_UNORM, 1);
> > -
> > -	return offset;
> > -}
> > -
> > -static uint32_t
> > -gen8_spin_media_kernel(struct intel_batchbuffer *batch,
> > -		       const uint32_t kernel[][4],
> > -		       size_t size)
> > -{
> > -	uint32_t offset;
> > -
> > -	offset = intel_batchbuffer_copy_data(batch, kernel, size, 64);
> > -
> > -	return offset;
> > -}
> > -
> > -static uint32_t
> > -gen8_spin_interface_descriptor(struct intel_batchbuffer *batch,
> > -			       struct igt_buf *dst)
> > -{
> > -	struct gen8_interface_descriptor_data *idd;
> > -	uint32_t offset;
> > -	uint32_t binding_table_offset, kernel_offset;
> > -
> > -	binding_table_offset = gen8_spin_binding_table(batch, dst);
> > -	kernel_offset = gen8_spin_media_kernel(batch, spin_kernel,
> > -					       sizeof(spin_kernel));
> > -
> > -	idd = intel_batchbuffer_subdata_alloc(batch, sizeof(*idd), 64);
> > -	offset = intel_batchbuffer_subdata_offset(batch, idd);
> > -
> > -	idd->desc0.kernel_start_pointer = (kernel_offset >> 6);
> > -
> > -	idd->desc2.single_program_flow = 1;
> > -	idd->desc2.floating_point_mode = GEN8_FLOATING_POINT_IEEE_754;
> > -
> > -	idd->desc3.sampler_count = 0;      /* 0 samplers used */
> > -	idd->desc3.sampler_state_pointer = 0;
> > -
> > -	idd->desc4.binding_table_entry_count = 0;
> > -	idd->desc4.binding_table_pointer = (binding_table_offset >> 5);
> > -
> > -	idd->desc5.constant_urb_entry_read_offset = 0;
> > -	idd->desc5.constant_urb_entry_read_length = 1; /* grf 1 */
> > -
> > -	return offset;
> > -}
> > -
> > -static void
> > -gen8_emit_state_base_address(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN8_STATE_BASE_ADDRESS | (16 - 2));
> > -
> > -	/* general */
> > -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> > -	OUT_BATCH(0);
> > -
> > -	/* stateless data port */
> > -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> > -
> > -	/* surface */
> > -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
> > -
> > -	/* dynamic */
> > -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
> > -		0, BASE_ADDRESS_MODIFY);
> > -
> > -	/* indirect */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* instruction */
> > -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> > -
> > -	/* general state buffer size */
> > -	OUT_BATCH(0xfffff000 | 1);
> > -	/* dynamic state buffer size */
> > -	OUT_BATCH(1 << 12 | 1);
> > -	/* indirect object buffer size */
> > -	OUT_BATCH(0xfffff000 | 1);
> > -	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
> > -	OUT_BATCH(1 << 12 | 1);
> > -}
> > -
> > -static void
> > -gen9_emit_state_base_address(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN8_STATE_BASE_ADDRESS | (19 - 2));
> > -
> > -	/* general */
> > -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> > -	OUT_BATCH(0);
> > -
> > -	/* stateless data port */
> > -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> > -
> > -	/* surface */
> > -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
> > -
> > -	/* dynamic */
> > -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
> > -		0, BASE_ADDRESS_MODIFY);
> > -
> > -	/* indirect */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* instruction */
> > -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> > -
> > -	/* general state buffer size */
> > -	OUT_BATCH(0xfffff000 | 1);
> > -	/* dynamic state buffer size */
> > -	OUT_BATCH(1 << 12 | 1);
> > -	/* indirect object buffer size */
> > -	OUT_BATCH(0xfffff000 | 1);
> > -	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
> > -	OUT_BATCH(1 << 12 | 1);
> > -
> > -	/* Bindless surface state base address */
> > -	OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0xfffff000);
> > -}
> > -
> > -static void
> > -gen8_emit_vfe_state(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
> > -
> > -	/* scratch buffer */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* number of threads & urb entries */
> > -	OUT_BATCH(2 << 8);
> > -
> > -	OUT_BATCH(0);
> > -
> > -	/* urb entry size & curbe size */
> > -	OUT_BATCH(2 << 16 |
> > -		2);
> > -
> > -	/* scoreboard */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -}
> > -
> > -static void
> > -gen8_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer)
> > -{
> > -	OUT_BATCH(GEN8_MEDIA_CURBE_LOAD | (4 - 2));
> > -	OUT_BATCH(0);
> > -	/* curbe total data length */
> > -	OUT_BATCH(64);
> > -	/* curbe data start address, is relative to the dynamics base address */
> > -	OUT_BATCH(curbe_buffer);
> > -}
> > -
> > -static void
> > -gen8_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
> > -				    uint32_t interface_descriptor)
> > -{
> > -	OUT_BATCH(GEN8_MEDIA_INTERFACE_DESCRIPTOR_LOAD | (4 - 2));
> > -	OUT_BATCH(0);
> > -	/* interface descriptor data length */
> > -	OUT_BATCH(sizeof(struct gen8_interface_descriptor_data));
> > -	/* interface descriptor address, is relative to the dynamics base address */
> > -	OUT_BATCH(interface_descriptor);
> > -}
> > -
> > -static void
> > -gen8_emit_media_state_flush(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN8_MEDIA_STATE_FLUSH | (2 - 2));
> > -	OUT_BATCH(0);
> > -}
> > -
> > -static void
> > -gen8_emit_media_objects(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> > -
> > -	/* interface descriptor offset */
> > -	OUT_BATCH(0);
> > -
> > -	/* without indirect data */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* scoreboard */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* inline data (xoffset, yoffset) */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -	gen8_emit_media_state_flush(batch);
> > -}
> > -
> > -static void
> > -gen8lp_emit_media_objects(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> > -
> > -	/* interface descriptor offset */
> > -	OUT_BATCH(0);
> > -
> > -	/* without indirect data */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* scoreboard */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* inline data (xoffset, yoffset) */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -}
> > -
> >   /*
> >    * This sets up the media pipeline,
> >    *
> > @@ -390,7 +81,7 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
> >   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
> >   	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> > -	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
> > +	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
> >   	igt_assert(batch->ptr < &batch->buffer[4095]);
> >   	/* media pipeline */
> > @@ -398,20 +89,20 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
> >   	gen8_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state(batch);
> > +	gen8_emit_vfe_state_spin(batch);
> > -	gen8_emit_curbe_load(batch, curbe_buffer);
> > +	gen7_emit_curbe_load(batch, curbe_buffer);
> > -	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
> > +	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> > -	gen8_emit_media_objects(batch);
> > +	gen8_emit_media_objects_spin(batch);
> >   	OUT_BATCH(MI_BATCH_BUFFER_END);
> >   	batch_end = intel_batchbuffer_align(batch, 8);
> >   	igt_assert(batch_end < BATCH_STATE_SPLIT);
> > -	gen8_render_flush(batch, batch_end);
> > +	gen7_render_flush(batch, batch_end);
> >   	intel_batchbuffer_reset(batch);
> >   }
> > @@ -428,7 +119,7 @@ gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
> >   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
> >   	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> > -	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
> > +	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
> >   	igt_assert(batch->ptr < &batch->buffer[4095]);
> >   	/* media pipeline */
> > @@ -436,20 +127,20 @@ gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
> >   	gen8_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state(batch);
> > +	gen8_emit_vfe_state_spin(batch);
> > -	gen8_emit_curbe_load(batch, curbe_buffer);
> > +	gen7_emit_curbe_load(batch, curbe_buffer);
> > -	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
> > +	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> > -	gen8lp_emit_media_objects(batch);
> > +	gen8lp_emit_media_objects_spin(batch);
> >   	OUT_BATCH(MI_BATCH_BUFFER_END);
> >   	batch_end = intel_batchbuffer_align(batch, 8);
> >   	igt_assert(batch_end < BATCH_STATE_SPLIT);
> > -	gen8_render_flush(batch, batch_end);
> > +	gen7_render_flush(batch, batch_end);
> >   	intel_batchbuffer_reset(batch);
> >   }
> > @@ -466,7 +157,7 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
> >   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
> >   	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> > -	interface_descriptor = gen8_spin_interface_descriptor(batch, dst);
> > +	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
> >   	igt_assert(batch->ptr < &batch->buffer[4095]);
> >   	/* media pipeline */
> > @@ -479,13 +170,13 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
> >   			GEN9_FORCE_MEDIA_AWAKE_MASK);
> >   	gen9_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state(batch);
> > +	gen8_emit_vfe_state_spin(batch);
> > -	gen8_emit_curbe_load(batch, curbe_buffer);
> > +	gen7_emit_curbe_load(batch, curbe_buffer);
> > -	gen8_emit_interface_descriptor_load(batch, interface_descriptor);
> > +	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> > -	gen8_emit_media_objects(batch);
> > +	gen8_emit_media_objects_spin(batch);
> >   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA |
> >   			GEN9_FORCE_MEDIA_AWAKE_DISABLE |
> > @@ -499,6 +190,6 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
> >   	batch_end = intel_batchbuffer_align(batch, 8);
> >   	igt_assert(batch_end < BATCH_STATE_SPLIT);
> > -	gen8_render_flush(batch, batch_end);
> > +	gen7_render_flush(batch, batch_end);
> >   	intel_batchbuffer_reset(batch);
> >   }
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v2 4/5] lib/gpu_fill: Further code unification in gpu_fill
  2018-04-27 17:19   ` Daniele Ceraolo Spurio
@ 2018-05-02 11:27     ` Katarzyna Dec
  0 siblings, 0 replies; 15+ messages in thread
From: Katarzyna Dec @ 2018-05-02 11:27 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: igt-dev

On Fri, Apr 27, 2018 at 10:19:45AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 27/04/18 03:03, Katarzyna Dec wrote:
> > We can unify gen7_emit_vfe_state and gen8_emit_vfe_state
> > functions for gpgpu/media_fill and media_spin by adding
> > parameters. gen8_emit_media_object was renamed to gen_*
> > and extended with additional offset parameters - we can
> > have one gen7_emit_media_objects for all tests.
> > I have renamed gen8_emit_media_object to gen_emit_*, because
> > funtion belongs to all gens and it would be odd to have
> > all named genX_* and only one without this prefix.
> > 
> > Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> > Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> > Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > ---
> >   lib/gpgpu_fill.c      |  26 ++++++++--
> >   lib/gpu_fill.c        | 129 +++++++++-----------------------------------------
> >   lib/gpu_fill.h        |  20 ++++----
> >   lib/media_fill_gen7.c |  10 +++-
> >   lib/media_fill_gen8.c |   8 +++-
> >   lib/media_fill_gen9.c |   8 +++-
> >   lib/media_spin.c      |  25 ++++++++--
> >   7 files changed, 97 insertions(+), 129 deletions(-)
> > 
> > diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> > index 52925a5c..3b89dd1b 100644
> > --- a/lib/gpgpu_fill.c
> > +++ b/lib/gpgpu_fill.c
> > @@ -106,6 +106,13 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> >   	uint32_t curbe_buffer, interface_descriptor;
> >   	uint32_t batch_end;
> > +	/* Parameters for gen7_emit_vfe_state. */
> > +	uint32_t threads = 1;
> > +	uint32_t urb_entries = 0;
> > +	uint32_t urb_size = 0;
> > +	uint32_t curbe_size = 1;
> > +	uint32_t mode = 1;
> > +
> 
> Those are mostly the same across the various functions. Maybe we can just
> use global defines with some comments, something like:
> 
> /* VFE STATE param */
> #define THREADS 1
> #define GEN7_URB_ENTRIES 0
> #define GEN8_URB_ENTRIES 1
> #define URB_SIZE 0   	/* size of 1 entry in 256 bits unit */
> #define CURBE_SIZE 1 	/* in 256 bits unit */
> #define GEN7_VFE_STATE_GPGPU_MODE 1
> 
I was not satisfied with declaring variables e.g. too much duplications
defines look more convienient :)
> >   	intel_batchbuffer_flush(batch);
> >   	/* setup states */
> > @@ -129,7 +136,8 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
> >   	gen7_emit_state_base_address(batch);
> > -	gen7_emit_vfe_state_gpgpu(batch);
> > +	gen7_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size,
> > +			    mode);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> >   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> >   	gen7_emit_gpgpu_walk(batch, x, y, width, height);
> > @@ -153,6 +161,12 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> >   	uint32_t curbe_buffer, interface_descriptor;
> >   	uint32_t batch_end;
> > +	/* Parameters for gen8_emit_vfe_state. */
> > +	uint32_t threads = 1;
> > +	uint32_t urb_entries = 1;
> > +	uint32_t urb_size = 0;
> > +	uint32_t curbe_size = 1;
> > +
> >   	intel_batchbuffer_flush(batch);
> >   	/* setup states */
> > @@ -176,7 +190,7 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_GPGPU);
> >   	gen8_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state_gpgpu(batch);
> > +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> >   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> >   	gen8_emit_gpgpu_walk(batch, x, y, width, height);
> > @@ -200,6 +214,12 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> >   	uint32_t curbe_buffer, interface_descriptor;
> >   	uint32_t batch_end;
> > +	/* Parameters for gen9_emit_vfe_state. */
> > +	uint32_t threads = 1;
> > +	uint32_t urb_entries = 1;
> > +	uint32_t urb_size = 0;
> > +	uint32_t curbe_size = 1;
> > +
> >   	intel_batchbuffer_flush(batch);
> >   	/* setup states */
> > @@ -224,7 +244,7 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> >   		  PIPELINE_SELECT_GPGPU);
> >   	gen9_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state_gpgpu(batch);
> > +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> >   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> >   	gen8_emit_gpgpu_walk(batch, x, y, width, height);
> > diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
> > index 25c0c810..57b95840 100644
> > --- a/lib/gpu_fill.c
> > +++ b/lib/gpu_fill.c
> > @@ -37,8 +37,7 @@ gen7_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end)
> >   }
> >   uint32_t
> > -gen7_fill_curbe_buffer_data(struct intel_batchbuffer *batch,
> > -			uint8_t color)
> > +gen7_fill_curbe_buffer_data(struct intel_batchbuffer *batch, uint8_t color)
> >   {
> >   	uint8_t *curbe_buffer;
> >   	uint32_t offset;
> > @@ -193,7 +192,9 @@ gen7_emit_state_base_address(struct intel_batchbuffer *batch)
> >   }
> >   void
> > -gen7_emit_vfe_state(struct intel_batchbuffer *batch)
> > +gen7_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
> > +		    uint32_t urb_entries, uint32_t urb_size,
> > +		    uint32_t curbe_size, uint32_t mode)
> >   {
> >   	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (8 - 2));
> > @@ -201,39 +202,15 @@ gen7_emit_vfe_state(struct intel_batchbuffer *batch)
> >   	OUT_BATCH(0);
> >   	/* number of threads & urb entries */
> > -	OUT_BATCH(1 << 16 |
> > -		2 << 8);
> > +	OUT_BATCH(threads << 16 |
> > +		urb_entries << 8 |
> > +		mode << 2); /* GPGPU mode */
> 
> Mode is used to select between media and GPGPU, so the comment could be
> updated to something like:
> 
> /* GPGPU vs media mode */
>
I will change comment here.
> >   	OUT_BATCH(0);
> >   	/* urb entry size & curbe size */
> > -	OUT_BATCH(2 << 16 |	/* in 256 bits unit */
> > -		  2);		/* in 256 bits unit */
> > -
> > -	/* scoreboard */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -}
> > -
> > -void
> > -gen7_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (8 - 2));
> > -
> > -	/* scratch buffer */
> > -	OUT_BATCH(0);
> > -
> > -	/* number of threads & urb entries */
> > -	OUT_BATCH(1 << 16 | /* max num of threads */
> > -		  0 << 8 | /* num of URB entry */
> > -		  1 << 2); /* GPGPU mode */
> > -
> > -	OUT_BATCH(0);
> > -
> > -	/* urb entry size & curbe size */
> > -	OUT_BATCH(0 << 16 |	/* URB entry size in 256 bits unit */
> > -		  1);		/* CURBE entry size in 256 bits unit */
> > +	OUT_BATCH(urb_size << 16 |	/* in 256 bits unit */
> > +		  curbe_size);		/* in 256 bits unit */
> >   	/* scoreboard */
> >   	OUT_BATCH(0);
> > @@ -271,32 +248,14 @@ gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
> >   void
> >   gen7_emit_media_objects(struct intel_batchbuffer *batch,
> > -			unsigned int x, unsigned int y,
> > +			unsigned int xoffset, unsigned int yoffset,
> >   			unsigned int width, unsigned int height)
> >   {
> >   	int i, j;
> >   	for (i = 0; i < width / 16; i++) {
> >   		for (j = 0; j < height / 16; j++) {
> > -			OUT_BATCH(GEN7_MEDIA_OBJECT | (8 - 2));
> > -
> > -			/* interface descriptor offset */
> > -			OUT_BATCH(0);
> > -
> > -			/* without indirect data */
> > -			OUT_BATCH(0);
> > -			OUT_BATCH(0);
> > -
> > -			/* scoreboard */
> > -			OUT_BATCH(0);
> > -			OUT_BATCH(0);
> > -
> > -			/* inline data (xoffset, yoffset) */
> > -			OUT_BATCH(x + i * 16);
> > -			OUT_BATCH(y + j * 16);
> > -			if (AT_LEAST_GEN(batch->devid, 8) &&
> > -			    !IS_CHERRYVIEW(batch->devid))
> > -				gen8_emit_media_state_flush(batch);
> > +			gen_emit_media_object(batch, xoffset, yoffset);
> 
> This is wrong, you want to pass in x + i * 16 and y + j * 16 and not x and y
> as they are. Probably the reason why CI is failing.
> 
> >   		}
> >   	}
> >   }
CI is not failing because of this changes, but I will check this code.

> > @@ -503,32 +462,9 @@ gen8_emit_media_state_flush(struct intel_batchbuffer *batch)
> >   }
> >   void
> > -gen8_emit_vfe_state(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (9 - 2));
> > -
> > -	/* scratch buffer */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* number of threads & urb entries */
> > -	OUT_BATCH(1 << 16 |
> > -		2 << 8);
> > -
> > -	OUT_BATCH(0);
> > -
> > -	/* urb entry size & curbe size */
> > -	OUT_BATCH(2 << 16 |
> > -		2);
> > -
> > -	/* scoreboard */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -}
> > -
> > -void
> > -gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
> > +gen8_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
> > +		    uint32_t urb_entries, uint32_t urb_size,
> > +		    uint32_t curbe_size)
> >   {
> >   	OUT_BATCH(GEN7_MEDIA_VFE_STATE | (9 - 2));
> > @@ -537,36 +473,14 @@ gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
> >   	OUT_BATCH(0);
> >   	/* number of threads & urb entries */
> > -	OUT_BATCH(1 << 16 | 1 << 8);
> > -
> > -	OUT_BATCH(0);
> > -
> > -	/* urb entry size & curbe size */
> > -	OUT_BATCH(0 << 16 | 1);
> > -
> > -	/* scoreboard */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -}
> > -
> > -void
> > -gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch)
> > -{
> > -	OUT_BATCH(GEN8_MEDIA_VFE_STATE | (9 - 2));
> > -
> > -	/* scratch buffer */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > -
> > -	/* number of threads & urb entries */
> > -	OUT_BATCH(2 << 8);
> > +	OUT_BATCH(threads << 16 |
> > +		urb_entries << 8);
> >   	OUT_BATCH(0);
> >   	/* urb entry size & curbe size */
> > -	OUT_BATCH(2 << 16 |
> > -		2);
> > +	OUT_BATCH(urb_size << 16 |
> > +		curbe_size);
> >   	/* scoreboard */
> >   	OUT_BATCH(0);
> > @@ -635,7 +549,8 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> >   }
> >   void
> > -gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
> > +gen_emit_media_object(struct intel_batchbuffer *batch,
> > +		       unsigned int xoffset, unsigned int yoffset)
> >   {
> >   	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> > @@ -651,8 +566,8 @@ gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
> >   	OUT_BATCH(0);
> >   	/* inline data (xoffset, yoffset) */
> > -	OUT_BATCH(0);
> > -	OUT_BATCH(0);
> > +	OUT_BATCH(xoffset);
> > +	OUT_BATCH(yoffset);
> >   	if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
> >   		gen8_emit_media_state_flush(batch);
> >   }
> > diff --git a/lib/gpu_fill.h b/lib/gpu_fill.h
> > index abb3b4c3..3a800198 100644
> > --- a/lib/gpu_fill.h
> > +++ b/lib/gpu_fill.h
> > @@ -67,10 +67,9 @@ void
> >   gen7_emit_state_base_address(struct intel_batchbuffer *batch);
> >   void
> > -gen7_emit_vfe_state(struct intel_batchbuffer *batch);
> > -
> > -void
> > -gen7_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
> > +gen7_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
> > +		    uint32_t urb_entries, uint32_t urb_size,
> > +		    uint32_t curbe_size, uint32_t mode);
> >   void
> >   gen7_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer);
> > @@ -111,13 +110,9 @@ void
> >   gen8_emit_media_state_flush(struct intel_batchbuffer *batch);
> >   void
> > -gen8_emit_vfe_state(struct intel_batchbuffer *batch);
> > -
> > -void
> > -gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch);
> > -
> > -void
> > -gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch);
> > +gen8_emit_vfe_state(struct intel_batchbuffer *batch, uint32_t threads,
> > +		    uint32_t urb_entries, uint32_t urb_size,
> > +		    uint32_t curbe_size);
> >   void
> >   gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> > @@ -125,7 +120,8 @@ gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> >   		     unsigned int width, unsigned int height);
> >   void
> > -gen8_emit_media_objects_spin(struct intel_batchbuffer *batch);
> > +gen_emit_media_object(struct intel_batchbuffer *batch, unsigned int xoffset,
> > +		  unsigned int yoffset);
> >   void
> >   gen9_emit_state_base_address(struct intel_batchbuffer *batch);
> > diff --git a/lib/media_fill_gen7.c b/lib/media_fill_gen7.c
> > index 3dc5617e..02dff04b 100644
> > --- a/lib/media_fill_gen7.c
> > +++ b/lib/media_fill_gen7.c
> > @@ -54,6 +54,13 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch,
> >   	uint32_t curbe_buffer, interface_descriptor;
> >   	uint32_t batch_end;
> > +	/* Parameters for gen7_emit_vfe_state. */
> > +	uint32_t threads = 1;
> > +	uint32_t urb_entries = 2;
> > +	uint32_t urb_size = 2;
> > +	uint32_t curbe_size = 2;
> > +	uint32_t mode = 0;
> > +
> 
> Same as the above, we can use defines for these (and in this case there are
> no differences in values across gens, apart from gen7 having to select the
> mode).
>
I will change it to #defines.
> >   	intel_batchbuffer_flush(batch);
> >   	/* setup states */
> > @@ -69,7 +76,8 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
> >   	gen7_emit_state_base_address(batch);
> > -	gen7_emit_vfe_state(batch);
> > +	gen7_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size,
> > +			    mode);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> > diff --git a/lib/media_fill_gen8.c b/lib/media_fill_gen8.c
> > index 63fe72eb..3103b5e7 100644
> > --- a/lib/media_fill_gen8.c
> > +++ b/lib/media_fill_gen8.c
> > @@ -57,6 +57,12 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch,
> >   	uint32_t curbe_buffer, interface_descriptor;
> >   	uint32_t batch_end;
> > +	/* Parameters for gen8_emit_vfe_state. */
> > +	uint32_t threads = 1;
> > +	uint32_t urb_entries = 2;
> > +	uint32_t urb_size = 2;
> > +	uint32_t curbe_size = 2;
> > +
> >   	intel_batchbuffer_flush(batch);
> >   	/* setup states */
> > @@ -72,7 +78,7 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
> >   	gen8_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state(batch);
> > +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> > diff --git a/lib/media_fill_gen9.c b/lib/media_fill_gen9.c
> > index 78e892f2..d783d0bd 100644
> > --- a/lib/media_fill_gen9.c
> > +++ b/lib/media_fill_gen9.c
> > @@ -54,6 +54,12 @@ gen9_media_fillfunc(struct intel_batchbuffer *batch,
> >   	uint32_t curbe_buffer, interface_descriptor;
> >   	uint32_t batch_end;
> > +	/* Parameters for gen9_emit_vfe_state. */
> > +	uint32_t threads = 1;
> > +	uint32_t urb_entries = 2;
> > +	uint32_t urb_size = 2;
> > +	uint32_t curbe_size = 2;
> > +
> >   	intel_batchbuffer_flush(batch);
> >   	/* setup states */
> > @@ -74,7 +80,7 @@ gen9_media_fillfunc(struct intel_batchbuffer *batch,
> >   			GEN9_FORCE_MEDIA_AWAKE_MASK);
> >   	gen9_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state(batch);
> > +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> > diff --git a/lib/media_spin.c b/lib/media_spin.c
> > index b323550a..2a8a6bdb 100644
> > --- a/lib/media_spin.c
> > +++ b/lib/media_spin.c
> > @@ -68,6 +68,11 @@ static const uint32_t spin_kernel[][4] = {
> >   #define BATCH_STATE_SPLIT 2048
> > +/* Offsets needed in gen_emit_media_object. In media_spin library this  * values do not matter.
> 
> Missing a newline here
> 
> > + */
> > +#define xoffset 0
> > +#define yoffset 0
> > +
> >   void
> >   gen8_media_spinfunc(struct intel_batchbuffer *batch,
> >   		    struct igt_buf *dst, uint32_t spins)
> > @@ -75,6 +80,12 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
> >   	uint32_t curbe_buffer, interface_descriptor;
> >   	uint32_t batch_end;
> > +	/* Parameters for gen8_emit_vfe_state. */
> > +	uint32_t threads = 2;
> > +	uint32_t urb_entries = 0;
> > +	uint32_t urb_size = 2;
> > +	uint32_t curbe_size = 2;
> > +
> >   	intel_batchbuffer_flush_with_context(batch, NULL);
> >   	/* setup states */
> > @@ -90,13 +101,13 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
> >   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
> >   	gen8_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state_spin(batch);
> > +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> >   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> > -	gen8_emit_media_objects_spin(batch);
> > +	gen_emit_media_object(batch, xoffset, yoffset);
> >   	OUT_BATCH(MI_BATCH_BUFFER_END);
> > @@ -114,6 +125,12 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
> >   	uint32_t curbe_buffer, interface_descriptor;
> >   	uint32_t batch_end;
> > +	/* Parameters for gen9_emit_vfe_state. */
> > +	uint32_t threads = 1;
> > +	uint32_t urb_entries = 1;
> > +	uint32_t urb_size = 0;
> > +	uint32_t curbe_size = 1;
> > +
> 
> both the gen8 and gen9 media spin called into gen8_emit_vfe_state_spin so
> they should have the same parameters. Any reason to change them for gen9? If
> this was just a cut & paste mistake and the parameters are indeed going to
> be the same I suggest switching to defines like in other places.
>
I will check that :)
> Daniele
> 
Thanks for the feedback Daniele :)
Kasia
> >   	intel_batchbuffer_flush_with_context(batch, NULL);
> >   	/* setup states */
> > @@ -134,13 +151,13 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
> >   			GEN9_FORCE_MEDIA_AWAKE_MASK);
> >   	gen9_emit_state_base_address(batch);
> > -	gen8_emit_vfe_state_spin(batch);
> > +	gen8_emit_vfe_state(batch, threads, urb_entries, urb_size, curbe_size);
> >   	gen7_emit_curbe_load(batch, curbe_buffer);
> >   	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> > -	gen8_emit_media_objects_spin(batch);
> > +	gen_emit_media_object(batch, xoffset, yoffset);
> >   	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA |
> >   			GEN9_FORCE_MEDIA_AWAKE_DISABLE |
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v2 2/5] lib/media_spin: Remove gen8lp_media_spin function
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 2/5] lib/media_spin: Remove gen8lp_media_spin function Katarzyna Dec
@ 2018-05-02 15:22   ` Daniele Ceraolo Spurio
  2018-05-02 15:41     ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-05-02 15:22 UTC (permalink / raw)
  To: Katarzyna Dec, igt-dev



On 27/04/18 03:03, Katarzyna Dec wrote:
> Gen8lp function are duplicates of Gen8 with difference in
> emit_media_objects - we cannot perform media_state_flush on CHT.
> Similar changes were done previously for gpgpu_fill and media_fill.
> 
> v2: replaced IS_BRW and IS_CHT with IS_GEN8
> 
> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   lib/gpu_fill.c          | 24 ++----------------------
>   lib/intel_batchbuffer.c |  4 +---
>   lib/media_spin.c        | 38 --------------------------------------
>   lib/media_spin.h        |  3 ---
>   4 files changed, 3 insertions(+), 66 deletions(-)
> 
> diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
> index f5fc61bb..8dab39df 100644
> --- a/lib/gpu_fill.c
> +++ b/lib/gpu_fill.c
> @@ -642,28 +642,8 @@ gen8_emit_media_objects_spin(struct intel_batchbuffer *batch)
>   	/* inline data (xoffset, yoffset) */
>   	OUT_BATCH(0);
>   	OUT_BATCH(0);
> -	gen8_emit_media_state_flush(batch);
> -}
> -
> -void
> -gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch)
> -{
> -	OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
> -
> -	/* interface descriptor offset */
> -	OUT_BATCH(0);
> -
> -	/* without indirect data */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* scoreboard */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> -
> -	/* inline data (xoffset, yoffset) */
> -	OUT_BATCH(0);
> -	OUT_BATCH(0);
> +	if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
> +		gen8_emit_media_state_flush(batch);

As of this patch the check on the gen is redundant because the function 
is gen8+ only. However, if we pull it out we need to re-add it in the 
next patch when we update the function to re-use it from 
gen7_emit_media_objects as well. Either way:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Daniele

>   }
>   
>   void
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index f87e6bed..cf7da44d 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -906,10 +906,8 @@ igt_media_spinfunc_t igt_get_media_spinfunc(int devid)
>   
>   	if (IS_GEN9(devid))
>   		spin = gen9_media_spinfunc;
> -	else if (IS_BROADWELL(devid))
> +	else if (IS_GEN8(devid))
>   		spin = gen8_media_spinfunc;
> -	else if (IS_CHERRYVIEW(devid))
> -		spin = gen8lp_media_spinfunc;
>   
>   	return spin;
>   }
> diff --git a/lib/media_spin.c b/lib/media_spin.c
> index 16ea8483..b4414bee 100644
> --- a/lib/media_spin.c
> +++ b/lib/media_spin.c
> @@ -106,44 +106,6 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
>   	intel_batchbuffer_reset(batch);
>   }
>   
> -void
> -gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
> -		      struct igt_buf *dst, uint32_t spins)
> -{
> -	uint32_t curbe_buffer, interface_descriptor;
> -	uint32_t batch_end;
> -
> -	intel_batchbuffer_flush_with_context(batch, NULL);
> -
> -	/* setup states */
> -	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
> -
> -	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> -	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
> -	igt_assert(batch->ptr < &batch->buffer[4095]);
> -
> -	/* media pipeline */
> -	batch->ptr = batch->buffer;
> -	OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
> -	gen8_emit_state_base_address(batch);
> -
> -	gen8_emit_vfe_state_spin(batch);
> -
> -	gen7_emit_curbe_load(batch, curbe_buffer);
> -
> -	gen7_emit_interface_descriptor_load(batch, interface_descriptor);
> -
> -	gen8lp_emit_media_objects_spin(batch);
> -
> -	OUT_BATCH(MI_BATCH_BUFFER_END);
> -
> -	batch_end = intel_batchbuffer_align(batch, 8);
> -	igt_assert(batch_end < BATCH_STATE_SPLIT);
> -
> -	gen7_render_flush(batch, batch_end);
> -	intel_batchbuffer_reset(batch);
> -}
> -
>   void
>   gen9_media_spinfunc(struct intel_batchbuffer *batch,
>   		    struct igt_buf *dst, uint32_t spins)
> diff --git a/lib/media_spin.h b/lib/media_spin.h
> index 8bc48291..57d8c2e2 100644
> --- a/lib/media_spin.h
> +++ b/lib/media_spin.h
> @@ -30,9 +30,6 @@
>   void gen8_media_spinfunc(struct intel_batchbuffer *batch,
>   			 struct igt_buf *dst, uint32_t spins);
>   
> -void gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
> -			   struct igt_buf *dst, uint32_t spins);
> -
>   void gen9_media_spinfunc(struct intel_batchbuffer *batch,
>   			 struct igt_buf *dst, uint32_t spins);
>   
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v2 5/5] lib: Rename library from gpu_fill to gpu_helpers
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 5/5] lib: Rename library from gpu_fill to gpu_helpers Katarzyna Dec
@ 2018-05-02 15:28   ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-05-02 15:28 UTC (permalink / raw)
  To: Katarzyna Dec, igt-dev



On 27/04/18 03:03, Katarzyna Dec wrote:
> After refactoring media_spin library - gpu_fill contains helper
> functions for render copy, *_fill functions and media_spin.
> Let's rename this library to gpu_helpers. This name will be more
> general.
> 

My personal preference would be to have something in the name that makes 
it clear that we have commands-related functions here and not generic 
gpu-related helpers. Maybe "gpu_cmds_helpers"? Anyway this is just a 
suggestion not a binding comment.

Daniele

> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v2 2/5] lib/media_spin: Remove gen8lp_media_spin function
  2018-05-02 15:22   ` Daniele Ceraolo Spurio
@ 2018-05-02 15:41     ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-05-02 15:41 UTC (permalink / raw)
  To: Katarzyna Dec, igt-dev


On 02/05/18 08:22, Daniele Ceraolo Spurio wrote:
> 
> 
> On 27/04/18 03:03, Katarzyna Dec wrote:
>> Gen8lp function are duplicates of Gen8 with difference in
>> emit_media_objects - we cannot perform media_state_flush on CHT.
>> Similar changes were done previously for gpgpu_fill and media_fill.
>>
>> v2: replaced IS_BRW and IS_CHT with IS_GEN8
>>
>> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
>> Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
>> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> ---
>>   lib/gpu_fill.c          | 24 ++----------------------
>>   lib/intel_batchbuffer.c |  4 +---
>>   lib/media_spin.c        | 38 --------------------------------------
>>   lib/media_spin.h        |  3 ---
>>   4 files changed, 3 insertions(+), 66 deletions(-)
>>
>> diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
>> index f5fc61bb..8dab39df 100644
>> --- a/lib/gpu_fill.c
>> +++ b/lib/gpu_fill.c
>> @@ -642,28 +642,8 @@ gen8_emit_media_objects_spin(struct 
>> intel_batchbuffer *batch)
>>       /* inline data (xoffset, yoffset) */
>>       OUT_BATCH(0);
>>       OUT_BATCH(0);
>> -    gen8_emit_media_state_flush(batch);
>> -}
>> -
>> -void
>> -gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch)
>> -{
>> -    OUT_BATCH(GEN8_MEDIA_OBJECT | (8 - 2));
>> -
>> -    /* interface descriptor offset */
>> -    OUT_BATCH(0);
>> -
>> -    /* without indirect data */
>> -    OUT_BATCH(0);
>> -    OUT_BATCH(0);
>> -
>> -    /* scoreboard */
>> -    OUT_BATCH(0);
>> -    OUT_BATCH(0);
>> -
>> -    /* inline data (xoffset, yoffset) */
>> -    OUT_BATCH(0);
>> -    OUT_BATCH(0);
>> +    if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
>> +        gen8_emit_media_state_flush(batch);
> 
> As of this patch the check on the gen is redundant because the function 
> is gen8+ only. However, if we pull it out we need to re-add it in the 
> next patch when we update the function to re-use it from 
> gen7_emit_media_objects as well. Either way:
> 
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> 
> Daniele
> 

I hadn't realized that you hadn't removed gen8lp_emit_media_objects_spin 
for the header (noticed while reviewing patch 3). That needs to go in 
this patch for consistency. R-b stands with this fixed.

Thanks,
Daniele

>>   }
>>   void
>> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
>> index f87e6bed..cf7da44d 100644
>> --- a/lib/intel_batchbuffer.c
>> +++ b/lib/intel_batchbuffer.c
>> @@ -906,10 +906,8 @@ igt_media_spinfunc_t igt_get_media_spinfunc(int 
>> devid)
>>       if (IS_GEN9(devid))
>>           spin = gen9_media_spinfunc;
>> -    else if (IS_BROADWELL(devid))
>> +    else if (IS_GEN8(devid))
>>           spin = gen8_media_spinfunc;
>> -    else if (IS_CHERRYVIEW(devid))
>> -        spin = gen8lp_media_spinfunc;
>>       return spin;
>>   }
>> diff --git a/lib/media_spin.c b/lib/media_spin.c
>> index 16ea8483..b4414bee 100644
>> --- a/lib/media_spin.c
>> +++ b/lib/media_spin.c
>> @@ -106,44 +106,6 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
>>       intel_batchbuffer_reset(batch);
>>   }
>> -void
>> -gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
>> -              struct igt_buf *dst, uint32_t spins)
>> -{
>> -    uint32_t curbe_buffer, interface_descriptor;
>> -    uint32_t batch_end;
>> -
>> -    intel_batchbuffer_flush_with_context(batch, NULL);
>> -
>> -    /* setup states */
>> -    batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>> -
>> -    curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
>> -    interface_descriptor = gen8_fill_interface_descriptor(batch, dst, 
>> spin_kernel, sizeof(spin_kernel));
>> -    igt_assert(batch->ptr < &batch->buffer[4095]);
>> -
>> -    /* media pipeline */
>> -    batch->ptr = batch->buffer;
>> -    OUT_BATCH(GEN8_PIPELINE_SELECT | PIPELINE_SELECT_MEDIA);
>> -    gen8_emit_state_base_address(batch);
>> -
>> -    gen8_emit_vfe_state_spin(batch);
>> -
>> -    gen7_emit_curbe_load(batch, curbe_buffer);
>> -
>> -    gen7_emit_interface_descriptor_load(batch, interface_descriptor);
>> -
>> -    gen8lp_emit_media_objects_spin(batch);
>> -
>> -    OUT_BATCH(MI_BATCH_BUFFER_END);
>> -
>> -    batch_end = intel_batchbuffer_align(batch, 8);
>> -    igt_assert(batch_end < BATCH_STATE_SPLIT);
>> -
>> -    gen7_render_flush(batch, batch_end);
>> -    intel_batchbuffer_reset(batch);
>> -}
>> -
>>   void
>>   gen9_media_spinfunc(struct intel_batchbuffer *batch,
>>               struct igt_buf *dst, uint32_t spins)
>> diff --git a/lib/media_spin.h b/lib/media_spin.h
>> index 8bc48291..57d8c2e2 100644
>> --- a/lib/media_spin.h
>> +++ b/lib/media_spin.h
>> @@ -30,9 +30,6 @@
>>   void gen8_media_spinfunc(struct intel_batchbuffer *batch,
>>                struct igt_buf *dst, uint32_t spins);
>> -void gen8lp_media_spinfunc(struct intel_batchbuffer *batch,
>> -               struct igt_buf *dst, uint32_t spins);
>> -
>>   void gen9_media_spinfunc(struct intel_batchbuffer *batch,
>>                struct igt_buf *dst, uint32_t spins);
>>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

* Re: [igt-dev] [PATCH i-g-t v2 3/5] lib: Adjust media_spin and gpu_fill to our code style
  2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 3/5] lib: Adjust media_spin and gpu_fill to our code style Katarzyna Dec
@ 2018-05-02 15:42   ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 15+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-05-02 15:42 UTC (permalink / raw)
  To: Katarzyna Dec, igt-dev



On 27/04/18 03:03, Katarzyna Dec wrote:
> Let's adjust code to our coding style during refactoring
> media_spin code.
> 
> v2: fixed minor typos
> 
> Signed-off-by: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>
> ---
>   lib/gpgpu_fill.c | 15 +++++-------
>   lib/gpu_fill.c   | 72 +++++++++++++++++++++++++++++++++-----------------------
>   lib/gpu_fill.h   | 26 ++++++++++----------
>   lib/media_spin.c |  6 +++--
>   4 files changed, 66 insertions(+), 53 deletions(-)
> 
> diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> index 010dde06..52925a5c 100644
> --- a/lib/gpgpu_fill.c
> +++ b/lib/gpgpu_fill.c
> @@ -112,9 +112,8 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
>   	/*
> -	 * const buffer needs to fill for every thread, but as we have just 1 thread
> -	 * per every group, so need only one curbe data.
> -	 *
> +	 * const buffer needs to fill for every thread, but as we have just 1
> +	 * thread per every group, so need only one curbe data.
>   	 * For each thread, just use thread group ID for buffer offset.
>   	 */
>   	curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
> @@ -160,9 +159,8 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
>   	/*
> -	 * const buffer needs to fill for every thread, but as we have just 1 thread
> -	 * per every group, so need only one curbe data.
> -	 *
> +	 * const buffer needs to fill for every thread, but as we have just 1
> +	 * thread per every group, so need only one curbe data.
>   	 * For each thread, just use thread group ID for buffer offset.
>   	 */
>   	curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
> @@ -208,9 +206,8 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
>   	/*
> -	 * const buffer needs to fill for every thread, but as we have just 1 thread
> -	 * per every group, so need only one curbe data.
> -	 *
> +	 * const buffer needs to fill for every thread, but as we have just 1
> +	 * thread per every group, so need only one curbe data.
>   	 * For each thread, just use thread group ID for buffer offset.
>   	 */
>   	curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
> diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
> index 8dab39df..25c0c810 100644
> --- a/lib/gpu_fill.c
> +++ b/lib/gpu_fill.c
> @@ -132,8 +132,8 @@ gen7_fill_kernel(struct intel_batchbuffer *batch,
>   }
>   
>   uint32_t
> -gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,
> -			       const uint32_t kernel[][4], size_t size)
> +gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf
> +			       *dst, const uint32_t kernel[][4], size_t size)

This here looks quite weird with the type on one line and the variable 
name on the next line.

>   {
>   	struct gen7_interface_descriptor_data *idd;
>   	uint32_t offset;
> @@ -171,16 +171,19 @@ gen7_emit_state_base_address(struct intel_batchbuffer *batch)
>   	OUT_BATCH(0);
>   
>   	/* surface */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> +	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> +		  BASE_ADDRESS_MODIFY);
>   
>   	/* dynamic */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> +	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> +		  BASE_ADDRESS_MODIFY);
>   
>   	/* indirect */
>   	OUT_BATCH(0);
>   
>   	/* instruction */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> +	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> +		  BASE_ADDRESS_MODIFY);
>   
>   	/* general/dynamic/indirect/instruction access Bound */
>   	OUT_BATCH(0);
> @@ -204,8 +207,8 @@ gen7_emit_vfe_state(struct intel_batchbuffer *batch)
>   	OUT_BATCH(0);
>   
>   	/* urb entry size & curbe size */
> -	OUT_BATCH(2 << 16 | 	/* in 256 bits unit */
> -		2);		/* in 256 bits unit */
> +	OUT_BATCH(2 << 16 |	/* in 256 bits unit */
> +		  2);		/* in 256 bits unit */
>   
>   	/* scoreboard */
>   	OUT_BATCH(0);
> @@ -229,7 +232,7 @@ gen7_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
>   	OUT_BATCH(0);
>   
>   	/* urb entry size & curbe size */
> -	OUT_BATCH(0 << 16 | 	/* URB entry size in 256 bits unit */
> +	OUT_BATCH(0 << 16 |	/* URB entry size in 256 bits unit */
>   		  1);		/* CURBE entry size in 256 bits unit */
>   
>   	/* scoreboard */
> @@ -250,7 +253,8 @@ gen7_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer)
>   }
>   
>   void
> -gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch, uint32_t interface_descriptor)
> +gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
> +				    uint32_t interface_descriptor)
>   {
>   	OUT_BATCH(GEN7_MEDIA_INTERFACE_DESCRIPTOR_LOAD | (4 - 2));
>   	OUT_BATCH(0);
> @@ -259,14 +263,16 @@ gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch, uint32_t in
>   		OUT_BATCH(sizeof(struct gen7_interface_descriptor_data));
>   	else
>   		OUT_BATCH(sizeof(struct gen8_interface_descriptor_data));
> -	/* interface descriptor address, is relative to the dynamics base address */
> +	/* interface descriptor address, is relative to the dynamics base
> +	 * address
> +	 */
>   	OUT_BATCH(interface_descriptor);
>   }
>   
>   void
>   gen7_emit_media_objects(struct intel_batchbuffer *batch,
> -			unsigned x, unsigned y,
> -			unsigned width, unsigned height)
> +			unsigned int x, unsigned int y,
> +			unsigned int width, unsigned int height)
>   {
>   	int i, j;
>   
> @@ -288,7 +294,8 @@ gen7_emit_media_objects(struct intel_batchbuffer *batch,
>   			/* inline data (xoffset, yoffset) */
>   			OUT_BATCH(x + i * 16);
>   			OUT_BATCH(y + j * 16);
> -			if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
> +			if (AT_LEAST_GEN(batch->devid, 8) &&
> +			    !IS_CHERRYVIEW(batch->devid))
>   				gen8_emit_media_state_flush(batch);
>   		}
>   	}
> @@ -296,8 +303,8 @@ gen7_emit_media_objects(struct intel_batchbuffer *batch,
>   
>   void
>   gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> -		     unsigned x, unsigned y,
> -		     unsigned width, unsigned height)
> +		     unsigned int x, unsigned int y,
> +		     unsigned int width, unsigned int height)
>   {
>   	uint32_t x_dim, y_dim, tmp, right_mask;
>   
> @@ -399,9 +406,8 @@ gen8_fill_surface_state(struct intel_batchbuffer *batch,
>   	ss->ss8.base_addr = buf->bo->offset;
>   
>   	ret = drm_intel_bo_emit_reloc(batch->bo,
> -				intel_batchbuffer_subdata_offset(batch, ss) + 8 * 4,
> -				buf->bo, 0,
> -				read_domain, write_domain);
> +				intel_batchbuffer_subdata_offset(batch, ss) +
> +				8 * 4, buf->bo, 0, read_domain, write_domain);

I would keep the sum on a single line here for readability even if it is 
over 80 chars. In general going slightly over 80 chars is tolerated if 
there is a readability benefit.

>   	igt_assert(ret == 0);
>   
>   	ss->ss2.height = igt_buf_height(buf) - 1;
> @@ -417,7 +423,9 @@ gen8_fill_surface_state(struct intel_batchbuffer *batch,
>   }
>   
>   uint32_t
> -gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,  const uint32_t kernel[][4], size_t size)
> +gen8_fill_interface_descriptor(struct intel_batchbuffer *batch,
> +			       struct igt_buf *dst, const uint32_t kernel[][4],
> +			       size_t size)
>   {
>   	struct gen8_interface_descriptor_data *idd;
>   	uint32_t offset;
> @@ -464,15 +472,16 @@ gen8_emit_state_base_address(struct intel_batchbuffer *batch)
>   	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
>   
>   	/* dynamic */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
> -		0, BASE_ADDRESS_MODIFY);
> +	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER |
> +		  I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
>   
>   	/* indirect */
>   	OUT_BATCH(0);
>   	OUT_BATCH(0);
>   
>   	/* instruction */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> +	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> +		  BASE_ADDRESS_MODIFY);
>   
>   	/* general state buffer size */
>   	OUT_BATCH(0xfffff000 | 1);
> @@ -480,7 +489,9 @@ gen8_emit_state_base_address(struct intel_batchbuffer *batch)
>   	OUT_BATCH(1 << 12 | 1);
>   	/* indirect object buffer size */
>   	OUT_BATCH(0xfffff000 | 1);
> -	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
> +	/* instruction buffer size, must set modify enable bit, otherwise it may
> +	 * result in GPU hang
> +	 */
>   	OUT_BATCH(1 << 12 | 1);
>   }
>   
> @@ -565,8 +576,8 @@ gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch)
>   
>   void
>   gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> -		     unsigned x, unsigned y,
> -		     unsigned width, unsigned height)
> +		     unsigned int x, unsigned int y,
> +		     unsigned int width, unsigned int height)
>   {
>   	uint32_t x_dim, y_dim, tmp, right_mask;
>   
> @@ -662,15 +673,16 @@ gen9_emit_state_base_address(struct intel_batchbuffer *batch)
>   	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
>   
>   	/* dynamic */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
> -		0, BASE_ADDRESS_MODIFY);
> +	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER |
> +		  I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
>   


Here as well I would keep the OR operation on a single line (which could 
be the second one if you want to stay within 80 chars)

Daniele

>   	/* indirect */
>   	OUT_BATCH(0);
>   	OUT_BATCH(0);
>   
>   	/* instruction */
> -	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> +	OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> +		  BASE_ADDRESS_MODIFY);
>   
>   	/* general state buffer size */
>   	OUT_BATCH(0xfffff000 | 1);
> @@ -678,7 +690,9 @@ gen9_emit_state_base_address(struct intel_batchbuffer *batch)
>   	OUT_BATCH(1 << 12 | 1);
>   	/* indirect object buffer size */
>   	OUT_BATCH(0xfffff000 | 1);
> -	/* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
> +	/* intruction buffer size, must set modify enable bit, otherwise it may
> +	 * result in GPU hang
> +	 */
>   	OUT_BATCH(1 << 12 | 1);
>   
>   	/* Bindless surface state base address */
> diff --git a/lib/gpu_fill.h b/lib/gpu_fill.h
> index 5335fe3f..abb3b4c3 100644
> --- a/lib/gpu_fill.h
> +++ b/lib/gpu_fill.h
> @@ -60,8 +60,8 @@ gen7_fill_kernel(struct intel_batchbuffer *batch,
>   		size_t size);
>   
>   uint32_t
> -gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,
> -			       const uint32_t kernel[][4], size_t size);
> +gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf
> +			       *dst, const uint32_t kernel[][4], size_t size);
>   
>   void
>   gen7_emit_state_base_address(struct intel_batchbuffer *batch);
> @@ -76,17 +76,18 @@ void
>   gen7_emit_curbe_load(struct intel_batchbuffer *batch, uint32_t curbe_buffer);
>   
>   void
> -gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch, uint32_t interface_descriptor);
> +gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch,
> +				    uint32_t interface_descriptor);
>   
>   void
>   gen7_emit_media_objects(struct intel_batchbuffer *batch,
> -			unsigned x, unsigned y,
> -			unsigned width, unsigned height);
> +			unsigned int x, unsigned int y,
> +			unsigned int width, unsigned int height);
>   
>   void
>   gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> -		     unsigned x, unsigned y,
> -		     unsigned width, unsigned height);
> +		     unsigned int x, unsigned int y,
> +		     unsigned int width, unsigned int height);
>   
>   uint32_t
>   gen8_spin_curbe_buffer_data(struct intel_batchbuffer *batch,
> @@ -99,7 +100,9 @@ gen8_fill_surface_state(struct intel_batchbuffer *batch,
>   			int is_dst);
>   
>   uint32_t
> -gen8_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,  const uint32_t kernel[][4], size_t size);
> +gen8_fill_interface_descriptor(struct intel_batchbuffer *batch,
> +			       struct igt_buf *dst, const uint32_t kernel[][4],
> +			       size_t size);
>   
>   void
>   gen8_emit_state_base_address(struct intel_batchbuffer *batch);
> @@ -118,15 +121,12 @@ gen8_emit_vfe_state_spin(struct intel_batchbuffer *batch);
>   
>   void
>   gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> -		     unsigned x, unsigned y,
> -		     unsigned width, unsigned height);
> +		     unsigned int x, unsigned int y,
> +		     unsigned int width, unsigned int height);
>   
>   void
>   gen8_emit_media_objects_spin(struct intel_batchbuffer *batch);
>   
> -void
> -gen8lp_emit_media_objects_spin(struct intel_batchbuffer *batch);
> -
>   void
>   gen9_emit_state_base_address(struct intel_batchbuffer *batch);
>   
> diff --git a/lib/media_spin.c b/lib/media_spin.c
> index b4414bee..b323550a 100644
> --- a/lib/media_spin.c
> +++ b/lib/media_spin.c
> @@ -81,7 +81,8 @@ gen8_media_spinfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
>   	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> -	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
> +	interface_descriptor = gen8_fill_interface_descriptor(batch, dst,
> +					      spin_kernel, sizeof(spin_kernel));
>   	igt_assert(batch->ptr < &batch->buffer[4095]);
>   
>   	/* media pipeline */
> @@ -119,7 +120,8 @@ gen9_media_spinfunc(struct intel_batchbuffer *batch,
>   	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
>   	curbe_buffer = gen8_spin_curbe_buffer_data(batch, spins);
> -	interface_descriptor = gen8_fill_interface_descriptor(batch, dst, spin_kernel, sizeof(spin_kernel));
> +	interface_descriptor = gen8_fill_interface_descriptor(batch, dst,
> +					      spin_kernel, sizeof(spin_kernel));
>   	igt_assert(batch->ptr < &batch->buffer[4095]);
>   
>   	/* media pipeline */
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

end of thread, other threads:[~2018-05-02 15:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-27 10:03 [igt-dev] [PATCH i-g-t v2 1/5] lib/media_spin: Move helper functions to gpu_fill library Katarzyna Dec
2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 2/5] lib/media_spin: Remove gen8lp_media_spin function Katarzyna Dec
2018-05-02 15:22   ` Daniele Ceraolo Spurio
2018-05-02 15:41     ` Daniele Ceraolo Spurio
2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 3/5] lib: Adjust media_spin and gpu_fill to our code style Katarzyna Dec
2018-05-02 15:42   ` Daniele Ceraolo Spurio
2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 4/5] lib/gpu_fill: Further code unification in gpu_fill Katarzyna Dec
2018-04-27 17:19   ` Daniele Ceraolo Spurio
2018-05-02 11:27     ` Katarzyna Dec
2018-04-27 10:03 ` [igt-dev] [PATCH i-g-t v2 5/5] lib: Rename library from gpu_fill to gpu_helpers Katarzyna Dec
2018-05-02 15:28   ` Daniele Ceraolo Spurio
2018-04-27 11:30 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/5] lib/media_spin: Move helper functions to gpu_fill library Patchwork
2018-04-27 14:30 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2018-04-27 17:47 ` [igt-dev] [PATCH i-g-t v2 1/5] " Daniele Ceraolo Spurio
2018-05-02 11:21   ` Katarzyna Dec

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