[PATCH RFC v2 0/5] s5p-fimc: Add interleaved image data capture support

[PATCH RFC v2 0/5] s5p-fimc: Add interleaved image data capture support

[Sylwester Nawrocki]

Hi All,

This patch series adds device/vendor specific media bus pixel code section
and defines S5C73MX camera specific media bus pixel code, along with
corresponding fourcc. I realize this isn't probably the best possible
solution but I don't know how to better handle this without major changes
in V4L2 API.

The third patch adds support for MIPI-CSI2 Embedded Data capture in
Samsung S5P/Exynos MIPI-CSIS device. It depends on patch
"[PATCH RFC] V4L: Add s_rx_buffer subdev video operation".

The fourth patch extends s5p-fimc driver to allow it to support
2-planar V4L2_PIX_FMT_S5C_UYVY_JPG format. More details can be found
in the patch summary. The [get/set]_frame_desc subdev callback are
used only to retrive from a sensor subdev required buffer size.
It depends on patch
"[PATCH RFC] V4L: Add get/set_frame_desc subdev callbacks"

The fifth patch adds [get/set]_frame_desc op handlers to the m5mols
driver as an example. I prepared also similar patch for S5C73M3
sensor where 2 frame description entries are used, but that driver
is not yet mainlined due to a few missing items in V4L2 required
to fully control it, so I didn't include that patch in this series.

Comments, suggestions welcome.

Thanks,
Sylwester

Sylwester Nawrocki (5):
  V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
  V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition
  s5p-csis: Add support for non-image data packets capture
  s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc
  m5mols: Implement .get_frame_desc subdev callback

 Documentation/DocBook/media/v4l/compat.xml         |   4 +
 Documentation/DocBook/media/v4l/pixfmt.xml         |   9 ++
 Documentation/DocBook/media/v4l/subdev-formats.xml |  45 ++++++++
 drivers/media/i2c/m5mols/m5mols.h                  |   9 ++
 drivers/media/i2c/m5mols/m5mols_capture.c          |   3 +
 drivers/media/i2c/m5mols/m5mols_core.c             |  47 ++++++++
 drivers/media/i2c/m5mols/m5mols_reg.h              |   1 +
 drivers/media/platform/s5p-fimc/fimc-capture.c     | 128 ++++++++++++++++++---
 drivers/media/platform/s5p-fimc/fimc-core.c        |  19 ++-
 drivers/media/platform/s5p-fimc/fimc-core.h        |  28 ++++-
 drivers/media/platform/s5p-fimc/fimc-reg.c         |  23 +++-
 drivers/media/platform/s5p-fimc/fimc-reg.h         |   3 +-
 drivers/media/platform/s5p-fimc/mipi-csis.c        |  59 +++++++++-
 include/linux/v4l2-mediabus.h                      |   5 +
 include/linux/videodev2.h                          |   1 +
 15 files changed, 351 insertions(+), 33 deletions(-)

--
1.7.11.3

[PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

[Sylwester Nawrocki]

This patch adds media bus pixel code for the interleaved JPEG/UYVY
image format used by S5C73MX Samsung cameras. This interleaved image
data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data.

It also defines an experimental vendor and device specific media bus
formats section and adds related DocBook documentation.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/DocBook/media/v4l/compat.xml         |  4 ++
 Documentation/DocBook/media/v4l/subdev-formats.xml | 45 ++++++++++++++++++++++
 include/linux/v4l2-mediabus.h                      |  5 +++
 3 files changed, 54 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index 98e8d08..5d2480b 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2605,6 +2605,10 @@ ioctls.</para>
         <listitem>
 	  <para>Support for frequency band enumeration: &VIDIOC-ENUM-FREQ-BANDS; ioctl.</para>
         </listitem>
+        <listitem>
+	  <para>Vendor and device specific media bus pixel formats.
+	    <xref linkend="v4l2-mbus-vendor-spec-fmts" />.</para>
+        </listitem>
       </itemizedlist>
     </section>
 
diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml b/Documentation/DocBook/media/v4l/subdev-formats.xml
index 49c532e..d7aa870 100644
--- a/Documentation/DocBook/media/v4l/subdev-formats.xml
+++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
@@ -2565,5 +2565,50 @@
 	</tgroup>
       </table>
     </section>
+
+    <section id="v4l2-mbus-vendor-spec-fmts">
+      <title>Vendor and Device Specific Formats</title>
+
+      <note>
+	<title> Experimental </title>
+	<para>This is an <link linkend="experimental">experimental</link>
+interface and may change in the future.</para>
+      </note>
+
+      <para> This section lists complex data formats that are either vendor or
+	device specific. These formats comprise raw and compressed image data
+	and optional meta-data within a single frame.
+      </para>
+
+      <para>The following table lists the existing vendor and device specific
+	formats.</para>
+
+      <table pgwide="0" frame="none" id="v4l2-mbus-pixelcode-vendor-specific">
+	<title>Vendor and device specific formats</title>
+	<tgroup cols="3">
+	  <colspec colname="id" align="left" />
+	  <colspec colname="code" align="left"/>
+	  <colspec colname="remarks" align="left"/>
+	  <thead>
+	    <row>
+	      <entry>Identifier</entry>
+	      <entry>Code</entry>
+	      <entry>Comments</entry>
+	    </row>
+	  </thead>
+	  <tbody valign="top">
+	    <row id="V4L2-MBUS-FMT-S5C-UYVY-JPG-1X8">
+	      <entry>V4L2_MBUS_FMT_S5C_UYVY_JPG_1X8</entry>
+	      <entry>0x8001</entry>
+	      <entry>
+		Interleaved raw UYVY and JPEG image format with embedded
+		meta-data, produced by S3C73M3 camera sensors.
+	      </entry>
+	    </row>
+	  </tbody>
+	</tgroup>
+      </table>
+    </section>
+
   </section>
 </section>
diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
index 5ea7f75..b98c566 100644
--- a/include/linux/v4l2-mediabus.h
+++ b/include/linux/v4l2-mediabus.h
@@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode {
 
 	/* JPEG compressed formats - next is 0x4002 */
 	V4L2_MBUS_FMT_JPEG_1X8 = 0x4001,
+
+	/* Vendor specific formats - next is 0x8002 */
+
+	/* S5C73M3 interleaved UYVY and JPEG */
+	V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 = 0x8001,
 };
 
 /**
-- 
1.7.11.3

[PATCH RFC 2/5] V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition

[Sylwester Nawrocki]

The V4L2_PIX_FMT_S5C_UYVY_JPG is a two-plane image format used by
Samsung S5C73M3 camera. The first plane contains interleaved UYVY
and JPEG data, followed by meta-data in form of offsets to the UYVU
data blocks.

The second plane contains additional meta-data needed for extracting
JPEG and UYVY data stream from the first plane, in particular an
offset to the first entry of an array of data offsets in the first
plane. The offsets are expressed as 4-byte unsigned integers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/DocBook/media/v4l/pixfmt.xml | 9 +++++++++
 include/linux/videodev2.h                  | 1 +
 2 files changed, 10 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml
index 1ddbfab..9caed9b 100644
--- a/Documentation/DocBook/media/v4l/pixfmt.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt.xml
@@ -996,6 +996,15 @@ the other bits are set to 0.</entry>
 	    <entry>Old 6-bit greyscale format. Only the most significant 6 bits of each byte are used,
 the other bits are set to 0.</entry>
 	  </row>
+	  <row id="V4L2-PIX-FMT-JPG-YUYV-S5C">
+	    <entry><constant>V4L2_PIX_FMT_S5C_YUYV_JPG</constant></entry>
+	    <entry>'S5CJ'</entry>
+	    <entry>Two-planar format used by Samsung S5C73MX cameras.The first
+plane contains interleaved JPEG and YUYV data, followed by meta data describing
+layout of the YUYV and JPEG data blocks. The second plane contains additional
+information about data layout in the first plane, like an offset to the array
+of offsets to YUVY data chunks.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 4862165..aa6fb4d 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -435,6 +435,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_KONICA420  v4l2_fourcc('K', 'O', 'N', 'I') /* YUV420 planar in blocks of 256 pixels */
 #define V4L2_PIX_FMT_JPGL	v4l2_fourcc('J', 'P', 'G', 'L') /* JPEG-Lite */
 #define V4L2_PIX_FMT_SE401      v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
+#define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'J') /* S5C73M3 interleaved YUYV/JPEG */
 
 /*
  *	F O R M A T   E N U M E R A T I O N
-- 
1.7.11.3

[PATCH RFC 3/5] s5p-csis: Add support for non-image data packets capture

[Sylwester Nawrocki]

MIPI-CSI has internal memory mapped buffers for the frame embedded
(non-image) data. There are two buffers, for even and odd frames which
need to be saved after an interrupt is raised. The packet data buffers
size is 4 KiB and there is no status register in the hardware where the
actual non-image data size can be read from. Hence the driver copies
whole packet data buffer into a buffer provided by the FIMC driver.
This will form a separate plane in the user buffer.

When FIMC DMA engine is stopped by the driver due the to user space
not keeping up with buffer de-queuing the MIPI-CSIS will still run,
however it must discard data which is not captured by FIMC. Which
frames are actually capture by MIPI-CSIS is determined by means of
the s_tx_buffer subdev callback. When it is not called after a single
embedded data frame has been captured and copied and before next
embedded data frame interrupt occurrs, subsequent embedded data frames
will be dropped.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-fimc/mipi-csis.c | 51 +++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c
index c1c5c86..306410f 100644
--- a/drivers/media/platform/s5p-fimc/mipi-csis.c
+++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
@@ -98,6 +98,11 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
 #define CSIS_MAX_PIX_WIDTH		0xffff
 #define CSIS_MAX_PIX_HEIGHT		0xffff
 
+/* Non-image packet data buffers */
+#define S5PCSIS_PKTDATA_ODD		0x2000
+#define S5PCSIS_PKTDATA_EVEN		0x3000
+#define S5PCSIS_PKTDATA_SIZE		SZ_4K
+
 enum {
 	CSIS_CLK_MUX,
 	CSIS_CLK_GATE,
@@ -144,6 +149,11 @@ static const struct s5pcsis_event s5pcsis_events[] = {
 };
 #define S5PCSIS_NUM_EVENTS ARRAY_SIZE(s5pcsis_events)
 
+struct csis_pktbuf {
+	u32 *data;
+	unsigned int len;
+};
+
 /**
  * struct csis_state - the driver's internal state data structure
  * @lock: mutex serializing the subdev and power management operations,
@@ -160,6 +170,7 @@ static const struct s5pcsis_event s5pcsis_events[] = {
  * @csis_fmt: current CSIS pixel format
  * @format: common media bus format for the source and sink pad
  * @slock: spinlock protecting structure members below
+ * @pkt_buf: the frame embedded (non-image) data buffer
  * @events: MIPI-CSIS event (error) counters
  */
 struct csis_state {
@@ -177,6 +188,7 @@ struct csis_state {
 	struct v4l2_mbus_framefmt format;
 
 	struct spinlock slock;
+	struct csis_pktbuf pkt_buf;
 	struct s5pcsis_event events[S5PCSIS_NUM_EVENTS];
 };
 
@@ -534,6 +546,21 @@ static int s5pcsis_get_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
 	return 0;
 }
 
+static int s5pcsis_s_rx_buffer(struct v4l2_subdev *sd, void *buf,
+			       unsigned int *size)
+{
+	struct csis_state *state = sd_to_csis_state(sd);
+	unsigned long flags;
+
+	spin_lock_irqsave(&state->slock, flags);
+	*size = min_t(unsigned int, *size, S5PCSIS_PKTDATA_SIZE);
+	state->pkt_buf.data = buf;
+	state->pkt_buf.len = *size;
+	spin_unlock_irqrestore(&state->slock, flags);
+
+	return 0;
+}
+
 static int s5pcsis_log_status(struct v4l2_subdev *sd)
 {
 	struct csis_state *state = sd_to_csis_state(sd);
@@ -571,6 +598,7 @@ static struct v4l2_subdev_pad_ops s5pcsis_pad_ops = {
 };
 
 static struct v4l2_subdev_video_ops s5pcsis_video_ops = {
+	.s_rx_buffer = s5pcsis_s_rx_buffer,
 	.s_stream = s5pcsis_s_stream,
 };
 
@@ -580,16 +608,39 @@ static struct v4l2_subdev_ops s5pcsis_subdev_ops = {
 	.video = &s5pcsis_video_ops,
 };
 
+static void s5pcsis_pkt_copy(struct csis_state *state, u32 *buf,
+			     u32 offset, u32 size)
+{
+	int i;
+
+	for (i = 0; i < S5PCSIS_PKTDATA_SIZE; i += 4, buf++)
+		*buf = s5pcsis_read(state, offset + i);
+}
+
 static irqreturn_t s5pcsis_irq_handler(int irq, void *dev_id)
 {
 	struct csis_state *state = dev_id;
+	struct csis_pktbuf *pktbuf = &state->pkt_buf;
 	unsigned long flags;
 	u32 status;
 
 	status = s5pcsis_read(state, S5PCSIS_INTSRC);
+	s5pcsis_write(state, S5PCSIS_INTSRC, status);
 
 	spin_lock_irqsave(&state->slock, flags);
 
+	if ((status & S5PCSIS_INTSRC_NON_IMAGE_DATA) && pktbuf->data) {
+		u32 offset;
+
+		if (status & S5PCSIS_INTSRC_EVEN)
+			offset = S5PCSIS_PKTDATA_EVEN;
+		else
+			offset = S5PCSIS_PKTDATA_ODD;
+
+		s5pcsis_pkt_copy(state, pktbuf->data, offset, pktbuf->len);
+		pktbuf->data = NULL;
+	}
+
 	/* Update the event/error counters */
 	if ((status & S5PCSIS_INTSRC_ERRORS) || debug) {
 		int i;
-- 
1.7.11.3

[PATCH RFC 4/5] s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc

[Sylwester Nawrocki]

The V4L2_PIX_FMT_S5C_YUYV_JPG image format consists of 2 planes, the
first containing interleaved JPEG/YUYV image data followed by meta-data
describing the interleaving method and the second plane containing
suplementary frame meta data.

The image data is transferred with MIPI-CSI "User Defined Byte-Based
Data 1" type and is captured to memory by FIMC DMA engine.

The meta data is transferred using MIPI-CSI2 "Embedded 8-bit non Image
Data" and it is captured in the MIPI-CSI slave device and copied to
the bridge provided buffer.

To make sure the size of allocated buffers is correct for the subdevs
configuration when VIDIOC_STREAMON ioctl is invoked, an additional
check is added at the video pipeline validation function.

Flag FMT_FLAGS_COMPRESSED indicates the buffer size must be retrieved
from a sensor subdev.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-fimc/fimc-capture.c | 128 +++++++++++++++++++++----
 drivers/media/platform/s5p-fimc/fimc-core.c    |  19 +++-
 drivers/media/platform/s5p-fimc/fimc-core.h    |  28 +++++-
 drivers/media/platform/s5p-fimc/fimc-reg.c     |  23 ++++-
 drivers/media/platform/s5p-fimc/fimc-reg.h     |   3 +-
 drivers/media/platform/s5p-fimc/mipi-csis.c    |   8 +-
 6 files changed, 176 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c b/drivers/media/platform/s5p-fimc/fimc-capture.c
index 98d420f..a1f6c3c 100644
--- a/drivers/media/platform/s5p-fimc/fimc-capture.c
+++ b/drivers/media/platform/s5p-fimc/fimc-capture.c
@@ -177,7 +177,9 @@ static int fimc_capture_config_update(struct fimc_ctx *ctx)

 void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf)
 {
+	struct v4l2_subdev *csis = fimc->pipeline.subdevs[IDX_CSIS];
 	struct fimc_vid_cap *cap = &fimc->vid_cap;
+	struct fimc_frame *f = &cap->ctx->d_frame;
 	struct fimc_vid_buffer *v_buf;
 	struct timeval *tv;
 	struct timespec ts;
@@ -216,6 +218,25 @@ void fimc_capture_irq_handler(struct fimc_dev *fimc, int deq_buf)
 		if (++cap->buf_index >= FIMC_MAX_OUT_BUFS)
 			cap->buf_index = 0;
 	}
+	/*
+	 * Set up a buffer at MIPI-CSIS if current image format
+	 * requires the frame embedded data capture.
+	 */
+	if (f->fmt->mdataplanes && !list_empty(&cap->active_buf_q)) {
+		unsigned int plane = ffs(f->fmt->mdataplanes) - 1;
+		unsigned int size = f->payload[plane];
+		s32 index = fimc_hw_get_frame_index(fimc);
+		void *vaddr;
+
+		list_for_each_entry(v_buf, &cap->active_buf_q, list) {
+			if (v_buf->index != index)
+				continue;
+			vaddr = vb2_plane_vaddr(&v_buf->vb, plane);
+			v4l2_subdev_call(csis, video, s_rx_buffer,
+					 vaddr, &size);
+			break;
+		}
+	}

 	if (cap->active_buf_cnt == 0) {
 		if (deq_buf)
@@ -351,6 +372,8 @@ static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *pfmt,
 		unsigned int size = (wh * fmt->depth[i]) / 8;
 		if (pixm)
 			sizes[i] = max(size, pixm->plane_fmt[i].sizeimage);
+		else if (fimc_fmt_is_user_defined(fmt->color))
+			sizes[i] = frame->payload[i];
 		else
 			sizes[i] = max_t(u32, size, frame->payload[i]);

@@ -611,10 +634,10 @@ static struct fimc_fmt *fimc_capture_try_format(struct fimc_ctx *ctx,
 	u32 mask = FMT_FLAGS_CAM;
 	struct fimc_fmt *ffmt;

-	/* Color conversion from/to JPEG is not supported */
+	/* Conversion from/to JPEG or User Defined format is not supported */
 	if (code && ctx->s_frame.fmt && pad == FIMC_SD_PAD_SOURCE &&
-	    fimc_fmt_is_jpeg(ctx->s_frame.fmt->color))
-		*code = V4L2_MBUS_FMT_JPEG_1X8;
+	    fimc_fmt_is_user_defined(ctx->s_frame.fmt->color))
+		*code = ctx->s_frame.fmt->mbus_code;

 	if (fourcc && *fourcc != V4L2_PIX_FMT_JPEG && pad != FIMC_SD_PAD_SINK)
 		mask |= FMT_FLAGS_M2M;
@@ -628,18 +651,19 @@ static struct fimc_fmt *fimc_capture_try_format(struct fimc_ctx *ctx,
 		*fourcc = ffmt->fourcc;

 	if (pad == FIMC_SD_PAD_SINK) {
-		max_w = fimc_fmt_is_jpeg(ffmt->color) ?
+		max_w = fimc_fmt_is_user_defined(ffmt->color) ?
 			pl->scaler_dis_w : pl->scaler_en_w;
 		/* Apply the camera input interface pixel constraints */
 		v4l_bound_align_image(width, max_t(u32, *width, 32), max_w, 4,
 				      height, max_t(u32, *height, 32),
 				      FIMC_CAMIF_MAX_HEIGHT,
-				      fimc_fmt_is_jpeg(ffmt->color) ? 3 : 1,
+				      fimc_fmt_is_user_defined(ffmt->color) ?
+				      3 : 1,
 				      0);
 		return ffmt;
 	}
 	/* Can't scale or crop in transparent (JPEG) transfer mode */
-	if (fimc_fmt_is_jpeg(ffmt->color)) {
+	if (fimc_fmt_is_user_defined(ffmt->color)) {
 		*width  = ctx->s_frame.f_width;
 		*height = ctx->s_frame.f_height;
 		return ffmt;
@@ -684,7 +708,7 @@ static void fimc_capture_try_selection(struct fimc_ctx *ctx,
 	u32 max_sc_h, max_sc_v;

 	/* In JPEG transparent transfer mode cropping is not supported */
-	if (fimc_fmt_is_jpeg(ctx->d_frame.fmt->color)) {
+	if (fimc_fmt_is_user_defined(ctx->d_frame.fmt->color)) {
 		r->width  = sink->f_width;
 		r->height = sink->f_height;
 		r->left   = r->top = 0;
@@ -847,6 +871,41 @@ static int fimc_pipeline_try_format(struct fimc_ctx *ctx,
 	return 0;
 }

+/* Query the sensor for maximum buffer size (only for compressed/blob data). */
+static int get_subdev_frame_desc(struct v4l2_subdev *sensor,
+				 struct v4l2_plane_pix_format *plane_fmt,
+				 unsigned int num_planes, bool try)
+{
+	struct v4l2_mbus_frame_desc fd;
+	int i, ret;
+
+	for (i = 0; i < num_planes; i++)
+		fd.entry[i].length = plane_fmt[i].sizeimage;
+
+	if (try)
+		/* Try to set requested maximum frame size at the sensor */
+		ret = v4l2_subdev_call(sensor, pad, set_frame_desc, 0, &fd);
+	else
+		ret = v4l2_subdev_call(sensor, pad, get_frame_desc, 0, &fd);
+
+	if (ret < 0)
+		return ret;
+
+	if (num_planes != fd.num_entries)
+		return -EINVAL;
+
+	for (i = 0; i < num_planes; i++)
+		plane_fmt[i].sizeimage = fd.entry[i].length;
+
+	if (fd.entry[0].length > FIMC_MAX_JPEG_BUF_SIZE) {
+		pr_err("%s: Unsupported buffer size: %u\n", __func__,
+		       fd.entry[0].length);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int fimc_cap_g_fmt_mplane(struct file *file, void *fh,
 				 struct v4l2_format *f)
 {
@@ -865,7 +924,7 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 	struct v4l2_mbus_framefmt mf;
 	struct fimc_fmt *ffmt = NULL;

-	if (pix->pixelformat == V4L2_PIX_FMT_JPEG) {
+	if (fimc_jpeg_fourcc(pix->pixelformat)) {
 		fimc_capture_try_format(ctx, &pix->width, &pix->height,
 					NULL, &pix->pixelformat,
 					FIMC_SD_PAD_SINK);
@@ -879,25 +938,31 @@ static int fimc_cap_try_fmt_mplane(struct file *file, void *fh,
 		return -EINVAL;

 	if (!fimc->vid_cap.user_subdev_api) {
-		mf.width  = pix->width;
+		mf.width = pix->width;
 		mf.height = pix->height;
-		mf.code   = ffmt->mbus_code;
+		mf.code = ffmt->mbus_code;
 		fimc_md_graph_lock(fimc);
 		fimc_pipeline_try_format(ctx, &mf, &ffmt, false);
 		fimc_md_graph_unlock(fimc);
-
-		pix->width	 = mf.width;
-		pix->height	 = mf.height;
+		pix->width = mf.width;
+		pix->height = mf.height;
 		if (ffmt)
 			pix->pixelformat = ffmt->fourcc;
 	}

 	fimc_adjust_mplane_format(ffmt, pix->width, pix->height, pix);
+
+	if (ffmt->flags & FMT_FLAGS_COMPRESSED)
+		get_subdev_frame_desc(fimc->pipeline.subdevs[IDX_SENSOR],
+				      pix->plane_fmt, ffmt->memplanes, true);
 	return 0;
 }

-static void fimc_capture_mark_jpeg_xfer(struct fimc_ctx *ctx, bool jpeg)
+static void fimc_capture_mark_jpeg_xfer(struct fimc_ctx *ctx,
+					enum fimc_color_fmt color)
 {
+	bool jpeg = fimc_fmt_is_user_defined(color);
+
 	ctx->scaler.enabled = !jpeg;
 	fimc_ctrls_activate(ctx, !jpeg);

@@ -920,7 +985,7 @@ static int fimc_capture_set_format(struct fimc_dev *fimc, struct v4l2_format *f)
 		return -EBUSY;

 	/* Pre-configure format at camera interface input, for JPEG only */
-	if (pix->pixelformat == V4L2_PIX_FMT_JPEG) {
+	if (fimc_jpeg_fourcc(pix->pixelformat)) {
 		fimc_capture_try_format(ctx, &pix->width, &pix->height,
 					NULL, &pix->pixelformat,
 					FIMC_SD_PAD_SINK);
@@ -953,7 +1018,16 @@ static int fimc_capture_set_format(struct fimc_dev *fimc, struct v4l2_format *f)
 	}

 	fimc_adjust_mplane_format(ff->fmt, pix->width, pix->height, pix);
-	for (i = 0; i < ff->fmt->colplanes; i++)
+
+	if (ff->fmt->flags & FMT_FLAGS_COMPRESSED) {
+		ret = get_subdev_frame_desc(fimc->pipeline.subdevs[IDX_SENSOR],
+					    pix->plane_fmt, ff->fmt->memplanes,
+					    true);
+		if (ret < 0)
+			return ret;
+	}
+
+	for (i = 0; i < ff->fmt->memplanes; i++)
 		ff->payload[i] = pix->plane_fmt[i].sizeimage;

 	set_frame_bounds(ff, pix->width, pix->height);
@@ -961,7 +1035,7 @@ static int fimc_capture_set_format(struct fimc_dev *fimc, struct v4l2_format *f)
 	if (!(ctx->state & FIMC_COMPOSE))
 		set_frame_crop(ff, 0, 0, pix->width, pix->height);

-	fimc_capture_mark_jpeg_xfer(ctx, fimc_fmt_is_jpeg(ff->fmt->color));
+	fimc_capture_mark_jpeg_xfer(ctx, ff->fmt->color);

 	/* Reset cropping and set format at the camera interface input */
 	if (!fimc->vid_cap.user_subdev_api) {
@@ -1063,6 +1137,24 @@ static int fimc_pipeline_validate(struct fimc_dev *fimc)
 		    src_fmt.format.height != sink_fmt.format.height ||
 		    src_fmt.format.code != sink_fmt.format.code)
 			return -EPIPE;
+
+		if (sd == fimc->pipeline.subdevs[IDX_SENSOR] &&
+		    fimc_user_defined_mbus_fmt(src_fmt.format.code)) {
+			struct v4l2_plane_pix_format plane_fmt[FIMC_MAX_PLANES];
+			struct fimc_frame *frame = &vid_cap->ctx->d_frame;
+			unsigned int i;
+
+			ret = get_subdev_frame_desc(sd, plane_fmt,
+						    frame->fmt->memplanes,
+						    false);
+			if (ret < 0)
+				return -EPIPE;
+
+			for (i = 0; i < frame->fmt->memplanes; i++) {
+				if (frame->payload[i] < plane_fmt[i].sizeimage)
+					return -EPIPE;
+			}
+		}
 	}
 	return 0;
 }
@@ -1424,7 +1516,7 @@ static int fimc_subdev_set_fmt(struct v4l2_subdev *sd,
 	/* Update RGB Alpha control state and value range */
 	fimc_alpha_ctrl_update(ctx);

-	fimc_capture_mark_jpeg_xfer(ctx, fimc_fmt_is_jpeg(ffmt->color));
+	fimc_capture_mark_jpeg_xfer(ctx, ffmt->color);

 	ff = fmt->pad == FIMC_SD_PAD_SINK ?
 		&ctx->s_frame : &ctx->d_frame;
diff --git a/drivers/media/platform/s5p-fimc/fimc-core.c b/drivers/media/platform/s5p-fimc/fimc-core.c
index 1a44540..8d0d2b9 100644
--- a/drivers/media/platform/s5p-fimc/fimc-core.c
+++ b/drivers/media/platform/s5p-fimc/fimc-core.c
@@ -184,7 +184,17 @@ static struct fimc_fmt fimc_formats[] = {
 		.memplanes	= 1,
 		.colplanes	= 1,
 		.mbus_code	= V4L2_MBUS_FMT_JPEG_1X8,
-		.flags		= FMT_FLAGS_CAM,
+		.flags		= FMT_FLAGS_CAM | FMT_FLAGS_COMPRESSED,
+	}, {
+		.name		= "S5C73MX interleaved UYVY/JPEG",
+		.fourcc		= V4L2_PIX_FMT_S5C_UYVY_JPG,
+		.color		= FIMC_FMT_YUYV_JPEG,
+		.depth		= { 8 },
+		.memplanes	= 2,
+		.colplanes	= 1,
+		.mdataplanes	= 0x2, /* plane 1 holds frame meta data */
+		.mbus_code	= V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8,
+		.flags		= FMT_FLAGS_CAM | FMT_FLAGS_COMPRESSED,
 	},
 };

@@ -371,7 +381,7 @@ int fimc_prepare_addr(struct fimc_ctx *ctx, struct vb2_buffer *vb,
 		default:
 			return -EINVAL;
 		}
-	} else {
+	} else if (!frame->fmt->mdataplanes) {
 		if (frame->fmt->memplanes >= 2)
 			paddr->cb = vb2_dma_contig_plane_dma_addr(vb, 1);

@@ -698,6 +708,11 @@ int fimc_fill_format(struct fimc_frame *frame, struct v4l2_format *f)
 		if (frame->fmt->colplanes == 1) /* packed formats */
 			bpl = (bpl * frame->fmt->depth[0]) / 8;
 		pixm->plane_fmt[i].bytesperline = bpl;
+
+		if (frame->fmt->flags & FMT_FLAGS_COMPRESSED) {
+			pixm->plane_fmt[i].sizeimage = frame->payload[i];
+			continue;
+		}
 		pixm->plane_fmt[i].sizeimage = (frame->o_width *
 			frame->o_height * frame->fmt->depth[i]) / 8;
 	}
diff --git a/drivers/media/platform/s5p-fimc/fimc-core.h b/drivers/media/platform/s5p-fimc/fimc-core.h
index cd716ba..c0040d7 100644
--- a/drivers/media/platform/s5p-fimc/fimc-core.h
+++ b/drivers/media/platform/s5p-fimc/fimc-core.h
@@ -40,6 +40,8 @@
 #define SCALER_MAX_VRATIO	64
 #define DMA_MIN_SIZE		8
 #define FIMC_CAMIF_MAX_HEIGHT	0x2000
+#define FIMC_MAX_JPEG_BUF_SIZE	(10 * SZ_1M)
+#define FIMC_MAX_PLANES		3

 /* indices to the clocks array */
 enum {
@@ -83,7 +85,7 @@ enum fimc_datapath {
 };

 enum fimc_color_fmt {
-	FIMC_FMT_RGB444 = 0x10,
+	FIMC_FMT_RGB444	= 0x10,
 	FIMC_FMT_RGB555,
 	FIMC_FMT_RGB565,
 	FIMC_FMT_RGB666,
@@ -95,14 +97,15 @@ enum fimc_color_fmt {
 	FIMC_FMT_CBYCRY422,
 	FIMC_FMT_CRYCBY422,
 	FIMC_FMT_YCBCR444_LOCAL,
-	FIMC_FMT_JPEG = 0x40,
-	FIMC_FMT_RAW8 = 0x80,
+	FIMC_FMT_RAW8 = 0x40,
 	FIMC_FMT_RAW10,
 	FIMC_FMT_RAW12,
+	FIMC_FMT_JPEG = 0x80,
+	FIMC_FMT_YUYV_JPEG = 0x100,
 };

+#define fimc_fmt_is_user_defined(x) (!!((x) & 0x180))
 #define fimc_fmt_is_rgb(x) (!!((x) & 0x10))
-#define fimc_fmt_is_jpeg(x) (!!((x) & 0x40))

 #define IS_M2M(__strt) ((__strt) == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE || \
 			__strt == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
@@ -139,6 +142,7 @@ enum fimc_color_fmt {
  * @memplanes: number of physically non-contiguous data planes
  * @colplanes: number of physically contiguous data planes
  * @depth: per plane driver's private 'number of bits per pixel'
+ * @mdataplanes: bitmask indicating meta data plane(s), (1 << plane_no)
  * @flags: flags indicating which operation mode format applies to
  */
 struct fimc_fmt {
@@ -149,12 +153,14 @@ struct fimc_fmt {
 	u16	memplanes;
 	u16	colplanes;
 	u8	depth[VIDEO_MAX_PLANES];
+	u16	mdataplanes;
 	u16	flags;
 #define FMT_FLAGS_CAM		(1 << 0)
 #define FMT_FLAGS_M2M_IN	(1 << 1)
 #define FMT_FLAGS_M2M_OUT	(1 << 2)
 #define FMT_FLAGS_M2M		(1 << 1 | 1 << 2)
 #define FMT_HAS_ALPHA		(1 << 3)
+#define FMT_FLAGS_COMPRESSED	(1 << 4)
 };

 /**
@@ -272,7 +278,7 @@ struct fimc_frame {
 	u32	offs_v;
 	u32	width;
 	u32	height;
-	unsigned long		payload[VIDEO_MAX_PLANES];
+	unsigned int		payload[VIDEO_MAX_PLANES];
 	struct fimc_addr	paddr;
 	struct fimc_dma_offset	dma_offset;
 	struct fimc_fmt		*fmt;
@@ -577,6 +583,18 @@ static inline int tiled_fmt(struct fimc_fmt *fmt)
 	return fmt->fourcc == V4L2_PIX_FMT_NV12MT;
 }

+static inline bool fimc_jpeg_fourcc(u32 pixelformat)
+{
+	return (pixelformat == V4L2_PIX_FMT_JPEG ||
+		pixelformat == V4L2_PIX_FMT_S5C_UYVY_JPG);
+}
+
+static inline bool fimc_user_defined_mbus_fmt(u32 code)
+{
+	return (code == V4L2_MBUS_FMT_JPEG_1X8 ||
+		code == V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8);
+}
+
 /* Return the alpha component bit mask */
 static inline int fimc_get_alpha_mask(struct fimc_fmt *fmt)
 {
diff --git a/drivers/media/platform/s5p-fimc/fimc-reg.c b/drivers/media/platform/s5p-fimc/fimc-reg.c
index 783408f..2c9d0c0 100644
--- a/drivers/media/platform/s5p-fimc/fimc-reg.c
+++ b/drivers/media/platform/s5p-fimc/fimc-reg.c
@@ -625,7 +625,7 @@ int fimc_hw_set_camera_source(struct fimc_dev *fimc,
 				cfg |= FIMC_REG_CISRCFMT_ITU601_16BIT;
 		} /* else defaults to ITU-R BT.656 8-bit */
 	} else if (cam->bus_type == FIMC_MIPI_CSI2) {
-		if (fimc_fmt_is_jpeg(f->fmt->color))
+		if (fimc_fmt_is_user_defined(f->fmt->color))
 			cfg |= FIMC_REG_CISRCFMT_ITU601_8BIT;
 	}

@@ -680,6 +680,7 @@ int fimc_hw_set_camera_type(struct fimc_dev *fimc,
 			tmp = FIMC_REG_CSIIMGFMT_YCBCR422_8BIT;
 			break;
 		case V4L2_MBUS_FMT_JPEG_1X8:
+		case V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8:
 			tmp = FIMC_REG_CSIIMGFMT_USER(1);
 			cfg |= FIMC_REG_CIGCTRL_CAM_JPEG;
 			break;
@@ -744,13 +745,13 @@ void fimc_hw_dis_capture(struct fimc_dev *dev)
 }

 /* Return an index to the buffer actually being written. */
-u32 fimc_hw_get_frame_index(struct fimc_dev *dev)
+s32 fimc_hw_get_frame_index(struct fimc_dev *dev)
 {
-	u32 reg;
+	s32 reg;

 	if (dev->variant->has_cistatus2) {
-		reg = readl(dev->regs + FIMC_REG_CISTATUS2) & 0x3F;
-		return reg > 0 ? --reg : reg;
+		reg = readl(dev->regs + FIMC_REG_CISTATUS2) & 0x3f;
+		return reg - 1;
 	}

 	reg = readl(dev->regs + FIMC_REG_CISTATUS);
@@ -759,6 +760,18 @@ u32 fimc_hw_get_frame_index(struct fimc_dev *dev)
 		FIMC_REG_CISTATUS_FRAMECNT_SHIFT;
 }

+/* Return an index to the buffer being written previously. */
+s32 fimc_hw_get_prev_frame_index(struct fimc_dev *dev)
+{
+	s32 reg;
+
+	if (!dev->variant->has_cistatus2)
+		return -1;
+
+	reg = readl(dev->regs + FIMC_REG_CISTATUS2);
+	return ((reg >> 7) & 0x3f) - 1;
+}
+
 /* Locking: the caller holds fimc->slock */
 void fimc_activate_capture(struct fimc_ctx *ctx)
 {
diff --git a/drivers/media/platform/s5p-fimc/fimc-reg.h b/drivers/media/platform/s5p-fimc/fimc-reg.h
index 579ac8a..b6abfc7 100644
--- a/drivers/media/platform/s5p-fimc/fimc-reg.h
+++ b/drivers/media/platform/s5p-fimc/fimc-reg.h
@@ -307,7 +307,8 @@ void fimc_hw_clear_irq(struct fimc_dev *dev);
 void fimc_hw_enable_scaler(struct fimc_dev *dev, bool on);
 void fimc_hw_activate_input_dma(struct fimc_dev *dev, bool on);
 void fimc_hw_dis_capture(struct fimc_dev *dev);
-u32 fimc_hw_get_frame_index(struct fimc_dev *dev);
+s32 fimc_hw_get_frame_index(struct fimc_dev *dev);
+s32 fimc_hw_get_prev_frame_index(struct fimc_dev *dev);
 void fimc_activate_capture(struct fimc_ctx *ctx);
 void fimc_deactivate_capture(struct fimc_dev *fimc);

diff --git a/drivers/media/platform/s5p-fimc/mipi-csis.c b/drivers/media/platform/s5p-fimc/mipi-csis.c
index 306410f..384a5df 100644
--- a/drivers/media/platform/s5p-fimc/mipi-csis.c
+++ b/drivers/media/platform/s5p-fimc/mipi-csis.c
@@ -216,7 +216,11 @@ static const struct csis_pix_format s5pcsis_formats[] = {
 		.code = V4L2_MBUS_FMT_JPEG_1X8,
 		.fmt_reg = S5PCSIS_CFG_FMT_USER(1),
 		.data_alignment = 32,
-	},
+	}, {
+		.code = V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8,
+		.fmt_reg = S5PCSIS_CFG_FMT_USER(1),
+		.data_alignment = 32,
+	}
 };

 #define s5pcsis_write(__csis, __r, __v) writel(__v, __csis->regs + __r)
@@ -280,7 +284,7 @@ static void __s5pcsis_set_format(struct csis_state *state)
 	struct v4l2_mbus_framefmt *mf = &state->format;
 	u32 val;

-	v4l2_dbg(1, debug, &state->sd, "fmt: %d, %d x %d\n",
+	v4l2_dbg(1, debug, &state->sd, "fmt: %#x, %d x %d\n",
 		 mf->code, mf->width, mf->height);

 	/* Color format */
--
1.7.11.3

[PATCH RFC 5/5] m5mols: Implement .get_frame_desc subdev callback

[Sylwester Nawrocki]

.get_frame_desc can be used by host interface driver to query
properties of captured frames, e.g. required memory buffer size.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/i2c/m5mols/m5mols.h         |  9 ++++++
 drivers/media/i2c/m5mols/m5mols_capture.c |  3 ++
 drivers/media/i2c/m5mols/m5mols_core.c    | 47 +++++++++++++++++++++++++++++++
 drivers/media/i2c/m5mols/m5mols_reg.h     |  1 +
 4 files changed, 60 insertions(+)

diff --git a/drivers/media/i2c/m5mols/m5mols.h b/drivers/media/i2c/m5mols/m5mols.h
index 15d3a4f..de3b755 100644
--- a/drivers/media/i2c/m5mols/m5mols.h
+++ b/drivers/media/i2c/m5mols/m5mols.h
@@ -19,6 +19,13 @@
 #include <media/v4l2-subdev.h>
 #include "m5mols_reg.h"
 
+
+/* An amount of data transmitted in addition to the value
+ * determined by CAPP_JPEG_SIZE_MAX register.
+ */
+#define M5MOLS_JPEG_TAGS_SIZE		0x20000
+#define M5MOLS_MAIN_JPEG_SIZE_MAX	(5 * SZ_1M)
+
 extern int m5mols_debug;
 
 enum m5mols_restype {
@@ -67,12 +74,14 @@ struct m5mols_exif {
 /**
  * struct m5mols_capture - Structure for the capture capability
  * @exif: EXIF information
+ * @buf_size: internal JPEG frame buffer size, in bytes
  * @main: size in bytes of the main image
  * @thumb: size in bytes of the thumb image, if it was accompanied
  * @total: total size in bytes of the produced image
  */
 struct m5mols_capture {
 	struct m5mols_exif exif;
+	unsigned int buf_size;
 	u32 main;
 	u32 thumb;
 	u32 total;
diff --git a/drivers/media/i2c/m5mols/m5mols_capture.c b/drivers/media/i2c/m5mols/m5mols_capture.c
index cb243bd..ab34cce 100644
--- a/drivers/media/i2c/m5mols/m5mols_capture.c
+++ b/drivers/media/i2c/m5mols/m5mols_capture.c
@@ -105,6 +105,7 @@ static int m5mols_capture_info(struct m5mols_info *info)
 
 int m5mols_start_capture(struct m5mols_info *info)
 {
+	unsigned int framesize = info->cap.buf_size - M5MOLS_JPEG_TAGS_SIZE;
 	struct v4l2_subdev *sd = &info->sd;
 	int ret;
 
@@ -121,6 +122,8 @@ int m5mols_start_capture(struct m5mols_info *info)
 	if (!ret)
 		ret = m5mols_write(sd, CAPP_MAIN_IMAGE_SIZE, info->resolution);
 	if (!ret)
+		ret = m5mols_write(sd, CAPP_JPEG_SIZE_MAX, framesize);
+	if (!ret)
 		ret = m5mols_set_mode(info, REG_CAPTURE);
 	if (!ret)
 		/* Wait until a frame is captured to ISP internal memory */
diff --git a/drivers/media/i2c/m5mols/m5mols_core.c b/drivers/media/i2c/m5mols/m5mols_core.c
index 933014f..c780689 100644
--- a/drivers/media/i2c/m5mols/m5mols_core.c
+++ b/drivers/media/i2c/m5mols/m5mols_core.c
@@ -599,6 +599,51 @@ static int m5mols_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
 	return ret;
 }
 
+static int m5mols_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				 struct v4l2_mbus_frame_desc *fd)
+{
+	struct m5mols_info *info = to_m5mols(sd);
+
+	if (pad != 0 || fd == NULL)
+		return -EINVAL;
+
+	mutex_lock(&info->lock);
+	/*
+	 * .get_frame_desc is only used for compressed formats,
+	 * thus we always return the capture frame parameters here.
+	 */
+	fd->entry[0].length = info->cap.buf_size;
+	fd->entry[0].pixelcode = info->ffmt[M5MOLS_RESTYPE_CAPTURE].code;
+	mutex_unlock(&info->lock);
+
+	fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
+	fd->num_entries = 1;
+
+	return 0;
+}
+
+static int m5mols_set_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				 struct v4l2_mbus_frame_desc *fd)
+{
+	struct m5mols_info *info = to_m5mols(sd);
+	struct v4l2_mbus_framefmt *mf = &info->ffmt[M5MOLS_RESTYPE_CAPTURE];
+
+	if (pad != 0 || fd == NULL)
+		return -EINVAL;
+
+	fd->entry[0].flags = V4L2_MBUS_FRAME_DESC_FL_LEN_MAX;
+	fd->num_entries = 1;
+	fd->entry[0].length = clamp_t(u32, fd->entry[0].length,
+				      mf->width * mf->height,
+				      M5MOLS_MAIN_JPEG_SIZE_MAX);
+	mutex_lock(&info->lock);
+	info->cap.buf_size = fd->entry[0].length;
+	mutex_unlock(&info->lock);
+
+	return 0;
+}
+
+
 static int m5mols_enum_mbus_code(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_fh *fh,
 				 struct v4l2_subdev_mbus_code_enum *code)
@@ -615,6 +660,8 @@ static struct v4l2_subdev_pad_ops m5mols_pad_ops = {
 	.enum_mbus_code	= m5mols_enum_mbus_code,
 	.get_fmt	= m5mols_get_fmt,
 	.set_fmt	= m5mols_set_fmt,
+	.get_frame_desc	= m5mols_get_frame_desc,
+	.set_frame_desc	= m5mols_set_frame_desc,
 };
 
 /**
diff --git a/drivers/media/i2c/m5mols/m5mols_reg.h b/drivers/media/i2c/m5mols/m5mols_reg.h
index 14d4be7..58d8027 100644
--- a/drivers/media/i2c/m5mols/m5mols_reg.h
+++ b/drivers/media/i2c/m5mols/m5mols_reg.h
@@ -310,6 +310,7 @@
 #define REG_JPEG		0x10
 
 #define CAPP_MAIN_IMAGE_SIZE	I2C_REG(CAT_CAPT_PARM, 0x01, 1)
+#define CAPP_JPEG_SIZE_MAX	I2C_REG(CAT_CAPT_PARM, 0x0f, 4)
 #define CAPP_JPEG_RATIO		I2C_REG(CAT_CAPT_PARM, 0x17, 1)
 
 #define CAPP_MCC_MODE		I2C_REG(CAT_CAPT_PARM, 0x1d, 1)
-- 
1.7.11.3

Re: [PATCH RFC v2 0/5] s5p-fimc: Add interleaved image data capture support

[Sylwester Nawrocki]

On 09/24/2012 04:55 PM, Sylwester Nawrocki wrote:
> Hi All,
> 
> This patch series adds device/vendor specific media bus pixel code section
> and defines S5C73MX camera specific media bus pixel code, along with
> corresponding fourcc. I realize this isn't probably the best possible
> solution but I don't know how to better handle this without major changes
> in V4L2 API.
> 
> The third patch adds support for MIPI-CSI2 Embedded Data capture in
> Samsung S5P/Exynos MIPI-CSIS device. It depends on patch
> "[PATCH RFC] V4L: Add s_rx_buffer subdev video operation".
> 
> The fourth patch extends s5p-fimc driver to allow it to support
> 2-planar V4L2_PIX_FMT_S5C_UYVY_JPG format. More details can be found
> in the patch summary. The [get/set]_frame_desc subdev callback are
> used only to retrive from a sensor subdev required buffer size.
> It depends on patch
> "[PATCH RFC] V4L: Add get/set_frame_desc subdev callbacks"
> 
> The fifth patch adds [get/set]_frame_desc op handlers to the m5mols
> driver as an example. I prepared also similar patch for S5C73M3
> sensor where 2 frame description entries are used, but that driver
> is not yet mainlined due to a few missing items in V4L2 required
> to fully control it, so I didn't include that patch in this series.

I forgot to mention that this patch series with all dependencies
can be found in git repository

git://git.infradead.org/users/kmpark/linux-samsung v4l-framedesc

http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/v4l-framedesc

> Comments, suggestions welcome.
> 
> Thanks,
> Sylwester
> 
> Sylwester Nawrocki (5):
>   V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
>   V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition
>   s5p-csis: Add support for non-image data packets capture
>   s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc
>   m5mols: Implement .get_frame_desc subdev callback
> 
>  Documentation/DocBook/media/v4l/compat.xml         |   4 +
>  Documentation/DocBook/media/v4l/pixfmt.xml         |   9 ++
>  Documentation/DocBook/media/v4l/subdev-formats.xml |  45 ++++++++
>  drivers/media/i2c/m5mols/m5mols.h                  |   9 ++
>  drivers/media/i2c/m5mols/m5mols_capture.c          |   3 +
>  drivers/media/i2c/m5mols/m5mols_core.c             |  47 ++++++++
>  drivers/media/i2c/m5mols/m5mols_reg.h              |   1 +
>  drivers/media/platform/s5p-fimc/fimc-capture.c     | 128 ++++++++++++++++++---
>  drivers/media/platform/s5p-fimc/fimc-core.c        |  19 ++-
>  drivers/media/platform/s5p-fimc/fimc-core.h        |  28 ++++-
>  drivers/media/platform/s5p-fimc/fimc-reg.c         |  23 +++-
>  drivers/media/platform/s5p-fimc/fimc-reg.h         |   3 +-
>  drivers/media/platform/s5p-fimc/mipi-csis.c        |  59 +++++++++-
>  include/linux/v4l2-mediabus.h                      |   5 +
>  include/linux/videodev2.h                          |   1 +
>  15 files changed, 351 insertions(+), 33 deletions(-)
> 
> --
> 1.7.11.3

Re: [PATCH RFC v2 0/5] s5p-fimc: Add interleaved image data capture support

[Laurent Pinchart]

Hi Sylwester,

On Monday 24 September 2012 16:55:41 Sylwester Nawrocki wrote:
> Hi All,
> 
> This patch series adds device/vendor specific media bus pixel code section
> and defines S5C73MX camera specific media bus pixel code, along with
> corresponding fourcc. I realize this isn't probably the best possible
> solution but I don't know how to better handle this without major changes
> in V4L2 API.

Well, given that the format is highly specific to your hardware, I don't think 
it's such a bad solution. It actually looks OK to me.

> The third patch adds support for MIPI-CSI2 Embedded Data capture in
> Samsung S5P/Exynos MIPI-CSIS device. It depends on patch
> "[PATCH RFC] V4L: Add s_rx_buffer subdev video operation".
> 
> The fourth patch extends s5p-fimc driver to allow it to support
> 2-planar V4L2_PIX_FMT_S5C_UYVY_JPG format. More details can be found
> in the patch summary. The [get/set]_frame_desc subdev callback are
> used only to retrive from a sensor subdev required buffer size.
> It depends on patch
> "[PATCH RFC] V4L: Add get/set_frame_desc subdev callbacks"
> 
> The fifth patch adds [get/set]_frame_desc op handlers to the m5mols
> driver as an example. I prepared also similar patch for S5C73M3
> sensor where 2 frame description entries are used, but that driver
> is not yet mainlined due to a few missing items in V4L2 required
> to fully control it, so I didn't include that patch in this series.
> 
> Comments, suggestions welcome.
> 
> Thanks,
> Sylwester
> 
> Sylwester Nawrocki (5):
>   V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
>   V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition
>   s5p-csis: Add support for non-image data packets capture
>   s5p-fimc: Add support for V4L2_PIX_FMT_S5C_UYVY_JPG fourcc
>   m5mols: Implement .get_frame_desc subdev callback
> 
>  Documentation/DocBook/media/v4l/compat.xml         |   4 +
>  Documentation/DocBook/media/v4l/pixfmt.xml         |   9 ++
>  Documentation/DocBook/media/v4l/subdev-formats.xml |  45 ++++++++
>  drivers/media/i2c/m5mols/m5mols.h                  |   9 ++
>  drivers/media/i2c/m5mols/m5mols_capture.c          |   3 +
>  drivers/media/i2c/m5mols/m5mols_core.c             |  47 ++++++++
>  drivers/media/i2c/m5mols/m5mols_reg.h              |   1 +
>  drivers/media/platform/s5p-fimc/fimc-capture.c     | 128 +++++++++++++++---
>  drivers/media/platform/s5p-fimc/fimc-core.c        |  19 ++-
>  drivers/media/platform/s5p-fimc/fimc-core.h        |  28 ++++-
>  drivers/media/platform/s5p-fimc/fimc-reg.c         |  23 +++-
>  drivers/media/platform/s5p-fimc/fimc-reg.h         |   3 +-
>  drivers/media/platform/s5p-fimc/mipi-csis.c        |  59 +++++++++-
>  include/linux/v4l2-mediabus.h                      |   5 +
>  include/linux/videodev2.h                          |   1 +
>  15 files changed, 351 insertions(+), 33 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

[Laurent Pinchart]

Hi Sylwester,

On Monday 24 September 2012 16:55:42 Sylwester Nawrocki wrote:
> This patch adds media bus pixel code for the interleaved JPEG/UYVY
> image format used by S5C73MX Samsung cameras. This interleaved image
> data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data.
> 
> It also defines an experimental vendor and device specific media bus
> formats section and adds related DocBook documentation.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  Documentation/DocBook/media/v4l/compat.xml         |  4 ++
>  Documentation/DocBook/media/v4l/subdev-formats.xml | 45 +++++++++++++++++++
>  include/linux/v4l2-mediabus.h                      |  5 +++
>  3 files changed, 54 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/compat.xml
> b/Documentation/DocBook/media/v4l/compat.xml index 98e8d08..5d2480b 100644
> --- a/Documentation/DocBook/media/v4l/compat.xml
> +++ b/Documentation/DocBook/media/v4l/compat.xml
> @@ -2605,6 +2605,10 @@ ioctls.</para>
>          <listitem>
>  	  <para>Support for frequency band enumeration: &VIDIOC-ENUM-FREQ-BANDS;
> ioctl.</para> </listitem>
> +        <listitem>
> +	  <para>Vendor and device specific media bus pixel formats.
> +	    <xref linkend="v4l2-mbus-vendor-spec-fmts" />.</para>
> +        </listitem>
>        </itemizedlist>
>      </section>
> 
> diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml
> b/Documentation/DocBook/media/v4l/subdev-formats.xml index 49c532e..d7aa870
> 100644
> --- a/Documentation/DocBook/media/v4l/subdev-formats.xml
> +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
> @@ -2565,5 +2565,50 @@
>  	</tgroup>
>        </table>
>      </section>
> +
> +    <section id="v4l2-mbus-vendor-spec-fmts">
> +      <title>Vendor and Device Specific Formats</title>
> +
> +      <note>
> +	<title> Experimental </title>

I don't think you need spaces across the title.

> +	<para>This is an <link linkend="experimental">experimental</link>
> +interface and may change in the future.</para>
> +      </note>
> +
> +      <para> This section lists complex data formats that are either vendor
> or
> +	device specific. These formats comprise raw and compressed image data
> +	and optional meta-data within a single frame.

That's currently true, but we could have other strange vendor-specific formats 
that don't interleave raw and compressed frames.

> +      </para>
> +
> +      <para>The following table lists the existing vendor and device
> specific
> +	formats.</para>
> +
> +      <table pgwide="0" frame="none"
> id="v4l2-mbus-pixelcode-vendor-specific"> +	<title>Vendor and device
> specific formats</title>
> +	<tgroup cols="3">
> +	  <colspec colname="id" align="left" />
> +	  <colspec colname="code" align="left"/>
> +	  <colspec colname="remarks" align="left"/>
> +	  <thead>
> +	    <row>
> +	      <entry>Identifier</entry>
> +	      <entry>Code</entry>
> +	      <entry>Comments</entry>
> +	    </row>
> +	  </thead>
> +	  <tbody valign="top">
> +	    <row id="V4L2-MBUS-FMT-S5C-UYVY-JPG-1X8">
> +	      <entry>V4L2_MBUS_FMT_S5C_UYVY_JPG_1X8</entry>
> +	      <entry>0x8001</entry>
> +	      <entry>
> +		Interleaved raw UYVY and JPEG image format with embedded
> +		meta-data, produced by S3C73M3 camera sensors.
> +	      </entry>
> +	    </row>
> +	  </tbody>
> +	</tgroup>
> +      </table>
> +    </section>
> +
>    </section>
>  </section>
> diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
> index 5ea7f75..b98c566 100644
> --- a/include/linux/v4l2-mediabus.h
> +++ b/include/linux/v4l2-mediabus.h
> @@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode {
> 
>  	/* JPEG compressed formats - next is 0x4002 */
>  	V4L2_MBUS_FMT_JPEG_1X8 = 0x4001,
> +
> +	/* Vendor specific formats - next is 0x8002 */

Anything wrong with 0x5000 as a base value ? :-)

> +
> +	/* S5C73M3 interleaved UYVY and JPEG */
> +	V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 = 0x8001,
>  };
> 
>  /**

-- 
Regards,

Laurent Pinchart

Re: [PATCH RFC 2/5] V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition

[Laurent Pinchart]

Hi Sylwester,

On Monday 24 September 2012 16:55:43 Sylwester Nawrocki wrote:
> The V4L2_PIX_FMT_S5C_UYVY_JPG is a two-plane image format used by
> Samsung S5C73M3 camera. The first plane contains interleaved UYVY
> and JPEG data, followed by meta-data in form of offsets to the UYVU
> data blocks.
> 
> The second plane contains additional meta-data needed for extracting
> JPEG and UYVY data stream from the first plane, in particular an
> offset to the first entry of an array of data offsets in the first
> plane. The offsets are expressed as 4-byte unsigned integers.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  Documentation/DocBook/media/v4l/pixfmt.xml | 9 +++++++++
>  include/linux/videodev2.h                  | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml
> b/Documentation/DocBook/media/v4l/pixfmt.xml index 1ddbfab..9caed9b 100644
> --- a/Documentation/DocBook/media/v4l/pixfmt.xml
> +++ b/Documentation/DocBook/media/v4l/pixfmt.xml
> @@ -996,6 +996,15 @@ the other bits are set to 0.</entry>
>  	    <entry>Old 6-bit greyscale format. Only the most significant 6 bits 
of
> each byte are used, the other bits are set to 0.</entry>
>  	  </row>
> +	  <row id="V4L2-PIX-FMT-JPG-YUYV-S5C">
> +	    <entry><constant>V4L2_PIX_FMT_S5C_YUYV_JPG</constant></entry>
> +	    <entry>'S5CJ'</entry>
> +	    <entry>Two-planar format used by Samsung S5C73MX cameras.The first
> +plane contains interleaved JPEG and YUYV data, followed by meta data
> describing +layout of the YUYV and JPEG data blocks. The second plane
> contains additional +information about data layout in the first plane, like
> an offset to the array +of offsets to YUVY data chunks.</entry>
> +	  </row>

I think you need to be a bit more precise here. You should document the format 
of the meta data, as that's required for applications to use the format.

>  	</tbody>
>        </tgroup>
>      </table>
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 4862165..aa6fb4d 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -435,6 +435,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_KONICA420  v4l2_fourcc('K', 'O', 'N', 'I') /* YUV420
> planar in blocks of 256 pixels */ #define
> V4L2_PIX_FMT_JPGL	v4l2_fourcc('J', 'P', 'G', 'L') /* JPEG-Lite */ #define
> V4L2_PIX_FMT_SE401      v4l2_fourcc('S', '4', '0', '1') /* se401 janggu
> compressed rgb */ +#define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5',
> 'C', 'J') /* S5C73M3 interleaved YUYV/JPEG */
> 
>  /*
>   *	F O R M A T   E N U M E R A T I O N
-- 
Regards,

Laurent Pinchart

Re: [PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

[Sylwester Nawrocki]

Hi Laurent,

Thanks for your review.

On 09/25/2012 01:42 PM, Laurent Pinchart wrote:
> On Monday 24 September 2012 16:55:42 Sylwester Nawrocki wrote:
>> This patch adds media bus pixel code for the interleaved JPEG/UYVY
>> image format used by S5C73MX Samsung cameras. This interleaved image
>> data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data.
>>
>> It also defines an experimental vendor and device specific media bus
>> formats section and adds related DocBook documentation.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  Documentation/DocBook/media/v4l/compat.xml         |  4 ++
>>  Documentation/DocBook/media/v4l/subdev-formats.xml | 45 +++++++++++++++++++
>>  include/linux/v4l2-mediabus.h                      |  5 +++
>>  3 files changed, 54 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/compat.xml
>> b/Documentation/DocBook/media/v4l/compat.xml index 98e8d08..5d2480b 100644
>> --- a/Documentation/DocBook/media/v4l/compat.xml
>> +++ b/Documentation/DocBook/media/v4l/compat.xml
>> @@ -2605,6 +2605,10 @@ ioctls.</para>
>>          <listitem>
>>  	  <para>Support for frequency band enumeration: &VIDIOC-ENUM-FREQ-BANDS;
>> ioctl.</para> </listitem>
>> +        <listitem>
>> +	  <para>Vendor and device specific media bus pixel formats.
>> +	    <xref linkend="v4l2-mbus-vendor-spec-fmts" />.</para>
>> +        </listitem>
>>        </itemizedlist>
>>      </section>
>>
>> diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml
>> b/Documentation/DocBook/media/v4l/subdev-formats.xml index 49c532e..d7aa870
>> 100644
>> --- a/Documentation/DocBook/media/v4l/subdev-formats.xml
>> +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
>> @@ -2565,5 +2565,50 @@
>>  	</tgroup>
>>        </table>
>>      </section>
>> +
>> +    <section id="v4l2-mbus-vendor-spec-fmts">
>> +      <title>Vendor and Device Specific Formats</title>
>> +
>> +      <note>
>> +	<title> Experimental </title>
> 
> I don't think you need spaces across the title.

Thanks for spotting this, I'll fix it and any other occurrences there.

>> +	<para>This is an <link linkend="experimental">experimental</link>
>> +interface and may change in the future.</para>
>> +      </note>
>> +
>> +      <para> This section lists complex data formats that are either vendor
>> or
>> +	device specific. These formats comprise raw and compressed image data
>> +	and optional meta-data within a single frame.
> 
> That's currently true, but we could have other strange vendor-specific formats 
> that don't interleave raw and compressed frames.

OK, let me remove that sentence then.

>> +      </para>
>> +
>> +      <para>The following table lists the existing vendor and device
>> specific
>> +	formats.</para>
>> +
>> +      <table pgwide="0" frame="none"
>> id="v4l2-mbus-pixelcode-vendor-specific"> +	<title>Vendor and device
>> specific formats</title>
>> +	<tgroup cols="3">
>> +	  <colspec colname="id" align="left" />
>> +	  <colspec colname="code" align="left"/>
>> +	  <colspec colname="remarks" align="left"/>
>> +	  <thead>
>> +	    <row>
>> +	      <entry>Identifier</entry>
>> +	      <entry>Code</entry>
>> +	      <entry>Comments</entry>
>> +	    </row>
>> +	  </thead>
>> +	  <tbody valign="top">
>> +	    <row id="V4L2-MBUS-FMT-S5C-UYVY-JPG-1X8">
>> +	      <entry>V4L2_MBUS_FMT_S5C_UYVY_JPG_1X8</entry>
>> +	      <entry>0x8001</entry>
>> +	      <entry>
>> +		Interleaved raw UYVY and JPEG image format with embedded
>> +		meta-data, produced by S3C73M3 camera sensors.
>> +	      </entry>
>> +	    </row>
>> +	  </tbody>
>> +	</tgroup>
>> +      </table>
>> +    </section>
>> +
>>    </section>
>>  </section>
>> diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
>> index 5ea7f75..b98c566 100644
>> --- a/include/linux/v4l2-mediabus.h
>> +++ b/include/linux/v4l2-mediabus.h
>> @@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode {
>>
>>  	/* JPEG compressed formats - next is 0x4002 */
>>  	V4L2_MBUS_FMT_JPEG_1X8 = 0x4001,
>> +
>> +	/* Vendor specific formats - next is 0x8002 */
> 
> Anything wrong with 0x5000 as a base value ? :-)

I think I was originally using this value but during discussions
the conclusion was to clearly separate this new range. I have no
strong preference, I'm going to revert it to 0x5000 in the next
iteration, unless someone raises objections.

>> +
>> +	/* S5C73M3 interleaved UYVY and JPEG */
>> +	V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 = 0x8001,
>>  };
>>
>>  /**
> 

Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

Re: [PATCH RFC 2/5] V4L: Add V4L2_PIX_FMT_S5C_UYVY_JPG fourcc definition

[Sylwester Nawrocki]

Hi Laurent,

On 09/25/2012 01:44 PM, Laurent Pinchart wrote:
>> diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml
>> b/Documentation/DocBook/media/v4l/pixfmt.xml index 1ddbfab..9caed9b 100644
>> --- a/Documentation/DocBook/media/v4l/pixfmt.xml
>> +++ b/Documentation/DocBook/media/v4l/pixfmt.xml
>> @@ -996,6 +996,15 @@ the other bits are set to 0.</entry>
>>  	    <entry>Old 6-bit greyscale format. Only the most significant 6 bits 
> of
>> each byte are used, the other bits are set to 0.</entry>
>>  	  </row>
>> +	  <row id="V4L2-PIX-FMT-JPG-YUYV-S5C">
>> +	    <entry><constant>V4L2_PIX_FMT_S5C_YUYV_JPG</constant></entry>
>> +	    <entry>'S5CJ'</entry>
>> +	    <entry>Two-planar format used by Samsung S5C73MX cameras.The first
>> +plane contains interleaved JPEG and YUYV data, followed by meta data
>> describing +layout of the YUYV and JPEG data blocks. The second plane
>> contains additional +information about data layout in the first plane, like
>> an offset to the array +of offsets to YUVY data chunks.</entry>
>> +	  </row>
> 
> I think you need to be a bit more precise here. You should document the format 
> of the meta data, as that's required for applications to use the format.

Right, I wasn't sure how detailed this DocBook description should be.
I thought about adding more detailed explanation of this format under
Documentation/video4linux/, but it probably makes more sense to add
a few more sentences here and make it complete and really useful.
Let me correct that in the next iteration.


Regards,
Sylwester

RE: [PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

[Vincent ABRIOU]

Hi Sylwester,

I'm wondering why don't you simply define V4L2_MBUS_FMT_UYVY_JPEG_1X8
without any reference to your camera?
Indeed, many other cameras could support Jpeg interleaved with YUV and it will
avoid to define a new media bus type for every cameras integrated under V4L2
supporting JPEG/YUV interleaving feature.

Thank you for your feedback.

Regards,
--
Vincent Abriou


> -----Original Message-----
> From: linux-media-owner@vger.kernel.org [mailto:linux-media-
> owner@vger.kernel.org] On Behalf Of Sylwester Nawrocki
> Sent: Tuesday, September 25, 2012 4:40 PM
> To: Laurent Pinchart
> Cc: linux-media@vger.kernel.org; a.hajda@samsung.com; sakari.ailus@iki.fi;
> hverkuil@xs4all.nl; kyungmin.park@samsung.com;
> sw0312.kim@samsung.com
> Subject: Re: [PATCH RFC 1/5] V4L: Add
> V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
> 
> Hi Laurent,
> 
> Thanks for your review.
> 
> On 09/25/2012 01:42 PM, Laurent Pinchart wrote:
> > On Monday 24 September 2012 16:55:42 Sylwester Nawrocki wrote:
> >> This patch adds media bus pixel code for the interleaved JPEG/UYVY
> >> image format used by S5C73MX Samsung cameras. This interleaved image
> >> data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data.
> >>
> >> It also defines an experimental vendor and device specific media bus
> >> formats section and adds related DocBook documentation.
> >>
> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> ---
> >>  Documentation/DocBook/media/v4l/compat.xml         |  4 ++
> >>  Documentation/DocBook/media/v4l/subdev-formats.xml | 45
> +++++++++++++++++++
> >>  include/linux/v4l2-mediabus.h                      |  5 +++
> >>  3 files changed, 54 insertions(+)
> >>
> >> diff --git a/Documentation/DocBook/media/v4l/compat.xml
> >> b/Documentation/DocBook/media/v4l/compat.xml index
> 98e8d08..5d2480b
> >> 100644
> >> --- a/Documentation/DocBook/media/v4l/compat.xml
> >> +++ b/Documentation/DocBook/media/v4l/compat.xml
> >> @@ -2605,6 +2605,10 @@ ioctls.</para>
> >>          <listitem>
> >>  	  <para>Support for frequency band enumeration:
> >> &VIDIOC-ENUM-FREQ-BANDS; ioctl.</para> </listitem>
> >> +        <listitem>
> >> +	  <para>Vendor and device specific media bus pixel formats.
> >> +	    <xref linkend="v4l2-mbus-vendor-spec-fmts" />.</para>
> >> +        </listitem>
> >>        </itemizedlist>
> >>      </section>
> >>
> >> diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml
> >> b/Documentation/DocBook/media/v4l/subdev-formats.xml index
> >> 49c532e..d7aa870
> >> 100644
> >> --- a/Documentation/DocBook/media/v4l/subdev-formats.xml
> >> +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
> >> @@ -2565,5 +2565,50 @@
> >>  	</tgroup>
> >>        </table>
> >>      </section>
> >> +
> >> +    <section id="v4l2-mbus-vendor-spec-fmts">
> >> +      <title>Vendor and Device Specific Formats</title>
> >> +
> >> +      <note>
> >> +	<title> Experimental </title>
> >
> > I don't think you need spaces across the title.
> 
> Thanks for spotting this, I'll fix it and any other occurrences there.
> 
> >> +	<para>This is an <link linkend="experimental">experimental</link>
> >> +interface and may change in the future.</para>
> >> +      </note>
> >> +
> >> +      <para> This section lists complex data formats that are either
> >> + vendor
> >> or
> >> +	device specific. These formats comprise raw and compressed image
> data
> >> +	and optional meta-data within a single frame.
> >
> > That's currently true, but we could have other strange vendor-specific
> > formats that don't interleave raw and compressed frames.
> 
> OK, let me remove that sentence then.
> 
> >> +      </para>
> >> +
> >> +      <para>The following table lists the existing vendor and device
> >> specific
> >> +	formats.</para>
> >> +
> >> +      <table pgwide="0" frame="none"
> >> id="v4l2-mbus-pixelcode-vendor-specific"> +	<title>Vendor and
> device
> >> specific formats</title>
> >> +	<tgroup cols="3">
> >> +	  <colspec colname="id" align="left" />
> >> +	  <colspec colname="code" align="left"/>
> >> +	  <colspec colname="remarks" align="left"/>
> >> +	  <thead>
> >> +	    <row>
> >> +	      <entry>Identifier</entry>
> >> +	      <entry>Code</entry>
> >> +	      <entry>Comments</entry>
> >> +	    </row>
> >> +	  </thead>
> >> +	  <tbody valign="top">
> >> +	    <row id="V4L2-MBUS-FMT-S5C-UYVY-JPG-1X8">
> >> +	      <entry>V4L2_MBUS_FMT_S5C_UYVY_JPG_1X8</entry>
> >> +	      <entry>0x8001</entry>
> >> +	      <entry>
> >> +		Interleaved raw UYVY and JPEG image format with
> embedded
> >> +		meta-data, produced by S3C73M3 camera sensors.
> >> +	      </entry>
> >> +	    </row>
> >> +	  </tbody>
> >> +	</tgroup>
> >> +      </table>
> >> +    </section>
> >> +
> >>    </section>
> >>  </section>
> >> diff --git a/include/linux/v4l2-mediabus.h
> >> b/include/linux/v4l2-mediabus.h index 5ea7f75..b98c566 100644
> >> --- a/include/linux/v4l2-mediabus.h
> >> +++ b/include/linux/v4l2-mediabus.h
> >> @@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode {
> >>
> >>  	/* JPEG compressed formats - next is 0x4002 */
> >>  	V4L2_MBUS_FMT_JPEG_1X8 = 0x4001,
> >> +
> >> +	/* Vendor specific formats - next is 0x8002 */
> >
> > Anything wrong with 0x5000 as a base value ? :-)
> 
> I think I was originally using this value but during discussions the conclusion
> was to clearly separate this new range. I have no strong preference, I'm
> going to revert it to 0x5000 in the next iteration, unless someone raises
> objections.
> 
> >> +
> >> +	/* S5C73M3 interleaved UYVY and JPEG */
> >> +	V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 = 0x8001,
> >>  };
> >>
> >>  /**
> >
> 
> Regards,
> --
> Sylwester Nawrocki
> Samsung Poland R&D Center
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

[Sylwester Nawrocki]

Hi Vincent,

On 10/09/2012 03:36 PM, Vincent ABRIOU wrote:
> Hi Sylwester,
> 
> I'm wondering why don't you simply define V4L2_MBUS_FMT_UYVY_JPEG_1X8
> without any reference to your camera?

Because it's not a plain UYVY/JPEG data. There is an additional meta-data
that follows interleaved UYVY/JPEG. It's all on a single User Defined 
MIPI CSI-2 DT. In addition to that there is some more meta data transmitted 
on MIPI CSI-2 Embedded Data DT. If there was no meta-data present at the
User Defined DT, then we could think about using generic 
V4L2_MBUS_FMT_UYVY_JPEG_1X8 pixel code and handling the meta-data on 
separate DT with the frame_desc calls.

Anyway this S5C media bus format is an experimental thing and if there are
cameras generating plain JPEG/YUV we need to search for better, more generic
solution.

> Indeed, many other cameras could support Jpeg interleaved with YUV and it will
> avoid to define a new media bus type for every cameras integrated under V4L2
> supporting JPEG/YUV interleaving feature.

Yes, we also need some interface to configure both streams, e.g. image
resolutions independently. Still having separate pixel codes for pairs of
JPEG and something else seems not optimal. Nevertheless I can't think of
any better solution right now, so applications are able to configure
selected format at a camera subdev.

I'm also wondering what are the interleaving methods in case of various
cameras. Even though they interleave same 2 standard formats, the way how
the interleaving happens could differ, couldn't it ? Such information
probably cannot be easily contained in the meta-data, unless there is some
standard (header) format for it. 

> Thank you for your feedback.
> 
> Regards,
> --
> Vincent Abriou

--

Regards,
Sylwester

Re: [PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

[Sakari Ailus]

Hi Sylwester and Vincent,

My apologies for the late reply on this topic. I've been quite busy lately.

Sylwester Nawrocki wrote:
> On 10/09/2012 03:36 PM, Vincent ABRIOU wrote:
>> Hi Sylwester,
>>
>> I'm wondering why don't you simply define V4L2_MBUS_FMT_UYVY_JPEG_1X8
>> without any reference to your camera?
>
> Because it's not a plain UYVY/JPEG data. There is an additional meta-data
> that follows interleaved UYVY/JPEG. It's all on a single User Defined
> MIPI CSI-2 DT. In addition to that there is some more meta data transmitted
> on MIPI CSI-2 Embedded Data DT. If there was no meta-data present at the
> User Defined DT, then we could think about using generic
> V4L2_MBUS_FMT_UYVY_JPEG_1X8 pixel code and handling the meta-data on
> separate DT with the frame_desc calls.
>
> Anyway this S5C media bus format is an experimental thing and if there are
> cameras generating plain JPEG/YUV we need to search for better, more generic
> solution.

Vincent: what's the frame layout that your sensor produces? There are two
cases that could be easy (sort of, everything's relative) that I can see for
the standard interfaces.

1. Different parts of the image are transmitted over different CSI-2
contexts. This way the receiver may separate them to separate memory
regions, and the end result is a single multi-plane buffer.

2. If the distance in octets of the intermittent image strides is constant,
then we could do some tricks with multi-plane buffers. The two planes of the
buffer could be interleaved, with correct base addresses.

I think Sylwester's case fits into neither of the above. A device-specific
format does not resolve configuring the two formats.

Both require adding plane-specific pixel codes, agreement over how frame
descriptors are used for describing frames of multiple independent content
planes, and how the multiple formats are configured on the sensor. Use of
sub-subdevs come to mind for the last one as an alternative --- this issue
stems from the fact we're using the same interface to model the bus and the
image format on that bus. It no longer works out the way it used to when
there are two formats on the same bus.

Each of these probably require a separate RFC and someone to implement the
changes.

Kind regards,

-- 
Sakari Ailus
sakari.ailus@iki.fi

RE: [PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

[Vincent ABRIOU]

Hi Sakari and Sylwester,

Sorry for the late answer.

> -----Original Message-----
> From: Sakari Ailus [mailto:sakari.ailus@iki.fi]
> Sent: Monday, October 15, 2012 8:36 PM
> To: Sylwester Nawrocki
> Cc: Vincent ABRIOU; linux-media@vger.kernel.org; a.hajda@samsung.com;
> Laurent Pinchart; hverkuil@xs4all.nl; kyungmin.park@samsung.com;
> sw0312.kim@samsung.com; Nicolas THERY; Jean-Marc VOLLE; Pierre-yves
> TALOUD; Willy POISSON
> Subject: Re: [PATCH RFC 1/5] V4L: Add
> V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
> 
> Hi Sylwester and Vincent,
> 
> My apologies for the late reply on this topic. I've been quite busy lately.
> 
> Sylwester Nawrocki wrote:
> > On 10/09/2012 03:36 PM, Vincent ABRIOU wrote:
> >> Hi Sylwester,
> >>
> >> I'm wondering why don't you simply define
> V4L2_MBUS_FMT_UYVY_JPEG_1X8
> >> without any reference to your camera?
> >
> > Because it's not a plain UYVY/JPEG data. There is an additional
> > meta-data that follows interleaved UYVY/JPEG. It's all on a single
> > User Defined MIPI CSI-2 DT. In addition to that there is some more
> > meta data transmitted on MIPI CSI-2 Embedded Data DT. If there was no
> > meta-data present at the User Defined DT, then we could think about
> > using generic
> > V4L2_MBUS_FMT_UYVY_JPEG_1X8 pixel code and handling the meta-data
> on
> > separate DT with the frame_desc calls.
> >
> > Anyway this S5C media bus format is an experimental thing and if there
> > are cameras generating plain JPEG/YUV we need to search for better,
> > more generic solution.
> 
> Vincent: what's the frame layout that your sensor produces? There are two
> cases that could be easy (sort of, everything's relative) that I can see for the
> standard interfaces.

The sensor I used interleaved Jpeg and YUV but I met 2 cases:
1/ sensor providing JPEG / YUV (using the same DT)
2/ sensor providing JPEG with a 1st DT, YUV with a 2nd DT and embedded data using a 3rd DT

> 
> 1. Different parts of the image are transmitted over different CSI-2 contexts.
> This way the receiver may separate them to separate memory regions, and
> the end result is a single multi-plane buffer.
> 
> 2. If the distance in octets of the intermittent image strides is constant, then
> we could do some tricks with multi-plane buffers. The two planes of the
> buffer could be interleaved, with correct base addresses.
> 
> I think Sylwester's case fits into neither of the above. A device-specific
> format does not resolve configuring the two formats.
> 
> Both require adding plane-specific pixel codes, agreement over how frame
> descriptors are used for describing frames of multiple independent content
> planes, and how the multiple formats are configured on the sensor. Use of
> sub-subdevs come to mind for the last one as an alternative --- this issue
> stems from the fact we're using the same interface to model the bus and the
> image format on that bus. It no longer works out the way it used to when
> there are two formats on the same bus.
> 

My point of view is that the only user's concern is how to retrieve the data, on which pad and how are they sorted?

The MBUS format to be use by the camera (and set by the user) should only define the overall structure of the CSI stream and could be generic (like V4L2_MBUS_FMT_UYVY_JPEG_1X8  or V4L2_MBUS_FMT_YUYV_JPEG_1X8 without any sensor model mentioned).
In the MBUS format naming, we don't care about describing the eventual metadata and other stream specificities link to the camera because the CSI stream details could be described in the v4l2_subdev_frame_format as initially proposed by Sakari (More flag descriptor could be added to describe the data to be transmitted).
It is then up to the CSI2 receiver to recover the CSI stream layout and to configure output pads and multi-plane buffer according to its capabilities. Multi-plan buffer attached to the pad need to have a plane-specific pixel code in order to warn user about the buffer content.
Then user could query the output pads of the CSI 2 receiver to discover on which pad the different pixel formats are outputted and how:

> Each of these probably require a separate RFC and someone to implement
> the changes.
> 
> Kind regards,
> 
> --
> Sakari Ailus
> sakari.ailus@iki.fi

Best Regards,

Vincent Abriou