From: Thomas Zimmermann <tzimmermann@suse.de> Date: 2023-08-28 13:22:32
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
@@ -715,68 +715,6 @@ static void dlfb_offload_damage(struct dlfb_data *dlfb, int x, int y, int width,schedule_work(&dlfb->damage_work);}-/*-*Pathtriggeredbyusermodeclientswhowritetofilesystem-*e.g.catfilename>/dev/fb1-*NotusedbyXWindowsortext-modeconsole.Butusefulfortesting.-*Slowbecauseofextracopyandwemustassumeallpixelsdirty.-*/-staticssize_tdlfb_ops_write(structfb_info*info,constchar__user*buf,-size_tcount,loff_t*ppos)-{-ssize_tresult;-structdlfb_data*dlfb=info->par;-u32offset=(u32)*ppos;--result=fb_sys_write(info,buf,count,ppos);--if(result>0){-intstart=max((int)(offset/info->fix.line_length),0);-intlines=min((u32)((result/info->fix.line_length)+1),-(u32)info->var.yres);--dlfb_handle_damage(dlfb,0,start,info->var.xres,-lines);-}--returnresult;-}--/* hardware has native COPY command (see libdlo), but not worth it for fbcon */-staticvoiddlfb_ops_copyarea(structfb_info*info,-conststructfb_copyarea*area)-{--structdlfb_data*dlfb=info->par;--sys_copyarea(info,area);--dlfb_offload_damage(dlfb,area->dx,area->dy,-area->width,area->height);-}--staticvoiddlfb_ops_imageblit(structfb_info*info,-conststructfb_image*image)-{-structdlfb_data*dlfb=info->par;--sys_imageblit(info,image);--dlfb_offload_damage(dlfb,image->dx,image->dy,-image->width,image->height);-}--staticvoiddlfb_ops_fillrect(structfb_info*info,-conststructfb_fillrect*rect)-{-structdlfb_data*dlfb=info->par;--sys_fillrect(info,rect);--dlfb_offload_damage(dlfb,rect->dx,rect->dy,rect->width,-rect->height);-}-/**NOTE:fb_defio.cisholdinginfo->fbdefio.mutex*TouchingANYframebuffermemorythattriggersapagefault
@@ -894,64 +894,6 @@ static int ufx_handle_damage(struct ufx_data *dev, int x, int y,return0;}-/* Path triggered by usermode clients who write to filesystem-*e.g.catfilename>/dev/fb1-*NotusedbyXWindowsortext-modeconsole.Butusefulfortesting.-*Slowbecauseofextracopyandwemustassumeallpixelsdirty.*/-staticssize_tufx_ops_write(structfb_info*info,constchar__user*buf,-size_tcount,loff_t*ppos)-{-ssize_tresult;-structufx_data*dev=info->par;-u32offset=(u32)*ppos;--result=fb_sys_write(info,buf,count,ppos);--if(result>0){-intstart=max((int)(offset/info->fix.line_length),0);-intlines=min((u32)((result/info->fix.line_length)+1),-(u32)info->var.yres);--ufx_handle_damage(dev,0,start,info->var.xres,lines);-}--returnresult;-}--staticvoidufx_ops_copyarea(structfb_info*info,-conststructfb_copyarea*area)-{--structufx_data*dev=info->par;--sys_copyarea(info,area);--ufx_handle_damage(dev,area->dx,area->dy,-area->width,area->height);-}--staticvoidufx_ops_imageblit(structfb_info*info,-conststructfb_image*image)-{-structufx_data*dev=info->par;--sys_imageblit(info,image);--ufx_handle_damage(dev,image->dx,image->dy,-image->width,image->height);-}--staticvoidufx_ops_fillrect(structfb_info*info,-conststructfb_fillrect*rect)-{-structufx_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*TouchingANYframebuffermemorythattriggersapagefault*infb_defiowillcauseadeadlock,whenitalsotriesto
From: Thomas Zimmermann <tzimmermann@suse.de> Date: 2023-08-28 13:22:34
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 <tzimmermann@suse.de>
---
drivers/video/fbdev/core/Kconfig | 6 ++++++
1 file changed, 6 insertions(+)
From: Thomas Zimmermann <tzimmermann@suse.de> Date: 2023-08-28 13:22:34
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 <tzimmermann@suse.de>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Dexuan Cui <decui@microsoft.com>
---
drivers/video/fbdev/Kconfig | 5 +---
drivers/video/fbdev/hyperv_fb.c | 48 ++++++++++-----------------------
2 files changed, 15 insertions(+), 38 deletions(-)
@@ -848,58 +848,38 @@ static int hvfb_blank(int blank, struct fb_info *info)return1;/* get fb_blank to set the colormap to all black */}-staticvoidhvfb_cfb_fillrect(structfb_info*p,-conststructfb_fillrect*rect)+staticvoidhvfb_ops_damage_range(structfb_info*info,off_toff,size_tlen){-structhvfb_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 */}-staticvoidhvfb_cfb_copyarea(structfb_info*p,-conststructfb_copyarea*area)+staticvoidhvfb_ops_damage_area(structfb_info*info,u32x,u32y,u32width,u32height){-structhvfb_par*par=p->par;+structhvfb_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);}-staticvoidhvfb_cfb_imageblit(structfb_info*p,-conststructfb_image*image)-{-structhvfb_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:GEN1codepathsallocatefromsystemorDMA-ablememory.Fixthe+*drivertousethe_SYSMEM_or_DMAMEM_helpersinthesecases.+*/+FB_GEN_DEFAULT_DEFERRED_IOMEM_OPS(hvfb_ops,+hvfb_ops_damage_range,+hvfb_ops_damage_area)staticconststructfb_opshvfb_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=" */staticvoidhvfb_get_option(structfb_info*info){
@@ -357,61 +357,6 @@ static void fbtft_deferred_io(struct fb_info *info, struct list_head *pagereflisdirty_lines_start,dirty_lines_end);}-staticvoidfbtft_fb_fillrect(structfb_info*info,-conststructfb_fillrect*rect)-{-structfbtft_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);-}--staticvoidfbtft_fb_copyarea(structfb_info*info,-conststructfb_copyarea*area)-{-structfbtft_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);-}--staticvoidfbtft_fb_imageblit(structfb_info*info,-conststructfb_image*image)-{-structfbtft_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);-}--staticssize_tfbtft_fb_write(structfb_info*info,constchar__user*buf,-size_tcount,loff_t*ppos)-{-structfbtft_par*par=info->par;-ssize_tres;--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);--returnres;-}-/* from pxafb.c */staticunsignedintchan_to_field(unsignedintchan,structfb_bitfield*bf){
@@ -473,16 +418,30 @@ static int fbtft_fb_blank(int blank, struct fb_info *info)returnret;}+staticvoidfbtft_ops_damage_range(structfb_info*info,off_toff,size_tlen)+{+structfbtft_par*par=info->par;++/* TODO: only mark changed area update all for now */+par->fbtftops.mkdirty(info,-1,0);+}++staticvoidfbtft_ops_damage_area(structfb_info*info,u32x,u32y,u32width,u32height)+{+structfbtft_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)+staticconststructfb_opsfbtft_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,};staticvoidfbtft_merge_fbtftops(structfbtft_ops*dst,structfbtft_ops*src)
@@ -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 */-staticvoidpicolcd_fb_fillrect(structfb_info*info,-conststructfb_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 */-staticvoidpicolcd_fb_copyarea(structfb_info*info,-conststructfb_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 */-staticvoidpicolcd_fb_imageblit(structfb_info*info,conststructfb_image*image)-{-if(!info->par)-return;-sys_imageblit(info,image);--schedule_delayed_work(&info->deferred_work,0);-}--/*-*thisistheslowpathfromuserspace.theycanseekandwriteto-*thefb.it'sinefficienttodoanythinglessthanafullscreendraw-*/-staticssize_tpicolcd_fb_write(structfb_info*info,constchar__user*buf,-size_tcount,loff_t*ppos)-{-ssize_tret;-if(!info->par)-return-ENODEV;-ret=fb_sys_write(info,buf,count,ppos);-if(ret>=0)-schedule_delayed_work(&info->deferred_work,0);-returnret;-}-staticintpicolcd_fb_blank(intblank,structfb_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)return0;}+staticvoidpicolcdfb_ops_damage_range(structfb_info*info,off_toff,size_tlen)+{+if(!info->par)+return;+schedule_delayed_work(&info->deferred_work,0);+}++staticvoidpicolcdfb_ops_damage_area(structfb_info*info,u32x,u32y,u32width,u32height)+{+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)+staticconststructfb_opspicolcdfb_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,};
From: Thomas Zimmermann <tzimmermann@suse.de> Date: 2023-08-28 13:22:37
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 <tzimmermann@suse.de>
---
drivers/staging/fbtft/fbtft-core.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
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 <tzimmermann@suse.de>
---
drivers/staging/fbtft/fbtft-core.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
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 <tzimmermann@suse.de>
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 13:00:17
Thomas Zimmermann [off-list 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 <tzimmermann@suse.de>
Cc: Steve Glendinning <steve.glendinning@shawell.net>
---
The patch looks good to me, but I've a question below.
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
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
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 13:05:58
Thomas Zimmermann [off-list 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 <tzimmermann@suse.de>
Cc: Bernie Thompson <bernie@plugable.com>
---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
[...]
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
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 13:10:59
Thomas Zimmermann [off-list 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 <tzimmermann@suse.de>
---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 13:18:59
Thomas Zimmermann [off-list 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 <tzimmermann@suse.de>
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 13:28:17
Thomas Zimmermann [off-list 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 <tzimmermann@suse.de>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <redacted>
Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 13:28:54
Thomas Zimmermann [off-list 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 <tzimmermann@suse.de>
---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 13:29:44
Thomas Zimmermann [off-list 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 <tzimmermann@suse.de>
---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
From: Thomas Zimmermann <tzimmermann@suse.de> Date: 2023-09-04 14:39:54
Hi Javier
Am 04.09.23 um 14:59 schrieb Javier Martinez Canillas:
Thomas Zimmermann [off-list ref] writes:
Hello Thomas,
quoted
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 <tzimmermann@suse.de>
Cc: Steve Glendinning <steve.glendinning@shawell.net>
---
The patch looks good to me, but I've a question below.
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
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)
From: Thomas Zimmermann <tzimmermann@suse.de> Date: 2023-09-04 14:43:12
Am 04.09.23 um 15:05 schrieb Javier Martinez Canillas:
Thomas Zimmermann [off-list ref] writes:
quoted
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 <tzimmermann@suse.de>
Cc: Bernie Thompson <bernie@plugable.com>
---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
[...]
From: Thomas Zimmermann <tzimmermann@suse.de> Date: 2023-09-04 14:45:28
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.
quoted
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
quoted
--
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)
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 15:27:21
Thomas Zimmermann [off-list ref] writes:
Hi Javier
Am 04.09.23 um 14:59 schrieb Javier Martinez Canillas:
quoted
Thomas Zimmermann [off-list ref] writes:
Hello Thomas,
quoted
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 <tzimmermann@suse.de>
Cc: Steve Glendinning <steve.glendinning@shawell.net>
---
The patch looks good to me, but I've a question below.
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
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.
quoted
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
From: Javier Martinez Canillas <javierm@redhat.com> Date: 2023-09-04 15:29:11
Thomas Zimmermann [off-list ref] writes:
Am 04.09.23 um 15:05 schrieb Javier Martinez Canillas:
quoted
Thomas Zimmermann [off-list ref] writes:
quoted
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 <tzimmermann@suse.de>
Cc: Bernie Thompson <bernie@plugable.com>
---
Acked-by: Javier Martinez Canillas <javierm@redhat.com>
[...]
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