soc-camera: pixelfmt translation serie

soc-camera: pixelfmt translation serie

[Robert Jarzmik]

Hi Guennadi,

This serie is prolongation of your work on the soc_camera
bus driver. I reposted your serie within this one, so that
this serie is a "whole", and can be applied on top of
v2.6.28-rc4 for testing purpose.
 => beware, the patch on the ov7272 was removed, shame on me

The patches are split into :

 - patch 1 .. 7 : copy paste of your patches, no
 modification (beware, ov.... was expunged, sorry again)

 - patch 8 : the translation framework
 - patch 9 : application of the framework to pxa_camera host
 - patch 10 : application of the framework to
 sh_mobile_ceu_camera host

 - patch 11 - 12 : fixes for YUV format handling in pxa_camera

This is the ground of the discussion. I know I still have to
add documentation around the new functions/structures. I
need to know if this is what you have in mind, to either
continue the work or stop and take a different path.

I should add this was tested with RGB565 and all YUV formats
on a pxa272 with a mt9m111 (as you could have expected :)).

Now is the time to improve the translation code. Happy review.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 01/13] soc-camera: merge .try_bus_param() into .try_fmt_cap()

[Robert Jarzmik]

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

.try_bus_param() method from struct soc_camera_host_ops is only called at one
location immediately before .try_fmt_cap(), there is no value in keeping these
two methods separate, merge them.

Signed-off-by: Guennadi Liakhovetski <lg@denx.de>
---
 drivers/media/video/pxa_camera.c           |    6 +++++-
 drivers/media/video/sh_mobile_ceu_camera.c |    6 +++++-
 drivers/media/video/soc_camera.c           |    5 -----
 include/media/soc_camera.h                 |    1 -
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index eb6be58..2a811f8 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -913,6 +913,11 @@ static int pxa_camera_set_fmt_cap(struct soc_camera_device *icd,
 static int pxa_camera_try_fmt_cap(struct soc_camera_device *icd,
 				  struct v4l2_format *f)
 {
+	int ret = pxa_camera_try_bus_param(icd, f->fmt.pix.pixelformat);
+
+	if (ret < 0)
+		return ret;
+
 	/* limit to pxa hardware capabilities */
 	if (f->fmt.pix.height < 32)
 		f->fmt.pix.height = 32;
@@ -1039,7 +1044,6 @@ static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
 	.reqbufs	= pxa_camera_reqbufs,
 	.poll		= pxa_camera_poll,
 	.querycap	= pxa_camera_querycap,
-	.try_bus_param	= pxa_camera_try_bus_param,
 	.set_bus_param	= pxa_camera_set_bus_param,
 };
 
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 2407607..a6b29a4 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -453,6 +453,11 @@ static int sh_mobile_ceu_set_fmt_cap(struct soc_camera_device *icd,
 static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,
 				     struct v4l2_format *f)
 {
+	int ret = sh_mobile_ceu_try_bus_param(icd, f->fmt.pix.pixelformat);
+
+	if (ret < 0)
+		return ret;
+
 	/* FIXME: calculate using depth and bus width */
 
 	if (f->fmt.pix.height < 4)
@@ -540,7 +545,6 @@ static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
 	.reqbufs	= sh_mobile_ceu_reqbufs,
 	.poll		= sh_mobile_ceu_poll,
 	.querycap	= sh_mobile_ceu_querycap,
-	.try_bus_param	= sh_mobile_ceu_try_bus_param,
 	.set_bus_param	= sh_mobile_ceu_set_bus_param,
 	.init_videobuf	= sh_mobile_ceu_init_videobuf,
 };
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 66ebe59..f406042 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -77,11 +77,6 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 		return -EINVAL;
 	}
 
-	/* test physical bus parameters */
-	ret = ici->ops->try_bus_param(icd, f->fmt.pix.pixelformat);
-	if (ret)
-		return ret;
-
 	/* limit format to hardware capabilities */
 	ret = ici->ops->try_fmt_cap(icd, f);
 
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index c5de7bb..5eb9540 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -73,7 +73,6 @@ struct soc_camera_host_ops {
 			      struct soc_camera_device *);
 	int (*reqbufs)(struct soc_camera_file *, struct v4l2_requestbuffers *);
 	int (*querycap)(struct soc_camera_host *, struct v4l2_capability *);
-	int (*try_bus_param)(struct soc_camera_device *, __u32);
 	int (*set_bus_param)(struct soc_camera_device *, __u32);
 	unsigned int (*poll)(struct file *, poll_table *);
 };
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 02/13] soc-camera: formatting fixes

[Robert Jarzmik]

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Minor formatting fixes

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/soc_camera.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index f406042..2d1f474 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -35,8 +35,8 @@ static LIST_HEAD(devices);
 static DEFINE_MUTEX(list_lock);
 static DEFINE_MUTEX(video_lock);
 
-const static struct soc_camera_data_format*
-format_by_fourcc(struct soc_camera_device *icd, unsigned int fourcc)
+const static struct soc_camera_data_format *format_by_fourcc(
+	struct soc_camera_device *icd, unsigned int fourcc)
 {
 	unsigned int i;
 
@@ -47,7 +47,7 @@ format_by_fourcc(struct soc_camera_device *icd, unsigned int fourcc)
 }
 
 static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
-				  struct v4l2_format *f)
+				      struct v4l2_format *f)
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
@@ -260,7 +260,7 @@ static int soc_camera_close(struct inode *inode, struct file *file)
 }
 
 static ssize_t soc_camera_read(struct file *file, char __user *buf,
-			   size_t count, loff_t *ppos)
+			       size_t count, loff_t *ppos)
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
@@ -305,7 +305,6 @@ static unsigned int soc_camera_poll(struct file *file, poll_table *pt)
 	return ici->ops->poll(file, pt);
 }
 
-
 static struct file_operations soc_camera_fops = {
 	.owner		= THIS_MODULE,
 	.open		= soc_camera_open,
@@ -317,9 +316,8 @@ static struct file_operations soc_camera_fops = {
 	.llseek		= no_llseek,
 };
 
-
 static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
-				struct v4l2_format *f)
+				    struct v4l2_format *f)
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
@@ -366,7 +364,7 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 }
 
 static int soc_camera_enum_fmt_vid_cap(struct file *file, void  *priv,
-				   struct v4l2_fmtdesc *f)
+				       struct v4l2_fmtdesc *f)
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
@@ -385,7 +383,7 @@ static int soc_camera_enum_fmt_vid_cap(struct file *file, void  *priv,
 }
 
 static int soc_camera_g_fmt_vid_cap(struct file *file, void *priv,
-				struct v4l2_format *f)
+				    struct v4l2_format *f)
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 03/13] soc-camera: let camera host drivers decide upon pixel format

[Robert Jarzmik]

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Pixel format requested by the user is not necessarily the same, as what
a sensor driver provides. There are situations, when a camera host driver
provides the required format, but requires a different format from the
sensor. Further, the list of formats, supported by sensors is pretty static
and can be pretty good described with a constant list of structures. Whereas
decisions, made by camera host drivers to support requested formats can be
quite complex, therefore it is better to let the host driver do the work.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/pxa_camera.c           |   32 +++++++++++++++-
 drivers/media/video/sh_mobile_ceu_camera.c |   32 +++++++++++++++-
 drivers/media/video/soc_camera.c           |   58 ++++++++++-----------------
 include/media/soc_camera.h                 |    3 +
 4 files changed, 87 insertions(+), 38 deletions(-)

diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 2a811f8..a375872 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -907,17 +907,43 @@ static int pxa_camera_try_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
 static int pxa_camera_set_fmt_cap(struct soc_camera_device *icd,
 				  __u32 pixfmt, struct v4l2_rect *rect)
 {
-	return icd->ops->set_fmt_cap(icd, pixfmt, rect);
+	const struct soc_camera_data_format *cam_fmt;
+	int ret;
+
+	/*
+	 * TODO: find a suitable supported by the SoC output format, check
+	 * whether the sensor supports one of acceptable input formats.
+	 */
+	if (pixfmt) {
+		cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
+		if (!cam_fmt)
+			return -EINVAL;
+	}
+
+	ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
+	if (pixfmt && !ret)
+		icd->current_fmt = cam_fmt;
+
+	return ret;
 }
 
 static int pxa_camera_try_fmt_cap(struct soc_camera_device *icd,
 				  struct v4l2_format *f)
 {
+	const struct soc_camera_data_format *cam_fmt;
 	int ret = pxa_camera_try_bus_param(icd, f->fmt.pix.pixelformat);
 
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * TODO: find a suitable supported by the SoC output format, check
+	 * whether the sensor supports one of acceptable input formats.
+	 */
+	cam_fmt = soc_camera_format_by_fourcc(icd, f->fmt.pix.pixelformat);
+	if (!cam_fmt)
+		return -EINVAL;
+
 	/* limit to pxa hardware capabilities */
 	if (f->fmt.pix.height < 32)
 		f->fmt.pix.height = 32;
@@ -929,6 +955,10 @@ static int pxa_camera_try_fmt_cap(struct soc_camera_device *icd,
 		f->fmt.pix.width = 2048;
 	f->fmt.pix.width &= ~0x01;
 
+	f->fmt.pix.bytesperline = f->fmt.pix.width *
+		DIV_ROUND_UP(cam_fmt->depth, 8);
+	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
+
 	/* limit to sensor capabilities */
 	return icd->ops->try_fmt_cap(icd, f);
 }
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index a6b29a4..1bacfc7 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -447,17 +447,43 @@ static int sh_mobile_ceu_try_bus_param(struct soc_camera_device *icd,
 static int sh_mobile_ceu_set_fmt_cap(struct soc_camera_device *icd,
 				     __u32 pixfmt, struct v4l2_rect *rect)
 {
-	return icd->ops->set_fmt_cap(icd, pixfmt, rect);
+	const struct soc_camera_data_format *cam_fmt;
+	int ret;
+
+	/*
+	 * TODO: find a suitable supported by the SoC output format, check
+	 * whether the sensor supports one of acceptable input formats.
+	 */
+	if (pixfmt) {
+		cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
+		if (!cam_fmt)
+			return -EINVAL;
+	}
+
+	ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
+	if (pixfmt && !ret)
+		icd->current_fmt = cam_fmt;
+
+	return ret;
 }
 
 static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,
 				     struct v4l2_format *f)
 {
+	const struct soc_camera_data_format *cam_fmt;
 	int ret = sh_mobile_ceu_try_bus_param(icd, f->fmt.pix.pixelformat);
 
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * TODO: find a suitable supported by the SoC output format, check
+	 * whether the sensor supports one of acceptable input formats.
+	 */
+	cam_fmt = soc_camera_format_by_fourcc(icd, f->fmt.pix.pixelformat);
+	if (!cam_fmt)
+		return -EINVAL;
+
 	/* FIXME: calculate using depth and bus width */
 
 	if (f->fmt.pix.height < 4)
@@ -471,6 +497,10 @@ static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,
 	f->fmt.pix.width &= ~0x01;
 	f->fmt.pix.height &= ~0x03;
 
+	f->fmt.pix.bytesperline = f->fmt.pix.width *
+		DIV_ROUND_UP(cam_fmt->depth, 8);
+	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
+
 	/* limit to sensor capabilities */
 	return icd->ops->try_fmt_cap(icd, f);
 }
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 2d1f474..afadb33 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -35,7 +35,7 @@ static LIST_HEAD(devices);
 static DEFINE_MUTEX(list_lock);
 static DEFINE_MUTEX(video_lock);
 
-const static struct soc_camera_data_format *format_by_fourcc(
+const struct soc_camera_data_format *soc_camera_format_by_fourcc(
 	struct soc_camera_device *icd, unsigned int fourcc)
 {
 	unsigned int i;
@@ -45,6 +45,7 @@ const static struct soc_camera_data_format *format_by_fourcc(
 			return icd->formats + i;
 	return NULL;
 }
+EXPORT_SYMBOL(soc_camera_format_by_fourcc);
 
 static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 				      struct v4l2_format *f)
@@ -54,25 +55,19 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 	struct soc_camera_host *ici =
 		to_soc_camera_host(icd->dev.parent);
 	enum v4l2_field field;
-	const struct soc_camera_data_format *fmt;
 	int ret;
 
 	WARN_ON(priv != file->private_data);
 
-	fmt = format_by_fourcc(icd, f->fmt.pix.pixelformat);
-	if (!fmt) {
-		dev_dbg(&icd->dev, "invalid format 0x%08x\n",
-			f->fmt.pix.pixelformat);
-		return -EINVAL;
-	}
-
-	dev_dbg(&icd->dev, "fmt: 0x%08x\n", fmt->fourcc);
-
+	/*
+	 * TODO: this might also have to migrate to host-drivers, if anyone
+	 * wishes to support other fields
+	 */
 	field = f->fmt.pix.field;
 
 	if (field == V4L2_FIELD_ANY) {
-		field = V4L2_FIELD_NONE;
-	} else if (V4L2_FIELD_NONE != field) {
+		f->fmt.pix.field = V4L2_FIELD_NONE;
+	} else if (field != V4L2_FIELD_NONE) {
 		dev_err(&icd->dev, "Field type invalid.\n");
 		return -EINVAL;
 	}
@@ -80,13 +75,6 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 	/* limit format to hardware capabilities */
 	ret = ici->ops->try_fmt_cap(icd, f);
 
-	/* calculate missing fields */
-	f->fmt.pix.field = field;
-	f->fmt.pix.bytesperline =
-		(f->fmt.pix.width * fmt->depth) >> 3;
-	f->fmt.pix.sizeimage =
-		f->fmt.pix.height * f->fmt.pix.bytesperline;
-
 	return ret;
 }
 
@@ -325,18 +313,10 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 		to_soc_camera_host(icd->dev.parent);
 	int ret;
 	struct v4l2_rect rect;
-	const static struct soc_camera_data_format *data_fmt;
 
 	WARN_ON(priv != file->private_data);
 
-	data_fmt = format_by_fourcc(icd, f->fmt.pix.pixelformat);
-	if (!data_fmt)
-		return -EINVAL;
-
-	/* buswidth may be further adjusted by the ici */
-	icd->buswidth = data_fmt->depth;
-
-	ret = soc_camera_try_fmt_vid_cap(file, icf, f);
+	ret = soc_camera_try_fmt_vid_cap(file, priv, f);
 	if (ret < 0)
 		return ret;
 
@@ -345,14 +325,21 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 	rect.width	= f->fmt.pix.width;
 	rect.height	= f->fmt.pix.height;
 	ret = ici->ops->set_fmt_cap(icd, f->fmt.pix.pixelformat, &rect);
-	if (ret < 0)
+	if (ret < 0) {
 		return ret;
+	} else if (!icd->current_fmt ||
+		   icd->current_fmt->fourcc != f->fmt.pix.pixelformat) {
+		dev_err(&ici->dev, "Host driver hasn't set up current "
+			"format correctly!\n");
+		return -EINVAL;
+	}
 
-	icd->current_fmt	= data_fmt;
+	/* buswidth may be further adjusted by the ici */
+	icd->buswidth		= icd->current_fmt->depth;
 	icd->width		= rect.width;
 	icd->height		= rect.height;
 	icf->vb_vidq.field	= f->fmt.pix.field;
-	if (V4L2_BUF_TYPE_VIDEO_CAPTURE != f->type)
+	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		dev_warn(&icd->dev, "Attention! Wrong buf-type %d\n",
 			 f->type);
 
@@ -394,10 +381,9 @@ static int soc_camera_g_fmt_vid_cap(struct file *file, void *priv,
 	f->fmt.pix.height	= icd->height;
 	f->fmt.pix.field	= icf->vb_vidq.field;
 	f->fmt.pix.pixelformat	= icd->current_fmt->fourcc;
-	f->fmt.pix.bytesperline	=
-		(f->fmt.pix.width * icd->current_fmt->depth) >> 3;
-	f->fmt.pix.sizeimage	=
-		f->fmt.pix.height * f->fmt.pix.bytesperline;
+	f->fmt.pix.bytesperline	= f->fmt.pix.width *
+		DIV_ROUND_UP(icd->current_fmt->depth, 8);
+	f->fmt.pix.sizeimage	= f->fmt.pix.height * f->fmt.pix.bytesperline;
 	dev_dbg(&icd->dev, "current_fmt->fourcc: 0x%08x\n",
 		icd->current_fmt->fourcc);
 	return 0;
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 5eb9540..c9b2b7f 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -105,6 +105,9 @@ extern void soc_camera_device_unregister(struct soc_camera_device *icd);
 extern int soc_camera_video_start(struct soc_camera_device *icd);
 extern void soc_camera_video_stop(struct soc_camera_device *icd);
 
+extern const struct soc_camera_data_format *soc_camera_format_by_fourcc(
+	struct soc_camera_device *icd, unsigned int fourcc);
+
 struct soc_camera_data_format {
 	char *name;
 	unsigned int depth;
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 04/13] soc-camera: simplify naming

[Robert Jarzmik]

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

We anyway don't follow the s_fmt_vid_cap / g_fmt_vid_cap / try_fmt_vid_cap
naming, and soc-camera is so far only about video capture, let's simplify
operation names a bit further. set_fmt_cap / try_fmt_cap wasn't a very good
choice too.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/mt9m001.c              |   14 +++++++-------
 drivers/media/video/mt9m111.c              |   12 ++++++------
 drivers/media/video/mt9v022.c              |   14 +++++++-------
 drivers/media/video/pxa_camera.c           |   16 ++++++++--------
 drivers/media/video/sh_mobile_ceu_camera.c |   16 ++++++++--------
 drivers/media/video/soc_camera.c           |    6 +++---
 drivers/media/video/soc_camera_platform.c  |   12 ++++++------
 include/media/soc_camera.h                 |   10 ++++------
 8 files changed, 49 insertions(+), 51 deletions(-)

diff --git a/drivers/media/video/mt9m001.c b/drivers/media/video/mt9m001.c
index 0c52437..0bcfef7 100644
--- a/drivers/media/video/mt9m001.c
+++ b/drivers/media/video/mt9m001.c
@@ -285,8 +285,8 @@ static unsigned long mt9m001_query_bus_param(struct soc_camera_device *icd)
 		width_flag;
 }
 
-static int mt9m001_set_fmt_cap(struct soc_camera_device *icd,
-		__u32 pixfmt, struct v4l2_rect *rect)
+static int mt9m001_set_fmt(struct soc_camera_device *icd,
+			   __u32 pixfmt, struct v4l2_rect *rect)
 {
 	struct mt9m001 *mt9m001 = container_of(icd, struct mt9m001, icd);
 	int ret;
@@ -298,7 +298,7 @@ static int mt9m001_set_fmt_cap(struct soc_camera_device *icd,
 		ret = reg_write(icd, MT9M001_VERTICAL_BLANKING, vblank);
 
 	/* The caller provides a supported format, as verified per
-	 * call to icd->try_fmt_cap() */
+	 * call to icd->try_fmt() */
 	if (!ret)
 		ret = reg_write(icd, MT9M001_COLUMN_START, rect->left);
 	if (!ret)
@@ -325,8 +325,8 @@ static int mt9m001_set_fmt_cap(struct soc_camera_device *icd,
 	return ret;
 }
 
-static int mt9m001_try_fmt_cap(struct soc_camera_device *icd,
-			       struct v4l2_format *f)
+static int mt9m001_try_fmt(struct soc_camera_device *icd,
+			   struct v4l2_format *f)
 {
 	if (f->fmt.pix.height < 32 + icd->y_skip_top)
 		f->fmt.pix.height = 32 + icd->y_skip_top;
@@ -447,8 +447,8 @@ static struct soc_camera_ops mt9m001_ops = {
 	.release		= mt9m001_release,
 	.start_capture		= mt9m001_start_capture,
 	.stop_capture		= mt9m001_stop_capture,
-	.set_fmt_cap		= mt9m001_set_fmt_cap,
-	.try_fmt_cap		= mt9m001_try_fmt_cap,
+	.set_fmt		= mt9m001_set_fmt,
+	.try_fmt		= mt9m001_try_fmt,
 	.set_bus_param		= mt9m001_set_bus_param,
 	.query_bus_param	= mt9m001_query_bus_param,
 	.controls		= mt9m001_controls,
diff --git a/drivers/media/video/mt9m111.c b/drivers/media/video/mt9m111.c
index 208ec6c..8062be5 100644
--- a/drivers/media/video/mt9m111.c
+++ b/drivers/media/video/mt9m111.c
@@ -475,8 +475,8 @@ static int mt9m111_set_pixfmt(struct soc_camera_device *icd, u32 pixfmt)
 	return ret;
 }
 
-static int mt9m111_set_fmt_cap(struct soc_camera_device *icd,
-			       __u32 pixfmt, struct v4l2_rect *rect)
+static int mt9m111_set_fmt(struct soc_camera_device *icd,
+			   __u32 pixfmt, struct v4l2_rect *rect)
 {
 	struct mt9m111 *mt9m111 = container_of(icd, struct mt9m111, icd);
 	int ret;
@@ -496,8 +496,8 @@ static int mt9m111_set_fmt_cap(struct soc_camera_device *icd,
 	return ret;
 }
 
-static int mt9m111_try_fmt_cap(struct soc_camera_device *icd,
-			       struct v4l2_format *f)
+static int mt9m111_try_fmt(struct soc_camera_device *icd,
+			   struct v4l2_format *f)
 {
 	if (f->fmt.pix.height > MT9M111_MAX_HEIGHT)
 		f->fmt.pix.height = MT9M111_MAX_HEIGHT;
@@ -620,8 +620,8 @@ static struct soc_camera_ops mt9m111_ops = {
 	.release		= mt9m111_release,
 	.start_capture		= mt9m111_start_capture,
 	.stop_capture		= mt9m111_stop_capture,
-	.set_fmt_cap		= mt9m111_set_fmt_cap,
-	.try_fmt_cap		= mt9m111_try_fmt_cap,
+	.set_fmt		= mt9m111_set_fmt,
+	.try_fmt		= mt9m111_try_fmt,
 	.query_bus_param	= mt9m111_query_bus_param,
 	.set_bus_param		= mt9m111_set_bus_param,
 	.controls		= mt9m111_controls,
diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
index 2584201..3a39f02 100644
--- a/drivers/media/video/mt9v022.c
+++ b/drivers/media/video/mt9v022.c
@@ -337,14 +337,14 @@ static unsigned long mt9v022_query_bus_param(struct soc_camera_device *icd)
 		width_flag;
 }
 
-static int mt9v022_set_fmt_cap(struct soc_camera_device *icd,
-		__u32 pixfmt, struct v4l2_rect *rect)
+static int mt9v022_set_fmt(struct soc_camera_device *icd,
+			   __u32 pixfmt, struct v4l2_rect *rect)
 {
 	struct mt9v022 *mt9v022 = container_of(icd, struct mt9v022, icd);
 	int ret;
 
 	/* The caller provides a supported format, as verified per call to
-	 * icd->try_fmt_cap(), datawidth is from our supported format list */
+	 * icd->try_fmt(), datawidth is from our supported format list */
 	switch (pixfmt) {
 	case V4L2_PIX_FMT_GREY:
 	case V4L2_PIX_FMT_Y16:
@@ -400,8 +400,8 @@ static int mt9v022_set_fmt_cap(struct soc_camera_device *icd,
 	return 0;
 }
 
-static int mt9v022_try_fmt_cap(struct soc_camera_device *icd,
-			       struct v4l2_format *f)
+static int mt9v022_try_fmt(struct soc_camera_device *icd,
+			   struct v4l2_format *f)
 {
 	if (f->fmt.pix.height < 32 + icd->y_skip_top)
 		f->fmt.pix.height = 32 + icd->y_skip_top;
@@ -538,8 +538,8 @@ static struct soc_camera_ops mt9v022_ops = {
 	.release		= mt9v022_release,
 	.start_capture		= mt9v022_start_capture,
 	.stop_capture		= mt9v022_stop_capture,
-	.set_fmt_cap		= mt9v022_set_fmt_cap,
-	.try_fmt_cap		= mt9v022_try_fmt_cap,
+	.set_fmt		= mt9v022_set_fmt,
+	.try_fmt		= mt9v022_try_fmt,
 	.set_bus_param		= mt9v022_set_bus_param,
 	.query_bus_param	= mt9v022_query_bus_param,
 	.controls		= mt9v022_controls,
diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index a375872..665eef2 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -904,8 +904,8 @@ static int pxa_camera_try_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
 	return soc_camera_bus_param_compatible(camera_flags, bus_flags) ? 0 : -EINVAL;
 }
 
-static int pxa_camera_set_fmt_cap(struct soc_camera_device *icd,
-				  __u32 pixfmt, struct v4l2_rect *rect)
+static int pxa_camera_set_fmt(struct soc_camera_device *icd,
+			      __u32 pixfmt, struct v4l2_rect *rect)
 {
 	const struct soc_camera_data_format *cam_fmt;
 	int ret;
@@ -920,15 +920,15 @@ static int pxa_camera_set_fmt_cap(struct soc_camera_device *icd,
 			return -EINVAL;
 	}
 
-	ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
+	ret = icd->ops->set_fmt(icd, pixfmt, rect);
 	if (pixfmt && !ret)
 		icd->current_fmt = cam_fmt;
 
 	return ret;
 }
 
-static int pxa_camera_try_fmt_cap(struct soc_camera_device *icd,
-				  struct v4l2_format *f)
+static int pxa_camera_try_fmt(struct soc_camera_device *icd,
+			      struct v4l2_format *f)
 {
 	const struct soc_camera_data_format *cam_fmt;
 	int ret = pxa_camera_try_bus_param(icd, f->fmt.pix.pixelformat);
@@ -960,7 +960,7 @@ static int pxa_camera_try_fmt_cap(struct soc_camera_device *icd,
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
 
 	/* limit to sensor capabilities */
-	return icd->ops->try_fmt_cap(icd, f);
+	return icd->ops->try_fmt(icd, f);
 }
 
 static int pxa_camera_reqbufs(struct soc_camera_file *icf,
@@ -1068,8 +1068,8 @@ static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
 	.remove		= pxa_camera_remove_device,
 	.suspend	= pxa_camera_suspend,
 	.resume		= pxa_camera_resume,
-	.set_fmt_cap	= pxa_camera_set_fmt_cap,
-	.try_fmt_cap	= pxa_camera_try_fmt_cap,
+	.set_fmt	= pxa_camera_set_fmt,
+	.try_fmt	= pxa_camera_try_fmt,
 	.init_videobuf	= pxa_camera_init_videobuf,
 	.reqbufs	= pxa_camera_reqbufs,
 	.poll		= pxa_camera_poll,
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 1bacfc7..367c4eb 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -444,8 +444,8 @@ static int sh_mobile_ceu_try_bus_param(struct soc_camera_device *icd,
 	return 0;
 }
 
-static int sh_mobile_ceu_set_fmt_cap(struct soc_camera_device *icd,
-				     __u32 pixfmt, struct v4l2_rect *rect)
+static int sh_mobile_ceu_set_fmt(struct soc_camera_device *icd,
+				 __u32 pixfmt, struct v4l2_rect *rect)
 {
 	const struct soc_camera_data_format *cam_fmt;
 	int ret;
@@ -460,15 +460,15 @@ static int sh_mobile_ceu_set_fmt_cap(struct soc_camera_device *icd,
 			return -EINVAL;
 	}
 
-	ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
+	ret = icd->ops->set_fmt(icd, pixfmt, rect);
 	if (pixfmt && !ret)
 		icd->current_fmt = cam_fmt;
 
 	return ret;
 }
 
-static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,
-				     struct v4l2_format *f)
+static int sh_mobile_ceu_try_fmt(struct soc_camera_device *icd,
+				 struct v4l2_format *f)
 {
 	const struct soc_camera_data_format *cam_fmt;
 	int ret = sh_mobile_ceu_try_bus_param(icd, f->fmt.pix.pixelformat);
@@ -502,7 +502,7 @@ static int sh_mobile_ceu_try_fmt_cap(struct soc_camera_device *icd,
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
 
 	/* limit to sensor capabilities */
-	return icd->ops->try_fmt_cap(icd, f);
+	return icd->ops->try_fmt(icd, f);
 }
 
 static int sh_mobile_ceu_reqbufs(struct soc_camera_file *icf,
@@ -570,8 +570,8 @@ static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
 	.owner		= THIS_MODULE,
 	.add		= sh_mobile_ceu_add_device,
 	.remove		= sh_mobile_ceu_remove_device,
-	.set_fmt_cap	= sh_mobile_ceu_set_fmt_cap,
-	.try_fmt_cap	= sh_mobile_ceu_try_fmt_cap,
+	.set_fmt	= sh_mobile_ceu_set_fmt,
+	.try_fmt	= sh_mobile_ceu_try_fmt,
 	.reqbufs	= sh_mobile_ceu_reqbufs,
 	.poll		= sh_mobile_ceu_poll,
 	.querycap	= sh_mobile_ceu_querycap,
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index afadb33..7304e73 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -73,7 +73,7 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 	}
 
 	/* limit format to hardware capabilities */
-	ret = ici->ops->try_fmt_cap(icd, f);
+	ret = ici->ops->try_fmt(icd, f);
 
 	return ret;
 }
@@ -324,7 +324,7 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 	rect.top	= icd->y_current;
 	rect.width	= f->fmt.pix.width;
 	rect.height	= f->fmt.pix.height;
-	ret = ici->ops->set_fmt_cap(icd, f->fmt.pix.pixelformat, &rect);
+	ret = ici->ops->set_fmt(icd, f->fmt.pix.pixelformat, &rect);
 	if (ret < 0) {
 		return ret;
 	} else if (!icd->current_fmt ||
@@ -553,7 +553,7 @@ static int soc_camera_s_crop(struct file *file, void *fh,
 	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	ret = ici->ops->set_fmt_cap(icd, 0, &a->c);
+	ret = ici->ops->set_fmt(icd, 0, &a->c);
 	if (!ret) {
 		icd->width	= a->c.width;
 		icd->height	= a->c.height;
diff --git a/drivers/media/video/soc_camera_platform.c b/drivers/media/video/soc_camera_platform.c
index bb7a9d4..c23871e 100644
--- a/drivers/media/video/soc_camera_platform.c
+++ b/drivers/media/video/soc_camera_platform.c
@@ -79,14 +79,14 @@ soc_camera_platform_query_bus_param(struct soc_camera_device *icd)
 	return p->bus_param;
 }
 
-static int soc_camera_platform_set_fmt_cap(struct soc_camera_device *icd,
-					   __u32 pixfmt, struct v4l2_rect *rect)
+static int soc_camera_platform_set_fmt(struct soc_camera_device *icd,
+				       __u32 pixfmt, struct v4l2_rect *rect)
 {
 	return 0;
 }
 
-static int soc_camera_platform_try_fmt_cap(struct soc_camera_device *icd,
-					   struct v4l2_format *f)
+static int soc_camera_platform_try_fmt(struct soc_camera_device *icd,
+				       struct v4l2_format *f)
 {
 	struct soc_camera_platform_info *p = soc_camera_platform_get_info(icd);
 
@@ -124,8 +124,8 @@ static struct soc_camera_ops soc_camera_platform_ops = {
 	.release		= soc_camera_platform_release,
 	.start_capture		= soc_camera_platform_start_capture,
 	.stop_capture		= soc_camera_platform_stop_capture,
-	.set_fmt_cap		= soc_camera_platform_set_fmt_cap,
-	.try_fmt_cap		= soc_camera_platform_try_fmt_cap,
+	.set_fmt		= soc_camera_platform_set_fmt,
+	.try_fmt		= soc_camera_platform_try_fmt,
 	.set_bus_param		= soc_camera_platform_set_bus_param,
 	.query_bus_param	= soc_camera_platform_query_bus_param,
 };
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index c9b2b7f..64eb4b5 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -66,9 +66,8 @@ struct soc_camera_host_ops {
 	void (*remove)(struct soc_camera_device *);
 	int (*suspend)(struct soc_camera_device *, pm_message_t state);
 	int (*resume)(struct soc_camera_device *);
-	int (*set_fmt_cap)(struct soc_camera_device *, __u32,
-			   struct v4l2_rect *);
-	int (*try_fmt_cap)(struct soc_camera_device *, struct v4l2_format *);
+	int (*set_fmt)(struct soc_camera_device *, __u32, struct v4l2_rect *);
+	int (*try_fmt)(struct soc_camera_device *, struct v4l2_format *);
 	void (*init_videobuf)(struct videobuf_queue *,
 			      struct soc_camera_device *);
 	int (*reqbufs)(struct soc_camera_file *, struct v4l2_requestbuffers *);
@@ -125,9 +124,8 @@ struct soc_camera_ops {
 	int (*release)(struct soc_camera_device *);
 	int (*start_capture)(struct soc_camera_device *);
 	int (*stop_capture)(struct soc_camera_device *);
-	int (*set_fmt_cap)(struct soc_camera_device *, __u32,
-			   struct v4l2_rect *);
-	int (*try_fmt_cap)(struct soc_camera_device *, struct v4l2_format *);
+	int (*set_fmt)(struct soc_camera_device *, __u32, struct v4l2_rect *);
+	int (*try_fmt)(struct soc_camera_device *, struct v4l2_format *);
 	unsigned long (*query_bus_param)(struct soc_camera_device *);
 	int (*set_bus_param)(struct soc_camera_device *, unsigned long);
 	int (*get_chip_id)(struct soc_camera_device *,
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 05/13] soc-camera: move pixel format handling to host drivers (part 2)

[Robert Jarzmik]

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Just letting camera host drivers handle the choice of a pixel format in
.vidioc_s_fmt_vid_cap() is not enough, pixel format enumeration should be
handled by host drivers too.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/pxa_camera.c           |   17 +++++++++++++++++
 drivers/media/video/sh_mobile_ceu_camera.c |   17 +++++++++++++++++
 drivers/media/video/soc_camera.c           |   12 +++---------
 include/media/soc_camera.h                 |    1 +
 4 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 665eef2..f7f621c 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -963,6 +963,22 @@ static int pxa_camera_try_fmt(struct soc_camera_device *icd,
 	return icd->ops->try_fmt(icd, f);
 }
 
+static int pxa_camera_enum_fmt(struct soc_camera_device *icd,
+			       struct v4l2_fmtdesc *f)
+{
+	const struct soc_camera_data_format *format;
+
+	if (f->index >= icd->num_formats)
+		return -EINVAL;
+
+	format = &icd->formats[f->index];
+
+	strlcpy(f->description, format->name, sizeof(f->description));
+	f->pixelformat = format->fourcc;
+
+	return 0;
+}
+
 static int pxa_camera_reqbufs(struct soc_camera_file *icf,
 			      struct v4l2_requestbuffers *p)
 {
@@ -1070,6 +1086,7 @@ static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
 	.resume		= pxa_camera_resume,
 	.set_fmt	= pxa_camera_set_fmt,
 	.try_fmt	= pxa_camera_try_fmt,
+	.enum_fmt	= pxa_camera_enum_fmt,
 	.init_videobuf	= pxa_camera_init_videobuf,
 	.reqbufs	= pxa_camera_reqbufs,
 	.poll		= pxa_camera_poll,
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 367c4eb..fdfe04f 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -505,6 +505,22 @@ static int sh_mobile_ceu_try_fmt(struct soc_camera_device *icd,
 	return icd->ops->try_fmt(icd, f);
 }
 
+static int sh_mobile_ceu_enum_fmt(struct soc_camera_device *icd,
+				  struct v4l2_fmtdesc *f)
+{
+	const struct soc_camera_data_format *format;
+
+	if (f->index >= icd->num_formats)
+		return -EINVAL;
+
+	format = &icd->formats[f->index];
+
+	strlcpy(f->description, format->name, sizeof(f->description));
+	f->pixelformat = format->fourcc;
+
+	return 0;
+}
+
 static int sh_mobile_ceu_reqbufs(struct soc_camera_file *icf,
 				 struct v4l2_requestbuffers *p)
 {
@@ -572,6 +588,7 @@ static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
 	.remove		= sh_mobile_ceu_remove_device,
 	.set_fmt	= sh_mobile_ceu_set_fmt,
 	.try_fmt	= sh_mobile_ceu_try_fmt,
+	.enum_fmt	= sh_mobile_ceu_enum_fmt,
 	.reqbufs	= sh_mobile_ceu_reqbufs,
 	.poll		= sh_mobile_ceu_poll,
 	.querycap	= sh_mobile_ceu_querycap,
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 7304e73..f5a1a86 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -355,18 +355,12 @@ static int soc_camera_enum_fmt_vid_cap(struct file *file, void  *priv,
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	const struct soc_camera_data_format *format;
+	struct soc_camera_host *ici =
+		to_soc_camera_host(icd->dev.parent);
 
 	WARN_ON(priv != file->private_data);
 
-	if (f->index >= icd->num_formats)
-		return -EINVAL;
-
-	format = &icd->formats[f->index];
-
-	strlcpy(f->description, format->name, sizeof(f->description));
-	f->pixelformat = format->fourcc;
-	return 0;
+	return ici->ops->enum_fmt(icd, f);
 }
 
 static int soc_camera_g_fmt_vid_cap(struct file *file, void *priv,
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 64eb4b5..9c3734c 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -68,6 +68,7 @@ struct soc_camera_host_ops {
 	int (*resume)(struct soc_camera_device *);
 	int (*set_fmt)(struct soc_camera_device *, __u32, struct v4l2_rect *);
 	int (*try_fmt)(struct soc_camera_device *, struct v4l2_format *);
+	int (*enum_fmt)(struct soc_camera_device *, struct v4l2_fmtdesc *);
 	void (*init_videobuf)(struct videobuf_queue *,
 			      struct soc_camera_device *);
 	int (*reqbufs)(struct soc_camera_file *, struct v4l2_requestbuffers *);
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 07/13] soc-camera: initialise fields on device-registration, more formatting cleanup

[Robert Jarzmik]

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Camera devices typically allocate their struct soc_camera_device objects per
kzalloc(), but soc_camera.c shall not rely on that and shall initialise
host_priv and use_count explicitly. Many camera and host methods are called
without being checked for NULL, let's check them at registration. Also merge
broken lines, that no longer exceed 80 characters.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/pxa_camera.c |   53 ++++++++++++++++---------------------
 drivers/media/video/soc_camera.c |   54 ++++++++++++++++++++++---------------
 2 files changed, 55 insertions(+), 52 deletions(-)

diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index f7f621c..56aeb07 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -143,8 +143,7 @@ static int pxa_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
 			      unsigned int *size)
 {
 	struct soc_camera_device *icd = vq->priv_data;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 
 	dev_dbg(&icd->dev, "count=%d, size=%d\n", *count, *size);
@@ -170,8 +169,7 @@ static int pxa_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
 static void free_buffer(struct videobuf_queue *vq, struct pxa_buffer *buf)
 {
 	struct soc_camera_device *icd = vq->priv_data;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 	struct videobuf_dmabuf *dma = videobuf_to_dma(&buf->vb);
 	int i;
@@ -247,8 +245,7 @@ static int pxa_videobuf_prepare(struct videobuf_queue *vq,
 		struct videobuf_buffer *vb, enum v4l2_field field)
 {
 	struct soc_camera_device *icd = vq->priv_data;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 	struct pxa_buffer *buf = container_of(vb, struct pxa_buffer, vb);
 	int ret;
@@ -367,8 +364,7 @@ static void pxa_videobuf_queue(struct videobuf_queue *vq,
 			       struct videobuf_buffer *vb)
 {
 	struct soc_camera_device *icd = vq->priv_data;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 	struct pxa_buffer *buf = container_of(vb, struct pxa_buffer, vb);
 	struct pxa_buffer *active;
@@ -772,8 +768,7 @@ static int test_platform_param(struct pxa_camera_dev *pcdev,
 
 static int pxa_camera_set_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
 {
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 	unsigned long dw, bpp, bus_flags, camera_flags, common_flags;
 	u32 cicr0, cicr1, cicr4 = 0;
@@ -890,8 +885,7 @@ static int pxa_camera_set_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
 
 static int pxa_camera_try_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
 {
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 	unsigned long bus_flags, camera_flags;
 	int ret = test_platform_param(pcdev, icd->buswidth, &bus_flags);
@@ -931,7 +925,8 @@ static int pxa_camera_try_fmt(struct soc_camera_device *icd,
 			      struct v4l2_format *f)
 {
 	const struct soc_camera_data_format *cam_fmt;
-	int ret = pxa_camera_try_bus_param(icd, f->fmt.pix.pixelformat);
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	int ret = pxa_camera_try_bus_param(icd, pix->pixelformat);
 
 	if (ret < 0)
 		return ret;
@@ -940,24 +935,24 @@ static int pxa_camera_try_fmt(struct soc_camera_device *icd,
 	 * TODO: find a suitable supported by the SoC output format, check
 	 * whether the sensor supports one of acceptable input formats.
 	 */
-	cam_fmt = soc_camera_format_by_fourcc(icd, f->fmt.pix.pixelformat);
+	cam_fmt = soc_camera_format_by_fourcc(icd, pix->pixelformat);
 	if (!cam_fmt)
 		return -EINVAL;
 
 	/* limit to pxa hardware capabilities */
-	if (f->fmt.pix.height < 32)
-		f->fmt.pix.height = 32;
-	if (f->fmt.pix.height > 2048)
-		f->fmt.pix.height = 2048;
-	if (f->fmt.pix.width < 48)
-		f->fmt.pix.width = 48;
-	if (f->fmt.pix.width > 2048)
-		f->fmt.pix.width = 2048;
-	f->fmt.pix.width &= ~0x01;
-
-	f->fmt.pix.bytesperline = f->fmt.pix.width *
+	if (pix->height < 32)
+		pix->height = 32;
+	if (pix->height > 2048)
+		pix->height = 2048;
+	if (pix->width < 48)
+		pix->width = 48;
+	if (pix->width > 2048)
+		pix->width = 2048;
+	pix->width &= ~0x01;
+
+	pix->bytesperline = pix->width *
 		DIV_ROUND_UP(cam_fmt->depth, 8);
-	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
+	pix->sizeimage = pix->height * pix->bytesperline;
 
 	/* limit to sensor capabilities */
 	return icd->ops->try_fmt(icd, f);
@@ -1028,8 +1023,7 @@ static int pxa_camera_querycap(struct soc_camera_host *ici,
 
 static int pxa_camera_suspend(struct soc_camera_device *icd, pm_message_t state)
 {
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 	int i = 0, ret = 0;
 
@@ -1047,8 +1041,7 @@ static int pxa_camera_suspend(struct soc_camera_device *icd, pm_message_t state)
 
 static int pxa_camera_resume(struct soc_camera_device *icd)
 {
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 	int i = 0, ret = 0;
 
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index f5a1a86..a63738c 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -52,8 +52,7 @@ static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	enum v4l2_field field;
 	int ret;
 
@@ -117,8 +116,7 @@ static int soc_camera_reqbufs(struct file *file, void *priv,
 	int ret;
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 
 	WARN_ON(priv != file->private_data);
 
@@ -282,8 +280,7 @@ static unsigned int soc_camera_poll(struct file *file, poll_table *pt)
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 
 	if (list_empty(&icf->vb_vidq.stream)) {
 		dev_err(&icd->dev, "Trying to poll with no queued buffers!\n");
@@ -309,8 +306,7 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	int ret;
 	struct v4l2_rect rect;
 
@@ -355,8 +351,7 @@ static int soc_camera_enum_fmt_vid_cap(struct file *file, void  *priv,
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 
 	WARN_ON(priv != file->private_data);
 
@@ -388,8 +383,7 @@ static int soc_camera_querycap(struct file *file, void  *priv,
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 
 	WARN_ON(priv != file->private_data);
 
@@ -540,8 +534,7 @@ static int soc_camera_s_crop(struct file *file, void *fh,
 {
 	struct soc_camera_file *icf = file->private_data;
 	struct soc_camera_device *icd = icf->icd;
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	int ret;
 
 	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
@@ -665,13 +658,9 @@ static int scan_add_device(struct soc_camera_device *icd)
 static int soc_camera_probe(struct device *dev)
 {
 	struct soc_camera_device *icd = to_soc_camera_dev(dev);
-	struct soc_camera_host *ici =
-		to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	int ret;
 
-	if (!icd->ops->probe)
-		return -ENODEV;
-
 	/* We only call ->add() here to activate and probe the camera.
 	 * We shall ->remove() and deactivate it immediately afterwards. */
 	ret = ici->ops->add(icd);
@@ -752,7 +741,17 @@ int soc_camera_host_register(struct soc_camera_host *ici)
 	int ret;
 	struct soc_camera_host *ix;
 
-	if (!ici->ops->init_videobuf || !ici->ops->add || !ici->ops->remove)
+	if (!ici || !ici->ops ||
+	    !ici->ops->try_fmt ||
+	    !ici->ops->set_fmt ||
+	    !ici->ops->enum_fmt ||
+	    !ici->ops->set_bus_param ||
+	    !ici->ops->querycap ||
+	    !ici->ops->init_videobuf ||
+	    !ici->ops->reqbufs ||
+	    !ici->ops->add ||
+	    !ici->ops->remove ||
+	    !ici->ops->poll)
 		return -EINVAL;
 
 	/* Number might be equal to the platform device ID */
@@ -820,7 +819,16 @@ int soc_camera_device_register(struct soc_camera_device *icd)
 	struct soc_camera_device *ix;
 	int num = -1, i;
 
-	if (!icd)
+	if (!icd || !icd->ops ||
+	    !icd->ops->probe ||
+	    !icd->ops->init ||
+	    !icd->ops->release ||
+	    !icd->ops->start_capture ||
+	    !icd->ops->stop_capture ||
+	    !icd->ops->set_fmt ||
+	    !icd->ops->try_fmt ||
+	    !icd->ops->query_bus_param ||
+	    !icd->ops->set_bus_param)
 		return -EINVAL;
 
 	for (i = 0; i < 256 && num < 0; i++) {
@@ -843,7 +851,9 @@ int soc_camera_device_register(struct soc_camera_device *icd)
 	snprintf(icd->dev.bus_id, sizeof(icd->dev.bus_id),
 		 "%u-%u", icd->iface, icd->devnum);
 
-	icd->dev.release = dummy_release;
+	icd->dev.release	= dummy_release;
+	icd->use_count		= 0;
+	icd->host_priv		= NULL;
 
 	return scan_add_device(icd);
 }
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 08/13] soc_camera: add format translation framework

[Robert Jarzmik]

Camera hosts rely on sensor formats available, as well as
host specific translations. We add a framework so that hosts
only have to define a translation table.

Based on this translation table, soc_camera will compute
available formats by matching sensor outputs to host
translations.

The host can add other formats which were not computed by
the translation mechanism, by implementing the
add_extra_format() host operation.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/video/soc_camera.c |  102 ++++++++++++++++++++++++++++++++++++-
 include/media/soc_camera.h       |   24 +++++++++
 2 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index a63738c..641b977 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -47,6 +47,77 @@ const struct soc_camera_data_format *soc_camera_format_by_fourcc(
 }
 EXPORT_SYMBOL(soc_camera_format_by_fourcc);
 
+/**
+ * @icd:	camera device
+ * @i:		index of the camera format to be considered
+ * @fmt:	if (fmt != NULL) it points to the beginning of an area,
+ *		sufficient for all extra formats, that the i's camera format
+ *		can be converted to, and this area has to be filled in
+ * @return:	number of extra formats found for this camera format
+ */
+static int soc_camera_format_generate(struct soc_camera_device *icd, int idx,
+				      struct soc_camera_computed_format *fmt)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_format_translate *trans;
+	int found = 0;
+
+	for (trans = ici->translate_fmt; trans->host_fmt.name; trans++ ) {
+		if (trans->host_fmt.depth != icd->formats[idx].depth)
+			continue;
+		if (trans->sensor_fourcc != icd->formats[idx].fourcc)
+			continue;
+		if (fmt) {
+			fmt[found].host_fmt = &trans->host_fmt;
+			fmt[found].sensor_fmt = &icd->formats[idx];
+		}
+		found++;
+	}
+	return found;
+}
+
+static int soc_camera_init_formats(struct soc_camera_device *icd)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	int i, j, host_fmts = 0;
+
+	if (!ici->translate_fmt) {
+		dev_err(&icd->dev, "No format translation table provided\n");
+		return -ENODATA;
+	}
+
+	for (i = 0; i < icd->num_formats; i++)
+		host_fmts += soc_camera_format_generate(icd, i, NULL);
+	for (i = 0; ici->ops->add_extra_format && i < icd->num_formats; i++)
+		host_fmts += ici->ops->add_extra_format(icd, i, NULL);
+
+	icd->available_fmts = vmalloc(sizeof(struct soc_camera_computed_format)
+				      * host_fmts);
+	if (!icd->available_fmts)
+		return -ENOMEM;
+
+	dev_dbg(&icd->dev, "Found %d sensor to host compatible formats :\n",
+		host_fmts);
+
+	for (i = 0, j = 0; i < icd->num_formats; i++)
+		j += soc_camera_format_generate(icd, i, &icd->available_fmts[j]);
+	for (i = 0; ici->ops->add_extra_format && i < icd->num_formats; i++)
+		j += ici->ops->add_extra_format(icd, i, &icd->available_fmts[j]);
+
+	for (i = 0; i < host_fmts; i++)
+		dev_dbg(&icd->dev, "\t%s <- sensor %s\n",
+			icd->available_fmts[i].host_fmt->name,
+			icd->available_fmts[i].sensor_fmt->name);
+
+	icd->num_available_fmts = host_fmts;
+	return 0;
+}
+
+static void soc_camera_free_formats(struct soc_camera_device *icd)
+{
+	vfree(icd->available_fmts);
+}
+
 static int soc_camera_try_fmt_vid_cap(struct file *file, void *priv,
 				      struct v4l2_format *f)
 {
@@ -193,6 +264,11 @@ static int soc_camera_open(struct inode *inode, struct file *file)
 	icf->icd = icd;
 	icd->use_count++;
 
+	/* Generate available formats */
+	ret = soc_camera_init_formats(icd);
+	if (ret)
+		goto egenfmt;
+
 	/* Now we really have to activate the camera */
 	if (icd->use_count == 1) {
 		ret = ici->ops->add(icd);
@@ -214,6 +290,8 @@ static int soc_camera_open(struct inode *inode, struct file *file)
 
 	/* All errors are entered with the video_lock held */
 eiciadd:
+	soc_camera_free_formats(icd);
+egenfmt:
 	module_put(ici->ops->owner);
 emgi:
 	module_put(icd->ops->owner);
@@ -231,6 +309,7 @@ static int soc_camera_close(struct inode *inode, struct file *file)
 	struct video_device *vdev = icd->vdev;
 
 	mutex_lock(&video_lock);
+	soc_camera_free_formats(icd);
 	icd->use_count--;
 	if (!icd->use_count)
 		ici->ops->remove(icd);
@@ -323,8 +402,7 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 	ret = ici->ops->set_fmt(icd, f->fmt.pix.pixelformat, &rect);
 	if (ret < 0) {
 		return ret;
-	} else if (!icd->current_fmt ||
-		   icd->current_fmt->fourcc != f->fmt.pix.pixelformat) {
+	} else if (!icd->current_fmt) {
 		dev_err(&ici->dev, "Host driver hasn't set up current "
 			"format correctly!\n");
 		return -EINVAL;
@@ -346,6 +424,22 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
 	return ici->ops->set_bus_param(icd, f->fmt.pix.pixelformat);
 }
 
+static int soc_camera_default_enum_fmt(struct soc_camera_device *icd,
+				       struct v4l2_fmtdesc *f)
+{
+	const struct soc_camera_data_format *format;
+
+	if (f->index >= icd->num_available_fmts)
+		return -EINVAL;
+
+	format = icd->available_fmts[f->index].host_fmt;
+
+	strlcpy(f->description, format->name, sizeof(f->description));
+	f->pixelformat = format->fourcc;
+
+	return 0;
+}
+
 static int soc_camera_enum_fmt_vid_cap(struct file *file, void  *priv,
 				       struct v4l2_fmtdesc *f)
 {
@@ -744,7 +838,6 @@ int soc_camera_host_register(struct soc_camera_host *ici)
 	if (!ici || !ici->ops ||
 	    !ici->ops->try_fmt ||
 	    !ici->ops->set_fmt ||
-	    !ici->ops->enum_fmt ||
 	    !ici->ops->set_bus_param ||
 	    !ici->ops->querycap ||
 	    !ici->ops->init_videobuf ||
@@ -754,6 +847,9 @@ int soc_camera_host_register(struct soc_camera_host *ici)
 	    !ici->ops->poll)
 		return -EINVAL;
 
+	if (!ici->ops->enum_fmt)
+		ici->ops->enum_fmt = soc_camera_default_enum_fmt;
+
 	/* Number might be equal to the platform device ID */
 	sprintf(ici->dev.bus_id, "camera_host%d", ici->nr);
 
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index a63f7fb..f800948 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -41,6 +41,8 @@ struct soc_camera_device {
 	const struct soc_camera_data_format *current_fmt;
 	const struct soc_camera_data_format *formats;
 	int num_formats;
+	struct soc_camera_computed_format *available_fmts;
+	int num_available_fmts;
 	struct module *owner;
 	void *host_priv;		/* per-device host private data */
 	/* soc_camera.c private count. Only accessed with video_lock held */
@@ -58,6 +60,7 @@ struct soc_camera_host {
 	unsigned char nr;				/* Host number */
 	void *priv;
 	char *drv_name;
+	const struct soc_camera_format_translate *translate_fmt;
 	struct soc_camera_host_ops *ops;
 };
 
@@ -76,6 +79,8 @@ struct soc_camera_host_ops {
 	int (*querycap)(struct soc_camera_host *, struct v4l2_capability *);
 	int (*set_bus_param)(struct soc_camera_device *, __u32);
 	unsigned int (*poll)(struct file *, poll_table *);
+	int (*add_extra_format)(struct soc_camera_device *icd, int idx,
+				struct soc_camera_computed_format *fmt);
 };
 
 struct soc_camera_link {
@@ -109,6 +114,13 @@ extern void soc_camera_video_stop(struct soc_camera_device *icd);
 extern const struct soc_camera_data_format *soc_camera_format_by_fourcc(
 	struct soc_camera_device *icd, unsigned int fourcc);
 
+#define COL_FMT(_name, _depth, _fourcc, _colorspace) \
+	{ .name = _name, .depth = _depth, .fourcc = _fourcc, \
+	.colorspace = _colorspace }
+#define RGB_FMT(_name, _depth, _fourcc) \
+	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_SRGB)
+#define JPG_FMT(_name, _depth, _fourcc) \
+	COL_FMT(_name, _depth, _fourcc, V4L2_COLORSPACE_JPEG)
 struct soc_camera_data_format {
 	char *name;
 	unsigned int depth;
@@ -116,6 +128,18 @@ struct soc_camera_data_format {
 	enum v4l2_colorspace colorspace;
 };
 
+#define LAST_FMT_TRANSLATION { COL_FMT(NULL, 0, 0, 0), 0 }
+struct soc_camera_format_translate {
+	struct soc_camera_data_format host_fmt;
+	__u32 sensor_fourcc;
+};
+
+struct soc_camera_computed_format {
+	struct soc_camera_data_format *host_fmt;
+	struct soc_camera_data_format *sensor_fmt;
+};
+
+
 struct soc_camera_ops {
 	struct module *owner;
 	int (*probe)(struct soc_camera_device *);
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 09/13] pxa_camera: use the translation framework

[Robert Jarzmik]

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Use the newly created translation framework for pxa camera
host.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/video/pxa_camera.c |   89 ++++++++++++++++++++++----------------
 1 files changed, 52 insertions(+), 37 deletions(-)

diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 56aeb07..3e7ce6f 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -27,6 +27,7 @@
 #include <linux/platform_device.h>
 #include <linux/mutex.h>
 #include <linux/clk.h>
+#include <linux/vmalloc.h>
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-dev.h>
@@ -693,10 +694,17 @@ static int pxa_camera_add_device(struct soc_camera_device *icd)
 
 	pxa_camera_activate(pcdev);
 	ret = icd->ops->init(icd);
+	if (ret < 0)
+		goto einit;
+
+	pcdev->icd = icd;
 
-	if (!ret)
-		pcdev->icd = icd;
+	mutex_unlock(&camera_lock);
+
+	return 0;
 
+einit:
+	pxa_camera_deactivate(pcdev);
 ebusy:
 	mutex_unlock(&camera_lock);
 
@@ -713,6 +721,8 @@ static void pxa_camera_remove_device(struct soc_camera_device *icd)
 	dev_info(&icd->dev, "PXA Camera driver detached from camera %d\n",
 		 icd->devnum);
 
+	mutex_lock(&camera_lock);
+
 	/* disable capture, disable interrupts */
 	CICR0 = 0x3ff;
 
@@ -725,7 +735,12 @@ static void pxa_camera_remove_device(struct soc_camera_device *icd)
 
 	pxa_camera_deactivate(pcdev);
 
+	vfree(icd->host_priv);
+	icd->host_priv = NULL;
+
 	pcdev->icd = NULL;
+
+	mutex_unlock(&camera_lock);
 }
 
 static int test_platform_param(struct pxa_camera_dev *pcdev,
@@ -898,23 +913,30 @@ static int pxa_camera_try_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
 	return soc_camera_bus_param_compatible(camera_flags, bus_flags) ? 0 : -EINVAL;
 }
 
+static struct soc_camera_data_format *
+pxa_camera_find_sensor_fmt(struct soc_camera_device *icd, __u32 pixfmt)
+{
+	int i;
+
+	if (pixfmt) {
+		for (i = 0; i < icd->num_available_fmts; i++)
+			if (icd->available_fmts[i].host_fmt->fourcc == pixfmt)
+				return icd->available_fmts[i].sensor_fmt;
+	}
+	return NULL;
+}
+
 static int pxa_camera_set_fmt(struct soc_camera_device *icd,
 			      __u32 pixfmt, struct v4l2_rect *rect)
 {
-	const struct soc_camera_data_format *cam_fmt;
+	const struct soc_camera_data_format *cam_fmt = NULL;
 	int ret;
 
-	/*
-	 * TODO: find a suitable supported by the SoC output format, check
-	 * whether the sensor supports one of acceptable input formats.
-	 */
-	if (pixfmt) {
-		cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
-		if (!cam_fmt)
-			return -EINVAL;
-	}
+	cam_fmt = pxa_camera_find_sensor_fmt(icd, pixfmt);
+	if (!cam_fmt)
+		return -EINVAL;
 
-	ret = icd->ops->set_fmt(icd, pixfmt, rect);
+	ret = icd->ops->set_fmt(icd, cam_fmt->fourcc, rect);
 	if (pixfmt && !ret)
 		icd->current_fmt = cam_fmt;
 
@@ -924,18 +946,16 @@ static int pxa_camera_set_fmt(struct soc_camera_device *icd,
 static int pxa_camera_try_fmt(struct soc_camera_device *icd,
 			      struct v4l2_format *f)
 {
-	const struct soc_camera_data_format *cam_fmt;
+	const struct soc_camera_data_format *cam_fmt = NULL;
 	struct v4l2_pix_format *pix = &f->fmt.pix;
-	int ret = pxa_camera_try_bus_param(icd, pix->pixelformat);
+	__u32 pixfmt = pix->pixelformat;
+	int ret = pxa_camera_try_bus_param(icd, pixfmt);
 
 	if (ret < 0)
 		return ret;
 
-	/*
-	 * TODO: find a suitable supported by the SoC output format, check
-	 * whether the sensor supports one of acceptable input formats.
-	 */
-	cam_fmt = soc_camera_format_by_fourcc(icd, pix->pixelformat);
+	cam_fmt = pxa_camera_find_sensor_fmt(icd, pixfmt);
+
 	if (!cam_fmt)
 		return -EINVAL;
 
@@ -958,22 +978,6 @@ static int pxa_camera_try_fmt(struct soc_camera_device *icd,
 	return icd->ops->try_fmt(icd, f);
 }
 
-static int pxa_camera_enum_fmt(struct soc_camera_device *icd,
-			       struct v4l2_fmtdesc *f)
-{
-	const struct soc_camera_data_format *format;
-
-	if (f->index >= icd->num_formats)
-		return -EINVAL;
-
-	format = &icd->formats[f->index];
-
-	strlcpy(f->description, format->name, sizeof(f->description));
-	f->pixelformat = format->fourcc;
-
-	return 0;
-}
-
 static int pxa_camera_reqbufs(struct soc_camera_file *icf,
 			      struct v4l2_requestbuffers *p)
 {
@@ -1079,7 +1083,6 @@ static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
 	.resume		= pxa_camera_resume,
 	.set_fmt	= pxa_camera_set_fmt,
 	.try_fmt	= pxa_camera_try_fmt,
-	.enum_fmt	= pxa_camera_enum_fmt,
 	.init_videobuf	= pxa_camera_init_videobuf,
 	.reqbufs	= pxa_camera_reqbufs,
 	.poll		= pxa_camera_poll,
@@ -1087,10 +1090,22 @@ static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
 	.set_bus_param	= pxa_camera_set_bus_param,
 };
 
+static struct soc_camera_format_translate pxa_pixfmt_translations[] = {
+	{ JPG_FMT("CbYCrY 16 bit", 16, V4L2_PIX_FMT_UYVY), V4L2_PIX_FMT_UYVY },
+	{ JPG_FMT("CrYCbY 16 bit", 16, V4L2_PIX_FMT_VYUY), V4L2_PIX_FMT_VYUY },
+	{ JPG_FMT("YCbYCr 16 bit", 16, V4L2_PIX_FMT_YUYV), V4L2_PIX_FMT_YUYV },
+	{ JPG_FMT("YCrYCb 16 bit", 16, V4L2_PIX_FMT_YVYU), V4L2_PIX_FMT_YVYU },
+	{ JPG_FMT("YUV planar", 16, V4L2_PIX_FMT_YUV422P), V4L2_PIX_FMT_UYVY },
+	{ RGB_FMT("RGB 555", 16, V4L2_PIX_FMT_RGB555), V4L2_PIX_FMT_RGB555 },
+	{ RGB_FMT("RGB 565", 16, V4L2_PIX_FMT_RGB565), V4L2_PIX_FMT_RGB565 },
+	LAST_FMT_TRANSLATION
+};
+
 /* Should be allocated dynamically too, but we have only one. */
 static struct soc_camera_host pxa_soc_camera_host = {
 	.drv_name		= PXA_CAM_DRV_NAME,
 	.ops			= &pxa_soc_camera_host_ops,
+	.translate_fmt		= pxa_pixfmt_translations,
 };
 
 static int pxa_camera_probe(struct platform_device *pdev)
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 10/13] sh_mobile_ceu_camera: (ab)use the translation framework

[Robert Jarzmik]

Use the newly created translation framework for SuperH
Mobile CEU camera host. This host doesn't use the generic
framework, and implements its own enumeration and format
availability.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/video/sh_mobile_ceu_camera.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index fdfe04f..9d139d1 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -596,6 +596,10 @@ static struct soc_camera_host_ops sh_mobile_ceu_host_ops = {
 	.init_videobuf	= sh_mobile_ceu_init_videobuf,
 };
 
+static struct soc_camera_format_translate *sh_mobile_ceu_pixfmt_translations = {
+	LAST_FMT_TRANSLATION,
+};
+
 static int sh_mobile_ceu_probe(struct platform_device *pdev)
 {
 	struct sh_mobile_ceu_dev *pcdev;
@@ -669,8 +673,9 @@ static int sh_mobile_ceu_probe(struct platform_device *pdev)
 	pcdev->ici.priv = pcdev;
 	pcdev->ici.dev.parent = &pdev->dev;
 	pcdev->ici.nr = pdev->id;
-	pcdev->ici.drv_name = pdev->dev.bus_id,
-	pcdev->ici.ops = &sh_mobile_ceu_host_ops,
+	pcdev->ici.drv_name = pdev->dev.bus_id;
+	pcdev->ici.ops = &sh_mobile_ceu_host_ops;
+	pcdev->ici.translate_fmt = sh_mobile_ceu_pixfmt_translations;
 
 	err = soc_camera_host_register(&pcdev->ici);
 	if (err)
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 11/13] pxa_camera: check that YUV formats are always 8 bit wide

[Robert Jarzmik]

The pxa only accepts YUV formats only when 8 bit bus mode is
selected. Add a check to ensure the right bus mode was
selected when trying to use 8 bit mode.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/video/pxa_camera.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 3e7ce6f..cd9d09e 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -781,12 +781,34 @@ static int test_platform_param(struct pxa_camera_dev *pcdev,
 	return 0;
 }
 
+static int is_yuv_format(__u32 pixfmt)
+{
+	switch (pixfmt) {
+	case V4L2_PIX_FMT_YVYU:
+	case V4L2_PIX_FMT_YUYV:
+	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_VYUY:
+	case V4L2_PIX_FMT_YUV422P:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
 static int pxa_camera_set_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
 {
 	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
 	struct pxa_camera_dev *pcdev = ici->priv;
 	unsigned long dw, bpp, bus_flags, camera_flags, common_flags;
 	u32 cicr0, cicr1, cicr4 = 0;
+
+	/*
+	 * As stated in PXA developer's manual, YUV formats only accept 8 bit
+	 * wide buswidth.
+	 */
+	if (is_yuv_format(pixfmt))
+		icd->buswidth = 8;
+
 	int ret = test_platform_param(pcdev, icd->buswidth, &bus_flags);
 
 	if (ret < 0)
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 12/13] pxa_camera: Fix YUV format handling.

[Robert Jarzmik]

Allows all YUV formats on pxa interface. Even if PXA capture
interface expects data in UYVY format, we allow all formats
considering the pxa bus is not making any translation.

For the special YUV planar format, we translate the pixel
format asked to the sensor to UYVY, which is the bus byte
order necessary (out of the sensor) for the pxa to make the
correct translation.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/video/pxa_camera.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index cd9d09e..fde14e7 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -894,7 +894,17 @@ static int pxa_camera_set_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
 	case V4L2_PIX_FMT_YUV422P:
 		pcdev->channels = 3;
 		cicr1 |= CICR1_YCBCR_F;
+		/*
+		 * Normally, pxa bus wants as input UYVY format.
+		 * We allow all YUV formats, as no translation is used, and the
+		 * YUV stream is just passed through without any transformation.
+		 * Note that UYVY is the only format that should be used if pxa
+		 * framebuffer Overlay2 is used.
+		 */
+	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_VYUY:
 	case V4L2_PIX_FMT_YUYV:
+	case V4L2_PIX_FMT_YVYU:
 		cicr1 |= CICR1_COLOR_SP_VAL(2);
 		break;
 	case V4L2_PIX_FMT_RGB555:
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 09/13] pxa_camera: use the translation framework

[Guennadi Liakhovetski]

Hi Robert,

a detailed review will follow, I will need some time for it. Just a couple 
of quick notes to this one:

On Wed, 12 Nov 2008, Robert Jarzmik wrote:

> From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

no, this one is not from me:-)

> 
> Use the newly created translation framework for pxa camera
> host.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  drivers/media/video/pxa_camera.c |   89 ++++++++++++++++++++++----------------
>  1 files changed, 52 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
> index 56aeb07..3e7ce6f 100644
> --- a/drivers/media/video/pxa_camera.c
> +++ b/drivers/media/video/pxa_camera.c

[snip]

> +static struct soc_camera_format_translate pxa_pixfmt_translations[] = {
> +	{ JPG_FMT("CbYCrY 16 bit", 16, V4L2_PIX_FMT_UYVY), V4L2_PIX_FMT_UYVY },
> +	{ JPG_FMT("CrYCbY 16 bit", 16, V4L2_PIX_FMT_VYUY), V4L2_PIX_FMT_VYUY },
> +	{ JPG_FMT("YCbYCr 16 bit", 16, V4L2_PIX_FMT_YUYV), V4L2_PIX_FMT_YUYV },
> +	{ JPG_FMT("YCrYCb 16 bit", 16, V4L2_PIX_FMT_YVYU), V4L2_PIX_FMT_YVYU },
> +	{ JPG_FMT("YUV planar", 16, V4L2_PIX_FMT_YUV422P), V4L2_PIX_FMT_UYVY },
> +	{ RGB_FMT("RGB 555", 16, V4L2_PIX_FMT_RGB555), V4L2_PIX_FMT_RGB555 },
> +	{ RGB_FMT("RGB 565", 16, V4L2_PIX_FMT_RGB565), V4L2_PIX_FMT_RGB565 },
> +	LAST_FMT_TRANSLATION
> +};
> +
>  /* Should be allocated dynamically too, but we have only one. */
>  static struct soc_camera_host pxa_soc_camera_host = {
>  	.drv_name		= PXA_CAM_DRV_NAME,
>  	.ops			= &pxa_soc_camera_host_ops,
> +	.translate_fmt		= pxa_pixfmt_translations,
>  };

Do I understand it right, that with this all Bayer and monochrome formats 
will stop working with pxa? If so - not good. Remember what we discussed 
about a default "pass-through" case?

Thanks
Guennadi
---
Guennadi Liakhovetski

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 09/13] pxa_camera: use the translation framework

[Magnus Damm]

Hi guys,

On Thu, Nov 13, 2008 at 6:19 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Wed, 12 Nov 2008, Robert Jarzmik wrote:
>> Use the newly created translation framework for pxa camera
>> host.
>>
>> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
>> ---
>>  drivers/media/video/pxa_camera.c |   89 ++++++++++++++++++++++----------------
>>  1 files changed, 52 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
>> index 56aeb07..3e7ce6f 100644
>> --- a/drivers/media/video/pxa_camera.c
>> +++ b/drivers/media/video/pxa_camera.c
>
> [snip]
>
>> +static struct soc_camera_format_translate pxa_pixfmt_translations[] = {
>> +     { JPG_FMT("CbYCrY 16 bit", 16, V4L2_PIX_FMT_UYVY), V4L2_PIX_FMT_UYVY },
>> +     { JPG_FMT("CrYCbY 16 bit", 16, V4L2_PIX_FMT_VYUY), V4L2_PIX_FMT_VYUY },
>> +     { JPG_FMT("YCbYCr 16 bit", 16, V4L2_PIX_FMT_YUYV), V4L2_PIX_FMT_YUYV },
>> +     { JPG_FMT("YCrYCb 16 bit", 16, V4L2_PIX_FMT_YVYU), V4L2_PIX_FMT_YVYU },
>> +     { JPG_FMT("YUV planar", 16, V4L2_PIX_FMT_YUV422P), V4L2_PIX_FMT_UYVY },
>> +     { RGB_FMT("RGB 555", 16, V4L2_PIX_FMT_RGB555), V4L2_PIX_FMT_RGB555 },
>> +     { RGB_FMT("RGB 565", 16, V4L2_PIX_FMT_RGB565), V4L2_PIX_FMT_RGB565 },
>> +     LAST_FMT_TRANSLATION
>> +};
>> +
>>  /* Should be allocated dynamically too, but we have only one. */
>>  static struct soc_camera_host pxa_soc_camera_host = {
>>       .drv_name               = PXA_CAM_DRV_NAME,
>>       .ops                    = &pxa_soc_camera_host_ops,
>> +     .translate_fmt          = pxa_pixfmt_translations,
>>  };
>
> Do I understand it right, that with this all Bayer and monochrome formats
> will stop working with pxa? If so - not good. Remember what we discussed
> about a default "pass-through" case?

Yeah, I'd like to have a default "pass-through" case for the SuperH
CEU driver as well. The host driver is totally unaware of all data
formats, with the exception of a few formats that can be translated
into NV12/NV21/NV16/NV61.

Cheers,

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 09/13] pxa_camera: use the translation framework

[Robert Jarzmik]

"Magnus Damm" <magnus.damm@gmail.com> writes:

> Hi guys,
>
> Yeah, I'd like to have a default "pass-through" case for the SuperH
> CEU driver as well. The host driver is totally unaware of all data
> formats, with the exception of a few formats that can be translated
> into NV12/NV21/NV16/NV61.

For the SuperH, the "pass-through" was kept if I coded correctly. The
translation API is not used at all, and the host driver should behave just as
before.

BTW, if you have the hardware, it would be good, when the review will be
advanced enough, to make some tests (thinking non-regression here).

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[PATCH 13/13] pxa_camera: add sensor format passthrough

[Robert Jarzmik]

Once the pixel translation layer is added and used,
pxa_camera should choose which native sensor formats to pass
through untouched (bayer and monochromas are good examples).

Add them in this patch.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/video/pxa_camera.c |   51 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index fde14e7..829037d 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -1010,6 +1010,56 @@ static int pxa_camera_try_fmt(struct soc_camera_device *icd,
 	return icd->ops->try_fmt(icd, f);
 }
 
+static bool depth_supported(struct soc_camera_device *icd, int i)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct pxa_camera_dev *pcdev = ici->priv;
+
+	switch (icd->formats[i].depth) {
+	case 8:
+		if (pcdev->platform_flags & PXA_CAMERA_DATAWIDTH_8)
+			return true;
+		return false;
+	case 9:
+		if (pcdev->platform_flags & PXA_CAMERA_DATAWIDTH_9)
+			return true;
+		return false;
+	case 10:
+		if (pcdev->platform_flags & PXA_CAMERA_DATAWIDTH_10)
+			return true;
+		return false;
+	}
+	return false;
+}
+
+static int pxa_camera_add_extra_fmt(struct soc_camera_device *icd, int idx,
+				    struct soc_camera_computed_format *fmt)
+{
+	struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
+	struct soc_camera_format_translate *trans;
+
+	for (trans = ici->translate_fmt; trans->host_fmt.name; trans++) {
+		if (trans->host_fmt.depth != icd->formats[idx].depth)
+			continue;
+		if (trans->sensor_fourcc != icd->formats[idx].fourcc)
+			continue;
+		return 0;
+	}
+
+	if (!depth_supported(icd, idx))
+		return 0;
+	/*
+	 * TODO: We should check if we really shall pass this format through
+	 *       For now, if the depth is supported, it is passed through
+	 */
+
+	if (fmt) {
+		fmt[0].host_fmt = &icd->formats[idx];
+		fmt[0].sensor_fmt = &icd->formats[idx];
+	}
+	return 1;
+}
+
 static int pxa_camera_reqbufs(struct soc_camera_file *icf,
 			      struct v4l2_requestbuffers *p)
 {
@@ -1115,6 +1165,7 @@ static struct soc_camera_host_ops pxa_soc_camera_host_ops = {
 	.resume		= pxa_camera_resume,
 	.set_fmt	= pxa_camera_set_fmt,
 	.try_fmt	= pxa_camera_try_fmt,
+	.add_extra_format = pxa_camera_add_extra_fmt,
 	.init_videobuf	= pxa_camera_init_videobuf,
 	.reqbufs	= pxa_camera_reqbufs,
 	.poll		= pxa_camera_poll,
-- 
1.5.6.5

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 09/13] pxa_camera: use the translation framework

[Robert Jarzmik]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> Do I understand it right, that with this all Bayer and monochrome formats 
> will stop working with pxa? If so - not good. Remember what we discussed 
> about a default "pass-through" case?
Oh yes, right. I'm an ass. To be more precise, a git ass.
I added the missing patch 13/13 ... sorry. This was the one which provided the
"pass-through".

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: [PATCH 09/13] pxa_camera: use the translation framework

[Magnus Damm]

Hi Robert,

On Fri, Nov 14, 2008 at 2:37 AM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> "Magnus Damm" <magnus.damm@gmail.com> writes:
>> Yeah, I'd like to have a default "pass-through" case for the SuperH
>> CEU driver as well. The host driver is totally unaware of all data
>> formats, with the exception of a few formats that can be translated
>> into NV12/NV21/NV16/NV61.
>
> For the SuperH, the "pass-through" was kept if I coded correctly. The
> translation API is not used at all, and the host driver should behave just as
> before.

That's nice, thank you. The current upstream SH Mobile CEU driver does
not need any translation, but I think your translation API may be
useful for my NV12/NV21 implementation patch posted yesterday. It
conflicts with basically everything. =)

For that patch I'd like to use pass through to support all camera
modes, but also add NVxx modes if a certain set of modes are exported
by the camera. Right now the patch is using some ugly malloc/free
operations to overload the camera mode list.

> BTW, if you have the hardware, it would be good, when the review will be
> advanced enough, to make some tests (thinking non-regression here).

I'll keep on up-porting the NV12 code and make sure that things are
working. Thanks!

/ magnus

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: soc-camera: pixelfmt translation serie

[Guennadi Liakhovetski]

Hi Robert,

On Wed, 12 Nov 2008, Robert Jarzmik wrote:

> Hi Guennadi,
> 
> This serie is prolongation of your work on the soc_camera
> bus driver. I reposted your serie within this one, so that
> this serie is a "whole", and can be applied on top of
> v2.6.28-rc4 for testing purpose.
>  => beware, the patch on the ov7272 was removed, shame on me
> 
> The patches are split into :
> 
>  - patch 1 .. 7 : copy paste of your patches, no
>  modification (beware, ov.... was expunged, sorry again)
> 
>  - patch 8 : the translation framework

Ok, I looked at your patches, I even have reviewed #8 and #9 (at least 
partially.) But I think I won't send those reviews. I have a different 
idea now:-) Don't worry, it probably won't mean more work for you. It will 
mean some more work for me, and some work for you too, but, simpler than 
what you have done in your patches, if we agree to my idea, of course:

1. On the first call to soc_camera_open() we generate a list of possible 
pixel formats.

2. on format enumeration we just walk the generated above list, so, I 
don't see a need to let host drivers overload this function. If we agree, 
this can stay central in soc_camera.c for all, then we drop my patch 
"soc-camera: move pixel format handling to host drivers (part 2)," adding 
.enum_fmt method to struct soc_camera_host_ops.

3. try_fmt and set_fmt are handled by the host driver, and that one 
decides which format to request from the sensor.

Now, seeing all the complexity in your patch-series, I would like to 
simplify it. I don't think we need to export to struct soc_camera_device 
the list of host-provided format conversions (your

	const struct soc_camera_format_translate *translate_fmt;

pointer.) So I would drop soc_camera_format_generate() and let host's 
->add_extra_formats() method do all the work - add both format types 
"special" and "pass-through." And because it now would handle not only 
extra formats, it should be called just "add_formats" or "get_formats" or 
"generate_formats"... Then available_formats I would just make a list of 
pointers to struct soc_camera_data_format, so the host driver would just 
either assign a pointer to a sensor provided format struct in case of 
pass-through, or a pointer to its own struct. Hosts would then just define 
a static array of these "extra" structs, similar to sensors. And we don't 
need your struct soc_camera_computed_format - how the host generates the 
required format and which format it requests from the sensor is their 
intimate business, we don't want to intrude in their private life:-)

How does this sound? If this is accepted, I would resend my patch-series 
without the above patch. And you can decide if you want to code this new 
patch or I shall do it.

If you do it, please, only do this one patch at first. After we have got 
it right, then we can add support to PXA and SuperH. Also we should 
preserve the current behaviour at least until the drivers are ported - 
just assign pointers to all sensor-provided formats to the newly-allocated 
list if the host doesn't provide an .add_formats method.

Thanks
Guennadi

>  - patch 9 : application of the framework to pxa_camera host
>  - patch 10 : application of the framework to
>  sh_mobile_ceu_camera host
> 
>  - patch 11 - 12 : fixes for YUV format handling in pxa_camera
> 
> This is the ground of the discussion. I know I still have to
> add documentation around the new functions/structures. I
> need to know if this is what you have in mind, to either
> continue the work or stop and take a different path.
> 
> I should add this was tested with RGB565 and all YUV formats
> on a pxa272 with a mt9m111 (as you could have expected :)).
> 
> Now is the time to improve the translation code. Happy review.
> 
> --
> Robert
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: soc-camera: pixelfmt translation serie

[Robert Jarzmik]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> Ok, I looked at your patches, I even have reviewed #8 and #9 (at least 
> partially.) But I think I won't send those reviews. I have a different 
> idea now:-) Don't worry, it probably won't mean more work for you. It will 
> mean some more work for me, and some work for you too, but, simpler than 
> what you have done in your patches, if we agree to my idea, of course:
>
> 1. On the first call to soc_camera_open() we generate a list of possible 
> pixel formats.
Ack.

> 2. on format enumeration we just walk the generated above list, so, I 
> don't see a need to let host drivers overload this function. If we agree, 
> this can stay central in soc_camera.c for all, then we drop my patch 
> "soc-camera: move pixel format handling to host drivers (part 2)," adding 
> .enum_fmt method to struct soc_camera_host_ops.
Ack.

> 3. try_fmt and set_fmt are handled by the host driver, and that one 
> decides which format to request from the sensor.
>
> Now, seeing all the complexity in your patch-series, I would like to 
> simplify it. I don't think we need to export to struct soc_camera_device 
> the list of host-provided format conversions (your
>
> 	const struct soc_camera_format_translate *translate_fmt;
>
> pointer.) So I would drop soc_camera_format_generate() and let host's 
> ->add_extra_formats() method do all the work - add both format types 
> "special" and "pass-through." And because it now would handle not only 
> extra formats, it should be called just "add_formats" or "get_formats" or 
> "generate_formats"... Then available_formats I would just make a list of 
> pointers to struct soc_camera_data_format, so the host driver would just 
> either assign a pointer to a sensor provided format struct in case of 
> pass-through, or a pointer to its own struct. Hosts would then just define 
> a static array of these "extra" structs, similar to sensors. And we don't 
> need your struct soc_camera_computed_format - how the host generates the 
> required format and which format it requests from the sensor is their 
> intimate business, we don't want to intrude in their private life:-)
>
> How does this sound? If this is accepted, I would resend my patch-series 
> without the above patch. And you can decide if you want to code this new 
> patch or I shall do it.

Sounds good. Let me add one constraint though. There should be somewhere (at
format generation for example) a debug way to show (printk, ...) each format
translation between host format and sensor format.

This was in the patch serie if soc_camera format generation function, and
provided tracability to the translation process.

Would you also duplicate current_fmt, so that the current host format and sensor
current format are available at sight ?

> If you do it, please, only do this one patch at first. After we have got 
> it right, then we can add support to PXA and SuperH. Also we should 
> preserve the current behaviour at least until the drivers are ported - 
> just assign pointers to all sensor-provided formats to the newly-allocated 
> list if the host doesn't provide an .add_formats method.
I'll wait for your patches this time, and won't generate a new one. Would you
please, once your first throw is ready, post a full serie as I did, so that I
can apply it all for test and review ? ;)

Cheers.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: soc-camera: pixelfmt translation serie

[Guennadi Liakhovetski]

On Sun, 16 Nov 2008, Robert Jarzmik wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> 
> > Ok, I looked at your patches, I even have reviewed #8 and #9 (at least 
> > partially.) But I think I won't send those reviews. I have a different 
> > idea now:-) Don't worry, it probably won't mean more work for you. It will 
> > mean some more work for me, and some work for you too, but, simpler than 
> > what you have done in your patches, if we agree to my idea, of course:
> >
> > 1. On the first call to soc_camera_open() we generate a list of possible 
> > pixel formats.
> Ack.
> 
> > 2. on format enumeration we just walk the generated above list, so, I 
> > don't see a need to let host drivers overload this function. If we agree, 
> > this can stay central in soc_camera.c for all, then we drop my patch 
> > "soc-camera: move pixel format handling to host drivers (part 2)," adding 
> > .enum_fmt method to struct soc_camera_host_ops.
> Ack.
> 
> > 3. try_fmt and set_fmt are handled by the host driver, and that one 
> > decides which format to request from the sensor.
> >
> > Now, seeing all the complexity in your patch-series, I would like to 
> > simplify it. I don't think we need to export to struct soc_camera_device 
> > the list of host-provided format conversions (your
> >
> > 	const struct soc_camera_format_translate *translate_fmt;
> >
> > pointer.) So I would drop soc_camera_format_generate() and let host's 
> > ->add_extra_formats() method do all the work - add both format types 
> > "special" and "pass-through." And because it now would handle not only 
> > extra formats, it should be called just "add_formats" or "get_formats" or 
> > "generate_formats"... Then available_formats I would just make a list of 
> > pointers to struct soc_camera_data_format, so the host driver would just 
> > either assign a pointer to a sensor provided format struct in case of 
> > pass-through, or a pointer to its own struct. Hosts would then just define 
> > a static array of these "extra" structs, similar to sensors. And we don't 
> > need your struct soc_camera_computed_format - how the host generates the 
> > required format and which format it requests from the sensor is their 
> > intimate business, we don't want to intrude in their private life:-)
> >
> > How does this sound? If this is accepted, I would resend my patch-series 
> > without the above patch. And you can decide if you want to code this new 
> > patch or I shall do it.
> 
> Sounds good. Let me add one constraint though. There should be somewhere (at
> format generation for example) a debug way to show (printk, ...) each format
> translation between host format and sensor format.
> 
> This was in the patch serie if soc_camera format generation function, and
> provided tracability to the translation process.

Yes, I saw this, and although it does look useful, I tend not to add the 
whole host format - sensor format infrastructure alone for this debug 
feature. I would restrict this generated format list to user (host) 
formats only - without exposing which sensor format the host has decided 
to use for it. We can either add this debug functionality either on a 
per-host basis, or implement a debug hook in host drivers? In any case I 
would prefer not to make this a part of the infrastructure for debugging 
alone.

> Would you also duplicate current_fmt, so that the current host format and sensor
> current format are available at sight ?

Why? Give me a real reason (apart from debugging) why we need to know in 
soc_camera.c which formats the host requests from the sensor for a 
specific output format or which format is currently configured on the 
sensor?

> > If you do it, please, only do this one patch at first. After we have got 
> > it right, then we can add support to PXA and SuperH. Also we should 
> > preserve the current behaviour at least until the drivers are ported - 
> > just assign pointers to all sensor-provided formats to the newly-allocated 
> > list if the host doesn't provide an .add_formats method.
> I'll wait for your patches this time, and won't generate a new one. Would you
> please, once your first throw is ready, post a full serie as I did, so that I
> can apply it all for test and review ? ;)

Well, would it be enough if I put the current state somewhere up as a 
quilt patch series, for instance? I don't want to repost all patches on 
each iteration.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: soc-camera: pixelfmt translation serie

[Robert Jarzmik]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> On Sun, 16 Nov 2008, Robert Jarzmik wrote:
>
>> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> Yes, I saw this, and although it does look useful, I tend not to add the 
> whole host format - sensor format infrastructure alone for this debug 
> feature. I would restrict this generated format list to user (host) 
> formats only - without exposing which sensor format the host has decided 
> to use for it. We can either add this debug functionality either on a 
> per-host basis, or implement a debug hook in host drivers? In any case I 
> would prefer not to make this a part of the infrastructure for debugging 
> alone.

Ah, but I think it is neceesary. The true purpose of soc_camera is to provide a
generic infrastructure to match sensors to hosts, isn't it ? So there should be
a way to dynamically display all available formats, and where they come from
(ie. which sensor provides it, and with which of its own formats => thinking
debugfs here).

Ideally, you will have a debugfs part telling what are the available formats, to
which (host format, sensor format) they refer, and which couple is the current
one.

>> Would you also duplicate current_fmt, so that the current host format and sensor
>> current format are available at sight ?
>
> Why? Give me a real reason (apart from debugging) why we need to know in 
> soc_camera.c which formats the host requests from the sensor for a 
> specific output format or which format is currently configured on the 
> sensor?
Exactly what you said, debug and tracability.

> Well, would it be enough if I put the current state somewhere up as a 
> quilt patch series, for instance? I don't want to repost all patches on 
> each iteration.
Very well, so just in the cover which of the previous patches should be applied
before your new serie. Or a git repository if you have one ...

Ah, and before I forget. The original idea behind the translation API was to
have the less code in each host for format list creation. I hope you keep in
mind that purpose. The less code in pxa_camera and sh_mobile_ceu_camera.c, the
better. Anyway, I'll see it in your post, and compare to the translation
framework, it's always easier to compare code than specifications :)

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

Re: soc-camera: pixelfmt translation serie

[Guennadi Liakhovetski]

On Sun, 16 Nov 2008, Robert Jarzmik wrote:

> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> 
> > On Sun, 16 Nov 2008, Robert Jarzmik wrote:
> >
> >> Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:
> > Yes, I saw this, and although it does look useful, I tend not to add the 
> > whole host format - sensor format infrastructure alone for this debug 
> > feature. I would restrict this generated format list to user (host) 
> > formats only - without exposing which sensor format the host has decided 
> > to use for it. We can either add this debug functionality either on a 
> > per-host basis, or implement a debug hook in host drivers? In any case I 
> > would prefer not to make this a part of the infrastructure for debugging 
> > alone.
> 
> Ah, but I think it is neceesary. The true purpose of soc_camera is to provide a
> generic infrastructure to match sensors to hosts, isn't it ?

Exactly - within v4l2 infrastructure.

> So there should be
> a way to dynamically display all available formats, and where they come from
> (ie. which sensor provides it, and with which of its own formats => thinking
> debugfs here).
> 
> Ideally, you will have a debugfs part telling what are the available formats, to
> which (host format, sensor format) they refer, and which couple is the current
> one.

Well, the thing is, the end user is only interested in "what can I get," 
not really "how this is going to be done" - at least not normally. And 
this is what the v4l2 API provides an answer to (what can I get) with the 
VIDIOC_ENUM_FMT ioctl. If you now impose this model: sensor provides 
format A, it is converted to format B by the host driver, and we present A 
and B for debugging, it might very well come to a point, where a host will 
have multi-stage image processing chain from A to B to C... What do you do 
then? And even now in many cases A == B, so in this case this information 
is superfluous. So, I think, this model sensor:A -> host:B is not generic 
enough to be exported to the user. Besides, we can always add it, so, for 
the beginning I would just try to get this conversions right with as 
little code and API modifications as possible.

> >> Would you also duplicate current_fmt, so that the current host format and sensor
> >> current format are available at sight ?
> >
> > Why? Give me a real reason (apart from debugging) why we need to know in 
> > soc_camera.c which formats the host requests from the sensor for a 
> > specific output format or which format is currently configured on the 
> > sensor?
> Exactly what you said, debug and tracability.
> 
> > Well, would it be enough if I put the current state somewhere up as a 
> > quilt patch series, for instance? I don't want to repost all patches on 
> > each iteration.
> Very well, so just in the cover which of the previous patches should be applied
> before your new serie. Or a git repository if you have one ...
> 
> Ah, and before I forget. The original idea behind the translation API was to
> have the less code in each host for format list creation. I hope you keep in
> mind that purpose. The less code in pxa_camera and sh_mobile_ceu_camera.c, the
> better.

...but also avoid putting code, that is too specific and restrictive in 
the core. So, I would rather put stuff in hardware drivers, then see - it 
is generic, let's make it once for all, than first put it centrally, and 
then see - it is not generic enough, we have to shift it down to hardware 
drivers.

> Anyway, I'll see it in your post, and compare to the translation
> framework, it's always easier to compare code than specifications :)

Yep, let's do that:-)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list