Re: [PATCH 3/7] v4l2: replace try_mbus_fmt by set_fmt

2015-05-02 Thread Guennadi Liakhovetski
Hi Hans,

Thanks for the patch! As already discussed on IRC, I realise, that this 
patch has already been merged. Sorry, didn't have the time to review it 
earlier. I'll provide some comments below, maybe someone decides to use 
them to improve respective locations.

On Thu, 9 Apr 2015, Hans Verkuil wrote:

 From: Hans Verkuil hans.verk...@cisco.com
 
 The try_mbus_fmt video op is a duplicate of the pad op. Replace all uses
 in sub-devices by the set_fmt() pad op.
 
 Since try_mbus_fmt and s_mbus_fmt both map to the set_fmt pad op (but
 with a different 'which' argument), this patch will replace both try_mbus_fmt
 and s_mbus_fmt by set_fmt.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Jonathan Corbet cor...@lwn.net
 Cc: Kamil Debski k.deb...@samsung.com
 ---
  drivers/media/i2c/adv7183.c| 36 
  drivers/media/i2c/mt9v011.c| 38 -
  drivers/media/i2c/ov7670.c | 27 +++-
  drivers/media/i2c/saa6752hs.c  | 28 +++--
  drivers/media/i2c/soc_camera/imx074.c  | 39 --
  drivers/media/i2c/soc_camera/mt9m001.c | 17 +---
  drivers/media/i2c/soc_camera/mt9m111.c | 31 ++
  drivers/media/i2c/soc_camera/mt9t031.c | 48 
 +++---
  drivers/media/i2c/soc_camera/mt9t112.c | 15 +--
  drivers/media/i2c/soc_camera/mt9v022.c | 17 +---
  drivers/media/i2c/soc_camera/ov2640.c  | 36 +---
  drivers/media/i2c/soc_camera/ov5642.c  | 34 +++
  drivers/media/i2c/soc_camera/ov6650.c  | 17 +---
  drivers/media/i2c/soc_camera/ov772x.c  | 15 +--
  drivers/media/i2c/soc_camera/ov9640.c  | 17 ++--
  drivers/media/i2c/soc_camera/ov9740.c  | 16 ++--
  drivers/media/i2c/soc_camera/rj54n1cb0c.c  | 40 +++---
  drivers/media/i2c/soc_camera/tw9910.c  | 15 +--
  drivers/media/i2c/sr030pc30.c  | 38 -
  drivers/media/i2c/vs6624.c | 28 ++---
  drivers/media/platform/soc_camera/sh_mobile_csi2.c | 35 
  21 files changed, 304 insertions(+), 283 deletions(-)

[snip]

 diff --git a/drivers/media/i2c/soc_camera/imx074.c 
 b/drivers/media/i2c/soc_camera/imx074.c
 index ba60ccf..f68c235 100644
 --- a/drivers/media/i2c/soc_camera/imx074.c
 +++ b/drivers/media/i2c/soc_camera/imx074.c
 @@ -153,14 +153,24 @@ static int reg_read(struct i2c_client *client, const 
 u16 addr)
   return buf[0]  0xff; /* no sign-extension */
  }
  
 -static int imx074_try_fmt(struct v4l2_subdev *sd,
 -   struct v4l2_mbus_framefmt *mf)
 +static int imx074_set_fmt(struct v4l2_subdev *sd,
 + struct v4l2_subdev_pad_config *cfg,
 + struct v4l2_subdev_format *format)
  {
 + struct v4l2_mbus_framefmt *mf = format-format;
   const struct imx074_datafmt *fmt = imx074_find_datafmt(mf-code);
 + struct i2c_client *client = v4l2_get_subdevdata(sd);
 + struct imx074 *priv = to_imx074(client);
 +
 + if (format-pad)
 + return -EINVAL;
  
   dev_dbg(sd-v4l2_dev-dev, %s(%u)\n, __func__, mf-code);
  
   if (!fmt) {
 + /* MIPI CSI could have changed the format, double-check */
 + if (format-which == V4L2_SUBDEV_FORMAT_ACTIVE)
 + return -EINVAL;
   mf-code= imx074_colour_fmts[0].code;
   mf-colorspace  = imx074_colour_fmts[0].colorspace;
   }
 @@ -169,24 +179,10 @@ static int imx074_try_fmt(struct v4l2_subdev *sd,
   mf-height  = IMX074_HEIGHT;
   mf-field   = V4L2_FIELD_NONE;
  
 - return 0;
 -}
 -
 -static int imx074_s_fmt(struct v4l2_subdev *sd,
 - struct v4l2_mbus_framefmt *mf)
 -{
 - struct i2c_client *client = v4l2_get_subdevdata(sd);
 - struct imx074 *priv = to_imx074(client);
 -
 - dev_dbg(sd-v4l2_dev-dev, %s(%u)\n, __func__, mf-code);
 -
 - /* MIPI CSI could have changed the format, double-check */
 - if (!imx074_find_datafmt(mf-code))
 - return -EINVAL;
 -
 - imx074_try_fmt(sd, mf);
 -
 - priv-fmt = imx074_find_datafmt(mf-code);
 + if (format-which == V4L2_SUBDEV_FORMAT_ACTIVE)
 + priv-fmt = imx074_find_datafmt(mf-code);

Called imx074_find_datafmt() above already, can just reuse fmt, right?

 + else
 + cfg-try_fmt = *mf;
  
   return 0;
  }
 @@ -282,8 +278,6 @@ static int imx074_g_mbus_config(struct v4l2_subdev *sd,
  
  static struct v4l2_subdev_video_ops imx074_subdev_video_ops = {
   .s_stream   = imx074_s_stream,
 - .s_mbus_fmt = imx074_s_fmt,
 - .try_mbus_fmt   = imx074_try_fmt,
   .g_crop = imx074_g_crop,
   .cropcap= 

[PATCH 3/7] v4l2: replace try_mbus_fmt by set_fmt

2015-04-09 Thread Hans Verkuil
From: Hans Verkuil hans.verk...@cisco.com

The try_mbus_fmt video op is a duplicate of the pad op. Replace all uses
in sub-devices by the set_fmt() pad op.

Since try_mbus_fmt and s_mbus_fmt both map to the set_fmt pad op (but
with a different 'which' argument), this patch will replace both try_mbus_fmt
and s_mbus_fmt by set_fmt.

Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
Cc: Jonathan Corbet cor...@lwn.net
Cc: Kamil Debski k.deb...@samsung.com
---
 drivers/media/i2c/adv7183.c| 36 
 drivers/media/i2c/mt9v011.c| 38 -
 drivers/media/i2c/ov7670.c | 27 +++-
 drivers/media/i2c/saa6752hs.c  | 28 +++--
 drivers/media/i2c/soc_camera/imx074.c  | 39 --
 drivers/media/i2c/soc_camera/mt9m001.c | 17 +---
 drivers/media/i2c/soc_camera/mt9m111.c | 31 ++
 drivers/media/i2c/soc_camera/mt9t031.c | 48 +++---
 drivers/media/i2c/soc_camera/mt9t112.c | 15 +--
 drivers/media/i2c/soc_camera/mt9v022.c | 17 +---
 drivers/media/i2c/soc_camera/ov2640.c  | 36 +---
 drivers/media/i2c/soc_camera/ov5642.c  | 34 +++
 drivers/media/i2c/soc_camera/ov6650.c  | 17 +---
 drivers/media/i2c/soc_camera/ov772x.c  | 15 +--
 drivers/media/i2c/soc_camera/ov9640.c  | 17 ++--
 drivers/media/i2c/soc_camera/ov9740.c  | 16 ++--
 drivers/media/i2c/soc_camera/rj54n1cb0c.c  | 40 +++---
 drivers/media/i2c/soc_camera/tw9910.c  | 15 +--
 drivers/media/i2c/sr030pc30.c  | 38 -
 drivers/media/i2c/vs6624.c | 28 ++---
 drivers/media/platform/soc_camera/sh_mobile_csi2.c | 35 
 21 files changed, 304 insertions(+), 283 deletions(-)

diff --git a/drivers/media/i2c/adv7183.c b/drivers/media/i2c/adv7183.c
index 9d58b75..e2dd161 100644
--- a/drivers/media/i2c/adv7183.c
+++ b/drivers/media/i2c/adv7183.c
@@ -431,10 +431,15 @@ static int adv7183_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int adv7183_try_mbus_fmt(struct v4l2_subdev *sd,
-   struct v4l2_mbus_framefmt *fmt)
+static int adv7183_set_fmt(struct v4l2_subdev *sd,
+   struct v4l2_subdev_pad_config *cfg,
+   struct v4l2_subdev_format *format)
 {
struct adv7183 *decoder = to_adv7183(sd);
+   struct v4l2_mbus_framefmt *fmt = format-format;
+
+   if (format-pad)
+   return -EINVAL;
 
fmt-code = MEDIA_BUS_FMT_UYVY8_2X8;
fmt-colorspace = V4L2_COLORSPACE_SMPTE170M;
@@ -447,16 +452,10 @@ static int adv7183_try_mbus_fmt(struct v4l2_subdev *sd,
fmt-width = 720;
fmt-height = 576;
}
-   return 0;
-}
-
-static int adv7183_s_mbus_fmt(struct v4l2_subdev *sd,
-   struct v4l2_mbus_framefmt *fmt)
-{
-   struct adv7183 *decoder = to_adv7183(sd);
-
-   adv7183_try_mbus_fmt(sd, fmt);
-   decoder-fmt = *fmt;
+   if (format-which == V4L2_SUBDEV_FORMAT_ACTIVE)
+   decoder-fmt = *fmt;
+   else
+   cfg-try_fmt = *fmt;
return 0;
 }
 
@@ -519,14 +518,13 @@ static const struct v4l2_subdev_video_ops 
adv7183_video_ops = {
.s_routing = adv7183_s_routing,
.querystd = adv7183_querystd,
.g_input_status = adv7183_g_input_status,
-   .try_mbus_fmt = adv7183_try_mbus_fmt,
-   .s_mbus_fmt = adv7183_s_mbus_fmt,
.s_stream = adv7183_s_stream,
 };
 
 static const struct v4l2_subdev_pad_ops adv7183_pad_ops = {
.enum_mbus_code = adv7183_enum_mbus_code,
.get_fmt = adv7183_get_fmt,
+   .set_fmt = adv7183_set_fmt,
 };
 
 static const struct v4l2_subdev_ops adv7183_ops = {
@@ -542,7 +540,9 @@ static int adv7183_probe(struct i2c_client *client,
struct v4l2_subdev *sd;
struct v4l2_ctrl_handler *hdl;
int ret;
-   struct v4l2_mbus_framefmt fmt;
+   struct v4l2_subdev_format fmt = {
+   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+   };
const unsigned *pin_array;
 
/* Check if the adapter supports the needed features */
@@ -612,9 +612,9 @@ static int adv7183_probe(struct i2c_client *client,
 
adv7183_writeregs(sd, adv7183_init_regs, ARRAY_SIZE(adv7183_init_regs));
adv7183_s_std(sd, decoder-std);
-   fmt.width = 720;
-   fmt.height = 576;
-   adv7183_s_mbus_fmt(sd, fmt);
+   fmt.format.width = 720;
+   fmt.format.height = 576;
+   adv7183_set_fmt(sd, NULL, fmt);
 
/* initialize the hardware to the default control values */
ret = v4l2_ctrl_handler_setup(hdl);
diff --git a/drivers/media/i2c/mt9v011.c