Message ID | 1324412889-17961-9-git-send-email-sakari.ailus@maxwell.research.nokia.com (mailing list archive) |
---|---|
State | RFC, archived |
Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from <linux-media-owner@vger.kernel.org>) id 1Rd6Ii-000359-2Y; Tue, 20 Dec 2011 21:28:40 +0100 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-3) with esmtp id 1Rd6Ih-0003pa-Ea; Tue, 20 Dec 2011 21:28:39 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752970Ab1LTU2f (ORCPT <rfc822;patchwork@linuxtv.org> + 3 others); Tue, 20 Dec 2011 15:28:35 -0500 Received: from smtp.nokia.com ([147.243.1.48]:35787 "EHLO mgw-sa02.nokia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752849Ab1LTU2P (ORCPT <rfc822;linux-media@vger.kernel.org>); Tue, 20 Dec 2011 15:28:15 -0500 Received: from maxwell.research.nokia.com (maxwell.research.nokia.com [172.21.50.162]) by mgw-sa02.nokia.com (Switch-3.4.4/Switch-3.4.4) with ESMTP id pBKKSBdA012955 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 20 Dec 2011 22:28:12 +0200 Received: from lanttu (unknown [192.168.239.74]) by maxwell.research.nokia.com (Postfix) with ESMTPS id DFC5438663A; Tue, 20 Dec 2011 22:28:11 +0200 (EET) Received: from sakke by lanttu with local (Exim 4.72) (envelope-from <sakari.ailus@maxwell.research.nokia.com>) id 1Rd6IH-0004jA-Ox; Tue, 20 Dec 2011 22:28:13 +0200 From: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com> To: linux-media@vger.kernel.org Cc: laurent.pinchart@ideasonboard.com, dacohen@gmail.com, snjw23@gmail.com Subject: [RFC 09/17] v4l: Add pad op for pipeline validation Date: Tue, 20 Dec 2011 22:28:01 +0200 Message-Id: <1324412889-17961-9-git-send-email-sakari.ailus@maxwell.research.nokia.com> X-Mailer: git-send-email 1.7.2.5 In-Reply-To: <4EF0EFC9.6080501@maxwell.research.nokia.com> References: <4EF0EFC9.6080501@maxwell.research.nokia.com> X-Nokia-AV: Clean Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2011.12.20.201815 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1100_1199 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, __ANY_URI 0, __CP_URI_IN_BODY 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MULTIPLE_RCPTS_CC_X2 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS ' X-LSpam-Score: -1.9 (-) X-LSpam-Report: No, score=-1.9 required=5.0 tests=BAYES_00=-1.9 autolearn=ham |
Commit Message
Sakari Ailus
Dec. 20, 2011, 8:28 p.m. UTC
From: Sakari Ailus <sakari.ailus@iki.fi> smiapp_pad_ops.validate_pipeline is intended to validate the full pipeline which is implemented by the driver to which the subdev implementing this op belongs to. The validate_pipeline op must also call validate_pipeline on any external entity which is linked to its sink pads. Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> --- include/media/v4l2-subdev.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
Comments
Hi Sakari, Thanks for the patch. On Tuesday 20 December 2011 21:28:01 Sakari Ailus wrote: > From: Sakari Ailus <sakari.ailus@iki.fi> > > smiapp_pad_ops.validate_pipeline is intended to validate the full pipeline > which is implemented by the driver to which the subdev implementing this op > belongs to. Should this be documented in Documentation/video4linux/v4l2-framework.txt ? > The validate_pipeline op must also call validate_pipeline on any external > entity which is linked to its sink pads. I'm uncertain about this. Subdev drivers would then have to implement the pipeline walk logic, resulting in duplicate code. Our current situation isn't optimal either: walking the pipeline should be implemented in the media code. Would it suit your needs if the validate_pipeline operation was replaced by a validate_link operation, with a media_pipeline_validate() function in the media core to walk the pipeline and call validate_link in each pad (or maybe each sink pad) ? > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > --- > include/media/v4l2-subdev.h | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 26eeaa4..a5ebe86 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -470,6 +470,7 @@ struct v4l2_subdev_pad_ops { > struct v4l2_subdev_selection *sel); > int (*set_selection)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, > struct v4l2_subdev_selection *sel); > + int (*validate_pipeline)(struct v4l2_subdev *sd); > }; > > struct v4l2_subdev_ops {
Hi Laurent, Thanks for the review!! Laurent Pinchart wrote: > On Tuesday 20 December 2011 21:28:01 Sakari Ailus wrote: >> From: Sakari Ailus <sakari.ailus@iki.fi> >> >> smiapp_pad_ops.validate_pipeline is intended to validate the full pipeline >> which is implemented by the driver to which the subdev implementing this op >> belongs to. > > Should this be documented in Documentation/video4linux/v4l2-framework.txt ? > >> The validate_pipeline op must also call validate_pipeline on any external >> entity which is linked to its sink pads. > > I'm uncertain about this. Subdev drivers would then have to implement the > pipeline walk logic, resulting in duplicate code. Our current situation isn't > optimal either: walking the pipeline should be implemented in the media code. > Would it suit your needs if the validate_pipeline operation was replaced by a > validate_link operation, with a media_pipeline_validate() function in the > media core to walk the pipeline and call validate_link in each pad (or maybe > each sink pad) ? Albeit I don't see the pipeline checking a big problem in the subdev drivers, that does seem like a viable alternative --- it's definitely more generic. Any benefit of that is directly bound to the existence of generic pipeline validation function, which definitely shouldn't be too difficult to write. It is also true that in practice, I assume, considering the pipeline validation inside subdev drivers, the SMIA++ driver is almost as complex any sensor driver will get in the near future. But once a practice has been established it's difficult to change that. I'm for validate_link() op. I assume that in the implementation the Media controller framework would walk the pipeline and call entity specific link_validate() ops. We already have link_setup() op there. Those would, in turn, check that the link requirements are fulfilled. How does that sound like to you? Regards,
Hi Sakari, On Sunday 08 January 2012 00:28:40 Sakari Ailus wrote: > Laurent Pinchart wrote: > > On Tuesday 20 December 2011 21:28:01 Sakari Ailus wrote: > >> From: Sakari Ailus <sakari.ailus@iki.fi> > >> > >> smiapp_pad_ops.validate_pipeline is intended to validate the full > >> pipeline which is implemented by the driver to which the subdev > >> implementing this op belongs to. > > > > Should this be documented in Documentation/video4linux/v4l2-framework.txt > > ? > > > >> The validate_pipeline op must also call validate_pipeline on any > >> external entity which is linked to its sink pads. > > > > I'm uncertain about this. Subdev drivers would then have to implement the > > pipeline walk logic, resulting in duplicate code. Our current situation > > isn't optimal either: walking the pipeline should be implemented in the > > media code. Would it suit your needs if the validate_pipeline operation > > was replaced by a validate_link operation, with a > > media_pipeline_validate() function in the media core to walk the > > pipeline and call validate_link in each pad (or maybe each sink pad) ? > > Albeit I don't see the pipeline checking a big problem in the subdev > drivers, that does seem like a viable alternative --- it's definitely > more generic. Any benefit of that is directly bound to the existence of > generic pipeline validation function, which definitely shouldn't be too > difficult to write. > > It is also true that in practice, I assume, considering the pipeline > validation inside subdev drivers, the SMIA++ driver is almost as complex > any sensor driver will get in the near future. But once a practice has > been established it's difficult to change that. > > I'm for validate_link() op. > > I assume that in the implementation the Media controller framework would > walk the pipeline and call entity specific link_validate() ops. We > already have link_setup() op there. Those would, in turn, check that the > link requirements are fulfilled. How does that sound like to you? That sounds good to me. If the operation isn't implemented, the core should just make sure that code, width and height are identical on the two ends of the link. That will be the usual requirement, checking it in the core will make drivers simpler.
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 26eeaa4..a5ebe86 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -470,6 +470,7 @@ struct v4l2_subdev_pad_ops { struct v4l2_subdev_selection *sel); int (*set_selection)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh, struct v4l2_subdev_selection *sel); + int (*validate_pipeline)(struct v4l2_subdev *sd); }; struct v4l2_subdev_ops {