# \[PATCH 0/8\] fbdev: Use helpers for deferred I/O - Subsystems: DISPLAYLINK USB 2.0 FRAMEBUFFER DRIVER (UDLFB), FBTFT Framebuffer drivers, FRAMEBUFFER LAYER, SMSC UFX6000 and UFX7000 USB to VGA DRIVER, FRAMEBUFFER CORE, HID CORE LAYER, PICOLCD HID DRIVER, STAGING SUBSYSTEM, THE REST - Maintainers: Bernie Thompson, Andy Shevchenko, Helge Deller, Steve Glendinning, Simona Vetter, Jiri Kosina, Benjamin Tissoires, Bruno Prémont, Greg Kroah-Hartman, Linus Torvalds - Series: 8 patches touching 9 files, +123/-314 - Status: LANDED - Landed in mainline. - List: linux-fbdev - 24 messages, 4 authors - Page: https://ratatoskr.run/linux-fbdev/2023/08/8935599/t ## \[PATCH 0/8\] fbdev: Use helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:32 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935599.md ```text Here's another patchset for deferred-I/O helpers. Update a number of fbdev drivers with deferred I/O to use fbdev's helper macros and Kconfig tokens. Generating and initializing via helpers macros will later allow for a fine-grained setup, depending on Kconfig options. For example, it will be possible to leave out file I/O if FB_DEVICE has not been set. Each driver in special in its own way. The smscufx and udlfb drivers support file I/O without damage updates. (That probably doesn't work correctly.) So they provide their own mmap code. The hyperv_fb driver sometimes operates on memory in I/O address spaces and fails to do damage handling correctly for write operations. The picolcd and fbtft drivers are outside of fbdev. Thomas Zimmermann (8): fbdev/smscufx: Use fb_ops helpers for deferred I/O fbdev/udlfb: Use fb_ops helpers for deferred I/O fbdev: Add Kconfig macro FB_IOMEM_HELPERS_DEFERRED fbdev/hyperv_fb: Use fb_ops helpers for deferred I/O hid: Remove trailing whitespace hid/picolcd: Use fb_ops helpers for deferred I/O staging/fbtft: Initialize fb_op struct as static const staging/fbtft: Use fb_ops helpers for deferred I/O drivers/hid/Kconfig | 8 +-- drivers/hid/hid-picolcd_fb.c | 73 ++++++---------------- drivers/staging/fbtft/Kconfig | 6 +- drivers/staging/fbtft/fbtft-core.c | 99 ++++++++---------------------- drivers/video/fbdev/Kconfig | 5 +- drivers/video/fbdev/core/Kconfig | 6 ++ drivers/video/fbdev/hyperv_fb.c | 48 +++++---------- drivers/video/fbdev/smscufx.c | 85 +++++++------------------ drivers/video/fbdev/udlfb.c | 89 +++++++-------------------- 9 files changed, 114 insertions(+), 305 deletions(-) -- 2.41.0 ``` ## \[PATCH 2/8\] fbdev/udlfb: Use fb\_ops helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:33 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935600.md ```text Generate callback functions for struct fb_ops with the fbdev macro FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to the generated functions with fbdev initializer macros. Signed-off-by: Thomas Zimmermann Cc: Bernie Thompson --- drivers/video/fbdev/udlfb.c | 89 +++++++++---------------------------- 1 file changed, 22 insertions(+), 67 deletions(-) diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c index b70762ead13c..2460ff4ac86b 100644 --- a/drivers/video/fbdev/udlfb.c +++ b/drivers/video/fbdev/udlfb.c @@ -715,68 +715,6 @@ static void dlfb_offload_damage(struct dlfb_data *dlfb, int x, int y, int width, schedule_work(&dlfb->damage_work); } -/* - * Path triggered by usermode clients who write to filesystem - * e.g. cat filename > /dev/fb1 - * Not used by X Windows or text-mode console. But useful for testing. - * Slow because of extra copy and we must assume all pixels dirty. - */ -static ssize_t dlfb_ops_write(struct fb_info *info, const char __user *buf, - size_t count, loff_t *ppos) -{ - ssize_t result; - struct dlfb_data *dlfb = info->par; - u32 offset = (u32) *ppos; - - result = fb_sys_write(info, buf, count, ppos); - - if (result > 0) { - int start = max((int)(offset / info->fix.line_length), 0); - int lines = min((u32)((result / info->fix.line_length) + 1), - (u32)info->var.yres); - - dlfb_handle_damage(dlfb, 0, start, info->var.xres, - lines); - } - - return result; -} - -/* hardware has native COPY command (see libdlo), but not worth it for fbcon */ -static void dlfb_ops_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - - struct dlfb_data *dlfb = info->par; - - sys_copyarea(info, area); - - dlfb_offload_damage(dlfb, area->dx, area->dy, - area->width, area->height); -} - -static void dlfb_ops_imageblit(struct fb_info *info, - const struct fb_image *image) -{ - struct dlfb_data *dlfb = info->par; - - sys_imageblit(info, image); - - dlfb_offload_damage(dlfb, image->dx, image->dy, - image->width, image->height); -} - -static void dlfb_ops_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct dlfb_data *dlfb = info->par; - - sys_fillrect(info, rect); - - dlfb_offload_damage(dlfb, rect->dx, rect->dy, rect->width, - rect->height); -} - /* * NOTE: fb_defio.c is holding info->fbdefio.mutex * Touching ANY framebuffer memory that triggers a page fault @@ -1186,14 +1124,31 @@ static int dlfb_ops_blank(int blank_mode, struct fb_info *info) return 0; } +static void dlfb_ops_damage_range(struct fb_info *info, off_t off, size_t len) +{ + struct dlfb_data *dlfb = info->par; + int start = max((int)(off / info->fix.line_length), 0); + int lines = min((u32)((len / info->fix.line_length) + 1), (u32)info->var.yres); + + dlfb_handle_damage(dlfb, 0, start, info->var.xres, lines); +} + +static void dlfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) +{ + struct dlfb_data *dlfb = info->par; + + dlfb_offload_damage(dlfb, x, y, width, height); +} + +FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(dlfb_ops, + dlfb_ops_damage_range, + dlfb_ops_damage_area) + static const struct fb_ops dlfb_ops = { .owner = THIS_MODULE, - .fb_read = fb_sys_read, - .fb_write = dlfb_ops_write, + __FB_DEFAULT_DEFERRED_OPS_RDWR(dlfb_ops), .fb_setcolreg = dlfb_ops_setcolreg, - .fb_fillrect = dlfb_ops_fillrect, - .fb_copyarea = dlfb_ops_copyarea, - .fb_imageblit = dlfb_ops_imageblit, + __FB_DEFAULT_DEFERRED_OPS_DRAW(dlfb_ops), .fb_mmap = dlfb_ops_mmap, .fb_ioctl = dlfb_ops_ioctl, .fb_open = dlfb_ops_open, -- 2.41.0 ``` ## \[PATCH 1/8\] fbdev/smscufx: Use fb\_ops helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:33 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935601.md ```text Generate callback functions for struct fb_ops with the fbdev macro FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to the generated functions with fbdev initializer macros. Signed-off-by: Thomas Zimmermann Cc: Steve Glendinning --- drivers/video/fbdev/smscufx.c | 85 +++++++++-------------------------- 1 file changed, 22 insertions(+), 63 deletions(-) diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c index 387d18706fec..90a77d19b236 100644 --- a/drivers/video/fbdev/smscufx.c +++ b/drivers/video/fbdev/smscufx.c @@ -894,64 +894,6 @@ static int ufx_handle_damage(struct ufx_data *dev, int x, int y, return 0; } -/* Path triggered by usermode clients who write to filesystem - * e.g. cat filename > /dev/fb1 - * Not used by X Windows or text-mode console. But useful for testing. - * Slow because of extra copy and we must assume all pixels dirty. */ -static ssize_t ufx_ops_write(struct fb_info *info, const char __user *buf, - size_t count, loff_t *ppos) -{ - ssize_t result; - struct ufx_data *dev = info->par; - u32 offset = (u32) *ppos; - - result = fb_sys_write(info, buf, count, ppos); - - if (result > 0) { - int start = max((int)(offset / info->fix.line_length), 0); - int lines = min((u32)((result / info->fix.line_length) + 1), - (u32)info->var.yres); - - ufx_handle_damage(dev, 0, start, info->var.xres, lines); - } - - return result; -} - -static void ufx_ops_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - - struct ufx_data *dev = info->par; - - sys_copyarea(info, area); - - ufx_handle_damage(dev, area->dx, area->dy, - area->width, area->height); -} - -static void ufx_ops_imageblit(struct fb_info *info, - const struct fb_image *image) -{ - struct ufx_data *dev = info->par; - - sys_imageblit(info, image); - - ufx_handle_damage(dev, image->dx, image->dy, - image->width, image->height); -} - -static void ufx_ops_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct ufx_data *dev = info->par; - - sys_fillrect(info, rect); - - ufx_handle_damage(dev, rect->dx, rect->dy, rect->width, - rect->height); -} - /* NOTE: fb_defio.c is holding info->fbdefio.mutex * Touching ANY framebuffer memory that triggers a page fault * in fb_defio will cause a deadlock, when it also tries to @@ -1279,14 +1221,31 @@ static int ufx_ops_blank(int blank_mode, struct fb_info *info) return 0; } +static void ufx_ops_damage_range(struct fb_info *info, off_t off, size_t len) +{ + struct ufx_data *dev = info->par; + int start = max((int)(off / info->fix.line_length), 0); + int lines = min((u32)((len / info->fix.line_length) + 1), (u32)info->var.yres); + + ufx_handle_damage(dev, 0, start, info->var.xres, lines); +} + +static void ufx_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) +{ + struct ufx_data *dev = info->par; + + ufx_handle_damage(dev, x, y, width, height); +} + +FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(ufx_ops, + ufx_ops_damage_range, + ufx_ops_damage_area) + static const struct fb_ops ufx_ops = { .owner = THIS_MODULE, - .fb_read = fb_sys_read, - .fb_write = ufx_ops_write, + __FB_DEFAULT_DEFERRED_OPS_RDWR(ufx_ops), .fb_setcolreg = ufx_ops_setcolreg, - .fb_fillrect = ufx_ops_fillrect, - .fb_copyarea = ufx_ops_copyarea, - .fb_imageblit = ufx_ops_imageblit, + __FB_DEFAULT_DEFERRED_OPS_DRAW(ufx_ops), .fb_mmap = ufx_ops_mmap, .fb_ioctl = ufx_ops_ioctl, .fb_open = ufx_ops_open, -- 2.41.0 ``` ## \[PATCH 5/8\] hid: Remove trailing whitespace - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:34 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935602.md ```text Fix coding style in Kconfig. No functional changes. Signed-off-by: Thomas Zimmermann Cc: Jiri Kosina Cc: Benjamin Tissoires --- drivers/hid/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index e11c1c803676..b50054a41c10 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -1037,7 +1037,7 @@ config HID_SONY * Guitar Hero PS3 and PC guitar dongles config SONY_FF - bool "Sony PS2/3/4 accessories force feedback support" + bool "Sony PS2/3/4 accessories force feedback support" depends on HID_SONY select INPUT_FF_MEMLESS help -- 2.41.0 ``` ## \[PATCH 3/8\] fbdev: Add Kconfig macro FB\_IOMEM\_HELPERS\_DEFERRED - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:34 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935603.md ```text The new Kconfig macro FB_IOMEM_HELPERS_DEFERRED selects fbdev's helpers for device I/O memory and deferred I/O. Drivers should use it if they perform damage updates on device I/O memory. Signed-off-by: Thomas Zimmermann --- drivers/video/fbdev/core/Kconfig | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig index baf7e852c75b..e80d6429f76a 100644 --- a/drivers/video/fbdev/core/Kconfig +++ b/drivers/video/fbdev/core/Kconfig @@ -151,6 +151,12 @@ config FB_IOMEM_HELPERS select FB_CFB_FILLRECT select FB_CFB_IMAGEBLIT +config FB_IOMEM_HELPERS_DEFERRED + bool + depends on FB_CORE + select FB_DEFERRED_IO + select FB_IOMEM_HELPERS + config FB_SYSMEM_HELPERS bool depends on FB_CORE -- 2.41.0 ``` ## \[PATCH 4/8\] fbdev/hyperv\_fb: Use fb\_ops helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:34 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935604.md ```text Generate callback functions for struct fb_ops with the fbdev macro FB_GEN_DEFAULT_DEFERRED_IOMEM_OPS(). Initialize struct fb_ops to the generated functions with fbdev initializer macros. The hyperv_fb driver is incomplete in its handling of deferred I/O and damage framebuffers. Write operations do no trigger damage handling. Fixing this is beyond the scope of this patch. Signed-off-by: Thomas Zimmermann Cc: "K. Y. Srinivasan" Cc: Haiyang Zhang Cc: Wei Liu Cc: Dexuan Cui --- drivers/video/fbdev/Kconfig | 5 +--- drivers/video/fbdev/hyperv_fb.c | 48 ++++++++++----------------------- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 79b1e4e542e7..4455bfd57f0e 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -1905,11 +1905,8 @@ config FB_BROADSHEET config FB_HYPERV tristate "Microsoft Hyper-V Synthetic Video support" depends on FB && HYPERV - select FB_CFB_FILLRECT - select FB_CFB_COPYAREA - select FB_CFB_IMAGEBLIT - select FB_DEFERRED_IO select DMA_CMA if HAVE_DMA_CONTIGUOUS && CMA + select FB_IOMEM_HELPERS_DEFERRED select VIDEO_NOMODESET help This framebuffer driver supports Microsoft Hyper-V Synthetic Video. diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index b9965cbdd764..2e27c6bd8044 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -848,58 +848,38 @@ static int hvfb_blank(int blank, struct fb_info *info) return 1; /* get fb_blank to set the colormap to all black */ } -static void hvfb_cfb_fillrect(struct fb_info *p, - const struct fb_fillrect *rect) +static void hvfb_ops_damage_range(struct fb_info *info, off_t off, size_t len) { - struct hvfb_par *par = p->par; - - cfb_fillrect(p, rect); - if (par->synchronous_fb) - synthvid_update(p, 0, 0, INT_MAX, INT_MAX); - else - hvfb_ondemand_refresh_throttle(par, rect->dx, rect->dy, - rect->width, rect->height); + /* TODO: implement damage handling */ } -static void hvfb_cfb_copyarea(struct fb_info *p, - const struct fb_copyarea *area) +static void hvfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) { - struct hvfb_par *par = p->par; + struct hvfb_par *par = info->par; - cfb_copyarea(p, area); if (par->synchronous_fb) - synthvid_update(p, 0, 0, INT_MAX, INT_MAX); + synthvid_update(info, 0, 0, INT_MAX, INT_MAX); else - hvfb_ondemand_refresh_throttle(par, area->dx, area->dy, - area->width, area->height); + hvfb_ondemand_refresh_throttle(par, x, y, width, height); } -static void hvfb_cfb_imageblit(struct fb_info *p, - const struct fb_image *image) -{ - struct hvfb_par *par = p->par; - - cfb_imageblit(p, image); - if (par->synchronous_fb) - synthvid_update(p, 0, 0, INT_MAX, INT_MAX); - else - hvfb_ondemand_refresh_throttle(par, image->dx, image->dy, - image->width, image->height); -} +/* + * TODO: GEN1 codepaths allocate from system or DMA-able memory. Fix the + * driver to use the _SYSMEM_ or _DMAMEM_ helpers in these cases. + */ +FB_GEN_DEFAULT_DEFERRED_IOMEM_OPS(hvfb_ops, + hvfb_ops_damage_range, + hvfb_ops_damage_area) static const struct fb_ops hvfb_ops = { .owner = THIS_MODULE, + FB_DEFAULT_DEFERRED_OPS(hvfb_ops), .fb_check_var = hvfb_check_var, .fb_set_par = hvfb_set_par, .fb_setcolreg = hvfb_setcolreg, - .fb_fillrect = hvfb_cfb_fillrect, - .fb_copyarea = hvfb_cfb_copyarea, - .fb_imageblit = hvfb_cfb_imageblit, .fb_blank = hvfb_blank, - .fb_mmap = fb_deferred_io_mmap, }; - /* Get options from kernel paramenter "video=" */ static void hvfb_get_option(struct fb_info *info) { -- 2.41.0 ``` ## \[PATCH 8/8\] staging/fbtft: Use fb\_ops helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:35 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935605.md ```text Generate callback functions for struct fb_ops with the fbdev macro FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to the generated functions with an fbdev initializer macro. Signed-off-by: Thomas Zimmermann --- drivers/staging/fbtft/Kconfig | 6 +-- drivers/staging/fbtft/fbtft-core.c | 87 ++++++++---------------------- 2 files changed, 24 insertions(+), 69 deletions(-) diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig index 5dda3c65a38e..77ab44362f16 100644 --- a/drivers/staging/fbtft/Kconfig +++ b/drivers/staging/fbtft/Kconfig @@ -4,12 +4,8 @@ menuconfig FB_TFT depends on FB && SPI depends on FB_DEVICE depends on GPIOLIB || COMPILE_TEST - select FB_SYS_FILLRECT - select FB_SYS_COPYAREA - select FB_SYS_IMAGEBLIT - select FB_SYS_FOPS - select FB_DEFERRED_IO select FB_BACKLIGHT + select FB_SYSMEM_HELPERS_DEFERRED config FB_TFT_AGM1264K_FL tristate "FB driver for the AGM1264K-FL LCD display" diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index e4a77a4e7be6..3626f429b002 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -357,61 +357,6 @@ static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagereflis dirty_lines_start, dirty_lines_end); } -static void fbtft_fb_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct fbtft_par *par = info->par; - - dev_dbg(info->dev, - "%s: dx=%d, dy=%d, width=%d, height=%d\n", - __func__, rect->dx, rect->dy, rect->width, rect->height); - sys_fillrect(info, rect); - - par->fbtftops.mkdirty(info, rect->dy, rect->height); -} - -static void fbtft_fb_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - struct fbtft_par *par = info->par; - - dev_dbg(info->dev, - "%s: dx=%d, dy=%d, width=%d, height=%d\n", - __func__, area->dx, area->dy, area->width, area->height); - sys_copyarea(info, area); - - par->fbtftops.mkdirty(info, area->dy, area->height); -} - -static void fbtft_fb_imageblit(struct fb_info *info, - const struct fb_image *image) -{ - struct fbtft_par *par = info->par; - - dev_dbg(info->dev, - "%s: dx=%d, dy=%d, width=%d, height=%d\n", - __func__, image->dx, image->dy, image->width, image->height); - sys_imageblit(info, image); - - par->fbtftops.mkdirty(info, image->dy, image->height); -} - -static ssize_t fbtft_fb_write(struct fb_info *info, const char __user *buf, - size_t count, loff_t *ppos) -{ - struct fbtft_par *par = info->par; - ssize_t res; - - dev_dbg(info->dev, - "%s: count=%zd, ppos=%llu\n", __func__, count, *ppos); - res = fb_sys_write(info, buf, count, ppos); - - /* TODO: only mark changed area update all for now */ - par->fbtftops.mkdirty(info, -1, 0); - - return res; -} - /* from pxafb.c */ static unsigned int chan_to_field(unsigned int chan, struct fb_bitfield *bf) { @@ -473,16 +418,30 @@ static int fbtft_fb_blank(int blank, struct fb_info *info) return ret; } +static void fbtft_ops_damage_range(struct fb_info *info, off_t off, size_t len) +{ + struct fbtft_par *par = info->par; + + /* TODO: only mark changed area update all for now */ + par->fbtftops.mkdirty(info, -1, 0); +} + +static void fbtft_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) +{ + struct fbtft_par *par = info->par; + + par->fbtftops.mkdirty(info, y, height); +} + +FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(fbtft_ops, + fbtft_ops_damage_range, + fbtft_ops_damage_area) + static const struct fb_ops fbtft_ops = { - .owner = THIS_MODULE; - .fb_read = fb_sys_read; - .fb_write = fbtft_fb_write; - .fb_fillrect = fbtft_fb_fillrect; - .fb_copyarea = fbtft_fb_copyarea; - .fb_imageblit = fbtft_fb_imageblit; - .fb_setcolreg = fbtft_fb_setcolreg; - .fb_blank = fbtft_fb_blank; - .fb_mmap = fb_deferred_io_mmap; + .owner = THIS_MODULE, + FB_DEFAULT_DEFERRED_OPS(fbtft_ops), + .fb_setcolreg = fbtft_fb_setcolreg, + .fb_blank = fbtft_fb_blank, }; static void fbtft_merge_fbtftops(struct fbtft_ops *dst, struct fbtft_ops *src) -- 2.41.0 ``` ## \[PATCH 6/8\] hid/picolcd: Use fb\_ops helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:35 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935607.md ```text Generate callback functions for struct fb_ops with the fbdev macro FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to the generated functions with an fbdev initializer macro. Signed-off-by: Thomas Zimmermann Cc: Jiri Kosina Cc: Benjamin Tissoires Cc: "Bruno Prémont" --- drivers/hid/Kconfig | 6 +-- drivers/hid/hid-picolcd_fb.c | 73 ++++++++++-------------------------- 2 files changed, 20 insertions(+), 59 deletions(-) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index b50054a41c10..dbf632bb7e26 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -871,11 +871,7 @@ config HID_PICOLCD_FB default !EXPERT depends on HID_PICOLCD depends on HID_PICOLCD=FB || FB=y - select FB_DEFERRED_IO - select FB_SYS_FILLRECT - select FB_SYS_COPYAREA - select FB_SYS_IMAGEBLIT - select FB_SYS_FOPS + select FB_SYSMEM_HELPERS_DEFERRED help Provide access to PicoLCD's 256x64 monochrome display via a framebuffer device. diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c index d726aaafb146..a4dccdcda26f 100644 --- a/drivers/hid/hid-picolcd_fb.c +++ b/drivers/hid/hid-picolcd_fb.c @@ -283,54 +283,6 @@ static void picolcd_fb_update(struct fb_info *info) mutex_unlock(&info->lock); } -/* Stub to call the system default and update the image on the picoLCD */ -static void picolcd_fb_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - if (!info->par) - return; - sys_fillrect(info, rect); - - schedule_delayed_work(&info->deferred_work, 0); -} - -/* Stub to call the system default and update the image on the picoLCD */ -static void picolcd_fb_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - if (!info->par) - return; - sys_copyarea(info, area); - - schedule_delayed_work(&info->deferred_work, 0); -} - -/* Stub to call the system default and update the image on the picoLCD */ -static void picolcd_fb_imageblit(struct fb_info *info, const struct fb_image *image) -{ - if (!info->par) - return; - sys_imageblit(info, image); - - schedule_delayed_work(&info->deferred_work, 0); -} - -/* - * this is the slow path from userspace. they can seek and write to - * the fb. it's inefficient to do anything less than a full screen draw - */ -static ssize_t picolcd_fb_write(struct fb_info *info, const char __user *buf, - size_t count, loff_t *ppos) -{ - ssize_t ret; - if (!info->par) - return -ENODEV; - ret = fb_sys_write(info, buf, count, ppos); - if (ret >= 0) - schedule_delayed_work(&info->deferred_work, 0); - return ret; -} - static int picolcd_fb_blank(int blank, struct fb_info *info) { /* We let fb notification do this for us via lcd/backlight device */ @@ -417,18 +369,31 @@ static int picolcd_set_par(struct fb_info *info) return 0; } +static void picolcdfb_ops_damage_range(struct fb_info *info, off_t off, size_t len) +{ + if (!info->par) + return; + schedule_delayed_work(&info->deferred_work, 0); +} + +static void picolcdfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) +{ + if (!info->par) + return; + schedule_delayed_work(&info->deferred_work, 0); +} + +FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(picolcdfb_ops, + picolcdfb_ops_damage_range, + picolcdfb_ops_damage_area) + static const struct fb_ops picolcdfb_ops = { .owner = THIS_MODULE, + FB_DEFAULT_DEFERRED_OPS(picolcdfb_ops), .fb_destroy = picolcd_fb_destroy, - .fb_read = fb_sys_read, - .fb_write = picolcd_fb_write, .fb_blank = picolcd_fb_blank, - .fb_fillrect = picolcd_fb_fillrect, - .fb_copyarea = picolcd_fb_copyarea, - .fb_imageblit = picolcd_fb_imageblit, .fb_check_var = picolcd_fb_check_var, .fb_set_par = picolcd_set_par, - .fb_mmap = fb_deferred_io_mmap, }; -- 2.41.0 ``` ## \[PATCH 7/8\] staging/fbtft: Initialize fb\_op struct as static const - From: Thomas Zimmermann \ - Date: 2023-08-28 13:22:37 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935606.md ```text Replace dynamic allocation of the fb_ops instance with static allocation. Initialize the fields at module-load time. The owner field changes to THIS_MODULE, as in all other fbdev drivers. Signed-off-by: Thomas Zimmermann --- drivers/staging/fbtft/fbtft-core.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index eac1d570f437..e4a77a4e7be6 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -473,6 +473,18 @@ static int fbtft_fb_blank(int blank, struct fb_info *info) return ret; } +static const struct fb_ops fbtft_ops = { + .owner = THIS_MODULE; + .fb_read = fb_sys_read; + .fb_write = fbtft_fb_write; + .fb_fillrect = fbtft_fb_fillrect; + .fb_copyarea = fbtft_fb_copyarea; + .fb_imageblit = fbtft_fb_imageblit; + .fb_setcolreg = fbtft_fb_setcolreg; + .fb_blank = fbtft_fb_blank; + .fb_mmap = fb_deferred_io_mmap; +}; + static void fbtft_merge_fbtftops(struct fbtft_ops *dst, struct fbtft_ops *src) { if (src->write) @@ -521,7 +533,6 @@ static void fbtft_merge_fbtftops(struct fbtft_ops *dst, struct fbtft_ops *src) * Creates a new frame buffer info structure. * * Also creates and populates the following structures: - * info->fbops * info->fbdefio * info->pseudo_palette * par->fbtftops @@ -536,7 +547,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display, { struct fb_info *info; struct fbtft_par *par; - struct fb_ops *fbops = NULL; struct fb_deferred_io *fbdefio = NULL; u8 *vmem = NULL; void *txbuf = NULL; @@ -611,10 +621,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display, if (!vmem) goto alloc_fail; - fbops = devm_kzalloc(dev, sizeof(struct fb_ops), GFP_KERNEL); - if (!fbops) - goto alloc_fail; - fbdefio = devm_kzalloc(dev, sizeof(struct fb_deferred_io), GFP_KERNEL); if (!fbdefio) goto alloc_fail; @@ -638,19 +644,9 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display, goto alloc_fail; info->screen_buffer = vmem; - info->fbops = fbops; + info->fbops = &fbtft_ops; info->fbdefio = fbdefio; - fbops->owner = dev->driver->owner; - fbops->fb_read = fb_sys_read; - fbops->fb_write = fbtft_fb_write; - fbops->fb_fillrect = fbtft_fb_fillrect; - fbops->fb_copyarea = fbtft_fb_copyarea; - 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_pagereflist = true; fbdefio->deferred_io = fbtft_deferred_io; -- 2.41.0 ``` ## Re: \[PATCH 7/8\] staging/fbtft: Initialize fb\_op struct as static const - From: Greg KH \ - Date: 2023-08-28 14:52:13 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935608.md ```text On Mon, Aug 28, 2023 at 03:14:23PM +0200, Thomas Zimmermann wrote: > Replace dynamic allocation of the fb_ops instance with static > allocation. Initialize the fields at module-load time. The owner > field changes to THIS_MODULE, as in all other fbdev drivers. > > Signed-off-by: Thomas Zimmermann > --- > drivers/staging/fbtft/fbtft-core.c | 30 +++++++++++++----------------- > 1 file changed, 13 insertions(+), 17 deletions(-) > Acked-by: Greg Kroah-Hartman ``` ## Re: \[PATCH 8/8\] staging/fbtft: Use fb\_ops helpers for deferred I/O - From: Greg KH \ - Date: 2023-08-28 14:52:14 - Message: https://ratatoskr.run/linux-fbdev/2023/08/8935609.md ```text On Mon, Aug 28, 2023 at 03:14:24PM +0200, Thomas Zimmermann wrote: > Generate callback functions for struct fb_ops with the fbdev macro > FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to > the generated functions with an fbdev initializer macro. > > Signed-off-by: Thomas Zimmermann Acked-by: Greg Kroah-Hartman ``` ## Re: \[PATCH 1/8\] fbdev/smscufx: Use fb\_ops helpers for deferred I/O - From: Javier Martinez Canillas \ - Date: 2023-09-04 13:00:17 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935621.md ```text Thomas Zimmermann [ref] writes: Hello Thomas, > Generate callback functions for struct fb_ops with the fbdev macro > FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to > the generated functions with fbdev initializer macros. > > Signed-off-by: Thomas Zimmermann > Cc: Steve Glendinning > --- The patch looks good to me, but I've a question below. Acked-by: Javier Martinez Canillas > drivers/video/fbdev/smscufx.c | 85 +++++++++-------------------------- > 1 file changed, 22 insertions(+), 63 deletions(-) > > diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c [...] > static const struct fb_ops ufx_ops = { > .owner = THIS_MODULE, > - .fb_read = fb_sys_read, > - .fb_write = ufx_ops_write, > + __FB_DEFAULT_DEFERRED_OPS_RDWR(ufx_ops), > .fb_setcolreg = ufx_ops_setcolreg, > - .fb_fillrect = ufx_ops_fillrect, > - .fb_copyarea = ufx_ops_copyarea, > - .fb_imageblit = ufx_ops_imageblit, > + __FB_DEFAULT_DEFERRED_OPS_DRAW(ufx_ops), > .fb_mmap = ufx_ops_mmap, There are no generated functions for .fb_mmap, I wonder what's the value of __FB_DEFAULT_DEFERRED_OPS_MMAP() ? Maybe just removing that macro and setting .fb_mmap = fb_deferred_io_mmap instead if there's no custom mmap handler would be easier to read ? Alternatively, __FB_DEFAULT_DEFERRED_OPS_MMAP() could still be left but not taking a __prefix argument since that is not used anyways ? -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 2/8\] fbdev/udlfb: Use fb\_ops helpers for deferred I/O - From: Javier Martinez Canillas \ - Date: 2023-09-04 13:05:58 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935622.md ```text Thomas Zimmermann [ref] writes: > Generate callback functions for struct fb_ops with the fbdev macro > FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to > the generated functions with fbdev initializer macros. > > Signed-off-by: Thomas Zimmermann > Cc: Bernie Thompson > --- Acked-by: Javier Martinez Canillas [...] > +static void dlfb_ops_damage_range(struct fb_info *info, off_t off, size_t len) > +{ > + struct dlfb_data *dlfb = info->par; > + int start = max((int)(off / info->fix.line_length), 0); > + int lines = min((u32)((len / info->fix.line_length) + 1), (u32)info->var.yres); > + > + dlfb_handle_damage(dlfb, 0, start, info->var.xres, lines); > +} > + > +static void dlfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) > +{ > + struct dlfb_data *dlfb = info->par; > + > + dlfb_offload_damage(dlfb, x, y, width, height); > +} > + These two are very similar to the helpers you added for the smscufx driver in patch #1. I guess there's room for further consolidation as follow-up ? -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 3/8\] fbdev: Add Kconfig macro FB\_IOMEM\_HELPERS\_DEFERRED - From: Javier Martinez Canillas \ - Date: 2023-09-04 13:10:59 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935623.md ```text Thomas Zimmermann [ref] writes: > The new Kconfig macro FB_IOMEM_HELPERS_DEFERRED selects fbdev's > helpers for device I/O memory and deferred I/O. Drivers should > use it if they perform damage updates on device I/O memory. > > Signed-off-by: Thomas Zimmermann > --- Acked-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 4/8\] fbdev/hyperv\_fb: Use fb\_ops helpers for deferred I/O - From: Javier Martinez Canillas \ - Date: 2023-09-04 13:18:59 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935624.md ```text Thomas Zimmermann [ref] writes: > Generate callback functions for struct fb_ops with the fbdev macro > FB_GEN_DEFAULT_DEFERRED_IOMEM_OPS(). Initialize struct fb_ops to > the generated functions with fbdev initializer macros. > > The hyperv_fb driver is incomplete in its handling of deferred I/O > and damage framebuffers. Write operations do no trigger damage handling. > Fixing this is beyond the scope of this patch. > > Signed-off-by: Thomas Zimmermann Acked-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 5/8\] hid: Remove trailing whitespace - From: Javier Martinez Canillas \ - Date: 2023-09-04 13:25:48 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935625.md ```text Thomas Zimmermann [ref] writes: > Fix coding style in Kconfig. No functional changes. > > Signed-off-by: Thomas Zimmermann > Cc: Jiri Kosina > Cc: Benjamin Tissoires > --- Acked-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 6/8\] hid/picolcd: Use fb\_ops helpers for deferred I/O - From: Javier Martinez Canillas \ - Date: 2023-09-04 13:28:17 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935626.md ```text Thomas Zimmermann [ref] writes: > Generate callback functions for struct fb_ops with the fbdev macro > FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to > the generated functions with an fbdev initializer macro. > > Signed-off-by: Thomas Zimmermann > Cc: Jiri Kosina > Cc: Benjamin Tissoires > Cc: "Bruno Prémont" > --- Acked-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 7/8\] staging/fbtft: Initialize fb\_op struct as static const - From: Javier Martinez Canillas \ - Date: 2023-09-04 13:28:54 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935627.md ```text Thomas Zimmermann [ref] writes: > Replace dynamic allocation of the fb_ops instance with static > allocation. Initialize the fields at module-load time. The owner > field changes to THIS_MODULE, as in all other fbdev drivers. > > Signed-off-by: Thomas Zimmermann > --- Acked-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 8/8\] staging/fbtft: Use fb\_ops helpers for deferred I/O - From: Javier Martinez Canillas \ - Date: 2023-09-04 13:29:44 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935628.md ```text Thomas Zimmermann [ref] writes: > Generate callback functions for struct fb_ops with the fbdev macro > FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to > the generated functions with an fbdev initializer macro. > > Signed-off-by: Thomas Zimmermann > --- Acked-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 1/8\] fbdev/smscufx: Use fb\_ops helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-09-04 14:39:54 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935629.md ```text Hi Javier Am 04.09.23 um 14:59 schrieb Javier Martinez Canillas: > Thomas Zimmermann [ref] writes: > > Hello Thomas, > >> Generate callback functions for struct fb_ops with the fbdev macro >> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to >> the generated functions with fbdev initializer macros. >> >> Signed-off-by: Thomas Zimmermann >> Cc: Steve Glendinning >> --- > > The patch looks good to me, but I've a question below. > > Acked-by: Javier Martinez Canillas > >> drivers/video/fbdev/smscufx.c | 85 +++++++++-------------------------- >> 1 file changed, 22 insertions(+), 63 deletions(-) >> >> diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c > > [...] > >> static const struct fb_ops ufx_ops = { >> .owner = THIS_MODULE, >> - .fb_read = fb_sys_read, >> - .fb_write = ufx_ops_write, >> + __FB_DEFAULT_DEFERRED_OPS_RDWR(ufx_ops), >> .fb_setcolreg = ufx_ops_setcolreg, >> - .fb_fillrect = ufx_ops_fillrect, >> - .fb_copyarea = ufx_ops_copyarea, >> - .fb_imageblit = ufx_ops_imageblit, >> + __FB_DEFAULT_DEFERRED_OPS_DRAW(ufx_ops), >> .fb_mmap = ufx_ops_mmap, > > There are no generated functions for .fb_mmap, I wonder what's the value > of __FB_DEFAULT_DEFERRED_OPS_MMAP() ? Maybe just removing that macro and > setting .fb_mmap = fb_deferred_io_mmap instead if there's no custom mmap > handler would be easier to read ? At least two drivers could use __FB_DEFAULT_DEFERRED_OPS_MMAP: picolcd-fb and hyperv_fb. At some point, we might want to set/clear fb_mmap depending on some Kconfig value. Having __FB_DEFAULT_DEFERRED_OPS_MMAP might be helpful then. > > Alternatively, __FB_DEFAULT_DEFERRED_OPS_MMAP() could still be left but > not taking a __prefix argument since that is not used anyways ? The driver optionally provides mmap without deferred I/O, hence the mmap function. That makes no sense, as these writes to the buffer would never make it to the device memory. But I didn't want to remove the code either. So I just left the existing function as-is. Usually, the deferred-I/O mmap is called immediately. [1] Best regards Thomas [1] https://elixir.bootlin.com/linux/v6.5.1/source/drivers/video/fbdev/smscufx.c#L784 > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) ``` ## Re: \[PATCH 2/8\] fbdev/udlfb: Use fb\_ops helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-09-04 14:43:12 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935630.md ```text Am 04.09.23 um 15:05 schrieb Javier Martinez Canillas: > Thomas Zimmermann [ref] writes: > >> Generate callback functions for struct fb_ops with the fbdev macro >> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to >> the generated functions with fbdev initializer macros. >> >> Signed-off-by: Thomas Zimmermann >> Cc: Bernie Thompson >> --- > > Acked-by: Javier Martinez Canillas > > [...] > >> +static void dlfb_ops_damage_range(struct fb_info *info, off_t off, size_t len) >> +{ >> + struct dlfb_data *dlfb = info->par; >> + int start = max((int)(off / info->fix.line_length), 0); >> + int lines = min((u32)((len / info->fix.line_length) + 1), (u32)info->var.yres); >> + >> + dlfb_handle_damage(dlfb, 0, start, info->var.xres, lines); >> +} >> + >> +static void dlfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) >> +{ >> + struct dlfb_data *dlfb = info->par; >> + >> + dlfb_offload_damage(dlfb, x, y, width, height); >> +} >> + > > These two are very similar to the helpers you added for the smscufx driver > in patch #1. I guess there's room for further consolidation as follow-up ? Maybe. I had patches that take the rectangle computation from [1] and turn it into a helper for these USB drivers. But it's an unrelated change, so I dropped them from this patchset. Best regards Thomas [1] https://elixir.bootlin.com/linux/v6.5.1/source/drivers/gpu/drm/drm_fb_helper.c#L641 > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) ``` ## Re: \[PATCH 1/8\] fbdev/smscufx: Use fb\_ops helpers for deferred I/O - From: Thomas Zimmermann \ - Date: 2023-09-04 14:45:28 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935631.md ```text Am 04.09.23 um 16:39 schrieb Thomas Zimmermann: [...] > At least two drivers could use __FB_DEFAULT_DEFERRED_OPS_MMAP: picolcd-fb and hyperv_fb. At some point, we might want to set/clear Both drivers are already in this patchset. > fb_mmap depending on some Kconfig value. Having __FB_DEFAULT_DEFERRED_OPS_MMAP might be helpful then. > >> >> Alternatively, __FB_DEFAULT_DEFERRED_OPS_MMAP() could still be left but >> not taking a __prefix argument since that is not used anyways ? > > The driver optionally provides mmap without deferred I/O, hence the mmap function. That makes no sense, as these writes to the buffer would never make it to the device memory. But I didn't want to remove the code either. So I just left the existing function as-is. Usually, the deferred-I/O mmap is called immediately. [1] > > Best regards > Thomas > > [1] https://elixir.bootlin.com/linux/v6.5.1/source/drivers/video/fbdev/smscufx.c#L784 > >> > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) ``` ## Re: \[PATCH 1/8\] fbdev/smscufx: Use fb\_ops helpers for deferred I/O - From: Javier Martinez Canillas \ - Date: 2023-09-04 15:27:21 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935632.md ```text Thomas Zimmermann [ref] writes: > Hi Javier > > Am 04.09.23 um 14:59 schrieb Javier Martinez Canillas: >> Thomas Zimmermann [ref] writes: >> >> Hello Thomas, >> >>> Generate callback functions for struct fb_ops with the fbdev macro >>> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to >>> the generated functions with fbdev initializer macros. >>> >>> Signed-off-by: Thomas Zimmermann >>> Cc: Steve Glendinning >>> --- >> >> The patch looks good to me, but I've a question below. >> >> Acked-by: Javier Martinez Canillas >> >>> drivers/video/fbdev/smscufx.c | 85 +++++++++-------------------------- >>> 1 file changed, 22 insertions(+), 63 deletions(-) >>> >>> diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c >> >> [...] >> >>> static const struct fb_ops ufx_ops = { >>> .owner = THIS_MODULE, >>> - .fb_read = fb_sys_read, >>> - .fb_write = ufx_ops_write, >>> + __FB_DEFAULT_DEFERRED_OPS_RDWR(ufx_ops), >>> .fb_setcolreg = ufx_ops_setcolreg, >>> - .fb_fillrect = ufx_ops_fillrect, >>> - .fb_copyarea = ufx_ops_copyarea, >>> - .fb_imageblit = ufx_ops_imageblit, >>> + __FB_DEFAULT_DEFERRED_OPS_DRAW(ufx_ops), >>> .fb_mmap = ufx_ops_mmap, >> >> There are no generated functions for .fb_mmap, I wonder what's the value >> of __FB_DEFAULT_DEFERRED_OPS_MMAP() ? Maybe just removing that macro and >> setting .fb_mmap = fb_deferred_io_mmap instead if there's no custom mmap >> handler would be easier to read ? > > At least two drivers could use __FB_DEFAULT_DEFERRED_OPS_MMAP: > picolcd-fb and hyperv_fb. At some point, we might want to set/clear > fb_mmap depending on some Kconfig value. Having > __FB_DEFAULT_DEFERRED_OPS_MMAP might be helpful then. > Got it, thanks for the explanation. >> >> Alternatively, __FB_DEFAULT_DEFERRED_OPS_MMAP() could still be left but >> not taking a __prefix argument since that is not used anyways ? > > The driver optionally provides mmap without deferred I/O, hence the mmap > function. That makes no sense, as these writes to the buffer would never > make it to the device memory. But I didn't want to remove the code > either. So I just left the existing function as-is. Usually, the > deferred-I/O mmap is called immediately. [1] > Makes sense. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ``` ## Re: \[PATCH 2/8\] fbdev/udlfb: Use fb\_ops helpers for deferred I/O - From: Javier Martinez Canillas \ - Date: 2023-09-04 15:29:11 - Message: https://ratatoskr.run/linux-fbdev/2023/09/8935633.md ```text Thomas Zimmermann [ref] writes: > Am 04.09.23 um 15:05 schrieb Javier Martinez Canillas: >> Thomas Zimmermann [ref] writes: >> >>> Generate callback functions for struct fb_ops with the fbdev macro >>> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(). Initialize struct fb_ops to >>> the generated functions with fbdev initializer macros. >>> >>> Signed-off-by: Thomas Zimmermann >>> Cc: Bernie Thompson >>> --- >> >> Acked-by: Javier Martinez Canillas >> >> [...] >> >>> +static void dlfb_ops_damage_range(struct fb_info *info, off_t off, size_t len) >>> +{ >>> + struct dlfb_data *dlfb = info->par; >>> + int start = max((int)(off / info->fix.line_length), 0); >>> + int lines = min((u32)((len / info->fix.line_length) + 1), (u32)info->var.yres); >>> + >>> + dlfb_handle_damage(dlfb, 0, start, info->var.xres, lines); >>> +} >>> + >>> +static void dlfb_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height) >>> +{ >>> + struct dlfb_data *dlfb = info->par; >>> + >>> + dlfb_offload_damage(dlfb, x, y, width, height); >>> +} >>> + >> >> These two are very similar to the helpers you added for the smscufx driver >> in patch #1. I guess there's room for further consolidation as follow-up ? > > Maybe. I had patches that take the rectangle computation from [1] and > turn it into a helper for these USB drivers. But it's an unrelated > change, so I dropped them from this patchset. > Great and yes, I meant as separate patch-set, not as a part of this one. > Best regards > Thomas > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat ```