diff mbox series

drm/i915: round_up the size to the alignment value

Message ID 20220314193206.534257-1-Arunpravin.PaneerSelvam@amd.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: round_up the size to the alignment value | expand

Commit Message

Paneer Selvam, Arunpravin March 14, 2022, 7:32 p.m. UTC 
handle instances when size is not aligned with the min_page_size.
Unigine Heaven has allocation requests for example required pages
are 161 and alignment request is 128. To allocate the left over
33 pages, continues the iteration to find the order value which
is 5 and 0 and when it compares with min_order = 7, triggers the
BUG_ON((order < min_order). To avoid this problem, round_up the
size to the alignment and enable the is_contiguous variable and
the block trimmed to the original size.

Signed-off-by: Arunpravin <Arunpravin.PaneerSelvam@amd.com>
---
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)


base-commit: b37605de46fef48555bf0cf463cccf355c51fac0

Comments

Matthew Auld March 15, 2022, 11:26 a.m. UTC | #1 
On Mon, 14 Mar 2022 at 19:32, Arunpravin
<Arunpravin.PaneerSelvam@amd.com> wrote:
>
> handle instances when size is not aligned with the min_page_size.
> Unigine Heaven has allocation requests for example required pages
> are 161 and alignment request is 128. To allocate the left over
> 33 pages, continues the iteration to find the order value which
> is 5 and 0 and when it compares with min_order = 7, triggers the
> BUG_ON((order < min_order). To avoid this problem, round_up the
> size to the alignment and enable the is_contiguous variable and
> the block trimmed to the original size.
>
> Signed-off-by: Arunpravin <Arunpravin.PaneerSelvam@amd.com>
> ---
>  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
> index 129f668f21ff..318aa731de5b 100644
> --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
> +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
> @@ -40,6 +40,7 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
>         struct i915_ttm_buddy_resource *bman_res;
>         struct drm_buddy *mm = &bman->mm;
>         unsigned long n_pages, lpfn;
> +       bool is_contiguous = 0;
>         u64 min_page_size;
>         u64 size;
>         int err;
> @@ -48,6 +49,9 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
>         if (!lpfn)
>                 lpfn = man->size;
>
> +       if (place->flags & TTM_PL_FLAG_CONTIGUOUS)
> +               is_contiguous = 1;
> +
>         bman_res = kzalloc(sizeof(*bman_res), GFP_KERNEL);
>         if (!bman_res)
>                 return -ENOMEM;
> @@ -71,7 +75,12 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
>
>         GEM_BUG_ON(min_page_size < mm->chunk_size);
>
> -       if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
> +       if (!is_contiguous && !IS_ALIGNED(size, min_page_size)) {
> +               size = round_up(size, min_page_size);
> +               is_contiguous = 1;
> +       }
> +
> +       if (is_contiguous) {

This looks like it will also do roundup_power_of_two()? I assume we
instead just want to feed the list_last_entry() block for trimming?

Anway, we should be able to just make this:

if (WARN_ON(!IS_ALIGNED(size, min_page_size))
    return -EINVAL;

That's at least the currently expected behaviour in i915, just that we
were previously not verifying it here.

>                 unsigned long pages;
>
>                 size = roundup_pow_of_two(size);
> @@ -106,7 +115,7 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
>         if (unlikely(err))
>                 goto err_free_blocks;
>
> -       if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
> +       if (is_contiguous) {
>                 u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT;
>
>                 mutex_lock(&bman->lock);
>
> base-commit: b37605de46fef48555bf0cf463cccf355c51fac0
> --
> 2.25.1
>
Paneer Selvam, Arunpravin March 15, 2022, 3:31 p.m. UTC | #2 
On 15/03/22 4:56 pm, Matthew Auld wrote:
> On Mon, 14 Mar 2022 at 19:32, Arunpravin
> <Arunpravin.PaneerSelvam@amd.com> wrote:
>>
>> handle instances when size is not aligned with the min_page_size.
>> Unigine Heaven has allocation requests for example required pages
>> are 161 and alignment request is 128. To allocate the left over
>> 33 pages, continues the iteration to find the order value which
>> is 5 and 0 and when it compares with min_order = 7, triggers the
>> BUG_ON((order < min_order). To avoid this problem, round_up the
>> size to the alignment and enable the is_contiguous variable and
>> the block trimmed to the original size.
>>
>> Signed-off-by: Arunpravin <Arunpravin.PaneerSelvam@amd.com>
>> ---
>>  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
>> index 129f668f21ff..318aa731de5b 100644
>> --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
>> +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
>> @@ -40,6 +40,7 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
>>         struct i915_ttm_buddy_resource *bman_res;
>>         struct drm_buddy *mm = &bman->mm;
>>         unsigned long n_pages, lpfn;
>> +       bool is_contiguous = 0;
>>         u64 min_page_size;
>>         u64 size;
>>         int err;
>> @@ -48,6 +49,9 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
>>         if (!lpfn)
>>                 lpfn = man->size;
>>
>> +       if (place->flags & TTM_PL_FLAG_CONTIGUOUS)
>> +               is_contiguous = 1;
>> +
>>         bman_res = kzalloc(sizeof(*bman_res), GFP_KERNEL);
>>         if (!bman_res)
>>                 return -ENOMEM;
>> @@ -71,7 +75,12 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
>>
>>         GEM_BUG_ON(min_page_size < mm->chunk_size);
>>
>> -       if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
>> +       if (!is_contiguous && !IS_ALIGNED(size, min_page_size)) {
>> +               size = round_up(size, min_page_size);
>> +               is_contiguous = 1;
>> +       }
>> +
>> +       if (is_contiguous) {
> 
> This looks like it will also do roundup_power_of_two()? I assume we
> instead just want to feed the list_last_entry() block for trimming?
> 
> Anway, we should be able to just make this:
> 
> if (WARN_ON(!IS_ALIGNED(size, min_page_size))
>     return -EINVAL;
> 
> That's at least the currently expected behaviour in i915, just that we
> were previously not verifying it here.

ok sure, then I make changes in amdgpu
> 
>>                 unsigned long pages;
>>
>>                 size = roundup_pow_of_two(size);
>> @@ -106,7 +115,7 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
>>         if (unlikely(err))
>>                 goto err_free_blocks;
>>
>> -       if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
>> +       if (is_contiguous) {
>>                 u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT;
>>
>>                 mutex_lock(&bman->lock);
>>
>> base-commit: b37605de46fef48555bf0cf463cccf355c51fac0
>> --
>> 2.25.1
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 129f668f21ff..318aa731de5b 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -40,6 +40,7 @@  static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
 	struct i915_ttm_buddy_resource *bman_res;
 	struct drm_buddy *mm = &bman->mm;
 	unsigned long n_pages, lpfn;
+	bool is_contiguous = 0;
 	u64 min_page_size;
 	u64 size;
 	int err;
@@ -48,6 +49,9 @@  static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
 	if (!lpfn)
 		lpfn = man->size;
 
+	if (place->flags & TTM_PL_FLAG_CONTIGUOUS)
+		is_contiguous = 1;
+
 	bman_res = kzalloc(sizeof(*bman_res), GFP_KERNEL);
 	if (!bman_res)
 		return -ENOMEM;
@@ -71,7 +75,12 @@  static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
 
 	GEM_BUG_ON(min_page_size < mm->chunk_size);
 
-	if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
+	if (!is_contiguous && !IS_ALIGNED(size, min_page_size)) {
+		size = round_up(size, min_page_size);
+		is_contiguous = 1;
+	}
+
+	if (is_contiguous) {
 		unsigned long pages;
 
 		size = roundup_pow_of_two(size);
@@ -106,7 +115,7 @@  static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
 	if (unlikely(err))
 		goto err_free_blocks;
 
-	if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {
+	if (is_contiguous) {
 		u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT;
 
 		mutex_lock(&bman->lock);