On Sat, Nov 14, 2020 at 01:22:22PM +0100, Greg Kroah-Hartman wrote:
On Sat, Nov 14, 2020 at 01:18:06PM +0100, Greg Kroah-Hartman wrote:
On Sat, Nov 14, 2020 at 03:10:21AM -0500, Peilin Ye wrote:
Thanks for reviewing! Questions about the last patch [5/5] though, it depends on the following assumption:
""" For each console `idx`, `vc_cons[idx].d->vc_font.data` and `fb_display[idx].fontdata` always point to the same buffer. """
Is this true? I think it is true by grepping for `fontdata`. I also noticed that fbcon.c is using `vc->vc_font.data` and `p->fontdata` interchangeably, see fbcon_get_requirement():
vc = vc_cons[fg_console].d; [...] p = &fb_display[fg_console]; caps->x = 1 << (vc->vc_font.width - 1); ^^^^^^^^^^^ caps->y = 1 << (vc->vc_font.height - 1); ^^^^^^^^^^^ caps->len = (p->userfont) ? FNTCHARCNT(p->fontdata) : 256; ^^^^^^^^^^^If it is true, then what is the point of using `fontdata` in `struct fbcon_display`? Just for the `userfont` flag? Should we delete `fontdata`, when we no longer need the `userfont` flag?
Yes, after a quick look, I think your analysis here is correct. So if you can delete that, it would be nice if possible.
I see, at the moment we still need `userfont` for refcount handling, I will try to delete both `fontdata` and `userfont` after refcount is taken care of.
In this sense I think [5/5] needs more testing. Do we have test files for fbcon, or should I try to write some tests from scratch?
I don't know of any direct tests, I usually just booted into that mode and saw if everything looked like it did before. There must be some tool that you can use to change the current font, as it seems to happen at boot time on some distros. I can't remember what it's called at the moment...
Ah, here's a hint: https://wiki.archlinux.org/index.php/Linux_console#Fonts
The setfont tool should help you out here.
Oh, I didn't know about this one. I'll go experimenting with it, thank you!
Peilin Ye