FW: [PATCH 1/2] drm/amdgpu:use FRAME_CNTL for new GFX ucode

Thu May 4 11:46:48 UTC 2017

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.