<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);">In that case, how do we know we can skip the gart setup in amdgpu_ttm_alloc_gart()?</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> Koenig, Christian <Christian.Koenig@amd.com><br> <b>Sent:</b> Friday, November 10, 2023 9:20 AM<br> <b>To:</b> Deucher, Alexander <Alexander.Deucher@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> <div>No, that's broken as well.<br> <br> The problem is in amdgpu_ttm_alloc_gart<span style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; color:rgb(0,0,0)">():<br> <br> if (addr != AMDGPU_BO_INVALID_OFFSET) {<br> bo->resource->start = addr >> PAGE_SHIFT;<br> return 0;<br> }<br> <br> bo->resource->start is relative to the GART address, so we can't assign the AGP address here in the first place.<br> <br> What we need to do is to drop this and call </span><span style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; color:rgb(0,0,0)">amdgpu_gmc_agp_addr() from</span><span style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; color:rgb(0,0,0)"> amdgpu_bo_gpu_offset_no_check().<br> <br> Regards,<br> Christian.<br> </span><br> <div class="x_moz-cite-prefix">Am 10.11.23 um 15:17 schrieb Deucher, Alexander:<br> </div> <blockquote type="cite"><style type="text/css" style="display:none">  </style> <p align="Left" style="font-family:Arial; font-size:10pt; color:#008000; margin:15pt; font-style:normal; font-weight:normal; text-decoration:none"> [Public]<br> </p> <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)"> <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="x_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="x_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> <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> Deucher, Alexander <a class="x_moz-txt-link-rfc2396E" href="mailto:Alexander.Deucher@amd.com"><Alexander.Deucher@amd.com></a><br> <b>Sent:</b> Friday, November 10, 2023 9:16 AM<br> <b>To:</b> Koenig, Christian <a class="x_moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"> <Christian.Koenig@amd.com></a>; Zhang, Yifan <a class="x_moz-txt-link-rfc2396E" href="mailto:Yifan1.Zhang@amd.com"> <Yifan1.Zhang@amd.com></a>; <a class="x_moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> <a class="x_moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"> <amd-gfx@lists.freedesktop.org></a><br> <b>Cc:</b> Zhang, Jesse(Jie) <a class="x_moz-txt-link-rfc2396E" href="mailto:Jesse.Zhang@amd.com"> <Jesse.Zhang@amd.com></a><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">  </style> <div dir="ltr"> <div class="x_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_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_x_elementToProof" style="font-family:Aptos,Aptos_EmbeddedFont,Aptos_MSFontService,Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> Alex</div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Koenig, Christian <a class="x_moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a><br> <b>Sent:</b> Friday, November 10, 2023 9:14 AM<br> <b>To:</b> Zhang, Yifan <a class="x_moz-txt-link-rfc2396E" href="mailto:Yifan1.Zhang@amd.com"> <Yifan1.Zhang@amd.com></a>; <a class="x_moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org"> amd-gfx@lists.freedesktop.org</a> <a class="x_moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"> <amd-gfx@lists.freedesktop.org></a><br> <b>Cc:</b> Deucher, Alexander <a class="x_moz-txt-link-rfc2396E" href="mailto:Alexander.Deucher@amd.com"> <Alexander.Deucher@amd.com></a>; Zhang, Jesse(Jie) <a class="x_moz-txt-link-rfc2396E" href="mailto:Jesse.Zhang@amd.com"> <Jesse.Zhang@amd.com></a><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_x_BodyFragment"><font size="2"><span style="font-size:11pt"> <div class="x_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 <a class="x_moz-txt-link-rfc2396E" href="mailto:Jesse.Zhang@amd.com"> <Jesse.Zhang@amd.com></a><br> > Signed-off-by: Yifan Zhang <a class="x_moz-txt-link-rfc2396E" href="mailto:yifan1.zhang@amd.com"> <yifan1.zhang@amd.com></a><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> </blockquote> <br> </div> </div> </body> </html>