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

[Koenig, Christian]

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)