Thread (5 messages) flat view 5 messages, 3 authors, 2023-09-04

Re: Framebuffer mmap on PowerPC

From: Thomas Zimmermann <hidden>
Date: 2023-09-04 07:24:55
Also in: dri-devel, linux-arch, linux-fbdev

Hi

Am 31.08.23 um 19:38 schrieb Christophe Leroy:

Le 31/08/2023 à 16:41, Thomas Zimmermann a écrit :
quoted
Hi,

there's a per-architecture function called fb_pgprotect() that sets
VMA's vm_page_prot for mmaped framebuffers. Most architectures use a
simple implementation based on pgprot_writecomine() [1] or
pgprot_noncached(). [2]

On PPC this function uses phys_mem_access_prot() and therefore requires
the mmap call's file struct. [3] Removing the file argument would help
with simplifying the caller of fb_pgprotect(). [4]

Why is the file even required on PPC?

Is it possible to replace phys_mem_access_prot() with something simpler
that does not use the file struct?
Looks like phys_mem_access_prot() defaults to calling pgprot_noncached()
see
https://elixir.bootlin.com/linux/v6.5/source/arch/powerpc/mm/mem.c#L37
but for a few platforms that's superseeded by
pci_phys_mem_access_prot(), see
https://elixir.bootlin.com/linux/v6.5/source/arch/powerpc/kernel/pci-common.c#L524

However, as far as I can see pci_phys_mem_access_prot() doesn't use file
so you could likely drop that argument on phys_mem_access_prot() on
powerpc. But when I for instance look at arm, I see that the file
argument is used, see
https://elixir.bootlin.com/linux/v6.5/source/arch/arm/mm/mmu.c#L713
Right, I've seen these various implementations. Luckily, the ARM 
framebuffers use a plain pgprot_writecombine() without any references on 
to file.
So, the simplest is maybe the following, allthough that's probably worth
a comment:
Could we drop the file argument from PPC's internal functions and 
provide this interface to fb_pgprotect()? phys_mem_access_prot() would 
be a trivial wrapper around that internal API. I'd provide a patch to do 
that.

Best regards
Thomas
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/include/asm/fb.h b/arch/powerpc/include/asm/fb.h
index 5f1a2e5f7654..8b9b856f476e 100644
--- a/arch/powerpc/include/asm/fb.h
+++ b/arch/powerpc/include/asm/fb.h
@@ -6,10 +6,9 @@

   #include <asm/page.h>

-static inline void fb_pgprotect(struct file *file, struct
vm_area_struct *vma,
-				unsigned long off)
+static inline void fb_pgprotect(struct vm_area_struct *vma, unsigned
long off)
   {
-	vma->vm_page_prot = phys_mem_access_prot(file, off >> PAGE_SHIFT,
+	vma->vm_page_prot = phys_mem_access_prot(NULL, off >> PAGE_SHIFT,
   						 vma->vm_end - vma->vm_start,
   						 vma->vm_page_prot);
   }


Christophe

quoted
Best regards
Thomas


[1]
https://elixir.bootlin.com/linux/v6.5/source/include/asm-generic/fb.h#L19
[2]
https://elixir.bootlin.com/linux/v6.5/source/arch/mips/include/asm/fb.h#L11
[3]
https://elixir.bootlin.com/linux/v6.5/source/arch/powerpc/include/asm/fb.h#L12
[4]
https://elixir.bootlin.com/linux/v6.5/source/drivers/video/fbdev/core/fbmem.c#L1299
-- 
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)

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help