[RFC] double buffering for atmel_lcdfb

Hello.

I try to implement double buffering for Atmel LCD display on AT91SAM9263 based 
board. I could not find any document about panning display on linux 
framebuffer, so I have some question about it and generic question about 
double buffer design. 

1) Is FBIOPAN_DISPLAY ioctl right way to implement double buffering?

2) What is the difference between ypanstep and ywrapstep, which one should be 
used for double buffering.

3) Does double buffer stuff should be turned on using module parameter, 
compile time parameter, board specific parameter (flag from platform device).
As it consume 2x memory I don't thing anyone want it by default.

I think double buffer for Atmel LCD should go mainline, of course after doing 
it reliable. After your comments and advice, I think I will be able to send 
right done and signed-off patch. 

Below is patch I have already done. I also attached program which I used for 
for tests - as far result are quite nice.

Cheers
Stanislaw Gruszka

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index b004036..d0b2bed 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c

@@ -7,7 +7,6 @@
  * License.  See the file COPYING in the main directory of this archive for
  * more details.
  */
-
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>

@@ -41,6 +40,8 @@
 #if defined(CONFIG_ARCH_AT91)
 #define	ATMEL_LCDFB_FBINFO_DEFAULT	FBINFO_DEFAULT
 
+static int double_buffering = 1;
+
 static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
 					struct fb_var_screeninfo *var)
 {

@@ -789,7 +790,13 @@ static int __init atmel_lcdfb_probe(struct 

platform_device *pdev)
 		 * Don't clear the framebuffer -- someone may have set
 		 * up a splash image.
 		 */
+		double_buffering = 0;
+ 		dev_info(dev, "dissable double buffering\n");
 	} else {
+ 		if (double_buffering) {
+ 			info->fix.ypanstep = info->var.yres_virtual;
+ 			info->var.yres_virtual *= 2;
+		}
 		/* alocate memory buffer */
 		ret = atmel_lcdfb_alloc_video_memory(sinfo);
 		if (ret < 0) {

On Wed, 6 Aug 2008, Stanislaw Gruszka wrote:

I try to implement double buffering for Atmel LCD display on AT91SAM9263 based 
board. I could not find any document about panning display on linux 
framebuffer, so I have some question about it and generic question about 
double buffer design. 

1) Is FBIOPAN_DISPLAY ioctl right way to implement double buffering?

Yes.

2) What is the difference between ypanstep and ywrapstep, which one should be 
used for double buffering.

Ypanstep is the allowed step increment for panning vertically.
E.g. for a screen with 200 lines and y.offset = 50, it will display
lines 50-249 of the frame buffer.

Ywrapstep is the allowed step increment for wrapping the screen
vertically.
E.g. for a screen with 200 lines and y.offset = 50, it will display
lines 50-199 followed by lines 0-49 of the frame buffer.

3) Does double buffer stuff should be turned on using module parameter, 
compile time parameter, board specific parameter (flag from platform device).
As it consume 2x memory I don't thing anyone want it by default.

No, let userspace ask for it when it specifies a large
fb_var_screeninfo.yres_virtual, if possible. If you have to allocate all
frame buffer memory at initialization time, you can use a board specific
parameter or a kernel command line parameter.

I would set it to 1, if there's no hardware limitation preventing this.

+ 			info->var.yres_virtual *= 2;

This is done at initialization time, hence it cannot be changed later by
using the ioctls?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Stanislaw Gruszka [off-list ref] wrote:

Below is patch I have already done. I also attached program which I used for 
for tests - as far result are quite nice.

I sent this patch a while ago, but I forgot to send Nicolas a test
program when he asked for it :-(

Could you give it a try? If it works for you, I think we can conclude
that y-panning indeed works on AT91 so the patch can be applied.

Haavard

From c1dc155e3c1a828faa4379a1f6f6de0bb58385cc Mon Sep 17 00:00:00 2001
From: Haavard Skinnemoen <redacted>
Date: Sat, 23 Jun 2007 17:38:26 +0200
Subject: [PATCH] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91

Panning in the y-direction can be done by simply changing the DMA base
address. This code is already in place, but FBIOPAN_DISPLAY will
currently fail because ypanstep is 0.

Set ypanstep to 1 to indicate that we do support y-panning and also
set the necessary acceleration flags on AT91 (AVR32 already have
them.)

Signed-off-by: Haavard Skinnemoen <redacted>
---
 drivers/video/atmel_lcdfb.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index b004036..c0a0364 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c

@@ -39,7 +39,9 @@
 #endif
 
 #if defined(CONFIG_ARCH_AT91)
-#define	ATMEL_LCDFB_FBINFO_DEFAULT	FBINFO_DEFAULT
+#define	ATMEL_LCDFB_FBINFO_DEFAULT	(FBINFO_DEFAULT \
+					 | FBINFO_PARTIAL_PAN_OK \
+					 | FBINFO_HWACCEL_YPAN)
 
 static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
 					struct fb_var_screeninfo *var)

@@ -177,7 +179,7 @@ static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata = {
 	.type		= FB_TYPE_PACKED_PIXELS,
 	.visual		= FB_VISUAL_TRUECOLOR,
 	.xpanstep	= 0,
-	.ypanstep	= 0,
+	.ypanstep	= 1,
 	.ywrapstep	= 0,
 	.accel		= FB_ACCEL_NONE,
 };

-- 
1.5.6.3


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Wednesday 06 August 2008 14:13:05 Haavard Skinnemoen napisał(a):

Stanislaw Gruszka [off-list ref] wrote:

quoted

Below is patch I have already done. I also attached program which I used
for for tests - as far result are quite nice.

I sent this patch a while ago, but I forgot to send Nicolas a test
program when he asked for it :-(

Could you give it a try? If it works for you, I think we can conclude
that y-panning indeed works on AT91 so the patch can be applied.

Yes, it works with my program with ypanstep == yres (I did not check with ypanstep == 1 yet), 

5 more quoted lines

I think some video memory allocation control is also needed for users which 
want to have frame buffer in main memory (this is a must with bigger LCDs)
Currently I just increase smem_len just before dma_alloc_writecombine() is called.  

@@ -241,7 +243,7 @@ static int atmel_lcdfb_alloc_video_memory(struct atmel_lcdfb_info *sinfo)
        struct fb_info *info = sinfo->info;
        struct fb_var_screeninfo *var = &info->var;

-       info->fix.smem_len = (var->xres_virtual * var->yres_virtual
+       info->fix.smem_len = (var->xres_virtual * var->yres_virtual * 2
                            * ((var->bits_per_pixel + 7) / 8));

        info->screen_base = dma_alloc_writecombine(info->device, 

Ok I know that I could just add memory resource to at91_lcdc_device, but
main RAM memory is managed by linux and need to be somehow allocated.
So maybe worth to add smem_len hint in at91_lcdc_device like this:

--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c

@@ -240,9 +242,11 @@ static int atmel_lcdfb_alloc_video_memory(struct atmel_lcdfb_info *sinfo)
 {
        struct fb_info *info = sinfo->info;
        struct fb_var_screeninfo *var = &info->var;
-
-       info->fix.smem_len = (var->xres_virtual * var->yres_virtual
+       unsigned int smem_len;
+
+       smem_len = (var->xres_virtual * var->yres_virtual
                            * ((var->bits_per_pixel + 7) / 8));
+       info->fix.smem_len = max(smem_len, sinfo->smem_len);

        info->screen_base = dma_alloc_writecombine(info->device, info->fix.smem_len,
                                        (dma_addr_t *)&info->fix.smem_start, GFP_KERNEL);

diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
index ed64862..7f3ff79 100644
--- a/include/video/atmel_lcdc.h
+++ b/include/video/atmel_lcdc.h

@@ -37,6 +37,7 @@ struct atmel_lcdfb_info {
        struct fb_info          *info;
        void __iomem            *mmio;
        unsigned long           irq_base;
+       unsigned int            smem_len;

        unsigned int            guard_time;
        struct platform_device  *pdev;

Regards
Stanislaw Gruszka

Wednesday 06 August 2008 15:53:03 Stanislaw Gruszka napisał(a):

Wednesday 06 August 2008 14:13:05 Haavard Skinnemoen napisał(a):

quoted

Stanislaw Gruszka [off-list ref] wrote:

quoted

Below is patch I have already done. I also attached program which I
used for for tests - as far result are quite nice.

I sent this patch a while ago, but I forgot to send Nicolas a test
program when he asked for it :-(

Could you give it a try? If it works for you, I think we can conclude
that y-panning indeed works on AT91 so the patch can be applied.

Yes, it works with my program with ypanstep == yres (I did not check with
ypanstep == 1 yet),

Panning by step 1 works too on my hardware.

quoted

From c1dc155e3c1a828faa4379a1f6f6de0bb58385cc Mon Sep 17 00:00:00 2001
From: Haavard Skinnemoen <redacted>
Date: Sat, 23 Jun 2007 17:38:26 +0200
Subject: [PATCH] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on
AT91

Panning in the y-direction can be done by simply changing the DMA base
address. This code is already in place, but FBIOPAN_DISPLAY will
currently fail because ypanstep is 0.

Set ypanstep to 1 to indicate that we do support y-panning and also
set the necessary acceleration flags on AT91 (AVR32 already have
them.)

Signed-off-by: Haavard Skinnemoen <redacted>
---
 drivers/video/atmel_lcdfb.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index b004036..c0a0364 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c

@@ -39,7 +39,9 @@
 #endif

 #if defined(CONFIG_ARCH_AT91)
-#define	ATMEL_LCDFB_FBINFO_DEFAULT	FBINFO_DEFAULT
+#define	ATMEL_LCDFB_FBINFO_DEFAULT	(FBINFO_DEFAULT \
+					 | FBINFO_PARTIAL_PAN_OK \
+					 | FBINFO_HWACCEL_YPAN)

 static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info

*sinfo, struct fb_var_screeninfo *var)

@@ -177,7 +179,7 @@ static struct fb_fix_screeninfo atmel_lcdfb_fix

__initdata = { .type		= FB_TYPE_PACKED_PIXELS,
 	.visual		= FB_VISUAL_TRUECOLOR,
 	.xpanstep	= 0,
-	.ypanstep	= 0,
+	.ypanstep	= 1,

51 more quoted lines

Any comments on this?

Cheers
Stanislaw Gruszka

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Stanislaw Gruszka [off-list ref] wrote:

I think some video memory allocation control is also needed for users which 
want to have frame buffer in main memory (this is a must with bigger LCDs)
Currently I just increase smem_len just before dma_alloc_writecombine() is called.  

I think your patch makes sense, but I suspect we also need to check
that the user-requested mode is actually possible given the amount of
memory available. That can be a separate patch though.

If you resubmit with a proper signed-off-by line, feel free to add

Acked-by: Haavard Skinnemoen <redacted>

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/