[PATCH] drm/radeon: remove visible vram size limit on bo allocation

[PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Alex Deucher]

Now that fallback to gtt is fixed for cpu access, we can
remove this limit.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_gem.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index fdd189b..07a13c9 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -55,8 +55,11 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
 		alignment = PAGE_SIZE;
 	}
 
-	/* maximun bo size is the minimun btw visible vram and gtt size */
-	max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
+	/* Maximum bo size is the gtt size since we use the gtt to handle
+	 * vram to system pool migrations.  We could probably remove this
+	 * check altogether with a little additional work.
+	 */
+	max_size = rdev->mc.gtt_size;
 	if (size > max_size) {
 		DRM_DEBUG("Allocation size %dMb bigger than %ldMb limit\n",
 			  size >> 20, max_size >> 20);
-- 
1.8.3.1

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Christian König]

Am 16.07.2014 19:26, schrieb Alex Deucher:
> Now that fallback to gtt is fixed for cpu access, we can
> remove this limit.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/radeon/radeon_gem.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index fdd189b..07a13c9 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -55,8 +55,11 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
>   		alignment = PAGE_SIZE;
>   	}
>   
> -	/* maximun bo size is the minimun btw visible vram and gtt size */
> -	max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
> +	/* Maximum bo size is the gtt size since we use the gtt to handle
> +	 * vram to system pool migrations.  We could probably remove this
> +	 * check altogether with a little additional work.
> +	 */
> +	max_size = rdev->mc.gtt_size;
>   	if (size > max_size) {
>   		DRM_DEBUG("Allocation size %dMb bigger than %ldMb limit\n",
>   			  size >> 20, max_size >> 20);

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Michel Dänzer]

On 17.07.2014 02:26, Alex Deucher wrote:
> Now that fallback to gtt is fixed for cpu access, we can
> remove this limit.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/radeon/radeon_gem.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index fdd189b..07a13c9 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -55,8 +55,11 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
>  		alignment = PAGE_SIZE;
>  	}
>  
> -	/* maximun bo size is the minimun btw visible vram and gtt size */
> -	max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
> +	/* Maximum bo size is the gtt size since we use the gtt to handle
> +	 * vram to system pool migrations.  We could probably remove this
> +	 * check altogether with a little additional work.
> +	 */
> +	max_size = rdev->mc.gtt_size;
>  	if (size > max_size) {
>  		DRM_DEBUG("Allocation size %dMb bigger than %ldMb limit\n",
>  			  size >> 20, max_size >> 20);

A BO of size rdev->mc.gtt_size can never actually be bound to GTT,
because we have some pinned BOs in there. I think it's a bit
disingenuous to let userspace allocate a BO that can never actually be
used by the GPU. :)

The hack I attached to
https://bugs.freedesktop.org/show_bug.cgi?id=78717 has a start for
dealing with that. I was running that patch for a while and didn't
notice any bad effects from it.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Christian König]

Am 17.07.2014 06:02, schrieb Michel Dänzer:
> On 17.07.2014 02:26, Alex Deucher wrote:
>> Now that fallback to gtt is fixed for cpu access, we can
>> remove this limit.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   drivers/gpu/drm/radeon/radeon_gem.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
>> index fdd189b..07a13c9 100644
>> --- a/drivers/gpu/drm/radeon/radeon_gem.c
>> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
>> @@ -55,8 +55,11 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
>>   		alignment = PAGE_SIZE;
>>   	}
>>   
>> -	/* maximun bo size is the minimun btw visible vram and gtt size */
>> -	max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
>> +	/* Maximum bo size is the gtt size since we use the gtt to handle
>> +	 * vram to system pool migrations.  We could probably remove this
>> +	 * check altogether with a little additional work.
>> +	 */
>> +	max_size = rdev->mc.gtt_size;
>>   	if (size > max_size) {
>>   		DRM_DEBUG("Allocation size %dMb bigger than %ldMb limit\n",
>>   			  size >> 20, max_size >> 20);
> A BO of size rdev->mc.gtt_size can never actually be bound to GTT,
> because we have some pinned BOs in there. I think it's a bit
> disingenuous to let userspace allocate a BO that can never actually be
> used by the GPU. :)
>
> The hack I attached to
> https://bugs.freedesktop.org/show_bug.cgi?id=78717 has a start for
> dealing with that. I was running that patch for a while and didn't
> notice any bad effects from it.

Haven't looked at the patch yet, but can't we just go over all existing 
allocations on PIN and figure out the largest free area and save that 
value? I mean pinning of GTT memory happens rarely and mostly on system 
startup.

Christian.

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Alex Deucher]

On Thu, Jul 17, 2014 at 10:28 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 17.07.2014 06:02, schrieb Michel Dänzer:
>
>> On 17.07.2014 02:26, Alex Deucher wrote:
>>>
>>> Now that fallback to gtt is fixed for cpu access, we can
>>> remove this limit.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>   drivers/gpu/drm/radeon/radeon_gem.c | 7 +++++--
>>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
>>> b/drivers/gpu/drm/radeon/radeon_gem.c
>>> index fdd189b..07a13c9 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_gem.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
>>> @@ -55,8 +55,11 @@ int radeon_gem_object_create(struct radeon_device
>>> *rdev, int size,
>>>                 alignment = PAGE_SIZE;
>>>         }
>>>   -     /* maximun bo size is the minimun btw visible vram and gtt size
>>> */
>>> -       max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
>>> +       /* Maximum bo size is the gtt size since we use the gtt to handle
>>> +        * vram to system pool migrations.  We could probably remove this
>>> +        * check altogether with a little additional work.
>>> +        */
>>> +       max_size = rdev->mc.gtt_size;
>>>         if (size > max_size) {
>>>                 DRM_DEBUG("Allocation size %dMb bigger than %ldMb
>>> limit\n",
>>>                           size >> 20, max_size >> 20);
>>
>> A BO of size rdev->mc.gtt_size can never actually be bound to GTT,
>> because we have some pinned BOs in there. I think it's a bit
>> disingenuous to let userspace allocate a BO that can never actually be
>> used by the GPU. :)
>>
>> The hack I attached to
>> https://bugs.freedesktop.org/show_bug.cgi?id=78717 has a start for
>> dealing with that. I was running that patch for a while and didn't
>> notice any bad effects from it.
>
>
> Haven't looked at the patch yet, but can't we just go over all existing
> allocations on PIN and figure out the largest free area and save that value?
> I mean pinning of GTT memory happens rarely and mostly on system startup.

yeah, I had the same thought.

Alex
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Alex Deucher]

[-- Attachment #1: Type: text/plain, Size: 2201 bytes --]

On Thu, Jul 17, 2014 at 10:28 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Am 17.07.2014 06:02, schrieb Michel Dänzer:
>
>> On 17.07.2014 02:26, Alex Deucher wrote:
>>>
>>> Now that fallback to gtt is fixed for cpu access, we can
>>> remove this limit.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>   drivers/gpu/drm/radeon/radeon_gem.c | 7 +++++--
>>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
>>> b/drivers/gpu/drm/radeon/radeon_gem.c
>>> index fdd189b..07a13c9 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_gem.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
>>> @@ -55,8 +55,11 @@ int radeon_gem_object_create(struct radeon_device
>>> *rdev, int size,
>>>                 alignment = PAGE_SIZE;
>>>         }
>>>   -     /* maximun bo size is the minimun btw visible vram and gtt size
>>> */
>>> -       max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
>>> +       /* Maximum bo size is the gtt size since we use the gtt to handle
>>> +        * vram to system pool migrations.  We could probably remove this
>>> +        * check altogether with a little additional work.
>>> +        */
>>> +       max_size = rdev->mc.gtt_size;
>>>         if (size > max_size) {
>>>                 DRM_DEBUG("Allocation size %dMb bigger than %ldMb
>>> limit\n",
>>>                           size >> 20, max_size >> 20);
>>
>> A BO of size rdev->mc.gtt_size can never actually be bound to GTT,
>> because we have some pinned BOs in there. I think it's a bit
>> disingenuous to let userspace allocate a BO that can never actually be
>> used by the GPU. :)
>>
>> The hack I attached to
>> https://bugs.freedesktop.org/show_bug.cgi?id=78717 has a start for
>> dealing with that. I was running that patch for a while and didn't
>> notice any bad effects from it.
>
>
> Haven't looked at the patch yet, but can't we just go over all existing
> allocations on PIN and figure out the largest free area and save that value?
> I mean pinning of GTT memory happens rarely and mostly on system startup.


How about that attached patches?

Alex

[-- Attachment #2: 0004-drm-radeon-remove-visible-vram-size-limit-on-bo-allo.patch --]
[-- Type: text/x-diff, Size: 2757 bytes --]

From 688901ef16d8f9ac28071d4dceb4e53dc21c5358 Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Thu, 17 Jul 2014 12:26:29 -0400
Subject: [PATCH 4/4] drm/radeon: remove visible vram size limit on bo
 allocation (v2)

Now that fallback to gtt is fixed for cpu access, we can
remove this limit.

v2: use new gart_pin_size to accurately track available gtt.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon.h     |  2 +-
 drivers/gpu/drm/radeon/radeon_gem.c | 13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index d26c61c..e66f831 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -546,7 +546,7 @@ struct radeon_gem {
 
 int radeon_gem_init(struct radeon_device *rdev);
 void radeon_gem_fini(struct radeon_device *rdev);
-int radeon_gem_object_create(struct radeon_device *rdev, int size,
+int radeon_gem_object_create(struct radeon_device *rdev, unsigned long size,
 				int alignment, int initial_domain,
 				u32 flags, bool discardable, bool kernel,
 				struct drm_gem_object **obj);
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 1ec6244..f9b9e51 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -40,7 +40,7 @@ void radeon_gem_object_free(struct drm_gem_object *gobj)
 	}
 }
 
-int radeon_gem_object_create(struct radeon_device *rdev, int size,
+int radeon_gem_object_create(struct radeon_device *rdev, unsigned long size,
 				int alignment, int initial_domain,
 				u32 flags, bool discardable, bool kernel,
 				struct drm_gem_object **obj)
@@ -55,10 +55,13 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
 		alignment = PAGE_SIZE;
 	}
 
-	/* maximun bo size is the minimun btw visible vram and gtt size */
-	max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
+	/* Maximum bo size is the gtt size since we use the gtt to handle
+	 * vram to system pool migrations.  We could probably remove this
+	 * check altogether with a little additional work.
+	 */
+	max_size = rdev->mc.gtt_size - rdev->gart_pin_size;
 	if (size > max_size) {
-		DRM_DEBUG("Allocation size %dMb bigger than %ldMb limit\n",
+		DRM_DEBUG("Allocation size %ldMb bigger than %ldMb limit\n",
 			  size >> 20, max_size >> 20);
 		return -ENOMEM;
 	}
@@ -72,7 +75,7 @@ retry:
 				initial_domain |= RADEON_GEM_DOMAIN_GTT;
 				goto retry;
 			}
-			DRM_ERROR("Failed to allocate GEM object (%d, %d, %u, %d)\n",
+			DRM_ERROR("Failed to allocate GEM object (%ld, %d, %u, %d)\n",
 				  size, initial_domain, alignment, r);
 		}
 		return r;
-- 
1.8.3.1


[-- Attachment #3: 0003-drm-radeon-use-vram-gart-pinned-size-in-radeon_do_te.patch --]
[-- Type: text/x-diff, Size: 1255 bytes --]

From 6f88ae35f1f9d0899d799ab77b99145d1350537c Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Thu, 17 Jul 2014 12:20:32 -0400
Subject: [PATCH 3/4] drm/radeon: use vram/gart pinned size in
 radeon_do_test_moves

Gives more accurate count and prevents failures when we can't
allocate memory for the tests.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_test.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_test.c b/drivers/gpu/drm/radeon/radeon_test.c
index 9c5b66c..5adf420 100644
--- a/drivers/gpu/drm/radeon/radeon_test.c
+++ b/drivers/gpu/drm/radeon/radeon_test.c
@@ -56,13 +56,7 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
 	/* Number of tests =
 	 * (Total GTT - IB pool - writeback page - ring buffers) / test size
 	 */
-	n = rdev->mc.gtt_size - RADEON_IB_POOL_SIZE*64*1024;
-	for (i = 0; i < RADEON_NUM_RINGS; ++i)
-		n -= rdev->ring[i].ring_size;
-	if (rdev->wb.wb_obj)
-		n -= RADEON_GPU_PAGE_SIZE;
-	if (rdev->ih.ring_obj)
-		n -= rdev->ih.ring_size;
+	n = rdev->mc.gtt_size - rdev->gart_pin_size;
 	n /= size;
 
 	gtt_obj = kzalloc(n * sizeof(*gtt_obj), GFP_KERNEL);
-- 
1.8.3.1


[-- Attachment #4: 0002-drm-radeon-use-vram-gart-pinned-size-in-radeon_gem_i.patch --]
[-- Type: text/x-diff, Size: 1506 bytes --]

From 002f8e43e2f3e2ac299a9121f502b24d243bb6a6 Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Thu, 17 Jul 2014 12:16:20 -0400
Subject: [PATCH 2/4] drm/radeon: use vram/gart pinned size in
 radeon_gem_info_ioctl

Gives a more accurate limit than the previous code.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_gem.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index ce1d61e..1ec6244 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -209,18 +209,15 @@ int radeon_gem_info_ioctl(struct drm_device *dev, void *data,
 	struct radeon_device *rdev = dev->dev_private;
 	struct drm_radeon_gem_info *args = data;
 	struct ttm_mem_type_manager *man;
-	unsigned i;
 
 	man = &rdev->mman.bdev.man[TTM_PL_VRAM];
 
 	args->vram_size = rdev->mc.real_vram_size;
 	args->vram_visible = (u64)man->size << PAGE_SHIFT;
-	if (rdev->stollen_vga_memory)
-		args->vram_visible -= radeon_bo_size(rdev->stollen_vga_memory);
-	args->vram_visible -= radeon_fbdev_total_size(rdev);
-	args->gart_size = rdev->mc.gtt_size - 4096 - RADEON_IB_POOL_SIZE*64*1024;
-	for(i = 0; i < RADEON_NUM_RINGS; ++i)
-		args->gart_size -= rdev->ring[i].ring_size;
+	args->vram_visible -= rdev->vram_pin_size;
+	args->gart_size = rdev->mc.gtt_size;
+	args->gart_size -= rdev->gart_pin_size;
+
 	return 0;
 }
 
-- 
1.8.3.1


[-- Attachment #5: 0001-drm-radeon-tracked-pinned-memory.patch --]
[-- Type: text/x-diff, Size: 2040 bytes --]

From d0d0a6dc30625b6e5f4708a8ad1b0016a339faac Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexander.deucher@amd.com>
Date: Thu, 17 Jul 2014 12:09:25 -0400
Subject: [PATCH 1/4] drm/radeon: tracked pinned memory

So we know how large an allocation we can allow.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon.h        |  4 ++++
 drivers/gpu/drm/radeon/radeon_object.c | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index a863472..d26c61c 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2348,6 +2348,10 @@ struct radeon_device {
 
 	struct dev_pm_domain vga_pm_domain;
 	bool have_disp_power_ref;
+
+	/* tracking pinned memory */
+	u64 vram_pin_size;
+	u64 gart_pin_size;
 };
 
 bool radeon_is_px(struct drm_device *dev);
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 1b05a4d..2c549cd 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -306,6 +306,10 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset,
 		bo->pin_count = 1;
 		if (gpu_addr != NULL)
 			*gpu_addr = radeon_bo_gpu_offset(bo);
+		if (domain == RADEON_GEM_DOMAIN_VRAM)
+			bo->rdev->vram_pin_size += radeon_bo_size(bo);
+		else
+			bo->rdev->gart_pin_size += radeon_bo_size(bo);
 	}
 	if (unlikely(r != 0))
 		dev_err(bo->rdev->dev, "%p pin failed\n", bo);
@@ -331,6 +335,12 @@ int radeon_bo_unpin(struct radeon_bo *bo)
 	for (i = 0; i < bo->placement.num_placement; i++)
 		bo->placements[i] &= ~TTM_PL_FLAG_NO_EVICT;
 	r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false);
+	if (likely(r == 0)) {
+		if (bo->tbo.mem.mem_type == TTM_PL_VRAM)
+			bo->rdev->vram_pin_size -= radeon_bo_size(bo);
+		else
+			bo->rdev->gart_pin_size -= radeon_bo_size(bo);
+	}
 	if (unlikely(r != 0))
 		dev_err(bo->rdev->dev, "%p validate failed for unpin\n", bo);
 	return r;
-- 
1.8.3.1


[-- Attachment #6: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Christian König]

Am 17.07.2014 18:29, schrieb Alex Deucher:
> On Thu, Jul 17, 2014 at 10:28 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Am 17.07.2014 06:02, schrieb Michel Dänzer:
>>
>>> On 17.07.2014 02:26, Alex Deucher wrote:
>>>> Now that fallback to gtt is fixed for cpu access, we can
>>>> remove this limit.
>>>>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/radeon/radeon_gem.c | 7 +++++--
>>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
>>>> b/drivers/gpu/drm/radeon/radeon_gem.c
>>>> index fdd189b..07a13c9 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_gem.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
>>>> @@ -55,8 +55,11 @@ int radeon_gem_object_create(struct radeon_device
>>>> *rdev, int size,
>>>>                  alignment = PAGE_SIZE;
>>>>          }
>>>>    -     /* maximun bo size is the minimun btw visible vram and gtt size
>>>> */
>>>> -       max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
>>>> +       /* Maximum bo size is the gtt size since we use the gtt to handle
>>>> +        * vram to system pool migrations.  We could probably remove this
>>>> +        * check altogether with a little additional work.
>>>> +        */
>>>> +       max_size = rdev->mc.gtt_size;
>>>>          if (size > max_size) {
>>>>                  DRM_DEBUG("Allocation size %dMb bigger than %ldMb
>>>> limit\n",
>>>>                            size >> 20, max_size >> 20);
>>> A BO of size rdev->mc.gtt_size can never actually be bound to GTT,
>>> because we have some pinned BOs in there. I think it's a bit
>>> disingenuous to let userspace allocate a BO that can never actually be
>>> used by the GPU. :)
>>>
>>> The hack I attached to
>>> https://bugs.freedesktop.org/show_bug.cgi?id=78717 has a start for
>>> dealing with that. I was running that patch for a while and didn't
>>> notice any bad effects from it.
>>
>> Haven't looked at the patch yet, but can't we just go over all existing
>> allocations on PIN and figure out the largest free area and save that value?
>> I mean pinning of GTT memory happens rarely and mostly on system startup.
>
> How about that attached patches?

LGTM. My thinking was more complicated, but this should be fine as well.

Patches are: Reviewed-by: Christian König <christian.koenig@amd.com>

Christian.

>
> Alex

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Michel Dänzer]

On 18.07.2014 01:29, Alex Deucher wrote:
> @@ -55,10 +55,13 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
>  		alignment = PAGE_SIZE;
>  	}
>  
> -	/* maximun bo size is the minimun btw visible vram and gtt size */
> -	max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
> +	/* Maximum bo size is the gtt size since we use the gtt to handle
> +	 * vram to system pool migrations.  We could probably remove this
> +	 * check altogether with a little additional work.
> +	 */

This comment needs updating: it's not the full GTT size anymore, and how
could we remove this check?


> @@ -331,6 +335,12 @@ int radeon_bo_unpin(struct radeon_bo *bo)
>  	for (i = 0; i < bo->placement.num_placement; i++)
>  		bo->placements[i] &= ~TTM_PL_FLAG_NO_EVICT;
>  	r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false);
> +	if (likely(r == 0)) {
> +		if (bo->tbo.mem.mem_type == TTM_PL_VRAM)
> +			bo->rdev->vram_pin_size -= radeon_bo_size(bo);
> +		else
> +			bo->rdev->gart_pin_size -= radeon_bo_size(bo);
> +	}
>  	if (unlikely(r != 0))
>  		dev_err(bo->rdev->dev, "%p validate failed for unpin\n", bo);

Might be better to convert the failure case to use else, no big deal though.


Other than that, the patches look good to me as well.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Alex Deucher]

On Thu, Jul 17, 2014 at 8:41 PM, Michel Dänzer <michel@daenzer.net> wrote:
> On 18.07.2014 01:29, Alex Deucher wrote:
>> @@ -55,10 +55,13 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
>>               alignment = PAGE_SIZE;
>>       }
>>
>> -     /* maximun bo size is the minimun btw visible vram and gtt size */
>> -     max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
>> +     /* Maximum bo size is the gtt size since we use the gtt to handle
>> +      * vram to system pool migrations.  We could probably remove this
>> +      * check altogether with a little additional work.
>> +      */
>
> This comment needs updating: it's not the full GTT size anymore, and how
> could we remove this check?

I'll update it.  We could remove it if we added support for breaking
down vram <-> system transfers into smaller operations.  E.g., if the
available vram size was twice the size of the available gtt space, we
could transfer the first half, then update the gart table and transfer
the second half.

>
>
>> @@ -331,6 +335,12 @@ int radeon_bo_unpin(struct radeon_bo *bo)
>>       for (i = 0; i < bo->placement.num_placement; i++)
>>               bo->placements[i] &= ~TTM_PL_FLAG_NO_EVICT;
>>       r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false);
>> +     if (likely(r == 0)) {
>> +             if (bo->tbo.mem.mem_type == TTM_PL_VRAM)
>> +                     bo->rdev->vram_pin_size -= radeon_bo_size(bo);
>> +             else
>> +                     bo->rdev->gart_pin_size -= radeon_bo_size(bo);
>> +     }
>>       if (unlikely(r != 0))
>>               dev_err(bo->rdev->dev, "%p validate failed for unpin\n", bo);
>
> Might be better to convert the failure case to use else, no big deal though.

I can fix that.

>
>
> Other than that, the patches look good to me as well.
>

Thanks!

Alex

>
> --
> Earthling Michel Dänzer            |                  http://www.amd.com
> Libre software enthusiast          |                Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: remove visible vram size limit on bo allocation

[Michel Dänzer]

On 18.07.2014 12:26, Alex Deucher wrote:
> On Thu, Jul 17, 2014 at 8:41 PM, Michel Dänzer <michel@daenzer.net> wrote:
>> On 18.07.2014 01:29, Alex Deucher wrote:
>>> @@ -55,10 +55,13 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
>>>               alignment = PAGE_SIZE;
>>>       }
>>>
>>> -     /* maximun bo size is the minimun btw visible vram and gtt size */
>>> -     max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
>>> +     /* Maximum bo size is the gtt size since we use the gtt to handle
>>> +      * vram to system pool migrations.  We could probably remove this
>>> +      * check altogether with a little additional work.
>>> +      */
>>
>> This comment needs updating: it's not the full GTT size anymore, and how
>> could we remove this check?
> 
> I'll update it.  We could remove it if we added support for breaking
> down vram <-> system transfers into smaller operations.  E.g., if the
> available vram size was twice the size of the available gtt space, we
> could transfer the first half, then update the gart table and transfer
> the second half.

Right, OTOH I suspect it's even less likely that BOs of (almost) the
size of VRAM can actually be used in VRAM, because e.g. scanout buffers
are pinned anywhere in VRAM.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel