[PATCH] drm/amdgpu: avoid using invalidate semaphore for picasso(v2)

[Christian König]

Am 10.12.19 um 12:03 schrieb Zhu, Changfeng:
> [AMD Official Use Only - Internal Distribution Only]
>
> OK, Chris.
>
> What's about SRIOV?
>
> Should we skip using semaphore registers for SRIOV now?
>
> I add REG32_SOC15_OFFSET(MMHUB, 0, mmVM_INVALIDATE_ENG0_SEM, i, 0x0);
> in mmhub_v1_0_program_invalidation.
>
> However, the problem still happens.

Sorry totally missed that reply. Do we have an idea of the root cause?

I mean we should probably commit a workaround, but investigate what's 
going on here.

Regards,
Christian.

>
> BR,
> Changfeng,
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Tuesday, December 10, 2019 6:55 PM
> To: Zhu, Changfeng <Changfeng.Zhu at amd.com>; amd-gfx at lists.freedesktop.org; Huang, Ray <Ray.Huang at amd.com>; Huang, Shimmer <Xinmei.Huang at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>
> Subject: Re: [PATCH] drm/amdgpu: avoid using invalidate semaphore for picasso(v2)
>
> Am 10.12.19 um 03:55 schrieb Changfeng.Zhu:
>> From: changzhu <Changfeng.Zhu at amd.com>
>>
>> It may cause timeout waiting for sem acquire in VM flush when using
>> invalidate semaphore for picasso. So it needs to avoid using
>> invalidate semaphore for piasso.
> It would probably be better to add a small helper function to decide if the semaphore registers should be used or not.
>
> E.g. something like "bool gmc_v9_0_use_semaphore(adev, vmhub...)"
>
> Apart from that looks good to me,
> Christian.
>
>> Change-Id: I6dc552bde180919cd5ba6c81c6d9e3f800043b03
>> Signed-off-by: changzhu <Changfeng.Zhu at amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 28 +++++++++++++++++++--------
>>    1 file changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 231ea9762cb5..601667246a1c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -464,8 +464,11 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>>    	 */
>>    
>>    	/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
>> -	if (vmhub == AMDGPU_MMHUB_0 ||
>> -	    vmhub == AMDGPU_MMHUB_1) {
>> +	if ((vmhub == AMDGPU_MMHUB_0 ||
>> +	     vmhub == AMDGPU_MMHUB_1) &&
>> +	    (!(adev->asic_type == CHIP_RAVEN &&
>> +	       adev->rev_id < 0x8 &&
>> +	       adev->pdev->device == 0x15d8))) {
>>    		for (j = 0; j < adev->usec_timeout; j++) {
>>    			/* a read return value of 1 means semaphore acuqire */
>>    			tmp = RREG32_NO_KIQ(hub->vm_inv_eng0_sem + eng); @@ -495,8
>> +498,11 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>>    	}
>>    
>>    	/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
>> -	if (vmhub == AMDGPU_MMHUB_0 ||
>> -	    vmhub == AMDGPU_MMHUB_1)
>> +	if ((vmhub == AMDGPU_MMHUB_0 ||
>> +	     vmhub == AMDGPU_MMHUB_1) &&
>> +	    (!(adev->asic_type == CHIP_RAVEN &&
>> +	       adev->rev_id < 0x8 &&
>> +	       adev->pdev->device == 0x15d8)))
>>    		/*
>>    		 * add semaphore release after invalidation,
>>    		 * write with 0 means semaphore release @@ -527,8 +533,11 @@
>> static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
>>    	 */
>>    
>>    	/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
>> -	if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
>> -	    ring->funcs->vmhub == AMDGPU_MMHUB_1)
>> +	if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
>> +	     ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
>> +	    (!(adev->asic_type == CHIP_RAVEN &&
>> +	       adev->rev_id < 0x8 &&
>> +	       adev->pdev->device == 0x15d8)))
>>    		/* a read return value of 1 means semaphore acuqire */
>>    		amdgpu_ring_emit_reg_wait(ring,
>>    					  hub->vm_inv_eng0_sem + eng, 0x1, 0x1); @@ -544,8 +553,11 @@
>> static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
>>    					    req, 1 << vmid);
>>    
>>    	/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
>> -	if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
>> -	    ring->funcs->vmhub == AMDGPU_MMHUB_1)
>> +	if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
>> +	     ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
>> +	    (!(adev->asic_type == CHIP_RAVEN &&
>> +	       adev->rev_id < 0x8 &&
>> +	       adev->pdev->device == 0x15d8)))
>>    		/*
>>    		 * add semaphore release after invalidation,
>>    		 * write with 0 means semaphore release