[PATCH] drm/amdgpu: Make KIQ read/write register routine be atomic

[Liu, Shaoyun]

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
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 at lists.freedesktop.org<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 at vodafone.de]
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/20170704/3d17b278/attachment.html>