[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZC2cwIJJlyDWge+B@phenom.ffwll.local>
Date: Wed, 5 Apr 2023 18:07:28 +0200
From: Daniel Vetter <daniel@...ll.ch>
To: Arnd Bergmann <arnd@...db.de>
Cc: Thomas Zimmermann <tzimmermann@...e.de>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Helge Deller <deller@....de>,
Javier Martinez Canillas <javierm@...hat.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Linux-Arch <linux-arch@...r.kernel.org>,
linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-snps-arc@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-ia64@...r.kernel.org,
loongarch@...ts.linux.dev, linux-m68k@...ts.linux-m68k.org,
linux-mips@...r.kernel.org, linux-parisc@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, linux-sh@...r.kernel.org,
sparclinux@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH 01/18] fbdev: Prepare generic architecture helpers
On Wed, Apr 05, 2023 at 05:53:03PM +0200, Arnd Bergmann wrote:
> On Wed, Apr 5, 2023, at 17:05, Thomas Zimmermann wrote:
> > Generic implementations of fb_pgprotect() and fb_is_primary_device()
> > have been in the source code for a long time. Prepare the header file
> > to make use of them.
> >
> > Improve the code by using an inline function for fb_pgprotect() and
> > by removing include statements.
> >
> > Symbols are protected by preprocessor guards. Architectures that
> > provide a symbol need to define a preprocessor token of the same
> > name and value. Otherwise the header file will provide a generic
> > implementation. This pattern has been taken from <asm/io.h>.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@...e.de>
>
> Moving this into generic code is good, but I'm not sure
> about the default for fb_pgprotect():
>
> > +
> > +#ifndef fb_pgprotect
> > +#define fb_pgprotect fb_pgprotect
> > +static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
> > + unsigned long off)
> > +{ }
> > +#endif
>
> I think most architectures will want the version we have on
> arc, arm, arm64, loongarch, and sh already:
>
> static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
> unsigned long off)
> {
> vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> }
>
> so I'd suggest making that version the default, and treating the
> empty ones (m68knommu, sparc32) as architecture specific
> workarounds.
Yeah I was about to type the same suggestion :-)
-Daniel
> I see that sparc64 and parisc use pgprot_uncached here, but as
> they don't define a custom pgprot_writecombine, this ends up being
> the same, and they can use the above definition as well.
>
> mips defines pgprot_writecombine but uses pgprot_noncached
> in fb_pgprotect(), which is probably a mistake and should have
> been updated as part of commit 4b050ba7a66c ("MIPS: pgtable.h:
> Implement the pgprot_writecombine function for MIPS").
>
> Arnd
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Powered by blists - more mailing lists