[PATCH 1/2] drm/amdgpu: Add per device sdma_doorbell_range field

Tue Dec 18 17:30:16 UTC 2018

The reason I set it to 20 is we reserved 10 doorbells for each sdma engine. See definition in amdgpu_doorbell.h, line 118. But setting it to 16 is fine as vega20 only have 8 sdma queues per engine. Since the vega20 doorbell layout will be reused for future asics. So setting it to 20 will cover asics which have 10 queues per engine, if there. 

Gfx and paging queue use the first and second doorbell page. Both of them can use up to 10 doorbells if they want.  

Regards,
Oak

-----Original Message-----
From: Yang, Philip <Philip.Yang at amd.com> 
Sent: Tuesday, December 18, 2018 12:02 PM
To: Zeng, Oak <Oak.Zeng at amd.com>; Partap Singh Rana, Dhirendra <Dhirendra.PartapSinghRana at amd.com>; amd-gfx at lists.freedesktop.org; Deng, Emily <Emily.Deng at amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: Add per device sdma_doorbell_range field

Thanks Oak for the clarification for Vega10.

For Vega20, 8 user space sdma queues should set doorbell range to 16, maybe you add gfx and page queue doorbell together to become 20? But gfx and page queue doorbell is on kernel space, user space sdma queues doorbell mapping is on user space, they can overlap and share same doorbell range.

Regards,
Philip

On 2018-12-18 11:43 a.m., Zeng, Oak wrote:
> Thank you Dhiren. So we will also set it to 4 in Linux device driver.
> 
> BTW, patch 2 set the doorbell range for vega20 from 2 to 20. With previous setting of 2, I believe only the first two user space sdma queues work. The rest 6 queues won't work because the doorbell ringing won't be routed to sdma engine due to previous bif doorbell range setting.
> 
> Regards,
> Oak
> 
> -----Original Message-----
> From: Partap Singh Rana, Dhirendra <Dhirendra.PartapSinghRana at amd.com>
> Sent: Tuesday, December 18, 2018 11:34 AM
> To: Zeng, Oak <Oak.Zeng at amd.com>; Yang, Philip <Philip.Yang at amd.com>; 
> amd-gfx at lists.freedesktop.org; Deng, Emily <Emily.Deng at amd.com>
> Subject: RE: [PATCH 1/2] drm/amdgpu: Add per device 
> sdma_doorbell_range field
> 
> Hi Oak,
> 
> Windows will set 4 dwords for both sdma0 and 1.
> 
> Thanks,
> Dhiren
> 
> -----Original Message-----
> From: Zeng, Oak
> Sent: Tuesday, December 18, 2018 10:52 AM
> To: Yang, Philip <Philip.Yang at amd.com>; amd-gfx at lists.freedesktop.org; 
> Partap Singh Rana, Dhirendra <Dhirendra.PartapSinghRana at amd.com>; 
> Deng, Emily <Emily.Deng at amd.com>
> Subject: RE: [PATCH 1/2] drm/amdgpu: Add per device 
> sdma_doorbell_range field
> 
> Thanks Philip.
> 
> For vega10, sdma doorbells are defined to be windows sriov compatible. Two qwords doorbell are defined for each sdma engine. See amdgpu_doorbell.h line 192. So program nbio sdma doorbell routing range to 4 (dwords) is correct. I am not sure why previously the doorbell range was programmed to 2, even though it is defined 4. @Partap Singh Rana, Dhirendra what value windows does host driver program BIF_SDMA0_DOORBELL_RANGE.SIZE for vega10?
> 
> @Deng, Emily I saw previously you changed the vega10 doorbell assignment for sriov. Did you copy the assignment from windows host driver? If yes, I assume windows host should set the bif doorbell range to 4 dwords, according to the assignment. Does changing bif doorbell range to 4 conflict with windows host driver?
> 
> People might wonder why programming it to 2 also worked before. The reason is, currently amdgpu only use one sdma ring per sdma engine. So it never used the "HI_PRI" ring and doorbell. Kfd uses two sdma rings per sdma engine, but for the second ring, it uses a doorbell with the same in page offset (0xf0) but mapped to the second doorbell page of the process.
> 
> So my understanding is, setting the doorbell range to 4 (see patch 2 of this series) is more consistent and no harm for current usage model. If we use it in the future, it should be fine. But let's see what is the windows host driver setting.
> 
> Regards,
> Oak
> 
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of 
> Yang, Philip
> Sent: Tuesday, December 18, 2018 9:57 AM
> To: Zeng, Oak <Oak.Zeng at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 1/2] drm/amdgpu: Add per device 
> sdma_doorbell_range field
> 
> 
> 
> On 2018-12-17 9:12 p.m., Zeng, Oak wrote:
>> Different ASIC has different sdma doorbell range. Add a per device 
>> sdma_doorbell_range field and initialize it.
>>
>> Change-Id: Idd980db1a72cfb373e24ac23ba3e48bb329ed4ad
>> Signed-off-by: Oak Zeng <Oak.Zeng at amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 2 ++
>>    drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c | 1 +
>>    drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c | 1 +
>>    3 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> index 35a0c05..1cfec06 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
>> @@ -72,6 +72,8 @@ struct amdgpu_doorbell_index {
>>    		} uvd_vce;
>>    	};
>>    	uint32_t max_assignment;
>> +	/* Per engine SDMA doorbell size in dword */
>> +	uint32_t sdma_doorbell_range;
>>    };
>>    
>>    typedef enum _AMDGPU_DOORBELL_ASSIGNMENT diff --git 
>> a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
>> b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
>> index b75d17b..4b5d60e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_reg_init.c
>> @@ -83,5 +83,6 @@ void vega10_doorbell_index_init(struct amdgpu_device *adev)
>>    	adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_DOORBELL64_VCE_RING6_7;
>>    	/* In unit of dword doorbell */
>>    	adev->doorbell_index.max_assignment = 
>> AMDGPU_DOORBELL64_MAX_ASSIGNMENT << 1;
>> +	adev->doorbell_index.sdma_doorbell_range = 4;
> Vega10 doorbell range was 2 dwords (one 64bit doorbell), change to 4 
> dwords may not work under SRIOV
>>    }
>>    
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
>> b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
>> index 63c542c..53716c5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c
>> @@ -86,5 +86,6 @@ void vega20_doorbell_index_init(struct amdgpu_device *adev)
>>    	adev->doorbell_index.uvd_vce.vce_ring4_5 = AMDGPU_VEGA20_DOORBELL64_VCE_RING4_5;
>>    	adev->doorbell_index.uvd_vce.vce_ring6_7 = AMDGPU_VEGA20_DOORBELL64_VCE_RING6_7;
>>    	adev->doorbell_index.max_assignment = 
>> AMDGPU_VEGA20_DOORBELL_MAX_ASSIGNMENT << 1;
>> +	adev->doorbell_index.sdma_doorbell_range = 20;
>>    }
>>    
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 