Created attachment 140348 [details] [review] [PATCH 1/3] ply-renderer: Fix activate handling Hi, The 3 patches which I'm attaching are related to this kernel patch: https://lkml.org/lkml/2018/6/26/489 Which delays fbcon taking over the console (and binding to fbdev devices) until there actually is some text output to the console. This is intended for use with the "quiet" cmdline option, in combination with a bootloader which leaves the vendor's logo / EFI bootgraphics put up by the firmware intact on the EFI framebuffer. The end goal here is a boot where the firmware shows its boot graphics and these stay in place for a couple of seconds until the GUI loads and the GUI then smoothly takes over the framebuffer without any distruptions. Plymouth details mode breaks with this change. The bringup of fbcon is now delayed till that switch to detailed mode (which causes the first output on the console when booting with "quiet" on the kernel commandline). The fbcon init on the switch to detailed mode fails, because plymouth keeps its drm-master rights when switching to detailed mode. The drm_master rights are not dropped, because of various issues in the code to activate/deactivate renderers. These issues cause the drm renderer plugin to not be deactivated when toggling modes. See the individual commit messages for details. Regards, Hans
Created attachment 140349 [details] [review] [PATCH 2/3] main: Do not activate renderers when showing details
Created attachment 140350 [details] [review] [PATCH 3/3] main: Move ply_device_manager_deactivate_renderers() into hide_splash()
Comment on attachment 140349 [details] [review] [PATCH 2/3] main: Do not activate renderers when showing details Review of attachment 140349 [details] [review]: ----------------------------------------------------------------- so your patches make sense to me, but I don't think this one is good enough. At the moment showing_details is just used to determine what action pressing the escape key should do..which way to toggle. You want to use it to know whether or not text is showing on screen, but the problem is, that details aren't the only case where text is showing on screen. we also show text for other text based splash plugins. I think probably we should add a function `ply_boot_splash_needs_graphics` or `ply_boot_splash_uses_pixel_displays` or something like that just does: `return splash->plugin_interface->add_pixel_display != NULL;` then activate_renderers would only get called when that function returns true.
i'm going to be gone next week, but feel free to build these patches in Fedora in the interim.
i pushed two of these: To ssh://git.freedesktop.org/git/plymouth 014c215..778e0fb master -> master
Thank you for the review and for pushing the 2 patches. Monday (tomorrow), I will modify the third patch as you suggested, retest and then attach it here. I will also kick of a F29 plymouth build with the 3 patches added then.
Created attachment 140422 [details] [review] [PATCH] show_theme: Only activate renderers if the splash uses pixel-displays Here is a patch replacing the "[PATCH 2/3] main: Do not activate renderers when showing details" patch as requested. This patch introduces and uses a new ply_boot_splash_uses_pixel_displays() function as suggested.
While working on this I noticed that there were quite a few compiler warning when building plymouth on Fedora 28, wrote 2 patches fixing these, see bug 107086. Also a gentle reminder that I also have some details mode related patches pending in bug 107048.
Created attachment 140444 [details] [review] [PATCH] main: Consistently only (de)activate renderers if the splash uses pixel-displays Hi, As discussed I've done a plymouth build for F29/rawhide including the 3 patches, using the 2nd version of the patch to conditionally activate the renderers. In hindsight that patch (as used in Fedora atm) is incomplete. I should have guarded all (de)activate)renderers calls with the same check. Both for consistency reasons and because not doing so actually causes problems (in some corner cases). Here is a patch to consistently have the guard everywhere. Regards, Hans
Thanks, squashed the two together and pushed: To ssh://git.freedesktop.org/git/plymouth 778e0fb..447c783 master -> master
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.