[PATCH v2 0/5] fbdev: Use regular I/O function for framebuffers

[PATCH v2 0/5] fbdev: Use regular I/O function for framebuffers

[Thomas Zimmermann]

(was: fbdev: Move framebuffer I/O helpers to <asm/fb.h>)

Fbdev provides helpers for framebuffer I/O, such as fb_readl(),
fb_writel() or fb_memcpy_to_fb(). The implementation of each helper
depends on the architecture, but they all come down to regular I/O
functions of similar names. So use the regular functions instead.

The first patch a simple whitespace cleanup.

Until now, <linux/fb.h> contained an include of <asm/io.h>. As this
will go away patches 2 to 4 prepare include statements in the various
drivers. Source files that use regular I/O helpers, such as readl(),
now include <linux/io.h>. Source files that use framebuffer I/O
helpers, such as fb_readl(), also include <linux/io.h>.

Patch 5 replaces the architecture-based if-else branching in 
<linux/fb.h> by define statements that map to Linux' I/O fucntions.

After this change has been merged and included in a few release
without complains, we can update the drivers to regular I/O functions
and remove the fbdev-specific defines.

The patchset has been built for a variety of platforms, such as x86-64,
arm, aarch64, ppc64, parisc, m64k, mips and sparc.

v2:
	* use Linux I/O helpers (Sam, Arnd)

Thomas Zimmermann (5):
  fbdev/matrox: Remove trailing whitespaces
  ipu-v3: Include <linux/io.h>
  fbdev: Include <linux/io.h> in various drivers
  fbdev: Include <linux/io.h> in drivers
  fbdev: Define framebuffer I/O from Linux' I/O functions

 drivers/gpu/ipu-v3/ipu-prv.h                |  1 +
 drivers/video/fbdev/arcfb.c                 |  1 +
 drivers/video/fbdev/arkfb.c                 |  1 +
 drivers/video/fbdev/aty/atyfb.h             |  2 +
 drivers/video/fbdev/aty/mach64_cursor.c     |  3 +-
 drivers/video/fbdev/chipsfb.c               |  1 +
 drivers/video/fbdev/cirrusfb.c              |  1 +
 drivers/video/fbdev/core/cfbcopyarea.c      |  2 +-
 drivers/video/fbdev/core/cfbfillrect.c      |  2 +
 drivers/video/fbdev/core/cfbimgblt.c        |  2 +
 drivers/video/fbdev/core/fbmem.c            |  1 +
 drivers/video/fbdev/core/svgalib.c          |  2 +-
 drivers/video/fbdev/hgafb.c                 |  2 +-
 drivers/video/fbdev/hitfb.c                 |  2 +-
 drivers/video/fbdev/kyro/fbdev.c            |  2 +-
 drivers/video/fbdev/matrox/matroxfb_accel.c |  8 ++-
 drivers/video/fbdev/matrox/matroxfb_base.h  |  6 +-
 drivers/video/fbdev/pm2fb.c                 |  1 +
 drivers/video/fbdev/pm3fb.c                 |  1 +
 drivers/video/fbdev/pvr2fb.c                |  1 +
 drivers/video/fbdev/s3fb.c                  |  1 +
 drivers/video/fbdev/sstfb.c                 |  2 +-
 drivers/video/fbdev/tdfxfb.c                |  2 +-
 drivers/video/fbdev/tridentfb.c             |  1 +
 drivers/video/fbdev/vga16fb.c               |  2 +-
 drivers/video/fbdev/vt8623fb.c              |  1 +
 drivers/video/fbdev/wmt_ge_rops.c           |  2 +
 include/linux/fb.h                          | 63 +++++----------------
 28 files changed, 52 insertions(+), 64 deletions(-)

-- 
2.40.0


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

[PATCH v2 1/5] fbdev/matrox: Remove trailing whitespaces

[Thomas Zimmermann]

Fix coding style. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +++---
 drivers/video/fbdev/matrox/matroxfb_base.h  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c b/drivers/video/fbdev/matrox/matroxfb_accel.c
index 9cb0685feddd..ce51227798a1 100644
--- a/drivers/video/fbdev/matrox/matroxfb_accel.c
+++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
@@ -88,7 +88,7 @@
 
 static inline void matrox_cfb4_pal(u_int32_t* pal) {
 	unsigned int i;
-	
+
 	for (i = 0; i < 16; i++) {
 		pal[i] = i * 0x11111111U;
 	}
@@ -96,7 +96,7 @@ static inline void matrox_cfb4_pal(u_int32_t* pal) {
 
 static inline void matrox_cfb8_pal(u_int32_t* pal) {
 	unsigned int i;
-	
+
 	for (i = 0; i < 16; i++) {
 		pal[i] = i * 0x01010101U;
 	}
@@ -482,7 +482,7 @@ static void matroxfb_1bpp_imageblit(struct matrox_fb_info *minfo, u_int32_t fgx,
 			/* Tell... well, why bother... */
 			while (height--) {
 				size_t i;
-				
+
 				for (i = 0; i < step; i += 4) {
 					/* Hope that there are at least three readable bytes beyond the end of bitmap */
 					fb_writel(get_unaligned((u_int32_t*)(chardata + i)),mmio.vaddr);
diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h b/drivers/video/fbdev/matrox/matroxfb_base.h
index 958be6805f87..c93c69bbcd57 100644
--- a/drivers/video/fbdev/matrox/matroxfb_base.h
+++ b/drivers/video/fbdev/matrox/matroxfb_base.h
@@ -301,9 +301,9 @@ struct matrox_altout {
 	int		(*verifymode)(void* altout_dev, u_int32_t mode);
 	int		(*getqueryctrl)(void* altout_dev,
 					struct v4l2_queryctrl* ctrl);
-	int		(*getctrl)(void* altout_dev, 
+	int		(*getctrl)(void *altout_dev,
 				   struct v4l2_control* ctrl);
-	int		(*setctrl)(void* altout_dev, 
+	int		(*setctrl)(void *altout_dev,
 				   struct v4l2_control* ctrl);
 };
 
-- 
2.40.0


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 1/5] fbdev/matrox: Remove trailing whitespaces

[Sam Ravnborg]

On Fri, Apr 28, 2023 at 11:27:07AM +0200, Thomas Zimmermann wrote:
> Fix coding style. No functional changes.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

[PATCH v2 2/5] ipu-v3: Include <linux/io.h>

[Thomas Zimmermann]

The code uses readl() and writel(). Include the header file to
get the declarations.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/ipu-v3/ipu-prv.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/ipu-v3/ipu-prv.h b/drivers/gpu/ipu-v3/ipu-prv.h
index 291ac1bab66d..d4621b1ea7f1 100644
--- a/drivers/gpu/ipu-v3/ipu-prv.h
+++ b/drivers/gpu/ipu-v3/ipu-prv.h
@@ -8,6 +8,7 @@
 
 struct ipu_soc;
 
+#include <linux/io.h>
 #include <linux/types.h>
 #include <linux/device.h>
 #include <linux/clk.h>
-- 
2.40.0


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 2/5] ipu-v3: Include <linux/io.h>

[Sam Ravnborg]

On Fri, Apr 28, 2023 at 11:27:08AM +0200, Thomas Zimmermann wrote:
> The code uses readl() and writel(). Include the header file to
> get the declarations.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/gpu/ipu-v3/ipu-prv.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-prv.h b/drivers/gpu/ipu-v3/ipu-prv.h
> index 291ac1bab66d..d4621b1ea7f1 100644
> --- a/drivers/gpu/ipu-v3/ipu-prv.h
> +++ b/drivers/gpu/ipu-v3/ipu-prv.h
> @@ -8,6 +8,7 @@
>  
>  struct ipu_soc;
>  
> +#include <linux/io.h>
>  #include <linux/types.h>
>  #include <linux/device.h>
>  #include <linux/clk.h>
> -- 
> 2.40.0

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

[PATCH v2 3/5] fbdev: Include <linux/io.h> in various drivers

[Thomas Zimmermann]

The code uses writel() and similar I/O-memory helpers. Include
the header file to get the declarations.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/arcfb.c       | 1 +
 drivers/video/fbdev/aty/atyfb.h   | 2 ++
 drivers/video/fbdev/wmt_ge_rops.c | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/drivers/video/fbdev/arcfb.c b/drivers/video/fbdev/arcfb.c
index 45e64016db32..d631d53f42ad 100644
--- a/drivers/video/fbdev/arcfb.c
+++ b/drivers/video/fbdev/arcfb.c
@@ -41,6 +41,7 @@
 #include <linux/vmalloc.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/io.h>
 #include <linux/fb.h>
 #include <linux/init.h>
 #include <linux/arcfb.h>
diff --git a/drivers/video/fbdev/aty/atyfb.h b/drivers/video/fbdev/aty/atyfb.h
index 465f55beb97f..30da3e82ed3c 100644
--- a/drivers/video/fbdev/aty/atyfb.h
+++ b/drivers/video/fbdev/aty/atyfb.h
@@ -3,8 +3,10 @@
  *  ATI Frame Buffer Device Driver Core Definitions
  */
 
+#include <linux/io.h>
 #include <linux/spinlock.h>
 #include <linux/wait.h>
+
     /*
      *  Elements of the hardware specific atyfb_par structure
      */
diff --git a/drivers/video/fbdev/wmt_ge_rops.c b/drivers/video/fbdev/wmt_ge_rops.c
index 42255d27a1db..99c7b0aea615 100644
--- a/drivers/video/fbdev/wmt_ge_rops.c
+++ b/drivers/video/fbdev/wmt_ge_rops.c
@@ -9,7 +9,9 @@
 
 #include <linux/module.h>
 #include <linux/fb.h>
+#include <linux/io.h>
 #include <linux/platform_device.h>
+
 #include "core/fb_draw.h"
 #include "wmt_ge_rops.h"
 
-- 
2.40.0


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 3/5] fbdev: Include <linux/io.h> in various drivers

[Sam Ravnborg]

On Fri, Apr 28, 2023 at 11:27:09AM +0200, Thomas Zimmermann wrote:
> The code uses writel() and similar I/O-memory helpers. Include
> the header file to get the declarations.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/video/fbdev/arcfb.c       | 1 +
>  drivers/video/fbdev/aty/atyfb.h   | 2 ++
>  drivers/video/fbdev/wmt_ge_rops.c | 2 ++
>  3 files changed, 5 insertions(+)
> 
> diff --git a/drivers/video/fbdev/arcfb.c b/drivers/video/fbdev/arcfb.c
> index 45e64016db32..d631d53f42ad 100644
> --- a/drivers/video/fbdev/arcfb.c
> +++ b/drivers/video/fbdev/arcfb.c
> @@ -41,6 +41,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
> +#include <linux/io.h>
>  #include <linux/fb.h>
>  #include <linux/init.h>
>  #include <linux/arcfb.h>
> diff --git a/drivers/video/fbdev/aty/atyfb.h b/drivers/video/fbdev/aty/atyfb.h
> index 465f55beb97f..30da3e82ed3c 100644
> --- a/drivers/video/fbdev/aty/atyfb.h
> +++ b/drivers/video/fbdev/aty/atyfb.h
> @@ -3,8 +3,10 @@
>   *  ATI Frame Buffer Device Driver Core Definitions
>   */
>  
> +#include <linux/io.h>
>  #include <linux/spinlock.h>
>  #include <linux/wait.h>
> +
>      /*
>       *  Elements of the hardware specific atyfb_par structure
>       */
> diff --git a/drivers/video/fbdev/wmt_ge_rops.c b/drivers/video/fbdev/wmt_ge_rops.c
> index 42255d27a1db..99c7b0aea615 100644
> --- a/drivers/video/fbdev/wmt_ge_rops.c
> +++ b/drivers/video/fbdev/wmt_ge_rops.c
> @@ -9,7 +9,9 @@
>  
>  #include <linux/module.h>
>  #include <linux/fb.h>
> +#include <linux/io.h>
>  #include <linux/platform_device.h>
> +
>  #include "core/fb_draw.h"
>  #include "wmt_ge_rops.h"
>  
> -- 
> 2.40.0

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

[PATCH v2 4/5] fbdev: Include <linux/io.h> in drivers

[Thomas Zimmermann]

Fbdev's main header file, <linux/fb.h>, includes <asm/io.h> to get
declarations of I/O helper functions. From these declaratons, it later
defines framebuffer I/O helpers, such as fb_{read,write}[bwlq]() or
fb_memset().

The framebuffer I/O helpers pre-date Linux' current I/O code and will
be replaced by regular I/O helpers. Prepare this change by adding an
include statement for <linux/io.h> to all source files that use the
framebuffer I/O helpers. They will still get declarations of the I/O
functions even after <linux/fb.h> has been cleaned up. Driver source
files that already include <asm/io.h> convert to <linux/io.h>.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/arkfb.c                 | 1 +
 drivers/video/fbdev/aty/mach64_cursor.c     | 3 +--
 drivers/video/fbdev/chipsfb.c               | 1 +
 drivers/video/fbdev/cirrusfb.c              | 1 +
 drivers/video/fbdev/core/cfbcopyarea.c      | 2 +-
 drivers/video/fbdev/core/cfbfillrect.c      | 2 ++
 drivers/video/fbdev/core/cfbimgblt.c        | 2 ++
 drivers/video/fbdev/core/fbmem.c            | 1 +
 drivers/video/fbdev/core/svgalib.c          | 2 +-
 drivers/video/fbdev/hgafb.c                 | 2 +-
 drivers/video/fbdev/hitfb.c                 | 2 +-
 drivers/video/fbdev/kyro/fbdev.c            | 2 +-
 drivers/video/fbdev/matrox/matroxfb_accel.c | 2 ++
 drivers/video/fbdev/matrox/matroxfb_base.h  | 2 +-
 drivers/video/fbdev/pm2fb.c                 | 1 +
 drivers/video/fbdev/pm3fb.c                 | 1 +
 drivers/video/fbdev/pvr2fb.c                | 1 +
 drivers/video/fbdev/s3fb.c                  | 1 +
 drivers/video/fbdev/sstfb.c                 | 2 +-
 drivers/video/fbdev/tdfxfb.c                | 2 +-
 drivers/video/fbdev/tridentfb.c             | 1 +
 drivers/video/fbdev/vga16fb.c               | 2 +-
 drivers/video/fbdev/vt8623fb.c              | 1 +
 23 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index 60a96fdb5dd8..c254ab6fbb7d 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -23,6 +23,7 @@
 #include <linux/fb.h>
 #include <linux/svga.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/pci.h>
 #include <linux/console.h> /* Why should fb driver call console functions? because console_lock() */
 #include <video/vga.h>
diff --git a/drivers/video/fbdev/aty/mach64_cursor.c b/drivers/video/fbdev/aty/mach64_cursor.c
index 4ad0331a8c57..c1347c37a146 100644
--- a/drivers/video/fbdev/aty/mach64_cursor.c
+++ b/drivers/video/fbdev/aty/mach64_cursor.c
@@ -5,11 +5,10 @@
 
 #include <linux/fb.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/string.h>
 #include "../core/fb_draw.h"
 
-#include <asm/io.h>
-
 #ifdef __sparc__
 #include <asm/fbio.h>
 #endif
diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index 7799d52a651f..9cb719afe033 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -26,6 +26,7 @@
 #include <linux/fb.h>
 #include <linux/pm.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/pci.h>
 #include <linux/console.h>
 
diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
index ba45e2147c52..89366a78b010 100644
--- a/drivers/video/fbdev/cirrusfb.c
+++ b/drivers/video/fbdev/cirrusfb.c
@@ -43,6 +43,7 @@
 #include <linux/delay.h>
 #include <linux/fb.h>
 #include <linux/init.h>
+#include <linux/io.h>
 
 #ifdef CONFIG_ZORRO
 #include <linux/zorro.h>
diff --git a/drivers/video/fbdev/core/cfbcopyarea.c b/drivers/video/fbdev/core/cfbcopyarea.c
index 6d4bfeecee35..fc3a99a50266 100644
--- a/drivers/video/fbdev/core/cfbcopyarea.c
+++ b/drivers/video/fbdev/core/cfbcopyarea.c
@@ -26,8 +26,8 @@
 #include <linux/kernel.h>
 #include <linux/string.h>
 #include <linux/fb.h>
+#include <linux/io.h>
 #include <asm/types.h>
-#include <asm/io.h>
 #include "fb_draw.h"
 
 #if BITS_PER_LONG == 32
diff --git a/drivers/video/fbdev/core/cfbfillrect.c b/drivers/video/fbdev/core/cfbfillrect.c
index ba9f58b2a5e8..15fd4927a031 100644
--- a/drivers/video/fbdev/core/cfbfillrect.c
+++ b/drivers/video/fbdev/core/cfbfillrect.c
@@ -13,6 +13,8 @@
  *  the native cpu endians. I also need to deal with MSB position in the word.
  *
  */
+
+#include <linux/io.h>
 #include <linux/module.h>
 #include <linux/string.h>
 #include <linux/fb.h>
diff --git a/drivers/video/fbdev/core/cfbimgblt.c b/drivers/video/fbdev/core/cfbimgblt.c
index 9ebda4e0dc7a..e8c957f51fd4 100644
--- a/drivers/video/fbdev/core/cfbimgblt.c
+++ b/drivers/video/fbdev/core/cfbimgblt.c
@@ -29,6 +29,8 @@
  *  Also need to add code to deal with cards endians that are different than
  *  the native cpu endians. I also need to deal with MSB position in the word.
  */
+
+#include <linux/io.h>
 #include <linux/module.h>
 #include <linux/string.h>
 #include <linux/fb.h>
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 3fd95a79e4c3..2047bffe4a6c 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -23,6 +23,7 @@
 #include <linux/mman.h>
 #include <linux/vt.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/linux_logo.h>
 #include <linux/proc_fs.h>
 #include <linux/platform_device.h>
diff --git a/drivers/video/fbdev/core/svgalib.c b/drivers/video/fbdev/core/svgalib.c
index 9e01322fabe3..c4731b95a36d 100644
--- a/drivers/video/fbdev/core/svgalib.c
+++ b/drivers/video/fbdev/core/svgalib.c
@@ -15,8 +15,8 @@
 #include <linux/string.h>
 #include <linux/fb.h>
 #include <linux/svga.h>
+#include <linux/io.h>
 #include <asm/types.h>
-#include <asm/io.h>
 
 
 /* Write a CRT register value spread across multiple registers */
diff --git a/drivers/video/fbdev/hgafb.c b/drivers/video/fbdev/hgafb.c
index 40879d9facdf..c83ac13f50b1 100644
--- a/drivers/video/fbdev/hgafb.c
+++ b/drivers/video/fbdev/hgafb.c
@@ -41,7 +41,7 @@
 #include <linux/init.h>
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
-#include <asm/io.h>
+#include <linux/io.h>
 #include <asm/vga.h>
 
 #if 0
diff --git a/drivers/video/fbdev/hitfb.c b/drivers/video/fbdev/hitfb.c
index bbb0f1d953cc..8509a5ad77cb 100644
--- a/drivers/video/fbdev/hitfb.c
+++ b/drivers/video/fbdev/hitfb.c
@@ -23,7 +23,7 @@
 
 #include <asm/machvec.h>
 #include <linux/uaccess.h>
-#include <asm/io.h>
+#include <linux/io.h>
 #include <asm/hd64461.h>
 #include <cpu/dac.h>
 
diff --git a/drivers/video/fbdev/kyro/fbdev.c b/drivers/video/fbdev/kyro/fbdev.c
index 0596573ef140..281fd26a5694 100644
--- a/drivers/video/fbdev/kyro/fbdev.c
+++ b/drivers/video/fbdev/kyro/fbdev.c
@@ -21,7 +21,7 @@
 #include <linux/ioctl.h>
 #include <linux/init.h>
 #include <linux/pci.h>
-#include <asm/io.h>
+#include <linux/io.h>
 #include <linux/uaccess.h>
 
 #include <video/kyro.h>
diff --git a/drivers/video/fbdev/matrox/matroxfb_accel.c b/drivers/video/fbdev/matrox/matroxfb_accel.c
index ce51227798a1..271c7839f66f 100644
--- a/drivers/video/fbdev/matrox/matroxfb_accel.c
+++ b/drivers/video/fbdev/matrox/matroxfb_accel.c
@@ -77,6 +77,8 @@
  *
  */
 
+#include <linux/io.h>
+
 #include "matroxfb_accel.h"
 #include "matroxfb_DAC1064.h"
 #include "matroxfb_Ti3026.h"
diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h b/drivers/video/fbdev/matrox/matroxfb_base.h
index c93c69bbcd57..2cd7c82ce307 100644
--- a/drivers/video/fbdev/matrox/matroxfb_base.h
+++ b/drivers/video/fbdev/matrox/matroxfb_base.h
@@ -36,6 +36,7 @@
 #include <linux/fb.h>
 #include <linux/console.h>
 #include <linux/selection.h>
+#include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/init.h>
 #include <linux/timer.h>
@@ -43,7 +44,6 @@
 #include <linux/spinlock.h>
 #include <linux/kd.h>
 
-#include <asm/io.h>
 #include <asm/unaligned.h>
 
 #if defined(CONFIG_PPC_PMAC)
diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c
index 47d212944f30..297abac721ea 100644
--- a/drivers/video/fbdev/pm2fb.c
+++ b/drivers/video/fbdev/pm2fb.c
@@ -38,6 +38,7 @@
 #include <linux/delay.h>
 #include <linux/fb.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/pci.h>
 #include <video/permedia2.h>
 #include <video/cvisionppc.h>
diff --git a/drivers/video/fbdev/pm3fb.c b/drivers/video/fbdev/pm3fb.c
index b46a471df9ae..78df01b1a507 100644
--- a/drivers/video/fbdev/pm3fb.c
+++ b/drivers/video/fbdev/pm3fb.c
@@ -32,6 +32,7 @@
 #include <linux/delay.h>
 #include <linux/fb.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/pci.h>
 
 #include <video/pm3fb.h>
diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index 6888127a5eb8..6aeb26293c70 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -56,6 +56,7 @@
 #include <linux/interrupt.h>
 #include <linux/fb.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/pci.h>
 
 #ifdef CONFIG_SH_DREAMCAST
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index 7d257489edcc..1efddb2ef6da 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -22,6 +22,7 @@
 #include <linux/fb.h>
 #include <linux/svga.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/pci.h>
 #include <linux/console.h> /* Why should fb driver call console functions? because console_lock() */
 #include <video/vga.h>
diff --git a/drivers/video/fbdev/sstfb.c b/drivers/video/fbdev/sstfb.c
index da296b2ab54a..f07c646aa2d1 100644
--- a/drivers/video/fbdev/sstfb.c
+++ b/drivers/video/fbdev/sstfb.c
@@ -88,7 +88,7 @@
 #include <linux/pci.h>
 #include <linux/delay.h>
 #include <linux/init.h>
-#include <asm/io.h>
+#include <linux/io.h>
 #include <linux/uaccess.h>
 #include <video/sstfb.h>
 
diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
index d17e5e1472aa..096a8cfd8d6d 100644
--- a/drivers/video/fbdev/tdfxfb.c
+++ b/drivers/video/fbdev/tdfxfb.c
@@ -74,7 +74,7 @@
 #include <linux/fb.h>
 #include <linux/init.h>
 #include <linux/pci.h>
-#include <asm/io.h>
+#include <linux/io.h>
 
 #include <video/tdfx.h>
 
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 6099b9768ba1..28e7647bdf82 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/fb.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/pci.h>
 #include <linux/slab.h>
 
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index 1a8ffdb2be26..2d185dbb839b 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -18,12 +18,12 @@
 #include <linux/mm.h>
 #include <linux/delay.h>
 #include <linux/fb.h>
+#include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/screen_info.h>
 
-#include <asm/io.h>
 #include <video/vga.h>
 
 #define MODE_SKIP4	1
diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index 034333ee6e45..1d4aee10a653 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -23,6 +23,7 @@
 #include <linux/fb.h>
 #include <linux/svga.h>
 #include <linux/init.h>
+#include <linux/io.h>
 #include <linux/pci.h>
 #include <linux/console.h> /* Why should fb driver call console functions? because console_lock() */
 #include <video/vga.h>
-- 
2.40.0


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 4/5] fbdev: Include <linux/io.h> in drivers

[Sam Ravnborg]

On Fri, Apr 28, 2023 at 11:27:10AM +0200, Thomas Zimmermann wrote:
> Fbdev's main header file, <linux/fb.h>, includes <asm/io.h> to get
> declarations of I/O helper functions. From these declaratons, it later
> defines framebuffer I/O helpers, such as fb_{read,write}[bwlq]() or
> fb_memset().
> 
> The framebuffer I/O helpers pre-date Linux' current I/O code and will
> be replaced by regular I/O helpers. Prepare this change by adding an
> include statement for <linux/io.h> to all source files that use the
> framebuffer I/O helpers. They will still get declarations of the I/O
> functions even after <linux/fb.h> has been cleaned up.
When fb.h uses a symbol from io.h, then it shall include that
file so it is self contained.
So it is wrong to push the io.h include to the users of
fb_{read,write,xxx}. Maybe fb.h only uses macros as is the case here,
but that is no excuse nt to include io.h.

Drop these changes.

> Driver source
> files that already include <asm/io.h> convert to <linux/io.h>.
This is a nice cleanup - we should keep that.

	Sam

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 4/5] fbdev: Include <linux/io.h> in drivers

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1578 bytes --]

Hi Sam

Am 28.04.23 um 15:07 schrieb Sam Ravnborg:
> On Fri, Apr 28, 2023 at 11:27:10AM +0200, Thomas Zimmermann wrote:
>> Fbdev's main header file, <linux/fb.h>, includes <asm/io.h> to get
>> declarations of I/O helper functions. From these declaratons, it later
>> defines framebuffer I/O helpers, such as fb_{read,write}[bwlq]() or
>> fb_memset().
>>
>> The framebuffer I/O helpers pre-date Linux' current I/O code and will
>> be replaced by regular I/O helpers. Prepare this change by adding an
>> include statement for <linux/io.h> to all source files that use the
>> framebuffer I/O helpers. They will still get declarations of the I/O
>> functions even after <linux/fb.h> has been cleaned up.
> When fb.h uses a symbol from io.h, then it shall include that
> file so it is self contained.
> So it is wrong to push the io.h include to the users of
> fb_{read,write,xxx}. Maybe fb.h only uses macros as is the case here,
> but that is no excuse nt to include io.h.
> 
> Drop these changes.

No problem. That was done with an eye on removing the fb_() macros 
entirely. But the discussion around patch 5 now goes in a different 
direction anyway.

Best regards
Thomas

> 
>> Driver source
>> files that already include <asm/io.h> convert to <linux/io.h>.
> This is a nice cleanup - we should keep that.
> 
> 	Sam

-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

[PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Thomas Zimmermann]

Implement framebuffer I/O helpers, such as fb_read*() and fb_write*()
with Linux' regular I/O functions. Remove all ifdef cases for the
various architectures.

Most of the supported architectures use __raw_() I/O functions or treat
framebuffer memory like regular memory. This is also implemented by the
architectures' I/O function, so we can use them instead.

Sparc uses SBus to connect to framebuffer devices. It provides respective
implementations of the framebuffer I/O helpers. The involved sbus_()
I/O helpers map to the same code as Sparc's regular I/O functions. As
with other platforms, we can use those instead.

We leave a TODO item to replace all fb_() functions with their regular
I/O counterparts throughout the fbdev drivers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 include/linux/fb.h | 63 +++++++++++-----------------------------------
 1 file changed, 15 insertions(+), 48 deletions(-)

diff --git a/include/linux/fb.h b/include/linux/fb.h
index 08cb47da71f8..4aa9e90edd17 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -15,7 +15,6 @@
 #include <linux/list.h>
 #include <linux/backlight.h>
 #include <linux/slab.h>
-#include <asm/io.h>
 
 struct vm_area_struct;
 struct fb_info;
@@ -511,58 +510,26 @@ struct fb_info {
  */
 #define STUPID_ACCELF_TEXT_SHIT
 
-// This will go away
-#if defined(__sparc__)
-
-/* We map all of our framebuffers such that big-endian accesses
- * are what we want, so the following is sufficient.
+/*
+ * TODO: Update fbdev drivers to call the I/O helpers directly and
+ *       remove the fb_() tokens.
  */
-
-// This will go away
-#define fb_readb sbus_readb
-#define fb_readw sbus_readw
-#define fb_readl sbus_readl
-#define fb_readq sbus_readq
-#define fb_writeb sbus_writeb
-#define fb_writew sbus_writew
-#define fb_writel sbus_writel
-#define fb_writeq sbus_writeq
-#define fb_memset sbus_memset_io
-#define fb_memcpy_fromfb sbus_memcpy_fromio
-#define fb_memcpy_tofb sbus_memcpy_toio
-
-#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) ||	\
-	defined(__hppa__) || defined(__sh__) || defined(__powerpc__) ||	\
-	defined(__arm__) || defined(__aarch64__) || defined(__mips__)
-
-#define fb_readb __raw_readb
-#define fb_readw __raw_readw
-#define fb_readl __raw_readl
-#define fb_readq __raw_readq
-#define fb_writeb __raw_writeb
-#define fb_writew __raw_writew
-#define fb_writel __raw_writel
-#define fb_writeq __raw_writeq
+#define fb_readb readb
+#define fb_readw readw
+#define fb_readl readl
+#if defined(CONFIG_64BIT)
+#define fb_readq readq
+#endif
+#define fb_writeb writeb
+#define fb_writew writew
+#define fb_writel writel
+#if defined(CONFIG_64BIT)
+#define fb_writeq writeq
+#endif
 #define fb_memset memset_io
 #define fb_memcpy_fromfb memcpy_fromio
 #define fb_memcpy_tofb memcpy_toio
 
-#else
-
-#define fb_readb(addr) (*(volatile u8 *) (addr))
-#define fb_readw(addr) (*(volatile u16 *) (addr))
-#define fb_readl(addr) (*(volatile u32 *) (addr))
-#define fb_readq(addr) (*(volatile u64 *) (addr))
-#define fb_writeb(b,addr) (*(volatile u8 *) (addr) = (b))
-#define fb_writew(b,addr) (*(volatile u16 *) (addr) = (b))
-#define fb_writel(b,addr) (*(volatile u32 *) (addr) = (b))
-#define fb_writeq(b,addr) (*(volatile u64 *) (addr) = (b))
-#define fb_memset memset
-#define fb_memcpy_fromfb memcpy
-#define fb_memcpy_tofb memcpy
-
-#endif
-
 #define FB_LEFT_POS(p, bpp)          (fb_be_math(p) ? (32 - (bpp)) : 0)
 #define FB_SHIFT_HIGH(p, val, bits)  (fb_be_math(p) ? (val) >> (bits) : \
 						      (val) << (bits))
-- 
2.40.0


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Robin Murphy]

On 2023-04-28 10:27, Thomas Zimmermann wrote:
> Implement framebuffer I/O helpers, such as fb_read*() and fb_write*()
> with Linux' regular I/O functions. Remove all ifdef cases for the
> various architectures.
> 
> Most of the supported architectures use __raw_() I/O functions or treat
> framebuffer memory like regular memory. This is also implemented by the
> architectures' I/O function, so we can use them instead.
> 
> Sparc uses SBus to connect to framebuffer devices. It provides respective
> implementations of the framebuffer I/O helpers. The involved sbus_()
> I/O helpers map to the same code as Sparc's regular I/O functions. As
> with other platforms, we can use those instead.
> 
> We leave a TODO item to replace all fb_() functions with their regular
> I/O counterparts throughout the fbdev drivers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   include/linux/fb.h | 63 +++++++++++-----------------------------------
>   1 file changed, 15 insertions(+), 48 deletions(-)
> 
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 08cb47da71f8..4aa9e90edd17 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -15,7 +15,6 @@
>   #include <linux/list.h>
>   #include <linux/backlight.h>
>   #include <linux/slab.h>
> -#include <asm/io.h>
>   
>   struct vm_area_struct;
>   struct fb_info;
> @@ -511,58 +510,26 @@ struct fb_info {
>    */
>   #define STUPID_ACCELF_TEXT_SHIT
>   
> -// This will go away
> -#if defined(__sparc__)
> -
> -/* We map all of our framebuffers such that big-endian accesses
> - * are what we want, so the following is sufficient.
> +/*
> + * TODO: Update fbdev drivers to call the I/O helpers directly and
> + *       remove the fb_() tokens.
>    */
> -
> -// This will go away
> -#define fb_readb sbus_readb
> -#define fb_readw sbus_readw
> -#define fb_readl sbus_readl
> -#define fb_readq sbus_readq
> -#define fb_writeb sbus_writeb
> -#define fb_writew sbus_writew
> -#define fb_writel sbus_writel
> -#define fb_writeq sbus_writeq
> -#define fb_memset sbus_memset_io
> -#define fb_memcpy_fromfb sbus_memcpy_fromio
> -#define fb_memcpy_tofb sbus_memcpy_toio
> -
> -#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) ||	\
> -	defined(__hppa__) || defined(__sh__) || defined(__powerpc__) ||	\
> -	defined(__arm__) || defined(__aarch64__) || defined(__mips__)
> -
> -#define fb_readb __raw_readb
> -#define fb_readw __raw_readw
> -#define fb_readl __raw_readl
> -#define fb_readq __raw_readq
> -#define fb_writeb __raw_writeb
> -#define fb_writew __raw_writew
> -#define fb_writel __raw_writel
> -#define fb_writeq __raw_writeq

Note that on at least some architectures, the __raw variants are 
native-endian, whereas the regular accessors are explicitly 
little-endian, so there is a slight risk of inadvertently changing 
behaviour on big-endian systems (MIPS most likely, but a few old ARM 
platforms run BE as well).

> +#define fb_readb readb
> +#define fb_readw readw
> +#define fb_readl readl
> +#if defined(CONFIG_64BIT)
> +#define fb_readq readq
> +#endif

You probably don't need to bother making these conditional - 32-bit 
architectures aren't forbidden from providing readq/writeq if they 
really want to, and drivers can also use the io-64-nonatomic headers for 
portability. The build will still fail in a sufficiently obvious manner 
if neither is true.

Thanks,
Robin.

> +#define fb_writeb writeb
> +#define fb_writew writew
> +#define fb_writel writel
> +#if defined(CONFIG_64BIT)
> +#define fb_writeq writeq
> +#endif
>   #define fb_memset memset_io
>   #define fb_memcpy_fromfb memcpy_fromio
>   #define fb_memcpy_tofb memcpy_toio
>   
> -#else
> -
> -#define fb_readb(addr) (*(volatile u8 *) (addr))
> -#define fb_readw(addr) (*(volatile u16 *) (addr))
> -#define fb_readl(addr) (*(volatile u32 *) (addr))
> -#define fb_readq(addr) (*(volatile u64 *) (addr))
> -#define fb_writeb(b,addr) (*(volatile u8 *) (addr) = (b))
> -#define fb_writew(b,addr) (*(volatile u16 *) (addr) = (b))
> -#define fb_writel(b,addr) (*(volatile u32 *) (addr) = (b))
> -#define fb_writeq(b,addr) (*(volatile u64 *) (addr) = (b))
> -#define fb_memset memset
> -#define fb_memcpy_fromfb memcpy
> -#define fb_memcpy_tofb memcpy
> -
> -#endif
> -
>   #define FB_LEFT_POS(p, bpp)          (fb_be_math(p) ? (32 - (bpp)) : 0)
>   #define FB_SHIFT_HIGH(p, val, bits)  (fb_be_math(p) ? (val) >> (bits) : \
>   						      (val) << (bits))

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Geert Uytterhoeven]

On Fri, Apr 28, 2023 at 2:18 PM Robin Murphy <robin.murphy@arm.com> wrote:
> On 2023-04-28 10:27, Thomas Zimmermann wrote:
> > Implement framebuffer I/O helpers, such as fb_read*() and fb_write*()
> > with Linux' regular I/O functions. Remove all ifdef cases for the
> > various architectures.
> >
> > Most of the supported architectures use __raw_() I/O functions or treat
> > framebuffer memory like regular memory. This is also implemented by the
> > architectures' I/O function, so we can use them instead.
> >
> > Sparc uses SBus to connect to framebuffer devices. It provides respective
> > implementations of the framebuffer I/O helpers. The involved sbus_()
> > I/O helpers map to the same code as Sparc's regular I/O functions. As
> > with other platforms, we can use those instead.
> >
> > We leave a TODO item to replace all fb_() functions with their regular
> > I/O counterparts throughout the fbdev drivers.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > ---
> >   include/linux/fb.h | 63 +++++++++++-----------------------------------
> >   1 file changed, 15 insertions(+), 48 deletions(-)
> >
> > diff --git a/include/linux/fb.h b/include/linux/fb.h
> > index 08cb47da71f8..4aa9e90edd17 100644
> > --- a/include/linux/fb.h
> > +++ b/include/linux/fb.h
> > @@ -15,7 +15,6 @@
> >   #include <linux/list.h>
> >   #include <linux/backlight.h>
> >   #include <linux/slab.h>
> > -#include <asm/io.h>
> >
> >   struct vm_area_struct;
> >   struct fb_info;
> > @@ -511,58 +510,26 @@ struct fb_info {
> >    */
> >   #define STUPID_ACCELF_TEXT_SHIT
> >
> > -// This will go away
> > -#if defined(__sparc__)
> > -
> > -/* We map all of our framebuffers such that big-endian accesses
> > - * are what we want, so the following is sufficient.
> > +/*
> > + * TODO: Update fbdev drivers to call the I/O helpers directly and
> > + *       remove the fb_() tokens.
> >    */
> > -
> > -// This will go away
> > -#define fb_readb sbus_readb
> > -#define fb_readw sbus_readw
> > -#define fb_readl sbus_readl
> > -#define fb_readq sbus_readq
> > -#define fb_writeb sbus_writeb
> > -#define fb_writew sbus_writew
> > -#define fb_writel sbus_writel
> > -#define fb_writeq sbus_writeq
> > -#define fb_memset sbus_memset_io
> > -#define fb_memcpy_fromfb sbus_memcpy_fromio
> > -#define fb_memcpy_tofb sbus_memcpy_toio
> > -
> > -#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) ||      \
> > -     defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || \
> > -     defined(__arm__) || defined(__aarch64__) || defined(__mips__)
> > -
> > -#define fb_readb __raw_readb
> > -#define fb_readw __raw_readw
> > -#define fb_readl __raw_readl
> > -#define fb_readq __raw_readq
> > -#define fb_writeb __raw_writeb
> > -#define fb_writew __raw_writew
> > -#define fb_writel __raw_writel
> > -#define fb_writeq __raw_writeq
>
> Note that on at least some architectures, the __raw variants are
> native-endian, whereas the regular accessors are explicitly
> little-endian, so there is a slight risk of inadvertently changing
> behaviour on big-endian systems (MIPS most likely, but a few old ARM
> platforms run BE as well).

Also on m68k, when ISA or PCI are enabled.

In addition, the non-raw variants may do some extras to guarantee
ordering, which you do not need on a frame buffer.

So I'd go for the __raw_*() variants everywhere.

Gr{oetje,eeting}s,

                        Geert

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

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

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 4084 bytes --]

Hi Robin, Geert

Am 28.04.23 um 14:27 schrieb Geert Uytterhoeven:
> On Fri, Apr 28, 2023 at 2:18 PM Robin Murphy <robin.murphy@arm.com> wrote:
>> On 2023-04-28 10:27, Thomas Zimmermann wrote:
>>> Implement framebuffer I/O helpers, such as fb_read*() and fb_write*()
>>> with Linux' regular I/O functions. Remove all ifdef cases for the
>>> various architectures.
>>>
>>> Most of the supported architectures use __raw_() I/O functions or treat
>>> framebuffer memory like regular memory. This is also implemented by the
>>> architectures' I/O function, so we can use them instead.
>>>
>>> Sparc uses SBus to connect to framebuffer devices. It provides respective
>>> implementations of the framebuffer I/O helpers. The involved sbus_()
>>> I/O helpers map to the same code as Sparc's regular I/O functions. As
>>> with other platforms, we can use those instead.
>>>
>>> We leave a TODO item to replace all fb_() functions with their regular
>>> I/O counterparts throughout the fbdev drivers.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>    include/linux/fb.h | 63 +++++++++++-----------------------------------
>>>    1 file changed, 15 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>>> index 08cb47da71f8..4aa9e90edd17 100644
>>> --- a/include/linux/fb.h
>>> +++ b/include/linux/fb.h
>>> @@ -15,7 +15,6 @@
>>>    #include <linux/list.h>
>>>    #include <linux/backlight.h>
>>>    #include <linux/slab.h>
>>> -#include <asm/io.h>
>>>
>>>    struct vm_area_struct;
>>>    struct fb_info;
>>> @@ -511,58 +510,26 @@ struct fb_info {
>>>     */
>>>    #define STUPID_ACCELF_TEXT_SHIT
>>>
>>> -// This will go away
>>> -#if defined(__sparc__)
>>> -
>>> -/* We map all of our framebuffers such that big-endian accesses
>>> - * are what we want, so the following is sufficient.
>>> +/*
>>> + * TODO: Update fbdev drivers to call the I/O helpers directly and
>>> + *       remove the fb_() tokens.
>>>     */
>>> -
>>> -// This will go away
>>> -#define fb_readb sbus_readb
>>> -#define fb_readw sbus_readw
>>> -#define fb_readl sbus_readl
>>> -#define fb_readq sbus_readq
>>> -#define fb_writeb sbus_writeb
>>> -#define fb_writew sbus_writew
>>> -#define fb_writel sbus_writel
>>> -#define fb_writeq sbus_writeq
>>> -#define fb_memset sbus_memset_io
>>> -#define fb_memcpy_fromfb sbus_memcpy_fromio
>>> -#define fb_memcpy_tofb sbus_memcpy_toio
>>> -
>>> -#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) ||      \
>>> -     defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || \
>>> -     defined(__arm__) || defined(__aarch64__) || defined(__mips__)
>>> -
>>> -#define fb_readb __raw_readb
>>> -#define fb_readw __raw_readw
>>> -#define fb_readl __raw_readl
>>> -#define fb_readq __raw_readq
>>> -#define fb_writeb __raw_writeb
>>> -#define fb_writew __raw_writew
>>> -#define fb_writel __raw_writel
>>> -#define fb_writeq __raw_writeq
>>
>> Note that on at least some architectures, the __raw variants are
>> native-endian, whereas the regular accessors are explicitly
>> little-endian, so there is a slight risk of inadvertently changing
>> behaviour on big-endian systems (MIPS most likely, but a few old ARM
>> platforms run BE as well).
> 
> Also on m68k, when ISA or PCI are enabled.
> 
> In addition, the non-raw variants may do some extras to guarantee
> ordering, which you do not need on a frame buffer.
> 
> So I'd go for the __raw_*() variants everywhere.

Ok, makes sense. But it also means that we won't be able to remove the 
fb_() helpers. If we go with this proposal, I'll add your comments to 
the header file, so it's clear why they're still there.

Best regards
Thomas

> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 912 bytes --]

Hi

Am 28.04.23 um 14:27 schrieb Geert Uytterhoeven:
[...]
> 
> In addition, the non-raw variants may do some extras to guarantee
> ordering, which you do not need on a frame buffer.

Given this comment, should we declare the fb_() helpers in 
<asm-generic/io.h> or <linux/io.h>?

I still don't like the idea of having the functions in <linux/fb.h>. We 
have code in DRM that also accesses framebuffer memory (via 
memcpy_toio()). It would make sense to use the fb_() helpers, if they 
are tailored towards this usecase.

Best regards
Thomas

> 
> So I'd go for the __raw_*() variants everywhere.
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Arnd Bergmann]

On Fri, Apr 28, 2023, at 13:27, Geert Uytterhoeven wrote:
> On Fri, Apr 28, 2023 at 2:18 PM Robin Murphy <robin.murphy@arm.com> wrote:
>> On 2023-04-28 10:27, Thomas Zimmermann wrote:

>> > -
>> > -#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) ||      \
>> > -     defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || \
>> > -     defined(__arm__) || defined(__aarch64__) || defined(__mips__)
>> > -
>> > -#define fb_readb __raw_readb
>> > -#define fb_readw __raw_readw
>> > -#define fb_readl __raw_readl
>> > -#define fb_readq __raw_readq
>> > -#define fb_writeb __raw_writeb
>> > -#define fb_writew __raw_writew
>> > -#define fb_writel __raw_writel
>> > -#define fb_writeq __raw_writeq
>>
>> Note that on at least some architectures, the __raw variants are
>> native-endian, whereas the regular accessors are explicitly
>> little-endian, so there is a slight risk of inadvertently changing
>> behaviour on big-endian systems (MIPS most likely, but a few old ARM
>> platforms run BE as well).
>
> Also on m68k, when ISA or PCI are enabled.
>
> In addition, the non-raw variants may do some extras to guarantee
> ordering, which you do not need on a frame buffer.
>
> So I'd go for the __raw_*() variants everywhere.

The only implementations in fbdev are

 1) sparc sbus
 2) __raw_writel
 3) direct pointer dereference

But none use the byte-swapping writel() implementations, and
the only ones that use the direct pointer dereference or sbus
are the ones on which these are defined the same as __raw_writel

      Arnd

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2513 bytes --]

Hi

Am 28.04.23 um 15:17 schrieb Arnd Bergmann:
> On Fri, Apr 28, 2023, at 13:27, Geert Uytterhoeven wrote:
>> On Fri, Apr 28, 2023 at 2:18 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>> On 2023-04-28 10:27, Thomas Zimmermann wrote:
> 
>>>> -
>>>> -#elif defined(__i386__) || defined(__alpha__) || defined(__x86_64__) ||      \
>>>> -     defined(__hppa__) || defined(__sh__) || defined(__powerpc__) || \
>>>> -     defined(__arm__) || defined(__aarch64__) || defined(__mips__)
>>>> -
>>>> -#define fb_readb __raw_readb
>>>> -#define fb_readw __raw_readw
>>>> -#define fb_readl __raw_readl
>>>> -#define fb_readq __raw_readq
>>>> -#define fb_writeb __raw_writeb
>>>> -#define fb_writew __raw_writew
>>>> -#define fb_writel __raw_writel
>>>> -#define fb_writeq __raw_writeq
>>>
>>> Note that on at least some architectures, the __raw variants are
>>> native-endian, whereas the regular accessors are explicitly
>>> little-endian, so there is a slight risk of inadvertently changing
>>> behaviour on big-endian systems (MIPS most likely, but a few old ARM
>>> platforms run BE as well).
>>
>> Also on m68k, when ISA or PCI are enabled.
>>
>> In addition, the non-raw variants may do some extras to guarantee
>> ordering, which you do not need on a frame buffer.
>>
>> So I'd go for the __raw_*() variants everywhere.
> 
> The only implementations in fbdev are
> 
>   1) sparc sbus
>   2) __raw_writel
>   3) direct pointer dereference
> 
> But none use the byte-swapping writel() implementations, and
> the only ones that use the direct pointer dereference or sbus
> are the ones on which these are defined the same as __raw_writel

After thinking a bit more about the requirements, I'd like to got back 
to v1, but with a different spin. We want to avoid ordering guarantees, 
so I looked at the _relaxed() helpers, but they seem to swap bytes to 
little endian.

I guess we can remove the fb_mem*() functions entirely. They are the 
same as the non-fb_ counterparts. For the fb read/write helpers, I'd 
like to add them to <asm-generic/fb.h> in a platform-neutral way. They'd 
be wrappers around __raw_(), as I wouldn't want invocations of  __raw_() 
functions in the fbdev drivers.

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)

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

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Arnd Bergmann]

On Sat, Apr 29, 2023, at 14:26, Thomas Zimmermann wrote:
> Am 28.04.23 um 15:17 schrieb Arnd Bergmann:
>> The only implementations in fbdev are
>> 
>>   1) sparc sbus
>>   2) __raw_writel
>>   3) direct pointer dereference
>> 
>> But none use the byte-swapping writel() implementations, and
>> the only ones that use the direct pointer dereference or sbus
>> are the ones on which these are defined the same as __raw_writel
>
> After thinking a bit more about the requirements, I'd like to got back 
> to v1, but with a different spin. We want to avoid ordering guarantees, 
> so I looked at the _relaxed() helpers, but they seem to swap bytes to 
> little endian.

Right, the _relaxed() oens are clearly wrong, aside from
the byteswap they also include barriers on some architectures
where the __raw_* version is more relaxed than the required
semantics for relaxed.

> I guess we can remove the fb_mem*() functions entirely. They are the 
> same as the non-fb_ counterparts.

These might actually be different in some cases, or sub-optimal
at the moment. memcpy()/memset() don't take __iomem pointers, so they
cause sparse warnings, while the memset_io()/memcpy_fromio()/
memcpy_toio() sometimes fall back to bytewise access that is slower
than word-sized copy. I only looked at the readl/writel style 
functions earlier, no idea what we want here.

> For the fb read/write helpers, I'd 
> like to add them to <asm-generic/fb.h> in a platform-neutral way. They'd 
> be wrappers around __raw_(), as I wouldn't want invocations of  __raw_() 
> functions in the fbdev drivers.

That sounds good to me.

     Arnd

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Sam Ravnborg]

Hi Thomas,

On Fri, Apr 28, 2023 at 11:27:11AM +0200, Thomas Zimmermann wrote:
> Implement framebuffer I/O helpers, such as fb_read*() and fb_write*()
> with Linux' regular I/O functions. Remove all ifdef cases for the
> various architectures.
> 
> Most of the supported architectures use __raw_() I/O functions or treat
> framebuffer memory like regular memory. This is also implemented by the
> architectures' I/O function, so we can use them instead.
> 
> Sparc uses SBus to connect to framebuffer devices. It provides respective
> implementations of the framebuffer I/O helpers. The involved sbus_()
> I/O helpers map to the same code as Sparc's regular I/O functions. As
> with other platforms, we can use those instead.
> 
> We leave a TODO item to replace all fb_() functions with their regular
> I/O counterparts throughout the fbdev drivers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  include/linux/fb.h | 63 +++++++++++-----------------------------------
>  1 file changed, 15 insertions(+), 48 deletions(-)
> 
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 08cb47da71f8..4aa9e90edd17 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -15,7 +15,6 @@
>  #include <linux/list.h>
>  #include <linux/backlight.h>
>  #include <linux/slab.h>
> -#include <asm/io.h>
>  
>  struct vm_area_struct;
>  struct fb_info;
> @@ -511,58 +510,26 @@ struct fb_info {
>   */
>  #define STUPID_ACCELF_TEXT_SHIT
>  
> -// This will go away
> -#if defined(__sparc__)
> -
> -/* We map all of our framebuffers such that big-endian accesses
> - * are what we want, so the following is sufficient.
> +/*
> + * TODO: Update fbdev drivers to call the I/O helpers directly and
> + *       remove the fb_() tokens.
>   */
When the __raw_* variants are used, as Geert points out, then I think
the memcpy / memset can be replaced, but the rest seems fine to keep.

My personal opinion is that __raw_* is for macro use etc, and not
something to use everywhere. So I like the fb_read/fb_write macros.
But that is just my color of the bikeshed.

	Sam

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 2813 bytes --]

Hi

Am 28.04.23 um 15:12 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Fri, Apr 28, 2023 at 11:27:11AM +0200, Thomas Zimmermann wrote:
>> Implement framebuffer I/O helpers, such as fb_read*() and fb_write*()
>> with Linux' regular I/O functions. Remove all ifdef cases for the
>> various architectures.
>>
>> Most of the supported architectures use __raw_() I/O functions or treat
>> framebuffer memory like regular memory. This is also implemented by the
>> architectures' I/O function, so we can use them instead.
>>
>> Sparc uses SBus to connect to framebuffer devices. It provides respective
>> implementations of the framebuffer I/O helpers. The involved sbus_()
>> I/O helpers map to the same code as Sparc's regular I/O functions. As
>> with other platforms, we can use those instead.
>>
>> We leave a TODO item to replace all fb_() functions with their regular
>> I/O counterparts throughout the fbdev drivers.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   include/linux/fb.h | 63 +++++++++++-----------------------------------
>>   1 file changed, 15 insertions(+), 48 deletions(-)
>>
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index 08cb47da71f8..4aa9e90edd17 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -15,7 +15,6 @@
>>   #include <linux/list.h>
>>   #include <linux/backlight.h>
>>   #include <linux/slab.h>
>> -#include <asm/io.h>
>>   
>>   struct vm_area_struct;
>>   struct fb_info;
>> @@ -511,58 +510,26 @@ struct fb_info {
>>    */
>>   #define STUPID_ACCELF_TEXT_SHIT
>>   
>> -// This will go away
>> -#if defined(__sparc__)
>> -
>> -/* We map all of our framebuffers such that big-endian accesses
>> - * are what we want, so the following is sufficient.
>> +/*
>> + * TODO: Update fbdev drivers to call the I/O helpers directly and
>> + *       remove the fb_() tokens.
>>    */
> When the __raw_* variants are used, as Geert points out, then I think
> the memcpy / memset can be replaced, but the rest seems fine to keep.

I'd either want the regular I/O functions or the fb_() wrappers, but not 
the __raw_() function. I'd also prefer to keep fb_ in front of memset 
and memcpy.  I'd be happy to have fb_() wrappers that are I/O helpers 
without ordering guarantees. I'd just wouldn't want them in <linux/fb.h>

Best regards
Thomas

> 
> My personal opinion is that __raw_* is for macro use etc, and not
> something to use everywhere. So I like the fb_read/fb_write macros.
> But that is just my color of the bikeshed.
> 
> 	Sam

-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Sam Ravnborg]

Hi Thomas,

On Fri, Apr 28, 2023 at 04:18:38PM +0200, Thomas Zimmermann wrote:
> I'd be happy to have fb_() wrappers that are I/O helpers without
> ordering guarantees. I'd just wouldn't want them in <linux/fb.h>

How about throwing them into a new drm_fb.h header file.
This header file could be the home for all the fb stuff that is
shared between drm and the legacy fbdev.

Then we may slowly migrate more fbdev stuff to drm and let the legacy
fbdev stuff use the maintained drm stuff.
Dunno, the pain may not be worth it.

	Sam

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 5/5] fbdev: Define framebuffer I/O from Linux' I/O functions

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 1029 bytes --]

Hi Sam

Am 28.04.23 um 18:54 schrieb Sam Ravnborg:
> Hi Thomas,
> 
> On Fri, Apr 28, 2023 at 04:18:38PM +0200, Thomas Zimmermann wrote:
>> I'd be happy to have fb_() wrappers that are I/O helpers without
>> ordering guarantees. I'd just wouldn't want them in <linux/fb.h>
> 
> How about throwing them into a new drm_fb.h header file.
> This header file could be the home for all the fb stuff that is
> shared between drm and the legacy fbdev.
> 
> Then we may slowly migrate more fbdev stuff to drm and let the legacy
> fbdev stuff use the maintained drm stuff.
> Dunno, the pain may not be worth it.

DRM might not be relevant if we can remove fb_mem*(). So I'd like to go 
back to something closer to v1 of these patches. See my reply to Arnd.

Best regards
Thomas

> 
> 	Sam

-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 0/5] fbdev: Use regular I/O function for framebuffers

[Sam Ravnborg]

Hi Thomas,

On Fri, Apr 28, 2023 at 11:27:06AM +0200, Thomas Zimmermann wrote:
> (was: fbdev: Move framebuffer I/O helpers to <asm/fb.h>)
> 
> Fbdev provides helpers for framebuffer I/O, such as fb_readl(),
> fb_writel() or fb_memcpy_to_fb(). The implementation of each helper
> depends on the architecture, but they all come down to regular I/O
> functions of similar names. So use the regular functions instead.
> 
> The first patch a simple whitespace cleanup.
> 
> Until now, <linux/fb.h> contained an include of <asm/io.h>. As this
> will go away patches 2 to 4 prepare include statements in the various
> drivers. Source files that use regular I/O helpers, such as readl(),
> now include <linux/io.h>. Source files that use framebuffer I/O
> helpers, such as fb_readl(), also include <linux/io.h>.
> 
> Patch 5 replaces the architecture-based if-else branching in 
> <linux/fb.h> by define statements that map to Linux' I/O fucntions.
> 
> After this change has been merged and included in a few release
> without complains, we can update the drivers to regular I/O functions
> and remove the fbdev-specific defines.
> 
> The patchset has been built for a variety of platforms, such as x86-64,
> arm, aarch64, ppc64, parisc, m64k, mips and sparc.
> 
> v2:
> 	* use Linux I/O helpers (Sam, Arnd)
Much better, thanks!

	Sam

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc