Daniel Vetter
2015-Dec-01 15:55 UTC
[Nouveau] [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-21Ben Skeggs asleep probably. Dave, can you pls pick this up? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Mario Kleiner
2015-Dec-01 17:30 UTC
[Nouveau] [PATCH] drm/nouveau: Fix pre-nv50 pageflip events
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 .... ...