[PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

Subsystems: framebuffer layer, the rest

STALE5631d

13 messages, 5 authors, 2011-03-02 · open the first message on its own page

[PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Damian Hobson-Garcia <hidden>
Date: 2011-02-23 10:16:49

Supports YCbCr420sp, YCbCr422sp, and YCbCr44sp, formats (bpp = 12, 16, and 24)
respectively.
Set .nonstd = SH_FB_YUV to enable YUV mode, and use bpp to distiguish between
the 3 modes.
Due to the encoding of YUV data, the framebuffer will clear to green instead of black.

Signed-off-by: Damian Hobson-Garcia <redacted>
---
 drivers/video/sh_mobile_lcdcfb.c |  142 ++++++++++++++++++++++++++++++--------
 drivers/video/sh_mobile_lcdcfb.h |    2 +-
 include/linux/sh_mobile_fb.h     |   14 ++++
 include/video/sh_mobile_lcdc.h   |    1 +
 4 files changed, 130 insertions(+), 29 deletions(-)
 create mode 100644 include/linux/sh_mobile_fb.h
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index bf12e53..f2814ea 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -24,6 +24,7 @@
 #include <video/sh_mobile_lcdc.h>
 #include <asm/atomic.h>
 
+#include <linux/sh_mobile_fb.h>
 #include "sh_mobile_lcdcfb.h"
 
 #define SIDE_B_OFFSET 0x1000
@@ -67,6 +68,7 @@ static unsigned long lcdc_offs_mainlcd[NR_CH_REGS] = {
 	[LDSM1R] = 0x428,
 	[LDSM2R] = 0x42c,
 	[LDSA1R] = 0x430,
+	[LDSA2R] = 0x434,
 	[LDMLSR] = 0x438,
 	[LDHCNR] = 0x448,
 	[LDHSYNR] = 0x44c,
@@ -151,6 +153,7 @@ static bool banked(int reg_nr)
 	case LDDFR:
 	case LDSM1R:
 	case LDSA1R:
+	case LDSA2R:
 	case LDMLSR:
 	case LDHCNR:
 	case LDHSYNR:
@@ -463,6 +466,7 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 	struct sh_mobile_lcdc_board_cfg	*board_cfg;
 	unsigned long tmp;
 	int bpp = 0;
+	unsigned long ldddsr;
 	int k, m;
 	int ret = 0;
 
@@ -541,16 +545,21 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 	}
 
 	/* word and long word swap */
-	switch (bpp) {
-	case 16:
-		lcdc_write(priv, _LDDDSR, lcdc_read(priv, _LDDDSR) | 6);
-		break;
-	case 24:
-		lcdc_write(priv, _LDDDSR, lcdc_read(priv, _LDDDSR) | 7);
-		break;
-	case 32:
-		lcdc_write(priv, _LDDDSR, lcdc_read(priv, _LDDDSR) | 4);
-		break;
+	ldddsr = lcdc_read(priv, _LDDDSR);
+	if  (priv->ch[0].info->var.nonstd & SH_FB_YUV)
+		lcdc_write(priv, _LDDDSR, ldddsr | 7);
+	else {
+		switch (bpp) {
+		case 16:
+			lcdc_write(priv, _LDDDSR, ldddsr | 6);
+			break;
+		case 24:
+			lcdc_write(priv, _LDDDSR, ldddsr | 7);
+			break;
+		case 32:
+			lcdc_write(priv, _LDDDSR, ldddsr | 4);
+			break;
+		}
 	}
 
 	for (k = 0; k < ARRAY_SIZE(priv->ch); k++) {
@@ -561,21 +570,40 @@ static int sh_mobile_lcdc_start(struct sh_mobile_lcdc_priv *priv)
 
 		/* set bpp format in PKF[4:0] */
 		tmp = lcdc_read_chan(ch, LDDFR);
-		tmp &= ~0x0001001f;
-		switch (ch->info->var.bits_per_pixel) {
-		case 16:
-			tmp |= 0x03;
-			break;
-		case 24:
-			tmp |= 0x0b;
-			break;
-		case 32:
-			break;
+		tmp &= ~0x0003031f;
+		if (ch->info->var.nonstd & SH_FB_YUV) {
+			tmp |= (ch->info->var.nonstd << 16);
+			switch (ch->info->var.bits_per_pixel) {
+			case 12:
+				break;
+			case 16:
+				tmp |= (0x1 << 8);
+				break;
+			case 24:
+				tmp |= (0x2 << 8);
+				break;
+			}
+		} else {
+			switch (ch->info->var.bits_per_pixel) {
+			case 16:
+				tmp |= 0x03;
+				break;
+			case 24:
+				tmp |= 0x0b;
+				break;
+			case 32:
+				break;
+			}
 		}
 		lcdc_write_chan(ch, LDDFR, tmp);
 
 		/* point out our frame buffer */
 		lcdc_write_chan(ch, LDSA1R, ch->info->fix.smem_start);
+		if (ch->info->var.nonstd & SH_FB_YUV)
+			lcdc_write_chan(ch, LDSA2R,
+				ch->info->fix.smem_start +
+				ch->info->var.xres *
+				ch->info->var.yres_virtual);
 
 		/* set line size */
 		lcdc_write_chan(ch, LDMLSR, ch->info->fix.line_length);
@@ -804,9 +832,15 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 	struct sh_mobile_lcdc_priv *priv = ch->lcdc;
 	unsigned long ldrcntr;
 	unsigned long new_pan_offset;
+	unsigned long base_addr_y, base_addr_c;
+	unsigned long c_offset;
 
-	new_pan_offset = (var->yoffset * info->fix.line_length) +
-		(var->xoffset * (info->var.bits_per_pixel / 8));
+	if (!(var->nonstd & SH_FB_YUV))
+		new_pan_offset = (var->yoffset * info->fix.line_length) +
+			(var->xoffset * (info->var.bits_per_pixel / 8));
+	else
+		new_pan_offset = (var->yoffset * info->fix.line_length) +
+			(var->xoffset);
 
 	if (new_pan_offset = ch->pan_offset)
 		return 0;	/* No change, do nothing */
@@ -814,7 +848,26 @@ static int sh_mobile_fb_pan_display(struct fb_var_screeninfo *var,
 	ldrcntr = lcdc_read(priv, _LDRCNTR);
 
 	/* Set the source address for the next refresh */
-	lcdc_write_chan_mirror(ch, LDSA1R, ch->dma_handle + new_pan_offset);
+	base_addr_y = ch->dma_handle + new_pan_offset;
+	if (var->nonstd & SH_FB_YUV) {
+		/* Set y offset */
+		c_offset = (var->yoffset *
+			info->fix.line_length *
+			(info->var.bits_per_pixel - 8)) / 8;
+		base_addr_c = ch->dma_handle + var->xres * var->yres_virtual +
+			c_offset;
+		/* Set x offset */
+		if (info->var.bits_per_pixel = 24)
+			base_addr_c += 2 * var->xoffset;
+		else
+			base_addr_c += var->xoffset;
+	} else
+		base_addr_c = 0;
+
+	lcdc_write_chan_mirror(ch, LDSA1R, base_addr_y);
+	if (base_addr_c)
+		lcdc_write_chan_mirror(ch, LDSA2R, base_addr_c);
+
 	if (lcdc_chan_is_sublcd(ch))
 		lcdc_write(ch->lcdc, _LDRCNTR, ldrcntr ^ LDRCNTR_SRS);
 	else
@@ -885,7 +938,10 @@ static void sh_mobile_fb_reconfig(struct fb_info *info)
 		/* Couldn't reconfigure, hopefully, can continue as before */
 		return;
 
-	info->fix.line_length = mode1.xres * (ch->cfg.bpp / 8);
+	if (info->var.nonstd & SH_FB_YUV)
+		info->fix.line_length = mode1.xres;
+	else
+		info->fix.line_length = mode1.xres * (ch->cfg.bpp / 8);
 
 	/*
 	 * fb_set_var() calls the notifier change internally, only if
@@ -980,8 +1036,22 @@ static struct fb_ops sh_mobile_lcdc_ops = {
 	.fb_check_var	= sh_mobile_check_var,
 };
 
-static int sh_mobile_lcdc_set_bpp(struct fb_var_screeninfo *var, int bpp)
+static int sh_mobile_lcdc_set_bpp(struct fb_var_screeninfo *var, int bpp,
+				   int nonstd)
 {
+	if (nonstd & SH_FB_YUV) {
+		switch (bpp) {
+		case 12:
+		case 16:
+		case 24:
+			var->bits_per_pixel = bpp;
+			var->nonstd = nonstd;
+			return 0;
+		default:
+			return -EINVAL;
+		}
+	}
+
 	switch (bpp) {
 	case 16: /* PKF[4:0] = 00011 - RGB 565 */
 		var->red.offset = 11;
@@ -1260,6 +1330,14 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 		     k < cfg->num_cfg && lcd_cfg;
 		     k++, lcd_cfg++) {
 			unsigned long size = lcd_cfg->yres * lcd_cfg->xres;
+			/* NV12 buffers must have even number of lines */
+			if ((cfg->nonstd & SH_FB_YUV) && cfg->bpp = 12 &&
+					(lcd_cfg->yres & 0x1)) {
+				dev_err(&pdev->dev, "yres must be multiple of 2"
+						" for YCbCr420 mode.\n");
+				error = -EINVAL;
+				goto err1;
+			}
 
 			if (size > max_size) {
 				max_cfg = lcd_cfg;
@@ -1274,7 +1352,11 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 				max_cfg->xres, max_cfg->yres);
 
 		info->fix = sh_mobile_lcdc_fix;
-		info->fix.smem_len = max_size * (cfg->bpp / 8) * 2;
+		info->fix.smem_len = max_size * 2 * cfg->bpp / 8;
+
+		 /* Only pan in 2 line steps for NV12 */
+		if ((cfg->nonstd & SH_FB_YUV) && cfg->bpp = 12)
+			info->fix.ypanstep = 2;
 
 		if (!mode) {
 			mode = &default_720p;
@@ -1292,7 +1374,7 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 		var->yres_virtual = var->yres * 2;
 		var->activate = FB_ACTIVATE_NOW;
 
-		error = sh_mobile_lcdc_set_bpp(var, cfg->bpp);
+		error = sh_mobile_lcdc_set_bpp(var, cfg->bpp, cfg->nonstd);
 		if (error)
 			break;
 
@@ -1316,7 +1398,11 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
 		}
 
 		info->fix.smem_start = ch->dma_handle;
-		info->fix.line_length = var->xres * (cfg->bpp / 8);
+		if (var->nonstd & SH_FB_YUV)
+			info->fix.line_length = var->xres;
+		else
+			info->fix.line_length = var->xres * (cfg->bpp / 8);
+
 		info->screen_base = buf;
 		info->device = &pdev->dev;
 		ch->display_var = *var;
diff --git a/drivers/video/sh_mobile_lcdcfb.h b/drivers/video/sh_mobile_lcdcfb.h
index 9ecee2f..c953cb0 100644
--- a/drivers/video/sh_mobile_lcdcfb.h
+++ b/drivers/video/sh_mobile_lcdcfb.h
@@ -8,7 +8,7 @@
 
 /* per-channel registers */
 enum { LDDCKPAT1R, LDDCKPAT2R, LDMT1R, LDMT2R, LDMT3R, LDDFR, LDSM1R,
-       LDSM2R, LDSA1R, LDMLSR, LDHCNR, LDHSYNR, LDVLNR, LDVSYNR, LDPMR,
+       LDSM2R, LDSA1R, LDSA2R, LDMLSR, LDHCNR, LDHSYNR, LDVLNR, LDVSYNR, LDPMR,
        LDHAJR,
        NR_CH_REGS };
 
diff --git a/include/linux/sh_mobile_fb.h b/include/linux/sh_mobile_fb.h
new file mode 100644
index 0000000..ec448bc
--- /dev/null
+++ b/include/linux/sh_mobile_fb.h
@@ -0,0 +1,14 @@
+/*
+ * SH-Mobile High-Definition Multimedia Interface (HDMI)
+ *
+ * Copyright (C) 2011, Damian Hobson-Garciax <dhobsong@igel.co.jp>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef SH_MOBILE_FB_H
+#define SH_MOBILE_FB_H
+
+#define SH_FB_YUV	0x1
+#endif /* SH_MOBILE_FB_H */
diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
index daabae5..650ff17 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -77,6 +77,7 @@ struct sh_mobile_lcdc_chan_cfg {
 	struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
 	struct sh_mobile_lcdc_board_cfg board_cfg;
 	struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
+	int nonstd;
 };
 
 struct sh_mobile_lcdc_info {
-- 
1.7.1

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Guennadi Liakhovetski <hidden>
Date: 2011-02-23 10:40:03

On Wed, 23 Feb 2011, Damian Hobson-Garcia wrote:
Supports YCbCr420sp, YCbCr422sp, and YCbCr44sp, formats (bpp = 12, 16, and 24)
respectively.
Set .nonstd = SH_FB_YUV to enable YUV mode, and use bpp to distiguish between
the 3 modes.
Due to the encoding of YUV data, the framebuffer will clear to green instead of black.

Signed-off-by: Damian Hobson-Garcia <redacted>
---
 drivers/video/sh_mobile_lcdcfb.c |  142 ++++++++++++++++++++++++++++++--------
 drivers/video/sh_mobile_lcdcfb.h |    2 +-
 include/linux/sh_mobile_fb.h     |   14 ++++
 include/video/sh_mobile_lcdc.h   |    1 +
 4 files changed, 130 insertions(+), 29 deletions(-)
 create mode 100644 include/linux/sh_mobile_fb.h
[snip]
quoted hunk
diff --git a/include/linux/sh_mobile_fb.h b/include/linux/sh_mobile_fb.h
new file mode 100644
index 0000000..ec448bc
--- /dev/null
+++ b/include/linux/sh_mobile_fb.h
@@ -0,0 +1,14 @@
+/*
+ * SH-Mobile High-Definition Multimedia Interface (HDMI)
+ *
+ * Copyright (C) 2011, Damian Hobson-Garciax <dhobsong@igel.co.jp>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef SH_MOBILE_FB_H
+#define SH_MOBILE_FB_H
+
+#define SH_FB_YUV	0x1
+#endif /* SH_MOBILE_FB_H */
diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
index daabae5..650ff17 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -77,6 +77,7 @@ struct sh_mobile_lcdc_chan_cfg {
 	struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
 	struct sh_mobile_lcdc_board_cfg board_cfg;
 	struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
+	int nonstd;
 };
 
 struct sh_mobile_lcdc_info {
-- 
1.7.1
Can't the SH_FB_YUV macro definition go into 
include/video/sh_mobile_lcdc.h too?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Damian <hidden>
Date: 2011-02-23 11:22:36

quoted
diff --git a/include/linux/sh_mobile_fb.h b/include/linux/sh_mobile_fb.h
new file mode 100644
index 0000000..ec448bc
--- /dev/null
+++ b/include/linux/sh_mobile_fb.h
@@ -0,0 +1,14 @@
+/*
+ * SH-Mobile High-Definition Multimedia Interface (HDMI)
+ *
+ * Copyright (C) 2011, Damian Hobson-Garciax<dhobsong@igel.co.jp>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef SH_MOBILE_FB_H
+#define SH_MOBILE_FB_H
+
+#define SH_FB_YUV	0x1
+#endif /* SH_MOBILE_FB_H */
diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
index daabae5..650ff17 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -77,6 +77,7 @@ struct sh_mobile_lcdc_chan_cfg {
  	struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
  	struct sh_mobile_lcdc_board_cfg board_cfg;
  	struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
+	int nonstd;
  };

  struct sh_mobile_lcdc_info {
--
1.7.1
Can't the SH_FB_YUV macro definition go into
include/video/sh_mobile_lcdc.h too?
My thinking behind separating this out was that I wanted this
define to be accessible from user space.  The reason is so that
an application can test the value of .nonstd against the flag to
know for sure if it is dealing with a YUV framebuffer or not.
I was under the impression that only headers under include/linux/ should 
be accessed from user space, but to be honest, I'm not sure about that.
If that is in fact not the case, then I totally agree that it can go
into include/video/sh_mobile_lcdc.h.
Do you know where the proper place for such a header would be?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
Thanks,
Damian
-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: James Simmons <hidden>
Date: 2011-02-23 14:58:31

quoted
Can't the SH_FB_YUV macro definition go into
include/video/sh_mobile_lcdc.h too?
My thinking behind separating this out was that I wanted this
define to be accessible from user space.  The reason is so that
an application can test the value of .nonstd against the flag to
know for sure if it is dealing with a YUV framebuffer or not.
I was under the impression that only headers under include/linux/ should be
accessed from user space, but to be honest, I'm not sure about that.
If that is in fact not the case, then I totally agree that it can go
into include/video/sh_mobile_lcdc.h.
Do you know where the proper place for such a header would be?
Originally the idea was headers in include/video would be card 
specific material to be used by userspace. Also those headers could be 
used also by the kernel driver as well.

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Magnus Damm <magnus.damm@gmail.com>
Date: 2011-02-23 23:28:30

Hi Damian,

On Wed, Feb 23, 2011 at 8:22 PM, Damian [off-list ref] wrote:
quoted
quoted
diff --git a/include/linux/sh_mobile_fb.h b/include/linux/sh_mobile_fb.h
new file mode 100644
index 0000000..ec448bc
--- /dev/null
+++ b/include/linux/sh_mobile_fb.h
@@ -0,0 +1,14 @@
+/*
+ * SH-Mobile High-Definition Multimedia Interface (HDMI)
+ *
+ * Copyright (C) 2011, Damian Hobson-Garciax<dhobsong@igel.co.jp>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef SH_MOBILE_FB_H
+#define SH_MOBILE_FB_H
+
+#define SH_FB_YUV      0x1
+#endif /* SH_MOBILE_FB_H */
diff --git a/include/video/sh_mobile_lcdc.h
b/include/video/sh_mobile_lcdc.h
index daabae5..650ff17 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -77,6 +77,7 @@ struct sh_mobile_lcdc_chan_cfg {
       struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
       struct sh_mobile_lcdc_board_cfg board_cfg;
       struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn
I/F */
+       int nonstd;
 };

 struct sh_mobile_lcdc_info {
--
1.7.1
Can't the SH_FB_YUV macro definition go into
include/video/sh_mobile_lcdc.h too?
My thinking behind separating this out was that I wanted this
define to be accessible from user space.  The reason is so that
an application can test the value of .nonstd against the flag to
know for sure if it is dealing with a YUV framebuffer or not.
Hm, but ideally we want something standard. How do you know the nonstd
flag is working as you expect from user space? All of a sudden you
have code that depends on what type of fbdev driver you are using.
This is ugly, but abstracting the nonstd interface does not make it
better IMO.

The nonstd thing is a hack, but for now it is close enough. V4L2 has
standard NV12/NV16 support already, but I don't think there is any
such thing for fbdev. So i see your patches as a stop-gap, but I
really don't want to make it more complicated than it has to be. So
exporting nonstd values in a header file to user space seems too
complicated IMO.

Please just live with the fact that nonstd is special for now. We need
to rework the entire LCDC/HDMI/DSI area to support multiple planes and
better PM anyway. Perhaps KMS is the way forward, or perhaps Media
Controller? Maybe both?
I was under the impression that only headers under include/linux/ should be
accessed from user space, but to be honest, I'm not sure about that.
If that is in fact not the case, then I totally agree that it can go
into include/video/sh_mobile_lcdc.h.
Please ditch the SH_FB_YUV constant all together. No need to build
some abstraction on top of a hackish interface. Just check if nonstd
is non-zero in the driver and assume that means YUV for now. That's
good enough.

Thanks,

/ magnus

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Damian <hidden>
Date: 2011-02-24 03:38:41

Hi Magnus,
On 2011/02/24 8:28, Magnus Damm wrote:
Hi Damian,

On Wed, Feb 23, 2011 at 8:22 PM, Damian[off-list ref]  wrote:
quoted
quoted
quoted
diff --git a/include/linux/sh_mobile_fb.h b/include/linux/sh_mobile_fb.h
new file mode 100644
index 0000000..ec448bc
--- /dev/null
+++ b/include/linux/sh_mobile_fb.h
@@ -0,0 +1,14 @@
+/*
+ * SH-Mobile High-Definition Multimedia Interface (HDMI)
+ *
+ * Copyright (C) 2011, Damian Hobson-Garciax<dhobsong@igel.co.jp>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef SH_MOBILE_FB_H
+#define SH_MOBILE_FB_H
+
+#define SH_FB_YUV      0x1
+#endif /* SH_MOBILE_FB_H */
diff --git a/include/video/sh_mobile_lcdc.h
b/include/video/sh_mobile_lcdc.h
index daabae5..650ff17 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -77,6 +77,7 @@ struct sh_mobile_lcdc_chan_cfg {
        struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
        struct sh_mobile_lcdc_board_cfg board_cfg;
        struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn
I/F */
+       int nonstd;
  };

  struct sh_mobile_lcdc_info {
--
1.7.1
Can't the SH_FB_YUV macro definition go into
include/video/sh_mobile_lcdc.h too?
My thinking behind separating this out was that I wanted this
define to be accessible from user space.  The reason is so that
an application can test the value of .nonstd against the flag to
know for sure if it is dealing with a YUV framebuffer or not.
Hm, but ideally we want something standard. How do you know the nonstd
flag is working as you expect from user space? All of a sudden you
have code that depends on what type of fbdev driver you are using.
This is ugly, but abstracting the nonstd interface does not make it
better IMO.

The nonstd thing is a hack, but for now it is close enough. V4L2 has
standard NV12/NV16 support already, but I don't think there is any
such thing for fbdev. So i see your patches as a stop-gap, but I
really don't want to make it more complicated than it has to be. So
exporting nonstd values in a header file to user space seems too
complicated IMO.

Please just live with the fact that nonstd is special for now. We need
to rework the entire LCDC/HDMI/DSI area to support multiple planes and
better PM anyway. Perhaps KMS is the way forward, or perhaps Media
Controller? Maybe both?
quoted
I was under the impression that only headers under include/linux/ should be
accessed from user space, but to be honest, I'm not sure about that.
If that is in fact not the case, then I totally agree that it can go
into include/video/sh_mobile_lcdc.h.
Please ditch the SH_FB_YUV constant all together. No need to build
some abstraction on top of a hackish interface. Just check if nonstd
is non-zero in the driver and assume that means YUV for now. That's
good enough.
Ok, I see what you're saying. But if the SH_FB_YUV flag is disappearing, 
I guess it makes sense to ditch the second patch in this series as well, 
since that's just further abstraction (albeit locally).
Thanks,

/ magnus
Thank you,
-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2011-02-24 06:05:38

On Thu, Feb 24, 2011 at 00:28, Magnus Damm [off-list ref] wrote:
On Wed, Feb 23, 2011 at 8:22 PM, Damian [off-list ref] wrote:
quoted
quoted
quoted
diff --git a/include/linux/sh_mobile_fb.h b/include/linux/sh_mobile_fb.h
new file mode 100644
index 0000000..ec448bc
--- /dev/null
+++ b/include/linux/sh_mobile_fb.h
@@ -0,0 +1,14 @@
+/*
+ * SH-Mobile High-Definition Multimedia Interface (HDMI)
+ *
+ * Copyright (C) 2011, Damian Hobson-Garciax<dhobsong@igel.co.jp>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef SH_MOBILE_FB_H
+#define SH_MOBILE_FB_H
+
+#define SH_FB_YUV      0x1
+#endif /* SH_MOBILE_FB_H */
diff --git a/include/video/sh_mobile_lcdc.h
b/include/video/sh_mobile_lcdc.h
index daabae5..650ff17 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -77,6 +77,7 @@ struct sh_mobile_lcdc_chan_cfg {
       struct sh_mobile_lcdc_lcd_size_cfg lcd_size_cfg;
       struct sh_mobile_lcdc_board_cfg board_cfg;
       struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn
I/F */
+       int nonstd;
 };

 struct sh_mobile_lcdc_info {
--
1.7.1
Can't the SH_FB_YUV macro definition go into
include/video/sh_mobile_lcdc.h too?
My thinking behind separating this out was that I wanted this
define to be accessible from user space.  The reason is so that
an application can test the value of .nonstd against the flag to
know for sure if it is dealing with a YUV framebuffer or not.
Hm, but ideally we want something standard. How do you know the nonstd
flag is working as you expect from user space? All of a sudden you
have code that depends on what type of fbdev driver you are using.
This is ugly, but abstracting the nonstd interface does not make it
better IMO.

The nonstd thing is a hack, but for now it is close enough. V4L2 has
standard NV12/NV16 support already, but I don't think there is any
such thing for fbdev. So i see your patches as a stop-gap, but I
really don't want to make it more complicated than it has to be. So
exporting nonstd values in a header file to user space seems too
complicated IMO.

Please just live with the fact that nonstd is special for now. We need
to rework the entire LCDC/HDMI/DSI area to support multiple planes and
better PM anyway. Perhaps KMS is the way forward, or perhaps Media
Controller? Maybe both?
quoted
I was under the impression that only headers under include/linux/ should be
accessed from user space, but to be honest, I'm not sure about that.
If that is in fact not the case, then I totally agree that it can go
into include/video/sh_mobile_lcdc.h.
Please ditch the SH_FB_YUV constant all together. No need to build
some abstraction on top of a hackish interface. Just check if nonstd
is non-zero in the driver and assume that means YUV for now. That's
good enough.
For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new FB_VISUAL_*
type instead, which indicates the fb_var_screeninfo.{red,green,blue} fields are
YCbCr instead of RGB.
Depending on the frame buffer organization, you also need new FB_TYPE_*/FB_AUX_*
types.

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

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Damian <hidden>
Date: 2011-03-01 03:13:28

Hi Geert,
On 2011/02/24 15:05, Geert Uytterhoeven wrote:
quoted
quoted
My thinking behind separating this out was that I wanted this
define to be accessible from user space.  The reason is so that
an application can test the value of .nonstd against the flag to
know for sure if it is dealing with a YUV framebuffer or not.
Hm, but ideally we want something standard. How do you know the nonstd
flag is working as you expect from user space? All of a sudden you
have code that depends on what type of fbdev driver you are using.
This is ugly, but abstracting the nonstd interface does not make it
better IMO.

The nonstd thing is a hack, but for now it is close enough. V4L2 has
standard NV12/NV16 support already, but I don't think there is any
such thing for fbdev. So i see your patches as a stop-gap, but I
really don't want to make it more complicated than it has to be. So
exporting nonstd values in a header file to user space seems too
complicated IMO.

Please just live with the fact that nonstd is special for now. We need
to rework the entire LCDC/HDMI/DSI area to support multiple planes and
better PM anyway. Perhaps KMS is the way forward, or perhaps Media
Controller? Maybe both?
quoted
I was under the impression that only headers under include/linux/ should be
accessed from user space, but to be honest, I'm not sure about that.
If that is in fact not the case, then I totally agree that it can go
into include/video/sh_mobile_lcdc.h.
Please ditch the SH_FB_YUV constant all together. No need to build
some abstraction on top of a hackish interface. Just check if nonstd
is non-zero in the driver and assume that means YUV for now. That's
good enough.
For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new FB_VISUAL_*
type instead, which indicates the fb_var_screeninfo.{red,green,blue} fields are
YCbCr instead of RGB.
Depending on the frame buffer organization, you also need new FB_TYPE_*/FB_AUX_*
types.
I just wanted to clarify here.  Is your comment about these new flags in 
specific reference to this patch or to Magnus' "going forward" comment? 
  It seems like the beginnings of a method to standardize YCbCr support 
in fbdev across all platforms.
Also, do I understand correctly that FB_VISUAL_ would specify the 
colorspace (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. 
planar, semiplanar, interleaved, etc)?  I'm not really sure what you are 
referring to with the FB_AUX_* however.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thanks very much,
Damian

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2011-03-01 08:07:39

On Tue, Mar 1, 2011 at 04:13, Damian [off-list ref] wrote:
On 2011/02/24 15:05, Geert Uytterhoeven wrote:
quoted
quoted
quoted
My thinking behind separating this out was that I wanted this
define to be accessible from user space.  The reason is so that
an application can test the value of .nonstd against the flag to
know for sure if it is dealing with a YUV framebuffer or not.
Hm, but ideally we want something standard. How do you know the nonstd
flag is working as you expect from user space? All of a sudden you
have code that depends on what type of fbdev driver you are using.
This is ugly, but abstracting the nonstd interface does not make it
better IMO.

The nonstd thing is a hack, but for now it is close enough. V4L2 has
standard NV12/NV16 support already, but I don't think there is any
such thing for fbdev. So i see your patches as a stop-gap, but I
really don't want to make it more complicated than it has to be. So
exporting nonstd values in a header file to user space seems too
complicated IMO.

Please just live with the fact that nonstd is special for now. We need
to rework the entire LCDC/HDMI/DSI area to support multiple planes and
better PM anyway. Perhaps KMS is the way forward, or perhaps Media
Controller? Maybe both?
quoted
I was under the impression that only headers under include/linux/ should
be
accessed from user space, but to be honest, I'm not sure about that.
If that is in fact not the case, then I totally agree that it can go
into include/video/sh_mobile_lcdc.h.
Please ditch the SH_FB_YUV constant all together. No need to build
some abstraction on top of a hackish interface. Just check if nonstd
is non-zero in the driver and assume that means YUV for now. That's
good enough.
For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new
FB_VISUAL_*
type instead, which indicates the fb_var_screeninfo.{red,green,blue}
fields are
YCbCr instead of RGB.
Depending on the frame buffer organization, you also need new
FB_TYPE_*/FB_AUX_*
types.
I just wanted to clarify here.  Is your comment about these new flags in
specific reference to this patch or to Magnus' "going forward" comment?  It
About new flags.
seems like the beginnings of a method to standardize YCbCr support in fbdev
across all platforms.
Also, do I understand correctly that FB_VISUAL_ would specify the colorspace
FB_VISUAL_* specifies how pixel values (which may be tuples) are mapped to
colors on the screen, so to me it looks like the sensible way to set up YCbCr.
(RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. planar,
semiplanar, interleaved, etc)?  I'm not really sure what you are referring
to with the FB_AUX_* however.
Yep, FB_TYPE_* specifies how pixel values/tuples are laid out in frame buffer
memory.

FB_AUX_* is only used if a specific value of FB_TYPE_* needs an additional
parameter (e.g. the interleave value for interleaved bitplanes).
Another possible use is the offset between the even and odd fields of
the screen,
for hardware that supports interlacing by storing the even and odd
fields separately.
I think I even posted patches for that several years ago... Indeed
http://lkml.indiana.edu/hypermail/linux/kernel/0304.0/0816.html

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

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Magnus Damm <magnus.damm@gmail.com>
Date: 2011-03-01 08:25:44

Hi Geert,

On Thu, Feb 24, 2011 at 3:05 PM, Geert Uytterhoeven
[off-list ref] wrote:
On Thu, Feb 24, 2011 at 00:28, Magnus Damm [off-list ref] wrote:
quoted
Please ditch the SH_FB_YUV constant all together. No need to build
some abstraction on top of a hackish interface. Just check if nonstd
is non-zero in the driver and assume that means YUV for now. That's
good enough.
For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new FB_VISUAL_*
type instead, which indicates the fb_var_screeninfo.{red,green,blue} fields are
YCbCr instead of RGB.
Depending on the frame buffer organization, you also need new FB_TYPE_*/FB_AUX_*
types.
I'm all for extending the common code instead of hiding code in
drivers. But I wonder how much overlap there is with V4L2 for
instance. I remember adding support for some NVxx formats for V4L2
some years ago. It's mainly used for Video input on Renesas SoCs
though:

http://kerneltrap.org/mailarchive/git-commits-head/2008/12/31/4560474

So I was hoping that something like the above could be added to fbdev
too, but it looks like more code is needed.

Do you have any idea on how to tie in the valid range of each color
channel? The LCDC hardware block can select between 0->255 range or
16->235/240 for the YUV channels. In V4L2 this is handled by
v4l2_colorspace, the 0->255 maps directly to V4L2_COLORSPACE_JPEG.

And how does all this relate to KMS? I'd prefer to keep this code in
one place if possible.

Thanks,

/ magnus

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2011-03-01 20:22:28

On Tue, Mar 1, 2011 at 09:25, Magnus Damm [off-list ref] wrote:
On Thu, Feb 24, 2011 at 3:05 PM, Geert Uytterhoeven
[off-list ref] wrote:
quoted
On Thu, Feb 24, 2011 at 00:28, Magnus Damm [off-list ref] wrote:
quoted
Please ditch the SH_FB_YUV constant all together. No need to build
some abstraction on top of a hackish interface. Just check if nonstd
is non-zero in the driver and assume that means YUV for now. That's
good enough.
For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new FB_VISUAL_*
type instead, which indicates the fb_var_screeninfo.{red,green,blue} fields are
YCbCr instead of RGB.
Depending on the frame buffer organization, you also need new FB_TYPE_*/FB_AUX_*
types.
I'm all for extending the common code instead of hiding code in
drivers. But I wonder how much overlap there is with V4L2 for
instance. I remember adding support for some NVxx formats for V4L2
some years ago. It's mainly used for Video input on Renesas SoCs
though:

http://kerneltrap.org/mailarchive/git-commits-head/2008/12/31/4560474

So I was hoping that something like the above could be added to fbdev
too, but it looks like more code is needed.

Do you have any idea on how to tie in the valid range of each color
channel? The LCDC hardware block can select between 0->255 range or
16->235/240 for the YUV channels. In V4L2 this is handled by
v4l2_colorspace, the 0->255 maps directly to V4L2_COLORSPACE_JPEG.
Unfortunately not. Unlike for the YCbCr visual, I don't see a field we
can easily
(ab)use for that. Except if it's limited to standard 16-235/240 Y vs.
full 0-255 Y,
for which we could just have 2 different visual types.
And how does all this relate to KMS? I'd prefer to keep this code in
one place if possible.
Good question!

I'm still puzzled about this KMS thing. If the name "Kernel Mode Setting"
covers it, then how does it compare to plain fbdev? Just additional frame buffer
memory management?

Furthermore, everybody states that "future X/desktop" will require KMS drivers.
How do/will we handle this on dumb frame buffers? It's not like we can't do
"advanced" things like compositing using the CPU. Transparency may stretch
it a bit on lower end CPUs, but you don't always need that.

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

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Magnus Damm <magnus.damm@gmail.com>
Date: 2011-03-01 08:59:00

On Thu, Feb 24, 2011 at 12:38 PM, Damian [off-list ref] wrote:
Ok, I see what you're saying. But if the SH_FB_YUV flag is disappearing, I
guess it makes sense to ditch the second patch in this series as well, since
that's just further abstraction (albeit locally).
Yep, I think so. Thanks for your help!

/ magnus

Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support

From: Damian <hidden>
Date: 2011-03-02 06:41:49

On 2011/03/01 17:59, Magnus Damm wrote:
On Thu, Feb 24, 2011 at 12:38 PM, Damian[off-list ref]  wrote:
quoted
Ok, I see what you're saying. But if the SH_FB_YUV flag is disappearing, I
guess it makes sense to ditch the second patch in this series as well, since
that's just further abstraction (albeit locally).
Yep, I think so. Thanks for your help!
Ok, then I will resubmit only the first patch of the series as
[PATCH v2] fbdev: sh_mobile_lcdc: Add YUV input support

Thanks,
Damian
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help