i.MX Framebuffer patches

i.MX Framebuffer patches

[Sascha Hauer]

Here are some patches for the i.MX framebuffer driver. They mainly prepare
the driver for use with other i.MX processors like the i.MX27.

Sascha



-------------------------------------------------------------------------
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=/

[PATCH] i.MX Framebuffer: remove gpio setup function

[Sascha Hauer]

Remove the gpio mux setup function from i.MX framebuffer driver.
This function is platform specific and thus should be done by
the board setup. As there are currently no in-kernel users
of this driver we do not break anything.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/video/imxfb.c |   53 -------------------------------------------------
 1 files changed, 0 insertions(+), 53 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index ccd9861..a990d43 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -357,57 +357,6 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 	return 0;
 }
 
-static void imxfb_setup_gpio(struct imxfb_info *fbi)
-{
-	int width;
-
-	LCDC_RMCR	&= ~(RMCR_LCDC_EN | RMCR_SELF_REF);
-
-	if( fbi->pcr & PCR_TFT )
-		width = 16;
-	else
-		width = 1 << ((fbi->pcr >> 28) & 0x3);
-
-	switch(width) {
-	case 16:
-		imx_gpio_mode(PD30_PF_LD15);
-		imx_gpio_mode(PD29_PF_LD14);
-		imx_gpio_mode(PD28_PF_LD13);
-		imx_gpio_mode(PD27_PF_LD12);
-		imx_gpio_mode(PD26_PF_LD11);
-		imx_gpio_mode(PD25_PF_LD10);
-		imx_gpio_mode(PD24_PF_LD9);
-		imx_gpio_mode(PD23_PF_LD8);
-	case 8:
-		imx_gpio_mode(PD22_PF_LD7);
-		imx_gpio_mode(PD21_PF_LD6);
-		imx_gpio_mode(PD20_PF_LD5);
-		imx_gpio_mode(PD19_PF_LD4);
-	case 4:
-		imx_gpio_mode(PD18_PF_LD3);
-		imx_gpio_mode(PD17_PF_LD2);
-	case 2:
-		imx_gpio_mode(PD16_PF_LD1);
-	case 1:
-		imx_gpio_mode(PD15_PF_LD0);
-	}
-
-	/* initialize GPIOs */
-	imx_gpio_mode(PD6_PF_LSCLK);
-	imx_gpio_mode(PD11_PF_CONTRAST);
-	imx_gpio_mode(PD14_PF_FLM_VSYNC);
-	imx_gpio_mode(PD13_PF_LP_HSYNC);
-	imx_gpio_mode(PD12_PF_ACD_OE);
-
-	/* These are only needed for Sharp HR TFT displays */
-	if (fbi->pcr & PCR_SHARP) {
-		imx_gpio_mode(PD7_PF_REV);
-		imx_gpio_mode(PD8_PF_CLS);
-		imx_gpio_mode(PD9_PF_PS);
-		imx_gpio_mode(PD10_PF_SPL_SPR);
-	}
-}
-
 #ifdef CONFIG_PM
 /*
  * Power management hooks.  Note that we won't be called from IRQ context,
@@ -594,8 +543,6 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto failed_cmap;
 
-	imxfb_setup_gpio(fbi);
-
 	imxfb_set_par(info);
 	ret = register_framebuffer(info);
 	if (ret < 0) {
-- 
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=/

Re: [PATCH] i.MX Framebuffer: remove gpio setup function

[Krzysztof Helt]

On Tue, 19 Aug 2008 17:06:40 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Remove the gpio mux setup function from i.MX framebuffer driver.
> This function is platform specific and thus should be done by
> the board setup. As there are currently no in-kernel users
> of this driver we do not break anything.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

This is patch type I like the most: only deletions.

Please number your patches with [PATCH 1/4], [PATCH 2/4], etc.
in next repost. Please also add line From: and description
to each patch.

>  drivers/video/imxfb.c |   53 -------------------------------------------------
>  1 files changed, 0 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index ccd9861..a990d43 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -357,57 +357,6 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
>  	return 0;
>  }
>  
> -static void imxfb_setup_gpio(struct imxfb_info *fbi)
> -{
> -	int width;
> -
> -	LCDC_RMCR	&= ~(RMCR_LCDC_EN | RMCR_SELF_REF);
> -
> -	if( fbi->pcr & PCR_TFT )
> -		width = 16;
> -	else
> -		width = 1 << ((fbi->pcr >> 28) & 0x3);
> -
> -	switch(width) {
> -	case 16:
> -		imx_gpio_mode(PD30_PF_LD15);
> -		imx_gpio_mode(PD29_PF_LD14);
> -		imx_gpio_mode(PD28_PF_LD13);
> -		imx_gpio_mode(PD27_PF_LD12);
> -		imx_gpio_mode(PD26_PF_LD11);
> -		imx_gpio_mode(PD25_PF_LD10);
> -		imx_gpio_mode(PD24_PF_LD9);
> -		imx_gpio_mode(PD23_PF_LD8);
> -	case 8:
> -		imx_gpio_mode(PD22_PF_LD7);
> -		imx_gpio_mode(PD21_PF_LD6);
> -		imx_gpio_mode(PD20_PF_LD5);
> -		imx_gpio_mode(PD19_PF_LD4);
> -	case 4:
> -		imx_gpio_mode(PD18_PF_LD3);
> -		imx_gpio_mode(PD17_PF_LD2);
> -	case 2:
> -		imx_gpio_mode(PD16_PF_LD1);
> -	case 1:
> -		imx_gpio_mode(PD15_PF_LD0);
> -	}
> -
> -	/* initialize GPIOs */
> -	imx_gpio_mode(PD6_PF_LSCLK);
> -	imx_gpio_mode(PD11_PF_CONTRAST);
> -	imx_gpio_mode(PD14_PF_FLM_VSYNC);
> -	imx_gpio_mode(PD13_PF_LP_HSYNC);
> -	imx_gpio_mode(PD12_PF_ACD_OE);
> -
> -	/* These are only needed for Sharp HR TFT displays */
> -	if (fbi->pcr & PCR_SHARP) {
> -		imx_gpio_mode(PD7_PF_REV);
> -		imx_gpio_mode(PD8_PF_CLS);
> -		imx_gpio_mode(PD9_PF_PS);
> -		imx_gpio_mode(PD10_PF_SPL_SPR);
> -	}
> -}
> -
>  #ifdef CONFIG_PM
>  /*
>   * Power management hooks.  Note that we won't be called from IRQ context,
> @@ -594,8 +543,6 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto failed_cmap;
>  
> -	imxfb_setup_gpio(fbi);
> -
>  	imxfb_set_par(info);
>  	ret = register_framebuffer(info);
>  	if (ret < 0) {
> -- 
> 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=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 

----------------------------------------------------------------------
Taniej na zagraniczne komorki!
Sprawdz >>>  http://link.interia.pl/f1ee3 


-------------------------------------------------------------------------
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=/

[PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Sascha Hauer]

From: Juergen Beisert <j.beisert@pengutronix.de>

This patch prepares the current i.MX1 framebuffer driver for usage in the
whole i.MX family. It switches to iowrite/ioread for register accesses.
Also it moves the register definitions to the driver where they belong.

Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
---
 drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
 drivers/video/imxfb.h |    4 +-
 2 files changed, 173 insertions(+), 39 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index a990d43..0622589 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -16,7 +16,6 @@
  *	linux-arm-kernel@lists.arm.linux.org.uk
  */
 
-//#define DEBUG 1
 
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -33,7 +32,6 @@
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 
-#include <mach/hardware.h>
 #include <asm/io.h>
 #include <mach/imxfb.h>
 
@@ -44,6 +42,124 @@
 
 #include "imxfb.h"
 
+#define DRIVER_NAME "imx-fb"
+
+#define LCDC_SSA	0x00
+
+#define LCDC_SIZE	0x04
+#define SIZE_XMAX(x)	((((x) >> 4) & 0x3f) << 20)
+#ifdef CONFIG_ARCH_IMX
+#define SIZE_YMAX(y)	((y) & 0x1ff)
+#else
+#define SIZE_YMAX(y)	((y) & 0x3ff)
+#endif
+
+#define LCDC_VPW	0x08
+#define VPW_VPW(x)	((x) & 0x3ff)
+
+#define LCDC_CPOS	0x0C
+#define CPOS_CC1	(1<<31)
+#define CPOS_CC0	(1<<30)
+#define CPOS_OP		(1<<28)
+#define CPOS_CXP(x)	(((x) & 3ff) << 16)
+#ifdef CONFIG_ARCH_IMX
+#define CPOS_CYP(y)	((y) & 0x1ff)
+#else
+#define CPOS_CYP(y)	((y) & 0x3ff)
+#endif
+
+#define LCDC_LCWHB	0x10
+#define LCWHB_BK_EN	(1<<31)
+#define LCWHB_CW(w)	(((w) & 0x1f) << 24)
+#define LCWHB_CH(h)	(((h) & 0x1f) << 16)
+#define LCWHB_BD(x)	((x) & 0xff)
+
+#define LCDC_LCHCC	0x14
+#ifdef CONFIG_ARCH_IMX
+#define LCHCC_CUR_COL_R(r) (((r) & 0x1f) << 11)
+#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 5)
+#define LCHCC_CUR_COL_B(b) ((b) & 0x1f)
+#else
+#define LCHCC_CUR_COL_R(r) (((r) & 0x3f) << 12)
+#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 6)
+#define LCHCC_CUR_COL_B(b) ((b) & 0x3f)
+#endif
+
+#define LCDC_PCR	0x18
+#define PCR_TFT		(1<<31)
+#define PCR_COLOR	(1<<30)
+#define PCR_PBSIZ_1	(0<<28)
+#define PCR_PBSIZ_2	(1<<28)
+#define PCR_PBSIZ_4	(2<<28)
+#define PCR_PBSIZ_8	(3<<28)
+#define PCR_BPIX_1	(0<<25)
+#define PCR_BPIX_2	(1<<25)
+#define PCR_BPIX_4	(2<<25)
+#define PCR_BPIX_8	(3<<25)
+#define PCR_BPIX_12	(4<<25)
+#define PCR_BPIX_16	(4<<25)
+#define PCR_PIXPOL	(1<<24)
+#define PCR_FLMPOL	(1<<23)
+#define PCR_LPPOL	(1<<22)
+#define PCR_CLKPOL	(1<<21)
+#define PCR_OEPOL	(1<<20)
+#define PCR_SCLKIDLE	(1<<19)
+#define PCR_END_SEL	(1<<18)
+#define PCR_END_BYTE_SWAP (1<<17)
+#define PCR_REV_VS	(1<<16)
+#define PCR_ACD_SEL	(1<<15)
+#define PCR_ACD(x)	(((x) & 0x7f) << 8)
+#define PCR_SCLK_SEL	(1<<7)
+#define PCR_SHARP	(1<<6)
+#define PCR_PCD(x)	((x) & 0x3f)
+
+#define LCDC_HCR	0x1C
+#define HCR_H_WIDTH(x)	(((x) & 0x3f) << 26)
+#define HCR_H_WAIT_1(x)	(((x) & 0xff) << 8)
+#define HCR_H_WAIT_2(x)	((x) & 0xff)
+
+#define LCDC_VCR	0x20
+#define VCR_V_WIDTH(x)	(((x) & 0x3f) << 26)
+#define VCR_V_WAIT_1(x)	(((x) & 0xff) << 8)
+#define VCR_V_WAIT_2(x)	((x) & 0xff)
+
+#define LCDC_POS	0x24
+#define POS_POS(x)	((x) & 1f)
+
+#define LCDC_LSCR1	0x28
+#define LSCR1_PS_RISE_DELAY(x)    (((x) & 0x7f) << 26)
+#define LSCR1_CLS_RISE_DELAY(x)   (((x) & 0x3f) << 16)
+#define LSCR1_REV_TOGGLE_DELAY(x) (((x) & 0xf) << 8)
+#define LSCR1_GRAY2(x)            (((x) & 0xf) << 4)
+#define LSCR1_GRAY1(x)            (((x) & 0xf))
+
+#define LCDC_PWMR	0x2C
+#define PWMR_CLS(x)	(((x) & 0x1ff) << 16)
+#define PWMR_LDMSK	(1<<15)
+#define PWMR_SCR1	(1<<10)
+#define PWMR_SCR0	(1<<9)
+#define PWMR_CC_EN	(1<<8)
+#define PWMR_PW(x)	((x) & 0xff)
+
+#define LCDC_DMACR	0x30
+#define DMACR_BURST	(1<<31)
+#define DMACR_HM(x)	(((x) & 0xf) << 16)
+#define DMACR_TM(x)	((x) &0xf)
+
+#define LCDC_RMCR	0x34
+#define RMCR_LCDC_EN	(1<<1)
+#define RMCR_SELF_REF	(1<<0)
+
+#define LCDC_LCDICR	0x38
+#define LCDICR_INT_SYN	(1<<2)
+#define LCDICR_INT_CON	(1)
+
+#define LCDC_LCDISR	0x40
+#define LCDISR_UDR_ERR	(1<<3)
+#define LCDISR_ERR_RES	(1<<2)
+#define LCDISR_EOF	(1<<1)
+#define LCDISR_BOF	(1<<0)
+
 static struct imxfb_rgb def_rgb_16 = {
 	.red	= { .offset = 8,  .length = 4, },
 	.green	= { .offset = 4,  .length = 4, },
@@ -67,7 +183,6 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 	return chan << bf->offset;
 }
 
-#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
 static int
 imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
 		       u_int trans, struct fb_info *info)
@@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
 		      (CNVT_TOHW(green,4) << 4) |
 		      CNVT_TOHW(blue,  4);
 
-		LCDC_PALETTE(regno) = val;
+		iowrite32(val, fbi->regs + 0x800 + (regno << 2));
 		ret = 0;
 	}
 	return ret;
@@ -235,18 +350,23 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 	pr_debug("Enabling LCD controller\n");
 
 	/* initialize LCDC */
-	LCDC_RMCR &= ~RMCR_LCDC_EN;		/* just to be safe... */
+	iowrite32(ioread32(fbi->regs + LCDC_RMCR) & ~RMCR_LCDC_EN,
+		fbi->regs + LCDC_RMCR);	/* just to be safe... */
+
+	iowrite32(fbi->screen_dma, fbi->regs + LCDC_SSA);
 
-	LCDC_SSA	= fbi->screen_dma;
 	/* physical screen start address	    */
-	LCDC_VPW	= VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4);
+	iowrite32(VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4),
+		fbi->regs + LCDC_VPW);
 
-	LCDC_POS	= 0x00000000;   /* panning offset 0 (0 pixel offset)        */
+	/* panning offset 0 (0 pixel offset)        */
+	iowrite32(0x00000000, fbi->regs + LCDC_POS);
 
 	/* disable hardware cursor */
-	LCDC_CPOS	&= ~(CPOS_CC0 | CPOS_CC1);
+	iowrite32(ioread32(fbi->regs + LCDC_CPOS) & ~(CPOS_CC0 | CPOS_CC1),
+		fbi->regs + LCDC_CPOS);
 
-	LCDC_RMCR = RMCR_LCDC_EN;
+	iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
 
 	if(fbi->backlight_power)
 		fbi->backlight_power(1);
@@ -263,7 +383,7 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 	if(fbi->lcd_power)
 		fbi->lcd_power(0);
 
-	LCDC_RMCR = 0;
+	iowrite32(0, fbi->regs + LCDC_RMCR);
 }
 
 static int imxfb_blank(int blank, struct fb_info *info)
@@ -340,19 +460,22 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 			info->fix.id, var->lower_margin);
 #endif
 
-	LCDC_HCR	= HCR_H_WIDTH(var->hsync_len) |
-	                  HCR_H_WAIT_1(var->left_margin) |
-			  HCR_H_WAIT_2(var->right_margin);
+	iowrite32(HCR_H_WIDTH(var->hsync_len) |
+		HCR_H_WAIT_1(var->left_margin) |
+		HCR_H_WAIT_2(var->right_margin),
+		fbi->regs + LCDC_HCR);
 
-	LCDC_VCR	= VCR_V_WIDTH(var->vsync_len) |
-	                  VCR_V_WAIT_1(var->upper_margin) |
-			  VCR_V_WAIT_2(var->lower_margin);
+	iowrite32(VCR_V_WIDTH(var->vsync_len) |
+		VCR_V_WAIT_1(var->upper_margin) |
+		VCR_V_WAIT_2(var->lower_margin),
+		fbi->regs + LCDC_VCR);
 
-	LCDC_SIZE	= SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres);
-	LCDC_PCR	= fbi->pcr;
-	LCDC_PWMR	= fbi->pwmr;
-	LCDC_LSCR1	= fbi->lscr1;
-	LCDC_DMACR	= fbi->dmacr;
+	iowrite32(SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres),
+			fbi->regs + LCDC_SIZE);
+	iowrite32(fbi->pcr, fbi->regs + LCDC_PCR);
+	iowrite32(fbi->pwmr, fbi->regs + LCDC_PWMR);
+	iowrite32(fbi->lscr1, fbi->regs + LCDC_LSCR1);
+	iowrite32(fbi->dmacr, fbi->regs + LCDC_DMACR);
 
 	return 0;
 }
@@ -384,10 +507,10 @@ static int imxfb_resume(struct platform_device *dev)
 #define imxfb_resume	NULL
 #endif
 
-static int __init imxfb_init_fbinfo(struct device *dev)
+static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 {
-	struct imxfb_mach_info *inf = dev->platform_data;
-	struct fb_info *info = dev_get_drvdata(dev);
+	struct imxfb_mach_info *inf = pdev->dev.platform_data;
+	struct fb_info *info = dev_get_drvdata(&pdev->dev);
 	struct imxfb_info *fbi = info->par;
 
 	pr_debug("%s\n",__func__);
@@ -397,7 +520,7 @@ static int __init imxfb_init_fbinfo(struct device *dev)
 		return -ENOMEM;
 
 	memset(fbi, 0, sizeof(struct imxfb_info));
-	fbi->dev = dev;
+	fbi->pdev = pdev;
 
 	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
 
@@ -465,7 +588,7 @@ static int __init imxfb_map_video_memory(struct fb_info *info)
 	struct imxfb_info *fbi = info->par;
 
 	fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
-	fbi->map_cpu = dma_alloc_writecombine(fbi->dev, fbi->map_size,
+	fbi->map_cpu = dma_alloc_writecombine(&fbi->pdev->dev, fbi->map_size,
 					&fbi->map_dma,GFP_KERNEL);
 
 	if (fbi->map_cpu) {
@@ -506,14 +629,21 @@ static int __init imxfb_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, info);
 
-	ret = imxfb_init_fbinfo(&pdev->dev);
+	ret = imxfb_init_fbinfo(pdev);
 	if( ret < 0 )
 		goto failed_init;
 
-	res = request_mem_region(res->start, res->end - res->start + 1, "IMXFB");
+	res = request_mem_region(res->start, res->end - res->start + 1,
+				DRIVER_NAME);
 	if (!res) {
 		ret = -EBUSY;
-		goto failed_regs;
+		goto failed_req;
+	}
+
+	fbi->regs = ioremap(res->start, res->end - res->start + 1);
+	if (fbi->regs == NULL) {
+		printk(KERN_ERR"Cannot map frame buffer registers\n");
+		goto failed_ioremap;
 	}
 
 	if (!inf->fixed_screen_cpu) {
@@ -559,18 +689,20 @@ failed_register:
 failed_cmap:
 	if (!inf->fixed_screen_cpu)
 		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
-		           fbi->map_dma);
+			fbi->map_dma);
 failed_map:
-	kfree(info->pseudo_palette);
-failed_regs:
+	iounmap(fbi->regs);
+failed_ioremap:
 	release_mem_region(res->start, res->end - res->start);
+failed_req:
+	kfree(info->pseudo_palette);
 failed_init:
 	platform_set_drvdata(pdev, NULL);
 	framebuffer_release(info);
 	return ret;
 }
 
-static int imxfb_remove(struct platform_device *pdev)
+static int __devexit imxfb_remove(struct platform_device *pdev)
 {
 	struct fb_info *info = platform_get_drvdata(pdev);
 	struct imxfb_info *fbi = info->par;
@@ -586,6 +718,7 @@ static int imxfb_remove(struct platform_device *pdev)
 	kfree(info->pseudo_palette);
 	framebuffer_release(info);
 
+	iounmap(fbi->regs);
 	release_mem_region(res->start, res->end - res->start + 1);
 	platform_set_drvdata(pdev, NULL);
 
@@ -600,19 +733,18 @@ void  imxfb_shutdown(struct platform_device * dev)
 }
 
 static struct platform_driver imxfb_driver = {
-	.probe		= imxfb_probe,
 	.suspend	= imxfb_suspend,
 	.resume		= imxfb_resume,
-	.remove		= imxfb_remove,
+	.remove		= __devexit_p(imxfb_remove),
 	.shutdown	= imxfb_shutdown,
 	.driver		= {
-		.name	= "imx-fb",
+		.name	= DRIVER_NAME,
 	},
 };
 
 int __init imxfb_init(void)
 {
-	return platform_driver_register(&imxfb_driver);
+	return platform_driver_probe(&imxfb_driver, imxfb_probe);
 }
 
 static void __exit imxfb_cleanup(void)
diff --git a/drivers/video/imxfb.h b/drivers/video/imxfb.h
index e837a8b..766297e 100644
--- a/drivers/video/imxfb.h
+++ b/drivers/video/imxfb.h
@@ -29,7 +29,9 @@ struct imxfb_rgb {
 #define NR_RGB	2
 
 struct imxfb_info {
-	struct device		*dev;
+	struct platform_device  *pdev;
+	void __iomem		*regs;
+
 	struct imxfb_rgb	*rgb[NR_RGB];
 
 	u_int			max_bpp;
-- 
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=/

Re: [PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Krzysztof Helt]

On Tue, 19 Aug 2008 17:06:41 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> From: Juergen Beisert <j.beisert@pengutronix.de>
> 
> This patch prepares the current i.MX1 framebuffer driver for usage in the
> whole i.MX family. It switches to iowrite/ioread for register accesses.
> Also it moves the register definitions to the driver where they belong.
> 
> Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
> ---
>  drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
>  drivers/video/imxfb.h |    4 +-
>  2 files changed, 173 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index a990d43..0622589 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -16,7 +16,6 @@
>   *	linux-arm-kernel@lists.arm.linux.org.uk
>   */
>  
> -//#define DEBUG 1
>  
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> @@ -33,7 +32,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/dma-mapping.h>
>  
> -#include <mach/hardware.h>
>  #include <asm/io.h>
>  #include <mach/imxfb.h>
>  
> @@ -44,6 +42,124 @@
>  
>  #include "imxfb.h"
>  
> +#define DRIVER_NAME "imx-fb"
> +

All constants below should really be put into the imxfb.h. There is already such a file.

> +#define LCDC_SSA	0x00
> +
> +#define LCDC_SIZE	0x04
> +#define SIZE_XMAX(x)	((((x) >> 4) & 0x3f) << 20)
> +#ifdef CONFIG_ARCH_IMX

Is it possible to use this driver without CONFIG_ARCH_IMX defined?

> +#define SIZE_YMAX(y)	((y) & 0x1ff)
> +#else
> +#define SIZE_YMAX(y)	((y) & 0x3ff)
> +#endif
> +
> +#define LCDC_VPW	0x08
> +#define VPW_VPW(x)	((x) & 0x3ff)
> +
> +#define LCDC_CPOS	0x0C
> +#define CPOS_CC1	(1<<31)
> +#define CPOS_CC0	(1<<30)
> +#define CPOS_OP		(1<<28)
> +#define CPOS_CXP(x)	(((x) & 3ff) << 16)
> +#ifdef CONFIG_ARCH_IMX
> +#define CPOS_CYP(y)	((y) & 0x1ff)
> +#else
> +#define CPOS_CYP(y)	((y) & 0x3ff)
> +#endif
> +
> +#define LCDC_LCWHB	0x10
> +#define LCWHB_BK_EN	(1<<31)
> +#define LCWHB_CW(w)	(((w) & 0x1f) << 24)
> +#define LCWHB_CH(h)	(((h) & 0x1f) << 16)
> +#define LCWHB_BD(x)	((x) & 0xff)
> +
> +#define LCDC_LCHCC	0x14
> +#ifdef CONFIG_ARCH_IMX
> +#define LCHCC_CUR_COL_R(r) (((r) & 0x1f) << 11)
> +#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 5)
> +#define LCHCC_CUR_COL_B(b) ((b) & 0x1f)
> +#else
> +#define LCHCC_CUR_COL_R(r) (((r) & 0x3f) << 12)
> +#define LCHCC_CUR_COL_G(g) (((g) & 0x3f) << 6)
> +#define LCHCC_CUR_COL_B(b) ((b) & 0x3f)
> +#endif
> +
> +#define LCDC_PCR	0x18
> +#define PCR_TFT		(1<<31)
> +#define PCR_COLOR	(1<<30)
> +#define PCR_PBSIZ_1	(0<<28)
> +#define PCR_PBSIZ_2	(1<<28)
> +#define PCR_PBSIZ_4	(2<<28)
> +#define PCR_PBSIZ_8	(3<<28)
> +#define PCR_BPIX_1	(0<<25)
> +#define PCR_BPIX_2	(1<<25)
> +#define PCR_BPIX_4	(2<<25)
> +#define PCR_BPIX_8	(3<<25)
> +#define PCR_BPIX_12	(4<<25)
> +#define PCR_BPIX_16	(4<<25)
> +#define PCR_PIXPOL	(1<<24)
> +#define PCR_FLMPOL	(1<<23)
> +#define PCR_LPPOL	(1<<22)
> +#define PCR_CLKPOL	(1<<21)
> +#define PCR_OEPOL	(1<<20)
> +#define PCR_SCLKIDLE	(1<<19)
> +#define PCR_END_SEL	(1<<18)
> +#define PCR_END_BYTE_SWAP (1<<17)
> +#define PCR_REV_VS	(1<<16)
> +#define PCR_ACD_SEL	(1<<15)
> +#define PCR_ACD(x)	(((x) & 0x7f) << 8)
> +#define PCR_SCLK_SEL	(1<<7)
> +#define PCR_SHARP	(1<<6)
> +#define PCR_PCD(x)	((x) & 0x3f)
> +
> +#define LCDC_HCR	0x1C
> +#define HCR_H_WIDTH(x)	(((x) & 0x3f) << 26)
> +#define HCR_H_WAIT_1(x)	(((x) & 0xff) << 8)
> +#define HCR_H_WAIT_2(x)	((x) & 0xff)
> +
> +#define LCDC_VCR	0x20
> +#define VCR_V_WIDTH(x)	(((x) & 0x3f) << 26)
> +#define VCR_V_WAIT_1(x)	(((x) & 0xff) << 8)
> +#define VCR_V_WAIT_2(x)	((x) & 0xff)
> +
> +#define LCDC_POS	0x24
> +#define POS_POS(x)	((x) & 1f)
> +
> +#define LCDC_LSCR1	0x28
> +#define LSCR1_PS_RISE_DELAY(x)    (((x) & 0x7f) << 26)
> +#define LSCR1_CLS_RISE_DELAY(x)   (((x) & 0x3f) << 16)
> +#define LSCR1_REV_TOGGLE_DELAY(x) (((x) & 0xf) << 8)
> +#define LSCR1_GRAY2(x)            (((x) & 0xf) << 4)
> +#define LSCR1_GRAY1(x)            (((x) & 0xf))
> +
> +#define LCDC_PWMR	0x2C
> +#define PWMR_CLS(x)	(((x) & 0x1ff) << 16)
> +#define PWMR_LDMSK	(1<<15)
> +#define PWMR_SCR1	(1<<10)
> +#define PWMR_SCR0	(1<<9)
> +#define PWMR_CC_EN	(1<<8)
> +#define PWMR_PW(x)	((x) & 0xff)
> +
> +#define LCDC_DMACR	0x30
> +#define DMACR_BURST	(1<<31)
> +#define DMACR_HM(x)	(((x) & 0xf) << 16)
> +#define DMACR_TM(x)	((x) &0xf)
> +
> +#define LCDC_RMCR	0x34
> +#define RMCR_LCDC_EN	(1<<1)
> +#define RMCR_SELF_REF	(1<<0)
> +
> +#define LCDC_LCDICR	0x38
> +#define LCDICR_INT_SYN	(1<<2)
> +#define LCDICR_INT_CON	(1)
> +
> +#define LCDC_LCDISR	0x40
> +#define LCDISR_UDR_ERR	(1<<3)
> +#define LCDISR_ERR_RES	(1<<2)
> +#define LCDISR_EOF	(1<<1)
> +#define LCDISR_BOF	(1<<0)
> +
>  static struct imxfb_rgb def_rgb_16 = {
>  	.red	= { .offset = 8,  .length = 4, },
>  	.green	= { .offset = 4,  .length = 4, },
> @@ -67,7 +183,6 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
>  	return chan << bf->offset;
>  }
>  
> -#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
>  static int
>  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
>  		       u_int trans, struct fb_info *info)
> @@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
>  		      (CNVT_TOHW(green,4) << 4) |
>  		      CNVT_TOHW(blue,  4);
>  
> -		LCDC_PALETTE(regno) = val;
> +		iowrite32(val, fbi->regs + 0x800 + (regno << 2));

One may define an inline function imxfb_write(fbi, val, reg) :

 imxfb_iowrite(fbi, val, reg) 
{
	iowrite32(val, fbi->regs + reg);
}

It would make the code more readable. The imxfb_ioread() could be used as well.

>  		ret = 0;
>  	}
>  	return ret;
> @@ -235,18 +350,23 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
>  	pr_debug("Enabling LCD controller\n");
>  
>  	/* initialize LCDC */
> -	LCDC_RMCR &= ~RMCR_LCDC_EN;		/* just to be safe... */
> +	iowrite32(ioread32(fbi->regs + LCDC_RMCR) & ~RMCR_LCDC_EN,
> +		fbi->regs + LCDC_RMCR);	/* just to be safe... */
> +
> +	iowrite32(fbi->screen_dma, fbi->regs + LCDC_SSA);
>  
> -	LCDC_SSA	= fbi->screen_dma;
>  	/* physical screen start address	    */
> -	LCDC_VPW	= VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4);
> +	iowrite32(VPW_VPW(fbi->max_xres * fbi->max_bpp / 8 / 4),
> +		fbi->regs + LCDC_VPW);
>  
> -	LCDC_POS	= 0x00000000;   /* panning offset 0 (0 pixel offset)        */
> +	/* panning offset 0 (0 pixel offset)        */
> +	iowrite32(0x00000000, fbi->regs + LCDC_POS);
>  
>  	/* disable hardware cursor */
> -	LCDC_CPOS	&= ~(CPOS_CC0 | CPOS_CC1);
> +	iowrite32(ioread32(fbi->regs + LCDC_CPOS) & ~(CPOS_CC0 | CPOS_CC1),
> +		fbi->regs + LCDC_CPOS);
>  
> -	LCDC_RMCR = RMCR_LCDC_EN;
> +	iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
>  
>  	if(fbi->backlight_power)
>  		fbi->backlight_power(1);
> @@ -263,7 +383,7 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
>  	if(fbi->lcd_power)
>  		fbi->lcd_power(0);
>  
> -	LCDC_RMCR = 0;
> +	iowrite32(0, fbi->regs + LCDC_RMCR);
>  }
>  
>  static int imxfb_blank(int blank, struct fb_info *info)
> @@ -340,19 +460,22 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
>  			info->fix.id, var->lower_margin);
>  #endif
>  
> -	LCDC_HCR	= HCR_H_WIDTH(var->hsync_len) |
> -	                  HCR_H_WAIT_1(var->left_margin) |
> -			  HCR_H_WAIT_2(var->right_margin);
> +	iowrite32(HCR_H_WIDTH(var->hsync_len) |
> +		HCR_H_WAIT_1(var->left_margin) |
> +		HCR_H_WAIT_2(var->right_margin),
> +		fbi->regs + LCDC_HCR);
>  
> -	LCDC_VCR	= VCR_V_WIDTH(var->vsync_len) |
> -	                  VCR_V_WAIT_1(var->upper_margin) |
> -			  VCR_V_WAIT_2(var->lower_margin);
> +	iowrite32(VCR_V_WIDTH(var->vsync_len) |
> +		VCR_V_WAIT_1(var->upper_margin) |
> +		VCR_V_WAIT_2(var->lower_margin),
> +		fbi->regs + LCDC_VCR);
>  
> -	LCDC_SIZE	= SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres);
> -	LCDC_PCR	= fbi->pcr;
> -	LCDC_PWMR	= fbi->pwmr;
> -	LCDC_LSCR1	= fbi->lscr1;
> -	LCDC_DMACR	= fbi->dmacr;
> +	iowrite32(SIZE_XMAX(var->xres) | SIZE_YMAX(var->yres),
> +			fbi->regs + LCDC_SIZE);
> +	iowrite32(fbi->pcr, fbi->regs + LCDC_PCR);
> +	iowrite32(fbi->pwmr, fbi->regs + LCDC_PWMR);
> +	iowrite32(fbi->lscr1, fbi->regs + LCDC_LSCR1);
> +	iowrite32(fbi->dmacr, fbi->regs + LCDC_DMACR);
>  
>  	return 0;
>  }
> @@ -384,10 +507,10 @@ static int imxfb_resume(struct platform_device *dev)
>  #define imxfb_resume	NULL
>  #endif
>  
> -static int __init imxfb_init_fbinfo(struct device *dev)
> +static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  {
> -	struct imxfb_mach_info *inf = dev->platform_data;
> -	struct fb_info *info = dev_get_drvdata(dev);
> +	struct imxfb_mach_info *inf = pdev->dev.platform_data;
> +	struct fb_info *info = dev_get_drvdata(&pdev->dev);
>  	struct imxfb_info *fbi = info->par;
>  
>  	pr_debug("%s\n",__func__);
> @@ -397,7 +520,7 @@ static int __init imxfb_init_fbinfo(struct device *dev)
>  		return -ENOMEM;
>  
>  	memset(fbi, 0, sizeof(struct imxfb_info));
> -	fbi->dev = dev;
> +	fbi->pdev = pdev;
>  
>  	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
>  
> @@ -465,7 +588,7 @@ static int __init imxfb_map_video_memory(struct fb_info *info)
>  	struct imxfb_info *fbi = info->par;
>  
>  	fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
> -	fbi->map_cpu = dma_alloc_writecombine(fbi->dev, fbi->map_size,
> +	fbi->map_cpu = dma_alloc_writecombine(&fbi->pdev->dev, fbi->map_size,

Use &pdev->dev instead &fbi->pdev->dev

>  					&fbi->map_dma,GFP_KERNEL);
>  
>  	if (fbi->map_cpu) {
> @@ -506,14 +629,21 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, info);
>  
> -	ret = imxfb_init_fbinfo(&pdev->dev);
> +	ret = imxfb_init_fbinfo(pdev);
>  	if( ret < 0 )
>  		goto failed_init;
>  
> -	res = request_mem_region(res->start, res->end - res->start + 1, "IMXFB");
> +	res = request_mem_region(res->start, res->end - res->start + 1,
> +				DRIVER_NAME);

There is a function resource_size(res) which does (res->end - res->start + 1).
It seems useful here and at few places below.

>  	if (!res) {
>  		ret = -EBUSY;
> -		goto failed_regs;
> +		goto failed_req;
> +	}
> +
> +	fbi->regs = ioremap(res->start, res->end - res->start + 1);
> +	if (fbi->regs == NULL) {
> +		printk(KERN_ERR"Cannot map frame buffer registers\n");
> +		goto failed_ioremap;
>  	}
>  
>  	if (!inf->fixed_screen_cpu) {
> @@ -559,18 +689,20 @@ failed_register:
>  failed_cmap:
>  	if (!inf->fixed_screen_cpu)
>  		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
> -		           fbi->map_dma);
> +			fbi->map_dma);
>  failed_map:
> -	kfree(info->pseudo_palette);
> -failed_regs:
> +	iounmap(fbi->regs);
> +failed_ioremap:
>  	release_mem_region(res->start, res->end - res->start);
> +failed_req:
> +	kfree(info->pseudo_palette);
>  failed_init:
>  	platform_set_drvdata(pdev, NULL);
>  	framebuffer_release(info);
>  	return ret;
>  }
>  
> -static int imxfb_remove(struct platform_device *pdev)
> +static int __devexit imxfb_remove(struct platform_device *pdev)
>  {
>  	struct fb_info *info = platform_get_drvdata(pdev);
>  	struct imxfb_info *fbi = info->par;
> @@ -586,6 +718,7 @@ static int imxfb_remove(struct platform_device *pdev)
>  	kfree(info->pseudo_palette);
>  	framebuffer_release(info);
>  
> +	iounmap(fbi->regs);

The fbi pointed does not exist here (after framebuffer_release(info)).

>  	release_mem_region(res->start, res->end - res->start + 1);
>  	platform_set_drvdata(pdev, NULL);
>  
> @@ -600,19 +733,18 @@ void  imxfb_shutdown(struct platform_device * dev)
>  }
>  
>  static struct platform_driver imxfb_driver = {
> -	.probe		= imxfb_probe,
>  	.suspend	= imxfb_suspend,
>  	.resume		= imxfb_resume,
> -	.remove		= imxfb_remove,
> +	.remove		= __devexit_p(imxfb_remove),
>  	.shutdown	= imxfb_shutdown,
>  	.driver		= {
> -		.name	= "imx-fb",
> +		.name	= DRIVER_NAME,
>  	},
>  };
>  
>  int __init imxfb_init(void)
>  {
> -	return platform_driver_register(&imxfb_driver);
> +	return platform_driver_probe(&imxfb_driver, imxfb_probe);
>  }
>  
>  static void __exit imxfb_cleanup(void)
> diff --git a/drivers/video/imxfb.h b/drivers/video/imxfb.h
> index e837a8b..766297e 100644
> --- a/drivers/video/imxfb.h
> +++ b/drivers/video/imxfb.h
> @@ -29,7 +29,9 @@ struct imxfb_rgb {
>  #define NR_RGB	2
>  
>  struct imxfb_info {
> -	struct device		*dev;
> +	struct platform_device  *pdev;

Is this field really useful? It can be dropped.

> +	void __iomem		*regs;
> +
>  	struct imxfb_rgb	*rgb[NR_RGB];
>  
>  	u_int			max_bpp;
> -- 
> 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=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 

---------------------------------------------------------------------- 
Igrzyska na Dzikim Zachodzie!
Sprawdz >> http://link.interia.pl/f1edc 
	


-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Sascha Hauer]

On Wed, Aug 20, 2008 at 05:31:59PM +0200, Krzysztof Helt wrote:
> On Tue, 19 Aug 2008 17:06:41 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > From: Juergen Beisert <j.beisert@pengutronix.de>
> > 
> > This patch prepares the current i.MX1 framebuffer driver for usage in the
> > whole i.MX family. It switches to iowrite/ioread for register accesses.
> > Also it moves the register definitions to the driver where they belong.
> > 
> > Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
> > ---
> >  drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
> >  drivers/video/imxfb.h |    4 +-
> >  2 files changed, 173 insertions(+), 39 deletions(-)
> > 
> > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > index a990d43..0622589 100644
> > --- a/drivers/video/imxfb.c
> > +++ b/drivers/video/imxfb.c
> > @@ -16,7 +16,6 @@
> >   *	linux-arm-kernel@lists.arm.linux.org.uk
> >   */
> >  
> > -//#define DEBUG 1
> >  
> >  #include <linux/module.h>
> >  #include <linux/kernel.h>
> > @@ -33,7 +32,6 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/dma-mapping.h>
> >  
> > -#include <mach/hardware.h>
> >  #include <asm/io.h>
> >  #include <mach/imxfb.h>
> >  
> > @@ -44,6 +42,124 @@
> >  
> >  #include "imxfb.h"
> >  
> > +#define DRIVER_NAME "imx-fb"
> > +
> 
> All constants below should really be put into the imxfb.h. There is already such a file.

AFAIK the-new-beautiful-way is not to have these header files next to
the .c files at all and put this stuff into the .c file. So maybe it's
better to move the rest of imxfb.h here aswell.


> 
> > +#define LCDC_SSA	0x00
> > +
> > +#define LCDC_SIZE	0x04
> > +#define SIZE_XMAX(x)	((((x) >> 4) & 0x3f) << 20)
> > +#ifdef CONFIG_ARCH_IMX
> 
> Is it possible to use this driver without CONFIG_ARCH_IMX defined?

Oops, not yet. These are the differences between IMX and MX2, the
architecture I'm currently preparing this driver for. This shouldn't
show up in this patch of course.


 >  
> > -#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
> >  static int
> >  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> >  		       u_int trans, struct fb_info *info)
> > @@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> >  		      (CNVT_TOHW(green,4) << 4) |
> >  		      CNVT_TOHW(blue,  4);
> >  
> > -		LCDC_PALETTE(regno) = val;
> > +		iowrite32(val, fbi->regs + 0x800 + (regno << 2));
> 
> One may define an inline function imxfb_write(fbi, val, reg) :
> 
>  imxfb_iowrite(fbi, val, reg) 
> {
> 	iowrite32(val, fbi->regs + reg);
> }
> 
> It would make the code more readable. The imxfb_ioread() could be used as well.
>

Well, I think this is a matter of taste, I prefer not using wrappers
around the access functions.



>  	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
> >  
> > @@ -465,7 +588,7 @@ static int __init imxfb_map_video_memory(struct fb_info *info)
> >  	struct imxfb_info *fbi = info->par;
> >  
> >  	fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
> > -	fbi->map_cpu = dma_alloc_writecombine(fbi->dev, fbi->map_size,
> > +	fbi->map_cpu = dma_alloc_writecombine(&fbi->pdev->dev, fbi->map_size,
> 
> Use &pdev->dev instead &fbi->pdev->dev

ok

> 
> >  					&fbi->map_dma,GFP_KERNEL);
> >  
> >  	if (fbi->map_cpu) {
> > @@ -506,14 +629,21 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, info);
> >  
> > -	ret = imxfb_init_fbinfo(&pdev->dev);
> > +	ret = imxfb_init_fbinfo(pdev);
> >  	if( ret < 0 )
> >  		goto failed_init;
> >  
> > -	res = request_mem_region(res->start, res->end - res->start + 1, "IMXFB");
> > +	res = request_mem_region(res->start, res->end - res->start + 1,
> > +				DRIVER_NAME);
> 
> There is a function resource_size(res) which does (res->end - res->start + 1).
> It seems useful here and at few places below.

ok, haven't seen it so far, thanks

 >  
> > -static int imxfb_remove(struct platform_device *pdev)
> > +static int __devexit imxfb_remove(struct platform_device *pdev)
> >  {
> >  	struct fb_info *info = platform_get_drvdata(pdev);
> >  	struct imxfb_info *fbi = info->par;
> > @@ -586,6 +718,7 @@ static int imxfb_remove(struct platform_device *pdev)
> >  	kfree(info->pseudo_palette);
> >  	framebuffer_release(info);
> >  
> > +	iounmap(fbi->regs);
> 
> The fbi pointed does not exist here (after framebuffer_release(info)).

Ack

 >  
> >  struct imxfb_info {
> > -	struct device		*dev;
> > +	struct platform_device  *pdev;
> 
> Is this field really useful? It can be dropped.

ok

-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Krzysztof Helt]

On Wed, 20 Aug 2008 18:31:09 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Wed, Aug 20, 2008 at 05:31:59PM +0200, Krzysztof Helt wrote:
> > On Tue, 19 Aug 2008 17:06:41 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > From: Juergen Beisert <j.beisert@pengutronix.de>
> > > 
> > > This patch prepares the current i.MX1 framebuffer driver for usage in the
> > > whole i.MX family. It switches to iowrite/ioread for register accesses.
> > > Also it moves the register definitions to the driver where they belong.
> > > 
> > > Signed-off-by: Juergen Beisert <j.beisert@pengutronix.de>
> > > ---
> > >  drivers/video/imxfb.c |  208 ++++++++++++++++++++++++++++++++++++++++---------
> > >  drivers/video/imxfb.h |    4 +-
> > >  2 files changed, 173 insertions(+), 39 deletions(-)
> > > 
> > > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > > index a990d43..0622589 100644
> > > --- a/drivers/video/imxfb.c
> > > +++ b/drivers/video/imxfb.c
> > > @@ -16,7 +16,6 @@
> > >   *	linux-arm-kernel@lists.arm.linux.org.uk
> > >   */
> > >  
> > > -//#define DEBUG 1
> > >  
> > >  #include <linux/module.h>
> > >  #include <linux/kernel.h>
> > > @@ -33,7 +32,6 @@
> > >  #include <linux/platform_device.h>
> > >  #include <linux/dma-mapping.h>
> > >  
> > > -#include <mach/hardware.h>
> > >  #include <asm/io.h>
> > >  #include <mach/imxfb.h>
> > >  
> > > @@ -44,6 +42,124 @@
> > >  
> > >  #include "imxfb.h"
> > >  
> > > +#define DRIVER_NAME "imx-fb"
> > > +
> > 
> > All constants below should really be put into the imxfb.h. There is already such a file.
> 
> AFAIK the-new-beautiful-way is not to have these header files next to
> the .c files at all and put this stuff into the .c file. So maybe it's
> better to move the rest of imxfb.h here aswell.
> 

I was not aware about "the-new-beautiful-way". I am just curious about this.
Normally, the headers for frame buffers are located at include/video. 

It can stay this way (no header) if you insist.

> 
>  >  
> > > -#define LCDC_PALETTE(x) __REG2(IMX_LCDC_BASE+0x800, (x)<<2)
> > >  static int
> > >  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> > >  		       u_int trans, struct fb_info *info)
> > > @@ -81,7 +196,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> > >  		      (CNVT_TOHW(green,4) << 4) |
> > >  		      CNVT_TOHW(blue,  4);
> > >  
> > > -		LCDC_PALETTE(regno) = val;
> > > +		iowrite32(val, fbi->regs + 0x800 + (regno << 2));
> > 
> > One may define an inline function imxfb_write(fbi, val, reg) :
> > 
> >  imxfb_iowrite(fbi, val, reg) 
> > {
> > 	iowrite32(val, fbi->regs + reg);
> > }
> > 
> > It would make the code more readable. The imxfb_ioread() could be used as well.
> >
> 
> Well, I think this is a matter of taste, I prefer not using wrappers
> around the access functions.
> 

Ok.

Regards,
Krzysztof

---------------------------------------------------------------------- 	
Prosty kredyt na wszystkie potrzeby. 
Od 1000 zl do 120 000 zl. 
Bez poreczycieli i zabezpieczen >>> http://link.interia.pl/f1edf


-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Use iowrite/ioread instead of direct pointer deref

[Geert Uytterhoeven]

On Wed, 20 Aug 2008, Krzysztof Helt wrote:
> On Wed, 20 Aug 2008 18:31:09 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Wed, Aug 20, 2008 at 05:31:59PM +0200, Krzysztof Helt wrote:
> > > On Tue, 19 Aug 2008 17:06:41 +0200
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > > +#define DRIVER_NAME "imx-fb"
> > > > +
> > > 
> > > All constants below should really be put into the imxfb.h. There is already such a file.
> > 
> > AFAIK the-new-beautiful-way is not to have these header files next to
> > the .c files at all and put this stuff into the .c file. So maybe it's
> > better to move the rest of imxfb.h here aswell.
> 
> I was not aware about "the-new-beautiful-way". I am just curious about this.
> Normally, the headers for frame buffers are located at include/video. 
> 
> It can stay this way (no header) if you insist.

If the header file is included by a single source file only, it doesn't
make much sense to have the separate header file.

include/video/ is for header files that are included by multiple source
files, some outside drivers/video/.

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=/

[PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
 1 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 0622589..e450e6b 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -1,6 +1,4 @@
 /*
- *  linux/drivers/video/imxfb.c
- *
  *  Freescale i.MX Frame Buffer device driver
  *
  *  Copyright (C) 2004 Sascha Hauer, Pengutronix
@@ -161,20 +159,21 @@
 #define LCDISR_BOF	(1<<0)
 
 static struct imxfb_rgb def_rgb_16 = {
-	.red	= { .offset = 8,  .length = 4, },
-	.green	= { .offset = 4,  .length = 4, },
-	.blue	= { .offset = 0,  .length = 4, },
-	.transp = { .offset = 0,  .length = 0, },
+	.red	= {.offset = 8, .length = 4,},
+	.green	= {.offset = 4, .length = 4,},
+	.blue	= {.offset = 0, .length = 4,},
+	.transp = {.offset = 0, .length = 0,},
 };
 
 static struct imxfb_rgb def_rgb_8 = {
-	.red	= { .offset = 0,  .length = 8, },
-	.green	= { .offset = 0,  .length = 8, },
-	.blue	= { .offset = 0,  .length = 8, },
-	.transp = { .offset = 0,  .length = 0, },
+	.red	= {.offset = 0, .length = 8,},
+	.green	= {.offset = 0, .length = 8,},
+	.blue	= {.offset = 0, .length = 8,},
+	.transp = {.offset = 0, .length = 0,},
 };
 
-static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *info);
+static int imxfb_activate_var(struct fb_var_screeninfo *var,
+		struct fb_info *info);
 
 static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 {
@@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 
 static int
 imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
-		       u_int trans, struct fb_info *info)
+		u_int trans, struct fb_info *info)
 {
 	struct imxfb_info *fbi = info->par;
 	u_int val, ret = 1;
@@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 
 	case FB_VISUAL_STATIC_PSEUDOCOLOR:
 	case FB_VISUAL_PSEUDOCOLOR:
-		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
+		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
+				info);
 		break;
 	}
 
@@ -336,8 +336,7 @@ static int imxfb_set_par(struct fb_info *info)
 		info->fix.visual = FB_VISUAL_STATIC_PSEUDOCOLOR;
 	}
 
-	info->fix.line_length = var->xres_virtual *
-				  var->bits_per_pixel / 8;
+	info->fix.line_length = var->xres_virtual * var->bits_per_pixel / 8;
 	fbi->palette_size = var->bits_per_pixel == 8 ? 256 : 16;
 
 	imxfb_activate_var(var, info);
@@ -368,9 +367,9 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 
 	iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
 
-	if(fbi->backlight_power)
+	if (fbi->backlight_power)
 		fbi->backlight_power(1);
-	if(fbi->lcd_power)
+	if (fbi->lcd_power)
 		fbi->lcd_power(1);
 }
 
@@ -378,9 +377,9 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 {
 	pr_debug("Disabling LCD controller\n");
 
-	if(fbi->backlight_power)
+	if (fbi->backlight_power)
 		fbi->backlight_power(0);
-	if(fbi->lcd_power)
+	if (fbi->lcd_power)
 		fbi->lcd_power(0);
 
 	iowrite32(0, fbi->regs + LCDC_RMCR);
@@ -488,7 +487,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct imxfb_info *fbi = platform_get_drvdata(dev);
-	pr_debug("%s\n",__func__);
+
+	pr_debug("%s\n", __func__);
 
 	imxfb_disable_controller(fbi);
 	return 0;
@@ -497,7 +497,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 static int imxfb_resume(struct platform_device *dev)
 {
 	struct imxfb_info *fbi = platform_get_drvdata(dev);
-	pr_debug("%s\n",__func__);
+
+	pr_debug("%s\n", __func__);
 
 	imxfb_enable_controller(fbi);
 	return 0;
@@ -515,31 +516,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 
 	pr_debug("%s\n",__func__);
 
-	info->pseudo_palette = kmalloc( sizeof(u32) * 16, GFP_KERNEL);
+	info->pseudo_palette = kmalloc(sizeof (u32) * 16, GFP_KERNEL);
 	if (!info->pseudo_palette)
 		return -ENOMEM;
 
-	memset(fbi, 0, sizeof(struct imxfb_info));
+	memset(fbi, 0, sizeof (struct imxfb_info));
 	fbi->pdev = pdev;
 
-	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
+	strlcpy(info->fix.id, IMX_NAME, sizeof (info->fix.id));
 
-	info->fix.type	= FB_TYPE_PACKED_PIXELS;
+	info->fix.type			= FB_TYPE_PACKED_PIXELS;
 	info->fix.type_aux		= 0;
 	info->fix.xpanstep		= 0;
 	info->fix.ypanstep		= 0;
 	info->fix.ywrapstep		= 0;
-	info->fix.accel	= FB_ACCEL_NONE;
+	info->fix.accel			= FB_ACCEL_NONE;
 
 	info->var.nonstd		= 0;
 	info->var.activate		= FB_ACTIVATE_NOW;
 	info->var.height		= -1;
 	info->var.width	= -1;
 	info->var.accel_flags		= 0;
-	info->var.vmode	= FB_VMODE_NONINTERLACED;
+	info->var.vmode			= FB_VMODE_NONINTERLACED;
 
 	info->fbops			= &imxfb_ops;
-	info->flags			= FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST;
+	info->flags			= FBINFO_FLAG_DEFAULT |
+					  FBINFO_READS_FAST;
 
 	fbi->rgb[RGB_16]		= &def_rgb_16;
 	fbi->rgb[RGB_8]			= &def_rgb_8;
@@ -577,11 +579,11 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 }
 
 /*
- *      Allocates the DRAM memory for the frame buffer.  This buffer is
- *	remapped into a non-cached, non-buffered, memory region to
- *      allow pixel writes to occur without flushing the cache.
- *      Once this area is remapped, all virtual memory access to the
- *      video memory should occur at the new region.
+ * Allocates the DRAM memory for the frame buffer.  This buffer is
+ * remapped into a non-cached, non-buffered, memory region to
+ * allow pixel writes to occur without flushing the cache.
+ * Once this area is remapped, all virtual memory access to the
+ * video memory should occur at the new region.
  */
 static int __init imxfb_map_video_memory(struct fb_info *info)
 {
@@ -612,17 +614,17 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	printk("i.MX Framebuffer driver\n");
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if(!res)
+	if (!res)
 		return -ENODEV;
 
 	inf = pdev->dev.platform_data;
-	if(!inf) {
+	if (!inf) {
 		dev_err(&pdev->dev,"No platform_data available\n");
 		return -ENOMEM;
 	}
 
 	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
-	if(!info)
+	if (!info)
 		return -ENOMEM;
 
 	fbi = info->par;
@@ -630,7 +632,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, info);
 
 	ret = imxfb_init_fbinfo(pdev);
-	if( ret < 0 )
+	if (ret < 0)
 		goto failed_init;
 
 	res = request_mem_region(res->start, res->end - res->start + 1,
@@ -669,7 +671,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	 */
 	imxfb_check_var(&info->var, info);
 
-	ret = fb_alloc_cmap(&info->cmap, 1<<info->var.bits_per_pixel, 0);
+	ret = fb_alloc_cmap(&info->cmap, 1 << info->var.bits_per_pixel, 0);
 	if (ret < 0)
 		goto failed_cmap;
 
-- 
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Krzysztof Helt]

On Tue, 19 Aug 2008 17:06:42 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
>  1 files changed, 40 insertions(+), 38 deletions(-)
> 

I don't like some of the changes like wrapping lines
which are not longer than 80 characters or removing
alignment vs previous line because it contains spaces.
There are also positive CS changes so not everything is bad.

> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index 0622589..e450e6b 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -1,6 +1,4 @@
>  /*
> - *  linux/drivers/video/imxfb.c
> - *
>   *  Freescale i.MX Frame Buffer device driver
>   *
>   *  Copyright (C) 2004 Sascha Hauer, Pengutronix
> @@ -161,20 +159,21 @@
>  #define LCDISR_BOF	(1<<0)
>  
>  static struct imxfb_rgb def_rgb_16 = {
> -	.red	= { .offset = 8,  .length = 4, },
> -	.green	= { .offset = 4,  .length = 4, },
> -	.blue	= { .offset = 0,  .length = 4, },
> -	.transp = { .offset = 0,  .length = 0, },
> +	.red	= {.offset = 8, .length = 4,},
> +	.green	= {.offset = 4, .length = 4,},
> +	.blue	= {.offset = 0, .length = 4,},
> +	.transp = {.offset = 0, .length = 0,},
>  };
>  
>  static struct imxfb_rgb def_rgb_8 = {
> -	.red	= { .offset = 0,  .length = 8, },
> -	.green	= { .offset = 0,  .length = 8, },
> -	.blue	= { .offset = 0,  .length = 8, },
> -	.transp = { .offset = 0,  .length = 0, },
> +	.red	= {.offset = 0, .length = 8,},
> +	.green	= {.offset = 0, .length = 8,},
> +	.blue	= {.offset = 0, .length = 8,},
> +	.transp = {.offset = 0, .length = 0,},
>  };
>  
> -static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *info);
> +static int imxfb_activate_var(struct fb_var_screeninfo *var,
> +		struct fb_info *info);
>  
>  static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
>  {
> @@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
>  
>  static int
>  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> -		       u_int trans, struct fb_info *info)
> +		u_int trans, struct fb_info *info)
>  {
>  	struct imxfb_info *fbi = info->par;
>  	u_int val, ret = 1;
> @@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
>  
>  	case FB_VISUAL_STATIC_PSEUDOCOLOR:
>  	case FB_VISUAL_PSEUDOCOLOR:
> -		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
> +		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
> +				info);
>  		break;
>  	}
>  
> @@ -336,8 +336,7 @@ static int imxfb_set_par(struct fb_info *info)
>  		info->fix.visual = FB_VISUAL_STATIC_PSEUDOCOLOR;
>  	}
>  
> -	info->fix.line_length = var->xres_virtual *
> -				  var->bits_per_pixel / 8;
> +	info->fix.line_length = var->xres_virtual * var->bits_per_pixel / 8;
>  	fbi->palette_size = var->bits_per_pixel == 8 ? 256 : 16;
>  
>  	imxfb_activate_var(var, info);
> @@ -368,9 +367,9 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
>  
>  	iowrite32(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
>  
> -	if(fbi->backlight_power)
> +	if (fbi->backlight_power)
>  		fbi->backlight_power(1);
> -	if(fbi->lcd_power)
> +	if (fbi->lcd_power)
>  		fbi->lcd_power(1);
>  }
>  
> @@ -378,9 +377,9 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
>  {
>  	pr_debug("Disabling LCD controller\n");
>  
> -	if(fbi->backlight_power)
> +	if (fbi->backlight_power)
>  		fbi->backlight_power(0);
> -	if(fbi->lcd_power)
> +	if (fbi->lcd_power)
>  		fbi->lcd_power(0);
>  
>  	iowrite32(0, fbi->regs + LCDC_RMCR);
> @@ -488,7 +487,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
>  static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
>  {
>  	struct imxfb_info *fbi = platform_get_drvdata(dev);
> -	pr_debug("%s\n",__func__);
> +
> +	pr_debug("%s\n", __func__);
>  
>  	imxfb_disable_controller(fbi);
>  	return 0;
> @@ -497,7 +497,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
>  static int imxfb_resume(struct platform_device *dev)
>  {
>  	struct imxfb_info *fbi = platform_get_drvdata(dev);
> -	pr_debug("%s\n",__func__);
> +
> +	pr_debug("%s\n", __func__);
>  
>  	imxfb_enable_controller(fbi);
>  	return 0;
> @@ -515,31 +516,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  
>  	pr_debug("%s\n",__func__);
>  
> -	info->pseudo_palette = kmalloc( sizeof(u32) * 16, GFP_KERNEL);
> +	info->pseudo_palette = kmalloc(sizeof (u32) * 16, GFP_KERNEL);
>  	if (!info->pseudo_palette)
>  		return -ENOMEM;
>  
> -	memset(fbi, 0, sizeof(struct imxfb_info));
> +	memset(fbi, 0, sizeof (struct imxfb_info));
>  	fbi->pdev = pdev;
>  
> -	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
> +	strlcpy(info->fix.id, IMX_NAME, sizeof (info->fix.id));
>  
> -	info->fix.type	= FB_TYPE_PACKED_PIXELS;
> +	info->fix.type			= FB_TYPE_PACKED_PIXELS;
>  	info->fix.type_aux		= 0;
>  	info->fix.xpanstep		= 0;
>  	info->fix.ypanstep		= 0;
>  	info->fix.ywrapstep		= 0;
> -	info->fix.accel	= FB_ACCEL_NONE;
> +	info->fix.accel			= FB_ACCEL_NONE;
>  
>  	info->var.nonstd		= 0;
>  	info->var.activate		= FB_ACTIVATE_NOW;
>  	info->var.height		= -1;
>  	info->var.width	= -1;
>  	info->var.accel_flags		= 0;
> -	info->var.vmode	= FB_VMODE_NONINTERLACED;
> +	info->var.vmode			= FB_VMODE_NONINTERLACED;
>  
>  	info->fbops			= &imxfb_ops;
> -	info->flags			= FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST;
> +	info->flags			= FBINFO_FLAG_DEFAULT |
> +					  FBINFO_READS_FAST;
>  
>  	fbi->rgb[RGB_16]		= &def_rgb_16;
>  	fbi->rgb[RGB_8]			= &def_rgb_8;
> @@ -577,11 +579,11 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  }
>  
>  /*
> - *      Allocates the DRAM memory for the frame buffer.  This buffer is
> - *	remapped into a non-cached, non-buffered, memory region to
> - *      allow pixel writes to occur without flushing the cache.
> - *      Once this area is remapped, all virtual memory access to the
> - *      video memory should occur at the new region.
> + * Allocates the DRAM memory for the frame buffer.  This buffer is
> + * remapped into a non-cached, non-buffered, memory region to
> + * allow pixel writes to occur without flushing the cache.
> + * Once this area is remapped, all virtual memory access to the
> + * video memory should occur at the new region.
>   */
>  static int __init imxfb_map_video_memory(struct fb_info *info)
>  {
> @@ -612,17 +614,17 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  	printk("i.MX Framebuffer driver\n");
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if(!res)
> +	if (!res)
>  		return -ENODEV;
>  
>  	inf = pdev->dev.platform_data;
> -	if(!inf) {
> +	if (!inf) {
>  		dev_err(&pdev->dev,"No platform_data available\n");
>  		return -ENOMEM;
>  	}
>  
>  	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
> -	if(!info)
> +	if (!info)
>  		return -ENOMEM;
>  
>  	fbi = info->par;
> @@ -630,7 +632,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, info);
>  
>  	ret = imxfb_init_fbinfo(pdev);
> -	if( ret < 0 )
> +	if (ret < 0)
>  		goto failed_init;
>  
>  	res = request_mem_region(res->start, res->end - res->start + 1,
> @@ -669,7 +671,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  	 */
>  	imxfb_check_var(&info->var, info);
>  
> -	ret = fb_alloc_cmap(&info->cmap, 1<<info->var.bits_per_pixel, 0);
> +	ret = fb_alloc_cmap(&info->cmap, 1 << info->var.bits_per_pixel, 0);
>  	if (ret < 0)
>  		goto failed_cmap;
>  
> -- 
> 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=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 

----------------------------------------------------------------------
Taniej na zagraniczne komorki!
Sprawdz >>>  http://link.interia.pl/f1ee3 


-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

On Wed, Aug 20, 2008 at 05:35:01PM +0200, Krzysztof Helt wrote:
> On Tue, 19 Aug 2008 17:06:42 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
> >  1 files changed, 40 insertions(+), 38 deletions(-)
> > 
> 
> I don't like some of the changes like wrapping lines
> which are not longer than 80 characters or removing
> alignment vs previous line because it contains spaces.
> There are also positive CS changes so not everything is bad.

Hm, this patch only wraps lines longer than 80 characters (in one case
it even merges two lines to one because it's still shorter than 80
chars). It also does not remove alignment vs the previous line, instead
it aligns some lines against the previous line. Maybe reading whitespace
changes in patches is a bit confusing because the +/- at line starts
mess it up.

Sascha

-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Krzysztof Helt]

On Wed, 20 Aug 2008 18:15:21 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Wed, Aug 20, 2008 at 05:35:01PM +0200, Krzysztof Helt wrote:
> > On Tue, 19 Aug 2008 17:06:42 +0200
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > >  drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
> > >  1 files changed, 40 insertions(+), 38 deletions(-)
> > > 
> > 
> > I don't like some of the changes like wrapping lines
> > which are not longer than 80 characters or removing
> > alignment vs previous line because it contains spaces.
> > There are also positive CS changes so not everything is bad.
> 
> Hm, this patch only wraps lines longer than 80 characters (in one case
> it even merges two lines to one because it's still shorter than 80
> chars).

The change around line 250 wraps 80 chars line.

@@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 
 	case FB_VISUAL_STATIC_PSEUDOCOLOR:
 	case FB_VISUAL_PSEUDOCOLOR:
-		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
+		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
+				info);
 		break;
 	}


> It also does not remove alignment vs the previous line, instead
> it aligns some lines against the previous line. 


@@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)

 static int
 imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
-                      u_int trans, struct fb_info *info)
+               u_int trans, struct fb_info *info)
 {
        struct imxfb_info *fbi = info->par;
        u_int val, ret = 1;

The removed line was previously aligned to start after the "(" in the previous line.
One could move "static int" into the same line.

You have added spaces between sizeof operator and the opening parenthesis.
It is pointed out by the checkpatch script.

> Maybe reading whitespace
> changes in patches is a bit confusing because the +/- at line starts
> mess it up.
>
 
The changes I pointed above are ones I don't like. They are not
bringing any benefit (nor better CS conformance nor readability).

However, I recognize your patch does more positive changes then these
pointed above. Also, my feelings toward some changes are my personal
opinion and I won't block the patch you have posted. I would like
to see the mentioned changes dropped but it is up to you.

Friendly regards,
Krzysztof

---------------------------------------------------------------------- 
Igrzyska na Dzikim Zachodzie!
Sprawdz >> http://link.interia.pl/f1edc 
	


-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

On Wed, Aug 20, 2008 at 07:35:43PM +0200, Krzysztof Helt wrote:
> On Wed, 20 Aug 2008 18:15:21 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > On Wed, Aug 20, 2008 at 05:35:01PM +0200, Krzysztof Helt wrote:
> > > On Tue, 19 Aug 2008 17:06:42 +0200
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > 
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > ---
> > > >  drivers/video/imxfb.c |   78 +++++++++++++++++++++++++------------------------
> > > >  1 files changed, 40 insertions(+), 38 deletions(-)
> > > > 
> > > 
> > > I don't like some of the changes like wrapping lines
> > > which are not longer than 80 characters or removing
> > > alignment vs previous line because it contains spaces.
> > > There are also positive CS changes so not everything is bad.
> > 
> > Hm, this patch only wraps lines longer than 80 characters (in one case
> > it even merges two lines to one because it's still shorter than 80
> > chars).
> 
> The change around line 250 wraps 80 chars line.
> 
> @@ -250,7 +249,8 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
>  
>  	case FB_VISUAL_STATIC_PSEUDOCOLOR:
>  	case FB_VISUAL_PSEUDOCOLOR:
> -		ret = imxfb_setpalettereg(regno, red, green, blue, trans, info);
> +		ret = imxfb_setpalettereg(regno, red, green, blue, trans,
> +				info);

Ok, you're right. I just saw my cursor blinking on position 81, but of
course that means the line is 80 characters long. My bad

>  		break;
>  	}
> 
> 
> > It also does not remove alignment vs the previous line, instead
> > it aligns some lines against the previous line. 
> 
> 
> @@ -185,7 +184,7 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
> 
>  static int
>  imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
> -                      u_int trans, struct fb_info *info)
> +               u_int trans, struct fb_info *info)
>  {
>         struct imxfb_info *fbi = info->par;
>         u_int val, ret = 1;
> 
> The removed line was previously aligned to start after the "(" in the previous line.
> One could move "static int" into the same line.
> 
> You have added spaces between sizeof operator and the opening parenthesis.
> It is pointed out by the checkpatch script.

Ah, yes. This one I have copied from a Lindent run.

> 
> > Maybe reading whitespace
> > changes in patches is a bit confusing because the +/- at line starts
> > mess it up.
> >
>  
> The changes I pointed above are ones I don't like. They are not
> bringing any benefit (nor better CS conformance nor readability).
> 
> However, I recognize your patch does more positive changes then these
> pointed above. Also, my feelings toward some changes are my personal
> opinion and I won't block the patch you have posted. I would like
> to see the mentioned changes dropped but it is up to you.

I will rework it.

Regards,
  Sascha


-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
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=/

[PATCH] i.MX Framebuffer: rename imxfb_mach_info to imx_fb_platform_data

[Sascha Hauer]

rename imxfb_mach_info to a name more common to kernel hackers

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/mach-imx/include/mach/imxfb.h |    5 +-
 drivers/video/imxfb.c                  |   68 ++++++++++++++++----------------
 2 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-imx/include/mach/imxfb.h b/arch/arm/mach-imx/include/mach/imxfb.h
index 3ed9ec8..ffcff5a 100644
--- a/arch/arm/mach-imx/include/mach/imxfb.h
+++ b/arch/arm/mach-imx/include/mach/imxfb.h
@@ -1,7 +1,7 @@
 /*
  * This structure describes the machine which we are running on.
  */
-struct imxfb_mach_info {
+struct imx_fb_platform_data {
 	u_long		pixclock;
 
 	u_short		xres;
@@ -34,4 +34,5 @@ struct imxfb_mach_info {
 	void (*lcd_power)(int);
 	void (*backlight_power)(int);
 };
-void set_imx_fb_info(struct imxfb_mach_info *hard_imx_fb_info);
+
+void set_imx_fb_info(struct imx_fb_platform_data *);
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index e450e6b..a4a4940 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -510,7 +510,7 @@ static int imxfb_resume(struct platform_device *dev)
 
 static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 {
-	struct imxfb_mach_info *inf = pdev->dev.platform_data;
+	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
 	struct fb_info *info = dev_get_drvdata(&pdev->dev);
 	struct imxfb_info *fbi = info->par;
 
@@ -546,32 +546,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 	fbi->rgb[RGB_16]		= &def_rgb_16;
 	fbi->rgb[RGB_8]			= &def_rgb_8;
 
-	fbi->max_xres			= inf->xres;
-	info->var.xres			= inf->xres;
-	info->var.xres_virtual		= inf->xres;
-	fbi->max_yres			= inf->yres;
-	info->var.yres			= inf->yres;
-	info->var.yres_virtual		= inf->yres;
-	fbi->max_bpp			= inf->bpp;
-	info->var.bits_per_pixel	= inf->bpp;
-	info->var.nonstd		= inf->nonstd;
-	info->var.pixclock		= inf->pixclock;
-	info->var.hsync_len		= inf->hsync_len;
-	info->var.left_margin		= inf->left_margin;
-	info->var.right_margin		= inf->right_margin;
-	info->var.vsync_len		= inf->vsync_len;
-	info->var.upper_margin		= inf->upper_margin;
-	info->var.lower_margin		= inf->lower_margin;
-	info->var.sync			= inf->sync;
-	info->var.grayscale		= inf->cmap_greyscale;
-	fbi->cmap_inverse		= inf->cmap_inverse;
-	fbi->cmap_static		= inf->cmap_static;
-	fbi->pcr			= inf->pcr;
-	fbi->lscr1			= inf->lscr1;
-	fbi->dmacr			= inf->dmacr;
-	fbi->pwmr			= inf->pwmr;
-	fbi->lcd_power			= inf->lcd_power;
-	fbi->backlight_power		= inf->backlight_power;
+	fbi->max_xres			= pdata->xres;
+	info->var.xres			= pdata->xres;
+	info->var.xres_virtual		= pdata->xres;
+	fbi->max_yres			= pdata->yres;
+	info->var.yres			= pdata->yres;
+	info->var.yres_virtual		= pdata->yres;
+	fbi->max_bpp			= pdata->bpp;
+	info->var.bits_per_pixel	= pdata->bpp;
+	info->var.nonstd		= pdata->nonstd;
+	info->var.pixclock		= pdata->pixclock;
+	info->var.hsync_len		= pdata->hsync_len;
+	info->var.left_margin		= pdata->left_margin;
+	info->var.right_margin		= pdata->right_margin;
+	info->var.vsync_len		= pdata->vsync_len;
+	info->var.upper_margin		= pdata->upper_margin;
+	info->var.lower_margin		= pdata->lower_margin;
+	info->var.sync			= pdata->sync;
+	info->var.grayscale		= pdata->cmap_greyscale;
+	fbi->cmap_inverse		= pdata->cmap_inverse;
+	fbi->cmap_static		= pdata->cmap_static;
+	fbi->pcr			= pdata->pcr;
+	fbi->lscr1			= pdata->lscr1;
+	fbi->dmacr			= pdata->dmacr;
+	fbi->pwmr			= pdata->pwmr;
+	fbi->lcd_power			= pdata->lcd_power;
+	fbi->backlight_power		= pdata->backlight_power;
 	info->fix.smem_len		= fbi->max_xres * fbi->max_yres *
 					  fbi->max_bpp / 8;
 
@@ -607,7 +607,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 {
 	struct imxfb_info *fbi;
 	struct fb_info *info;
-	struct imxfb_mach_info *inf;
+	struct imx_fb_platform_data *pdata;
 	struct resource *res;
 	int ret;
 
@@ -617,8 +617,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	if (!res)
 		return -ENODEV;
 
-	inf = pdev->dev.platform_data;
-	if (!inf) {
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
 		dev_err(&pdev->dev,"No platform_data available\n");
 		return -ENOMEM;
 	}
@@ -648,7 +648,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 		goto failed_ioremap;
 	}
 
-	if (!inf->fixed_screen_cpu) {
+	if (!pdata->fixed_screen_cpu) {
 		ret = imxfb_map_video_memory(info);
 		if (ret) {
 			dev_err(&pdev->dev, "Failed to allocate video RAM: %d\n", ret);
@@ -657,8 +657,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
 		}
 	} else {
 		/* Fixed framebuffer mapping enables location of the screen in eSRAM */
-		fbi->map_cpu = inf->fixed_screen_cpu;
-		fbi->map_dma = inf->fixed_screen_dma;
+		fbi->map_cpu = pdata->fixed_screen_cpu;
+		fbi->map_dma = pdata->fixed_screen_dma;
 		info->screen_base = fbi->map_cpu;
 		fbi->screen_cpu = fbi->map_cpu;
 		fbi->screen_dma = fbi->map_dma;
@@ -689,7 +689,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 failed_register:
 	fb_dealloc_cmap(&info->cmap);
 failed_cmap:
-	if (!inf->fixed_screen_cpu)
+	if (!pdata->fixed_screen_cpu)
 		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
 			fbi->map_dma);
 failed_map:
-- 
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=/

Re: [PATCH] i.MX Framebuffer: rename imxfb_mach_info to imx_fb_platform_data

[Krzysztof Helt]

On Tue, 19 Aug 2008 17:06:43 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> rename imxfb_mach_info to a name more common to kernel hackers
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/mach-imx/include/mach/imxfb.h |    5 +-
>  drivers/video/imxfb.c                  |   68 ++++++++++++++++----------------
>  2 files changed, 37 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/include/mach/imxfb.h b/arch/arm/mach-imx/include/mach/imxfb.h
> index 3ed9ec8..ffcff5a 100644
> --- a/arch/arm/mach-imx/include/mach/imxfb.h
> +++ b/arch/arm/mach-imx/include/mach/imxfb.h
> @@ -1,7 +1,7 @@
>  /*
>   * This structure describes the machine which we are running on.
>   */
> -struct imxfb_mach_info {
> +struct imx_fb_platform_data {
>  	u_long		pixclock;
>  
>  	u_short		xres;
> @@ -34,4 +34,5 @@ struct imxfb_mach_info {
>  	void (*lcd_power)(int);
>  	void (*backlight_power)(int);
>  };
> -void set_imx_fb_info(struct imxfb_mach_info *hard_imx_fb_info);
> +
> +void set_imx_fb_info(struct imx_fb_platform_data *);
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index e450e6b..a4a4940 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -510,7 +510,7 @@ static int imxfb_resume(struct platform_device *dev)
>  
>  static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  {
> -	struct imxfb_mach_info *inf = pdev->dev.platform_data;
> +	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
>  	struct fb_info *info = dev_get_drvdata(&pdev->dev);
>  	struct imxfb_info *fbi = info->par;
>  
> @@ -546,32 +546,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  	fbi->rgb[RGB_16]		= &def_rgb_16;
>  	fbi->rgb[RGB_8]			= &def_rgb_8;
>  
> -	fbi->max_xres			= inf->xres;
> -	info->var.xres			= inf->xres;
> -	info->var.xres_virtual		= inf->xres;
> -	fbi->max_yres			= inf->yres;
> -	info->var.yres			= inf->yres;
> -	info->var.yres_virtual		= inf->yres;
> -	fbi->max_bpp			= inf->bpp;
> -	info->var.bits_per_pixel	= inf->bpp;
> -	info->var.nonstd		= inf->nonstd;
> -	info->var.pixclock		= inf->pixclock;
> -	info->var.hsync_len		= inf->hsync_len;
> -	info->var.left_margin		= inf->left_margin;
> -	info->var.right_margin		= inf->right_margin;
> -	info->var.vsync_len		= inf->vsync_len;
> -	info->var.upper_margin		= inf->upper_margin;
> -	info->var.lower_margin		= inf->lower_margin;
> -	info->var.sync			= inf->sync;
> -	info->var.grayscale		= inf->cmap_greyscale;
> -	fbi->cmap_inverse		= inf->cmap_inverse;
> -	fbi->cmap_static		= inf->cmap_static;
> -	fbi->pcr			= inf->pcr;
> -	fbi->lscr1			= inf->lscr1;
> -	fbi->dmacr			= inf->dmacr;
> -	fbi->pwmr			= inf->pwmr;
> -	fbi->lcd_power			= inf->lcd_power;
> -	fbi->backlight_power		= inf->backlight_power;
> +	fbi->max_xres			= pdata->xres;
> +	info->var.xres			= pdata->xres;
> +	info->var.xres_virtual		= pdata->xres;
> +	fbi->max_yres			= pdata->yres;
> +	info->var.yres			= pdata->yres;
> +	info->var.yres_virtual		= pdata->yres;
> +	fbi->max_bpp			= pdata->bpp;
> +	info->var.bits_per_pixel	= pdata->bpp;

These max_foo fields seem redundant. Especially, the max_bpp.
Most ARM chips can handle any bpp regardless the connected 
display (I don't know the iMX chips). Also, if resolution switching
is not allowed (usual for LCD displays) max_xres/yres is also 
redundant.

Otherwise, the patch is fine.

Regards,
Krzysztof

----------------------------------------------------------------------
Podbij Dziki Zachod, wygraj laptopa lub iPoda 
>> http://link.interia.pl/f1ede


-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: rename imxfb_mach_info to imx_fb_platform_data

[Sascha Hauer]

On Wed, Aug 20, 2008 at 05:51:12PM +0200, Krzysztof Helt wrote:
> On Tue, 19 Aug 2008 17:06:43 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > rename imxfb_mach_info to a name more common to kernel hackers
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  arch/arm/mach-imx/include/mach/imxfb.h |    5 +-
> >  drivers/video/imxfb.c                  |   68 ++++++++++++++++----------------
> >  2 files changed, 37 insertions(+), 36 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/include/mach/imxfb.h b/arch/arm/mach-imx/include/mach/imxfb.h
> > index 3ed9ec8..ffcff5a 100644
> > --- a/arch/arm/mach-imx/include/mach/imxfb.h
> > +++ b/arch/arm/mach-imx/include/mach/imxfb.h
> > @@ -1,7 +1,7 @@
> >  /*
> >   * This structure describes the machine which we are running on.
> >   */
> > -struct imxfb_mach_info {
> > +struct imx_fb_platform_data {
> >  	u_long		pixclock;
> >  
> >  	u_short		xres;
> > @@ -34,4 +34,5 @@ struct imxfb_mach_info {
> >  	void (*lcd_power)(int);
> >  	void (*backlight_power)(int);
> >  };
> > -void set_imx_fb_info(struct imxfb_mach_info *hard_imx_fb_info);
> > +
> > +void set_imx_fb_info(struct imx_fb_platform_data *);
> > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > index e450e6b..a4a4940 100644
> > --- a/drivers/video/imxfb.c
> > +++ b/drivers/video/imxfb.c
> > @@ -510,7 +510,7 @@ static int imxfb_resume(struct platform_device *dev)
> >  
> >  static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> >  {
> > -	struct imxfb_mach_info *inf = pdev->dev.platform_data;
> > +	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
> >  	struct fb_info *info = dev_get_drvdata(&pdev->dev);
> >  	struct imxfb_info *fbi = info->par;
> >  
> > @@ -546,32 +546,32 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> >  	fbi->rgb[RGB_16]		= &def_rgb_16;
> >  	fbi->rgb[RGB_8]			= &def_rgb_8;
> >  
> > -	fbi->max_xres			= inf->xres;
> > -	info->var.xres			= inf->xres;
> > -	info->var.xres_virtual		= inf->xres;
> > -	fbi->max_yres			= inf->yres;
> > -	info->var.yres			= inf->yres;
> > -	info->var.yres_virtual		= inf->yres;
> > -	fbi->max_bpp			= inf->bpp;
> > -	info->var.bits_per_pixel	= inf->bpp;
> > -	info->var.nonstd		= inf->nonstd;
> > -	info->var.pixclock		= inf->pixclock;
> > -	info->var.hsync_len		= inf->hsync_len;
> > -	info->var.left_margin		= inf->left_margin;
> > -	info->var.right_margin		= inf->right_margin;
> > -	info->var.vsync_len		= inf->vsync_len;
> > -	info->var.upper_margin		= inf->upper_margin;
> > -	info->var.lower_margin		= inf->lower_margin;
> > -	info->var.sync			= inf->sync;
> > -	info->var.grayscale		= inf->cmap_greyscale;
> > -	fbi->cmap_inverse		= inf->cmap_inverse;
> > -	fbi->cmap_static		= inf->cmap_static;
> > -	fbi->pcr			= inf->pcr;
> > -	fbi->lscr1			= inf->lscr1;
> > -	fbi->dmacr			= inf->dmacr;
> > -	fbi->pwmr			= inf->pwmr;
> > -	fbi->lcd_power			= inf->lcd_power;
> > -	fbi->backlight_power		= inf->backlight_power;
> > +	fbi->max_xres			= pdata->xres;
> > +	info->var.xres			= pdata->xres;
> > +	info->var.xres_virtual		= pdata->xres;
> > +	fbi->max_yres			= pdata->yres;
> > +	info->var.yres			= pdata->yres;
> > +	info->var.yres_virtual		= pdata->yres;
> > +	fbi->max_bpp			= pdata->bpp;
> > +	info->var.bits_per_pixel	= pdata->bpp;
> 
> These max_foo fields seem redundant. Especially, the max_bpp.
> Most ARM chips can handle any bpp regardless the connected 
> display (I don't know the iMX chips). Also, if resolution switching
> is not allowed (usual for LCD displays) max_xres/yres is also 
> redundant.

Ok, I'll prepare another patch for this and let this one as is so that
it still only contains renaming.

Sascha

> 
> Otherwise, the patch is fine.
> 
> Regards,
> Krzysztof
> 
> ----------------------------------------------------------------------
> Podbij Dziki Zachod, wygraj laptopa lub iPoda 
> >> http://link.interia.pl/f1ede
> 
> 

-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
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=/

i.MX framebuffer patches

[Sascha Hauer]

These patches are the rework of the patches I sent some time ago based
on comments from Krzysztof Helt, Andrew Dyer and Russell King. The patches
are also available as git:

git://pasiphae.extern.pengutronix.de/git/imx/linux-2.6.git imxfb

Sascha



-------------------------------------------------------------------------
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=/

[PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/video/imxfb.c |   70 +++++++++++++++++++++++-------------------------
 1 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 7b3400c..25323ea 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -1,6 +1,4 @@
 /*
- *  linux/drivers/video/imxfb.c
- *
  *  Freescale i.MX Frame Buffer device driver
  *
  *  Copyright (C) 2004 Sascha Hauer, Pengutronix
@@ -168,20 +166,21 @@ struct imxfb_info {
 #define MIN_YRES	64
 
 static struct imxfb_rgb def_rgb_16 = {
-	.red	= { .offset = 8,  .length = 4, },
-	.green	= { .offset = 4,  .length = 4, },
-	.blue	= { .offset = 0,  .length = 4, },
-	.transp = { .offset = 0,  .length = 0, },
+	.red	= {.offset = 8, .length = 4,},
+	.green	= {.offset = 4, .length = 4,},
+	.blue	= {.offset = 0, .length = 4,},
+	.transp = {.offset = 0, .length = 0,},
 };
 
 static struct imxfb_rgb def_rgb_8 = {
-	.red	= { .offset = 0,  .length = 8, },
-	.green	= { .offset = 0,  .length = 8, },
-	.blue	= { .offset = 0,  .length = 8, },
-	.transp = { .offset = 0,  .length = 0, },
+	.red	= {.offset = 0, .length = 8,},
+	.green	= {.offset = 0, .length = 8,},
+	.blue	= {.offset = 0, .length = 8,},
+	.transp = {.offset = 0, .length = 0,},
 };
 
-static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *info);
+static int imxfb_activate_var(struct fb_var_screeninfo *var,
+		struct fb_info *info);
 
 static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 {
@@ -190,9 +189,8 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 	return chan << bf->offset;
 }
 
-static int
-imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
-		       u_int trans, struct fb_info *info)
+static int imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
+		u_int trans, struct fb_info *info)
 {
 	struct imxfb_info *fbi = info->par;
 	u_int val, ret = 1;
@@ -209,8 +207,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
 	return ret;
 }
 
-static int
-imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
+static int imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 		   u_int trans, struct fb_info *info)
 {
 	struct imxfb_info *fbi = info->par;
@@ -270,8 +267,7 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
  *    yres, xres_virtual, yres_virtual, xoffset, yoffset, grayscale,
  *    bitfields, horizontal timing, vertical timing.
  */
-static int
-imxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
+static int imxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
 	struct imxfb_info *fbi = info->par;
 	int rgbidx;
@@ -343,8 +339,7 @@ static int imxfb_set_par(struct fb_info *info)
 		info->fix.visual = FB_VISUAL_STATIC_PSEUDOCOLOR;
 	}
 
-	info->fix.line_length = var->xres_virtual *
-				  var->bits_per_pixel / 8;
+	info->fix.line_length = var->xres_virtual * var->bits_per_pixel / 8;
 	fbi->palette_size = var->bits_per_pixel == 8 ? 256 : 16;
 
 	imxfb_activate_var(var, info);
@@ -375,9 +370,9 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 
 	writel(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
 
-	if(fbi->backlight_power)
+	if (fbi->backlight_power)
 		fbi->backlight_power(1);
-	if(fbi->lcd_power)
+	if (fbi->lcd_power)
 		fbi->lcd_power(1);
 }
 
@@ -385,9 +380,9 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 {
 	pr_debug("Disabling LCD controller\n");
 
-	if(fbi->backlight_power)
+	if (fbi->backlight_power)
 		fbi->backlight_power(0);
-	if(fbi->lcd_power)
+	if (fbi->lcd_power)
 		fbi->lcd_power(0);
 
 	writel(0, fbi->regs + LCDC_RMCR);
@@ -495,7 +490,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct imxfb_info *fbi = platform_get_drvdata(dev);
-	pr_debug("%s\n",__func__);
+
+	pr_debug("%s\n", __func__);
 
 	imxfb_disable_controller(fbi);
 	return 0;
@@ -504,7 +500,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 static int imxfb_resume(struct platform_device *dev)
 {
 	struct imxfb_info *fbi = platform_get_drvdata(dev);
-	pr_debug("%s\n",__func__);
+
+	pr_debug("%s\n", __func__);
 
 	imxfb_enable_controller(fbi);
 	return 0;
@@ -522,7 +519,7 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 
 	pr_debug("%s\n",__func__);
 
-	info->pseudo_palette = kmalloc( sizeof(u32) * 16, GFP_KERNEL);
+	info->pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL);
 	if (!info->pseudo_palette)
 		return -ENOMEM;
 
@@ -530,22 +527,23 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 
 	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
 
-	info->fix.type	= FB_TYPE_PACKED_PIXELS;
+	info->fix.type			= FB_TYPE_PACKED_PIXELS;
 	info->fix.type_aux		= 0;
 	info->fix.xpanstep		= 0;
 	info->fix.ypanstep		= 0;
 	info->fix.ywrapstep		= 0;
-	info->fix.accel	= FB_ACCEL_NONE;
+	info->fix.accel			= FB_ACCEL_NONE;
 
 	info->var.nonstd		= 0;
 	info->var.activate		= FB_ACTIVATE_NOW;
 	info->var.height		= -1;
 	info->var.width	= -1;
 	info->var.accel_flags		= 0;
-	info->var.vmode	= FB_VMODE_NONINTERLACED;
+	info->var.vmode			= FB_VMODE_NONINTERLACED;
 
 	info->fbops			= &imxfb_ops;
-	info->flags			= FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST;
+	info->flags			= FBINFO_FLAG_DEFAULT |
+					  FBINFO_READS_FAST;
 
 	fbi->rgb[RGB_16]		= &def_rgb_16;
 	fbi->rgb[RGB_8]			= &def_rgb_8;
@@ -593,17 +591,17 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	printk("i.MX Framebuffer driver\n");
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if(!res)
+	if (!res)
 		return -ENODEV;
 
 	inf = pdev->dev.platform_data;
-	if(!inf) {
+	if (!inf) {
 		dev_err(&pdev->dev,"No platform_data available\n");
 		return -ENOMEM;
 	}
 
 	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
-	if(!info)
+	if (!info)
 		return -ENOMEM;
 
 	fbi = info->par;
@@ -611,7 +609,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, info);
 
 	ret = imxfb_init_fbinfo(pdev);
-	if( ret < 0 )
+	if (ret < 0)
 		goto failed_init;
 
 	res = request_mem_region(res->start, resource_size(res),
@@ -658,7 +656,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	 */
 	imxfb_check_var(&info->var, info);
 
-	ret = fb_alloc_cmap(&info->cmap, 1<<info->var.bits_per_pixel, 0);
+	ret = fb_alloc_cmap(&info->cmap, 1 << info->var.bits_per_pixel, 0);
 	if (ret < 0)
 		goto failed_cmap;
 
-- 
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Krzysztof Helt]

On Tue,  2 Sep 2008 13:24:51 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/video/imxfb.c |   70 +++++++++++++++++++++++-------------------------
>  1 files changed, 34 insertions(+), 36 deletions(-)
> 

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

One offtopic comment: do you really want the FBINFO_READS_FAST set?

It means that the driver prefer copyarea to imgblit function. It is
usually slower on most drivers (opposite on chips with attached
very fast memory: 3Dfx, GeForce, probably new ATI chips as well).

Please test your driver with and without this option if possible.

Kind regards,
Krzysztof

---------------------------------------------------------------
Nasilaja sie kradzieze.
Mieszkancy osiedli zaniepokojeni.
Prosimy o pomoc w tej sprawie >>> http://link.interia.pl/f1eef



-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

On Wed, Sep 03, 2008 at 09:19:17PM +0200, Krzysztof Helt wrote:
> On Tue,  2 Sep 2008 13:24:51 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/video/imxfb.c |   70 +++++++++++++++++++++++-------------------------
> >  1 files changed, 34 insertions(+), 36 deletions(-)
> > 
> 
> Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> One offtopic comment: do you really want the FBINFO_READS_FAST set?
> 
> It means that the driver prefer copyarea to imgblit function. It is
> usually slower on most drivers (opposite on chips with attached
> very fast memory: 3Dfx, GeForce, probably new ATI chips as well).
> 
> Please test your driver with and without this option if possible.

I'll keep that in mind and test it. I'm sure this flag never went in on
purpose, it went in with copy-paste.

Regards,
 Sascha

-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Juergen Beisert]

On Mittwoch, 3. September 2008, Krzysztof Helt wrote:
> On Tue,  2 Sep 2008 13:24:51 +0200
>
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/video/imxfb.c |   70
> > +++++++++++++++++++++++------------------------- 1 files changed, 34
> > insertions(+), 36 deletions(-)
>
> Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
>
> One offtopic comment: do you really want the FBINFO_READS_FAST set?
>
> It means that the driver prefer copyarea to imgblit function. It is

Did you ever get the framework to use the copyarea function the driver 
provides? I tried it several times and this function was never called. 
Independent from any flags I set or clear.

jbe

-------------------------------------------------------------------------
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=/

[PATCH] i.MX Framebuffer: Cleanup Coding style

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/video/imxfb.c |   70 +++++++++++++++++++++++-------------------------
 1 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 7de3796..00c7560 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -1,6 +1,4 @@
 /*
- *  linux/drivers/video/imxfb.c
- *
  *  Freescale i.MX Frame Buffer device driver
  *
  *  Copyright (C) 2004 Sascha Hauer, Pengutronix
@@ -168,20 +166,21 @@ struct imxfb_info {
 #define MIN_YRES	64
 
 static struct imxfb_rgb def_rgb_16 = {
-	.red	= { .offset = 8,  .length = 4, },
-	.green	= { .offset = 4,  .length = 4, },
-	.blue	= { .offset = 0,  .length = 4, },
-	.transp = { .offset = 0,  .length = 0, },
+	.red	= {.offset = 8, .length = 4,},
+	.green	= {.offset = 4, .length = 4,},
+	.blue	= {.offset = 0, .length = 4,},
+	.transp = {.offset = 0, .length = 0,},
 };
 
 static struct imxfb_rgb def_rgb_8 = {
-	.red	= { .offset = 0,  .length = 8, },
-	.green	= { .offset = 0,  .length = 8, },
-	.blue	= { .offset = 0,  .length = 8, },
-	.transp = { .offset = 0,  .length = 0, },
+	.red	= {.offset = 0, .length = 8,},
+	.green	= {.offset = 0, .length = 8,},
+	.blue	= {.offset = 0, .length = 8,},
+	.transp = {.offset = 0, .length = 0,},
 };
 
-static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *info);
+static int imxfb_activate_var(struct fb_var_screeninfo *var,
+		struct fb_info *info);
 
 static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 {
@@ -190,9 +189,8 @@ static inline u_int chan_to_field(u_int chan, struct fb_bitfield *bf)
 	return chan << bf->offset;
 }
 
-static int
-imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
-		       u_int trans, struct fb_info *info)
+static int imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
+		u_int trans, struct fb_info *info)
 {
 	struct imxfb_info *fbi = info->par;
 	u_int val, ret = 1;
@@ -209,8 +207,7 @@ imxfb_setpalettereg(u_int regno, u_int red, u_int green, u_int blue,
 	return ret;
 }
 
-static int
-imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
+static int imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 		   u_int trans, struct fb_info *info)
 {
 	struct imxfb_info *fbi = info->par;
@@ -270,8 +267,7 @@ imxfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
  *    yres, xres_virtual, yres_virtual, xoffset, yoffset, grayscale,
  *    bitfields, horizontal timing, vertical timing.
  */
-static int
-imxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
+static int imxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
 	struct imxfb_info *fbi = info->par;
 	int rgbidx;
@@ -343,8 +339,7 @@ static int imxfb_set_par(struct fb_info *info)
 		info->fix.visual = FB_VISUAL_STATIC_PSEUDOCOLOR;
 	}
 
-	info->fix.line_length = var->xres_virtual *
-				  var->bits_per_pixel / 8;
+	info->fix.line_length = var->xres_virtual * var->bits_per_pixel / 8;
 	fbi->palette_size = var->bits_per_pixel == 8 ? 256 : 16;
 
 	imxfb_activate_var(var, info);
@@ -375,9 +370,9 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 
 	writel(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
 
-	if(fbi->backlight_power)
+	if (fbi->backlight_power)
 		fbi->backlight_power(1);
-	if(fbi->lcd_power)
+	if (fbi->lcd_power)
 		fbi->lcd_power(1);
 }
 
@@ -385,9 +380,9 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 {
 	pr_debug("Disabling LCD controller\n");
 
-	if(fbi->backlight_power)
+	if (fbi->backlight_power)
 		fbi->backlight_power(0);
-	if(fbi->lcd_power)
+	if (fbi->lcd_power)
 		fbi->lcd_power(0);
 
 	writel(0, fbi->regs + LCDC_RMCR);
@@ -495,7 +490,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 {
 	struct imxfb_info *fbi = platform_get_drvdata(dev);
-	pr_debug("%s\n",__func__);
+
+	pr_debug("%s\n", __func__);
 
 	imxfb_disable_controller(fbi);
 	return 0;
@@ -504,7 +500,8 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 static int imxfb_resume(struct platform_device *dev)
 {
 	struct imxfb_info *fbi = platform_get_drvdata(dev);
-	pr_debug("%s\n",__func__);
+
+	pr_debug("%s\n", __func__);
 
 	imxfb_enable_controller(fbi);
 	return 0;
@@ -522,7 +519,7 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 
 	pr_debug("%s\n",__func__);
 
-	info->pseudo_palette = kmalloc( sizeof(u32) * 16, GFP_KERNEL);
+	info->pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL);
 	if (!info->pseudo_palette)
 		return -ENOMEM;
 
@@ -530,22 +527,23 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 
 	strlcpy(info->fix.id, IMX_NAME, sizeof(info->fix.id));
 
-	info->fix.type	= FB_TYPE_PACKED_PIXELS;
+	info->fix.type			= FB_TYPE_PACKED_PIXELS;
 	info->fix.type_aux		= 0;
 	info->fix.xpanstep		= 0;
 	info->fix.ypanstep		= 0;
 	info->fix.ywrapstep		= 0;
-	info->fix.accel	= FB_ACCEL_NONE;
+	info->fix.accel			= FB_ACCEL_NONE;
 
 	info->var.nonstd		= 0;
 	info->var.activate		= FB_ACTIVATE_NOW;
 	info->var.height		= -1;
 	info->var.width	= -1;
 	info->var.accel_flags		= 0;
-	info->var.vmode	= FB_VMODE_NONINTERLACED;
+	info->var.vmode			= FB_VMODE_NONINTERLACED;
 
 	info->fbops			= &imxfb_ops;
-	info->flags			= FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST;
+	info->flags			= FBINFO_FLAG_DEFAULT |
+					  FBINFO_READS_FAST;
 
 	fbi->rgb[RGB_16]		= &def_rgb_16;
 	fbi->rgb[RGB_8]			= &def_rgb_8;
@@ -593,17 +591,17 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	printk("i.MX Framebuffer driver\n");
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if(!res)
+	if (!res)
 		return -ENODEV;
 
 	inf = pdev->dev.platform_data;
-	if(!inf) {
+	if (!inf) {
 		dev_err(&pdev->dev,"No platform_data available\n");
 		return -ENOMEM;
 	}
 
 	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
-	if(!info)
+	if (!info)
 		return -ENOMEM;
 
 	fbi = info->par;
@@ -611,7 +609,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, info);
 
 	ret = imxfb_init_fbinfo(pdev);
-	if( ret < 0 )
+	if (ret < 0)
 		goto failed_init;
 
 	res = request_mem_region(res->start, resource_size(res),
@@ -658,7 +656,7 @@ static int __init imxfb_probe(struct platform_device *pdev)
 	 */
 	imxfb_check_var(&info->var, info);
 
-	ret = fb_alloc_cmap(&info->cmap, 1<<info->var.bits_per_pixel, 0);
+	ret = fb_alloc_cmap(&info->cmap, 1 << info->var.bits_per_pixel, 0);
 	if (ret < 0)
 		goto failed_cmap;
 
-- 
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[krzysztof.h1]

> On Mittwoch, 3. September 2008, Krzysztof Helt wrote:
> > On Tue,  2 Sep 2008 13:24:51 +0200
> >
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > >  drivers/video/imxfb.c |   70
> > > +++++++++++++++++++++++------------------------- 1 files changed, 34
> > > insertions(+), 36 deletions(-)
> >
> > Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> >
> > One offtopic comment: do you really want the FBINFO_READS_FAST set?
> >
> > It means that the driver prefer copyarea to imgblit function. It is
> 
> Did you ever get the framework to use the copyarea function the driver 
> provides? I tried it several times and this function was never called. 
> Independent from any flags I set or clear.
> 
> jbe
> 

I do not have atmel hardware to test. I did used the copyarea on some other drivers (tridentfb, pm2fb). The tdfxfb driver uses now copyarea function for console scrolling. I will inspect out the atmel driver and give you some hints.

Regards,
Krzysztof
> 


---------------------------------------------------------------
Nasilaja sie kradzieze.
Mieszkancy osiedli zaniepokojeni.
Prosimy o pomoc w tej sprawie >>> http://link.interia.pl/f1eef



-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Juergen Beisert]

On Donnerstag, 4. September 2008, krzysztof.h1@poczta.fm wrote:
> > On Mittwoch, 3. September 2008, Krzysztof Helt wrote:
> > > On Tue,  2 Sep 2008 13:24:51 +0200
> > >
> > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > ---
> > > >  drivers/video/imxfb.c |   70
> > > > +++++++++++++++++++++++------------------------- 1 files changed, 34
> > > > insertions(+), 36 deletions(-)
> > >
> > > Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> > >
> > > One offtopic comment: do you really want the FBINFO_READS_FAST set?
> > >
> > > It means that the driver prefer copyarea to imgblit function. It is
> >
> > Did you ever get the framework to use the copyarea function the driver
> > provides? I tried it several times and this function was never called.
> > Independent from any flags I set or clear.
> >
> > jbe
>
> I do not have atmel hardware to test.

Its not architecture specific.

> I did used the copyarea on some other drivers (tridentfb, pm2fb).

How did you use them?

> The tdfxfb driver uses now copyarea function for console scrolling.

Are you sure, this function will be called? I wrote two drivers for ARM and 
x86 graphics hardware and provide a copyarea function for both of them. The 
console framework ignores both. Only the fillrect and imageblit function is 
used from the frambuffer console code.

In driver/video/console/fbcon.c I always fail in function fbcon_scroll(). 
The "dir" switch/case never reaches one of the "switch (p->scrollmode)" 
statements.

jbe

-------------------------------------------------------------------------
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=/

Re: [PATCH] i.MX Framebuffer: Cleanup Coding style

[Krzysztof Helt]

On Thu, 4 Sep 2008 17:08:49 +0200
Juergen Beisert <juergen127@kreuzholzen.de> wrote:

> On Donnerstag, 4. September 2008, krzysztof.h1@poczta.fm wrote:
> > > On Mittwoch, 3. September 2008, Krzysztof Helt wrote:
> > > > On Tue,  2 Sep 2008 13:24:51 +0200
> > > >
> > > > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > ---
> > > > >  drivers/video/imxfb.c |   70
> > > > > +++++++++++++++++++++++------------------------- 1 files changed, 34
> > > > > insertions(+), 36 deletions(-)
> > > >
> > > > Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> > > >
> > > > One offtopic comment: do you really want the FBINFO_READS_FAST set?
> > > >
> > > > It means that the driver prefer copyarea to imgblit function. It is
> > >
> > > Did you ever get the framework to use the copyarea function the driver
> > > provides? I tried it several times and this function was never called.
> > > Independent from any flags I set or clear.
> > >

The minimum is to set the flag FBINFO_HWACCEL_COPYAREA
into fb_info->flags. If you do not announce the accelerated copyarea
function it is not used (the imageblit is preferred).
If you defined both flags FBINFO_HWACCEL_IMAGEBLIT and
FBINFO_HWACCEL_COPYAREA you have to specify the
FBINFO_READS_FAST to prefer the copyarea. As no of this flag is
set in the imxfb driver the imageblit is used.

> 
> > I did used the copyarea on some other drivers (tridentfb, pm2fb).
> 
> How did you use them?
> 

Simply, the console use them for scrolling. The tridentfb still
uses the copyarea for scrolling on families other then Blade3d. 

> > The tdfxfb driver uses now copyarea function for console scrolling.
> 
> Are you sure, this function will be called? 

Yes. I can recheck if you want, but there is a measurable speed difference 
if you define FBINFO_READS_FAST or not on the 3Dfx hardware.

Please check this threads:
http://marc.info/?l=linux-fbdev-devel&m=117852288316471&w=2
http://marc.info/?l=linux-fbdev-devel&m=117852288316471&w=2

and some threads around. Since then the copyarea is much faster.

> I wrote two drivers for ARM and 
> x86 graphics hardware and provide a copyarea function for both of them. The 
> console framework ignores both. Only the fillrect and imageblit function is 
> used from the frambuffer console code.
> 
The imageblit is preferred over copyarea is it is usually faster. The fillrect 
is always used.

> In driver/video/console/fbcon.c I always fail in function fbcon_scroll(). 
> The "dir" switch/case never reaches one of the "switch (p->scrollmode)" 
> statements.
> 

Please check the constants mentioned above and try again.

Regards,
Krzysztof


----------------------------------------------------------------------
Tylko u nas - swieza krew w rozsadnych cenach!
>> http://link.interia.pl/f1eed


-------------------------------------------------------------------------
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=/