Hi Javier Am 04.09.23 um 14:59 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmermann@xxxxxxx> 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@xxxxxxx> Cc: Steve Glendinning <steve.glendinning@xxxxxxxxxxx> ---The patch looks good to me, but I've a question below. Acked-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>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)
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
