Re: [PATCH 2/7] v4l2: replace video op g_mbus_fmt by pad op get_fmt

2015-06-16 Thread Laurent Pinchart 
Hi Hans,

On Monday 15 June 2015 12:25:37 Hans Verkuil wrote:
 On 06/15/2015 12:08 AM, Laurent Pinchart wrote:
  On Thursday 09 April 2015 12:21:23 Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
  
  The g_mbus_fmt video op is a duplicate of the pad op. Replace all uses
  by the get_fmt pad op and remove the video op.
  
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
  Cc: Prabhakar Lad prabhakar.cse...@gmail.com
  Cc: Kamil Debski k.deb...@samsung.com
  
  [snip]
  
  diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
  index f2f87b7..e4fa074 100644
  --- a/drivers/media/i2c/tvp5150.c
  +++ b/drivers/media/i2c/tvp5150.c
  @@ -828,14 +828,18 @@ static int tvp5150_enum_mbus_code(struct
  v4l2_subdev *sd,
 return 0;
   }
  
  -static int tvp5150_mbus_fmt(struct v4l2_subdev *sd,
  -  struct v4l2_mbus_framefmt *f)
  +static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
  +  struct v4l2_subdev_pad_config *cfg,
  +  struct v4l2_subdev_format *format)
   {
  +  struct v4l2_mbus_framefmt *f;
 struct tvp5150 *decoder = to_tvp5150(sd);
  
  -  if (f == NULL)
  +  if (!format || format-pad)
 return -EINVAL;
  
  +  f = format-format;
  +
 tvp5150_reset(sd, 0);
  
  This resets the device every time a get or set format is issued, even for
  TRY formats. I don't think that's right.
  
  Do you have any idea why this is needed ? The code was introduced in
  commit ec2c4f3f93cb ([media] media: tvp5150: Add mbus_fmt callbacks),
  with Javier listed as the author but Mauro being the only SoB.
 
 I have no idea why this would be needed. I agree with you that it seems
 unnecessary. Note that I don't think this is ever used with TRY formats
 today, but still it doesn't look right for SET formats either.

How do we fix that, should we remove it and see what breaks ? :-)

 f-width = decoder-rect.width;
  
  @@ -1069,9 +1073,6 @@ static const struct v4l2_subdev_tuner_ops
  tvp5150_tuner_ops = { static const struct v4l2_subdev_video_ops
  tvp5150_video_ops = {
 .s_std = tvp5150_s_std,
 .s_routing = tvp5150_s_routing,
  -  .s_mbus_fmt = tvp5150_mbus_fmt,
  -  .try_mbus_fmt = tvp5150_mbus_fmt,
  -  .g_mbus_fmt = tvp5150_mbus_fmt,
 .s_crop = tvp5150_s_crop,
 .g_crop = tvp5150_g_crop,
 .cropcap = tvp5150_cropcap,
  @@ -1086,6 +1087,8 @@ static const struct v4l2_subdev_vbi_ops
  tvp5150_vbi_ops = {
  
   static const struct v4l2_subdev_pad_ops tvp5150_pad_ops = {
 .enum_mbus_code = tvp5150_enum_mbus_code,
  +  .set_fmt = tvp5150_fill_fmt,
  +  .get_fmt = tvp5150_fill_fmt,
   };
   
   static const struct v4l2_subdev_ops tvp5150_ops = {

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] v4l2: replace video op g_mbus_fmt by pad op get_fmt

2015-06-15 Thread Hans Verkuil 
On 06/15/2015 12:08 AM, Laurent Pinchart wrote:
 Hi Hans,
 
 (CC'ing Javier Martin)
 
 On Thursday 09 April 2015 12:21:23 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 The g_mbus_fmt video op is a duplicate of the pad op. Replace all uses
 by the get_fmt pad op and remove the video op.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Prabhakar Lad prabhakar.cse...@gmail.com
 Cc: Kamil Debski k.deb...@samsung.com
 
 [snip]
 
 diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
 index f2f87b7..e4fa074 100644
 --- a/drivers/media/i2c/tvp5150.c
 +++ b/drivers/media/i2c/tvp5150.c
 @@ -828,14 +828,18 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev
 *sd, return 0;
  }

 -static int tvp5150_mbus_fmt(struct v4l2_subdev *sd,
 -struct v4l2_mbus_framefmt *f)
 +static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
 +struct v4l2_subdev_pad_config *cfg,
 +struct v4l2_subdev_format *format)
  {
 +struct v4l2_mbus_framefmt *f;
  struct tvp5150 *decoder = to_tvp5150(sd);

 -if (f == NULL)
 +if (!format || format-pad)
  return -EINVAL;

 +f = format-format;
 +
  tvp5150_reset(sd, 0);
 
 This resets the device every time a get or set format is issued, even for TRY 
 formats. I don't think that's right.
 
 Do you have any idea why this is needed ? The code was introduced in commit 
 ec2c4f3f93cb ([media] media: tvp5150: Add mbus_fmt callbacks), with Javier 
 listed as the author but Mauro being the only SoB.

I have no idea why this would be needed. I agree with you that it seems
unnecessary. Note that I don't think this is ever used with TRY formats today,
but still it doesn't look right for SET formats either.

Regards,

Hans

 
  f-width = decoder-rect.width;
 @@ -1069,9 +1073,6 @@ static const struct v4l2_subdev_tuner_ops
 tvp5150_tuner_ops = { static const struct v4l2_subdev_video_ops
 tvp5150_video_ops = {
  .s_std = tvp5150_s_std,
  .s_routing = tvp5150_s_routing,
 -.s_mbus_fmt = tvp5150_mbus_fmt,
 -.try_mbus_fmt = tvp5150_mbus_fmt,
 -.g_mbus_fmt = tvp5150_mbus_fmt,
  .s_crop = tvp5150_s_crop,
  .g_crop = tvp5150_g_crop,
  .cropcap = tvp5150_cropcap,
 @@ -1086,6 +1087,8 @@ static const struct v4l2_subdev_vbi_ops
 tvp5150_vbi_ops = {

  static const struct v4l2_subdev_pad_ops tvp5150_pad_ops = {
  .enum_mbus_code = tvp5150_enum_mbus_code,
 +.set_fmt = tvp5150_fill_fmt,
 +.get_fmt = tvp5150_fill_fmt,
  };

  static const struct v4l2_subdev_ops tvp5150_ops = {
 

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] v4l2: replace video op g_mbus_fmt by pad op get_fmt

2015-06-14 Thread Laurent Pinchart 
Hi Hans,

(CC'ing Javier Martin)

On Thursday 09 April 2015 12:21:23 Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 The g_mbus_fmt video op is a duplicate of the pad op. Replace all uses
 by the get_fmt pad op and remove the video op.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Prabhakar Lad prabhakar.cse...@gmail.com
 Cc: Kamil Debski k.deb...@samsung.com

[snip]

 diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
 index f2f87b7..e4fa074 100644
 --- a/drivers/media/i2c/tvp5150.c
 +++ b/drivers/media/i2c/tvp5150.c
 @@ -828,14 +828,18 @@ static int tvp5150_enum_mbus_code(struct v4l2_subdev
 *sd, return 0;
  }
 
 -static int tvp5150_mbus_fmt(struct v4l2_subdev *sd,
 - struct v4l2_mbus_framefmt *f)
 +static int tvp5150_fill_fmt(struct v4l2_subdev *sd,
 + struct v4l2_subdev_pad_config *cfg,
 + struct v4l2_subdev_format *format)
  {
 + struct v4l2_mbus_framefmt *f;
   struct tvp5150 *decoder = to_tvp5150(sd);
 
 - if (f == NULL)
 + if (!format || format-pad)
   return -EINVAL;
 
 + f = format-format;
 +
   tvp5150_reset(sd, 0);

This resets the device every time a get or set format is issued, even for TRY 
formats. I don't think that's right.

Do you have any idea why this is needed ? The code was introduced in commit 
ec2c4f3f93cb ([media] media: tvp5150: Add mbus_fmt callbacks), with Javier 
listed as the author but Mauro being the only SoB.

   f-width = decoder-rect.width;
 @@ -1069,9 +1073,6 @@ static const struct v4l2_subdev_tuner_ops
 tvp5150_tuner_ops = { static const struct v4l2_subdev_video_ops
 tvp5150_video_ops = {
   .s_std = tvp5150_s_std,
   .s_routing = tvp5150_s_routing,
 - .s_mbus_fmt = tvp5150_mbus_fmt,
 - .try_mbus_fmt = tvp5150_mbus_fmt,
 - .g_mbus_fmt = tvp5150_mbus_fmt,
   .s_crop = tvp5150_s_crop,
   .g_crop = tvp5150_g_crop,
   .cropcap = tvp5150_cropcap,
 @@ -1086,6 +1087,8 @@ static const struct v4l2_subdev_vbi_ops
 tvp5150_vbi_ops = {
 
  static const struct v4l2_subdev_pad_ops tvp5150_pad_ops = {
   .enum_mbus_code = tvp5150_enum_mbus_code,
 + .set_fmt = tvp5150_fill_fmt,
 + .get_fmt = tvp5150_fill_fmt,
  };
 
  static const struct v4l2_subdev_ops tvp5150_ops = {

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] v4l2: replace video op g_mbus_fmt by pad op get_fmt

2015-04-16 Thread Lad, Prabhakar 
Hi Hans,

Thanks for the patch.

On Thu, Apr 9, 2015 at 11:21 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 The g_mbus_fmt video op is a duplicate of the pad op. Replace all uses
 by the get_fmt pad op and remove the video op.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Prabhakar Lad prabhakar.cse...@gmail.com
 Cc: Kamil Debski k.deb...@samsung.com
 ---
[Snip]
  drivers/media/i2c/tvp514x.c| 35 ++
  drivers/media/i2c/tvp7002.c| 28 ---
  drivers/media/platform/am437x/am437x-vpfe.c|  6 +--
  drivers/media/platform/davinci/vpfe_capture.c  | 19 

For the above,

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] v4l2: replace video op g_mbus_fmt by pad op get_fmt

2015-04-15 Thread Guennadi Liakhovetski 
On Thu, 9 Apr 2015, Hans Verkuil wrote:

 From: Hans Verkuil hans.verk...@cisco.com
 
 The g_mbus_fmt video op is a duplicate of the pad op. Replace all uses
 by the get_fmt pad op and remove the video op.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Prabhakar Lad prabhakar.cse...@gmail.com
 Cc: Kamil Debski k.deb...@samsung.com
 ---

for soc-camera:

Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de

Thanks
Guennadi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] v4l2: replace video op g_mbus_fmt by pad op get_fmt

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

The g_mbus_fmt video op is a duplicate of the pad op. Replace all uses
by the get_fmt pad op and remove the video op.

Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
Cc: Prabhakar Lad prabhakar.cse...@gmail.com
Cc: Kamil Debski k.deb...@samsung.com
---
 drivers/media/i2c/adv7170.c| 11 +++--
 drivers/media/i2c/adv7175.c| 11 +++--
 drivers/media/i2c/adv7183.c| 12 +++--
 drivers/media/i2c/adv7842.c| 14 --
 drivers/media/i2c/ak881x.c | 24 --
 drivers/media/i2c/ml86v7667.c  | 14 --
 drivers/media/i2c/saa6752hs.c  | 14 +-
 drivers/media/i2c/soc_camera/imx074.c  | 11 +++--
 drivers/media/i2c/soc_camera/mt9m001.c | 11 +++--
 drivers/media/i2c/soc_camera/mt9m111.c | 11 +++--
 drivers/media/i2c/soc_camera/mt9t031.c | 11 +++--
 drivers/media/i2c/soc_camera/mt9t112.c | 11 +++--
 drivers/media/i2c/soc_camera/mt9v022.c | 11 +++--
 drivers/media/i2c/soc_camera/ov2640.c  | 11 +++--
 drivers/media/i2c/soc_camera/ov5642.c  | 11 +++--
 drivers/media/i2c/soc_camera/ov6650.c  | 11 +++--
 drivers/media/i2c/soc_camera/ov772x.c  | 11 +++--
 drivers/media/i2c/soc_camera/rj54n1cb0c.c  | 11 +++--
 drivers/media/i2c/soc_camera/tw9910.c  | 11 +++--
 drivers/media/i2c/sr030pc30.c  | 12 +++--
 drivers/media/i2c/tvp514x.c| 35 ++
 drivers/media/i2c/tvp5150.c| 15 +++---
 drivers/media/i2c/tvp7002.c| 28 ---
 drivers/media/i2c/vs6624.c | 12 +++--
 drivers/media/pci/saa7134/saa7134-empress.c|  9 ++--
 drivers/media/platform/am437x/am437x-vpfe.c|  6 +--
 drivers/media/platform/davinci/vpfe_capture.c  | 19 
 drivers/media/platform/s5p-tv/hdmi_drv.c   | 12 +++--
 drivers/media/platform/s5p-tv/mixer_drv.c  | 15 --
 drivers/media/platform/s5p-tv/sdo_drv.c| 14 --
 drivers/media/platform/soc_camera/mx2_camera.c | 13 --
 drivers/media/platform/soc_camera/mx3_camera.c | 25 +-
 drivers/media/platform/soc_camera/omap1_camera.c   | 17 ---
 drivers/media/platform/soc_camera/pxa_camera.c | 21 +
 drivers/media/platform/soc_camera/rcar_vin.c   | 46 ++
 .../platform/soc_camera/sh_mobile_ceu_camera.c | 54 --
 drivers/media/platform/soc_camera/soc_camera.c | 15 +++---
 .../platform/soc_camera/soc_camera_platform.c  |  9 ++--
 include/media/v4l2-subdev.h|  4 --
 39 files changed, 352 insertions(+), 261 deletions(-)

diff --git a/drivers/media/i2c/adv7170.c b/drivers/media/i2c/adv7170.c
index cfe963b..58d0a3c 100644
--- a/drivers/media/i2c/adv7170.c
+++ b/drivers/media/i2c/adv7170.c
@@ -273,11 +273,16 @@ static int adv7170_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int adv7170_g_fmt(struct v4l2_subdev *sd,
-   struct v4l2_mbus_framefmt *mf)
+static int adv7170_get_fmt(struct v4l2_subdev *sd,
+   struct v4l2_subdev_pad_config *cfg,
+   struct v4l2_subdev_format *format)
 {
+   struct v4l2_mbus_framefmt *mf = format-format;
u8 val = adv7170_read(sd, 0x7);
 
+   if (format-pad)
+   return -EINVAL;
+
if ((val  0x40) == (1  6))
mf-code = MEDIA_BUS_FMT_UYVY8_1X16;
else
@@ -323,11 +328,11 @@ static const struct v4l2_subdev_video_ops 
adv7170_video_ops = {
.s_std_output = adv7170_s_std_output,
.s_routing = adv7170_s_routing,
.s_mbus_fmt = adv7170_s_fmt,
-   .g_mbus_fmt = adv7170_g_fmt,
 };
 
 static const struct v4l2_subdev_pad_ops adv7170_pad_ops = {
.enum_mbus_code = adv7170_enum_mbus_code,
+   .get_fmt = adv7170_get_fmt,
 };
 
 static const struct v4l2_subdev_ops adv7170_ops = {
diff --git a/drivers/media/i2c/adv7175.c b/drivers/media/i2c/adv7175.c
index 3f40304..f744345 100644
--- a/drivers/media/i2c/adv7175.c
+++ b/drivers/media/i2c/adv7175.c
@@ -311,11 +311,16 @@ static int adv7175_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
 }
 
-static int adv7175_g_fmt(struct v4l2_subdev *sd,
-   struct v4l2_mbus_framefmt *mf)
+static int adv7175_get_fmt(struct v4l2_subdev *sd,
+   struct v4l2_subdev_pad_config *cfg,
+   struct v4l2_subdev_format *format)
 {
+   struct v4l2_mbus_framefmt *mf = format-format;
u8 val = adv7175_read(sd, 0x7);
 
+   if (format-pad)
+   return -EINVAL;
+
if ((val  0x40) == (1  6))
mf-code = MEDIA_BUS_FMT_UYVY8_1X16;
else
@@ -376,11