[PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

[PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

On constrained r100 systems compiz would fail to start due to a lack
of memory, we can just fallback place the objects rather than completely
failing it works a lot better.

v2:
fixes issue identified by Michel when pinning could happen in a busy placement domain.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h        |    4 +++-
 drivers/gpu/drm/radeon/radeon_object.c |   22 +++++++++++++---------
 drivers/gpu/drm/radeon/radeon_ttm.c    |    6 +++---
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index e912098..a23a6ab 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -240,7 +240,9 @@ struct radeon_bo {
 	struct list_head		list;
 	/* Protected by tbo.reserved */
 	u32				placements[3];
+	u32				busy_placements[3];
 	struct ttm_placement		placement;
+	struct ttm_placement		busy_placement;
 	struct ttm_buffer_object	tbo;
 	struct ttm_bo_kmap_obj		kmap;
 	unsigned			pin_count;
@@ -1227,7 +1229,7 @@ extern void radeon_surface_init(struct radeon_device *rdev);
 extern int radeon_cs_parser_init(struct radeon_cs_parser *p, void *data);
 extern void radeon_legacy_set_clock_gating(struct radeon_device *rdev, int enable);
 extern void radeon_atom_set_clock_gating(struct radeon_device *rdev, int enable);
-extern void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain);
+extern void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain, bool pinned);
 extern bool radeon_ttm_bo_is_radeon_bo(struct ttm_buffer_object *bo);
 extern void radeon_vram_location(struct radeon_device *rdev, struct radeon_mc *mc, u64 base);
 extern void radeon_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc);
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 6a8617b..ab3bc7b 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -64,17 +64,21 @@ bool radeon_ttm_bo_is_radeon_bo(struct ttm_buffer_object *bo)
 	return false;
 }
 
-void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
+void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain, bool pinned)
 {
-	u32 c = 0;
+	u32 c = 0, b = 0;
 
 	rbo->placement.fpfn = 0;
 	rbo->placement.lpfn = 0;
 	rbo->placement.placement = rbo->placements;
-	rbo->placement.busy_placement = rbo->placements;
+	rbo->placement.busy_placement = rbo->busy_placements;
 	if (domain & RADEON_GEM_DOMAIN_VRAM)
 		rbo->placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
 					TTM_PL_FLAG_VRAM;
+	/* add busy placement to TTM if VRAM is only option */
+	if (domain == RADEON_GEM_DOMAIN_VRAM && pinned == false) {
+		rbo->busy_placements[b++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
+	}
 	if (domain & RADEON_GEM_DOMAIN_GTT)
 		rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
 	if (domain & RADEON_GEM_DOMAIN_CPU)
@@ -82,7 +86,7 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
 	if (!c)
 		rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
 	rbo->placement.num_placement = c;
-	rbo->placement.num_busy_placement = c;
+	rbo->placement.num_busy_placement = b;
 }
 
 int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
@@ -110,7 +114,7 @@ int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
 	bo->surface_reg = -1;
 	INIT_LIST_HEAD(&bo->list);
 
-	radeon_ttm_placement_from_domain(bo, domain);
+	radeon_ttm_placement_from_domain(bo, domain, false);
 	/* Kernel allocation are uninterruptible */
 	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
 			&bo->placement, 0, 0, !kernel, NULL, size,
@@ -185,7 +189,7 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr)
 			*gpu_addr = radeon_bo_gpu_offset(bo);
 		return 0;
 	}
-	radeon_ttm_placement_from_domain(bo, domain);
+	radeon_ttm_placement_from_domain(bo, domain, true);
 	if (domain == RADEON_GEM_DOMAIN_VRAM) {
 		/* force to pin into visible video ram */
 		bo->placement.lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;
@@ -325,10 +329,10 @@ int radeon_bo_list_validate(struct list_head *head)
 		if (!bo->pin_count) {
 			if (lobj->wdomain) {
 				radeon_ttm_placement_from_domain(bo,
-								lobj->wdomain);
+								 lobj->wdomain, false);
 			} else {
 				radeon_ttm_placement_from_domain(bo,
-								lobj->rdomain);
+								 lobj->rdomain, false);
 			}
 			r = ttm_bo_validate(&bo->tbo, &bo->placement,
 						true, false, false);
@@ -516,7 +520,7 @@ int radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
 		offset = bo->mem.mm_node->start << PAGE_SHIFT;
 		if ((offset + size) > rdev->mc.visible_vram_size) {
 			/* hurrah the memory is not visible ! */
-			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_VRAM);
+			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_VRAM, false);
 			rbo->placement.lpfn = rdev->mc.visible_vram_size >> PAGE_SHIFT;
 			r = ttm_bo_validate(bo, &rbo->placement, false, true, false);
 			if (unlikely(r != 0))
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index af98f45..6557b44 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -205,13 +205,13 @@ static void radeon_evict_flags(struct ttm_buffer_object *bo,
 	switch (bo->mem.mem_type) {
 	case TTM_PL_VRAM:
 		if (rbo->rdev->cp.ready == false)
-			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_CPU);
+			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_CPU, false);
 		else
-			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_GTT);
+			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_GTT, false);
 		break;
 	case TTM_PL_TT:
 	default:
-		radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_CPU);
+		radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_CPU, false);
 	}
 	*placement = rbo->placement;
 }
-- 
1.6.6.1


------------------------------------------------------------------------------
--

Re: [PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

[Michel Dänzer]

On Die, 2010-04-27 at 12:34 +1000, Dave Airlie wrote: 
> From: Dave Airlie <airlied@redhat.com>
> 
> On constrained r100 systems compiz would fail to start due to a lack
> of memory, we can just fallback place the objects rather than completely
> failing it works a lot better.
> 
> v2:
> fixes issue identified by Michel when pinning could happen in a busy placement domain.

[...]


> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 6a8617b..ab3bc7b 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -110,7 +114,7 @@ int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
>  	bo->surface_reg = -1;
>  	INIT_LIST_HEAD(&bo->list);
>  
> -	radeon_ttm_placement_from_domain(bo, domain);
> +	radeon_ttm_placement_from_domain(bo, domain, false);
>  	/* Kernel allocation are uninterruptible */
>  	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
>  			&bo->placement, 0, 0, !kernel, NULL, size,

Not sure it's a good idea to unconditionally allow BOs to be created in
GTT if userspace requested VRAM. E.g. this would at least defeat the
purpose of UploadToScreen if not turn it into pure overhead.


> @@ -325,10 +329,10 @@ int radeon_bo_list_validate(struct list_head *head)
>  		if (!bo->pin_count) {
>  			if (lobj->wdomain) {
>  				radeon_ttm_placement_from_domain(bo,
> -								lobj->wdomain);
> +								 lobj->wdomain, false);
>  			} else {
>  				radeon_ttm_placement_from_domain(bo,
> -								lobj->rdomain);
> +								 lobj->rdomain, false);
>  			}
>  			r = ttm_bo_validate(&bo->tbo, &bo->placement,
>  						true, false, false);

How about something like the below (completely untested) instead? This
should try to respect the userspace request first and only allow falling
back from VRAM to GTT if that failed. (Maybe the new bool parameter
shouldn't be called 'pinned' then but something like
'allow_vram_fallback' and its sense inverted)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index d10f246..b6f5177 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -322,17 +322,25 @@ int radeon_bo_list_validate(struct list_head *head)
 	list_for_each_entry(lobj, head, list) {
 		bo = lobj->bo;
 		if (!bo->pin_count) {
+			bool pinned = true;
+
+retry:
 			if (lobj->wdomain) {
 				radeon_ttm_placement_from_domain(bo,
-								lobj->wdomain);
+								 lobj->wdomain, pinned);
 			} else {
 				radeon_ttm_placement_from_domain(bo,
-								lobj->rdomain);
+								 lobj->rdomain, pinned);
 			}
 			r = ttm_bo_validate(&bo->tbo, &bo->placement,
 						true, false);
-			if (unlikely(r))
+			if (unlikely(r)) {
+				if (pinned) {
+					pinned = false;
+					goto retry;
+				}
 				return r;
+			}
 		}
 		lobj->gpu_offset = radeon_bo_gpu_offset(bo);
 		lobj->tiling_flags = bo->tiling_flags;


> @@ -516,7 +520,7 @@ int radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
>  		offset = bo->mem.mm_node->start << PAGE_SHIFT;
>  		if ((offset + size) > rdev->mc.visible_vram_size) {
>  			/* hurrah the memory is not visible ! */
> -			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_VRAM);
> +			radeon_ttm_placement_from_domain(rbo, RADEON_GEM_DOMAIN_VRAM, false);
>  			rbo->placement.lpfn = rdev->mc.visible_vram_size >> PAGE_SHIFT;
>  			r = ttm_bo_validate(bo, &rbo->placement, false, true, false);
>  			if (unlikely(r != 0))

This will impose rbo->placement.lpfn for GTT as well, but it's only
required for VRAM.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer

------------------------------------------------------------------------------
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

[Michel Dänzer]

[ Moving to the new list ]

On Die, 2010-04-27 at 12:34 +1000, Dave Airlie wrote: 
> From: Dave Airlie <airlied@redhat.com>
> 
> On constrained r100 systems compiz would fail to start due to a lack
> of memory, we can just fallback place the objects rather than completely
> failing it works a lot better.
> 
> v2:
> fixes issue identified by Michel when pinning could happen in a busy placement domain.

[...]


> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 6a8617b..ab3bc7b 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -64,17 +64,21 @@ bool radeon_ttm_bo_is_radeon_bo(struct ttm_buffer_object *bo)
>  	return false;
>  }
>  
> -void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
> +void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain, bool pinned)
>  {
> -	u32 c = 0;
> +	u32 c = 0, b = 0;
>  
>  	rbo->placement.fpfn = 0;
>  	rbo->placement.lpfn = 0;
>  	rbo->placement.placement = rbo->placements;
> -	rbo->placement.busy_placement = rbo->placements;
> +	rbo->placement.busy_placement = rbo->busy_placements;
>  	if (domain & RADEON_GEM_DOMAIN_VRAM)
>  		rbo->placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
>  					TTM_PL_FLAG_VRAM;
> +	/* add busy placement to TTM if VRAM is only option */
> +	if (domain == RADEON_GEM_DOMAIN_VRAM && pinned == false) {
> +		rbo->busy_placements[b++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
> +	}
>  	if (domain & RADEON_GEM_DOMAIN_GTT)
>  		rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
>  	if (domain & RADEON_GEM_DOMAIN_CPU)
> @@ -82,7 +86,7 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
>  	if (!c)
>  		rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
>  	rbo->placement.num_placement = c;
> -	rbo->placement.num_busy_placement = c;
> +	rbo->placement.num_busy_placement = b;
>  }
>  
>  int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,

BTW, this means there won't be any busy placements if (domain !=
RADEON_GEM_DOMAIN_VRAM || pinned). Not sure if that's a problem.


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

Re: [PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

[Dave Airlie]

2010/4/27 Michel Dänzer <michel@daenzer.net>:
> [ Moving to the new list ]
>
> On Die, 2010-04-27 at 12:34 +1000, Dave Airlie wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> On constrained r100 systems compiz would fail to start due to a lack
>> of memory, we can just fallback place the objects rather than completely
>> failing it works a lot better.
>>
>> v2:
>> fixes issue identified by Michel when pinning could happen in a busy placement domain.
>
> [...]
>
>
>> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
>> index 6a8617b..ab3bc7b 100644
>> --- a/drivers/gpu/drm/radeon/radeon_object.c
>> +++ b/drivers/gpu/drm/radeon/radeon_object.c
>> @@ -64,17 +64,21 @@ bool radeon_ttm_bo_is_radeon_bo(struct ttm_buffer_object *bo)
>>       return false;
>>  }
>>
>> -void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
>> +void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain, bool pinned)
>>  {
>> -     u32 c = 0;
>> +     u32 c = 0, b = 0;
>>
>>       rbo->placement.fpfn = 0;
>>       rbo->placement.lpfn = 0;
>>       rbo->placement.placement = rbo->placements;
>> -     rbo->placement.busy_placement = rbo->placements;
>> +     rbo->placement.busy_placement = rbo->busy_placements;
>>       if (domain & RADEON_GEM_DOMAIN_VRAM)
>>               rbo->placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
>>                                       TTM_PL_FLAG_VRAM;
>> +     /* add busy placement to TTM if VRAM is only option */
>> +     if (domain == RADEON_GEM_DOMAIN_VRAM && pinned == false) {
>> +             rbo->busy_placements[b++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
>> +     }
>>       if (domain & RADEON_GEM_DOMAIN_GTT)
>>               rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
>>       if (domain & RADEON_GEM_DOMAIN_CPU)
>> @@ -82,7 +86,7 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
>>       if (!c)
>>               rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
>>       rbo->placement.num_placement = c;
>> -     rbo->placement.num_busy_placement = c;
>> +     rbo->placement.num_busy_placement = b;
>>  }
>>
>>  int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
>
> BTW, this means there won't be any busy placements if (domain !=
> RADEON_GEM_DOMAIN_VRAM || pinned). Not sure if that's a problem.
>

Well there isn't really anywhere else to put things, I don't think
falling GTT back to VRAM will ever be a case we have to worry about as
much, its not like should be pinning anything major in GTT especially
in the middle.

Falling back to system placement would sort of suck since the GPU
can't access it.

Dave.

Re: [PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

[Michel Dänzer]

On Die, 2010-04-27 at 20:14 +1000, Dave Airlie wrote: 
> 2010/4/27 Michel Dänzer <michel@daenzer.net>:
> > [ Moving to the new list ]
> >
> > On Die, 2010-04-27 at 12:34 +1000, Dave Airlie wrote:
> >> From: Dave Airlie <airlied@redhat.com>
> >>
> >> On constrained r100 systems compiz would fail to start due to a lack
> >> of memory, we can just fallback place the objects rather than completely
> >> failing it works a lot better.
> >>
> >> v2:
> >> fixes issue identified by Michel when pinning could happen in a busy placement domain.
> >
> > [...]
> >
> >
> >> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> >> index 6a8617b..ab3bc7b 100644
> >> --- a/drivers/gpu/drm/radeon/radeon_object.c
> >> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> >> @@ -64,17 +64,21 @@ bool radeon_ttm_bo_is_radeon_bo(struct ttm_buffer_object *bo)
> >>       return false;
> >>  }
> >>
> >> -void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
> >> +void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain, bool pinned)
> >>  {
> >> -     u32 c = 0;
> >> +     u32 c = 0, b = 0;
> >>
> >>       rbo->placement.fpfn = 0;
> >>       rbo->placement.lpfn = 0;
> >>       rbo->placement.placement = rbo->placements;
> >> -     rbo->placement.busy_placement = rbo->placements;
> >> +     rbo->placement.busy_placement = rbo->busy_placements;
> >>       if (domain & RADEON_GEM_DOMAIN_VRAM)
> >>               rbo->placements[c++] = TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED |
> >>                                       TTM_PL_FLAG_VRAM;
> >> +     /* add busy placement to TTM if VRAM is only option */
> >> +     if (domain == RADEON_GEM_DOMAIN_VRAM && pinned == false) {
> >> +             rbo->busy_placements[b++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
> >> +     }
> >>       if (domain & RADEON_GEM_DOMAIN_GTT)
> >>               rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_TT;
> >>       if (domain & RADEON_GEM_DOMAIN_CPU)
> >> @@ -82,7 +86,7 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
> >>       if (!c)
> >>               rbo->placements[c++] = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
> >>       rbo->placement.num_placement = c;
> >> -     rbo->placement.num_busy_placement = c;
> >> +     rbo->placement.num_busy_placement = b;
> >>  }
> >>
> >>  int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
> >
> > BTW, this means there won't be any busy placements if (domain !=
> > RADEON_GEM_DOMAIN_VRAM || pinned). Not sure if that's a problem.
> >
> 
> Well there isn't really anywhere else to put things, I don't think
> falling GTT back to VRAM will ever be a case we have to worry about as
> much, its not like should be pinning anything major in GTT especially
> in the middle.
> 
> Falling back to system placement would sort of suck since the GPU
> can't access it.

Sure, but then the busy placements in the existing code would seem
redundant as well, and I was wondering if there might have been a reason
for them to be there anyway.


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

Re: [PATCH] drm/radeon/bo: add some fallback placements for VRAM only objects. (v2)

[Michel Dänzer]

On Die, 2010-04-27 at 10:31 +0200, Michel Dänzer wrote: 
> On Die, 2010-04-27 at 12:34 +1000, Dave Airlie wrote: 
> > From: Dave Airlie <airlied@redhat.com>
> > 
> > On constrained r100 systems compiz would fail to start due to a lack
> > of memory, we can just fallback place the objects rather than completely
> > failing it works a lot better.
> > 
> > v2:
> > fixes issue identified by Michel when pinning could happen in a busy placement domain.
> 
> [...]
> 
> How about something like the below (completely untested) instead? This
> should try to respect the userspace request first and only allow falling
> back from VRAM to GTT if that failed. (Maybe the new bool parameter
> shouldn't be called 'pinned' then but something like
> 'allow_vram_fallback' and its sense inverted)

The alternative patch below is tested to not regress for me, but of
course I'm not affected by the failure to put things in VRAM. Uwe /
'Chicken Shack', does this fix your problem with compiz?

This doesn't cover radeon_bo_fault_reserve_notify() because my tree
doesn't have the invisible VRAM stuff yet, but it'll be easy to extend
this approach to there as well, and it shouldn't affect the immediate
problem we're trying to solve.


From c3009dd4df63a65fd15a485265fbdabfb1360c97 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= <daenzer@vmware.com>
Date: Wed, 28 Apr 2010 09:24:38 +0200
Subject: [PATCH] drm/radeon/kms: Fall back to GTT if BO creation in / validation to VRAM failed.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Michel Dänzer <daenzer@vmware.com>
---
 drivers/gpu/drm/radeon/radeon_object.c |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index d10f246..1dfb056 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -109,16 +109,22 @@ int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
 	bo->surface_reg = -1;
 	INIT_LIST_HEAD(&bo->list);
 
+retry:
 	radeon_ttm_placement_from_domain(bo, domain);
 	/* Kernel allocation are uninterruptible */
 	r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
 			&bo->placement, 0, 0, !kernel, NULL, size,
 			&radeon_ttm_bo_destroy);
 	if (unlikely(r != 0)) {
-		if (r != -ERESTARTSYS)
+		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;
@@ -313,6 +319,7 @@ int radeon_bo_list_validate(struct list_head *head)
 {
 	struct radeon_bo_list *lobj;
 	struct radeon_bo *bo;
+	u32 domain;
 	int r;
 
 	r = radeon_bo_list_reserve(head);
@@ -322,17 +329,19 @@ int radeon_bo_list_validate(struct list_head *head)
 	list_for_each_entry(lobj, head, list) {
 		bo = lobj->bo;
 		if (!bo->pin_count) {
-			if (lobj->wdomain) {
-				radeon_ttm_placement_from_domain(bo,
-								lobj->wdomain);
-			} else {
-				radeon_ttm_placement_from_domain(bo,
-								lobj->rdomain);
-			}
-			r = ttm_bo_validate(&bo->tbo, &bo->placement,
-						true, false);
-			if (unlikely(r))
+			domain = lobj->wdomain ? lobj->wdomain : lobj->rdomain;
+
+retry:
+			radeon_ttm_placement_from_domain(bo, domain);
+			r = ttm_bo_validate(&bo->tbo, &bo->placement, true, false);
+			if (unlikely(r)) {
+				if (r != -ERESTARTSYS &&
+				    domain == RADEON_GEM_DOMAIN_VRAM) {
+					domain |= RADEON_GEM_DOMAIN_GTT;
+					goto retry;
+				}
 				return r;
+			}
 		}
 		lobj->gpu_offset = radeon_bo_gpu_offset(bo);
 		lobj->tiling_flags = bo->tiling_flags;
-- 
1.7.0.5



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