<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Arial;font-size:10pt;color:#008000;margin:15pt;font-style:normal;font-weight:normal;text-decoration:none;" align="Left">
[Public]<br>
</p>
<br>
<div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; color: rgb(0, 0, 0);">I think the proper fix is probably to just drop the addition of agp_start in amdgpu_gmc_agp_addr().</span></div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; color: rgb(0, 0, 0);"><br>
</span></div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; color: rgb(0, 0, 0);">Alex</span></div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Deucher, Alexander <Alexander.Deucher@amd.com><br>
<b>Sent:</b> Friday, November 10, 2023 9:16 AM<br>
<b>To:</b> Koenig, Christian <Christian.Koenig@amd.com>; Zhang, Yifan <Yifan1.Zhang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Zhang, Jesse(Jie) <Jesse.Zhang@amd.com><br>
<b>Subject:</b> Re: [PATCH] drm/amdgpu: exclude domain start when calucales offset for AGP aperture BOs</font>
<div> </div>
</div>
<style type="text/css" style="display:none">
<!--
p
        {margin-top:0;
        margin-bottom:0}
-->
</style>
<div dir="ltr">
<div class="x_elementToProof" style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
It happens in amdgpu_gmc_agp_addr() which is called from amdgpu_ttm_alloc_gart().</div>
<div class="x_elementToProof" style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<div class="x_elementToProof" style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
Alex</div>
<div id="x_appendonsend"></div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Koenig, Christian <Christian.Koenig@amd.com><br>
<b>Sent:</b> Friday, November 10, 2023 9:14 AM<br>
<b>To:</b> Zhang, Yifan <Yifan1.Zhang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Deucher, Alexander <Alexander.Deucher@amd.com>; Zhang, Jesse(Jie) <Jesse.Zhang@amd.com><br>
<b>Subject:</b> Re: [PATCH] drm/amdgpu: exclude domain start when calucales offset for AGP aperture BOs</font>
<div> </div>
</div>
<div class="x_BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="x_PlainText">Am 10.11.23 um 13:52 schrieb Yifan Zhang:<br>
> For BOs in AGP aperture, tbo.resource->start includes AGP aperture start.<br>
<br>
<br>
Well big NAK to that. tbo.resource->start should never ever include the <br>
AGP aperture start in the first place.<br>
<br>
How did that happen?<br>
<br>
Regards,<br>
Christian.<br>
<br>
> Don't add it again in amdgpu_bo_gpu_offset. This issue was mitigated due to<br>
> GART aperture start was 0 until this patch ("a013c94d5aca drm/amdgpu/gmc11:<br>
> set gart placement GC11") changes GART start to a non-zero value.<br>
><br>
> Reported-by: Jesse Zhang <Jesse.Zhang@amd.com><br>
> Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com><br>
> ---<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    |  7 +++++++<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h    |  1 +<br>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 ++++++++--<br>
>   3 files changed, 16 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c<br>
> index 5f71414190e9..00e940eb69ab 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c<br>
> @@ -169,6 +169,13 @@ int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr,<br>
>        return 0;<br>
>   }<br>
>   <br>
> +bool bo_in_agp_aperture(struct amdgpu_bo *bo)<br>
> +{<br>
> +     struct ttm_buffer_object *tbo = &(bo->tbo);<br>
> +     struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);<br>
> +<br>
> +     return (tbo->resource->start << PAGE_SHIFT) > adev->gmc.agp_start;<br>
> +}<br>
>   /**<br>
>    * amdgpu_gmc_agp_addr - return the address in the AGP address space<br>
>    *<br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h<br>
> index e699d1ca8deb..448dc08e83de 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h<br>
> @@ -393,6 +393,7 @@ int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev, void *cpu_pt_addr,<br>
>                                uint64_t flags);<br>
>   uint64_t amdgpu_gmc_pd_addr(struct amdgpu_bo *bo);<br>
>   uint64_t amdgpu_gmc_agp_addr(struct ttm_buffer_object *bo);<br>
> +bool bo_in_agp_aperture(struct amdgpu_bo *bo);<br>
>   void amdgpu_gmc_sysvm_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc);<br>
>   void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,<br>
>                              u64 base);<br>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c<br>
> index cef920a93924..91a011d63ab4 100644<br>
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c<br>
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c<br>
> @@ -39,6 +39,7 @@<br>
>   #include "amdgpu.h"<br>
>   #include "amdgpu_trace.h"<br>
>   #include "amdgpu_amdkfd.h"<br>
> +#include "amdgpu_gmc.h"<br>
>   <br>
>   /**<br>
>    * DOC: amdgpu_object<br>
> @@ -1529,8 +1530,13 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)<br>
>        struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);<br>
>        uint64_t offset;<br>
>   <br>
> -     offset = (bo->tbo.resource->start << PAGE_SHIFT) +<br>
> -              amdgpu_ttm_domain_start(adev, bo->tbo.resource->mem_type);<br>
> +     /* tbo.resource->start includes agp_start for AGP BOs */<br>
> +     if (bo_in_agp_aperture(bo)) {<br>
> +             offset = (bo->tbo.resource->start << PAGE_SHIFT);<br>
> +     } else {<br>
> +             offset = (bo->tbo.resource->start << PAGE_SHIFT) +<br>
> +                      amdgpu_ttm_domain_start(adev, bo->tbo.resource->mem_type);<br>
> +     }<br>
>   <br>
>        return amdgpu_gmc_sign_extend(offset);<br>
>   }<br>
<br>
</div>
</span></font></div>
</div>
</div>
</body>
</html>