Re: [PATCH 2/3] drm/amdgpu: fix and cleanup cpu visible VRAM handling

[zhoucm1 <zhoucm1-5C7GfCeVMHo@public.gmane.org>]

On 2018年04月09日 18:18, Christian König wrote:
> The detection if a BO was placed in CPU visible VRAM was incorrect.
>
> Fix it and merge it with the correct detection in amdgpu_ttm.c
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c     |  6 ++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 21 +++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 19 +++----------------
>   3 files changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index de69ab12bb55..68af2f878bc9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -382,8 +382,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
>   
>   	p->bytes_moved += ctx.bytes_moved;
>   	if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
> -	    bo->tbo.mem.mem_type == TTM_PL_VRAM &&
> -	    bo->tbo.mem.start < adev->gmc.visible_vram_size >> PAGE_SHIFT)
> +	    amdgpu_bo_in_cpu_visible_vram(bo))
>   		p->bytes_moved_vis += ctx.bytes_moved;
>   
>   	if (unlikely(r == -ENOMEM) && domain != bo->allowed_domains &&
> @@ -437,8 +436,7 @@ 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 &&
> -			bo->tbo.mem.mem_type == TTM_PL_VRAM &&
> -			bo->tbo.mem.start < adev->gmc.visible_vram_size >> PAGE_SHIFT;
> +			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_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index deb10ecd89a1..0e9555526950 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -193,6 +193,27 @@ static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo)
>   	}
>   }
>   
> +/**
> + * amdgpu_bo_in_cpu_visible_vram - check if BO is (partly) in visible VRAM
> + */
> +static inline bool amdgpu_bo_in_cpu_visible_vram(struct amdgpu_bo *bo)
> +{
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +	unsigned fpfn = adev->gmc.visible_vram_size >> PAGE_SHIFT;
> +	struct drm_mm_node *node = bo->tbo.mem.mm_node;
> +	unsigned long pages_left;
> +
> +	if (bo->tbo.mem.mem_type != TTM_PL_VRAM)
> +		return false;
> +
> +	for (pages_left = bo->tbo.mem.num_pages; pages_left;
> +	     pages_left -= node->size, node++)
> +		if (node->start < fpfn)
> +			return true;
> +
> +	return false;
> +}
> +
>   /**
>    * amdgpu_bo_explicit_sync - return whether the bo is explicitly synced
>    */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index d2ab40494a4c..ab829fb3e840 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -222,20 +222,8 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
>   		if (!adev->mman.buffer_funcs_enabled) {
>   			amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_CPU);
>   		} else if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
> -			   !(abo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)) {
> -			unsigned fpfn = adev->gmc.visible_vram_size >> PAGE_SHIFT;
> -			struct drm_mm_node *node = bo->mem.mm_node;
> -			unsigned long pages_left;
> -
> -			for (pages_left = bo->mem.num_pages;
> -			     pages_left;
> -			     pages_left -= node->size, node++) {
> -				if (node->start < fpfn)
> -					break;
> -			}
> -
> -			if (!pages_left)
> -				goto gtt;
> +			   !(abo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) &&
> +			   amdgpu_bo_in_cpu_visible_vram(abo)) {
>   
>   			/* Try evicting to the CPU inaccessible part of VRAM
>   			 * first, but only set GTT as busy placement, so this
> @@ -244,12 +232,11 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
>   			 */
>   			amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_VRAM |
>   							 AMDGPU_GEM_DOMAIN_GTT);
> -			abo->placements[0].fpfn = fpfn;
> +			abo->placements[0].fpfn = adev->gmc.visible_vram_size >> PAGE_SHIFT;
>   			abo->placements[0].lpfn = 0;
>   			abo->placement.busy_placement = &abo->placements[1];
>   			abo->placement.num_busy_placement = 1;
>   		} else {
> -gtt:
>   			amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_GTT);
>   		}
>   		break;

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