diff mbox series

[RFC,v1,3/5] sticon: Allow to get max font width and height

Message ID	0c724c30a36aefe2cb4470182cf868d256fbabcd.1708011391.git.legion@kernel.org
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel+bounces-67214-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161; From: Alexey Gladkov <legion@kernel.org> To: LKML <linux-kernel@vger.kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Jiri Slaby <jirislaby@kernel.org> Subject: [RFC PATCH v1 3/5] sticon: Allow to get max font width and height Date: Thu, 15 Feb 2024 15:37:22 +0000 Message-ID: <0c724c30a36aefe2cb4470182cf868d256fbabcd.1708011391.git.legion@kernel.org> In-Reply-To: <cover.1708011391.git.legion@kernel.org> References: <cover.1708011391.git.legion@kernel.org> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	VT: Add ability to get font requirements \| [RFC,v1,0/5] VT: Add ability to get font requirements [RFC,v1,3/5] sticon: Allow to get max font width and height

Commit Message

Alexey Gladkov Feb. 15, 2024, 3:37 p.m. UTC

  Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/console/sticon.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Comments

Jiri Slaby Feb. 16, 2024, 7:10 a.m. UTC | #1

On 15. 02. 24, 16:37, Alexey Gladkov wrote:
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ---
>   drivers/video/console/sticon.c | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
> index 992a4fa431aa..63368b3ff9c5 100644
> --- a/drivers/video/console/sticon.c
> +++ b/drivers/video/console/sticon.c
> @@ -56,6 +56,11 @@
>   #define BLANK 0
>   static int vga_is_gfx;
>   
> +#define STICON_MIN_FONT_WIDTH 6
> +#define STICON_MIN_FONT_HEIGHT 6
> +#define STICON_MAX_FONT_WIDTH 32
> +#define STICON_MAX_FONT_HEIGHT 32
> +
>   #define STI_DEF_FONT	sticon_sti->font
>   
>   /* borrowed from fbcon.c */
> @@ -180,8 +185,10 @@ static int sticon_set_font(struct vc_data *vc, struct console_font *op,
>   	struct sti_cooked_font *cooked_font;
>   	unsigned char *data = op->data, *p;
>   
> -	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
> -	    || (op->charcount != 256 && op->charcount != 512))
> +	if ((w < STICON_MIN_FONT_WIDTH) || (h < STICON_MIN_FONT_HEIGHT) ||
> +	    (w > STICON_MAX_FONT_WIDTH) || (h > STICON_MAX_FONT_HEIGHT) ||

Will
!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH)
look better?

> +	    (vpitch != 32) ||
> +	    (op->charcount != 256 && op->charcount != 512))
>   		return -EINVAL;
>   	pitch = ALIGN(w, 8) / 8;
>   	bpc = pitch * h;
> @@ -273,6 +280,15 @@ static int sticon_font_set(struct vc_data *vc, struct console_font *font,
>   	return sticon_set_font(vc, font, vpitch);
>   }
>   
> +static int sticon_font_info(struct vc_data *vc, struct console_font *font)
> +{
> +	font->width = STICON_MAX_FONT_WIDTH;
> +	font->height = STICON_MAX_FONT_HEIGHT;
> +	font->charcount = 512;

Why not to have a macro for this constant?

> +
> +	return 0;
> +}
> +
>   static void sticon_init(struct vc_data *c, int init)
>   {
>       struct sti_struct *sti = sticon_sti;
> @@ -371,6 +387,7 @@ static const struct consw sti_con = {
>   	.con_scroll		= sticon_scroll,
>   	.con_switch		= sticon_switch,
>   	.con_blank		= sticon_blank,
> +	.con_font_info		= sticon_font_info,
>   	.con_font_set		= sticon_font_set,
>   	.con_font_default	= sticon_font_default,
>   	.con_build_attr		= sticon_build_attr,

Alexey Gladkov Feb. 16, 2024, 1:03 p.m. UTC | #2

On Fri, Feb 16, 2024 at 08:10:15AM +0100, Jiri Slaby wrote:
> On 15. 02. 24, 16:37, Alexey Gladkov wrote:
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > ---
> >   drivers/video/console/sticon.c | 21 +++++++++++++++++++--
> >   1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
> > index 992a4fa431aa..63368b3ff9c5 100644
> > --- a/drivers/video/console/sticon.c
> > +++ b/drivers/video/console/sticon.c
> > @@ -56,6 +56,11 @@
> >   #define BLANK 0
> >   static int vga_is_gfx;
> >   
> > +#define STICON_MIN_FONT_WIDTH 6
> > +#define STICON_MIN_FONT_HEIGHT 6
> > +#define STICON_MAX_FONT_WIDTH 32
> > +#define STICON_MAX_FONT_HEIGHT 32
> > +
> >   #define STI_DEF_FONT	sticon_sti->font
> >   
> >   /* borrowed from fbcon.c */
> > @@ -180,8 +185,10 @@ static int sticon_set_font(struct vc_data *vc, struct console_font *op,
> >   	struct sti_cooked_font *cooked_font;
> >   	unsigned char *data = op->data, *p;
> >   
> > -	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
> > -	    || (op->charcount != 256 && op->charcount != 512))
> > +	if ((w < STICON_MIN_FONT_WIDTH) || (h < STICON_MIN_FONT_HEIGHT) ||
> > +	    (w > STICON_MAX_FONT_WIDTH) || (h > STICON_MAX_FONT_HEIGHT) ||
> 
> Will
> !in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH)
> look better?

Sure! Much better. I made this patch to illustrate what I mean. I will
finish it if you find the idea itself acceptable.

> > +	    (vpitch != 32) ||
> > +	    (op->charcount != 256 && op->charcount != 512))
> >   		return -EINVAL;
> >   	pitch = ALIGN(w, 8) / 8;
> >   	bpc = pitch * h;
> > @@ -273,6 +280,15 @@ static int sticon_font_set(struct vc_data *vc, struct console_font *font,
> >   	return sticon_set_font(vc, font, vpitch);
> >   }
> >   
> > +static int sticon_font_info(struct vc_data *vc, struct console_font *font)
> > +{
> > +	font->width = STICON_MAX_FONT_WIDTH;
> > +	font->height = STICON_MAX_FONT_HEIGHT;
> > +	font->charcount = 512;
> 
> Why not to have a macro for this constant?

Because I don’t know how to correctly transfer this limit to userspace.
I mean, we have the code:

if (op->charcount != 256 && op->charcount != 512)
	return -EINVAL;

So 512 is not exactly the maximum number of glyphs. And such checks in the
code begin to look very strange.

if (op->charcount != 256 && op->charcount != STICON_MAX_FONT_GLYPHS)
	return -EINVAL;

So I decided not to give the value 512 a name.

In fact, I think this should be passed to userspace as a binary flag.
Something like this:

struct console_font_info {
	unsigned int min_width, min_height;
	unsigned int max_width, max_height;
	unsigned int fnt_flags;
}

enum console_font_flags {
	FNT_FLAG_LOW_SIZE  = 1U << 0, // 256
	FNT_FLAG_HIGH_SIZE = 1U << 1, // 512
};

> > +
> > +	return 0;
> > +}
> > +
> >   static void sticon_init(struct vc_data *c, int init)
> >   {
> >       struct sti_struct *sti = sticon_sti;
> > @@ -371,6 +387,7 @@ static const struct consw sti_con = {
> >   	.con_scroll		= sticon_scroll,
> >   	.con_switch		= sticon_switch,
> >   	.con_blank		= sticon_blank,
> > +	.con_font_info		= sticon_font_info,
> >   	.con_font_set		= sticon_font_set,
> >   	.con_font_default	= sticon_font_default,
> >   	.con_build_attr		= sticon_build_attr,
> 
> -- 
> js
> suse labs
>

diff mbox series

Patch

diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 992a4fa431aa..63368b3ff9c5 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -56,6 +56,11 @@ 
 #define BLANK 0
 static int vga_is_gfx;
 
+#define STICON_MIN_FONT_WIDTH 6
+#define STICON_MIN_FONT_HEIGHT 6
+#define STICON_MAX_FONT_WIDTH 32
+#define STICON_MAX_FONT_HEIGHT 32
+
 #define STI_DEF_FONT	sticon_sti->font
 
 /* borrowed from fbcon.c */
@@ -180,8 +185,10 @@  static int sticon_set_font(struct vc_data *vc, struct console_font *op,
 	struct sti_cooked_font *cooked_font;
 	unsigned char *data = op->data, *p;
 
-	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if ((w < STICON_MIN_FONT_WIDTH) || (h < STICON_MIN_FONT_HEIGHT) ||
+	    (w > STICON_MAX_FONT_WIDTH) || (h > STICON_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) ||
+	    (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 	pitch = ALIGN(w, 8) / 8;
 	bpc = pitch * h;
@@ -273,6 +280,15 @@  static int sticon_font_set(struct vc_data *vc, struct console_font *font,
 	return sticon_set_font(vc, font, vpitch);
 }
 
+static int sticon_font_info(struct vc_data *vc, struct console_font *font)
+{
+	font->width = STICON_MAX_FONT_WIDTH;
+	font->height = STICON_MAX_FONT_HEIGHT;
+	font->charcount = 512;
+
+	return 0;
+}
+
 static void sticon_init(struct vc_data *c, int init)
 {
     struct sti_struct *sti = sticon_sti;
@@ -371,6 +387,7 @@  static const struct consw sti_con = {
 	.con_scroll		= sticon_scroll,
 	.con_switch		= sticon_switch,
 	.con_blank		= sticon_blank,
+	.con_font_info		= sticon_font_info,
 	.con_font_set		= sticon_font_set,
 	.con_font_default	= sticon_font_default,
 	.con_build_attr		= sticon_build_attr,