[Intel-xe] [PATCH v2 6/6] drm/xe/buddy: add compatible and intersects hooks

[Matthew Auld]

On 20/03/2023 14:15, Gwan-gyeong Mun wrote:
> 
> 
> On 3/8/23 12:17 PM, Matthew Auld wrote:
>> Copy this from i915. We need .compatible for lmem -> lmem transfers, so
>> they don't just get nooped by ttm, if need to move something from
>> mappable to non-mappble or vice versa. The .intersects is needed for
>> eviction, to determine if a victim resource is worth eviction. e.g if we
>> need mappable space there is no point in evicting a resource that has
>> zero mappable pages.
>>
>> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 62 ++++++++++++++++++++++++++++
>>   1 file changed, 62 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c 
>> b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>> index f703e962f499..8dd33ac65499 100644
>> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>> @@ -230,9 +230,71 @@ static void xe_ttm_vram_mgr_debug(struct 
>> ttm_resource_manager *man,
>>       drm_printf(printer, "man size:%llu\n", man->size);
>>   }
>> +static bool xe_ttm_vram_mgr_intersects(struct ttm_resource_manager *man,
>> +                       struct ttm_resource *res,
>> +                       const struct ttm_place *place,
>> +                       size_t size)
>> +{
>> +    struct xe_ttm_vram_mgr *mgr = to_xe_ttm_vram_mgr(man);
>> +    struct xe_ttm_vram_mgr_resource *vres =
>> +        to_xe_ttm_vram_mgr_resource(res);
>> +    struct drm_buddy *mm = &mgr->mm;
>> +    struct drm_buddy_block *block;
>> +
>> +    if (!place->fpfn && !place->lpfn)
>> +        return true;
>> +Hi Matt,
> I understand that ttm_resource_intersects() returns true in the 
> following case,
> 
> if (!place || !man->func->intersects)
>          return true;
> 
> so this line can handle it like this, but in a real scenario, is it 
> possible to have a situation where place->fpfn==0 and place->lpfn==0?

Yeah, fpfn = 0 and lpfn = 0 is the common case, where the user doesn't 
care specifically where it is placed in VRAM. Therefore everything 
intersects (potentially valuable to evict), and is also always going to 
be compatible, so we just always return true.

> 
> Except for the routine that does the same check in 
> xe_ttm_vram_mgr_compatible(), the rest of it looks good.
> 
> br,
> G.G.
>> +    if (!place->fpfn && place->lpfn == mgr->visible_size >> PAGE_SHIFT)
>> +        return vres->used_visible_size > 0;
>> +
>> +    list_for_each_entry(block, &vres->blocks, link) {
>> +        unsigned long fpfn =
>> +            drm_buddy_block_offset(block) >> PAGE_SHIFT;
>> +        unsigned long lpfn = fpfn +
>> +            (drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
>> +
>> +        if (place->fpfn < lpfn && place->lpfn > fpfn)
>> +            return true;
>> +    }
>> +
>> +    return false;
>> +}
>> +
>> +static bool xe_ttm_vram_mgr_compatible(struct ttm_resource_manager *man,
>> +                       struct ttm_resource *res,
>> +                       const struct ttm_place *place,
>> +                       size_t size)
>> +{
>> +    struct xe_ttm_vram_mgr *mgr = to_xe_ttm_vram_mgr(man);
>> +    struct xe_ttm_vram_mgr_resource *vres =
>> +        to_xe_ttm_vram_mgr_resource(res);
>> +    struct drm_buddy *mm = &mgr->mm;
>> +    struct drm_buddy_block *block;
>> +
>> +    if (!place->fpfn && !place->lpfn)
>> +        return true;
>> +
>> +    if (!place->fpfn && place->lpfn == mgr->visible_size >> PAGE_SHIFT)
>> +        return vres->used_visible_size == size;
>> +
>> +    list_for_each_entry(block, &vres->blocks, link) {
>> +        unsigned long fpfn =
>> +            drm_buddy_block_offset(block) >> PAGE_SHIFT;
>> +        unsigned long lpfn = fpfn +
>> +            (drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
>> +
>> +        if (fpfn < place->fpfn || lpfn > place->lpfn)
>> +            return false;
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>   static const struct ttm_resource_manager_func xe_ttm_vram_mgr_func = {
>>       .alloc    = xe_ttm_vram_mgr_new,
>>       .free    = xe_ttm_vram_mgr_del,
>> +    .intersects = xe_ttm_vram_mgr_intersects,
>> +    .compatible = xe_ttm_vram_mgr_compatible,
>>       .debug    = xe_ttm_vram_mgr_debug
>>   };