diff mbox series

[5/6] fbdev: Move CFB read and write code into helper functions

Message ID 20230425142846.730-6-tzimmermann@suse.de
State New
Headers show
Series drm,fbdev: Use fbdev's I/O helpers | expand

Commit Message

Thomas Zimmermann April 25, 2023, 2:28 p.m. UTC 
Move the existing CFB read and write code for I/O memory into
the new helpers fb_cfb_read() and fb_cfb_write(). Make them the
default fp_ops. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/core/Makefile      |   2 +-
 drivers/video/fbdev/core/fb_cfb_fops.c | 126 +++++++++++++++++++++++++
 drivers/video/fbdev/core/fbmem.c       | 113 +---------------------
 include/linux/fb.h                     |  10 ++
 4 files changed, 139 insertions(+), 112 deletions(-)
 create mode 100644 drivers/video/fbdev/core/fb_cfb_fops.c

Comments

Thomas Zimmermann April 28, 2023, 11:20 a.m. UTC | #1 
Hi

Am 26.04.23 um 17:21 schrieb Geert Uytterhoeven:
> Hi Thomas,
> 
> On Wed, Apr 26, 2023 at 5:06 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Am 26.04.23 um 17:01 schrieb Geert Uytterhoeven:
>>> On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>>> Move the existing CFB read and write code for I/O memory into
>>>> the new helpers fb_cfb_read() and fb_cfb_write(). Make them the
>>>> default fp_ops. No functional changes.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> ---
>>>>    drivers/video/fbdev/core/Makefile      |   2 +-
>>>>    drivers/video/fbdev/core/fb_cfb_fops.c | 126 +++++++++++++++++++++++++
>>>>    drivers/video/fbdev/core/fbmem.c       | 113 +---------------------
>>>>    include/linux/fb.h                     |  10 ++
>>>>    4 files changed, 139 insertions(+), 112 deletions(-)
>>>>    create mode 100644 drivers/video/fbdev/core/fb_cfb_fops.c
>>>
>>> While the general idea is fine, please do not call any of this "cfb",
>>> as it is not related to chunky color frame buffer formats.
>>> All of these operate on the raw frame buffer contents.
>>
>> Shall I call it fb_raw_() or fb_io_()?
> 
> Given fb_memcpy_fromfb() is mapped to memcpy_fromio() on
> most architectures, I'd go for fb_io_*().

Ok, makes sense.

> 
>> CFB is used by the drawing helpers, which are usually used together with
>> this code. Hence the current naming.
> 
> That's because your drawing helpers operate (only) on chunky color
> frame buffer formats ;-)

Should we rename the CFB drawing functions to fb_io_ then? AFAICT they 
are the same algorithms as in the fb_sys_ functions; just with I/O memory.

Best regards
Thomas

> 
> Gr{oetje,eeting}s,
> 
>                          Geert
>
Geert Uytterhoeven April 28, 2023, 12:20 p.m. UTC | #2 
Hi Thomas,

On Fri, Apr 28, 2023 at 1:20 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Am 26.04.23 um 17:21 schrieb Geert Uytterhoeven:
> > On Wed, Apr 26, 2023 at 5:06 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >> Am 26.04.23 um 17:01 schrieb Geert Uytterhoeven:
> >>> On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>>> Move the existing CFB read and write code for I/O memory into
> >>>> the new helpers fb_cfb_read() and fb_cfb_write(). Make them the
> >>>> default fp_ops. No functional changes.
> >>>>
> >>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >>>> ---
> >>>>    drivers/video/fbdev/core/Makefile      |   2 +-
> >>>>    drivers/video/fbdev/core/fb_cfb_fops.c | 126 +++++++++++++++++++++++++
> >>>>    drivers/video/fbdev/core/fbmem.c       | 113 +---------------------
> >>>>    include/linux/fb.h                     |  10 ++
> >>>>    4 files changed, 139 insertions(+), 112 deletions(-)
> >>>>    create mode 100644 drivers/video/fbdev/core/fb_cfb_fops.c
> >>>
> >>> While the general idea is fine, please do not call any of this "cfb",
> >>> as it is not related to chunky color frame buffer formats.
> >>> All of these operate on the raw frame buffer contents.
> >>
> >> Shall I call it fb_raw_() or fb_io_()?
> >
> > Given fb_memcpy_fromfb() is mapped to memcpy_fromio() on
> > most architectures, I'd go for fb_io_*().
>
> Ok, makes sense.
>
> >> CFB is used by the drawing helpers, which are usually used together with
> >> this code. Hence the current naming.
> >
> > That's because your drawing helpers operate (only) on chunky color
> > frame buffer formats ;-)
>
> Should we rename the CFB drawing functions to fb_io_ then? AFAICT they
> are the same algorithms as in the fb_sys_ functions; just with I/O memory.

I don't know if that's worth the churn.
Historically, the frame buffer was usually located in dedicated memory,
hence the drawing operations operated on I/O memory.
With the advent of unified memory architectures, the fb_sys_*()
functions were introduced.

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 08fabce76b74..cb7534a80305 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -2,7 +2,7 @@ 
 obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
-                                     modedb.o fbcvt.o fb_cmdline.o
+                                     modedb.o fbcvt.o fb_cmdline.o fb_cfb_fops.o
 fb-$(CONFIG_FB_DEFERRED_IO)       += fb_defio.o
 
 ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE),y)
diff --git a/drivers/video/fbdev/core/fb_cfb_fops.c b/drivers/video/fbdev/core/fb_cfb_fops.c
new file mode 100644
index 000000000000..f6000166eda4
--- /dev/null
+++ b/drivers/video/fbdev/core/fb_cfb_fops.c
@@ -0,0 +1,126 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/fb.h>
+#include <linux/module.h>
+#include <linux/uaccess.h>
+
+ssize_t fb_cfb_read(struct fb_info *info, char __user *buf, size_t count, loff_t *ppos)
+{
+	unsigned long p = *ppos;
+	u8 *buffer, *dst;
+	u8 __iomem *src;
+	int c, cnt = 0, err = 0;
+	unsigned long total_size;
+
+	if (!info->screen_base)
+		return -ENODEV;
+
+	total_size = info->screen_size;
+
+	if (total_size == 0)
+		total_size = info->fix.smem_len;
+
+	if (p >= total_size)
+		return 0;
+
+	if (count >= total_size)
+		count = total_size;
+
+	if (count + p > total_size)
+		count = total_size - p;
+
+	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+	src = (u8 __iomem *)(info->screen_base + p);
+
+	if (info->fbops->fb_sync)
+		info->fbops->fb_sync(info);
+
+	while (count) {
+		c  = (count > PAGE_SIZE) ? PAGE_SIZE : count;
+		dst = buffer;
+		fb_memcpy_fromfb(dst, src, c);
+		dst += c;
+		src += c;
+
+		if (copy_to_user(buf, buffer, c)) {
+			err = -EFAULT;
+			break;
+		}
+		*ppos += c;
+		buf += c;
+		cnt += c;
+		count -= c;
+	}
+
+	kfree(buffer);
+
+	return cnt ? cnt : err;
+}
+EXPORT_SYMBOL(fb_cfb_read);
+
+ssize_t fb_cfb_write(struct fb_info *info, const char __user *buf, size_t count, loff_t *ppos)
+{
+	unsigned long p = *ppos;
+	u8 *buffer, *src;
+	u8 __iomem *dst;
+	int c, cnt = 0, err = 0;
+	unsigned long total_size;
+
+	if (!info->screen_base)
+		return -ENODEV;
+
+	total_size = info->screen_size;
+
+	if (total_size == 0)
+		total_size = info->fix.smem_len;
+
+	if (p > total_size)
+		return -EFBIG;
+
+	if (count > total_size) {
+		err = -EFBIG;
+		count = total_size;
+	}
+
+	if (count + p > total_size) {
+		if (!err)
+			err = -ENOSPC;
+
+		count = total_size - p;
+	}
+
+	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+	dst = (u8 __iomem *)(info->screen_base + p);
+
+	if (info->fbops->fb_sync)
+		info->fbops->fb_sync(info);
+
+	while (count) {
+		c = (count > PAGE_SIZE) ? PAGE_SIZE : count;
+		src = buffer;
+
+		if (copy_from_user(src, buf, c)) {
+			err = -EFAULT;
+			break;
+		}
+
+		fb_memcpy_tofb(dst, src, c);
+		dst += c;
+		src += c;
+		*ppos += c;
+		buf += c;
+		cnt += c;
+		count -= c;
+	}
+
+	kfree(buffer);
+
+	return (cnt) ? cnt : err;
+}
+EXPORT_SYMBOL(fb_cfb_write);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index b993bb97058f..be6c75f3dfd0 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -761,12 +761,7 @@  static struct fb_info *file_fb_info(struct file *file)
 static ssize_t
 fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
-	unsigned long p = *ppos;
 	struct fb_info *info = file_fb_info(file);
-	u8 *buffer, *dst;
-	u8 __iomem *src;
-	int c, cnt = 0, err = 0;
-	unsigned long total_size;
 
 	if (!info)
 		return -ENODEV;
@@ -777,64 +772,13 @@  fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	if (info->fbops->fb_read)
 		return info->fbops->fb_read(info, buf, count, ppos);
 
-	if (!info->screen_base)
-		return -ENODEV;
-
-	total_size = info->screen_size;
-
-	if (total_size == 0)
-		total_size = info->fix.smem_len;
-
-	if (p >= total_size)
-		return 0;
-
-	if (count >= total_size)
-		count = total_size;
-
-	if (count + p > total_size)
-		count = total_size - p;
-
-	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
-			 GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
-
-	src = (u8 __iomem *) (info->screen_base + p);
-
-	if (info->fbops->fb_sync)
-		info->fbops->fb_sync(info);
-
-	while (count) {
-		c  = (count > PAGE_SIZE) ? PAGE_SIZE : count;
-		dst = buffer;
-		fb_memcpy_fromfb(dst, src, c);
-		dst += c;
-		src += c;
-
-		if (copy_to_user(buf, buffer, c)) {
-			err = -EFAULT;
-			break;
-		}
-		*ppos += c;
-		buf += c;
-		cnt += c;
-		count -= c;
-	}
-
-	kfree(buffer);
-
-	return cnt ? cnt : err;
+	return fb_cfb_read(info, buf, count, ppos);
 }
 
 static ssize_t
 fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
-	unsigned long p = *ppos;
 	struct fb_info *info = file_fb_info(file);
-	u8 *buffer, *src;
-	u8 __iomem *dst;
-	int c, cnt = 0, err = 0;
-	unsigned long total_size;
 
 	if (!info)
 		return -ENODEV;
@@ -845,60 +789,7 @@  fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	if (info->fbops->fb_write)
 		return info->fbops->fb_write(info, buf, count, ppos);
 
-	if (!info->screen_base)
-		return -ENODEV;
-
-	total_size = info->screen_size;
-
-	if (total_size == 0)
-		total_size = info->fix.smem_len;
-
-	if (p > total_size)
-		return -EFBIG;
-
-	if (count > total_size) {
-		err = -EFBIG;
-		count = total_size;
-	}
-
-	if (count + p > total_size) {
-		if (!err)
-			err = -ENOSPC;
-
-		count = total_size - p;
-	}
-
-	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
-			 GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
-
-	dst = (u8 __iomem *) (info->screen_base + p);
-
-	if (info->fbops->fb_sync)
-		info->fbops->fb_sync(info);
-
-	while (count) {
-		c = (count > PAGE_SIZE) ? PAGE_SIZE : count;
-		src = buffer;
-
-		if (copy_from_user(src, buf, c)) {
-			err = -EFAULT;
-			break;
-		}
-
-		fb_memcpy_tofb(dst, src, c);
-		dst += c;
-		src += c;
-		*ppos += c;
-		buf += c;
-		cnt += c;
-		count -= c;
-	}
-
-	kfree(buffer);
-
-	return (cnt) ? cnt : err;
+	return fb_cfb_write(info, buf, count, ppos);
 }
 
 int
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 08cb47da71f8..3b1644c79973 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -576,9 +576,19 @@  struct fb_info {
 extern int fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var);
 extern int fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var);
 extern int fb_blank(struct fb_info *info, int blank);
+
+/*
+ * Drawing operations where framebuffer is in video RAM
+ */
+
 extern void cfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
 extern void cfb_copyarea(struct fb_info *info, const struct fb_copyarea *area);
 extern void cfb_imageblit(struct fb_info *info, const struct fb_image *image);
+extern ssize_t fb_cfb_read(struct fb_info *info, char __user *buf, size_t count,
+			   loff_t *ppos);
+extern ssize_t fb_cfb_write(struct fb_info *info, const char __user *buf,
+			    size_t count, loff_t *ppos);
+
 /*
  * Drawing operations where framebuffer is in system RAM
  */