[PATCH 0/4] arch/parisc: Detect primary framebuffer from device

[PATCH 0/4] arch/parisc: Detect primary framebuffer from device

[Thomas Zimmermann]

On parisc, change detection of the primary framebuffer to test for
the Linux device instead of fbdev's fb_info in fb_is_primary_device().
Makes the test independent from fbdev.

This patchset is part of a larger effort to clean up the low-level
display handling. There are various functions that attempt to detect
the system's primary framebuffer device, such as in vgaarb, [1]
fbcon, [2] or fbmon. [3] This code should be unified in a single helper
that implements the test. The function fb_is_primary_device() already
does this, but requires fbdev on parisc. With the patchset applied, the
parisc implementation tests directly with the Linux device. No fbdev is
required.

Patch 1 adds the framebuffer's Linux device to the STI ROM structures,
which represents the graphics firmware. Patches 2 updates the stifb
driver to refer to the correct Linux device. The device is used in
patch 3 to change the test in fb_is_primary_device(). Patch 4 removes
the obsolete fb_info from the STI ROM structures.

A later patchset will update the interface of fb_is_primary_device() to
receive a Linux device instead of an instance of fb_info. This involves
several architectures, so it better done in a separate patch.

[1] https://elixir.bootlin.com/linux/v6.6/source/drivers/pci/vgaarb.c#L557
[2] https://elixir.bootlin.com/linux/v6.6/source/drivers/video/fbdev/core/fbcon.c#L2943
[3] https://elixir.bootlin.com/linux/v6.6/source/drivers/video/fbdev/core/fbmon.c#L1503

Thomas Zimmermann (4):
  video/sticore: Store ROM device in STI struct
  fbdev/stifb: Allocate fb_info instance with framebuffer_alloc()
  arch/parisc: Detect primary video device from device instance
  video/sticore: Remove info field from STI struct

 arch/parisc/video/fbdev.c   |   2 +-
 drivers/video/fbdev/stifb.c | 109 ++++++++++++++++++------------------
 drivers/video/sticore.c     |   5 ++
 include/video/sticore.h     |   6 +-
 4 files changed, 65 insertions(+), 57 deletions(-)


base-commit: 8da6351b7194938a876184d34c4c0802e805d3cf
-- 
2.43.0

[PATCH 1/4] video/sticore: Store ROM device in STI struct

[Thomas Zimmermann]

Store the ROM's parent device in each STI struct, so we can associate
the STI framebuffer with a device.

The new field will eventually replace the fbdev subsystem's info field,
which the function fb_is_primary_device() currently requires to detect
the firmware's output. By using the device instead of the framebuffer
info, a later patch can generalize the helper for use in non-fbdev code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/sticore.c | 5 +++++
 include/video/sticore.h | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/video/sticore.c b/drivers/video/sticore.c
index c3765ad6eedf..7115b325817f 100644
--- a/drivers/video/sticore.c
+++ b/drivers/video/sticore.c
@@ -1041,6 +1041,9 @@ static int __init sticore_pa_init(struct parisc_device *dev)
 
 	print_pa_hwpath(dev, sti->pa_path);
 	sticore_check_for_default_sti(sti, sti->pa_path);
+
+	sti->dev = &dev->dev;
+
 	return 0;
 }
 
@@ -1084,6 +1087,8 @@ static int sticore_pci_init(struct pci_dev *pd, const struct pci_device_id *ent)
 		pr_warn("Unable to handle STI device '%s'\n", pci_name(pd));
 		return -ENODEV;
 	}
+
+	sti->dev = &pd->dev;
 #endif /* CONFIG_PCI */
 
 	return 0;
diff --git a/include/video/sticore.h b/include/video/sticore.h
index 012b5b46ad7d..9d993e22805d 100644
--- a/include/video/sticore.h
+++ b/include/video/sticore.h
@@ -2,6 +2,7 @@
 #ifndef STICORE_H
 #define STICORE_H
 
+struct device;
 struct fb_info;
 
 /* generic STI structures & functions */
@@ -370,6 +371,9 @@ struct sti_struct {
 	/* pointer to the fb_info where this STI device is used */
 	struct fb_info *info;
 
+	/* pointer to the parent device */
+	struct device *dev;
+
 	/* pointer to all internal data */
 	struct sti_all_data *sti_data;
 
-- 
2.43.0

[PATCH 2/4] fbdev/stifb: Allocate fb_info instance with framebuffer_alloc()

[Thomas Zimmermann]

Allocate stifb's instance of fb_info with framebuffer_alloc(). This
is the preferred way of creating fb_info with associated driver data
stored in struct fb_info.par. Requires several, but minor, changes
through out the driver's code.

The intended side effect of this patch is that the new instance of
struct fb_info now has its device field correctly set to the parent
device of the STI ROM. A later patch can detect if the device is the
firmware's primary output. It is also now correctly located within
the Linux device hierarchy.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/stifb.c | 106 +++++++++++++++++++-----------------
 1 file changed, 56 insertions(+), 50 deletions(-)

diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
index 548d992f8cb1..36e6bcab83aa 100644
--- a/drivers/video/fbdev/stifb.c
+++ b/drivers/video/fbdev/stifb.c
@@ -103,7 +103,7 @@ typedef struct {
 } ngle_rom_t;
 
 struct stifb_info {
-	struct fb_info info;
+	struct fb_info *info;
 	unsigned int id;
 	ngle_rom_t ngle_rom;
 	struct sti_struct *sti;
@@ -153,15 +153,15 @@ static int __initdata stifb_bpp_pref[MAX_STI_ROMS];
 #define REG_44		0x210030
 #define REG_45		0x210034
 
-#define READ_BYTE(fb,reg)		gsc_readb((fb)->info.fix.mmio_start + (reg))
-#define READ_WORD(fb,reg)		gsc_readl((fb)->info.fix.mmio_start + (reg))
+#define READ_BYTE(fb, reg)		gsc_readb((fb)->info->fix.mmio_start + (reg))
+#define READ_WORD(fb, reg)		gsc_readl((fb)->info->fix.mmio_start + (reg))
 
 
 #ifndef DEBUG_STIFB_REGS
 # define  DEBUG_OFF()
 # define  DEBUG_ON()
-# define WRITE_BYTE(value,fb,reg)	gsc_writeb((value),(fb)->info.fix.mmio_start + (reg))
-# define WRITE_WORD(value,fb,reg)	gsc_writel((value),(fb)->info.fix.mmio_start + (reg))
+# define WRITE_BYTE(value, fb, reg)	gsc_writeb((value), (fb)->info->fix.mmio_start + (reg))
+# define WRITE_WORD(value, fb, reg)	gsc_writel((value), (fb)->info->fix.mmio_start + (reg))
 #else
   static int debug_on = 1;
 # define  DEBUG_OFF() debug_on=0
@@ -169,11 +169,11 @@ static int __initdata stifb_bpp_pref[MAX_STI_ROMS];
 # define WRITE_BYTE(value,fb,reg)	do { if (debug_on) \
 						printk(KERN_DEBUG "%30s: WRITE_BYTE(0x%06x) = 0x%02x (old=0x%02x)\n", \
 							__func__, reg, value, READ_BYTE(fb,reg)); 		  \
-					gsc_writeb((value),(fb)->info.fix.mmio_start + (reg)); } while (0)
+					gsc_writeb((value), (fb)->info->fix.mmio_start + (reg)); } while (0)
 # define WRITE_WORD(value,fb,reg)	do { if (debug_on) \
 						printk(KERN_DEBUG "%30s: WRITE_WORD(0x%06x) = 0x%08x (old=0x%08x)\n", \
 							__func__, reg, value, READ_WORD(fb,reg)); 		  \
-					gsc_writel((value),(fb)->info.fix.mmio_start + (reg)); } while (0)
+					gsc_writel((value), (fb)->info->fix.mmio_start + (reg)); } while (0)
 #endif /* DEBUG_STIFB_REGS */
 
 
@@ -210,13 +210,13 @@ SETUP_FB(struct stifb_info *fb)
 			reg10_value = 0x13601000;
 			break;
 		case S9000_ID_A1439A:
-			if (fb->info.var.bits_per_pixel == 32)
+			if (fb->info->var.bits_per_pixel == 32)
 				reg10_value = 0xBBA0A000;
 			else
 				reg10_value = 0x13601000;
 			break;
 		case S9000_ID_HCRX:
-			if (fb->info.var.bits_per_pixel == 32)
+			if (fb->info->var.bits_per_pixel == 32)
 				reg10_value = 0xBBA0A000;
 			else
 				reg10_value = 0x13602000;
@@ -254,7 +254,7 @@ static void
 FINISH_IMAGE_COLORMAP_ACCESS(struct stifb_info *fb)
 {
 	WRITE_WORD(0x400, fb, REG_2);
-	if (fb->info.var.bits_per_pixel == 32) {
+	if (fb->info->var.bits_per_pixel == 32) {
 		WRITE_WORD(0x83000100, fb, REG_1);
 	} else {
 		if (fb->id == S9000_ID_ARTIST || fb->id == CRT_ID_VISUALIZE_EG)
@@ -503,7 +503,7 @@ static void
 ngleSetupAttrPlanes(struct stifb_info *fb, int BufferNumber)
 {
 	SETUP_ATTR_ACCESS(fb, BufferNumber);
-	SET_ATTR_SIZE(fb, fb->info.var.xres, fb->info.var.yres);
+	SET_ATTR_SIZE(fb, fb->info->var.xres, fb->info->var.yres);
 	FINISH_ATTR_ACCESS(fb);
 	SETUP_FB(fb);
 }
@@ -526,9 +526,9 @@ rattlerSetupPlanes(struct stifb_info *fb)
 	SETUP_FB(fb);
 	fb->id = saved_id;
 
-	for (y = 0; y < fb->info.var.yres; ++y)
-		fb_memset_io(fb->info.screen_base + y * fb->info.fix.line_length,
-			     0xff, fb->info.var.xres * fb->info.var.bits_per_pixel/8);
+	for (y = 0; y < fb->info->var.yres; ++y)
+		fb_memset_io(fb->info->screen_base + y * fb->info->fix.line_length,
+			     0xff, fb->info->var.xres * fb->info->var.bits_per_pixel/8);
 
 	CRX24_SET_OVLY_MASK(fb);
 	SETUP_FB(fb);
@@ -607,7 +607,7 @@ setHyperLutBltCtl(struct stifb_info *fb, int offsetWithinLut, int length)
 	lutBltCtl.fields.lutType = HYPER_CMAP_TYPE;
 
 	/* Expect lutIndex to be 0 or 1 for image cmaps, 2 or 3 for overlay cmaps */
-	if (fb->info.var.bits_per_pixel == 8)
+	if (fb->info->var.bits_per_pixel == 8)
 		lutBltCtl.fields.lutOffset = 2 * 256;
 	else
 		lutBltCtl.fields.lutOffset = 0 * 256;
@@ -688,7 +688,7 @@ ngleResetAttrPlanes(struct stifb_info *fb, unsigned int ctlPlaneReg)
 					       DataDynamic, MaskOtc,
 					       BGx(0), FGx(0)));
 	packed_dst = 0;
-	packed_len = (fb->info.var.xres << 16) | fb->info.var.yres;
+	packed_len = (fb->info->var.xres << 16) | fb->info->var.yres;
 	GET_FIFO_SLOTS(fb, nFreeFifoSlots, 2);
 	NGLE_SET_DSTXY(fb, packed_dst);
 	SET_LENXY_START_RECFILL(fb, packed_len);
@@ -738,7 +738,7 @@ ngleClearOverlayPlanes(struct stifb_info *fb, int mask, int data)
         NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, mask);
 
         packed_dst = 0;
-        packed_len = (fb->info.var.xres << 16) | fb->info.var.yres;
+	packed_len = (fb->info->var.xres << 16) | fb->info->var.yres;
         NGLE_SET_DSTXY(fb, packed_dst);
 
 	/* Write zeroes to overlay planes */
@@ -760,7 +760,7 @@ hyperResetPlanes(struct stifb_info *fb, int enable)
 	NGLE_LOCK(fb);
 
 	if (IS_24_DEVICE(fb))
-		if (fb->info.var.bits_per_pixel == 32)
+		if (fb->info->var.bits_per_pixel == 32)
 			controlPlaneReg = 0x04000F00;
 		else
 			controlPlaneReg = 0x00000F00;   /* 0x00000800 should be enough, but lets clear all 4 bits */
@@ -890,7 +890,7 @@ SETUP_HCRX(struct stifb_info *fb)
 	GET_FIFO_SLOTS(fb, nFreeFifoSlots, 7);
 
 	if (IS_24_DEVICE(fb)) {
-		hyperbowl = (fb->info.var.bits_per_pixel == 32) ?
+		hyperbowl = (fb->info->var.bits_per_pixel == 32) ?
 			HYPERBOWL_MODE01_8_24_LUT0_TRANSPARENT_LUT1_OPAQUE :
 			HYPERBOWL_MODE01_8_24_LUT0_OPAQUE_LUT1_OPAQUE;
 
@@ -924,21 +924,21 @@ SETUP_HCRX(struct stifb_info *fb)
 static int
 stifb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
-	struct stifb_info *fb = container_of(info, struct stifb_info, info);
+	struct stifb_info *fb = info->par;
 
-	if (var->xres != fb->info.var.xres ||
-	    var->yres != fb->info.var.yres ||
-	    var->bits_per_pixel != fb->info.var.bits_per_pixel)
+	if (var->xres != fb->info->var.xres ||
+	    var->yres != fb->info->var.yres ||
+	    var->bits_per_pixel != fb->info->var.bits_per_pixel)
 		return -EINVAL;
 
 	var->xres_virtual = var->xres;
 	var->yres_virtual = var->yres;
 	var->xoffset = 0;
 	var->yoffset = 0;
-	var->grayscale = fb->info.var.grayscale;
-	var->red.length = fb->info.var.red.length;
-	var->green.length = fb->info.var.green.length;
-	var->blue.length = fb->info.var.blue.length;
+	var->grayscale = fb->info->var.grayscale;
+	var->red.length = fb->info->var.red.length;
+	var->green.length = fb->info->var.green.length;
+	var->blue.length = fb->info->var.blue.length;
 
 	return 0;
 }
@@ -947,7 +947,7 @@ static int
 stifb_setcolreg(u_int regno, u_int red, u_int green,
 	      u_int blue, u_int transp, struct fb_info *info)
 {
-	struct stifb_info *fb = container_of(info, struct stifb_info, info);
+	struct stifb_info *fb = info->par;
 	u32 color;
 
 	if (regno >= NR_PALETTE)
@@ -961,7 +961,7 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
 
 	START_IMAGE_COLORMAP_ACCESS(fb);
 
-	if (unlikely(fb->info.var.grayscale)) {
+	if (unlikely(fb->info->var.grayscale)) {
 		/* gray = 0.30*R + 0.59*G + 0.11*B */
 		color = ((red * 77) +
 			 (green * 151) +
@@ -972,10 +972,10 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
 			 (blue));
 	}
 
-	if (fb->info.fix.visual == FB_VISUAL_DIRECTCOLOR) {
-		struct fb_var_screeninfo *var = &fb->info.var;
+	if (fb->info->fix.visual == FB_VISUAL_DIRECTCOLOR) {
+		struct fb_var_screeninfo *var = &fb->info->var;
 		if (regno < 16)
-			((u32 *)fb->info.pseudo_palette)[regno] =
+			((u32 *)fb->info->pseudo_palette)[regno] =
 				regno << var->red.offset |
 				regno << var->green.offset |
 				regno << var->blue.offset;
@@ -1007,7 +1007,7 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
 static int
 stifb_blank(int blank_mode, struct fb_info *info)
 {
-	struct stifb_info *fb = container_of(info, struct stifb_info, info);
+	struct stifb_info *fb = info->par;
 	int enable = (blank_mode == 0) ? ENABLE : DISABLE;
 
 	switch (fb->id) {
@@ -1036,12 +1036,12 @@ stifb_blank(int blank_mode, struct fb_info *info)
 static void
 stifb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
 {
-	struct stifb_info *fb = container_of(info, struct stifb_info, info);
+	struct stifb_info *fb = info->par;
 
 	SETUP_COPYAREA(fb);
 
 	SETUP_HW(fb);
-	if (fb->info.var.bits_per_pixel == 32) {
+	if (fb->info->var.bits_per_pixel == 32) {
 		WRITE_WORD(0xBBA0A000, fb, REG_10);
 
 		NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, 0xffffffff);
@@ -1075,15 +1075,15 @@ stifb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
 static void
 stifb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
 {
-	struct stifb_info *fb = container_of(info, struct stifb_info, info);
+	struct stifb_info *fb = info->par;
 
 	if (rect->rop != ROP_COPY ||
-	    (fb->id == S9000_ID_HCRX && fb->info.var.bits_per_pixel == 32))
+	    (fb->id == S9000_ID_HCRX && fb->info->var.bits_per_pixel == 32))
 		return cfb_fillrect(info, rect);
 
 	SETUP_HW(fb);
 
-	if (fb->info.var.bits_per_pixel == 32) {
+	if (fb->info->var.bits_per_pixel == 32) {
 		WRITE_WORD(0xBBA0A000, fb, REG_10);
 
 		NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, 0xffffffff);
@@ -1141,7 +1141,7 @@ stifb_init_display(struct stifb_info *fb)
         switch (id) {
 	 case S9000_ID_A1659A:
 	 case S9000_ID_A1439A:
-	    if (fb->info.var.bits_per_pixel == 32)
+	    if (fb->info->var.bits_per_pixel == 32)
 		ngleSetupAttrPlanes(fb, BUFF1_CMAP3);
 	    else {
 		ngleSetupAttrPlanes(fb, BUFF1_CMAP0);
@@ -1151,7 +1151,7 @@ stifb_init_display(struct stifb_info *fb)
 	    break;
 	 case S9000_ID_ARTIST:
 	 case CRT_ID_VISUALIZE_EG:
-	    if (fb->info.var.bits_per_pixel == 32)
+	    if (fb->info->var.bits_per_pixel == 32)
 		ngleSetupAttrPlanes(fb, BUFF1_CMAP3);
 	    else {
 		ngleSetupAttrPlanes(fb, ARTIST_CMAP0);
@@ -1193,11 +1193,11 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
 	char *dev_name;
 	int bpp, xres, yres;
 
-	fb = kzalloc(sizeof(*fb), GFP_ATOMIC);
-	if (!fb)
+	info = framebuffer_alloc(sizeof(*fb), sti->dev);
+	if (!info)
 		return -ENOMEM;
-
-	info = &fb->info;
+	fb = info->par;
+	fb->info = info;
 
 	/* set struct to a known state */
 	fix = &info->fix;
@@ -1390,10 +1390,10 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
 
 	/* save for primary gfx device detection & unregister_framebuffer() */
 	sti->info = info;
-	if (register_framebuffer(&fb->info) < 0)
+	if (register_framebuffer(fb->info) < 0)
 		goto out_err4;
 
-	fb_info(&fb->info, "%s %dx%d-%d frame buffer device, %s, id: %04x, mmio: 0x%04lx\n",
+	fb_info(fb->info, "%s %dx%d-%d frame buffer device, %s, id: %04x, mmio: 0x%04lx\n",
 		fix->id,
 		var->xres,
 		var->yres,
@@ -1402,6 +1402,8 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
 		fb->id,
 		fix->mmio_start);
 
+	dev_set_drvdata(sti->dev, info);
+
 	return 0;
 
 
@@ -1414,7 +1416,7 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
 out_err1:
 	iounmap(info->screen_base);
 out_err0:
-	kfree(fb);
+	framebuffer_release(info);
 	sti->info = NULL;
 	return -ENXIO;
 }
@@ -1480,15 +1482,19 @@ stifb_cleanup(void)
 		sti = sti_get_rom(i);
 		if (!sti)
 			break;
-		if (sti->info) {
-			struct fb_info *info = sti->info;
-			unregister_framebuffer(sti->info);
+		if (sti->dev) {
+			struct fb_info *info = dev_get_drvdata(sti->dev);
+
+			if (!info)
+				continue;
+			unregister_framebuffer(info);
 			release_mem_region(info->fix.mmio_start, info->fix.mmio_len);
 		        release_mem_region(info->fix.smem_start, info->fix.smem_len);
 				if (info->screen_base)
 					iounmap(info->screen_base);
 		        fb_dealloc_cmap(&info->cmap);
 		        framebuffer_release(info);
+			dev_set_drvdata(sti->dev, NULL);
 		}
 		sti->info = NULL;
 	}
-- 
2.43.0

[PATCH 3/4] arch/parisc: Detect primary video device from device instance

[Thomas Zimmermann]

Update fb_is_primary device() on parisc to detect the primary display
device from the Linux device instance. Aligns the code with the other
architectures. A later patch will remove the fbdev dependency from the
function's interface.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 arch/parisc/video/fbdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c
index 137561d98246..e4f8ac99fc9e 100644
--- a/arch/parisc/video/fbdev.c
+++ b/arch/parisc/video/fbdev.c
@@ -21,6 +21,6 @@ int fb_is_primary_device(struct fb_info *info)
 		return true;
 
 	/* return true if it's the default built-in framebuffer driver */
-	return (sti->info == info);
+	return (sti->dev == info->device);
 }
 EXPORT_SYMBOL(fb_is_primary_device);
-- 
2.43.0

[PATCH 4/4] video/sticore: Remove info field from STI struct

[Thomas Zimmermann]

The info field in struct sti_struct was used to detect the default
display device. That test is now done with the respective Linux device
and the info field is unused. Remove it.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/stifb.c | 3 ---
 include/video/sticore.h     | 4 ----
 2 files changed, 7 deletions(-)

diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
index 36e6bcab83aa..2de0e675fd15 100644
--- a/drivers/video/fbdev/stifb.c
+++ b/drivers/video/fbdev/stifb.c
@@ -1389,7 +1389,6 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
 	}
 
 	/* save for primary gfx device detection & unregister_framebuffer() */
-	sti->info = info;
 	if (register_framebuffer(fb->info) < 0)
 		goto out_err4;
 
@@ -1417,7 +1416,6 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
 	iounmap(info->screen_base);
 out_err0:
 	framebuffer_release(info);
-	sti->info = NULL;
 	return -ENXIO;
 }
 
@@ -1496,7 +1494,6 @@ stifb_cleanup(void)
 		        framebuffer_release(info);
 			dev_set_drvdata(sti->dev, NULL);
 		}
-		sti->info = NULL;
 	}
 }
 
diff --git a/include/video/sticore.h b/include/video/sticore.h
index 9d993e22805d..823666af1871 100644
--- a/include/video/sticore.h
+++ b/include/video/sticore.h
@@ -3,7 +3,6 @@
 #define STICORE_H
 
 struct device;
-struct fb_info;
 
 /* generic STI structures & functions */
 
@@ -368,9 +367,6 @@ struct sti_struct {
 	/* PCI data structures (pg. 17ff from sti.pdf) */
 	u8 rm_entry[16]; /* pci region mapper array == pci config space offset */
 
-	/* pointer to the fb_info where this STI device is used */
-	struct fb_info *info;
-
 	/* pointer to the parent device */
 	struct device *dev;
 
-- 
2.43.0

Re: [PATCH 2/4] fbdev/stifb: Allocate fb_info instance with framebuffer_alloc()

[Helge Deller]

Hi Thomas,

On 12/20/23 14:22, Thomas Zimmermann wrote:
> Allocate stifb's instance of fb_info with framebuffer_alloc(). This
> is the preferred way of creating fb_info with associated driver data
> stored in struct fb_info.par. Requires several, but minor, changes
> through out the driver's code.
>
> The intended side effect of this patch is that the new instance of
> struct fb_info now has its device field correctly set to the parent
> device of the STI ROM. A later patch can detect if the device is the
> firmware's primary output. It is also now correctly located within
> the Linux device hierarchy.

Thanks for this cleanup series!
Sadly stifb then crashes at bootup.

Please include this copy&pasted hunk in this patch, which then fixes the crash:

diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
index 2de0e675fd15..f8bbd8d6f5b2 100644
--- a/drivers/video/fbdev/stifb.c
+++ b/drivers/video/fbdev/stifb.c
@@ -1158,7 +1158,7 @@ stifb_init_display(struct stifb_info *fb)
             }
             break;
         }
-       stifb_blank(0, (struct fb_info *)fb);   /* 0=enable screen */
+       stifb_blank(0, fb->info);       /* 0=enable screen */



With this applied, you may add to the series:

Tested-by: Helge Deller <deller@gmx.de>
Reviewed-by: Helge Deller <deller@gmx.de>

Thanks!
Helge

>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/video/fbdev/stifb.c | 106 +++++++++++++++++++-----------------
>   1 file changed, 56 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
> index 548d992f8cb1..36e6bcab83aa 100644
> --- a/drivers/video/fbdev/stifb.c
> +++ b/drivers/video/fbdev/stifb.c
> @@ -103,7 +103,7 @@ typedef struct {
>   } ngle_rom_t;
>
>   struct stifb_info {
> -	struct fb_info info;
> +	struct fb_info *info;
>   	unsigned int id;
>   	ngle_rom_t ngle_rom;
>   	struct sti_struct *sti;
> @@ -153,15 +153,15 @@ static int __initdata stifb_bpp_pref[MAX_STI_ROMS];
>   #define REG_44		0x210030
>   #define REG_45		0x210034
>
> -#define READ_BYTE(fb,reg)		gsc_readb((fb)->info.fix.mmio_start + (reg))
> -#define READ_WORD(fb,reg)		gsc_readl((fb)->info.fix.mmio_start + (reg))
> +#define READ_BYTE(fb, reg)		gsc_readb((fb)->info->fix.mmio_start + (reg))
> +#define READ_WORD(fb, reg)		gsc_readl((fb)->info->fix.mmio_start + (reg))
>
>
>   #ifndef DEBUG_STIFB_REGS
>   # define  DEBUG_OFF()
>   # define  DEBUG_ON()
> -# define WRITE_BYTE(value,fb,reg)	gsc_writeb((value),(fb)->info.fix.mmio_start + (reg))
> -# define WRITE_WORD(value,fb,reg)	gsc_writel((value),(fb)->info.fix.mmio_start + (reg))
> +# define WRITE_BYTE(value, fb, reg)	gsc_writeb((value), (fb)->info->fix.mmio_start + (reg))
> +# define WRITE_WORD(value, fb, reg)	gsc_writel((value), (fb)->info->fix.mmio_start + (reg))
>   #else
>     static int debug_on = 1;
>   # define  DEBUG_OFF() debug_on=0
> @@ -169,11 +169,11 @@ static int __initdata stifb_bpp_pref[MAX_STI_ROMS];
>   # define WRITE_BYTE(value,fb,reg)	do { if (debug_on) \
>   						printk(KERN_DEBUG "%30s: WRITE_BYTE(0x%06x) = 0x%02x (old=0x%02x)\n", \
>   							__func__, reg, value, READ_BYTE(fb,reg)); 		  \
> -					gsc_writeb((value),(fb)->info.fix.mmio_start + (reg)); } while (0)
> +					gsc_writeb((value), (fb)->info->fix.mmio_start + (reg)); } while (0)
>   # define WRITE_WORD(value,fb,reg)	do { if (debug_on) \
>   						printk(KERN_DEBUG "%30s: WRITE_WORD(0x%06x) = 0x%08x (old=0x%08x)\n", \
>   							__func__, reg, value, READ_WORD(fb,reg)); 		  \
> -					gsc_writel((value),(fb)->info.fix.mmio_start + (reg)); } while (0)
> +					gsc_writel((value), (fb)->info->fix.mmio_start + (reg)); } while (0)
>   #endif /* DEBUG_STIFB_REGS */
>
>
> @@ -210,13 +210,13 @@ SETUP_FB(struct stifb_info *fb)
>   			reg10_value = 0x13601000;
>   			break;
>   		case S9000_ID_A1439A:
> -			if (fb->info.var.bits_per_pixel == 32)
> +			if (fb->info->var.bits_per_pixel == 32)
>   				reg10_value = 0xBBA0A000;
>   			else
>   				reg10_value = 0x13601000;
>   			break;
>   		case S9000_ID_HCRX:
> -			if (fb->info.var.bits_per_pixel == 32)
> +			if (fb->info->var.bits_per_pixel == 32)
>   				reg10_value = 0xBBA0A000;
>   			else
>   				reg10_value = 0x13602000;
> @@ -254,7 +254,7 @@ static void
>   FINISH_IMAGE_COLORMAP_ACCESS(struct stifb_info *fb)
>   {
>   	WRITE_WORD(0x400, fb, REG_2);
> -	if (fb->info.var.bits_per_pixel == 32) {
> +	if (fb->info->var.bits_per_pixel == 32) {
>   		WRITE_WORD(0x83000100, fb, REG_1);
>   	} else {
>   		if (fb->id == S9000_ID_ARTIST || fb->id == CRT_ID_VISUALIZE_EG)
> @@ -503,7 +503,7 @@ static void
>   ngleSetupAttrPlanes(struct stifb_info *fb, int BufferNumber)
>   {
>   	SETUP_ATTR_ACCESS(fb, BufferNumber);
> -	SET_ATTR_SIZE(fb, fb->info.var.xres, fb->info.var.yres);
> +	SET_ATTR_SIZE(fb, fb->info->var.xres, fb->info->var.yres);
>   	FINISH_ATTR_ACCESS(fb);
>   	SETUP_FB(fb);
>   }
> @@ -526,9 +526,9 @@ rattlerSetupPlanes(struct stifb_info *fb)
>   	SETUP_FB(fb);
>   	fb->id = saved_id;
>
> -	for (y = 0; y < fb->info.var.yres; ++y)
> -		fb_memset_io(fb->info.screen_base + y * fb->info.fix.line_length,
> -			     0xff, fb->info.var.xres * fb->info.var.bits_per_pixel/8);
> +	for (y = 0; y < fb->info->var.yres; ++y)
> +		fb_memset_io(fb->info->screen_base + y * fb->info->fix.line_length,
> +			     0xff, fb->info->var.xres * fb->info->var.bits_per_pixel/8);
>
>   	CRX24_SET_OVLY_MASK(fb);
>   	SETUP_FB(fb);
> @@ -607,7 +607,7 @@ setHyperLutBltCtl(struct stifb_info *fb, int offsetWithinLut, int length)
>   	lutBltCtl.fields.lutType = HYPER_CMAP_TYPE;
>
>   	/* Expect lutIndex to be 0 or 1 for image cmaps, 2 or 3 for overlay cmaps */
> -	if (fb->info.var.bits_per_pixel == 8)
> +	if (fb->info->var.bits_per_pixel == 8)
>   		lutBltCtl.fields.lutOffset = 2 * 256;
>   	else
>   		lutBltCtl.fields.lutOffset = 0 * 256;
> @@ -688,7 +688,7 @@ ngleResetAttrPlanes(struct stifb_info *fb, unsigned int ctlPlaneReg)
>   					       DataDynamic, MaskOtc,
>   					       BGx(0), FGx(0)));
>   	packed_dst = 0;
> -	packed_len = (fb->info.var.xres << 16) | fb->info.var.yres;
> +	packed_len = (fb->info->var.xres << 16) | fb->info->var.yres;
>   	GET_FIFO_SLOTS(fb, nFreeFifoSlots, 2);
>   	NGLE_SET_DSTXY(fb, packed_dst);
>   	SET_LENXY_START_RECFILL(fb, packed_len);
> @@ -738,7 +738,7 @@ ngleClearOverlayPlanes(struct stifb_info *fb, int mask, int data)
>           NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, mask);
>
>           packed_dst = 0;
> -        packed_len = (fb->info.var.xres << 16) | fb->info.var.yres;
> +	packed_len = (fb->info->var.xres << 16) | fb->info->var.yres;
>           NGLE_SET_DSTXY(fb, packed_dst);
>
>   	/* Write zeroes to overlay planes */
> @@ -760,7 +760,7 @@ hyperResetPlanes(struct stifb_info *fb, int enable)
>   	NGLE_LOCK(fb);
>
>   	if (IS_24_DEVICE(fb))
> -		if (fb->info.var.bits_per_pixel == 32)
> +		if (fb->info->var.bits_per_pixel == 32)
>   			controlPlaneReg = 0x04000F00;
>   		else
>   			controlPlaneReg = 0x00000F00;   /* 0x00000800 should be enough, but lets clear all 4 bits */
> @@ -890,7 +890,7 @@ SETUP_HCRX(struct stifb_info *fb)
>   	GET_FIFO_SLOTS(fb, nFreeFifoSlots, 7);
>
>   	if (IS_24_DEVICE(fb)) {
> -		hyperbowl = (fb->info.var.bits_per_pixel == 32) ?
> +		hyperbowl = (fb->info->var.bits_per_pixel == 32) ?
>   			HYPERBOWL_MODE01_8_24_LUT0_TRANSPARENT_LUT1_OPAQUE :
>   			HYPERBOWL_MODE01_8_24_LUT0_OPAQUE_LUT1_OPAQUE;
>
> @@ -924,21 +924,21 @@ SETUP_HCRX(struct stifb_info *fb)
>   static int
>   stifb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>   {
> -	struct stifb_info *fb = container_of(info, struct stifb_info, info);
> +	struct stifb_info *fb = info->par;
>
> -	if (var->xres != fb->info.var.xres ||
> -	    var->yres != fb->info.var.yres ||
> -	    var->bits_per_pixel != fb->info.var.bits_per_pixel)
> +	if (var->xres != fb->info->var.xres ||
> +	    var->yres != fb->info->var.yres ||
> +	    var->bits_per_pixel != fb->info->var.bits_per_pixel)
>   		return -EINVAL;
>
>   	var->xres_virtual = var->xres;
>   	var->yres_virtual = var->yres;
>   	var->xoffset = 0;
>   	var->yoffset = 0;
> -	var->grayscale = fb->info.var.grayscale;
> -	var->red.length = fb->info.var.red.length;
> -	var->green.length = fb->info.var.green.length;
> -	var->blue.length = fb->info.var.blue.length;
> +	var->grayscale = fb->info->var.grayscale;
> +	var->red.length = fb->info->var.red.length;
> +	var->green.length = fb->info->var.green.length;
> +	var->blue.length = fb->info->var.blue.length;
>
>   	return 0;
>   }
> @@ -947,7 +947,7 @@ static int
>   stifb_setcolreg(u_int regno, u_int red, u_int green,
>   	      u_int blue, u_int transp, struct fb_info *info)
>   {
> -	struct stifb_info *fb = container_of(info, struct stifb_info, info);
> +	struct stifb_info *fb = info->par;
>   	u32 color;
>
>   	if (regno >= NR_PALETTE)
> @@ -961,7 +961,7 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
>
>   	START_IMAGE_COLORMAP_ACCESS(fb);
>
> -	if (unlikely(fb->info.var.grayscale)) {
> +	if (unlikely(fb->info->var.grayscale)) {
>   		/* gray = 0.30*R + 0.59*G + 0.11*B */
>   		color = ((red * 77) +
>   			 (green * 151) +
> @@ -972,10 +972,10 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
>   			 (blue));
>   	}
>
> -	if (fb->info.fix.visual == FB_VISUAL_DIRECTCOLOR) {
> -		struct fb_var_screeninfo *var = &fb->info.var;
> +	if (fb->info->fix.visual == FB_VISUAL_DIRECTCOLOR) {
> +		struct fb_var_screeninfo *var = &fb->info->var;
>   		if (regno < 16)
> -			((u32 *)fb->info.pseudo_palette)[regno] =
> +			((u32 *)fb->info->pseudo_palette)[regno] =
>   				regno << var->red.offset |
>   				regno << var->green.offset |
>   				regno << var->blue.offset;
> @@ -1007,7 +1007,7 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
>   static int
>   stifb_blank(int blank_mode, struct fb_info *info)
>   {
> -	struct stifb_info *fb = container_of(info, struct stifb_info, info);
> +	struct stifb_info *fb = info->par;
>   	int enable = (blank_mode == 0) ? ENABLE : DISABLE;
>
>   	switch (fb->id) {
> @@ -1036,12 +1036,12 @@ stifb_blank(int blank_mode, struct fb_info *info)
>   static void
>   stifb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
>   {
> -	struct stifb_info *fb = container_of(info, struct stifb_info, info);
> +	struct stifb_info *fb = info->par;
>
>   	SETUP_COPYAREA(fb);
>
>   	SETUP_HW(fb);
> -	if (fb->info.var.bits_per_pixel == 32) {
> +	if (fb->info->var.bits_per_pixel == 32) {
>   		WRITE_WORD(0xBBA0A000, fb, REG_10);
>
>   		NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, 0xffffffff);
> @@ -1075,15 +1075,15 @@ stifb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
>   static void
>   stifb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
>   {
> -	struct stifb_info *fb = container_of(info, struct stifb_info, info);
> +	struct stifb_info *fb = info->par;
>
>   	if (rect->rop != ROP_COPY ||
> -	    (fb->id == S9000_ID_HCRX && fb->info.var.bits_per_pixel == 32))
> +	    (fb->id == S9000_ID_HCRX && fb->info->var.bits_per_pixel == 32))
>   		return cfb_fillrect(info, rect);
>
>   	SETUP_HW(fb);
>
> -	if (fb->info.var.bits_per_pixel == 32) {
> +	if (fb->info->var.bits_per_pixel == 32) {
>   		WRITE_WORD(0xBBA0A000, fb, REG_10);
>
>   		NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, 0xffffffff);
> @@ -1141,7 +1141,7 @@ stifb_init_display(struct stifb_info *fb)
>           switch (id) {
>   	 case S9000_ID_A1659A:
>   	 case S9000_ID_A1439A:
> -	    if (fb->info.var.bits_per_pixel == 32)
> +	    if (fb->info->var.bits_per_pixel == 32)
>   		ngleSetupAttrPlanes(fb, BUFF1_CMAP3);
>   	    else {
>   		ngleSetupAttrPlanes(fb, BUFF1_CMAP0);
> @@ -1151,7 +1151,7 @@ stifb_init_display(struct stifb_info *fb)
>   	    break;
>   	 case S9000_ID_ARTIST:
>   	 case CRT_ID_VISUALIZE_EG:
> -	    if (fb->info.var.bits_per_pixel == 32)
> +	    if (fb->info->var.bits_per_pixel == 32)
>   		ngleSetupAttrPlanes(fb, BUFF1_CMAP3);
>   	    else {
>   		ngleSetupAttrPlanes(fb, ARTIST_CMAP0);
> @@ -1193,11 +1193,11 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
>   	char *dev_name;
>   	int bpp, xres, yres;
>
> -	fb = kzalloc(sizeof(*fb), GFP_ATOMIC);
> -	if (!fb)
> +	info = framebuffer_alloc(sizeof(*fb), sti->dev);
> +	if (!info)
>   		return -ENOMEM;
> -
> -	info = &fb->info;
> +	fb = info->par;
> +	fb->info = info;
>
>   	/* set struct to a known state */
>   	fix = &info->fix;
> @@ -1390,10 +1390,10 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
>
>   	/* save for primary gfx device detection & unregister_framebuffer() */
>   	sti->info = info;
> -	if (register_framebuffer(&fb->info) < 0)
> +	if (register_framebuffer(fb->info) < 0)
>   		goto out_err4;
>
> -	fb_info(&fb->info, "%s %dx%d-%d frame buffer device, %s, id: %04x, mmio: 0x%04lx\n",
> +	fb_info(fb->info, "%s %dx%d-%d frame buffer device, %s, id: %04x, mmio: 0x%04lx\n",
>   		fix->id,
>   		var->xres,
>   		var->yres,
> @@ -1402,6 +1402,8 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
>   		fb->id,
>   		fix->mmio_start);
>
> +	dev_set_drvdata(sti->dev, info);
> +
>   	return 0;
>
>
> @@ -1414,7 +1416,7 @@ static int __init stifb_init_fb(struct sti_struct *sti, int bpp_pref)
>   out_err1:
>   	iounmap(info->screen_base);
>   out_err0:
> -	kfree(fb);
> +	framebuffer_release(info);
>   	sti->info = NULL;
>   	return -ENXIO;
>   }
> @@ -1480,15 +1482,19 @@ stifb_cleanup(void)
>   		sti = sti_get_rom(i);
>   		if (!sti)
>   			break;
> -		if (sti->info) {
> -			struct fb_info *info = sti->info;
> -			unregister_framebuffer(sti->info);
> +		if (sti->dev) {
> +			struct fb_info *info = dev_get_drvdata(sti->dev);
> +
> +			if (!info)
> +				continue;
> +			unregister_framebuffer(info);
>   			release_mem_region(info->fix.mmio_start, info->fix.mmio_len);
>   		        release_mem_region(info->fix.smem_start, info->fix.smem_len);
>   				if (info->screen_base)
>   					iounmap(info->screen_base);
>   		        fb_dealloc_cmap(&info->cmap);
>   		        framebuffer_release(info);
> +			dev_set_drvdata(sti->dev, NULL);
>   		}
>   		sti->info = NULL;
>   	}

Re: [PATCH 2/4] fbdev/stifb: Allocate fb_info instance with framebuffer_alloc()

[Thomas Zimmermann]

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

Hi Helge

Am 30.12.23 um 09:35 schrieb Helge Deller:
> Hi Thomas,
> 
> On 12/20/23 14:22, Thomas Zimmermann wrote:
>> Allocate stifb's instance of fb_info with framebuffer_alloc(). This
>> is the preferred way of creating fb_info with associated driver data
>> stored in struct fb_info.par. Requires several, but minor, changes
>> through out the driver's code.
>>
>> The intended side effect of this patch is that the new instance of
>> struct fb_info now has its device field correctly set to the parent
>> device of the STI ROM. A later patch can detect if the device is the
>> firmware's primary output. It is also now correctly located within
>> the Linux device hierarchy.
> 
> Thanks for this cleanup series!
> Sadly stifb then crashes at bootup.
> 
> Please include this copy&pasted hunk in this patch, which then fixes the 
> crash:
> 
> diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
> index 2de0e675fd15..f8bbd8d6f5b2 100644
> --- a/drivers/video/fbdev/stifb.c
> +++ b/drivers/video/fbdev/stifb.c
> @@ -1158,7 +1158,7 @@ stifb_init_display(struct stifb_info *fb)
>              }
>              break;
>          }
> -       stifb_blank(0, (struct fb_info *)fb);   /* 0=enable screen */
> +       stifb_blank(0, fb->info);       /* 0=enable screen */
> 
> 
> 
> With this applied, you may add to the series:
> 
> Tested-by: Helge Deller <deller@gmx.de>
> Reviewed-by: Helge Deller <deller@gmx.de>

Thanks so much for testing and fixing the problem.

Which tree should this go through?

Best regards
Thomas

> 
> Thanks!
> Helge
> 
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/video/fbdev/stifb.c | 106 +++++++++++++++++++-----------------
>>   1 file changed, 56 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
>> index 548d992f8cb1..36e6bcab83aa 100644
>> --- a/drivers/video/fbdev/stifb.c
>> +++ b/drivers/video/fbdev/stifb.c
>> @@ -103,7 +103,7 @@ typedef struct {
>>   } ngle_rom_t;
>>
>>   struct stifb_info {
>> -    struct fb_info info;
>> +    struct fb_info *info;
>>       unsigned int id;
>>       ngle_rom_t ngle_rom;
>>       struct sti_struct *sti;
>> @@ -153,15 +153,15 @@ static int __initdata stifb_bpp_pref[MAX_STI_ROMS];
>>   #define REG_44        0x210030
>>   #define REG_45        0x210034
>>
>> -#define READ_BYTE(fb,reg)        gsc_readb((fb)->info.fix.mmio_start 
>> + (reg))
>> -#define READ_WORD(fb,reg)        gsc_readl((fb)->info.fix.mmio_start 
>> + (reg))
>> +#define READ_BYTE(fb, reg)        
>> gsc_readb((fb)->info->fix.mmio_start + (reg))
>> +#define READ_WORD(fb, reg)        
>> gsc_readl((fb)->info->fix.mmio_start + (reg))
>>
>>
>>   #ifndef DEBUG_STIFB_REGS
>>   # define  DEBUG_OFF()
>>   # define  DEBUG_ON()
>> -# define WRITE_BYTE(value,fb,reg)    
>> gsc_writeb((value),(fb)->info.fix.mmio_start + (reg))
>> -# define WRITE_WORD(value,fb,reg)    
>> gsc_writel((value),(fb)->info.fix.mmio_start + (reg))
>> +# define WRITE_BYTE(value, fb, reg)    gsc_writeb((value), 
>> (fb)->info->fix.mmio_start + (reg))
>> +# define WRITE_WORD(value, fb, reg)    gsc_writel((value), 
>> (fb)->info->fix.mmio_start + (reg))
>>   #else
>>     static int debug_on = 1;
>>   # define  DEBUG_OFF() debug_on=0
>> @@ -169,11 +169,11 @@ static int __initdata stifb_bpp_pref[MAX_STI_ROMS];
>>   # define WRITE_BYTE(value,fb,reg)    do { if (debug_on) \
>>                           printk(KERN_DEBUG "%30s: WRITE_BYTE(0x%06x) 
>> = 0x%02x (old=0x%02x)\n", \
>>                               __func__, reg, value, 
>> READ_BYTE(fb,reg));           \
>> -                    gsc_writeb((value),(fb)->info.fix.mmio_start + 
>> (reg)); } while (0)
>> +                    gsc_writeb((value), (fb)->info->fix.mmio_start + 
>> (reg)); } while (0)
>>   # define WRITE_WORD(value,fb,reg)    do { if (debug_on) \
>>                           printk(KERN_DEBUG "%30s: WRITE_WORD(0x%06x) 
>> = 0x%08x (old=0x%08x)\n", \
>>                               __func__, reg, value, 
>> READ_WORD(fb,reg));           \
>> -                    gsc_writel((value),(fb)->info.fix.mmio_start + 
>> (reg)); } while (0)
>> +                    gsc_writel((value), (fb)->info->fix.mmio_start + 
>> (reg)); } while (0)
>>   #endif /* DEBUG_STIFB_REGS */
>>
>>
>> @@ -210,13 +210,13 @@ SETUP_FB(struct stifb_info *fb)
>>               reg10_value = 0x13601000;
>>               break;
>>           case S9000_ID_A1439A:
>> -            if (fb->info.var.bits_per_pixel == 32)
>> +            if (fb->info->var.bits_per_pixel == 32)
>>                   reg10_value = 0xBBA0A000;
>>               else
>>                   reg10_value = 0x13601000;
>>               break;
>>           case S9000_ID_HCRX:
>> -            if (fb->info.var.bits_per_pixel == 32)
>> +            if (fb->info->var.bits_per_pixel == 32)
>>                   reg10_value = 0xBBA0A000;
>>               else
>>                   reg10_value = 0x13602000;
>> @@ -254,7 +254,7 @@ static void
>>   FINISH_IMAGE_COLORMAP_ACCESS(struct stifb_info *fb)
>>   {
>>       WRITE_WORD(0x400, fb, REG_2);
>> -    if (fb->info.var.bits_per_pixel == 32) {
>> +    if (fb->info->var.bits_per_pixel == 32) {
>>           WRITE_WORD(0x83000100, fb, REG_1);
>>       } else {
>>           if (fb->id == S9000_ID_ARTIST || fb->id == CRT_ID_VISUALIZE_EG)
>> @@ -503,7 +503,7 @@ static void
>>   ngleSetupAttrPlanes(struct stifb_info *fb, int BufferNumber)
>>   {
>>       SETUP_ATTR_ACCESS(fb, BufferNumber);
>> -    SET_ATTR_SIZE(fb, fb->info.var.xres, fb->info.var.yres);
>> +    SET_ATTR_SIZE(fb, fb->info->var.xres, fb->info->var.yres);
>>       FINISH_ATTR_ACCESS(fb);
>>       SETUP_FB(fb);
>>   }
>> @@ -526,9 +526,9 @@ rattlerSetupPlanes(struct stifb_info *fb)
>>       SETUP_FB(fb);
>>       fb->id = saved_id;
>>
>> -    for (y = 0; y < fb->info.var.yres; ++y)
>> -        fb_memset_io(fb->info.screen_base + y * 
>> fb->info.fix.line_length,
>> -                 0xff, fb->info.var.xres * 
>> fb->info.var.bits_per_pixel/8);
>> +    for (y = 0; y < fb->info->var.yres; ++y)
>> +        fb_memset_io(fb->info->screen_base + y * 
>> fb->info->fix.line_length,
>> +                 0xff, fb->info->var.xres * 
>> fb->info->var.bits_per_pixel/8);
>>
>>       CRX24_SET_OVLY_MASK(fb);
>>       SETUP_FB(fb);
>> @@ -607,7 +607,7 @@ setHyperLutBltCtl(struct stifb_info *fb, int 
>> offsetWithinLut, int length)
>>       lutBltCtl.fields.lutType = HYPER_CMAP_TYPE;
>>
>>       /* Expect lutIndex to be 0 or 1 for image cmaps, 2 or 3 for 
>> overlay cmaps */
>> -    if (fb->info.var.bits_per_pixel == 8)
>> +    if (fb->info->var.bits_per_pixel == 8)
>>           lutBltCtl.fields.lutOffset = 2 * 256;
>>       else
>>           lutBltCtl.fields.lutOffset = 0 * 256;
>> @@ -688,7 +688,7 @@ ngleResetAttrPlanes(struct stifb_info *fb, 
>> unsigned int ctlPlaneReg)
>>                              DataDynamic, MaskOtc,
>>                              BGx(0), FGx(0)));
>>       packed_dst = 0;
>> -    packed_len = (fb->info.var.xres << 16) | fb->info.var.yres;
>> +    packed_len = (fb->info->var.xres << 16) | fb->info->var.yres;
>>       GET_FIFO_SLOTS(fb, nFreeFifoSlots, 2);
>>       NGLE_SET_DSTXY(fb, packed_dst);
>>       SET_LENXY_START_RECFILL(fb, packed_len);
>> @@ -738,7 +738,7 @@ ngleClearOverlayPlanes(struct stifb_info *fb, int 
>> mask, int data)
>>           NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, mask);
>>
>>           packed_dst = 0;
>> -        packed_len = (fb->info.var.xres << 16) | fb->info.var.yres;
>> +    packed_len = (fb->info->var.xres << 16) | fb->info->var.yres;
>>           NGLE_SET_DSTXY(fb, packed_dst);
>>
>>       /* Write zeroes to overlay planes */
>> @@ -760,7 +760,7 @@ hyperResetPlanes(struct stifb_info *fb, int enable)
>>       NGLE_LOCK(fb);
>>
>>       if (IS_24_DEVICE(fb))
>> -        if (fb->info.var.bits_per_pixel == 32)
>> +        if (fb->info->var.bits_per_pixel == 32)
>>               controlPlaneReg = 0x04000F00;
>>           else
>>               controlPlaneReg = 0x00000F00;   /* 0x00000800 should be 
>> enough, but lets clear all 4 bits */
>> @@ -890,7 +890,7 @@ SETUP_HCRX(struct stifb_info *fb)
>>       GET_FIFO_SLOTS(fb, nFreeFifoSlots, 7);
>>
>>       if (IS_24_DEVICE(fb)) {
>> -        hyperbowl = (fb->info.var.bits_per_pixel == 32) ?
>> +        hyperbowl = (fb->info->var.bits_per_pixel == 32) ?
>>               HYPERBOWL_MODE01_8_24_LUT0_TRANSPARENT_LUT1_OPAQUE :
>>               HYPERBOWL_MODE01_8_24_LUT0_OPAQUE_LUT1_OPAQUE;
>>
>> @@ -924,21 +924,21 @@ SETUP_HCRX(struct stifb_info *fb)
>>   static int
>>   stifb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>>   {
>> -    struct stifb_info *fb = container_of(info, struct stifb_info, info);
>> +    struct stifb_info *fb = info->par;
>>
>> -    if (var->xres != fb->info.var.xres ||
>> -        var->yres != fb->info.var.yres ||
>> -        var->bits_per_pixel != fb->info.var.bits_per_pixel)
>> +    if (var->xres != fb->info->var.xres ||
>> +        var->yres != fb->info->var.yres ||
>> +        var->bits_per_pixel != fb->info->var.bits_per_pixel)
>>           return -EINVAL;
>>
>>       var->xres_virtual = var->xres;
>>       var->yres_virtual = var->yres;
>>       var->xoffset = 0;
>>       var->yoffset = 0;
>> -    var->grayscale = fb->info.var.grayscale;
>> -    var->red.length = fb->info.var.red.length;
>> -    var->green.length = fb->info.var.green.length;
>> -    var->blue.length = fb->info.var.blue.length;
>> +    var->grayscale = fb->info->var.grayscale;
>> +    var->red.length = fb->info->var.red.length;
>> +    var->green.length = fb->info->var.green.length;
>> +    var->blue.length = fb->info->var.blue.length;
>>
>>       return 0;
>>   }
>> @@ -947,7 +947,7 @@ static int
>>   stifb_setcolreg(u_int regno, u_int red, u_int green,
>>             u_int blue, u_int transp, struct fb_info *info)
>>   {
>> -    struct stifb_info *fb = container_of(info, struct stifb_info, info);
>> +    struct stifb_info *fb = info->par;
>>       u32 color;
>>
>>       if (regno >= NR_PALETTE)
>> @@ -961,7 +961,7 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
>>
>>       START_IMAGE_COLORMAP_ACCESS(fb);
>>
>> -    if (unlikely(fb->info.var.grayscale)) {
>> +    if (unlikely(fb->info->var.grayscale)) {
>>           /* gray = 0.30*R + 0.59*G + 0.11*B */
>>           color = ((red * 77) +
>>                (green * 151) +
>> @@ -972,10 +972,10 @@ stifb_setcolreg(u_int regno, u_int red, u_int 
>> green,
>>                (blue));
>>       }
>>
>> -    if (fb->info.fix.visual == FB_VISUAL_DIRECTCOLOR) {
>> -        struct fb_var_screeninfo *var = &fb->info.var;
>> +    if (fb->info->fix.visual == FB_VISUAL_DIRECTCOLOR) {
>> +        struct fb_var_screeninfo *var = &fb->info->var;
>>           if (regno < 16)
>> -            ((u32 *)fb->info.pseudo_palette)[regno] =
>> +            ((u32 *)fb->info->pseudo_palette)[regno] =
>>                   regno << var->red.offset |
>>                   regno << var->green.offset |
>>                   regno << var->blue.offset;
>> @@ -1007,7 +1007,7 @@ stifb_setcolreg(u_int regno, u_int red, u_int 
>> green,
>>   static int
>>   stifb_blank(int blank_mode, struct fb_info *info)
>>   {
>> -    struct stifb_info *fb = container_of(info, struct stifb_info, info);
>> +    struct stifb_info *fb = info->par;
>>       int enable = (blank_mode == 0) ? ENABLE : DISABLE;
>>
>>       switch (fb->id) {
>> @@ -1036,12 +1036,12 @@ stifb_blank(int blank_mode, struct fb_info *info)
>>   static void
>>   stifb_copyarea(struct fb_info *info, const struct fb_copyarea *area)
>>   {
>> -    struct stifb_info *fb = container_of(info, struct stifb_info, info);
>> +    struct stifb_info *fb = info->par;
>>
>>       SETUP_COPYAREA(fb);
>>
>>       SETUP_HW(fb);
>> -    if (fb->info.var.bits_per_pixel == 32) {
>> +    if (fb->info->var.bits_per_pixel == 32) {
>>           WRITE_WORD(0xBBA0A000, fb, REG_10);
>>
>>           NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, 0xffffffff);
>> @@ -1075,15 +1075,15 @@ stifb_copyarea(struct fb_info *info, const 
>> struct fb_copyarea *area)
>>   static void
>>   stifb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
>>   {
>> -    struct stifb_info *fb = container_of(info, struct stifb_info, info);
>> +    struct stifb_info *fb = info->par;
>>
>>       if (rect->rop != ROP_COPY ||
>> -        (fb->id == S9000_ID_HCRX && fb->info.var.bits_per_pixel == 32))
>> +        (fb->id == S9000_ID_HCRX && fb->info->var.bits_per_pixel == 32))
>>           return cfb_fillrect(info, rect);
>>
>>       SETUP_HW(fb);
>>
>> -    if (fb->info.var.bits_per_pixel == 32) {
>> +    if (fb->info->var.bits_per_pixel == 32) {
>>           WRITE_WORD(0xBBA0A000, fb, REG_10);
>>
>>           NGLE_REALLY_SET_IMAGE_PLANEMASK(fb, 0xffffffff);
>> @@ -1141,7 +1141,7 @@ stifb_init_display(struct stifb_info *fb)
>>           switch (id) {
>>        case S9000_ID_A1659A:
>>        case S9000_ID_A1439A:
>> -        if (fb->info.var.bits_per_pixel == 32)
>> +        if (fb->info->var.bits_per_pixel == 32)
>>           ngleSetupAttrPlanes(fb, BUFF1_CMAP3);
>>           else {
>>           ngleSetupAttrPlanes(fb, BUFF1_CMAP0);
>> @@ -1151,7 +1151,7 @@ stifb_init_display(struct stifb_info *fb)
>>           break;
>>        case S9000_ID_ARTIST:
>>        case CRT_ID_VISUALIZE_EG:
>> -        if (fb->info.var.bits_per_pixel == 32)
>> +        if (fb->info->var.bits_per_pixel == 32)
>>           ngleSetupAttrPlanes(fb, BUFF1_CMAP3);
>>           else {
>>           ngleSetupAttrPlanes(fb, ARTIST_CMAP0);
>> @@ -1193,11 +1193,11 @@ static int __init stifb_init_fb(struct 
>> sti_struct *sti, int bpp_pref)
>>       char *dev_name;
>>       int bpp, xres, yres;
>>
>> -    fb = kzalloc(sizeof(*fb), GFP_ATOMIC);
>> -    if (!fb)
>> +    info = framebuffer_alloc(sizeof(*fb), sti->dev);
>> +    if (!info)
>>           return -ENOMEM;
>> -
>> -    info = &fb->info;
>> +    fb = info->par;
>> +    fb->info = info;
>>
>>       /* set struct to a known state */
>>       fix = &info->fix;
>> @@ -1390,10 +1390,10 @@ static int __init stifb_init_fb(struct 
>> sti_struct *sti, int bpp_pref)
>>
>>       /* save for primary gfx device detection & 
>> unregister_framebuffer() */
>>       sti->info = info;
>> -    if (register_framebuffer(&fb->info) < 0)
>> +    if (register_framebuffer(fb->info) < 0)
>>           goto out_err4;
>>
>> -    fb_info(&fb->info, "%s %dx%d-%d frame buffer device, %s, id: 
>> %04x, mmio: 0x%04lx\n",
>> +    fb_info(fb->info, "%s %dx%d-%d frame buffer device, %s, id: %04x, 
>> mmio: 0x%04lx\n",
>>           fix->id,
>>           var->xres,
>>           var->yres,
>> @@ -1402,6 +1402,8 @@ static int __init stifb_init_fb(struct 
>> sti_struct *sti, int bpp_pref)
>>           fb->id,
>>           fix->mmio_start);
>>
>> +    dev_set_drvdata(sti->dev, info);
>> +
>>       return 0;
>>
>>
>> @@ -1414,7 +1416,7 @@ static int __init stifb_init_fb(struct 
>> sti_struct *sti, int bpp_pref)
>>   out_err1:
>>       iounmap(info->screen_base);
>>   out_err0:
>> -    kfree(fb);
>> +    framebuffer_release(info);
>>       sti->info = NULL;
>>       return -ENXIO;
>>   }
>> @@ -1480,15 +1482,19 @@ stifb_cleanup(void)
>>           sti = sti_get_rom(i);
>>           if (!sti)
>>               break;
>> -        if (sti->info) {
>> -            struct fb_info *info = sti->info;
>> -            unregister_framebuffer(sti->info);
>> +        if (sti->dev) {
>> +            struct fb_info *info = dev_get_drvdata(sti->dev);
>> +
>> +            if (!info)
>> +                continue;
>> +            unregister_framebuffer(info);
>>               release_mem_region(info->fix.mmio_start, 
>> info->fix.mmio_len);
>>                   release_mem_region(info->fix.smem_start, 
>> info->fix.smem_len);
>>                   if (info->screen_base)
>>                       iounmap(info->screen_base);
>>                   fb_dealloc_cmap(&info->cmap);
>>                   framebuffer_release(info);
>> +            dev_set_drvdata(sti->dev, NULL);
>>           }
>>           sti->info = NULL;
>>       }
> 

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

Re: [PATCH 1/4] video/sticore: Store ROM device in STI struct

[Helge Deller]

On 12/20/23 14:22, Thomas Zimmermann wrote:
> Store the ROM's parent device in each STI struct, so we can associate
> the STI framebuffer with a device.
>
> The new field will eventually replace the fbdev subsystem's info field,
> which the function fb_is_primary_device() currently requires to detect
> the firmware's output. By using the device instead of the framebuffer
> info, a later patch can generalize the helper for use in non-fbdev code.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/video/sticore.c | 5 +++++
>   include/video/sticore.h | 4 ++++
>   2 files changed, 9 insertions(+)

Series applied to fbdev git tree.

Thanks!
Helge