Replace liftoff_device_apply with liftoff_output_apply

Compositors need to drive multiple connectors, each with its own vblank timings.
For each device, there's one separate rendering loop per output.

It's not possible to call liftoff_device_apply each time we want to submit a new
frame to one output, because this could touch another's output state, submitting
a new frame there in the process. When the other output will submit a new frame,
it'll get EBUSY (can't submit two frames without waiting for vblank).

Closes: https://github.com/emersion/libliftoff/issues/21

README.md

@@ -36,8 +36,8 @@ liftoff_layer_set_property(layer, "FB_ID", fb_id);
 /* Probably setup more properties and more layers */
 
 req = drmModeAtomicAlloc();
-if (!liftoff_device_apply(device, req)) {
-	perror("liftoff_device_apply");
+if (!liftoff_output_apply(output, req)) {
+	perror("liftoff_output_apply");
 	exit(1);
 }
 

alloc.c

@@ -509,23 +509,23 @@ static bool layer_needs_realloc(struct liftoff_layer *layer)
 	return false;
 }
 
-static bool reuse_previous_alloc(struct liftoff_device *device,
+static bool reuse_previous_alloc(struct liftoff_output *output,
 				 drmModeAtomicReq *req)
 {
-	struct liftoff_output *output;
+	struct liftoff_device *device;
 	struct liftoff_layer *layer;
 	int cursor;
 	bool compatible;
 
-	liftoff_list_for_each(output, &device->outputs, link) {
-		if (output->layers_changed) {
-			return false;
-		}
+	device = output->device;
 
-		liftoff_list_for_each(layer, &output->layers, link) {
-			if (layer_needs_realloc(layer)) {
-				return false;
-			}
+	if (output->layers_changed) {
+		return false;
+	}
+
+	liftoff_list_for_each(layer, &output->layers, link) {
+		if (layer_needs_realloc(layer)) {
+			return false;
 		}
 	}
 
@@ -542,17 +542,14 @@ static bool reuse_previous_alloc(struct liftoff_device *device,
 	return true;
 }
 
-static void mark_layers_clean(struct liftoff_device *device)
+static void mark_layers_clean(struct liftoff_output *output)
 {
-	struct liftoff_output *output;
 	struct liftoff_layer *layer;
 
-	liftoff_list_for_each(output, &device->outputs, link) {
-		output->layers_changed = false;
+	output->layers_changed = false;
 
-		liftoff_list_for_each(layer, &output->layers, link) {
-			layer_mark_clean(layer);
-		}
+	liftoff_list_for_each(layer, &output->layers, link) {
+		layer_mark_clean(layer);
 	}
 }
 
@@ -575,9 +572,9 @@ static void update_layers_priority(struct liftoff_device *device)
 	}
 }
 
-bool liftoff_device_apply(struct liftoff_device *device, drmModeAtomicReq *req)
+bool liftoff_output_apply(struct liftoff_output *output, drmModeAtomicReq *req)
 {
-	struct liftoff_output *output;
+	struct liftoff_device *device;
 	struct liftoff_plane *plane;
 	struct liftoff_layer *layer;
 	struct alloc_result result;
@@ -585,16 +582,18 @@ bool liftoff_device_apply(struct liftoff_device *device, drmModeAtomicReq *req)
 	size_t i;
 	bool compatible;
 
+	device = output->device;
+
 	update_layers_priority(device);
 
-	if (reuse_previous_alloc(device, req)) {
+	if (reuse_previous_alloc(output, req)) {
 		liftoff_log(LIFTOFF_DEBUG, "Re-using previous plane allocation");
 		return true;
 	}
 
 	/* Unset all existing plane and layer mappings. */
 	liftoff_list_for_each(plane, &device->planes, link) {
-		if (plane->layer != NULL) {
+		if (plane->layer != NULL && plane->layer->output == output) {
 			plane->layer->plane = NULL;
 			plane->layer = NULL;
 		}
@@ -623,66 +622,59 @@ bool liftoff_device_apply(struct liftoff_device *device, drmModeAtomicReq *req)
 		return false;
 	}
 
-	/* TODO: maybe start by allocating the primary plane on each output to
-	 * make sure we can display at least something without hitting bandwidth
-	 * issues? Also: be fair when mapping planes to outputs, don't give all
-	 * planes to a single output. Also: don't treat each output separately,
-	 * allocate planes for all outputs at once. */
-	liftoff_list_for_each(output, &device->outputs, link) {
-		/* For each plane, try to find a layer. Don't do it the other
-		 * way around (ie. for each layer, try to find a plane) because
-		 * some drivers want user-space to enable the primary plane
-		 * before any other plane. */
+	/* For each plane, try to find a layer. Don't do it the other
+	 * way around (ie. for each layer, try to find a plane) because
+	 * some drivers want user-space to enable the primary plane
+	 * before any other plane. */
 
-		result.best_score = -1;
-		memset(result.best, 0, result.planes_len * sizeof(*result.best));
-		result.has_composition_layer = output->composition_layer != NULL;
-		result.non_composition_layers_len =
-			liftoff_list_length(&output->layers);
-		if (output->composition_layer != NULL) {
-			result.non_composition_layers_len--;
-		}
-		step.plane_link = device->planes.next;
-		step.plane_idx = 0;
-		step.score = 0;
-		step.last_layer_zpos = INT_MAX;
-		step.composited = false;
-		if (!output_choose_layers(output, &result, &step)) {
-			return false;
+	result.best_score = -1;
+	memset(result.best, 0, result.planes_len * sizeof(*result.best));
+	result.has_composition_layer = output->composition_layer != NULL;
+	result.non_composition_layers_len =
+		liftoff_list_length(&output->layers);
+	if (output->composition_layer != NULL) {
+		result.non_composition_layers_len--;
+	}
+	step.plane_link = device->planes.next;
+	step.plane_idx = 0;
+	step.score = 0;
+	step.last_layer_zpos = INT_MAX;
+	step.composited = false;
+	if (!output_choose_layers(output, &result, &step)) {
+		return false;
+	}
+
+	liftoff_log(LIFTOFF_DEBUG,
+		    "Found plane allocation for output %p with "
+		    "score=%d", (void *)output, result.best_score);
+
+	/* Apply the best allocation */
+	i = 0;
+	liftoff_list_for_each(plane, &device->planes, link) {
+		layer = result.best[i];
+		i++;
+		if (layer == NULL) {
+			continue;
 		}
 
 		liftoff_log(LIFTOFF_DEBUG,
-			    "Found plane allocation for output %p with "
-			    "score=%d", (void *)output, result.best_score);
+			    "Assigning layer %p to plane %"PRIu32,
+			    (void *)layer, plane->id);
 
-		/* Apply the best allocation */
-		i = 0;
-		liftoff_list_for_each(plane, &device->planes, link) {
-			layer = result.best[i];
-			i++;
-			if (layer == NULL) {
-				continue;
-			}
+		assert(plane->layer == NULL);
+		assert(layer->plane == NULL);
+		plane->layer = layer;
+		layer->plane = plane;
+	}
 
-			liftoff_log(LIFTOFF_DEBUG,
-				    "Assigning layer %p to plane %"PRIu32,
-				    (void *)layer, plane->id);
-
-			assert(plane->layer == NULL);
-			assert(layer->plane == NULL);
-			plane->layer = layer;
-			layer->plane = plane;
-		}
-
-		if (!apply_current(device, req)) {
-			return false;
-		}
+	if (!apply_current(device, req)) {
+		return false;
 	}
 
 	free(step.alloc);
 	free(result.best);
 
-	mark_layers_clean(device);
+	mark_layers_clean(output);
 
 	return true;
 }

example/compositor.c

@@ -193,8 +193,8 @@ int main(int argc, char *argv[])
 	liftoff_output_set_composition_layer(output, composition_layer);
 
 	req = drmModeAtomicAlloc();
-	if (!liftoff_device_apply(device, req)) {
-		perror("liftoff_device_commit");
+	if (!liftoff_output_apply(output, req)) {
+		perror("liftoff_output_apply");
 		return 1;
 	}
 

example/dynamic.c

@@ -29,6 +29,7 @@ struct example_layer {
 
 static int drm_fd = -1;
 static struct liftoff_device *device = NULL;
+static struct liftoff_output *output = NULL;
 static struct example_layer layers[LAYERS_LEN] = {0};
 static size_t active_layer_idx = 2;
 
@@ -107,8 +108,8 @@ static bool draw(void)
 	draw_layer(drm_fd, active_layer);
 
 	req = drmModeAtomicAlloc();
-	if (!liftoff_device_apply(device, req)) {
-		perror("liftoff_device_commit");
+	if (!liftoff_output_apply(output, req)) {
+		perror("liftoff_output_apply");
 		return false;
 	}
 
@@ -140,7 +141,6 @@ int main(int argc, char *argv[])
 	drmModeRes *drm_res;
 	drmModeCrtc *crtc;
 	drmModeConnector *connector;
-	struct liftoff_output *output;
 	size_t i;
 	int ret;
 

example/multi-output.c

@@ -154,9 +154,11 @@ int main(int argc, char *argv[])
 	}
 
 	req = drmModeAtomicAlloc();
-	if (!liftoff_device_apply(device, req)) {
-		perror("liftoff_device_commit");
-		return 1;
+	for (i = 0; i < outputs_len; i++) {
+		if (!liftoff_output_apply(outputs[i], req)) {
+			perror("liftoff_output_apply");
+			return 1;
+		}
 	}
 
 	ret = drmModeAtomicCommit(drm_fd, req, DRM_MODE_ATOMIC_NONBLOCK, NULL);

example/simple.c

@@ -128,8 +128,8 @@ int main(int argc, char *argv[])
 	}
 
 	req = drmModeAtomicAlloc();
-	if (!liftoff_device_apply(device, req)) {
-		perror("liftoff_device_commit");
+	if (!liftoff_output_apply(output, req)) {
+		perror("liftoff_output_apply");
 		return 1;
 	}
 

include/libliftoff.h

@@ -22,8 +22,7 @@ void liftoff_device_destroy(struct liftoff_device *device);
  * Callers are expected to commit `req` afterwards and can read the layer to
  * plane mapping with `liftoff_layer_get_plane_id`.
  */
-bool liftoff_device_apply(struct liftoff_device *device,
-			   drmModeAtomicReq *req);
+bool liftoff_output_apply(struct liftoff_output *output, drmModeAtomicReq *req);
 
 /**
  * Make the device manage a CRTC's planes. The returned output allows callers

test/bench.c

@@ -99,7 +99,7 @@ int main(int argc, char *argv[])
 	clock_gettime(CLOCK_MONOTONIC, &start);
 
 	req = drmModeAtomicAlloc();
-	ok = liftoff_device_apply(device, req);
+	ok = liftoff_output_apply(output, req);
 	assert(ok);
 	drmModeAtomicFree(req);
 

test/test_alloc.c

@@ -596,7 +596,7 @@ static void run_test(struct test_layer *test_layers)
 	}
 
 	req = drmModeAtomicAlloc();
-	ok = liftoff_device_apply(device, req);
+	ok = liftoff_output_apply(output, req);
 	assert(ok);
 	drmModeAtomicFree(req);
 
@@ -657,7 +657,7 @@ static void test_basic(void)
 	liftoff_mock_plane_add_compatible_layer(mock_plane, layer);
 
 	req = drmModeAtomicAlloc();
-	ok = liftoff_device_apply(device, req);
+	ok = liftoff_output_apply(output, req);
 	assert(ok);
 	assert(liftoff_mock_plane_get_layer(mock_plane, req) == layer);
 	drmModeAtomicFree(req);

test/test_dynamic.c

@@ -63,7 +63,7 @@ int main(int argc, char *argv[]) {
 	liftoff_mock_plane_add_compatible_layer(mock_plane, layer);
 
 	req = drmModeAtomicAlloc();
-	ok = liftoff_device_apply(device, req);
+	ok = liftoff_output_apply(output, req);
 	assert(ok);
 	assert(liftoff_mock_plane_get_layer(mock_plane, req) == layer);
 	drmModeAtomicFree(req);
@@ -96,7 +96,7 @@ int main(int argc, char *argv[]) {
 		return 1;
 	}
 
-	ok = liftoff_device_apply(device, req);
+	ok = liftoff_output_apply(output, req);
 	assert(ok);
 	assert(liftoff_mock_plane_get_layer(mock_plane, req) == layer);
 	if (want_reuse_prev_alloc) {

test/test_priority.c

@@ -72,7 +72,7 @@ int main(int argc, char *argv[]) {
 
 			liftoff_layer_set_property(layer, "FB_ID", fbs[j % 2]);
 
-			ok = liftoff_device_apply(device, req);
+			ok = liftoff_output_apply(output, req);
 			assert(ok);
 		}