From 5429302e78c9ae6fdd47841782a88eb6d7f54fc4 Mon Sep 17 00:00:00 2001 From: Leandro Ribeiro Date: Tue, 12 Oct 2021 14:48:36 -0300 Subject: [PATCH] backend-drm: add KMS plane's formats to per-surface dma-buf feedback In commit "libweston: add initial dma-buf feedback implementation" we've added initial support to dma-buf feedback, but it was not using the feedback from DRM-backend to add useful per-surface feedback. In this patch we add this. The scanout tranche of the per-surface feedback is based on the union of formats/modifiers of the primary and overlay planes available. These are intersected with the formats/modifiers supported by the renderer device. Also, it's important to mention that the scene can change a lot and we can't predict much. So this patch also adds hysteresis to the dma-buf feedback. We wait a few seconds to be sure that we reached stability before adding or removing the scanout tranche from dma-buf feedback and resending them. This help us to avoid spamming clients and leading to unnecessary buffer reallocations on their end. Here's an example of what we want to avoid: 1. We detect that a view was not placed in a plane only because its format is not supported by the plane, so we add the scanout tranche to the feedback and send the events. 2. A few milliseconds after, the view gets occluded. So now the view can't be placed in a plane anymore. We need to remove the scanout tranche and resend the feedback with formats/modifiers optimal for the renderer device. The client will then reallocate its buffers. 3. A few milliseconds after, the view that was causing the occlusion gets minimized. So we got back to the first situation, in which the format of the view is not compatible with the plane. Then we need to add a scanout tranche and resend the feedback... This patch is based on previous work of Scott Anderson (@ascent). Signed-off-by: Leandro Ribeiro Signed-off-by: Scott Anderson Reviewed-by: Daniel Stone --- libweston/backend-drm/drm-internal.h | 10 +++ libweston/backend-drm/drm.c | 75 ++++++++++++++++ libweston/backend-drm/state-propose.c | 101 +++++++++++++++++++++ libweston/linux-dmabuf.c | 125 +++++++++++++++++++++++++- libweston/linux-dmabuf.h | 32 +++++++ 5 files changed, 340 insertions(+), 3 deletions(-) diff --git a/libweston/backend-drm/drm-internal.h b/libweston/backend-drm/drm-internal.h index 4e697aa1..1d3170c3 100644 --- a/libweston/backend-drm/drm-internal.h +++ b/libweston/backend-drm/drm-internal.h @@ -238,6 +238,16 @@ enum try_view_on_plane_failure_reasons { FAILURE_REASONS_ADD_FB_FAILED = (1 << 3), }; +/** + * We use this to keep track of actions we need to do with the dma-buf feedback + * in order to keep it up-to-date with the info we get from the DRM-backend. + */ +enum actions_needed_dmabuf_feedback { + ACTION_NEEDED_NONE = 0, + ACTION_NEEDED_ADD_SCANOUT_TRANCHE = (1 << 0), + ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE = (1 << 1), +}; + struct drm_backend { struct weston_backend base; struct weston_compositor *compositor; diff --git a/libweston/backend-drm/drm.c b/libweston/backend-drm/drm.c index 480b13c2..5f6d6552 100644 --- a/libweston/backend-drm/drm.c +++ b/libweston/backend-drm/drm.c @@ -1669,6 +1669,65 @@ drm_output_deinit_planes(struct drm_output *output) output->scanout_plane = NULL; } +static struct weston_drm_format_array * +get_scanout_formats(struct drm_backend *b) +{ + struct weston_compositor *ec = b->compositor; + const struct weston_drm_format_array *renderer_formats; + struct weston_drm_format_array *scanout_formats, union_planes_formats; + struct drm_plane *plane; + int ret; + + /* If we got here it means that dma-buf feedback is supported and that + * the renderer has formats/modifiers to expose. */ + assert(ec->renderer->get_supported_formats != NULL); + renderer_formats = ec->renderer->get_supported_formats(ec); + + scanout_formats = zalloc(sizeof(*scanout_formats)); + if (!scanout_formats) { + weston_log("%s: out of memory\n", __func__); + return NULL; + } + + weston_drm_format_array_init(&union_planes_formats); + weston_drm_format_array_init(scanout_formats); + + /* Compute the union of the format/modifiers of the KMS planes */ + wl_list_for_each(plane, &b->plane_list, link) { + /* The scanout formats are used by the dma-buf feedback. But for + * now cursor planes do not support dma-buf buffers, only wl_shm + * buffers. So we skip cursor planes here. */ + if (plane->type == WDRM_PLANE_TYPE_CURSOR) + continue; + + ret = weston_drm_format_array_join(&union_planes_formats, + &plane->formats); + if (ret < 0) + goto err; + } + + /* Compute the intersection between the union of format/modifiers of KMS + * planes and the formats supported by the renderer */ + ret = weston_drm_format_array_replace(scanout_formats, + renderer_formats); + if (ret < 0) + goto err; + + ret = weston_drm_format_array_intersect(scanout_formats, + &union_planes_formats); + if (ret < 0) + goto err; + + weston_drm_format_array_fini(&union_planes_formats); + + return scanout_formats; + +err: + weston_drm_format_array_fini(&union_planes_formats); + weston_drm_format_array_fini(scanout_formats); + return NULL; +} + /** Pick a CRTC and reserve it for the output. * * On failure, the output remains without a CRTC. @@ -2917,6 +2976,7 @@ drm_backend_create(struct weston_compositor *compositor, struct wl_event_loop *loop; const char *seat_id = default_seat; const char *session_seat; + struct weston_drm_format_array *scanout_formats; drmModeRes *res; int ret; @@ -3082,6 +3142,21 @@ drm_backend_create(struct weston_compositor *compositor, if (linux_dmabuf_setup(compositor) < 0) weston_log("Error: initializing dmabuf " "support failed.\n"); + if (compositor->default_dmabuf_feedback) { + /* We were able to create the compositor's default + * dma-buf feedback in the renderer, that means that the + * table was already created and populated with + * renderer's format/modifier pairs. So now we must + * compute the scanout formats indices in the table */ + scanout_formats = get_scanout_formats(b); + if (!scanout_formats) + goto err_udev_monitor; + ret = weston_dmabuf_feedback_format_table_set_scanout_indices(compositor->dmabuf_feedback_format_table, + scanout_formats); + weston_drm_format_array_fini(scanout_formats); + if (ret < 0) + goto err_udev_monitor; + } if (weston_direct_display_setup(compositor) < 0) weston_log("Error: initializing direct-display " "support failed.\n"); diff --git a/libweston/backend-drm/state-propose.c b/libweston/backend-drm/state-propose.c index d0f80e3b..247762f4 100644 --- a/libweston/backend-drm/state-propose.c +++ b/libweston/backend-drm/state-propose.c @@ -649,6 +649,101 @@ drm_output_check_zpos_plane_states(struct drm_output_state *state) } } +static bool +dmabuf_feedback_maybe_update(struct drm_backend *b, struct weston_view *ev, + uint32_t try_view_on_plane_failure_reasons) +{ + struct weston_dmabuf_feedback *dmabuf_feedback = ev->surface->dmabuf_feedback; + struct weston_dmabuf_feedback_tranche *scanout_tranche; + dev_t scanout_dev = b->drm.devnum; + uint32_t scanout_flags = ZWP_LINUX_DMABUF_FEEDBACK_V1_TRANCHE_FLAGS_SCANOUT; + uint32_t action_needed = ACTION_NEEDED_NONE; + struct timespec current_time, delta_time; + const time_t MAX_TIME_SECONDS = 2; + + /* Find out what we need to do with the dma-buf feedback */ + if (try_view_on_plane_failure_reasons & FAILURE_REASONS_FORCE_RENDERER) + action_needed |= ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE; + if (try_view_on_plane_failure_reasons & + (FAILURE_REASONS_ADD_FB_FAILED | + FAILURE_REASONS_FB_FORMAT_INCOMPATIBLE | + FAILURE_REASONS_DMABUF_MODIFIER_INVALID)) + action_needed |= ACTION_NEEDED_ADD_SCANOUT_TRANCHE; + + assert(action_needed != (ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE | + ACTION_NEEDED_ADD_SCANOUT_TRANCHE)); + + /* Look for scanout tranche. If not found, add it but in disabled mode + * (we still don't know if we'll have to send it to clients). This + * simplifies the code. */ + scanout_tranche = + weston_dmabuf_feedback_find_tranche(dmabuf_feedback, scanout_dev, + scanout_flags, SCANOUT_PREF); + if (!scanout_tranche) { + scanout_tranche = + weston_dmabuf_feedback_tranche_create(dmabuf_feedback, + b->compositor->dmabuf_feedback_format_table, + scanout_dev, scanout_flags, + SCANOUT_PREF); + scanout_tranche->active = false; + } + + /* No actions needed, so disarm timer and return */ + if (action_needed == ACTION_NEEDED_NONE || + (action_needed == ACTION_NEEDED_ADD_SCANOUT_TRANCHE && + scanout_tranche->active) || + (action_needed == ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE && + !scanout_tranche->active)) { + dmabuf_feedback->action_needed = ACTION_NEEDED_NONE; + return false; + } + + /* We hit this if: + * + * 1. timer is still off, or + * 2. the action needed when it was set to on does not match the most + * recent needed action we've detected. + * + * So we reset the timestamp, set the timer to on it with the most + * recent needed action, return and leave the timer running. */ + if (dmabuf_feedback->action_needed == ACTION_NEEDED_NONE || + dmabuf_feedback->action_needed != action_needed) { + clock_gettime(CLOCK_MONOTONIC, &dmabuf_feedback->timer); + dmabuf_feedback->action_needed = action_needed; + return false; + /* Timer is already on and the action needed when it was set to on does + * not conflict with the most recent needed action we've detected. If + * more than MAX_TIME_SECONDS has passed, we need to resend the dma-buf + * feedback. Otherwise, return and leave the timer running. */ + } else { + clock_gettime(CLOCK_MONOTONIC, ¤t_time); + delta_time.tv_sec = current_time.tv_sec - + dmabuf_feedback->timer.tv_sec; + if (delta_time.tv_sec < MAX_TIME_SECONDS) + return false; + } + + /* If we got here it means that the timer has triggered, so we have + * pending actions with the dma-buf feedback. So we update and resend + * them. */ + if (action_needed == ACTION_NEEDED_ADD_SCANOUT_TRANCHE) + scanout_tranche->active = true; + else if (action_needed == ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE) + scanout_tranche->active = false; + else + assert(0); + + drm_debug(b, "\t[repaint] Need to update and resend the " + "dma-buf feedback for surface of view %p\n", ev); + weston_dmabuf_feedback_send_all(dmabuf_feedback, + b->compositor->dmabuf_feedback_format_table); + + /* Set the timer to off */ + dmabuf_feedback->action_needed = ACTION_NEEDED_NONE; + + return true; +} + static struct drm_plane_state * drm_output_prepare_plane_view(struct drm_output_state *state, struct weston_view *ev, @@ -1107,6 +1202,12 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data) if (!(ev->output_mask & (1u << output->base.id))) continue; + /* Update dmabuf-feedback if needed */ + if (ev->surface->dmabuf_feedback) + dmabuf_feedback_maybe_update(b, ev, + pnode->try_view_on_plane_failure_reasons); + pnode->try_view_on_plane_failure_reasons = FAILURE_REASONS_NONE; + /* Test whether this buffer can ever go into a plane: * non-shm, or small enough to be a cursor. * diff --git a/libweston/linux-dmabuf.c b/libweston/linux-dmabuf.c index 52307edc..66702a4c 100644 --- a/libweston/linux-dmabuf.c +++ b/libweston/linux-dmabuf.c @@ -35,7 +35,6 @@ #include #include "linux-dmabuf.h" -#include "linux-dmabuf-unstable-v1-server-protocol.h" #include "shared/os-compatibility.h" #include "libweston-internal.h" #include "shared/weston-drm-fourcc.h" @@ -425,6 +424,7 @@ weston_dmabuf_feedback_tranche_create(struct weston_dmabuf_feedback *dmabuf_feed return NULL; } + tranche->active = true; tranche->target_device = target_device; tranche->flags = flags; tranche->preference = preference; @@ -437,8 +437,11 @@ weston_dmabuf_feedback_tranche_create(struct weston_dmabuf_feedback *dmabuf_feed goto err; } } else if (flags & ZWP_LINUX_DMABUF_FEEDBACK_V1_TRANCHE_FLAGS_SCANOUT) { - /* we still don't support scanout tranches */ - assert(0); + if (wl_array_copy(&tranche->formats_indices, + &format_table->scanout_formats_indices) < 0) { + weston_log("%s: out of memory\n", __func__); + goto err; + } } else { weston_log("error: for now we just have renderer and scanout " "tranches, can't create other type of tranche\n"); @@ -523,6 +526,7 @@ weston_dmabuf_feedback_format_table_create(const struct weston_drm_format_array return NULL; } wl_array_init(&format_table->renderer_formats_indices); + wl_array_init(&format_table->scanout_formats_indices); /* Creates formats file table and mmap it */ format_table->size = weston_drm_format_array_count_pairs(renderer_formats) * @@ -566,6 +570,7 @@ WL_EXPORT void weston_dmabuf_feedback_format_table_destroy(struct weston_dmabuf_feedback_format_table *format_table) { wl_array_release(&format_table->renderer_formats_indices); + wl_array_release(&format_table->scanout_formats_indices); munmap(format_table->data, format_table->size); close(format_table->fd); @@ -573,6 +578,73 @@ weston_dmabuf_feedback_format_table_destroy(struct weston_dmabuf_feedback_format free(format_table); } +static int +format_table_get_format_index(struct weston_dmabuf_feedback_format_table *format_table, + uint32_t format, uint64_t modifier, uint16_t *index_out) +{ + uint16_t index; + unsigned int num_elements = format_table->size / sizeof(index); + + for (index = 0; index < num_elements; index++) { + if (format_table->data[index].format == format && + format_table->data[index].modifier == modifier) { + *index_out = index; + return 0; + } + } + + return -1; +} + +/** Set scanout formats indices in the dma-buf feedback format table + * + * The table consists of the formats supported by the renderer. A dma-buf + * feedback scanout tranche consists of the union of the KMS plane's formats + * intersected with the renderer formats. With this function we compute the + * indices of these plane's formats in the table and save them in the + * table->scanout_formats_indices, allowing us to create scanout tranches. + * + * @param format_table The dma-buf feedback format table + * @param scanout_formats The scanout formats + * @return 0 on success, -1 on failure + */ +WL_EXPORT int +weston_dmabuf_feedback_format_table_set_scanout_indices(struct weston_dmabuf_feedback_format_table *format_table, + const struct weston_drm_format_array *scanout_formats) +{ + struct weston_drm_format *fmt; + unsigned int num_modifiers; + const uint64_t *modifiers; + uint16_t index, *index_ptr; + unsigned int i; + int ret; + + wl_array_for_each(fmt, &scanout_formats->arr) { + modifiers = weston_drm_format_get_modifiers(fmt, &num_modifiers); + for (i = 0; i < num_modifiers; i++) { + index_ptr = + wl_array_add(&format_table->scanout_formats_indices, + sizeof(index)); + if (!index_ptr) + goto err; + + ret = format_table_get_format_index(format_table, fmt->format, + modifiers[i], &index); + if (ret < 0) + goto err; + + *index_ptr = index; + } + } + + return 0; + +err: + wl_array_release(&format_table->scanout_formats_indices); + wl_array_init(&format_table->scanout_formats_indices); + return -1; +} + /** Creates dma-buf feedback object * * @param main_device The main device of the dma-buf feedback @@ -617,6 +689,29 @@ weston_dmabuf_feedback_destroy(struct weston_dmabuf_feedback *dmabuf_feedback) free(dmabuf_feedback); } +/** Find tranche in a dma-buf feedback object + * + * @param dmabuf_feedback The dma-buf feedback object where to look for + * @param target_device The target device of the tranche + * @param flags The flags of the tranche + * @param preference The preference of the tranche + * @return The tranche, or NULL if it was not found + */ +WL_EXPORT struct weston_dmabuf_feedback_tranche * +weston_dmabuf_feedback_find_tranche(struct weston_dmabuf_feedback *dmabuf_feedback, + dev_t target_device, uint32_t flags, + enum weston_dmabuf_feedback_tranche_preference preference) +{ + struct weston_dmabuf_feedback_tranche *tranche; + + wl_list_for_each(tranche, &dmabuf_feedback->tranche_list, link) + if (tranche->target_device == target_device && + tranche->flags == flags && tranche->preference == preference) + return tranche; + + return NULL; +} + static void weston_dmabuf_feedback_send(struct weston_dmabuf_feedback *dmabuf_feedback, struct weston_dmabuf_feedback_format_table *format_table, @@ -652,6 +747,9 @@ weston_dmabuf_feedback_send(struct weston_dmabuf_feedback *dmabuf_feedback, /* send events for each tranche */ wl_list_for_each(tranche, &dmabuf_feedback->tranche_list, link) { + if (!tranche->active) + continue; + /* tranche_target_device event */ *dev = tranche->target_device; zwp_linux_dmabuf_feedback_v1_send_tranche_target_device(res, &device); @@ -672,6 +770,27 @@ weston_dmabuf_feedback_send(struct weston_dmabuf_feedback *dmabuf_feedback, wl_array_release(&device); } +/** Sends the feedback events for a dma-buf feedback object + * + * Given a dma-buf feedback object, this will send events to clients that are + * subscribed to it. This is useful for the per-surface dma-buf feedback, which + * is dynamic and can change throughout compositor's life. These changes results + * in the need to resend the feedback events to clients. + * + * @param dmabuf_feedback The weston_dmabuf_feedback object + * @param format_table The dma-buf feedback formats table + */ +WL_EXPORT void +weston_dmabuf_feedback_send_all(struct weston_dmabuf_feedback *dmabuf_feedback, + struct weston_dmabuf_feedback_format_table *format_table) +{ + struct wl_resource *res; + + wl_resource_for_each(res, &dmabuf_feedback->resource_list) + weston_dmabuf_feedback_send(dmabuf_feedback, + format_table, res, false); +} + static void dmabuf_feedback_resource_destroy(struct wl_resource *resource) { diff --git a/libweston/linux-dmabuf.h b/libweston/linux-dmabuf.h index 48960547..7cae93c5 100644 --- a/libweston/linux-dmabuf.h +++ b/libweston/linux-dmabuf.h @@ -27,6 +27,7 @@ #define WESTON_LINUX_DMABUF_H #include +#include "linux-dmabuf-unstable-v1-server-protocol.h" #define MAX_DMABUF_PLANES 4 @@ -98,6 +99,9 @@ struct weston_dmabuf_feedback_format_table { * of formats supported by the renderer, this goes from 0 to the number * of pairs in the table. */ struct wl_array renderer_formats_indices; + /* Indices of the scanout formats (union of KMS plane's supported + * formats intersected with the renderer formats). */ + struct wl_array scanout_formats_indices; }; struct weston_dmabuf_feedback { @@ -113,12 +117,27 @@ struct weston_dmabuf_feedback { /* weston_dmabuf_feedback_tranche::link */ struct wl_list tranche_list; + + /* We use this timer to know if the scene has stabilized and that would + * be useful to resend dma-buf feedback events to clients. Consider the + * timer off when action_needed == ACTION_NEEDED_NONE. See enum + * actions_needed_dmabuf_feedback. */ + struct timespec timer; + uint32_t action_needed; }; struct weston_dmabuf_feedback_tranche { /* weston_dmabuf_feedback::tranche_list */ struct wl_list link; + /* Instead of destroying tranches and reconstructing them when necessary + * (it can be expensive), we have this flag to know if the tranche + * should be advertised or not. This is particularly useful for the + * scanout tranche, as based on the DRM-backend feedback and the current + * scene (which changes a lot during compositor lifetime) we can decide + * to send it or not. */ + bool active; + dev_t target_device; uint32_t flags; enum weston_dmabuf_feedback_tranche_preference preference; @@ -152,12 +171,25 @@ weston_dmabuf_feedback_create(dev_t main_device); void weston_dmabuf_feedback_destroy(struct weston_dmabuf_feedback *dmabuf_feedback); +void +weston_dmabuf_feedback_send_all(struct weston_dmabuf_feedback *dmabuf_feedback, + struct weston_dmabuf_feedback_format_table *format_table); + +struct weston_dmabuf_feedback_tranche * +weston_dmabuf_feedback_find_tranche(struct weston_dmabuf_feedback *dmabuf_feedback, + dev_t target_device, uint32_t flags, + enum weston_dmabuf_feedback_tranche_preference preference); + struct weston_dmabuf_feedback_format_table * weston_dmabuf_feedback_format_table_create(const struct weston_drm_format_array *renderer_formats); void weston_dmabuf_feedback_format_table_destroy(struct weston_dmabuf_feedback_format_table *format_table); +int +weston_dmabuf_feedback_format_table_set_scanout_indices(struct weston_dmabuf_feedback_format_table *format_table, + const struct weston_drm_format_array *scanout_formats); + struct weston_dmabuf_feedback_tranche * weston_dmabuf_feedback_tranche_create(struct weston_dmabuf_feedback *dmabuf_feedback, struct weston_dmabuf_feedback_format_table *format_table,