I'll confirm it with HW today -----Original Message----- From: Nicolai Hähnle [mailto:nhaehnle@xxxxxxxxx] Sent: Thursday, May 04, 2017 7:47 PM To: Liu, Monk <Monk.Liu at amd.com>; amd-gfx at lists.freedesktop.org Subject: Re: FW: [PATCH 1/2] drm/amdgpu:use FRAME_CNTL for new GFX ucode On 04.05.2017 13:43, Liu, Monk wrote: > And for VI, even user use the old firmware, it is still safe to use > TMZ package, > > But I will double check with CP team today, thanks for your remind Right, I'm sure that old firmware + your change is safe. The question is: Is it safe to use the new firmware _without_ your change. If the answer is "no" for gfx8, we have a problem. Cheers, Nicolai > > BR Monk > > -----Original Message----- > From: Liu, Monk > Sent: Thursday, May 04, 2017 7:42 PM > To: 'Nicolai Hähnle' <nhaehnle at gmail.com>; > amd-gfx at lists.freedesktop.org > Subject: RE: FW: [PATCH 1/2] drm/amdgpu:use FRAME_CNTL for new GFX > ucode > > No, this CP side change is from VI, and ported to AI > > > -----Original Message----- > From: Nicolai Hähnle [mailto:nhaehnle at gmail.com] > Sent: Thursday, May 04, 2017 7:33 PM > To: Liu, Monk <Monk.Liu at amd.com>; amd-gfx at lists.freedesktop.org > Subject: Re: FW: [PATCH 1/2] drm/amdgpu:use FRAME_CNTL for new GFX > ucode > > On 04.05.2017 12:48, Liu, Monk wrote: >> >> >> -----Original Message----- >> From: Monk Liu [mailto:Monk.Liu at amd.com] >> Sent: Thursday, May 04, 2017 6:47 PM >> To: Liu, Monk <Monk.Liu at amd.com> >> Cc: Liu, Monk <Monk.Liu at amd.com> >> Subject: [PATCH 1/2] drm/amdgpu:use FRAME_CNTL for new GFX ucode >> >> VI/AI affected: > > I thought this was a gfx9-only change? If it's gfx8 also, we're going to have pretty bad compatibility issues when new firmware is used with old kernel... > > Cheers, > Nicolai > > >> >> CP/HW team requires KMD insert FRAME_CONTROL(end) after the last IB and before the fence of this DMAframe. >> >> this is to make sure the cache are flushed, and it's a must change no matter MCBP/SR-IOV or bare-metal case because new CP hw won't do the cache flush for each IB anymore, it just leaves it to KMD now. >> >> with this patch, certain MCBP hang issue when rendering vulkan/chained-ib are resolved. >> >> Change-Id: I34ee7528aa32e704b2850bc6d50774b24c29b840 >> Signed-off-by: Monk Liu <Monk.Liu at amd.com> >> Reviewed-by: Christian König <christian.koenig at amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + >> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 3 +++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 + >> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 7 +++++++ >> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 7 +++++++ >> 5 files changed, 19 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> index 0ee4d87..f59a1e5 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> @@ -1828,6 +1828,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring >> *ring) #define amdgpu_ring_emit_cntxcntl(r, d) >> (r)->funcs->emit_cntxcntl((r), (d)) #define amdgpu_ring_emit_rreg(r, >> d) (r)->funcs->emit_rreg((r), (d)) #define amdgpu_ring_emit_wreg(r, >> d, v) (r)->funcs->emit_wreg((r), (d), (v)) >> +#define amdgpu_ring_emit_tmz(r, b) (r)->funcs->emit_tmz((r), (b)) >> #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 4480e01..11a22fa 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c >> @@ -206,6 +206,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs, >> need_ctx_switch = false; >> } >> >> + if (ring->funcs->emit_tmz) >> + amdgpu_ring_emit_tmz(ring, false); >> + >> if (ring->funcs->emit_hdp_invalidate #ifdef CONFIG_X86_64 >> && !(adev->flags & AMD_IS_APU) >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> index 5786cc3..981ef08 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> @@ -142,6 +142,7 @@ struct amdgpu_ring_funcs { >> void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags); >> void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg); >> void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t >> val); >> + void (*emit_tmz)(struct amdgpu_ring *ring, bool start); >> }; >> >> struct amdgpu_ring { >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c >> index 4144fc3..90998f6 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c >> @@ -6665,6 +6665,12 @@ static void gfx_v8_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne >> ring->ring[offset] = (ring->ring_size >> 2) - offset + cur; } >> >> +static void gfx_v8_0_ring_emit_tmz(struct amdgpu_ring *ring, bool >> +start) { >> + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0)); >> + amdgpu_ring_write(ring, FRAME_CMD(start ? 0 : 1)); /* frame_end */ >> +} >> + >> >> static void gfx_v8_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg) { @@ -6946,6 +6952,7 @@ static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_gfx = { >> .emit_cntxcntl = gfx_v8_ring_emit_cntxcntl, >> .init_cond_exec = gfx_v8_0_ring_emit_init_cond_exec, >> .patch_cond_exec = gfx_v8_0_ring_emit_patch_cond_exec, >> + .emit_tmz = gfx_v8_0_ring_emit_tmz, >> }; >> >> static const struct amdgpu_ring_funcs gfx_v8_0_ring_funcs_compute = >> { diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> index 3bf7992..a9ca891 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >> @@ -3245,6 +3245,12 @@ static void gfx_v9_0_ring_emit_patch_cond_exec(struct amdgpu_ring *ring, unsigne >> ring->ring[offset] = (ring->ring_size>>2) - offset + cur; } >> >> +static void gfx_v9_0_ring_emit_tmz(struct amdgpu_ring *ring, bool >> +start) { >> + amdgpu_ring_write(ring, PACKET3(PACKET3_FRAME_CONTROL, 0)); >> + amdgpu_ring_write(ring, FRAME_CMD(start ? 0 : 1)); /* frame_end */ >> +} >> + >> static void gfx_v9_0_ring_emit_rreg(struct amdgpu_ring *ring, uint32_t reg) { >> struct amdgpu_device *adev = ring->adev; @@ -3579,6 +3585,7 @@ static const struct amdgpu_ring_funcs gfx_v9_0_ring_funcs_gfx = { >> .emit_cntxcntl = gfx_v9_ring_emit_cntxcntl, >> .init_cond_exec = gfx_v9_0_ring_emit_init_cond_exec, >> .patch_cond_exec = gfx_v9_0_ring_emit_patch_cond_exec, >> + .emit_tmz = gfx_v9_0_ring_emit_tmz, >> }; >> >> static const struct amdgpu_ring_funcs gfx_v9_0_ring_funcs_compute = >> { >> -- >> 2.7.4 >> >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> > > > -- > Lerne, wie die Welt wirklich ist, > Aber vergiss niemals, wie sie sein sollte. > -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte.
