On Fri, Jun 30, 2017 at 8:47 AM, Christian König <deathsimple@xxxxxxxxxxx> wrote: > Am 30.06.2017 um 04:24 schrieb 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? > > > Well, the flag was never intended to be used by userspace. > > See the history was more like we need something in the kernel to place the > BO in CPU accessible VRAM. > > Then the closed source UMD came along and said hey we have the concept of > two different heaps for visible and invisible VRAM, how does that maps to > amdgpu? Mesa stopped using CPU_ACCESS_REQUIRED a couple of days ago. Marek _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
