On 2018-11-16 4:35 p.m., Alex Deucher wrote: > On Fri, Nov 16, 2018 at 2:08 PM Yang, Philip <Philip.Yang@xxxxxxx> wrote: >> Because increase SDMA_DOORBELL_RANGE to add new SDMA doorbell for paging queue will >> break SRIOV, instead we can reserve and map two doorbell pages for amdgpu, paging >> queues doorbell index use same index as SDMA gfx queues index but on second page. >> >> For Vega20, after we change doorbell layout to increase SDMA doorbell for 8 SDMA RLC >> queues later, we could use new doorbell index for paging queue. >> >> Change-Id: I9adb965f16ee4089d261d9a22231337739184e49 >> Signed-off-by: Philip Yang <Philip.Yang@xxxxxxx> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 13 +++++++++ >> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 33 +++++++++++++--------- >> 2 files changed, 33 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index 590588a82471..7a54760591ae 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -534,6 +534,19 @@ static int amdgpu_device_doorbell_init(struct amdgpu_device *adev) >> if (adev->doorbell.num_doorbells == 0) >> return -EINVAL; >> >> + /* For Vega, reserve and map two pages on doorbell BAR since SDMA >> + * paging queue doorbell use the second page >> + */ >> + switch (adev->asic_type) { >> + case CHIP_VEGA10: >> + case CHIP_VEGA12: >> + case CHIP_VEGA20: >> + adev->doorbell.num_doorbells *= 2; >> + break; >> + default: >> + break; >> + } >> + > To future proof this, maybe we should just change this to: > if (asic_type >= VEGA10) > number_doorbells *= 2; > OK >> adev->doorbell.ptr = ioremap(adev->doorbell.base, >> adev->doorbell.num_doorbells * >> sizeof(u32)); >> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c >> index f4490cdd9804..db5a71db9f10 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); > This looks like a bug fix that should be a separate patch. OK >> /* set minor_ptr_update to 0 after wptr programed */ >> WREG32_SDMA(i, mmSDMA0_PAGE_MINOR_PTR_UPDATE, 0); >> @@ -1504,18 +1502,14 @@ 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 >> - : (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset >> + (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) // doorbell size 2 dwords >> + : (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1); // doorbell size 2 dwords >> else >> ring->doorbell_index = (i == 0) ? >> - (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) //get DWORD offset >> - : (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1); // get DWORD offset >> - >> + (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) // doorbell size 2 dwords >> + : (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1); // doorbell size 2 dwords >> >> sprintf(ring->name, "sdma%d", i); >> r = amdgpu_ring_init(adev, ring, 1024, >> @@ -1529,7 +1523,20 @@ 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; >> + >> + /* paging queue use same doorbell index/routing as gfx queue >> + * with 0x400 (4096 dwords) offset on second doorbell page >> + */ >> + if (adev->asic_type == CHIP_VEGA10) >> + ring->doorbell_index = (i == 0) ? >> + (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE0 << 1) >> + : (AMDGPU_VEGA10_DOORBELL64_sDMA_ENGINE1 << 1); >> + else >> + ring->doorbell_index = (i == 0) ? >> + (AMDGPU_DOORBELL64_sDMA_ENGINE0 << 1) >> + : (AMDGPU_DOORBELL64_sDMA_ENGINE1 << 1); >> + ring->doorbell_index += 0x400; > I don't quite understand how this works. Why don't we have to adjust > the doorbell range registers in the nbio code? > > Alex nbio doorbell routing use low bits (11bits or 12bits?), so doorbell index 0x1E0 and 0x5E0 (dwords address 0x780 and 0x1780) will both route to same SDMA engine0. Philip >> sprintf(ring->name, "page%d", i); >> r = amdgpu_ring_init(adev, ring, 1024, >> -- >> 2.17.1 >> >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx