Re: [PATCH] drm/ttm: rename ttm_place::fpfn/lpfn to param1/param2
From: Christian König
Date: Fri Jun 19 2026 - 03:24:43 EST
On 6/18/26 20:41, Tvrtko Ursulin wrote:
>
> On 18/06/2026 15:39, Arunpravin Paneer Selvam wrote:
>> The fpfn/lpfn fields in struct ttm_place were named after page frame
>> numbers, but they are really just placement parameters passed to the
>> backend resource manager. Rename them to the generic param1/param2
>> and document that their interpretation is backend-defined. The VRAM
>> range manager continues to treat them as the first and last valid
>> page frame number, so behaviour is unchanged.
>>
>> This decouples the API from PFN/range-specific semantics so that
>> these fields can be used more flexibly in the future (e.g.,
>> mask-based or segment-aware placement constraints). No functional
>> change.
>>
>> Suggested-by: Christian König <christian.koenig@xxxxxxx>
>> Cc: Christian König <christian.koenig@xxxxxxx>
>> Cc: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
>> Cc: Matthew Auld <matthew.auld@xxxxxxxxx>
>> Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
>> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@xxxxxxx>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 16 +++---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 4 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 8 +--
>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 46 +++++++--------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 24 ++++----
>> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 4 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 16 +++---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 32 +++++------
>> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +-
>> .../gpu/drm/amd/ras/ras_mgr/amdgpu_ras_cmd.c | 2 +-
>> drivers/gpu/drm/drm_gem_vram_helper.c | 4 +-
>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 18 +++---
>> drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 56 +++++++++----------
>> drivers/gpu/drm/i915/intel_region_ttm.c | 14 ++---
>> drivers/gpu/drm/loongson/lsdc_ttm.c | 4 +-
>> drivers/gpu/drm/nouveau/nouveau_bo.c | 22 ++++----
>> drivers/gpu/drm/nouveau/nouveau_mem.c | 8 +--
>> drivers/gpu/drm/qxl/qxl_object.c | 4 +-
>> drivers/gpu/drm/qxl/qxl_ttm.c | 4 +-
>> drivers/gpu/drm/radeon/radeon_object.c | 28 +++++-----
>> drivers/gpu/drm/radeon/radeon_ttm.c | 18 +++---
>> drivers/gpu/drm/radeon/radeon_uvd.c | 8 +--
>> drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c | 16 +++---
>> drivers/gpu/drm/ttm/tests/ttm_mock_manager.c | 8 +--
>> drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +-
>> drivers/gpu/drm/ttm/ttm_range_manager.c | 18 +++---
>> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 30 +++++-----
>> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 8 +--
>> drivers/gpu/drm/xe/xe_bo.c | 24 ++++----
>> drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 40 ++++++-------
>> include/drm/ttm/ttm_placement.h | 15 +++--
>> 32 files changed, 259 insertions(+), 252 deletions(-)
>
> <snip>
>> if (unlikely(ret)) {
>> @@ -124,8 +124,8 @@ static bool ttm_range_man_intersects(struct ttm_resource_manager *man,
>> u32 num_pages = PFN_UP(size);
>> /* Don't evict BOs outside of the requested placement range */
>> - if (place->fpfn >= (node->start + num_pages) ||
>> - (place->lpfn && place->lpfn <= node->start))
>> + if (place->param1 >= (node->start + num_pages) ||
>> + (place->param2 && place->param2 <= node->start))
>> return false;
>> return true;
>> @@ -139,8 +139,8 @@ static bool ttm_range_man_compatible(struct ttm_resource_manager *man,
>> struct drm_mm_node *node = &to_ttm_range_mgr_node(res)->mm_nodes[0];
>> u32 num_pages = PFN_UP(size);
>> - if (node->start < place->fpfn ||
>> - (place->lpfn && (node->start + num_pages) > place->lpfn))
>> + if (node->start < place->param1 ||
>> + (place->param2 && (node->start + num_pages) > place->param2))
>> return false;
>>
>
> Not very readable ie. good for ease of maintenance - how about an union? Would also avoid having to touch all the drivers.
>
> Something like:
>
> union {
> struct {
> u64 fpfn;
> u64 lpfn;
> };
> u64 drvparam[2];
> };
I like that idea, it also makes the patch way more smaler.
A bit more background might help: The fpfn/lpfn parameters are specific to the ttm_range_manager backend, but we are planning to use them completely different (e.g. as bitmask) for driver specific backends.
@Arun could you add some comments and use that approach as well?
Thanks,
Christian.
>
> Regards,
>
> Tvrtko
>> diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
>> index ab2639e42c54..2c38674be102 100644
>> --- a/include/drm/ttm/ttm_placement.h
>> +++ b/include/drm/ttm/ttm_placement.h
>> @@ -73,16 +73,23 @@
>> /**
>> * struct ttm_place
>> *
>> - * @fpfn: first valid page frame number to put the object
>> - * @lpfn: last valid page frame number to put the object
>> + * @param1: generic placement parameter, interpretation depends on the
>> + * backend resource manager. For range-based managers (e.g. the
>> + * VRAM range manager, the buddy managers in amdgpu/i915/xe and
>> + * the TTM range manager) this is the start of the allowed range,
>> + * typically expressed as a page frame number.
>> + * @param2: generic placement parameter, interpretation depends on the
>> + * backend resource manager. For range-based managers this is the
>> + * exclusive end of the allowed range (a value of 0 means
>> + * "no upper bound").
>> * @mem_type: One of TTM_PL_* where the resource should be allocated from.
>> * @flags: memory domain and caching flags for the object
>> *
>> * Structure indicating a possible place to put an object.
>> */
>> struct ttm_place {
>> - uint64_t fpfn;
>> - uint64_t lpfn;
>> + uint64_t param1;
>> + uint64_t param2;
>> uint32_t mem_type;
>> uint32_t flags;
>> };
>>
>>