[PATCH 0/3] tty/vt: fix various 512 glyph font issues

[PATCH 0/3] tty/vt: fix various 512 glyph font issues

[Zsolt Kajtar]

Recently I've run into an issue where a monochrome fbcon behaved strange
with a 512 glyph font. That's mostly a fbcon problem but will need
changes in vt as well. However this series is not about that yet.

Instead it's about 3 related problems which were found at the same time.
These are specific to vt only and are a problem with vgacon.

These patches were prepared to apply on tty-next.

Zsolt Kajtar (3):
  tty/vt: 8th bit location in vc_uniscr routines
  tty/vt: Prevent 8th bit corruption with soft cursor
  tty/vt: Fix unreadable kernel messages on vgacon

 drivers/tty/vt/vt.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

-- 
2.30.2

[PATCH 1/3] tty/vt: 8th bit location in vc_uniscr routines

[Zsolt Kajtar]

Both vc_uniscr_check and vc_uniscr_copy_line assume that the 8th bit of
glyph is also the 8th bit in the screen buffer. However this is only the
case for fbcon at the moment. Vgacon has it on the 11th and so the
conversion won't work correctly in that case. The patch corrects this
oversight.

Signed-off-by: Zsolt Kajtar <soci@c64.rulez.org>
---
 drivers/tty/vt/vt.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 869261141..c6c931047 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -493,7 +493,7 @@ int vc_uniscr_check(struct vc_data *vc)
 {
 	u32 **uni_lines;
 	unsigned short *p;
-	int x, y, mask;
+	int x, y;
 
 	WARN_CONSOLE_UNLOCKED();
 
@@ -514,11 +514,14 @@ int vc_uniscr_check(struct vc_data *vc)
 	 * unicode content will be available after a complete screen refresh.
 	 */
 	p = (unsigned short *)vc->vc_origin;
-	mask = vc->vc_hi_font_mask | 0xff;
 	for (y = 0; y < vc->vc_rows; y++) {
 		u32 *line = uni_lines[y];
 		for (x = 0; x < vc->vc_cols; x++) {
-			u16 glyph = scr_readw(p++) & mask;
+			u16 w = scr_readw(p++);
+			u16 glyph = w & 0xff;
+
+			if (w & vc->vc_hi_font_mask)
+				glyph |= 0x100;
 			line[x] = inverse_translate(vc, glyph, true);
 		}
 	}
@@ -561,10 +564,13 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
 		 * buffer of its own.
 		 */
 		u16 *p = (u16 *)pos;
-		int mask = vc->vc_hi_font_mask | 0xff;
 		u32 *uni_buf = dest;
 		while (nr--) {
-			u16 glyph = scr_readw(p++) & mask;
+			u16 w = scr_readw(p++);
+			u16 glyph = w & 0xff;
+
+			if (w & vc->vc_hi_font_mask)
+				glyph |= 0x100;
 			*uni_buf++ = inverse_translate(vc, glyph, true);
 		}
 	}
-- 
2.30.2

Re: [PATCH 1/3] tty/vt: 8th bit location in vc_uniscr routines

[Jiri Slaby]

On 29. 08. 25, 21:49, Zsolt Kajtar wrote:
> Both vc_uniscr_check and vc_uniscr_copy_line assume that the 8th bit of
> glyph is also the 8th bit in the screen buffer. However this is only the
> case for fbcon at the moment. Vgacon has it on the 11th and so the
> conversion won't work correctly in that case. The patch corrects this
> oversight.
> 
> Signed-off-by: Zsolt Kajtar <soci@c64.rulez.org>
> ---
>   drivers/tty/vt/vt.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 869261141..c6c931047 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -493,7 +493,7 @@ int vc_uniscr_check(struct vc_data *vc)
>   {
>   	u32 **uni_lines;
>   	unsigned short *p;
> -	int x, y, mask;
> +	int x, y;
>   
>   	WARN_CONSOLE_UNLOCKED();
>   
> @@ -514,11 +514,14 @@ int vc_uniscr_check(struct vc_data *vc)
>   	 * unicode content will be available after a complete screen refresh.
>   	 */
>   	p = (unsigned short *)vc->vc_origin;
> -	mask = vc->vc_hi_font_mask | 0xff;
>   	for (y = 0; y < vc->vc_rows; y++) {
>   		u32 *line = uni_lines[y];
>   		for (x = 0; x < vc->vc_cols; x++) {
> -			u16 glyph = scr_readw(p++) & mask;
> +			u16 w = scr_readw(p++);
> +			u16 glyph = w & 0xff;
> +
> +			if (w & vc->vc_hi_font_mask)
> +				glyph |= 0x100;

This makes sense, but introduce a helper, please.

>   			line[x] = inverse_translate(vc, glyph, true);
>   		}
>   	}
> @@ -561,10 +564,13 @@ void vc_uniscr_copy_line(const struct vc_data *vc, void *dest, bool viewed,
>   		 * buffer of its own.
>   		 */
>   		u16 *p = (u16 *)pos;
> -		int mask = vc->vc_hi_font_mask | 0xff;
>   		u32 *uni_buf = dest;
>   		while (nr--) {
> -			u16 glyph = scr_readw(p++) & mask;
> +			u16 w = scr_readw(p++);
> +			u16 glyph = w & 0xff;
> +
> +			if (w & vc->vc_hi_font_mask)
> +				glyph |= 0x100;

And use here as well.

>   			*uni_buf++ = inverse_translate(vc, glyph, true);
>   		}
>   	}


-- 
js
suse labs

[PATCH 2/3] tty/vt: Prevent 8th bit corruption with soft cursor

[Zsolt Kajtar]

The attributes of the soft cursor are configurable and one would rightly
expect that only the attributes are to be affected. But that's not
guaranteed for a font with 512 glyphs as the 8th bit is in the attribute
byte. This patch makes sure that really only the attribute bits are
changed by the cursor and not the glyph's appearance.

Signed-off-by: Zsolt Kajtar <soci@c64.rulez.org>
---
 drivers/tty/vt/vt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index c6c931047..d54f4d24e 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -823,6 +823,7 @@ static void add_softcursor(struct vc_data *vc)
 {
 	int i = scr_readw((u16 *) vc->vc_pos);
 	u32 type = vc->vc_cursor_type;
+	int mask = vc->vc_hi_font_mask | 0xff;
 
 	if (!(type & CUR_SW))
 		return;
@@ -836,6 +837,7 @@ static void add_softcursor(struct vc_data *vc)
 		i ^= CUR_BG;
 	if ((type & CUR_INVERT_FG_BG) && (i & CUR_FG) == ((i & CUR_BG) >> 4))
 		i ^= CUR_FG;
+	i = (i & ~mask) | (softcursor_original & mask);
 	scr_writew(i, (u16 *)vc->vc_pos);
 	if (con_should_update(vc))
 		con_putc(vc, i, vc->state.y, vc->state.x);
-- 
2.30.2

Re: [PATCH 2/3] tty/vt: Prevent 8th bit corruption with soft cursor

[Jiri Slaby]

On 29. 08. 25, 21:49, Zsolt Kajtar wrote:
> The attributes of the soft cursor are configurable and one would rightly
> expect that only the attributes are to be affected. But that's not
> guaranteed for a font with 512 glyphs as the 8th bit is in the attribute
> byte. This patch makes sure that really only the attribute bits are
> changed by the cursor and not the glyph's appearance.
> 
> Signed-off-by: Zsolt Kajtar <soci@c64.rulez.org>
> ---
>   drivers/tty/vt/vt.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index c6c931047..d54f4d24e 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -823,6 +823,7 @@ static void add_softcursor(struct vc_data *vc)
>   {
>   	int i = scr_readw((u16 *) vc->vc_pos);

It's terrible how the current code uses weird types (int here).

>   	u32 type = vc->vc_cursor_type;
> +	int mask = vc->vc_hi_font_mask | 0xff;

But since you are introducing a new one, use u16, not int. Esp. when it 
is a mask.
>   	if (!(type & CUR_SW))
>   		return;
> @@ -836,6 +837,7 @@ static void add_softcursor(struct vc_data *vc)
>   		i ^= CUR_BG;
>   	if ((type & CUR_INVERT_FG_BG) && (i & CUR_FG) == ((i & CUR_BG) >> 4))
>   		i ^= CUR_FG;
> +	i = (i & ~mask) | (softcursor_original & mask);
>   	scr_writew(i, (u16 *)vc->vc_pos);
>   	if (con_should_update(vc))
>   		con_putc(vc, i, vc->state.y, vc->state.x);

thanks,
-- 
js
suse labs

[PATCH 3/3] tty/vt: Fix unreadable kernel messages on vgacon

[Zsolt Kajtar]

When a 512 glyph font is loaded on vgacon and the bold attributes are in
effect then the kernel console output (printk) becomes unreadable. It is
because the brightness bit (used for bold) is at the same place where
the 8th bit of the glyph index is. This patch adds the missing masking
to ensure the output will be displayed using the lower half of the font.

Signed-off-by: Zsolt Kajtar <soci@c64.rulez.org>
---
 drivers/tty/vt/vt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index d54f4d24e..4c8c87f21 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3356,7 +3356,8 @@ static void vt_console_print(struct console *co, const char *b, unsigned count)
 				continue;
 		}
 		vc_uniscr_putc(vc, c);
-		scr_writew((vc->vc_attr << 8) + c, (unsigned short *)vc->vc_pos);
+		scr_writew(((vc->vc_attr << 8) & ~vc->vc_hi_font_mask) | c,
+			   (unsigned short *)vc->vc_pos);
 		notify_write(vc, c);
 		cnt++;
 		if (vc->state.x == vc->vc_cols - 1) {
-- 
2.30.2

Re: [PATCH 3/3] tty/vt: Fix unreadable kernel messages on vgacon

[Jiri Slaby]

On 29. 08. 25, 21:49, Zsolt Kajtar wrote:
> When a 512 glyph font is loaded on vgacon and the bold attributes are in
> effect then the kernel console output (printk) becomes unreadable. It is
> because the brightness bit (used for bold) is at the same place where
> the 8th bit of the glyph index is. This patch adds the missing masking
> to ensure the output will be displayed using the lower half of the font.
> 
> Signed-off-by: Zsolt Kajtar <soci@c64.rulez.org>

Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

I assume you checked all writes to the gfx mem?

> ---
>   drivers/tty/vt/vt.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index d54f4d24e..4c8c87f21 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3356,7 +3356,8 @@ static void vt_console_print(struct console *co, const char *b, unsigned count)
>   				continue;
>   		}
>   		vc_uniscr_putc(vc, c);
> -		scr_writew((vc->vc_attr << 8) + c, (unsigned short *)vc->vc_pos);
> +		scr_writew(((vc->vc_attr << 8) & ~vc->vc_hi_font_mask) | c,
> +			   (unsigned short *)vc->vc_pos);
>   		notify_write(vc, c);
>   		cnt++;
>   		if (vc->state.x == vc->vc_cols - 1) {


-- 
js
suse labs