Re: [PATCH 6/6] drm/amdgpu: handle CPU access for split VRAM buffers

From: "Nicolai Hähnle" <nhaehnle@gmail.com>
To: "Christian König" <deathsimple@vodafone.de>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 6/6] drm/amdgpu: handle CPU access for split VRAM buffers
Date: Mon, 3 Apr 2017 18:25:47 +0200	[thread overview]
Message-ID: <056486db-3545-8bc3-c9da-2a0b24d62cfc@gmail.com> (raw)
In-Reply-To: <1490953652-3703-6-git-send-email-deathsimple@vodafone.de>

On 31.03.2017 11:47, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> This avoids merging them together on page fault.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Acked-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  4 +---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 16 ++++++++++++----
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 387d190..10237a8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -927,8 +927,7 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
>  	size = bo->mem.num_pages << PAGE_SHIFT;
>  	offset = bo->mem.start << PAGE_SHIFT;
>  	/* TODO: figure out how to map scattered VRAM to the CPU */
> -	if ((offset + size) <= adev->mc.visible_vram_size &&
> -	    (abo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS))
> +	if ((offset + size) <= adev->mc.visible_vram_size)
>  		return 0;
>
>  	/* Can't move a pinned BO to visible VRAM */
> @@ -936,7 +935,6 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
>  		return -EINVAL;
>
>  	/* hurrah the memory is not visible ! */
> -	abo->flags |= AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS;
>  	amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_VRAM);
>  	lpfn =	adev->mc.visible_vram_size >> PAGE_SHIFT;
>  	for (i = 0; i < abo->placement.num_placement; i++) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 524abca..10b793a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -529,9 +529,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>  	case TTM_PL_TT:
>  		break;
>  	case TTM_PL_VRAM:
> -		if (mem->start == AMDGPU_BO_INVALID_OFFSET)
> -			return -EINVAL;
> -
>  		mem->bus.offset = mem->start << PAGE_SHIFT;
>  		/* check if it's visible */
>  		if ((mem->bus.offset + mem->bus.size) > adev->mc.visible_vram_size)

I believe the various mem->bus members are now unused, aren't they? 
Unless I missed something, it's best to clean this up and no longer set 
them (or set them to some obvious poison values).

Cheers,
Nicolai

> @@ -549,6 +546,17 @@ static void amdgpu_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_re
>  {
>  }
>
> +static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
> +					   unsigned long page_offset)
> +{
> +	struct drm_mm_node *mm = bo->mem.mm_node;
> +	uint64_t size = mm->size;
> +
> +	mm += page_offset / size;
> +	page_offset %= size;
> +	return (bo->mem.bus.base >> PAGE_SHIFT) + mm->start + page_offset;
> +}
> +
>  /*
>   * TTM backend functions.
>   */
> @@ -1064,7 +1072,7 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
>  	.fault_reserve_notify = &amdgpu_bo_fault_reserve_notify,
>  	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
>  	.io_mem_free = &amdgpu_ttm_io_mem_free,
> -	.io_mem_pfn = ttm_bo_default_io_mem_pfn,
> +	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
>  };
>
>  int amdgpu_ttm_init(struct amdgpu_device *adev)
>

-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel