What do you mean HDP flush code ? 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 BR Monk From: Liu, Shaoyun Sent: Tuesday, July 04, 2017 2:14 PM To: Liu, Monk <Monk.Liu at amd.com>; Christian König <deathsimple at vodafone.de>; Michel Dänzer <michel at daenzer.net> Cc: amd-gfx at lists.freedesktop.org Subject: RE: [PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic Hi , Monk 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 . Regards Shaoyun.liu From: Liu, Monk Sent: Monday, July 03, 2017 5:45 AM To: Liu, Shaoyun; Christian König; Michel Dänzer Cc: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> Subject: Re: [PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic Hi Shaoyun looks you want to make KIQ reg access be atomic & UN-interruptible, 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. why not implement another KIQ reg access function ? e.g. : amdgpu_virt_kiq_rreg_atomic(...); amdgpu_virt_kiq_wreg_atomic(...); that way you can satisfy your requirement and not introduce unknown issue for other places that calling original virt_kiq_r/weg() function. 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 ) BR Monk ________________________________ From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx<mailto:amd-gfx-bounces at lists.freedesktop.org>> on behalf of Liu, Shaoyun <Shaoyun.Liu at amd.com<mailto:Shaoyun.Liu at amd.com>> Sent: Friday, June 30, 2017 10:55:39 PM To: Christian König; Michel Dänzer Cc: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> Subject: RE: [PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic 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 [mailto:deathsimple@xxxxxxxxxxx] Sent: Friday, June 30, 2017 3:57 AM To: Michel Dänzer; Liu, Shaoyun Cc: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> 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 amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> https://lists.freedesktop.org/mailman/listinfo/amd-gfx -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170705/9d3fbecf/attachment-0001.html>
