Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v4)

[Greg KH]

On Tue, Dec 15, 2015 at 03:17:21PM +, Emil Velikov wrote:
> On 15 December 2015 at 15:01, poma  wrote:
> > On 15.12.2015 12:21, Emil Velikov wrote:
> >> On 15 December 2015 at 11:11, poma  wrote:
> >>
> >>>
> >>> Apparently not reached @stable (stable: 4.3.3 2015-12-15),
> >>> so here's one more time.
> >>>
> >> It has reached 4.4-rcX and will get picked by the stable maintainer
> >> (Greg?) in due time. Meanwhile you can ask your distro maintainers to
> >> apply it locally until we get an official release that includes it.
> >>
> >> -Emil
> >>
> >
> > It is all but unknown ;)
> > https://bugzilla.redhat.com/show_bug.cgi?id=1281368
> >
> > Emil, the point is - if it has -not- reached sta...@vger.kernel.org, how 
> > can it be applied, in the first place.
> >
> The same way many others do ? I'd imagine there is a tool/script which
> parses through the development tree, which would explain why (many?)
> people explicitly suppress git from sending an email yet things still
> work. There is extra information in the documentation [1] if you're
> interested.

Yes, this is in the queue, along with 276 other patches that I need to
apply as well.  The backlog is large this time of year, sorry.

greg k-h
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v4)

[Emil Velikov]

On 15 December 2015 at 11:11, poma  wrote:

>
> Apparently not reached @stable (stable: 4.3.3 2015-12-15),
> so here's one more time.
>
It has reached 4.4-rcX and will get picked by the stable maintainer
(Greg?) in due time. Meanwhile you can ask your distro maintainers to
apply it locally until we get an official release that includes it.

-Emil
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v4)

[poma]

On 10.11.2015 17:41, Thierry Reding wrote:
> On Tue, Nov 10, 2015 at 05:37:31PM +0100, Thierry Reding wrote:
>> From: Daniel Vetter 
>>
>> Apparently pre-nv50 pageflip events happen before the actual vblank
>> period. Therefore that functionality got semi-disabled in
>>
>> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
>> Author: Mario Kleiner 
>> 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 
>> Date:   Wed Aug 12 17:00:31 2015 +0200
>>
>> drm/irq: Make pipe unsigned and name consistent
>>
>> Triggering 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.
>>
>> v2 (Mario Kleiner):
>> - Fix function prototypes in drmP.h
>> - Add missing vblank_put() for pageflip completion without
>>   pageflip event.
>> - Initialize sequence number for queued pageflip event to avoid
>>   trouble in drm_handle_vblank_events().
>> - Remove dead code and spelling fix.
>>
>> v3 (Mario Kleiner):
>> - Add a signed-off-by and cc stable tag per Ilja's advice.
>>
>> v4 (Thierry Reding):
>> - Fix kerneldoc typo, discovered by Michel Dänzer
>> - Rearrange tags and changelog
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
>> Cc: Thierry Reding 
>> Cc: Mario Kleiner 
>> Cc: Ben Skeggs 
>> Cc: Ilia Mirkin 
>> Signed-off-by: Daniel Vetter 
>> Reviewed-by: Mario Kleiner 
>> Cc: sta...@vger.kernel.org # v4.3
>> Signed-off-by: Mario Kleiner 
>> Signed-off-by: Thierry Reding 
>> ---
>>  drivers/gpu/drm/drm_irq.c | 54 
>> ++-
>>  drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++-
>>  include/drm/drmP.h|  4 +++
>>  3 files changed, 68 insertions(+), 9 deletions(-)
> 
> Hi Dave,
> 
> It'd be great if you could queue this up for fixes, since it gets rid of
> a WARN_ON() that is triggered on a number of cards in v4.3. I realize
> that this is a tad big for stable, but it's the right way to fix this.
> If you'd prefer something smaller, I think we can fix the regression
> using a one-line band-aid and then apply this one on top for v4.4.
> 
> Thierry
> 


Apparently not reached @stable (stable: 4.3.3 2015-12-15),
so here's one more time.


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v4)

[poma]

On 15.12.2015 12:21, Emil Velikov wrote:
> On 15 December 2015 at 11:11, poma  wrote:
> 
>>
>> Apparently not reached @stable (stable: 4.3.3 2015-12-15),
>> so here's one more time.
>>
> It has reached 4.4-rcX and will get picked by the stable maintainer
> (Greg?) in due time. Meanwhile you can ask your distro maintainers to
> apply it locally until we get an official release that includes it.
> 
> -Emil
> 

It is all but unknown ;)
https://bugzilla.redhat.com/show_bug.cgi?id=1281368

Emil, the point is - if it has -not- reached sta...@vger.kernel.org, how can it 
be applied, in the first place.

Aye

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v4)

[Emil Velikov]

On 15 December 2015 at 15:01, poma  wrote:
> On 15.12.2015 12:21, Emil Velikov wrote:
>> On 15 December 2015 at 11:11, poma  wrote:
>>
>>>
>>> Apparently not reached @stable (stable: 4.3.3 2015-12-15),
>>> so here's one more time.
>>>
>> It has reached 4.4-rcX and will get picked by the stable maintainer
>> (Greg?) in due time. Meanwhile you can ask your distro maintainers to
>> apply it locally until we get an official release that includes it.
>>
>> -Emil
>>
>
> It is all but unknown ;)
> https://bugzilla.redhat.com/show_bug.cgi?id=1281368
>
> Emil, the point is - if it has -not- reached sta...@vger.kernel.org, how can 
> it be applied, in the first place.
>
The same way many others do ? I'd imagine there is a tool/script which
parses through the development tree, which would explain why (many?)
people explicitly suppress git from sending an email yet things still
work. There is extra information in the documentation [1] if you're
interested.

-Emil
[1] https://www.kernel.org/doc/Documentation/stable_kernel_rules.txt
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[poma]

On 02.12.2015 09:55, Daniel Vetter wrote:
> On Wed, Dec 02, 2015 at 06:40:32AM +0100, poma wrote:
>> On Tue, Dec 1, 2015 at 6:30 PM, Mario Kleiner
>>  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::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 
> 
> Kernel log needed if the drm device isn't there. And this is pretty much
> impossible, worst case modesetting is functionally busted.
> -Daniel
> 


Pardon me,
I missed 0 in EXTRAVERSION, therefore version magic not so pretty.

$ sed -i 's/-rc3/\-0.rc3/' Makefile

[ 1771.699138] checking generic (f700 e0) vs hw (d000 1000)
[ 1771.699143] checking generic (f700 e0) vs hw (f600 200)
[ 1771.699144] fb: switching to nouveaufb from VESA VGA
[ 1771.699271] Console: switching to colour dummy device 80x25
[ 1771.699450] nouveau :02:00.0: NVIDIA G98 (098200a2)
[ 1771.813968] nouveau :02:00.0: bios: version 62.98.2c.00.00
[ 1771.834802] nouveau :02:00.0: bios: M0203T not found
[ 1771.834819] nouveau :02:00.0: bios: M0203E not matched!
[ 1771.834830] nouveau :02:00.0: fb: 512 MiB DDR2
[ 1774.593921] [TTM] Zone  kernel: Available graphics memory: 1891762 kiB
[ 1774.593926] [TTM] Initializing pool allocator
[ 1774.593932] [TTM] Initializing DMA pool allocator
[ 1774.593948] nouveau :02:00.0: DRM: VRAM: 512 MiB
[ 1774.593950] nouveau :02:00.0: DRM: GART: 1048576 MiB
[ 1774.593954] nouveau :02:00.0: DRM: TMDS table version 2.0
[ 1774.593956] nouveau :02:00.0: DRM: DCB version 4.0
[ 1774.593959] nouveau :02:00.0: DRM: DCB outp 00: 02000300 0028
[ 1774.593962] nouveau :02:00.0: DRM: DCB outp 01: 01000302 00020030
[ 1774.593964] nouveau :02:00.0: DRM: DCB outp 02: 04011310 0028
[ 1774.593966] nouveau :02:00.0: DRM: DCB outp 03: 010223f1 00c0c080
[ 1774.593969] nouveau :02:00.0: DRM: DCB conn 00: 1030
[ 1774.593971] nouveau :02:00.0: DRM: DCB conn 01: 0100
[ 1774.593973] nouveau :02:00.0: DRM: DCB conn 02: 0210
[ 1774.593975] nouveau :02:00.0: DRM: DCB conn 03: 0211
[ 1774.593977] nouveau :02:00.0: DRM: DCB conn 04: 0213
[ 1774.595827] nouveau :02:00.0: DRM: failed to create encoder 0/1/0: -19
[ 1774.595832] nouveau :02:00.0: DRM: TV-1 has no encoders, removing
[ 1774.595867] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[ 1774.595870] [drm] Driver supports precise vblank timestamp query.
[ 1774.600023] CE: hpet increased min_delta_ns to 11521 nsec
[ 1774.605699] nouveau :02:00.0: DRM: MM: using M2MF for buffer copies
[ 1774.690808] nouveau :02:00.0: DRM: allocated 1024x768 fb: 0x5, bo 
8800c9a46000
[ 1774.690960] fbcon: nouveaufb (fb0) is primary device
[ 1774.746581] Console: switching to colour frame buffer device 128x48
[ 1774.747411] nouveau :02:00.0: fb0: nouveaufb frame buffer device
[ 1774.750093] [drm] Initialized nouveau 1.3.1 20120801 for :02:00.0 on 
minor 0


Tested-by: poma 


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Daniel Vetter]

On Wed, Dec 02, 2015 at 06:40:32AM +0100, poma wrote:
> On Tue, Dec 1, 2015 at 6:30 PM, Mario Kleiner
>  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::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 

Kernel log needed if the drm device isn't there. And this is pretty much
impossible, worst case modesetting is functionally busted.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Mario Kleiner]

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  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 
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 
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


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[poma]

On Tue, Dec 1, 2015 at 6:30 PM, Mario Kleiner
 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::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 
...
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Daniel Vetter]

On Tue, Dec 01, 2015 at 04:08:16PM +0100, poma wrote:
> On Mon, Nov 16, 2015 at 4:11 PM, Daniel Vetter  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 
> >> > 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 
> >> > 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
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[poma]

On Mon, Nov 16, 2015 at 4:11 PM, Daniel Vetter  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 
>> > 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 
>> > 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
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Daniel Vetter]

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 
> > 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 
> > 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3) -> v4

[Thierry Reding]

On Wed, Nov 11, 2015 at 09:12:33PM +0100, poma wrote:
> On 10.11.2015 17:25, Mario Kleiner wrote:
> > On 11/10/2015 05:00 PM, Thierry Reding wrote:
> >> On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote:
> >>> From: Daniel Vetter 
> >>>
> >>> Apparently pre-nv50 pageflip events happen before the actual vblank
> >>> period. Therefore that functionality got semi-disabled in
> >>>
> >>> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
> >>> Author: Mario Kleiner 
> >>> 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 
> >>> 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.
> >>>
> >>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
> >>> Cc: Thierry Reding 
> >>> Cc: Mario Kleiner 
> >>> Cc: Ben Skeggs 
> >>> Cc: Ilia Mirkin 
> >>>
> >>> v2 (mario): Integrate my own review comments into Daniels patch.
> >>> - Fix function prototypes in drmP.h
> >>> - Add missing vblank_put() for pageflip completion without
> >>>   pageflip event.
> >>> - Initialize sequence number for queued pageflip event to avoidng
> >>>   trouble in drm_handle_vblank_events().
> >>> - Remove dead code and spelling fix.
> >>>
> >>> v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice.
> >>>
> >>> Signed-off-by: Daniel Vetter 
> >>> (v1) Reviewed-by: Mario Kleiner 
> >>> (v2/v3) Signed-off-by: Mario Kleiner 
> >>>
> >>> Cc: sta...@vger.kernel.org # v4.3
> >>> ---
> >>>   drivers/gpu/drm/drm_irq.c | 54 
> >>> ++-
> >>>   drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++-
> >>>   include/drm/drmP.h|  4 +++
> >>>   3 files changed, 68 insertions(+), 9 deletions(-)
> >>
> >> This looks good to me. Let me clean this up a little and submit it to
> >> Dave.
> >>
> >> Thierry
> >>
> > 
> > Btw., if somebody has a functional old card for testing this, it should 
> > be easy to verify if it works on pre-nv50. If it would not work it would 
> > deliver the pageflip event 1 frame delayed, so at least on standard 
> > nouveau + default DRI2 + default double-buffering the rate for a tight 
> > loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, 
> > quite noticeable. Afaik we also have Piglit tests for OML_sync_control 
> > which would likely fail if this would be broken.
> > 
> > Oh and if someone has tips on how to resurrect an old nv-40 PC (booted 
> > with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind 
> > hearing them. It would be nice to still be able to use that card for 
> > testing.
> > 
> > thanks,
> > -mario
> 
> 
> [ cut here ]
> WARNING: CPU: 0 PID: 313 at lib/dma-debug.c:1205 check_sync+0x169/0x6e0()
> nouveau :01:00.0: DMA-API: device driver tries to sync DMA memory it has 
> not allocated [device address=0xc0bf6468] [size=4096 bytes]
> Modules linked in: nouveau(+) ...
> CPU: 0 PID: 313 Comm: systemd-udevd Not tainted 4.3.0-3.fc22.i686+debug #1
> ...
> Call Trace:
>  [] dump_stack+0x48/0x69
>  [] warn_slowpath_common+0x87/0xc0
>  [] ? check_sync+0x169/0x6e0
>  [] ? check_sync+0x169/0x6e0
>  [] warn_slowpath_fmt+0x3e/0x60
>  [] check_sync+0x169/0x6e0
>  [] debug_dma_sync_single_for_device+0x7d/0x90
>  [] ? ttm_bo_del_sub_from_lru+0x18/0x50 [ttm]
>  [] ? text_poke_bp+0xd0/0xd0
>  [] nouveau_bo_sync_for_device+0x8b/0xa0 [nouveau]
>  [] nouveau_bo_validate+0x34/0x40 [nouveau]
>  [] nouveau_bo_pin+0x188/0x290 [nouveau]
>  [] ? nv10_bo_put_tile_region+0x80/0x80 [nouveau]
>  [] nouveau_channel_prep+0xfd/0x2c0 [nouveau]
>  [] nouveau_channel_new+0x57/0x7a0 [nouveau]
>  [] ? kfree+0xdc/0x280
>  [] ? nvif_object_sclass_put+0x12/0x20 [nouveau]
>  [] 

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3) -> v4

[poma]

On 12.11.2015 14:48, Thierry Reding wrote:
> On Wed, Nov 11, 2015 at 09:12:33PM +0100, poma wrote:
>> On 10.11.2015 17:25, Mario Kleiner wrote:
>>> On 11/10/2015 05:00 PM, Thierry Reding wrote:
 On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote:
> From: Daniel Vetter 
>
> Apparently pre-nv50 pageflip events happen before the actual vblank
> period. Therefore that functionality got semi-disabled in
>
> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
> Author: Mario Kleiner 
> 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 
> 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.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
> Cc: Thierry Reding 
> Cc: Mario Kleiner 
> Cc: Ben Skeggs 
> Cc: Ilia Mirkin 
>
> v2 (mario): Integrate my own review comments into Daniels patch.
> - Fix function prototypes in drmP.h
> - Add missing vblank_put() for pageflip completion without
>   pageflip event.
> - Initialize sequence number for queued pageflip event to avoidng
>   trouble in drm_handle_vblank_events().
> - Remove dead code and spelling fix.
>
> v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice.
>
> Signed-off-by: Daniel Vetter 
> (v1) Reviewed-by: Mario Kleiner 
> (v2/v3) Signed-off-by: Mario Kleiner 
>
> Cc: sta...@vger.kernel.org # v4.3
> ---
>   drivers/gpu/drm/drm_irq.c | 54 
> ++-
>   drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++-
>   include/drm/drmP.h|  4 +++
>   3 files changed, 68 insertions(+), 9 deletions(-)

 This looks good to me. Let me clean this up a little and submit it to
 Dave.

 Thierry

>>>
>>> Btw., if somebody has a functional old card for testing this, it should 
>>> be easy to verify if it works on pre-nv50. If it would not work it would 
>>> deliver the pageflip event 1 frame delayed, so at least on standard 
>>> nouveau + default DRI2 + default double-buffering the rate for a tight 
>>> loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, 
>>> quite noticeable. Afaik we also have Piglit tests for OML_sync_control 
>>> which would likely fail if this would be broken.
>>>
>>> Oh and if someone has tips on how to resurrect an old nv-40 PC (booted 
>>> with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind 
>>> hearing them. It would be nice to still be able to use that card for 
>>> testing.
>>>
>>> thanks,
>>> -mario
>>
>>
>> [ cut here ]
>> WARNING: CPU: 0 PID: 313 at lib/dma-debug.c:1205 check_sync+0x169/0x6e0()
>> nouveau :01:00.0: DMA-API: device driver tries to sync DMA memory it has 
>> not allocated [device address=0xc0bf6468] [size=4096 bytes]
>> Modules linked in: nouveau(+) ...
>> CPU: 0 PID: 313 Comm: systemd-udevd Not tainted 4.3.0-3.fc22.i686+debug #1
>> ...
>> Call Trace:
>>  [] dump_stack+0x48/0x69
>>  [] warn_slowpath_common+0x87/0xc0
>>  [] ? check_sync+0x169/0x6e0
>>  [] ? check_sync+0x169/0x6e0
>>  [] warn_slowpath_fmt+0x3e/0x60
>>  [] check_sync+0x169/0x6e0
>>  [] debug_dma_sync_single_for_device+0x7d/0x90
>>  [] ? ttm_bo_del_sub_from_lru+0x18/0x50 [ttm]
>>  [] ? text_poke_bp+0xd0/0xd0
>>  [] nouveau_bo_sync_for_device+0x8b/0xa0 [nouveau]
>>  [] nouveau_bo_validate+0x34/0x40 [nouveau]
>>  [] nouveau_bo_pin+0x188/0x290 [nouveau]
>>  [] ? nv10_bo_put_tile_region+0x80/0x80 [nouveau]
>>  [] nouveau_channel_prep+0xfd/0x2c0 [nouveau]
>>  [] nouveau_channel_new+0x57/0x7a0 [nouveau]
>>  [] ? 

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3) -> v4

[poma]

On 10.11.2015 17:25, Mario Kleiner wrote:
> On 11/10/2015 05:00 PM, Thierry Reding wrote:
>> On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote:
>>> From: Daniel Vetter 
>>>
>>> Apparently pre-nv50 pageflip events happen before the actual vblank
>>> period. Therefore that functionality got semi-disabled in
>>>
>>> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
>>> Author: Mario Kleiner 
>>> 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 
>>> 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.
>>>
>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
>>> Cc: Thierry Reding 
>>> Cc: Mario Kleiner 
>>> Cc: Ben Skeggs 
>>> Cc: Ilia Mirkin 
>>>
>>> v2 (mario): Integrate my own review comments into Daniels patch.
>>> - Fix function prototypes in drmP.h
>>> - Add missing vblank_put() for pageflip completion without
>>>   pageflip event.
>>> - Initialize sequence number for queued pageflip event to avoidng
>>>   trouble in drm_handle_vblank_events().
>>> - Remove dead code and spelling fix.
>>>
>>> v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice.
>>>
>>> Signed-off-by: Daniel Vetter 
>>> (v1) Reviewed-by: Mario Kleiner 
>>> (v2/v3) Signed-off-by: Mario Kleiner 
>>>
>>> Cc: sta...@vger.kernel.org # v4.3
>>> ---
>>>   drivers/gpu/drm/drm_irq.c | 54 
>>> ++-
>>>   drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++-
>>>   include/drm/drmP.h|  4 +++
>>>   3 files changed, 68 insertions(+), 9 deletions(-)
>>
>> This looks good to me. Let me clean this up a little and submit it to
>> Dave.
>>
>> Thierry
>>
> 
> Btw., if somebody has a functional old card for testing this, it should 
> be easy to verify if it works on pre-nv50. If it would not work it would 
> deliver the pageflip event 1 frame delayed, so at least on standard 
> nouveau + default DRI2 + default double-buffering the rate for a tight 
> loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, 
> quite noticeable. Afaik we also have Piglit tests for OML_sync_control 
> which would likely fail if this would be broken.
> 
> Oh and if someone has tips on how to resurrect an old nv-40 PC (booted 
> with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind 
> hearing them. It would be nice to still be able to use that card for 
> testing.
> 
> thanks,
> -mario


[ cut here ]
WARNING: CPU: 0 PID: 313 at lib/dma-debug.c:1205 check_sync+0x169/0x6e0()
nouveau :01:00.0: DMA-API: device driver tries to sync DMA memory it has 
not allocated [device address=0xc0bf6468] [size=4096 bytes]
Modules linked in: nouveau(+) ...
CPU: 0 PID: 313 Comm: systemd-udevd Not tainted 4.3.0-3.fc22.i686+debug #1
...
Call Trace:
 [] dump_stack+0x48/0x69
 [] warn_slowpath_common+0x87/0xc0
 [] ? check_sync+0x169/0x6e0
 [] ? check_sync+0x169/0x6e0
 [] warn_slowpath_fmt+0x3e/0x60
 [] check_sync+0x169/0x6e0
 [] debug_dma_sync_single_for_device+0x7d/0x90
 [] ? ttm_bo_del_sub_from_lru+0x18/0x50 [ttm]
 [] ? text_poke_bp+0xd0/0xd0
 [] nouveau_bo_sync_for_device+0x8b/0xa0 [nouveau]
 [] nouveau_bo_validate+0x34/0x40 [nouveau]
 [] nouveau_bo_pin+0x188/0x290 [nouveau]
 [] ? nv10_bo_put_tile_region+0x80/0x80 [nouveau]
 [] nouveau_channel_prep+0xfd/0x2c0 [nouveau]
 [] nouveau_channel_new+0x57/0x7a0 [nouveau]
 [] ? kfree+0xdc/0x280
 [] ? nvif_object_sclass_put+0x12/0x20 [nouveau]
 [] nouveau_drm_load+0x596/0x8d0 [nouveau]
 [] ? trace_hardirqs_on_caller+0x12c/0x1d0
 [] ? drm_minor_register+0x89/0x120 [drm]
 [] drm_dev_register+0x96/0xa0 [drm]
 [] drm_get_pci_dev+0x79/0x1c0 [drm]
 [] ? pcibios_set_master+0x4e/0xa0
 [] nouveau_drm_probe+0x1ee/0x220 [nouveau]
 [] 

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3)

[Thierry Reding]

On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote:
> From: Daniel Vetter 
> 
> Apparently pre-nv50 pageflip events happen before the actual vblank
> period. Therefore that functionality got semi-disabled in
> 
> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
> Author: Mario Kleiner 
> 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 
> 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.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
> Cc: Thierry Reding 
> Cc: Mario Kleiner 
> Cc: Ben Skeggs 
> Cc: Ilia Mirkin 
> 
> v2 (mario): Integrate my own review comments into Daniels patch.
>- Fix function prototypes in drmP.h
>- Add missing vblank_put() for pageflip completion without
>  pageflip event.
>- Initialize sequence number for queued pageflip event to avoid
>  trouble in drm_handle_vblank_events().
>- Remove dead code and spelling fix.
> 
> v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice.
> 
> Signed-off-by: Daniel Vetter 
> (v1) Reviewed-by: Mario Kleiner 
> (v2/v3) Signed-off-by: Mario Kleiner 
> 
> Cc: sta...@vger.kernel.org # v4.3
> ---
>  drivers/gpu/drm/drm_irq.c | 54 
> ++-
>  drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++-
>  include/drm/drmP.h|  4 +++
>  3 files changed, 68 insertions(+), 9 deletions(-)

This looks good to me. Let me clean this up a little and submit it to
Dave.

Thierry


signature.asc
Description: PGP signature
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v3)

[Mario Kleiner]

On 11/10/2015 05:00 PM, Thierry Reding wrote:

On Tue, Nov 10, 2015 at 03:54:52PM +0100, Mario Kleiner wrote:

From: Daniel Vetter 

Apparently pre-nv50 pageflip events happen before the actual vblank
period. Therefore that functionality got semi-disabled in

commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
Author: Mario Kleiner 
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 
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.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
Cc: Thierry Reding 
Cc: Mario Kleiner 
Cc: Ben Skeggs 
Cc: Ilia Mirkin 

v2 (mario): Integrate my own review comments into Daniels patch.
- Fix function prototypes in drmP.h
- Add missing vblank_put() for pageflip completion without
  pageflip event.
- Initialize sequence number for queued pageflip event to avoidng
  trouble in drm_handle_vblank_events().
- Remove dead code and spelling fix.

v3 (mario): Add a signed-off-by and cc stable tag per Ilja's advice.

Signed-off-by: Daniel Vetter 
(v1) Reviewed-by: Mario Kleiner 
(v2/v3) Signed-off-by: Mario Kleiner 

Cc: sta...@vger.kernel.org # v4.3
---
  drivers/gpu/drm/drm_irq.c | 54 ++-
  drivers/gpu/drm/nouveau/nouveau_display.c | 19 ++-
  include/drm/drmP.h|  4 +++
  3 files changed, 68 insertions(+), 9 deletions(-)


This looks good to me. Let me clean this up a little and submit it to
Dave.

Thierry



Btw., if somebody has a functional old card for testing this, it should 
be easy to verify if it works on pre-nv50. If it would not work it would 
deliver the pageflip event 1 frame delayed, so at least on standard 
nouveau + default DRI2 + default double-buffering the rate for a tight 
loop of page-flipped swaps should go down to 30 fps on a 60 Hz display, 
quite noticeable. Afaik we also have Piglit tests for OML_sync_control 
which would likely fail if this would be broken.


Oh and if someone has tips on how to resurrect an old nv-40 PC (booted 
with BIOS only) graphics card in a MacPro (EFI boot), i wouldn't mind 
hearing them. It would be nice to still be able to use that card for 
testing.


thanks,
-mario
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Mario Kleiner]

Hi,

i just sent out a (v2) of Daniels patch, with my review comments and 
reviewed-by for the code already applied to the code for convenience. 
Interspersed below in the patch the review comments for a few small bugs.


This and Daniels original patch is only compile tested. I still have 
that GeForce 7800 GTX, but unfortunately i don't have the original PC 
anymore for testing it. Today i tried to put the card as a 2nd non-boot 
card into a MacPro for testing, but the EFI based Mac apparently didn't 
like that old PC card that much, so testing was a no go. Bootup ended 
with some nouveau MMIO read and write faults and then lockup. Usually 
more recent NVidia PC cards do work in Macs under Linux with nouveau as 
non-boot gpus, but for some reason this one doesn't.


Anyway, after digging through my old e-mail conversation with Ben from a 
year ago, i think Daniel's patch should work and solve the problem quite 
elegantly:


iirc Ben explained to me that on pre-nv50, nouveau_flip_complete() 
(which calls nouveau_finish_page_flip()), is not triggered by an actual 
pageflip interrupt, but by a fifo software interrupt programmed to fire 
shortly before the vblank. On my test card it fired in the last scanline 
before vblank, probably at the end of active scanout. 
nouveau_flip_complete() would first call nouveau_finish_page_flip() to 
send the pageflip event, and then manually flip to the new framebuffer 
by calling  nv_set_crtc_base(). I think/assume nv_set_crtc_base() is not 
itself synchronized to vblank, so we should get the correct behaviour:


1. Shortly before start of vblank: fifo sw interrupt -> 
nouveau_flip_complete() -> nouveau_finish_page_flip() queues pageflip 
event for later delivery by vblank irq handler -> nv_set_crtc_base() 
flips to the new fb. Return from irq.


2. A few scanlines later, vblank irq fires -> drm_handle_vblank() 
updates vblank count and timestamps -> drm_handle_vblank_events() 
dispatches queued pageflip completion event from 1), now tagged with 
proper vblank count and timestamp of flip completion.


thanks,
-mario


On 11/06/2015 06:19 PM, Thierry Reding wrote:

Cc += Mario Kleiner, Mario, can you take a look whether this proposed
solution makes sense and fixes the issues you were seeing back when you
posted the patch in commit:

commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
Author: Mario Kleiner 
Date:   Tue May 13 00:42:08 2014 +0200

 drm/nouveau/kms/nv04-nv40: fix pageflip events via special case.

 Cards with nv04 display engine can't reliably use vblank
 counts and timestamps computed via drm_handle_vblank(), as
 the function gets invoked after sending the pageflip events.

 Fix this by defaulting to the old crtcid = -1 fallback path
 on <= NV-50 cards, and only using the precise path on NV-50
 and later.

 Signed-off-by: Mario Kleiner 
 Signed-off-by: Ben Skeggs 
 Cc:  # 3.13+

Do you happen to still have the setup around where you saw this?

Thierry

On Fri, Oct 30, 2015 at 10:55:40PM +0100, 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 
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 
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.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
Cc: Thierry Reding 
Cc: Mario Kleiner 
Cc: Ben Skeggs 
Cc: Ilia Mirkin 
Signed-off-by: Daniel Vetter 
---

Note that due to lack of hw this is completely untested. But I think
it's the right way to fix this.
-Daniel
---
  drivers/gpu/drm/drm_irq.c | 56 ++-
  drivers/gpu/drm/nouveau/nouveau_display.c | 16 -
  include/drm/drmP.h   

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v2)

[Mario Kleiner]

On 11/09/2015 02:02 PM, Ilia Mirkin wrote:

On Mon, Nov 9, 2015 at 7:57 AM, Mario Kleiner
 wrote:

From: Daniel Vetter 

Apparently pre-nv50 pageflip events happen before the actual vblank
period. Therefore that functionality got semi-disabled in

commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
Author: Mario Kleiner 
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 
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.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
Cc: Thierry Reding 
Cc: Mario Kleiner 
Cc: Ben Skeggs 
Cc: Ilia Mirkin 

v2 (mario): Integrate my own review comments into Daniels patch.
- Fix function prototypes in drmP.h
- Add missing vblank_put() for pageflip completion without
  pageflip event.
- Initialize sequence number for queued pageflip event to avoid
  trouble in drm_handle_vblank_events().
- Remove dead code and spelling fix.

Signed-off-by: Daniel Vetter 
Reviewed-by: Mario Kleiner 


Without commenting on the actual patch, a few points of procedure:

(a) If you're sending the patch, you're supposed to add your
Signed-off-by. So you'd keep Daniel's and add yours.


I thought my tiny fixes didn't warrant adding a signed off by myself, 
but if that was wrong, consider it added:


v2: Signed-off-by: Mario Kleiner 


(b) Since this is triggering warns for real people in real situations,
tack on a

Cc: sta...@vger.kernel.org # v4.3



Ah, sorry, this is already a problem in a released kernel? I thought 
this was something new and so far only for drm-next.


ciao,
-mario


Cheers,

   -ilia


___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events (v2)

[Ilia Mirkin]

On Mon, Nov 9, 2015 at 7:57 AM, Mario Kleiner
 wrote:
> From: Daniel Vetter 
>
> Apparently pre-nv50 pageflip events happen before the actual vblank
> period. Therefore that functionality got semi-disabled in
>
> commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
> Author: Mario Kleiner 
> 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 
> 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.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
> Cc: Thierry Reding 
> Cc: Mario Kleiner 
> Cc: Ben Skeggs 
> Cc: Ilia Mirkin 
>
> v2 (mario): Integrate my own review comments into Daniels patch.
>- Fix function prototypes in drmP.h
>- Add missing vblank_put() for pageflip completion without
>  pageflip event.
>- Initialize sequence number for queued pageflip event to avoid
>  trouble in drm_handle_vblank_events().
>- Remove dead code and spelling fix.
>
> Signed-off-by: Daniel Vetter 
> Reviewed-by: Mario Kleiner 

Without commenting on the actual patch, a few points of procedure:

(a) If you're sending the patch, you're supposed to add your
Signed-off-by. So you'd keep Daniel's and add yours.
(b) Since this is triggering warns for real people in real situations,
tack on a

Cc: sta...@vger.kernel.org # v4.3

Cheers,

  -ilia
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

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

[Mario Kleiner]

Sorry for the late reply! Looking into it...
-mario

On 11/06/2015 06:19 PM, Thierry Reding wrote:

Cc += Mario Kleiner, Mario, can you take a look whether this proposed
solution makes sense and fixes the issues you were seeing back when you
posted the patch in commit:

commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
Author: Mario Kleiner 
Date:   Tue May 13 00:42:08 2014 +0200

 drm/nouveau/kms/nv04-nv40: fix pageflip events via special case.

 Cards with nv04 display engine can't reliably use vblank
 counts and timestamps computed via drm_handle_vblank(), as
 the function gets invoked after sending the pageflip events.

 Fix this by defaulting to the old crtcid = -1 fallback path
 on <= NV-50 cards, and only using the precise path on NV-50
 and later.

 Signed-off-by: Mario Kleiner 
 Signed-off-by: Ben Skeggs 
 Cc:  # 3.13+

Do you happen to still have the setup around where you saw this?

Thierry

On Fri, Oct 30, 2015 at 10:55:40PM +0100, 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 
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 
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.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
Cc: Thierry Reding 
Cc: Mario Kleiner 
Cc: Ben Skeggs 
Cc: Ilia Mirkin 
Signed-off-by: Daniel Vetter 
---

Note that due to lack of hw this is completely untested. But I think
it's the right way to fix this.
-Daniel
---
  drivers/gpu/drm/drm_irq.c | 56 ++-
  drivers/gpu/drm/nouveau/nouveau_display.c | 16 -
  include/drm/drmP.h|  4 +++
  3 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 46dbc34b81ba..b3e1f58666a6 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -972,7 +972,8 @@ static void send_vblank_event(struct drm_device *dev,
struct drm_pending_vblank_event *e,
unsigned long seq, struct timeval *now)
  {
-   WARN_ON_SMP(!spin_is_locked(>event_lock));
+   assert_spin_locked(>event_lock);
+
e->event.sequence = seq;
e->event.tv_sec = now->tv_sec;
e->event.tv_usec = now->tv_usec;
@@ -985,6 +986,59 @@ static void send_vblank_event(struct drm_device *dev,
  }

  /**
+ * drm_arm_vblank_event - arm vblanke event after pageflip
+ * @dev: DRM device
+ * @pipe: CRTC index
+ * @e: the event to prepare to send
+ *
+ * A lot of drivers need to generate vblank events for the very next vblank
+ * interrupt. For example when the page flip interrupt happens when the page
+ * flip gets armed, but not when it actually executes within the next vblank
+ * period. This helper function implements exactly the required vblank arming
+ * behaviour.
+ *
+ * Caller must hold event lock. Caller must also hold a vblank reference for 
the
+ * event @e, which will be dropped when the next vblank arrives.
+ *
+ * This is the legacy version of drm_crtc_arm_vblank_event().
+ */
+void drm_arm_vblank_event(struct drm_device *dev, unsigned int pipe,
+ struct drm_pending_vblank_event *e)
+{
+   struct timeval now;
+   unsigned int seq;
+
+   assert_spin_locked(>event_lock);
+
+   e->pipe = pipe;
+   list_add_tail(>base.link, >vblank_event_list);
+}
+EXPORT_SYMBOL(drm_arm_vblank_event);
+
+/**
+ * drm_arm_vblank_event - arm vblanke event after pageflip
+ * @crtc: the source CRTC of the vblank event
+ * @e: the event to send
+ *
+ * A lot of drivers need to generate vblank events for the very next vblank
+ * interrupt. For example when the page flip 

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

[Thierry Reding]

Cc += Mario Kleiner, Mario, can you take a look whether this proposed
solution makes sense and fixes the issues you were seeing back when you
posted the patch in commit:

commit af4870e406126b7ac0ae7c7ce5751f25ebe60f28
Author: Mario Kleiner 
Date:   Tue May 13 00:42:08 2014 +0200

drm/nouveau/kms/nv04-nv40: fix pageflip events via special case.

Cards with nv04 display engine can't reliably use vblank
counts and timestamps computed via drm_handle_vblank(), as
the function gets invoked after sending the pageflip events.

Fix this by defaulting to the old crtcid = -1 fallback path
on <= NV-50 cards, and only using the precise path on NV-50
and later.

Signed-off-by: Mario Kleiner 
Signed-off-by: Ben Skeggs 
Cc:  # 3.13+

Do you happen to still have the setup around where you saw this?

Thierry

On Fri, Oct 30, 2015 at 10:55:40PM +0100, 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 
> 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 
> 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.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
> Cc: Thierry Reding 
> Cc: Mario Kleiner 
> Cc: Ben Skeggs 
> Cc: Ilia Mirkin 
> Signed-off-by: Daniel Vetter 
> ---
> 
> Note that due to lack of hw this is completely untested. But I think
> it's the right way to fix this.
> -Daniel
> ---
>  drivers/gpu/drm/drm_irq.c | 56 
> ++-
>  drivers/gpu/drm/nouveau/nouveau_display.c | 16 -
>  include/drm/drmP.h|  4 +++
>  3 files changed, 66 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 46dbc34b81ba..b3e1f58666a6 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -972,7 +972,8 @@ static void send_vblank_event(struct drm_device *dev,
>   struct drm_pending_vblank_event *e,
>   unsigned long seq, struct timeval *now)
>  {
> - WARN_ON_SMP(!spin_is_locked(>event_lock));
> + assert_spin_locked(>event_lock);
> +
>   e->event.sequence = seq;
>   e->event.tv_sec = now->tv_sec;
>   e->event.tv_usec = now->tv_usec;
> @@ -985,6 +986,59 @@ static void send_vblank_event(struct drm_device *dev,
>  }
>  
>  /**
> + * drm_arm_vblank_event - arm vblanke event after pageflip
> + * @dev: DRM device
> + * @pipe: CRTC index
> + * @e: the event to prepare to send
> + *
> + * A lot of drivers need to generate vblank events for the very next vblank
> + * interrupt. For example when the page flip interrupt happens when the page
> + * flip gets armed, but not when it actually executes within the next vblank
> + * period. This helper function implements exactly the required vblank arming
> + * behaviour.
> + *
> + * Caller must hold event lock. Caller must also hold a vblank reference for 
> the
> + * event @e, which will be dropped when the next vblank arrives.
> + *
> + * This is the legacy version of drm_crtc_arm_vblank_event().
> + */
> +void drm_arm_vblank_event(struct drm_device *dev, unsigned int pipe,
> +   struct drm_pending_vblank_event *e)
> +{
> + struct timeval now;
> + unsigned int seq;
> +
> + assert_spin_locked(>event_lock);
> +
> + e->pipe = pipe;
> + list_add_tail(>base.link, >vblank_event_list);
> +}
> +EXPORT_SYMBOL(drm_arm_vblank_event);
> +
> +/**
> + * drm_arm_vblank_event - arm vblanke event after pageflip
> + * @crtc: the source CRTC of the vblank event
> + * @e: the event to send
> + *
> + * A lot of drivers need to generate vblank events for the 

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

[Michel Dänzer]

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 
> 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 
> 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.


> +/**
> + * drm_arm_vblank_event - arm vblanke event after pageflip
> + * @crtc: the source CRTC of the vblank event

Ye olde vblanke event? ;) (typo, same in the previous comment)

The function name in this comment doesn't match the name of the function
it describes.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau