Re: [PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

From: Mario Kleiner <mario.kleiner.de@gmail.com>
To: "Christian König" <deathsimple@vodafone.de>,
	"Michel Dänzer" <michel@daenzer.net>,
	"Mike Lothian" <mike@fireburn.co.uk>,
	"Daniel Vetter" <daniel@ffwll.ch>
Cc: Nayan Deshmukh <nayan26deshmukh@gmail.com>,
	amd-gfx list <amd-gfx@lists.freedesktop.org>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.
Date: Sat, 5 Nov 2016 02:17:13 +0100	[thread overview]
Message-ID: <153e3c35-ffe4-b938-2903-4b33ecf8558b@gmail.com> (raw)
In-Reply-To: <5e655b6e-82ac-05ed-080e-4dc7c270bec4@vodafone.de>

On 10/28/2016 07:48 PM, Christian König wrote:
> Am 28.10.2016 um 19:37 schrieb Mario Kleiner:
>>
>>
>> On 10/28/2016 03:34 AM, Michel Dänzer wrote:
>>> On 27/10/16 10:33 PM, Mike Lothian wrote:
>>>>
>>>> Just another gentle ping to see where you are with this?
>>>
>>> I haven't got a chance to look into this any further.
>>>
>>>
>>
>> Fwiw., as a proof of concept, the attached experimental patch does
>> work as tested on Intel HD Haswell + AMD R9 380 Tonga under amdgpu and
>> DRI3/Present when applied to drm-next (updated from a few days ago).
>> With DRI_PRIME=1 tearing for page-flipped fullscreen windows is gone
>> under all loads. The tearing with "windowed" windows now looks as
>> expected for regular tearing not related to Prime.
>
> Yeah, that's pretty much what I had in mind as well. You additionally
> need to wait for the shared fences when you export the BO for the first
> time, but that's only a nitpick.
>
>>
>> ftrace confirms the i915 driver's pageflip function is waiting on the
>> fence in reservation_object_wait_timeout_rcu() as it should.
>>
>> That entry->tv.shared needs to be set false for such buffers in
>> amdgpu_bo_list_set() makes sense to me, as that is part of the buffer
>> validation for command stream submission. There are other places in
>> the driver where tv.shared is set, which i didn't check so far.
>>
>> I don't know which of these would need to be updated with a "exported
>> bo" check as well, e.g., for video decoding or maybe gpu compute?
>> Adding or removing the check to amdgpu_gem_va_update_vm(), e.g., made
>> no difference. I assume that makes sense because that functions seems
>> to deal with amdgpu internal vm page tables or page table entries for
>> such a bo, not with something visible to external clients?
>
> Yes, exactly. VM updates doesn't matter for anybody else than amdgpu.
> Additional to that we don't even add a fence to the shared slot we
> reserve (could probably drop that for optimization).
>
> Please remove the debugging stuff and the extra code on the VM updates
> and add a reservation_object_wait_timeout_rcu() to the export path and
> we should be good to go.
>

Done. Patch v2 just sent out after retesting with Tonga only and Intel + 
Tonga renderoffload. Would be cool if we could get this into 4.9-rc.

Ideally also backported to 4.8, given it is a simple change, as that 
would be the next official kernel for *buntu 16.04-LTS and derivatives, 
but that's probably breaking the rules as it doesn't fix a regression?

thanks,
-mario

> Regards,
> Christian.
>
>>
>> All i can say is it fixes 3D rendering under DRI3 + Prime +
>> pageflipping without causing any obvious new problems.
>>
>> -mario
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel