[RFC,v1,0/5] VT: Add ability to get font requirements

Message ID cover.1708011391.git.legion@kernel.org
Headers
Series VT: Add ability to get font requirements |

Message

Alexey Gladkov Feb. 15, 2024, 3:37 p.m. UTC
  We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
loaded. No console driver supports tall fonts. Unfortunately, userspace
cannot distinguish the lack of support in the driver from errors in the
font itself. In all cases, EINVAL will be returned.

How about adding another operator to get the supported font size so that
userspace can handle this situation correctly?

I mean something like this proof-of-concept.

---

Alexey Gladkov (5):
  VT: Add KD_FONT_OP_GET_INFO operation
  newport_con: Allow to get max font width and height
  sticon: Allow to get max font width and height
  vgacon: Allow to get max font width and height
  fbcon: Allow to get max font width and height

 drivers/tty/vt/vt.c                 | 27 +++++++++++++++++++++++++++
 drivers/tty/vt/vt_ioctl.c           |  2 +-
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 drivers/video/console/sticon.c      | 21 +++++++++++++++++++--
 drivers/video/console/vgacon.c      | 17 ++++++++++++++++-
 drivers/video/fbdev/core/fbcon.c    | 18 +++++++++++++++++-
 include/linux/console.h             |  1 +
 include/uapi/linux/kd.h             |  1 +
 8 files changed, 99 insertions(+), 9 deletions(-)
  

Comments

Jiri Slaby Feb. 16, 2024, 7:21 a.m. UTC | #1
On 15. 02. 24, 16:37, Alexey Gladkov wrote:
> We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
> loaded. No console driver supports tall fonts.

I thought fbcon can, no? If not, we should likely remove all the 
KD_FONT_OP_SET_TALL checks here and there.

> Unfortunately, userspace
> cannot distinguish the lack of support in the driver from errors in the
> font itself. In all cases, EINVAL will be returned.

Yeah, AFAIR userspace just tries many possibilities and sees what trial 
worked.

> How about adding another operator to get the supported font size so that
> userspace can handle this situation correctly?

The whole font interface is horrid (and we got rid of v1 interface only 
recently). Like (ab)using height = vpitch in KD_FONT_OP_SET_TALL :(.

So perhaps, as a band-aid, this might be fine (note you give no 
opportunity to find out supported vpitch for example). Eventually, we 
need to invent a v3 interface with some better font_op struct with 
reserved fields for future use and so on.

> I mean something like this proof-of-concept.
> 
> ---
> 
> Alexey Gladkov (5):
>    VT: Add KD_FONT_OP_GET_INFO operation
>    newport_con: Allow to get max font width and height
>    sticon: Allow to get max font width and height
>    vgacon: Allow to get max font width and height
>    fbcon: Allow to get max font width and height
> 
>   drivers/tty/vt/vt.c                 | 27 +++++++++++++++++++++++++++
>   drivers/tty/vt/vt_ioctl.c           |  2 +-
>   drivers/video/console/newport_con.c | 21 +++++++++++++++++----
>   drivers/video/console/sticon.c      | 21 +++++++++++++++++++--
>   drivers/video/console/vgacon.c      | 17 ++++++++++++++++-
>   drivers/video/fbdev/core/fbcon.c    | 18 +++++++++++++++++-
>   include/linux/console.h             |  1 +
>   include/uapi/linux/kd.h             |  1 +
>   8 files changed, 99 insertions(+), 9 deletions(-)
>
  
Alexey Gladkov Feb. 16, 2024, 1:26 p.m. UTC | #2
On Fri, Feb 16, 2024 at 08:21:38AM +0100, Jiri Slaby wrote:
> On 15. 02. 24, 16:37, Alexey Gladkov wrote:
> > We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
> > loaded. No console driver supports tall fonts.
> 
> I thought fbcon can, no? If not, we should likely remove all the 
> KD_FONT_OP_SET_TALL checks here and there.

I thought so too until kbd users started trying to use such fonts. A month
after adding KD_FONT_OP_SET_TALL, support for large fonts was turned off
in fbcon:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=2b09d5d364986f724f17001ccfe4126b9b43a0be

But I don't think we need to remove KD_FONT_OP_SET_TALL completely. Maybe
support for large fonts can be fixed.

I suggested GET_INFO to solve the problem in general. Even if there is no
SET_TALL, the problem still remains. For example newport only supports
8x16 fonts.

> > Unfortunately, userspace
> > cannot distinguish the lack of support in the driver from errors in the
> > font itself. In all cases, EINVAL will be returned.
> 
> Yeah, AFAIR userspace just tries many possibilities and sees what trial 
> worked.

Yep. In case of big font, I don’t know how to improve the error message in
setfont. The EINVAL is very confusing for users.

> > How about adding another operator to get the supported font size so that
> > userspace can handle this situation correctly?
> 
> The whole font interface is horrid (and we got rid of v1 interface only 
> recently). Like (ab)using height = vpitch in KD_FONT_OP_SET_TALL :(.

Yep.

> So perhaps, as a band-aid, this might be fine (note you give no 
> opportunity to find out supported vpitch for example).

The vpitch can be 32 or if the height is greater, then vpitch = fnt_height

> Eventually, we need to invent a v3 interface with some better font_op
> struct with reserved fields for future use and so on.

Yes, yes, yes! Can we discuss this, pleeeeese? :)

> 
> > I mean something like this proof-of-concept.
> > 
> > ---
> > 
> > Alexey Gladkov (5):
> >    VT: Add KD_FONT_OP_GET_INFO operation
> >    newport_con: Allow to get max font width and height
> >    sticon: Allow to get max font width and height
> >    vgacon: Allow to get max font width and height
> >    fbcon: Allow to get max font width and height
> > 
> >   drivers/tty/vt/vt.c                 | 27 +++++++++++++++++++++++++++
> >   drivers/tty/vt/vt_ioctl.c           |  2 +-
> >   drivers/video/console/newport_con.c | 21 +++++++++++++++++----
> >   drivers/video/console/sticon.c      | 21 +++++++++++++++++++--
> >   drivers/video/console/vgacon.c      | 17 ++++++++++++++++-
> >   drivers/video/fbdev/core/fbcon.c    | 18 +++++++++++++++++-
> >   include/linux/console.h             |  1 +
> >   include/uapi/linux/kd.h             |  1 +
> >   8 files changed, 99 insertions(+), 9 deletions(-)
> > 
> 
> -- 
> js
> suse labs
>
  
Samuel Thibault Feb. 16, 2024, 1:45 p.m. UTC | #3
Alexey Gladkov, le ven. 16 févr. 2024 14:26:38 +0100, a ecrit:
> On Fri, Feb 16, 2024 at 08:21:38AM +0100, Jiri Slaby wrote:
> > On 15. 02. 24, 16:37, Alexey Gladkov wrote:
> > > We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
> > > loaded. No console driver supports tall fonts.
> > 
> > I thought fbcon can, no? If not, we should likely remove all the 
> > KD_FONT_OP_SET_TALL checks here and there.
> 
> I thought so too until kbd users started trying to use such fonts. A month
> after adding KD_FONT_OP_SET_TALL, support for large fonts was turned off
> in fbcon:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=2b09d5d364986f724f17001ccfe4126b9b43a0be
> 
> But I don't think we need to remove KD_FONT_OP_SET_TALL completely. Maybe
> support for large fonts can be fixed.

Some users *need* it to be fixed, because they need large fonts to be
able to read their screen.

Samuel
  
Alexey Gladkov Feb. 16, 2024, 2:40 p.m. UTC | #4
On Fri, Feb 16, 2024 at 02:45:22PM +0100, Samuel Thibault wrote:
> Alexey Gladkov, le ven. 16 févr. 2024 14:26:38 +0100, a ecrit:
> > On Fri, Feb 16, 2024 at 08:21:38AM +0100, Jiri Slaby wrote:
> > > On 15. 02. 24, 16:37, Alexey Gladkov wrote:
> > > > We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
> > > > loaded. No console driver supports tall fonts.
> > > 
> > > I thought fbcon can, no? If not, we should likely remove all the 
> > > KD_FONT_OP_SET_TALL checks here and there.
> > 
> > I thought so too until kbd users started trying to use such fonts. A month
> > after adding KD_FONT_OP_SET_TALL, support for large fonts was turned off
> > in fbcon:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=2b09d5d364986f724f17001ccfe4126b9b43a0be
> > 
> > But I don't think we need to remove KD_FONT_OP_SET_TALL completely. Maybe
> > support for large fonts can be fixed.
> 
> Some users *need* it to be fixed, because they need large fonts to be
> able to read their screen.

I totally understand that. That's why I don't think it's necessary to
remove or block SET_TALL. And that's also why I added you to the thread.
  
Jiri Slaby Feb. 21, 2024, 7:04 a.m. UTC | #5
Cc Thomas (fbdev/fbcon fadeaway -- search SUSE below)

On 16. 02. 24, 14:26, Alexey Gladkov wrote:
> On Fri, Feb 16, 2024 at 08:21:38AM +0100, Jiri Slaby wrote:
>> On 15. 02. 24, 16:37, Alexey Gladkov wrote:
>>> We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
>>> loaded. No console driver supports tall fonts.
>>
>> I thought fbcon can, no? If not, we should likely remove all the
>> KD_FONT_OP_SET_TALL checks here and there.
> 
> I thought so too until kbd users started trying to use such fonts. A month
> after adding KD_FONT_OP_SET_TALL, support for large fonts was turned off
> in fbcon:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=2b09d5d364986f724f17001ccfe4126b9b43a0be

Hmmm -- it has been a year!

> But I don't think we need to remove KD_FONT_OP_SET_TALL completely. Maybe
> support for large fonts can be fixed.

Either someone needs it so (not necessarily the same) someone fixes it, 
or we remove it.

Note that this whole fbdev is deprecated in favor of (simple)drm. I 
wonder if we want to invest any time to fix this mess at all? Or we 
simply drop the defunct parts now.

I believe SUSE and Fedora (and others very likely too) are currently in 
the process of moving away from fbdev completely at last. I don't know 
the current state, though. Thomas?

>> Eventually, we need to invent a v3 interface with some better font_op
>> struct with reserved fields for future use and so on.
> 
> Yes, yes, yes! Can we discuss this, pleeeeese? :)

You need not discuss at first, propose something ;).

thanks,