From 439c38ccc75a7fae1edbef55a35254f550d9846b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 28 Sep 2022 15:14:00 +0200 Subject: [PATCH 1/2] 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 --- 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 cba24250..23b8b378 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, From 5dedca818ffd0370146ef62ecfaa8c591737414e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 28 Sep 2022 15:16:15 +0200 Subject: [PATCH 2/2] 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 Signed-off-by: Hans de Goede --- src/libply-splash-core/ply-device-manager.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c index 23b8b378..600132ca 100644 --- a/src/libply-splash-core/ply-device-manager.c +++ b/src/libply-splash-core/ply-device-manager.c @@ -313,6 +313,10 @@ create_devices_for_udev_device (ply_device_manager_t *manager, ply_trace ("device subsystem is %s", subsystem); if (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) { @@ -458,13 +462,7 @@ verify_add_or_change (ply_device_manager_t *manager, return true; subsystem = udev_device_get_subsystem (device); - - if (strcmp (subsystem, SUBSYSTEM_DRM) == 0) { - if (!verify_drm_device (device)) { - ply_trace ("ignoring since we only handle SimpleDRM devices after timeout"); - return false; - } - } else { + if (strcmp (subsystem, SUBSYSTEM_DRM)) { ply_trace ("ignoring since we only handle subsystem %s devices after timeout", subsystem); return false; }