Framebuffer mmap on PowerPC

[Thomas Zimmermann]

Hi

Am 31.08.23 um 19:38 schrieb Christophe Leroy:
> 
> 
> Le 31/08/2023 à 16:41, Thomas Zimmermann a écrit :
>> 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

> 
> 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
> 
> 
>>
>> 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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20230901/0be25bc9/attachment-0001.sig>