[PATCH] drm/amdgpu: Consolidate visible vs. real vram check.

[Christian König]

Am 12.06.2018 um 20:37 schrieb Andrey Grodzovsky:
> Move all instnaces of this check into a function in amdgpu_gmc.h
> Rename the original function to a more proper name.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>

There seems to be two more candidates for that cleanup:

drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:    if 
(adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:        } else if 
(adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&

With those two fixed as well the patch is Reviewed-by: Christian König 
<christian.koenig at amd.com>.

Thanks,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 11 ++++++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 15 +++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 20 ++++----------------
>   3 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 12f0d18..f98be69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -31,6 +31,7 @@
>   #include <drm/drm_syncobj.h>
>   #include "amdgpu.h"
>   #include "amdgpu_trace.h"
> +#include "amdgpu_gmc.h"
>   
>   static int amdgpu_cs_user_fence_chunk(struct amdgpu_cs_parser *p,
>   				      struct drm_amdgpu_cs_chunk_fence *data,
> @@ -302,7 +303,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev,
>   	*max_bytes = us_to_bytes(adev, adev->mm_stats.accum_us);
>   
>   	/* Do the same for visible VRAM if half of it is free */
> -	if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size) {
> +	if (!amdgpu_gmc_vram_full_visible(&adev->gmc)) {
>   		u64 total_vis_vram = adev->gmc.visible_vram_size;
>   		u64 used_vis_vram =
>   			amdgpu_vram_mgr_vis_usage(&adev->mman.bdev.man[TTM_PL_VRAM]);
> @@ -359,7 +360,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
>   	 * to move it. Don't move anything if the threshold is zero.
>   	 */
>   	if (p->bytes_moved < p->bytes_moved_threshold) {
> -		if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
> +		if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
>   		    (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)) {
>   			/* And don't move a CPU_ACCESS_REQUIRED BO to limited
>   			 * visible VRAM if we've depleted our allowance to do
> @@ -381,7 +382,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
>   	r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>   
>   	p->bytes_moved += ctx.bytes_moved;
> -	if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
> +	if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
>   	    amdgpu_bo_in_cpu_visible_vram(bo))
>   		p->bytes_moved_vis += ctx.bytes_moved;
>   
> @@ -434,8 +435,8 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
>   
>   		/* Good we can try to move this BO somewhere else */
>   		update_bytes_moved_vis =
> -			adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
> -			amdgpu_bo_in_cpu_visible_vram(bo);
> +				!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
> +				amdgpu_bo_in_cpu_visible_vram(bo);
>   		amdgpu_ttm_placement_from_domain(bo, other);
>   		r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>   		p->bytes_moved += ctx.bytes_moved;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 893c249..6cb4948 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -109,4 +109,19 @@ struct amdgpu_gmc {
>   	const struct amdgpu_gmc_funcs	*gmc_funcs;
>   };
>   
> +/**
> + * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * Returns:
> + * True if full VRAM is visible through the BAR
> + */
> +static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
> +{
> +	WARN_ON(gmc->real_vram_size < gmc->visible_vram_size);
> +
> +	return (gmc->real_vram_size == gmc->visible_vram_size);
> +}
> +
>   #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 7f03f8c..6d22942 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -33,6 +33,7 @@
>   #include "amdgpu.h"
>   #include "amdgpu_trace.h"
>   #include "amdgpu_amdkfd.h"
> +#include "amdgpu_gmc.h"
>   
>   /**
>    * DOC: GPUVM
> @@ -669,19 +670,6 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   }
>   
>   /**
> - * amdgpu_vm_is_large_bar - Check if BAR is large enough
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Returns:
> - * True if BAR is large enough.
> - */
> -static bool amdgpu_vm_is_large_bar(struct amdgpu_device *adev)
> -{
> -	return (adev->gmc.real_vram_size == adev->gmc.visible_vram_size);
> -}
> -
> -/**
>    * amdgpu_vm_flush - hardware flush the vm
>    *
>    * @ring: ring to use for flush
> @@ -2579,7 +2567,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	}
>   	DRM_DEBUG_DRIVER("VM update mode is %s\n",
>   			 vm->use_cpu_for_update ? "CPU" : "SDMA");
> -	WARN_ONCE((vm->use_cpu_for_update & !amdgpu_vm_is_large_bar(adev)),
> +	WARN_ONCE((vm->use_cpu_for_update & !amdgpu_gmc_vram_full_visible(&adev->gmc)),
>   		  "CPU update of VM recommended only for large BAR system\n");
>   	vm->last_update = NULL;
>   
> @@ -2699,7 +2687,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>   	vm->pte_support_ats = pte_support_ats;
>   	DRM_DEBUG_DRIVER("VM update mode is %s\n",
>   			 vm->use_cpu_for_update ? "CPU" : "SDMA");
> -	WARN_ONCE((vm->use_cpu_for_update & !amdgpu_vm_is_large_bar(adev)),
> +	WARN_ONCE((vm->use_cpu_for_update & !amdgpu_gmc_vram_full_visible(&adev->gmc)),
>   		  "CPU update of VM recommended only for large BAR system\n");
>   
>   	if (vm->pasid) {
> @@ -2877,7 +2865,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
>   	 */
>   #ifdef CONFIG_X86_64
>   	if (amdgpu_vm_update_mode == -1) {
> -		if (amdgpu_vm_is_large_bar(adev))
> +		if (amdgpu_gmc_vram_full_visible(&adev->gmc))
>   			adev->vm_manager.vm_update_mode =
>   				AMDGPU_VM_USE_CPU_FOR_COMPUTE;
>   		else