Re: [PATCH 02/10] drm/ttm: remove ttm_bo_cleanup_memtype_use

[Thomas Hellstrom <thellstrom@vmware.com>]

Hi,

On 11/12/2012 03:00 PM, Maarten Lankhorst wrote:
> move to release_list instead

Can you describe why this change is made? cleanup? reorder locks in a 
later patch?
Also please describe why you need move_notify and ttm unbind / destroy 
to be outside of
reservation, because that's the main change in this patch and it's not 
even mentioned in the
commit message.

Thanks,
Thomas


>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c | 47 +++++++++++++-------------------------------
>   1 file changed, 14 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 9c48e8f..74d6e7c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -143,12 +143,20 @@ static void ttm_bo_release_list(struct kref *list_kref)
>   	BUG_ON(atomic_read(&bo->kref.refcount));
>   	BUG_ON(atomic_read(&bo->cpu_writers));
>   	BUG_ON(bo->sync_obj != NULL);
> -	BUG_ON(bo->mem.mm_node != NULL);
>   	BUG_ON(!list_empty(&bo->lru));
>   	BUG_ON(!list_empty(&bo->ddestroy));
>   
> -	if (bo->ttm)
> +	if (bo->bdev->driver->move_notify)
> +		bo->bdev->driver->move_notify(bo, NULL);
> +
> +	if (bo->ttm) {
> +		ttm_tt_unbind(bo->ttm);
>   		ttm_tt_destroy(bo->ttm);
> +		bo->ttm = NULL;
> +	}
> +	ttm_bo_mem_put(bo, &bo->mem);
> +	BUG_ON(bo->mem.mm_node != NULL);
> +
>   	atomic_dec(&bo->glob->bo_count);
>   	if (bo->destroy)
>   		bo->destroy(bo);
> @@ -466,35 +474,6 @@ out_err:
>   	return ret;
>   }
>   
> -/**
> - * Call bo::reserved.
> - * Will release GPU memory type usage on destruction.
> - * This is the place to put in driver specific hooks to release
> - * driver private resources.
> - * Will release the bo::reserved lock.
> - */
> -
> -static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
> -{
> -	if (bo->bdev->driver->move_notify)
> -		bo->bdev->driver->move_notify(bo, NULL);
> -
> -	if (bo->ttm) {
> -		ttm_tt_unbind(bo->ttm);
> -		ttm_tt_destroy(bo->ttm);
> -		bo->ttm = NULL;
> -	}
> -	ttm_bo_mem_put(bo, &bo->mem);
> -
> -	atomic_set(&bo->reserved, 0);
> -
> -	/*
> -	 * Make processes trying to reserve really pick it up.
> -	 */
> -	smp_mb__after_atomic_dec();
> -	wake_up_all(&bo->event_queue);
> -}
> -
>   static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
>   {
>   	struct ttm_bo_device *bdev = bo->bdev;
> @@ -523,8 +502,9 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo)
>   		spin_unlock(&bdev->fence_lock);
>   		put_count = ttm_bo_del_from_lru(bo);
>   
> +		atomic_set(&bo->reserved, 0);
> +		wake_up_all(&bo->event_queue);
>   		spin_unlock(&glob->lru_lock);
> -		ttm_bo_cleanup_memtype_use(bo);
>   
>   		ttm_bo_list_ref_sub(bo, put_count, true);
>   
> @@ -619,8 +599,9 @@ retry_reserve:
>   	list_del_init(&bo->ddestroy);
>   	++put_count;
>   
> +	atomic_set(&bo->reserved, 0);
> +	wake_up_all(&bo->event_queue);
>   	spin_unlock(&glob->lru_lock);
> -	ttm_bo_cleanup_memtype_use(bo);
>   
>   	ttm_bo_list_ref_sub(bo, put_count, true);
>