Revert patch to immediately switch to text mode on first renderer plugin error (#2270030)

This fixes unwanted text mode when drm-plugin init races with simpledrm
unregistration

Resolves: rhbz#2270030
This commit is contained in:
Hans de Goede 2024-05-07 18:54:19 +02:00 committed by Ray Strode
parent 312997bdbf
commit 0c49f34d21
9 changed files with 60 additions and 509 deletions

View File

@ -1,34 +0,0 @@
From 5d758194621ae40ef2b2613fd86101652114cbd3 Mon Sep 17 00:00:00 2001
From: Oleg Solovyov <mcpain@altlinux.org>
Date: Wed, 8 Dec 2021 19:15:47 +0300
Subject: [PATCH] drm: Retry setting scanout buffer on failure
Plymouth currently assumes that setting the scanout buffer will succeed.
if it fails because of a driver bug or transient failure it should try
again next frame.
This commit adds that error handling code.
---
src/plugins/renderers/drm/plugin.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/plugins/renderers/drm/plugin.c b/src/plugins/renderers/drm/plugin.c
index 02e9413..10711c6 100644
--- a/src/plugins/renderers/drm/plugin.c
+++ b/src/plugins/renderers/drm/plugin.c
@@ -1628,9 +1628,9 @@ reset_scan_out_buffer_if_needed (ply_renderer_backend_t *backend,
return false;
if (head->scan_out_buffer_needs_reset) {
- ply_renderer_head_set_scan_out_buffer (backend, head,
- head->scan_out_buffer_id);
- head->scan_out_buffer_needs_reset = false;
+ did_reset = ply_renderer_head_set_scan_out_buffer (backend, head,
+ head->scan_out_buffer_id);
+ head->scan_out_buffer_needs_reset = !did_reset;
return true;
}
--
2.35.1

View File

@ -0,0 +1,52 @@
From 4a8c0a4231d35ac060f41b582596684cfba7c9ae Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 7 May 2024 12:42:10 +0200
Subject: [PATCH] ply-device-manager: Revert "Fall back to text plugin if no
renderers installed"
The drm renderer may fail to open /dev/dri/card# with -ENOENT when trying
to open/probe a simpledrm registered drm device and the open races with
that drm device being removed to be replaced by a new drm device registered
by the native GPU driver (e.g. i915 / amdgpu).
Switching to text mode immediately when this race gets hit is undesirable,
as it causes text mode on systems where plymouth would run in graphics
mode before. Remove the immediate switch to text mode on -ENOENT.
Delaying the switch to textmode until the timeout as before.
This reverts commit 03842d5201e4486fe62635c7b470eb94696f985d.
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2270030
---
src/libply-splash-core/ply-device-manager.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c
index 59e579dd..d75ac6c5 100644
--- a/src/libply-splash-core/ply-device-manager.c
+++ b/src/libply-splash-core/ply-device-manager.c
@@ -52,7 +52,6 @@
static void create_devices_from_udev (ply_device_manager_t *manager);
#endif
-static void create_non_graphical_devices (ply_device_manager_t *manager);
static bool create_devices_for_terminal_and_renderer_type (ply_device_manager_t *manager,
const char *device_path,
ply_terminal_t *terminal,
@@ -1102,13 +1101,6 @@ create_devices_for_terminal_and_renderer_type (ply_device_manager_t *manager,
renderer = ply_renderer_new (renderer_type, device_path, terminal);
if (renderer != NULL && !ply_renderer_open (renderer)) {
- if (errno == ENOENT) {
- ply_trace ("No renderer plugins installed, creating non-graphical devices");
- ply_renderer_free (renderer);
- create_non_graphical_devices (manager);
- manager->device_timeout_elapsed = true;
- return false;
- }
ply_trace ("could not open renderer for %s", device_path);
ply_renderer_free (renderer);
renderer = NULL;
--
2.44.0

View File

@ -1,142 +0,0 @@
From b41e40e065c60e76b9721747492875c3454440dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mateusz=20Pi=C3=B3rkowski?= <mati7337@protonmail.ch>
Date: Tue, 22 Feb 2022 15:41:49 +0000
Subject: [PATCH] Add support for CSI sequences
---
src/libply-splash-core/ply-keyboard.c | 52 +++++++++++++++++++++++++--
src/main.c | 2 ++
src/plugins/renderers/x11/plugin.c | 19 ++++++++++
3 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/src/libply-splash-core/ply-keyboard.c b/src/libply-splash-core/ply-keyboard.c
index b174261..b40e961 100644
--- a/src/libply-splash-core/ply-keyboard.c
+++ b/src/libply-splash-core/ply-keyboard.c
@@ -48,6 +48,12 @@
#define KEY_RETURN '\n'
#define KEY_BACKSPACE '\177'
+#define CSI_SEQUENCE_PREFIX "\033["
+#define CSI_SEQUENCE_MINIMUM_LENGTH (strlen (CSI_SEQUENCE_PREFIX) + 1)
+
+#define FUNCTION_KEY_SEQUENCE_PREFIX (CSI_SEQUENCE_PREFIX "[")
+#define FUNCTION_KEY_SEQUENCE_MINIMUM_LENGTH (strlen (FUNCTION_KEY_SEQUENCE_PREFIX) + 1)
+
typedef void (*ply_keyboard_handler_t) (void *);
typedef struct
@@ -195,7 +201,11 @@ process_keyboard_input (ply_keyboard_t *keyboard,
wchar_t key;
ply_list_node_t *node;
- if ((ssize_t) mbrtowc (&key, keyboard_input, character_size, NULL) > 0) {
+ if (keyboard_input[0] == KEY_ESCAPE && character_size >= 2){
+ /* Escape sequence */
+ ply_buffer_append_bytes (keyboard->line_buffer,
+ keyboard_input, character_size);
+ } else if ((ssize_t) mbrtowc (&key, keyboard_input, character_size, NULL) > 0) {
switch (key) {
case KEY_CTRL_U:
case KEY_CTRL_W:
@@ -270,8 +280,46 @@ on_key_event (ply_keyboard_t *keyboard,
while (i < size) {
ssize_t character_size;
char *keyboard_input;
+ size_t bytes_left = size - i;
+
+ /* Control Sequence Introducer sequences
+ */
+ if(bytes_left >= FUNCTION_KEY_SEQUENCE_MINIMUM_LENGTH &&
+ strncmp (bytes + i, FUNCTION_KEY_SEQUENCE_PREFIX,
+ strlen (FUNCTION_KEY_SEQUENCE_PREFIX)) == 0) {
+ /* Special case - CSI [ after which the next character
+ * is a function key
+ */
+ process_keyboard_input (keyboard, bytes + i, 4);
+ i += 4;
+ continue;
+ } else if(bytes_left >= CSI_SEQUENCE_MINIMUM_LENGTH && /* At least CSI + final byte */
+ strncmp (bytes + i, CSI_SEQUENCE_PREFIX,
+ strlen (CSI_SEQUENCE_PREFIX)) == 0) {
+ ssize_t csi_seq_size;
+ csi_seq_size = 0;
+ for (size_t j = strlen (CSI_SEQUENCE_PREFIX); j < bytes_left; j++) {
+ if ((bytes[i + j] >= 0x40) &&
+ (bytes[i + j] <= 0x7E)) {
+ /* Final byte found */
+ csi_seq_size = j + 1;
+ break;
+ }
+ /* We presume if we aren't at the final byte, the intermediate
+ * bytes will be in the range 0x20-0x2F, but we don't validate
+ * that, since it's not really clear how invalid sequences should
+ * be handled, and letting them through to the keyboard input
+ * handlers seems just as reasonable as alternatives.
+ */
+ }
+ if (csi_seq_size == 0) /* No final byte found */
+ continue;
+ process_keyboard_input (keyboard, bytes + i, csi_seq_size);
+ i += csi_seq_size;
+ continue;
+ }
- character_size = (ssize_t) ply_utf8_character_get_size (bytes + i, size - i);
+ character_size = (ssize_t) ply_utf8_character_get_size (bytes + i, bytes_left);
if (character_size < 0)
break;
diff --git a/src/main.c b/src/main.c
index 1cb8f6c..bedab7d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1516,6 +1516,8 @@ on_keyboard_input (state_t *state,
ply_buffer_clear (state->entry_buffer);
ply_list_remove_node (state->entry_triggers, node);
free (entry_trigger);
+ } else if (character_size >= 2 && keyboard_input[0] == '\033') {
+ /* Ignore escape sequences */
} else {
ply_buffer_append_bytes (state->entry_buffer, keyboard_input, character_size);
}
diff --git a/src/plugins/renderers/x11/plugin.c b/src/plugins/renderers/x11/plugin.c
index bfb039a..e7169f3 100644
--- a/src/plugins/renderers/x11/plugin.c
+++ b/src/plugins/renderers/x11/plugin.c
@@ -61,6 +61,21 @@
#include "ply-renderer.h"
#include "ply-renderer-plugin.h"
+static const char *function_key_escape_sequence[] = {
+ "\033[[A", /* F1 */
+ "\033[[B", /* F2 */
+ "\033[[C", /* F3 */
+ "\033[[D", /* F4 */
+ "\033[[E", /* F5 */
+ "\033[17~", /* F6 */
+ "\033[18~", /* F7 */
+ "\033[19~", /* F8 */
+ "\033[20~", /* F9 */
+ "\033[21~", /* F10 */
+ "\033[22~", /* F11 */
+ "\033[23~", /* F12 */
+};
+
struct _ply_renderer_head
{
ply_renderer_backend_t *backend;
@@ -482,6 +497,10 @@ on_key_event (GtkWidget *widget,
ply_buffer_append_bytes (input_source->key_buffer, "\033", 1);
} else if (event->keyval == GDK_KEY_BackSpace) { /* Backspace */
ply_buffer_append_bytes (input_source->key_buffer, "\177", 1);
+ } else if (GDK_KEY_F1 <= event->keyval &&
+ GDK_KEY_F12 >= event->keyval) { /* F1-F12 */
+ const char *key = function_key_escape_sequence[event->keyval - GDK_KEY_F1];
+ ply_buffer_append_bytes (input_source->key_buffer, key, strlen(key));
} else {
gchar bytes[7];
int byte_count;
--
2.35.1

View File

@ -1,50 +0,0 @@
From ab4238d54813561d3ad0e50684504d67e4c5ba96 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 7 Mar 2022 12:25:56 +0100
Subject: [PATCH 3/4] ply-utils: Reintroduce ply_string_has_prefix helper
ply_string_has_prefix was dropped in commit c7965ea19abf ("ply-utils:
Drop unused ply_string_has_prefix helper"). We have a need for this
helper again, so reintroduce it.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
src/libply/ply-utils.c | 9 +++++++++
src/libply/ply-utils.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/src/libply/ply-utils.c b/src/libply/ply-utils.c
index f90ac40..f457579 100644
--- a/src/libply/ply-utils.c
+++ b/src/libply/ply-utils.c
@@ -459,6 +459,15 @@ ply_free_string_array (char **array)
free (array);
}
+bool
+ply_string_has_prefix (const char *str, const char *prefix)
+{
+ if (str == NULL || prefix == NULL)
+ return false;
+
+ return strncmp (str, prefix, strlen (prefix)) == 0;
+}
+
double
ply_get_timestamp (void)
{
diff --git a/src/libply/ply-utils.h b/src/libply/ply-utils.h
index 47bb3f2..7572cca 100644
--- a/src/libply/ply-utils.h
+++ b/src/libply/ply-utils.h
@@ -85,6 +85,7 @@ bool ply_fd_has_data (int fd);
bool ply_set_fd_as_blocking (int fd);
char **ply_copy_string_array (const char *const *array);
void ply_free_string_array (char **array);
+bool ply_string_has_prefix (const char *str, const char *prefix);
double ply_get_timestamp (void);
void ply_save_errno (void);
--
2.35.1

View File

@ -1,94 +0,0 @@
From 83b385061ccbf5a46ea77f7f12c1c7bfc72a09f2 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 28 Feb 2022 16:07:11 +0100
Subject: [PATCH 4/4] ply-device-manager: Treat SimpleDRM drm devices as fbdev
devices
Simple-framebuffer devices driven by simpledrm lack information
like panel-rotation info and physical size, causing the splash
to briefly render on its side / without HiDPI scaling, switching
to the correct rendering when the native driver loads.
To avoid this treat simpledrm devices as fbdev devices and only
use them after the timeout.
Also adds 2 exceptions to this:
1. If nomodeset is passed on the kernel commandline then no native
drivers will load, so in this case it is best to immediately use
SimpleDRM devices when they are detected.
2. On some devics the firmware leave the panel black at boot. In this
case it is desirable to show the splash to the user ASAP so that there
is some visual feedback that the device is booting. Add a support for a
"plymouth.use-simpledrm" kernel cmdline option to show the splash
immediately on SimpleDRM devices rather then waiting for the native
driver to load.
Closes #167
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
src/libply-splash-core/ply-device-manager.c | 39 +++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c
index bb548ef..82b89f3 100644
--- a/src/libply-splash-core/ply-device-manager.c
+++ b/src/libply-splash-core/ply-device-manager.c
@@ -405,6 +405,40 @@ on_drm_udev_add_or_change (ply_device_manager_t *manager,
}
}
+static bool
+verify_drm_device (struct udev_device *device)
+{
+ const char *id_path;
+
+ /*
+ * Simple-framebuffer devices driven by simpledrm lack information
+ * like panel-rotation info and physical size, causing the splash
+ * to briefly render on its side / without HiDPI scaling, switching
+ * to the correct rendering when the native driver loads.
+ * To avoid this treat simpledrm devices as fbdev devices and only
+ * use them after the timeout.
+ */
+ id_path = udev_device_get_property_value (device, "ID_PATH");
+ if (!ply_string_has_prefix (id_path, "platform-simple-framebuffer"))
+ return true; /* Not a SimpleDRM device */
+
+ /*
+ * With nomodeset, no native drivers will load, so SimpleDRM devices
+ * should be used immediately.
+ */
+ if (ply_kernel_command_line_has_argument ("nomodeset"))
+ return true;
+
+ /*
+ * Some firmwares leave the panel black at boot. Allow enabling SimpleDRM
+ * use from the cmdline to show something to the user ASAP.
+ */
+ if (ply_kernel_command_line_has_argument ("plymouth.use-simpledrm"))
+ return true;
+
+ return false;
+}
+
static bool
verify_add_or_change (ply_device_manager_t *manager,
const char *action,
@@ -423,6 +457,11 @@ verify_add_or_change (ply_device_manager_t *manager,
ply_trace ("ignoring since we're already using text splash for local console");
return false;
}
+
+ if (!verify_drm_device (device)) {
+ ply_trace ("ignoring since we only handle SimpleDRM devices after timeout");
+ return false;
+ }
} else {
ply_trace ("ignoring since we only handle subsystem %s devices after timeout", subsystem);
return false;
--
2.35.1

View File

@ -1,105 +0,0 @@
From f775a38aefa3797d3697330aea78b341914e59f5 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Wed, 28 Sep 2022 15:14:00 +0200
Subject: [PATCH 5/6] ply-device-manager: Move verify_drm_device() higher up in
the file
Move verify_drm_device() higher up in ply-device-manager.c, this is
a preparation patch for the next patch in this series.
This is a pure move without any changes to the moved block.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
src/libply-splash-core/ply-device-manager.c | 68 ++++++++++-----------
1 file changed, 34 insertions(+), 34 deletions(-)
diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c
index cba2425..23b8b37 100644
--- a/src/libply-splash-core/ply-device-manager.c
+++ b/src/libply-splash-core/ply-device-manager.c
@@ -262,6 +262,40 @@ fb_device_has_drm_device (ply_device_manager_t *manager,
return has_drm_device;
}
+static bool
+verify_drm_device (struct udev_device *device)
+{
+ const char *id_path;
+
+ /*
+ * Simple-framebuffer devices driven by simpledrm lack information
+ * like panel-rotation info and physical size, causing the splash
+ * to briefly render on its side / without HiDPI scaling, switching
+ * to the correct rendering when the native driver loads.
+ * To avoid this treat simpledrm devices as fbdev devices and only
+ * use them after the timeout.
+ */
+ id_path = udev_device_get_property_value (device, "ID_PATH");
+ if (!ply_string_has_prefix (id_path, "platform-simple-framebuffer"))
+ return true; /* Not a SimpleDRM device */
+
+ /*
+ * With nomodeset, no native drivers will load, so SimpleDRM devices
+ * should be used immediately.
+ */
+ if (ply_kernel_command_line_has_argument ("nomodeset"))
+ return true;
+
+ /*
+ * Some firmwares leave the panel black at boot. Allow enabling SimpleDRM
+ * use from the cmdline to show something to the user ASAP.
+ */
+ if (ply_kernel_command_line_has_argument ("plymouth.use-simpledrm"))
+ return true;
+
+ return false;
+}
+
static bool
create_devices_for_udev_device (ply_device_manager_t *manager,
struct udev_device *device)
@@ -404,40 +438,6 @@ on_drm_udev_add_or_change (ply_device_manager_t *manager,
}
}
-static bool
-verify_drm_device (struct udev_device *device)
-{
- const char *id_path;
-
- /*
- * Simple-framebuffer devices driven by simpledrm lack information
- * like panel-rotation info and physical size, causing the splash
- * to briefly render on its side / without HiDPI scaling, switching
- * to the correct rendering when the native driver loads.
- * To avoid this treat simpledrm devices as fbdev devices and only
- * use them after the timeout.
- */
- id_path = udev_device_get_property_value (device, "ID_PATH");
- if (!ply_string_has_prefix (id_path, "platform-simple-framebuffer"))
- return true; /* Not a SimpleDRM device */
-
- /*
- * With nomodeset, no native drivers will load, so SimpleDRM devices
- * should be used immediately.
- */
- if (ply_kernel_command_line_has_argument ("nomodeset"))
- return true;
-
- /*
- * Some firmwares leave the panel black at boot. Allow enabling SimpleDRM
- * use from the cmdline to show something to the user ASAP.
- */
- if (ply_kernel_command_line_has_argument ("plymouth.use-simpledrm"))
- return true;
-
- return false;
-}
-
static bool
verify_add_or_change (ply_device_manager_t *manager,
const char *action,
--
2.37.3

View File

@ -1,84 +0,0 @@
From d32a748fe5c24d5cc3b3dd671a045102373dd392 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Wed, 28 Sep 2022 15:16:15 +0200
Subject: [PATCH 6/6] ply-device-manager: Also ignore SimpleDRM devs in
coldplug enumeration path
Plymouth has 2 hw discovery paths:
1. Enumerating devices already known by udev at plymouth startup
2. Devices which are hotplugged after startup
At boot we have udevd which is enumerating hw and plymouthd racing
with each other, which means that plymouthd may discover the new
SimpleDRM device through either 1. or 2.
Before this patch a check for SimpleDRM was missing from path 1, causing
it to be treated as a normal device instead of being ignored as intended:
plymouth-debug.log for the simpledrm being enumerated in path 1:
ply-device-manager.c:344: create_devices_for_subsystem:
found device /sys/devices/platform/simple-framebuffer.0/drm/card0
ply-device-manager.c:351: create_devices_for_subsystem:
device is initialized
ply-device-manager.c:360: create_devices_for_subsystem:
found node /dev/dri/card0
ply-device-manager.c:283: create_devices_for_udev_device:
found DRM device /dev/dri/card0
ply-device-manager.c:885: create_devices_for_terminal_and_rende:
creating devices for /dev/dri/card0 (renderer type: 1)
plymouth-debug.log for the simpledrm *not* being enumerated in path 1:
ply-device-manager.c:344: create_devices_for_subsystem:
found device /sys/devices/platform/simple-framebuffer.0/drm/card0
ply-device-manager.c:367: create_devices_for_subsystem:
it's not initialized
followed by path 2 enumerating the device very shortly after this:
ply-device-manager.c:532: on_udev_event:
got add event for device /dev/dri/card0
ply-device-manager.c:462: verify_add_or_change:
ignoring since we only handle SimpleDRM devices after timeout
Note how path 2 does correctly ignore SimpleDRM devices, where as
path 1 does not. This commit fixes this by moving the verify_drm_device()
check in to create_devices_for_udev_device() which runs in both paths.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2127663
Reported-by: Michael Catanzaro <mcatanza@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
src/libply-splash-core/ply-device-manager.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c
index 23b8b37..820a282 100644
--- a/src/libply-splash-core/ply-device-manager.c
+++ b/src/libply-splash-core/ply-device-manager.c
@@ -314,6 +314,10 @@ create_devices_for_udev_device (ply_devi
ply_trace ("device subsystem is %s", subsystem);
if (subsystem != NULL && strcmp (subsystem, SUBSYSTEM_DRM) == 0) {
+ if (!manager->device_timeout_elapsed && !verify_drm_device (device)) {
+ ply_trace ("ignoring since we only handle SimpleDRM devices after timeout");
+ return false;
+ }
ply_trace ("found DRM device %s", device_path);
renderer_type = PLY_RENDERER_TYPE_DRM;
} else if (strcmp (subsystem, SUBSYSTEM_FRAME_BUFFER) == 0) {
@@ -457,11 +461,6 @@ verify_add_or_change (ply_device_manager
ply_trace ("ignoring since we're already using text splash for local console");
return false;
}
-
- if (!verify_drm_device (device)) {
- ply_trace ("ignoring since we only handle SimpleDRM devices after timeout");
- return false;
- }
} else {
ply_trace ("ignoring since we only handle subsystem %s devices after timeout", subsystem);
return false;
--
2.37.3

View File

@ -12,3 +12,5 @@ addFilter("incoherent-logrotate-file")
addFilter("obsolete-not-provided plymouth-plugin-throbgress")
# Some files are shared between themes
addFilter("files-duplicate")
# Some of our packages have no binaries, this is fine
addFilter("no-binary")

View File

@ -18,6 +18,12 @@ Patch: 0001-Revert-src-Hide-console-text-when-splash-is-requeste.patch
# https://bugzilla.redhat.com/show_bug.cgi?id=2271337
Patch: 0001-ply-boot-splash-Set-unbuffered-input-when-creating-a.patch
# Revert patch to immediately switch to text mode on first renderer plugin error
# Fixes unwanted text mode when drm-plugin init races with simpledrm unregistration
# https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/319
# https://bugzilla.redhat.com/show_bug.cgi?id=2270030
Patch: 0001-ply-device-manager-Revert-Fall-back-to-text-plugin-i.patch
BuildRequires: meson
BuildRequires: system-logos
BuildRequires: gcc libtool git