Thanks, I will disable paging queue doorbell on Vega10 and Vega12 with
SRIOV.
Philip
On 2018-11-15 1:01 p.m., Kuehling, Felix wrote:
> You changed the doorbell routing in NBIO. I think that won't work for SR-IOV,
> because it's not controlled by the guest OS there. We may need to disable
> paging queue doorbell on Vega10 and Vega12 with SRIOV. For Vega20 we plan to
> change the doorbell layout before it goes to production (Oak started working
> on that). So on Vega20 we should be able to enable the doorbell for the
> paging queue.
>
> Regards,
> Felix
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of Yang,
> Philip
> Sent: Thursday, November 15, 2018 12:54 PM
> To: Alex Deucher <alexdeuc...@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH] drm/amdgpu: enable paging queue doorbell support
>
> On 2018-11-15 11:43 a.m., Alex Deucher wrote:
>> On Thu, Nov 15, 2018 at 11:08 AM Yang, Philip <philip.y...@amd.com> wrote:
>>> paging queues doorbell index use existing assignment
>>> sDMA_HI_PRI_ENGINE0/1 index, and increase SDMA_DOORBELL_RANGE size
>>> from 2 dwords to 4 dwords to enable the new doorbell index.
>>>
>>> Change-Id: I9adb965f16ee4089d261d9a22231337739184e49
>>> Signed-off-by: Philip Yang <philip.y...@amd.com>
>> Is there a specific fw version requirement for this? If so, we need
>> to add a check. Also will this break SR-IOV due to the doorbell
>> mapping requirements for other OSes? Have we resolved that yet?
>>
> Yes, new FW is required to enable paging queue, it's good idea to check FW
> version when we enable paging queue later after new FW is checked in. paging
> queue is not enabled yet.
> This change will not take effect until paging queue is enabled.
>
> This does not change doorbell mapping so it will not break SR-IOV. My
> understanding is Doorbell mapping is good for vega10/12/20, but need update
> for future chip.
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c | 2 +-
>>> drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c | 2 +-
>>> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 25 +++++++++++++++++--------
>>> 3 files changed, 19 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
>>> b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
>>> index 6f9c54978cc1..0eb42c29ecac 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v6_1.c
>>> @@ -80,7 +80,7 @@ static void nbio_v6_1_sdma_doorbell_range(struct
>>> amdgpu_device *adev, int instan
>>>
>>> if (use_doorbell) {
>>> doorbell_range = REG_SET_FIELD(doorbell_range,
>>> BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
>>> - doorbell_range = REG_SET_FIELD(doorbell_range,
>>> BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
>>> + doorbell_range = REG_SET_FIELD(doorbell_range,
>>> + BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
>>> } else
>>> doorbell_range = REG_SET_FIELD(doorbell_range,
>>> BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
>>> b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
>>> index f8cee95d61cc..9342ee03d7d4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/nbio_v7_4.c
>>> @@ -76,7 +76,7 @@ static void nbio_v7_4_sdma_doorbell_range(struct
>>> amdgpu_device *adev, int instan
>>>
>>> if (use_doorbell) {
>>> doorbell_range = REG_SET_FIELD(doorbell_range,
>>> BIF_SDMA0_DOORBELL_RANGE, OFFSET, doorbell_index);
>>> - doorbell_range = REG_SET_FIELD(doorbell_range,
>>> BIF_SDMA0_DOORBELL_RANGE, SIZE, 2);
>>> + doorbell_range = REG_SET_FIELD(doorbell_range,
>>> + BIF_SDMA0_DOORBELL_RANGE, SIZE, 4);
>>> } else
>>> doorbell_range = REG_SET_FIELD(doorbell_range,
>>> BIF_SDMA0_DOORBELL_RANGE, SIZE, 0);
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> index f4490cdd9804..96c9e83204b7 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> @@ -925,11 +925,9 @@ static void sdma_v4_0_page_resume(struct amdgpu_device
>>> *adev, unsigned int i)
>>> OFFSET, ring->doorbell_index);
>>> WREG32_SDMA(i, mmSDMA0_PAGE_DOORBELL, doorbell);
>>> WREG32_SDMA(i, mmSDMA0_PAGE_DOORBELL_OFFSET, doorbell_offset);
>>> - /* TODO: enable doorbell support */
>>> - /*adev->nbio_funcs->sdma_doorbell_range(adev, i, ring->use_doorbell,
>>> - ring->doorbell_index);*/
>>>
>>> - sdma_v4_0_ring_set_wptr(ring);
>>> + /* paging queue doorbell index is already enabled at
>>> sdma_v4_0_gfx_resume */
>>> + sdma_v4_0_page_ring_set_wptr(ring);
>>>
>>> /* set minor_ptr_update to 0 after wptr programed */
>>> WREG32_SDMA(i, mmSDMA0_PAGE_MINOR_PTR_UPDATE, 0); @@ -1504,9
>>> +1502,6 @@ static int sdma_v4_0_sw_init(void *handle)
>>> ring->ring_obj = NULL;
>>> ring->use_doorbell = true;
>>>
>>> - DRM_INFO("use_doorbell being set to: [%s]\n",
>>> - ring->use_doorbell?"true":"false");
>>> -
>>> if (adev->asic_type == CHIP_VEGA10)
>>> ring->doorbell_index = (i == 0) ?
>>>
>>> (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset @@ -1516,6
>>> +1511,8 @@ static int sdma_v4_0_sw_init(void *handle)
>>> (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1)
>>> //get DWORD offset
>>> : (AMDGPU_DOORBELL64_sDMA_ENGINE1 <<
>>> 1); // get DWORD offset
>>>
>>> + DRM_INFO("use_doorbell being set to: [%s] doorbell index
>>> %d\n",
>>> + ring->use_doorbell?"true":"false",
>>> + ring->doorbell_index);
>>>
>> I think we can drop these messages in general. Looks like leftover
>> debugging output.
>>
> Ok, change it to DRM_DEBUG
>>> sprintf(ring->name, "sdma%d", i);
>>> r = amdgpu_ring_init(adev, ring, 1024, @@ -1529,7
>>> +1526,19 @@ static int sdma_v4_0_sw_init(void *handle)
>>> if (adev->sdma.has_page_queue) {
>>> ring = &adev->sdma.instance[i].page;
>>> ring->ring_obj = NULL;
>>> - ring->use_doorbell = false;
>>> + ring->use_doorbell = true;
>>> +
>>> + if (adev->asic_type == CHIP_VEGA10)
>>> + ring->doorbell_index = (i == 0) ?
>>> +
>>> (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
>>> + :
>>> (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
>>> + else
>>> + ring->doorbell_index = (i == 0) ?
>>> +
>>> (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE0 << 1)
>>> + :
>>> + (AMDGPU_VEGA10_DOORBELL64_sDMA_HI_PRI_ENGINE1 << 1);
>>> +
>> Copy paste typo? Seems like the if and else are the same.
>>
>> Alex
> Yes, copy paste error. Thanks
>>> + DRM_INFO("paging queue doorbell being set to: [%s]
>>> doorbell index %d\n",
>>> + ring->use_doorbell?"true":"false",
>>> + ring->doorbell_index);
>>>
>>> sprintf(ring->name, "page%d", i);
>>> r = amdgpu_ring_init(adev, ring, 1024,
>>> --
>>> 2.17.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
