From ceb4a1ff9eb1ec2f71cab95bebc9fc3838411d54 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Sat, 19 Oct 2019 14:02:54 +0300 Subject: [PATCH] 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 | 4 +- alloc.c | 132 +++++++++++++++++++---------------------- example/compositor.c | 4 +- example/dynamic.c | 6 +- example/multi-output.c | 8 ++- example/simple.c | 4 +- include/libliftoff.h | 3 +- test/bench.c | 2 +- test/test_alloc.c | 4 +- test/test_dynamic.c | 4 +- test/test_priority.c | 2 +- 11 files changed, 83 insertions(+), 90 deletions(-) diff --git a/README.md b/README.md index 75b1145..75ce13c 100644 --- a/README.md +++ b/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); } diff --git a/alloc.c b/alloc.c index ac4550f..22c9e9d 100644 --- a/alloc.c +++ b/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; } diff --git a/example/compositor.c b/example/compositor.c index cb1b964..66533db 100644 --- a/example/compositor.c +++ b/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; } diff --git a/example/dynamic.c b/example/dynamic.c index cba56a3..e45d96f 100644 --- a/example/dynamic.c +++ b/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; diff --git a/example/multi-output.c b/example/multi-output.c index 186968f..6f6d539 100644 --- a/example/multi-output.c +++ b/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); diff --git a/example/simple.c b/example/simple.c index 187e9d7..e4f27d8 100644 --- a/example/simple.c +++ b/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; } diff --git a/include/libliftoff.h b/include/libliftoff.h index 5122778..b7cb955 100644 --- a/include/libliftoff.h +++ b/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 diff --git a/test/bench.c b/test/bench.c index aadec45..72fd7fa 100644 --- a/test/bench.c +++ b/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); diff --git a/test/test_alloc.c b/test/test_alloc.c index b0994fa..00e6eca 100644 --- a/test/test_alloc.c +++ b/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); diff --git a/test/test_dynamic.c b/test/test_dynamic.c index a4a1143..7532425 100644 --- a/test/test_dynamic.c +++ b/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) { diff --git a/test/test_priority.c b/test/test_priority.c index 8260e3c..43b0279 100644 --- a/test/test_priority.c +++ b/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); }