[AMD Official Use Only - General]
If aggregated doorbell will not be intended for user queue oversubscription, it's ok to remove it.
Reviewed-by: Jack Xiao <Jack.Xiao@xxxxxxx>
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Alex Deucher
Sent: Friday, October 13, 2023 9:53 PM
To: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH] drm/amdgpu/mes11: remove aggregated doorbell code
Ping?
On Wed, Oct 11, 2023 at 2:27 PM Alex Deucher <alexander.deucher@xxxxxxx> wrote:
>
> It's not enabled in hardware so the code is dead.
> Remove it.
>
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 86
> +++++--------------------- drivers/gpu/drm/amd/amdgpu/mes_v11_0.c |
> 56 ----------------- drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 83
> ++++++++-----------------
> 3 files changed, 40 insertions(+), 185 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index 27b224b0688a..91c07ab4f14e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -5170,45 +5170,17 @@ static u64 gfx_v11_0_ring_get_wptr_gfx(struct
> amdgpu_ring *ring) static void gfx_v11_0_ring_set_wptr_gfx(struct
> amdgpu_ring *ring) {
> struct amdgpu_device *adev = ring->adev;
> - uint32_t *wptr_saved;
> - uint32_t *is_queue_unmap;
> - uint64_t aggregated_db_index;
> - uint32_t mqd_size = adev->mqds[AMDGPU_HW_IP_GFX].mqd_size;
> - uint64_t wptr_tmp;
>
> - if (ring->is_mes_queue) {
> - wptr_saved = (uint32_t *)(ring->mqd_ptr + mqd_size);
> - is_queue_unmap = (uint32_t *)(ring->mqd_ptr + mqd_size +
> - sizeof(uint32_t));
> - aggregated_db_index =
> - amdgpu_mes_get_aggregated_doorbell_index(adev,
> - ring->hw_prio);
> -
> - wptr_tmp = ring->wptr & ring->buf_mask;
> - atomic64_set((atomic64_t *)ring->wptr_cpu_addr, wptr_tmp);
> - *wptr_saved = wptr_tmp;
> - /* assume doorbell always being used by mes mapped queue */
> - if (*is_queue_unmap) {
> - WDOORBELL64(aggregated_db_index, wptr_tmp);
> - WDOORBELL64(ring->doorbell_index, wptr_tmp);
> - } else {
> - WDOORBELL64(ring->doorbell_index, wptr_tmp);
> -
> - if (*is_queue_unmap)
> - WDOORBELL64(aggregated_db_index, wptr_tmp);
> - }
> + if (ring->use_doorbell) {
> + /* XXX check if swapping is necessary on BE */
> + atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
> + ring->wptr);
> + WDOORBELL64(ring->doorbell_index, ring->wptr);
> } else {
> - if (ring->use_doorbell) {
> - /* XXX check if swapping is necessary on BE */
> - atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
> - ring->wptr);
> - WDOORBELL64(ring->doorbell_index, ring->wptr);
> - } else {
> - WREG32_SOC15(GC, 0, regCP_RB0_WPTR,
> - lower_32_bits(ring->wptr));
> - WREG32_SOC15(GC, 0, regCP_RB0_WPTR_HI,
> - upper_32_bits(ring->wptr));
> - }
> + WREG32_SOC15(GC, 0, regCP_RB0_WPTR,
> + lower_32_bits(ring->wptr));
> + WREG32_SOC15(GC, 0, regCP_RB0_WPTR_HI,
> + upper_32_bits(ring->wptr));
> }
> }
>
> @@ -5233,42 +5205,14 @@ static u64
> gfx_v11_0_ring_get_wptr_compute(struct amdgpu_ring *ring) static void
> gfx_v11_0_ring_set_wptr_compute(struct amdgpu_ring *ring) {
> struct amdgpu_device *adev = ring->adev;
> - uint32_t *wptr_saved;
> - uint32_t *is_queue_unmap;
> - uint64_t aggregated_db_index;
> - uint32_t mqd_size = adev->mqds[AMDGPU_HW_IP_COMPUTE].mqd_size;
> - uint64_t wptr_tmp;
>
> - if (ring->is_mes_queue) {
> - wptr_saved = (uint32_t *)(ring->mqd_ptr + mqd_size);
> - is_queue_unmap = (uint32_t *)(ring->mqd_ptr + mqd_size +
> - sizeof(uint32_t));
> - aggregated_db_index =
> - amdgpu_mes_get_aggregated_doorbell_index(adev,
> - ring->hw_prio);
> -
> - wptr_tmp = ring->wptr & ring->buf_mask;
> - atomic64_set((atomic64_t *)ring->wptr_cpu_addr, wptr_tmp);
> - *wptr_saved = wptr_tmp;
> - /* assume doorbell always used by mes mapped queue */
> - if (*is_queue_unmap) {
> - WDOORBELL64(aggregated_db_index, wptr_tmp);
> - WDOORBELL64(ring->doorbell_index, wptr_tmp);
> - } else {
> - WDOORBELL64(ring->doorbell_index, wptr_tmp);
> -
> - if (*is_queue_unmap)
> - WDOORBELL64(aggregated_db_index, wptr_tmp);
> - }
> + /* XXX check if swapping is necessary on BE */
> + if (ring->use_doorbell) {
> + atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
> + ring->wptr);
> + WDOORBELL64(ring->doorbell_index, ring->wptr);
> } else {
> - /* XXX check if swapping is necessary on BE */
> - if (ring->use_doorbell) {
> - atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
> - ring->wptr);
> - WDOORBELL64(ring->doorbell_index, ring->wptr);
> - } else {
> - BUG(); /* only DOORBELL method supported on gfx11 now */
> - }
> + BUG(); /* only DOORBELL method supported on gfx11 now
> + */
> }
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> index 31b26e6f0b30..4dfec56e1b7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> @@ -414,60 +414,6 @@ static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes)
> offsetof(union MESAPI_SET_HW_RESOURCES,
> api_status)); }
>
> -static void mes_v11_0_init_aggregated_doorbell(struct amdgpu_mes
> *mes) -{
> - struct amdgpu_device *adev = mes->adev;
> - uint32_t data;
> -
> - data = RREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL1);
> - data &= ~(CP_MES_DOORBELL_CONTROL1__DOORBELL_OFFSET_MASK |
> - CP_MES_DOORBELL_CONTROL1__DOORBELL_EN_MASK |
> - CP_MES_DOORBELL_CONTROL1__DOORBELL_HIT_MASK);
> - data |= mes->aggregated_doorbells[AMDGPU_MES_PRIORITY_LEVEL_LOW] <<
> - CP_MES_DOORBELL_CONTROL1__DOORBELL_OFFSET__SHIFT;
> - data |= 1 << CP_MES_DOORBELL_CONTROL1__DOORBELL_EN__SHIFT;
> - WREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL1, data);
> -
> - data = RREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL2);
> - data &= ~(CP_MES_DOORBELL_CONTROL2__DOORBELL_OFFSET_MASK |
> - CP_MES_DOORBELL_CONTROL2__DOORBELL_EN_MASK |
> - CP_MES_DOORBELL_CONTROL2__DOORBELL_HIT_MASK);
> - data |= mes->aggregated_doorbells[AMDGPU_MES_PRIORITY_LEVEL_NORMAL] <<
> - CP_MES_DOORBELL_CONTROL2__DOORBELL_OFFSET__SHIFT;
> - data |= 1 << CP_MES_DOORBELL_CONTROL2__DOORBELL_EN__SHIFT;
> - WREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL2, data);
> -
> - data = RREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL3);
> - data &= ~(CP_MES_DOORBELL_CONTROL3__DOORBELL_OFFSET_MASK |
> - CP_MES_DOORBELL_CONTROL3__DOORBELL_EN_MASK |
> - CP_MES_DOORBELL_CONTROL3__DOORBELL_HIT_MASK);
> - data |= mes->aggregated_doorbells[AMDGPU_MES_PRIORITY_LEVEL_MEDIUM] <<
> - CP_MES_DOORBELL_CONTROL3__DOORBELL_OFFSET__SHIFT;
> - data |= 1 << CP_MES_DOORBELL_CONTROL3__DOORBELL_EN__SHIFT;
> - WREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL3, data);
> -
> - data = RREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL4);
> - data &= ~(CP_MES_DOORBELL_CONTROL4__DOORBELL_OFFSET_MASK |
> - CP_MES_DOORBELL_CONTROL4__DOORBELL_EN_MASK |
> - CP_MES_DOORBELL_CONTROL4__DOORBELL_HIT_MASK);
> - data |= mes->aggregated_doorbells[AMDGPU_MES_PRIORITY_LEVEL_HIGH] <<
> - CP_MES_DOORBELL_CONTROL4__DOORBELL_OFFSET__SHIFT;
> - data |= 1 << CP_MES_DOORBELL_CONTROL4__DOORBELL_EN__SHIFT;
> - WREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL4, data);
> -
> - data = RREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL5);
> - data &= ~(CP_MES_DOORBELL_CONTROL5__DOORBELL_OFFSET_MASK |
> - CP_MES_DOORBELL_CONTROL5__DOORBELL_EN_MASK |
> - CP_MES_DOORBELL_CONTROL5__DOORBELL_HIT_MASK);
> - data |= mes->aggregated_doorbells[AMDGPU_MES_PRIORITY_LEVEL_REALTIME] <<
> - CP_MES_DOORBELL_CONTROL5__DOORBELL_OFFSET__SHIFT;
> - data |= 1 << CP_MES_DOORBELL_CONTROL5__DOORBELL_EN__SHIFT;
> - WREG32_SOC15(GC, 0, regCP_MES_DOORBELL_CONTROL5, data);
> -
> - data = 1 << CP_HQD_GFX_CONTROL__DB_UPDATED_MSG_EN__SHIFT;
> - WREG32_SOC15(GC, 0, regCP_HQD_GFX_CONTROL, data);
> -}
> -
> static const struct amdgpu_mes_funcs mes_v11_0_funcs = {
> .add_hw_queue = mes_v11_0_add_hw_queue,
> .remove_hw_queue = mes_v11_0_remove_hw_queue, @@ -1243,8
> +1189,6 @@ static int mes_v11_0_hw_init(void *handle)
> if (r)
> goto failure;
>
> - mes_v11_0_init_aggregated_doorbell(&adev->mes);
> -
> r = mes_v11_0_query_sched_status(&adev->mes);
> if (r) {
> DRM_ERROR("MES is busy\n"); diff --git
> a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> index 1f8122fd1ad7..7e4d5188cbfa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> @@ -156,68 +156,35 @@ static uint64_t sdma_v6_0_ring_get_wptr(struct
> amdgpu_ring *ring) static void sdma_v6_0_ring_set_wptr(struct
> amdgpu_ring *ring) {
> struct amdgpu_device *adev = ring->adev;
> - uint32_t *wptr_saved;
> - uint32_t *is_queue_unmap;
> - uint64_t aggregated_db_index;
> - uint32_t mqd_size = adev->mqds[AMDGPU_HW_IP_DMA].mqd_size;
> -
> - DRM_DEBUG("Setting write pointer\n");
> -
> - if (ring->is_mes_queue) {
> - wptr_saved = (uint32_t *)(ring->mqd_ptr + mqd_size);
> - is_queue_unmap = (uint32_t *)(ring->mqd_ptr + mqd_size +
> - sizeof(uint32_t));
> - aggregated_db_index =
> - amdgpu_mes_get_aggregated_doorbell_index(adev,
> - ring->hw_prio);
>
> + if (ring->use_doorbell) {
> + DRM_DEBUG("Using doorbell -- "
> + "wptr_offs == 0x%08x "
> + "lower_32_bits(ring->wptr) << 2 == 0x%08x "
> + "upper_32_bits(ring->wptr) << 2 == 0x%08x\n",
> + ring->wptr_offs,
> + lower_32_bits(ring->wptr << 2),
> + upper_32_bits(ring->wptr << 2));
> + /* XXX check if swapping is necessary on BE */
> atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
> ring->wptr << 2);
> - *wptr_saved = ring->wptr << 2;
> - if (*is_queue_unmap) {
> - WDOORBELL64(aggregated_db_index, ring->wptr << 2);
> - DRM_DEBUG("calling WDOORBELL64(0x%08x, 0x%016llx)\n",
> - ring->doorbell_index, ring->wptr << 2);
> - WDOORBELL64(ring->doorbell_index, ring->wptr << 2);
> - } else {
> - DRM_DEBUG("calling WDOORBELL64(0x%08x, 0x%016llx)\n",
> - ring->doorbell_index, ring->wptr << 2);
> - WDOORBELL64(ring->doorbell_index, ring->wptr << 2);
> -
> - if (*is_queue_unmap)
> - WDOORBELL64(aggregated_db_index,
> - ring->wptr << 2);
> - }
> + DRM_DEBUG("calling WDOORBELL64(0x%08x, 0x%016llx)\n",
> + ring->doorbell_index, ring->wptr << 2);
> + WDOORBELL64(ring->doorbell_index, ring->wptr << 2);
> } else {
> - if (ring->use_doorbell) {
> - DRM_DEBUG("Using doorbell -- "
> - "wptr_offs == 0x%08x "
> - "lower_32_bits(ring->wptr) << 2 == 0x%08x "
> - "upper_32_bits(ring->wptr) << 2 == 0x%08x\n",
> - ring->wptr_offs,
> - lower_32_bits(ring->wptr << 2),
> - upper_32_bits(ring->wptr << 2));
> - /* XXX check if swapping is necessary on BE */
> - atomic64_set((atomic64_t *)ring->wptr_cpu_addr,
> - ring->wptr << 2);
> - DRM_DEBUG("calling WDOORBELL64(0x%08x, 0x%016llx)\n",
> - ring->doorbell_index, ring->wptr << 2);
> - WDOORBELL64(ring->doorbell_index, ring->wptr << 2);
> - } else {
> - DRM_DEBUG("Not using doorbell -- "
> - "regSDMA%i_GFX_RB_WPTR == 0x%08x "
> - "regSDMA%i_GFX_RB_WPTR_HI == 0x%08x\n",
> - ring->me,
> - lower_32_bits(ring->wptr << 2),
> - ring->me,
> - upper_32_bits(ring->wptr << 2));
> - WREG32_SOC15_IP(GC, sdma_v6_0_get_reg_offset(adev,
> - ring->me, regSDMA0_QUEUE0_RB_WPTR),
> - lower_32_bits(ring->wptr << 2));
> - WREG32_SOC15_IP(GC, sdma_v6_0_get_reg_offset(adev,
> - ring->me, regSDMA0_QUEUE0_RB_WPTR_HI),
> - upper_32_bits(ring->wptr << 2));
> - }
> + DRM_DEBUG("Not using doorbell -- "
> + "regSDMA%i_GFX_RB_WPTR == 0x%08x "
> + "regSDMA%i_GFX_RB_WPTR_HI == 0x%08x\n",
> + ring->me,
> + lower_32_bits(ring->wptr << 2),
> + ring->me,
> + upper_32_bits(ring->wptr << 2));
> + WREG32_SOC15_IP(GC, sdma_v6_0_get_reg_offset(adev,
> + ring->me, regSDMA0_QUEUE0_RB_WPTR),
> + lower_32_bits(ring->wptr << 2));
> + WREG32_SOC15_IP(GC, sdma_v6_0_get_reg_offset(adev,
> + ring->me, regSDMA0_QUEUE0_RB_WPTR_HI),
> + upper_32_bits(ring->wptr << 2));
> }
> }
>
> --
> 2.41.0
>
