[PATCH 0/3] media: ov2640: fix two problems

[PATCH 0/3] media: ov2640: fix two problems

[Akinobu Mita]

This patch series contains two bugfixes and a preparatory change for
ov2640 driver.

Akinobu Mita (3):
  media: ov2640: set default window and format code at probe time
  media: ov2640: make VIDIOC_SUBDEV_G_FMT ioctl work with
    V4L2_SUBDEV_FORMAT_TRY
  media: ov2640: set all mbus format field when G_FMT and S_FMT ioctls

 drivers/media/i2c/ov2640.c | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
-- 
2.7.4

[PATCH 1/3] media: ov2640: set default window and format code at probe time

[Akinobu Mita]

Set default window and format code at probe time instead of always checking
if they have not been set yet when VIDIOC_SUBDEV_G_FMT ioctl is called.

This change simplifies the next patch (make VIDIOC_SUBDEV_G_FMT ioctl work
with V4L2_SUBDEV_FORMAT_TRY).

Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/media/i2c/ov2640.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index d8e91bc..a07e6f2 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -842,9 +842,6 @@ static int ov2640_set_params(struct i2c_client *client,
 	u8 val;
 	int ret;
 
-	if (!win)
-		return -EINVAL;
-
 	switch (code) {
 	case MEDIA_BUS_FMT_RGB565_2X8_BE:
 		dev_dbg(&client->dev, "%s: Selected cfmt RGB565 BE", __func__);
@@ -929,10 +926,6 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
 	if (format->pad)
 		return -EINVAL;
 
-	if (!priv->win) {
-		priv->win = ov2640_select_win(SVGA_WIDTH, SVGA_HEIGHT);
-		priv->cfmt_code = MEDIA_BUS_FMT_UYVY8_2X8;
-	}
 
 	mf->width	= priv->win->width;
 	mf->height	= priv->win->height;
@@ -1193,6 +1186,9 @@ static int ov2640_probe(struct i2c_client *client,
 	if (ret)
 		goto err_clk;
 
+	priv->win = ov2640_select_win(SVGA_WIDTH, SVGA_HEIGHT);
+	priv->cfmt_code = MEDIA_BUS_FMT_UYVY8_2X8;
+
 	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
 	priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
 			      V4L2_SUBDEV_FL_HAS_EVENTS;
-- 
2.7.4

[PATCH 2/3] media: ov2640: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY

[Akinobu Mita]

The VIDIOC_SUBDEV_G_FMT ioctl for this driver doesn't recognize
V4L2_SUBDEV_FORMAT_TRY and always works as if V4L2_SUBDEV_FORMAT_ACTIVE
is specified.

Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/media/i2c/ov2640.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index a07e6f2..5f888f5 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -926,6 +926,12 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
 	if (format->pad)
 		return -EINVAL;
 
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+		mf = v4l2_subdev_get_try_format(sd, cfg, 0);
+		format->format = *mf;
+
+		return 0;
+	}
 
 	mf->width	= priv->win->width;
 	mf->height	= priv->win->height;
@@ -992,6 +998,26 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
 	return ret;
 }
 
+static int ov2640_init_cfg(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_pad_config *cfg)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ov2640_priv *priv = to_ov2640(client);
+	struct v4l2_mbus_framefmt *try_fmt =
+		v4l2_subdev_get_try_format(sd, cfg, 0);
+
+	try_fmt->width = priv->win->width;
+	try_fmt->height = priv->win->height;
+	try_fmt->code = priv->cfmt_code;
+	try_fmt->colorspace = V4L2_COLORSPACE_SRGB;
+	try_fmt->field = V4L2_FIELD_NONE;
+	try_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+	try_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
+	try_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+	return 0;
+}
+
 static int ov2640_enum_mbus_code(struct v4l2_subdev *sd,
 		struct v4l2_subdev_pad_config *cfg,
 		struct v4l2_subdev_mbus_code_enum *code)
@@ -1101,6 +1127,7 @@ static const struct v4l2_subdev_core_ops ov2640_subdev_core_ops = {
 };
 
 static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
+	.init_cfg	= ov2640_init_cfg,
 	.enum_mbus_code = ov2640_enum_mbus_code,
 	.get_selection	= ov2640_get_selection,
 	.get_fmt	= ov2640_get_fmt,
-- 
2.7.4

Re: [PATCH 2/3] media: ov2640: make VIDIOC_SUBDEV_G_FMT ioctl work with V4L2_SUBDEV_FORMAT_TRY

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4919 bytes --]

Hi Akinobu,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.20-rc5 next-20181207]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Akinobu-Mita/media-ov2640-fix-two-problems/20181208-165345
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x011-201848 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/media//i2c/ov2640.c: In function 'ov2640_get_fmt':
>> drivers/media//i2c/ov2640.c:930:8: error: implicit declaration of function 'v4l2_subdev_get_try_format'; did you mean 'v4l2_subdev_notify_event'? [-Werror=implicit-function-declaration]
      mf = v4l2_subdev_get_try_format(sd, cfg, 0);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~
           v4l2_subdev_notify_event
>> drivers/media//i2c/ov2640.c:930:6: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
      mf = v4l2_subdev_get_try_format(sd, cfg, 0);
         ^
   drivers/media//i2c/ov2640.c: In function 'ov2640_init_cfg':
>> drivers/media//i2c/ov2640.c:1007:3: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
      v4l2_subdev_get_try_format(sd, cfg, 0);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +930 drivers/media//i2c/ov2640.c

   917	
   918	static int ov2640_get_fmt(struct v4l2_subdev *sd,
   919			struct v4l2_subdev_pad_config *cfg,
   920			struct v4l2_subdev_format *format)
   921	{
   922		struct v4l2_mbus_framefmt *mf = &format->format;
   923		struct i2c_client  *client = v4l2_get_subdevdata(sd);
   924		struct ov2640_priv *priv = to_ov2640(client);
   925	
   926		if (format->pad)
   927			return -EINVAL;
   928	
   929		if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
 > 930			mf = v4l2_subdev_get_try_format(sd, cfg, 0);
   931			format->format = *mf;
   932	
   933			return 0;
   934		}
   935	
   936		mf->width	= priv->win->width;
   937		mf->height	= priv->win->height;
   938		mf->code	= priv->cfmt_code;
   939		mf->colorspace	= V4L2_COLORSPACE_SRGB;
   940		mf->field	= V4L2_FIELD_NONE;
   941	
   942		return 0;
   943	}
   944	
   945	static int ov2640_set_fmt(struct v4l2_subdev *sd,
   946			struct v4l2_subdev_pad_config *cfg,
   947			struct v4l2_subdev_format *format)
   948	{
   949		struct v4l2_mbus_framefmt *mf = &format->format;
   950		struct i2c_client *client = v4l2_get_subdevdata(sd);
   951		struct ov2640_priv *priv = to_ov2640(client);
   952		const struct ov2640_win_size *win;
   953		int ret = 0;
   954	
   955		if (format->pad)
   956			return -EINVAL;
   957	
   958		mutex_lock(&priv->lock);
   959	
   960		/* select suitable win */
   961		win = ov2640_select_win(mf->width, mf->height);
   962		mf->width	= win->width;
   963		mf->height	= win->height;
   964	
   965		mf->field	= V4L2_FIELD_NONE;
   966		mf->colorspace	= V4L2_COLORSPACE_SRGB;
   967	
   968		switch (mf->code) {
   969		case MEDIA_BUS_FMT_RGB565_2X8_BE:
   970		case MEDIA_BUS_FMT_RGB565_2X8_LE:
   971		case MEDIA_BUS_FMT_YUYV8_2X8:
   972		case MEDIA_BUS_FMT_UYVY8_2X8:
   973		case MEDIA_BUS_FMT_YVYU8_2X8:
   974		case MEDIA_BUS_FMT_VYUY8_2X8:
   975			break;
   976		default:
   977			mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
   978			break;
   979		}
   980	
   981		if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
   982			struct ov2640_priv *priv = to_ov2640(client);
   983	
   984			if (priv->streaming) {
   985				ret = -EBUSY;
   986				goto out;
   987			}
   988			/* select win */
   989			priv->win = win;
   990			/* select format */
   991			priv->cfmt_code = mf->code;
   992		} else {
   993			cfg->try_fmt = *mf;
   994		}
   995	out:
   996		mutex_unlock(&priv->lock);
   997	
   998		return ret;
   999	}
  1000	
  1001	static int ov2640_init_cfg(struct v4l2_subdev *sd,
  1002				   struct v4l2_subdev_pad_config *cfg)
  1003	{
  1004		struct i2c_client *client = v4l2_get_subdevdata(sd);
  1005		struct ov2640_priv *priv = to_ov2640(client);
  1006		struct v4l2_mbus_framefmt *try_fmt =
> 1007			v4l2_subdev_get_try_format(sd, cfg, 0);
  1008	
  1009		try_fmt->width = priv->win->width;
  1010		try_fmt->height = priv->win->height;
  1011		try_fmt->code = priv->cfmt_code;
  1012		try_fmt->colorspace = V4L2_COLORSPACE_SRGB;
  1013		try_fmt->field = V4L2_FIELD_NONE;
  1014		try_fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
  1015		try_fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
  1016		try_fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
  1017	
  1018		return 0;
  1019	}
  1020	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29989 bytes --]

[PATCH 3/3] media: ov2640: set all mbus format field when G_FMT and S_FMT ioctls

[Akinobu Mita]

This driver doesn't set all members of mbus format field when the
VIDIOC_SUBDEV_{S,G}_FMT ioctls are called.

This is detected by v4l2-compliance.

Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/media/i2c/ov2640.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 5f888f5..439c6da 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -938,6 +938,9 @@ static int ov2640_get_fmt(struct v4l2_subdev *sd,
 	mf->code	= priv->cfmt_code;
 	mf->colorspace	= V4L2_COLORSPACE_SRGB;
 	mf->field	= V4L2_FIELD_NONE;
+	mf->ycbcr_enc	= V4L2_YCBCR_ENC_DEFAULT;
+	mf->quantization = V4L2_QUANTIZATION_DEFAULT;
+	mf->xfer_func	= V4L2_XFER_FUNC_DEFAULT;
 
 	return 0;
 }
@@ -964,6 +967,9 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
 
 	mf->field	= V4L2_FIELD_NONE;
 	mf->colorspace	= V4L2_COLORSPACE_SRGB;
+	mf->ycbcr_enc	= V4L2_YCBCR_ENC_DEFAULT;
+	mf->quantization = V4L2_QUANTIZATION_DEFAULT;
+	mf->xfer_func	= V4L2_XFER_FUNC_DEFAULT;
 
 	switch (mf->code) {
 	case MEDIA_BUS_FMT_RGB565_2X8_BE:
-- 
2.7.4