[Intel-gfx] [PATCH v3 6/6] drm/ttm: Switch to using the new res callback
Arunpravin Paneer Selvam
arunpravin.paneerselvam at amd.com
Fri Jul 29 06:33:27 UTC 2022
On 7/28/2022 9:07 PM, Matthew Auld wrote:
> On 28/07/2022 15:33, Arunpravin Paneer Selvam wrote:
>> Apply new intersect and compatible callback instead
>> of having a generic placement range verfications.
>>
>> v2: Added a separate callback for compatiblilty
>> checks (Christian)
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> Signed-off-by: Arunpravin Paneer Selvam
>> <Arunpravin.PaneerSelvam at amd.com>
>
> There is also some code at the bottom of i915_ttm_buddy_man_alloc()
> playing games with res->start, which I think can be safely deleted
> with this series (now that we have proper ->compatible() hook).
>
> Also, is the plan to remove res->start completely, or does that still
> have a use?
yes we should remove res->start completely, I am working on it, planning
to send in a separate series as amdgpu uses it in many places, and in
some places we set res->start to AMDGPU_BO_INVALID_OFFSET,
I should find an alternative to indicate the invalid offset BO. Also,
res->start used in drm/drm_gem_vram_helper.c at drm_gem_vram_pg_offset()
function. I am removing all the dependencies, I will send the
patches in a separate series. I think i915 doesn't use res->start in its
own driver code.
>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 45 +++++++------------------
>> drivers/gpu/drm/ttm/ttm_bo.c | 9 +++--
>> drivers/gpu/drm/ttm/ttm_resource.c | 5 +--
>> 3 files changed, 20 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 170935c294f5..7d25a10395c0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1328,11 +1328,12 @@ uint64_t amdgpu_ttm_tt_pte_flags(struct
>> amdgpu_device *adev, struct ttm_tt *ttm,
>> static bool amdgpu_ttm_bo_eviction_valuable(struct
>> ttm_buffer_object *bo,
>> const struct ttm_place *place)
>> {
>> - unsigned long num_pages = bo->resource->num_pages;
>> struct dma_resv_iter resv_cursor;
>> - struct amdgpu_res_cursor cursor;
>> struct dma_fence *f;
>> + if (!amdgpu_bo_is_amdgpu_bo(bo))
>> + return ttm_bo_eviction_valuable(bo, place);
>> +
>> /* Swapout? */
>> if (bo->resource->mem_type == TTM_PL_SYSTEM)
>> return true;
>> @@ -1351,40 +1352,20 @@ static bool
>> amdgpu_ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>> return false;
>> }
>> - switch (bo->resource->mem_type) {
>> - case AMDGPU_PL_PREEMPT:
>> - /* Preemptible BOs don't own system resources managed by the
>> - * driver (pages, VRAM, GART space). They point to resources
>> - * owned by someone else (e.g. pageable memory in user mode
>> - * or a DMABuf). They are used in a preemptible context so we
>> - * can guarantee no deadlocks and good QoS in case of MMU
>> - * notifiers or DMABuf move notifiers from the resource owner.
>> - */
>> + /* Preemptible BOs don't own system resources managed by the
>> + * driver (pages, VRAM, GART space). They point to resources
>> + * owned by someone else (e.g. pageable memory in user mode
>> + * or a DMABuf). They are used in a preemptible context so we
>> + * can guarantee no deadlocks and good QoS in case of MMU
>> + * notifiers or DMABuf move notifiers from the resource owner.
>> + */
>> + if (bo->resource->mem_type == AMDGPU_PL_PREEMPT)
>> return false;
>> - case TTM_PL_TT:
>> - if (amdgpu_bo_is_amdgpu_bo(bo) &&
>> - amdgpu_bo_encrypted(ttm_to_amdgpu_bo(bo)))
>> - return false;
>> - return true;
>> - case TTM_PL_VRAM:
>> - /* Check each drm MM node individually */
>> - amdgpu_res_first(bo->resource, 0, (u64)num_pages << PAGE_SHIFT,
>> - &cursor);
>> - while (cursor.remaining) {
>> - if (place->fpfn < PFN_DOWN(cursor.start + cursor.size)
>> - && !(place->lpfn &&
>> - place->lpfn <= PFN_DOWN(cursor.start)))
>> - return true;
>> -
>> - amdgpu_res_next(&cursor, cursor.size);
>> - }
>> + if (bo->resource->mem_type == TTM_PL_TT &&
>> + amdgpu_bo_encrypted(ttm_to_amdgpu_bo(bo)))
>> return false;
>> - default:
>> - break;
>> - }
>> -
>> return ttm_bo_eviction_valuable(bo, place);
>> }
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>> b/drivers/gpu/drm/ttm/ttm_bo.c
>> index c1bd006a5525..03409409e43e 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -518,6 +518,9 @@ static int ttm_bo_evict(struct ttm_buffer_object
>> *bo,
>> bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
>> const struct ttm_place *place)
>> {
>> + struct ttm_resource *res = bo->resource;
>> + struct ttm_device *bdev = bo->bdev;
>> +
>> dma_resv_assert_held(bo->base.resv);
>> if (bo->resource->mem_type == TTM_PL_SYSTEM)
>> return true;
>> @@ -525,11 +528,7 @@ bool ttm_bo_eviction_valuable(struct
>> ttm_buffer_object *bo,
>> /* Don't evict this BO if it's outside of the
>> * requested placement range
>> */
>> - if (place->fpfn >= (bo->resource->start +
>> bo->resource->num_pages) ||
>> - (place->lpfn && place->lpfn <= bo->resource->start))
>> - return false;
>> -
>> - return true;
>> + return ttm_resource_intersect(bdev, res, place, bo->base.size);
>> }
>> EXPORT_SYMBOL(ttm_bo_eviction_valuable);
>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
>> b/drivers/gpu/drm/ttm/ttm_resource.c
>> index 84a6fe9e976e..c745faf72b1a 100644
>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>> @@ -316,6 +316,8 @@ static bool ttm_resource_places_compat(struct
>> ttm_resource *res,
>> const struct ttm_place *places,
>> unsigned num_placement)
>> {
>> + struct ttm_buffer_object *bo = res->bo;
>> + struct ttm_device *bdev = bo->bdev;
>> unsigned i;
>> if (res->placement & TTM_PL_FLAG_TEMPORARY)
>> @@ -324,8 +326,7 @@ static bool ttm_resource_places_compat(struct
>> ttm_resource *res,
>> for (i = 0; i < num_placement; i++) {
>> const struct ttm_place *heap = &places[i];
>> - if (res->start < heap->fpfn || (heap->lpfn &&
>> - (res->start + res->num_pages) > heap->lpfn))
>> + if (!ttm_resource_compatible(bdev, res, heap, bo->base.size))
>> continue;
>> if ((res->mem_type == heap->mem_type) &&
More information about the Intel-gfx
mailing list