[PATCH 4/5] drm/amdgpu: Set/clear CPU_ACCESS_REQUIRED flag on page fault and CS

[Michel Dänzer]

On 29/06/17 07:05 PM, Daniel Vetter wrote:
> On Thu, Jun 29, 2017 at 06:58:05PM +0900, Michel Dänzer wrote:
>> On 29/06/17 05:23 PM, Christian König wrote:
>>> Am 29.06.2017 um 04:35 schrieb Michel Dänzer:
>>>> On 29/06/17 08:26 AM, John Brooks wrote:
>>>>> On Wed, Jun 28, 2017 at 03:05:32PM +0200, Christian König wrote:
>>>>>>>
>>>>>>> Instead of the flag being set in stone at BO creation, set the flag
>>>>>>> when a
>>>>>>> page fault occurs so that it goes somewhere CPU-visible, and clear
>>>>>>> it when
>>>>>>> the BO is requested by the GPU.
>>>>>>>
>>>>>>> However, clearing the CPU_ACCESS_REQUIRED flag may move BOs in GTT to
>>>>>>> invisible VRAM, where they may promptly generate another page
>>>>>>> fault. When
>>>>>>> BOs are constantly moved back and forth like this, it is highly
>>>>>>> detrimental
>>>>>>> to performance. Only clear the flag on CS if:
>>>>>>>
>>>>>>> - The BO wasn't page faulted for a certain amount of time
>>>>>>> (currently 10
>>>>>>>    seconds), and
>>>>>>> - its last page fault didn't occur too soon (currently 500ms) after
>>>>>>> its
>>>>>>>    last CS request, or vice versa.
>>>>>>>
>>>>>>> Setting the flag in amdgpu_fault_reserve_notify() also means that
>>>>>>> we can
>>>>>>> remove the loop to restrict lpfn to the end of visible VRAM, because
>>>>>>> amdgpu_ttm_placement_init() will do it for us.
>>>>>> I'm fine with the general approach, but I'm still absolutely not
>>>>>> keen about
>>>>>> clearing the flag when userspace has originally specified it.
>>>> Is there any specific concern you have about that?
>>>
>>> Yeah, quite a bunch actually. We want to use this flag for P2P buffer
>>> sharing in the future as well and I don't intent to add another one like
>>> CPU_ACCESS_REALLY_REQUIRED or something like this.
>>
>> Won't a BO need to be pinned while it's being shared with another device?
> 
> That's an artifact of the current kernel implementation, I think we could
> do better (but for current use-cases where we share a bunch of scanouts
> and maybe a few pixmaps it's pointless). I wouldn't bet uapi on this never
> changing.

Surely there will need to be some kind of transaction though to let the
driver know when a BO starts/stops being shared with another device?
Either via the existing dma-buf callbacks, or something similar. We
can't rely on userspace setting a "CPU access" flag to make sure a BO
can be shared with other devices?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer