[PATCH weston v10 13/61] compositor-drm: Introduce fb_last member

[Daniel Stone]

Previously, framebuffers were stored as fb_current and fb_pending.
In this scheme, current was the last buffer that the kernel/hardware had
acknowledged displaying: a framebuffer would be created, set as
fb_pending, and Weston would request the kernel display it. When the
kernel signals that the request was completed and the hardware had made
the buffer current (i.e. page_flip_handler / vblank_handler), we would
unreference the old fb_current, and promote fb_pending to fb_current.

In other words, the view is 'which buffer has turned to light?'.

This patch changes them to a tristate of fb_last, fb_current and
fb_pending, based around the kernel's view of the current state.
fb_pending is used purely as a staging area for request construction;
when the kernel acknowledges a request (e.g. drmModePageFlip returns 0),
the previous buffer is moved to fb_last, and this new buffer to
fb_current. When the kernel signals that the request has completed and
the hardware has made the buffer current, we simply unreference and
clear fb_last, without touching fb_current/fb_pending.

The view here is now 'which state is current in the kernel?'.

As all state changes are incremental on the last state submitted to the
kernel, even if the hardware has not yet been able to make it current,
this simplifies state tracking: all state submissions will always be
relative to fb_current, rather than the previous
(fb_pending) ? fb_pending : fb_current.

The use of fb_pending is strictly bounded between a repaint cycle
(including a grouped set of repaints) beginning, and those repaints
being flushed to the kernel.

fb_current will always be valid between an output's first repaint
flush, and when a disable/destroy request has been processed. For a
plane, it will be valid when a repaint cycle enabling that plane has
been flushed, and when a repaint cycle disabling that plane has been
flushed.

fb_last is only present when a repaint request for the output/plane has
been submitted, but not yet completed by the hardware.

This is the same set of constructs which will be used for storing
plane/output state objects in future patches.

Signed-off-by: Daniel Stone <daniels at collabora.com>
---
 libweston/compositor-drm.c | 61 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 48 insertions(+), 13 deletions(-)

v10: Rewrite commit message. Add comments.

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index e27bc1d4..7004ef9a 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -190,7 +190,15 @@ struct drm_output {
 	uint32_t gbm_format;
 
 	struct weston_plane fb_plane;
-	struct drm_fb *fb_current, *fb_pending;
+
+	/* The last framebuffer submitted to the kernel for this CRTC. */
+	struct drm_fb *fb_current;
+	/* The previously-submitted framebuffer, where the hardware has not
+	 * yet acknowledged display of fb_current. */
+	struct drm_fb *fb_last;
+	/* Framebuffer we are going to submit to the kernel when the current
+	 * repaint is flushed. */
+	struct drm_fb *fb_pending;
 
 	struct drm_fb *dumb[2];
 	pixman_image_t *image[2];
@@ -212,7 +220,6 @@ struct drm_sprite {
 
 	struct weston_plane plane;
 
-	struct drm_fb *fb_current, *fb_pending;
 	struct drm_output *output;
 	struct drm_backend *backend;
 
@@ -220,6 +227,15 @@ struct drm_sprite {
 	uint32_t plane_id;
 	uint32_t count_formats;
 
+	/* The last framebuffer submitted to the kernel for this plane. */
+	struct drm_fb *fb_current;
+	/* The previously-submitted framebuffer, where the hardware has not
+	 * yet acknowledged display of fb_current. */
+	struct drm_fb *fb_last;
+	/* Framebuffer we are going to submit to the kernel when the current
+	 * repaint is flushed. */
+	struct drm_fb *fb_pending;
+
 	int32_t src_x, src_y;
 	uint32_t src_w, src_h;
 	uint32_t dest_x, dest_y;
@@ -836,6 +852,8 @@ drm_output_repaint(struct weston_output *output_base,
 	if (output->disable_pending || output->destroy_pending)
 		return -1;
 
+	assert(!output->fb_last);
+
 	drm_output_render(output, damage);
 	if (!output->fb_pending)
 		return -1;
@@ -861,6 +879,10 @@ drm_output_repaint(struct weston_output *output_base,
 		goto err_pageflip;
 	}
 
+	output->fb_last = output->fb_current;
+	output->fb_current = output->fb_pending;
+	output->fb_pending = NULL;
+
 	output->page_flip_pending = 1;
 
 	if (output->pageflip_timer)
@@ -879,6 +901,8 @@ drm_output_repaint(struct weston_output *output_base,
 			.request.sequence = 1,
 		};
 
+		/* XXX: Set output much earlier, so we don't attempt to place
+		 *      planes on entirely the wrong output. */
 		if ((!s->fb_current && !s->fb_pending) ||
 		    !drm_sprite_crtc_supported(output, s))
 			continue;
@@ -910,6 +934,9 @@ drm_output_repaint(struct weston_output *output_base,
 		}
 
 		s->output = output;
+		s->fb_last = s->fb_current;
+		s->fb_current = s->fb_pending;
+		s->fb_pending = NULL;
 		output->vblank_pending = 1;
 	}
 
@@ -1023,9 +1050,9 @@ vblank_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec,
 	drm_output_update_msc(output, frame);
 	output->vblank_pending = 0;
 
-	drm_fb_unref(s->fb_current);
-	s->fb_current = s->fb_pending;
-	s->fb_pending = NULL;
+	assert(s->fb_last || s->fb_current);
+	drm_fb_unref(s->fb_last);
+	s->fb_last = NULL;
 
 	if (!output->page_flip_pending) {
 		/* Stop the pageflip timer instead of rearming it here */
@@ -1057,9 +1084,8 @@ page_flip_handler(int fd, unsigned int frame,
 	 * we just want to page flip to the current buffer to get an accurate
 	 * timestamp */
 	if (output->page_flip_pending) {
-		drm_fb_unref(output->fb_current);
-		output->fb_current = output->fb_pending;
-		output->fb_pending = NULL;
+		drm_fb_unref(output->fb_last);
+		output->fb_last = NULL;
 	}
 
 	output->page_flip_pending = 0;
@@ -1593,10 +1619,16 @@ drm_output_switch_mode(struct weston_output *output_base, struct weston_mode *mo
 	output->base.current_mode->flags =
 		WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED;
 
-	/* reset rendering stuff. */
+	/* XXX: This drops our current buffer too early, before we've started
+	 *      displaying it. Ideally this should be much more atomic and
+	 *      integrated with a full repaint cycle, rather than doing a
+	 *      sledgehammer modeswitch first, and only later showing new
+	 *      content.
+	 */
 	drm_fb_unref(output->fb_current);
-	drm_fb_unref(output->fb_pending);
-	output->fb_current = output->fb_pending = NULL;
+	assert(!output->fb_last);
+	assert(!output->fb_pending);
+	output->fb_last = output->fb_current = NULL;
 
 	if (b->use_pixman) {
 		drm_output_fini_pixman(output);
@@ -2632,8 +2664,9 @@ drm_output_deinit(struct weston_output *base)
 	struct drm_output *output = to_drm_output(base);
 	struct drm_backend *b = to_drm_backend(base->compositor);
 
-	/* output->fb_pending must not be set here;
+	/* output->fb_last and output->fb_pending must not be set here;
 	 * destroy_pending/disable_pending exist to guarantee exactly this. */
+	assert(!output->fb_last);
 	assert(!output->fb_pending);
 	drm_fb_unref(output->fb_current);
 	output->fb_current = NULL;
@@ -2823,6 +2856,7 @@ create_sprites(struct drm_backend *b)
 
 		sprite->possible_crtcs = plane->possible_crtcs;
 		sprite->plane_id = plane->plane_id;
+		sprite->fb_last = NULL;
 		sprite->fb_current = NULL;
 		sprite->fb_pending = NULL;
 		sprite->backend = b;
@@ -2854,8 +2888,9 @@ destroy_sprites(struct drm_backend *backend)
 				sprite->plane_id,
 				output->crtc_id, 0, 0,
 				0, 0, 0, 0, 0, 0, 0, 0);
+		assert(!sprite->fb_last);
+		assert(!sprite->fb_pending);
 		drm_fb_unref(sprite->fb_current);
-		drm_fb_unref(sprite->fb_pending);
 		weston_plane_release(&sprite->plane);
 		free(sprite);
 	}
-- 
2.12.2