On 02.09.25 10:08, Timur Kristóf wrote:
> On Tue, 2025-09-02 at 08:43 +0200, Christian König wrote:
>> On 01.09.25 12:00, Timur Kristóf wrote:
>>> To avoid confusion with dwords.
>>>
>>> Signed-off-by: Timur Kristóf <[email protected]>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 122a88294883..9ffadc029ef8 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -220,7 +220,7 @@ void amdgpu_bo_placement_from_domain(struct
>>> amdgpu_bo *abo, u32 domain)
>>> * amdgpu_bo_create_reserved - create reserved BO for kernel use
>>> *
>>> * @adev: amdgpu device object
>>> - * @size: size for the new BO
>>> + * @size: size for the new BO in bytes
>>
>> That is actually incorrect. The size is in arbitrary units.
>>
>> For OA and GWS it is the number of HW engines they represent, for GDS
>> it is in bytes and for VRAM and GTT it is in bytes but rounded up to
>> a page size.
>>
>
> Can you point me to the part of the code where this is actually
> determined?
See amdgpu_bo_create():
/* Note that GDS/GWS/OA allocates 1 page per byte/resource. */
if (bp->domain & (AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
/* GWS and OA don't need any alignment. */
page_align = bp->byte_align;
size <<= PAGE_SHIFT;
} else if (bp->domain & AMDGPU_GEM_DOMAIN_GDS) {
/* Both size and alignment must be a multiple of 4. */
page_align = ALIGN(bp->byte_align, 4);
size = ALIGN(size, 4) << PAGE_SHIFT;
} else {
/* Memory should be aligned at least to a page size. */
page_align = ALIGN(bp->byte_align, PAGE_SIZE) >> PAGE_SHIFT;
size = ALIGN(size, PAGE_SIZE);
}
The GDS/GWS/OA handling here is actually a hack I'm trying to remove for years.
It's just that GEM/TTM assumes that all BOs are PAGE_SIZE based objects and so
we have to do this tricky workaround.
> If it's that complicated, then I think the function could definitely
> benefit from more documentation. All I could find was that both
> ttm_resource::size and drm_gem_object::size are documented to be in
> bytes.
IIRC we have actually documented that quite extensively. I'm just not sure of
hand where, @Alex?
Maybe the UAPI? Probably best to add a reference to that to the functions
creating a kernel BO.
Regards,
Christian.
>
>
>>
>>> * @align: alignment for the new BO
>>> * @domain: where to place it
>>> * @bo_ptr: used to initialize BOs in structures
>>> @@ -317,7 +317,7 @@ int amdgpu_bo_create_reserved(struct
>>> amdgpu_device *adev,
>>> * amdgpu_bo_create_kernel - create BO for kernel use
>>> *
>>> * @adev: amdgpu device object
>>> - * @size: size for the new BO
>>> + * @size: size for the new BO in bytes
>>> * @align: alignment for the new BO
>>> * @domain: where to place it
>>> * @bo_ptr: used to initialize BOs in structures
