[PATCHv3 00/22] marvell-ccic: drop and fix formats

[PATCHv3 00/22] marvell-ccic: drop and fix formats

[Hans Verkuil]

This v3 patch series replaces patch 18 from the first series.

Patch 18 and 19 are unchanged from patches 18 and 21 from the
second series.

Patches 20-21 replace the RGB444 format by the newly defined XBGR444
format (X means that the 'alpha' channel should be ignored and is not
filled in). The actual layout in memory remains unchanged.

Patch 22 fixes the Bayer format.

All tested on my OLPC XO-1 laptop.

Regards,

	Hans

[PATCHv3 18/22] marvell-ccic: fix Y'CbCr ordering

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Various formats had their byte ordering implemented incorrectly, and
the V4L2_PIX_FMT_UYVY is actually impossible to create, instead you
get V4L2_PIX_FMT_YVYU.

This was working before commit ad6ac452227b7cb93ac79beec092850d178740b1
("add new formats support for marvell-ccic driver"). That commit broke
the original format support and the OLPC XO-1 laptop showed wrong
colors ever since (if you are crazy enough to attempt to run the latest
kernel on it, like I did).

The email addresses of the authors of that patch are no longer valid,
so without a way to reach them and ask them about their test setup
I am going with what I can test on the OLPC laptop.

If this breaks something for someone on their non-OLPC setup, then
contact the linux-media mailinglist. My suspicion however is that
that commit went in untested.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/media/platform/marvell-ccic/mcam-core.c | 14 +++++++-------
 drivers/media/platform/marvell-ccic/mcam-core.h |  8 ++++----
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 9343051..3f016fb 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -117,8 +117,8 @@ static struct mcam_format_struct {
 		.planar		= false,
 	},
 	{
-		.desc		= "UYVY 4:2:2",
-		.pixelformat	= V4L2_PIX_FMT_UYVY,
+		.desc		= "YVYU 4:2:2",
+		.pixelformat	= V4L2_PIX_FMT_YVYU,
 		.mbus_code	= MEDIA_BUS_FMT_YUYV8_2X8,
 		.bpp		= 2,
 		.planar		= false,
@@ -751,7 +751,7 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
 
 	switch (fmt->pixelformat) {
 	case V4L2_PIX_FMT_YUYV:
-	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_YVYU:
 		widthy = fmt->width * 2;
 		widthuv = 0;
 		break;
@@ -783,15 +783,15 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
 	case V4L2_PIX_FMT_YUV420:
 	case V4L2_PIX_FMT_YVU420:
 		mcam_reg_write_mask(cam, REG_CTRL0,
-			C0_DF_YUV | C0_YUV_420PL | C0_YUVE_YVYU, C0_DF_MASK);
+			C0_DF_YUV | C0_YUV_420PL | C0_YUVE_VYUY, C0_DF_MASK);
 		break;
 	case V4L2_PIX_FMT_YUYV:
 		mcam_reg_write_mask(cam, REG_CTRL0,
-			C0_DF_YUV | C0_YUV_PACKED | C0_YUVE_UYVY, C0_DF_MASK);
+			C0_DF_YUV | C0_YUV_PACKED | C0_YUVE_NOSWAP, C0_DF_MASK);
 		break;
-	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_YVYU:
 		mcam_reg_write_mask(cam, REG_CTRL0,
-			C0_DF_YUV | C0_YUV_PACKED | C0_YUVE_YUYV, C0_DF_MASK);
+			C0_DF_YUV | C0_YUV_PACKED | C0_YUVE_SWAP24, C0_DF_MASK);
 		break;
 	case V4L2_PIX_FMT_RGB444:
 		mcam_reg_write_mask(cam, REG_CTRL0,
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
index 2847e06..97167f6 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -331,10 +331,10 @@ int mccic_resume(struct mcam_camera *cam);
 #define	  C0_YUVE_YVYU	  0x00010000	/* Y1CrY0Cb		*/
 #define	  C0_YUVE_VYUY	  0x00020000	/* CrY1CbY0		*/
 #define	  C0_YUVE_UYVY	  0x00030000	/* CbY1CrY0		*/
-#define	  C0_YUVE_XYUV	  0x00000000	/* 420: .YUV		*/
-#define	  C0_YUVE_XYVU	  0x00010000	/* 420: .YVU		*/
-#define	  C0_YUVE_XUVY	  0x00020000	/* 420: .UVY		*/
-#define	  C0_YUVE_XVUY	  0x00030000	/* 420: .VUY		*/
+#define	  C0_YUVE_NOSWAP  0x00000000	/* no bytes swapping	*/
+#define	  C0_YUVE_SWAP13  0x00010000	/* swap byte 1 and 3	*/
+#define	  C0_YUVE_SWAP24  0x00020000	/* swap byte 2 and 4	*/
+#define	  C0_YUVE_SWAP1324 0x00030000	/* swap bytes 1&3 and 2&4 */
 /* Bayer bits 18,19 if needed */
 #define	  C0_EOF_VSYNC	  0x00400000	/* Generate EOF by VSYNC */
 #define	  C0_VEDGE_CTRL   0x00800000	/* Detect falling edge of VSYNC */
-- 
2.1.4

[PATCHv3 19/22] marvell-ccic: drop support for PIX_FMT_422P

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

I cannot get this format to work, the colors keep coming out wrong.
Since this has never worked I just drop support for this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/media/platform/marvell-ccic/mcam-core.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 3f016fb..5b60da1 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -124,13 +124,6 @@ static struct mcam_format_struct {
 		.planar		= false,
 	},
 	{
-		.desc		= "YUV 4:2:2 PLANAR",
-		.pixelformat	= V4L2_PIX_FMT_YUV422P,
-		.mbus_code	= MEDIA_BUS_FMT_YUYV8_2X8,
-		.bpp		= 1,
-		.planar		= true,
-	},
-	{
 		.desc		= "YUV 4:2:0 PLANAR",
 		.pixelformat	= V4L2_PIX_FMT_YUV420,
 		.mbus_code	= MEDIA_BUS_FMT_YUYV8_2X8,
@@ -352,10 +345,6 @@ static void mcam_write_yuv_bases(struct mcam_camera *cam,
 	y = base;
 
 	switch (fmt->pixelformat) {
-	case V4L2_PIX_FMT_YUV422P:
-		u = y + pixel_count;
-		v = u + pixel_count / 2;
-		break;
 	case V4L2_PIX_FMT_YUV420:
 		u = y + pixel_count;
 		v = u + pixel_count / 4;
@@ -755,7 +744,6 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
 		widthy = fmt->width * 2;
 		widthuv = 0;
 		break;
-	case V4L2_PIX_FMT_YUV422P:
 	case V4L2_PIX_FMT_YUV420:
 	case V4L2_PIX_FMT_YVU420:
 		widthy = fmt->width;
@@ -776,10 +764,6 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
 	 * Tell the controller about the image format we are using.
 	 */
 	switch (fmt->pixelformat) {
-	case V4L2_PIX_FMT_YUV422P:
-		mcam_reg_write_mask(cam, REG_CTRL0,
-			C0_DF_YUV | C0_YUV_PLANAR | C0_YUVE_YVYU, C0_DF_MASK);
-		break;
 	case V4L2_PIX_FMT_YUV420:
 	case V4L2_PIX_FMT_YVU420:
 		mcam_reg_write_mask(cam, REG_CTRL0,
@@ -1373,9 +1357,6 @@ static int mcam_vidioc_try_fmt_vid_cap(struct file *filp, void *priv,
 	v4l2_fill_pix_format(pix, &mbus_fmt);
 	pix->bytesperline = pix->width * f->bpp;
 	switch (f->pixelformat) {
-	case V4L2_PIX_FMT_YUV422P:
-		pix->sizeimage = pix->height * pix->bytesperline * 2;
-		break;
 	case V4L2_PIX_FMT_YUV420:
 	case V4L2_PIX_FMT_YVU420:
 		pix->sizeimage = pix->height * pix->bytesperline * 3 / 2;
-- 
2.1.4

[PATCHv3 20/22] videodev2.h: add V4L2_PIX_FMT_XBGR444

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This is similar to XRGB444 but with the red and blue components swapped. It
turns out that the RGB444 format in marvell-ccic driver really was this
variant, so we need a proper definition of that.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 .../DocBook/media/v4l/pixfmt-packed-rgb.xml        | 22 ++++++++++++++++++++++
 include/uapi/linux/videodev2.h                     |  7 ++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
index 6ab4f0f..cd6cc81 100644
--- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
@@ -171,6 +171,28 @@ for a pixel lie next to each other in memory.</para>
 	    <entry>r<subscript>1</subscript></entry>
 	    <entry>r<subscript>0</subscript></entry>
 	  </row>
+	  <row id="V4L2-PIX-FMT-XBGR444">
+	    <entry><constant>V4L2_PIX_FMT_XBGR444</constant></entry>
+	    <entry>'XB12'</entry>
+	    <entry></entry>
+	    <entry>g</entry>
+	    <entry>g</entry>
+	    <entry>g</entry>
+	    <entry>g</entry>
+	    <entry>r<subscript>3</subscript></entry>
+	    <entry>r<subscript>2</subscript></entry>
+	    <entry>r<subscript>1</subscript></entry>
+	    <entry>r<subscript>0</subscript></entry>
+	    <entry></entry>
+	    <entry>-<subscript>3</subscript></entry>
+	    <entry>-<subscript>2</subscript></entry>
+	    <entry>-<subscript>1</subscript></entry>
+	    <entry>-<subscript>0</subscript></entry>
+	    <entry>b<subscript>3</subscript></entry>
+	    <entry>b<subscript>2</subscript></entry>
+	    <entry>b<subscript>1</subscript></entry>
+	    <entry>b<subscript>0</subscript></entry>
+	  </row>
 	  <row id="V4L2-PIX-FMT-ARGB555">
 	    <entry><constant>V4L2_PIX_FMT_ARGB555</constant></entry>
 	    <entry>'AR15'</entry>
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index fbdc360..0b015c1 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -375,9 +375,10 @@ struct v4l2_pix_format {
 
 /* RGB formats */
 #define V4L2_PIX_FMT_RGB332  v4l2_fourcc('R', 'G', 'B', '1') /*  8  RGB-3-3-2     */
-#define V4L2_PIX_FMT_RGB444  v4l2_fourcc('R', '4', '4', '4') /* 16  xxxxrrrr ggggbbbb */
-#define V4L2_PIX_FMT_ARGB444 v4l2_fourcc('A', 'R', '1', '2') /* 16  aaaarrrr ggggbbbb */
-#define V4L2_PIX_FMT_XRGB444 v4l2_fourcc('X', 'R', '1', '2') /* 16  xxxxrrrr ggggbbbb */
+#define V4L2_PIX_FMT_RGB444  v4l2_fourcc('R', '4', '4', '4') /* 16  RGB-4-4-4 */
+#define V4L2_PIX_FMT_ARGB444 v4l2_fourcc('A', 'R', '1', '2') /* 16  RGB-4-4-4-4 */
+#define V4L2_PIX_FMT_XRGB444 v4l2_fourcc('X', 'R', '1', '2') /* 16  RGB-4-4-4-4 */
+#define V4L2_PIX_FMT_XBGR444 v4l2_fourcc('X', 'B', '1', '2') /* 16  BGR-4-4-4-4 */
 #define V4L2_PIX_FMT_RGB555  v4l2_fourcc('R', 'G', 'B', 'O') /* 16  RGB-5-5-5     */
 #define V4L2_PIX_FMT_ARGB555 v4l2_fourcc('A', 'R', '1', '5') /* 16  ARGB-1-5-5-5  */
 #define V4L2_PIX_FMT_XRGB555 v4l2_fourcc('X', 'R', '1', '5') /* 16  XRGB-1-5-5-5  */
-- 
2.1.4

[PATCHv3 21/22] marvell-ccic: replace RGB444 by XBGR444

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The RGB444 pixel format as implemented in this driver really implements
the variant where R and B are swapped, XBGR444. So replace RGB444 by XBGR444.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/marvell-ccic/mcam-core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 5b60da1..55b3563 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -138,8 +138,8 @@ static struct mcam_format_struct {
 		.planar		= true,
 	},
 	{
-		.desc		= "RGB 444",
-		.pixelformat	= V4L2_PIX_FMT_RGB444,
+		.desc		= "BGR 444",
+		.pixelformat	= V4L2_PIX_FMT_XBGR444,
 		.mbus_code	= MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE,
 		.bpp		= 2,
 		.planar		= false,
@@ -777,10 +777,9 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
 		mcam_reg_write_mask(cam, REG_CTRL0,
 			C0_DF_YUV | C0_YUV_PACKED | C0_YUVE_SWAP24, C0_DF_MASK);
 		break;
-	case V4L2_PIX_FMT_RGB444:
+	case V4L2_PIX_FMT_XBGR444:
 		mcam_reg_write_mask(cam, REG_CTRL0,
 			C0_DF_RGB | C0_RGBF_444 | C0_RGB4_XRGB, C0_DF_MASK);
-		/* Alpha value? */
 		break;
 	case V4L2_PIX_FMT_RGB565:
 		mcam_reg_write_mask(cam, REG_CTRL0,
-- 
2.1.4

[PATCHv3 22/22] Fix V4L2_PIX_FMT_SBGGR8 support

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The REG_CTRL0 register was never written if this format was selected,
instead an error was logged and whatever was last set in that register
was used.

Surprisingly, that seems to work if YUYV was selected, but we should
program this register explicitly.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/marvell-ccic/mcam-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 55b3563..6b62be1 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -785,6 +785,10 @@ static void mcam_ctlr_image(struct mcam_camera *cam)
 		mcam_reg_write_mask(cam, REG_CTRL0,
 			C0_DF_RGB | C0_RGBF_565 | C0_RGB5_BGGR, C0_DF_MASK);
 		break;
+	case V4L2_PIX_FMT_SBGGR8:
+		mcam_reg_write_mask(cam, REG_CTRL0,
+			C0_DF_RGB | C0_RGB5_GRBG, C0_DF_MASK);
+		break;
 	default:
 		cam_err(cam, "camera: unknown format: %#x\n", fmt->pixelformat);
 		break;
-- 
2.1.4

Re: [PATCHv3 00/22] marvell-ccic: drop and fix formats

[Hans Verkuil]

On 03/14/2015 12:46 PM, Hans Verkuil wrote:
> This v3 patch series replaces patch 18 from the first series.
> 
> Patch 18 and 19 are unchanged from patches 18 and 21 from the
> second series.
> 
> Patches 20-21 replace the RGB444 format by the newly defined XBGR444
> format (X means that the 'alpha' channel should be ignored and is not
> filled in). The actual layout in memory remains unchanged.
> 
> Patch 22 fixes the Bayer format.
> 
> All tested on my OLPC XO-1 laptop.

Jon, ping!

Patch 18 is merged and I have your Ack for patch 19, but I'd like your Ack
as well for patches 20-22, if possible.

Regards,

	Hans

Re: [PATCHv3 00/22] marvell-ccic: drop and fix formats

[Jonathan Corbet]

On Sat, 04 Apr 2015 16:54:42 +0200
Hans Verkuil <hverkuil@xs4all.nl> wrote:

> Jon, ping!
> 
> Patch 18 is merged and I have your Ack for patch 19, but I'd like your Ack
> as well for patches 20-22, if possible.

Sorry, I'm traveling and dealing with a bunch of stuff...lots of balls
falling all over the floor.

You can add my ack to those patches, but let me just toss out my thoughts
for consideration.  At one point, the "RGB444" format was what the OLPC
XO was going to use for performance reasons.  As I recall, when you
looked recently, that was no longer the case.  So that makes it safe to
fix things without worrying that somebody might just be crazy enough to
put a 4.x kernel on an XO and have their video be messed up.

If, however, we're going to do that, we could also just fix RGB444 to
actually return RGB.  I'd have to go back and look (and I'm far from the
manual at the moment), but I believe it was a pretty simple tweak to flip
the nibbles around.

jon