Re: [PATCH 5/6] fbdev: Move CFB read and write code into helper functions

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Thomas,

On Fri, Apr 28, 2023 at 1:20 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Am 26.04.23 um 17:21 schrieb Geert Uytterhoeven:
> > On Wed, Apr 26, 2023 at 5:06 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >> Am 26.04.23 um 17:01 schrieb Geert Uytterhoeven:
> >>> On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >>>> Move the existing CFB read and write code for I/O memory into
> >>>> the new helpers fb_cfb_read() and fb_cfb_write(). Make them the
> >>>> default fp_ops. No functional changes.
> >>>>
> >>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >>>> ---
> >>>>    drivers/video/fbdev/core/Makefile      |   2 +-
> >>>>    drivers/video/fbdev/core/fb_cfb_fops.c | 126 +++++++++++++++++++++++++
> >>>>    drivers/video/fbdev/core/fbmem.c       | 113 +---------------------
> >>>>    include/linux/fb.h                     |  10 ++
> >>>>    4 files changed, 139 insertions(+), 112 deletions(-)
> >>>>    create mode 100644 drivers/video/fbdev/core/fb_cfb_fops.c
> >>>
> >>> While the general idea is fine, please do not call any of this "cfb",
> >>> as it is not related to chunky color frame buffer formats.
> >>> All of these operate on the raw frame buffer contents.
> >>
> >> Shall I call it fb_raw_() or fb_io_()?
> >
> > Given fb_memcpy_fromfb() is mapped to memcpy_fromio() on
> > most architectures, I'd go for fb_io_*().
>
> Ok, makes sense.
>
> >> CFB is used by the drawing helpers, which are usually used together with
> >> this code. Hence the current naming.
> >
> > That's because your drawing helpers operate (only) on chunky color
> > frame buffer formats ;-)
>
> Should we rename the CFB drawing functions to fb_io_ then? AFAICT they
> are the same algorithms as in the fb_sys_ functions; just with I/O memory.

I don't know if that's worth the churn.
Historically, the frame buffer was usually located in dedicated memory,
hence the drawing operations operated on I/O memory.
With the advent of unified memory architectures, the fb_sys_*()
functions were introduced.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds