Re: [PATCH v5 30/35] omap3isp: Move CCDC link validation to ccdc_link_validate()

2012-03-07 Thread Laurent Pinchart
Hi Sakari,

Thanks for the patch.

On Tuesday 06 March 2012 18:33:11 Sakari Ailus wrote:
 Perform CCDC link validation in ccdc_link_validate() instead of
 isp_video_validate_pipeline(). Also perform maximum data rate check in
 isp_video_check_external_subdevs().
 
 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi

[snip]

 diff --git a/drivers/media/video/omap3isp/ispvideo.c
 b/drivers/media/video/omap3isp/ispvideo.c index 6d4ad87..51075b3 100644
 --- a/drivers/media/video/omap3isp/ispvideo.c
 +++ b/drivers/media/video/omap3isp/ispvideo.c

[snip]

 @@ -950,6 +867,7 @@ static int isp_video_check_external_subdevs(struct
 isp_pipeline *pipe) struct v4l2_subdev_format fmt;
   struct v4l2_ext_controls ctrls;
   struct v4l2_ext_control ctrl;
 + unsigned int rate = UINT_MAX;

You can move this variable inside the if statement below.

   int i;
   int ret = 0;
 
 @@ -1002,6 +920,16 @@ static int isp_video_check_external_subdevs(struct
 isp_pipeline *pipe)
 
   pipe-external_rate = ctrl.value64;
 
 + if (pipe-entities  (1  isp-isp_ccdc.subdev.entity.id)) {
 + /*
 +  * Check that maximum allowed CCDC pixel rate isn't
 +  * exceeded by the pixel rate.

What's wrong with 80 columns, really ? :-)

 +  */
 + omap3isp_ccdc_max_rate(isp-isp_ccdc, rate);
 + if (pipe-external_rate  rate)
 + return -ENOSPC;
 + }
 +
   return 0;
  }

Pending those two changes,

Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

-- 
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 v5 30/35] omap3isp: Move CCDC link validation to ccdc_link_validate()

2012-03-07 Thread Sakari Ailus
Hi Laurent,

Thanks for the comments.

On Wed, Mar 07, 2012 at 12:00:48PM +0100, Laurent Pinchart wrote:
 On Tuesday 06 March 2012 18:33:11 Sakari Ailus wrote:
  Perform CCDC link validation in ccdc_link_validate() instead of
  isp_video_validate_pipeline(). Also perform maximum data rate check in
  isp_video_check_external_subdevs().
  
  Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 
 [snip]
 
  diff --git a/drivers/media/video/omap3isp/ispvideo.c
  b/drivers/media/video/omap3isp/ispvideo.c index 6d4ad87..51075b3 100644
  --- a/drivers/media/video/omap3isp/ispvideo.c
  +++ b/drivers/media/video/omap3isp/ispvideo.c
 
 [snip]
 
  @@ -950,6 +867,7 @@ static int isp_video_check_external_subdevs(struct
  isp_pipeline *pipe) struct v4l2_subdev_format fmt;
  struct v4l2_ext_controls ctrls;
  struct v4l2_ext_control ctrl;
  +   unsigned int rate = UINT_MAX;
 
 You can move this variable inside the if statement below.

Fixed.

  int i;
  int ret = 0;
  
  @@ -1002,6 +920,16 @@ static int isp_video_check_external_subdevs(struct
  isp_pipeline *pipe)
  
  pipe-external_rate = ctrl.value64;
  
  +   if (pipe-entities  (1  isp-isp_ccdc.subdev.entity.id)) {
  +   /*
  +* Check that maximum allowed CCDC pixel rate isn't
  +* exceeded by the pixel rate.
 
 What's wrong with 80 columns, really ? :-)

Is it said somewhere in CodingStyle the comments have to be wrapped at 80
characters and no earlier? It's like speed limits: you are allowed to drive
slower than the limit. :-)

  +*/
  +   omap3isp_ccdc_max_rate(isp-isp_ccdc, rate);
  +   if (pipe-external_rate  rate)
  +   return -ENOSPC;
  +   }
  +
  return 0;
   }
 
 Pending those two changes,
 
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

Thanks. :-)

Cheers,

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
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 v5 30/35] omap3isp: Move CCDC link validation to ccdc_link_validate()

2012-03-06 Thread Sakari Ailus
Perform CCDC link validation in ccdc_link_validate() instead of
isp_video_validate_pipeline(). Also perform maximum data rate check in
isp_video_check_external_subdevs().

Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
 drivers/media/video/omap3isp/ispccdc.c  |   65 +
 drivers/media/video/omap3isp/ispvideo.c |   94 ---
 2 files changed, 76 insertions(+), 83 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c 
b/drivers/media/video/omap3isp/ispccdc.c
index cd1bf6d..84f230e 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -2000,6 +2000,69 @@ static int ccdc_set_format(struct v4l2_subdev *sd, 
struct v4l2_subdev_fh *fh,
 }
 
 /*
+ * Decide whether desired output pixel code can be obtained with
+ * the lane shifter by shifting the input pixel code.
+ * @in: input pixelcode to shifter
+ * @out: output pixelcode from shifter
+ * @additional_shift: # of bits the sensor's LSB is offset from CAMEXT[0]
+ *
+ * return true if the combination is possible
+ * return false otherwise
+ */
+static bool ccdc_is_shiftable(enum v4l2_mbus_pixelcode in,
+ enum v4l2_mbus_pixelcode out,
+ unsigned int additional_shift)
+{
+   const struct isp_format_info *in_info, *out_info;
+
+   if (in == out)
+   return true;
+
+   in_info = omap3isp_video_format_info(in);
+   out_info = omap3isp_video_format_info(out);
+
+   if ((in_info-flavor == 0) || (out_info-flavor == 0))
+   return false;
+
+   if (in_info-flavor != out_info-flavor)
+   return false;
+
+   return in_info-bpp - out_info-bpp + additional_shift = 6;
+}
+
+static int ccdc_link_validate(struct v4l2_subdev *sd,
+ struct media_link *link,
+ struct v4l2_subdev_format *source_fmt,
+ struct v4l2_subdev_format *sink_fmt)
+{
+   struct isp_ccdc_device *ccdc = v4l2_get_subdevdata(sd);
+   unsigned long parallel_shift;
+
+   /* Check if the two ends match */
+   if (source_fmt-format.width != sink_fmt-format.width ||
+   source_fmt-format.height != sink_fmt-format.height)
+   return -EPIPE;
+
+   /* We've got a parallel sensor here. */
+   if (ccdc-input == CCDC_INPUT_PARALLEL) {
+   struct isp_parallel_platform_data *pdata =
+   ((struct isp_v4l2_subdevs_group *)
+ media_entity_to_v4l2_subdev(link-source-entity)
+ -host_priv)-bus.parallel;
+   parallel_shift = pdata-data_lane_shift * 2;
+   } else {
+   parallel_shift = 0;
+   }
+
+   /* Lane shifter may be used to drop bits on CCDC sink pad */
+   if (!ccdc_is_shiftable(source_fmt-format.code,
+  sink_fmt-format.code, parallel_shift))
+   return -EPIPE;
+
+   return 0;
+}
+
+/*
  * ccdc_init_formats - Initialize formats on all pads
  * @sd: ISP CCDC V4L2 subdevice
  * @fh: V4L2 subdev file handle
@@ -2041,6 +2104,7 @@ static const struct v4l2_subdev_pad_ops ccdc_v4l2_pad_ops 
= {
.enum_frame_size = ccdc_enum_frame_size,
.get_fmt = ccdc_get_format,
.set_fmt = ccdc_set_format,
+   .link_validate = ccdc_link_validate,
 };
 
 /* V4L2 subdev operations */
@@ -2150,6 +2214,7 @@ static int ccdc_link_setup(struct media_entity *entity,
 /* media operations */
 static const struct media_entity_operations ccdc_media_ops = {
.link_setup = ccdc_link_setup,
+   .link_validate = v4l2_subdev_link_validate,
 };
 
 void omap3isp_ccdc_unregister_entities(struct isp_ccdc_device *ccdc)
diff --git a/drivers/media/video/omap3isp/ispvideo.c 
b/drivers/media/video/omap3isp/ispvideo.c
index 6d4ad87..51075b3 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -130,37 +130,6 @@ omap3isp_video_format_info(enum v4l2_mbus_pixelcode code)
 }
 
 /*
- * Decide whether desired output pixel code can be obtained with
- * the lane shifter by shifting the input pixel code.
- * @in: input pixelcode to shifter
- * @out: output pixelcode from shifter
- * @additional_shift: # of bits the sensor's LSB is offset from CAMEXT[0]
- *
- * return true if the combination is possible
- * return false otherwise
- */
-static bool isp_video_is_shiftable(enum v4l2_mbus_pixelcode in,
-   enum v4l2_mbus_pixelcode out,
-   unsigned int additional_shift)
-{
-   const struct isp_format_info *in_info, *out_info;
-
-   if (in == out)
-   return true;
-
-   in_info = omap3isp_video_format_info(in);
-   out_info = omap3isp_video_format_info(out);
-
-   if ((in_info-flavor == 0) || (out_info-flavor == 0))
-   return false;
-
-   if (in_info-flavor != out_info-flavor)
-   return false;
-
-   return