[PATCHES][RFC] cleaning up asm/vga.h situation

[PATCHES][RFC] cleaning up asm/vga.h situation

[Al Viro]

	asm/vga.h and linux/vt_buffer.h are playing odd games that might
or might not have made sense once upon a time, but currently they are
too convoluted for no good reason.

	vt_buffer.h defines the primitives for access to vt contents -
scr_{read,write,memcpy,memmove,memset}w().  In all cases it's an array of
16bit values - attribute/symbol pairs.	Normally it's just a kmalloc'ed
buffer; there are few exceptions:
	* vgacon.c has the active console contents directly in VRAM;
for such consoles vt.c code will end up accessing that.
	* vgacon.c and mdacon.c use those to access the text-mode screen
image directly in VRAM.

	VRAM is little-endian; for kmalloc'ed buffers the endianness is
up to us - all accesses will be via scr_...  primitives, so in absence
of VGA_CONSOLE and MDA_CONSOLE we can simply keep them host-endian and
turn the accessors into obvious inlines; that's the default.

	The few architectures that might have VGA_CONSOLE or MDA_CONSOLE
may override that.  Overrides are too convoluted - we have 4 macros
(VT_BUF_HAVE_{RW,MEMCPYW,MEMSETW,MEMMOVEW}) asm/vga.h may define to
control what gets overridden.  Situation:
	* alpha: needs separate handling for mem and iomem cases,
overrides everything.
	* arm: fine with defaults.
	* mips: keeps everything little-endian, nothing special needed
for VRAM access.  Overrides everything, replacements for memcpy/memmove
are identical to defaults.
	* powerpc: more than slightly ridiculous - if VGA_CONSOLE or
MDA_CONSOLE is enabled, same as mips, defaults otherwise.  The ridiculous
part is that VGA_CONSOLE is actually impossible to enable there, but
MDA_CONSOLE (that is to say, support of ISA Hercules cards for the second
monitor) *is* possible.  On the boxen with ISA slots, that is...
	* sparc: overrides everything, replacements are all identical to
defaults.  Neither vgacon or mdacon are possible there, so that's fine,
but why bother with overrides in the first place?
	* x86: fine with defaults.

Defaults themselves are also overcomplicated - there's e.g. handling
of the case when we'd overridden read/write, but not memcpy (which
never happens).

	There are several other things in asm/vga.h, in addition
to these overrides.  One is the helpers for byte VRAM accesses -
vga_{read,write}b(), used only by vgacon.c and only to read/modify the
screen font.  Hell knows whether it really needs to be done byte-by-byte;
pointer is always to VRAM.  Incidentally, the loops in there are
just weird - 8192 bytes of iomem read or written one by one, with
cond_resched() after each.  Apparently, qemu with 64 vcpu managed to be
stuck in that for more than a minute...  Another thing is VGA_MAP_MEM()
- finding where VRAM is mapped.  Used by vgacon, mdacon and vga16fb
(the last one - x86-only).

	All of those are provided by asm/vga.h on anything that
might support any of the users, so there's no point keeping them in
asm-generic/vga.h - as the matter of fact, that file can be empty,
which would kill arch/sh/include/asm/vga.h.

	Patch series cleaning that up lives in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.vt_buffer
Individual patches in followups; please, review.

Shortlog:
Al Viro (4):
      vt_buffer.h: get rid of dead code in default scr_...() instances
      asm/vga.h: don't bother with scr_mem{cpy,move}v() unless we need to
      sparc: get rid of asm/vga.h
      empty include/asm-generic/vga.h

Diffstat:
 arch/mips/include/asm/vga.h    |  4 ---
 arch/powerpc/include/asm/vga.h |  5 ----
 arch/sh/include/asm/vga.h      |  7 -----
 arch/sparc/include/asm/vga.h   | 60 ------------------------------------------
 include/asm-generic/vga.h      | 23 +---------------
 include/linux/vt_buffer.h      | 24 -----------------
 6 files changed, 1 insertion(+), 122 deletions(-)
 delete mode 100644 arch/sh/include/asm/vga.h
 delete mode 100644 arch/sparc/include/asm/vga.h

[PATCH 1/4] vt_buffer.h: get rid of dead code in default scr_...() instances

[Al Viro]

Only 4 architectures define VT_BUF_HAVE_RW (alpha, mips, powerpc, sparc)
and all of them define VT_BUF_HAVE_MEM{SET,CPY,MOVE}W.  In other
words, the code under #ifdef VT_BUF_HAVE_RW in default scr_mem...w()
instances won't be compiled anyway.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/vt_buffer.h | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/include/linux/vt_buffer.h b/include/linux/vt_buffer.h
index 919d999a8c1d..b6eeb8cb6070 100644
--- a/include/linux/vt_buffer.h
+++ b/include/linux/vt_buffer.h
@@ -28,45 +28,21 @@
 #ifndef VT_BUF_HAVE_MEMSETW
 static inline void scr_memsetw(u16 *s, u16 c, unsigned int count)
 {
-#ifdef VT_BUF_HAVE_RW
-	count /= 2;
-	while (count--)
-		scr_writew(c, s++);
-#else
 	memset16(s, c, count / 2);
-#endif
 }
 #endif
 
 #ifndef VT_BUF_HAVE_MEMCPYW
 static inline void scr_memcpyw(u16 *d, const u16 *s, unsigned int count)
 {
-#ifdef VT_BUF_HAVE_RW
-	count /= 2;
-	while (count--)
-		scr_writew(scr_readw(s++), d++);
-#else
 	memcpy(d, s, count);
-#endif
 }
 #endif
 
 #ifndef VT_BUF_HAVE_MEMMOVEW
 static inline void scr_memmovew(u16 *d, const u16 *s, unsigned int count)
 {
-#ifdef VT_BUF_HAVE_RW
-	if (d < s)
-		scr_memcpyw(d, s, count);
-	else {
-		count /= 2;
-		d += count;
-		s += count;
-		while (count--)
-			scr_writew(scr_readw(--s), --d);
-	}
-#else
 	memmove(d, s, count);
-#endif
 }
 #endif
 
-- 
2.39.5

[PATCH 2/4] asm/vga.h: don't bother with scr_mem{cpy,move}v() unless we need to

[Al Viro]

... if they are identical to fallbacks, just leave them alone.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/mips/include/asm/vga.h    |  4 ----
 arch/powerpc/include/asm/vga.h |  5 -----
 arch/sparc/include/asm/vga.h   | 16 ----------------
 3 files changed, 25 deletions(-)

diff --git a/arch/mips/include/asm/vga.h b/arch/mips/include/asm/vga.h
index 0136e0366698..491c2b5aeb81 100644
--- a/arch/mips/include/asm/vga.h
+++ b/arch/mips/include/asm/vga.h
@@ -47,10 +47,6 @@ static inline void scr_memsetw(u16 *s, u16 v, unsigned int count)
 	memset16(s, cpu_to_le16(v), count / 2);
 }
 
-#define scr_memcpyw(d, s, c) memcpy(d, s, c)
-#define scr_memmovew(d, s, c) memmove(d, s, c)
-#define VT_BUF_HAVE_MEMCPYW
-#define VT_BUF_HAVE_MEMMOVEW
 #define VT_BUF_HAVE_MEMSETW
 
 #endif /* _ASM_VGA_H */
diff --git a/arch/powerpc/include/asm/vga.h b/arch/powerpc/include/asm/vga.h
index fcf721682a71..f2dc40e1c52a 100644
--- a/arch/powerpc/include/asm/vga.h
+++ b/arch/powerpc/include/asm/vga.h
@@ -40,11 +40,6 @@ static inline void scr_memsetw(u16 *s, u16 v, unsigned int n)
 	memset16(s, cpu_to_le16(v), n / 2);
 }
 
-#define VT_BUF_HAVE_MEMCPYW
-#define VT_BUF_HAVE_MEMMOVEW
-#define scr_memcpyw	memcpy
-#define scr_memmovew	memmove
-
 #endif /* !CONFIG_VGA_CONSOLE && !CONFIG_MDA_CONSOLE */
 
 #ifdef __powerpc64__
diff --git a/arch/sparc/include/asm/vga.h b/arch/sparc/include/asm/vga.h
index 2952d667d936..cc2bc60b4c8b 100644
--- a/arch/sparc/include/asm/vga.h
+++ b/arch/sparc/include/asm/vga.h
@@ -14,8 +14,6 @@
 
 #define VT_BUF_HAVE_RW
 #define VT_BUF_HAVE_MEMSETW
-#define VT_BUF_HAVE_MEMCPYW
-#define VT_BUF_HAVE_MEMMOVEW
 
 #undef scr_writew
 #undef scr_readw
@@ -41,20 +39,6 @@ static inline void scr_memsetw(u16 *p, u16 v, unsigned int n)
 	memset16(p, cpu_to_le16(v), n / 2);
 }
 
-static inline void scr_memcpyw(u16 *d, u16 *s, unsigned int n)
-{
-	BUG_ON((long) d >= 0);
-
-	memcpy(d, s, n);
-}
-
-static inline void scr_memmovew(u16 *d, u16 *s, unsigned int n)
-{
-	BUG_ON((long) d >= 0);
-
-	memmove(d, s, n);
-}
-
 #define VGA_MAP_MEM(x,s) (x)
 
 #endif
-- 
2.39.5

[PATCH 3/4] sparc: get rid of asm/vga.h

[Al Viro]

The only thing we are using it for on sparc is telling vt_buffer.h
to pick what it would pick by default anyway - we are not accessing
any VRAM here...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/sparc/include/asm/vga.h | 44 ------------------------------------
 1 file changed, 44 deletions(-)
 delete mode 100644 arch/sparc/include/asm/vga.h

diff --git a/arch/sparc/include/asm/vga.h b/arch/sparc/include/asm/vga.h
deleted file mode 100644
index cc2bc60b4c8b..000000000000
--- a/arch/sparc/include/asm/vga.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- *	Access to VGA videoram
- *
- *	(c) 1998 Martin Mares <mj@ucw.cz>
- */
-
-#ifndef _LINUX_ASM_VGA_H_
-#define _LINUX_ASM_VGA_H_
-
-#include <linux/bug.h>
-#include <linux/string.h>
-#include <asm/types.h>
-
-#define VT_BUF_HAVE_RW
-#define VT_BUF_HAVE_MEMSETW
-
-#undef scr_writew
-#undef scr_readw
-
-static inline void scr_writew(u16 val, u16 *addr)
-{
-	BUG_ON((long) addr >= 0);
-
-	*addr = val;
-}
-
-static inline u16 scr_readw(const u16 *addr)
-{
-	BUG_ON((long) addr >= 0);
-
-	return *addr;
-}
-
-static inline void scr_memsetw(u16 *p, u16 v, unsigned int n)
-{
-	BUG_ON((long) p >= 0);
-
-	memset16(p, cpu_to_le16(v), n / 2);
-}
-
-#define VGA_MAP_MEM(x,s) (x)
-
-#endif
-- 
2.39.5

empty include/asm-generic/vga.h

[Al Viro]

all places that use anything defined in it (vgacon, mdacon and
vga16fb) are built only on architectures that have all that
stuff in their native asm/vga.h

allows to kill stub asm/vga.h on sh, while we are at it...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/sh/include/asm/vga.h |  7 -------
 include/asm-generic/vga.h | 23 +----------------------
 2 files changed, 1 insertion(+), 29 deletions(-)
 delete mode 100644 arch/sh/include/asm/vga.h

diff --git a/arch/sh/include/asm/vga.h b/arch/sh/include/asm/vga.h
deleted file mode 100644
index 089fbdc6c0b1..000000000000
--- a/arch/sh/include/asm/vga.h
+++ /dev/null
@@ -1,7 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __ASM_SH_VGA_H
-#define __ASM_SH_VGA_H
-
-/* Stupid drivers. */
-
-#endif /* __ASM_SH_VGA_H */
diff --git a/include/asm-generic/vga.h b/include/asm-generic/vga.h
index adf91a783b5c..5dcaf4ae904a 100644
--- a/include/asm-generic/vga.h
+++ b/include/asm-generic/vga.h
@@ -1,25 +1,4 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-/*
- *	Access to VGA videoram
- *
- *	(c) 1998 Martin Mares <mj@ucw.cz>
- */
 #ifndef __ASM_GENERIC_VGA_H
 #define __ASM_GENERIC_VGA_H
-
-/*
- *	On most architectures that support VGA, we can just
- *	recalculate addresses and then access the videoram
- *	directly without any black magic.
- *
- *	Everyone else needs to ioremap the address and use
- *	proper I/O accesses.
- */
-#ifndef VGA_MAP_MEM
-#define VGA_MAP_MEM(x, s) (unsigned long)phys_to_virt(x)
-#endif
-
-#define vga_readb(x) (*(x))
-#define vga_writeb(x, y) (*(y) = (x))
-
-#endif /* _ASM_GENERIC_VGA_H */
+#endif /* __ASM_GENERIC_VGA_H */
-- 
2.39.5

Re: [PATCHES][RFC] cleaning up asm/vga.h situation

[Geert Uytterhoeven]

Hi Al,

On Wed, Nov 6, 2024 at 7:10 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>         * powerpc: more than slightly ridiculous - if VGA_CONSOLE or
> MDA_CONSOLE is enabled, same as mips, defaults otherwise.  The ridiculous
> part is that VGA_CONSOLE is actually impossible to enable there, but
> MDA_CONSOLE (that is to say, support of ISA Hercules cards for the second
> monitor) *is* possible.  On the boxen with ISA slots, that is...

Once upon a time, in the previous century, you could enable VGA_CONSOLE,
run an x86 BIOS emulator, and get a VGA text console on a (second)
"PC style" (aka normal) graphics card in a CHRP PPC box...
With CHRP support being removed, there is no point in resurrecting that.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds