On Fri, Nov 13, 2020 at 10:16:33PM +0100, Daniel Vetter wrote:
On Thu, Nov 12, 2020 at 07:02:21AM -0500, Peilin Ye wrote:
Peilin Ye (5): console: Delete unused con_font_copy() callback implementations console: Delete dummy con_font_set() and con_font_default() callback implementations Fonts: Add charcount field to font_desc parisc/sticore: Avoid hard-coding built-in font charcount fbcon: Avoid using FNTCHARCNT() and hard-coded built-in font charcount
Patches all look good to me, if Greg is ok with me applying the entire pile to drm-misc-next I'll do that next week.
On Fri, Nov 13, 2020 at 11:47:51PM +0100, Greg Kroah-Hartman wrote:
Reviewed-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
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?
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?
Thank you, Peilin Ye