[PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page

linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page
@ 2022-04-26 12:03 Thomas Zimmermann
  2022-04-26 12:03 ` [PATCH v3 1/5] fbdev: Put mmap for deferred I/O into drivers Thomas Zimmermann
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2022-04-26 12:03 UTC (permalink / raw)
  To: javierm, daniel, deller, airlied, maarten.lankhorst
  Cc: dri-devel, linux-fbdev, Thomas Zimmermann

Rework the fbdev deferred-I/O to not interfere with fields of struct
page. All references from deferred-I/O code to fields in struct page
are gone. The rsp state is help in a separate pageref structure.

v3:
	* rename pagelist to pagereflist (Sam)
	* use pageref->offset over page->index (Javier)
v2:
	* split off from a larger patchset [1]

[1] https://lore.kernel.org/dri-devel/20220303205839.28484-1-tzimmermann@suse.de/

Thomas Zimmermann (5):
  fbdev: Put mmap for deferred I/O into drivers
  fbdev: Track deferred-I/O pages in pageref struct
  fbdev: Refactor implementation of page_mkwrite
  fbdev: Rename pagelist to pagereflist for deferred I/O
  fbdev: Use pageref offset for deferred-I/O writeback

 drivers/gpu/drm/drm_fb_helper.c        |  15 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c     |  10 +-
 drivers/hid/hid-picolcd_fb.c           |   3 +-
 drivers/staging/fbtft/fbtft-core.c     |  21 ++-
 drivers/video/fbdev/broadsheetfb.c     |  27 ++--
 drivers/video/fbdev/core/fb_defio.c    | 212 +++++++++++++++++--------
 drivers/video/fbdev/core/fbmem.c       |  21 +--
 drivers/video/fbdev/hecubafb.c         |   4 +-
 drivers/video/fbdev/hyperv_fb.c        |  10 +-
 drivers/video/fbdev/metronomefb.c      |  23 ++-
 drivers/video/fbdev/sh_mobile_lcdcfb.c |  27 ++--
 drivers/video/fbdev/smscufx.c          |  13 +-
 drivers/video/fbdev/ssd1307fb.c        |   4 +-
 drivers/video/fbdev/udlfb.c            |  18 +--
 drivers/video/fbdev/xen-fbfront.c      |  10 +-
 include/drm/drm_fb_helper.h            |   3 +-
 include/linux/fb.h                     |  17 +-
 17 files changed, 264 insertions(+), 174 deletions(-)


base-commit: 0e7deff6446a4ba2c75f499a0bfa80cd6a15c129
-- 
2.36.0


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

* [PATCH v3 1/5] fbdev: Put mmap for deferred I/O into drivers
  2022-04-26 12:03 [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
@ 2022-04-26 12:03 ` Thomas Zimmermann
  2022-04-28  9:43   ` Dan Carpenter
  2022-04-26 12:03 ` [PATCH v3 2/5] fbdev: Track deferred-I/O pages in pageref struct Thomas Zimmermann
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2022-04-26 12:03 UTC (permalink / raw)
  To: javierm, daniel, deller, airlied, maarten.lankhorst
  Cc: dri-devel, linux-fbdev, Thomas Zimmermann

The fbdev mmap function fb_mmap() unconditionally overrides the
driver's implementation if deferred I/O has been activated. This
makes it hard to implement mmap with anything but a vmalloc()'ed
software buffer. That is specifically a problem for DRM, where
video memory is maintained by a memory manager.

Leave the mmap handling to drivers and expect them to call the
helper for deferred I/O by thmeselves.

v3:
	* fix warning if fb_mmap is missing (kernel test robot)
v2:
	* print a helpful error message if the defio setup is
	  incorrect (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fb_helper.c        |  4 +++-
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c     |  1 +
 drivers/hid/hid-picolcd_fb.c           |  1 +
 drivers/staging/fbtft/fbtft-core.c     |  1 +
 drivers/video/fbdev/broadsheetfb.c     |  1 +
 drivers/video/fbdev/core/fb_defio.c    |  1 +
 drivers/video/fbdev/core/fbmem.c       | 21 +++++++++++----------
 drivers/video/fbdev/hecubafb.c         |  1 +
 drivers/video/fbdev/hyperv_fb.c        |  1 +
 drivers/video/fbdev/metronomefb.c      |  1 +
 drivers/video/fbdev/sh_mobile_lcdcfb.c |  6 ++++++
 drivers/video/fbdev/smscufx.c          |  3 +++
 drivers/video/fbdev/ssd1307fb.c        |  1 +
 drivers/video/fbdev/udlfb.c            |  3 +++
 drivers/video/fbdev/xen-fbfront.c      |  1 +
 15 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d265a73313c9..d06ce0e92d66 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2118,7 +2118,9 @@ static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 
-	if (fb_helper->dev->driver->gem_prime_mmap)
+	if (drm_fbdev_use_shadow_fb(fb_helper))
+		return fb_deferred_io_mmap(info, vma);
+	else if (fb_helper->dev->driver->gem_prime_mmap)
 		return fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma);
 	else
 		return -ENODEV;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index adf17c740656..02a4a4fa3da3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -619,6 +619,7 @@ static const struct fb_ops vmw_fb_ops = {
 	.fb_imageblit = vmw_fb_imageblit,
 	.fb_pan_display = vmw_fb_pan_display,
 	.fb_blank = vmw_fb_blank,
+	.fb_mmap = fb_deferred_io_mmap,
 };
 
 int vmw_fb_init(struct vmw_private *vmw_priv)
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index 33c102a60992..78515e6bacf0 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -428,6 +428,7 @@ static const struct fb_ops picolcdfb_ops = {
 	.fb_imageblit = picolcd_fb_imageblit,
 	.fb_check_var = picolcd_fb_check_var,
 	.fb_set_par   = picolcd_set_par,
+	.fb_mmap      = fb_deferred_io_mmap,
 };
 
 
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 9c4d797e7ae4..d4e14f7c9d57 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -652,6 +652,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
 	fbops->fb_imageblit =      fbtft_fb_imageblit;
 	fbops->fb_setcolreg =      fbtft_fb_setcolreg;
 	fbops->fb_blank     =      fbtft_fb_blank;
+	fbops->fb_mmap      =      fb_deferred_io_mmap;
 
 	fbdefio->delay =           HZ / fps;
 	fbdefio->sort_pagelist =   true;
diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
index b9054f658838..528bc0902338 100644
--- a/drivers/video/fbdev/broadsheetfb.c
+++ b/drivers/video/fbdev/broadsheetfb.c
@@ -1055,6 +1055,7 @@ static const struct fb_ops broadsheetfb_ops = {
 	.fb_fillrect	= broadsheetfb_fillrect,
 	.fb_copyarea	= broadsheetfb_copyarea,
 	.fb_imageblit	= broadsheetfb_imageblit,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static struct fb_deferred_io broadsheetfb_defio = {
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index 6aaf6d0abf39..6924d489a289 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -181,6 +181,7 @@ int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	vma->vm_private_data = info;
 	return 0;
 }
+EXPORT_SYMBOL_GPL(fb_deferred_io_mmap);
 
 /* workqueue callback */
 static void fb_deferred_io_work(struct work_struct *work)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 84427470367b..e6b625548927 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1334,7 +1334,6 @@ static int
 fb_mmap(struct file *file, struct vm_area_struct * vma)
 {
 	struct fb_info *info = file_fb_info(file);
-	int (*fb_mmap_fn)(struct fb_info *info, struct vm_area_struct *vma);
 	unsigned long mmio_pgoff;
 	unsigned long start;
 	u32 len;
@@ -1343,14 +1342,7 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
 		return -ENODEV;
 	mutex_lock(&info->mm_lock);
 
-	fb_mmap_fn = info->fbops->fb_mmap;
-
-#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
-	if (info->fbdefio)
-		fb_mmap_fn = fb_deferred_io_mmap;
-#endif
-
-	if (fb_mmap_fn) {
+	if (info->fbops->fb_mmap) {
 		int res;
 
 		/*
@@ -1358,9 +1350,18 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
 		 * SME protection is removed ahead of the call
 		 */
 		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
-		res = fb_mmap_fn(info, vma);
+		res = info->fbops->fb_mmap(info, vma);
 		mutex_unlock(&info->mm_lock);
 		return res;
+#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
+	} else if (info->fbdefio) {
+		/*
+		 * FB deferred I/O wants you to handle mmap in your drivers. At a
+		 * minimum, point struct fb_ops.fb_mmap to fb_deferred_io_mmap().
+		 */
+		dev_warn_once(info->dev, "fbdev mmap not set up for deferred I/O.\n");
+		return -ENODEV;
+#endif
 	}
 
 	/*
diff --git a/drivers/video/fbdev/hecubafb.c b/drivers/video/fbdev/hecubafb.c
index 00d77105161a..2c483e2cf9ec 100644
--- a/drivers/video/fbdev/hecubafb.c
+++ b/drivers/video/fbdev/hecubafb.c
@@ -204,6 +204,7 @@ static const struct fb_ops hecubafb_ops = {
 	.fb_fillrect	= hecubafb_fillrect,
 	.fb_copyarea	= hecubafb_copyarea,
 	.fb_imageblit	= hecubafb_imageblit,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static struct fb_deferred_io hecubafb_defio = {
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index c8e0ea27caf1..d7f6abf827b9 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -909,6 +909,7 @@ static const struct fb_ops hvfb_ops = {
 	.fb_copyarea = hvfb_cfb_copyarea,
 	.fb_imageblit = hvfb_cfb_imageblit,
 	.fb_blank = hvfb_blank,
+	.fb_mmap = fb_deferred_io_mmap,
 };
 
 
diff --git a/drivers/video/fbdev/metronomefb.c b/drivers/video/fbdev/metronomefb.c
index af858dd23ea6..2541f2fe065b 100644
--- a/drivers/video/fbdev/metronomefb.c
+++ b/drivers/video/fbdev/metronomefb.c
@@ -564,6 +564,7 @@ static const struct fb_ops metronomefb_ops = {
 	.fb_fillrect	= metronomefb_fillrect,
 	.fb_copyarea	= metronomefb_copyarea,
 	.fb_imageblit	= metronomefb_imageblit,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static struct fb_deferred_io metronomefb_defio = {
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index aa4ebe3192ec..1dc5079e4518 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -1483,6 +1483,9 @@ sh_mobile_lcdc_overlay_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct sh_mobile_lcdc_overlay *ovl = info->par;
 
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(info, vma);
+
 	return dma_mmap_coherent(ovl->channel->lcdc->dev, vma, ovl->fb_mem,
 				 ovl->dma_handle, ovl->fb_size);
 }
@@ -1957,6 +1960,9 @@ sh_mobile_lcdc_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct sh_mobile_lcdc_chan *ch = info->par;
 
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(info, vma);
+
 	return dma_mmap_coherent(ch->lcdc->dev, vma, ch->fb_mem,
 				 ch->dma_handle, ch->fb_size);
 }
diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 28768c272b73..9383f8d0914c 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -779,6 +779,9 @@ static int ufx_ops_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
 	unsigned long page, pos;
 
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(info, vma);
+
 	if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
 		return -EINVAL;
 	if (size > info->fix.smem_len)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index c6d5df31111d..7547b4628afc 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -368,6 +368,7 @@ static const struct fb_ops ssd1307fb_ops = {
 	.fb_fillrect	= ssd1307fb_fillrect,
 	.fb_copyarea	= ssd1307fb_copyarea,
 	.fb_imageblit	= ssd1307fb_imageblit,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static void ssd1307fb_deferred_io(struct fb_info *info,
diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
index b6ec0b8e2b72..9e5a33396ef9 100644
--- a/drivers/video/fbdev/udlfb.c
+++ b/drivers/video/fbdev/udlfb.c
@@ -326,6 +326,9 @@ static int dlfb_ops_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
 	unsigned long page, pos;
 
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(info, vma);
+
 	if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
 		return -EINVAL;
 	if (size > info->fix.smem_len)
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 6826f986da43..6c8846eba2fb 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -338,6 +338,7 @@ static const struct fb_ops xenfb_fb_ops = {
 	.fb_imageblit	= xenfb_imageblit,
 	.fb_check_var	= xenfb_check_var,
 	.fb_set_par     = xenfb_set_par,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static irqreturn_t xenfb_event_handler(int rq, void *dev_id)
-- 
2.36.0


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

* Re: [PATCH v3 1/5] fbdev: Put mmap for deferred I/O into drivers
  2022-04-26 12:03 ` [PATCH v3 1/5] fbdev: Put mmap for deferred I/O into drivers Thomas Zimmermann
@ 2022-04-28  9:43   ` Dan Carpenter
  0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2022-04-28  9:43 UTC (permalink / raw)
  To: kbuild, Thomas Zimmermann, javierm, daniel, deller, airlied,
	maarten.lankhorst
  Cc: lkp, kbuild-all, linux-fbdev, Thomas Zimmermann, dri-devel

Hi Thomas,

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/fbdev-Decouple-deferred-I-O-from-struct-page/20220426-200655
base:   0e7deff6446a4ba2c75f499a0bfa80cd6a15c129
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220428/202204280832.sGHcYdgQ-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/video/fbdev/core/fbmem.c:1389 fb_mmap() warn: inconsistent returns '&info->mm_lock'.

Old smatch warnings:
drivers/video/fbdev/core/fbmem.c:1660 do_register_framebuffer() error: buffer overflow 'registered_fb' 32 <= 32 (assuming for loop doesn't break)

vim +1389 drivers/video/fbdev/core/fbmem.c

^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1333  static int
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1334  fb_mmap(struct file *file, struct vm_area_struct * vma)
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1335  {
c47747fde931c0 drivers/video/fbmem.c            Linus Torvalds    2011-05-11  1336  	struct fb_info *info = file_fb_info(file);
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1337  	unsigned long mmio_pgoff;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1338  	unsigned long start;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1339  	u32 len;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1340  
c47747fde931c0 drivers/video/fbmem.c            Linus Torvalds    2011-05-11  1341  	if (!info)
c47747fde931c0 drivers/video/fbmem.c            Linus Torvalds    2011-05-11  1342  		return -ENODEV;
537a1bf059fa31 drivers/video/fbmem.c            Krzysztof Helt    2009-06-30  1343  	mutex_lock(&info->mm_lock);
12281c8dda5a3b drivers/video/fbdev/core/fbmem.c Jani Nikula       2019-11-29  1344  
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1345  	if (info->fbops->fb_mmap) {
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1346  		int res;
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1347  
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1348  		/*
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1349  		 * The framebuffer needs to be accessed decrypted, be sure
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1350  		 * SME protection is removed ahead of the call
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1351  		 */
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1352  		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1353  		res = info->fbops->fb_mmap(info, vma);
537a1bf059fa31 drivers/video/fbmem.c            Krzysztof Helt    2009-06-30  1354  		mutex_unlock(&info->mm_lock);
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1355  		return res;
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1356  #if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1357  	} else if (info->fbdefio) {
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1358  		/*
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1359  		 * FB deferred I/O wants you to handle mmap in your drivers. At a
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1360  		 * minimum, point struct fb_ops.fb_mmap to fb_deferred_io_mmap().
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1361  		 */
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1362  		dev_warn_once(info->dev, "fbdev mmap not set up for deferred I/O.\n");
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1363  		return -ENODEV;

mutex_unlock(&info->mm_lock); before the return.

8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1364  #endif
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1365  	}
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1366  
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1367  	/*
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1368  	 * Ugh. This can be either the frame buffer mapping, or
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1369  	 * if pgoff points past it, the mmio mapping.
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1370  	 */
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1371  	start = info->fix.smem_start;
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1372  	len = info->fix.smem_len;
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1373  	mmio_pgoff = PAGE_ALIGN((start & ~PAGE_MASK) + len) >> PAGE_SHIFT;
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1374  	if (vma->vm_pgoff >= mmio_pgoff) {
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1375  		if (info->var.accel_flags) {
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1376  			mutex_unlock(&info->mm_lock);
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1377  			return -EINVAL;
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1378  		}
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1379  
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1380  		vma->vm_pgoff -= mmio_pgoff;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1381  		start = info->fix.mmio_start;
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1382  		len = info->fix.mmio_len;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1383  	}
537a1bf059fa31 drivers/video/fbmem.c            Krzysztof Helt    2009-06-30  1384  	mutex_unlock(&info->mm_lock);
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1385  
c07fbfd17e614a drivers/video/fbmem.c            Daniel De Graaf   2010-08-10  1386  	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1387  	fb_pgprotect(file, vma, start);
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1388  
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19 @1389  	return vm_iomap_memory(vma, start, len);
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1390  }

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp


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

* [PATCH v3 2/5] fbdev: Track deferred-I/O pages in pageref struct
  2022-04-26 12:03 [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
  2022-04-26 12:03 ` [PATCH v3 1/5] fbdev: Put mmap for deferred I/O into drivers Thomas Zimmermann
@ 2022-04-26 12:03 ` Thomas Zimmermann
  2022-04-26 12:03 ` [PATCH v3 3/5] fbdev: Refactor implementation of page_mkwrite Thomas Zimmermann
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2022-04-26 12:03 UTC (permalink / raw)
  To: javierm, daniel, deller, airlied, maarten.lankhorst
  Cc: dri-devel, linux-fbdev, Thomas Zimmermann

Store the per-page state for fbdev's deferred I/O in struct
fb_deferred_io_pageref. Maintain a list of pagerefs for the pages
that have to be written back to video memory. Update all affected
drivers.

As with pages before, fbdev acquires a pageref when an mmaped page
of the framebuffer is being written to. It holds the pageref in a
list of all currently written pagerefs until it flushes the written
pages to video memory. Writeback occurs periodically. After writeback
fbdev releases all pagerefs and builds up a new dirty list until the
next writeback occurs.

Using pagerefs has a number of benefits.

For pages of the framebuffer, the deferred I/O code used struct
page.lru as an entry into the list of dirty pages. The lru field is
owned by the page cache, which makes deferred I/O incompatible with
some memory pages (e.g., most notably DRM's GEM SHMEM allocator).
struct fb_deferred_io_pageref now provides an entry into a list of
dirty framebuffer pages, freeing lru for use with the page cache.

Drivers also assumed that struct page.index is the page offset into
the framebuffer. This is not true for DRM buffers, which are located
at various offset within a mapped area. struct fb_deferred_io_pageref
explicitly stores an offset into the framebuffer. struct page.index
is now only the page offset into the mapped area.

These changes will allow DRM to use fbdev deferred I/O without an
intermediate shadow buffer.

v3:
	* use pageref->offset for sorting
	* fix grammar in comment
v2:
	* minor fixes in commit message

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fb_helper.c        |   6 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c     |   5 +-
 drivers/staging/fbtft/fbtft-core.c     |   5 +-
 drivers/video/fbdev/broadsheetfb.c     |   5 +-
 drivers/video/fbdev/core/fb_defio.c    | 156 ++++++++++++++++---------
 drivers/video/fbdev/hyperv_fb.c        |   5 +-
 drivers/video/fbdev/metronomefb.c      |   5 +-
 drivers/video/fbdev/sh_mobile_lcdcfb.c |   6 +-
 drivers/video/fbdev/smscufx.c          |   5 +-
 drivers/video/fbdev/udlfb.c            |   5 +-
 drivers/video/fbdev/xen-fbfront.c      |   5 +-
 include/linux/fb.h                     |  11 +-
 12 files changed, 145 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d06ce0e92d66..dd1d72d58b35 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -717,13 +717,13 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
 			       struct list_head *pagelist)
 {
 	unsigned long start, end, min, max;
-	struct page *page;
+	struct fb_deferred_io_pageref *pageref;
 	struct drm_rect damage_area;
 
 	min = ULONG_MAX;
 	max = 0;
-	list_for_each_entry(page, pagelist, lru) {
-		start = page->index << PAGE_SHIFT;
+	list_for_each_entry(pageref, pagelist, list) {
+		start = pageref->offset;
 		end = start + PAGE_SIZE;
 		min = min(min, start);
 		max = max(max, end);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index 02a4a4fa3da3..db02e17e12b6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -322,12 +322,13 @@ static void vmw_deferred_io(struct fb_info *info,
 	struct vmw_fb_par *par = info->par;
 	unsigned long start, end, min, max;
 	unsigned long flags;
-	struct page *page;
+	struct fb_deferred_io_pageref *pageref;
 	int y1, y2;
 
 	min = ULONG_MAX;
 	max = 0;
-	list_for_each_entry(page, pagelist, lru) {
+	list_for_each_entry(pageref, pagelist, list) {
+		struct page *page = pageref->page;
 		start = page->index << PAGE_SHIFT;
 		end = start + PAGE_SIZE - 1;
 		min = min(min, start);
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index d4e14f7c9d57..e9662822e1ef 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -326,7 +326,7 @@ static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagelist)
 {
 	struct fbtft_par *par = info->par;
 	unsigned int dirty_lines_start, dirty_lines_end;
-	struct page *page;
+	struct fb_deferred_io_pageref *pageref;
 	unsigned long index;
 	unsigned int y_low = 0, y_high = 0;
 	int count = 0;
@@ -340,7 +340,8 @@ static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagelist)
 	spin_unlock(&par->dirty_lock);
 
 	/* Mark display lines as dirty */
-	list_for_each_entry(page, pagelist, lru) {
+	list_for_each_entry(pageref, pagelist, list) {
+		struct page *page = pageref->page;
 		count++;
 		index = page->index << PAGE_SHIFT;
 		y_low = index / info->fix.line_length;
diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
index 528bc0902338..6afc6ef4cb5e 100644
--- a/drivers/video/fbdev/broadsheetfb.c
+++ b/drivers/video/fbdev/broadsheetfb.c
@@ -934,7 +934,7 @@ static void broadsheetfb_dpy_deferred_io(struct fb_info *info,
 {
 	u16 y1 = 0, h = 0;
 	int prev_index = -1;
-	struct page *cur;
+	struct fb_deferred_io_pageref *pageref;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
 	int h_inc;
 	u16 yres = info->var.yres;
@@ -944,7 +944,8 @@ static void broadsheetfb_dpy_deferred_io(struct fb_info *info,
 	h_inc = DIV_ROUND_UP(PAGE_SIZE , xres);
 
 	/* walk the written page list and swizzle the data */
-	list_for_each_entry(cur, &fbdefio->pagelist, lru) {
+	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+		struct page *cur = pageref->page;
 		if (prev_index < 0) {
 			/* just starting so assign first page */
 			y1 = (cur->index << PAGE_SHIFT) / xres;
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index 6924d489a289..1bdef5c241f7 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -36,6 +36,60 @@ static struct page *fb_deferred_io_page(struct fb_info *info, unsigned long offs
 	return page;
 }
 
+static struct fb_deferred_io_pageref *fb_deferred_io_pageref_get(struct fb_info *info,
+								 unsigned long offset,
+								 struct page *page)
+{
+	struct fb_deferred_io *fbdefio = info->fbdefio;
+	struct list_head *pos = &fbdefio->pagelist;
+	unsigned long pgoff = offset >> PAGE_SHIFT;
+	struct fb_deferred_io_pageref *pageref, *cur;
+
+	if (WARN_ON_ONCE(pgoff >= info->npagerefs))
+		return NULL; /* incorrect allocation size */
+
+	/* 1:1 mapping between pageref and page offset */
+	pageref = &info->pagerefs[pgoff];
+
+	/*
+	 * This check is to catch the case where a new process could start
+	 * writing to the same page through a new PTE. This new access
+	 * can cause a call to .page_mkwrite even if the original process'
+	 * PTE is marked writable.
+	 */
+	if (!list_empty(&pageref->list))
+		goto pageref_already_added;
+
+	pageref->page = page;
+	pageref->offset = pgoff << PAGE_SHIFT;
+
+	if (unlikely(fbdefio->sort_pagelist)) {
+		/*
+		 * We loop through the list of pagerefs before adding in
+		 * order to keep the pagerefs sorted. This has significant
+		 * overhead of O(n^2) with n being the number of written
+		 * pages. If possible, drivers should try to work with
+		 * unsorted page lists instead.
+		 */
+		list_for_each_entry(cur, &info->fbdefio->pagelist, list) {
+			if (cur->offset > pageref->offset)
+				break;
+		}
+		pos = &cur->list;
+	}
+
+	list_add_tail(&pageref->list, pos);
+
+pageref_already_added:
+	return pageref;
+}
+
+static void fb_deferred_io_pageref_put(struct fb_deferred_io_pageref *pageref,
+				       struct fb_info *info)
+{
+	list_del_init(&pageref->list);
+}
+
 /* this is to find and return the vmalloc-ed fb pages */
 static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
 {
@@ -59,7 +113,7 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
 		printk(KERN_ERR "no mapping available\n");
 
 	BUG_ON(!page->mapping);
-	page->index = vmf->pgoff;
+	page->index = vmf->pgoff; /* for page_mkclean() */
 
 	vmf->page = page;
 	return 0;
@@ -95,7 +149,11 @@ static vm_fault_t fb_deferred_io_mkwrite(struct vm_fault *vmf)
 	struct page *page = vmf->page;
 	struct fb_info *info = vmf->vma->vm_private_data;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
-	struct list_head *pos = &fbdefio->pagelist;
+	struct fb_deferred_io_pageref *pageref;
+	unsigned long offset;
+	vm_fault_t ret;
+
+	offset = (vmf->address - vmf->vma->vm_start);
 
 	/* this is a callback we get when userspace first tries to
 	write to the page. we schedule a workqueue. that workqueue
@@ -112,6 +170,12 @@ static vm_fault_t fb_deferred_io_mkwrite(struct vm_fault *vmf)
 	if (fbdefio->first_io && list_empty(&fbdefio->pagelist))
 		fbdefio->first_io(info);
 
+	pageref = fb_deferred_io_pageref_get(info, offset, page);
+	if (WARN_ON_ONCE(!pageref)) {
+		ret = VM_FAULT_OOM;
+		goto err_mutex_unlock;
+	}
+
 	/*
 	 * We want the page to remain locked from ->page_mkwrite until
 	 * the PTE is marked dirty to avoid page_mkclean() being called
@@ -120,47 +184,17 @@ static vm_fault_t fb_deferred_io_mkwrite(struct vm_fault *vmf)
 	 * Do this by locking the page here and informing the caller
 	 * about it with VM_FAULT_LOCKED.
 	 */
-	lock_page(page);
-
-	/*
-	 * This check is to catch the case where a new process could start
-	 * writing to the same page through a new PTE. This new access
-	 * can cause a call to .page_mkwrite even if the original process'
-	 * PTE is marked writable.
-	 *
-	 * TODO: The lru field is owned by the page cache; hence the name.
-	 *       We dequeue in fb_deferred_io_work() after flushing the
-	 *       page's content into video memory. Instead of lru, fbdefio
-	 *       should have it's own field.
-	 */
-	if (!list_empty(&page->lru))
-		goto page_already_added;
-
-	if (unlikely(fbdefio->sort_pagelist)) {
-		/*
-		 * We loop through the pagelist before adding in order to
-		 * keep the pagelist sorted. This has significant overhead
-		 * of O(n^2) with n being the number of written pages. If
-		 * possible, drivers should try to work with unsorted page
-		 * lists instead.
-		 */
-		struct page *cur;
-
-		list_for_each_entry(cur, &fbdefio->pagelist, lru) {
-			if (cur->index > page->index)
-				break;
-		}
-		pos = &cur->lru;
-	}
-
-	list_add_tail(&page->lru, pos);
+	lock_page(pageref->page);
 
-page_already_added:
 	mutex_unlock(&fbdefio->lock);
 
 	/* come back after delay to process the deferred IO */
 	schedule_delayed_work(&info->deferred_work, fbdefio->delay);
 	return VM_FAULT_LOCKED;
+
+err_mutex_unlock:
+	mutex_unlock(&fbdefio->lock);
+	return ret;
 }
 
 static const struct vm_operations_struct fb_deferred_io_vm_ops = {
@@ -186,15 +220,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_mmap);
 /* workqueue callback */
 static void fb_deferred_io_work(struct work_struct *work)
 {
-	struct fb_info *info = container_of(work, struct fb_info,
-						deferred_work.work);
-	struct list_head *node, *next;
-	struct page *cur;
+	struct fb_info *info = container_of(work, struct fb_info, deferred_work.work);
+	struct fb_deferred_io_pageref *pageref, *next;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
 
 	/* here we mkclean the pages, then do all deferred IO */
 	mutex_lock(&fbdefio->lock);
-	list_for_each_entry(cur, &fbdefio->pagelist, lru) {
+	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+		struct page *cur = pageref->page;
 		lock_page(cur);
 		page_mkclean(cur);
 		unlock_page(cur);
@@ -204,30 +237,48 @@ static void fb_deferred_io_work(struct work_struct *work)
 	fbdefio->deferred_io(info, &fbdefio->pagelist);
 
 	/* clear the list */
-	list_for_each_safe(node, next, &fbdefio->pagelist) {
-		list_del_init(node);
-	}
+	list_for_each_entry_safe(pageref, next, &fbdefio->pagelist, list)
+		fb_deferred_io_pageref_put(pageref, info);
+
 	mutex_unlock(&fbdefio->lock);
 }
 
-void fb_deferred_io_init(struct fb_info *info)
+int fb_deferred_io_init(struct fb_info *info)
 {
 	struct fb_deferred_io *fbdefio = info->fbdefio;
-	struct page *page;
-	unsigned int i;
+	struct fb_deferred_io_pageref *pagerefs;
+	unsigned long npagerefs, i;
+	int ret;
 
 	BUG_ON(!fbdefio);
+
+	if (WARN_ON(!info->fix.smem_len))
+		return -EINVAL;
+
 	mutex_init(&fbdefio->lock);
 	INIT_DELAYED_WORK(&info->deferred_work, fb_deferred_io_work);
 	INIT_LIST_HEAD(&fbdefio->pagelist);
 	if (fbdefio->delay == 0) /* set a default of 1 s */
 		fbdefio->delay = HZ;
 
-	/* initialize all the page lists one time */
-	for (i = 0; i < info->fix.smem_len; i += PAGE_SIZE) {
-		page = fb_deferred_io_page(info, i);
-		INIT_LIST_HEAD(&page->lru);
+	npagerefs = DIV_ROUND_UP(info->fix.smem_len, PAGE_SIZE);
+
+	/* alloc a page ref for each page of the display memory */
+	pagerefs = kvcalloc(npagerefs, sizeof(*pagerefs), GFP_KERNEL);
+	if (!pagerefs) {
+		ret = -ENOMEM;
+		goto err;
 	}
+	for (i = 0; i < npagerefs; ++i)
+		INIT_LIST_HEAD(&pagerefs[i].list);
+	info->npagerefs = npagerefs;
+	info->pagerefs = pagerefs;
+
+	return 0;
+
+err:
+	mutex_destroy(&fbdefio->lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(fb_deferred_io_init);
 
@@ -254,6 +305,7 @@ void fb_deferred_io_cleanup(struct fb_info *info)
 		page->mapping = NULL;
 	}
 
+	kvfree(info->pagerefs);
 	mutex_destroy(&fbdefio->lock);
 }
 EXPORT_SYMBOL_GPL(fb_deferred_io_cleanup);
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index d7f6abf827b9..51eb57ea68f7 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -424,7 +424,7 @@ static void synthvid_deferred_io(struct fb_info *p,
 				 struct list_head *pagelist)
 {
 	struct hvfb_par *par = p->par;
-	struct page *page;
+	struct fb_deferred_io_pageref *pageref;
 	unsigned long start, end;
 	int y1, y2, miny, maxy;
 
@@ -437,7 +437,8 @@ static void synthvid_deferred_io(struct fb_info *p,
 	 * in synthvid_update function by clamping the y2
 	 * value to yres.
 	 */
-	list_for_each_entry(page, pagelist, lru) {
+	list_for_each_entry(pageref, pagelist, list) {
+		struct page *page = pageref->page;
 		start = page->index << PAGE_SHIFT;
 		end = start + PAGE_SIZE - 1;
 		y1 = start / p->fix.line_length;
diff --git a/drivers/video/fbdev/metronomefb.c b/drivers/video/fbdev/metronomefb.c
index 2541f2fe065b..8352fa3f4cef 100644
--- a/drivers/video/fbdev/metronomefb.c
+++ b/drivers/video/fbdev/metronomefb.c
@@ -469,12 +469,13 @@ static void metronomefb_dpy_deferred_io(struct fb_info *info,
 				struct list_head *pagelist)
 {
 	u16 cksum;
-	struct page *cur;
+	struct fb_deferred_io_pageref *pageref;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct metronomefb_par *par = info->par;
 
 	/* walk the written page list and swizzle the data */
-	list_for_each_entry(cur, &fbdefio->pagelist, lru) {
+	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+		struct page *cur = pageref->page;
 		cksum = metronomefb_dpy_update_page(par,
 					(cur->index << PAGE_SHIFT));
 		par->metromem_img_csum -= par->csum_table[cur->index];
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index 1dc5079e4518..7fcc85352033 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -440,13 +440,15 @@ static int sh_mobile_lcdc_sginit(struct fb_info *info,
 {
 	struct sh_mobile_lcdc_chan *ch = info->par;
 	unsigned int nr_pages_max = ch->fb_size >> PAGE_SHIFT;
-	struct page *page;
+	struct fb_deferred_io_pageref *pageref;
 	int nr_pages = 0;
 
 	sg_init_table(ch->sglist, nr_pages_max);
 
-	list_for_each_entry(page, pagelist, lru)
+	list_for_each_entry(pageref, pagelist, list) {
+		struct page *page = pageref->page;
 		sg_set_page(&ch->sglist[nr_pages++], page, PAGE_SIZE, 0);
+	}
 
 	return nr_pages;
 }
diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 9383f8d0914c..eb108f56ea04 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -958,7 +958,7 @@ static void ufx_ops_fillrect(struct fb_info *info,
 static void ufx_dpy_deferred_io(struct fb_info *info,
 				struct list_head *pagelist)
 {
-	struct page *cur;
+	struct fb_deferred_io_pageref *pageref;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct ufx_data *dev = info->par;
 
@@ -969,9 +969,10 @@ static void ufx_dpy_deferred_io(struct fb_info *info,
 		return;
 
 	/* walk the written page list and render each to device */
-	list_for_each_entry(cur, &fbdefio->pagelist, lru) {
+	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
 		/* create a rectangle of full screen width that encloses the
 		 * entire dirty framebuffer page */
+		struct page *cur = pageref->page;
 		const int x = 0;
 		const int width = dev->info->var.xres;
 		const int y = (cur->index << PAGE_SHIFT) / (width * 2);
diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
index 9e5a33396ef9..8249ed8700d4 100644
--- a/drivers/video/fbdev/udlfb.c
+++ b/drivers/video/fbdev/udlfb.c
@@ -784,7 +784,7 @@ static void dlfb_ops_fillrect(struct fb_info *info,
 static void dlfb_dpy_deferred_io(struct fb_info *info,
 				struct list_head *pagelist)
 {
-	struct page *cur;
+	struct fb_deferred_io_pageref *pageref;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct dlfb_data *dlfb = info->par;
 	struct urb *urb;
@@ -811,7 +811,8 @@ static void dlfb_dpy_deferred_io(struct fb_info *info,
 	cmd = urb->transfer_buffer;
 
 	/* walk the written page list and render each to device */
-	list_for_each_entry(cur, &fbdefio->pagelist, lru) {
+	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+		struct page *cur = pageref->page;
 
 		if (dlfb_render_hline(dlfb, &urb, (char *) info->fix.smem_start,
 				  &cmd, cur->index << PAGE_SHIFT,
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 6c8846eba2fb..608fcde767d3 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -185,13 +185,14 @@ static void xenfb_deferred_io(struct fb_info *fb_info,
 			      struct list_head *pagelist)
 {
 	struct xenfb_info *info = fb_info->par;
-	struct page *page;
+	struct fb_deferred_io_pageref *pageref;
 	unsigned long beg, end;
 	int y1, y2, miny, maxy;
 
 	miny = INT_MAX;
 	maxy = 0;
-	list_for_each_entry(page, pagelist, lru) {
+	list_for_each_entry(pageref, pagelist, list) {
+		struct page *page = pageref->page;
 		beg = page->index << PAGE_SHIFT;
 		end = beg + PAGE_SIZE - 1;
 		y1 = beg / fb_info->fix.line_length;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f95da1af9ff6..a332590c0fae 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -201,6 +201,13 @@ struct fb_pixmap {
 };
 
 #ifdef CONFIG_FB_DEFERRED_IO
+struct fb_deferred_io_pageref {
+	struct page *page;
+	unsigned long offset;
+	/* private */
+	struct list_head list;
+};
+
 struct fb_deferred_io {
 	/* delay between mkwrite and deferred handler */
 	unsigned long delay;
@@ -468,6 +475,8 @@ struct fb_info {
 #endif
 #ifdef CONFIG_FB_DEFERRED_IO
 	struct delayed_work deferred_work;
+	unsigned long npagerefs;
+	struct fb_deferred_io_pageref *pagerefs;
 	struct fb_deferred_io *fbdefio;
 #endif
 
@@ -661,7 +670,7 @@ static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch,
 
 /* drivers/video/fb_defio.c */
 int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma);
-extern void fb_deferred_io_init(struct fb_info *info);
+extern int  fb_deferred_io_init(struct fb_info *info);
 extern void fb_deferred_io_open(struct fb_info *info,
 				struct inode *inode,
 				struct file *file);
-- 
2.36.0


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

* [PATCH v3 3/5] fbdev: Refactor implementation of page_mkwrite
  2022-04-26 12:03 [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
  2022-04-26 12:03 ` [PATCH v3 1/5] fbdev: Put mmap for deferred I/O into drivers Thomas Zimmermann
  2022-04-26 12:03 ` [PATCH v3 2/5] fbdev: Track deferred-I/O pages in pageref struct Thomas Zimmermann
@ 2022-04-26 12:03 ` Thomas Zimmermann
  2022-04-26 12:03 ` [PATCH v3 4/5] fbdev: Rename pagelist to pagereflist for deferred I/O Thomas Zimmermann
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2022-04-26 12:03 UTC (permalink / raw)
  To: javierm, daniel, deller, airlied, maarten.lankhorst
  Cc: dri-devel, linux-fbdev, Thomas Zimmermann

Refactor the page-write handler for deferred I/O. Drivers use the
function to let fbdev track written pages of mmap'ed framebuffer
memory.

v3:
	* keep locking within track-pages function for readability (Sam)
v2:
	* don't export the helper until we have an external caller

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

diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index 1bdef5c241f7..3f9bb4725119 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -143,26 +143,17 @@ int fb_deferred_io_fsync(struct file *file, loff_t start, loff_t end, int datasy
 }
 EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
 
-/* vm_ops->page_mkwrite handler */
-static vm_fault_t fb_deferred_io_mkwrite(struct vm_fault *vmf)
+/*
+ * Adds a page to the dirty list. Call this from struct
+ * vm_operations_struct.page_mkwrite.
+ */
+static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
+					    struct page *page)
 {
-	struct page *page = vmf->page;
-	struct fb_info *info = vmf->vma->vm_private_data;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct fb_deferred_io_pageref *pageref;
-	unsigned long offset;
 	vm_fault_t ret;
 
-	offset = (vmf->address - vmf->vma->vm_start);
-
-	/* this is a callback we get when userspace first tries to
-	write to the page. we schedule a workqueue. that workqueue
-	will eventually mkclean the touched pages and execute the
-	deferred framebuffer IO. then if userspace touches a page
-	again, we repeat the same scheme */
-
-	file_update_time(vmf->vma->vm_file);
-
 	/* protect against the workqueue changing the page list */
 	mutex_lock(&fbdefio->lock);
 
@@ -197,6 +188,38 @@ static vm_fault_t fb_deferred_io_mkwrite(struct vm_fault *vmf)
 	return ret;
 }
 
+/*
+ * fb_deferred_io_page_mkwrite - Mark a page as written for deferred I/O
+ * @fb_info: The fbdev info structure
+ * @vmf: The VM fault
+ *
+ * This is a callback we get when userspace first tries to
+ * write to the page. We schedule a workqueue. That workqueue
+ * will eventually mkclean the touched pages and execute the
+ * deferred framebuffer IO. Then if userspace touches a page
+ * again, we repeat the same scheme.
+ *
+ * Returns:
+ * VM_FAULT_LOCKED on success, or a VM_FAULT error otherwise.
+ */
+static vm_fault_t fb_deferred_io_page_mkwrite(struct fb_info *info, struct vm_fault *vmf)
+{
+	unsigned long offset = vmf->address - vmf->vma->vm_start;
+	struct page *page = vmf->page;
+
+	file_update_time(vmf->vma->vm_file);
+
+	return fb_deferred_io_track_page(info, offset, page);
+}
+
+/* vm_ops->page_mkwrite handler */
+static vm_fault_t fb_deferred_io_mkwrite(struct vm_fault *vmf)
+{
+	struct fb_info *info = vmf->vma->vm_private_data;
+
+	return fb_deferred_io_page_mkwrite(info, vmf);
+}
+
 static const struct vm_operations_struct fb_deferred_io_vm_ops = {
 	.fault		= fb_deferred_io_fault,
 	.page_mkwrite	= fb_deferred_io_mkwrite,
-- 
2.36.0


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

* [PATCH v3 4/5] fbdev: Rename pagelist to pagereflist for deferred I/O
  2022-04-26 12:03 [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2022-04-26 12:03 ` [PATCH v3 3/5] fbdev: Refactor implementation of page_mkwrite Thomas Zimmermann
@ 2022-04-26 12:03 ` Thomas Zimmermann
  2022-04-29  7:21   ` Javier Martinez Canillas
  2022-04-26 12:03 ` [PATCH v3 5/5] fbdev: Use pageref offset for deferred-I/O writeback Thomas Zimmermann
  2022-04-26 12:05 ` [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
  5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2022-04-26 12:03 UTC (permalink / raw)
  To: javierm, daniel, deller, airlied, maarten.lankhorst
  Cc: dri-devel, linux-fbdev, Thomas Zimmermann, Sam Ravnborg

Rename various instances of pagelist to pagereflist. The list now
stores pageref structures, so the new name is more appropriate.

In their write-back helpers, several fbdev drivers refer to the
pageref list in struct fb_deferred_io instead of using the one
supplied as argument to the function. Convert them over to the
supplied one. It's the same instance, so no change of behavior
occurs.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Suggested-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/drm_fb_helper.c        |  7 +++----
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c     |  5 ++---
 drivers/hid/hid-picolcd_fb.c           |  2 +-
 drivers/staging/fbtft/fbtft-core.c     | 10 +++++-----
 drivers/video/fbdev/broadsheetfb.c     | 12 +++++-------
 drivers/video/fbdev/core/fb_defio.c    | 18 +++++++++---------
 drivers/video/fbdev/hecubafb.c         |  3 +--
 drivers/video/fbdev/hyperv_fb.c        |  5 ++---
 drivers/video/fbdev/metronomefb.c      | 12 +++++-------
 drivers/video/fbdev/sh_mobile_lcdcfb.c | 16 +++++++---------
 drivers/video/fbdev/smscufx.c          |  8 +++-----
 drivers/video/fbdev/ssd1307fb.c        |  3 +--
 drivers/video/fbdev/udlfb.c            |  8 +++-----
 drivers/video/fbdev/xen-fbfront.c      |  5 ++---
 include/drm/drm_fb_helper.h            |  3 +--
 include/linux/fb.h                     |  6 +++---
 16 files changed, 53 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index dd1d72d58b35..5ad2b6a2778c 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -708,13 +708,12 @@ static void drm_fb_helper_memory_range_to_clip(struct fb_info *info, off_t off,
 /**
  * drm_fb_helper_deferred_io() - fbdev deferred_io callback function
  * @info: fb_info struct pointer
- * @pagelist: list of mmap framebuffer pages that have to be flushed
+ * @pagereflist: list of mmap framebuffer pages that have to be flushed
  *
  * This function is used as the &fb_deferred_io.deferred_io
  * callback function for flushing the fbdev mmap writes.
  */
-void drm_fb_helper_deferred_io(struct fb_info *info,
-			       struct list_head *pagelist)
+void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	unsigned long start, end, min, max;
 	struct fb_deferred_io_pageref *pageref;
@@ -722,7 +721,7 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
 
 	min = ULONG_MAX;
 	max = 0;
-	list_for_each_entry(pageref, pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		start = pageref->offset;
 		end = start + PAGE_SIZE;
 		min = min(min, start);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index db02e17e12b6..9a3dc5c4eec8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -316,8 +316,7 @@ static int vmw_fb_pan_display(struct fb_var_screeninfo *var,
 	return 0;
 }
 
-static void vmw_deferred_io(struct fb_info *info,
-			    struct list_head *pagelist)
+static void vmw_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	struct vmw_fb_par *par = info->par;
 	unsigned long start, end, min, max;
@@ -327,7 +326,7 @@ static void vmw_deferred_io(struct fb_info *info,
 
 	min = ULONG_MAX;
 	max = 0;
-	list_for_each_entry(pageref, pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		struct page *page = pageref->page;
 		start = page->index << PAGE_SHIFT;
 		end = start + PAGE_SIZE - 1;
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index 78515e6bacf0..de61c08fabea 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -433,7 +433,7 @@ static const struct fb_ops picolcdfb_ops = {
 
 
 /* Callback from deferred IO workqueue */
-static void picolcd_fb_deferred_io(struct fb_info *info, struct list_head *pagelist)
+static void picolcd_fb_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	picolcd_fb_update(info);
 }
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index e9662822e1ef..8774bffe94cc 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -322,7 +322,7 @@ static void fbtft_mkdirty(struct fb_info *info, int y, int height)
 	schedule_delayed_work(&info->deferred_work, fbdefio->delay);
 }
 
-static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagelist)
+static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	struct fbtft_par *par = info->par;
 	unsigned int dirty_lines_start, dirty_lines_end;
@@ -340,7 +340,7 @@ static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagelist)
 	spin_unlock(&par->dirty_lock);
 
 	/* Mark display lines as dirty */
-	list_for_each_entry(pageref, pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		struct page *page = pageref->page;
 		count++;
 		index = page->index << PAGE_SHIFT;
@@ -655,9 +655,9 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
 	fbops->fb_blank     =      fbtft_fb_blank;
 	fbops->fb_mmap      =      fb_deferred_io_mmap;
 
-	fbdefio->delay =           HZ / fps;
-	fbdefio->sort_pagelist =   true;
-	fbdefio->deferred_io =     fbtft_deferred_io;
+	fbdefio->delay =            HZ / fps;
+	fbdefio->sort_pagereflist = true;
+	fbdefio->deferred_io =      fbtft_deferred_io;
 	fb_deferred_io_init(info);
 
 	snprintf(info->fix.id, sizeof(info->fix.id), "%s", dev->driver->name);
diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
index 6afc6ef4cb5e..883a3ac03189 100644
--- a/drivers/video/fbdev/broadsheetfb.c
+++ b/drivers/video/fbdev/broadsheetfb.c
@@ -929,13 +929,11 @@ static void broadsheetfb_dpy_update(struct broadsheetfb_par *par)
 }
 
 /* this is called back from the deferred io workqueue */
-static void broadsheetfb_dpy_deferred_io(struct fb_info *info,
-				struct list_head *pagelist)
+static void broadsheetfb_dpy_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	u16 y1 = 0, h = 0;
 	int prev_index = -1;
 	struct fb_deferred_io_pageref *pageref;
-	struct fb_deferred_io *fbdefio = info->fbdefio;
 	int h_inc;
 	u16 yres = info->var.yres;
 	u16 xres = info->var.xres;
@@ -944,7 +942,7 @@ static void broadsheetfb_dpy_deferred_io(struct fb_info *info,
 	h_inc = DIV_ROUND_UP(PAGE_SIZE , xres);
 
 	/* walk the written page list and swizzle the data */
-	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		struct page *cur = pageref->page;
 		if (prev_index < 0) {
 			/* just starting so assign first page */
@@ -1060,9 +1058,9 @@ static const struct fb_ops broadsheetfb_ops = {
 };
 
 static struct fb_deferred_io broadsheetfb_defio = {
-	.delay		= HZ/4,
-	.sort_pagelist	= true,
-	.deferred_io	= broadsheetfb_dpy_deferred_io,
+	.delay			= HZ/4,
+	.sort_pagereflist	= true,
+	.deferred_io		= broadsheetfb_dpy_deferred_io,
 };
 
 static int broadsheetfb_probe(struct platform_device *dev)
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index 3f9bb4725119..c730253ab85c 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -41,7 +41,7 @@ static struct fb_deferred_io_pageref *fb_deferred_io_pageref_get(struct fb_info
 								 struct page *page)
 {
 	struct fb_deferred_io *fbdefio = info->fbdefio;
-	struct list_head *pos = &fbdefio->pagelist;
+	struct list_head *pos = &fbdefio->pagereflist;
 	unsigned long pgoff = offset >> PAGE_SHIFT;
 	struct fb_deferred_io_pageref *pageref, *cur;
 
@@ -63,7 +63,7 @@ static struct fb_deferred_io_pageref *fb_deferred_io_pageref_get(struct fb_info
 	pageref->page = page;
 	pageref->offset = pgoff << PAGE_SHIFT;
 
-	if (unlikely(fbdefio->sort_pagelist)) {
+	if (unlikely(fbdefio->sort_pagereflist)) {
 		/*
 		 * We loop through the list of pagerefs before adding in
 		 * order to keep the pagerefs sorted. This has significant
@@ -71,7 +71,7 @@ static struct fb_deferred_io_pageref *fb_deferred_io_pageref_get(struct fb_info
 		 * pages. If possible, drivers should try to work with
 		 * unsorted page lists instead.
 		 */
-		list_for_each_entry(cur, &info->fbdefio->pagelist, list) {
+		list_for_each_entry(cur, &fbdefio->pagereflist, list) {
 			if (cur->offset > pageref->offset)
 				break;
 		}
@@ -158,7 +158,7 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
 	mutex_lock(&fbdefio->lock);
 
 	/* first write in this cycle, notify the driver */
-	if (fbdefio->first_io && list_empty(&fbdefio->pagelist))
+	if (fbdefio->first_io && list_empty(&fbdefio->pagereflist))
 		fbdefio->first_io(info);
 
 	pageref = fb_deferred_io_pageref_get(info, offset, page);
@@ -249,18 +249,18 @@ static void fb_deferred_io_work(struct work_struct *work)
 
 	/* here we mkclean the pages, then do all deferred IO */
 	mutex_lock(&fbdefio->lock);
-	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+	list_for_each_entry(pageref, &fbdefio->pagereflist, list) {
 		struct page *cur = pageref->page;
 		lock_page(cur);
 		page_mkclean(cur);
 		unlock_page(cur);
 	}
 
-	/* driver's callback with pagelist */
-	fbdefio->deferred_io(info, &fbdefio->pagelist);
+	/* driver's callback with pagereflist */
+	fbdefio->deferred_io(info, &fbdefio->pagereflist);
 
 	/* clear the list */
-	list_for_each_entry_safe(pageref, next, &fbdefio->pagelist, list)
+	list_for_each_entry_safe(pageref, next, &fbdefio->pagereflist, list)
 		fb_deferred_io_pageref_put(pageref, info);
 
 	mutex_unlock(&fbdefio->lock);
@@ -280,7 +280,7 @@ int fb_deferred_io_init(struct fb_info *info)
 
 	mutex_init(&fbdefio->lock);
 	INIT_DELAYED_WORK(&info->deferred_work, fb_deferred_io_work);
-	INIT_LIST_HEAD(&fbdefio->pagelist);
+	INIT_LIST_HEAD(&fbdefio->pagereflist);
 	if (fbdefio->delay == 0) /* set a default of 1 s */
 		fbdefio->delay = HZ;
 
diff --git a/drivers/video/fbdev/hecubafb.c b/drivers/video/fbdev/hecubafb.c
index 2c483e2cf9ec..eb1eaadc1bbb 100644
--- a/drivers/video/fbdev/hecubafb.c
+++ b/drivers/video/fbdev/hecubafb.c
@@ -115,8 +115,7 @@ static void hecubafb_dpy_update(struct hecubafb_par *par)
 }
 
 /* this is called back from the deferred io workqueue */
-static void hecubafb_dpy_deferred_io(struct fb_info *info,
-				struct list_head *pagelist)
+static void hecubafb_dpy_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	hecubafb_dpy_update(info->par);
 }
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 51eb57ea68f7..550cf0990070 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -420,8 +420,7 @@ static void hvfb_docopy(struct hvfb_par *par,
 }
 
 /* Deferred IO callback */
-static void synthvid_deferred_io(struct fb_info *p,
-				 struct list_head *pagelist)
+static void synthvid_deferred_io(struct fb_info *p, struct list_head *pagereflist)
 {
 	struct hvfb_par *par = p->par;
 	struct fb_deferred_io_pageref *pageref;
@@ -437,7 +436,7 @@ static void synthvid_deferred_io(struct fb_info *p,
 	 * in synthvid_update function by clamping the y2
 	 * value to yres.
 	 */
-	list_for_each_entry(pageref, pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		struct page *page = pageref->page;
 		start = page->index << PAGE_SHIFT;
 		end = start + PAGE_SIZE - 1;
diff --git a/drivers/video/fbdev/metronomefb.c b/drivers/video/fbdev/metronomefb.c
index 8352fa3f4cef..f581c73d39df 100644
--- a/drivers/video/fbdev/metronomefb.c
+++ b/drivers/video/fbdev/metronomefb.c
@@ -465,16 +465,14 @@ static u16 metronomefb_dpy_update_page(struct metronomefb_par *par, int index)
 }
 
 /* this is called back from the deferred io workqueue */
-static void metronomefb_dpy_deferred_io(struct fb_info *info,
-				struct list_head *pagelist)
+static void metronomefb_dpy_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	u16 cksum;
 	struct fb_deferred_io_pageref *pageref;
-	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct metronomefb_par *par = info->par;
 
 	/* walk the written page list and swizzle the data */
-	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		struct page *cur = pageref->page;
 		cksum = metronomefb_dpy_update_page(par,
 					(cur->index << PAGE_SHIFT));
@@ -569,9 +567,9 @@ static const struct fb_ops metronomefb_ops = {
 };
 
 static struct fb_deferred_io metronomefb_defio = {
-	.delay		= HZ,
-	.sort_pagelist	= true,
-	.deferred_io	= metronomefb_dpy_deferred_io,
+	.delay			= HZ,
+	.sort_pagereflist	= true,
+	.deferred_io		= metronomefb_dpy_deferred_io,
 };
 
 static int metronomefb_probe(struct platform_device *dev)
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index 7fcc85352033..d36c2e63f516 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -435,8 +435,7 @@ static struct sh_mobile_lcdc_sys_bus_ops sh_mobile_lcdc_sys_bus_ops = {
 	.read_data	= lcdc_sys_read_data,
 };
 
-static int sh_mobile_lcdc_sginit(struct fb_info *info,
-				  struct list_head *pagelist)
+static int sh_mobile_lcdc_sginit(struct fb_info *info, struct list_head *pagereflist)
 {
 	struct sh_mobile_lcdc_chan *ch = info->par;
 	unsigned int nr_pages_max = ch->fb_size >> PAGE_SHIFT;
@@ -445,7 +444,7 @@ static int sh_mobile_lcdc_sginit(struct fb_info *info,
 
 	sg_init_table(ch->sglist, nr_pages_max);
 
-	list_for_each_entry(pageref, pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		struct page *page = pageref->page;
 		sg_set_page(&ch->sglist[nr_pages++], page, PAGE_SIZE, 0);
 	}
@@ -453,8 +452,7 @@ static int sh_mobile_lcdc_sginit(struct fb_info *info,
 	return nr_pages;
 }
 
-static void sh_mobile_lcdc_deferred_io(struct fb_info *info,
-				       struct list_head *pagelist)
+static void sh_mobile_lcdc_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	struct sh_mobile_lcdc_chan *ch = info->par;
 	const struct sh_mobile_lcdc_panel_cfg *panel = &ch->cfg->panel_cfg;
@@ -463,7 +461,7 @@ static void sh_mobile_lcdc_deferred_io(struct fb_info *info,
 	sh_mobile_lcdc_clk_on(ch->lcdc);
 
 	/*
-	 * It's possible to get here without anything on the pagelist via
+	 * It's possible to get here without anything on the pagereflist via
 	 * sh_mobile_lcdc_deferred_io_touch() or via a userspace fsync()
 	 * invocation. In the former case, the acceleration routines are
 	 * stepped in to when using the framebuffer console causing the
@@ -473,12 +471,12 @@ static void sh_mobile_lcdc_deferred_io(struct fb_info *info,
 	 * acceleration routines have their own methods for writing in
 	 * that still need to be updated.
 	 *
-	 * The fsync() and empty pagelist case could be optimized for,
+	 * The fsync() and empty pagereflist case could be optimized for,
 	 * but we don't bother, as any application exhibiting such
 	 * behaviour is fundamentally broken anyways.
 	 */
-	if (!list_empty(pagelist)) {
-		unsigned int nr_pages = sh_mobile_lcdc_sginit(info, pagelist);
+	if (!list_empty(pagereflist)) {
+		unsigned int nr_pages = sh_mobile_lcdc_sginit(info, pagereflist);
 
 		/* trigger panel update */
 		dma_map_sg(ch->lcdc->dev, ch->sglist, nr_pages, DMA_TO_DEVICE);
diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index eb108f56ea04..7f00605bc0d1 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -955,12 +955,10 @@ static void ufx_ops_fillrect(struct fb_info *info,
  *   Touching ANY framebuffer memory that triggers a page fault
  *   in fb_defio will cause a deadlock, when it also tries to
  *   grab the same mutex. */
-static void ufx_dpy_deferred_io(struct fb_info *info,
-				struct list_head *pagelist)
+static void ufx_dpy_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
-	struct fb_deferred_io_pageref *pageref;
-	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct ufx_data *dev = info->par;
+	struct fb_deferred_io_pageref *pageref;
 
 	if (!fb_defio)
 		return;
@@ -969,7 +967,7 @@ static void ufx_dpy_deferred_io(struct fb_info *info,
 		return;
 
 	/* walk the written page list and render each to device */
-	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		/* create a rectangle of full screen width that encloses the
 		 * entire dirty framebuffer page */
 		struct page *cur = pageref->page;
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 7547b4628afc..5c765655d000 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -371,8 +371,7 @@ static const struct fb_ops ssd1307fb_ops = {
 	.fb_mmap	= fb_deferred_io_mmap,
 };
 
-static void ssd1307fb_deferred_io(struct fb_info *info,
-				struct list_head *pagelist)
+static void ssd1307fb_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	ssd1307fb_update_display(info->par);
 }
diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
index 8249ed8700d4..c3758613abff 100644
--- a/drivers/video/fbdev/udlfb.c
+++ b/drivers/video/fbdev/udlfb.c
@@ -781,11 +781,9 @@ static void dlfb_ops_fillrect(struct fb_info *info,
  *   in fb_defio will cause a deadlock, when it also tries to
  *   grab the same mutex.
  */
-static void dlfb_dpy_deferred_io(struct fb_info *info,
-				struct list_head *pagelist)
+static void dlfb_dpy_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	struct fb_deferred_io_pageref *pageref;
-	struct fb_deferred_io *fbdefio = info->fbdefio;
 	struct dlfb_data *dlfb = info->par;
 	struct urb *urb;
 	char *cmd;
@@ -811,7 +809,7 @@ static void dlfb_dpy_deferred_io(struct fb_info *info,
 	cmd = urb->transfer_buffer;
 
 	/* walk the written page list and render each to device */
-	list_for_each_entry(pageref, &fbdefio->pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		struct page *cur = pageref->page;
 
 		if (dlfb_render_hline(dlfb, &urb, (char *) info->fix.smem_start,
@@ -984,7 +982,7 @@ static int dlfb_ops_open(struct fb_info *info, int user)
 
 		if (fbdefio) {
 			fbdefio->delay = DL_DEFIO_WRITE_DELAY;
-			fbdefio->sort_pagelist = true;
+			fbdefio->sort_pagereflist = true;
 			fbdefio->deferred_io = dlfb_dpy_deferred_io;
 		}
 
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 608fcde767d3..bc8244b9ce03 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -181,8 +181,7 @@ static void xenfb_refresh(struct xenfb_info *info,
 		xenfb_do_update(info, x1, y1, x2 - x1 + 1, y2 - y1 + 1);
 }
 
-static void xenfb_deferred_io(struct fb_info *fb_info,
-			      struct list_head *pagelist)
+static void xenfb_deferred_io(struct fb_info *fb_info, struct list_head *pagereflist)
 {
 	struct xenfb_info *info = fb_info->par;
 	struct fb_deferred_io_pageref *pageref;
@@ -191,7 +190,7 @@ static void xenfb_deferred_io(struct fb_info *fb_info,
 
 	miny = INT_MAX;
 	maxy = 0;
-	list_for_each_entry(pageref, pagelist, list) {
+	list_for_each_entry(pageref, pagereflist, list) {
 		struct page *page = pageref->page;
 		beg = page->index << PAGE_SHIFT;
 		end = beg + PAGE_SIZE - 1;
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 3af4624368d8..329607ca65c0 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -229,8 +229,7 @@ void drm_fb_helper_fill_info(struct fb_info *info,
 			     struct drm_fb_helper *fb_helper,
 			     struct drm_fb_helper_surface_size *sizes);
 
-void drm_fb_helper_deferred_io(struct fb_info *info,
-			       struct list_head *pagelist);
+void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagereflist);
 
 ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf,
 			       size_t count, loff_t *ppos);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index a332590c0fae..69c67c70fa78 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -211,9 +211,9 @@ struct fb_deferred_io_pageref {
 struct fb_deferred_io {
 	/* delay between mkwrite and deferred handler */
 	unsigned long delay;
-	bool sort_pagelist; /* sort pagelist by offset */
-	struct mutex lock; /* mutex that protects the page list */
-	struct list_head pagelist; /* list of touched pages */
+	bool sort_pagereflist; /* sort pagelist by offset */
+	struct mutex lock; /* mutex that protects the pageref list */
+	struct list_head pagereflist; /* list of pagerefs for touched pages */
 	/* callback */
 	void (*first_io)(struct fb_info *info);
 	void (*deferred_io)(struct fb_info *info, struct list_head *pagelist);
-- 
2.36.0


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

* Re: [PATCH v3 4/5] fbdev: Rename pagelist to pagereflist for deferred I/O
  2022-04-26 12:03 ` [PATCH v3 4/5] fbdev: Rename pagelist to pagereflist for deferred I/O Thomas Zimmermann
@ 2022-04-29  7:21   ` Javier Martinez Canillas
  2022-04-29  7:27     ` Thomas Zimmermann
  0 siblings, 1 reply; 12+ messages in thread
From: Javier Martinez Canillas @ 2022-04-29  7:21 UTC (permalink / raw)
  To: Thomas Zimmermann, daniel, deller, airlied, maarten.lankhorst
  Cc: linux-fbdev, Sam Ravnborg, dri-devel

Hello Thomas,

On 4/26/22 14:03, Thomas Zimmermann wrote:
> Rename various instances of pagelist to pagereflist. The list now
> stores pageref structures, so the new name is more appropriate.
> 
> In their write-back helpers, several fbdev drivers refer to the
> pageref list in struct fb_deferred_io instead of using the one
> supplied as argument to the function. Convert them over to the
> supplied one. It's the same instance, so no change of behavior
> occurs.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Suggested-by: Sam Ravnborg <sam@ravnborg.org>

I thougt the convention was to have Suggested-by before your S-o-B.

The patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


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

* Re: [PATCH v3 4/5] fbdev: Rename pagelist to pagereflist for deferred I/O
  2022-04-29  7:21   ` Javier Martinez Canillas
@ 2022-04-29  7:27     ` Thomas Zimmermann
  2022-04-29  7:38       ` Javier Martinez Canillas
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2022-04-29  7:27 UTC (permalink / raw)
  To: Javier Martinez Canillas, daniel, deller, airlied,
	maarten.lankhorst
  Cc: linux-fbdev, Sam Ravnborg, dri-devel


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

Hi

Am 29.04.22 um 09:21 schrieb Javier Martinez Canillas:
> Hello Thomas,
> 
> On 4/26/22 14:03, Thomas Zimmermann wrote:
>> Rename various instances of pagelist to pagereflist. The list now
>> stores pageref structures, so the new name is more appropriate.
>>
>> In their write-back helpers, several fbdev drivers refer to the
>> pageref list in struct fb_deferred_io instead of using the one
>> supplied as argument to the function. Convert them over to the
>> supplied one. It's the same instance, so no change of behavior
>> occurs.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
> 
> I thougt the convention was to have Suggested-by before your S-o-B.

I always thought that the convention was that the authors S-o-B goes 
first. But 'git log' indeed indicates otherwise. I will change this 
before merging the patch.

> 
> The patch looks good to me.
> 
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 

Thanks

Best regards
Thomas

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

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

* Re: [PATCH v3 4/5] fbdev: Rename pagelist to pagereflist for deferred I/O
  2022-04-29  7:27     ` Thomas Zimmermann
@ 2022-04-29  7:38       ` Javier Martinez Canillas
  0 siblings, 0 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2022-04-29  7:38 UTC (permalink / raw)
  To: Thomas Zimmermann, daniel, deller, airlied, maarten.lankhorst
  Cc: linux-fbdev, Sam Ravnborg, dri-devel

On 4/29/22 09:27, Thomas Zimmermann wrote:
> Hi
> 
> Am 29.04.22 um 09:21 schrieb Javier Martinez Canillas:
>> Hello Thomas,
>>
>> On 4/26/22 14:03, Thomas Zimmermann wrote:
>>> Rename various instances of pagelist to pagereflist. The list now
>>> stores pageref structures, so the new name is more appropriate.
>>>
>>> In their write-back helpers, several fbdev drivers refer to the
>>> pageref list in struct fb_deferred_io instead of using the one
>>> supplied as argument to the function. Convert them over to the
>>> supplied one. It's the same instance, so no change of behavior
>>> occurs.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Suggested-by: Sam Ravnborg <sam@ravnborg.org>
>>
>> I thougt the convention was to have Suggested-by before your S-o-B.
> 
> I always thought that the convention was that the authors S-o-B goes 
> first. But 'git log' indeed indicates otherwise. I will change this 
> before merging the patch.
> 

The way I understand it, is that besides other things it indicates a
chronological order of events. So the S-o-B will go first, followed
by any Reviewed-by, Acked-by, etc gathered.

But if someone suggests a change to you, that means the suggestion
happened before you wrote the patch and is expected to go first.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


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

* [PATCH v3 5/5] fbdev: Use pageref offset for deferred-I/O writeback
  2022-04-26 12:03 [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2022-04-26 12:03 ` [PATCH v3 4/5] fbdev: Rename pagelist to pagereflist for deferred I/O Thomas Zimmermann
@ 2022-04-26 12:03 ` Thomas Zimmermann
  2022-04-29  7:34   ` Javier Martinez Canillas
  2022-04-26 12:05 ` [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
  5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2022-04-26 12:03 UTC (permalink / raw)
  To: javierm, daniel, deller, airlied, maarten.lankhorst
  Cc: dri-devel, linux-fbdev, Thomas Zimmermann

Use pageref->offset instead of page->index for deferred-I/O writeback
where appropriate. Distinguishes between file-mapping offset and video-
memory offset. While at it, also remove unnecessary references to
struct page.

Fbdev's deferred-I/O code uses the two related page->index and
pageref->offset. The former is the page offset in the mapped file,
the latter is the byte offset in the video memory (or fbdev screen
buffer). It's the same value for fbdev drivers, but for DRM the values
can be different. Because GEM buffer objects are mapped at an offset
in the DRM device file, page->index has this offset added to it as well.
We currently don't hit this case in DRM, because all affected mappings
of GEM memory are performed with an internal, intermediate shadow buffer.

The value of page->index is required by page_mkclean(), which we
call to reset the mappings during the writeback phase of the deferred
I/O. The value of pageref->offset is for conveniently getting an offset
into video memory in fb helpers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Suggested-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c     |  3 +--
 drivers/staging/fbtft/fbtft-core.c     |  9 +++------
 drivers/video/fbdev/broadsheetfb.c     | 13 ++++++-------
 drivers/video/fbdev/hyperv_fb.c        |  3 +--
 drivers/video/fbdev/metronomefb.c      |  9 ++++-----
 drivers/video/fbdev/sh_mobile_lcdcfb.c |  3 +--
 drivers/video/fbdev/smscufx.c          |  3 +--
 drivers/video/fbdev/udlfb.c            |  6 ++----
 drivers/video/fbdev/xen-fbfront.c      |  3 +--
 9 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index 9a3dc5c4eec8..0ba9739f406d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -327,8 +327,7 @@ static void vmw_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 	min = ULONG_MAX;
 	max = 0;
 	list_for_each_entry(pageref, pagereflist, list) {
-		struct page *page = pageref->page;
-		start = page->index << PAGE_SHIFT;
+		start = pageref->offset;
 		end = start + PAGE_SIZE - 1;
 		min = min(min, start);
 		max = max(max, end);
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8774bffe94cc..60b2278d8b16 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -327,7 +327,6 @@ static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagereflis
 	struct fbtft_par *par = info->par;
 	unsigned int dirty_lines_start, dirty_lines_end;
 	struct fb_deferred_io_pageref *pageref;
-	unsigned long index;
 	unsigned int y_low = 0, y_high = 0;
 	int count = 0;
 
@@ -341,14 +340,12 @@ static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagereflis
 
 	/* Mark display lines as dirty */
 	list_for_each_entry(pageref, pagereflist, list) {
-		struct page *page = pageref->page;
 		count++;
-		index = page->index << PAGE_SHIFT;
-		y_low = index / info->fix.line_length;
-		y_high = (index + PAGE_SIZE - 1) / info->fix.line_length;
+		y_low = pageref->offset / info->fix.line_length;
+		y_high = (pageref->offset + PAGE_SIZE - 1) / info->fix.line_length;
 		dev_dbg(info->device,
 			"page->index=%lu y_low=%d y_high=%d\n",
-			page->index, y_low, y_high);
+			pageref->page->index, y_low, y_high);
 		if (y_high > info->var.yres - 1)
 			y_high = info->var.yres - 1;
 		if (y_low < dirty_lines_start)
diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
index 883a3ac03189..55e62dd96f9b 100644
--- a/drivers/video/fbdev/broadsheetfb.c
+++ b/drivers/video/fbdev/broadsheetfb.c
@@ -932,7 +932,7 @@ static void broadsheetfb_dpy_update(struct broadsheetfb_par *par)
 static void broadsheetfb_dpy_deferred_io(struct fb_info *info, struct list_head *pagereflist)
 {
 	u16 y1 = 0, h = 0;
-	int prev_index = -1;
+	unsigned long prev_offset = ULONG_MAX;
 	struct fb_deferred_io_pageref *pageref;
 	int h_inc;
 	u16 yres = info->var.yres;
@@ -943,22 +943,21 @@ static void broadsheetfb_dpy_deferred_io(struct fb_info *info, struct list_head
 
 	/* walk the written page list and swizzle the data */
 	list_for_each_entry(pageref, pagereflist, list) {
-		struct page *cur = pageref->page;
-		if (prev_index < 0) {
+		if (prev_offset == ULONG_MAX) {
 			/* just starting so assign first page */
-			y1 = (cur->index << PAGE_SHIFT) / xres;
+			y1 = pageref->offset / xres;
 			h = h_inc;
-		} else if ((prev_index + 1) == cur->index) {
+		} else if ((prev_offset + PAGE_SIZE) == pageref->offset) {
 			/* this page is consecutive so increase our height */
 			h += h_inc;
 		} else {
 			/* page not consecutive, issue previous update first */
 			broadsheetfb_dpy_update_pages(info->par, y1, y1 + h);
 			/* start over with our non consecutive page */
-			y1 = (cur->index << PAGE_SHIFT) / xres;
+			y1 = pageref->offset / xres;
 			h = h_inc;
 		}
-		prev_index = cur->index;
+		prev_offset = pageref->offset;
 	}
 
 	/* if we still have any pages to update we do so now */
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 550cf0990070..8359a513b600 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -437,8 +437,7 @@ static void synthvid_deferred_io(struct fb_info *p, struct list_head *pagereflis
 	 * value to yres.
 	 */
 	list_for_each_entry(pageref, pagereflist, list) {
-		struct page *page = pageref->page;
-		start = page->index << PAGE_SHIFT;
+		start = pageref->offset;
 		end = start + PAGE_SIZE - 1;
 		y1 = start / p->fix.line_length;
 		y2 = end / p->fix.line_length;
diff --git a/drivers/video/fbdev/metronomefb.c b/drivers/video/fbdev/metronomefb.c
index f581c73d39df..9fd4bb85d735 100644
--- a/drivers/video/fbdev/metronomefb.c
+++ b/drivers/video/fbdev/metronomefb.c
@@ -473,11 +473,10 @@ static void metronomefb_dpy_deferred_io(struct fb_info *info, struct list_head *
 
 	/* walk the written page list and swizzle the data */
 	list_for_each_entry(pageref, pagereflist, list) {
-		struct page *cur = pageref->page;
-		cksum = metronomefb_dpy_update_page(par,
-					(cur->index << PAGE_SHIFT));
-		par->metromem_img_csum -= par->csum_table[cur->index];
-		par->csum_table[cur->index] = cksum;
+		unsigned long pgoffset = pageref->offset >> PAGE_SHIFT;
+		cksum = metronomefb_dpy_update_page(par, pageref->offset);
+		par->metromem_img_csum -= par->csum_table[pgoffset];
+		par->csum_table[pgoffset] = cksum;
 		par->metromem_img_csum += cksum;
 	}
 
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index d36c2e63f516..de86a04782fd 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -445,8 +445,7 @@ static int sh_mobile_lcdc_sginit(struct fb_info *info, struct list_head *pageref
 	sg_init_table(ch->sglist, nr_pages_max);
 
 	list_for_each_entry(pageref, pagereflist, list) {
-		struct page *page = pageref->page;
-		sg_set_page(&ch->sglist[nr_pages++], page, PAGE_SIZE, 0);
+		sg_set_page(&ch->sglist[nr_pages++], pageref->page, PAGE_SIZE, 0);
 	}
 
 	return nr_pages;
diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 7f00605bc0d1..d7aa5511c361 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -970,10 +970,9 @@ static void ufx_dpy_deferred_io(struct fb_info *info, struct list_head *pagerefl
 	list_for_each_entry(pageref, pagereflist, list) {
 		/* create a rectangle of full screen width that encloses the
 		 * entire dirty framebuffer page */
-		struct page *cur = pageref->page;
 		const int x = 0;
 		const int width = dev->info->var.xres;
-		const int y = (cur->index << PAGE_SHIFT) / (width * 2);
+		const int y = pageref->offset / (width * 2);
 		int height = (PAGE_SIZE / (width * 2)) + 1;
 		height = min(height, (int)(dev->info->var.yres - y));
 
diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
index c3758613abff..e19cb8cc7a66 100644
--- a/drivers/video/fbdev/udlfb.c
+++ b/drivers/video/fbdev/udlfb.c
@@ -810,11 +810,9 @@ static void dlfb_dpy_deferred_io(struct fb_info *info, struct list_head *pageref
 
 	/* walk the written page list and render each to device */
 	list_for_each_entry(pageref, pagereflist, list) {
-		struct page *cur = pageref->page;
-
 		if (dlfb_render_hline(dlfb, &urb, (char *) info->fix.smem_start,
-				  &cmd, cur->index << PAGE_SHIFT,
-				  PAGE_SIZE, &bytes_identical, &bytes_sent))
+				      &cmd, pageref->offset, PAGE_SIZE,
+				      &bytes_identical, &bytes_sent))
 			goto error;
 		bytes_rendered += PAGE_SIZE;
 	}
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index bc8244b9ce03..3bed357a9870 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -191,8 +191,7 @@ static void xenfb_deferred_io(struct fb_info *fb_info, struct list_head *pageref
 	miny = INT_MAX;
 	maxy = 0;
 	list_for_each_entry(pageref, pagereflist, list) {
-		struct page *page = pageref->page;
-		beg = page->index << PAGE_SHIFT;
+		beg = pageref->offset;
 		end = beg + PAGE_SIZE - 1;
 		y1 = beg / fb_info->fix.line_length;
 		y2 = end / fb_info->fix.line_length;
-- 
2.36.0


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

* Re: [PATCH v3 5/5] fbdev: Use pageref offset for deferred-I/O writeback
  2022-04-26 12:03 ` [PATCH v3 5/5] fbdev: Use pageref offset for deferred-I/O writeback Thomas Zimmermann
@ 2022-04-29  7:34   ` Javier Martinez Canillas
  0 siblings, 0 replies; 12+ messages in thread
From: Javier Martinez Canillas @ 2022-04-29  7:34 UTC (permalink / raw)
  To: Thomas Zimmermann, daniel, deller, airlied, maarten.lankhorst
  Cc: linux-fbdev, dri-devel

On 4/26/22 14:03, Thomas Zimmermann wrote:
> Use pageref->offset instead of page->index for deferred-I/O writeback
> where appropriate. Distinguishes between file-mapping offset and video-
> memory offset. While at it, also remove unnecessary references to
> struct page.
> 
> Fbdev's deferred-I/O code uses the two related page->index and
> pageref->offset. The former is the page offset in the mapped file,
> the latter is the byte offset in the video memory (or fbdev screen
> buffer). It's the same value for fbdev drivers, but for DRM the values
> can be different. Because GEM buffer objects are mapped at an offset
> in the DRM device file, page->index has this offset added to it as well.
> We currently don't hit this case in DRM, because all affected mappings
> of GEM memory are performed with an internal, intermediate shadow buffer.
> 
> The value of page->index is required by page_mkclean(), which we
> call to reset the mappings during the writeback phase of the deferred
> I/O. The value of pageref->offset is for conveniently getting an offset
> into video memory in fb helpers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Suggested-by: Javier Martinez Canillas <javierm@redhat.com>
> ---

Thanks for including this patch.

[snip]

>  	list_for_each_entry(pageref, pagereflist, list) {
> -		struct page *page = pageref->page;
> -		start = page->index << PAGE_SHIFT;
> +		start = pageref->offset;

It also has the side effect that makes the code easier to read and as you
said in the change description, eliminates the references to struct page.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat


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

* Re: [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page
  2022-04-26 12:03 [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
                   ` (4 preceding siblings ...)
  2022-04-26 12:03 ` [PATCH v3 5/5] fbdev: Use pageref offset for deferred-I/O writeback Thomas Zimmermann
@ 2022-04-26 12:05 ` Thomas Zimmermann
  5 siblings, 0 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2022-04-26 12:05 UTC (permalink / raw)
  To: javierm, daniel, deller, airlied, maarten.lankhorst, Sam Ravnborg
  Cc: dri-devel, linux-fbdev


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

(cc'ing Sam)

Am 26.04.22 um 14:03 schrieb Thomas Zimmermann:
> Rework the fbdev deferred-I/O to not interfere with fields of struct
> page. All references from deferred-I/O code to fields in struct page
> are gone. The rsp state is help in a separate pageref structure.
> 
> v3:
> 	* rename pagelist to pagereflist (Sam)
> 	* use pageref->offset over page->index (Javier)
> v2:
> 	* split off from a larger patchset [1]
> 
> [1] https://lore.kernel.org/dri-devel/20220303205839.28484-1-tzimmermann@suse.de/
> 
> Thomas Zimmermann (5):
>    fbdev: Put mmap for deferred I/O into drivers
>    fbdev: Track deferred-I/O pages in pageref struct
>    fbdev: Refactor implementation of page_mkwrite
>    fbdev: Rename pagelist to pagereflist for deferred I/O
>    fbdev: Use pageref offset for deferred-I/O writeback
> 
>   drivers/gpu/drm/drm_fb_helper.c        |  15 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_fb.c     |  10 +-
>   drivers/hid/hid-picolcd_fb.c           |   3 +-
>   drivers/staging/fbtft/fbtft-core.c     |  21 ++-
>   drivers/video/fbdev/broadsheetfb.c     |  27 ++--
>   drivers/video/fbdev/core/fb_defio.c    | 212 +++++++++++++++++--------
>   drivers/video/fbdev/core/fbmem.c       |  21 +--
>   drivers/video/fbdev/hecubafb.c         |   4 +-
>   drivers/video/fbdev/hyperv_fb.c        |  10 +-
>   drivers/video/fbdev/metronomefb.c      |  23 ++-
>   drivers/video/fbdev/sh_mobile_lcdcfb.c |  27 ++--
>   drivers/video/fbdev/smscufx.c          |  13 +-
>   drivers/video/fbdev/ssd1307fb.c        |   4 +-
>   drivers/video/fbdev/udlfb.c            |  18 +--
>   drivers/video/fbdev/xen-fbfront.c      |  10 +-
>   include/drm/drm_fb_helper.h            |   3 +-
>   include/linux/fb.h                     |  17 +-
>   17 files changed, 264 insertions(+), 174 deletions(-)
> 
> 
> base-commit: 0e7deff6446a4ba2c75f499a0bfa80cd6a15c129

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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

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

end of thread, other threads:[~2022-04-29  7:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-26 12:03 [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann
2022-04-26 12:03 ` [PATCH v3 1/5] fbdev: Put mmap for deferred I/O into drivers Thomas Zimmermann
2022-04-28  9:43   ` Dan Carpenter
2022-04-26 12:03 ` [PATCH v3 2/5] fbdev: Track deferred-I/O pages in pageref struct Thomas Zimmermann
2022-04-26 12:03 ` [PATCH v3 3/5] fbdev: Refactor implementation of page_mkwrite Thomas Zimmermann
2022-04-26 12:03 ` [PATCH v3 4/5] fbdev: Rename pagelist to pagereflist for deferred I/O Thomas Zimmermann
2022-04-29  7:21   ` Javier Martinez Canillas
2022-04-29  7:27     ` Thomas Zimmermann
2022-04-29  7:38       ` Javier Martinez Canillas
2022-04-26 12:03 ` [PATCH v3 5/5] fbdev: Use pageref offset for deferred-I/O writeback Thomas Zimmermann
2022-04-29  7:34   ` Javier Martinez Canillas
2022-04-26 12:05 ` [PATCH v3 0/5] fbdev: Decouple deferred I/O from struct page Thomas Zimmermann

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