dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Max Staudt <mstaudt@suse.de>
To: Ray Strode <halfline@gmail.com>
Cc: b.zolnierkie@samsung.com, linux-fbdev@vger.kernel.org,
	michal@markovi.net, sndirsch@suse.com, oneukum@suse.com,
	tiwai@suse.com, dri-devel@lists.freedesktop.org,
	"Linux-Kernel@Vger. Kernel. Org" <linux-kernel@vger.kernel.org>,
	"Bero Rosenkränzer" <bernhard.rosenkranzer@linaro.org>,
	philm@manjaro.org
Subject: Re: [RFC PATCH v2 00/13] Kernel based bootsplash
Date: Thu, 21 Dec 2017 17:32:42 +0100	[thread overview]
Message-ID: <9d89a924-2e43-01df-5211-0fb69a6b1a0c@suse.de> (raw)
In-Reply-To: <CAA_Uwz+3d_iEUje3M4LPTg-cWTmDwWZLonZAe1pUHn6sWEKBJw@mail.gmail.com>

On 12/21/2017 03:51 PM, Ray Strode wrote:
> Hi,
> 
> On Wed, Dec 20, 2017 at 11:44 AM Max Staudt <mstaudt@suse.de> wrote:
>> It'd be nice to see this bug fixed, as it happens only occasionally (as is the nature of a
>> race condition), and was thus really hard to debug. I'm sure it can drive people insane,
>> as they try to find out whether they've disabled Ctrl-Alt-Fx in their xorg.conf, but really
>> it's Plymouth getting the system into a bad state. I probably owe a bald patch on my
>> head to this bug.
> Okay, reading through the code I do see how what you're describing could happen.
> 
> I sketched out a patch here:
> 
> https://cgit.freedesktop.org/plymouth/commit/?h=fix-vt-management-on-deactivate&id=0b5b790d628f4c96e3ab9fbc0daba67a29b850da
> 
> that I think should fix it.  I need to do some testing with it (ideally rig up
> a reproducer) before I push it.

Hmm, I haven't looked at what manager->renderers_activated means, but from just looking at the diff, it looks like it could solve the problem. Please do test it though! I'm afraid I can't really tell you how to rig up a reproducer, since it's a race condition. Maybe a sleep() in gdm, and then forcefully emptying the udev queue?

Are you sure that process_udev_event (manager) will do the right thing?
Will it keep a list of events not processed?

What if a card is plugged in, then unplugged? Would Plymouth then handle the plugin first, see that the card isn't there, and fail gracefully? And will it handle the unplug gracefully if the card wasn't there in the first place?

Or what if I plug in two cards - it needs to keep a list of events for this case, otherwise it will only detect one card when it resumes udev processing.

Maybe these concerns are unnecessary - I haven't looked at the full Plymouth code since. Just ideas to keep in mind when rigging up the patch.


Thanks for looking into a fix!


>> This is exactly where the kernel bootsplash is useful. Since it starts even before any
>> userspace program is loaded, it can close this gap.
>>
>> I've even tried it in combination with Plymouth: Plymouth is just another graphical
>> application, so it simply pops up "on top", just like X would. The two splashes
>> integrate flawlessly.
> I just wish it used our modern graphics platform instead of the
> deprecated subsystem.

I see, and I share your concern that legacy interfaces should die.
But with the current architecture in the kernel, building it on DRM wouldn't make sense, sorry.
Also, it would exclude the efifb case, which is decidedly a design requirement.


>> One could argue that one could put all DRM drivers into the initrd. Ubuntu does this,
>> and the initrd is ~40 MB in size. Not nice.
> well, that 40mb isn't just graphics drivers...
> ╎❯ du -sh /lib/modules/`uname -r`/kernel/drivers/gpu
> 2.7M /lib/modules/4.14.6-300.fc27.x86_64/kernel/drivers/gpu
> 
> 3M isn't too awful.

Oh, true. Weird, then I must have gotten something mixed up. That means there's truly tons of stuff in that initrd.


> But really you have two choices as I see it:
> 
> 1) make the initrd support new hardware
> 2) make the initrd be taylored to a specific configuration.
> 
> I actually think ubuntu has it right by doing 1. it's going to give
> the best user experience.
> (not just with graphics but other new hardware too).

Yes. Except when the mechanism fails.

And it doesn't cover the time before the driver is loaded.


> But if you do 2) then it's not unreasonable if things break with new
> hardware.

Agreed.


> Now
> ideally, the breakage would be as isolated as possible.  I mean maybe
> it's okay if the
> boot splash breaks (or shows a text splash),

Yup.


> but it's not okay if the
> bootsplash sort of
> works using /dev/fb, but then causes Xorg to break.  So we should
> probably change
> plymouth to avoid falling back to /dev/fb in the case where new
> hardware got added.
> Could probably fix this by detecting if kms is around when the initrd
> is generated,
> and adding a config option to skip fb renderer in that case.  or
> something like that.

That's not possible. When generating the initrd, you don't know where it will actually be booted next.

Practical example: Last year's installation media on next year's hardware.


> But the easy answer is to just fix the initrd to have the graphics drivers.

See above - you can't guarantee that I'm afraid.

Unless the distro decides to not care about vesafb/efifb, and just show the text mode plymouth splash in case no KMS driver has been loaded until then. That's what Fedora does when booted on non-EFI machines, since it boots in VGA text (non-graphics) mode. But ideally, we'd have a graphical splash in as many cases as possible. If you boot your Fedora machine in a framebuffer mode, or in EFI mode, you'll unleash these issues.


>> So let's take SUSE. They don't have a finishing transition, the splash simply stops
>> and is hidden at once. Such a splash makes sense to be shown instantly, right?
> I don't think it makes sense for animations to lack transitions.
> animations without
> transitions look buggy or unfinished. they should fade out or finish
> the loop, or
> whatever.  If it's a static image it should fade to black or the
> background color.

Umm... yeah, that's a design decision. I'm afraid that's not my department ;)

What about the delay? Do you agree that with such a simple, no-transition splash, it makes sense to reduce the delay to 0?


> (going to be away from the computer for a few days after this message
> so probably won't reply for a while to further discussion)

Yes, me too. I'll be back in 2018.

Thank you for your feedback and for fixing Plymouth!


Max

  reply	other threads:[~2017-12-21 16:32 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13 19:47 [RFC PATCH v2 00/13] Kernel based bootsplash Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 01/13] bootsplash: Initial implementation showing black screen Max Staudt
2017-12-13 23:55   ` Randy Dunlap
2017-12-14 15:37     ` Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 02/13] bootsplash: Add file reading and picture rendering Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 03/13] bootsplash: Flush framebuffer after drawing Max Staudt
2017-12-13 21:35   ` Daniel Vetter
2017-12-14 15:36     ` Max Staudt
2017-12-19 12:23       ` Daniel Vetter
2017-12-19 13:34         ` Max Staudt
2017-12-19 13:57           ` Daniel Vetter
2017-12-19 14:07             ` Oliver Neukum
2017-12-31 12:53               ` Alan Cox
2018-01-03 18:04                 ` Max Staudt
2017-12-19 15:41             ` Max Staudt
2017-12-19 16:02               ` Daniel Vetter
2017-12-19 16:23                 ` Max Staudt
2017-12-20  9:45                   ` Daniel Vetter
2017-12-19 16:09               ` Daniel Vetter
2017-12-19 16:26                 ` Max Staudt
2017-12-19 21:01               ` Ray Strode
2017-12-20 13:14                 ` Max Staudt
2017-12-20 15:35                   ` Ray Strode
2017-12-20 16:52                     ` Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 04/13] bootsplash: Add corner positioning Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 05/13] bootsplash: Add animation support Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 06/13] vt: Redraw bootsplash fully on console_unblank Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 07/13] vt: Add keyboard hook to disable bootsplash Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 08/13] sysrq: Disable bootsplash on SAK Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 09/13] fbcon: Disable bootsplash on oops Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 10/13] Documentation: Add bootsplash main documentation Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 11/13] bootsplash: sysfs entries to load and unload files Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 12/13] tools/bootsplash: Add a basic splash file creation tool Max Staudt
2017-12-13 19:47 ` [RFC PATCH v2 13/13] tools/bootsplash: Add script and data to create sample file Max Staudt
2017-12-13 19:52 ` [RFC PATCH v2 00/13] Kernel based bootsplash Max Staudt
2017-12-19 16:16 ` Daniel Vetter
2017-12-19 17:04   ` Max Staudt
2017-12-19 17:26     ` Daniel Vetter
2017-12-19 18:40       ` Max Staudt
2017-12-20  9:43         ` Daniel Vetter
2017-12-20 10:06           ` Neil Armstrong
2017-12-20 10:14             ` Daniel Vetter
2017-12-20 14:55               ` Max Staudt
2017-12-20 15:11                 ` Daniel Vetter
2017-12-20 15:19                   ` Daniel Vetter
2017-12-20 15:22                     ` Daniel Vetter
2017-12-20 16:23                     ` Max Staudt
2017-12-20 16:15                   ` Max Staudt
2017-12-31 12:44                   ` Alan Cox
2018-01-03 18:00                     ` Max Staudt
2017-12-20 14:16             ` Max Staudt
2017-12-20 14:10           ` Max Staudt
2017-12-31 12:35         ` Alan Cox
2018-01-03 17:56           ` Max Staudt
2017-12-19 20:30     ` Ray Strode
2017-12-20 13:03       ` Max Staudt
2017-12-20 15:21         ` Ray Strode
2017-12-20 16:44           ` Max Staudt
2017-12-21 14:51             ` Ray Strode
2017-12-21 16:32               ` Max Staudt [this message]
2017-12-20 11:08   ` Johannes Thumshirn
2017-12-20 11:22     ` Daniel Stone
2017-12-20 12:48       ` Johannes Thumshirn
2017-12-29 17:13   ` Jani Nikula
2018-01-03 17:38     ` Max Staudt
2017-12-21  9:48 ` Daniel Vetter
2017-12-21 16:52   ` Max Staudt
2017-12-21 15:00 ` Philip Müller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9d89a924-2e43-01df-5211-0fb69a6b1a0c@suse.de \
    --to=mstaudt@suse.de \
    --cc=b.zolnierkie@samsung.com \
    --cc=bernhard.rosenkranzer@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=halfline@gmail.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal@markovi.net \
    --cc=oneukum@suse.com \
    --cc=philm@manjaro.org \
    --cc=sndirsch@suse.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).