<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Maybe I'm mixing up issues. The navi10/14 issue that was fixed on navi12 was fixed in amdgpu_ids.c in</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <span>commit a2bd77bbde791202267c25478bbcbe71bb4ecdd5<br> </span> <div>Author: Christian König <christian.koenig@amd.com><br> </div> <div>Date: Thu Feb 7 12:10:29 2019 +0100<br> </div> <div><br> </div> <div> drm/amdgpu: disable concurrent flushes for Navi10 v2<br> </div> <div> <br> </div> <div> Navi10 have a bug in the SDMA which can theoretically cause memory<br> </div> <div> corruption with concurrent VMID flushes<br> </div> <div> <br> </div> <div> v2: explicitely check Navi10<br> </div> <div> <br> </div> <div> Signed-off-by: Christian König <christian.koenig@amd.com><br> </div> <div> Reviewed-by: Alex Deucher <alexander.deucher@amd.com><br> </div> <div> Signed-off-by: Alex Deucher <alexander.deucher@amd.com><br> </div> <span></span></div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> This is a different issue and may apply to all navi parts. so maybe the patch is fine as is.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Alex<br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </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> Thursday, September 26, 2019 2:02 PM<br> <b>To:</b> Yuan, Xiaojie <Xiaojie.Yuan@amd.com><br> <b>Cc:</b> Alex Deucher <alexdeucher@gmail.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br> <b>Subject:</b> Re: [PATCH] drm/amdgpu/gmc10: apply the 'invalidation from sdma' workaround for navi</font> <div> </div> </div> <style type="text/css" style="display:none">  </style> <div dir="ltr"> <div dir="auto">Well then you didn't figured out the root cause correctly. <div dir="auto"><br> </div> <div dir="auto">This is to avoid data corruption with the SDMA on Navi 10/14 and should definitely not applied to Navi 12.</div> <div dir="auto"><br> </div> <div dir="auto">The hardware team went through quite some work to avoid this.</div> <div dir="auto"><br> </div> <div dir="auto">Christian.</div> </div> <div class="x_gmail_extra"><br> <div class="x_gmail_quote">Am 26.09.2019 19:38 schrieb "Yuan, Xiaojie" <Xiaojie.Yuan@amd.com>:<br type="attribution"> </div> </div> <div> <div style="font-family:Calibri,Helvetica,sans-serif; font-size:11pt; color:rgb(0,0,0)"> Hi Alex / Christian,</div> <div style="font-family:Calibri,Helvetica,sans-serif; font-size:11pt; color:rgb(0,0,0)"> <br> </div> <div style="font-family:Calibri,Helvetica,sans-serif; font-size:11pt; color:rgb(0,0,0)"> When gfxoff is enabled for Navi12, I observed sdma0 hang while launching desktop. When this workaround is applied, the issue fades away.</div> <div style="font-family:Calibri,Helvetica,sans-serif; font-size:11pt; color:rgb(0,0,0)"> That's why I included this workaround for Navi12 as well.<br> </div> <div style="font-family:Calibri,Helvetica,sans-serif; font-size:11pt; color:rgb(0,0,0)"> <br> </div> <div style="font-family:Calibri,Helvetica,sans-serif; font-size:11pt; color:rgb(0,0,0)"> BR,</div> <div style="font-family:Calibri,Helvetica,sans-serif; font-size:11pt; color:rgb(0,0,0)"> Xiaojie<br> </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> Thursday, September 26, 2019 10:20 PM<br> <b>To:</b> Alex Deucher <alexdeucher@gmail.com><br> <b>Cc:</b> Deucher, Alexander <Alexander.Deucher@amd.com>; Yuan, Xiaojie <Xiaojie.Yuan@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br> <b>Subject:</b> Re: [PATCH] drm/amdgpu/gmc10: apply the 'invalidation from sdma' workaround for navi</font> <div> </div> </div> <div> <div dir="auto"> <div><br> <div class="x_x_gmail_extra"><br> <div class="x_x_gmail_quote">Am 26.09.2019 15:51 schrieb Alex Deucher <alexdeucher@gmail.com>:<br type="attribution"> <blockquote class="x_x_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex"> <div><font size="2"><span style="font-size:11pt"> <div>On Thu, Sep 26, 2019 at 9:47 AM Koenig, Christian<br> <Christian.Koenig@amd.com> wrote:<br> ><br> > Am 26.09.19 um 15:40 schrieb Alex Deucher:<br> > > On Thu, Sep 26, 2019 at 8:29 AM Christian König<br> > > <ckoenig.leichtzumerken@gmail.com> wrote:<br> > >> Stop, wait a second guys!<br> > >><br> > >> This will disable the workaround for Navi10 and that is certainly not correct:<br> > >><br> > >> !(adev->asic_type >= CHIP_NAVI10 && adev->asic_type <= CHIP_NAVI12)<br> > >><br> > > Actually, I think it's correct. When I merged the baco patch, I<br> > > accidentally dropped the navi checks. E.g.,<br> > > @@ -245,8 +245,9 @@ static void gmc_v10_0_flush_gpu_tlb(struct<br> > > amdgpu_device *adev,<br> > > mutex_lock(&adev->mman.gtt_window_lock);<br> > ><br> > > gmc_v10_0_flush_vm_hub(adev, vmid, AMDGPU_MMHUB, 0);<br> > > - if (!adev->mman.buffer_funcs_enabled || !adev->ib_pool_ready ||<br> > > - adev->asic_type != CHIP_NAVI10) {<br> > > + if (!adev->mman.buffer_funcs_enabled ||<br> > > + !adev->ib_pool_ready ||<br> > > + adev->in_gpu_reset) {<br> > > gmc_v10_0_flush_vm_hub(adev, vmid, AMDGPU_GFXHUB, 0);<br> > > mutex_unlock(&adev->mman.gtt_window_lock);<br> > > return;<br> > > I think it should have been<br> > > adev->asic_type != CHIP_NAVI10 && adev->asic_type != CHIP_NAVI14 &&<br> > > adev->asic_type != CHIP_NAVI12<br> ><br> > My last status is that Navi12 is not supposed to need that workaround,<br> > that's why we checked Navi10 and later Navi14 separately.<br> ><br> > It's possible that I miss-read the !(adev->asic_type >= CHIP_NAVI10 &&<br> > adev->asic_type <= CHIP_NAVI12) check here, but either way that looks to<br> > complicated to me.<br> ><br> > We should rather mention every affected asic type separately here.<br> <br> Good point. navi12 should be dropped from the check. How about the following?<br> </div> </span></font></div> </blockquote> </div> </div> </div> <div dir="auto"><br> </div> <div dir="auto">I would rather test explicitly for Navi 10 and 14, cause we can't be sure if there won't be another variant in the future.</div> <div dir="auto"><br> </div> <div dir="auto">Christian.</div> <div dir="auto"><br> </div> <div dir="auto"> <div class="x_x_gmail_extra"> <div class="x_x_gmail_quote"> <blockquote class="x_x_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex"> <div><font size="2"><span style="font-size:11pt"> <div><br> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c<br> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c<br> index 241a4e57cf4a..280bbd7ca8a0 100644<br> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c<br> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c<br> @@ -292,7 +292,8 @@ static void gmc_v10_0_flush_gpu_tlb(struct<br> amdgpu_device *adev, uint32_t vmid,<br> <br> if (!adev->mman.buffer_funcs_enabled ||<br> !adev->ib_pool_ready ||<br> - adev->in_gpu_reset) {<br> + adev->in_gpu_reset ||<br> + (adev->asic_type == CHIP_NAVI12)) {<br> gmc_v10_0_flush_vm_hub(adev, vmid, AMDGPU_GFXHUB_0, 0);<br> mutex_unlock(&adev->mman.gtt_window_lock);<br> return;<br> <br> Alex<br> <br> ><br> > Regards,<br> > Christian.<br> ><br> > ><br> > > Alex<br> > ><br> > >> Christian.<br> > >><br> > >><br> > >> Am 26.09.19 um 14:26 schrieb Deucher, Alexander:<br> > >><br> > >> Please add a patch description. With that fixed:<br> > >> Reviewed-by: Alex Deucher <alexander.deucher@amd.com><br> > >> ________________________________<br> > >> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Yuan, Xiaojie <Xiaojie.Yuan@amd.com><br> > >> Sent: Thursday, September 26, 2019 4:09 AM<br> > >> To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br> > >> Cc: alexdeucher@gmail.com <alexdeucher@gmail.com><br> > >> Subject: Re: [PATCH] drm/amdgpu/gmc10: apply the 'invalidation from sdma' workaround for navi<br> > >><br> > >> Hi Alex,<br> > >><br> > >> This patch is to add the asic_type check which is missing after drm-next branch rebase.<br> > >><br> > >> BR,<br> > >> Xiaojie<br> > >> ________________________________<br> > >> From: Yuan, Xiaojie <Xiaojie.Yuan@amd.com><br> > >> Sent: Thursday, September 26, 2019 4:08 PM<br> > >> To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br> > >> Cc: alexdeucher@gmail.com <alexdeucher@gmail.com>; Yuan, Xiaojie <Xiaojie.Yuan@amd.com><br> > >> Subject: [PATCH] drm/amdgpu/gmc10: apply the 'invalidation from sdma' workaround for navi<br> > >><br> > >> Fixes: 767acabdac81 ("drm/amd/powerplay: add baco smu reset function for smu11")<br> > >> Signed-off-by: Xiaojie Yuan <xiaojie.yuan@amd.com><br> > >> ---<br> > >> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 1 +<br> > >> 1 file changed, 1 insertion(+)<br> > >><br> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c<br> > >> index cb3f61873baa..dc2e68e019eb 100644<br> > >> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c<br> > >> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c<br> > >> @@ -292,6 +292,7 @@ static void gmc_v10_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,<br> > >><br> > >> if (!adev->mman.buffer_funcs_enabled ||<br> > >> !adev->ib_pool_ready ||<br> > >> + !(adev->asic_type >= CHIP_NAVI10 && adev->asic_type <= CHIP_NAVI12) ||<br> > >> adev->in_gpu_reset) {<br> > >> gmc_v10_0_flush_vm_hub(adev, vmid, AMDGPU_GFXHUB_0, 0);<br> > >> mutex_unlock(&adev->mman.gtt_window_lock);<br> > >> --<br> > >> 2.20.1<br> > >><br> > >><br> > >> _______________________________________________<br> > >> amd-gfx mailing list<br> > >> amd-gfx@lists.freedesktop.org<br> > >> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br> > >><br> > >><br> ><br> </div> </span></font></div> </blockquote> </div> <br> </div> </div> </div> </div> </div> </div> </body> </html>