[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 at vodafone.de]
> Sent: Monday, August 29, 2016 4:06 PM
> To: Liu, Monk <Monk.Liu at amd.com>; amd-gfx at 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 at 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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx