* [PATCH 2/4] drm/radeon: use vzalloc for gart pages
2012-10-23 12:23 [PATCH 1/4] drm/radeon: fix and simplify pot argument checks Christian König
@ 2012-10-23 12:23 ` Christian König
2012-10-23 12:23 ` [PATCH 3/4] drm/radeon: move size limits to gem_object_create Christian König
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2012-10-23 12:23 UTC (permalink / raw)
To: dri-devel
When allocating more than 2GB of GART the array of pages
gets to big for kzalloc, use vzmalloc instead.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon_gart.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 9a64f8c..631973d 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -355,14 +355,13 @@ int radeon_gart_init(struct radeon_device *rdev)
DRM_INFO("GART: num cpu pages %u, num gpu pages %u\n",
rdev->gart.num_cpu_pages, rdev->gart.num_gpu_pages);
/* Allocate pages table */
- rdev->gart.pages = kzalloc(sizeof(void *) * rdev->gart.num_cpu_pages,
- GFP_KERNEL);
+ rdev->gart.pages = vzalloc(sizeof(void *) * rdev->gart.num_cpu_pages);
if (rdev->gart.pages == NULL) {
radeon_gart_fini(rdev);
return -ENOMEM;
}
- rdev->gart.pages_addr = kzalloc(sizeof(dma_addr_t) *
- rdev->gart.num_cpu_pages, GFP_KERNEL);
+ rdev->gart.pages_addr = vzalloc(sizeof(dma_addr_t) *
+ rdev->gart.num_cpu_pages);
if (rdev->gart.pages_addr == NULL) {
radeon_gart_fini(rdev);
return -ENOMEM;
@@ -388,8 +387,8 @@ void radeon_gart_fini(struct radeon_device *rdev)
radeon_gart_unbind(rdev, 0, rdev->gart.num_cpu_pages);
}
rdev->gart.ready = false;
- kfree(rdev->gart.pages);
- kfree(rdev->gart.pages_addr);
+ vfree(rdev->gart.pages);
+ vfree(rdev->gart.pages_addr);
rdev->gart.pages = NULL;
rdev->gart.pages_addr = NULL;
--
1.7.9.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] drm/radeon: move size limits to gem_object_create.
2012-10-23 12:23 [PATCH 1/4] drm/radeon: fix and simplify pot argument checks Christian König
2012-10-23 12:23 ` [PATCH 2/4] drm/radeon: use vzalloc for gart pages Christian König
@ 2012-10-23 12:23 ` Christian König
2012-10-23 12:23 ` [PATCH 4/4] drm/radeon: move the retry " Christian König
2012-10-23 12:44 ` [PATCH 1/4] drm/radeon: fix and simplify pot argument checks Michel Dänzer
3 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2012-10-23 12:23 UTC (permalink / raw)
To: dri-devel
Driver internal users shouldn't be limited in their allocation size.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon_gem.c | 10 ++++++++++
drivers/gpu/drm/radeon/radeon_object.c | 9 ---------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index f38fbcc..dfee7bb 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -53,6 +53,7 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
struct drm_gem_object **obj)
{
struct radeon_bo *robj;
+ unsigned long max_size;
int r;
*obj = NULL;
@@ -60,6 +61,15 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
if (alignment < PAGE_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);
+ if (size > max_size) {
+ printk(KERN_WARNING "%s:%d alloc size %dMb bigger than %ldMb limit\n",
+ __func__, __LINE__, size >> 20, max_size >> 20);
+ return -ENOMEM;
+ }
+
r = radeon_bo_create(rdev, size, alignment, kernel, initial_domain, NULL, &robj);
if (r) {
if (r != -ERESTARTSYS)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 8b27dd6..f404944 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -105,7 +105,6 @@ int radeon_bo_create(struct radeon_device *rdev,
struct radeon_bo *bo;
enum ttm_bo_type type;
unsigned long page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
- unsigned long max_size = 0;
size_t acc_size;
int r;
@@ -121,14 +120,6 @@ int radeon_bo_create(struct radeon_device *rdev,
}
*bo_ptr = NULL;
- /* maximun bo size is the minimun btw visible vram and gtt size */
- max_size = min(rdev->mc.visible_vram_size, rdev->mc.gtt_size);
- if ((page_align << PAGE_SHIFT) >= max_size) {
- printk(KERN_WARNING "%s:%d alloc size %ldM bigger than %ldMb limit\n",
- __func__, __LINE__, page_align >> (20 - PAGE_SHIFT), max_size >> 20);
- return -ENOMEM;
- }
-
acc_size = ttm_bo_dma_acc_size(&rdev->mman.bdev, size,
sizeof(struct radeon_bo));
--
1.7.9.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] drm/radeon: move the retry to gem_object_create
2012-10-23 12:23 [PATCH 1/4] drm/radeon: fix and simplify pot argument checks Christian König
2012-10-23 12:23 ` [PATCH 2/4] drm/radeon: use vzalloc for gart pages Christian König
2012-10-23 12:23 ` [PATCH 3/4] drm/radeon: move size limits to gem_object_create Christian König
@ 2012-10-23 12:23 ` Christian König
2012-10-23 12:44 ` [PATCH 1/4] drm/radeon: fix and simplify pot argument checks Michel Dänzer
3 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2012-10-23 12:23 UTC (permalink / raw)
To: dri-devel
When internal users want VRAM we shouldn't return GART memory instead.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon_gem.c | 8 +++++++-
drivers/gpu/drm/radeon/radeon_object.c | 10 ----------
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index dfee7bb..fe5c1f6 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -70,11 +70,17 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
return -ENOMEM;
}
+retry:
r = radeon_bo_create(rdev, size, alignment, kernel, initial_domain, NULL, &robj);
if (r) {
- if (r != -ERESTARTSYS)
+ if (r != -ERESTARTSYS) {
+ if (initial_domain == RADEON_GEM_DOMAIN_VRAM) {
+ initial_domain |= RADEON_GEM_DOMAIN_GTT;
+ goto retry;
+ }
DRM_ERROR("Failed to allocate GEM object (%d, %d, %u, %d)\n",
size, initial_domain, alignment, r);
+ }
return r;
}
*obj = &robj->gem_base;
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index f404944..b91118c 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -123,7 +123,6 @@ int radeon_bo_create(struct radeon_device *rdev,
acc_size = ttm_bo_dma_acc_size(&rdev->mman.bdev, size,
sizeof(struct radeon_bo));
-retry:
bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL);
if (bo == NULL)
return -ENOMEM;
@@ -145,15 +144,6 @@ retry:
acc_size, sg, &radeon_ttm_bo_destroy);
up_read(&rdev->pm.mclk_lock);
if (unlikely(r != 0)) {
- if (r != -ERESTARTSYS) {
- if (domain == RADEON_GEM_DOMAIN_VRAM) {
- domain |= RADEON_GEM_DOMAIN_GTT;
- goto retry;
- }
- dev_err(rdev->dev,
- "object_init failed for (%lu, 0x%08X)\n",
- size, domain);
- }
return r;
}
*bo_ptr = bo;
--
1.7.9.5
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/4] drm/radeon: fix and simplify pot argument checks
2012-10-23 12:23 [PATCH 1/4] drm/radeon: fix and simplify pot argument checks Christian König
` (2 preceding siblings ...)
2012-10-23 12:23 ` [PATCH 4/4] drm/radeon: move the retry " Christian König
@ 2012-10-23 12:44 ` Michel Dänzer
2012-10-23 13:40 ` Christian König
3 siblings, 1 reply; 6+ messages in thread
From: Michel Dänzer @ 2012-10-23 12:44 UTC (permalink / raw)
To: Christian König; +Cc: dri-devel
On Die, 2012-10-23 at 14:23 +0200, Christian König wrote:
> GART and VRAM size limits need to be a power of two.
> Fix values greater than 1GB and simplify those checks a bit.
>
> Signed-off-by: Christian König <deathsimple@vodafone.de>
> ---
> drivers/gpu/drm/radeon/radeon_device.c | 55 ++++++++++++--------------------
> 1 file changed, 21 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index bd13ca0..3277aa1 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -835,6 +835,19 @@ static unsigned int radeon_vga_set_decode(void *cookie, bool state)
> }
>
> /**
> + * radeon_check_pot_argument - check that argument is a power of two
> + *
> + * @arg: value to check
> + *
> + * Validates that a certain argument is a power of two (all asics).
> + * Returns true if argument is valid.
> + */
> +static bool radeon_ckeck_pot_argument(int arg)
> +{
> + return (arg & ((1 << __fls(arg)) - 1)) == 0;
> +}
This could be simplified as
return (arg & (arg - 1)) == 0;
> - radeon_vram_limit = radeon_vram_limit << 20;
> + radeon_vram_limit = (uint64_t)radeon_vram_limit << 20;
Does this cast have any effect without also changing the type of
radeon_vram_limit to something other than int? If the point is to allow
the shift to set the sign bit, I think casting to unsigned int or
uint32_t would be slightly less confusing, but a comment is probably
warranted anyway to prevent this from getting broken accidentally.
The commit message of patch 2 has a typo (vzmalloc instead of vzalloc),
other than that patches 2-4 are
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Debian, X and DRI developer
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/4] drm/radeon: fix and simplify pot argument checks
2012-10-23 12:44 ` [PATCH 1/4] drm/radeon: fix and simplify pot argument checks Michel Dänzer
@ 2012-10-23 13:40 ` Christian König
0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2012-10-23 13:40 UTC (permalink / raw)
To: Michel Dänzer; +Cc: dri-devel
On 23.10.2012 14:44, Michel Dänzer wrote:
> On Die, 2012-10-23 at 14:23 +0200, Christian König wrote:
>> GART and VRAM size limits need to be a power of two.
>> Fix values greater than 1GB and simplify those checks a bit.
>>
>> Signed-off-by: Christian König <deathsimple@vodafone.de>
>> ---
>> drivers/gpu/drm/radeon/radeon_device.c | 55 ++++++++++++--------------------
>> 1 file changed, 21 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
>> index bd13ca0..3277aa1 100644
>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>> @@ -835,6 +835,19 @@ static unsigned int radeon_vga_set_decode(void *cookie, bool state)
>> }
>>
>> /**
>> + * radeon_check_pot_argument - check that argument is a power of two
>> + *
>> + * @arg: value to check
>> + *
>> + * Validates that a certain argument is a power of two (all asics).
>> + * Returns true if argument is valid.
>> + */
>> +static bool radeon_ckeck_pot_argument(int arg)
>> +{
>> + return (arg & ((1 << __fls(arg)) - 1)) == 0;
>> +}
> This could be simplified as
>
> return (arg & (arg - 1)) == 0;
Fixed.
>
>
>> - radeon_vram_limit = radeon_vram_limit << 20;
>> + radeon_vram_limit = (uint64_t)radeon_vram_limit << 20;
> Does this cast have any effect without also changing the type of
> radeon_vram_limit to something other than int? If the point is to allow
> the shift to set the sign bit, I think casting to unsigned int or
> uint32_t would be slightly less confusing, but a comment is probably
> warranted anyway to prevent this from getting broken accidentally.
Well, I actually focused on the GART limit instead, and just tried to
fix that in the same way.
But wait a moment, modifying the radeon_vmram_limit here is quite faulty
anyway, cause this code gets executed for each card in the system. So
the vram limit won't work for the second card any more (and might
produce quite unusable results).
Going to fix that.
>
>
> The commit message of patch 2 has a typo (vzmalloc instead of vzalloc),
> other than that patches 2-4 are
>
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Thanks for the review, going to send out a v2 of the patchset soon.
Christian.
^ permalink raw reply [flat|nested] 6+ messages in thread