From 2e875c999d2eba77ad8575265f005a2632d86851 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 7 Sep 2019 22:44:49 +0200 Subject: [PATCH] Add a patch fixing issues when using cards which default to the radeon kms driver with the amdgpu kms driver (rhbz#1490490) Extend default DeviceTimeout to 8 seconds (rhbz#1737221) --- plymouth-more-updates.patch | 198 ++++++++++++++++++++++++++++++++++++ plymouth.spec | 7 +- 2 files changed, 204 insertions(+), 1 deletion(-) diff --git a/plymouth-more-updates.patch b/plymouth-more-updates.patch index 5f61264..9269adb 100644 --- a/plymouth-more-updates.patch +++ b/plymouth-more-updates.patch @@ -670,3 +670,201 @@ index 795bded..2e596d5 100644 -- 2.21.0 +From 4b3be154867ddfc3315a39e57d23a374142c7d2f Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Sun, 25 Aug 2019 18:01:59 +0200 +Subject: [PATCH 1/3] two-step: bgrt: Add workaround for desktops which do not + use the golden ratio + +On desktops (no panel) we normally do not use the BGRT provided +xoffset and yoffset because the resolution they are intended for +may be differtent then the resolution of the current display. + +On some desktops (no panel) the image gets centered not only +horizontally, but also vertically. In this case our default of using +the golden ratio for the vertical position causes the BGRT image +to jump. + +To avoid this this commits adds an extra check to see if the provided +xoffset and yoffset perfectly center the image and in that case uses them. + +An example of a system needing this workaround is the Minix Neo Z83-4. + +Signed-off-by: Hans de Goede +--- + src/plugins/splash/two-step/plugin.c | 18 ++++++++++++++++++ + 1 file changed, 18 insertions(+) + +diff --git a/src/plugins/splash/two-step/plugin.c b/src/plugins/splash/two-step/plugin.c +index 2e596d5..b5ef926 100644 +--- a/src/plugins/splash/two-step/plugin.c ++++ b/src/plugins/splash/two-step/plugin.c +@@ -506,6 +506,24 @@ view_set_bgrt_background (view_t *view) + } + } + ++ /* ++ * On desktops (no panel) we normally do not use the BGRT provided ++ * xoffset and yoffset because the resolution they are intended for ++ * may be differtent then the resolution of the current display. ++ * ++ * On some desktops (no panel) the image gets centered not only ++ * horizontally, but also vertically. In this case our default of using ++ * the golden ratio for the vertical position causes the BGRT image ++ * to jump. To avoid this we check here if the provided xoffset and ++ * yoffset perfectly center the image and in that case we use them. ++ */ ++ if (!have_panel_props && screen_scale == 1 && ++ (screen_width - width ) / 2 == sysfs_x_offset && ++ (screen_height - height) / 2 == sysfs_y_offset) { ++ x_offset = sysfs_x_offset; ++ y_offset = sysfs_y_offset; ++ } ++ + ply_trace ("using %dx%d bgrt image centered at %dx%d for %dx%d screen", + width, height, x_offset, y_offset, screen_width, screen_height); + +-- +2.23.0 + +From 7bd45c782eb7a76005ab7f92de4b51fc2ecb8068 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Fri, 6 Sep 2019 17:21:45 +0200 +Subject: [PATCH 2/3] ply-device-manager: De-activate and close renderer on + device hot unplug + +When a device gets hot unplugged, we should de-activate and close the +renderer before freeing it. + +Speficially this fixes a problem where plymout's display "freezes" when +used with AMD GPUs which can be handled by both the radeon or the amdgpu +kms driver and the user has added kernel commandline options to prefer +the amdgpu driver. In this case the following happens: + +1) radeon driver gets loaded, creates /dev/dri/card0 generating a udev + "add /dev/dri/card0" event. +2) radeon driver's "load" method checks if it should bind, comes to the + conclusion it should not, causing the probe of the device to be + aborted and /dev/dri/card0 to be unregistered generating a udev + "remove /dev/dri/card0" event +3) amdgpu driver loads, creates /dev/dri/card0 generating another udev + "add /dev/dri/card0" event. +4) plymouth loads while udev is still catching up with kernel events, + plymouth sees the first "add" event opens /dev/dri/card0 (which is + already managed by amdgpu at this time) finds outputs, shows splash +5) plymouth gets the "remove" events tears down the renderer, but + since deactivate and close were not called before this commit, + plymouth keeps the master rights attached to the old-fd which is + leaked at this point +6) plymouth gets the second "add" event brings everything back up, but + it cannot get master rights, since there can only be 1 master and the + leaked fd is the master. This leads to lots of: + "Couldn't set scan out buffer" errors and plymouth appears frozen + because of this, this is esp. bad with the disk encryption unlock screen +7) When gdm starts, it deactivates plymouth, but does not yet tell it to quit + to avoid flickering. The deactivate makes plymouth drop its master rights, + but this happens on the new broken fd which never got the master rights. + This also makes it impossible for gdm to get master rights, also breaking + gdm, with the following error in the logs: + "(EE) wl_drm@4: error 0: authenicate failed" + +Note in theory calling ply_renderer_close() on teardown (from +ply_device_manager_free()) should be fine too, but for some reason +this causes the framebuffer to be all messed up (looks like wrong pitch / +tiling) when this is done after gdm has taken over the framebuffer. + +Where as just exiting without calling drmModeRmFB, letting the kernel +clean up behind us, does not trigger this. This feels like it is a kernel +bug bug and I will file a bug for this. But for now we should not call +ply_renderer_close() on teardown to avoid this issue (which would be a +regression). + +BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1490490 +Signed-off-by: Hans de Goede +--- + src/libply-splash-core/ply-device-manager.c | 14 +++++++++++--- + 1 file changed, 11 insertions(+), 3 deletions(-) + +diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c +index e2a9eae..160b8cb 100644 +--- a/src/libply-splash-core/ply-device-manager.c ++++ b/src/libply-splash-core/ply-device-manager.c +@@ -169,7 +169,8 @@ free_keyboards_for_renderer (ply_device_manager_t *manager, + + static void + free_devices_from_device_path (ply_device_manager_t *manager, +- const char *device_path) ++ const char *device_path, ++ bool close) + { + void *key = NULL; + void *renderer = NULL; +@@ -187,6 +188,13 @@ free_devices_from_device_path (ply_device_manager_t *manager, + + ply_hashtable_remove (manager->renderers, (void *) device_path); + free (key); ++ ++ if (manager->renderers_activated) ++ ply_renderer_deactivate (renderer); ++ ++ if (close) ++ ply_renderer_close (renderer); ++ + ply_renderer_free (renderer); + } + +@@ -309,7 +317,7 @@ free_devices_for_udev_device (ply_device_manager_t *manager, + device_path = udev_device_get_devnode (device); + + if (device_path != NULL) +- free_devices_from_device_path (manager, device_path); ++ free_devices_from_device_path (manager, device_path, true); + } + + static bool +@@ -557,7 +565,7 @@ free_renderer (char *device_path, + ply_renderer_t *renderer, + ply_device_manager_t *manager) + { +- free_devices_from_device_path (manager, device_path); ++ free_devices_from_device_path (manager, device_path, false); + } + + static void +-- +2.23.0 + +From 443415375a58f2f58af9991a55a8f4aaff83a7c9 Mon Sep 17 00:00:00 2001 +From: Hans de Goede +Date: Fri, 6 Sep 2019 17:40:06 +0200 +Subject: [PATCH 3/3] plymouthd.defaults: Bump default DeviceTimeout to 8 + seconds + +The amdgpu driver needs up to 5 seconds to initialize / boot some +AMD GPUs, which makes our 5 second timeout much too close to the actual +needed time, leading to systems where sometimes the boot is fine and the +next boot we fallback to a text based splash because we won the race from +the GPU probe code. + +This commit bumps the default DeviceTimeout to 8 seconds giving us a +decent marging here without making it overly long. + +BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1737221 +Signed-off-by: Hans de Goede +--- + src/plymouthd.defaults | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/plymouthd.defaults b/src/plymouthd.defaults +index 9e3a342..7f247fd 100644 +--- a/src/plymouthd.defaults ++++ b/src/plymouthd.defaults +@@ -3,4 +3,4 @@ + [Daemon] + Theme=spinner + ShowDelay=0 +-DeviceTimeout=5 ++DeviceTimeout=8 +-- +2.23.0 + diff --git a/plymouth.spec b/plymouth.spec index 55482d4..e0141fb 100644 --- a/plymouth.spec +++ b/plymouth.spec @@ -1,7 +1,7 @@ Summary: Graphical Boot Animation and Logger Name: plymouth Version: 0.9.4 -Release: 8%{?dist} +Release: 9%{?dist} License: GPLv2+ URL: http://www.freedesktop.org/wiki/Software/Plymouth @@ -415,6 +415,11 @@ fi %changelog +* Sat Sep 7 2019 Hans de Goede - 0.9.4-9 +- Add a patch fixing issues when using cards which default to the radeon + kms driver with the amdgpu kms driver (rhbz#1490490) +- Extend default DeviceTimeout to 8 seconds (rhbz#1737221) + * Fri Jul 26 2019 Fedora Release Engineering - 0.9.4-8 - Rebuilt for https://fedoraproject.org/wiki/Fedora_31_Mass_Rebuild