[PATCH] drm/buddy: Add start address support to trim function
Matthew Auld
matthew.auld at intel.com
Fri Jul 19 10:31:40 UTC 2024
On 17/07/2024 16:02, Paneer Selvam, Arunpravin wrote:
>
>
> On 7/16/2024 3:34 PM, Matthew Auld wrote:
>> On 16/07/2024 10:50, Paneer Selvam, Arunpravin wrote:
>>> Hi Matthew,
>>>
>>> On 7/10/2024 6:20 PM, Matthew Auld wrote:
>>>> On 10/07/2024 07:03, Paneer Selvam, Arunpravin wrote:
>>>>> Thanks Alex.
>>>>>
>>>>> Hi Matthew,
>>>>> Any comments?
>>>>
>>>> Do we not pass the required address alignment when allocating the
>>>> pages in the first place?
>>> If address alignment is really useful, we can add that in the
>>> drm_buddy_alloc_blocks() function.
>>
>> I mean don't we already pass the min page size, which should give us
>> matching physical address alignment?
> I think we don't need to align the address to the passed min_block_size
> value for all the contiguous
> buffers, so I thought that decision we can leave it to the drivers and
> they can achieve that through trim function
> in this kind of a specific request.
I would have assumed it would be simpler to use min_block_size and then
trim the size, if it's too big? That would then also take care of the
try_harder case?
Also how are we dealing with the multi-block try_harder case? AFAICT we
only allow trimming single block atm, or is it not possible to trigger
that path here? Or are we handling that somehow?
>
> https://patchwork.freedesktop.org/series/136150/
> We are getting this sparse error from the Intel CI. Do you think these
> errors are introduced with this patches?
I think it's safe to ignore, there seem to be other series with the same
thing.
>
> Thanks,
> Arun.
>>
>>>
>>> Thanks,
>>> Arun.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Arun.
>>>>>
>>>>> On 7/9/2024 1:42 AM, Alex Deucher wrote:
>>>>>> On Thu, Jul 4, 2024 at 4:40 AM Arunpravin Paneer Selvam
>>>>>> <Arunpravin.PaneerSelvam at amd.com> wrote:
>>>>>>> - Add a new start parameter in trim function to specify exact
>>>>>>> address from where to start the trimming. This would help us
>>>>>>> in situations like if drivers would like to do address alignment
>>>>>>> for specific requirements.
>>>>>>>
>>>>>>> - Add a new flag DRM_BUDDY_TRIM_DISABLE. Drivers can use this
>>>>>>> flag to disable the allocator trimming part. This patch enables
>>>>>>> the drivers control trimming and they can do it themselves
>>>>>>> based on the application requirements.
>>>>>>>
>>>>>>> v1:(Matthew)
>>>>>>> - check new_start alignment with min chunk_size
>>>>>>> - use range_overflows()
>>>>>>>
>>>>>>> Signed-off-by: Arunpravin Paneer Selvam
>>>>>>> <Arunpravin.PaneerSelvam at amd.com>
>>>>>> Series is:
>>>>>> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>>>>>>
>>>>>> I'd like to take this series through the amdgpu tree if there are no
>>>>>> objections as it's required for display buffers on some chips and I'd
>>>>>> like to make sure it lands in 6.11.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Alex
>>>>>>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/drm_buddy.c | 25
>>>>>>> +++++++++++++++++++++++--
>>>>>>> drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 2 +-
>>>>>>> include/drm/drm_buddy.h | 2 ++
>>>>>>> 3 files changed, 26 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/drm_buddy.c
>>>>>>> b/drivers/gpu/drm/drm_buddy.c
>>>>>>> index 94f8c34fc293..8cebe1fa4e9d 100644
>>>>>>> --- a/drivers/gpu/drm/drm_buddy.c
>>>>>>> +++ b/drivers/gpu/drm/drm_buddy.c
>>>>>>> @@ -851,6 +851,7 @@ static int __alloc_contig_try_harder(struct
>>>>>>> drm_buddy *mm,
>>>>>>> * drm_buddy_block_trim - free unused pages
>>>>>>> *
>>>>>>> * @mm: DRM buddy manager
>>>>>>> + * @start: start address to begin the trimming.
>>>>>>> * @new_size: original size requested
>>>>>>> * @blocks: Input and output list of allocated blocks.
>>>>>>> * MUST contain single block as input to be trimmed.
>>>>>>> @@ -866,11 +867,13 @@ static int __alloc_contig_try_harder(struct
>>>>>>> drm_buddy *mm,
>>>>>>> * 0 on success, error code on failure.
>>>>>>> */
>>>>>>> int drm_buddy_block_trim(struct drm_buddy *mm,
>>>>>>> + u64 *start,
>>>>>>> u64 new_size,
>>>>>>> struct list_head *blocks)
>>>>>>> {
>>>>>>> struct drm_buddy_block *parent;
>>>>>>> struct drm_buddy_block *block;
>>>>>>> + u64 block_start, block_end;
>>>>>>> LIST_HEAD(dfs);
>>>>>>> u64 new_start;
>>>>>>> int err;
>>>>>>> @@ -882,6 +885,9 @@ int drm_buddy_block_trim(struct drm_buddy *mm,
>>>>>>> struct drm_buddy_block,
>>>>>>> link);
>>>>>>>
>>>>>>> + block_start = drm_buddy_block_offset(block);
>>>>>>> + block_end = block_start + drm_buddy_block_size(mm, block);
>>>>>>> +
>>>>>>> if (WARN_ON(!drm_buddy_block_is_allocated(block)))
>>>>>>> return -EINVAL;
>>>>>>>
>>>>>>> @@ -894,6 +900,20 @@ int drm_buddy_block_trim(struct drm_buddy *mm,
>>>>>>> if (new_size == drm_buddy_block_size(mm, block))
>>>>>>> return 0;
>>>>>>>
>>>>>>> + new_start = block_start;
>>>>>>> + if (start) {
>>>>>>> + new_start = *start;
>>>>>>> +
>>>>>>> + if (new_start < block_start)
>>>>>>> + return -EINVAL;
>>>>>>> +
>>>>>>> + if (!IS_ALIGNED(new_start, mm->chunk_size))
>>>>>>> + return -EINVAL;
>>>>>>> +
>>>>>>> + if (range_overflows(new_start, new_size, block_end))
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> +
>>>>>>> list_del(&block->link);
>>>>>>> mark_free(mm, block);
>>>>>>> mm->avail += drm_buddy_block_size(mm, block);
>>>>>>> @@ -904,7 +924,6 @@ int drm_buddy_block_trim(struct drm_buddy *mm,
>>>>>>> parent = block->parent;
>>>>>>> block->parent = NULL;
>>>>>>>
>>>>>>> - new_start = drm_buddy_block_offset(block);
>>>>>>> list_add(&block->tmp_link, &dfs);
>>>>>>> err = __alloc_range(mm, &dfs, new_start, new_size,
>>>>>>> blocks, NULL);
>>>>>>> if (err) {
>>>>>>> @@ -1066,7 +1085,8 @@ int drm_buddy_alloc_blocks(struct drm_buddy
>>>>>>> *mm,
>>>>>>> } while (1);
>>>>>>>
>>>>>>> /* Trim the allocated block to the required size */
>>>>>>> - if (original_size != size) {
>>>>>>> + if (!(flags & DRM_BUDDY_TRIM_DISABLE) &&
>>>>>>> + original_size != size) {
>>>>>>> struct list_head *trim_list;
>>>>>>> LIST_HEAD(temp);
>>>>>>> u64 trim_size;
>>>>>>> @@ -1083,6 +1103,7 @@ int drm_buddy_alloc_blocks(struct drm_buddy
>>>>>>> *mm,
>>>>>>> }
>>>>>>>
>>>>>>> drm_buddy_block_trim(mm,
>>>>>>> + NULL,
>>>>>>> trim_size,
>>>>>>> trim_list);
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>>>>> b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>>>>> index fe3779fdba2c..423b261ea743 100644
>>>>>>> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>>>>> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
>>>>>>> @@ -150,7 +150,7 @@ static int xe_ttm_vram_mgr_new(struct
>>>>>>> ttm_resource_manager *man,
>>>>>>> } while (remaining_size);
>>>>>>>
>>>>>>> if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
>>>>>>> - if (!drm_buddy_block_trim(mm, vres->base.size,
>>>>>>> &vres->blocks))
>>>>>>> + if (!drm_buddy_block_trim(mm, NULL,
>>>>>>> vres->base.size, &vres->blocks))
>>>>>>> size = vres->base.size;
>>>>>>> }
>>>>>>>
>>>>>>> diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
>>>>>>> index 82570f77e817..0c2f735f0265 100644
>>>>>>> --- a/include/drm/drm_buddy.h
>>>>>>> +++ b/include/drm/drm_buddy.h
>>>>>>> @@ -27,6 +27,7 @@
>>>>>>> #define DRM_BUDDY_CONTIGUOUS_ALLOCATION BIT(2)
>>>>>>> #define DRM_BUDDY_CLEAR_ALLOCATION BIT(3)
>>>>>>> #define DRM_BUDDY_CLEARED BIT(4)
>>>>>>> +#define DRM_BUDDY_TRIM_DISABLE BIT(5)
>>>>>>>
>>>>>>> struct drm_buddy_block {
>>>>>>> #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
>>>>>>> @@ -155,6 +156,7 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
>>>>>>> unsigned long flags);
>>>>>>>
>>>>>>> int drm_buddy_block_trim(struct drm_buddy *mm,
>>>>>>> + u64 *start,
>>>>>>> u64 new_size,
>>>>>>> struct list_head *blocks);
>>>>>>>
>>>>>>> --
>>>>>>> 2.25.1
>>>>>>>
>>>>>
>>>
>
More information about the amd-gfx
mailing list