[PATCH] drm/amd/amdgpu: fix the inst passed to reg read write under sriov

Zhao, Victor Victor.Zhao at amd.com
Mon May 20 16:08:35 UTC 2024


[AMD Official Use Only - AMD Internal Distribution Only]

Thanks for pointing that out. You are right, kiq is taking logical xcc id and should not be affected.
I'm sending out a new patch, please help review again.

Thanks,
Victor

-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar at amd.com>
Sent: Monday, May 20, 2024 8:02 PM
To: Zhao, Victor <Victor.Zhao at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/amdgpu: fix the inst passed to reg read write under sriov



On 5/20/2024 4:44 PM, Victor Zhao wrote:
> the inst passed to reg read/write should be physical instance.
> Fix the miss matched code.
>
> Signed-off-by: Victor Zhao <Victor.Zhao at amd.com>
> ---
>  .../drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c   |  6 ++---
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  2 +-
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c       |  8 +++---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c         | 26 +++++++++----------
>  4 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
> index a5c7259cf2a3..319e6793053a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gc_9_4_3.c
> @@ -300,7 +300,7 @@ static int kgd_gfx_v9_4_3_hqd_load(struct amdgpu_device *adev, void *mqd,
>       hqd_end = SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
> regCP_HQD_AQL_DISPATCH_ID_HI);
>
>       for (reg = hqd_base; reg <= hqd_end; reg++)
> -             WREG32_XCC(reg, mqd_hqd[reg - hqd_base], inst);
> +             WREG32_XCC(reg, mqd_hqd[reg - hqd_base], GET_INST(GC, inst));

Why this needs to be done? Isn't the expectation that it goes to the right KIQ/RLCG as those are also indexed by logical XCC ids?

Thanks,
Lijo

>
>
>       /* Activate doorbell logic before triggering WPTR poll. */ @@
> -493,12 +493,12 @@ static uint32_t kgd_gfx_v9_4_3_set_address_watch(
>       WREG32_XCC((SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
>                       regTCP_WATCH0_ADDR_H) +
>                       (watch_id * TCP_WATCH_STRIDE)),
> -                     watch_address_high, inst);
> +                     watch_address_high, GET_INST(GC, inst));
>
>       WREG32_XCC((SOC15_REG_OFFSET(GC, GET_INST(GC, inst),
>                       regTCP_WATCH0_ADDR_L) +
>                       (watch_id * TCP_WATCH_STRIDE)),
> -                     watch_address_low, inst);
> +                     watch_address_low, GET_INST(GC, inst));
>
>       return watch_address_cntl;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> index 5a35a8ca8922..76be23dcea31 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> @@ -239,7 +239,7 @@ int kgd_gfx_v9_hqd_load(struct amdgpu_device
> *adev, void *mqd,
>
>       for (reg = hqd_base;
>            reg <= SOC15_REG_OFFSET(GC, GET_INST(GC, inst), mmCP_HQD_PQ_WPTR_HI); reg++)
> -             WREG32_XCC(reg, mqd_hqd[reg - hqd_base], inst);
> +             WREG32_XCC(reg, mqd_hqd[reg - hqd_base], GET_INST(GC, inst));
>
>
>       /* Activate doorbell logic before triggering WPTR poll. */ diff
> --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> index 07b299ec7169..349ece5a27ee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c
> @@ -2812,16 +2812,16 @@ static void
> gfx_v9_4_3_xcc_set_compute_eop_interrupt_state(
>
>       switch (state) {
>       case AMDGPU_IRQ_STATE_DISABLE:
> -             mec_int_cntl = RREG32_XCC(mec_int_cntl_reg, xcc_id);
> +             mec_int_cntl = RREG32_XCC(mec_int_cntl_reg, GET_INST(GC, xcc_id));
>               mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
>                                            TIME_STAMP_INT_ENABLE, 0);
> -             WREG32_XCC(mec_int_cntl_reg, mec_int_cntl, xcc_id);
> +             WREG32_XCC(mec_int_cntl_reg, mec_int_cntl, GET_INST(GC, xcc_id));
>               break;
>       case AMDGPU_IRQ_STATE_ENABLE:
> -             mec_int_cntl = RREG32_XCC(mec_int_cntl_reg, xcc_id);
> +             mec_int_cntl = RREG32_XCC(mec_int_cntl_reg, GET_INST(GC, xcc_id));
>               mec_int_cntl = REG_SET_FIELD(mec_int_cntl, CP_ME1_PIPE0_INT_CNTL,
>                                            TIME_STAMP_INT_ENABLE, 1);
> -             WREG32_XCC(mec_int_cntl_reg, mec_int_cntl, xcc_id);
> +             WREG32_XCC(mec_int_cntl_reg, mec_int_cntl, GET_INST(GC, xcc_id));
>               break;
>       default:
>               break;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 094c08cb98e7..aca842668c56 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -496,14 +496,14 @@ static int gmc_v9_0_vm_fault_interrupt_state(struct amdgpu_device *adev,
>                               if (j >= AMDGPU_MMHUB0(0))
>                                       tmp = RREG32_SOC15_IP(MMHUB, reg);
>                               else
> -                                     tmp = RREG32_XCC(reg, j);
> +                                     tmp = RREG32_XCC(reg, GET_INST(GC, j));
>
>                               tmp &= ~bits;
>
>                               if (j >= AMDGPU_MMHUB0(0))
>                                       WREG32_SOC15_IP(MMHUB, reg, tmp);
>                               else
> -                                     WREG32_XCC(reg, tmp, j);
> +                                     WREG32_XCC(reg, tmp, GET_INST(GC, j));
>                       }
>               }
>               break;
> @@ -524,14 +524,14 @@ static int gmc_v9_0_vm_fault_interrupt_state(struct amdgpu_device *adev,
>                               if (j >= AMDGPU_MMHUB0(0))
>                                       tmp = RREG32_SOC15_IP(MMHUB, reg);
>                               else
> -                                     tmp = RREG32_XCC(reg, j);
> +                                     tmp = RREG32_XCC(reg, GET_INST(GC, j));
>
>                               tmp |= bits;
>
>                               if (j >= AMDGPU_MMHUB0(0))
>                                       WREG32_SOC15_IP(MMHUB, reg, tmp);
>                               else
> -                                     WREG32_XCC(reg, tmp, j);
> +                                     WREG32_XCC(reg, tmp, GET_INST(GC, j));
>                       }
>               }
>               break;
> @@ -844,7 +844,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>       ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
>
>       if (vmhub >= AMDGPU_MMHUB0(0))
> -             inst = GET_INST(GC, 0);
> +             inst = 0;
>       else
>               inst = vmhub;
>
> @@ -876,9 +876,9 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>               for (j = 0; j < adev->usec_timeout; j++) {
>                       /* a read return value of 1 means semaphore acquire */
>                       if (vmhub >= AMDGPU_MMHUB0(0))
> -                             tmp = RREG32_SOC15_IP_NO_KIQ(MMHUB, sem, inst);
> +                             tmp = RREG32_SOC15_IP_NO_KIQ(MMHUB, sem, GET_INST(GC, inst));
>                       else
> -                             tmp = RREG32_SOC15_IP_NO_KIQ(GC, sem, inst);
> +                             tmp = RREG32_SOC15_IP_NO_KIQ(GC, sem, GET_INST(GC, inst));
>                       if (tmp & 0x1)
>                               break;
>                       udelay(1);
> @@ -889,9 +889,9 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>       }
>
>       if (vmhub >= AMDGPU_MMHUB0(0))
> -             WREG32_SOC15_IP_NO_KIQ(MMHUB, req, inv_req, inst);
> +             WREG32_SOC15_IP_NO_KIQ(MMHUB, req, inv_req, GET_INST(GC, inst));
>       else
> -             WREG32_SOC15_IP_NO_KIQ(GC, req, inv_req, inst);
> +             WREG32_SOC15_IP_NO_KIQ(GC, req, inv_req, GET_INST(GC, inst));
>
>       /*
>        * Issue a dummy read to wait for the ACK register to @@ -904,9
> +904,9 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device
> *adev, uint32_t vmid,
>
>       for (j = 0; j < adev->usec_timeout; j++) {
>               if (vmhub >= AMDGPU_MMHUB0(0))
> -                     tmp = RREG32_SOC15_IP_NO_KIQ(MMHUB, ack, inst);
> +                     tmp = RREG32_SOC15_IP_NO_KIQ(MMHUB, ack, GET_INST(GC, inst));
>               else
> -                     tmp = RREG32_SOC15_IP_NO_KIQ(GC, ack, inst);
> +                     tmp = RREG32_SOC15_IP_NO_KIQ(GC, ack, GET_INST(GC, inst));
>               if (tmp & (1 << vmid))
>                       break;
>               udelay(1);
> @@ -919,9 +919,9 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>                * write with 0 means semaphore release
>                */
>               if (vmhub >= AMDGPU_MMHUB0(0))
> -                     WREG32_SOC15_IP_NO_KIQ(MMHUB, sem, 0, inst);
> +                     WREG32_SOC15_IP_NO_KIQ(MMHUB, sem, 0, GET_INST(GC, inst));
>               else
> -                     WREG32_SOC15_IP_NO_KIQ(GC, sem, 0, inst);
> +                     WREG32_SOC15_IP_NO_KIQ(GC, sem, 0, GET_INST(GC, inst));
>       }
>
>       spin_unlock(&adev->gmc.invalidate_lock);


More information about the amd-gfx mailing list