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@xxxxxxxxx>
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@xxxxxxxxx>
Signed-off-by: Ben Skeggs <bskeggs@xxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx> # 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@xxxxxxxxx>
> 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@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> Cc: Mario Kleiner <mario.kleiner.de@xxxxxxxxx>
> Cc: Ben Skeggs <bskeggs@xxxxxxxxxx>
> Cc: Ilia Mirkin <imirkin@xxxxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
