* [PATCH] drm/amdgpu: Always align dumb buffer at PAGE_SIZE
@ 2022-09-22 23:04 Lepton Wu
2022-09-23 5:14 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Lepton Wu @ 2022-09-22 23:04 UTC (permalink / raw)
To: amd-gfx; +Cc: marek.olsak, Lepton Wu, christian.koenig, bas
Since size has been aligned to PAGE_SIZE already, just align it
to PAGE_SIZE so later the buffer can be used as a texture in mesa
after https://cgit.freedesktop.org/mesa/mesa/commit/?id=f7a4051b8
Otherwise, si_texture_create_object will fail at line
"buf->alignment < tex->surface.alignment"
Signed-off-by: Lepton Wu <ytht.net@gmail.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 8ef31d687ef3b..8dca0c920d3ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -928,7 +928,7 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
args->size = ALIGN(args->size, PAGE_SIZE);
domain = amdgpu_bo_get_preferred_domain(adev,
amdgpu_display_supported_domains(adev, flags));
- r = amdgpu_gem_object_create(adev, args->size, 0, domain, flags,
+ r = amdgpu_gem_object_create(adev, args->size, PAGE_SIZE, domain, flags,
ttm_bo_type_device, NULL, &gobj);
if (r)
return -ENOMEM;
--
2.37.3.998.g577e59143f-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Always align dumb buffer at PAGE_SIZE
2022-09-22 23:04 [PATCH] drm/amdgpu: Always align dumb buffer at PAGE_SIZE Lepton Wu
@ 2022-09-23 5:14 ` Christian König
2022-09-23 5:28 ` lepton
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2022-09-23 5:14 UTC (permalink / raw)
To: Lepton Wu, amd-gfx; +Cc: marek.olsak, bas
Am 23.09.22 um 01:04 schrieb Lepton Wu:
> Since size has been aligned to PAGE_SIZE already, just align it
> to PAGE_SIZE so later the buffer can be used as a texture in mesa
> after https://cgit.freedesktop.org/mesa/mesa/commit/?id=f7a4051b8
> Otherwise, si_texture_create_object will fail at line
> "buf->alignment < tex->surface.alignment"
I don't think that those Mesa checks are a good idea in the first place.
The alignment value is often specified as zero when it doesn't matter
because the minimum alignment can never be less than the page size.
Christian.
>
> Signed-off-by: Lepton Wu <ytht.net@gmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 8ef31d687ef3b..8dca0c920d3ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -928,7 +928,7 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
> args->size = ALIGN(args->size, PAGE_SIZE);
> domain = amdgpu_bo_get_preferred_domain(adev,
> amdgpu_display_supported_domains(adev, flags));
> - r = amdgpu_gem_object_create(adev, args->size, 0, domain, flags,
> + r = amdgpu_gem_object_create(adev, args->size, PAGE_SIZE, domain, flags,
> ttm_bo_type_device, NULL, &gobj);
> if (r)
> return -ENOMEM;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Always align dumb buffer at PAGE_SIZE
2022-09-23 5:14 ` Christian König
@ 2022-09-23 5:28 ` lepton
2022-09-23 5:30 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: lepton @ 2022-09-23 5:28 UTC (permalink / raw)
To: Christian König; +Cc: marek.olsak, amd-gfx, bas
On Thu, Sep 22, 2022 at 10:14 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 23.09.22 um 01:04 schrieb Lepton Wu:
> > Since size has been aligned to PAGE_SIZE already, just align it
> > to PAGE_SIZE so later the buffer can be used as a texture in mesa
> > after https://cgit.freedesktop.org/mesa/mesa/commit/?id=f7a4051b8
> > Otherwise, si_texture_create_object will fail at line
> > "buf->alignment < tex->surface.alignment"
>
> I don't think that those Mesa checks are a good idea in the first place.
>
> The alignment value is often specified as zero when it doesn't matter
> because the minimum alignment can never be less than the page size.
Are you suggesting to change those mesa checks? While that can be
done, I still think a kernel side "fix" is still
useful since it doesn't hurt while can fix issues for some versions of mesa.
>
> Christian.
>
> >
> > Signed-off-by: Lepton Wu <ytht.net@gmail.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > index 8ef31d687ef3b..8dca0c920d3ce 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > @@ -928,7 +928,7 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
> > args->size = ALIGN(args->size, PAGE_SIZE);
> > domain = amdgpu_bo_get_preferred_domain(adev,
> > amdgpu_display_supported_domains(adev, flags));
> > - r = amdgpu_gem_object_create(adev, args->size, 0, domain, flags,
> > + r = amdgpu_gem_object_create(adev, args->size, PAGE_SIZE, domain, flags,
> > ttm_bo_type_device, NULL, &gobj);
> > if (r)
> > return -ENOMEM;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Always align dumb buffer at PAGE_SIZE
2022-09-23 5:28 ` lepton
@ 2022-09-23 5:30 ` Christian König
2022-09-23 6:23 ` Marek Olšák
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2022-09-23 5:30 UTC (permalink / raw)
To: lepton; +Cc: marek.olsak, amd-gfx, bas
Am 23.09.22 um 07:28 schrieb lepton:
> On Thu, Sep 22, 2022 at 10:14 PM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 23.09.22 um 01:04 schrieb Lepton Wu:
>>> Since size has been aligned to PAGE_SIZE already, just align it
>>> to PAGE_SIZE so later the buffer can be used as a texture in mesa
>>> after https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fmesa%2Fmesa%2Fcommit%2F%3Fid%3Df7a4051b8&data=05%7C01%7Cchristian.koenig%40amd.com%7C645f6878a7bd487588b708da9d246c4c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637995077041120091%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=NMEAl8TByDLQFWW1d%2FaJfiGrXc4mpwL5dxNH0M0QH84%3D&reserved=0
>>> Otherwise, si_texture_create_object will fail at line
>>> "buf->alignment < tex->surface.alignment"
>> I don't think that those Mesa checks are a good idea in the first place.
>>
>> The alignment value is often specified as zero when it doesn't matter
>> because the minimum alignment can never be less than the page size.
> Are you suggesting to change those mesa checks?
Yes, the minimum alignment of allocations is always 4096 because that's
the page size of the GPU.
> While that can be
> done, I still think a kernel side "fix" is still
> useful since it doesn't hurt while can fix issues for some versions of mesa.
No, we have tons of places where we don't specify and alignment for
buffers because it never mattered. I certainly don't want to fix all of
those.
Regards,
Christian.
>> Christian.
>>
>>> Signed-off-by: Lepton Wu <ytht.net@gmail.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index 8ef31d687ef3b..8dca0c920d3ce 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -928,7 +928,7 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
>>> args->size = ALIGN(args->size, PAGE_SIZE);
>>> domain = amdgpu_bo_get_preferred_domain(adev,
>>> amdgpu_display_supported_domains(adev, flags));
>>> - r = amdgpu_gem_object_create(adev, args->size, 0, domain, flags,
>>> + r = amdgpu_gem_object_create(adev, args->size, PAGE_SIZE, domain, flags,
>>> ttm_bo_type_device, NULL, &gobj);
>>> if (r)
>>> return -ENOMEM;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Always align dumb buffer at PAGE_SIZE
2022-09-23 5:30 ` Christian König
@ 2022-09-23 6:23 ` Marek Olšák
2022-09-26 9:21 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Marek Olšák @ 2022-09-23 6:23 UTC (permalink / raw)
To: Christian König
Cc: Marek Olšák, lepton, amd-gfx mailing list,
Bas Nieuwenhuizen
[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]
The kernel could report the true alignment from the ioctl instead of 0.
Marek
On Fri, Sep 23, 2022 at 1:31 AM Christian König <christian.koenig@amd.com>
wrote:
> Am 23.09.22 um 07:28 schrieb lepton:
> > On Thu, Sep 22, 2022 at 10:14 PM Christian König
> > <christian.koenig@amd.com> wrote:
> >> Am 23.09.22 um 01:04 schrieb Lepton Wu:
> >>> Since size has been aligned to PAGE_SIZE already, just align it
> >>> to PAGE_SIZE so later the buffer can be used as a texture in mesa
> >>> after
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fmesa%2Fmesa%2Fcommit%2F%3Fid%3Df7a4051b8&data=05%7C01%7Cchristian.koenig%40amd.com%7C645f6878a7bd487588b708da9d246c4c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637995077041120091%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=NMEAl8TByDLQFWW1d%2FaJfiGrXc4mpwL5dxNH0M0QH84%3D&reserved=0
> >>> Otherwise, si_texture_create_object will fail at line
> >>> "buf->alignment < tex->surface.alignment"
> >> I don't think that those Mesa checks are a good idea in the first place.
> >>
> >> The alignment value is often specified as zero when it doesn't matter
> >> because the minimum alignment can never be less than the page size.
> > Are you suggesting to change those mesa checks?
>
> Yes, the minimum alignment of allocations is always 4096 because that's
> the page size of the GPU.
>
> > While that can be
> > done, I still think a kernel side "fix" is still
> > useful since it doesn't hurt while can fix issues for some versions of
> mesa.
>
> No, we have tons of places where we don't specify and alignment for
> buffers because it never mattered. I certainly don't want to fix all of
> those.
>
> Regards,
> Christian.
>
> >> Christian.
> >>
> >>> Signed-off-by: Lepton Wu <ytht.net@gmail.com>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> index 8ef31d687ef3b..8dca0c920d3ce 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> @@ -928,7 +928,7 @@ int amdgpu_mode_dumb_create(struct drm_file
> *file_priv,
> >>> args->size = ALIGN(args->size, PAGE_SIZE);
> >>> domain = amdgpu_bo_get_preferred_domain(adev,
> >>> amdgpu_display_supported_domains(adev,
> flags));
> >>> - r = amdgpu_gem_object_create(adev, args->size, 0, domain, flags,
> >>> + r = amdgpu_gem_object_create(adev, args->size, PAGE_SIZE,
> domain, flags,
> >>> ttm_bo_type_device, NULL, &gobj);
> >>> if (r)
> >>> return -ENOMEM;
>
>
[-- Attachment #2: Type: text/html, Size: 4358 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Always align dumb buffer at PAGE_SIZE
2022-09-23 6:23 ` Marek Olšák
@ 2022-09-26 9:21 ` Christian König
0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2022-09-26 9:21 UTC (permalink / raw)
To: Marek Olšák
Cc: Marek Olšák, lepton, amd-gfx mailing list,
Bas Nieuwenhuizen
[-- Attachment #1: Type: text/plain, Size: 3921 bytes --]
I was thinking about that as well, yes.
Might be a good idea to just change the alignment check in
amdgpu_bo_create():
/* Memory should be aligned at least to a page size. */
page_align = ALIGN(bp->byte_align, PAGE_SIZE) >>
PAGE_SHIFT;
Something like ALIGN(bp->byte_align ?: 1, PAGE_SIZE) should already do it.
Christian.
Am 23.09.22 um 08:23 schrieb Marek Olšák:
> The kernel could report the true alignment from the ioctl instead of 0.
>
> Marek
>
> On Fri, Sep 23, 2022 at 1:31 AM Christian König
> <christian.koenig@amd.com> wrote:
>
> Am 23.09.22 um 07:28 schrieb lepton:
> > On Thu, Sep 22, 2022 at 10:14 PM Christian König
> > <christian.koenig@amd.com> wrote:
> >> Am 23.09.22 um 01:04 schrieb Lepton Wu:
> >>> Since size has been aligned to PAGE_SIZE already, just align it
> >>> to PAGE_SIZE so later the buffer can be used as a texture in mesa
> >>> after
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fmesa%2Fmesa%2Fcommit%2F%3Fid%3Df7a4051b8&data=05%7C01%7Cchristian.koenig%40amd.com%7C645f6878a7bd487588b708da9d246c4c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637995077041120091%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=NMEAl8TByDLQFWW1d%2FaJfiGrXc4mpwL5dxNH0M0QH84%3D&reserved=0
> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fmesa%2Fmesa%2Fcommit%2F%3Fid%3Df7a4051b8&data=05%7C01%7Cchristian.koenig%40amd.com%7C69c0881fd97d4de3380408da9d2c336b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637995111117539790%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=KMiCYD7blJCjO9zcfsqmy9%2FfYbtAmMHbrwpzHP7F5O0%3D&reserved=0>
> >>> Otherwise, si_texture_create_object will fail at line
> >>> "buf->alignment < tex->surface.alignment"
> >> I don't think that those Mesa checks are a good idea in the
> first place.
> >>
> >> The alignment value is often specified as zero when it doesn't
> matter
> >> because the minimum alignment can never be less than the page size.
> > Are you suggesting to change those mesa checks?
>
> Yes, the minimum alignment of allocations is always 4096 because
> that's
> the page size of the GPU.
>
> > While that can be
> > done, I still think a kernel side "fix" is still
> > useful since it doesn't hurt while can fix issues for some
> versions of mesa.
>
> No, we have tons of places where we don't specify and alignment for
> buffers because it never mattered. I certainly don't want to fix
> all of
> those.
>
> Regards,
> Christian.
>
> >> Christian.
> >>
> >>> Signed-off-by: Lepton Wu <ytht.net@gmail.com>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> index 8ef31d687ef3b..8dca0c920d3ce 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> @@ -928,7 +928,7 @@ int amdgpu_mode_dumb_create(struct
> drm_file *file_priv,
> >>> args->size = ALIGN(args->size, PAGE_SIZE);
> >>> domain = amdgpu_bo_get_preferred_domain(adev,
> >>> amdgpu_display_supported_domains(adev, flags));
> >>> - r = amdgpu_gem_object_create(adev, args->size, 0,
> domain, flags,
> >>> + r = amdgpu_gem_object_create(adev, args->size,
> PAGE_SIZE, domain, flags,
> >>> ttm_bo_type_device, NULL, &gobj);
> >>> if (r)
> >>> return -ENOMEM;
>
[-- Attachment #2: Type: text/html, Size: 6825 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-26 9:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-22 23:04 [PATCH] drm/amdgpu: Always align dumb buffer at PAGE_SIZE Lepton Wu
2022-09-23 5:14 ` Christian König
2022-09-23 5:28 ` lepton
2022-09-23 5:30 ` Christian König
2022-09-23 6:23 ` Marek Olšák
2022-09-26 9:21 ` Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.