|
[Public]
It happens in amdgpu_gmc_agp_addr() which is called from amdgpu_ttm_alloc_gart().
Alex
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: Friday, November 10, 2023 9:14 AM To: Zhang, Yifan <Yifan1.Zhang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx> Subject: Re: [PATCH] drm/amdgpu: exclude domain start when calucales offset for AGP aperture BOs Am 10.11.23 um 13:52 schrieb Yifan Zhang:
> For BOs in AGP aperture, tbo.resource->start includes AGP aperture start. Well big NAK to that. tbo.resource->start should never ever include the AGP aperture start in the first place. How did that happen? Regards, Christian. > Don't add it again in amdgpu_bo_gpu_offset. This issue was mitigated due to > GART aperture start was 0 until this patch ("a013c94d5aca drm/amdgpu/gmc11: > set gart placement GC11") changes GART start to a non-zero value. > > Reported-by: Jesse Zhang <Jesse.Zhang@xxxxxxx> > Signed-off-by: Yifan Zhang <yifan1.zhang@xxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 7 +++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 ++++++++-- > 3 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > index 5f71414190e9..00e940eb69ab 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c > @@ -169,6 +169,13 @@ int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr, > return 0; > } > > +bool bo_in_agp_aperture(struct amdgpu_bo *bo) > +{ > + struct ttm_buffer_object *tbo = &(bo->tbo); > + struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); > + > + return (tbo->resource->start << PAGE_SHIFT) > adev->gmc.agp_start; > +} > /** > * amdgpu_gmc_agp_addr - return the address in the AGP address space > * > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h > index e699d1ca8deb..448dc08e83de 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h > @@ -393,6 +393,7 @@ int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr, > uint64_t flags); > uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo); > uint64_t amdgpu_gmc_agp_addr(struct ttm_buffer_object *bo); > +bool bo_in_agp_aperture(struct amdgpu_bo *bo); > void amdgpu_gmc_sysvm_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc); > void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc, > u64 base); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index cef920a93924..91a011d63ab4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -39,6 +39,7 @@ > #include "amdgpu.h" > #include "amdgpu_trace.h" > #include "amdgpu_amdkfd.h" > +#include "amdgpu_gmc.h" > > /** > * DOC: amdgpu_object > @@ -1529,8 +1530,13 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo) > struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); > uint64_t offset; > > - offset = (bo->tbo.resource->start << PAGE_SHIFT) + > - amdgpu_ttm_domain_start(adev, bo->tbo.resource->mem_type); > + /* tbo.resource->start includes agp_start for AGP BOs */ > + if (bo_in_agp_aperture(bo)) { > + offset = (bo->tbo.resource->start << PAGE_SHIFT); > + } else { > + offset = (bo->tbo.resource->start << PAGE_SHIFT) + > + amdgpu_ttm_domain_start(adev, bo->tbo.resource->mem_type); > + } > > return amdgpu_gmc_sign_extend(offset); > } |
