dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb
@ 2023-12-06 13:50 Thomas Zimmermann
  2023-12-06 13:50 ` [PATCH v3 1/4] fbdev/efifb: Replace references to global screen_info by local pointer Thomas Zimmermann
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2023-12-06 13:50 UTC (permalink / raw)
  To: javierm, deller, pjones, sui.jingfeng
  Cc: linux-fbdev, Thomas Zimmermann, dri-devel

Replace the global instance of screen_info with the per-device instance
that is set by the sysfb code. The use of the global screen_info should
be limited and ideally be pushed into per-architecture code.

v3:
	* better const correctness in efifb (Sui)
	* fix build for deferred takeover (kernel test robot)
v2:
	* comment on devm_kmemdup() usage (Javier)

Thomas Zimmermann (4):
  fbdev/efifb: Replace references to global screen_info by local pointer
  fbdev/efifb: Use screen_info pointer from device
  fbdev/vesafb: Replace references to global screen_info by local
    pointer
  fbdev/vesafb: Use screen_info pointer from device

 drivers/video/fbdev/efifb.c  | 136 +++++++++++++++++++----------------
 drivers/video/fbdev/vesafb.c |  78 ++++++++++++--------
 2 files changed, 123 insertions(+), 91 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/4] fbdev/efifb: Replace references to global screen_info by local pointer
  2023-12-06 13:50 [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann
@ 2023-12-06 13:50 ` Thomas Zimmermann
  2023-12-06 13:50 ` [PATCH v3 2/4] fbdev/efifb: Use screen_info pointer from device Thomas Zimmermann
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2023-12-06 13:50 UTC (permalink / raw)
  To: javierm, deller, pjones, sui.jingfeng
  Cc: linux-fbdev, Thomas Zimmermann, dri-devel

Get the global screen_info's address once and access the data via
this pointer. Limits the use of global state.

v3:
	* use const screen_info in several places (Sui)
	* fix build for deferred takeover (kernel test robot)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/efifb.c | 124 +++++++++++++++++++-----------------
 1 file changed, 64 insertions(+), 60 deletions(-)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index f9b4ddd592ce..6b3b5cca1d41 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -108,7 +108,7 @@ static int efifb_setcolreg(unsigned regno, unsigned red, unsigned green,
  */
 #if defined CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER && \
     defined CONFIG_ACPI_BGRT
-static void efifb_copy_bmp(u8 *src, u32 *dst, int width, struct screen_info *si)
+static void efifb_copy_bmp(u8 *src, u32 *dst, int width, const struct screen_info *si)
 {
 	u8 r, g, b;
 
@@ -130,7 +130,7 @@ static void efifb_copy_bmp(u8 *src, u32 *dst, int width, struct screen_info *si)
  * resolution still fits, it will be displayed very close to the right edge of
  * the display looking quite bad. This function checks for this.
  */
-static bool efifb_bgrt_sanity_check(struct screen_info *si, u32 bmp_width)
+static bool efifb_bgrt_sanity_check(const struct screen_info *si, u32 bmp_width)
 {
 	/*
 	 * All x86 firmwares horizontally center the image (the yoffset
@@ -141,16 +141,15 @@ static bool efifb_bgrt_sanity_check(struct screen_info *si, u32 bmp_width)
 	return bgrt_tab.image_offset_x == expected_xoffset;
 }
 #else
-static bool efifb_bgrt_sanity_check(struct screen_info *si, u32 bmp_width)
+static bool efifb_bgrt_sanity_check(const struct screen_info *si, u32 bmp_width)
 {
 	return true;
 }
 #endif
 
-static void efifb_show_boot_graphics(struct fb_info *info)
+static void efifb_show_boot_graphics(struct fb_info *info, const struct screen_info *si)
 {
 	u32 bmp_width, bmp_height, bmp_pitch, dst_x, y, src_y;
-	struct screen_info *si = &screen_info;
 	struct bmp_file_header *file_header;
 	struct bmp_dib_header *dib_header;
 	void *bgrt_image = NULL;
@@ -247,7 +246,8 @@ static void efifb_show_boot_graphics(struct fb_info *info)
 	pr_warn("efifb: Ignoring BGRT: unexpected or invalid BMP data\n");
 }
 #else
-static inline void efifb_show_boot_graphics(struct fb_info *info) {}
+static inline void efifb_show_boot_graphics(struct fb_info *info, const struct screen_info *si)
+{ }
 #endif
 
 /*
@@ -282,7 +282,7 @@ static const struct fb_ops efifb_ops = {
 	.fb_setcolreg	= efifb_setcolreg,
 };
 
-static int efifb_setup(char *options)
+static int efifb_setup(struct screen_info *si, char *options)
 {
 	char *this_opt;
 
@@ -290,16 +290,16 @@ static int efifb_setup(char *options)
 		while ((this_opt = strsep(&options, ",")) != NULL) {
 			if (!*this_opt) continue;
 
-			efifb_setup_from_dmi(&screen_info, this_opt);
+			efifb_setup_from_dmi(si, this_opt);
 
 			if (!strncmp(this_opt, "base:", 5))
-				screen_info.lfb_base = simple_strtoul(this_opt+5, NULL, 0);
+				si->lfb_base = simple_strtoul(this_opt+5, NULL, 0);
 			else if (!strncmp(this_opt, "stride:", 7))
-				screen_info.lfb_linelength = simple_strtoul(this_opt+7, NULL, 0) * 4;
+				si->lfb_linelength = simple_strtoul(this_opt+7, NULL, 0) * 4;
 			else if (!strncmp(this_opt, "height:", 7))
-				screen_info.lfb_height = simple_strtoul(this_opt+7, NULL, 0);
+				si->lfb_height = simple_strtoul(this_opt+7, NULL, 0);
 			else if (!strncmp(this_opt, "width:", 6))
-				screen_info.lfb_width = simple_strtoul(this_opt+6, NULL, 0);
+				si->lfb_width = simple_strtoul(this_opt+6, NULL, 0);
 			else if (!strcmp(this_opt, "nowc"))
 				mem_flags &= ~EFI_MEMORY_WC;
 			else if (!strcmp(this_opt, "nobgrt"))
@@ -310,15 +310,15 @@ static int efifb_setup(char *options)
 	return 0;
 }
 
-static inline bool fb_base_is_valid(void)
+static inline bool fb_base_is_valid(struct screen_info *si)
 {
-	if (screen_info.lfb_base)
+	if (si->lfb_base)
 		return true;
 
-	if (!(screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE))
+	if (!(si->capabilities & VIDEO_CAPABILITY_64BIT_BASE))
 		return false;
 
-	if (screen_info.ext_lfb_base)
+	if (si->ext_lfb_base)
 		return true;
 
 	return false;
@@ -329,7 +329,10 @@ static ssize_t name##_show(struct device *dev,				\
 			   struct device_attribute *attr,		\
 			   char *buf)					\
 {									\
-	return sprintf(buf, fmt "\n", (screen_info.lfb_##name));	\
+	struct screen_info *si = dev_get_platdata(dev);			\
+	if (!si)							\
+		return -ENODEV;						\
+	return sprintf(buf, fmt "\n", (si->lfb_##name));		\
 }									\
 static DEVICE_ATTR_RO(name)
 
@@ -356,6 +359,7 @@ static u64 bar_offset;
 
 static int efifb_probe(struct platform_device *dev)
 {
+	struct screen_info *si = &screen_info;
 	struct fb_info *info;
 	struct efifb_par *par;
 	int err, orientation;
@@ -365,48 +369,48 @@ static int efifb_probe(struct platform_device *dev)
 	char *option = NULL;
 	efi_memory_desc_t md;
 
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
+	if (si->orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
 		return -ENODEV;
 
 	if (fb_get_options("efifb", &option))
 		return -ENODEV;
-	efifb_setup(option);
+	efifb_setup(si, option);
 
 	/* We don't get linelength from UGA Draw Protocol, only from
 	 * EFI Graphics Protocol.  So if it's not in DMI, and it's not
 	 * passed in from the user, we really can't use the framebuffer.
 	 */
-	if (!screen_info.lfb_linelength)
+	if (!si->lfb_linelength)
 		return -ENODEV;
 
-	if (!screen_info.lfb_depth)
-		screen_info.lfb_depth = 32;
-	if (!screen_info.pages)
-		screen_info.pages = 1;
-	if (!fb_base_is_valid()) {
+	if (!si->lfb_depth)
+		si->lfb_depth = 32;
+	if (!si->pages)
+		si->pages = 1;
+	if (!fb_base_is_valid(si)) {
 		printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
 		return -ENODEV;
 	}
 	printk(KERN_INFO "efifb: probing for efifb\n");
 
 	/* just assume they're all unset if any are */
-	if (!screen_info.blue_size) {
-		screen_info.blue_size = 8;
-		screen_info.blue_pos = 0;
-		screen_info.green_size = 8;
-		screen_info.green_pos = 8;
-		screen_info.red_size = 8;
-		screen_info.red_pos = 16;
-		screen_info.rsvd_size = 8;
-		screen_info.rsvd_pos = 24;
+	if (!si->blue_size) {
+		si->blue_size = 8;
+		si->blue_pos = 0;
+		si->green_size = 8;
+		si->green_pos = 8;
+		si->red_size = 8;
+		si->red_pos = 16;
+		si->rsvd_size = 8;
+		si->rsvd_pos = 24;
 	}
 
-	efifb_fix.smem_start = screen_info.lfb_base;
+	efifb_fix.smem_start = si->lfb_base;
 
-	if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE) {
+	if (si->capabilities & VIDEO_CAPABILITY_64BIT_BASE) {
 		u64 ext_lfb_base;
 
-		ext_lfb_base = (u64)(unsigned long)screen_info.ext_lfb_base << 32;
+		ext_lfb_base = (u64)(unsigned long)si->ext_lfb_base << 32;
 		efifb_fix.smem_start |= ext_lfb_base;
 	}
 
@@ -417,10 +421,10 @@ static int efifb_probe(struct platform_device *dev)
 		efifb_fix.smem_start = bar_resource->start + bar_offset;
 	}
 
-	efifb_defined.bits_per_pixel = screen_info.lfb_depth;
-	efifb_defined.xres = screen_info.lfb_width;
-	efifb_defined.yres = screen_info.lfb_height;
-	efifb_fix.line_length = screen_info.lfb_linelength;
+	efifb_defined.bits_per_pixel = si->lfb_depth;
+	efifb_defined.xres = si->lfb_width;
+	efifb_defined.yres = si->lfb_height;
+	efifb_fix.line_length = si->lfb_linelength;
 
 	/*   size_vmode -- that is the amount of memory needed for the
 	 *                 used video mode, i.e. the minimum amount of
@@ -430,7 +434,7 @@ static int efifb_probe(struct platform_device *dev)
 	/*   size_total -- all video memory we have. Used for
 	 *                 entries, ressource allocation and bounds
 	 *                 checking. */
-	size_total = screen_info.lfb_size;
+	size_total = si->lfb_size;
 	if (size_total < size_vmode)
 		size_total = size_vmode;
 
@@ -505,14 +509,14 @@ static int efifb_probe(struct platform_device *dev)
 		goto err_release_fb;
 	}
 
-	efifb_show_boot_graphics(info);
+	efifb_show_boot_graphics(info, si);
 
 	pr_info("efifb: framebuffer at 0x%lx, using %dk, total %dk\n",
 	       efifb_fix.smem_start, size_remap/1024, size_total/1024);
 	pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
 	       efifb_defined.xres, efifb_defined.yres,
 	       efifb_defined.bits_per_pixel, efifb_fix.line_length,
-	       screen_info.pages);
+	       si->pages);
 
 	efifb_defined.xres_virtual = efifb_defined.xres;
 	efifb_defined.yres_virtual = efifb_fix.smem_len /
@@ -526,26 +530,26 @@ static int efifb_probe(struct platform_device *dev)
 	efifb_defined.left_margin  = (efifb_defined.xres / 8) & 0xf8;
 	efifb_defined.hsync_len    = (efifb_defined.xres / 8) & 0xf8;
 
-	efifb_defined.red.offset    = screen_info.red_pos;
-	efifb_defined.red.length    = screen_info.red_size;
-	efifb_defined.green.offset  = screen_info.green_pos;
-	efifb_defined.green.length  = screen_info.green_size;
-	efifb_defined.blue.offset   = screen_info.blue_pos;
-	efifb_defined.blue.length   = screen_info.blue_size;
-	efifb_defined.transp.offset = screen_info.rsvd_pos;
-	efifb_defined.transp.length = screen_info.rsvd_size;
+	efifb_defined.red.offset    = si->red_pos;
+	efifb_defined.red.length    = si->red_size;
+	efifb_defined.green.offset  = si->green_pos;
+	efifb_defined.green.length  = si->green_size;
+	efifb_defined.blue.offset   = si->blue_pos;
+	efifb_defined.blue.length   = si->blue_size;
+	efifb_defined.transp.offset = si->rsvd_pos;
+	efifb_defined.transp.length = si->rsvd_size;
 
 	pr_info("efifb: %s: "
 	       "size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
 	       "Truecolor",
-	       screen_info.rsvd_size,
-	       screen_info.red_size,
-	       screen_info.green_size,
-	       screen_info.blue_size,
-	       screen_info.rsvd_pos,
-	       screen_info.red_pos,
-	       screen_info.green_pos,
-	       screen_info.blue_pos);
+	       si->rsvd_size,
+	       si->red_size,
+	       si->green_size,
+	       si->blue_size,
+	       si->rsvd_pos,
+	       si->red_pos,
+	       si->green_pos,
+	       si->blue_pos);
 
 	efifb_fix.ypanstep  = 0;
 	efifb_fix.ywrapstep = 0;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/4] fbdev/efifb: Use screen_info pointer from device
  2023-12-06 13:50 [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann
  2023-12-06 13:50 ` [PATCH v3 1/4] fbdev/efifb: Replace references to global screen_info by local pointer Thomas Zimmermann
@ 2023-12-06 13:50 ` Thomas Zimmermann
  2023-12-06 13:50 ` [PATCH v3 3/4] fbdev/vesafb: Replace references to global screen_info by local pointer Thomas Zimmermann
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2023-12-06 13:50 UTC (permalink / raw)
  To: javierm, deller, pjones, sui.jingfeng
  Cc: linux-fbdev, Thomas Zimmermann, dri-devel

Use the screen_info instance from the device instead of dereferencing
the global screen_info state. Decouples the driver from per-architecture
code. Duplicated the screen_info data, so that efifb can modify it at
will.

v2:
	* comment on devm_kmemdup() usage (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Sui Jingfeng <sui.jingfeng@linux.dev>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/efifb.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 6b3b5cca1d41..10fc14ad5d12 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -359,7 +359,7 @@ static u64 bar_offset;
 
 static int efifb_probe(struct platform_device *dev)
 {
-	struct screen_info *si = &screen_info;
+	struct screen_info *si;
 	struct fb_info *info;
 	struct efifb_par *par;
 	int err, orientation;
@@ -369,6 +369,18 @@ static int efifb_probe(struct platform_device *dev)
 	char *option = NULL;
 	efi_memory_desc_t md;
 
+	/*
+	 * If we fail probing the device, the kernel might try a different
+	 * driver. We get a copy of the attached screen_info, so that we can
+	 * modify its values without affecting later drivers.
+	 */
+	si = dev_get_platdata(&dev->dev);
+	if (!si)
+		return -ENODEV;
+	si = devm_kmemdup(&dev->dev, si, sizeof(*si), GFP_KERNEL);
+	if (!si)
+		return -ENOMEM;
+
 	if (si->orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
 		return -ENODEV;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/4] fbdev/vesafb: Replace references to global screen_info by local pointer
  2023-12-06 13:50 [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann
  2023-12-06 13:50 ` [PATCH v3 1/4] fbdev/efifb: Replace references to global screen_info by local pointer Thomas Zimmermann
  2023-12-06 13:50 ` [PATCH v3 2/4] fbdev/efifb: Use screen_info pointer from device Thomas Zimmermann
@ 2023-12-06 13:50 ` Thomas Zimmermann
  2023-12-06 13:50 ` [PATCH v3 4/4] fbdev/vesafb: Use screen_info pointer from device Thomas Zimmermann
  2023-12-18 11:23 ` [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann
  4 siblings, 0 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2023-12-06 13:50 UTC (permalink / raw)
  To: javierm, deller, pjones, sui.jingfeng
  Cc: linux-fbdev, Thomas Zimmermann, dri-devel

Get the global screen_info's address once and access the data via
this pointer. Limits the use of global state.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/vesafb.c | 66 +++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 31 deletions(-)

diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index c0edceea0a79..ea89accbec38 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -243,6 +243,7 @@ static int vesafb_setup(char *options)
 
 static int vesafb_probe(struct platform_device *dev)
 {
+	struct screen_info *si = &screen_info;
 	struct fb_info *info;
 	struct vesafb_par *par;
 	int i, err;
@@ -255,17 +256,17 @@ static int vesafb_probe(struct platform_device *dev)
 	fb_get_options("vesafb", &option);
 	vesafb_setup(option);
 
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_VLFB)
+	if (si->orig_video_isVGA != VIDEO_TYPE_VLFB)
 		return -ENODEV;
 
-	vga_compat = (screen_info.capabilities & 2) ? 0 : 1;
-	vesafb_fix.smem_start = screen_info.lfb_base;
-	vesafb_defined.bits_per_pixel = screen_info.lfb_depth;
+	vga_compat = (si->capabilities & 2) ? 0 : 1;
+	vesafb_fix.smem_start = si->lfb_base;
+	vesafb_defined.bits_per_pixel = si->lfb_depth;
 	if (15 == vesafb_defined.bits_per_pixel)
 		vesafb_defined.bits_per_pixel = 16;
-	vesafb_defined.xres = screen_info.lfb_width;
-	vesafb_defined.yres = screen_info.lfb_height;
-	vesafb_fix.line_length = screen_info.lfb_linelength;
+	vesafb_defined.xres = si->lfb_width;
+	vesafb_defined.yres = si->lfb_height;
+	vesafb_fix.line_length = si->lfb_linelength;
 	vesafb_fix.visual   = (vesafb_defined.bits_per_pixel == 8) ?
 		FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
 
@@ -277,7 +278,7 @@ static int vesafb_probe(struct platform_device *dev)
 	/*   size_total -- all video memory we have. Used for mtrr
 	 *                 entries, resource allocation and bounds
 	 *                 checking. */
-	size_total = screen_info.lfb_size * 65536;
+	size_total = si->lfb_size * 65536;
 	if (vram_total)
 		size_total = vram_total * 1024 * 1024;
 	if (size_total < size_vmode)
@@ -297,7 +298,7 @@ static int vesafb_probe(struct platform_device *dev)
 	vesafb_fix.smem_len = size_remap;
 
 #ifndef __i386__
-	screen_info.vesapm_seg = 0;
+	si->vesapm_seg = 0;
 #endif
 
 	if (!request_mem_region(vesafb_fix.smem_start, size_total, "vesafb")) {
@@ -317,23 +318,26 @@ static int vesafb_probe(struct platform_device *dev)
 	par = info->par;
 	info->pseudo_palette = par->pseudo_palette;
 
-	par->base = screen_info.lfb_base;
+	par->base = si->lfb_base;
 	par->size = size_total;
 
 	printk(KERN_INFO "vesafb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
-	       vesafb_defined.xres, vesafb_defined.yres, vesafb_defined.bits_per_pixel, vesafb_fix.line_length, screen_info.pages);
+	       vesafb_defined.xres, vesafb_defined.yres, vesafb_defined.bits_per_pixel,
+	       vesafb_fix.line_length, si->pages);
 
-	if (screen_info.vesapm_seg) {
+	if (si->vesapm_seg) {
 		printk(KERN_INFO "vesafb: protected mode interface info at %04x:%04x\n",
-		       screen_info.vesapm_seg,screen_info.vesapm_off);
+		       si->vesapm_seg, si->vesapm_off);
 	}
 
-	if (screen_info.vesapm_seg < 0xc000)
+	if (si->vesapm_seg < 0xc000)
 		ypan = pmi_setpal = 0; /* not available or some DOS TSR ... */
 
 	if (ypan || pmi_setpal) {
+		unsigned long pmi_phys;
 		unsigned short *pmi_base;
-		pmi_base  = (unsigned short*)phys_to_virt(((unsigned long)screen_info.vesapm_seg << 4) + screen_info.vesapm_off);
+		pmi_phys  = ((unsigned long)si->vesapm_seg << 4) + si->vesapm_off;
+		pmi_base  = (unsigned short *)phys_to_virt(pmi_phys);
 		pmi_start = (void*)((char*)pmi_base + pmi_base[1]);
 		pmi_pal   = (void*)((char*)pmi_base + pmi_base[2]);
 		printk(KERN_INFO "vesafb: pmi: set display start = %p, set palette = %p\n",pmi_start,pmi_pal);
@@ -377,14 +381,14 @@ static int vesafb_probe(struct platform_device *dev)
 	vesafb_defined.left_margin  = (vesafb_defined.xres / 8) & 0xf8;
 	vesafb_defined.hsync_len    = (vesafb_defined.xres / 8) & 0xf8;
 
-	vesafb_defined.red.offset    = screen_info.red_pos;
-	vesafb_defined.red.length    = screen_info.red_size;
-	vesafb_defined.green.offset  = screen_info.green_pos;
-	vesafb_defined.green.length  = screen_info.green_size;
-	vesafb_defined.blue.offset   = screen_info.blue_pos;
-	vesafb_defined.blue.length   = screen_info.blue_size;
-	vesafb_defined.transp.offset = screen_info.rsvd_pos;
-	vesafb_defined.transp.length = screen_info.rsvd_size;
+	vesafb_defined.red.offset    = si->red_pos;
+	vesafb_defined.red.length    = si->red_size;
+	vesafb_defined.green.offset  = si->green_pos;
+	vesafb_defined.green.length  = si->green_size;
+	vesafb_defined.blue.offset   = si->blue_pos;
+	vesafb_defined.blue.length   = si->blue_size;
+	vesafb_defined.transp.offset = si->rsvd_pos;
+	vesafb_defined.transp.length = si->rsvd_size;
 
 	if (vesafb_defined.bits_per_pixel <= 8) {
 		depth = vesafb_defined.green.length;
@@ -399,14 +403,14 @@ static int vesafb_probe(struct platform_device *dev)
 	       (vesafb_defined.bits_per_pixel > 8) ?
 	       "Truecolor" : (vga_compat || pmi_setpal) ?
 	       "Pseudocolor" : "Static Pseudocolor",
-	       screen_info.rsvd_size,
-	       screen_info.red_size,
-	       screen_info.green_size,
-	       screen_info.blue_size,
-	       screen_info.rsvd_pos,
-	       screen_info.red_pos,
-	       screen_info.green_pos,
-	       screen_info.blue_pos);
+	       si->rsvd_size,
+	       si->red_size,
+	       si->green_size,
+	       si->blue_size,
+	       si->rsvd_pos,
+	       si->red_pos,
+	       si->green_pos,
+	       si->blue_pos);
 
 	vesafb_fix.ypanstep  = ypan     ? 1 : 0;
 	vesafb_fix.ywrapstep = (ypan>1) ? 1 : 0;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 4/4] fbdev/vesafb: Use screen_info pointer from device
  2023-12-06 13:50 [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2023-12-06 13:50 ` [PATCH v3 3/4] fbdev/vesafb: Replace references to global screen_info by local pointer Thomas Zimmermann
@ 2023-12-06 13:50 ` Thomas Zimmermann
  2023-12-18 11:23 ` [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann
  4 siblings, 0 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2023-12-06 13:50 UTC (permalink / raw)
  To: javierm, deller, pjones, sui.jingfeng
  Cc: linux-fbdev, Thomas Zimmermann, dri-devel

Use the screen_info instance from the device instead of dereferencing
the global screen_info state. Decouples the driver from per-architecture
code. Duplicated the screen_info data, so that vesafb can modify it at
will.

v2:
	* comment on devm_kmemdup() usage (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/vesafb.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index ea89accbec38..8ab64ae4cad3 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -243,7 +243,7 @@ static int vesafb_setup(char *options)
 
 static int vesafb_probe(struct platform_device *dev)
 {
-	struct screen_info *si = &screen_info;
+	struct screen_info *si;
 	struct fb_info *info;
 	struct vesafb_par *par;
 	int i, err;
@@ -252,6 +252,18 @@ static int vesafb_probe(struct platform_device *dev)
 	unsigned int size_total;
 	char *option = NULL;
 
+	/*
+	 * If we fail probing the device, the kernel might try a different
+	 * driver. We get a copy of the attached screen_info, so that we can
+	 * modify its values without affecting later drivers.
+	 */
+	si = dev_get_platdata(&dev->dev);
+	if (!si)
+		return -ENODEV;
+	si = devm_kmemdup(&dev->dev, si, sizeof(*si), GFP_KERNEL);
+	if (!si)
+		return -ENOMEM;
+
 	/* ignore error return of fb_get_options */
 	fb_get_options("vesafb", &option);
 	vesafb_setup(option);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb
  2023-12-06 13:50 [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2023-12-06 13:50 ` [PATCH v3 4/4] fbdev/vesafb: Use screen_info pointer from device Thomas Zimmermann
@ 2023-12-18 11:23 ` Thomas Zimmermann
  4 siblings, 0 replies; 6+ messages in thread
From: Thomas Zimmermann @ 2023-12-18 11:23 UTC (permalink / raw)
  To: javierm, deller, pjones, sui.jingfeng; +Cc: linux-fbdev, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1222 bytes --]

I'm going to merge this patchset through drm-misc-next

Am 06.12.23 um 14:50 schrieb Thomas Zimmermann:
> Replace the global instance of screen_info with the per-device instance
> that is set by the sysfb code. The use of the global screen_info should
> be limited and ideally be pushed into per-architecture code.
> 
> v3:
> 	* better const correctness in efifb (Sui)
> 	* fix build for deferred takeover (kernel test robot)
> v2:
> 	* comment on devm_kmemdup() usage (Javier)
> 
> Thomas Zimmermann (4):
>    fbdev/efifb: Replace references to global screen_info by local pointer
>    fbdev/efifb: Use screen_info pointer from device
>    fbdev/vesafb: Replace references to global screen_info by local
>      pointer
>    fbdev/vesafb: Use screen_info pointer from device
> 
>   drivers/video/fbdev/efifb.c  | 136 +++++++++++++++++++----------------
>   drivers/video/fbdev/vesafb.c |  78 ++++++++++++--------
>   2 files changed, 123 insertions(+), 91 deletions(-)
> 

-- 
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)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-12-18 18:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-06 13:50 [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann
2023-12-06 13:50 ` [PATCH v3 1/4] fbdev/efifb: Replace references to global screen_info by local pointer Thomas Zimmermann
2023-12-06 13:50 ` [PATCH v3 2/4] fbdev/efifb: Use screen_info pointer from device Thomas Zimmermann
2023-12-06 13:50 ` [PATCH v3 3/4] fbdev/vesafb: Replace references to global screen_info by local pointer Thomas Zimmermann
2023-12-06 13:50 ` [PATCH v3 4/4] fbdev/vesafb: Use screen_info pointer from device Thomas Zimmermann
2023-12-18 11:23 ` [PATCH v3 0/4] fbdev: Remove global screen_info in efifb/vesafb Thomas Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).