vt: Draw logos per CPU core

[cem]

Summary

This feature is inspired by another Unix-alike OS commonly found on
airplane headrests.

A number of logos[0] are drawn at top of framebuffer during boot,
based on the number of active SMP CPUs[1]. Console buffer output
continues to scroll in the screen area below logos/sprites[2].

After some time[3] has passed, the logos are erased leaving the
entire terminal for use.

Includes two 80x80 vga16 beastie graphics and an 80x80 vga16 orb
graphic. (The graphics are RLE compressed to save some space -- 3x 3200
bytes uncompressed, or 4208 compressed.)

[0]: The user may select the style of logo with

kern.vt.splash_cpu_style=(0|1|2)

[1]: Or the number may be overridden with tunable kern.vt.splash_ncpu.
[2]: https://www.youtube.com/watch?v=UP2jizfr3_o
[3]: Configurable with kern.vt.splash_cpu_duration (seconds, def. 10).

Test Plan

Compile, reboot.

[emaste]

How do you handle ncpu greater than the number of Beasties that fit on a row?

[cse_cem_gmail_com]

In D2181#3, @emaste wrote:

How do you handle ncpu greater than the number of Beasties that fit on a row?

They are truncated at the edge of the row. See vtterm_draw_beasties around line 384:

ncpu = MIN(ncpu, vd->vd_width / vt_beastie_width);

[cse_cem_gmail_com]

In D2181#3, @emaste wrote:

How do you handle ncpu greater than the number of Beasties that fit on a row?

E.g. here is a video configured with the Orb graphic, with ncpus overridden to sixteen:
https://www.youtube.com/watch?v=MOMDSnk_1kE

Before i915kms loads, they are truncated at eight. After it loads, you can see all sixteen. Yes, this could be improved for ThunderX.

[bdrewery]

This is really cool!

[cse_cem_gmail_com]

Ping. What can I do to help move this forward?

[ed]

I think one of the important things to do is to keep vt(4) simple. syscons(4) became so complex that nobody really understood how it worked anymore. I am afraid that patches that add gimmicks like these negatively affect the overall readability of vt(4) for little functional gain.

That said, I wouldn't mind seeing features like these added as long as they are separate from files like vt_core.c itself. Maybe we need a more generic interface for registering boot splashes/window borders/decorations/etc.

Thoughts?

[cse_cem_gmail_com]

In D2181#10, @ed wrote:

I think one of the important things to do is to keep vt(4) simple. syscons(4) became so complex that nobody really understood how it worked anymore. I am afraid that patches that add gimmicks like these negatively affect the overall readability of vt(4) for little functional gain.

If you've looked at the patch, there's very little change to existing code paths. I don't think it significantly affects readability at all.

That said, I wouldn't mind seeing features like these added as long as they are separate from files like vt_core.c itself. Maybe we need a more generic interface for registering boot splashes/window borders/decorations/etc.

It would not be hard to rework the vt integration slightly, as you suggest; have the beasties logic reserve some border area and the vt code draw inside the border.

Thoughts?

To incorrectly paraphrase Knuth, I think premature generalization is some sort of evil. We don't want too much abstraction. But it would not be hard to move (most) of this to another source file and make the hooks slightly more general.

[adrian]

Hi,

This is a really slow way of drawing things. We should finally (!) put an image blitter method in that takes a colour bitmap and draws it using the most efficient method possible.

For VGA, it's not efficient to do 8 RMW passes for each pixel being plotted. That may be fine in a VM and maybe for beastie, but it's not good enough for a general "draw image here" thing (eg splash screen.)

So I like this as a cute hack, but i'd be even happier if we finally un-mucked the VT layer:

• there should be a way to translate VT colours to hardware colours. Not everything is 16 colour VGA;
• there should be an image blit routine that isn't black-and-white - ie, it takes an array of pixel data and transparency data, and writes that out. for VGA it'll be the most efficient way of doing the planar access we need.
• it's likely that all the VT routines for font rendering, etc should not being using the 4 bit VT colour, but should at least have the option of 8 bit VGA, 16 bit RGB, etc. Right now we can't even do > 16 colour terminals, which is silly.

[cse_cem_gmail_com]

In D2181#12, @adrian wrote:

This is a really slow way of drawing things.

Sort of...

We should finally (!) put an image blitter method in that takes a colour bitmap and draws it using the most efficient method possible.

There is something like this — there is a vd_bitblt_bitmap() method. I initially used it, however I found that whatever driver is used by default ignored the mask argument. Every layer thus erased every layer below it, ruining the effect. In practice, we only ever draw once or twice (we do *not* have to redraw when the text draws or scrolls, for example — only when the resolution changes or we load a new kms driver) and it's plenty fast enough (faster than the human eye).

For VGA, it's not efficient to do 8 RMW passes for each pixel being plotted. That may be fine in a VM and maybe for beastie, but it's not good enough for a general "draw image here" thing (eg splash screen.)

It's plenty good enough for a splash screen, as far as I can tell. Of course, something that takes a larger bitmap would be nicer. If you want to go and fix every driver's vd_bitblt_bitmap method, I'd appreciate it :-).

So I like this as a cute hack, but i'd be even happier if we finally un-mucked the VT layer:

• there should be a way to translate VT colours to hardware colours. Not everything is 16 colour VGA;
• there should be an image blit routine that isn't black-and-white - ie, it takes an array of pixel data and transparency data, and writes that out. for VGA it'll be the most efficient way of doing the planar access we need.

vd_bitblt_bitmap. It just doesn't handle transparency correctly.

• it's likely that all the VT routines for font rendering, etc should not being using the 4 bit VT colour, but should at least have the option of 8 bit VGA, 16 bit RGB, etc. Right now we can't even do > 16 colour terminals, which is silly.

Yeah, agreed.

[adrian]

Again, the problem with the bitmap blit function is that it's only for one colour at a time. That's terrible for doing large scale image drawing, but fine for doing font glyph rendering.

So yeah, we really should overhaul the VT interface for doing colour management and blitting before we continue. :)

[ed]

In D2181#14, @adrian wrote:

So yeah, we really should overhaul the VT interface for doing colour management and blitting before we continue. :)

Though I agree the blitting in vt(4) is far from optimal, is it really the case that the current code is unsuitable for the amount of image drawing we do right now? These images are only drawn once during boot and likely already disappear before the user starts to switch virtual terminals. In other words, the images only get drawn once.

Should this work really block on reimplementing the blitting?

[adrian]

Real VGA drawing is terribly slow. If we wanted to do any kind of splash screen animation then yes, you'll want to have real image drawing implemented in vt.

It's also terrible for colour maps - trying to make a /nice/ splash screen using the default 16 colour palette is very unfun.

[dumbbell]

This feature is really cool!

I agree with Ed: vt_core.c is already large and this new code should go to a separate file; IMHO, the splash screen code should be moved as well but that is a different topic.

Furthermore, I would not use the name "beastie" in function and variable names because it could be anything (like the orb).

One question: would it make sense to unify the current splash screen code and the per-CPU logo code? In other word, configure an area for the text (possibly the whole screen if there is nothing to draw, or no area at all if the splash screen takes the whole surface) and draw whatever we want on the remaining space? I feel the splash screen and the per-CPU features are really close.

[cse_cem_gmail_com]

In D2181#43862, @dumbbell wrote:

This feature is really cool!

Thanks!

I agree with Ed: vt_core.c is already large and this new code should go to a separate file; IMHO, the splash screen code should be moved as well but that is a different topic.

Ok.

Furthermore, I would not use the name "beastie" in function and variable names because it could be anything (like the orb).

Ok.

One question: would it make sense to unify the current splash screen code and the per-CPU logo code? In other word, configure an area for the text (possibly the whole screen if there is nothing to draw, or no area at all if the splash screen takes the whole surface) and draw whatever we want on the remaining space? I feel the splash screen and the per-CPU features are really close.

The current splash screen draws over the entire screen and then removes itself after a few seconds. I don't recall what the text drawing stuff is doing at that time. I guess it's similar behavior ("reserve this much screen area for graphics stuff, draw this stuff, erase it after N seconds").

[cse_cem_gmail_com]

Shuffle most new code to a separate file under dev/vt/; mostly use the word 'logo' in place of the word 'beastie' when referring to one of these things.

[cem]

Rebase on r285636. No other changes.

I have a bit now! I assume your silence means you have no problem with the
patch as is. Thanks.

[dumbbell]

Hi Conrad!

Sorry for not getting back to you, I still had no time to test your patch :-(

I have one question about EVT_SYSCTL_INT. Other than that, I see nothing wrong, but again, I didn't try it yet.

sys/dev/vt/vt.h

90 ↗

(On Diff #7025)

I don't understand the difference between VT_SYSCTL_INT and EVT_SYSCTL_INT.

[cem]

Thanks for taking another look. I appreciate it!

sys/dev/vt/vt.h

90 ↗

(On Diff #7025)

The E is for "Exported" or "External". The VT_SYSCTL_INTs have static visibility and can only be accessed in the file they are defined in. (Yes, we could remove static from VT_SYSCTL_INT and add it back, as needed, in front of users, but there are a lot of users and that would be a bigger diff. It could go in first as a separate patch, though.)

Meanwhile, _kern_vt is only visible in vt_core.c and I wanted to add some sysctls below kern.vt to be used in vt_cpulogos.c.

[markj]

This looks good to me modulo some style nits and the question about the sysctls. I'm completely unfamiliar with vt though, so one of the other reviewers should ok this before committing.

The new tunables should also be documented in vt(4), but that can be done in a separate review/commit if you like.

sys/dev/vt/vt.h

90 ↗

(On Diff #7025)

Hm, do these actually need to be sysctls? They're only relevant at boot time if I understand correctly, so I'd think that you could just use TUNABLE_INT in vt_cpulogos.c.

Aside from that, EVT is a strange prefix because VT_/vt_ is the namespace. VT_ESYSCTL_INT would make more sense to me.

176 ↗

(On Diff #7025)

This could be sorted before vt_suspend.

376 ↗

(On Diff #7025)

This should be sorted before the other declarations, I think.

sys/dev/vt/vt_cpulogos.c

27 ↗

(On Diff #7025)

There should be a __FBSDID marker here.

39 ↗

(On Diff #7025)

Is there a reason these declarations aren't under the #ifdef DEV_SPLASH?

Alternately, can this file be made conditional on the "splash" kernel option?

78 ↗

(On Diff #7025)

The vt code seems to consistently use "unsigned int" rather than just "unsigned".

[dumbbell]

sys/dev/vt/vt.h

90 ↗

(On Diff #7025)

Why not remove static from VT_SYSCTL_INT's definition and use a single macro for all sysctls?

[cem]

sys/dev/vt/vt.h

90 ↗

(On Diff #7025)

Hm, do these actually need to be sysctls? They're only relevant at boot time if I understand correctly, so I'd think that you could just use TUNABLE_INT in vt_cpulogos.c.

Is there a preference to not expose tunables as sysctls? Not a big deal.

Why not remove static from VT_SYSCTL_INT's definition and use a single macro for all sysctls?

That should work. I think that's my preferred change here.

176 ↗

(On Diff #7025)

Ok.

376 ↗

(On Diff #7025)

Right.

sys/dev/vt/vt_cpulogos.c

27 ↗

(On Diff #7025)

Hm, ok.

39 ↗

(On Diff #7025)

Alternately, can this file be made conditional on the "splash" kernel option?

I guess so. I forget why I didn't do that originally. I'll give it a spin.

78 ↗

(On Diff #7025)

Will fix.

[cem]

Address CR feedback.

[emaste]

sys/dev/vt/vt.h

86 ↗

(On Diff #7075)

As alluded to earlier: when you're ready I'd suggest committing the static change first in a separate commit.

sys/dev/vt/vt_core.c

120 ↗

(On Diff #7075)

Add static to these?

[cem]

sys/dev/vt/vt.h

86 ↗

(On Diff #7075)

Ok.

sys/dev/vt/vt_core.c

120 ↗

(On Diff #7075)

Sure.

[cem]

Add 'statics'. The static changes to VT_SYSCTL_INTs will be committed in a
separate revision, but I'm uploading the combined patch to phabricator per
Markj's request.