From 02b1cfe08fc3f441cc6b6b24b4fc9fb56e9d815c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 10 Jun 2022 19:51:56 +0200 Subject: [PATCH] onscreen/native: Handle unexpected scanout failures async We test direct client buffer scanout using a TEST_ONLY commit on atomic, and with various conditions in non-atomic, but if we end up failing to actually commit despite this, handle the fallout asynchronously. What this means is that we'll reschedule a new frame immediately. For this to work, the same scanout buffer needs to be avoided for the same CRTC. This is done by using the newly added signal on the CoglScanout object to let the MetaWaylandBuffer object mark the current buffer as non-working for the onsrceen that it failed on. This allows to re-try buffers on the same onscreen when new ones are attached. This queues a full damage, since we consumed the qeued redraw rect. The redraw rect wasn't lost - it was accumulated to make sure the whole primary plane was redrawed according to the damage region, whenever we would end up no longer doing direct scanout, but this accumulation only works when we're not intentionally stopping to scanout. For now, lets just damage the whole view, it's just an graceful fallback in response to an unexpected error anyway. Part-of: --- .../native/meta-kms-impl-device-atomic.c | 19 ++--- .../native/meta-kms-impl-device-simple.c | 12 +-- src/backends/native/meta-kms-types.h | 3 +- src/backends/native/meta-kms-update-private.h | 2 - src/backends/native/meta-kms-update.c | 22 ------ src/backends/native/meta-kms-update.h | 1 - src/backends/native/meta-kms.c | 23 +----- src/backends/native/meta-onscreen-native.c | 72 +++++++++++------- src/wayland/meta-wayland-buffer.c | 74 ++++++++++++++++--- src/wayland/meta-wayland-buffer.h | 2 + 10 files changed, 126 insertions(+), 104 deletions(-) diff --git a/src/backends/native/meta-kms-impl-device-atomic.c b/src/backends/native/meta-kms-impl-device-atomic.c index 8e5b7207a..456d742ca 100644 --- a/src/backends/native/meta-kms-impl-device-atomic.c +++ b/src/backends/native/meta-kms-impl-device-atomic.c @@ -1046,17 +1046,14 @@ meta_kms_impl_device_atomic_process_update (MetaKmsImplDevice *impl_device, err: meta_topic (META_DEBUG_KMS, "[atomic] KMS update failed: %s", error->message); - if (!(flags & META_KMS_UPDATE_FLAG_PRESERVE_ON_ERROR)) - { - process_entries (impl_device, - update, - req, - blob_ids, - meta_kms_update_get_page_flip_listeners (update), - error, - discard_page_flip_listener, - NULL); - } + process_entries (impl_device, + update, + req, + blob_ids, + meta_kms_update_get_page_flip_listeners (update), + error, + discard_page_flip_listener, + NULL); if (req) drmModeAtomicFree (req); diff --git a/src/backends/native/meta-kms-impl-device-simple.c b/src/backends/native/meta-kms-impl-device-simple.c index 199e39cae..7aa39cce5 100644 --- a/src/backends/native/meta-kms-impl-device-simple.c +++ b/src/backends/native/meta-kms-impl-device-simple.c @@ -1166,8 +1166,7 @@ maybe_dispatch_page_flips (MetaKmsImplDevice *impl_device, *failed_planes = g_list_prepend (*failed_planes, plane_feedback); } - if (!(flags & META_KMS_UPDATE_FLAG_PRESERVE_ON_ERROR)) - meta_kms_page_flip_data_discard_in_impl (page_flip_data, *error); + meta_kms_page_flip_data_discard_in_impl (page_flip_data, *error); goto err; } @@ -1176,14 +1175,11 @@ maybe_dispatch_page_flips (MetaKmsImplDevice *impl_device, return TRUE; err: - if (!(flags & META_KMS_UPDATE_FLAG_PRESERVE_ON_ERROR)) + for (l = page_flip_datas; l; l = l->next) { - for (l = page_flip_datas; l; l = l->next) - { - MetaKmsPageFlipData *page_flip_data = l->data; + MetaKmsPageFlipData *page_flip_data = l->data; - meta_kms_page_flip_data_discard_in_impl (page_flip_data, *error); - } + meta_kms_page_flip_data_discard_in_impl (page_flip_data, *error); } g_list_free (page_flip_datas); diff --git a/src/backends/native/meta-kms-types.h b/src/backends/native/meta-kms-types.h index 5762babc7..8e035ac1a 100644 --- a/src/backends/native/meta-kms-types.h +++ b/src/backends/native/meta-kms-types.h @@ -79,8 +79,7 @@ typedef enum _MetaKmsResourceChanges typedef enum _MetaKmsUpdateFlag { META_KMS_UPDATE_FLAG_NONE = 0, - META_KMS_UPDATE_FLAG_PRESERVE_ON_ERROR = 1 << 0, - META_KMS_UPDATE_FLAG_TEST_ONLY = 1 << 1, + META_KMS_UPDATE_FLAG_TEST_ONLY = 1 << 0, } MetaKmsUpdateFlag; typedef enum _MetaKmsPlaneType MetaKmsPlaneType; diff --git a/src/backends/native/meta-kms-update-private.h b/src/backends/native/meta-kms-update-private.h index b45f0cbec..343628a49 100644 --- a/src/backends/native/meta-kms-update-private.h +++ b/src/backends/native/meta-kms-update-private.h @@ -170,8 +170,6 @@ GList * meta_kms_update_get_mode_sets (MetaKmsUpdate *update); META_EXPORT_TEST GList * meta_kms_update_get_page_flip_listeners (MetaKmsUpdate *update); -void meta_kms_update_drop_defunct_page_flip_listeners (MetaKmsUpdate *update); - META_EXPORT_TEST GList * meta_kms_update_get_connector_updates (MetaKmsUpdate *update); diff --git a/src/backends/native/meta-kms-update.c b/src/backends/native/meta-kms-update.c index 2c8cdad1d..4d0bda8f5 100644 --- a/src/backends/native/meta-kms-update.c +++ b/src/backends/native/meta-kms-update.c @@ -462,28 +462,6 @@ meta_kms_update_add_page_flip_listener (MetaKmsUpdate *upd listener); } -void -meta_kms_update_drop_defunct_page_flip_listeners (MetaKmsUpdate *update) -{ - GList *l; - - l = update->page_flip_listeners; - while (l) - { - MetaKmsPageFlipListener *listener = l->data; - GList *l_next = l->next; - - if (listener->flags & META_KMS_PAGE_FLIP_LISTENER_FLAG_DROP_ON_ERROR) - { - meta_kms_page_flip_listener_unref (listener); - update->page_flip_listeners = - g_list_delete_link (update->page_flip_listeners, l); - } - - l = l_next; - } -} - void meta_kms_update_set_custom_page_flip (MetaKmsUpdate *update, MetaKmsCustomPageFlipFunc func, diff --git a/src/backends/native/meta-kms-update.h b/src/backends/native/meta-kms-update.h index 0915d0fe1..b4b696115 100644 --- a/src/backends/native/meta-kms-update.h +++ b/src/backends/native/meta-kms-update.h @@ -46,7 +46,6 @@ typedef enum _MetaKmsAssignPlaneFlag enum _MetaKmsPageFlipListenerFlag { META_KMS_PAGE_FLIP_LISTENER_FLAG_NONE = 0, - META_KMS_PAGE_FLIP_LISTENER_FLAG_DROP_ON_ERROR = 1 << 0, }; struct _MetaKmsPageFlipListenerVtable diff --git a/src/backends/native/meta-kms.c b/src/backends/native/meta-kms.c index 07a282c7e..45cd00a14 100644 --- a/src/backends/native/meta-kms.c +++ b/src/backends/native/meta-kms.c @@ -270,28 +270,7 @@ meta_kms_post_pending_update_sync (MetaKms *kms, result_listeners = meta_kms_update_take_result_listeners (update); - if (feedback->error && - flags & META_KMS_UPDATE_FLAG_PRESERVE_ON_ERROR) - { - GList *l; - - meta_kms_update_unlock (update); - - for (l = feedback->failed_planes; l; l = l->next) - { - MetaKmsPlane *plane = l->data; - - meta_kms_update_drop_plane_assignment (update, plane); - } - - meta_kms_update_drop_defunct_page_flip_listeners (update); - - meta_kms_add_pending_update (kms, update); - } - else - { - meta_kms_update_free (update); - } + meta_kms_update_free (update); for (l = result_listeners; l; l = l->next) { diff --git a/src/backends/native/meta-onscreen-native.c b/src/backends/native/meta-onscreen-native.c index 934ea630c..d7979d1fa 100644 --- a/src/backends/native/meta-onscreen-native.c +++ b/src/backends/native/meta-onscreen-native.c @@ -1266,6 +1266,40 @@ meta_onscreen_native_is_buffer_scanout_compatible (CoglOnscreen *onscreen, return result == META_KMS_FEEDBACK_PASSED; } +static void +on_scanout_update_result (const MetaKmsFeedback *kms_feedback, + gpointer user_data) +{ + MetaOnscreenNative *onscreen_native = META_ONSCREEN_NATIVE (user_data); + CoglOnscreen *onscreen = COGL_ONSCREEN (onscreen_native); + const GError *error; + CoglFrameInfo *frame_info; + + error = meta_kms_feedback_get_error (kms_feedback); + if (!error) + return; + + if (!g_error_matches (error, + G_IO_ERROR, + G_IO_ERROR_PERMISSION_DENIED)) + { + ClutterStageView *view = CLUTTER_STAGE_VIEW (onscreen_native->view); + + g_warning ("Direct scanout page flip failed: %s", error->message); + + cogl_scanout_notify_failed (COGL_SCANOUT (onscreen_native->gbm.next_fb), + onscreen); + clutter_stage_view_add_redraw_clip (view, NULL); + clutter_stage_view_schedule_update_now (view); + } + + frame_info = cogl_onscreen_peek_head_frame_info (onscreen); + frame_info->flags |= COGL_FRAME_INFO_FLAG_SYMBOLIC; + + meta_onscreen_native_notify_frame_complete (onscreen); + meta_onscreen_native_clear_next_fb (onscreen); +} + static gboolean meta_onscreen_native_direct_scanout (CoglOnscreen *onscreen, CoglScanout *scanout, @@ -1293,9 +1327,7 @@ meta_onscreen_native_direct_scanout (CoglOnscreen *onscreen, GError *fill_timings_error = NULL; MetaKmsCrtc *kms_crtc; MetaKmsDevice *kms_device; - MetaKmsUpdateFlag flags; - g_autoptr (MetaKmsFeedback) kms_feedback = NULL; - const GError *feedback_error; + MetaKmsUpdate *kms_update; power_save_mode = meta_monitor_manager_get_power_save_mode (monitor_manager); if (power_save_mode != META_POWER_SAVE_ON) @@ -1351,40 +1383,28 @@ meta_onscreen_native_direct_scanout (CoglOnscreen *onscreen, } } + kms_crtc = meta_crtc_kms_get_kms_crtc (META_CRTC_KMS (onscreen_native->crtc)); + kms_device = meta_kms_crtc_get_device (kms_crtc); + kms_update = meta_kms_ensure_pending_update (kms, kms_device); + + meta_kms_update_add_result_listener (kms_update, + on_scanout_update_result, + onscreen_native); + meta_onscreen_native_flip_crtc (onscreen, onscreen_native->view, onscreen_native->crtc, - META_KMS_PAGE_FLIP_LISTENER_FLAG_DROP_ON_ERROR, + META_KMS_PAGE_FLIP_LISTENER_FLAG_NONE, NULL, 0); - kms_crtc = meta_crtc_kms_get_kms_crtc (META_CRTC_KMS (onscreen_native->crtc)); - kms_device = meta_kms_crtc_get_device (kms_crtc); - meta_topic (META_DEBUG_KMS, "Posting direct scanout update for CRTC %u (%s)", meta_kms_crtc_get_id (kms_crtc), meta_kms_device_get_path (kms_device)); - flags = META_KMS_UPDATE_FLAG_PRESERVE_ON_ERROR; - kms_feedback = meta_kms_post_pending_update_sync (kms, kms_device, flags); - switch (meta_kms_feedback_get_result (kms_feedback)) - { - case META_KMS_FEEDBACK_PASSED: - clutter_frame_set_result (frame, - CLUTTER_FRAME_RESULT_PENDING_PRESENTED); - break; - case META_KMS_FEEDBACK_FAILED: - feedback_error = meta_kms_feedback_get_error (kms_feedback); - - if (g_error_matches (feedback_error, - G_IO_ERROR, G_IO_ERROR_PERMISSION_DENIED)) - break; - - g_clear_object (&onscreen_native->gbm.next_fb); - g_propagate_error (error, g_error_copy (feedback_error)); - return FALSE; - } + clutter_frame_set_result (frame, CLUTTER_FRAME_RESULT_PENDING_PRESENTED); + meta_kms_post_pending_update_sync (kms, kms_device, META_KMS_UPDATE_FLAG_NONE); return TRUE; } diff --git a/src/wayland/meta-wayland-buffer.c b/src/wayland/meta-wayland-buffer.c index 66ece538a..628aeee1c 100644 --- a/src/wayland/meta-wayland-buffer.c +++ b/src/wayland/meta-wayland-buffer.c @@ -547,6 +547,43 @@ egl_stream_buffer_attach (MetaWaylandBuffer *buffer, } #endif /* HAVE_WAYLAND_EGLSTREAM */ +static void +on_onscreen_destroyed (gpointer user_data, + GObject *where_the_onscreen_was) +{ + MetaWaylandBuffer *buffer = user_data; + + g_hash_table_remove (buffer->tainted_scanout_onscreens, + where_the_onscreen_was); +} + +static void +on_scanout_failed (CoglScanout *scanout, + CoglOnscreen *onscreen, + MetaWaylandBuffer *buffer) +{ + if (!buffer->tainted_scanout_onscreens) + buffer->tainted_scanout_onscreens = g_hash_table_new (NULL, NULL); + + g_hash_table_add (buffer->tainted_scanout_onscreens, onscreen); + g_object_weak_ref (G_OBJECT (onscreen), on_onscreen_destroyed, buffer); +} + +static void +clear_tainted_scanout_onscreens (MetaWaylandBuffer *buffer) +{ + GHashTableIter iter; + CoglOnscreen *onscreen; + + if (!buffer->tainted_scanout_onscreens) + return; + + g_hash_table_iter_init (&iter, buffer->tainted_scanout_onscreens); + while (g_hash_table_iter_next (&iter, (gpointer *) &onscreen, NULL)) + g_object_weak_unref (G_OBJECT (onscreen), on_onscreen_destroyed, buffer); + g_hash_table_remove_all (buffer->tainted_scanout_onscreens); +} + /** * meta_wayland_buffer_attach: * @buffer: a pointer to a #MetaWaylandBuffer @@ -573,6 +610,8 @@ meta_wayland_buffer_attach (MetaWaylandBuffer *buffer, { COGL_TRACE_BEGIN_SCOPED (MetaWaylandBufferAttach, "WaylandBuffer (attach)"); + clear_tainted_scanout_onscreens (buffer); + if (!meta_wayland_buffer_is_realized (buffer)) { /* The buffer should have been realized at surface commit time */ @@ -797,24 +836,31 @@ CoglScanout * meta_wayland_buffer_try_acquire_scanout (MetaWaylandBuffer *buffer, CoglOnscreen *onscreen) { + CoglScanout *scanout = NULL; + COGL_TRACE_BEGIN_SCOPED (MetaWaylandBufferTryScanout, "WaylandBuffer (try scanout)"); + if (buffer->tainted_scanout_onscreens && + g_hash_table_lookup (buffer->tainted_scanout_onscreens, onscreen)) + { + meta_topic (META_DEBUG_RENDER, "Buffer scanout capability tainted"); + return NULL; + } + switch (buffer->type) { case META_WAYLAND_BUFFER_TYPE_SHM: case META_WAYLAND_BUFFER_TYPE_SINGLE_PIXEL: +#ifdef HAVE_WAYLAND_EGLSTREAM + case META_WAYLAND_BUFFER_TYPE_EGL_STREAM: +#endif meta_topic (META_DEBUG_RENDER, "Buffer type not scanout compatible"); return NULL; case META_WAYLAND_BUFFER_TYPE_EGL_IMAGE: - return try_acquire_egl_image_scanout (buffer, onscreen); -#ifdef HAVE_WAYLAND_EGLSTREAM - case META_WAYLAND_BUFFER_TYPE_EGL_STREAM: - meta_topic (META_DEBUG_RENDER, - "Buffer type not scanout compatible"); - return NULL; -#endif + scanout = try_acquire_egl_image_scanout (buffer, onscreen); + break; case META_WAYLAND_BUFFER_TYPE_DMA_BUF: { MetaWaylandDmaBufBuffer *dma_buf; @@ -823,15 +869,20 @@ meta_wayland_buffer_try_acquire_scanout (MetaWaylandBuffer *buffer, if (!dma_buf) return NULL; - return meta_wayland_dma_buf_try_acquire_scanout (dma_buf, onscreen); + scanout = meta_wayland_dma_buf_try_acquire_scanout (dma_buf, onscreen); + break; } case META_WAYLAND_BUFFER_TYPE_UNKNOWN: g_warn_if_reached (); return NULL; } - g_assert_not_reached (); - return NULL; + g_return_val_if_fail (scanout, NULL); + + g_signal_connect (scanout, "scanout-failed", + G_CALLBACK (on_scanout_failed), buffer); + + return scanout; } static void @@ -841,6 +892,9 @@ meta_wayland_buffer_finalize (GObject *object) g_warn_if_fail (buffer->use_count == 0); + clear_tainted_scanout_onscreens (buffer); + g_clear_pointer (&buffer->tainted_scanout_onscreens, g_hash_table_unref); + g_clear_pointer (&buffer->egl_image.texture, cogl_object_unref); #ifdef HAVE_WAYLAND_EGLSTREAM g_clear_pointer (&buffer->egl_stream.texture, cogl_object_unref); diff --git a/src/wayland/meta-wayland-buffer.h b/src/wayland/meta-wayland-buffer.h index f0f352d71..af70aa3bf 100644 --- a/src/wayland/meta-wayland-buffer.h +++ b/src/wayland/meta-wayland-buffer.h @@ -80,6 +80,8 @@ struct _MetaWaylandBuffer MetaWaylandSinglePixelBuffer *single_pixel_buffer; CoglTexture *texture; } single_pixel; + + GHashTable *tainted_scanout_onscreens; }; #define META_TYPE_WAYLAND_BUFFER (meta_wayland_buffer_get_type ())