Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats
Hi Hans
On Wed, 2 Dec 2009, Hans Verkuil wrote:
On Tuesday 01 December 2009 16:22:55 Guennadi Liakhovetski wrote:
On Tue, 1 Dec 2009, Hans Verkuil wrote:
On Monday 30 November 2009 14:49:07 Guennadi Liakhovetski wrote:
Right, how about this:
/*
* These pixel codes uniquely identify data formats on the media bus.
Mostly * they correspond to similarly named V4L2_PIX_FMT_* formats,
format 0 is * reserved, V4L2_MBUS_FMT_FIXED shall be used by
host-client pairs, where the * data format is fixed. Additionally,
2X8 means that one pixel is transferred * in two 8-bit samples, BE
or LE specify in which order those samples are * transferred over the
bus: LE means that the least significant bits are * transferred
first, BE means that the most significant bits are transferred *
first, and PADHI and PADLO define which bits - low or high, in the
* incomplete high byte, are filled with padding bits.
*/
enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_FIXED = 1,
V4L2_MBUS_FMT_YUYV_2X8_LE,
V4L2_MBUS_FMT_YVYU_2X8_LE,
V4L2_MBUS_FMT_UYVY_2X8_LE,
V4L2_MBUS_FMT_VYUY_2X8_LE,
These possibly may need a comment saying that each Y/U/V sample is 8 bits
wide. I'm not sure how far we want to go with systematic naming schemes
here. Adding a short comment if there is a possible ambiguity is probably
sufficient.
Is there an ambiguity? Aren't these formats standardised?
HDMI receivers/transmitters can do YUV with 8, 10 or 12 bits. So when you say
YUYV_2X8_LE do you mean that 10 bits are transported over two bytes, or that
a Y and a U (or V) are transferred one after another? From the absence of a
PADHI or PADLO I can infer that it is the latter, but it is not exactly
obvious.
Actually, why not name these formats YUYV8, etc. and the order of the bytes
going over the bus is just the order of the text 'YUYV'. There is not really
any big or little endian issues since you just need to know the sequence of
Ys, Us and Vs.
Ok, we could keep discussing these things for a while, but I don't think
we have that time, and it's not _that_ important to me what these things
will be called - will use whatever names there are.
Just to explain 2X8 - this notation comes from packing and means, to get
one _pixel_ you need two 8-bit wide samples. With YUYV one pixel is
defined as YU or YV, so, that gives you (at most) 8 bits per component.
Ok, I'm planning to submit a version of this patch a bit later today with
names like
enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_FIXED = 1,
V4L2_MBUS_FMT_YUYV8,
V4L2_MBUS_FMT_YVYU8,
according to your last suggestion.
Do we then have
to explain what rgb555 means etc?
V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
V4L2_MBUS_FMT_RGB555X_2X8_PADHI_LE,
Shouldn't this be: V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE? Since the 555X
format is just the big-endian variant of the RGB555 if I am not mistaken.
No, don't think so. As an RGB555X format it is sent in LE order, if you
send RGB555X in BE order you get RGB555 (without an X). In fact, you'll
never have a RGB555X_BE format, because, that's just the RGB555_LE. So,
you may only have BE variants for formats, whoch byte-swapped variants do
not have an own name.
RGB 5:5:5 consists of 16 bits argg gggb ('a' is either padding or an
alpha bit).
From what I read, RGB555 has high bit unused. With the alpha bit (or
transparency bit) it's already RGBA or RGBT.
RGB 5:5:5 over an 8 bit data bus is either with the MSB byte first (big
endian
aka RGB555X aka RGB555_2X8_PADHI_BE) or with the LSB byte first (little
endian
aka RGB555 aka RGB555_2X8_PADHI_LE).
The use of RGB555X in the pixel formats is a really ugly accident of history.
'RGB555' is the name of the format, and _LE or _BE should define what the
order of the LSB and MSB over the data bus is.
So, I'll make them
V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,
and 555X will just vanish?
V4L2_MBUS_FMT_RGB565_2X8_LE,
V4L2_MBUS_FMT_RGB565X_2X8_LE,
Ditto.
V4L2_MBUS_FMT_SBGGR8_1X8,
V4L2_MBUS_FMT_SBGGR10_1X10,
V4L2_MBUS_FMT_GREY_1X8,
This is also 8 bits per sample, right? This might be renamed to
GREY8_1X8.
I named it after V4L2_PIX_FMT_GREY. If we ever get GREY7 or similar, I
think, we anyway will need a new fourcc code for it, then we'll call the
MBUS_FMT similarly.
Why not do it right from the start? Frankly, the PIX_FMT names aren't that
great. And since this will become a public API in the future I think it is
reasonable to spend some time on it (and it is the reason why I'm so picky
about it :-) ).
The whole then becomes:
/*
* These pixel codes uniquely identify data formats on the media bus. Mostly
* they correspond to similarly named V4L2_PIX_FMT_*
Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats
On Monday 30 November 2009 14:49:07 Guennadi Liakhovetski wrote: On Mon, 30 Nov 2009, Hans Verkuil wrote: On Thu, 26 Nov 2009, Guennadi Liakhovetski wrote: From 8b24c617e1ac4d324538a3eec476d48b85c2091f Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski g.liakhovet...@gmx.de Date: Thu, 26 Nov 2009 18:20:45 +0100 Subject: [PATCH] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats Video subdevices, like cameras, decoders, connect to video bridges over specialised busses. Data is being transferred over these busses in various formats, which only loosely correspond to fourcc codes, describing how video data is stored in RAM. This is not a one-to-one correspondence, therefore we cannot use fourcc codes to configure subdevice output data formats. This patch adds codes for several such on-the-bus formats and an API, similar to the familiar .s_fmt(), .g_fmt(), .try_fmt(), .enum_fmt() API for configuring those codes. After all users of the old API in struct v4l2_subdev_video_ops are converted, it will be removed. Also add helper routines to support generic pass-through mode for the soc-camera framework. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- v2 - v3: more comments: 1. moved enum v4l2_mbus_packing, enum v4l2_mbus_order, and struct v4l2_mbus_pixelfmt to soc-camera specific header, renamed them into soc-namespace. 2. commented enum v4l2_mbus_pixelcode and removed unused values. v1 - v2: addressed comments from Hans, namely: 1. renamed image bus to media bus, now using mbus as a shorthand in function and data type names. 2. made media-bus helper functions soc-camera local. 3. moved colorspace from struct v4l2_mbus_pixelfmt to struct v4l2_mbus_framefmt. 4. added documentation for data types and enums. 5. added V4L2_MBUS_FMT_FIXED = 1, format as the first in enum. soc-camera driver port will follow tomorrow. Thanks Guennadi drivers/media/video/Makefile |2 +- drivers/media/video/soc_mediabus.c | 157 include/media/soc_mediabus.h | 84 +++ include/media/v4l2-mediabus.h | 59 ++ include/media/v4l2-subdev.h| 19 - cut diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h new file mode 100644 index 000..359840c --- /dev/null +++ b/include/media/v4l2-mediabus.h @@ -0,0 +1,59 @@ +/* + * Media Bus API header + * + * Copyright (C) 2009, Guennadi Liakhovetski g.liakhovet...@gmx.de + * + * 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 V4L2_MEDIABUS_H +#define V4L2_MEDIABUS_H + +/* + * These pixel codes uniquely identify data formats on the media bus. Mostly + * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0 is + * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs, where the + * data format is fixed. Additionally, 2X8 means that one pixel is transferred + * in two 8-bit samples, BE or LE specify in which order those samples + * should be stored in RAM, and PADHI and PADLO define which bits - low or + * high, in the incomplete high byte, are filled with padding bits. + */ Hi Guennadi, This still needs some improvement. There are two things that need to be changed here: 1) add the width of the data bus to the format name: so V4L2_MBUS_FMT_YUYV becomes _FMT_YUYV_8. This is required since I know several video receivers that can be programmed to use one of several data widths (8, 10, 12 bits per color component). Perhaps _1X8 is even better since that is nicely consistent with the 2X8 suffix that you already use. The PADHI and PADLO naming convention is fine and should be used whereever it is neeeded. Ditto for BE and LE. Ok, my first thought was no, there only very seldom will be a need for a different padding / order / bus-width, but the second thought was but if there ever will be one, you'll have to rename the original code too... 2) Rephrase 'BE or LE specify in which order those samples should be stored in RAM' to: 'BE or LE specify in which order those samples are transferred over the bus: BE means that the most significant bits are transferred first, LE means that the least significant bits are transferred first.' This also means that formats like RGB555 need to be renamed to RGB555_2X8_PADHI_LE. I think that this would make this list of pixelcodes unambiguous and understandable. Right, how about this: /* * These pixel codes uniquely identify data formats on the media bus. Mostly * they correspond to similarly
Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats
On Mon, 30 Nov 2009, Hans Verkuil wrote: On Thu, 26 Nov 2009, Guennadi Liakhovetski wrote: From 8b24c617e1ac4d324538a3eec476d48b85c2091f Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski g.liakhovet...@gmx.de Date: Thu, 26 Nov 2009 18:20:45 +0100 Subject: [PATCH] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats Video subdevices, like cameras, decoders, connect to video bridges over specialised busses. Data is being transferred over these busses in various formats, which only loosely correspond to fourcc codes, describing how video data is stored in RAM. This is not a one-to-one correspondence, therefore we cannot use fourcc codes to configure subdevice output data formats. This patch adds codes for several such on-the-bus formats and an API, similar to the familiar .s_fmt(), .g_fmt(), .try_fmt(), .enum_fmt() API for configuring those codes. After all users of the old API in struct v4l2_subdev_video_ops are converted, it will be removed. Also add helper routines to support generic pass-through mode for the soc-camera framework. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- v2 - v3: more comments: 1. moved enum v4l2_mbus_packing, enum v4l2_mbus_order, and struct v4l2_mbus_pixelfmt to soc-camera specific header, renamed them into soc-namespace. 2. commented enum v4l2_mbus_pixelcode and removed unused values. v1 - v2: addressed comments from Hans, namely: 1. renamed image bus to media bus, now using mbus as a shorthand in function and data type names. 2. made media-bus helper functions soc-camera local. 3. moved colorspace from struct v4l2_mbus_pixelfmt to struct v4l2_mbus_framefmt. 4. added documentation for data types and enums. 5. added V4L2_MBUS_FMT_FIXED = 1, format as the first in enum. soc-camera driver port will follow tomorrow. Thanks Guennadi drivers/media/video/Makefile |2 +- drivers/media/video/soc_mediabus.c | 157 include/media/soc_mediabus.h | 84 +++ include/media/v4l2-mediabus.h | 59 ++ include/media/v4l2-subdev.h| 19 - cut diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h new file mode 100644 index 000..359840c --- /dev/null +++ b/include/media/v4l2-mediabus.h @@ -0,0 +1,59 @@ +/* + * Media Bus API header + * + * Copyright (C) 2009, Guennadi Liakhovetski g.liakhovet...@gmx.de + * + * 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 V4L2_MEDIABUS_H +#define V4L2_MEDIABUS_H + +/* + * These pixel codes uniquely identify data formats on the media bus. Mostly + * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0 is + * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs, where the + * data format is fixed. Additionally, 2X8 means that one pixel is transferred + * in two 8-bit samples, BE or LE specify in which order those samples + * should be stored in RAM, and PADHI and PADLO define which bits - low or + * high, in the incomplete high byte, are filled with padding bits. + */ Hi Guennadi, This still needs some improvement. There are two things that need to be changed here: 1) add the width of the data bus to the format name: so V4L2_MBUS_FMT_YUYV becomes _FMT_YUYV_8. This is required since I know several video receivers that can be programmed to use one of several data widths (8, 10, 12 bits per color component). Perhaps _1X8 is even better since that is nicely consistent with the 2X8 suffix that you already use. The PADHI and PADLO naming convention is fine and should be used whereever it is neeeded. Ditto for BE and LE. Ok, my first thought was no, there only very seldom will be a need for a different padding / order / bus-width, but the second thought was but if there ever will be one, you'll have to rename the original code too... 2) Rephrase 'BE or LE specify in which order those samples should be stored in RAM' to: 'BE or LE specify in which order those samples are transferred over the bus: BE means that the most significant bits are transferred first, LE means that the least significant bits are transferred first.' This also means that formats like RGB555 need to be renamed to RGB555_2X8_PADHI_LE. I think that this would make this list of pixelcodes unambiguous and understandable. Right, how about this: /* * These pixel codes uniquely identify data formats on the media bus. Mostly * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0 is * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs, where the * data format is fixed. Additionally, 2X8 means
[PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats
From 8b24c617e1ac4d324538a3eec476d48b85c2091f Mon Sep 17 00:00:00 2001
From: Guennadi Liakhovetski g.liakhovet...@gmx.de
Date: Thu, 26 Nov 2009 18:20:45 +0100
Subject: [PATCH] v4l: add a media-bus API for configuring v4l2 subdev pixel and
frame formats
Video subdevices, like cameras, decoders, connect to video bridges over
specialised busses. Data is being transferred over these busses in various
formats, which only loosely correspond to fourcc codes, describing how video
data is stored in RAM. This is not a one-to-one correspondence, therefore we
cannot use fourcc codes to configure subdevice output data formats. This patch
adds codes for several such on-the-bus formats and an API, similar to the
familiar .s_fmt(), .g_fmt(), .try_fmt(), .enum_fmt() API for configuring those
codes. After all users of the old API in struct v4l2_subdev_video_ops are
converted, it will be removed. Also add helper routines to support generic
pass-through mode for the soc-camera framework.
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
v2 - v3: more comments:
1. moved enum v4l2_mbus_packing, enum v4l2_mbus_order, and struct
v4l2_mbus_pixelfmt to soc-camera specific header, renamed them into
soc-namespace.
2. commented enum v4l2_mbus_pixelcode and removed unused values.
v1 - v2: addressed comments from Hans, namely:
1. renamed image bus to media bus, now using mbus as a shorthand in
function and data type names.
2. made media-bus helper functions soc-camera local.
3. moved colorspace from struct v4l2_mbus_pixelfmt to struct
v4l2_mbus_framefmt.
4. added documentation for data types and enums.
5. added
V4L2_MBUS_FMT_FIXED = 1,
format as the first in enum.
soc-camera driver port will follow tomorrow.
Thanks
Guennadi
drivers/media/video/Makefile |2 +-
drivers/media/video/soc_mediabus.c | 157
include/media/soc_mediabus.h | 84 +++
include/media/v4l2-mediabus.h | 59 ++
include/media/v4l2-subdev.h| 19 -
5 files changed, 319 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/video/soc_mediabus.c
create mode 100644 include/media/soc_mediabus.h
create mode 100644 include/media/v4l2-mediabus.h
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 7a2dcc3..e7bc8da 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -149,7 +149,7 @@ obj-$(CONFIG_VIDEO_VIVI) += vivi.o
obj-$(CONFIG_VIDEO_CX23885) += cx23885/
obj-$(CONFIG_VIDEO_OMAP2) += omap2cam.o
-obj-$(CONFIG_SOC_CAMERA) += soc_camera.o
+obj-$(CONFIG_SOC_CAMERA) += soc_camera.o soc_mediabus.o
obj-$(CONFIG_SOC_CAMERA_PLATFORM) += soc_camera_platform.o
# soc-camera host drivers have to be linked after camera drivers
obj-$(CONFIG_VIDEO_MX1)+= mx1_camera.o
diff --git a/drivers/media/video/soc_mediabus.c
b/drivers/media/video/soc_mediabus.c
new file mode 100644
index 000..f293a8d
--- /dev/null
+++ b/drivers/media/video/soc_mediabus.c
@@ -0,0 +1,157 @@
+/*
+ * soc-camera media bus helper routines
+ *
+ * Copyright (C) 2009, Guennadi Liakhovetski g.liakhovet...@gmx.de
+ *
+ * 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.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+
+#include media/v4l2-device.h
+#include media/v4l2-mediabus.h
+#include media/soc_mediabus.h
+
+#define MBUS_IDX(f) (V4L2_MBUS_FMT_ ## f - V4L2_MBUS_FMT_FIXED - 1)
+
+static const struct soc_mbus_pixelfmt mbus_fmt[] = {
+ [MBUS_IDX(YUYV)] = {
+ .fourcc = V4L2_PIX_FMT_YUYV,
+ .name = YUYV,
+ .bits_per_sample= 8,
+ .packing= SOC_MBUS_PACKING_2X8_PADHI,
+ .order = SOC_MBUS_ORDER_LE,
+ }, [MBUS_IDX(YVYU)] = {
+ .fourcc = V4L2_PIX_FMT_YVYU,
+ .name = YVYU,
+ .bits_per_sample= 8,
+ .packing= SOC_MBUS_PACKING_2X8_PADHI,
+ .order = SOC_MBUS_ORDER_LE,
+ }, [MBUS_IDX(UYVY)] = {
+ .fourcc = V4L2_PIX_FMT_UYVY,
+ .name = UYVY,
+ .bits_per_sample= 8,
+ .packing= SOC_MBUS_PACKING_2X8_PADHI,
+ .order = SOC_MBUS_ORDER_LE,
+ }, [MBUS_IDX(VYUY)] = {
+ .fourcc = V4L2_PIX_FMT_VYUY,
+ .name = VYUY,
+ .bits_per_sample= 8,
+ .packing= SOC_MBUS_PACKING_2X8_PADHI,
+ .order = SOC_MBUS_ORDER_LE,
+ },
