On Tue, 2025-09-02 at 13:10 +0200, Christian König wrote: > 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.
Thanks Christian. If this is already documented, then I agree it's best to refer to the pre-existing docs. I'll do that (once we figure out where it is). > > 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
