[PATCH] drm/nouveau: Fix pre-nv50 pageflip events
Mario Kleiner
mario.kleiner.de at gmail.com
Sat Nov 7 15:12:42 PST 2015
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 at 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 at gmail.com>
> Signed-off-by: Ben Skeggs <bskeggs at redhat.com>
> Cc: <stable at 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 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.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106431
>> Cc: Thierry Reding <treding at nvidia.com>
>> Cc: Mario Kleiner <mario.kleiner.de at gmail.com>
>> Cc: Ben Skeggs <bskeggs at redhat.com>
>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
>> Signed-off-by: Daniel Vetter <daniel.vetter at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list