On Mon, Nov 16, 2020 at 11:09:49AM +0100, Daniel Vetter wrote: > On Sat, Nov 14, 2020 at 07:47:16AM -0500, Peilin Ye wrote: > > 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. > > +1 on sunsetting fondata. I think there's a bunch of these in fbcon code, > because the console subsystem is older than the standard "allow drivers to > embed the subsystem struct into their own private struct" subclassing > model. So lots of global arrays indexed by the console id :-/ Yeah, I saw your comments about registered_fb[] :( > We're also trying to start some kind of test suite for fbdev chardev ioctl > interface in the gpu test suite. fbcon tests are maybe more related to > tty/vt, and I have no idea whether anything exists there already. > > But if you want to do some automated testcases for fbcon and there's > absolutely no other home for them, the gpu test suite might be an option > of last resort too: > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-and-validation Oh, I didn't know about igt-gpu-tools, thanks for the pointer! Now, since charcount is taken care of, and font_desc now contains all fields of console_font, I think it is a good time to replace console_font with font_desc in vc_data. Then we can get rid of FNTSUM(), FNTSIZE(), then (finally) REFCOUNT(). I will start working on vc_data, after done enough testing on [5/5], ofc. Thanks, Peilin Ye _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
