fb_defio: generic write support

This patch implements generic write support that can be used by deferred IO client drivers. It reuses the same deferred IO callback mechanism in existing client drivers and uses the pagelist to inform the client driver which pages were modified in the write call. It should be possible to extend this same method to imageblit/copyarea/fillrect as well. metronomefb's write has been removed in favour of using this mechanism. This patch depends on Magnus' patchset to add defio support for dma/kmalloc-ed memory.

---
 drivers/video/Kconfig       |    4 ++
 drivers/video/fb_defio.c    |  116 +++++++++++++++++++++++++++++++++---------
 drivers/video/metronomefb.c |   48 +-----------------
 include/linux/fb.h          |    2 +
 4 files changed, 98 insertions(+), 72 deletions(-)

index 3f3ce13..a96f62e 100644
@@ -179,6 +179,10 @@ config FB_SYS_FOPS
 config FB_DEFERRED_IO
 	bool
 	depends on FB
+	select FB_SYS_FILLRECT
+	select FB_SYS_COPYAREA
+	select FB_SYS_IMAGEBLIT
+	select FB_SYS_FOPS
 
 config FB_HECUBA
 	tristate

index 75a81ea..59e4f32 100644
@@ -983,6 +983,8 @@ extern void fb_deferred_io_open(struct fb_info *info,
 extern void fb_deferred_io_cleanup(struct fb_info *info);
 extern int fb_deferred_io_fsync(struct file *file, struct dentry *dentry,
 				int datasync);
+ssize_t fb_defio_write(struct fb_info *info, const char __user *buf,
+				size_t count, loff_t *ppos);
 
 static inline bool fb_be_math(struct fb_info *info)
 {

1.5.2.3

Hi Jaya,

Good plan, I agree that having more fine grained dirty page handling makes sense. However, if this or a dirty rectangle is better performance wise is not so clear. I'd say that for the write() operation this pagelist modification makes sense, but for fillrect/copyarea/imageblit it becomes more complicated since we depend on video mode all of a sudden. But maybe that's a non-issue.

In my mind, the best strategy is to look at drivers and see which interface they need:

- hecubafb: full frames
- sh_mobile_lcdcfb: full frames
- metronomefb: pages
- xen-fbfront: area

Basically we have three different requirements. For full frames the struct page handling is just plain overhead. So is any dirty area handling code. Just having the deferred io callback by itself is enough for the full frame type of drivers. Metronomefb would be very happy to just receive pages. And xen just a dirty area, unless I'm misunderstanding the code. The sh_mobile_lcdc hardware also supports partial screen update, so more fine grained dirty area/page handling is doable there as well.

I suggest adding a flag variable and two flags that enable the pagelist and the dirty area. The full frame drivers keep the flags unset to get improved performance (no need to traverse the pagelist anymore) and the other drivers can select which modes that should be enabled depending on the hardware requirements.

Or just have a single flag enabling the pagelist, and provide helper code for area based drivers that converts dirty pages to rectangles suitable for that type of hardware. The latter may be a bit too complicated though.

This may be a good first step in the right direction though. I'd like to add code on top of this that disables the pagelist handling for full frame drivers though. Any thoughts?

Cheers,

/ magnus

Yes, you're right.

I think your original insight is correct, it can be complicated to translate between pages and the structures used for the fillrect/copyarea/imageblit operations.

hecubafb uses pages. oh wait, I see, I had posted a patch to support pages but I never got it merged. I'll put that on the todo list.

Agreed.

Yes, this sounds reasonable.

I think I agree that conversion routines would start getting messy and require hardware specific knowledge in which case, I think we might as well leave it for the drivers to do.

Sounds like a reasonable plan. However, I think you're planning to do page support in sh_mobile and hecubafb should be page based so that would indicate there won't be any full frame drivers in the long term, right? That said, I think its still valuable to have that code to make that a possible option.

Thanks,
jaya