Stop re-using a previous configuration when FB attributes change

If FB attributes change, we might be able to take advantage of a
plane we couldn't  use before.

Closes: https://gitlab.freedesktop.org/emersion/libliftoff/-/issues/71

alloc.c

@@ -517,6 +517,18 @@ apply_current(struct liftoff_device *device, drmModeAtomicReq *req)
 	return 0;
 }
 
+static bool
+fb_info_needs_realloc(const drmModeFB2 *a, const drmModeFB2 *b)
+{
+	if (a->width != b->width || a->height != b->height ||
+	    a->pixel_format != b->pixel_format || a->modifier != b->modifier)
+		return true;
+
+	/* TODO: consider checking pitch and offset? */
+
+	return false;
+}
+
 static bool
 layer_needs_realloc(struct liftoff_layer *layer)
 {
@@ -529,21 +541,30 @@ layer_needs_realloc(struct liftoff_layer *layer)
 
 	for (i = 0; i < layer->props_len; i++) {
 		prop = &layer->props[i];
-		if (prop->value == prop->prev_value) {
-			continue;
-		}
 
 		/* If FB_ID changes from non-zero to zero, we don't need to
 		 * display this layer anymore, so we may be able to re-use its
 		 * plane for another layer. If FB_ID changes from zero to
 		 * non-zero, we might be able to find a plane for this layer.
-		 * If FB_ID changes from non-zero to non-zero, we can try to
+		 * If FB_ID changes from non-zero to non-zero and the FB
-		 * re-use the previous allocation. */
+		 * attributes didn't change, we can try to re-use the previous
+		 * allocation. */
 		if (strcmp(prop->name, "FB_ID") == 0) {
 			if (prop->value == 0 || prop->prev_value == 0) {
 				return true;
 			}
-			/* TODO: check format/modifier is the same? */
+
+			if (fb_info_needs_realloc(&layer->fb_info,
+						  &layer->prev_fb_info)) {
+				return true;
+			}
+
+			continue;
+		}
+
+		/* For all properties except FB_ID, we can skip realloc if the
+		 * value didn't change. */
+		if (prop->value == prop->prev_value) {
 			continue;
 		}
 
@@ -639,6 +660,22 @@ update_layers_priority(struct liftoff_device *device)
 	}
 }
 
+static void
+update_layers_fb_info(struct liftoff_output *output)
+{
+	struct liftoff_layer *layer;
+
+	/* We don't know what the library user did in-between
+	 * liftoff_output_apply() calls. They might've removed the FB and
+	 * re-created a completely different one which happens to have the same
+	 * FB ID. */
+	liftoff_list_for_each(layer, &output->layers, link) {
+		memset(&layer->fb_info, 0, sizeof(layer->fb_info));
+		layer_cache_fb_info(layer);
+		/* TODO: propagate error? */
+	}
+}
+
 static void
 log_reuse(struct liftoff_output *output)
 {
@@ -697,6 +734,7 @@ liftoff_output_apply(struct liftoff_output *output, drmModeAtomicReq *req,
 	device = output->device;
 
 	update_layers_priority(device);
+	update_layers_fb_info(output);
 
 	ret = reuse_previous_alloc(output, req, flags);
 	if (ret == 0) {

include/private.h

@@ -51,6 +51,7 @@ struct liftoff_layer {
 	int current_priority, pending_priority;
 	/* prop added or force_composition changed */
 	bool changed;
+	drmModeFB2 fb_info, prev_fb_info; /* cached FB info */
 };
 
 struct liftoff_layer_property {
@@ -107,6 +108,9 @@ layer_has_fb(struct liftoff_layer *layer);
 bool
 layer_is_visible(struct liftoff_layer *layer);
 
+int
+layer_cache_fb_info(struct liftoff_layer *layer);
+
 int
 plane_apply(struct liftoff_plane *plane, struct liftoff_layer *layer,
 	    drmModeAtomicReq *req);

layer.c

@@ -1,6 +1,7 @@
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
+#include <xf86drm.h>
 #include "private.h"
 
 struct liftoff_layer *
@@ -178,6 +179,7 @@ layer_mark_clean(struct liftoff_layer *layer)
 	size_t i;
 
 	layer->changed = false;
+	layer->prev_fb_info = layer->fb_info;
 
 	for (i = 0; i < layer->props_len; i++) {
 		layer->props[i].prev_value = layer->props[i].value;
@@ -239,3 +241,58 @@ layer_is_visible(struct liftoff_layer *layer)
 		return layer_has_fb(layer);
 	}
 }
+
+int
+layer_cache_fb_info(struct liftoff_layer *layer)
+{
+	struct liftoff_layer_property *fb_id_prop;
+	drmModeFB2 *fb_info;
+	size_t i, j, num_planes;
+	int ret;
+
+	fb_id_prop = layer_get_property(layer, "FB_ID");
+	if (fb_id_prop == NULL || fb_id_prop->value == 0) {
+		memset(&layer->fb_info, 0, sizeof(layer->fb_info));
+		return 0;
+	}
+
+	if (layer->fb_info.fb_id == fb_id_prop->value) {
+		return 0;
+	}
+
+	fb_info = drmModeGetFB2(layer->output->device->drm_fd, fb_id_prop->value);
+	if (fb_info == NULL) {
+		if (errno == EINVAL) {
+			return 0; /* old kernel */
+		}
+		return -errno;
+	}
+
+	/* drmModeGetFB2() always creates new GEM handles -- close these, we
+	 * won't use them and we don't want to leak them */
+	num_planes = sizeof(fb_info->handles) / sizeof(fb_info->handles[0]);
+	for (i = 0; i < num_planes; i++) {
+		if (fb_info->handles[i] == 0) {
+			continue;
+		}
+
+		ret = drmCloseBufferHandle(layer->output->device->drm_fd,
+					   fb_info->handles[i]);
+		if (ret != 0) {
+			liftoff_log_errno(LIFTOFF_ERROR, "drmCloseBufferHandle");
+			continue;
+		}
+
+		/* Make sure we don't double-close a handle */
+		for (j = i + 1; j < num_planes; j++) {
+			if (fb_info->handles[j] == fb_info->handles[i]) {
+				fb_info->handles[j] = 0;
+			}
+		}
+		fb_info->handles[i] = 0;
+	}
+
+	layer->fb_info = *fb_info;
+	drmModeFreeFB2(fb_info);
+	return 0;
+}