All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jerome Glisse <j.glisse@gmail.com>
To: Maarten Lankhorst <m.b.lankhorst@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH WW 11/13] drm/radeon: get rid of ttm_bo_is_reserved usage
Date: Thu, 27 Jun 2013 18:05:15 -0400	[thread overview]
Message-ID: <20130627220514.GE4380@gmail.com> (raw)
In-Reply-To: <1372333708-29884-12-git-send-email-maarten.lankhorst@canonical.com>

On Thu, Jun 27, 2013 at 01:48:26PM +0200, Maarten Lankhorst wrote:
> Try to use lockdep_assert_held or other alternatives where possible.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

Reviewed-by: Jerome Glisse <jglisse@redhat.com>

> ---
>  drivers/gpu/drm/radeon/radeon_object.c |  8 ++--
>  drivers/gpu/drm/radeon/radeon_object.h |  5 ---
>  drivers/gpu/drm/radeon/radeon_test.c   | 75 +++++++++++++++++-----------------
>  3 files changed, 43 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index d850dc6..0219d26 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -400,7 +400,7 @@ int radeon_bo_get_surface_reg(struct radeon_bo *bo)
>  	int steal;
>  	int i;
>  
> -	BUG_ON(!radeon_bo_is_reserved(bo));
> +	lockdep_assert_held(&bo->tbo.resv->lock.base);
>  
>  	if (!bo->tiling_flags)
>  		return 0;
> @@ -526,7 +526,8 @@ void radeon_bo_get_tiling_flags(struct radeon_bo *bo,
>  				uint32_t *tiling_flags,
>  				uint32_t *pitch)
>  {
> -	BUG_ON(!radeon_bo_is_reserved(bo));
> +	lockdep_assert_held(&bo->tbo.resv->lock.base);
> +
>  	if (tiling_flags)
>  		*tiling_flags = bo->tiling_flags;
>  	if (pitch)
> @@ -536,7 +537,8 @@ void radeon_bo_get_tiling_flags(struct radeon_bo *bo,
>  int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved,
>  				bool force_drop)
>  {
> -	BUG_ON(!radeon_bo_is_reserved(bo) && !force_drop);
> +	if (!force_drop)
> +		lockdep_assert_held(&bo->tbo.resv->lock.base);
>  
>  	if (!(bo->tiling_flags & RADEON_TILING_SURFACE))
>  		return 0;
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
> index 456ad6b..91519a5 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -98,11 +98,6 @@ static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
>  	return bo->tbo.num_pages << PAGE_SHIFT;
>  }
>  
> -static inline bool radeon_bo_is_reserved(struct radeon_bo *bo)
> -{
> -	return ttm_bo_is_reserved(&bo->tbo);
> -}
> -
>  static inline unsigned radeon_bo_ngpu_pages(struct radeon_bo *bo)
>  {
>  	return (bo->tbo.num_pages << PAGE_SHIFT) / RADEON_GPU_PAGE_SIZE;
> diff --git a/drivers/gpu/drm/radeon/radeon_test.c b/drivers/gpu/drm/radeon/radeon_test.c
> index bbed4af..f4d6bce 100644
> --- a/drivers/gpu/drm/radeon/radeon_test.c
> +++ b/drivers/gpu/drm/radeon/radeon_test.c
> @@ -35,7 +35,6 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  {
>  	struct radeon_bo *vram_obj = NULL;
>  	struct radeon_bo **gtt_obj = NULL;
> -	struct radeon_fence *fence = NULL;
>  	uint64_t gtt_addr, vram_addr;
>  	unsigned i, n, size;
>  	int r, ring;
> @@ -81,37 +80,38 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  	}
>  	r = radeon_bo_reserve(vram_obj, false);
>  	if (unlikely(r != 0))
> -		goto out_cleanup;
> +		goto out_unref;
>  	r = radeon_bo_pin(vram_obj, RADEON_GEM_DOMAIN_VRAM, &vram_addr);
>  	if (r) {
>  		DRM_ERROR("Failed to pin VRAM object\n");
> -		goto out_cleanup;
> +		goto out_unres;
>  	}
>  	for (i = 0; i < n; i++) {
>  		void *gtt_map, *vram_map;
>  		void **gtt_start, **gtt_end;
>  		void **vram_start, **vram_end;
> +		struct radeon_fence *fence = NULL;
>  
>  		r = radeon_bo_create(rdev, size, PAGE_SIZE, true,
>  				     RADEON_GEM_DOMAIN_GTT, NULL, gtt_obj + i);
>  		if (r) {
>  			DRM_ERROR("Failed to create GTT object %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean;
>  		}
>  
>  		r = radeon_bo_reserve(gtt_obj[i], false);
>  		if (unlikely(r != 0))
> -			goto out_cleanup;
> +			goto out_lclean_unref;
>  		r = radeon_bo_pin(gtt_obj[i], RADEON_GEM_DOMAIN_GTT, &gtt_addr);
>  		if (r) {
>  			DRM_ERROR("Failed to pin GTT object %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean_unres;
>  		}
>  
>  		r = radeon_bo_kmap(gtt_obj[i], &gtt_map);
>  		if (r) {
>  			DRM_ERROR("Failed to map GTT object %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean_unpin;
>  		}
>  
>  		for (gtt_start = gtt_map, gtt_end = gtt_map + size;
> @@ -127,13 +127,13 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  			r = radeon_copy_blit(rdev, gtt_addr, vram_addr, size / RADEON_GPU_PAGE_SIZE, &fence);
>  		if (r) {
>  			DRM_ERROR("Failed GTT->VRAM copy %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean_unpin;
>  		}
>  
>  		r = radeon_fence_wait(fence, false);
>  		if (r) {
>  			DRM_ERROR("Failed to wait for GTT->VRAM fence %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean_unpin;
>  		}
>  
>  		radeon_fence_unref(&fence);
> @@ -141,7 +141,7 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  		r = radeon_bo_kmap(vram_obj, &vram_map);
>  		if (r) {
>  			DRM_ERROR("Failed to map VRAM object after copy %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean_unpin;
>  		}
>  
>  		for (gtt_start = gtt_map, gtt_end = gtt_map + size,
> @@ -160,7 +160,7 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  					  (vram_addr - rdev->mc.vram_start +
>  					   (void*)gtt_start - gtt_map));
>  				radeon_bo_kunmap(vram_obj);
> -				goto out_cleanup;
> +				goto out_lclean_unpin;
>  			}
>  			*vram_start = vram_start;
>  		}
> @@ -173,13 +173,13 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  			r = radeon_copy_blit(rdev, vram_addr, gtt_addr, size / RADEON_GPU_PAGE_SIZE, &fence);
>  		if (r) {
>  			DRM_ERROR("Failed VRAM->GTT copy %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean_unpin;
>  		}
>  
>  		r = radeon_fence_wait(fence, false);
>  		if (r) {
>  			DRM_ERROR("Failed to wait for VRAM->GTT fence %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean_unpin;
>  		}
>  
>  		radeon_fence_unref(&fence);
> @@ -187,7 +187,7 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  		r = radeon_bo_kmap(gtt_obj[i], &gtt_map);
>  		if (r) {
>  			DRM_ERROR("Failed to map GTT object after copy %d\n", i);
> -			goto out_cleanup;
> +			goto out_lclean_unpin;
>  		}
>  
>  		for (gtt_start = gtt_map, gtt_end = gtt_map + size,
> @@ -206,7 +206,7 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  					  (gtt_addr - rdev->mc.gtt_start +
>  					   (void*)vram_start - vram_map));
>  				radeon_bo_kunmap(gtt_obj[i]);
> -				goto out_cleanup;
> +				goto out_lclean_unpin;
>  			}
>  		}
>  
> @@ -214,31 +214,32 @@ static void radeon_do_test_moves(struct radeon_device *rdev, int flag)
>  
>  		DRM_INFO("Tested GTT->VRAM and VRAM->GTT copy for GTT offset 0x%llx\n",
>  			 gtt_addr - rdev->mc.gtt_start);
> +		continue;
> +
> +out_lclean_unpin:
> +		radeon_bo_unpin(gtt_obj[i]);
> +out_lclean_unres:
> +		radeon_bo_unreserve(gtt_obj[i]);
> +out_lclean_unref:
> +		radeon_bo_unref(&gtt_obj[i]);
> +out_lclean:
> +		for (--i; i >= 0; --i) {
> +			radeon_bo_unpin(gtt_obj[i]);
> +			radeon_bo_unreserve(gtt_obj[i]);
> +			radeon_bo_unref(&gtt_obj[i]);
> +		}
> +		if (fence)
> +			radeon_fence_unref(&fence);
> +		break;
>  	}
>  
> +	radeon_bo_unpin(vram_obj);
> +out_unres:
> +	radeon_bo_unreserve(vram_obj);
> +out_unref:
> +	radeon_bo_unref(&vram_obj);
>  out_cleanup:
> -	if (vram_obj) {
> -		if (radeon_bo_is_reserved(vram_obj)) {
> -			radeon_bo_unpin(vram_obj);
> -			radeon_bo_unreserve(vram_obj);
> -		}
> -		radeon_bo_unref(&vram_obj);
> -	}
> -	if (gtt_obj) {
> -		for (i = 0; i < n; i++) {
> -			if (gtt_obj[i]) {
> -				if (radeon_bo_is_reserved(gtt_obj[i])) {
> -					radeon_bo_unpin(gtt_obj[i]);
> -					radeon_bo_unreserve(gtt_obj[i]);
> -				}
> -				radeon_bo_unref(&gtt_obj[i]);
> -			}
> -		}
> -		kfree(gtt_obj);
> -	}
> -	if (fence) {
> -		radeon_fence_unref(&fence);
> -	}
> +	kfree(gtt_obj);
>  	if (r) {
>  		printk(KERN_WARNING "Error while testing BO move.\n");
>  	}
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2013-06-27 22:05 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-27 11:48 [PATCH WW 00/13] Convert TTM to Wound/wait mutexes Maarten Lankhorst
2013-06-27 11:48 ` [PATCH WW 01/13] reservation: cross-device reservation support, v4 Maarten Lankhorst
2013-06-27 21:45   ` Jerome Glisse
2013-06-27 11:48 ` [PATCH WW 02/13] drm/ttm: make ttm reservation calls behave like reservation calls Maarten Lankhorst
2013-06-27 21:47   ` Jerome Glisse
2013-06-27 11:48 ` [PATCH WW 03/13] drm/nouveau: make flipping lockdep safe Maarten Lankhorst
2013-06-27 11:48 ` [PATCH WW 04/13] drm/ttm: convert to the reservation api Maarten Lankhorst
2013-06-27 22:03   ` Jerome Glisse
2013-06-27 11:48 ` [PATCH WW 05/13] drm/ast: inline reservations Maarten Lankhorst
2013-06-27 11:48 ` [PATCH WW 06/13] drm/cirrus: " Maarten Lankhorst
2013-06-27 11:48 ` [PATCH WW 07/13] drm/mgag200: " Maarten Lankhorst
2013-06-27 11:48 ` [PATCH WW 08/13] drm/radeon: " Maarten Lankhorst
2013-06-27 22:04   ` Jerome Glisse
2013-06-27 11:48 ` [PATCH WW 09/13] drm/ttm: inline ttm_bo_reserve and related calls Maarten Lankhorst
2013-06-27 12:21   ` Daniel Vetter
2013-06-27 11:48 ` [PATCH WW 10/13] drm/ttm: get rid of ttm_bo_is_reserved usage Maarten Lankhorst
2013-06-27 12:23   ` Daniel Vetter
2013-06-27 11:48 ` [PATCH WW 11/13] drm/radeon: " Maarten Lankhorst
2013-06-27 22:05   ` Jerome Glisse [this message]
2013-06-27 11:48 ` [PATCH WW 12/13] drm/vmwgfx: " Maarten Lankhorst
2013-06-27 11:48 ` [PATCH WW 13/13] drm/ttm: get rid of ttm_bo_is_reserved Maarten Lankhorst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130627220514.GE4380@gmail.com \
    --to=j.glisse@gmail.com \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=m.b.lankhorst@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.