Re: [PATCH] drm/ttm: Don't evict BOs outside of the requested placement range

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

On 09.10.2014 19:22, Alan Swanson wrote:
> On 2014-10-09 07:02, Michel Dänzer wrote:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> The radeon driver uses placement range restrictions for several reasons,
>> in particular to make sure BOs in VRAM can be accessed by the CPU, e.g.
>> during a page fault.
>>
>> Without this change, TTM could evict other BOs while trying to satisfy
>> the requested placement, even if the evicted BOs were outside of the
>> requested placement range. Doing so didn't free up any space in the
>> requested placement range, so the (potentially high) eviction cost was
>> incurred for no benefit.
>>
>> Nominating for stable because radeon driver changes in 3.17 made this
>> much more noticeable than before.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84662
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> ---
>>  drivers/gpu/drm/ttm/ttm_bo.c | 20 +++++++++++++++++---
>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 8f5cec6..407fa2d 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -709,6 +709,7 @@ out:
>>
>>  static int ttm_mem_evict_first(struct ttm_bo_device *bdev,
>>                  uint32_t mem_type,
>> +                const struct ttm_place *place,
>>                  bool interruptible,
>>                  bool no_wait_gpu)
>>  {
>> @@ -720,8 +721,21 @@ static int ttm_mem_evict_first(struct
>> ttm_bo_device *bdev,
>>      spin_lock(&glob->lru_lock);
>>      list_for_each_entry(bo, &man->lru, lru) {
>>          ret = __ttm_bo_reserve(bo, false, true, false, NULL);
>> -        if (!ret)
>> +        if (!ret) {
>> +            if (place && (place->fpfn || place->lpfn)) {
>> +                /* Don't evict this BO if it's outside of the
>> +                 * requested placement range
>> +                 */
>> +                if (place->fpfn >= (bo->mem.start + bo->mem.size) ||
>> +                    (place->lpfn && place->lpfn <= bo->mem.start)) {
>> +                    __ttm_bo_unreserve(bo);
>> +                    ret = -EBUSY;
>> +                    continue;
>> +                }
>> +            }
>> +
>>              break;
>> +        }
>>      }
>>
>>      if (ret) {
>> @@ -782,7 +796,7 @@ static int ttm_bo_mem_force_space(struct
>> ttm_buffer_object *bo,
>>              return ret;
>>          if (mem->mm_node)
>>              break;
>> -        ret = ttm_mem_evict_first(bdev, mem_type,
>> +        ret = ttm_mem_evict_first(bdev, mem_type, place,
>>                        interruptible, no_wait_gpu);
>>          if (unlikely(ret != 0))
>>              return ret;

[...]

> I believe you need to "s/place/placement/" over this patch.

The fpfn and lpfn members were moved from struct ttm_placement to a new 
struct ttm_place in f1217ed09f827e42a49ffa6a5aab672aa6f57a65.

If you mean something else, please elaborate.


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