diff mbox

[U-Boot,01/12] video: Add S3C24xx framebuffer support

Message ID 1405989293-6629-1-git-send-email-marex@denx.de
State Changes Requested
Delegated to: Wolfgang Denk
Headers show

Commit Message

Marek Vasut July 22, 2014, 12:34 a.m. UTC 
Add basic framebuffer driver for the S3C24xx family of CPUs.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Lukasz Majewski <l.majewski@samsung.com>
Cc: Minkyu Kang <mk7.kang@samsung.com>
Cc: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/video/Makefile      |   1 +
 drivers/video/cfb_console.c |   2 +-
 drivers/video/s3c-fb.c      | 172 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+), 1 deletion(-)
 create mode 100644 drivers/video/s3c-fb.c

Comments

Wolfgang Denk July 22, 2014, 9:25 a.m. UTC | #1 
Dear Marek,

In message <1405989293-6629-1-git-send-email-marex@denx.de> you wrote:
> Add basic framebuffer driver for the S3C24xx family of CPUs.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Anatolij Gustschin <agust@denx.de>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Lukasz Majewski <l.majewski@samsung.com>
> Cc: Minkyu Kang <mk7.kang@samsung.com>
> Cc: Vladimir Zapolskiy <vz@mleia.com>
> ---
>  drivers/video/Makefile      |   1 +
>  drivers/video/cfb_console.c |   2 +-
>  drivers/video/s3c-fb.c      | 172 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 174 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/video/s3c-fb.c
> 
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 945f35d..7441783 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_MB86R0xGDC) += mb86r0xgdc.o videomodes.o
>  obj-$(CONFIG_VIDEO_MX3) += mx3fb.o videomodes.o
>  obj-$(CONFIG_VIDEO_IPUV3) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
>  obj-$(CONFIG_VIDEO_MXS) += mxsfb.o videomodes.o
> +obj-$(CONFIG_VIDEO_S3C) += s3c-fb.o videomodes.o
>  obj-$(CONFIG_VIDEO_OMAP3) += omap3_dss.o
>  obj-$(CONFIG_VIDEO_SANDBOX_SDL) += sandbox_sdl.o
>  obj-$(CONFIG_VIDEO_SED13806) += sed13806.o

can you please fix the sort oder of this ist?  Thanks.

...
> +	/* Suck display configuration from "videomode" variable */
> +	penv = getenv("videomode");
> +	if (!penv) {
> +		puts("S3CFB: 'videomode' variable not set!\n");
> +		return NULL;
> +	}
> +
> +	bpp = video_get_params(&mode, penv);

Should there not be some error handling in case we pass invalid data?

> +	/* Allocate framebuffer */
> +	fb = memalign(S3CFB_ALIGN, roundup(panel.memSize, S3CFB_ALIGN));
> +	if (!fb) {
> +		printf("S3CFB: Error allocating framebuffer!\n");
> +		return NULL;
> +	}

Should we not use the gd->fb_base frame buffer allocation as provided
in lib/board.c ?


Best regards,

Wolfgang Denk
Marek Vasut July 23, 2014, 3 a.m. UTC | #2 
On Tuesday, July 22, 2014 at 11:25:09 AM, Wolfgang Denk wrote:
[...]
> > diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> > index 945f35d..7441783 100644
> > --- a/drivers/video/Makefile
> > +++ b/drivers/video/Makefile
> > @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_MB86R0xGDC) += mb86r0xgdc.o
> > videomodes.o
> > 
> >  obj-$(CONFIG_VIDEO_MX3) += mx3fb.o videomodes.o
> >  obj-$(CONFIG_VIDEO_IPUV3) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
> >  obj-$(CONFIG_VIDEO_MXS) += mxsfb.o videomodes.o
> > 
> > +obj-$(CONFIG_VIDEO_S3C) += s3c-fb.o videomodes.o
> > 
> >  obj-$(CONFIG_VIDEO_OMAP3) += omap3_dss.o
> >  obj-$(CONFIG_VIDEO_SANDBOX_SDL) += sandbox_sdl.o
> >  obj-$(CONFIG_VIDEO_SED13806) += sed13806.o
> 
> can you please fix the sort oder of this ist?  Thanks.

Will do in V2, thanks.

> ...
> 
> > +	/* Suck display configuration from "videomode" variable */
> > +	penv = getenv("videomode");
> > +	if (!penv) {
> > +		puts("S3CFB: 'videomode' variable not set!\n");
> > +		return NULL;
> > +	}
> > +
> > +	bpp = video_get_params(&mode, penv);
> 
> Should there not be some error handling in case we pass invalid data?

There is one, about 10 lines further there is a 'switch (bpp) {}' statement. The 
default branch will trigger a fail.

> > +	/* Allocate framebuffer */
> > +	fb = memalign(S3CFB_ALIGN, roundup(panel.memSize, S3CFB_ALIGN));
> > +	if (!fb) {
> > +		printf("S3CFB: Error allocating framebuffer!\n");
> > +		return NULL;
> > +	}
> 
> Should we not use the gd->fb_base frame buffer allocation as provided
> in lib/board.c ?

I use CONFIG_VIDEO , not CONFIG_LCD here. My understanding is that CONFIG_VIDEO 
is what should be used and CONFIG_LCD is legacy. Please correct me if I'm wrong. 
Only the CONFIG_LCD will reserve the memory, CONFIG_VIDEO will not. It might be 
a good idea to explicitly set gd->fb_addr in the driver though. What do you 
think ?

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 945f35d..7441783 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -33,6 +33,7 @@  obj-$(CONFIG_VIDEO_MB86R0xGDC) += mb86r0xgdc.o videomodes.o
 obj-$(CONFIG_VIDEO_MX3) += mx3fb.o videomodes.o
 obj-$(CONFIG_VIDEO_IPUV3) += mxc_ipuv3_fb.o ipu_common.o ipu_disp.o
 obj-$(CONFIG_VIDEO_MXS) += mxsfb.o videomodes.o
+obj-$(CONFIG_VIDEO_S3C) += s3c-fb.o videomodes.o
 obj-$(CONFIG_VIDEO_OMAP3) += omap3_dss.o
 obj-$(CONFIG_VIDEO_SANDBOX_SDL) += sandbox_sdl.o
 obj-$(CONFIG_VIDEO_SED13806) += sed13806.o
diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
index b52e9ed..4a56da8 100644
--- a/drivers/video/cfb_console.c
+++ b/drivers/video/cfb_console.c
@@ -135,7 +135,7 @@ 
 #endif
 #endif
 
-#ifdef CONFIG_VIDEO_MXS
+#if defined(CONFIG_VIDEO_MXS) || defined(CONFIG_VIDEO_S3C)
 #define VIDEO_FB_16BPP_WORD_SWAP
 #endif
 
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
new file mode 100644
index 0000000..521eb75
--- /dev/null
+++ b/drivers/video/s3c-fb.c
@@ -0,0 +1,172 @@ 
+/*
+ * S3C24x0 LCD driver
+ *
+ * NOTE: Only 16/24 bpp operation with TFT LCD is supported.
+ *
+ * Copyright (C) 2014 Marek Vasut <marex@denx.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <malloc.h>
+#include <video_fb.h>
+
+#include <asm/errno.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24x0_cpu.h>
+
+#include "videomodes.h"
+
+static GraphicDevice panel;
+
+/* S3C requires the FB to be 4MiB aligned. */
+#define S3CFB_ALIGN			(4 << 20)
+
+#define S3CFB_LCDCON1_CLKVAL(x)		((x) << 8)
+#define S3CFB_LCDCON1_PNRMODE_TFT	(0x3 << 5)
+#define S3CFB_LCDCON1_BPPMODE_TFT_16BPP	(0xc << 1)
+#define S3CFB_LCDCON1_BPPMODE_TFT_24BPP	(0xd << 1)
+
+#define S3CFB_LCDCON2_VBPD(x)		((x) << 24)
+#define S3CFB_LCDCON2_LINEVAL(x)	((x) << 14)
+#define S3CFB_LCDCON2_VFPD(x)		((x) << 6)
+#define S3CFB_LCDCON2_VSPW(x)		((x) << 0)
+
+#define S3CFB_LCDCON3_HBPD(x)		((x) << 19)
+#define S3CFB_LCDCON3_HOZVAL(x)		((x) << 8)
+#define S3CFB_LCDCON3_HFPD(x)		((x) << 0)
+
+#define S3CFB_LCDCON4_HSPW(x)		((x) << 0)
+
+#define S3CFB_LCDCON5_BPP24BL		(1 << 12)
+#define S3CFB_LCDCON5_FRM565		(1 << 11)
+#define S3CFB_LCDCON5_HWSWP		(1 << 0)
+
+#define	PS2KHZ(ps)			(1000000000UL / (ps))
+
+/*
+ * Example:
+ * setenv videomode video=ctfb:x:800,y:480,depth:16,mode:0,\
+ *            pclk:30066,le:41,ri:89,up:45,lo:12,\
+ *            hs:1,vs:1,sync:100663296,vmode:0
+ */
+static void s3c_lcd_init(GraphicDevice *panel,
+			struct ctfb_res_modes *mode, int bpp)
+{
+	uint32_t clk_divider;
+	struct s3c24x0_lcd *regs = s3c24x0_get_base_lcd();
+
+	/* Stop the controller. */
+	clrbits_le32(&regs->lcdcon1, 1);
+
+	/* Calculate clock divider. */
+	clk_divider = (get_HCLK() / PS2KHZ(mode->pixclock)) / 1000;
+	clk_divider = DIV_ROUND_UP(clk_divider, 2);
+	if (clk_divider)
+		clk_divider -= 1;
+
+	/* Program LCD configuration. */
+	switch (bpp) {
+	case 16:
+		writel(S3CFB_LCDCON1_BPPMODE_TFT_16BPP |
+		       S3CFB_LCDCON1_PNRMODE_TFT |
+		       S3CFB_LCDCON1_CLKVAL(clk_divider),
+		       &regs->lcdcon1);
+		writel(S3CFB_LCDCON5_HWSWP | S3CFB_LCDCON5_FRM565,
+		       &regs->lcdcon5);
+		break;
+	case 24:
+		writel(S3CFB_LCDCON1_BPPMODE_TFT_24BPP |
+		       S3CFB_LCDCON1_PNRMODE_TFT |
+		       S3CFB_LCDCON1_CLKVAL(clk_divider),
+		       &regs->lcdcon1);
+		writel(S3CFB_LCDCON5_BPP24BL, &regs->lcdcon5);
+		break;
+	}
+
+	writel(S3CFB_LCDCON2_LINEVAL(mode->yres - 1) |
+	       S3CFB_LCDCON2_VBPD(mode->upper_margin - 1) |
+	       S3CFB_LCDCON2_VFPD(mode->lower_margin - 1) |
+	       S3CFB_LCDCON2_VSPW(mode->vsync_len - 1),
+	       &regs->lcdcon2);
+
+	writel(S3CFB_LCDCON3_HBPD(mode->right_margin - 1) |
+	       S3CFB_LCDCON3_HFPD(mode->left_margin - 1) |
+	       S3CFB_LCDCON3_HOZVAL(mode->xres - 1),
+	       &regs->lcdcon3);
+
+	writel(S3CFB_LCDCON4_HSPW(mode->hsync_len - 1),
+	       &regs->lcdcon4);
+
+	/* Write FB address. */
+	writel(panel->frameAdrs >> 1, &regs->lcdsaddr1);
+	writel((panel->frameAdrs +
+	       (mode->xres * mode->yres * panel->gdfBytesPP)) >> 1,
+	       &regs->lcdsaddr2);
+	writel(mode->xres * bpp / 16, &regs->lcdsaddr3);
+
+	/* Start the controller. */
+	setbits_le32(&regs->lcdcon1, 1);
+}
+
+void *video_hw_init(void)
+{
+	int bpp = -1;
+	char *penv;
+	void *fb;
+	struct ctfb_res_modes mode;
+
+	puts("Video: ");
+
+	/* Suck display configuration from "videomode" variable */
+	penv = getenv("videomode");
+	if (!penv) {
+		puts("S3CFB: 'videomode' variable not set!\n");
+		return NULL;
+	}
+
+	bpp = video_get_params(&mode, penv);
+
+	/* fill in Graphic device struct */
+	sprintf(panel.modeIdent, "%dx%dx%d", mode.xres, mode.yres, bpp);
+
+	panel.winSizeX = mode.xres;
+	panel.winSizeY = mode.yres;
+	panel.plnSizeX = mode.xres;
+	panel.plnSizeY = mode.yres;
+
+	switch (bpp) {
+	case 24:
+		panel.gdfBytesPP = 4;
+		panel.gdfIndex = GDF_32BIT_X888RGB;
+		break;
+	case 16:
+		panel.gdfBytesPP = 2;
+		panel.gdfIndex = GDF_16BIT_565RGB;
+		break;
+	default:
+		printf("S3CFB: Invalid BPP specified! (bpp = %i)\n", bpp);
+		return NULL;
+	}
+
+	panel.memSize = mode.xres * mode.yres * panel.gdfBytesPP;
+
+	/* Allocate framebuffer */
+	fb = memalign(S3CFB_ALIGN, roundup(panel.memSize, S3CFB_ALIGN));
+	if (!fb) {
+		printf("S3CFB: Error allocating framebuffer!\n");
+		return NULL;
+	}
+
+	/* Wipe framebuffer */
+	memset(fb, 0, panel.memSize);
+
+	panel.frameAdrs = (u32)fb;
+
+	printf("%s\n", panel.modeIdent);
+
+	/* Start framebuffer */
+	s3c_lcd_init(&panel, &mode, bpp);
+
+	return (void *)&panel;
+}