Re: [v4,02/13] fbdev: Add initializer macros for struct fb_ops

[Thomas Zimmermann <tzimmermann@suse.de>]

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

Hi

Am 24.05.23 um 22:57 schrieb Sui Jingfeng:
> Hi,
> 
> 
> we love your patch:
> 
> 
> On 2023/5/24 17:21, Thomas Zimmermann wrote:
>> For framebuffers in I/O and system memory, add macros that set
>> struct fb_ops to the respective callback functions.
>>
>> For deferred I/O, add macros that generate callback functions with
>> damage handling. Add initializer macros that set struct fb_ops to
>> the generated callbacks.
>>
>> These macros can remove a lot boilerplate code from fbdev drivers.
>> The drivers are supposed to use the macro that is required for its
>> framebuffer. Each macro is split into smaller helpers, so that
>> drivers with non-standard callbacks can pick and customize callbacks
>> as needed. There are individual helper macros for read/write, mmap
>> and drawing.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   include/linux/fb.h | 112 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 112 insertions(+)
>>
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index 2cf8efcb9e32..731472a2bb62 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -538,9 +538,31 @@ extern ssize_t fb_io_read(struct fb_info *info, 
>> char __user *buf,
>>   extern ssize_t fb_io_write(struct fb_info *info, const char __user 
>> *buf,
>>                  size_t count, loff_t *ppos);
>> +/*
>> + * Initializes struct fb_ops for framebuffers in I/O memory.
>> + */
>> +
>> +#define __FB_DEFAULT_IO_OPS_RDWR \
>> +    .fb_read    = fb_io_read, \
>> +    .fb_write    = fb_io_write
>> +
>> +#define __FB_DEFAULT_IO_OPS_DRAW \
>> +        .fb_fillrect    = cfb_fillrect, \
>> +        .fb_copyarea    = cfb_copyarea, \
>> +        .fb_imageblit    = cfb_imageblit
> 
> Here,  it seems that your text editor replace the tap with space, but 
> I'm OK.
> 
> I'm asking because I see other __FB__DEFAULT_* macro begin with tabs.

Yeah, these are mistakes. I'll fix that with the next version.

Best regards
Thomas

> 
>> +#define __FB_DEFAULT_IO_OPS_MMAP \
>> +    .fb_mmap    = NULL // default implementation
>> +
>> +#define FB_DEFAULT_IO_OPS \
>> +    __FB_DEFAULT_IO_OPS_RDWR, \
>> +    __FB_DEFAULT_IO_OPS_DRAW, \
>> +    __FB_DEFAULT_IO_OPS_MMAP
>> +
>>   /*
>>    * Drawing operations where framebuffer is in system RAM
>>    */
>> +
>>   extern void sys_fillrect(struct fb_info *info, const struct 
>> fb_fillrect *rect);
>>   extern void sys_copyarea(struct fb_info *info, const struct 
>> fb_copyarea *area);
>>   extern void sys_imageblit(struct fb_info *info, const struct 
>> fb_image *image);
>> @@ -549,6 +571,27 @@ extern ssize_t fb_sys_read(struct fb_info *info, 
>> char __user *buf,
>>   extern ssize_t fb_sys_write(struct fb_info *info, const char __user 
>> *buf,
>>                   size_t count, loff_t *ppos);
>> +/*
>> + * Initializes struct fb_ops for framebuffers in system memory.
>> + */
>> +
>> +#define __FB_DEFAULT_SYS_OPS_RDWR \
>> +    .fb_read    = fb_sys_read, \
>> +    .fb_write    = fb_sys_write
>> +
>> +#define __FB_DEFAULT_SYS_OPS_DRAW \
>> +        .fb_fillrect    = sys_fillrect, \
>> +        .fb_copyarea    = sys_copyarea, \
>> +        .fb_imageblit    = sys_imageblit
>> +
>> +#define __FB_DEFAULT_SYS_OPS_MMAP \
>> +    .fb_mmap    = NULL // default implementation
>> +
>> +#define FB_DEFAULT_SYS_OPS \
>> +    __FB_DEFAULT_SYS_OPS_RDWR, \
>> +    __FB_DEFAULT_SYS_OPS_DRAW, \
>> +    __FB_DEFAULT_SYS_OPS_MMAP
>> +
>>   /* drivers/video/fbmem.c */
>>   extern int register_framebuffer(struct fb_info *fb_info);
>>   extern void unregister_framebuffer(struct fb_info *fb_info);
>> @@ -604,6 +647,75 @@ extern void fb_deferred_io_cleanup(struct fb_info 
>> *info);
>>   extern int fb_deferred_io_fsync(struct file *file, loff_t start,
>>                   loff_t end, int datasync);
>> +/*
>> + * Generate callbacks for deferred I/O
>> + */
>> +
>> +#define __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, 
>> __mode) \
>> +    static ssize_t __prefix ## _defio_read(struct fb_info *info, char 
>> __user *buf, \
>> +                           size_t count, loff_t *ppos) \
>> +    { \
>> +        return fb_ ## __mode ## _read(info, buf, count, ppos); \
>> +    } \
>> +    static ssize_t __prefix ## _defio_write(struct fb_info *info, 
>> const char __user *buf, \
>> +                        size_t count, loff_t *ppos) \
>> +    { \
>> +        unsigned long offset = *ppos; \
>> +        ssize_t ret = fb_ ## __mode ## _write(info, buf, count, ppos); \
>> +        if (ret > 0) \
>> +            __damage_range(info, offset, ret); \
>> +        return ret; \
>> +    }
>> +
>> +#define __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, 
>> __mode) \
>> +    static void __prefix ## _defio_fillrect(struct fb_info *info, \
>> +                        const struct fb_fillrect *rect) \
>> +    { \
>> +        __mode ## _fillrect(info, rect); \
>> +        __damage_area(info, rect->dx, rect->dy, rect->width, 
>> rect->height); \
>> +    } \
>> +    static void __prefix ## _defio_copyarea(struct fb_info *info, \
>> +                        const struct fb_copyarea *area) \
>> +    { \
>> +        __mode ## _copyarea(info, area); \
>> +        __damage_area(info, area->dx, area->dy, area->width, 
>> area->height); \
>> +    } \
>> +    static void __prefix ## _defio_imageblit(struct fb_info *info, \
>> +                         const struct fb_image *image) \
>> +    { \
>> +        __mode ## _imageblit(info, image); \
>> +        __damage_area(info, image->dx, image->dy, image->width, 
>> image->height); \
>> +    }
>> +
>> +#define FB_GEN_DEFAULT_DEFERRED_IO_OPS(__prefix, __damage_range, 
>> __damage_area) \
>> +    __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, io) \
>> +    __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, cfb)
>> +
>> +#define FB_GEN_DEFAULT_DEFERRED_SYS_OPS(__prefix, __damage_range, 
>> __damage_area) \
>> +    __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, sys) \
>> +    __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, sys)
>> +
>> +/*
>> + * Initializes struct fb_ops for deferred I/O.
>> + */
>> +
>> +#define __FB_DEFAULT_DEFERRED_OPS_RDWR(__prefix) \
>> +    .fb_read    = __prefix ## _defio_read, \
>> +    .fb_write    = __prefix ## _defio_write
>> +
>> +#define __FB_DEFAULT_DEFERRED_OPS_DRAW(__prefix) \
>> +        .fb_fillrect    = __prefix ## _defio_fillrect, \
>> +        .fb_copyarea    = __prefix ## _defio_copyarea, \
>> +        .fb_imageblit    = __prefix ## _defio_imageblit
> 
> Here also,  '.fb_fillrect', '.fb_copyarea' and '.fb_imageblit' begin 
> with space, but I'm OK.
> 
> I'm asking because I see other __FB__DEFAULT_* macro begin with tabs.
> 
>> +#define __FB_DEFAULT_DEFERRED_OPS_MMAP(__prefix) \
>> +    .fb_mmap    = fb_deferred_io_mmap
>> +
>> +#define FB_DEFAULT_DEFERRED_OPS(__prefix) \
>> +    __FB_DEFAULT_DEFERRED_OPS_RDWR(__prefix), \
>> +    __FB_DEFAULT_DEFERRED_OPS_DRAW(__prefix), \
>> +    __FB_DEFAULT_DEFERRED_OPS_MMAP(__prefix)
>> +
>>   static inline bool fb_be_math(struct fb_info *info)
>>   {
>>   #ifdef CONFIG_FB_FOREIGN_ENDIAN

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]