Re: [PATCH 01/10] fbdev: Add fb_ops init macros for framebuffers in DMA-able memory

Javier Martinez Canillas
2023-07-05T08:24:12Z

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Add initializer macros for struct fb_ops for framebuffers in DMA-able
> memory areas. Also add a corresponding Kconfig token. As of now, this
> is equivalent to system framebuffers and mostly useful for labeling
> drivers correctly.
>
> A later patch may add a generic DMA-specific mmap operation. Linux
> offers a number of dma_mmap_*() helpers for different use cases.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Helge Deller <deller@gmx.de>
> ---
> drivers/video/fbdev/Kconfig | 8 ++++++++
> include/linux/fb.h | 13 +++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index cecf15418632..f14229757311 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -168,6 +168,14 @@ config FB_DEFERRED_IO
> bool
> depends on FB
>
> +config FB_DMA_HELPERS
> + bool
> + depends on FB
> + select FB_SYS_COPYAREA
> + select FB_SYS_FILLRECT
> + select FB_SYS_FOPS
> + select FB_SYS_IMAGEBLIT
> +
> config FB_IO_HELPERS
> bool
> depends on FB
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 1d5c13f34b09..1191a78c5289 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -594,6 +594,19 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
> __FB_DEFAULT_SYS_OPS_DRAW, \
> __FB_DEFAULT_SYS_OPS_MMAP
>
> +/*
> + * Helpers for framebuffers in DMA-able memory
> + */
> +

The comment for I/O memory helpers says:

/*
* Initializes struct fb_ops for framebuffers in I/O memory.
*/

I think that would be good to have consistency between these two,
so something like:

/*
* Initializes struct fb_ops for framebuffers in DMA-able memory.
*/

> +#define __FB_DEFAULT_DMA_OPS_RDWR \
> + .fb_read = fb_sys_read, \
> + .fb_write = fb_sys_write
> +
> +#define __FB_DEFAULT_DMA_OPS_DRAW \
> + .fb_fillrect = sys_fillrect, \
> + .fb_copyarea = sys_copyarea, \
> + .fb_imageblit = sys_imageblit
> +

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

--
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat