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

[Gwan-gyeong Mun]

On 3/20/23 4:25 PM, Matthew Auld wrote:
> 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.
> 
Thanks for the detailed explanation.

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>

Br,
G.G.
>>
>> 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
>>>   };