On 2018-11-15 11:43 a.m., Alex Deucher wrote: > On Thu, Nov 15, 2018 at 11:08 AM Yang, Philip <Philip.Yang@xxxxxxx> 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.Yang@xxxxxxx> > 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@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx