Re: [PATCH] drm/ttm: add minimum residency constraint for bo eviction

[Thomas Hellstrom <thomas@shipmail.org>]

On 11/28/2012 04:58 PM, j.glisse@gmail.com wrote:
> From: Jerome Glisse <jglisse@redhat.com>
>
> This patch add a minimum residency time configurable for each memory
> pool (VRAM, GTT, ...). Intention is to avoid having a lot of memory
> eviction from VRAM up to a point where the GPU pretty much spend all
> it's time moving things in and out.

This patch seems odd to me.

It seems the net effect is to refuse evictions from VRAM and make buffers go
somewhere else, and that makes things faster?

Why don't they go there in the first place instead of trying to force 
them into VRAM,
when VRAM is full?

/Thomas

>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> ---
>   drivers/gpu/drm/radeon/radeon_ttm.c | 3 +++
>   drivers/gpu/drm/ttm/ttm_bo.c        | 7 +++++++
>   include/drm/ttm/ttm_bo_api.h        | 1 +
>   include/drm/ttm/ttm_bo_driver.h     | 1 +
>   4 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 5ebe1b3..88722c4 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -129,11 +129,13 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>   	switch (type) {
>   	case TTM_PL_SYSTEM:
>   		/* System memory */
> +		man->minimum_residency_time_ms = 0;
>   		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>   		man->available_caching = TTM_PL_MASK_CACHING;
>   		man->default_caching = TTM_PL_FLAG_CACHED;
>   		break;
>   	case TTM_PL_TT:
> +		man->minimum_residency_time_ms = 0;
>   		man->func = &ttm_bo_manager_func;
>   		man->gpu_offset = rdev->mc.gtt_start;
>   		man->available_caching = TTM_PL_MASK_CACHING;
> @@ -156,6 +158,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>   		break;
>   	case TTM_PL_VRAM:
>   		/* "On-card" video ram */
> +		man->minimum_residency_time_ms = 500;
>   		man->func = &ttm_bo_manager_func;
>   		man->gpu_offset = rdev->mc.vram_start;
>   		man->flags = TTM_MEMTYPE_FLAG_FIXED |
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 39dcc58..40476121 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -452,6 +452,7 @@ moved:
>   		bo->cur_placement = bo->mem.placement;
>   	} else
>   		bo->offset = 0;
> +	bo->jiffies = jiffies;
>   
>   	return 0;
>   
> @@ -810,6 +811,12 @@ retry:
>   	}
>   
>   	bo = list_first_entry(&man->lru, struct ttm_buffer_object, lru);
> +
> +	if (time_after(jiffies, bo->jiffies) && jiffies_to_msecs(jiffies - bo->jiffies) >= man->minimum_residency_time_ms) {
> +		spin_unlock(&glob->lru_lock);
> +		return -EBUSY;
> +	}
> +
>   	kref_get(&bo->list_kref);
>   
>   	if (!list_empty(&bo->ddestroy)) {
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index e8028ad..9e12313 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -275,6 +275,7 @@ struct ttm_buffer_object {
>   
>   	unsigned long offset;
>   	uint32_t cur_placement;
> +	unsigned long jiffies;
>   
>   	struct sg_table *sg;
>   };
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index d803b92..7f60a18e6 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -280,6 +280,7 @@ struct ttm_mem_type_manager {
>   	struct mutex io_reserve_mutex;
>   	bool use_io_reserve_lru;
>   	bool io_reserve_fastpath;
> +	unsigned long minimum_residency_time_ms;
>   
>   	/*
>   	 * Protected by @io_reserve_mutex: