[PATCH 0/4] fix GFX8 dmaframe scheme (v2)

[PATCH 0/4] fix GFX8 dmaframe scheme (v2)

[Monk Liu]

rebase patches on latest staging04.6

Monk Liu (4):
  drm/amdgpu:add switch buffer to end of CS (v2)
  drm/amdgpu:new method to sync ce&de
  drm/amdgpu:change job->ctx field name
  drm/amdgpu:implement CONTEXT_CONTROL

 drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 13 ++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 14 +++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 28 ++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 63 +++++++++++++++++++++++----------
 5 files changed, 87 insertions(+), 33 deletions(-)

-- 
1.9.1

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

[PATCH 1/4] drm/amdgpu:add switch buffer to end of CS (v2)

[Monk Liu]

sync switch buffer scheme with windows kmd for gfx v8,
step1: append a switch_buffer to the end of CS

v2:rebase on latest staging

Change-Id: Ief8539b2ad91ccb38b9adbfb54e27d8282f3a3bd
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 2 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 7 +++++++
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 545789f..7518dab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -320,6 +320,7 @@ struct amdgpu_ring_funcs {
 	/* note usage for clock and power gating */
 	void (*begin_use)(struct amdgpu_ring *ring);
 	void (*end_use)(struct amdgpu_ring *ring);
+	void (*emit_switch_buffer) (struct amdgpu_ring *ring);
 };
 
 /*
@@ -2262,6 +2263,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
 #define amdgpu_ring_emit_gds_switch(r, v, db, ds, wb, ws, ab, as) (r)->funcs->emit_gds_switch((r), (v), (db), (ds), (wb), (ws), (ab), (as))
 #define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
 #define amdgpu_ring_emit_hdp_invalidate(r) (r)->funcs->emit_hdp_invalidate((r))
+#define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
 #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
 #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
 #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index f5810f7..11f2fba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -210,6 +210,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 		amdgpu_ring_patch_cond_exec(ring, patch_offset);
 
 	ring->current_ctx = ctx;
+	if (ring->funcs->emit_switch_buffer)
+		amdgpu_ring_emit_switch_buffer(ring);
 	amdgpu_ring_commit(ring);
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 6e01392..c14ba70 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6091,6 +6091,12 @@ static void gfx_v8_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
 	amdgpu_ring_write(ring, upper_32_bits(seq));
 }
 
+static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
+{
+	amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
+	amdgpu_ring_write(ring, 0);
+}
+
 static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
 						 enum amdgpu_interrupt_state state)
 {
@@ -6272,6 +6278,7 @@ static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_gfx = {
 	.test_ib = gfx_v8_0_ring_test_ib,
 	.insert_nop = amdgpu_ring_insert_nop,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
+	.emit_switch_buffer = gfx_v8_ring_emit_sb,
 };
 
 static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_compute = {
-- 
1.9.1

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

[PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Monk Liu]

sync switch buffer scheme with windows kmd for gfx v8,
step2:
Insert 128NOP after&before VM flush to prevent CE vm fault.

Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index c14ba70..fbc57a7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -5915,12 +5915,6 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 {
 	u32 header, control = 0;
 
-	/* insert SWITCH_BUFFER packet before first IB in the ring frame */
-	if (ctx_switch) {
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-	}
-
 	if (ib->flags & AMDGPU_IB_FLAG_CE)
 		header = PACKET3(PACKET3_INDIRECT_BUFFER_CONST, 2);
 	else
@@ -5990,14 +5984,6 @@ static void gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, seq);
 	amdgpu_ring_write(ring, 0xffffffff);
 	amdgpu_ring_write(ring, 4); /* poll interval */
-
-	if (usepfp) {
-		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-	}
 }
 
 static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
@@ -6005,6 +5991,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
 {
 	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
 
+	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE finish CEIB */
+	amdgpu_ring_insert_nop(ring, 128);
+
 	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
 	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
 				 WRITE_DATA_DST_SEL(0)) |
@@ -6044,11 +6033,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
 		/* sync PFP to ME, otherwise we might get invalid PFP reads */
 		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
 		amdgpu_ring_write(ring, 0x0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
 	}
+
+	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush finish */
+	amdgpu_ring_insert_nop(ring, 128);
 }
 
 static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring *ring)
-- 
1.9.1

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

[PATCH 3/4] drm/amdgpu:change job->ctx field name

[Monk Liu]

job->ctx actually is a fence_context of the entity
it belongs to, naming it as ctx is too vague, and
we'll need add amdgpu_ctx into the job structure
later.

Change-Id: I71e73912e26372639ea44a18d9e36d58d936f19a
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 12 ++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7518dab..1254410 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1239,7 +1239,7 @@ struct amdgpu_job {
 	struct fence		*fence; /* the hw fence */
 	uint32_t		num_ibs;
 	void			*owner;
-	uint64_t		ctx;
+	uint64_t		fence_ctx; /* the fence_context this job uses */
 	bool                    vm_needs_flush;
 	unsigned		vm_id;
 	uint64_t		vm_pd_addr;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index b0e11ca..2d4e005 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -931,7 +931,7 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	}
 
 	job->owner = p->filp;
-	job->ctx = entity->fence_context;
+	job->fence_ctx = entity->fence_context;
 	p->fence = fence_get(&job->base.s_fence->finished);
 	cs->out.handle = amdgpu_ctx_add_fence(p->ctx, ring, p->fence);
 	job->uf_sequence = cs->out.handle;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 11f2fba..04263f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -124,7 +124,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	bool skip_preamble, need_ctx_switch;
 	unsigned patch_offset = ~0;
 	struct amdgpu_vm *vm;
-	uint64_t ctx;
+	uint64_t fence_ctx;
 
 	unsigned i;
 	int r = 0;
@@ -135,10 +135,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	/* ring tests don't use a job */
 	if (job) {
 		vm = job->vm;
-		ctx = job->ctx;
+		fence_ctx = job->fence_ctx;
 	} else {
 		vm = NULL;
-		ctx = 0;
+		fence_ctx = 0;
 	}
 
 	if (!ring->ready) {
@@ -174,8 +174,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	/* always set cond_exec_polling to CONTINUE */
 	*ring->cond_exe_cpu_addr = 1;
 
-	skip_preamble = ring->current_ctx == ctx;
-	need_ctx_switch = ring->current_ctx != ctx;
+	skip_preamble = ring->current_ctx == fence_ctx;
+	need_ctx_switch = ring->current_ctx != fence_ctx;
 	for (i = 0; i < num_ibs; ++i) {
 		ib = &ibs[i];
 
@@ -209,7 +209,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	if (patch_offset != ~0 && ring->funcs->patch_cond_exec)
 		amdgpu_ring_patch_cond_exec(ring, patch_offset);
 
-	ring->current_ctx = ctx;
+	ring->current_ctx = fence_ctx;
 	if (ring->funcs->emit_switch_buffer)
 		amdgpu_ring_emit_switch_buffer(ring);
 	amdgpu_ring_commit(ring);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 6674d40..ac8d401 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -124,7 +124,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct amdgpu_ring *ring,
 		return r;
 
 	job->owner = owner;
-	job->ctx = entity->fence_context;
+	job->fence_ctx = entity->fence_context;
 	*f = fence_get(&job->base.s_fence->finished);
 	amdgpu_job_free_resources(job);
 	amd_sched_entity_push_job(&job->base);
-- 
1.9.1

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

[PATCH 4/4] drm/amdgpu:implement CONTEXT_CONTROL

[Monk Liu]

use CONTEXT_CONTROL package to dynamically skip
preamble IB and other load_xxx command in sequence.

Change-Id: I4b87ca84ea8c11ba4f7fb4c0e8a5be537ccde851
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |  9 +++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 12 ++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 16 +++++++++-------
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 1254410..0de5f08 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -321,6 +321,7 @@ struct amdgpu_ring_funcs {
 	void (*begin_use)(struct amdgpu_ring *ring);
 	void (*end_use)(struct amdgpu_ring *ring);
 	void (*emit_switch_buffer) (struct amdgpu_ring *ring);
+	void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
 };
 
 /*
@@ -965,6 +966,7 @@ struct amdgpu_ctx {
 	spinlock_t		ring_lock;
 	struct fence            **fences;
 	struct amdgpu_ctx_ring	rings[AMDGPU_MAX_RINGS];
+	bool preamble_presented;
 };
 
 struct amdgpu_ctx_mgr {
@@ -1227,8 +1229,13 @@ struct amdgpu_cs_parser {
 
 	/* user fence */
 	struct amdgpu_bo_list_entry	uf_entry;
+	bool preamble_present; /* True means this command submit involves a preamble IB */
 };
 
+#define PREAMBLE_IB_PRESENT 		(1 << 0) /* bit set means command submit involves a preamble IB */
+#define PREAMBLE_IB_PRESENT_FIRST	(1 << 1) /* bit set means preamble IB is first presented in belonging context */
+#define HAVE_CTX_SWITCH		(1 << 2) /* bit set means context switch occured */
+
 struct amdgpu_job {
 	struct amd_sched_job    base;
 	struct amdgpu_device	*adev;
@@ -1237,6 +1244,7 @@ struct amdgpu_job {
 	struct amdgpu_sync	sync;
 	struct amdgpu_ib	*ibs;
 	struct fence		*fence; /* the hw fence */
+	uint32_t		preamble_status;
 	uint32_t		num_ibs;
 	void			*owner;
 	uint64_t		fence_ctx; /* the fence_context this job uses */
@@ -2264,6 +2272,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
 #define amdgpu_ring_emit_hdp_flush(r) (r)->funcs->emit_hdp_flush((r))
 #define amdgpu_ring_emit_hdp_invalidate(r) (r)->funcs->emit_hdp_invalidate((r))
 #define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
+#define amdgpu_ring_emit_cntxcntl(r, d) (r)->funcs->emit_cntxcntl((r), (d))
 #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
 #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
 #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 2d4e005..6d8c050 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -792,6 +792,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
 		if (r)
 			return r;
 
+		if (ib->flags & AMDGPU_IB_FLAG_PREAMBLE)
+			parser->preamble_present = true;
+
 		if (parser->job->ring && parser->job->ring != ring)
 			return -EINVAL;
 
@@ -930,6 +933,12 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 		return r;
 	}
 
+	if (p->preamble_present) {
+		job->preamble_status |= PREAMBLE_IB_PRESENT;
+		if (!p->ctx->preamble_presented)
+			job->preamble_status |= PREAMBLE_IB_PRESENT_FIRST;
+	}
+
 	job->owner = p->filp;
 	job->fence_ctx = entity->fence_context;
 	p->fence = fence_get(&job->base.s_fence->finished);
@@ -940,6 +949,9 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p,
 	trace_amdgpu_cs_ioctl(job);
 	amd_sched_entity_push_job(&job->base);
 
+	if (p->preamble_present)
+		p->ctx->preamble_presented = true;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 04263f0..b12b5ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -121,10 +121,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 {
 	struct amdgpu_device *adev = ring->adev;
 	struct amdgpu_ib *ib = &ibs[0];
-	bool skip_preamble, need_ctx_switch;
+	bool need_ctx_switch;
 	unsigned patch_offset = ~0;
 	struct amdgpu_vm *vm;
 	uint64_t fence_ctx;
+	uint32_t status = 0;
 
 	unsigned i;
 	int r = 0;
@@ -174,15 +175,16 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 	/* always set cond_exec_polling to CONTINUE */
 	*ring->cond_exe_cpu_addr = 1;
 
-	skip_preamble = ring->current_ctx == fence_ctx;
 	need_ctx_switch = ring->current_ctx != fence_ctx;
+	if (job && ring->funcs->emit_cntxcntl) {
+		if (need_ctx_switch)
+			status |= HAVE_CTX_SWITCH;
+		status |= job->preamble_status;
+		amdgpu_ring_emit_cntxcntl(ring, status);
+	}
+
 	for (i = 0; i < num_ibs; ++i) {
 		ib = &ibs[i];
-
-		/* drop preamble IBs if we don't have a context switch */
-		if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) && skip_preamble)
-			continue;
-
 		amdgpu_ring_emit_ib(ring, ib, job ? job->vm_id : 0,
 				    need_ctx_switch);
 		need_ctx_switch = false;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index fbc57a7..5cf53d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6085,6 +6085,36 @@ static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, 0);
 }
 
+static void gfx_v8_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
+{
+	uint32_t dw2 = 0;
+
+	dw2 |= 0x80000000; /* set load_enable otherwise this package is just NOPs */
+	if (flags & HAVE_CTX_SWITCH) {
+		/* set load_global_config & load_global_uconfig */
+		dw2 |= 0x8001;
+		/* set load_cs_sh_regs */
+		dw2 |= 0x01000000;
+		/* set load_per_context_state & load_gfx_sh_regs for GFX */
+		if (ring->type != AMDGPU_RING_TYPE_COMPUTE)
+			dw2 |= 0x10002;
+
+		/* set load_ce_ram if preamble presented */
+		if (PREAMBLE_IB_PRESENT & flags)
+			dw2 |= 0x10000000;
+	} else {
+		/* still load_ce_ram if this is the first time preamble presented
+		 * although there is no context switch happens.
+		 */
+		if (PREAMBLE_IB_PRESENT_FIRST & flags)
+			dw2 |= 0x10000000;
+	}
+
+	amdgpu_ring_write(ring, PACKET3(PACKET3_CONTEXT_CONTROL, 1));
+	amdgpu_ring_write(ring, dw2);
+	amdgpu_ring_write(ring, 0);
+}
+
 static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
 						 enum amdgpu_interrupt_state state)
 {
@@ -6267,6 +6297,7 @@ static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_gfx = {
 	.insert_nop = amdgpu_ring_insert_nop,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
 	.emit_switch_buffer = gfx_v8_ring_emit_sb,
+	.emit_cntxcntl = gfx_v8_ring_emit_cntxcntl,
 };
 
 static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_compute = {
@@ -6285,6 +6316,7 @@ static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_compute = {
 	.test_ib = gfx_v8_0_ring_test_ib,
 	.insert_nop = amdgpu_ring_insert_nop,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
+	.emit_cntxcntl = gfx_v8_ring_emit_cntxcntl,
 };
 
 static void gfx_v8_0_set_ring_funcs(struct amdgpu_device *adev)
-- 
1.9.1

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

Re: [PATCH 0/4] fix GFX8 dmaframe scheme (v2)

[Christian König]

Patch #1-#3 are Reviewed-by: Christian König <christian.koenig@amd.com>.

For patch #4 I still need to take a closer look, but it look good in 
general as well.

Regards,
Christian.

Am 29.08.2016 um 10:55 schrieb Monk Liu:
> rebase patches on latest staging04.6
>
> Monk Liu (4):
>    drm/amdgpu:add switch buffer to end of CS (v2)
>    drm/amdgpu:new method to sync ce&de
>    drm/amdgpu:change job->ctx field name
>    drm/amdgpu:implement CONTEXT_CONTROL
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 13 ++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 14 +++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 28 ++++++++-------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 63 +++++++++++++++++++++++----------
>   5 files changed, 87 insertions(+), 33 deletions(-)
>

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

[PATCH 0/4] fix GFX8 dma frame scheme

[Monk Liu]

Sync gfx8.0 DMAframe to windows scheme:
1)only one SWITCH_BUFFER to the end of CS
2)insert 128 nop before&after VM flushy to prevent CE vm fault
3)use CONTEXT_CONTROL to dynamically control preamble IB skipping

Monk Liu (4):
  drm/amdgpu:add switch buffer to end of CS
  drm/amdgpu:new method to sync ce&de
  drm/amdgpu:change job->ctx field name
  drm/amdgpu:implement CONTEXT_CONTROL

 drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 13 ++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 14 +++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 28 ++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 63 +++++++++++++++++++++++----------
 5 files changed, 87 insertions(+), 33 deletions(-)

-- 
1.9.1

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

[PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Monk Liu]

sync switch buffer scheme with windows kmd for gfx v8,
step2:
Insert 128NOP after&before VM flush to prevent CE vm fault.

Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 6d4d07f..ce1e616 100755
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -5936,12 +5936,6 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 {
 	u32 header, control = 0;
 
-	/* insert SWITCH_BUFFER packet before first IB in the ring frame */
-	if (ctx_switch) {
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-	}
-
 	if (ib->flags & AMDGPU_IB_FLAG_CE)
 		header = PACKET3(PACKET3_INDIRECT_BUFFER_CONST, 2);
 	else
@@ -6011,14 +6005,6 @@ static void gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, seq);
 	amdgpu_ring_write(ring, 0xffffffff);
 	amdgpu_ring_write(ring, 4); /* poll interval */
-
-	if (usepfp) {
-		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-	}
 }
 
 static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
@@ -6026,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
 {
 	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
 
+	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE finish CEIB */
+	amdgpu_ring_insert_nop(ring, 128);
+
 	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
 	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
 				 WRITE_DATA_DST_SEL(0)) |
@@ -6065,11 +6054,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
 		/* sync PFP to ME, otherwise we might get invalid PFP reads */
 		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
 		amdgpu_ring_write(ring, 0x0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
 	}
+
+	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush finish */
+	amdgpu_ring_insert_nop(ring, 128);
 }
 
 static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring *ring)
-- 
1.9.1

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

[PATCH 1/4] drm/amdgpu:add switch buffer to end of CS

[Monk Liu]

sync switch buffer scheme with windows kmd for gfx v8,
Now always and only insert one switch buffer to the
end of CS.

Change-Id: Ief8539b2ad91ccb38b9adbfb54e27d8282f3a3bd
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c |  2 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 13 +++++++------
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index cb0098a..a935831 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -338,6 +338,7 @@ struct amdgpu_ring_funcs {
 	void (*end_use)(struct amdgpu_ring *ring);
 	void (*emit_wreg) (struct amdgpu_ring *ring, uint32_t offset, uint32_t val);
 	void (*emit_rreg) (struct amdgpu_ring *ring, uint32_t offset);
+	void (*emit_switch_buffer) (struct amdgpu_ring *ring);
 };
 
 /*
@@ -2372,6 +2373,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
 #define amdgpu_ring_emit_hdp_invalidate(r) (r)->funcs->emit_hdp_invalidate((r))
 #define amdgpu_ring_emit_wreg(r, i, v) (r)->funcs->emit_wreg((r), (i), (v))
 #define amdgpu_ring_emit_rreg(r, i) (r)->funcs->emit_rreg((r), (i))
+#define amdgpu_ring_emit_switch_buffer(r) (r)->funcs->emit_switch_buffer((r))
 #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
 #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
 #define amdgpu_ring_patch_cond_exec(r,o) (r)->funcs->patch_cond_exec((r),(o))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index a31d7ef..029ee79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -210,6 +210,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
 		amdgpu_ring_patch_cond_exec(ring, patch_offset);
 
 	ring->current_ctx = ctx;
+	if (ring->funcs->emit_switch_buffer)
+		amdgpu_ring_emit_switch_buffer(ring);
 	amdgpu_ring_commit(ring);
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index dfa2288..26fced0 100755
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -5936,12 +5936,6 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 {
 	u32 header, control = 0;
 
-	/* insert SWITCH_BUFFER packet before first IB in the ring frame */
-	if (ctx_switch) {
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-	}
-
 	if (ib->flags & AMDGPU_IB_FLAG_CE)
 		header = PACKET3(PACKET3_INDIRECT_BUFFER_CONST, 2);
 	else
@@ -6170,6 +6164,12 @@ static void gfx_v8_0_ring_emit_wreg_kiq(struct amdgpu_ring *ring, u32 idx, u32 v
 	amdgpu_ring_write(ring, val);
 }
 
+static void gfx_v8_ring_emit_sb(struct amdgpu_ring *ring)
+{
+	amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
+	amdgpu_ring_write(ring, 0);
+}
+
 static void gfx_v8_0_set_gfx_eop_interrupt_state(struct amdgpu_device *adev,
 						 enum amdgpu_interrupt_state state)
 {
@@ -6477,6 +6477,7 @@ static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_gfx = {
 	.test_ib = gfx_v8_0_ring_test_ib,
 	.insert_nop = amdgpu_ring_insert_nop,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
+	.emit_switch_buffer = gfx_v8_ring_emit_sb,
 };
 
 static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_compute = {
-- 
1.9.1

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

[PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Monk Liu]

CE & DE can have most up to 128dw as the gap between them
so to sync CE nad DE we don't need double SWITCH_BUFFERs any
more, which is urgly and harm performance, we only need
insert 128NOP after VM flush to prevent CE vm fault.

Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 26fced0..ce1e616 100755
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6005,14 +6005,6 @@ static void gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, seq);
 	amdgpu_ring_write(ring, 0xffffffff);
 	amdgpu_ring_write(ring, 4); /* poll interval */
-
-	if (usepfp) {
-		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-	}
 }
 
 static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
@@ -6020,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
 {
 	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
 
+	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE finish CEIB */
+	amdgpu_ring_insert_nop(ring, 128);
+
 	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
 	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
 				 WRITE_DATA_DST_SEL(0)) |
@@ -6059,11 +6054,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
 		/* sync PFP to ME, otherwise we might get invalid PFP reads */
 		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
 		amdgpu_ring_write(ring, 0x0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
 	}
+
+	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush finish */
+	amdgpu_ring_insert_nop(ring, 128);
 }
 
 static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring *ring)
-- 
1.9.1

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

Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Christian König]

Am 29.08.2016 um 04:55 schrieb Monk Liu:
> CE & DE can have most up to 128dw as the gap between them
> so to sync CE nad DE we don't need double SWITCH_BUFFERs any
> more, which is urgly and harm performance, we only need
> insert 128NOP after VM flush to prevent CE vm fault.
>
> Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

Looks good to me, but only the GFX engines have a CE. So syncing on the 
compute engines is pretty much pointless.

So I suggest that you move this into the "usepfp" if branch as well.

With that fixed the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 18 ++++++------------
>   1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 26fced0..ce1e616 100755
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6005,14 +6005,6 @@ static void gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
>   	amdgpu_ring_write(ring, seq);
>   	amdgpu_ring_write(ring, 0xffffffff);
>   	amdgpu_ring_write(ring, 4); /* poll interval */
> -
> -	if (usepfp) {
> -		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -	}
>   }
>   
>   static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
> @@ -6020,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>   {
>   	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
>   
> +	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE finish CEIB */
> +	amdgpu_ring_insert_nop(ring, 128);
> +
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
>   	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
>   				 WRITE_DATA_DST_SEL(0)) |
> @@ -6059,11 +6054,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>   		/* sync PFP to ME, otherwise we might get invalid PFP reads */
>   		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
>   		amdgpu_ring_write(ring, 0x0);
> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
>   	}
> +
> +	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush finish */
> +	amdgpu_ring_insert_nop(ring, 128);
>   }
>   
>   static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring *ring)


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

RE: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Liu, Monk]

No, compute ring also can leverage constant engines, that's depend on OCL umd behavior
I just make sure KMD do nothing wrong 

BR Monk

-----Original Message-----
From: Christian König [mailto:deathsimple@vodafone.de] 
Sent: Monday, August 29, 2016 4:06 PM
To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

Am 29.08.2016 um 04:55 schrieb Monk Liu:
> CE & DE can have most up to 128dw as the gap between them so to sync 
> CE nad DE we don't need double SWITCH_BUFFERs any more, which is urgly 
> and harm performance, we only need insert 128NOP after VM flush to 
> prevent CE vm fault.
>
> Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

Looks good to me, but only the GFX engines have a CE. So syncing on the compute engines is pretty much pointless.

So I suggest that you move this into the "usepfp" if branch as well.

With that fixed the patch is Reviewed-by: Christian König <christian.koenig@amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 18 ++++++------------
>   1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 26fced0..ce1e616 100755
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6005,14 +6005,6 @@ static void gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
>   	amdgpu_ring_write(ring, seq);
>   	amdgpu_ring_write(ring, 0xffffffff);
>   	amdgpu_ring_write(ring, 4); /* poll interval */
> -
> -	if (usepfp) {
> -		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -	}
>   }
>   
>   static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring, @@ 
> -6020,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>   {
>   	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
>   
> +	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE finish CEIB */
> +	amdgpu_ring_insert_nop(ring, 128);
> +
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
>   	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
>   				 WRITE_DATA_DST_SEL(0)) |
> @@ -6059,11 +6054,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>   		/* sync PFP to ME, otherwise we might get invalid PFP reads */
>   		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
>   		amdgpu_ring_write(ring, 0x0);
> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
>   	}
> +
> +	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush finish */
> +	amdgpu_ring_insert_nop(ring, 128);
>   }
>   
>   static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring *ring)


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

Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Christian König]

Hui what?

It feels like a hundred times I asked if the compute engine has a CE as 
well, but so far the answer was always No. That would explain a whole 
bunch of problems we had with the compute rings as well.

In this case the patch is good as it is, please just rebase it.

Christian.

Am 29.08.2016 um 10:14 schrieb Liu, Monk:
> No, compute ring also can leverage constant engines, that's depend on OCL umd behavior
> I just make sure KMD do nothing wrong
>
> BR Monk
>
> -----Original Message-----
> From: Christian König [mailto:deathsimple@vodafone.de]
> Sent: Monday, August 29, 2016 4:06 PM
> To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
>
> Am 29.08.2016 um 04:55 schrieb Monk Liu:
>> CE & DE can have most up to 128dw as the gap between them so to sync
>> CE nad DE we don't need double SWITCH_BUFFERs any more, which is urgly
>> and harm performance, we only need insert 128NOP after VM flush to
>> prevent CE vm fault.
>>
>> Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> Looks good to me, but only the GFX engines have a CE. So syncing on the compute engines is pretty much pointless.
>
> So I suggest that you move this into the "usepfp" if branch as well.
>
> With that fixed the patch is Reviewed-by: Christian König <christian.koenig@amd.com>.
>
> Regards,
> Christian.
>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 18 ++++++------------
>>    1 file changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index 26fced0..ce1e616 100755
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -6005,14 +6005,6 @@ static void gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
>>    	amdgpu_ring_write(ring, seq);
>>    	amdgpu_ring_write(ring, 0xffffffff);
>>    	amdgpu_ring_write(ring, 4); /* poll interval */
>> -
>> -	if (usepfp) {
>> -		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>> -		amdgpu_ring_write(ring, 0);
>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>> -		amdgpu_ring_write(ring, 0);
>> -	}
>>    }
>>    
>>    static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring, @@
>> -6020,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>>    {
>>    	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
>>    
>> +	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE finish CEIB */
>> +	amdgpu_ring_insert_nop(ring, 128);
>> +
>>    	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
>>    	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
>>    				 WRITE_DATA_DST_SEL(0)) |
>> @@ -6059,11 +6054,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>>    		/* sync PFP to ME, otherwise we might get invalid PFP reads */
>>    		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
>>    		amdgpu_ring_write(ring, 0x0);
>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>> -		amdgpu_ring_write(ring, 0);
>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>> -		amdgpu_ring_write(ring, 0);
>>    	}
>> +
>> +	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush finish */
>> +	amdgpu_ring_insert_nop(ring, 128);
>>    }
>>    
>>    static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring *ring)
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

RE: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Liu, Monk]

But speaking with practice attitude: at least close source UMD OCL doesn't use CE at all, and I guess MESA either ...

BR Monk

-----Original Message-----
From: Christian König [mailto:deathsimple@vodafone.de] 
Sent: Monday, August 29, 2016 4:25 PM
To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

Hui what?

It feels like a hundred times I asked if the compute engine has a CE as well, but so far the answer was always No. That would explain a whole bunch of problems we had with the compute rings as well.

In this case the patch is good as it is, please just rebase it.

Christian.

Am 29.08.2016 um 10:14 schrieb Liu, Monk:
> No, compute ring also can leverage constant engines, that's depend on 
> OCL umd behavior I just make sure KMD do nothing wrong
>
> BR Monk
>
> -----Original Message-----
> From: Christian König [mailto:deathsimple@vodafone.de]
> Sent: Monday, August 29, 2016 4:06 PM
> To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
>
> Am 29.08.2016 um 04:55 schrieb Monk Liu:
>> CE & DE can have most up to 128dw as the gap between them so to sync 
>> CE nad DE we don't need double SWITCH_BUFFERs any more, which is 
>> urgly and harm performance, we only need insert 128NOP after VM flush 
>> to prevent CE vm fault.
>>
>> Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> Looks good to me, but only the GFX engines have a CE. So syncing on the compute engines is pretty much pointless.
>
> So I suggest that you move this into the "usepfp" if branch as well.
>
> With that fixed the patch is Reviewed-by: Christian König <christian.koenig@amd.com>.
>
> Regards,
> Christian.
>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 18 ++++++------------
>>    1 file changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index 26fced0..ce1e616 100755
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -6005,14 +6005,6 @@ static void gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
>>    	amdgpu_ring_write(ring, seq);
>>    	amdgpu_ring_write(ring, 0xffffffff);
>>    	amdgpu_ring_write(ring, 4); /* poll interval */
>> -
>> -	if (usepfp) {
>> -		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>> -		amdgpu_ring_write(ring, 0);
>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>> -		amdgpu_ring_write(ring, 0);
>> -	}
>>    }
>>    
>>    static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring, 
>> @@
>> -6020,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>>    {
>>    	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
>>    
>> +	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE finish CEIB */
>> +	amdgpu_ring_insert_nop(ring, 128);
>> +
>>    	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
>>    	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
>>    				 WRITE_DATA_DST_SEL(0)) |
>> @@ -6059,11 +6054,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>>    		/* sync PFP to ME, otherwise we might get invalid PFP reads */
>>    		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
>>    		amdgpu_ring_write(ring, 0x0);
>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>> -		amdgpu_ring_write(ring, 0);
>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>> -		amdgpu_ring_write(ring, 0);
>>    	}
>> +
>> +	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush finish */
>> +	amdgpu_ring_insert_nop(ring, 128);
>>    }
>>    
>>    static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring 
>> *ring)
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Christian König]

Well a perfect example of why we shouldn't take a look at what userspace 
does, but rather what the hardware can do when we design the IOCTLs.

I'm trying to raise awareness for this for quite a while now, but a lot 
of people seem to think when the UMD doesn't do something the kernel 
doesn't need to handle this situation.

Thanks for that info,
Christian.

Am 29.08.2016 um 10:28 schrieb Liu, Monk:
> But speaking with practice attitude: at least close source UMD OCL doesn't use CE at all, and I guess MESA either ...
>
> BR Monk
>
> -----Original Message-----
> From: Christian König [mailto:deathsimple@vodafone.de]
> Sent: Monday, August 29, 2016 4:25 PM
> To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
>
> Hui what?
>
> It feels like a hundred times I asked if the compute engine has a CE as well, but so far the answer was always No. That would explain a whole bunch of problems we had with the compute rings as well.
>
> In this case the patch is good as it is, please just rebase it.
>
> Christian.
>
> Am 29.08.2016 um 10:14 schrieb Liu, Monk:
>> No, compute ring also can leverage constant engines, that's depend on
>> OCL umd behavior I just make sure KMD do nothing wrong
>>
>> BR Monk
>>
>> -----Original Message-----
>> From: Christian König [mailto:deathsimple@vodafone.de]
>> Sent: Monday, August 29, 2016 4:06 PM
>> To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
>>
>> Am 29.08.2016 um 04:55 schrieb Monk Liu:
>>> CE & DE can have most up to 128dw as the gap between them so to sync
>>> CE nad DE we don't need double SWITCH_BUFFERs any more, which is
>>> urgly and harm performance, we only need insert 128NOP after VM flush
>>> to prevent CE vm fault.
>>>
>>> Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
>>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>> Looks good to me, but only the GFX engines have a CE. So syncing on the compute engines is pretty much pointless.
>>
>> So I suggest that you move this into the "usepfp" if branch as well.
>>
>> With that fixed the patch is Reviewed-by: Christian König <christian.koenig@amd.com>.
>>
>> Regards,
>> Christian.
>>
>>> ---
>>>     drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 18 ++++++------------
>>>     1 file changed, 6 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> index 26fced0..ce1e616 100755
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>> @@ -6005,14 +6005,6 @@ static void gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
>>>     	amdgpu_ring_write(ring, seq);
>>>     	amdgpu_ring_write(ring, 0xffffffff);
>>>     	amdgpu_ring_write(ring, 4); /* poll interval */
>>> -
>>> -	if (usepfp) {
>>> -		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
>>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>>> -		amdgpu_ring_write(ring, 0);
>>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>>> -		amdgpu_ring_write(ring, 0);
>>> -	}
>>>     }
>>>     
>>>     static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>>> @@
>>> -6020,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>>>     {
>>>     	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
>>>     
>>> +	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE finish CEIB */
>>> +	amdgpu_ring_insert_nop(ring, 128);
>>> +
>>>     	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
>>>     	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
>>>     				 WRITE_DATA_DST_SEL(0)) |
>>> @@ -6059,11 +6054,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
>>>     		/* sync PFP to ME, otherwise we might get invalid PFP reads */
>>>     		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
>>>     		amdgpu_ring_write(ring, 0x0);
>>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>>> -		amdgpu_ring_write(ring, 0);
>>> -		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
>>> -		amdgpu_ring_write(ring, 0);
>>>     	}
>>> +
>>> +	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush finish */
>>> +	amdgpu_ring_insert_nop(ring, 128);
>>>     }
>>>     
>>>     static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring
>>> *ring)
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


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

RE: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Monday, August 29, 2016 4:25 AM
> To: Liu, Monk; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
> 
> Hui what?
> 
> It feels like a hundred times I asked if the compute engine has a CE as
> well, but so far the answer was always No. That would explain a whole
> bunch of problems we had with the compute rings as well.

Can we get some clarification?  I doesn't have a dedicated CE as far as I know.  Can the CE be used in conjunction with MEC as well as the PFP/ME?

Alex

> 
> In this case the patch is good as it is, please just rebase it.
> 
> Christian.
> 
> Am 29.08.2016 um 10:14 schrieb Liu, Monk:
> > No, compute ring also can leverage constant engines, that's depend on OCL
> umd behavior
> > I just make sure KMD do nothing wrong
> >
> > BR Monk
> >
> > -----Original Message-----
> > From: Christian König [mailto:deathsimple@vodafone.de]
> > Sent: Monday, August 29, 2016 4:06 PM
> > To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
> > Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
> >
> > Am 29.08.2016 um 04:55 schrieb Monk Liu:
> >> CE & DE can have most up to 128dw as the gap between them so to sync
> >> CE nad DE we don't need double SWITCH_BUFFERs any more, which is
> urgly
> >> and harm performance, we only need insert 128NOP after VM flush to
> >> prevent CE vm fault.
> >>
> >> Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
> >> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> > Looks good to me, but only the GFX engines have a CE. So syncing on the
> compute engines is pretty much pointless.
> >
> > So I suggest that you move this into the "usepfp" if branch as well.
> >
> > With that fixed the patch is Reviewed-by: Christian König
> <christian.koenig@amd.com>.
> >
> > Regards,
> > Christian.
> >
> >> ---
> >>    drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 18 ++++++------------
> >>    1 file changed, 6 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> index 26fced0..ce1e616 100755
> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> @@ -6005,14 +6005,6 @@ static void
> gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
> >>    	amdgpu_ring_write(ring, seq);
> >>    	amdgpu_ring_write(ring, 0xffffffff);
> >>    	amdgpu_ring_write(ring, 4); /* poll interval */
> >> -
> >> -	if (usepfp) {
> >> -		/* synce CE with ME to prevent CE fetch CEIB before context
> switch done */
> >> -		amdgpu_ring_write(ring,
> PACKET3(PACKET3_SWITCH_BUFFER, 0));
> >> -		amdgpu_ring_write(ring, 0);
> >> -		amdgpu_ring_write(ring,
> PACKET3(PACKET3_SWITCH_BUFFER, 0));
> >> -		amdgpu_ring_write(ring, 0);
> >> -	}
> >>    }
> >>
> >>    static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
> @@
> >> -6020,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct
> amdgpu_ring *ring,
> >>    {
> >>    	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
> >>
> >> +	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE
> finish CEIB */
> >> +	amdgpu_ring_insert_nop(ring, 128);
> >> +
> >>    	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
> >>    	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
> >>    				 WRITE_DATA_DST_SEL(0)) |
> >> @@ -6059,11 +6054,10 @@ static void
> gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
> >>    		/* sync PFP to ME, otherwise we might get invalid PFP reads
> */
> >>    		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME,
> 0));
> >>    		amdgpu_ring_write(ring, 0x0);
> >> -		amdgpu_ring_write(ring,
> PACKET3(PACKET3_SWITCH_BUFFER, 0));
> >> -		amdgpu_ring_write(ring, 0);
> >> -		amdgpu_ring_write(ring,
> PACKET3(PACKET3_SWITCH_BUFFER, 0));
> >> -		amdgpu_ring_write(ring, 0);
> >>    	}
> >> +
> >> +	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush
> finish */
> >> +	amdgpu_ring_insert_nop(ring, 128);
> >>    }
> >>
> >>    static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring *ring)
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Marek Olšák]

On Mon, Aug 29, 2016 at 3:37 PM, Deucher, Alexander
<Alexander.Deucher@amd.com> wrote:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
>> Of Christian König
>> Sent: Monday, August 29, 2016 4:25 AM
>> To: Liu, Monk; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
>>
>> Hui what?
>>
>> It feels like a hundred times I asked if the compute engine has a CE as
>> well, but so far the answer was always No. That would explain a whole
>> bunch of problems we had with the compute rings as well.
>
> Can we get some clarification?  I doesn't have a dedicated CE as far as I know.  Can the CE be used in conjunction with MEC as well as the PFP/ME?

From the CP program spec, CE can only be used with DE and DE = CP.ME.
Also, CE is described in the Kernel Graphics Queue section. There is
no mention of CE in the compute sections.

Thus, compute IBs can't be paired with CE IBs.

Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Liu, Monk]

Yeah, CPC cannot use constant engine.

BR Monk

-----Original Message-----
From: Marek Olšák [mailto:maraeo@gmail.com] 
Sent: Tuesday, September 06, 2016 3:13 AM
To: Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: Christian König <deathsimple@vodafone.de>; Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

On Mon, Aug 29, 2016 at 3:37 PM, Deucher, Alexander <Alexander.Deucher@amd.com> wrote:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On 
>> Behalf Of Christian König
>> Sent: Monday, August 29, 2016 4:25 AM
>> To: Liu, Monk; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
>>
>> Hui what?
>>
>> It feels like a hundred times I asked if the compute engine has a CE 
>> as well, but so far the answer was always No. That would explain a 
>> whole bunch of problems we had with the compute rings as well.
>
> Can we get some clarification?  I doesn't have a dedicated CE as far as I know.  Can the CE be used in conjunction with MEC as well as the PFP/ME?

From the CP program spec, CE can only be used with DE and DE = CP.ME.
Also, CE is described in the Kernel Graphics Queue section. There is no mention of CE in the compute sections.

Thus, compute IBs can't be paired with CE IBs.

Marek
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/4] drm/amdgpu:new method to sync ce&de

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Monk Liu
> Sent: Sunday, August 28, 2016 10:56 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Liu, Monk
> Subject: [PATCH 2/4] drm/amdgpu:new method to sync ce&de
> 
> CE & DE can have most up to 128dw as the gap between them
> so to sync CE nad DE we don't need double SWITCH_BUFFERs any
> more, which is urgly and harm performance, we only need
> insert 128NOP after VM flush to prevent CE vm fault.
> 
> Change-Id: Ibec954ce4c817ad7d3bce89c2bcb95b6c6bb5411
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 26fced0..ce1e616 100755
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -6005,14 +6005,6 @@ static void
> gfx_v8_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
>  	amdgpu_ring_write(ring, seq);
>  	amdgpu_ring_write(ring, 0xffffffff);
>  	amdgpu_ring_write(ring, 4); /* poll interval */
> -
> -	if (usepfp) {
> -		/* synce CE with ME to prevent CE fetch CEIB before context
> switch done */
> -		amdgpu_ring_write(ring,
> PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -		amdgpu_ring_write(ring,
> PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -	}
>  }
> 
>  static void gfx_v8_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
> @@ -6020,6 +6012,9 @@ static void gfx_v8_0_ring_emit_vm_flush(struct
> amdgpu_ring *ring,
>  {
>  	int usepfp = (ring->type == AMDGPU_RING_TYPE_GFX);
> 
> +	/* GFX8 emits 128 dw nop to prevent DE do vm_flush before CE
> finish CEIB */
> +	amdgpu_ring_insert_nop(ring, 128);

Here and below we can make the nop insertion conditional on usepfp since there is no CE on the compute MEC.

Alex

> +
>  	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
>  	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(usepfp) |
>  				 WRITE_DATA_DST_SEL(0)) |
> @@ -6059,11 +6054,10 @@ static void gfx_v8_0_ring_emit_vm_flush(struct
> amdgpu_ring *ring,
>  		/* sync PFP to ME, otherwise we might get invalid PFP reads
> */
>  		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME,
> 0));
>  		amdgpu_ring_write(ring, 0x0);
> -		amdgpu_ring_write(ring,
> PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
> -		amdgpu_ring_write(ring,
> PACKET3(PACKET3_SWITCH_BUFFER, 0));
> -		amdgpu_ring_write(ring, 0);
>  	}
> +
> +	/* GFX8 emits 128 dw nop to prevent CE access VM before vm_flush
> finish */
> +	amdgpu_ring_insert_nop(ring, 128);
>  }
> 
>  static u32 gfx_v8_0_ring_get_rptr_compute(struct amdgpu_ring *ring)
> --
> 1.9.1
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx