public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: linux-media@vger.kernel.org, hverkuil@xs4all.nl,
	teturtia@gmail.com, dacohen@gmail.com, snjw23@gmail.com,
	andriy.shevchenko@linux.intel.com, t.stanislaws@samsung.com,
	tuukkat76@gmail.com, k.debski@gmail.com, riverful@gmail.com
Subject: Re: [PATCH v3 26/33] omap3isp: Default link validation for ccp2, csi2, preview and resizer
Date: Wed, 22 Feb 2012 12:01:26 +0100	[thread overview]
Message-ID: <4620159.TXeRQHhZdd@avalon> (raw)
In-Reply-To: <1329703032-31314-26-git-send-email-sakari.ailus@iki.fi>

Hi Sakari,

Thanks for the patch.

On Monday 20 February 2012 03:57:05 Sakari Ailus wrote:
> Use default link validation for ccp2, csi2, preview and resizer. On ccp2,
> csi2 and ccdc we also collect information on external subdevs as one may be
> connected to those entities.
> 
> The CCDC link validation still must be done separately.
> 
> Also set pipe->external correctly as we go
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  drivers/media/video/omap3isp/ispccdc.c    |   23 +++++++++++++++++++++++
>  drivers/media/video/omap3isp/ispccp2.c    |   20 ++++++++++++++++++++
>  drivers/media/video/omap3isp/ispcsi2.c    |   19 +++++++++++++++++++
>  drivers/media/video/omap3isp/isppreview.c |    2 ++
>  drivers/media/video/omap3isp/ispresizer.c |    2 ++
>  drivers/media/video/omap3isp/ispvideo.c   |    4 ++++
>  6 files changed, 70 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/omap3isp/ispccdc.c
> b/drivers/media/video/omap3isp/ispccdc.c index a74a797..6aff241 100644
> --- a/drivers/media/video/omap3isp/ispccdc.c
> +++ b/drivers/media/video/omap3isp/ispccdc.c
> @@ -1999,6 +1999,27 @@ static int ccdc_set_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_fh *fh, return 0;
>  }
> 
> +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);
> +	struct isp_pipeline *pipe = to_isp_pipeline(&ccdc->subdev.entity);
> +	int rval;
> +
> +	/* We've got a parallel sensor here. */
> +	if (ccdc->input == CCDC_INPUT_PARALLEL) {
> +		pipe->external =
> +			media_entity_to_v4l2_subdev(link->source->entity);
> +		rval = omap3isp_get_external_info(pipe, link);
> +		if (rval < 0)
> +			return 0;
> +	}

Pending my comments on 25/33, this wouldn't be needed in this patch, and could 
be squashed with 27/33.

> +
> +	return 0;
> +}
> +
>  /*
>   * ccdc_init_formats - Initialize formats on all pads
>   * @sd: ISP CCDC V4L2 subdevice
> @@ -2041,6 +2062,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 +2172,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/ispccp2.c
> b/drivers/media/video/omap3isp/ispccp2.c index 70ddbf3..4fb34ee 100644
> --- a/drivers/media/video/omap3isp/ispccp2.c
> +++ b/drivers/media/video/omap3isp/ispccp2.c
> @@ -819,6 +819,24 @@ static int ccp2_set_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_fh *fh, return 0;
>  }
> 
> +static int ccp2_link_validate(struct v4l2_subdev *sd, struct media_link
> *link, +			      struct v4l2_subdev_format *source_fmt,
> +			      struct v4l2_subdev_format *sink_fmt)
> +{
> +	struct isp_ccp2_device *ccp2 = v4l2_get_subdevdata(sd);
> +	struct isp_pipeline *pipe = to_isp_pipeline(&ccp2->subdev.entity);
> +	int rval;
> +
> +	pipe->external = media_entity_to_v4l2_subdev(link->source->entity);
> +	rval = omap3isp_get_external_info(pipe, link);
> +	if (rval < 0)
> +		return rval;
> +
> +	return v4l2_subdev_link_validate_default(sd, link, source_fmt,
> +						 sink_fmt);

That's the default behaviour, if omap3isp_get_external_info() is moved to 
ispvideo.c you can just leave .link_validate as NULL.

> +}
> +
> +
>  /*
>   * ccp2_init_formats - Initialize formats on all pads
>   * @sd: ISP CCP2 V4L2 subdevice
> @@ -925,6 +943,7 @@ static const struct v4l2_subdev_pad_ops ccp2_sd_pad_ops
> = { .enum_frame_size = ccp2_enum_frame_size,
>  	.get_fmt = ccp2_get_format,
>  	.set_fmt = ccp2_set_format,
> +	.link_validate = ccp2_link_validate,
>  };
> 
>  /* subdev operations */
> @@ -1021,6 +1040,7 @@ static int ccp2_link_setup(struct media_entity
> *entity, /* media operations */
>  static const struct media_entity_operations ccp2_media_ops = {
>  	.link_setup = ccp2_link_setup,
> +	.link_validate = v4l2_subdev_link_validate,
>  };
> 
>  /*
> diff --git a/drivers/media/video/omap3isp/ispcsi2.c
> b/drivers/media/video/omap3isp/ispcsi2.c index fcb5168..9313f7c 100644
> --- a/drivers/media/video/omap3isp/ispcsi2.c
> +++ b/drivers/media/video/omap3isp/ispcsi2.c
> @@ -1012,6 +1012,23 @@ static int csi2_set_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_fh *fh, return 0;
>  }
> 
> +static int csi2_link_validate(struct v4l2_subdev *sd, struct media_link
> *link, +			      struct v4l2_subdev_format *source_fmt,
> +			      struct v4l2_subdev_format *sink_fmt)
> +{
> +	struct isp_csi2_device *csi2 = v4l2_get_subdevdata(sd);
> +	struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity);
> +	int rval;
> +
> +	pipe->external = media_entity_to_v4l2_subdev(link->source->entity);
> +	rval = omap3isp_get_external_info(pipe, link);
> +	if (rval < 0)
> +		return rval;
> +
> +	return v4l2_subdev_link_validate_default(sd, link, source_fmt,
> +						 sink_fmt);

Same here.

> +}
> +
>  /*
>   * csi2_init_formats - Initialize formats on all pads
>   * @sd: ISP CSI2 V4L2 subdevice
> @@ -1107,6 +1124,7 @@ static const struct v4l2_subdev_pad_ops csi2_pad_ops =
> { .enum_frame_size = csi2_enum_frame_size,
>  	.get_fmt = csi2_get_format,
>  	.set_fmt = csi2_set_format,
> +	.link_validate = csi2_link_validate,
>  };
> 
>  /* subdev operations */
> @@ -1181,6 +1199,7 @@ static int csi2_link_setup(struct media_entity
> *entity, /* media operations */
>  static const struct media_entity_operations csi2_media_ops = {
>  	.link_setup = csi2_link_setup,
> +	.link_validate = v4l2_subdev_link_validate,
>  };
> 
>  void omap3isp_csi2_unregister_entities(struct isp_csi2_device *csi2)
> diff --git a/drivers/media/video/omap3isp/isppreview.c
> b/drivers/media/video/omap3isp/isppreview.c index 6d0fb2c..c2bf500 100644
> --- a/drivers/media/video/omap3isp/isppreview.c
> +++ b/drivers/media/video/omap3isp/isppreview.c
> @@ -1981,6 +1981,7 @@ static const struct v4l2_subdev_pad_ops
> preview_v4l2_pad_ops = { .set_fmt = preview_set_format,
>  	.get_crop = preview_get_crop,
>  	.set_crop = preview_set_crop,
> +	.link_validate = v4l2_subdev_link_validate_default,

You can leave this as NULL as well.

>  };
> 
>  /* subdev operations */
> @@ -2076,6 +2077,7 @@ static int preview_link_setup(struct media_entity
> *entity, /* media operations */
>  static const struct media_entity_operations preview_media_ops = {
>  	.link_setup = preview_link_setup,
> +	.link_validate = v4l2_subdev_link_validate,
>  };
> 
>  void omap3isp_preview_unregister_entities(struct isp_prev_device *prev)
> diff --git a/drivers/media/video/omap3isp/ispresizer.c
> b/drivers/media/video/omap3isp/ispresizer.c index 6958a9e..6ce2349 100644
> --- a/drivers/media/video/omap3isp/ispresizer.c
> +++ b/drivers/media/video/omap3isp/ispresizer.c
> @@ -1532,6 +1532,7 @@ static const struct v4l2_subdev_pad_ops
> resizer_v4l2_pad_ops = { .set_fmt = resizer_set_format,
>  	.get_crop = resizer_g_crop,
>  	.set_crop = resizer_s_crop,
> +	.link_validate = v4l2_subdev_link_validate_default,

And this too.

>  };
> 
>  /* subdev operations */
> @@ -1603,6 +1604,7 @@ static int resizer_link_setup(struct media_entity
> *entity, /* media operations */
>  static const struct media_entity_operations resizer_media_ops = {
>  	.link_setup = resizer_link_setup,
> +	.link_validate = v4l2_subdev_link_validate,
>  };
> 
>  void omap3isp_resizer_unregister_entities(struct isp_res_device *res)
> diff --git a/drivers/media/video/omap3isp/ispvideo.c
> b/drivers/media/video/omap3isp/ispvideo.c index 17522db..f1c68ca 100644
> --- a/drivers/media/video/omap3isp/ispvideo.c
> +++ b/drivers/media/video/omap3isp/ispvideo.c
> @@ -993,6 +993,10 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) */
>  	pipe = video->video.entity.pipe
>  	     ? to_isp_pipeline(&video->video.entity) : &video->pipe;
> +	pipe->external = NULL;
> +	pipe->external_rate = 0;
> +	pipe->external_bpp = 0;
> +
>  	ret = media_entity_pipeline_start(&video->video.entity, &pipe->pipe);
>  	if (ret < 0)
>  		goto err_media_entity_pipeline_start;
-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2012-02-22 11:01 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-20  1:56 [PATCH v3 0/33] V4L2 subdev and sensor control changes, SMIA++ driver and N9 camera board code Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 01/33] v4l: Introduce integer menu controls Sakari Ailus
2012-02-20 17:36   ` Sylwester Nawrocki
2012-02-20  1:56 ` [PATCH v3 02/33] v4l: Document " Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 03/33] vivi: Add an integer menu test control Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 04/33] v4l: VIDIOC_SUBDEV_S_SELECTION and VIDIOC_SUBDEV_G_SELECTION IOCTLs Sakari Ailus
2012-02-21 14:34   ` Laurent Pinchart
2012-02-23  5:49     ` Sakari Ailus
2012-02-21 16:15   ` Laurent Pinchart
2012-02-23  6:01     ` Sakari Ailus
2012-02-27  0:22       ` Laurent Pinchart
2012-02-27  0:57         ` Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 05/33] v4l: vdev_to_v4l2_subdev() should have return type "struct v4l2_subdev *" Sakari Ailus
2012-02-21 14:37   ` Laurent Pinchart
2012-02-20  1:56 ` [PATCH v3 06/33] v4l: Check pad number in get try pointer functions Sakari Ailus
2012-02-21 14:42   ` Laurent Pinchart
2012-02-23  5:57     ` Sakari Ailus
2012-02-27  0:33       ` Laurent Pinchart
2012-02-27 12:27         ` Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 07/33] v4l: Support s_crop and g_crop through s/g_selection Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 08/33] v4l: Add subdev selections documentation: svg and dia files Sakari Ailus
2012-02-21 15:00   ` Laurent Pinchart
2012-02-26 18:56     ` Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 09/33] v4l: Add subdev selections documentation Sakari Ailus
2012-02-21 16:41   ` Laurent Pinchart
2012-02-26 21:42     ` Sakari Ailus
2012-02-28 11:42       ` Laurent Pinchart
2012-03-02 12:24         ` Sakari Ailus
2012-03-02 17:54           ` Laurent Pinchart
2012-03-02 18:01             ` Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 10/33] v4l: Mark VIDIOC_SUBDEV_G_CROP and VIDIOC_SUBDEV_S_CROP obsolete Sakari Ailus
2012-02-21 16:42   ` Laurent Pinchart
2012-02-20  1:56 ` [PATCH v3 11/33] v4l: Image source control class Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 12/33] v4l: Image processing " Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 13/33] v4l: Document raw bayer 4CC codes Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 14/33] v4l: Add DPCM compressed formats Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 15/33] media: Add link_validate() op to check links to the sink pad Sakari Ailus
2012-02-22 10:05   ` Laurent Pinchart
2012-02-23 15:04     ` Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 16/33] v4l: Improve sub-device documentation for pad ops Sakari Ailus
2012-02-22 10:06   ` Laurent Pinchart
2012-02-20  1:56 ` [PATCH v3 17/33] v4l: Implement v4l2_subdev_link_validate() Sakari Ailus
2012-02-22 10:14   ` Laurent Pinchart
2012-02-23 16:07     ` Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 18/33] v4l: Allow changing control handler lock Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 19/33] omap3isp: Support additional in-memory compressed bayer formats Sakari Ailus
2012-02-20  1:56 ` [PATCH v3 20/33] omap3isp: Move definitions required by board code under include/media Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 21/33] omap3: add definition for CONTROL_CAMERA_PHY_CTRL Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 22/33] omap3isp: Assume media_entity_pipeline_start may fail Sakari Ailus
2012-02-22 10:48   ` Laurent Pinchart
2012-02-26  1:08     ` Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 23/33] omap3isp: Add lane configuration to platform data Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 24/33] omap3isp: Add information on external subdev to struct isp_pipeline Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 25/33] omap3isp: Introduce omap3isp_get_external_info() Sakari Ailus
2012-02-22 10:55   ` Laurent Pinchart
2012-02-26  1:09     ` Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 26/33] omap3isp: Default link validation for ccp2, csi2, preview and resizer Sakari Ailus
2012-02-22 11:01   ` Laurent Pinchart [this message]
2012-02-25  1:34     ` Sakari Ailus
2012-02-26 23:14       ` Laurent Pinchart
2012-02-26 23:40         ` Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 27/33] omap3isp: Implement proper CCDC link validation, check pixel rate Sakari Ailus
2012-02-22 11:11   ` Laurent Pinchart
2012-02-25  1:42     ` Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 28/33] omap3isp: Move setting constaints above media_entity_pipeline_start Sakari Ailus
2012-02-22 11:12   ` Laurent Pinchart
2012-02-25  1:46     ` Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 29/33] omap3isp: Configure CSI-2 phy based on platform data Sakari Ailus
2012-02-22 11:21   ` Laurent Pinchart
2012-02-25  1:49     ` Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 30/33] omap3isp: Add resizer data rate configuration to resizer_set_stream Sakari Ailus
2012-02-22 11:24   ` Laurent Pinchart
2012-02-26  1:10     ` Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 31/33] omap3isp: Remove isp_validate_pipeline and other old stuff Sakari Ailus
2012-02-22 11:26   ` Laurent Pinchart
2012-02-25  1:52     ` Sakari Ailus
2012-02-20  1:57 ` [PATCH v3 32/33] smiapp: Add driver Sakari Ailus
2012-02-27 15:38   ` Laurent Pinchart
2012-02-29  5:41     ` Sakari Ailus
2012-02-29  9:35       ` Laurent Pinchart
2012-02-29 10:00         ` Sylwester Nawrocki
2012-03-01 14:01         ` Sakari Ailus
2012-03-01 14:56           ` Laurent Pinchart
2012-02-20  1:57 ` [PATCH v3 33/33] rm680: Add camera init Sakari Ailus
2012-02-27  1:06   ` Laurent Pinchart
2012-02-28 19:05     ` Sakari Ailus
2012-02-20  2:03 ` [PATCH v3 0/33] V4L2 subdev and sensor control changes, SMIA++ driver and N9 camera board code Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4620159.TXeRQHhZdd@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=dacohen@gmail.com \
    --cc=hverkuil@xs4all.nl \
    --cc=k.debski@gmail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=riverful@gmail.com \
    --cc=sakari.ailus@iki.fi \
    --cc=snjw23@gmail.com \
    --cc=t.stanislaws@samsung.com \
    --cc=teturtia@gmail.com \
    --cc=tuukkat76@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox