[AMD Official Use Only - General]
> -----Original Message-----
> From: Kuehling, Felix <felix.kuehl...@amd.com>
> Sent: Tuesday, April 4, 2023 9:52 PM
> To: Xiao, Shane <shane.x...@amd.com>; amd-gfx@lists.freedesktop.org;
> Koenig, Christian <christian.koe...@amd.com>
> Cc: Liu, Aaron <aaron....@amd.com>; Guo, Shikai <shikai....@amd.com>
> Subject: Re: [PATCH 2/3] amd/amdgpu: Inherit coherence flags base on original
> BO flags
>
> Am 2023-04-04 um 05:56 schrieb Shane Xiao:
> > For SG BO to DMA-map userptrs on other GPUs, the SG BO need inherit
> > MTYPEs in PTEs from original BO.
>
> Good catch. See two comments inline.
>
>
> >
> > If we set the flags, the device can be coherent with the CPUs and other
> > GPUs.
> >
> > Signed-off-by: Shane Xiao <shane.x...@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index 33cda358cb9e..bcb0a7b32703 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -253,14 +253,22 @@ create_dmamap_sg_bo(struct amdgpu_device
> *adev,
> > {
> > struct drm_gem_object *gem_obj;
> > int ret, align;
> > + uint64_t flags = 0;
> >
> > ret = amdgpu_bo_reserve(mem->bo, false);
> > if (ret)
> > return ret;
> >
> > align = 1;
> > + if(mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR)
> > + {
> > + flags |= mem->bo->flags
> &(AMDGPU_GEM_CREATE_CPU_GTT_USWC |
>
> I think userptrs never use USWC because the pages are not allocated by the
> driver. You can drop this flag.
OK. I will do it.
>
>
> > + AMDGPU_GEM_CREATE_COHERENT |
> AMDGPU_GEM_CREATE_UNCACHED);
> > + align = PAGE_SIZE;
>
> Isn't a page alignment implicit anyway? I don't see why we need to use a
> different alignment for userptrs. If PAGE_SIZE is needed for this case,
> we can use the same for all cases We don't even need a local variable
> for this.
Yes, a page alignment is implicit, and the local variable will be removed.
Best Regards,
Shane
>
> Regards,
> Felix
>
>
> > + }
> > +
> > ret = amdgpu_gem_object_create(adev, mem->bo->tbo.base.size, align,
> > - AMDGPU_GEM_DOMAIN_CPU,
> AMDGPU_GEM_CREATE_PREEMPTIBLE,
> > + AMDGPU_GEM_DOMAIN_CPU,
> AMDGPU_GEM_CREATE_PREEMPTIBLE | flags,
> > ttm_bo_type_sg, mem->bo->tbo.base.resv, &gem_obj);
> >
> > amdgpu_bo_unreserve(mem->bo);
