| Message ID | 1446242140-6766-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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@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. > +/** > + * 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.
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 <mario.kleiner.de@gmail.com>
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 <mario.kleiner.de@gmail.com>
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Cc: <stable@vger.kernel.org> # 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 <mario.kleiner.de@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@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.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>
> 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(&dev->event_lock));
> + assert_spin_locked(&dev->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(&dev->event_lock);
> +
> + e->pipe = pipe;
> + list_add_tail(&e->base.link, &dev->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 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 native KMS version of drm_send_vblank_event().
> + */
> +void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
> + struct drm_pending_vblank_event *e)
> +{
> + drm_arm_vblank_event(crtc->dev, drm_crtc_index(crtc), e);
> +}
> +EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
> +
> +/**
> * drm_send_vblank_event - helper to send vblank event after pageflip
> * @dev: DRM device
> * @pipe: CRTC index
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 184445d4abbf..041e5f84538c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -826,7 +826,6 @@ nouveau_finish_page_flip(struct nouveau_channel *chan,
> struct drm_device *dev = drm->dev;
> struct nouveau_page_flip_state *s;
> unsigned long flags;
> - int crtcid = -1;
>
> spin_lock_irqsave(&dev->event_lock, flags);
>
> @@ -838,16 +837,15 @@ nouveau_finish_page_flip(struct nouveau_channel *chan,
>
> s = list_first_entry(&fctx->flip, struct nouveau_page_flip_state, head);
> if (s->event) {
> - /* Vblank timestamps/counts are only correct on >= NV-50 */
> - if (drm->device.info.family >= NV_DEVICE_INFO_V0_TESLA)
> - crtcid = s->crtc;
> -
> - drm_send_vblank_event(dev, crtcid, s->event);
> + if (drm->device.info.family < NV_DEVICE_INFO_V0_TESLA) {
> + drm_arm_vblank_event(dev, s->crtc, s->event);
> + } else {
> + drm_send_vblank_event(dev, s->crtc, s->event);
> + /* Give up ownership of vblank for page-flipped crtc */
> + drm_vblank_put(dev, s->crtc);
> + }
> }
>
> - /* Give up ownership of vblank for page-flipped crtc */
> - drm_vblank_put(dev, s->crtc);
> -
> list_del(&s->head);
> if (ps)
> *ps = *s;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index eb513341b6ee..4c91ac419d5d 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -948,6 +948,10 @@ extern void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe,
> struct drm_pending_vblank_event *e);
> extern void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
> struct drm_pending_vblank_event *e);
> +void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe,
> + struct drm_pending_vblank_event *e);
> +void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
> + struct drm_pending_vblank_event *e);
> extern bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
> extern bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
> extern int drm_vblank_get(struct drm_device *dev, unsigned int pipe);
> --
> 2.5.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
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 <mario.kleiner.de@gmail.com> > 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 <mario.kleiner.de@gmail.com> > Signed-off-by: Ben Skeggs <bskeggs@redhat.com> > Cc: <stable@vger.kernel.org> # 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 <mario.kleiner.de@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@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. >> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431 >> Cc: Thierry Reding <treding@nvidia.com> >> Cc: Mario Kleiner <mario.kleiner.de@gmail.com> >> Cc: Ben Skeggs <bskeggs@redhat.com> >> Cc: Ilia Mirkin <imirkin@alum.mit.edu> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> >> 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(&dev->event_lock)); >> + assert_spin_locked(&dev->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(&dev->event_lock); >> + >> + e->pipe = pipe; >> + list_add_tail(&e->base.link, &dev->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 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 native KMS version of drm_send_vblank_event(). >> + */ >> +void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, >> + struct drm_pending_vblank_event *e) >> +{ >> + drm_arm_vblank_event(crtc->dev, drm_crtc_index(crtc), e); >> +} >> +EXPORT_SYMBOL(drm_crtc_arm_vblank_event); >> + >> +/** >> * drm_send_vblank_event - helper to send vblank event after pageflip >> * @dev: DRM device >> * @pipe: CRTC index >> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c >> index 184445d4abbf..041e5f84538c 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_display.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c >> @@ -826,7 +826,6 @@ nouveau_finish_page_flip(struct nouveau_channel *chan, >> struct drm_device *dev = drm->dev; >> struct nouveau_page_flip_state *s; >> unsigned long flags; >> - int crtcid = -1; >> >> spin_lock_irqsave(&dev->event_lock, flags); >> >> @@ -838,16 +837,15 @@ nouveau_finish_page_flip(struct nouveau_channel *chan, >> >> s = list_first_entry(&fctx->flip, struct nouveau_page_flip_state, head); >> if (s->event) { >> - /* Vblank timestamps/counts are only correct on >= NV-50 */ >> - if (drm->device.info.family >= NV_DEVICE_INFO_V0_TESLA) >> - crtcid = s->crtc; >> - >> - drm_send_vblank_event(dev, crtcid, s->event); >> + if (drm->device.info.family < NV_DEVICE_INFO_V0_TESLA) { >> + drm_arm_vblank_event(dev, s->crtc, s->event); >> + } else { >> + drm_send_vblank_event(dev, s->crtc, s->event); >> + /* Give up ownership of vblank for page-flipped crtc */ >> + drm_vblank_put(dev, s->crtc); >> + } >> } >> >> - /* Give up ownership of vblank for page-flipped crtc */ >> - drm_vblank_put(dev, s->crtc); >> - >> list_del(&s->head); >> if (ps) >> *ps = *s; >> diff --git a/include/drm/drmP.h b/include/drm/drmP.h >> index eb513341b6ee..4c91ac419d5d 100644 >> --- a/include/drm/drmP.h >> +++ b/include/drm/drmP.h >> @@ -948,6 +948,10 @@ extern void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe, >> struct drm_pending_vblank_event *e); >> extern void drm_crtc_send_vblank_event(struct drm_crtc *crtc, >> struct drm_pending_vblank_event *e); >> +void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe, >> + struct drm_pending_vblank_event *e); >> +void drm_crtc_send_vblank_event(struct drm_crtc *crtc, >> + struct drm_pending_vblank_event *e); >> extern bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe); >> extern bool drm_crtc_handle_vblank(struct drm_crtc *crtc); >> extern int drm_vblank_get(struct drm_device *dev, unsigned int pipe); >> -- >> 2.5.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
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 <mario.kleiner.de@gmail.com> > 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 <mario.kleiner.de@gmail.com> > Signed-off-by: Ben Skeggs <bskeggs@redhat.com> > Cc: <stable@vger.kernel.org> # 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 <mario.kleiner.de@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@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. >> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431 >> Cc: Thierry Reding <treding@nvidia.com> >> Cc: Mario Kleiner <mario.kleiner.de@gmail.com> >> Cc: Ben Skeggs <bskeggs@redhat.com> >> Cc: Ilia Mirkin <imirkin@alum.mit.edu> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> >> 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(&dev->event_lock)); >> + assert_spin_locked(&dev->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 Typo vblanke -> vblank >> + * @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; Dead code: now and seq are not used. >> + assert_spin_locked(&dev->event_lock); >> + >> + e->pipe = pipe; -> Add this missing init: + e->event.sequence = drm_vblank_count(dev, pipe); Otherwise target sequence number for dispatch of the pageflip event by drm_handle_vblank_events() will be zero and we get in trouble for a failing comparison for running vblank count > 2^23 and clients probably hang? >> + list_add_tail(&e->base.link, &dev->vblank_event_list); >> +} >> +EXPORT_SYMBOL(drm_arm_vblank_event); >> + >> +/** >> + * drm_arm_vblank_event - arm vblanke event after pageflip vblanke -> vblank >> + * @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 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 native KMS version of drm_send_vblank_event(). >> + */ >> +void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, >> + struct drm_pending_vblank_event *e) >> +{ >> + drm_arm_vblank_event(crtc->dev, drm_crtc_index(crtc), e); >> +} >> +EXPORT_SYMBOL(drm_crtc_arm_vblank_event); >> + >> +/** >> * drm_send_vblank_event - helper to send vblank event after pageflip >> * @dev: DRM device >> * @pipe: CRTC index >> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c >> index 184445d4abbf..041e5f84538c 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_display.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c >> @@ -826,7 +826,6 @@ nouveau_finish_page_flip(struct nouveau_channel *chan, >> struct drm_device *dev = drm->dev; >> struct nouveau_page_flip_state *s; >> unsigned long flags; >> - int crtcid = -1; >> >> spin_lock_irqsave(&dev->event_lock, flags); >> >> @@ -838,16 +837,15 @@ nouveau_finish_page_flip(struct nouveau_channel *chan, >> >> s = list_first_entry(&fctx->flip, struct nouveau_page_flip_state, head); >> if (s->event) { >> - /* Vblank timestamps/counts are only correct on >= NV-50 */ >> - if (drm->device.info.family >= NV_DEVICE_INFO_V0_TESLA) >> - crtcid = s->crtc; >> - >> - drm_send_vblank_event(dev, crtcid, s->event); >> + if (drm->device.info.family < NV_DEVICE_INFO_V0_TESLA) { >> + drm_arm_vblank_event(dev, s->crtc, s->event); >> + } else { >> + drm_send_vblank_event(dev, s->crtc, s->event); >> + /* Give up ownership of vblank for page-flipped crtc */ >> + drm_vblank_put(dev, s->crtc); >> + } >> } Need a drm_vblank_put() here in a else branch, so refcounting is balanced in case of !s->event. >> >> - /* Give up ownership of vblank for page-flipped crtc */ >> - drm_vblank_put(dev, s->crtc); >> - >> list_del(&s->head); >> if (ps) >> *ps = *s; >> diff --git a/include/drm/drmP.h b/include/drm/drmP.h >> index eb513341b6ee..4c91ac419d5d 100644 >> --- a/include/drm/drmP.h >> +++ b/include/drm/drmP.h >> @@ -948,6 +948,10 @@ extern void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe, >> struct drm_pending_vblank_event *e); >> extern void drm_crtc_send_vblank_event(struct drm_crtc *crtc, >> struct drm_pending_vblank_event *e); >> +void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe, >> + struct drm_pending_vblank_event *e); >> +void drm_crtc_send_vblank_event(struct drm_crtc *crtc, >> + struct drm_pending_vblank_event *e); -> Copy & Paste error, wrong function prototype. >> extern bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe); >> extern bool drm_crtc_handle_vblank(struct drm_crtc *crtc); >> extern int drm_vblank_get(struct drm_device *dev, unsigned int pipe); >> -- >> 2.5.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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@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
On Mon, Nov 16, 2015 at 4:11 PM, Daniel Vetter <daniel@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@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@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
On Tue, Dec 01, 2015 at 04:08:16PM +0100, poma wrote: > On Mon, Nov 16, 2015 at 4:11 PM, Daniel Vetter <daniel@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@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@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
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@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@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@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@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 .... ...
On Wed, Dec 02, 2015 at 06:40:32AM +0100, poma wrote: > On Tue, Dec 1, 2015 at 6:30 PM, Mario Kleiner > <mario.kleiner.de@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 .... Kernel log needed if the drm device isn't there. And this is pretty much impossible, worst case modesetting is functionally busted. -Daniel
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 >> <mario.kleiner.de@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 .... > > 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 (f7000000 e00000) vs hw (d0000000 10000000) [ 1771.699143] checking generic (f7000000 e00000) vs hw (f6000000 2000000) [ 1771.699144] fb: switching to nouveaufb from VESA VGA [ 1771.699271] Console: switching to colour dummy device 80x25 [ 1771.699450] nouveau 0000:02:00.0: NVIDIA G98 (098200a2) [ 1771.813968] nouveau 0000:02:00.0: bios: version 62.98.2c.00.00 [ 1771.834802] nouveau 0000:02:00.0: bios: M0203T not found [ 1771.834819] nouveau 0000:02:00.0: bios: M0203E not matched! [ 1771.834830] nouveau 0000: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 0000:02:00.0: DRM: VRAM: 512 MiB [ 1774.593950] nouveau 0000:02:00.0: DRM: GART: 1048576 MiB [ 1774.593954] nouveau 0000:02:00.0: DRM: TMDS table version 2.0 [ 1774.593956] nouveau 0000:02:00.0: DRM: DCB version 4.0 [ 1774.593959] nouveau 0000:02:00.0: DRM: DCB outp 00: 02000300 00000028 [ 1774.593962] nouveau 0000:02:00.0: DRM: DCB outp 01: 01000302 00020030 [ 1774.593964] nouveau 0000:02:00.0: DRM: DCB outp 02: 04011310 00000028 [ 1774.593966] nouveau 0000:02:00.0: DRM: DCB outp 03: 010223f1 00c0c080 [ 1774.593969] nouveau 0000:02:00.0: DRM: DCB conn 00: 00001030 [ 1774.593971] nouveau 0000:02:00.0: DRM: DCB conn 01: 00000100 [ 1774.593973] nouveau 0000:02:00.0: DRM: DCB conn 02: 00000210 [ 1774.593975] nouveau 0000:02:00.0: DRM: DCB conn 03: 00000211 [ 1774.593977] nouveau 0000:02:00.0: DRM: DCB conn 04: 00000213 [ 1774.595827] nouveau 0000:02:00.0: DRM: failed to create encoder 0/1/0: -19 [ 1774.595832] nouveau 0000: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 0000:02:00.0: DRM: MM: using M2MF for buffer copies [ 1774.690808] nouveau 0000:02:00.0: DRM: allocated 1024x768 fb: 0x50000, bo ffff8800c9a46000 [ 1774.690960] fbcon: nouveaufb (fb0) is primary device [ 1774.746581] Console: switching to colour frame buffer device 128x48 [ 1774.747411] nouveau 0000:02:00.0: fb0: nouveaufb frame buffer device [ 1774.750093] [drm] Initialized nouveau 1.3.1 20120801 for 0000:02:00.0 on minor 0 Tested-by: poma <pomidorabelisima@gmail.com>
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(&dev->event_lock)); + assert_spin_locked(&dev->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(&dev->event_lock); + + e->pipe = pipe; + list_add_tail(&e->base.link, &dev->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 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 native KMS version of drm_send_vblank_event(). + */ +void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, + struct drm_pending_vblank_event *e) +{ + drm_arm_vblank_event(crtc->dev, drm_crtc_index(crtc), e); +} +EXPORT_SYMBOL(drm_crtc_arm_vblank_event); + +/** * drm_send_vblank_event - helper to send vblank event after pageflip * @dev: DRM device * @pipe: CRTC index diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 184445d4abbf..041e5f84538c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -826,7 +826,6 @@ nouveau_finish_page_flip(struct nouveau_channel *chan, struct drm_device *dev = drm->dev; struct nouveau_page_flip_state *s; unsigned long flags; - int crtcid = -1; spin_lock_irqsave(&dev->event_lock, flags); @@ -838,16 +837,15 @@ nouveau_finish_page_flip(struct nouveau_channel *chan, s = list_first_entry(&fctx->flip, struct nouveau_page_flip_state, head); if (s->event) { - /* Vblank timestamps/counts are only correct on >= NV-50 */ - if (drm->device.info.family >= NV_DEVICE_INFO_V0_TESLA) - crtcid = s->crtc; - - drm_send_vblank_event(dev, crtcid, s->event); + if (drm->device.info.family < NV_DEVICE_INFO_V0_TESLA) { + drm_arm_vblank_event(dev, s->crtc, s->event); + } else { + drm_send_vblank_event(dev, s->crtc, s->event); + /* Give up ownership of vblank for page-flipped crtc */ + drm_vblank_put(dev, s->crtc); + } } - /* Give up ownership of vblank for page-flipped crtc */ - drm_vblank_put(dev, s->crtc); - list_del(&s->head); if (ps) *ps = *s; diff --git a/include/drm/drmP.h b/include/drm/drmP.h index eb513341b6ee..4c91ac419d5d 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -948,6 +948,10 @@ extern void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe, struct drm_pending_vblank_event *e); extern void drm_crtc_send_vblank_event(struct drm_crtc *crtc, struct drm_pending_vblank_event *e); +void drm_send_vblank_event(struct drm_device *dev, unsigned int pipe, + struct drm_pending_vblank_event *e); +void drm_crtc_send_vblank_event(struct drm_crtc *crtc, + struct drm_pending_vblank_event *e); extern bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe); extern bool drm_crtc_handle_vblank(struct drm_crtc *crtc); extern int drm_vblank_get(struct drm_device *dev, unsigned int pipe);
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@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@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. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431 Cc: Thierry Reding <treding@nvidia.com> Cc: Mario Kleiner <mario.kleiner.de@gmail.com> Cc: Ben Skeggs <bskeggs@redhat.com> Cc: Ilia Mirkin <imirkin@alum.mit.edu> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- 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(-)