<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <meta name="Generator" content="Microsoft Word 14 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple"> <div class="WordSection1"> It’s inside the gmc_v9_0_gart_flush_gpu_tlb function where it calls the nbio_v6_0_hdp_flush . I already sent out the commit for review .you can check for details . <o:p></o:p> <o:p> </o:p> Regards<o:p></o:p> Shaoyun.liu<o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in"> From: Liu, Monk Sent: Tuesday, July 04, 2017 11:26 PM To: Liu, Shaoyun; Christian König; Michel Dänzer Cc: amd-gfx@lists.freedesktop.org Subject: RE: [PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic<o:p></o:p> </div> </div> <o:p> </o:p> What do you mean HDP flush code ?<o:p></o:p> <o:p> </o:p> Can you elaborate it ? the only HDP flush I remember via CPU is the one in GMC_V9’s gpu_tlb_flush, I saw it is still using regular register access, but I agree change it to CPU direct access <o:p></o:p> <o:p> </o:p> BR Monk <o:p></o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Liu, Shaoyun Sent: Tuesday, July 04, 2017 2:14 PM To: Liu, Monk <<a href="mailto:Monk.Liu@amd.com">Monk.Liu@amd.com</a>>; Christian König <<a href="mailto:deathsimple@vodafone.de">deathsimple@vodafone.de</a>>; Michel Dänzer <<a href="mailto:michel@daenzer.net">michel@daenzer.net</a>> Cc: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Subject: RE: [PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic<o:p></o:p> </div> </div> <o:p> </o:p> Hi , Monk <o:p></o:p> The TLB flush code is not used from IRQ , but it’s been called while holding spin_lock . This is the reason why I want to change the routine be atomic. After check the register spec again , I found that the TLB invalidation and HDP coherency cntl register are safe to use in the VFs and in current code we already try to avoid use KIQ in TLB flush function by directly program the TLB invalidate registers but just missed one place on HDP flush . So I think it ‘s better to fix the HDP flush code and leave current KIQ code un-touched . <o:p></o:p> <o:p> </o:p> Regards<o:p></o:p> Shaoyun.liu<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> <div> <div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in"> From: Liu, Monk Sent: Monday, July 03, 2017 5:45 AM To: Liu, Shaoyun; Christian König; Michel Dänzer Cc: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Subject: Re: [PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic<o:p></o:p> </div> </div> <o:p> </o:p> <div> <div id="x_divtagdefaultwrapper"> Hi Shaoyun <o:p></o:p> <o:p> </o:p> looks you want to make KIQ reg access be atomic & UN-interruptible,<o:p></o:p> I think most user of KIQ reg access is not in atomic context, so your patch only benefit for the place using KIQ from IRQ.<o:p></o:p> why not implement another KIQ reg access function ? e.g. :<o:p></o:p> <div> <o:p> </o:p> <div> amdgpu_virt_kiq_rreg_atomic(...);<o:p></o:p> amdgpu_virt_kiq_wreg_atomic(...);<o:p></o:p> </div> <o:p> </o:p> </div> <o:p> </o:p> that way you can satisfy your requirement and not introduce unknown issue for other places that calling original virt_kiq_r/weg() function.<o:p></o:p> <o:p> </o:p> because busy polling have chance to hang cpu in SR-IOV (imagine 16 VF, and many VF doing FLR one by one, your busy polling may let guest kernel cpu stuck in ATOMIC_CONTEXT for more than 5 seconds )<o:p></o:p> <o:p> </o:p> BR Monk<o:p></o:p> </div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="x_divRplyFwdMsg"> From: amd-gfx <<a href="mailto:amd-gfx-bounces@lists.freedesktop.org">amd-gfx-bounces@lists.freedesktop.org</a>> on behalf of Liu, Shaoyun <<a href="mailto:Shaoyun.Liu@amd.com">Shaoyun.Liu@amd.com</a>> Sent: Friday, June 30, 2017 10:55:39 PM To: Christian König; Michel Dänzer Cc: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Subject: RE: [PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic <o:p></o:p> <div> <o:p></o:p> </div> </div> </div> <div> Hi , Christian The new code actually will not use the fence function , it just need a memory that expose both CPU and GPU address . Do you really want to add the wrap functions that just expose the CPU and GPU address in this case ? Regards Shaoyun.liu -----Original Message----- From: Christian König [<a href="mailto:deathsimple@vodafone.de">mailto:deathsimple@vodafone.de</a>] Sent: Friday, June 30, 2017 3:57 AM To: Michel Dänzer; Liu, Shaoyun Cc: <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Subject: Re: [PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic Am 30.06.2017 um 03:21 schrieb Michel Dänzer: > On 30/06/17 06:08 AM, Shaoyun Liu wrote: >> 1. Use spin lock instead of mutex in KIQ 2. Directly write to KIQ >> fence address instead of using fence_emit() 3. Disable the interrupt >> for KIQ read/write and use CPU polling > This list indicates that this patch should be split up in at least > three patches. :) Yeah, apart from that is is not a good idea to mess with the fence internals directly in the KIQ code, please add a helper in the fence code for this. Regards, Christian. _______________________________________________ amd-gfx mailing list <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><o:p></o:p> </div> </div> </body> </html>