2023-06-29 12:21:51

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 00/12] arch,fbdev: Move screen_info into arch/

The variables screen_info and edid_info provide information about
the system's screen, and possibly EDID data of the connected display.
Both are defined and set by architecture code. But both variables are
declared in non-arch header files. Dependencies are at bease loosely
tracked. To resolve this, move the global state screen_info and its
companion edid_info into arch/. Only declare them on architectures
that define them. List dependencies on the variables in the Kconfig
files. Also clean up the callers.

Patch 1 to 4 resolve a number of unnecessary include statements of
<linux/screen_info.h>. The header should only be included in source
files that access struct screen_info.

Patches 5 to 7 move the declaration of screen_info and edid_info to
<asm-generic/screen_info.h>. Architectures that provide either set
a Kconfig token to enable them.

Patches 8 to 9 make users of screen_info depend on the architecture's
feature.

Finally, patches 10 to 12 rework fbdev's handling of firmware EDID
data to make use of existing helpers and the refactored edid_info.

Tested on x86-64. Built for a variety of platforms.

Future directions: with the patchset in place, it will become possible
to provide screen_info and edid_info only if there are users. Some
architectures do this by testing for CONFIG_VT, CONFIG_DUMMY_CONSOLE,
etc. A more uniform approach would be nice. We should also attempt
to minimize access to the global screen_info as much as possible. To
do so, some drivers, such as efifb and vesafb, would require an update.
The firmware's EDID data could possibly made available outside of fbdev.
For example, the simpledrm and ofdrm drivers could provide such data
to userspace compositors.

Thomas Zimmermann (12):
efi: Do not include <linux/screen_info.h> from EFI header
fbdev/sm712fb: Do not include <linux/screen_info.h>
sysfb: Do not include <linux/screen_info.h> from sysfb header
staging/sm750fb: Do not include <linux/screen_info.h>
arch: Remove trailing whitespaces
arch: Declare screen_info in <asm/screen_info.h>
arch/x86: Declare edid_info in <asm/screen_info.h>
drivers/firmware: Remove trailing whitespaces
drivers: Add dependencies on CONFIG_ARCH_HAS_SCREEN_INFO
fbdev/core: Use fb_is_primary_device() in fb_firmware_edid()
fbdev/core: Protect edid_info with CONFIG_ARCH_HAS_EDID_INFO
fbdev/core: Define empty fb_firmware_edid() in <linux/fb.h>

arch/Kconfig | 12 +++++++
arch/alpha/Kconfig | 1 +
arch/arm/Kconfig | 1 +
arch/arm/kernel/efi.c | 2 ++
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/efi.c | 1 +
arch/csky/Kconfig | 1 +
arch/hexagon/Kconfig | 1 +
arch/ia64/Kconfig | 5 +--
arch/loongarch/Kconfig | 1 +
arch/mips/Kconfig | 1 +
arch/nios2/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/riscv/Kconfig | 1 +
arch/sh/Kconfig | 7 ++--
arch/sparc/Kconfig | 1 +
arch/x86/Kconfig | 2 ++
arch/xtensa/Kconfig | 1 +
drivers/firmware/Kconfig | 3 +-
drivers/firmware/efi/Kconfig | 1 +
drivers/firmware/efi/libstub/efi-stub-entry.c | 2 ++
drivers/firmware/efi/libstub/screen_info.c | 2 ++
drivers/gpu/drm/Kconfig | 1 +
drivers/hv/Kconfig | 1 +
drivers/staging/sm750fb/sm750.c | 1 -
drivers/staging/sm750fb/sm750_accel.c | 1 -
drivers/staging/sm750fb/sm750_cursor.c | 1 -
drivers/staging/sm750fb/sm750_hw.c | 1 -
drivers/video/console/Kconfig | 2 ++
drivers/video/fbdev/Kconfig | 4 +++
drivers/video/fbdev/core/fbmon.c | 34 ++++++-------------
drivers/video/fbdev/i810/i810-i2c.c | 2 +-
drivers/video/fbdev/intelfb/intelfbdrv.c | 2 +-
drivers/video/fbdev/nvidia/nv_i2c.c | 2 +-
drivers/video/fbdev/savage/savagefb-i2c.c | 2 +-
drivers/video/fbdev/sm712fb.c | 9 +++--
include/asm-generic/Kbuild | 1 +
include/asm-generic/screen_info.h | 18 ++++++++++
include/linux/efi.h | 3 +-
include/linux/fb.h | 10 +++++-
include/linux/screen_info.h | 2 +-
include/linux/sysfb.h | 3 +-
include/video/edid.h | 3 --
43 files changed, 105 insertions(+), 47 deletions(-)
create mode 100644 include/asm-generic/screen_info.h


base-commit: d2f0af8472494398a42153684b790b723a79f143
--
2.41.0



2023-06-29 12:22:11

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 12/12] fbdev/core: Define empty fb_firmware_edid() in <linux/fb.h>

Move the empty declaration of fb_firmware_edid() to <linux/fb.h>.
Follow common style and avoid the overhead of exporting the symbol.
No functional changes.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Randy Dunlap <[email protected]>
---
drivers/video/fbdev/core/fbmon.c | 7 +------
include/linux/fb.h | 10 +++++++++-
2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 9ae063021e431..d45bd8a18c2f2 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -1496,13 +1496,8 @@ const unsigned char *fb_firmware_edid(struct fb_info *info)

return edid;
}
-#else
-const unsigned char *fb_firmware_edid(struct fb_info *info)
-{
- return NULL;
-}
-#endif
EXPORT_SYMBOL(fb_firmware_edid);
+#endif

EXPORT_SYMBOL(fb_parse_edid);
EXPORT_SYMBOL(fb_edid_to_monspecs);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 5ffd2223326bf..e949532ffc109 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -761,7 +761,6 @@ extern int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var,
extern int fb_validate_mode(const struct fb_var_screeninfo *var,
struct fb_info *info);
extern int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var);
-extern const unsigned char *fb_firmware_edid(struct fb_info *info);
extern void fb_edid_to_monspecs(unsigned char *edid,
struct fb_monspecs *specs);
extern void fb_destroy_modedb(struct fb_videomode *modedb);
@@ -774,6 +773,15 @@ extern int of_get_fb_videomode(struct device_node *np,
extern int fb_videomode_from_videomode(const struct videomode *vm,
struct fb_videomode *fbmode);

+#if defined(CONFIG_FIRMWARE_EDID)
+const unsigned char *fb_firmware_edid(struct fb_info *info);
+#else
+static inline const unsigned char *fb_firmware_edid(struct fb_info *info)
+{
+ return NULL;
+}
+#endif
+
/* drivers/video/modedb.c */
#define VESA_MODEDB_SIZE 43
#define DMT_SIZE 0x50
--
2.41.0


2023-06-29 12:27:21

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 02/12] fbdev/sm712fb: Do not include <linux/screen_info.h>

Sm712fb's dependency on <linux/screen_info.h> is artificial in that
it only uses struct screen_info for its internals. Replace the use of
struct screen_info with a custom data structure and remove the include
of <linux/screen_info.h>.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Sudip Mukherjee <[email protected]>
Cc: Teddy Wang <[email protected]>
Cc: Helge Deller <[email protected]>
---
drivers/video/fbdev/sm712fb.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
index b7ad3c644e138..f929091da4e77 100644
--- a/drivers/video/fbdev/sm712fb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -27,12 +27,17 @@
#include <linux/uaccess.h>
#include <linux/module.h>
#include <linux/console.h>
-#include <linux/screen_info.h>

#include <linux/pm.h>

#include "sm712.h"

+struct smtcfb_screen_info {
+ u16 lfb_width;
+ u16 lfb_height;
+ u16 lfb_depth;
+};
+
/*
* Private structure
*/
@@ -829,7 +834,7 @@ static const struct modeinit vgamode[] = {
},
};

-static struct screen_info smtc_scr_info;
+static struct smtcfb_screen_info smtc_scr_info;

static char *mode_option;

--
2.41.0


2023-06-29 12:27:21

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 06/12] arch: Declare screen_info in <asm/screen_info.h>

The variable screen_info does not exist on all architectures. Declare
it in <asm-generic/screen_info.h>. All architectures that do declare it
will provide it via <asm/screen_info.h>.

Add the Kconfig token ARCH_HAS_SCREEN_INFO to guard against access on
architectures that don't provide screen_info.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Richard Henderson <[email protected]>
Cc: Ivan Kokshaysky <[email protected]>
Cc: Matt Turner <[email protected]>
Cc: Russell King <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Guo Ren <[email protected]>
Cc: Brian Cain <[email protected]>
Cc: Huacai Chen <[email protected]>
Cc: WANG Xuerui <[email protected]>
Cc: Thomas Bogendoerfer <[email protected]>
Cc: Dinh Nguyen <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Paul Walmsley <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Albert Ou <[email protected]>
Cc: Yoshinori Sato <[email protected]>
Cc: Rich Felker <[email protected]>
Cc: John Paul Adrian Glaubitz <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: [email protected]
Cc: "H. Peter Anvin" <[email protected]>
Cc: Chris Zankel <[email protected]>
Cc: Max Filippov <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Sami Tolvanen <[email protected]>
Cc: Juerg Haefliger <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Anshuman Khandual <[email protected]>
Cc: Niklas Schnelle <[email protected]>
Cc: "Russell King (Oracle)" <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Cc: "Mike Rapoport (IBM)" <[email protected]>
Cc: "Kirill A. Shutemov" <[email protected]>
Cc: Zi Yan <[email protected]>
---
arch/Kconfig | 6 ++++++
arch/alpha/Kconfig | 1 +
arch/arm/Kconfig | 1 +
arch/arm64/Kconfig | 1 +
arch/csky/Kconfig | 1 +
arch/hexagon/Kconfig | 1 +
arch/ia64/Kconfig | 1 +
arch/loongarch/Kconfig | 1 +
arch/mips/Kconfig | 1 +
arch/nios2/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/riscv/Kconfig | 1 +
arch/sh/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/x86/Kconfig | 1 +
arch/xtensa/Kconfig | 1 +
drivers/video/Kconfig | 3 +++
include/asm-generic/Kbuild | 1 +
include/asm-generic/screen_info.h | 12 ++++++++++++
include/linux/screen_info.h | 2 +-
20 files changed, 38 insertions(+), 1 deletion(-)
create mode 100644 include/asm-generic/screen_info.h

diff --git a/arch/Kconfig b/arch/Kconfig
index 205fd23e0cada..2f58293fd7bcb 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1466,6 +1466,12 @@ config ARCH_HAS_NONLEAF_PMD_YOUNG
address translations. Page table walkers that clear the accessed bit
may use this capability to reduce their search space.

+config ARCH_HAS_SCREEN_INFO
+ bool
+ help
+ Selected by architectures that provide a global instance of
+ screen_info.
+
source "kernel/gcov/Kconfig"

source "scripts/gcc-plugins/Kconfig"
diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index a5c2b1aa46b02..d749011d88b14 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -4,6 +4,7 @@ config ALPHA
default y
select ARCH_32BIT_USTAT_F_TINODE
select ARCH_HAS_CURRENT_STACK_POINTER
+ select ARCH_HAS_SCREEN_INFO
select ARCH_MIGHT_HAVE_PC_PARPORT
select ARCH_MIGHT_HAVE_PC_SERIO
select ARCH_NO_PREEMPT
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0fb4b218f6658..a9d01ee67a90e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -15,6 +15,7 @@ config ARM
select ARCH_HAS_MEMBARRIER_SYNC_CORE
select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
select ARCH_HAS_PTE_SPECIAL if ARM_LPAE
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_SETUP_DMA_OPS
select ARCH_HAS_SET_MEMORY
select ARCH_STACKWALK
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 343e1e1cae10a..21addc4715bb3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -36,6 +36,7 @@ config ARM64
select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
select ARCH_HAS_PTE_DEVMAP
select ARCH_HAS_PTE_SPECIAL
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_SETUP_DMA_OPS
select ARCH_HAS_SET_DIRECT_MAP
select ARCH_HAS_SET_MEMORY
diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
index 4df1f8c9d170b..28444e581fc1f 100644
--- a/arch/csky/Kconfig
+++ b/arch/csky/Kconfig
@@ -10,6 +10,7 @@ config CSKY
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_HAS_CURRENT_STACK_POINTER
+ select ARCH_HAS_SCREEN_INFO
select ARCH_INLINE_READ_LOCK if !PREEMPTION
select ARCH_INLINE_READ_LOCK_BH if !PREEMPTION
select ARCH_INLINE_READ_LOCK_IRQ if !PREEMPTION
diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
index 54eadf2651786..cc683c0a43d34 100644
--- a/arch/hexagon/Kconfig
+++ b/arch/hexagon/Kconfig
@@ -5,6 +5,7 @@ comment "Linux Kernel Configuration for Hexagon"
config HEXAGON
def_bool y
select ARCH_32BIT_OFF_T
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select ARCH_NO_PREEMPT
select DMA_GLOBAL_POOL
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index e79f15e32a451..8b1e785e6d53d 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -10,6 +10,7 @@ config IA64
bool
select ARCH_BINFMT_ELF_EXTRA_PHDRS
select ARCH_HAS_DMA_MARK_CLEAN
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_STRNCPY_FROM_USER
select ARCH_HAS_STRNLEN_USER
select ARCH_MIGHT_HAVE_PC_PARPORT
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index d38b066fc931b..6aab2fb7753da 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -13,6 +13,7 @@ config LOONGARCH
select ARCH_HAS_FORTIFY_SOURCE
select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
select ARCH_HAS_PTE_SPECIAL
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_INLINE_READ_LOCK if !PREEMPTION
select ARCH_INLINE_READ_LOCK_BH if !PREEMPTION
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 675a8660cb85a..c0ae09789cb6d 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -10,6 +10,7 @@ config MIPS
select ARCH_HAS_KCOV
select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE if !EVA
select ARCH_HAS_PTE_SPECIAL if !(32BIT && CPU_HAS_RIXI)
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_STRNCPY_FROM_USER
select ARCH_HAS_STRNLEN_USER
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
index e5936417d3cd3..7183eea282212 100644
--- a/arch/nios2/Kconfig
+++ b/arch/nios2/Kconfig
@@ -3,6 +3,7 @@ config NIOS2
def_bool y
select ARCH_32BIT_OFF_T
select ARCH_HAS_DMA_PREP_COHERENT
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_SYNC_DMA_FOR_CPU
select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select ARCH_HAS_DMA_SET_UNCACHED
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index bff5820b7cda1..b1acad3076180 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -148,6 +148,7 @@ config PPC
select ARCH_HAS_PTE_DEVMAP if PPC_BOOK3S_64
select ARCH_HAS_PTE_SPECIAL
select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_SET_MEMORY
select ARCH_HAS_STRICT_KERNEL_RWX if (PPC_BOOK3S || PPC_8xx || 40x) && !HIBERNATION
select ARCH_HAS_STRICT_KERNEL_RWX if PPC_85xx && !HIBERNATION && !RANDOMIZE_BASE
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 5966ad97c30c3..b5a48f8424af9 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -29,6 +29,7 @@ config RISCV
select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
select ARCH_HAS_PMEM_API
select ARCH_HAS_PTE_SPECIAL
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_SET_DIRECT_MAP if MMU
select ARCH_HAS_SET_MEMORY if MMU
select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 04b9550cf0070..001f5149952b4 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -10,6 +10,7 @@ config SUPERH
select ARCH_HAS_GIGANTIC_PAGE
select ARCH_HAS_GCOV_PROFILE_ALL
select ARCH_HAS_PTE_SPECIAL
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HIBERNATION_POSSIBLE if MMU
select ARCH_MIGHT_HAVE_PC_PARPORT
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 8535e19062f65..e4bfb80b48cfe 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -13,6 +13,7 @@ config 64BIT
config SPARC
bool
default y
+ select ARCH_HAS_SCREEN_INFO
select ARCH_MIGHT_HAVE_PC_PARPORT if SPARC64 && PCI
select ARCH_MIGHT_HAVE_PC_SERIO
select DMA_OPS
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 53bab123a8ee4..d7c2bf4ee403d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -91,6 +91,7 @@ config X86
select ARCH_HAS_NONLEAF_PMD_YOUNG if PGTABLE_LEVELS > 2
select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64
select ARCH_HAS_COPY_MC if X86_64
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_SET_MEMORY
select ARCH_HAS_SET_DIRECT_MAP
select ARCH_HAS_STRICT_KERNEL_RWX
diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
index 3c6e5471f025b..c6cbd7459939c 100644
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -8,6 +8,7 @@ config XTENSA
select ARCH_HAS_DMA_PREP_COHERENT if MMU
select ARCH_HAS_GCOV_PROFILE_ALL
select ARCH_HAS_KCOV
+ select ARCH_HAS_SCREEN_INFO
select ARCH_HAS_SYNC_DMA_FOR_CPU if MMU
select ARCH_HAS_SYNC_DMA_FOR_DEVICE if MMU
select ARCH_HAS_DMA_SET_UNCACHED if MMU
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 8b2b9ac37c3df..d4a72bea56be0 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -21,6 +21,9 @@ config STI_CORE
config VIDEO_CMDLINE
bool

+config ARCH_HAS_SCREEN_INFO
+ bool
+
config VIDEO_NOMODESET
bool
default n
diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
index 941be574bbe00..5e5d4158a4b4b 100644
--- a/include/asm-generic/Kbuild
+++ b/include/asm-generic/Kbuild
@@ -47,6 +47,7 @@ mandatory-y += percpu.h
mandatory-y += pgalloc.h
mandatory-y += preempt.h
mandatory-y += rwonce.h
+mandatory-y += screen_info.h
mandatory-y += sections.h
mandatory-y += serial.h
mandatory-y += shmparam.h
diff --git a/include/asm-generic/screen_info.h b/include/asm-generic/screen_info.h
new file mode 100644
index 0000000000000..6fd0e50fabfcd
--- /dev/null
+++ b/include/asm-generic/screen_info.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_GENERIC_SCREEN_INFO_H
+#define _ASM_GENERIC_SCREEN_INFO_H
+
+#include <uapi/linux/screen_info.h>
+
+#if defined(CONFIG_ARCH_HAS_SCREEN_INFO)
+extern struct screen_info screen_info;
+#endif
+
+#endif /* _ASM_GENERIC_SCREEN_INFO_H */
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index eab7081392d50..c764b9a51c24b 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -4,6 +4,6 @@

#include <uapi/linux/screen_info.h>

-extern struct screen_info screen_info;
+#include <asm/screen_info.h>

#endif /* _SCREEN_INFO_H */
--
2.41.0


2023-06-29 12:27:23

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 05/12] arch: Remove trailing whitespaces

Fix coding style. No functional changes.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Yoshinori Sato <[email protected]>
Cc: Rich Felker <[email protected]>
Cc: John Paul Adrian Glaubitz <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: "Kirill A. Shutemov" <[email protected]>
Cc: Anshuman Khandual <[email protected]>
Cc: Niklas Schnelle <[email protected]>
Cc: Zi Yan <[email protected]>
Cc: "Mike Rapoport (IBM)" <[email protected]>
Cc: Peter Zijlstra <[email protected]>
---
arch/ia64/Kconfig | 4 ++--
arch/sh/Kconfig | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 21fa63ce5ffc0..e79f15e32a451 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -260,7 +260,7 @@ config PERMIT_BSP_REMOVE
default n
help
Say Y here if your platform SAL will support removal of BSP with HOTPLUG_CPU
- support.
+ support.

config FORCE_CPEI_RETARGET
bool "Force assumption that CPEI can be re-targeted"
@@ -335,7 +335,7 @@ config IA64_PALINFO
config IA64_MC_ERR_INJECT
tristate "MC error injection support"
help
- Adds support for MC error injection. If enabled, the kernel
+ Adds support for MC error injection. If enabled, the kernel
will provide a sysfs interface for user applications to
call MC error injection PAL procedures to inject various errors.
This is a useful tool for MCA testing.
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 9652d367fc377..04b9550cf0070 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -234,7 +234,7 @@ config CPU_SUBTYPE_SH7201
select CPU_SH2A
select CPU_HAS_FPU
select SYS_SUPPORTS_SH_MTU2
-
+
config CPU_SUBTYPE_SH7203
bool "Support SH7203 processor"
select CPU_SH2A
@@ -496,7 +496,7 @@ config CPU_SUBTYPE_SH7366
endchoice

source "arch/sh/mm/Kconfig"
-
+
source "arch/sh/Kconfig.cpu"

source "arch/sh/boards/Kconfig"
@@ -647,7 +647,7 @@ config GUSA
This is the default implementation for both UP and non-ll/sc
CPUs, and is used by the libc, amongst others.

- For additional information, design information can be found
+ For additional information, design information can be found
in <http://lc.linux.or.jp/lc2002/papers/niibe0919p.pdf>.

This should only be disabled for special cases where alternate
--
2.41.0


2023-06-29 12:27:27

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 03/12] sysfb: Do not include <linux/screen_info.h> from sysfb header

The header file <linux/sysfb.h> does not need anything from
<linux/screen_info.h>. Declare struct screen_info and remove
the include statements.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
---
include/linux/sysfb.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
index c1ef5fc60a3cb..19cb803dd5ecd 100644
--- a/include/linux/sysfb.h
+++ b/include/linux/sysfb.h
@@ -9,7 +9,8 @@

#include <linux/kernel.h>
#include <linux/platform_data/simplefb.h>
-#include <linux/screen_info.h>
+
+struct screen_info;

enum {
M_I17, /* 17-Inch iMac */
--
2.41.0


2023-06-29 12:27:35

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h>

The global variable edid_info contains the firmware's EDID information
as an extension to the regular screen_info on x86. Therefore move it to
<asm/screen_info.h>.

Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on
architectures that don't provide edid_info. Select it on x86.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: [email protected]
Cc: "H. Peter Anvin" <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Sami Tolvanen <[email protected]>
Cc: Juerg Haefliger <[email protected]>
---
arch/Kconfig | 6 ++++++
arch/x86/Kconfig | 1 +
drivers/video/Kconfig | 3 ---
include/asm-generic/screen_info.h | 6 ++++++
include/video/edid.h | 3 ---
5 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 2f58293fd7bcb..ee9866e4df2b0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -1466,6 +1466,12 @@ config ARCH_HAS_NONLEAF_PMD_YOUNG
address translations. Page table walkers that clear the accessed bit
may use this capability to reduce their search space.

+config ARCH_HAS_EDID_INFO
+ bool
+ help
+ Selected by architectures that provide a global instance of
+ edid_info.
+
config ARCH_HAS_SCREEN_INFO
bool
help
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d7c2bf4ee403d..ee81855116be7 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -76,6 +76,7 @@ config X86
select ARCH_HAS_DEBUG_VM_PGTABLE if !X86_PAE
select ARCH_HAS_DEVMEM_IS_ALLOWED
select ARCH_HAS_EARLY_DEBUG if KGDB
+ select ARCH_HAS_EDID_INFO
select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_FAST_MULTIPLIER
select ARCH_HAS_FORTIFY_SOURCE
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d4a72bea56be0..8b2b9ac37c3df 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -21,9 +21,6 @@ config STI_CORE
config VIDEO_CMDLINE
bool

-config ARCH_HAS_SCREEN_INFO
- bool
-
config VIDEO_NOMODESET
bool
default n
diff --git a/include/asm-generic/screen_info.h b/include/asm-generic/screen_info.h
index 6fd0e50fabfcd..5efc99c296b40 100644
--- a/include/asm-generic/screen_info.h
+++ b/include/asm-generic/screen_info.h
@@ -5,6 +5,12 @@

#include <uapi/linux/screen_info.h>

+#include <video/edid.h>
+
+#if defined(CONFIG_ARCH_HAS_EDID_INFO)
+extern struct edid_info edid_info;
+#endif
+
#if defined(CONFIG_ARCH_HAS_SCREEN_INFO)
extern struct screen_info screen_info;
#endif
diff --git a/include/video/edid.h b/include/video/edid.h
index f614371e9116a..52aabb7060321 100644
--- a/include/video/edid.h
+++ b/include/video/edid.h
@@ -4,7 +4,4 @@

#include <uapi/video/edid.h>

-#ifdef CONFIG_X86
-extern struct edid_info edid_info;
-#endif
#endif /* __linux_video_edid_h__ */
--
2.41.0


2023-06-29 12:27:36

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 10/12] fbdev/core: Use fb_is_primary_device() in fb_firmware_edid()

Detect the primary VGA device with fb_is_primary_device() in the
implementation of fb_firmware_edid(). Remove the existing code.

Adapt the function to receive an instance of struct fb_info and
update all callers.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Antonino Daplas <[email protected]>
Cc: Maik Broemme <[email protected]>
Cc: Randy Dunlap <[email protected]>
---
drivers/video/fbdev/core/fbmon.c | 25 +++++++----------------
drivers/video/fbdev/i810/i810-i2c.c | 2 +-
drivers/video/fbdev/intelfb/intelfbdrv.c | 2 +-
drivers/video/fbdev/nvidia/nv_i2c.c | 2 +-
drivers/video/fbdev/savage/savagefb-i2c.c | 2 +-
include/linux/fb.h | 2 +-
6 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 79e5bfbdd34c2..35be4431f649a 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -28,7 +28,6 @@
*/
#include <linux/fb.h>
#include <linux/module.h>
-#include <linux/pci.h>
#include <linux/slab.h>
#include <video/edid.h>
#include <video/of_videomode.h>
@@ -1482,31 +1481,21 @@ int fb_validate_mode(const struct fb_var_screeninfo *var, struct fb_info *info)
}

#if defined(CONFIG_FIRMWARE_EDID) && defined(CONFIG_X86)
-
-/*
- * We need to ensure that the EDID block is only returned for
- * the primary graphics adapter.
- */
-
-const unsigned char *fb_firmware_edid(struct device *device)
+const unsigned char *fb_firmware_edid(struct fb_info *info)
{
- struct pci_dev *dev = NULL;
- struct resource *res = NULL;
unsigned char *edid = NULL;

- if (device)
- dev = to_pci_dev(device);
-
- if (dev)
- res = &dev->resource[PCI_ROM_RESOURCE];
-
- if (res && res->flags & IORESOURCE_ROM_SHADOW)
+ /*
+ * We need to ensure that the EDID block is only
+ * returned for the primary graphics adapter.
+ */
+ if (fb_is_primary_device(info))
edid = edid_info.dummy;

return edid;
}
#else
-const unsigned char *fb_firmware_edid(struct device *device)
+const unsigned char *fb_firmware_edid(struct fb_info *info)
{
return NULL;
}
diff --git a/drivers/video/fbdev/i810/i810-i2c.c b/drivers/video/fbdev/i810/i810-i2c.c
index 7db17d0d8a8cf..b605e96620c1f 100644
--- a/drivers/video/fbdev/i810/i810-i2c.c
+++ b/drivers/video/fbdev/i810/i810-i2c.c
@@ -161,7 +161,7 @@ int i810_probe_i2c_connector(struct fb_info *info, u8 **out_edid, int conn)
if (conn < par->ddc_num) {
edid = fb_ddc_read(&par->chan[conn].adapter);
} else {
- const u8 *e = fb_firmware_edid(info->device);
+ const u8 *e = fb_firmware_edid(info);

if (e != NULL) {
DPRINTK("i810-i2c: Getting EDID from BIOS\n");
diff --git a/drivers/video/fbdev/intelfb/intelfbdrv.c b/drivers/video/fbdev/intelfb/intelfbdrv.c
index a81095b2b1ea5..4633a75e3a613 100644
--- a/drivers/video/fbdev/intelfb/intelfbdrv.c
+++ b/drivers/video/fbdev/intelfb/intelfbdrv.c
@@ -1024,7 +1024,7 @@ static int intelfb_init_var(struct intelfb_info *dinfo)
sizeof(struct fb_var_screeninfo));
msrc = 5;
} else {
- const u8 *edid_s = fb_firmware_edid(&dinfo->pdev->dev);
+ const u8 *edid_s = fb_firmware_edid(dinfo->info);
u8 *edid_d = NULL;

if (edid_s) {
diff --git a/drivers/video/fbdev/nvidia/nv_i2c.c b/drivers/video/fbdev/nvidia/nv_i2c.c
index 0b48965a6420c..632e7d622ebfa 100644
--- a/drivers/video/fbdev/nvidia/nv_i2c.c
+++ b/drivers/video/fbdev/nvidia/nv_i2c.c
@@ -159,7 +159,7 @@ int nvidia_probe_i2c_connector(struct fb_info *info, int conn, u8 **out_edid)

if (!edid && conn == 1) {
/* try to get from firmware */
- const u8 *e = fb_firmware_edid(info->device);
+ const u8 *e = fb_firmware_edid(info);

if (e != NULL)
edid = kmemdup(e, EDID_LENGTH, GFP_KERNEL);
diff --git a/drivers/video/fbdev/savage/savagefb-i2c.c b/drivers/video/fbdev/savage/savagefb-i2c.c
index 80fa87e2ae2ff..cf9c376f76526 100644
--- a/drivers/video/fbdev/savage/savagefb-i2c.c
+++ b/drivers/video/fbdev/savage/savagefb-i2c.c
@@ -227,7 +227,7 @@ int savagefb_probe_i2c_connector(struct fb_info *info, u8 **out_edid)

if (!edid) {
/* try to get from firmware */
- const u8 *e = fb_firmware_edid(info->device);
+ const u8 *e = fb_firmware_edid(info);

if (e)
edid = kmemdup(e, EDID_LENGTH, GFP_KERNEL);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 1d5c13f34b098..5ffd2223326bf 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -761,7 +761,7 @@ extern int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var,
extern int fb_validate_mode(const struct fb_var_screeninfo *var,
struct fb_info *info);
extern int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var);
-extern const unsigned char *fb_firmware_edid(struct device *device);
+extern const unsigned char *fb_firmware_edid(struct fb_info *info);
extern void fb_edid_to_monspecs(unsigned char *edid,
struct fb_monspecs *specs);
extern void fb_destroy_modedb(struct fb_videomode *modedb);
--
2.41.0


2023-06-29 12:27:45

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 09/12] drivers: Add dependencies on CONFIG_ARCH_HAS_SCREEN_INFO

Various files within drivers/ depend on the declaration of the
screen_info variable. Add this information to Kconfig.

In the case of the dummy console, the dependency exists only
on ARM. Ignore it on !ARM platforms.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: "K. Y. Srinivasan" <[email protected]>
Cc: Haiyang Zhang <[email protected]>
Cc: Wei Liu <[email protected]>
Cc: Dexuan Cui <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Javier Martinez Canillas <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Andy Shevchenko <[email protected]>
---
drivers/firmware/Kconfig | 1 +
drivers/firmware/efi/Kconfig | 1 +
drivers/gpu/drm/Kconfig | 1 +
drivers/hv/Kconfig | 1 +
drivers/video/console/Kconfig | 2 ++
drivers/video/fbdev/Kconfig | 4 ++++
6 files changed, 10 insertions(+)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 0432737bbb8b4..65bbf7422f1db 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -228,6 +228,7 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT

config SYSFB
bool
+ depends on ARCH_HAS_SCREEN_INFO
select BOOT_VESA_SUPPORT

config SYSFB_SIMPLEFB
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 043ca31c114eb..963d305421e50 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -225,6 +225,7 @@ config EFI_DISABLE_PCI_DMA
config EFI_EARLYCON
def_bool y
depends on SERIAL_EARLYCON && !ARM && !IA64
+ depends on ARCH_HAS_SCREEN_INFO
select FONT_SUPPORT
select ARCH_USE_MEMREMAP_PROT

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index afb3b2f5f4253..63dfc3391d2ff 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -366,6 +366,7 @@ source "drivers/gpu/drm/sprd/Kconfig"
config DRM_HYPERV
tristate "DRM Support for Hyper-V synthetic video device"
depends on DRM && PCI && MMU && HYPERV
+ depends on ARCH_HAS_SCREEN_INFO
select DRM_KMS_HELPER
select DRM_GEM_SHMEM_HELPER
help
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 00242107d62e0..3730c9b42a9bd 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -6,6 +6,7 @@ config HYPERV
tristate "Microsoft Hyper-V client drivers"
depends on (X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
|| (ACPI && ARM64 && !CPU_BIG_ENDIAN)
+ depends on ARCH_HAS_SCREEN_INFO
select PARAVIRT
select X86_HV_CALLBACK_VECTOR if X86
select OF_EARLY_FLATTREE if OF
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index a2a88d42edf0c..b39e2ae039783 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -10,6 +10,7 @@ config VGA_CONSOLE
depends on !4xx && !PPC_8xx && !SPARC && !M68K && !PARISC && !SUPERH && \
(!ARM || ARCH_FOOTBRIDGE || ARCH_INTEGRATOR || ARCH_NETWINDER) && \
!ARM64 && !ARC && !MICROBLAZE && !OPENRISC && !S390 && !UML
+ depends on ARCH_HAS_SCREEN_INFO
select APERTURE_HELPERS if (DRM || FB || VFIO_PCI_CORE)
default y
help
@@ -48,6 +49,7 @@ config SGI_NEWPORT_CONSOLE

config DUMMY_CONSOLE
bool
+ depends on ARCH_HAS_SCREEN_INFO || !ARM
default y

config DUMMY_CONSOLE_COLUMNS
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index cecf15418632d..e11d85e802bc7 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -655,6 +655,7 @@ config FB_UVESA
config FB_VESA
bool "VESA VGA graphics support"
depends on (FB = y) && X86
+ depends on ARCH_HAS_SCREEN_INFO
select APERTURE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -669,6 +670,7 @@ config FB_VESA
config FB_EFI
bool "EFI-based Framebuffer Support"
depends on (FB = y) && !IA64 && EFI
+ depends on ARCH_HAS_SCREEN_INFO
select APERTURE_HELPERS
select DRM_PANEL_ORIENTATION_QUIRKS
select FB_CFB_FILLRECT
@@ -1089,6 +1091,7 @@ config FB_CARILLO_RANCH
config FB_INTEL
tristate "Intel 830M/845G/852GM/855GM/865G/915G/945G/945GM/965G/965GM support"
depends on FB && PCI && X86 && AGP_INTEL && EXPERT
+ depends on ARCH_HAS_SCREEN_INFO
select FB_MODE_HELPERS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -2185,6 +2188,7 @@ config FB_BROADSHEET
config FB_HYPERV
tristate "Microsoft Hyper-V Synthetic Video support"
depends on FB && HYPERV
+ depends on ARCH_HAS_SCREEN_INFO
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
--
2.41.0


2023-06-29 12:27:56

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 11/12] fbdev/core: Protect edid_info with CONFIG_ARCH_HAS_EDID_INFO

Guard usage of edid_info with CONFIG_ARCH_HAS_EDID_INFO instead
of CONFIG_X86. No functional changes.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Helge Deller <[email protected]>
Cc: Randy Dunlap <[email protected]>
---
drivers/video/fbdev/core/fbmon.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 35be4431f649a..9ae063021e431 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -1480,17 +1480,19 @@ int fb_validate_mode(const struct fb_var_screeninfo *var, struct fb_info *info)
-EINVAL : 0;
}

-#if defined(CONFIG_FIRMWARE_EDID) && defined(CONFIG_X86)
+#if defined(CONFIG_FIRMWARE_EDID)
const unsigned char *fb_firmware_edid(struct fb_info *info)
{
unsigned char *edid = NULL;

+#if defined(CONFIG_ARCH_HAS_EDID_INFO)
/*
* We need to ensure that the EDID block is only
* returned for the primary graphics adapter.
*/
if (fb_is_primary_device(info))
edid = edid_info.dummy;
+#endif

return edid;
}
--
2.41.0


2023-06-29 12:29:00

by Thomas Zimmermann

[permalink] [raw]
Subject: [PATCH 04/12] staging/sm750fb: Do not include <linux/screen_info.h>

The sm750fb driver does not need anything from <linux/screen_info.h>.
Remove the include statements.

Signed-off-by: Thomas Zimmermann <[email protected]>
Cc: Sudip Mukherjee <[email protected]>
Cc: Teddy Wang <[email protected]>
---
drivers/staging/sm750fb/sm750.c | 1 -
drivers/staging/sm750fb/sm750_accel.c | 1 -
drivers/staging/sm750fb/sm750_cursor.c | 1 -
drivers/staging/sm750fb/sm750_hw.c | 1 -
4 files changed, 4 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 55e302a27847d..c260f73cf570a 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -14,7 +14,6 @@
#include <linux/mm_types.h>
#include <linux/vmalloc.h>
#include <linux/pagemap.h>
-#include <linux/screen_info.h>
#include <linux/console.h>

#include "sm750.h"
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 24b9077a634a6..44b9e3fe3a41d 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -14,7 +14,6 @@
#include <linux/pagemap.h>
#include <linux/console.h>
#include <linux/platform_device.h>
-#include <linux/screen_info.h>

#include "sm750.h"
#include "sm750_accel.h"
diff --git a/drivers/staging/sm750fb/sm750_cursor.c b/drivers/staging/sm750fb/sm750_cursor.c
index 43e6f52c2551f..eea4d1bd36ce7 100644
--- a/drivers/staging/sm750fb/sm750_cursor.c
+++ b/drivers/staging/sm750fb/sm750_cursor.c
@@ -14,7 +14,6 @@
#include <linux/pagemap.h>
#include <linux/console.h>
#include <linux/platform_device.h>
-#include <linux/screen_info.h>

#include "sm750.h"
#include "sm750_cursor.h"
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 55cb00e8b0d1c..71247eaf26eef 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -17,7 +17,6 @@
#include <asm/mtrr.h>
#endif
#include <linux/platform_device.h>
-#include <linux/screen_info.h>
#include <linux/sizes.h>

#include "sm750.h"
--
2.41.0


2023-06-29 12:47:18

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h>

On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
> The global variable edid_info contains the firmware's EDID information
> as an extension to the regular screen_info on x86. Therefore move it to
> <asm/screen_info.h>.
>
> Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on
> architectures that don't provide edid_info. Select it on x86.

I'm not sure we need another symbol in addition to
CONFIG_FIRMWARE_EDID. Since all the code behind that
existing symbol is also x86 specific, would it be enough
to just add either 'depends on X86' or 'depends on X86 ||
COMPILE_TEST' there?

Arnd

2023-06-29 13:04:31

by WANG Xuerui

[permalink] [raw]
Subject: Re: [PATCH 06/12] arch: Declare screen_info in <asm/screen_info.h>

On 2023/6/29 19:45, Thomas Zimmermann wrote:
> The variable screen_info does not exist on all architectures. Declare
> it in <asm-generic/screen_info.h>. All architectures that do declare it
> will provide it via <asm/screen_info.h>.
>
> Add the Kconfig token ARCH_HAS_SCREEN_INFO to guard against access on
> architectures that don't provide screen_info.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Richard Henderson <[email protected]>
> Cc: Ivan Kokshaysky <[email protected]>
> Cc: Matt Turner <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Guo Ren <[email protected]>
> Cc: Brian Cain <[email protected]>
> Cc: Huacai Chen <[email protected]>
> Cc: WANG Xuerui <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Dinh Nguyen <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Nicholas Piggin <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: Paul Walmsley <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Albert Ou <[email protected]>
> Cc: Yoshinori Sato <[email protected]>
> Cc: Rich Felker <[email protected]>
> Cc: John Paul Adrian Glaubitz <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: [email protected]
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Chris Zankel <[email protected]>
> Cc: Max Filippov <[email protected]>
> Cc: Helge Deller <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: "Paul E. McKenney" <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Sami Tolvanen <[email protected]>
> Cc: Juerg Haefliger <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Niklas Schnelle <[email protected]>
> Cc: "Russell King (Oracle)" <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Cc: "Mike Rapoport (IBM)" <[email protected]>
> Cc: "Kirill A. Shutemov" <[email protected]>
> Cc: Zi Yan <[email protected]>
> ---
> arch/Kconfig | 6 ++++++
> arch/alpha/Kconfig | 1 +
> arch/arm/Kconfig | 1 +
> arch/arm64/Kconfig | 1 +
> arch/csky/Kconfig | 1 +
> arch/hexagon/Kconfig | 1 +
> arch/ia64/Kconfig | 1 +
> arch/loongarch/Kconfig | 1 +
> arch/mips/Kconfig | 1 +
> arch/nios2/Kconfig | 1 +
> arch/powerpc/Kconfig | 1 +
> arch/riscv/Kconfig | 1 +
> arch/sh/Kconfig | 1 +
> arch/sparc/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> arch/xtensa/Kconfig | 1 +
> drivers/video/Kconfig | 3 +++
> include/asm-generic/Kbuild | 1 +
> include/asm-generic/screen_info.h | 12 ++++++++++++
> include/linux/screen_info.h | 2 +-
> 20 files changed, 38 insertions(+), 1 deletion(-)
> create mode 100644 include/asm-generic/screen_info.h
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 205fd23e0cada..2f58293fd7bcb 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1466,6 +1466,12 @@ config ARCH_HAS_NONLEAF_PMD_YOUNG
> address translations. Page table walkers that clear the accessed bit
> may use this capability to reduce their search space.
>
> +config ARCH_HAS_SCREEN_INFO
> + bool
> + help
> + Selected by architectures that provide a global instance of
> + screen_info.
> +
> source "kernel/gcov/Kconfig"
>
> source "scripts/gcc-plugins/Kconfig"
> [snip]
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index d38b066fc931b..6aab2fb7753da 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -13,6 +13,7 @@ config LOONGARCH
> select ARCH_HAS_FORTIFY_SOURCE
> select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
> select ARCH_HAS_PTE_SPECIAL
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_INLINE_READ_LOCK if !PREEMPTION
> select ARCH_INLINE_READ_LOCK_BH if !PREEMPTION
> [snip]

Acked-by: WANG Xuerui <[email protected]> # loongarch

Thanks!

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/


2023-06-29 13:10:53

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 06/12] arch: Declare screen_info in <asm/screen_info.h>

On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:

> diff --git a/include/asm-generic/screen_info.h
> b/include/asm-generic/screen_info.h
> new file mode 100644
> index 0000000000000..6fd0e50fabfcd
> --- /dev/null
> +++ b/include/asm-generic/screen_info.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_GENERIC_SCREEN_INFO_H
> +#define _ASM_GENERIC_SCREEN_INFO_H
> +
> +#include <uapi/linux/screen_info.h>
> +
> +#if defined(CONFIG_ARCH_HAS_SCREEN_INFO)
> +extern struct screen_info screen_info;
> +#endif
> +
> +#endif /* _ASM_GENERIC_SCREEN_INFO_H */
> diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
> index eab7081392d50..c764b9a51c24b 100644
> --- a/include/linux/screen_info.h
> +++ b/include/linux/screen_info.h
> @@ -4,6 +4,6 @@
>
> #include <uapi/linux/screen_info.h>
>
> -extern struct screen_info screen_info;
> +#include <asm/screen_info.h>
>

What is the purpose of adding a file in asm-generic? If all
architectures use the same generic file, I'd just leave the
declaration in include/linux/. I wouldn't bother adding the
#ifdef either, but I can see how that helps turn a link
error into an earlier compile error.

Arnd

2023-06-29 13:16:13

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h>

Hi

Am 29.06.23 um 14:35 schrieb Arnd Bergmann:
> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>> The global variable edid_info contains the firmware's EDID information
>> as an extension to the regular screen_info on x86. Therefore move it to
>> <asm/screen_info.h>.
>>
>> Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on
>> architectures that don't provide edid_info. Select it on x86.
>
> I'm not sure we need another symbol in addition to
> CONFIG_FIRMWARE_EDID. Since all the code behind that
> existing symbol is also x86 specific, would it be enough
> to just add either 'depends on X86' or 'depends on X86 ||
> COMPILE_TEST' there?

FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO
announces an architecture feature. They do different things.

Right now, ARCH_HAS_EDID_INFO only works on the old BIOS-based VESA
systems. In the future, I want to add support for EDID data from EFI and
OF as well. It would be stored in edid_info. I assume that the new
symbol will become useful then.

Before this patchset, I originally wanted to make use of edid_info in
the simpledrm graphics driver. But I found that the rsp code could use
some work first. Maybe the patchset are already tailored towards the
future changes.

Best regards
Thomas

>
> Arnd

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2023-06-29 13:33:54

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h>

On Thu, Jun 29, 2023, at 15:01, Thomas Zimmermann wrote:
> Am 29.06.23 um 14:35 schrieb Arnd Bergmann:
>> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>>> The global variable edid_info contains the firmware's EDID information
>>> as an extension to the regular screen_info on x86. Therefore move it to
>>> <asm/screen_info.h>.
>>>
>>> Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on
>>> architectures that don't provide edid_info. Select it on x86.
>>
>> I'm not sure we need another symbol in addition to
>> CONFIG_FIRMWARE_EDID. Since all the code behind that
>> existing symbol is also x86 specific, would it be enough
>> to just add either 'depends on X86' or 'depends on X86 ||
>> COMPILE_TEST' there?
>
> FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO
> announces an architecture feature. They do different things.

I still have trouble seeing the difference.

> Right now, ARCH_HAS_EDID_INFO only works on the old BIOS-based VESA
> systems. In the future, I want to add support for EDID data from EFI and
> OF as well. It would be stored in edid_info. I assume that the new
> symbol will become useful then.

I don't see why an OF based system would have the same limitation
as legacy BIOS with supporting only a single monitor, if we need
to have a generic representation of EDID data in DT, that would
probably be in a per device property anyway.

I suppose you could use FIRMWARE_EDID on EFI or OF systems without
the need for a global edid_info structure, but that would not
share any code with the current fb_firmware_edid() function.

Arnd

2023-06-29 13:35:20

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 06/12] arch: Declare screen_info in <asm/screen_info.h>

Hi

Am 29.06.23 um 15:03 schrieb Arnd Bergmann:
> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>
>> diff --git a/include/asm-generic/screen_info.h
>> b/include/asm-generic/screen_info.h
>> new file mode 100644
>> index 0000000000000..6fd0e50fabfcd
>> --- /dev/null
>> +++ b/include/asm-generic/screen_info.h
>> @@ -0,0 +1,12 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef _ASM_GENERIC_SCREEN_INFO_H
>> +#define _ASM_GENERIC_SCREEN_INFO_H
>> +
>> +#include <uapi/linux/screen_info.h>
>> +
>> +#if defined(CONFIG_ARCH_HAS_SCREEN_INFO)
>> +extern struct screen_info screen_info;
>> +#endif
>> +
>> +#endif /* _ASM_GENERIC_SCREEN_INFO_H */
>> diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
>> index eab7081392d50..c764b9a51c24b 100644
>> --- a/include/linux/screen_info.h
>> +++ b/include/linux/screen_info.h
>> @@ -4,6 +4,6 @@
>>
>> #include <uapi/linux/screen_info.h>
>>
>> -extern struct screen_info screen_info;
>> +#include <asm/screen_info.h>
>>
>
> What is the purpose of adding a file in asm-generic? If all
> architectures use the same generic file, I'd just leave the
> declaration in include/linux/. I wouldn't bother adding the

That appears a bit 'un-clean' for something that is defined in
architecture? But OK, I would not object.

> #ifdef either, but I can see how that helps turn a link
> error into an earlier compile error.

Yes, that's intentional. If there's a Kconfig token anyway, we can also
fail early during the build.

Best regards
Thomas

>
> Arnd

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2023-06-29 13:41:49

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 00/12] arch,fbdev: Move screen_info into arch/

On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
> The variables screen_info and edid_info provide information about
> the system's screen, and possibly EDID data of the connected display.
> Both are defined and set by architecture code. But both variables are
> declared in non-arch header files. Dependencies are at bease loosely
> tracked. To resolve this, move the global state screen_info and its
> companion edid_info into arch/. Only declare them on architectures
> that define them. List dependencies on the variables in the Kconfig
> files. Also clean up the callers.
>
> Patch 1 to 4 resolve a number of unnecessary include statements of
> <linux/screen_info.h>. The header should only be included in source
> files that access struct screen_info.
>
> Patches 5 to 7 move the declaration of screen_info and edid_info to
> <asm-generic/screen_info.h>. Architectures that provide either set
> a Kconfig token to enable them.
>
> Patches 8 to 9 make users of screen_info depend on the architecture's
> feature.
>
> Finally, patches 10 to 12 rework fbdev's handling of firmware EDID
> data to make use of existing helpers and the refactored edid_info.
>
> Tested on x86-64. Built for a variety of platforms.

This all looks like a nice cleanup!

> Future directions: with the patchset in place, it will become possible
> to provide screen_info and edid_info only if there are users. Some
> architectures do this by testing for CONFIG_VT, CONFIG_DUMMY_CONSOLE,
> etc. A more uniform approach would be nice. We should also attempt
> to minimize access to the global screen_info as much as possible. To
> do so, some drivers, such as efifb and vesafb, would require an update.
> The firmware's EDID data could possibly made available outside of fbdev.
> For example, the simpledrm and ofdrm drivers could provide such data
> to userspace compositors.

I suspect that most architectures that provide a screen_info only
have this in order to compile the framebuffer drivers, and provide
hardcoded data that does not even reflect any real hardware.

We can probably reduce the number of architectures that do this
a lot, especially if we get EFI out of the picture.

Arnd

2023-06-29 14:44:46

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 00/12] arch,fbdev: Move screen_info into arch/

On Thu, Jun 29, 2023, at 15:31, Arnd Bergmann wrote:
> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>>
>> Future directions: with the patchset in place, it will become possible
>> to provide screen_info and edid_info only if there are users. Some
>> architectures do this by testing for CONFIG_VT, CONFIG_DUMMY_CONSOLE,
>> etc. A more uniform approach would be nice. We should also attempt
>> to minimize access to the global screen_info as much as possible. To
>> do so, some drivers, such as efifb and vesafb, would require an update.
>> The firmware's EDID data could possibly made available outside of fbdev.
>> For example, the simpledrm and ofdrm drivers could provide such data
>> to userspace compositors.
>
> I suspect that most architectures that provide a screen_info only
> have this in order to compile the framebuffer drivers, and provide
> hardcoded data that does not even reflect any real hardware.
>
> We can probably reduce the number of architectures that do this
> a lot, especially if we get EFI out of the picture.

I tried to have another look at who uses what, and here are
some observations:

- EFIFB and hyperv are the only ones that are relevant on modern
systmes, and they are only used on systems using EFI, so they
could use a separate data structure that is defined as part of
drivers/firmware/efi. This would likely mean we don't have to
define a separate screen_info for arm64, loongarch, ia64 and
riscv, and could separate the legacy vgacon/vesafb stuff on
arm32 from the efi side.

- FB_SIS can likely be marked 'depends on X86' like FB_INTEL,
it seems to depend on x86 BOOT_VESA_SUPPORT.

- FB_VGA16 is currently support on powerpc and enabled on
one defconfig (pasemi), which I'm fairly sure is a mistake,
so this could be made x86 specific as well.

- VGA_CONSOLE has a complicated Kconfig dependency list that
lists platforms without VGA support but I think this is better
expressed with a positive list. It looks like csky, hexagon,
nios2 and xtensa should be excluded here and not provide
screen_info.

- arm and mips only need to provide screen_info on machines
with VGA_CONSOLE. On Arm we have a dependency on
footbridge/integrator/netwinder, while on mips the
dependency can be added to the platforms that fill
the info (mips, malta, sibyte, sni).

- DUMMY_CONSOLE only uses screen_info on arm, and this should
likely be limited to the three obsolete machines that
support VGACON. Almost all Arm machines use DT these days
and won't ever fill the screen info dynamically.

Arnd

2023-06-29 14:45:39

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 00/12] arch,fbdev: Move screen_info into arch/

Hi

Am 29.06.23 um 15:31 schrieb Arnd Bergmann:
> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>> The variables screen_info and edid_info provide information about
>> the system's screen, and possibly EDID data of the connected display.
>> Both are defined and set by architecture code. But both variables are
>> declared in non-arch header files. Dependencies are at bease loosely
>> tracked. To resolve this, move the global state screen_info and its
>> companion edid_info into arch/. Only declare them on architectures
>> that define them. List dependencies on the variables in the Kconfig
>> files. Also clean up the callers.
>>
>> Patch 1 to 4 resolve a number of unnecessary include statements of
>> <linux/screen_info.h>. The header should only be included in source
>> files that access struct screen_info.
>>
>> Patches 5 to 7 move the declaration of screen_info and edid_info to
>> <asm-generic/screen_info.h>. Architectures that provide either set
>> a Kconfig token to enable them.
>>
>> Patches 8 to 9 make users of screen_info depend on the architecture's
>> feature.
>>
>> Finally, patches 10 to 12 rework fbdev's handling of firmware EDID
>> data to make use of existing helpers and the refactored edid_info.
>>
>> Tested on x86-64. Built for a variety of platforms.
>
> This all looks like a nice cleanup!

I guess that patches 1 to 4 are uncontroversial and could be landed
quickly. Patches 10 to 12 are probably as well.

>
>> Future directions: with the patchset in place, it will become possible
>> to provide screen_info and edid_info only if there are users. Some
>> architectures do this by testing for CONFIG_VT, CONFIG_DUMMY_CONSOLE,
>> etc. A more uniform approach would be nice. We should also attempt
>> to minimize access to the global screen_info as much as possible. To
>> do so, some drivers, such as efifb and vesafb, would require an update.
>> The firmware's EDID data could possibly made available outside of fbdev.
>> For example, the simpledrm and ofdrm drivers could provide such data
>> to userspace compositors.
>
> I suspect that most architectures that provide a screen_info only
> have this in order to compile the framebuffer drivers, and provide
> hardcoded data that does not even reflect any real hardware.

That's quite possible. Only x86's bootparam and EFI code sets
screen_info from external data. The rest is hardcoded. A number of
architectures protect screen_info with CONFIG_VT, CONFIG_DUMMY_CONSOLE,
etc. In a later patchset, I wanted to change this such that these users
of screen_info would enable the feature via select in their Kconfig.

Do you know the reason for this branch in dummycon:

https://elixir.bootlin.com/linux/v6.4/source/drivers/video/console/dummycon.c#L21

What is special about arm that dummycon uses the screeninfo?

>
> We can probably reduce the number of architectures that do this
> a lot, especially if we get EFI out of the picture.

Can you elaborate?

Best regards
Thomas

>
> Arnd

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2023-06-29 15:12:32

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 00/12] arch,fbdev: Move screen_info into arch/

On Thu, Jun 29, 2023, at 16:15, Thomas Zimmermann wrote:
> Am 29.06.23 um 15:31 schrieb Arnd Bergmann:
>> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>>> Future directions: with the patchset in place, it will become possible
>>> to provide screen_info and edid_info only if there are users. Some
>>> architectures do this by testing for CONFIG_VT, CONFIG_DUMMY_CONSOLE,
>>> etc. A more uniform approach would be nice. We should also attempt
>>> to minimize access to the global screen_info as much as possible. To
>>> do so, some drivers, such as efifb and vesafb, would require an update.
>>> The firmware's EDID data could possibly made available outside of fbdev.
>>> For example, the simpledrm and ofdrm drivers could provide such data
>>> to userspace compositors.
>>
>> I suspect that most architectures that provide a screen_info only
>> have this in order to compile the framebuffer drivers, and provide
>> hardcoded data that does not even reflect any real hardware.
>
> That's quite possible. Only x86's bootparam and EFI code sets
> screen_info from external data. The rest is hardcoded. A number of
> architectures protect screen_info with CONFIG_VT, CONFIG_DUMMY_CONSOLE,
> etc. In a later patchset, I wanted to change this such that these users
> of screen_info would enable the feature via select in their Kconfig.
>
> Do you know the reason for this branch in dummycon:
>
> https://elixir.bootlin.com/linux/v6.4/source/drivers/video/console/dummycon.c#L21
>
> What is special about arm that dummycon uses the screeninfo?

I can only guess myself, but I see that the values are only ever
set from the old ATAGS data, and not from DT on any of the
modern ones, and my interpretation is that this is meant to
match whatever the vga console was set to on the three
platforms that support vgacon.

I see this was added in linux-2.1.111, just before the
corresponding sparc specific hack was removed, but I don't have
patch descriptions from that era. Russell might remember, or know
if that is actually still needed.

Arnd

2023-06-30 08:04:46

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h>

Hi

Am 29.06.23 um 15:21 schrieb Arnd Bergmann:
> On Thu, Jun 29, 2023, at 15:01, Thomas Zimmermann wrote:
>> Am 29.06.23 um 14:35 schrieb Arnd Bergmann:
>>> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>>>> The global variable edid_info contains the firmware's EDID information
>>>> as an extension to the regular screen_info on x86. Therefore move it to
>>>> <asm/screen_info.h>.
>>>>
>>>> Add the Kconfig token ARCH_HAS_EDID_INFO to guard against access on
>>>> architectures that don't provide edid_info. Select it on x86.
>>>
>>> I'm not sure we need another symbol in addition to
>>> CONFIG_FIRMWARE_EDID. Since all the code behind that
>>> existing symbol is also x86 specific, would it be enough
>>> to just add either 'depends on X86' or 'depends on X86 ||
>>> COMPILE_TEST' there?
>>
>> FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO
>> announces an architecture feature. They do different things.
>
> I still have trouble seeing the difference.

The idea here is that ARCH_HAS_ signals the architecture's support for
the feature. Drivers set 'depends on' in their Kconfig.

Another Kconfig token, VIDEO_SCREEN_INFO or FIRMWARE_EDID, would then
actually enable the feature. Drivers select VIDEO_SCREEN_INFO or
FIRMWARE_EDID and the architectures contains code like

#ifdef VIDEO_SCREEN_INFO
struct screen_info screen_info = {
/* set values here */
}
#endif

This allows us to disable code that requires screen_info/edid_info, but
also disable screen_info/edid_info unless such code has been enabled in
the kernel config.

Some architectures currently mimic this by guarding screen_info with
ifdef CONFIG_VT or similar. I'd like to make this more flexible. The
cost of a few more internal Kconfig tokens seems negligible.

>
>> Right now, ARCH_HAS_EDID_INFO only works on the old BIOS-based VESA
>> systems. In the future, I want to add support for EDID data from EFI and
>> OF as well. It would be stored in edid_info. I assume that the new
>> symbol will become useful then.
>
> I don't see why an OF based system would have the same limitation
> as legacy BIOS with supporting only a single monitor, if we need
> to have a generic representation of EDID data in DT, that would
> probably be in a per device property anyway.

Sorry that was my mistake. OF has nothing to do with this.

>
> I suppose you could use FIRMWARE_EDID on EFI or OF systems without
> the need for a global edid_info structure, but that would not
> share any code with the current fb_firmware_edid() function.

The current code is build on top of screen_info and edid_info. I'd
preferably not replace that, if possible.

Best regards
Thomas

>
> Arnd

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2023-06-30 12:20:58

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h>

On Fri, Jun 30, 2023, at 09:46, Thomas Zimmermann wrote:
> Am 29.06.23 um 15:21 schrieb Arnd Bergmann:
>> On Thu, Jun 29, 2023, at 15:01, Thomas Zimmermann wrote:
>>> Am 29.06.23 um 14:35 schrieb Arnd Bergmann:
>>>> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>
>>>
>>> FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO
>>> announces an architecture feature. They do different things.
>>
>> I still have trouble seeing the difference.
>
> The idea here is that ARCH_HAS_ signals the architecture's support for
> the feature. Drivers set 'depends on' in their Kconfig.
>
> Another Kconfig token, VIDEO_SCREEN_INFO or FIRMWARE_EDID, would then
> actually enable the feature. Drivers select VIDEO_SCREEN_INFO or
> FIRMWARE_EDID and the architectures contains code like

Fair enough. In that case, I guess FIRMWARE_EDID will just depend on
ARCH_HAS_EDID_INFO, or possibly "depends on FIRMWARE_EDID || EFI"
after it starts calling into an EFI specific function, right?

> #ifdef VIDEO_SCREEN_INFO
> struct screen_info screen_info = {
> /* set values here */
> }
> #endif
>
> This allows us to disable code that requires screen_info/edid_info, but
> also disable screen_info/edid_info unless such code has been enabled in
> the kernel config.
>
> Some architectures currently mimic this by guarding screen_info with
> ifdef CONFIG_VT or similar. I'd like to make this more flexible. The
> cost of a few more internal Kconfig tokens seems negligible.

I definitely get it for the screen_info, which needs the complexity.
For ARCHARCH_HAS_EDID_INFO I would hope that it's never selected by
anything other than x86, so I would still go with just a dependency
on x86 for simplicity, but I don't mind having the extra symbol if that
keeps it more consistent with how the screen_info is handled.

>> I suppose you could use FIRMWARE_EDID on EFI or OF systems without
>> the need for a global edid_info structure, but that would not
>> share any code with the current fb_firmware_edid() function.
>
> The current code is build on top of screen_info and edid_info. I'd
> preferably not replace that, if possible.

One way I could imagine this looking in the end would be
something like

struct screen_info *fb_screen_info(struct device *dev)
{
struct screen_info *si = NULL;

if (IS_ENABLED(CONFIG_EFI))
si = efi_get_screen_info(dev);

if (IS_ENABLED(CONFIG_ARCH_HAS_SCREEN_INFO) && !si)
si = screen_info;

return si;
}

corresponding to fb_firmware_edid(). With this, any driver
that wants to access screen_info would call this function
instead of using the global pointer, plus either NULL pointer
check or a CONFIG_ARCH_HAS_SCREEN_INFO dependency.

This way we could completely eliminate the global screen_info
on arm64, riscv, and loongarch but still use the efi and
hyperv framebuffer/drm drivers.

Arnd

2023-07-04 16:33:39

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 04/12] staging/sm750fb: Do not include <linux/screen_info.h>

Thomas Zimmermann <[email protected]> writes:

> The sm750fb driver does not need anything from <linux/screen_info.h>.
> Remove the include statements.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Sudip Mukherjee <[email protected]>
> Cc: Teddy Wang <[email protected]>
> ---

Reviewed-by: Javier Martinez Canillas <[email protected]>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2023-07-04 16:36:25

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 02/12] fbdev/sm712fb: Do not include <linux/screen_info.h>

Thomas Zimmermann <[email protected]> writes:

> Sm712fb's dependency on <linux/screen_info.h> is artificial in that
> it only uses struct screen_info for its internals. Replace the use of
> struct screen_info with a custom data structure and remove the include
> of <linux/screen_info.h>.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Sudip Mukherjee <[email protected]>
> Cc: Teddy Wang <[email protected]>
> Cc: Helge Deller <[email protected]>
> ---

Reviewed-by: Javier Martinez Canillas <[email protected]>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2023-07-04 16:37:35

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 03/12] sysfb: Do not include <linux/screen_info.h> from sysfb header

Thomas Zimmermann <[email protected]> writes:

> The header file <linux/sysfb.h> does not need anything from
> <linux/screen_info.h>. Declare struct screen_info and remove
> the include statements.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> ---

Reviewed-by: Javier Martinez Canillas <[email protected]>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2023-07-04 16:38:38

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 05/12] arch: Remove trailing whitespaces

Thomas Zimmermann <[email protected]> writes:

> Fix coding style. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Yoshinori Sato <[email protected]>
> Cc: Rich Felker <[email protected]>
> Cc: John Paul Adrian Glaubitz <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: "Kirill A. Shutemov" <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Niklas Schnelle <[email protected]>
> Cc: Zi Yan <[email protected]>
> Cc: "Mike Rapoport (IBM)" <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> ---

Reviewed-by: Javier Martinez Canillas <[email protected]>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2023-07-05 01:26:27

by Sui Jingfeng

[permalink] [raw]
Subject: Re: [06/12] arch: Declare screen_info in <asm/screen_info.h>

Hi, Thomas


I love your patch, yet after applied your patch, the linux kernel fail
to compile on my LoongArch machine.


```

  CC      arch/loongarch/kernel/efi.o
arch/loongarch/kernel/efi.c: In function ‘init_screen_info’:
arch/loongarch/kernel/efi.c:77:54: error: invalid application of
‘sizeof’ to incomplete type ‘struct screen_info’
   77 |         si = early_memremap(screen_info_table, sizeof(*si));
      |                                                      ^
arch/loongarch/kernel/efi.c:82:9: error: ‘screen_info’ undeclared (first
use in this function)
   82 |         screen_info = *si;
      |         ^~~~~~~~~~~
arch/loongarch/kernel/efi.c:82:9: note: each undeclared identifier is
reported only once for each function it appears in
arch/loongarch/kernel/efi.c:82:23: error: invalid use of undefined type
‘struct screen_info’
   82 |         screen_info = *si;
      |                       ^
arch/loongarch/kernel/efi.c:83:29: error: invalid application of
‘sizeof’ to incomplete type ‘struct screen_info’
   83 |         memset(si, 0, sizeof(*si));
      |                             ^
arch/loongarch/kernel/efi.c:84:34: error: invalid application of
‘sizeof’ to incomplete type ‘struct screen_info’
   84 |         early_memunmap(si, sizeof(*si));
      |                                  ^
make[3]: *** [scripts/Makefile.build:252: arch/loongarch/kernel/efi.o]
Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:494: arch/loongarch/kernel] Error 2
make[1]: *** [scripts/Makefile.build:494: arch/loongarch] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:2026: .] Error 2

```

On 2023/6/29 19:45, Thomas Zimmermann wrote:
> The variable screen_info does not exist on all architectures. Declare
> it in <asm-generic/screen_info.h>. All architectures that do declare it
> will provide it via <asm/screen_info.h>.
>
> Add the Kconfig token ARCH_HAS_SCREEN_INFO to guard against access on
> architectures that don't provide screen_info.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Richard Henderson <[email protected]>
> Cc: Ivan Kokshaysky <[email protected]>
> Cc: Matt Turner <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Guo Ren <[email protected]>
> Cc: Brian Cain <[email protected]>
> Cc: Huacai Chen <[email protected]>
> Cc: WANG Xuerui <[email protected]>
> Cc: Thomas Bogendoerfer <[email protected]>
> Cc: Dinh Nguyen <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Nicholas Piggin <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: Paul Walmsley <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Albert Ou <[email protected]>
> Cc: Yoshinori Sato <[email protected]>
> Cc: Rich Felker <[email protected]>
> Cc: John Paul Adrian Glaubitz <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: [email protected]
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Chris Zankel <[email protected]>
> Cc: Max Filippov <[email protected]>
> Cc: Helge Deller <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: "Paul E. McKenney" <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Sami Tolvanen <[email protected]>
> Cc: Juerg Haefliger <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Niklas Schnelle <[email protected]>
> Cc: "Russell King (Oracle)" <[email protected]>
> Cc: Linus Walleij <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Cc: "Mike Rapoport (IBM)" <[email protected]>
> Cc: "Kirill A. Shutemov" <[email protected]>
> Cc: Zi Yan <[email protected]>
> Acked-by: WANG Xuerui <[email protected]> # loongarch
> ---
> arch/Kconfig | 6 ++++++
> arch/alpha/Kconfig | 1 +
> arch/arm/Kconfig | 1 +
> arch/arm64/Kconfig | 1 +
> arch/csky/Kconfig | 1 +
> arch/hexagon/Kconfig | 1 +
> arch/ia64/Kconfig | 1 +
> arch/loongarch/Kconfig | 1 +
> arch/mips/Kconfig | 1 +
> arch/nios2/Kconfig | 1 +
> arch/powerpc/Kconfig | 1 +
> arch/riscv/Kconfig | 1 +
> arch/sh/Kconfig | 1 +
> arch/sparc/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> arch/xtensa/Kconfig | 1 +
> drivers/video/Kconfig | 3 +++
> include/asm-generic/Kbuild | 1 +
> include/asm-generic/screen_info.h | 12 ++++++++++++
> include/linux/screen_info.h | 2 +-
> 20 files changed, 38 insertions(+), 1 deletion(-)
> create mode 100644 include/asm-generic/screen_info.h
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 205fd23e0cada..2f58293fd7bcb 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -1466,6 +1466,12 @@ config ARCH_HAS_NONLEAF_PMD_YOUNG
> address translations. Page table walkers that clear the accessed bit
> may use this capability to reduce their search space.
>
> +config ARCH_HAS_SCREEN_INFO
> + bool
> + help
> + Selected by architectures that provide a global instance of
> + screen_info.
> +
> source "kernel/gcov/Kconfig"
>
> source "scripts/gcc-plugins/Kconfig"
> diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
> index a5c2b1aa46b02..d749011d88b14 100644
> --- a/arch/alpha/Kconfig
> +++ b/arch/alpha/Kconfig
> @@ -4,6 +4,7 @@ config ALPHA
> default y
> select ARCH_32BIT_USTAT_F_TINODE
> select ARCH_HAS_CURRENT_STACK_POINTER
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_MIGHT_HAVE_PC_PARPORT
> select ARCH_MIGHT_HAVE_PC_SERIO
> select ARCH_NO_PREEMPT
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 0fb4b218f6658..a9d01ee67a90e 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -15,6 +15,7 @@ config ARM
> select ARCH_HAS_MEMBARRIER_SYNC_CORE
> select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
> select ARCH_HAS_PTE_SPECIAL if ARM_LPAE
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_SETUP_DMA_OPS
> select ARCH_HAS_SET_MEMORY
> select ARCH_STACKWALK
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 343e1e1cae10a..21addc4715bb3 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -36,6 +36,7 @@ config ARM64
> select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
> select ARCH_HAS_PTE_DEVMAP
> select ARCH_HAS_PTE_SPECIAL
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_SETUP_DMA_OPS
> select ARCH_HAS_SET_DIRECT_MAP
> select ARCH_HAS_SET_MEMORY
> diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
> index 4df1f8c9d170b..28444e581fc1f 100644
> --- a/arch/csky/Kconfig
> +++ b/arch/csky/Kconfig
> @@ -10,6 +10,7 @@ config CSKY
> select ARCH_USE_QUEUED_RWLOCKS
> select ARCH_USE_QUEUED_SPINLOCKS
> select ARCH_HAS_CURRENT_STACK_POINTER
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_INLINE_READ_LOCK if !PREEMPTION
> select ARCH_INLINE_READ_LOCK_BH if !PREEMPTION
> select ARCH_INLINE_READ_LOCK_IRQ if !PREEMPTION
> diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
> index 54eadf2651786..cc683c0a43d34 100644
> --- a/arch/hexagon/Kconfig
> +++ b/arch/hexagon/Kconfig
> @@ -5,6 +5,7 @@ comment "Linux Kernel Configuration for Hexagon"
> config HEXAGON
> def_bool y
> select ARCH_32BIT_OFF_T
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> select ARCH_NO_PREEMPT
> select DMA_GLOBAL_POOL
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index e79f15e32a451..8b1e785e6d53d 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -10,6 +10,7 @@ config IA64
> bool
> select ARCH_BINFMT_ELF_EXTRA_PHDRS
> select ARCH_HAS_DMA_MARK_CLEAN
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_STRNCPY_FROM_USER
> select ARCH_HAS_STRNLEN_USER
> select ARCH_MIGHT_HAVE_PC_PARPORT
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index d38b066fc931b..6aab2fb7753da 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -13,6 +13,7 @@ config LOONGARCH
> select ARCH_HAS_FORTIFY_SOURCE
> select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
> select ARCH_HAS_PTE_SPECIAL
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_INLINE_READ_LOCK if !PREEMPTION
> select ARCH_INLINE_READ_LOCK_BH if !PREEMPTION
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 675a8660cb85a..c0ae09789cb6d 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -10,6 +10,7 @@ config MIPS
> select ARCH_HAS_KCOV
> select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE if !EVA
> select ARCH_HAS_PTE_SPECIAL if !(32BIT && CPU_HAS_RIXI)
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_STRNCPY_FROM_USER
> select ARCH_HAS_STRNLEN_USER
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
> index e5936417d3cd3..7183eea282212 100644
> --- a/arch/nios2/Kconfig
> +++ b/arch/nios2/Kconfig
> @@ -3,6 +3,7 @@ config NIOS2
> def_bool y
> select ARCH_32BIT_OFF_T
> select ARCH_HAS_DMA_PREP_COHERENT
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_SYNC_DMA_FOR_CPU
> select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> select ARCH_HAS_DMA_SET_UNCACHED
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index bff5820b7cda1..b1acad3076180 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -148,6 +148,7 @@ config PPC
> select ARCH_HAS_PTE_DEVMAP if PPC_BOOK3S_64
> select ARCH_HAS_PTE_SPECIAL
> select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_SET_MEMORY
> select ARCH_HAS_STRICT_KERNEL_RWX if (PPC_BOOK3S || PPC_8xx || 40x) && !HIBERNATION
> select ARCH_HAS_STRICT_KERNEL_RWX if PPC_85xx && !HIBERNATION && !RANDOMIZE_BASE
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 5966ad97c30c3..b5a48f8424af9 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -29,6 +29,7 @@ config RISCV
> select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
> select ARCH_HAS_PMEM_API
> select ARCH_HAS_PTE_SPECIAL
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_SET_DIRECT_MAP if MMU
> select ARCH_HAS_SET_MEMORY if MMU
> select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
> diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
> index 04b9550cf0070..001f5149952b4 100644
> --- a/arch/sh/Kconfig
> +++ b/arch/sh/Kconfig
> @@ -10,6 +10,7 @@ config SUPERH
> select ARCH_HAS_GIGANTIC_PAGE
> select ARCH_HAS_GCOV_PROFILE_ALL
> select ARCH_HAS_PTE_SPECIAL
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> select ARCH_HIBERNATION_POSSIBLE if MMU
> select ARCH_MIGHT_HAVE_PC_PARPORT
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 8535e19062f65..e4bfb80b48cfe 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -13,6 +13,7 @@ config 64BIT
> config SPARC
> bool
> default y
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_MIGHT_HAVE_PC_PARPORT if SPARC64 && PCI
> select ARCH_MIGHT_HAVE_PC_SERIO
> select DMA_OPS
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 53bab123a8ee4..d7c2bf4ee403d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -91,6 +91,7 @@ config X86
> select ARCH_HAS_NONLEAF_PMD_YOUNG if PGTABLE_LEVELS > 2
> select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64
> select ARCH_HAS_COPY_MC if X86_64
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_SET_MEMORY
> select ARCH_HAS_SET_DIRECT_MAP
> select ARCH_HAS_STRICT_KERNEL_RWX
> diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
> index 3c6e5471f025b..c6cbd7459939c 100644
> --- a/arch/xtensa/Kconfig
> +++ b/arch/xtensa/Kconfig
> @@ -8,6 +8,7 @@ config XTENSA
> select ARCH_HAS_DMA_PREP_COHERENT if MMU
> select ARCH_HAS_GCOV_PROFILE_ALL
> select ARCH_HAS_KCOV
> + select ARCH_HAS_SCREEN_INFO
> select ARCH_HAS_SYNC_DMA_FOR_CPU if MMU
> select ARCH_HAS_SYNC_DMA_FOR_DEVICE if MMU
> select ARCH_HAS_DMA_SET_UNCACHED if MMU
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 8b2b9ac37c3df..d4a72bea56be0 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -21,6 +21,9 @@ config STI_CORE
> config VIDEO_CMDLINE
> bool
>
> +config ARCH_HAS_SCREEN_INFO
> + bool
> +
> config VIDEO_NOMODESET
> bool
> default n
> diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
> index 941be574bbe00..5e5d4158a4b4b 100644
> --- a/include/asm-generic/Kbuild
> +++ b/include/asm-generic/Kbuild
> @@ -47,6 +47,7 @@ mandatory-y += percpu.h
> mandatory-y += pgalloc.h
> mandatory-y += preempt.h
> mandatory-y += rwonce.h
> +mandatory-y += screen_info.h
> mandatory-y += sections.h
> mandatory-y += serial.h
> mandatory-y += shmparam.h
> diff --git a/include/asm-generic/screen_info.h b/include/asm-generic/screen_info.h
> new file mode 100644
> index 0000000000000..6fd0e50fabfcd
> --- /dev/null
> +++ b/include/asm-generic/screen_info.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_GENERIC_SCREEN_INFO_H
> +#define _ASM_GENERIC_SCREEN_INFO_H
> +
> +#include <uapi/linux/screen_info.h>
> +
> +#if defined(CONFIG_ARCH_HAS_SCREEN_INFO)
> +extern struct screen_info screen_info;
> +#endif
> +
> +#endif /* _ASM_GENERIC_SCREEN_INFO_H */
> diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
> index eab7081392d50..c764b9a51c24b 100644
> --- a/include/linux/screen_info.h
> +++ b/include/linux/screen_info.h
> @@ -4,6 +4,6 @@
>
> #include <uapi/linux/screen_info.h>
>
> -extern struct screen_info screen_info;
> +#include <asm/screen_info.h>
>
> #endif /* _SCREEN_INFO_H */


2023-07-05 01:42:13

by Sui Jingfeng

[permalink] [raw]
Subject: Re: [05/12] arch: Remove trailing whitespaces


On 2023/6/29 19:45, Thomas Zimmermann wrote:
> Fix coding style. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Yoshinori Sato <[email protected]>
> Cc: Rich Felker <[email protected]>
> Cc: John Paul Adrian Glaubitz <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: "Kirill A. Shutemov" <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Niklas Schnelle <[email protected]>
> Cc: Zi Yan <[email protected]>
> Cc: "Mike Rapoport (IBM)" <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Reviewed-by: Javier Martinez Canillas <[email protected]>

Reviewed-by: Sui Jingfeng <[email protected]>

> ---
> arch/ia64/Kconfig | 4 ++--
> arch/sh/Kconfig | 6 +++---
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index 21fa63ce5ffc0..e79f15e32a451 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -260,7 +260,7 @@ config PERMIT_BSP_REMOVE
> default n
> help
> Say Y here if your platform SAL will support removal of BSP with HOTPLUG_CPU
> - support.
> + support.
>
> config FORCE_CPEI_RETARGET
> bool "Force assumption that CPEI can be re-targeted"
> @@ -335,7 +335,7 @@ config IA64_PALINFO
> config IA64_MC_ERR_INJECT
> tristate "MC error injection support"
> help
> - Adds support for MC error injection. If enabled, the kernel
> + Adds support for MC error injection. If enabled, the kernel
> will provide a sysfs interface for user applications to
> call MC error injection PAL procedures to inject various errors.
> This is a useful tool for MCA testing.
> diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
> index 9652d367fc377..04b9550cf0070 100644
> --- a/arch/sh/Kconfig
> +++ b/arch/sh/Kconfig
> @@ -234,7 +234,7 @@ config CPU_SUBTYPE_SH7201
> select CPU_SH2A
> select CPU_HAS_FPU
> select SYS_SUPPORTS_SH_MTU2
> -
> +
> config CPU_SUBTYPE_SH7203
> bool "Support SH7203 processor"
> select CPU_SH2A
> @@ -496,7 +496,7 @@ config CPU_SUBTYPE_SH7366
> endchoice
>
> source "arch/sh/mm/Kconfig"
> -
> +
> source "arch/sh/Kconfig.cpu"
>
> source "arch/sh/boards/Kconfig"
> @@ -647,7 +647,7 @@ config GUSA
> This is the default implementation for both UP and non-ll/sc
> CPUs, and is used by the libc, amongst others.
>
> - For additional information, design information can be found
> + For additional information, design information can be found
> in <http://lc.linux.or.jp/lc2002/papers/niibe0919p.pdf>.
>
> This should only be disabled for special cases where alternate


2023-07-05 01:43:05

by Sui Jingfeng

[permalink] [raw]
Subject: Re: [03/12] sysfb: Do not include <linux/screen_info.h> from sysfb header


On 2023/6/29 19:45, Thomas Zimmermann wrote:
> The header file <linux/sysfb.h> does not need anything from
> <linux/screen_info.h>. Declare struct screen_info and remove
> the include statements.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Javier Martinez Canillas <[email protected]>
> Reviewed-by: Javier Martinez Canillas <[email protected]>


Reviewed-by: Sui Jingfeng <[email protected]>


> ---
> include/linux/sysfb.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
> index c1ef5fc60a3cb..19cb803dd5ecd 100644
> --- a/include/linux/sysfb.h
> +++ b/include/linux/sysfb.h
> @@ -9,7 +9,8 @@
>
> #include <linux/kernel.h>
> #include <linux/platform_data/simplefb.h>
> -#include <linux/screen_info.h>
> +
> +struct screen_info;
>
> enum {
> M_I17, /* 17-Inch iMac */


2023-07-05 01:44:18

by Sui Jingfeng

[permalink] [raw]
Subject: Re: [11/12] fbdev/core: Protect edid_info with CONFIG_ARCH_HAS_EDID_INFO

Hi,


On 2023/6/29 19:45, Thomas Zimmermann wrote:
> Guard usage of edid_info with CONFIG_ARCH_HAS_EDID_INFO instead
> of CONFIG_X86. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>


Reviewed-by: Sui Jingfeng <[email protected]>


> Cc: Daniel Vetter <[email protected]>
> Cc: Helge Deller <[email protected]>
> Cc: Randy Dunlap <[email protected]>
> ---
> drivers/video/fbdev/core/fbmon.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
> index 35be4431f649a..9ae063021e431 100644
> --- a/drivers/video/fbdev/core/fbmon.c
> +++ b/drivers/video/fbdev/core/fbmon.c
> @@ -1480,17 +1480,19 @@ int fb_validate_mode(const struct fb_var_screeninfo *var, struct fb_info *info)
> -EINVAL : 0;
> }
>
> -#if defined(CONFIG_FIRMWARE_EDID) && defined(CONFIG_X86)
> +#if defined(CONFIG_FIRMWARE_EDID)
> const unsigned char *fb_firmware_edid(struct fb_info *info)
> {
> unsigned char *edid = NULL;
>
> +#if defined(CONFIG_ARCH_HAS_EDID_INFO)
> /*
> * We need to ensure that the EDID block is only
> * returned for the primary graphics adapter.
> */
> if (fb_is_primary_device(info))
> edid = edid_info.dummy;
> +#endif
>
> return edid;
> }


2023-07-05 08:07:58

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [06/12] arch: Declare screen_info in <asm/screen_info.h>

Hi

Am 05.07.23 um 03:21 schrieb Sui Jingfeng:
> Hi, Thomas
>
>
> I love your patch, yet after applied your patch, the linux kernel fail
> to compile on my LoongArch machine.

screen_info is missing. I think this should be solved with your update
to patch 1.

Best regards
Thomas

>
>
> ```
>
>   CC      arch/loongarch/kernel/efi.o
> arch/loongarch/kernel/efi.c: In function ‘init_screen_info’:
> arch/loongarch/kernel/efi.c:77:54: error: invalid application of
> ‘sizeof’ to incomplete type ‘struct screen_info’
>    77 |         si = early_memremap(screen_info_table, sizeof(*si));
>       |                                                      ^
> arch/loongarch/kernel/efi.c:82:9: error: ‘screen_info’ undeclared (first
> use in this function)
>    82 |         screen_info = *si;
>       |         ^~~~~~~~~~~
> arch/loongarch/kernel/efi.c:82:9: note: each undeclared identifier is
> reported only once for each function it appears in
> arch/loongarch/kernel/efi.c:82:23: error: invalid use of undefined type
> ‘struct screen_info’
>    82 |         screen_info = *si;
>       |                       ^
> arch/loongarch/kernel/efi.c:83:29: error: invalid application of
> ‘sizeof’ to incomplete type ‘struct screen_info’
>    83 |         memset(si, 0, sizeof(*si));
>       |                             ^
> arch/loongarch/kernel/efi.c:84:34: error: invalid application of
> ‘sizeof’ to incomplete type ‘struct screen_info’
>    84 |         early_memunmap(si, sizeof(*si));
>       |                                  ^
> make[3]: *** [scripts/Makefile.build:252: arch/loongarch/kernel/efi.o]
> Error 1
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [scripts/Makefile.build:494: arch/loongarch/kernel] Error 2
> make[1]: *** [scripts/Makefile.build:494: arch/loongarch] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:2026: .] Error 2
>
> ```
>
> On 2023/6/29 19:45, Thomas Zimmermann wrote:
>> The variable screen_info does not exist on all architectures. Declare
>> it in <asm-generic/screen_info.h>. All architectures that do declare it
>> will provide it via <asm/screen_info.h>.
>>
>> Add the Kconfig token ARCH_HAS_SCREEN_INFO to guard against access on
>> architectures that don't provide screen_info.
>>
>> Signed-off-by: Thomas Zimmermann <[email protected]>
>> Cc: Richard Henderson <[email protected]>
>> Cc: Ivan Kokshaysky <[email protected]>
>> Cc: Matt Turner <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: Catalin Marinas <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Guo Ren <[email protected]>
>> Cc: Brian Cain <[email protected]>
>> Cc: Huacai Chen <[email protected]>
>> Cc: WANG Xuerui <[email protected]>
>> Cc: Thomas Bogendoerfer <[email protected]>
>> Cc: Dinh Nguyen <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Cc: Nicholas Piggin <[email protected]>
>> Cc: Christophe Leroy <[email protected]>
>> Cc: Paul Walmsley <[email protected]>
>> Cc: Palmer Dabbelt <[email protected]>
>> Cc: Albert Ou <[email protected]>
>> Cc: Yoshinori Sato <[email protected]>
>> Cc: Rich Felker <[email protected]>
>> Cc: John Paul Adrian Glaubitz <[email protected]>
>> Cc: "David S. Miller" <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: Dave Hansen <[email protected]>
>> Cc: [email protected]
>> Cc: "H. Peter Anvin" <[email protected]>
>> Cc: Chris Zankel <[email protected]>
>> Cc: Max Filippov <[email protected]>
>> Cc: Helge Deller <[email protected]>
>> Cc: Arnd Bergmann <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Cc: "Paul E. McKenney" <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Frederic Weisbecker <[email protected]>
>> Cc: Andrew Morton <[email protected]>
>> Cc: Ard Biesheuvel <[email protected]>
>> Cc: Sami Tolvanen <[email protected]>
>> Cc: Juerg Haefliger <[email protected]>
>> Cc: Geert Uytterhoeven <[email protected]>
>> Cc: Anshuman Khandual <[email protected]>
>> Cc: Niklas Schnelle <[email protected]>
>> Cc: "Russell King (Oracle)" <[email protected]>
>> Cc: Linus Walleij <[email protected]>
>> Cc: Sebastian Reichel <[email protected]>
>> Cc: "Mike Rapoport (IBM)" <[email protected]>
>> Cc: "Kirill A. Shutemov" <[email protected]>
>> Cc: Zi Yan <[email protected]>
>> Acked-by: WANG Xuerui <[email protected]> # loongarch
>> ---
>>   arch/Kconfig                      |  6 ++++++
>>   arch/alpha/Kconfig                |  1 +
>>   arch/arm/Kconfig                  |  1 +
>>   arch/arm64/Kconfig                |  1 +
>>   arch/csky/Kconfig                 |  1 +
>>   arch/hexagon/Kconfig              |  1 +
>>   arch/ia64/Kconfig                 |  1 +
>>   arch/loongarch/Kconfig            |  1 +
>>   arch/mips/Kconfig                 |  1 +
>>   arch/nios2/Kconfig                |  1 +
>>   arch/powerpc/Kconfig              |  1 +
>>   arch/riscv/Kconfig                |  1 +
>>   arch/sh/Kconfig                   |  1 +
>>   arch/sparc/Kconfig                |  1 +
>>   arch/x86/Kconfig                  |  1 +
>>   arch/xtensa/Kconfig               |  1 +
>>   drivers/video/Kconfig             |  3 +++
>>   include/asm-generic/Kbuild        |  1 +
>>   include/asm-generic/screen_info.h | 12 ++++++++++++
>>   include/linux/screen_info.h       |  2 +-
>>   20 files changed, 38 insertions(+), 1 deletion(-)
>>   create mode 100644 include/asm-generic/screen_info.h
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index 205fd23e0cada..2f58293fd7bcb 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -1466,6 +1466,12 @@ config ARCH_HAS_NONLEAF_PMD_YOUNG
>>         address translations. Page table walkers that clear the
>> accessed bit
>>         may use this capability to reduce their search space.
>> +config ARCH_HAS_SCREEN_INFO
>> +    bool
>> +    help
>> +      Selected by architectures that provide a global instance of
>> +      screen_info.
>> +
>>   source "kernel/gcov/Kconfig"
>>   source "scripts/gcc-plugins/Kconfig"
>> diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
>> index a5c2b1aa46b02..d749011d88b14 100644
>> --- a/arch/alpha/Kconfig
>> +++ b/arch/alpha/Kconfig
>> @@ -4,6 +4,7 @@ config ALPHA
>>       default y
>>       select ARCH_32BIT_USTAT_F_TINODE
>>       select ARCH_HAS_CURRENT_STACK_POINTER
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_MIGHT_HAVE_PC_PARPORT
>>       select ARCH_MIGHT_HAVE_PC_SERIO
>>       select ARCH_NO_PREEMPT
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 0fb4b218f6658..a9d01ee67a90e 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -15,6 +15,7 @@ config ARM
>>       select ARCH_HAS_MEMBARRIER_SYNC_CORE
>>       select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>       select ARCH_HAS_PTE_SPECIAL if ARM_LPAE
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_SETUP_DMA_OPS
>>       select ARCH_HAS_SET_MEMORY
>>       select ARCH_STACKWALK
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 343e1e1cae10a..21addc4715bb3 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -36,6 +36,7 @@ config ARM64
>>       select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>       select ARCH_HAS_PTE_DEVMAP
>>       select ARCH_HAS_PTE_SPECIAL
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_SETUP_DMA_OPS
>>       select ARCH_HAS_SET_DIRECT_MAP
>>       select ARCH_HAS_SET_MEMORY
>> diff --git a/arch/csky/Kconfig b/arch/csky/Kconfig
>> index 4df1f8c9d170b..28444e581fc1f 100644
>> --- a/arch/csky/Kconfig
>> +++ b/arch/csky/Kconfig
>> @@ -10,6 +10,7 @@ config CSKY
>>       select ARCH_USE_QUEUED_RWLOCKS
>>       select ARCH_USE_QUEUED_SPINLOCKS
>>       select ARCH_HAS_CURRENT_STACK_POINTER
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_INLINE_READ_LOCK if !PREEMPTION
>>       select ARCH_INLINE_READ_LOCK_BH if !PREEMPTION
>>       select ARCH_INLINE_READ_LOCK_IRQ if !PREEMPTION
>> diff --git a/arch/hexagon/Kconfig b/arch/hexagon/Kconfig
>> index 54eadf2651786..cc683c0a43d34 100644
>> --- a/arch/hexagon/Kconfig
>> +++ b/arch/hexagon/Kconfig
>> @@ -5,6 +5,7 @@ comment "Linux Kernel Configuration for Hexagon"
>>   config HEXAGON
>>       def_bool y
>>       select ARCH_32BIT_OFF_T
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_SYNC_DMA_FOR_DEVICE
>>       select ARCH_NO_PREEMPT
>>       select DMA_GLOBAL_POOL
>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
>> index e79f15e32a451..8b1e785e6d53d 100644
>> --- a/arch/ia64/Kconfig
>> +++ b/arch/ia64/Kconfig
>> @@ -10,6 +10,7 @@ config IA64
>>       bool
>>       select ARCH_BINFMT_ELF_EXTRA_PHDRS
>>       select ARCH_HAS_DMA_MARK_CLEAN
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_STRNCPY_FROM_USER
>>       select ARCH_HAS_STRNLEN_USER
>>       select ARCH_MIGHT_HAVE_PC_PARPORT
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index d38b066fc931b..6aab2fb7753da 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -13,6 +13,7 @@ config LOONGARCH
>>       select ARCH_HAS_FORTIFY_SOURCE
>>       select ARCH_HAS_NMI_SAFE_THIS_CPU_OPS
>>       select ARCH_HAS_PTE_SPECIAL
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>>       select ARCH_INLINE_READ_LOCK if !PREEMPTION
>>       select ARCH_INLINE_READ_LOCK_BH if !PREEMPTION
>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>> index 675a8660cb85a..c0ae09789cb6d 100644
>> --- a/arch/mips/Kconfig
>> +++ b/arch/mips/Kconfig
>> @@ -10,6 +10,7 @@ config MIPS
>>       select ARCH_HAS_KCOV
>>       select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE if !EVA
>>       select ARCH_HAS_PTE_SPECIAL if !(32BIT && CPU_HAS_RIXI)
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_STRNCPY_FROM_USER
>>       select ARCH_HAS_STRNLEN_USER
>>       select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>> diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
>> index e5936417d3cd3..7183eea282212 100644
>> --- a/arch/nios2/Kconfig
>> +++ b/arch/nios2/Kconfig
>> @@ -3,6 +3,7 @@ config NIOS2
>>       def_bool y
>>       select ARCH_32BIT_OFF_T
>>       select ARCH_HAS_DMA_PREP_COHERENT
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_SYNC_DMA_FOR_CPU
>>       select ARCH_HAS_SYNC_DMA_FOR_DEVICE
>>       select ARCH_HAS_DMA_SET_UNCACHED
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index bff5820b7cda1..b1acad3076180 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -148,6 +148,7 @@ config PPC
>>       select ARCH_HAS_PTE_DEVMAP        if PPC_BOOK3S_64
>>       select ARCH_HAS_PTE_SPECIAL
>>       select ARCH_HAS_SCALED_CPUTIME        if
>> VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_SET_MEMORY
>>       select ARCH_HAS_STRICT_KERNEL_RWX    if (PPC_BOOK3S || PPC_8xx
>> || 40x) && !HIBERNATION
>>       select ARCH_HAS_STRICT_KERNEL_RWX    if PPC_85xx && !HIBERNATION
>> && !RANDOMIZE_BASE
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 5966ad97c30c3..b5a48f8424af9 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -29,6 +29,7 @@ config RISCV
>>       select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>       select ARCH_HAS_PMEM_API
>>       select ARCH_HAS_PTE_SPECIAL
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_SET_DIRECT_MAP if MMU
>>       select ARCH_HAS_SET_MEMORY if MMU
>>       select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
>> diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
>> index 04b9550cf0070..001f5149952b4 100644
>> --- a/arch/sh/Kconfig
>> +++ b/arch/sh/Kconfig
>> @@ -10,6 +10,7 @@ config SUPERH
>>       select ARCH_HAS_GIGANTIC_PAGE
>>       select ARCH_HAS_GCOV_PROFILE_ALL
>>       select ARCH_HAS_PTE_SPECIAL
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>>       select ARCH_HIBERNATION_POSSIBLE if MMU
>>       select ARCH_MIGHT_HAVE_PC_PARPORT
>> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
>> index 8535e19062f65..e4bfb80b48cfe 100644
>> --- a/arch/sparc/Kconfig
>> +++ b/arch/sparc/Kconfig
>> @@ -13,6 +13,7 @@ config 64BIT
>>   config SPARC
>>       bool
>>       default y
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_MIGHT_HAVE_PC_PARPORT if SPARC64 && PCI
>>       select ARCH_MIGHT_HAVE_PC_SERIO
>>       select DMA_OPS
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 53bab123a8ee4..d7c2bf4ee403d 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -91,6 +91,7 @@ config X86
>>       select ARCH_HAS_NONLEAF_PMD_YOUNG    if PGTABLE_LEVELS > 2
>>       select ARCH_HAS_UACCESS_FLUSHCACHE    if X86_64
>>       select ARCH_HAS_COPY_MC            if X86_64
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_SET_MEMORY
>>       select ARCH_HAS_SET_DIRECT_MAP
>>       select ARCH_HAS_STRICT_KERNEL_RWX
>> diff --git a/arch/xtensa/Kconfig b/arch/xtensa/Kconfig
>> index 3c6e5471f025b..c6cbd7459939c 100644
>> --- a/arch/xtensa/Kconfig
>> +++ b/arch/xtensa/Kconfig
>> @@ -8,6 +8,7 @@ config XTENSA
>>       select ARCH_HAS_DMA_PREP_COHERENT if MMU
>>       select ARCH_HAS_GCOV_PROFILE_ALL
>>       select ARCH_HAS_KCOV
>> +    select ARCH_HAS_SCREEN_INFO
>>       select ARCH_HAS_SYNC_DMA_FOR_CPU if MMU
>>       select ARCH_HAS_SYNC_DMA_FOR_DEVICE if MMU
>>       select ARCH_HAS_DMA_SET_UNCACHED if MMU
>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>> index 8b2b9ac37c3df..d4a72bea56be0 100644
>> --- a/drivers/video/Kconfig
>> +++ b/drivers/video/Kconfig
>> @@ -21,6 +21,9 @@ config STI_CORE
>>   config VIDEO_CMDLINE
>>       bool
>> +config ARCH_HAS_SCREEN_INFO
>> +    bool
>> +
>>   config VIDEO_NOMODESET
>>       bool
>>       default n
>> diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
>> index 941be574bbe00..5e5d4158a4b4b 100644
>> --- a/include/asm-generic/Kbuild
>> +++ b/include/asm-generic/Kbuild
>> @@ -47,6 +47,7 @@ mandatory-y += percpu.h
>>   mandatory-y += pgalloc.h
>>   mandatory-y += preempt.h
>>   mandatory-y += rwonce.h
>> +mandatory-y += screen_info.h
>>   mandatory-y += sections.h
>>   mandatory-y += serial.h
>>   mandatory-y += shmparam.h
>> diff --git a/include/asm-generic/screen_info.h
>> b/include/asm-generic/screen_info.h
>> new file mode 100644
>> index 0000000000000..6fd0e50fabfcd
>> --- /dev/null
>> +++ b/include/asm-generic/screen_info.h
>> @@ -0,0 +1,12 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef _ASM_GENERIC_SCREEN_INFO_H
>> +#define _ASM_GENERIC_SCREEN_INFO_H
>> +
>> +#include <uapi/linux/screen_info.h>
>> +
>> +#if defined(CONFIG_ARCH_HAS_SCREEN_INFO)
>> +extern struct screen_info screen_info;
>> +#endif
>> +
>> +#endif /* _ASM_GENERIC_SCREEN_INFO_H */
>> diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
>> index eab7081392d50..c764b9a51c24b 100644
>> --- a/include/linux/screen_info.h
>> +++ b/include/linux/screen_info.h
>> @@ -4,6 +4,6 @@
>>   #include <uapi/linux/screen_info.h>
>> -extern struct screen_info screen_info;
>> +#include <asm/screen_info.h>
>>   #endif /* _SCREEN_INFO_H */
>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2023-07-05 08:17:38

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 10/12] fbdev/core: Use fb_is_primary_device() in fb_firmware_edid()

Thomas Zimmermann <[email protected]> writes:

> Detect the primary VGA device with fb_is_primary_device() in the
> implementation of fb_firmware_edid(). Remove the existing code.
>

An explanation about why this is possible would be useful here.

> Adapt the function to receive an instance of struct fb_info and
> update all callers.
>

[...]

> -const unsigned char *fb_firmware_edid(struct device *device)
> +const unsigned char *fb_firmware_edid(struct fb_info *info)
> {
> - struct pci_dev *dev = NULL;
> - struct resource *res = NULL;
> unsigned char *edid = NULL;
>
> - if (device)
> - dev = to_pci_dev(device);
> -
> - if (dev)
> - res = &dev->resource[PCI_ROM_RESOURCE];
> -
> - if (res && res->flags & IORESOURCE_ROM_SHADOW)

This open codes what used to be the fb_is_primary_device() logic before
commit 5ca1479cd35d ("fbdev: Simplify fb_is_primary_device for x86").
But now after that commit there is functional change since the ROM
shadowing check would be dropped.

I believe that's OK and Sima explains in their commit message that
vga_default_device() should be enough and the check is redundant.

Still, I think that this change should be documented in your commit
message.

With that change,

Reviewed-by: Javier Martinez Canillas <[email protected]>

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


2023-07-05 08:23:00

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h>

Hi Arnd

Am 30.06.23 um 13:53 schrieb Arnd Bergmann:
> On Fri, Jun 30, 2023, at 09:46, Thomas Zimmermann wrote:
>> Am 29.06.23 um 15:21 schrieb Arnd Bergmann:
>>> On Thu, Jun 29, 2023, at 15:01, Thomas Zimmermann wrote:
>>>> Am 29.06.23 um 14:35 schrieb Arnd Bergmann:
>>>>> On Thu, Jun 29, 2023, at 13:45, Thomas Zimmermann wrote:
>>
>>>>
>>>> FIRMWARE_EDID is a user-selectable feature, while ARCH_HAS_EDID_INFO
>>>> announces an architecture feature. They do different things.
>>>
>>> I still have trouble seeing the difference.
>>
>> The idea here is that ARCH_HAS_ signals the architecture's support for
>> the feature. Drivers set 'depends on' in their Kconfig.
>>
>> Another Kconfig token, VIDEO_SCREEN_INFO or FIRMWARE_EDID, would then
>> actually enable the feature. Drivers select VIDEO_SCREEN_INFO or
>> FIRMWARE_EDID and the architectures contains code like
>
> Fair enough. In that case, I guess FIRMWARE_EDID will just depend on
> ARCH_HAS_EDID_INFO, or possibly "depends on FIRMWARE_EDID || EFI"
> after it starts calling into an EFI specific function, right?
>
>> #ifdef VIDEO_SCREEN_INFO
>> struct screen_info screen_info = {
>> /* set values here */
>> }
>> #endif
>>
>> This allows us to disable code that requires screen_info/edid_info, but
>> also disable screen_info/edid_info unless such code has been enabled in
>> the kernel config.
>>
>> Some architectures currently mimic this by guarding screen_info with
>> ifdef CONFIG_VT or similar. I'd like to make this more flexible. The
>> cost of a few more internal Kconfig tokens seems negligible.
>
> I definitely get it for the screen_info, which needs the complexity.
> For ARCHARCH_HAS_EDID_INFO I would hope that it's never selected by
> anything other than x86, so I would still go with just a dependency
> on x86 for simplicity, but I don't mind having the extra symbol if that
> keeps it more consistent with how the screen_info is handled.

Well, I'd like to add edid_info to platforms with EFI. What would be
arm/arm64 and loongarch, I guess. See below for the future plans.

>
>>> I suppose you could use FIRMWARE_EDID on EFI or OF systems without
>>> the need for a global edid_info structure, but that would not
>>> share any code with the current fb_firmware_edid() function.
>>
>> The current code is build on top of screen_info and edid_info. I'd
>> preferably not replace that, if possible.
>
> One way I could imagine this looking in the end would be
> something like
>
> struct screen_info *fb_screen_info(struct device *dev)
> {
> struct screen_info *si = NULL;
>
> if (IS_ENABLED(CONFIG_EFI))
> si = efi_get_screen_info(dev);
>
> if (IS_ENABLED(CONFIG_ARCH_HAS_SCREEN_INFO) && !si)
> si = screen_info;
>
> return si;
> }
>
> corresponding to fb_firmware_edid(). With this, any driver
> that wants to access screen_info would call this function
> instead of using the global pointer, plus either NULL pointer
> check or a CONFIG_ARCH_HAS_SCREEN_INFO dependency.
>
> This way we could completely eliminate the global screen_info
> on arm64, riscv, and loongarch but still use the efi and
> hyperv framebuffer/drm drivers.

If possible, I'd like to remove global screen_info and edid_info
entirely from fbdev and the various consoles.

We currently use screen_info to set up the generic framebuffer device in
drivers/firmware/sysfb.c. I'd like to use edid_info here as well, so
that the generic graphics drivers can get EDID information.

For the few fbdev drivers and consoles that require the global
screen_info/edid_info, I'd rather provide lookup functions in sysfb
(e.g., sysfb_get_screen_info(), sysfb_get_edid_info()). The global
screen_info/edid_info state would then become an internal artifact of
the sysfb code.

Hopefully that explains some of the decisions made in this patchset.

Best regards
Thomas

>
> Arnd

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2023-07-18 15:40:34

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 07/12] arch/x86: Declare edid_info in <asm/screen_info.h>

On Wed, Jul 5, 2023, at 10:18, Thomas Zimmermann wrote:
> Am 30.06.23 um 13:53 schrieb Arnd Bergmann:
>> On Fri, Jun 30, 2023, at 09:46, Thomas Zimmermann wrote:
>>> Am 29.06.23 um 15:21 schrieb Arnd Bergmann:
>>
>> I definitely get it for the screen_info, which needs the complexity.
>> For ARCHARCH_HAS_EDID_INFO I would hope that it's never selected by
>> anything other than x86, so I would still go with just a dependency
>> on x86 for simplicity, but I don't mind having the extra symbol if that
>> keeps it more consistent with how the screen_info is handled.
>
> Well, I'd like to add edid_info to platforms with EFI. What would be
> arm/arm64 and loongarch, I guess. See below for the future plans.

To be clear: I don't mind using a 'struct edid_info' being passed
around between subsystems, that is clearly an improvement over
'struct screen_info'. It's the global variable that seems like
an artifact of linux-2.4 days, and I think we can do better than that.

>>>> I suppose you could use FIRMWARE_EDID on EFI or OF systems without
>>>> the need for a global edid_info structure, but that would not
>>>> share any code with the current fb_firmware_edid() function.
>>>
>>> The current code is build on top of screen_info and edid_info. I'd
>>> preferably not replace that, if possible.
>>
>> One way I could imagine this looking in the end would be
>> something like
>>
>> struct screen_info *fb_screen_info(struct device *dev)
>> {
>> struct screen_info *si = NULL;
>>
>> if (IS_ENABLED(CONFIG_EFI))
>> si = efi_get_screen_info(dev);
>>
>> if (IS_ENABLED(CONFIG_ARCH_HAS_SCREEN_INFO) && !si)
>> si = screen_info;
>>
>> return si;
>> }
>>
>> corresponding to fb_firmware_edid(). With this, any driver
>> that wants to access screen_info would call this function
>> instead of using the global pointer, plus either NULL pointer
>> check or a CONFIG_ARCH_HAS_SCREEN_INFO dependency.
>>
>> This way we could completely eliminate the global screen_info
>> on arm64, riscv, and loongarch but still use the efi and
>> hyperv framebuffer/drm drivers.
>
> If possible, I'd like to remove global screen_info and edid_info
> entirely from fbdev and the various consoles.

ok

> We currently use screen_info to set up the generic framebuffer device in
> drivers/firmware/sysfb.c. I'd like to use edid_info here as well, so
> that the generic graphics drivers can get EDID information.
>
> For the few fbdev drivers and consoles that require the global
> screen_info/edid_info, I'd rather provide lookup functions in sysfb
> (e.g., sysfb_get_screen_info(), sysfb_get_edid_info()). The global
> screen_info/edid_info state would then become an internal artifact of
> the sysfb code.
>
> Hopefully that explains some of the decisions made in this patchset.

I spent some more time looking at the screen_info side, after my
first set of patches to refine the #ifdefs, and I think we don't
even need to make screen_info available to non-x86 drivers at all:

- All the vgacon users except for x86 can just register a static
screen_info (or simplified into a simpler structure) with the
driver itself. This even includes ia64, which does not support
EFI framebuffers.

- The VESA, vga16, SIS, Intel and HyperV framebuffer drivers only
need access to screen_info on x86. HyperV is the only driver that
can currently access the data from EFI firmware on arm64, but
that is only used for 'gen 1' guests, which I'm pretty sure
only exist on x86.

- All the other references to screen_info are specific to EFI
firmware, so we can move the global definition from arm,
arm64, loongarch, riscv and ia64 into the EFI firmware
code itself. It is still accessed by efifb and efi-earlycon
at this point.

I have uploaded version 2 of my series to
https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/log/?h=screen-info-v2
and will send it out after I get the green light from build
bots.

Arnd