[PATCH] drm/amdgpu: Eliminate the set_pde_pte function pointer in amdgpu_gmc_funcs

Christian König ckoenig.leichtzumerken at gmail.com
Tue Feb 26 10:00:59 UTC 2019


Am 25.02.19 um 23:47 schrieb Zhao, Yong:
> All the gmc_*_set_pde_pte functions are the same across different ASICs,
> so we can eliminate the set_pde_pte function pointer and instead use a
> generic function.
>
> Signed-off-by: Yong Zhao <Yong.Zhao at amd.com>

I think I prefer keeping the GMC specific description of the PTEs/PDEs 
in the gmc_v* files.

But apart from that the patch is Reviewed-by: Christian König 
<christian.koenig at amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 59 ++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 10 ++---
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   | 15 -------
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 26 -----------
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 46 -------------------
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 60 -------------------------
>   6 files changed, 62 insertions(+), 154 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index d73367cab4f3..a7b78e740431 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -79,6 +79,65 @@ uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo)
>   	return pd_addr;
>   }
>   
> +/**
> + * amdgpu_gmc_set_pte_pde - update the page tables using CPU
> + *
> + * @adev: amdgpu_device pointer
> + * @cpu_pt_addr: cpu address of the page table
> + * @gpu_page_idx: entry in the page table to update
> + * @addr: dst addr to write into pte/pde
> + * @flags: access flags
> + *
> + * Update the page tables using CPU.
> + */
> +int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr,
> +				uint32_t gpu_page_idx, uint64_t addr,
> +				uint64_t flags)
> +{
> +	void __iomem *ptr = (void *)cpu_pt_addr;
> +	uint64_t value;
> +
> +	/*
> +	 * PTE format on VEGA 10:
> +	 * 63:59 reserved
> +	 * 58:57 mtype
> +	 * 56 F
> +	 * 55 L
> +	 * 54 P
> +	 * 53 SW
> +	 * 52 T
> +	 * 50:48 reserved
> +	 * 47:12 4k physical page base address
> +	 * 11:7 fragment
> +	 * 6 write
> +	 * 5 read
> +	 * 4 exe
> +	 * 3 Z
> +	 * 2 snooped
> +	 * 1 system
> +	 * 0 valid
> +	 *
> +	 * PDE format on VEGA 10:
> +	 * 63:59 block fragment size
> +	 * 58:55 reserved
> +	 * 54 P
> +	 * 53:48 reserved
> +	 * 47:6 physical base address of PD or PTE
> +	 * 5:3 reserved
> +	 * 2 C
> +	 * 1 system
> +	 * 0 valid
> +	 */
> +
> +	/*
> +	 * The following is for PTE only. GART does not have PDEs.
> +	*/
> +	value = addr & 0x0000FFFFFFFFF000ULL;
> +	value |= flags;
> +	writeq(value, ptr + (gpu_page_idx * 8));
> +	return 0;
> +}
> +
>   /**
>    * amdgpu_gmc_agp_addr - return the address in the AGP address space
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 81e6070d255b..d6c10b4d68c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -71,12 +71,6 @@ struct amdgpu_gmc_funcs {
>   	/* Change the VMID -> PASID mapping */
>   	void (*emit_pasid_mapping)(struct amdgpu_ring *ring, unsigned vmid,
>   				   unsigned pasid);
> -	/* write pte/pde updates using the cpu */
> -	int (*set_pte_pde)(struct amdgpu_device *adev,
> -			   void *cpu_pt_addr, /* cpu addr of page table */
> -			   uint32_t gpu_page_idx, /* pte/pde to update */
> -			   uint64_t addr, /* addr to write into pte/pde */
> -			   uint64_t flags); /* access flags */
>   	/* enable/disable PRT support */
>   	void (*set_prt)(struct amdgpu_device *adev, bool enable);
>   	/* set pte flags based per asic */
> @@ -155,7 +149,6 @@ struct amdgpu_gmc {
>   #define amdgpu_gmc_flush_gpu_tlb(adev, vmid, type) (adev)->gmc.gmc_funcs->flush_gpu_tlb((adev), (vmid), (type))
>   #define amdgpu_gmc_emit_flush_gpu_tlb(r, vmid, addr) (r)->adev->gmc.gmc_funcs->emit_flush_gpu_tlb((r), (vmid), (addr))
>   #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
> -#define amdgpu_gmc_set_pte_pde(adev, pt, idx, addr, flags) (adev)->gmc.gmc_funcs->set_pte_pde((adev), (pt), (idx), (addr), (flags))
>   #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
>   #define amdgpu_gmc_get_pte_flags(adev, flags) (adev)->gmc.gmc_funcs->get_vm_pte_flags((adev),(flags))
>   
> @@ -189,6 +182,9 @@ static inline uint64_t amdgpu_gmc_sign_extend(uint64_t addr)
>   
>   void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
>   			       uint64_t *addr, uint64_t *flags);
> +int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr,
> +				uint32_t gpu_page_idx, uint64_t addr,
> +				uint64_t flags);
>   uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);
>   uint64_t amdgpu_gmc_agp_addr(struct ttm_buffer_object *bo);
>   void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 9fc3296592fe..8a812e11a50b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -383,20 +383,6 @@ static uint64_t gmc_v6_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
>   	return pd_addr;
>   }
>   
> -static int gmc_v6_0_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr,
> -				uint32_t gpu_page_idx, uint64_t addr,
> -				uint64_t flags)
> -{
> -	void __iomem *ptr = (void *)cpu_pt_addr;
> -	uint64_t value;
> -
> -	value = addr & 0xFFFFFFFFFFFFF000ULL;
> -	value |= flags;
> -	writeq(value, ptr + (gpu_page_idx * 8));
> -
> -	return 0;
> -}
> -
>   static uint64_t gmc_v6_0_get_vm_pte_flags(struct amdgpu_device *adev,
>   					  uint32_t flags)
>   {
> @@ -1169,7 +1155,6 @@ static const struct amd_ip_funcs gmc_v6_0_ip_funcs = {
>   static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v6_0_flush_gpu_tlb,
>   	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
> -	.set_pte_pde = gmc_v6_0_set_pte_pde,
>   	.set_prt = gmc_v6_0_set_prt,
>   	.get_vm_pde = gmc_v6_0_get_vm_pde,
>   	.get_vm_pte_flags = gmc_v6_0_get_vm_pte_flags
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 761dcfb2fec0..f9f5bef64feb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -460,31 +460,6 @@ static void gmc_v7_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
>   	amdgpu_ring_emit_wreg(ring, mmIH_VMID_0_LUT + vmid, pasid);
>   }
>   
> -/**
> - * gmc_v7_0_set_pte_pde - update the page tables using MMIO
> - *
> - * @adev: amdgpu_device pointer
> - * @cpu_pt_addr: cpu address of the page table
> - * @gpu_page_idx: entry in the page table to update
> - * @addr: dst addr to write into pte/pde
> - * @flags: access flags
> - *
> - * Update the page tables using the CPU.
> - */
> -static int gmc_v7_0_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr,
> -				 uint32_t gpu_page_idx, uint64_t addr,
> -				 uint64_t flags)
> -{
> -	void __iomem *ptr = (void *)cpu_pt_addr;
> -	uint64_t value;
> -
> -	value = addr & 0xFFFFFFFFFFFFF000ULL;
> -	value |= flags;
> -	writeq(value, ptr + (gpu_page_idx * 8));
> -
> -	return 0;
> -}
> -
>   static uint64_t gmc_v7_0_get_vm_pte_flags(struct amdgpu_device *adev,
>   					  uint32_t flags)
>   {
> @@ -1376,7 +1351,6 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v7_0_flush_gpu_tlb,
>   	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
> -	.set_pte_pde = gmc_v7_0_set_pte_pde,
>   	.set_prt = gmc_v7_0_set_prt,
>   	.get_vm_pte_flags = gmc_v7_0_get_vm_pte_flags,
>   	.get_vm_pde = gmc_v7_0_get_vm_pde
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 34440672f938..11135e7ee790 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -662,51 +662,6 @@ static void gmc_v8_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
>   	amdgpu_ring_emit_wreg(ring, mmIH_VMID_0_LUT + vmid, pasid);
>   }
>   
> -/**
> - * gmc_v8_0_set_pte_pde - update the page tables using MMIO
> - *
> - * @adev: amdgpu_device pointer
> - * @cpu_pt_addr: cpu address of the page table
> - * @gpu_page_idx: entry in the page table to update
> - * @addr: dst addr to write into pte/pde
> - * @flags: access flags
> - *
> - * Update the page tables using the CPU.
> - */
> -static int gmc_v8_0_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr,
> -				uint32_t gpu_page_idx, uint64_t addr,
> -				uint64_t flags)
> -{
> -	void __iomem *ptr = (void *)cpu_pt_addr;
> -	uint64_t value;
> -
> -	/*
> -	 * PTE format on VI:
> -	 * 63:40 reserved
> -	 * 39:12 4k physical page base address
> -	 * 11:7 fragment
> -	 * 6 write
> -	 * 5 read
> -	 * 4 exe
> -	 * 3 reserved
> -	 * 2 snooped
> -	 * 1 system
> -	 * 0 valid
> -	 *
> -	 * PDE format on VI:
> -	 * 63:59 block fragment size
> -	 * 58:40 reserved
> -	 * 39:1 physical base address of PTE
> -	 * bits 5:1 must be 0.
> -	 * 0 valid
> -	 */
> -	value = addr & 0x000000FFFFFFF000ULL;
> -	value |= flags;
> -	writeq(value, ptr + (gpu_page_idx * 8));
> -
> -	return 0;
> -}
> -
>   static uint64_t gmc_v8_0_get_vm_pte_flags(struct amdgpu_device *adev,
>   					  uint32_t flags)
>   {
> @@ -1743,7 +1698,6 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v8_0_flush_gpu_tlb,
>   	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
> -	.set_pte_pde = gmc_v8_0_set_pte_pde,
>   	.set_prt = gmc_v8_0_set_prt,
>   	.get_vm_pte_flags = gmc_v8_0_get_vm_pte_flags,
>   	.get_vm_pde = gmc_v8_0_get_vm_pde
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 600259b4e291..4309819ae33f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -466,65 +466,6 @@ static void gmc_v9_0_emit_pasid_mapping(struct amdgpu_ring *ring, unsigned vmid,
>   	amdgpu_ring_emit_wreg(ring, reg, pasid);
>   }
>   
> -/**
> - * gmc_v9_0_set_pte_pde - update the page tables using MMIO
> - *
> - * @adev: amdgpu_device pointer
> - * @cpu_pt_addr: cpu address of the page table
> - * @gpu_page_idx: entry in the page table to update
> - * @addr: dst addr to write into pte/pde
> - * @flags: access flags
> - *
> - * Update the page tables using the CPU.
> - */
> -static int gmc_v9_0_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr,
> -				uint32_t gpu_page_idx, uint64_t addr,
> -				uint64_t flags)
> -{
> -	void __iomem *ptr = (void *)cpu_pt_addr;
> -	uint64_t value;
> -
> -	/*
> -	 * PTE format on VEGA 10:
> -	 * 63:59 reserved
> -	 * 58:57 mtype
> -	 * 56 F
> -	 * 55 L
> -	 * 54 P
> -	 * 53 SW
> -	 * 52 T
> -	 * 50:48 reserved
> -	 * 47:12 4k physical page base address
> -	 * 11:7 fragment
> -	 * 6 write
> -	 * 5 read
> -	 * 4 exe
> -	 * 3 Z
> -	 * 2 snooped
> -	 * 1 system
> -	 * 0 valid
> -	 *
> -	 * PDE format on VEGA 10:
> -	 * 63:59 block fragment size
> -	 * 58:55 reserved
> -	 * 54 P
> -	 * 53:48 reserved
> -	 * 47:6 physical base address of PD or PTE
> -	 * 5:3 reserved
> -	 * 2 C
> -	 * 1 system
> -	 * 0 valid
> -	 */
> -
> -	/*
> -	 * The following is for PTE only. GART does not have PDEs.
> -	*/
> -	value = addr & 0x0000FFFFFFFFF000ULL;
> -	value |= flags;
> -	writeq(value, ptr + (gpu_page_idx * 8));
> -	return 0;
> -}
> -
>   static uint64_t gmc_v9_0_get_vm_pte_flags(struct amdgpu_device *adev,
>   						uint32_t flags)
>   
> @@ -593,7 +534,6 @@ static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>   	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>   	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>   	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
> -	.set_pte_pde = gmc_v9_0_set_pte_pde,
>   	.get_vm_pte_flags = gmc_v9_0_get_vm_pte_flags,
>   	.get_vm_pde = gmc_v9_0_get_vm_pde
>   };



More information about the amd-gfx mailing list