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

["Michel Dänzer" <michel@daenzer.net>]

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