| Message ID | 20230510110557.14343-1-tzimmermann@suse.de |
|---|---|
| Headers | show |
| Series | fbdev: Move framebuffer I/O helpers to <asm/fb.h> | expand |
Hi Thomas, On Wed, May 10, 2023 at 1:06 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Implement framebuffer I/O helpers, such as fb_read*() and fb_write*(), > in the architecture's <asm/fb.h> header file or the generic one. > > The common case has been the use of regular I/O functions, such as > __raw_readb() or memset_io(). A few architectures used plain system- > memory reads and writes. Sparc used helpers for its SBus. > > The architectures that used special cases provide the same code in > their __raw_*() I/O helpers. So the patch replaces this code with the > __raw_*() functions and moves it to <asm-generic/fb.h> for all > architectures. > > v6: > * fix fb_readq()/fb_writeq() on 64-bit mips (kernel test robot) > v5: > * include <linux/io.h> in <asm-generic/fb>; fix s390 build > v4: > * ia64, loongarch, sparc64: add fb_mem*() to arch headers > to keep current semantics (Arnd) > v3: > * implement all architectures with generic helpers > * support reordering and native byte order (Geert, Arnd) > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Tested-by: Sui Jingfeng <suijingfeng@loongson.cn> > Reviewed-by: Arnd Bergmann <arnd@arndb.de> > > add mips fb_q() > --- a/arch/mips/include/asm/fb.h > +++ b/arch/mips/include/asm/fb.h > @@ -12,6 +12,28 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma, > } > #define fb_pgprotect fb_pgprotect > > +/* > + * MIPS doesn't define __raw_ I/O macros, so the helpers > + * in <asm-generic/fb.h> don't generate fb_readq() and > + * fb_write(). We have to provide them here. MIPS does not include <asm-generic/io.h>, nor define its own __raw_readq() and __raw_writeq()... > + * > + * TODO: Convert MIPS to generic I/O. The helpers below can > + * then be removed. > + */ > +#ifdef CONFIG_64BIT > +static inline u64 fb_readq(const volatile void __iomem *addr) > +{ > + return __raw_readq(addr); ... so how can this call work? > +} > +#define fb_readq fb_readq > + > +static inline void fb_writeq(u64 b, volatile void __iomem *addr) > +{ > + __raw_writeq(b, addr); > +} > +#define fb_writeq fb_writeq > +#endif > + > #include <asm-generic/fb.h> Gr{oetje,eeting}s, Geert
Hi Geert Am 10.05.23 um 16:34 schrieb Geert Uytterhoeven: > Hi Thomas, > > On Wed, May 10, 2023 at 4:20 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Am 10.05.23 um 14:34 schrieb Geert Uytterhoeven: >>> On Wed, May 10, 2023 at 1:06 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: >>>> Implement framebuffer I/O helpers, such as fb_read*() and fb_write*(), >>>> in the architecture's <asm/fb.h> header file or the generic one. >>>> >>>> The common case has been the use of regular I/O functions, such as >>>> __raw_readb() or memset_io(). A few architectures used plain system- >>>> memory reads and writes. Sparc used helpers for its SBus. >>>> >>>> The architectures that used special cases provide the same code in >>>> their __raw_*() I/O helpers. So the patch replaces this code with the >>>> __raw_*() functions and moves it to <asm-generic/fb.h> for all >>>> architectures. >>>> >>>> v6: >>>> * fix fb_readq()/fb_writeq() on 64-bit mips (kernel test robot) >>>> v5: >>>> * include <linux/io.h> in <asm-generic/fb>; fix s390 build >>>> v4: >>>> * ia64, loongarch, sparc64: add fb_mem*() to arch headers >>>> to keep current semantics (Arnd) >>>> v3: >>>> * implement all architectures with generic helpers >>>> * support reordering and native byte order (Geert, Arnd) >>>> >>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>>> Tested-by: Sui Jingfeng <suijingfeng@loongson.cn> >>>> Reviewed-by: Arnd Bergmann <arnd@arndb.de> > >>>> --- a/arch/mips/include/asm/fb.h >>>> +++ b/arch/mips/include/asm/fb.h >>>> @@ -12,6 +12,28 @@ static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma, >>>> } >>>> #define fb_pgprotect fb_pgprotect >>>> >>>> +/* >>>> + * MIPS doesn't define __raw_ I/O macros, so the helpers >>>> + * in <asm-generic/fb.h> don't generate fb_readq() and >>>> + * fb_write(). We have to provide them here. >>> >>> MIPS does not include <asm-generic/io.h>, nor define its own >> >> I know, that's why the TODO says to convert it to generic I/O. >> >>> __raw_readq() and __raw_writeq()... >> >> It doesn't define those macros, but it generates function calls of the >> same names. Follow the macros at >> >> >> https://elixir.bootlin.com/linux/latest/source/arch/mips/include/asm/io.h#L357 >> >> It expands to a variety of helpers, including __raw_*(). > > Thanks, I forgot MIPS is using these grep-unfriendly factories... > >>>> + * >>>> + * TODO: Convert MIPS to generic I/O. The helpers below can >>>> + * then be removed. >>>> + */ >>>> +#ifdef CONFIG_64BIT >>>> +static inline u64 fb_readq(const volatile void __iomem *addr) >>>> +{ >>>> + return __raw_readq(addr); >>> >>> ... so how can this call work? >> >> On 64-bit builds, there's __raw_readq() and __raw_writeq(). >> >> At first, I tried to do the right thing and convert MIPS to work with >> <asm-generic/io.h>. But that created a ton of follow-up errors in other >> headers. So for now, it's better to handle this problem in asm/fb.h. > > So isn't just adding > > #define __raw_readq __raw_readq > #define __raw_writeq __raw_writeq > > to arch/mips/include/asm/io.h sufficient to make <asm-generic/fb.h> > do the right thing? That works. I had a patch that adds all missing defines to MIPS' io.h. Then I went with the current fix, which is self-contained within fbdev. But I'd leave it to arch maintainers. Best regards Thomas > > Gr{oetje,eeting}s, > > Geert >
Fbdev provides helpers for framebuffer I/O, such as fb_readl(), fb_writel() or fb_memcpy_to_fb(). The implementation of each helper depends on the architecture, but they are all equivalent to regular I/O functions of similar names. So use regular functions instead and move all helpers into <asm-generic/fb.h> The first patch a simple whitespace cleanup. Until now, <linux/fb.h> contained an include of <asm/io.h>. As this will go away, patches 2 to 4 prepare include statements in the various drivers. Source files that use regular I/O helpers, such as readl(), now include <linux/io.h>. Source files that use framebuffer I/O helpers, such as fb_readl(), now include <linux/fb.h>. Patch 5 replaces the architecture-based if-else branching in <linux/fb.h> by helpers in <asm-generic/fb.h>. All helpers use Linux' existing I/O functions. Patch 6 harmonizes naming among fbdev and existing I/O functions. The patchset has been built for a variety of platforms, such as x86-64, arm, aarch64, ppc64, parisc, m64k, mips and sparc. v6: * fix build on 64-bit mips (kernel test robot) * update fb_io_fops.c v5: * fix build on s390 v4: * keep fb_mem*() as-is on ia64, loongarch, sparc64 (Arnd) * don't include <asm/fb.h> (Sam) v3: * add the new helpers in <asm-generic/fb.h> * support reordering and native byte order (Geert, Arnd) v2: * use Linux I/O helpers (Sam, Arnd) Thomas Zimmermann (6): fbdev/matrox: Remove trailing whitespaces ipu-v3: Include <linux/io.h> fbdev: Include <linux/io.h> in various drivers fbdev: Include <linux/fb.h> instead of <asm/fb.h> fbdev: Move framebuffer I/O helpers into <asm/fb.h> fbdev: Rename fb_mem*() helpers arch/ia64/include/asm/fb.h | 20 ++++ arch/loongarch/include/asm/fb.h | 21 ++++ arch/mips/include/asm/fb.h | 22 +++++ arch/parisc/video/fbdev.c | 3 +- arch/sparc/include/asm/fb.h | 20 ++++ arch/sparc/video/fbdev.c | 1 - arch/x86/video/fbdev.c | 2 - drivers/gpu/ipu-v3/ipu-prv.h | 1 + drivers/staging/sm750fb/sm750.c | 2 +- drivers/video/fbdev/arcfb.c | 1 + drivers/video/fbdev/aty/atyfb.h | 2 + drivers/video/fbdev/aty/mach64_cursor.c | 2 +- drivers/video/fbdev/chipsfb.c | 2 +- drivers/video/fbdev/core/fb_io_fops.c | 4 +- drivers/video/fbdev/core/fbcon.c | 1 - drivers/video/fbdev/core/fbmem.c | 2 - drivers/video/fbdev/kyro/fbdev.c | 2 +- drivers/video/fbdev/matrox/matroxfb_accel.c | 6 +- drivers/video/fbdev/matrox/matroxfb_base.h | 4 +- drivers/video/fbdev/pvr2fb.c | 2 +- drivers/video/fbdev/sstfb.c | 2 +- drivers/video/fbdev/stifb.c | 4 +- drivers/video/fbdev/tdfxfb.c | 2 +- drivers/video/fbdev/wmt_ge_rops.c | 2 + include/asm-generic/fb.h | 102 ++++++++++++++++++++ include/linux/fb.h | 55 +---------- 26 files changed, 210 insertions(+), 77 deletions(-)