[PATCH 2/3] drm/amdgpu: Expose *_setup_vm_pt_regs for kfd to use

[Zhao, Yong]

Fixed as suggested, added the descriptions, and pushed them. Thanks.

Yong
________________________________
From: Koenig, Christian
Sent: Friday, October 19, 2018 3:14:14 AM
To: Zhao, Yong; amd-gfx at lists.freedesktop.org; brahma_sw_dev
Subject: Re: [PATCH 2/3] drm/amdgpu: Expose *_setup_vm_pt_regs for kfd to use

Am 18.10.18 um 23:13 schrieb Zhao, Yong:
> Change-Id: I2ea27c4749a454506fecf75bb5b78b09bde9cb28
> Signed-off-by: Yong Zhao <Yong.Zhao at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 20 +++++++++++++++-----
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h    |  6 ++++++
>   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  | 20 +++++++++++++++-----
>   3 files changed, 36 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> index ceb7847..34145a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> @@ -35,15 +35,25 @@ u64 gfxhub_v1_0_get_mc_fb_offset(struct amdgpu_device *adev)
>        return (u64)RREG32_SOC15(GC, 0, mmMC_VM_FB_OFFSET) << 24;
>   }
>
> +void gfxhub_v1_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
> +                             uint64_t value)

I would rather give the parameter some meaningful name, e.g.
page_table_base or similar.

> +{
> +     /* two registers distance between mmVM_CONTEXT0_* to mmVM_CONTEXT1_* */
> +     int offset = mmVM_CONTEXT1_PAGE_TABLE_BASE_ADDR_LO32
> +                     - mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32;
> +
> +     WREG32_SOC15_OFFSET(GC, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32,
> +                             offset * vmid, lower_32_bits(value));
> +
> +     WREG32_SOC15_OFFSET(GC, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32,
> +                             offset * vmid, upper_32_bits(value));
> +}
> +
>   static void gfxhub_v1_0_init_gart_pt_regs(struct amdgpu_device *adev)
>   {
>        uint64_t value = amdgpu_gmc_pd_addr(adev->gart.bo);
>
> -     WREG32_SOC15(GC, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32,
> -                  lower_32_bits(value));
> -
> -     WREG32_SOC15(GC, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32,
> -                  upper_32_bits(value));
> +     gfxhub_v1_0_setup_vm_pt_regs(adev, 0, value);

Since that function is now a mostly meaningless wrapper I would just go
ahead and call gfxhub_v1_0_setup_vm_pt_regs directly as long as it
doesn't duplicates the code to much.

Same applies of course to the mmhub variant.

Apart from that looks good to me,
Christian.

>   }
>
>   static void gfxhub_v1_0_init_gart_aperture_regs(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
> index b030ca5..008ab08 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
> @@ -27,4 +27,10 @@
>   extern const struct amd_ip_funcs gmc_v9_0_ip_funcs;
>   extern const struct amdgpu_ip_block_version gmc_v9_0_ip_block;
>
> +/* amdgpu_amdkfd*.c */
> +void gfxhub_v1_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
> +                             uint64_t value);
> +void mmhub_v1_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
> +                             uint64_t value);
> +
>   #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> index 14649f8..8e18be0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> @@ -52,15 +52,25 @@ u64 mmhub_v1_0_get_fb_location(struct amdgpu_device *adev)
>        return base;
>   }
>
> +void mmhub_v1_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
> +                             uint64_t value)
> +{
> +     /* two registers distance between mmVM_CONTEXT0_* to mmVM_CONTEXT1_* */
> +     int offset = mmVM_CONTEXT1_PAGE_TABLE_BASE_ADDR_LO32
> +                     - mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32;
> +
> +     WREG32_SOC15_OFFSET(MMHUB, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32,
> +                     offset * vmid, lower_32_bits(value));
> +
> +     WREG32_SOC15_OFFSET(MMHUB, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32,
> +                     offset * vmid, upper_32_bits(value));
> +}
> +
>   static void mmhub_v1_0_init_gart_pt_regs(struct amdgpu_device *adev)
>   {
>        uint64_t value = amdgpu_gmc_pd_addr(adev->gart.bo);
>
> -     WREG32_SOC15(MMHUB, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32,
> -                  lower_32_bits(value));
> -
> -     WREG32_SOC15(MMHUB, 0, mmVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32,
> -                  upper_32_bits(value));
> +     mmhub_v1_0_setup_vm_pt_regs(adev, 0, value);
>   }
>
>   static void mmhub_v1_0_init_gart_aperture_regs(struct amdgpu_device *adev)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20181019/f65f5033/attachment-0001.html>