Nouveau - Dec 2015 - [PATCH] drm/nouveau: Fix pre-nv50 pageflip events

On Tue, Dec 01, 2015 at 04:08:16PM +0100, poma wrote:
> On Mon, Nov 16, 2015 at 4:11 PM, Daniel Vetter <daniel at ffwll.ch>
wrote:
> > On Mon, Nov 02, 2015 at 04:45:00PM +0900, Michel Dänzer wrote:
> >> On 31.10.2015 06:55, Daniel Vetter wrote:
> >> > Apparently pre-nv50 pageflip events happen before the actual
vblank
> >> > period. Therefore that functionality got semi-disabled in
> >> >
> >> > commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
> >> > Author: Mario Kleiner <mario.kleiner.de at gmail.com>
> >> > Date:   Tue May 13 00:42:08 2014 +0200
> >> >
> >> >     drm/nouveau/kms/nv04-nv40: fix pageflip events via
special case.
> >> >
> >> > Unfortunately that hack got uprooted in
> >> >
> >> > commit cc1ef118fc099295ae6aabbacc8af94d8d8885eb
> >> > Author: Thierry Reding <treding at nvidia.com>
> >> > Date:   Wed Aug 12 17:00:31 2015 +0200
> >> >
> >> >     drm/irq: Make pipe unsigned and name consistent
> >> >
> >> > Trigering a warning when trying to sample the vblank
timestamp for a
> >> > non-existing pipe. There's a few ways to fix this:
> >> >
> >> > - Open-code the old behaviour, which just enshrines this
slight
> >> >   breakage of the userspace ABI.
> >> >
> >> > - Revert Mario's commit and again inflict broken
timestamps, again not
> >> >   pretty.
> >> >
> >> > - Fix this for real by delaying the pageflip TS until the
next vblank
> >> >   interrupt, thereby making it accurate.
> >> >
> >> > This patch implements the third option. Since having a page
flip
> >> > interrupt that happens when the pageflip gets armed and not
when it
> >> > completes in the next vblank seems to be fairly common (older
i915 hw
> >> > works very similarly) create a new helper to arm vblank
events for
> >> > such drivers.
> >>
> >> What happens when the page flip interrupt arrives during a
vertical
> >> blank period?  Presumably the userspace event will be deferred
until the
> >> next vertical blank period, but the flip might already take effect
in
> >> the current one.
> >
> > Hm yeah there's a tiny race if your update handler for the
pageflip can
> > race with your vblank handler. That's impossible here since
it's all done
> > from the same hw irq hanlder, and since that is single-threaded there
> > shouldn't be a problem, as long as vblank handling are pageflip
are
> > ordered correctly.
> >
> > Might be worth a note in the kerneldoc though that this function
isn't
> > perfectly foolproof.
> > -Daniel
> 
> 
> Is there any updates in this respect?
> 
> drm-nouveau-Fix-pre-nv50-pageflip-events-v4.patch
> https://patchwork.kernel.org/patch/7591531
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=106431
> Reported:     2015-10-21
Ben Skeggs asleep probably. Dave, can you pls pick this up?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

When we are at it, the one with the title "[PATCH] drm/nouveau: Use 
drm_vblank_on/off consistently" from Daniel, which has a reviewed and 
tested by me also never made it into nouveau.

Maybe pick that up as well?

-mario

On 12/01/2015 04:55 PM, Daniel Vetter wrote:
> On Tue, Dec 01, 2015 at 04:08:16PM +0100, poma wrote:
>> On Mon, Nov 16, 2015 at 4:11 PM, Daniel Vetter <daniel at
ffwll.ch> wrote:
>>> On Mon, Nov 02, 2015 at 04:45:00PM +0900, Michel Dänzer wrote:
>>>> On 31.10.2015 06:55, Daniel Vetter wrote:
>>>>> Apparently pre-nv50 pageflip events happen before the
actual vblank
>>>>> period. Therefore that functionality got semi-disabled in
>>>>>
>>>>> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
>>>>> Author: Mario Kleiner <mario.kleiner.de at gmail.com>
>>>>> Date:   Tue May 13 00:42:08 2014 +0200
>>>>>
>>>>>      drm/nouveau/kms/nv04-nv40: fix pageflip events via
special case.
>>>>>
>>>>> Unfortunately that hack got uprooted in
>>>>>
>>>>> commit cc1ef118fc099295ae6aabbacc8af94d8d8885eb
>>>>> Author: Thierry Reding <treding at nvidia.com>
>>>>> Date:   Wed Aug 12 17:00:31 2015 +0200
>>>>>
>>>>>      drm/irq: Make pipe unsigned and name consistent
>>>>>
>>>>> Trigering a warning when trying to sample the vblank
timestamp for a
>>>>> non-existing pipe. There's a few ways to fix this:
>>>>>
>>>>> - Open-code the old behaviour, which just enshrines this
slight
>>>>>    breakage of the userspace ABI.
>>>>>
>>>>> - Revert Mario's commit and again inflict broken
timestamps, again not
>>>>>    pretty.
>>>>>
>>>>> - Fix this for real by delaying the pageflip TS until the
next vblank
>>>>>    interrupt, thereby making it accurate.
>>>>>
>>>>> This patch implements the third option. Since having a page
flip
>>>>> interrupt that happens when the pageflip gets armed and not
when it
>>>>> completes in the next vblank seems to be fairly common
(older i915 hw
>>>>> works very similarly) create a new helper to arm vblank
events for
>>>>> such drivers.
>>>>
>>>> What happens when the page flip interrupt arrives during a
vertical
>>>> blank period?  Presumably the userspace event will be deferred
until the
>>>> next vertical blank period, but the flip might already take
effect in
>>>> the current one.
>>>
>>> Hm yeah there's a tiny race if your update handler for the
pageflip can
>>> race with your vblank handler. That's impossible here since
it's all done
>>> from the same hw irq hanlder, and since that is single-threaded
there
>>> shouldn't be a problem, as long as vblank handling are pageflip
are
>>> ordered correctly.
>>>
>>> Might be worth a note in the kerneldoc though that this function
isn't
>>> perfectly foolproof.
>>> -Daniel
>>
>>
>> Is there any updates in this respect?
>>
>> drm-nouveau-Fix-pre-nv50-pageflip-events-v4.patch
>> https://patchwork.kernel.org/patch/7591531
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=106431
>> Reported:     2015-10-21
>
> Ben Skeggs asleep probably. Dave, can you pls pick this up?
> -Daniel
>

On Tue, Dec 1, 2015 at 6:30 PM, Mario Kleiner
<mario.kleiner.de at gmail.com> wrote:
> When we are at it, the one with the title "[PATCH] drm/nouveau: Use
> drm_vblank_on/off consistently" from Daniel, which has a reviewed and
tested
> by me also never made it into nouveau.
>
> Maybe pick that up as well?
>
> -mario
>
If you refer to
"[1/3] drm/nouveau: Use drm_vblank_on/off consistently"
https://patchwork.freedesktop.org/patch/50771

this is the result:
patched 4.4.0-0.rc3.git0.1.fc24.x86_64 with it,
i.e. 1-3-drm-nouveau-Use-drm_vblank_on-off-consistently.patch

# cat /var/log/Xorg.0.log
[   126.360]
X.Org X Server 1.18.0
...
[   126.909] (EE) [drm] Failed to open DRM device for pci:0000:02:00.0: -19
[   126.909] (EE) No devices detected.
[   126.909] (EE)
Fatal server error:
[   126.909] (EE) no screens found(EE)
[   126.909] (EE)
Please consult ....
...