[PATCH 1/2] drm/amdgpu: Reorganize *_flush_gpu_tlb() for kfd to use

Kuehling, Felix Felix.Kuehling at amd.com
Tue Oct 23 20:37:45 UTC 2018


It occurred to me that the flush_type is a hardware-specific value, but
you're using it in a hardware-abstracted interface. If the meaning of
the flush type values changes in future HW-generations, we'll need to
define an abstract enum that gets translated to the respective HW values
in the HW-specific code.

Anyway, this works for now.

One more cosmetic comment inline ...

Other than that the series is Reviewed-by: Felix Kuehling
<Felix.Kuehling at amd.com>

Regards,
  Felix

On 2018-10-23 2:15 p.m., Zhao, Yong wrote:
> Add a flush_type parameter to that series of functions.
>
> Change-Id: I3dcd71955297c53b181f82e7078981230c642c01
> Signed-off-by: Yong Zhao <Yong.Zhao at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h  |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c    |  5 +++--
>  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c    |  5 +++--
>  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c    |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    | 19 ++++++++++---------
>  6 files changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index 11fea28..9a212aa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -248,7 +248,7 @@ int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>  	}
>  	mb();
>  	amdgpu_asic_flush_hdp(adev, NULL);
> -	amdgpu_gmc_flush_gpu_tlb(adev, 0);
> +	amdgpu_gmc_flush_gpu_tlb(adev, 0, 0);
>  	return 0;
>  }
>  
> @@ -331,7 +331,7 @@ int amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
>  
>  	mb();
>  	amdgpu_asic_flush_hdp(adev, NULL);
> -	amdgpu_gmc_flush_gpu_tlb(adev, 0);
> +	amdgpu_gmc_flush_gpu_tlb(adev, 0, 0);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 6fa7ef4..4c5f18c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -64,7 +64,7 @@ struct amdgpu_vmhub {
>  struct amdgpu_gmc_funcs {
>  	/* flush the vm tlb via mmio */
>  	void (*flush_gpu_tlb)(struct amdgpu_device *adev,
> -			      uint32_t vmid);
> +			      uint32_t vmid, uint32_t flush_type);
>  	/* flush the vm tlb via ring */
>  	uint64_t (*emit_flush_gpu_tlb)(struct amdgpu_ring *ring, unsigned vmid,
>  				       uint64_t pd_addr);
> @@ -151,7 +151,7 @@ struct amdgpu_gmc {
>  	struct amdgpu_xgmi xgmi;
>  };
>  
> -#define amdgpu_gmc_flush_gpu_tlb(adev, vmid) (adev)->gmc.gmc_funcs->flush_gpu_tlb((adev), (vmid))
> +#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))
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index e1c2b4e..2821d1d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -358,7 +358,8 @@ static int gmc_v6_0_mc_init(struct amdgpu_device *adev)
>  	return 0;
>  }
>  
> -static void gmc_v6_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid)
> +static void gmc_v6_0_flush_gpu_tlb(struct amdgpu_device *adev,
> +				uint32_t vmid, uint32_t flush_type)
>  {
>  	WREG32(mmVM_INVALIDATE_REQUEST, 1 << vmid);
>  }
> @@ -580,7 +581,7 @@ static int gmc_v6_0_gart_enable(struct amdgpu_device *adev)
>  	else
>  		gmc_v6_0_set_fault_enable_default(adev, true);
>  
> -	gmc_v6_0_flush_gpu_tlb(adev, 0);
> +	gmc_v6_0_flush_gpu_tlb(adev, 0, 0);
>  	dev_info(adev->dev, "PCIE GART of %uM enabled (table at 0x%016llX).\n",
>  		 (unsigned)(adev->gmc.gart_size >> 20),
>  		 (unsigned long long)table_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 910c4ce..761dcfb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -430,7 +430,8 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>   *
>   * Flush the TLB for the requested page table (CIK).
>   */
> -static void gmc_v7_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid)
> +static void gmc_v7_0_flush_gpu_tlb(struct amdgpu_device *adev,
> +				uint32_t vmid, uint32_t flush_type)
>  {
>  	/* bits 0-15 are the VM contexts0-15 */
>  	WREG32(mmVM_INVALIDATE_REQUEST, 1 << vmid);
> @@ -698,7 +699,7 @@ static int gmc_v7_0_gart_enable(struct amdgpu_device *adev)
>  		WREG32(mmCHUB_CONTROL, tmp);
>  	}
>  
> -	gmc_v7_0_flush_gpu_tlb(adev, 0);
> +	gmc_v7_0_flush_gpu_tlb(adev, 0, 0);
>  	DRM_INFO("PCIE GART of %uM enabled (table at 0x%016llX).\n",
>  		 (unsigned)(adev->gmc.gart_size >> 20),
>  		 (unsigned long long)table_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 1d3265c..531aaf3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -611,7 +611,7 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>   * Flush the TLB for the requested page table (CIK).
>   */
>  static void gmc_v8_0_flush_gpu_tlb(struct amdgpu_device *adev,
> -					uint32_t vmid)
> +				uint32_t vmid, uint32_t flush_type)
>  {
>  	/* bits 0-15 are the VM contexts0-15 */
>  	WREG32(mmVM_INVALIDATE_REQUEST, 1 << vmid);
> @@ -920,7 +920,7 @@ static int gmc_v8_0_gart_enable(struct amdgpu_device *adev)
>  	else
>  		gmc_v8_0_set_fault_enable_default(adev, true);
>  
> -	gmc_v8_0_flush_gpu_tlb(adev, 0);
> +	gmc_v8_0_flush_gpu_tlb(adev, 0, 0);
>  	DRM_INFO("PCIE GART of %uM enabled (table at 0x%016llX).\n",
>  		 (unsigned)(adev->gmc.gart_size >> 20),
>  		 (unsigned long long)table_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index f35d7a5..79d8a84 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -293,14 +293,15 @@ static void gmc_v9_0_set_irq_funcs(struct amdgpu_device *adev)
>  	adev->gmc.vm_fault.funcs = &gmc_v9_0_irq_funcs;
>  }
>  
> -static uint32_t gmc_v9_0_get_invalidate_req(unsigned int vmid)
> +static uint32_t gmc_v9_0_get_invalidate_req(unsigned int vmid,
> +					uint32_t flush_type)
>  {
>  	u32 req = 0;
>  
>  	/* invalidate using legacy mode on vmid*/

This comment is no longer accurate.


>  	req = REG_SET_FIELD(req, VM_INVALIDATE_ENG0_REQ,
>  			    PER_VMID_INVALIDATE_REQ, 1 << vmid);
> -	req = REG_SET_FIELD(req, VM_INVALIDATE_ENG0_REQ, FLUSH_TYPE, 0);
> +	req = REG_SET_FIELD(req, VM_INVALIDATE_ENG0_REQ, FLUSH_TYPE, flush_type);
>  	req = REG_SET_FIELD(req, VM_INVALIDATE_ENG0_REQ, INVALIDATE_L2_PTES, 1);
>  	req = REG_SET_FIELD(req, VM_INVALIDATE_ENG0_REQ, INVALIDATE_L2_PDE0, 1);
>  	req = REG_SET_FIELD(req, VM_INVALIDATE_ENG0_REQ, INVALIDATE_L2_PDE1, 1);
> @@ -362,24 +363,24 @@ static signed long  amdgpu_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
>   */
>  
>  /**
> - * gmc_v9_0_flush_gpu_tlb - gart tlb flush callback
> + * gmc_v9_0_flush_gpu_tlb - tlb flush with certain type
>   *
>   * @adev: amdgpu_device pointer
>   * @vmid: vm instance to flush
> + * @flush_type: the flush type
>   *
> - * Flush the TLB for the requested page table.
> + * Flush the TLB for the requested page table using certain type.
>   */
>  static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
> -					uint32_t vmid)
> +				uint32_t vmid, uint32_t flush_type)
>  {
> -	/* Use register 17 for GART */
>  	const unsigned eng = 17;
>  	unsigned i, j;
>  	int r;
>  
>  	for (i = 0; i < AMDGPU_MAX_VMHUBS; ++i) {
>  		struct amdgpu_vmhub *hub = &adev->vmhub[i];
> -		u32 tmp = gmc_v9_0_get_invalidate_req(vmid);
> +		u32 tmp = gmc_v9_0_get_invalidate_req(vmid, flush_type);
>  
>  		if (adev->gfx.kiq.ring.ready &&
>  		    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev)) &&
> @@ -429,7 +430,7 @@ static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
>  {
>  	struct amdgpu_device *adev = ring->adev;
>  	struct amdgpu_vmhub *hub = &adev->vmhub[ring->funcs->vmhub];
> -	uint32_t req = gmc_v9_0_get_invalidate_req(vmid);
> +	uint32_t req = gmc_v9_0_get_invalidate_req(vmid, 0);
>  	unsigned eng = ring->vm_inv_eng;
>  
>  	amdgpu_ring_emit_wreg(ring, hub->ctx0_ptb_addr_lo32 + (2 * vmid),
> @@ -1122,7 +1123,7 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
>  
>  	gfxhub_v1_0_set_fault_enable_default(adev, value);
>  	mmhub_v1_0_set_fault_enable_default(adev, value);
> -	gmc_v9_0_flush_gpu_tlb(adev, 0);
> +	gmc_v9_0_flush_gpu_tlb(adev, 0, 0);
>  
>  	DRM_INFO("PCIE GART of %uM enabled (table at 0x%016llX).\n",
>  		 (unsigned)(adev->gmc.gart_size >> 20),


More information about the amd-gfx mailing list