Attachment 134743 Details for Bug 103025 – Queue vblank keepalive

[patch] Queue vblank keepalive

0001-sna-present-Queue-the-keepalive-vblank.patch (text/plain), 8.04 KB, created by Chris Wilson on 2017-10-08 11:15:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2017-10-08 11:15:09 UTC

Size: 8.04 KB

patch

obsolete

>From eb1eefb600f56ee6d772e7c65e4ac8c4b085393d Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>Date: Sat, 7 Oct 2017 16:58:14 +0100
>Subject: [PATCH xf86-video-intel] sna/present: Queue the keepalive vblank
>
>Insert the keepalive vblank into the sorted list of msc carefully. This
>way we can discard redundant keepalives - as we don't want to queue a
>second event for the same vblank needlessly.
>
>Reported-by: Adric Blake <promarbler14@gmail.com>
>References: https://bugs.freedesktop.org/show_bug.cgi?id=103025#c13
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> src/sna/sna.h         |  11 +++++-
> src/sna/sna_display.c |   2 +
> src/sna/sna_present.c | 104 +++++++++++++++++++++++++++++---------------------
> 3 files changed, 72 insertions(+), 45 deletions(-)
>
>diff --git a/src/sna/sna.h b/src/sna/sna.h
>index 7861110..a65f652 100644
>--- a/src/sna/sna.h
>+++ b/src/sna/sna.h
>@@ -633,7 +633,7 @@ extern bool sna_crtc_set_sprite_rotation(xf86CrtcPtr crtc, unsigned idx, uint32_
> extern uint32_t sna_crtc_to_sprite(xf86CrtcPtr crtc, unsigned idx);
> extern bool sna_crtc_is_transformed(xf86CrtcPtr crtc);
> 
>-#define CRTC_VBLANK 0x3
>+#define CRTC_VBLANK 0x7
> #define CRTC_ON 0x80000000
> 
> uint32_t sna_crtc_id(xf86CrtcPtr crtc);
>@@ -645,6 +645,11 @@ static inline unsigned long *sna_crtc_flags(xf86CrtcPtr crtc)
> 	return flags;
> }
> 
>+static inline struct list *sna_crtc_vblank_queue(xf86CrtcPtr crtc)
>+{
>+	return (struct list *)(sna_crtc_flags(crtc) + 1);
>+}
>+
> static inline unsigned sna_crtc_pipe(xf86CrtcPtr crtc)
> {
> 	return *sna_crtc_flags(crtc) >> 8 & 0xff;
>@@ -657,12 +662,14 @@ static inline bool sna_crtc_is_on(xf86CrtcPtr crtc)
> 
> static inline void sna_crtc_set_vblank(xf86CrtcPtr crtc)
> {
>-	assert((*sna_crtc_flags(crtc) & CRTC_VBLANK) < 3);
>+	DBG(("%s: current vblank count: %d\n", __FUNCTION__, *sna_crtc_flags(crtc) & CRTC_VBLANK));
>+	assert((*sna_crtc_flags(crtc) & CRTC_VBLANK) < CRTC_VBLANK);
> 	++*sna_crtc_flags(crtc);
> }
> 
> static inline void sna_crtc_clear_vblank(xf86CrtcPtr crtc)
> {
>+	DBG(("%s: current vblank count: %d\n", __FUNCTION__, *sna_crtc_flags(crtc) & CRTC_VBLANK));
> 	assert(*sna_crtc_flags(crtc) & CRTC_VBLANK);
> 	--*sna_crtc_flags(crtc);
> }
>diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
>index 3f70d53..c3e16b9 100644
>--- a/src/sna/sna_display.c
>+++ b/src/sna/sna_display.c
>@@ -193,6 +193,7 @@ struct sna_cursor {
> 
> struct sna_crtc {
> 	unsigned long flags;
>+	struct list vblank_queue;
> 	uint32_t id;
> 	xf86CrtcPtr base;
> 	struct drm_mode_modeinfo kmode;
>@@ -3522,6 +3523,7 @@ sna_crtc_add(ScrnInfoPtr scrn, unsigned id)
> 	if (sna_crtc == NULL)
> 		return false;
> 
>+	list_init(&sna_crtc->vblank_queue);
> 	sna_crtc->id = id;
> 
> 	VG_CLEAR(get_pipe);
>diff --git a/src/sna/sna_present.c b/src/sna/sna_present.c
>index efe8038..05149a2 100644
>--- a/src/sna/sna_present.c
>+++ b/src/sna/sna_present.c
>@@ -197,6 +197,16 @@ static uint32_t msc_to_delay(xf86CrtcPtr crtc, uint64_t target)
> 	return MIN(delay, INT32_MAX);
> }
> 
>+static void add_to_vblank_queue(struct sna_present_event *info,
>+				int delta)
>+{
>+	info->queued = true;
>+	if (delta == 1 && info->crtc) {
>+		sna_crtc_set_vblank(info->crtc);
>+		info->crtc = mark_crtc(info->crtc);
>+	}
>+}
>+
> static CARD32 sna_fake_vblank_handler(OsTimerPtr timer, CARD32 now, void *data)
> {
> 	struct sna_present_event *info = data;
>@@ -225,11 +235,7 @@ static CARD32 sna_fake_vblank_handler(OsTimerPtr timer, CARD32 now, void *data)
> 				vbl.request.signal = (uintptr_t)MARK_PRESENT(info);
> 				if (sna_wait_vblank(info->sna, &vbl, sna_crtc_pipe(info->crtc)) == 0) {
> 					DBG(("%s: scheduled new vblank event for %lld\n", __FUNCTION__, (long long)info->target_msc));
>-					info->queued = true;
>-					if (delta == 1) {
>-						sna_crtc_set_vblank(info->crtc);
>-						info->crtc = mark_crtc(info->crtc);
>-					}
>+					add_to_vblank_queue(info, delta);
> 					free(timer);
> 					return 0;
> 				}
>@@ -326,11 +332,7 @@ static bool sna_present_queue(struct sna_present_event *info,
> 		if (!sna_fake_vblank(info))
> 			return false;
> 	} else {
>-		info->queued = true;
>-		if (delta == 1) {
>-			sna_crtc_set_vblank(info->crtc);
>-			info->crtc = mark_crtc(info->crtc);
>-		}
>+		add_to_vblank_queue(info, delta);
> 	}
> 
> 	return true;
>@@ -360,6 +362,44 @@ sna_present_get_crtc(WindowPtr window)
> 	return NULL;
> }
> 
>+static void add_keepalive(struct sna *sna, xf86CrtcPtr crtc, uint64_t msc)
>+{
>+	struct sna_present_event *info, *tmp;
>+	union drm_wait_vblank vbl;
>+
>+	list_for_each_entry(tmp, sna_crtc_vblank_queue(crtc), link) {
>+		if (tmp->target_msc == msc)
>+			return;
>+
>+		if ((int64_t)(tmp->target_msc - msc) > 0) {
>+			DBG(("%s: previous target_msc=%lld invalid for coalescing\n",
>+			     __FUNCTION__, (long long)tmp->target_msc));
>+			break;
>+		}
>+	}
>+
>+	info = info_alloc(sna);
>+	if (!info)
>+		return;
>+
>+	info->crtc = crtc;
>+	info->sna = sna;
>+	info->target_msc = msc;
>+	info->event_id = (uint64_t *)(info + 1);
>+	info->n_event_id = 0;
>+
>+	VG_CLEAR(vbl);
>+	vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
>+	vbl.request.sequence = msc;
>+	vbl.request.signal = (uintptr_t)MARK_PRESENT(info);
>+
>+	if (sna_wait_vblank(info->sna, &vbl, sna_crtc_pipe(info->crtc)) == 0) {
>+		list_add_tail(&info->link, &tmp->link);
>+		add_to_vblank_queue(info, 1);
>+	} else
>+		info_free(info);
>+}
>+
> static int
> sna_present_get_ust_msc(RRCrtcPtr crtc, CARD64 *ust, CARD64 *msc)
> {
>@@ -377,34 +417,10 @@ sna_present_get_ust_msc(RRCrtcPtr crtc, CARD64 *ust, CARD64 *msc)
> 	vbl.request.type = DRM_VBLANK_RELATIVE;
> 	vbl.request.sequence = 0;
> 	if (sna_wait_vblank(sna, &vbl, sna_crtc_pipe(crtc->devPrivate)) == 0) {
>-		struct sna_present_event *info;
>-
> 		*ust = ust64(vbl.reply.tval_sec, vbl.reply.tval_usec);
> 		*msc = sna_crtc_record_vblank(crtc->devPrivate, &vbl);
> 
>-		info = info_alloc(sna);
>-		if (info) {
>-			info->crtc = crtc->devPrivate;
>-			info->sna = sna;
>-			info->target_msc = *msc + 1;
>-			info->event_id = (uint64_t *)(info + 1);
>-			info->n_event_id = 0;
>-
>-			vbl.request.type =
>-				DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
>-			vbl.request.sequence = info->target_msc;
>-			vbl.request.signal = (uintptr_t)MARK_PRESENT(info);
>-
>-			if (sna_wait_vblank(info->sna, &vbl,
>-					    sna_crtc_pipe(info->crtc)) == 0) {
>-				list_add(&info->link,
>-					 &sna->present.vblank_queue);
>-				info->queued = true;
>-				sna_crtc_set_vblank(info->crtc);
>-				info->crtc = mark_crtc(info->crtc);
>-			} else
>-				info_free(info);
>-		}
>+		add_keepalive(sna, crtc->devPrivate, *msc + 1);
> 	} else {
> 		const struct ust_msc *swap;
> last:
>@@ -477,9 +493,8 @@ sna_present_queue_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
> 	if (warn_unless(msc - swap->msc < 1ull<<31))
> 		return BadValue;
> 
>-	list_for_each_entry(tmp, &sna->present.vblank_queue, link) {
>-		if (tmp->target_msc == msc &&
>-		    unmask_crtc(tmp->crtc) == crtc->devPrivate) {
>+	list_for_each_entry(tmp, sna_crtc_vblank_queue(crtc), link) {
>+		if (tmp->target_msc == msc) {
> 			uint64_t *events = tmp->event_id;
> 
> 			if (tmp->n_event_id &&
>@@ -692,8 +707,10 @@ present_flip_handler(struct drm_event_vblank *event, void *data)
> 		swap.tv_sec = event->tv_sec;
> 		swap.tv_usec = event->tv_usec;
> 		swap.msc = event->sequence;
>-	} else
>+	} else {
>+		info->crtc = unmask_crtc(info->crtc);
> 		swap = *sna_crtc_last_swap(info->crtc);
>+	}
> 
> 	DBG(("%s: pipe=%d, tv=%d.%06d msc=%lld (target %lld), event=%lld complete%s\n", __FUNCTION__,
> 	     info->crtc ? sna_crtc_pipe(info->crtc) : -1,
>@@ -702,8 +719,11 @@ present_flip_handler(struct drm_event_vblank *event, void *data)
> 	     (long long)info->event_id[0],
> 	     info->target_msc && info->target_msc == swap.msc ? "" : ": MISS"));
> 	present_event_notify(info->event_id[0], swap_ust(&swap), swap.msc);
>-	if (info->crtc)
>+	if (info->crtc) {
> 		sna_crtc_clear_vblank(info->crtc);
>+		if (!sna_crtc_has_vblank(info->crtc))
>+			add_keepalive(info->sna, info->crtc, swap.msc + 1);
>+	}
> 
> 	if (info->sna->present.unflip) {
> 		DBG(("%s: executing queued unflip (event=%lld)\n", __FUNCTION__, (long long)info->sna->present.unflip));
>@@ -747,9 +767,7 @@ flip(struct sna *sna,
> 		return FALSE;
> 	}
> 
>-	info->queued = true;
>-	if (info->crtc)
>-		sna_crtc_set_vblank(info->crtc);
>+	add_to_vblank_queue(info, 1);
> 	return TRUE;
> }
> 
>-- 
>2.7.4
>

Actions: View | Splinter Review

Attachments on bug 103025: 134543 | 134705 | 134706 | 134707 | 134724 | 134725 | 134731 | 134732 | 134733 | 134743 | 134744 | 134745 | 136074 | 136075 | 136126 | 136129 | 136142 | 136146 | 136147 | 136174 | 136232 | 136235 | 137781 | 140636 | 140899 | 144828 | 144829 | 144830

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.