Re: [PATCH 2/3] drm/amdgpu: Expose gmc_v9_0_flush_gpu_tlb_helper() for kfd to use

["Koenig, Christian" <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>]

Am 22.10.18 um 20:41 schrieb Zhao, Yong:
> Change-Id: I3dcd71955297c53b181f82e7078981230c642c01
> Signed-off-by: Yong Zhao <Yong.Zhao@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 64 ++++++++++++++++++++---------------
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h |  3 ++
>   2 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index f35d7a5..6f96545 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*/
>   	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);
> @@ -354,32 +355,15 @@ static signed long  amdgpu_kiq_reg_write_reg_wait(struct amdgpu_device *adev,
>   	return r;
>   }
>   
> -/*
> - * GART
> - * VMID 0 is the physical GPU addresses as used by the kernel.
> - * VMIDs 1-15 are used for userspace clients and are handled
> - * by the amdgpu vm/hsa code.
> - */
> -
> -/**
> - * gmc_v9_0_flush_gpu_tlb - gart tlb flush callback
> - *
> - * @adev: amdgpu_device pointer
> - * @vmid: vm instance to flush
> - *
> - * Flush the TLB for the requested page table.
> - */
> -static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
> -					uint32_t vmid)
> +void gmc_v9_0_flush_gpu_tlb_helper(struct amdgpu_device *adev, uint32_t vmid,
> +				uint32_t flush_type, uint32_t eng, bool lock)

This one needs kernel documentation and a better name would be nice.

In general sounds like a nice cleanup, but I exposing ASIC specific 
functions is not necessarily a good idea.

We should probably move the ASIC specific KFD code into this file instead.

Christian.

>   {
> -	/* 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)) &&
> @@ -390,7 +374,8 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
>   				continue;
>   		}
>   
> -		spin_lock(&adev->gmc.invalidate_lock);
> +		if (lock)
> +			spin_lock(&adev->gmc.invalidate_lock);
>   
>   		WREG32_NO_KIQ(hub->vm_inv_eng0_req + eng, tmp);
>   
> @@ -403,7 +388,8 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
>   			cpu_relax();
>   		}
>   		if (j < 100) {
> -			spin_unlock(&adev->gmc.invalidate_lock);
> +			if (lock)
> +				spin_unlock(&adev->gmc.invalidate_lock);
>   			continue;
>   		}
>   
> @@ -416,20 +402,44 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
>   			udelay(1);
>   		}
>   		if (j < adev->usec_timeout) {
> -			spin_unlock(&adev->gmc.invalidate_lock);
> +			if (lock)
> +				spin_unlock(&adev->gmc.invalidate_lock);
>   			continue;
>   		}
> -		spin_unlock(&adev->gmc.invalidate_lock);
> +		if (lock)
> +			spin_unlock(&adev->gmc.invalidate_lock);
>   		DRM_ERROR("Timeout waiting for VM flush ACK!\n");
>   	}
>   }
>   
> +/*
> + * GART
> + * VMID 0 is the physical GPU addresses as used by the kernel.
> + * VMIDs 1-15 are used for userspace clients and are handled
> + * by the amdgpu vm/hsa code.
> + */
> +
> +/**
> + * gmc_v9_0_flush_gpu_tlb - gart tlb flush callback
> + *
> + * @adev: amdgpu_device pointer
> + * @vmid: vm instance to flush
> + *
> + * Flush the TLB for the requested page table.
> + */
> +static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev,
> +					uint32_t vmid)
> +{
> +	/* Use engine 17 for amdgpu */
> +	gmc_v9_0_flush_gpu_tlb_helper(adev, vmid, 0, 17, true);
> +}
> +
>   static uint64_t gmc_v9_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
>   					    unsigned vmid, uint64_t pd_addr)
>   {
>   	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),
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
> index 1fd178a6..56f504a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.h
> @@ -33,4 +33,7 @@ void gfxhub_v1_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
>   void mmhub_v1_0_setup_vm_pt_regs(struct amdgpu_device *adev, uint32_t vmid,
>   				uint64_t page_table_base);
>   
> +void gmc_v9_0_flush_gpu_tlb_helper(struct amdgpu_device *adev, uint32_t vmid,
> +				uint32_t flush_type, uint32_t eng, bool lock);
> +
>   #endif

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx