Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1383500 > unrolled thread

[PATCH 5/8] fbdev: fb_defio: Export fb_deferred_io_mmap

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 5/8] fbdev: fb_defio: Export fb_deferred_io_mmap Noralf Trønnes <noralf@tronnes.org> - 2016-04-20 17:40 +0200
    Re: [PATCH 5/8] fbdev: fb_defio: Export fb_deferred_io_mmap Daniel Vetter <daniel@ffwll.ch> - 2016-04-20 19:50 +0200
      Re: [PATCH 5/8] fbdev: fb_defio: Export fb_deferred_io_mmap Noralf Trønnes <noralf@tronnes.org> - 2016-04-20 20:40 +0200
        Re: [PATCH 5/8] fbdev: fb_defio: Export fb_deferred_io_mmap Daniel Vetter <daniel@ffwll.ch> - 2016-04-21 09:40 +0200

#1383500 — [PATCH 5/8] fbdev: fb_defio: Export fb_deferred_io_mmap

Export fb_deferred_io_mmap so drivers can change vma->vm_page_prot.
When the framebuffer memory is allocated using dma_alloc_writecombine()
instead of vmalloc(), I get cache syncing problems.
This solves it:

static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info,
					  struct vm_area_struct *vma)
{
	fb_deferred_io_mmap(info, vma);
	vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);

	return 0;
}

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/video/fbdev/core/fb_defio.c | 3 ++-
 include/linux/fb.h                  | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index 57721c7..74b5bca 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -164,7 +164,7 @@ static const struct address_space_operations fb_deferred_io_aops = {
 	.set_page_dirty = fb_deferred_io_set_page_dirty,
 };
 
-static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
+int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	vma->vm_ops = &fb_deferred_io_vm_ops;
 	vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
@@ -173,6 +173,7 @@ static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	vma->vm_private_data = info;
 	return 0;
 }
+EXPORT_SYMBOL(fb_deferred_io_mmap);
 
 /* workqueue callback */
 static void fb_deferred_io_work(struct work_struct *work)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index dfe8835..a964d07 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -673,6 +673,7 @@ static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch,
 }
 
 /* drivers/video/fb_defio.c */
+int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma);
 extern void fb_deferred_io_init(struct fb_info *info);
 extern void fb_deferred_io_open(struct fb_info *info,
 				struct inode *inode,
-- 
2.2.2

[toc] | [next] | [standalone]

#1383583

On Wed, Apr 20, 2016 at 05:25:26PM +0200, Noralf Trønnes wrote:
> Export fb_deferred_io_mmap so drivers can change vma->vm_page_prot.
> When the framebuffer memory is allocated using dma_alloc_writecombine()
> instead of vmalloc(), I get cache syncing problems.
> This solves it:
> 
> static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info,
> 					  struct vm_area_struct *vma)
> {
> 	fb_deferred_io_mmap(info, vma);
> 	vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);

Hm, do we need pgpropt_writecombine? There recently was some discussion
(on the arc platform) that fbdev pgprots need to be fixed up in fbdev
code. I have no idea, just repeating from memory ...
-Daniel

> 
> 	return 0;
> }
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/video/fbdev/core/fb_defio.c | 3 ++-
>  include/linux/fb.h                  | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index 57721c7..74b5bca 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -164,7 +164,7 @@ static const struct address_space_operations fb_deferred_io_aops = {
>  	.set_page_dirty = fb_deferred_io_set_page_dirty,
>  };
>  
> -static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
> +int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
>  {
>  	vma->vm_ops = &fb_deferred_io_vm_ops;
>  	vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
> @@ -173,6 +173,7 @@ static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
>  	vma->vm_private_data = info;
>  	return 0;
>  }
> +EXPORT_SYMBOL(fb_deferred_io_mmap);
>  
>  /* workqueue callback */
>  static void fb_deferred_io_work(struct work_struct *work)
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index dfe8835..a964d07 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -673,6 +673,7 @@ static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch,
>  }
>  
>  /* drivers/video/fb_defio.c */
> +int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma);
>  extern void fb_deferred_io_init(struct fb_info *info);
>  extern void fb_deferred_io_open(struct fb_info *info,
>  				struct inode *inode,
> -- 
> 2.2.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1383631

Den 20.04.2016 19:44, skrev Daniel Vetter:
> On Wed, Apr 20, 2016 at 05:25:26PM +0200, Noralf Trønnes wrote:
>> Export fb_deferred_io_mmap so drivers can change vma->vm_page_prot.
>> When the framebuffer memory is allocated using dma_alloc_writecombine()
>> instead of vmalloc(), I get cache syncing problems.
>> This solves it:
>>
>> static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info,
>> 					  struct vm_area_struct *vma)
>> {
>> 	fb_deferred_io_mmap(info, vma);
>> 	vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> Hm, do we need pgpropt_writecombine? There recently was some discussion
> (on the arc platform) that fbdev pgprots need to be fixed up in fbdev
> code. I have no idea, just repeating from memory ...

I need it or else I get partial lines that doesn't get updated on the 
display.
fbdev code that doesn't set (struct fb_ops *)->fb_mmap, gets this for free
in the default fb_mmap implementation (drivers/video/fbdev/core/fbmem.c).
It calls fb_pgprotect() at the end which is an architecture specific
function that on many platforms uses pgprot_writecombine(), but not on all.
And looking at some of the fb_mmap implementations, some of them sets
vm_page_prot to nocache for instance, so I think the safest bet is to do
this here and not in the fbdev core. And we can't call fb_pgprotect() from
fb_deferred_io_mmap() either because we don't have access to the file
pointer that powerpc needs.
I think the case you refer to was solved with using fb_pgprotect() for
the platform in question and it didn't involve deferred io.

> -Daniel
>
>> 	return 0;
>> }
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/video/fbdev/core/fb_defio.c | 3 ++-
>>   include/linux/fb.h                  | 1 +
>>   2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
>> index 57721c7..74b5bca 100644
>> --- a/drivers/video/fbdev/core/fb_defio.c
>> +++ b/drivers/video/fbdev/core/fb_defio.c
>> @@ -164,7 +164,7 @@ static const struct address_space_operations fb_deferred_io_aops = {
>>   	.set_page_dirty = fb_deferred_io_set_page_dirty,
>>   };
>>   
>> -static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
>> +int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
>>   {
>>   	vma->vm_ops = &fb_deferred_io_vm_ops;
>>   	vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
>> @@ -173,6 +173,7 @@ static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
>>   	vma->vm_private_data = info;
>>   	return 0;
>>   }
>> +EXPORT_SYMBOL(fb_deferred_io_mmap);
>>   
>>   /* workqueue callback */
>>   static void fb_deferred_io_work(struct work_struct *work)
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index dfe8835..a964d07 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -673,6 +673,7 @@ static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch,
>>   }
>>   
>>   /* drivers/video/fb_defio.c */
>> +int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma);
>>   extern void fb_deferred_io_init(struct fb_info *info);
>>   extern void fb_deferred_io_open(struct fb_info *info,
>>   				struct inode *inode,
>> -- 
>> 2.2.2
>>

[toc] | [prev] | [next] | [standalone]

#1383910

On Wed, Apr 20, 2016 at 08:33:17PM +0200, Noralf Trønnes wrote:
> 
> Den 20.04.2016 19:44, skrev Daniel Vetter:
> >On Wed, Apr 20, 2016 at 05:25:26PM +0200, Noralf Trønnes wrote:
> >>Export fb_deferred_io_mmap so drivers can change vma->vm_page_prot.
> >>When the framebuffer memory is allocated using dma_alloc_writecombine()
> >>instead of vmalloc(), I get cache syncing problems.
> >>This solves it:
> >>
> >>static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info,
> >>					  struct vm_area_struct *vma)
> >>{
> >>	fb_deferred_io_mmap(info, vma);
> >>	vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> >Hm, do we need pgpropt_writecombine? There recently was some discussion
> >(on the arc platform) that fbdev pgprots need to be fixed up in fbdev
> >code. I have no idea, just repeating from memory ...
> 
> I need it or else I get partial lines that doesn't get updated on the
> display.
> fbdev code that doesn't set (struct fb_ops *)->fb_mmap, gets this for free
> in the default fb_mmap implementation (drivers/video/fbdev/core/fbmem.c).
> It calls fb_pgprotect() at the end which is an architecture specific
> function that on many platforms uses pgprot_writecombine(), but not on all.
> And looking at some of the fb_mmap implementations, some of them sets
> vm_page_prot to nocache for instance, so I think the safest bet is to do
> this here and not in the fbdev core. And we can't call fb_pgprotect() from
> fb_deferred_io_mmap() either because we don't have access to the file
> pointer that powerpc needs.
> I think the case you refer to was solved with using fb_pgprotect() for
> the platform in question and it didn't involve deferred io.

Argh, oh well. I guess another case (besides the mmap thing in udl where
the default defio support from fbdev goes boom) where this isn't the most
awesome thing ever.

Please add your explanation to the commit message for posterity.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web