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>

3 files changed, 75 insertions, 45 deletions

diff --git a/src/sna/sna.h b/src/sna/sna.h
index 7861110a..a65f652e 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 3f70d536..c3e16b9e 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 efe80387..a2ce808f 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_crtc_vblank(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_crtc_vblank(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_crtc_vblank(info, delta);
 	}
 
 	return true;
@@ -360,6 +362,47 @@ 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) {
+			DBG(("%s: vblank already queued for target_msc=%lld\n",
+			     __FUNCTION__, (long long)msc));
+			return;
+		}
+
+		if ((int64_t)(tmp->target_msc - msc) > 0)
+			break;
+	}
+
+	DBG(("%s: adding keepalive for target_msc=%lld\n",
+	     __FUNCTION__, (long long)msc));
+
+	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_crtc_vblank(info, 1);
+	} else
+		info_free(info);
+}
+
 static int
 sna_present_get_ust_msc(RRCrtcPtr crtc, CARD64 *ust, CARD64 *msc)
 {
@@ -377,34 +420,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 +496,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->devPrivate), link) {
+		if (tmp->target_msc == msc) {
 			uint64_t *events = tmp->event_id;
 
 			if (tmp->n_event_id &&
@@ -692,8 +710,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 +722,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 +770,7 @@ flip(struct sna *sna,
 		return FALSE;
 	}
 
-	info->queued = true;
-	if (info->crtc)
-		sna_crtc_set_vblank(info->crtc);
+	add_to_crtc_vblank(info, 1);
 	return TRUE;
 }