[{"id":335,"web_url":"https://patchwork.libcamera.org/comment/335/","msgid":"<20190115203824.GA7393@bigcity.dyn.berto.se>","date":"2019-01-15T20:38:24","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your patch.\n\nOn 2019-01-11 17:51:09 +0100, Jacopo Mondi wrote:\n> Add a pipeline handler skeleton for the Intel IPU3 device.\n> Tested with on Soraka, listing detected cameras on the system and\n> verifying the pipeline handler gets properly matched.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n> \n> Let's start by simply matching the pipeline handler with the device\n> it is running on. The here created single camera gets properly enumerated\n> on Soraka by the 'list-cameras' test:\n> \n> ./test/list-cameras\n> [0:35:35.453249952]   DBG pipeline_handler.cpp:119 Pipeline handler: \"PipeHandlerVimc\" registered\n> [0:35:35.453538626]   DBG pipeline_handler.cpp:119 Pipeline handler: \"PipelineHandlerIPU3\" registered\n> [0:35:35.458316459]   DBG device_enumerator.cpp:214 New media device: ipu3-imgu created from: /dev/media1\n> [0:35:35.469071318]   DBG device_enumerator.cpp:214 New media device: ipu3-cio2 created from: /dev/media0\n> [0:35:35.475305874]   DBG device_enumerator.cpp:255 Succesfull match for media device: ipu3-cio2\n> [0:35:35.475343991]   DBG device_enumerator.cpp:255 Succesfull match for media device: ipu3-imgu\n> [0:35:35.475354057]   DBG pipeline_handler.cpp:150 Pipeline handler: \"PipelineHandlerIPU3\" matched\n> - IPU3 Camera\n> \n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp    | 119 ++++++++++++++++++++++++\n>  src/libcamera/pipeline/ipu3/meson.build |   3 +\n>  src/libcamera/pipeline/meson.build      |   2 +\n>  3 files changed, 124 insertions(+)\n>  create mode 100644 src/libcamera/pipeline/ipu3/ipu3.cpp\n>  create mode 100644 src/libcamera/pipeline/ipu3/meson.build\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> new file mode 100644\n> index 0000000..477a9a2\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -0,0 +1,119 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipu3.cpp - Pipeline handler for Intel IPU3\n> + */\n> +\n> +#include <libcamera/camera.h>\n> +\n> +#include \"device_enumerator.h\"\n> +#include \"media_device.h\"\n> +#include \"pipeline_handler.h\"\n> +\n> +#include \"log.h\"\n> +\n> +namespace libcamera {\n> +\n> +class PipelineHandlerIPU3 : public PipelineHandler\n> +{\n> +public:\n> +\tPipelineHandlerIPU3();\n> +\t~PipelineHandlerIPU3();\n> +\n> +\tbool match(DeviceEnumerator *enumerator);\n> +\n> +\tunsigned int count();\n> +\tCamera *camera(unsigned int id) final;\n> +\n> +private:\n> +\tMediaDevice *cio2_;\n> +\tMediaDevice *imgu_;\n> +\n> +\tCamera *camera_;\n> +};\n> +\n> +PipelineHandlerIPU3::PipelineHandlerIPU3()\n> +\t: cio2_(nullptr), imgu_(nullptr), camera_(nullptr)\n> +{\n> +}\n> +\n> +PipelineHandlerIPU3::~PipelineHandlerIPU3()\n> +{\n> +\tif (cio2_)\n> +\t\tcio2_->release();\n> +\tif (imgu_)\n> +\t\timgu_->release();\n> +\tif (camera_)\n> +\t\tcamera_->put();\n> +\n> +\tcio2_ = nullptr;\n> +\timgu_ = nullptr;\n> +\tcamera_ = nullptr;\n> +}\n> +\n> +unsigned int PipelineHandlerIPU3::count()\n> +{\n> +\treturn 1;\n> +}\n> +\n> +Camera *PipelineHandlerIPU3::camera(unsigned int id)\n> +{\n> +\tif (id != 0)\n> +\t\treturn nullptr;\n> +\n> +\treturn camera_;\n> +}\n> +\n> +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> +{\n> +\tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> +\tcio2_dm.add(\"ipu3-csi2 0\");\n> +\tcio2_dm.add(\"ipu3-cio2 0\");\n> +\tcio2_dm.add(\"ipu3-csi2 1\");\n> +\tcio2_dm.add(\"ipu3-cio2 1\");\n> +\tcio2_dm.add(\"ipu3-csi2 2\");\n> +\tcio2_dm.add(\"ipu3-cio2 2\");\n> +\tcio2_dm.add(\"ipu3-csi2 3\");\n> +\tcio2_dm.add(\"ipu3-cio2 3\");\n> +\n> +\tcio2_ = enumerator->search(cio2_dm);\n> +\tif (!cio2_)\n> +\t\treturn false;\n> +\n> +\tcio2_->acquire();\n> +\n> +\tDeviceMatch imgu_dm(\"ipu3-imgu\");\n> +\timgu_dm.add(\"ipu3-imgu 0\");\n> +\timgu_dm.add(\"ipu3-imgu 0 input\");\n> +\timgu_dm.add(\"ipu3-imgu 0 parameters\");\n> +\timgu_dm.add(\"ipu3-imgu 0 output\");\n> +\timgu_dm.add(\"ipu3-imgu 0 viewfinder\");\n> +\timgu_dm.add(\"ipu3-imgu 0 3a stat\");\n> +\timgu_dm.add(\"ipu3-imgu 1\");\n> +\timgu_dm.add(\"ipu3-imgu 1 input\");\n> +\timgu_dm.add(\"ipu3-imgu 1 parameters\");\n> +\timgu_dm.add(\"ipu3-imgu 1 output\");\n> +\timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n> +\timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n> +\n> +\timgu_ = enumerator->search(imgu_dm);\n> +\tif (!imgu_) {\n> +\t\tcio2_->release();\n> +\t\treturn false;\n> +\t}\n> +\n> +\timgu_->acquire();\n\nI would reorder this a bit.\n\n    ...\n\n    cio2_ = enumerator->search(cio2_dm);\n    if (!cio2_)\n        return false;\n\n    imgu_ = enumerator->search(imgu_dm);\n    if (!imgu_)\n        return false;\n\n    cio2_->acquire();\n    imgu_->acquire();\n\n    ...\n\nI don't feel strongly about this so if others have a different view I \nwill yield to public opinion :-)\n\n> +\n> +\t/*\n> +\t * TODO: create cameras. As of now, just create a dummy one\n> +\t * to verify enumeration and matching on IPU3.\n> +\t */\n> +\tcamera_ = new Camera(\"IPU3 Camera\");\n> +\n> +\treturn true;\n> +}\n> +\n> +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build\n> new file mode 100644\n> index 0000000..0ab766a\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/ipu3/meson.build\n> @@ -0,0 +1,3 @@\n> +libcamera_sources += files([\n> +    'ipu3.cpp',\n> +])\n> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build\n> index 615ecd2..811c075 100644\n> --- a/src/libcamera/pipeline/meson.build\n> +++ b/src/libcamera/pipeline/meson.build\n> @@ -1,3 +1,5 @@\n>  libcamera_sources += files([\n>      'vimc.cpp',\n>  ])\n> +\n> +subdir('ipu3')\n> --\n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E0E3060C96\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Jan 2019 21:38:27 +0100 (CET)","by mail-lf1-x141.google.com with SMTP id a8so3123647lfk.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Jan 2019 12:38:27 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tk21-v6sm721354ljc.15.2019.01.15.12.38.24\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 15 Jan 2019 12:38:25 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=36h4n8LbmFRofNpLA+PCVDyxzHKlk2CgT6Jw3dmR7dw=;\n\tb=KFmP0jNAeHnAdskMIWVKhXDR/ivjvXULQIprRRzy4Kp/yEhAfzPwDO8UUU9huQr2hG\n\t4leJjrpbi0o6VpQahvH+D5350RgYi0JvyNV3gGkZzxSy/66TyTxNrBRHhxMvyqpdg1ft\n\t+JfGhHmP57PVZvD+HinoIH5TNoeYQLQHUhKKmxBvQpDA3l+ir1IDrGpjP5z7AIcMhRL/\n\tvWD6b22NUh37+P6L6HcSHvruqNlFHV3iIPWgnFH9VuZ5mhmDwNjYri8Qo7ROodo3W/Zs\n\tV8YUdDRRPyOXo4XQY7z5MziC6NaxZEwUxVD/Lv+yINppPp64Yqq9+D43N+iQ0/axlDd8\n\tNmHQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=36h4n8LbmFRofNpLA+PCVDyxzHKlk2CgT6Jw3dmR7dw=;\n\tb=tmDCx+cCLLWXeXphFnBVeMPKvY7/aU+uBeS1liIvpSFhYf+5O4mocobh+GlOKqbhXR\n\twZqrr8EqaA+BbsApnPDndncg77jhnHWIPfk6eK6mZM9KU/vk21FqLgca4WQvyY5VS3am\n\ts/o69WvN0+wfBTy66pWbTyaaqgpRaVtg6JpLKRs/+jlgncJEX/RESx6kdk2y1XftjM41\n\tNdgeddY+qMz87bcglw4P6Yk+YOP7Q5QcKf0LiZCUv93a7XfDfzZvUmUg1N7XeQvN5w+H\n\taUyyAP/zi0acdl4arnJRn1Kbx5BrwnS7MNGccZJGGEKGNScQj1iCUDw6ZigAoJi4bSVr\n\t24bQ==","X-Gm-Message-State":"AJcUukeYqPB4ic5FLnh7aiTCvEsDLs9HqaEM119EQg/0pfDb6dP/ecLq\n\tcHqRzxaPOKrQTW8OPxslmxpGuA==","X-Google-Smtp-Source":"ALg8bN4jvHDYz0swCwudvQWxodXps9vMXVOmxTo7zNMZimu7Uvb701l7r9kes1Z9f+FKeSYiZfYapA==","X-Received":"by 2002:a19:d486:: with SMTP id\n\tl128mr4054888lfg.114.1547584706939; \n\tTue, 15 Jan 2019 12:38:26 -0800 (PST)","Date":"Tue, 15 Jan 2019 21:38:24 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190115203824.GA7393@bigcity.dyn.berto.se>","References":"<20190111165109.26803-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190111165109.26803-1-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 15 Jan 2019 20:38:28 -0000"}},{"id":338,"web_url":"https://patchwork.libcamera.org/comment/338/","msgid":"<20190115212507.GA28045@pendragon.ideasonboard.com>","date":"2019-01-15T21:25:07","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Tue, Jan 15, 2019 at 09:38:24PM +0100, Niklas Söderlund wrote:\n> On 2019-01-11 17:51:09 +0100, Jacopo Mondi wrote:\n> > Add a pipeline handler skeleton for the Intel IPU3 device.\n> > Tested with on Soraka, listing detected cameras on the system and\n> > verifying the pipeline handler gets properly matched.\n> > \n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> > \n> > Let's start by simply matching the pipeline handler with the device\n> > it is running on. The here created single camera gets properly enumerated\n> > on Soraka by the 'list-cameras' test:\n> > \n> > ./test/list-cameras\n> > [0:35:35.453249952]   DBG pipeline_handler.cpp:119 Pipeline handler: \"PipeHandlerVimc\" registered\n> > [0:35:35.453538626]   DBG pipeline_handler.cpp:119 Pipeline handler: \"PipelineHandlerIPU3\" registered\n> > [0:35:35.458316459]   DBG device_enumerator.cpp:214 New media device: ipu3-imgu created from: /dev/media1\n> > [0:35:35.469071318]   DBG device_enumerator.cpp:214 New media device: ipu3-cio2 created from: /dev/media0\n> > [0:35:35.475305874]   DBG device_enumerator.cpp:255 Succesfull match for media device: ipu3-cio2\n> > [0:35:35.475343991]   DBG device_enumerator.cpp:255 Succesfull match for media device: ipu3-imgu\n> > [0:35:35.475354057]   DBG pipeline_handler.cpp:150 Pipeline handler: \"PipelineHandlerIPU3\" matched\n> > - IPU3 Camera\n> > \n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp    | 119 ++++++++++++++++++++++++\n> >  src/libcamera/pipeline/ipu3/meson.build |   3 +\n> >  src/libcamera/pipeline/meson.build      |   2 +\n> >  3 files changed, 124 insertions(+)\n> >  create mode 100644 src/libcamera/pipeline/ipu3/ipu3.cpp\n> >  create mode 100644 src/libcamera/pipeline/ipu3/meson.build\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > new file mode 100644\n> > index 0000000..477a9a2\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n\n[snip]\n\n> > +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > +{\n> > +\tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> > +\tcio2_dm.add(\"ipu3-csi2 0\");\n> > +\tcio2_dm.add(\"ipu3-cio2 0\");\n> > +\tcio2_dm.add(\"ipu3-csi2 1\");\n> > +\tcio2_dm.add(\"ipu3-cio2 1\");\n> > +\tcio2_dm.add(\"ipu3-csi2 2\");\n> > +\tcio2_dm.add(\"ipu3-cio2 2\");\n> > +\tcio2_dm.add(\"ipu3-csi2 3\");\n> > +\tcio2_dm.add(\"ipu3-cio2 3\");\n> > +\n> > +\tcio2_ = enumerator->search(cio2_dm);\n> > +\tif (!cio2_)\n> > +\t\treturn false;\n> > +\n> > +\tcio2_->acquire();\n> > +\n> > +\tDeviceMatch imgu_dm(\"ipu3-imgu\");\n> > +\timgu_dm.add(\"ipu3-imgu 0\");\n> > +\timgu_dm.add(\"ipu3-imgu 0 input\");\n> > +\timgu_dm.add(\"ipu3-imgu 0 parameters\");\n> > +\timgu_dm.add(\"ipu3-imgu 0 output\");\n> > +\timgu_dm.add(\"ipu3-imgu 0 viewfinder\");\n> > +\timgu_dm.add(\"ipu3-imgu 0 3a stat\");\n> > +\timgu_dm.add(\"ipu3-imgu 1\");\n> > +\timgu_dm.add(\"ipu3-imgu 1 input\");\n> > +\timgu_dm.add(\"ipu3-imgu 1 parameters\");\n> > +\timgu_dm.add(\"ipu3-imgu 1 output\");\n> > +\timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n> > +\timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n> > +\n> > +\timgu_ = enumerator->search(imgu_dm);\n> > +\tif (!imgu_) {\n> > +\t\tcio2_->release();\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\timgu_->acquire();\n> \n> I would reorder this a bit.\n> \n>     ...\n> \n>     cio2_ = enumerator->search(cio2_dm);\n>     if (!cio2_)\n>         return false;\n> \n>     imgu_ = enumerator->search(imgu_dm);\n>     if (!imgu_)\n>         return false;\n> \n>     cio2_->acquire();\n>     imgu_->acquire();\n> \n>     ...\n> \n> I don't feel strongly about this so if others have a different view I \n> will yield to public opinion :-)\n\nI was about to mention the same, so I can only agree :-)\n\nFurthermore, I think we can match on driver name only in this case,\ncan't we ?\n\n> > +\n> > +\t/*\n> > +\t * TODO: create cameras. As of now, just create a dummy one\n> > +\t * to verify enumeration and matching on IPU3.\n> > +\t */\n> > +\tcamera_ = new Camera(\"IPU3 Camera\");\n> > +\n> > +\treturn true;\n> > +}\n> > +\n> > +REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);\n> > +\n> > +} /* namespace libcamera */\n\n[snip]","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4538460C6A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Jan 2019 22:25:07 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 87804530;\n\tTue, 15 Jan 2019 22:25:06 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547587506;\n\tbh=ZOMVj3ZndhrrXUrSaBvrIFmrGTHBsInxVeMTsaXJJEU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RVhYPkfbywrhvi/VDOKRr8f09FjEeZ/kmvk4xt58ugqjPvNhZvmVF5vHl5HZf3U9H\n\tDN/+Yy5jrqnNcYhk+fOqAVVWBzIadqJ00mU+lApoJZ9Zmvs2RhOgQao6E45ML4f+Aa\n\tui3JdTBF37laBwM5GnRvCUaSAAVOuVJ9R9sbVQzo=","Date":"Tue, 15 Jan 2019 23:25:07 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190115212507.GA28045@pendragon.ideasonboard.com>","References":"<20190111165109.26803-1-jacopo@jmondi.org>\n\t<20190115203824.GA7393@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190115203824.GA7393@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 15 Jan 2019 21:25:07 -0000"}},{"id":341,"web_url":"https://patchwork.libcamera.org/comment/341/","msgid":"<20190115214349.GC7393@bigcity.dyn.berto.se>","date":"2019-01-15T21:43:49","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi,\n\nFirst off, sorry Jacopo I now see there is a later version of this patch \nposted. I should have commented on that one.\n\nOn 2019-01-15 23:25:07 +0200, Laurent Pinchart wrote:\n\n[snip]\n\n> > > +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > > +{\n> > > +\tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> > > +\tcio2_dm.add(\"ipu3-csi2 0\");\n> > > +\tcio2_dm.add(\"ipu3-cio2 0\");\n> > > +\tcio2_dm.add(\"ipu3-csi2 1\");\n> > > +\tcio2_dm.add(\"ipu3-cio2 1\");\n> > > +\tcio2_dm.add(\"ipu3-csi2 2\");\n> > > +\tcio2_dm.add(\"ipu3-cio2 2\");\n> > > +\tcio2_dm.add(\"ipu3-csi2 3\");\n> > > +\tcio2_dm.add(\"ipu3-cio2 3\");\n> > > +\n> > > +\tcio2_ = enumerator->search(cio2_dm);\n> > > +\tif (!cio2_)\n> > > +\t\treturn false;\n> > > +\n> > > +\tcio2_->acquire();\n> > > +\n> > > +\tDeviceMatch imgu_dm(\"ipu3-imgu\");\n> > > +\timgu_dm.add(\"ipu3-imgu 0\");\n> > > +\timgu_dm.add(\"ipu3-imgu 0 input\");\n> > > +\timgu_dm.add(\"ipu3-imgu 0 parameters\");\n> > > +\timgu_dm.add(\"ipu3-imgu 0 output\");\n> > > +\timgu_dm.add(\"ipu3-imgu 0 viewfinder\");\n> > > +\timgu_dm.add(\"ipu3-imgu 0 3a stat\");\n> > > +\timgu_dm.add(\"ipu3-imgu 1\");\n> > > +\timgu_dm.add(\"ipu3-imgu 1 input\");\n> > > +\timgu_dm.add(\"ipu3-imgu 1 parameters\");\n> > > +\timgu_dm.add(\"ipu3-imgu 1 output\");\n> > > +\timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n> > > +\timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n> > > +\n> > > +\timgu_ = enumerator->search(imgu_dm);\n> > > +\tif (!imgu_) {\n> > > +\t\tcio2_->release();\n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\timgu_->acquire();\n> > \n> > I would reorder this a bit.\n> > \n> >     ...\n> > \n> >     cio2_ = enumerator->search(cio2_dm);\n> >     if (!cio2_)\n> >         return false;\n> > \n> >     imgu_ = enumerator->search(imgu_dm);\n> >     if (!imgu_)\n> >         return false;\n> > \n> >     cio2_->acquire();\n> >     imgu_->acquire();\n> > \n> >     ...\n> > \n> > I don't feel strongly about this so if others have a different view I \n> > will yield to public opinion :-)\n> \n> I was about to mention the same, so I can only agree :-)\n> \n> Furthermore, I think we can match on driver name only in this case,\n> can't we ?\n\nI think we need to design some sort of guide lines here. It's true we \ncould match on driver name only here. By doing so we no longer ensures \nthat the media devices have all the entities we expect it to have, and \nmay therefor accept a media device which is\n\n1.  From a different kernel version/patch series.\n2.  Not probed completely.\n3.  Expresses a variation of hardware with more or less capabilities \n    then the one the pipeline handler was developed for.\n\nReason 1 should not be a real issue, but API breakages do happen... This \ncould then be solved with a check of the running kernel version which \nthe matching logic could be expanded to cover. I'm thinking here of UVC \nwhere a lot of the entity names are not static and we would need to \ndepend on the driver name alone.\n\nReason 2 is valid and a issue discussed at some V4L2 conference and is \nthe main blocker for video devices can't be register at probe time. Once \nthe kernel have a way to express that a media graph is complete we \nshould use that in libcamera.\n\nReason 3 is a real problem as we don't know what variations of a \nhardware block will be released in a slightly different form in the \nfuture which a driver would be extended to support.","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8215C60C6A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Jan 2019 22:43:51 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id l10so3233875lfh.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 15 Jan 2019 13:43:51 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\ts24sm815162lfc.30.2019.01.15.13.43.50\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 15 Jan 2019 13:43:50 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=ZBo/FUxf5gQTtqU5G9ea3nvPzkuHSzUs08KtjY9Sjic=;\n\tb=yvbRR/bc5eORjqnk6yAWNuWYe0dBFTXsAjG8JyIqCl5GZg0dymmz1b6Pr+b65cYENi\n\tl6/u+aQK6bY+GO5gIYq40NZdmDmoglsPNHP4UcSFr1Qr8LtdPUBr5W8QeCEw/aAFv2xj\n\tfgY7he1tawwrj3QVMU2+LC1R5fnH4DdSQiyTw7QGrzB+EDNE3HUGsz9ZJygZk5J7ngq3\n\tzDZsYt6AR5k7ddPY3gscv3eq3BIy4DMew2bSXQnLRtzmO4SFm4psbNys781wv3Rbznrw\n\tcli9+fupT10ooCTUDb2x4Wfd1sYi+WtCOVUKNMGyJZZejraXmCvNvb+yNxAKHqdyOJYb\n\tCUTw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=ZBo/FUxf5gQTtqU5G9ea3nvPzkuHSzUs08KtjY9Sjic=;\n\tb=EcBNGZErCfczyS6QjWXbF9ECNfp/NGxwpiYncT4ijmyuwm8hkZrxVTh3psHcm1NXS6\n\t0KglxUVGcWcsRLowTOJdPJJh6G6sCRxekdhiTZf8MMLDnv5VA4ZofHyDRjvr8LjEVxmX\n\tjNd5vD9JpSagM6T2xk8Ec4SuDtS17cVKDgxFnPxg9ORZIfWhwglH5bG0SQh5aDDJUWmV\n\tAHjHESxtTSfkyEmgpdVtAhc+OzihwIW7jNUgx9pw3rwwIx3psSMmI2u3MnqmHansjttD\n\tZh8kjHAg2dKpUT9YmOS/tBtLr+nMUHiV6wg8Tlcek6zC6+57HecbI3h7onK80NJoRWJN\n\tht4w==","X-Gm-Message-State":"AJcUuke8s/QfMYITv9IRoXCA3oK8LJfCGhhCdMnpbN0of+Vdb1upqmHF\n\tiR3C+1IynG9AqM4D+hpb1LvZ6fDb6Pk=","X-Google-Smtp-Source":"ALg8bN7pnhxxg7GTZJBL5eD6G3Vn6ECqwz4PROTYMVX2H9bGlnmX02nGZheI5VuPiGITNKtc9w7l3A==","X-Received":"by 2002:a19:d5:: with SMTP id 204mr4149452lfa.116.1547588630791; \n\tTue, 15 Jan 2019 13:43:50 -0800 (PST)","Date":"Tue, 15 Jan 2019 22:43:49 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190115214349.GC7393@bigcity.dyn.berto.se>","References":"<20190111165109.26803-1-jacopo@jmondi.org>\n\t<20190115203824.GA7393@bigcity.dyn.berto.se>\n\t<20190115212507.GA28045@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190115212507.GA28045@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 15 Jan 2019 21:43:51 -0000"}},{"id":372,"web_url":"https://patchwork.libcamera.org/comment/372/","msgid":"<20190116154230.GB20097@pendragon.ideasonboard.com>","date":"2019-01-16T15:42:30","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Tue, Jan 15, 2019 at 10:43:49PM +0100, Niklas Söderlund wrote:\n> Hi,\n> \n> First off, sorry Jacopo I now see there is a later version of this patch \n> posted. I should have commented on that one.\n> \n> On 2019-01-15 23:25:07 +0200, Laurent Pinchart wrote:\n> \n> [snip]\n> \n> >>> +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> >>> +{\n> >>> +\tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> >>> +\tcio2_dm.add(\"ipu3-csi2 0\");\n> >>> +\tcio2_dm.add(\"ipu3-cio2 0\");\n> >>> +\tcio2_dm.add(\"ipu3-csi2 1\");\n> >>> +\tcio2_dm.add(\"ipu3-cio2 1\");\n> >>> +\tcio2_dm.add(\"ipu3-csi2 2\");\n> >>> +\tcio2_dm.add(\"ipu3-cio2 2\");\n> >>> +\tcio2_dm.add(\"ipu3-csi2 3\");\n> >>> +\tcio2_dm.add(\"ipu3-cio2 3\");\n> >>> +\n> >>> +\tcio2_ = enumerator->search(cio2_dm);\n> >>> +\tif (!cio2_)\n> >>> +\t\treturn false;\n> >>> +\n> >>> +\tcio2_->acquire();\n> >>> +\n> >>> +\tDeviceMatch imgu_dm(\"ipu3-imgu\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 0\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 0 input\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 0 parameters\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 0 output\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 0 viewfinder\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 0 3a stat\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 1\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 1 input\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 1 parameters\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 1 output\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n> >>> +\timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n> >>> +\n> >>> +\timgu_ = enumerator->search(imgu_dm);\n> >>> +\tif (!imgu_) {\n> >>> +\t\tcio2_->release();\n> >>> +\t\treturn false;\n> >>> +\t}\n> >>> +\n> >>> +\timgu_->acquire();\n> >> \n> >> I would reorder this a bit.\n> >> \n> >>     ...\n> >> \n> >>     cio2_ = enumerator->search(cio2_dm);\n> >>     if (!cio2_)\n> >>         return false;\n> >> \n> >>     imgu_ = enumerator->search(imgu_dm);\n> >>     if (!imgu_)\n> >>         return false;\n> >> \n> >>     cio2_->acquire();\n> >>     imgu_->acquire();\n> >> \n> >>     ...\n> >> \n> >> I don't feel strongly about this so if others have a different view I \n> >> will yield to public opinion :-)\n> > \n> > I was about to mention the same, so I can only agree :-)\n> > \n> > Furthermore, I think we can match on driver name only in this case,\n> > can't we ?\n> \n> I think we need to design some sort of guide lines here. It's true we \n> could match on driver name only here. By doing so we no longer ensures \n> that the media devices have all the entities we expect it to have, and \n> may therefor accept a media device which is\n> \n> 1.  From a different kernel version/patch series.\n> 2.  Not probed completely.\n> 3.  Expresses a variation of hardware with more or less capabilities \n>     then the one the pipeline handler was developed for.\n> \n> Reason 1 should not be a real issue, but API breakages do happen... This \n> could then be solved with a check of the running kernel version which \n> the matching logic could be expanded to cover. I'm thinking here of UVC \n> where a lot of the entity names are not static and we would need to \n> depend on the driver name alone.\n\nI think we should add a kernel version (or rather media device version)\ncheck in the DeviceMatch in any case, it can be useful.\n\n> Reason 2 is valid and a issue discussed at some V4L2 conference and is \n> the main blocker for video devices can't be register at probe time. Once \n> the kernel have a way to express that a media graph is complete we \n> should use that in libcamera.\n\nIt is valid, but not for all devices. For the ImgU for instance, we have\nno external entity, so it shouldn't be an issue.\n\nFurthermore, for the CIO2, we indeed want to make sure all entities are\nsuccessfully probed, but we don't know what sensor are present in the\nsystem, so the only entities we can match on anyway are internal to the\nCIO2, which seems a bit pointless to me.\n\n> Reason 3 is a real problem as we don't know what variations of a \n> hardware block will be released in a slightly different form in the \n> future which a driver would be extended to support.\n\nSame as for reason 2, this is valid, but not for all devices.\n\nIn the IPU3 case I believe matching on entities would only be useful to\nhandle the first issue you pointed out, and I wonder if we need to do so\nnow.","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0747060B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 16 Jan 2019 16:42:30 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 630A14F8;\n\tWed, 16 Jan 2019 16:42:29 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547653349;\n\tbh=Qv2SlcOXzO2vo7hPre5GtU5K0hS4+cUmfy4nD3HfQns=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GXZia/T8wzguvx0hg26rZ9ENc85n3J10I3ZM4uUYKmYoLb6fivJuCMeQ4pElLvXys\n\tjYYAd3sC3VpdgsdHwlN8nNTfrtzvNgKX/HPjfvoNWEIeEYYo4fSfsp6wEd+zbFvvOL\n\tbta8RQnK6YuZSkVCkUlLq3ffVDc4nNnHQhoRlUtM=","Date":"Wed, 16 Jan 2019 17:42:30 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190116154230.GB20097@pendragon.ideasonboard.com>","References":"<20190111165109.26803-1-jacopo@jmondi.org>\n\t<20190115203824.GA7393@bigcity.dyn.berto.se>\n\t<20190115212507.GA28045@pendragon.ideasonboard.com>\n\t<20190115214349.GC7393@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190115214349.GC7393@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 16 Jan 2019 15:42:30 -0000"}},{"id":375,"web_url":"https://patchwork.libcamera.org/comment/375/","msgid":"<20190116155358.GJ6484@bigcity.dyn.berto.se>","date":"2019-01-16T15:53:58","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"HI Laurent,\n\nOn 2019-01-16 17:42:30 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Tue, Jan 15, 2019 at 10:43:49PM +0100, Niklas Söderlund wrote:\n> > Hi,\n> > \n> > First off, sorry Jacopo I now see there is a later version of this patch \n> > posted. I should have commented on that one.\n> > \n> > On 2019-01-15 23:25:07 +0200, Laurent Pinchart wrote:\n> > \n> > [snip]\n> > \n> > >>> +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > >>> +{\n> > >>> +\tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> > >>> +\tcio2_dm.add(\"ipu3-csi2 0\");\n> > >>> +\tcio2_dm.add(\"ipu3-cio2 0\");\n> > >>> +\tcio2_dm.add(\"ipu3-csi2 1\");\n> > >>> +\tcio2_dm.add(\"ipu3-cio2 1\");\n> > >>> +\tcio2_dm.add(\"ipu3-csi2 2\");\n> > >>> +\tcio2_dm.add(\"ipu3-cio2 2\");\n> > >>> +\tcio2_dm.add(\"ipu3-csi2 3\");\n> > >>> +\tcio2_dm.add(\"ipu3-cio2 3\");\n> > >>> +\n> > >>> +\tcio2_ = enumerator->search(cio2_dm);\n> > >>> +\tif (!cio2_)\n> > >>> +\t\treturn false;\n> > >>> +\n> > >>> +\tcio2_->acquire();\n> > >>> +\n> > >>> +\tDeviceMatch imgu_dm(\"ipu3-imgu\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 0\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 0 input\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 0 parameters\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 0 output\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 0 viewfinder\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 0 3a stat\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 1\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 1 input\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 1 parameters\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 1 output\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n> > >>> +\timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n> > >>> +\n> > >>> +\timgu_ = enumerator->search(imgu_dm);\n> > >>> +\tif (!imgu_) {\n> > >>> +\t\tcio2_->release();\n> > >>> +\t\treturn false;\n> > >>> +\t}\n> > >>> +\n> > >>> +\timgu_->acquire();\n> > >> \n> > >> I would reorder this a bit.\n> > >> \n> > >>     ...\n> > >> \n> > >>     cio2_ = enumerator->search(cio2_dm);\n> > >>     if (!cio2_)\n> > >>         return false;\n> > >> \n> > >>     imgu_ = enumerator->search(imgu_dm);\n> > >>     if (!imgu_)\n> > >>         return false;\n> > >> \n> > >>     cio2_->acquire();\n> > >>     imgu_->acquire();\n> > >> \n> > >>     ...\n> > >> \n> > >> I don't feel strongly about this so if others have a different view I \n> > >> will yield to public opinion :-)\n> > > \n> > > I was about to mention the same, so I can only agree :-)\n> > > \n> > > Furthermore, I think we can match on driver name only in this case,\n> > > can't we ?\n> > \n> > I think we need to design some sort of guide lines here. It's true we \n> > could match on driver name only here. By doing so we no longer ensures \n> > that the media devices have all the entities we expect it to have, and \n> > may therefor accept a media device which is\n> > \n> > 1.  From a different kernel version/patch series.\n> > 2.  Not probed completely.\n> > 3.  Expresses a variation of hardware with more or less capabilities \n> >     then the one the pipeline handler was developed for.\n> > \n> > Reason 1 should not be a real issue, but API breakages do happen... This \n> > could then be solved with a check of the running kernel version which \n> > the matching logic could be expanded to cover. I'm thinking here of UVC \n> > where a lot of the entity names are not static and we would need to \n> > depend on the driver name alone.\n> \n> I think we should add a kernel version (or rather media device version)\n> check in the DeviceMatch in any case, it can be useful.\n> \n> > Reason 2 is valid and a issue discussed at some V4L2 conference and is \n> > the main blocker for video devices can't be register at probe time. Once \n> > the kernel have a way to express that a media graph is complete we \n> > should use that in libcamera.\n> \n> It is valid, but not for all devices. For the ImgU for instance, we have\n> no external entity, so it shouldn't be an issue.\n> \n> Furthermore, for the CIO2, we indeed want to make sure all entities are\n> successfully probed, but we don't know what sensor are present in the\n> system, so the only entities we can match on anyway are internal to the\n> CIO2, which seems a bit pointless to me.\n> \n> > Reason 3 is a real problem as we don't know what variations of a \n> > hardware block will be released in a slightly different form in the \n> > future which a driver would be extended to support.\n> \n> Same as for reason 2, this is valid, but not for all devices.\n> \n> In the IPU3 case I believe matching on entities would only be useful to\n> handle the first issue you pointed out, and I wonder if we need to do so\n> now.\n\nI agree with your comments.\n\nMy point is that we should think about these issues when developing the \nfirst pipeline handlers. Even if it's not strictly needed for the IPU3 \nit might be a good idea to match on all entities we know should be there \n(or not). Cargo cult programming is areal thing :-)","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6A3B360B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 16 Jan 2019 16:54:00 +0100 (CET)","by mail-lj1-x241.google.com with SMTP id t9-v6so5860423ljh.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 16 Jan 2019 07:54:00 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tl63sm1250905lfl.76.2019.01.16.07.53.58\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 16 Jan 2019 07:53:58 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=vIvpe3i7JrqXpD4PVF2VPRG7AGE3X212SMyJwdP8uaE=;\n\tb=VxGtL5lpssdluoaBVJgt9ZJeB0ChnI7zbJEnEMXAv5JhcxDcS2RRO880KVuPHo93/G\n\tRV7KTvWjmmC+a29ucXmmwRXuPGyg2ep19of3lY5oS7AqwsFI2imhDmRKW11czuf2fHLD\n\tj/uSVpin6W5+uqTwI0iZ2YV1xoHElrPSB6tMzyYAeDX9cdMTkegUjckaZHtdykl8katn\n\tpYsp6KUcbw6hzBKL49KzTs400PTTWbakncp4lgdX4gdRbY7JeLsrC5rHC2CiX8U8UzzS\n\tzwDspEzIlAuCfMRZTCz8g5WBcmwX6aME6AW0ZCU6IBAvo+acA4oJgTkh6tsCLJm7/7Cp\n\tEKpg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=vIvpe3i7JrqXpD4PVF2VPRG7AGE3X212SMyJwdP8uaE=;\n\tb=I1C0DUjjsZUULoS6Y4euWC83lz6ffQ1BU0HL1VFRUQ59R1JeT/UhCsNheGZT/NELPQ\n\tbyi96Zx40awzmOf1u6GSIhiz/432XDSgHqPNhauCACWZ3q8y/cxL24nPZouyZDm1kkWX\n\tNdCU7zap69XLjXXZlvfozK/th6VJJR94bmUEXg4ETckZG7cd+K1Wtx2W7KZvQjXtz8mX\n\tdTCO7rRlVDW+KC3McF5eooLeH8CWX4RxBK6rnhE2+RrVCFR8x86Z7nx4mBKhvVnJ1YQS\n\tqnqa2YnAnivmNXLvpHogSb3fu02akV4ldNi0I1diBkD+0a//kigdIjGwzP0sW0XI1IF6\n\t3tHQ==","X-Gm-Message-State":"AJcUukd5ICx84fJjfPnqo0/MnJwzz2wxIqWcwOYpuxDtHn8JT0yedvCT\n\t1pd/5BDjqKxV8u1I9NoohuGMgg==","X-Google-Smtp-Source":"ALg8bN5qSDGpwFhdppyRal0K7zSLc21WVvvplShcINGmy8exQfO/aqIeElrOw7GMbDpUoPBhi4xTbQ==","X-Received":"by 2002:a2e:4a19:: with SMTP id\n\tx25-v6mr6994026lja.19.1547654039640; \n\tWed, 16 Jan 2019 07:53:59 -0800 (PST)","Date":"Wed, 16 Jan 2019 16:53:58 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190116155358.GJ6484@bigcity.dyn.berto.se>","References":"<20190111165109.26803-1-jacopo@jmondi.org>\n\t<20190115203824.GA7393@bigcity.dyn.berto.se>\n\t<20190115212507.GA28045@pendragon.ideasonboard.com>\n\t<20190115214349.GC7393@bigcity.dyn.berto.se>\n\t<20190116154230.GB20097@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190116154230.GB20097@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 16 Jan 2019 15:54:00 -0000"}},{"id":376,"web_url":"https://patchwork.libcamera.org/comment/376/","msgid":"<20190116155549.GC20097@pendragon.ideasonboard.com>","date":"2019-01-16T15:55:49","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Wed, Jan 16, 2019 at 04:53:58PM +0100, Niklas Söderlund wrote:\n> On 2019-01-16 17:42:30 +0200, Laurent Pinchart wrote:\n> > On Tue, Jan 15, 2019 at 10:43:49PM +0100, Niklas Söderlund wrote:\n> >> Hi,\n> >> \n> >> First off, sorry Jacopo I now see there is a later version of this patch \n> >> posted. I should have commented on that one.\n> >> \n> >> On 2019-01-15 23:25:07 +0200, Laurent Pinchart wrote:\n> >> \n> >> [snip]\n> >> \n> >>>>> +bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> >>>>> +{\n> >>>>> +\tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> >>>>> +\tcio2_dm.add(\"ipu3-csi2 0\");\n> >>>>> +\tcio2_dm.add(\"ipu3-cio2 0\");\n> >>>>> +\tcio2_dm.add(\"ipu3-csi2 1\");\n> >>>>> +\tcio2_dm.add(\"ipu3-cio2 1\");\n> >>>>> +\tcio2_dm.add(\"ipu3-csi2 2\");\n> >>>>> +\tcio2_dm.add(\"ipu3-cio2 2\");\n> >>>>> +\tcio2_dm.add(\"ipu3-csi2 3\");\n> >>>>> +\tcio2_dm.add(\"ipu3-cio2 3\");\n> >>>>> +\n> >>>>> +\tcio2_ = enumerator->search(cio2_dm);\n> >>>>> +\tif (!cio2_)\n> >>>>> +\t\treturn false;\n> >>>>> +\n> >>>>> +\tcio2_->acquire();\n> >>>>> +\n> >>>>> +\tDeviceMatch imgu_dm(\"ipu3-imgu\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 0\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 0 input\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 0 parameters\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 0 output\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 0 viewfinder\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 0 3a stat\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 1\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 1 input\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 1 parameters\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 1 output\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n> >>>>> +\timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n> >>>>> +\n> >>>>> +\timgu_ = enumerator->search(imgu_dm);\n> >>>>> +\tif (!imgu_) {\n> >>>>> +\t\tcio2_->release();\n> >>>>> +\t\treturn false;\n> >>>>> +\t}\n> >>>>> +\n> >>>>> +\timgu_->acquire();\n> >>>> \n> >>>> I would reorder this a bit.\n> >>>> \n> >>>> ...\n> >>>> \n> >>>> cio2_ = enumerator->search(cio2_dm);\n> >>>> if (!cio2_)\n> >>>> return false;\n> >>>> \n> >>>> imgu_ = enumerator->search(imgu_dm);\n> >>>> if (!imgu_)\n> >>>> return false;\n> >>>> \n> >>>> cio2_->acquire();\n> >>>> imgu_->acquire();\n> >>>> \n> >>>> ...\n> >>>> \n> >>>> I don't feel strongly about this so if others have a different view I \n> >>>> will yield to public opinion :-)\n> >>> \n> >>> I was about to mention the same, so I can only agree :-)\n> >>> \n> >>> Furthermore, I think we can match on driver name only in this case,\n> >>> can't we ?\n> >> \n> >> I think we need to design some sort of guide lines here. It's true we \n> >> could match on driver name only here. By doing so we no longer ensures \n> >> that the media devices have all the entities we expect it to have, and \n> >> may therefor accept a media device which is\n> >> \n> >> 1.  From a different kernel version/patch series.\n> >> 2.  Not probed completely.\n> >> 3.  Expresses a variation of hardware with more or less capabilities \n> >> then the one the pipeline handler was developed for.\n> >> \n> >> Reason 1 should not be a real issue, but API breakages do happen... This \n> >> could then be solved with a check of the running kernel version which \n> >> the matching logic could be expanded to cover. I'm thinking here of UVC \n> >> where a lot of the entity names are not static and we would need to \n> >> depend on the driver name alone.\n> > \n> > I think we should add a kernel version (or rather media device version)\n> > check in the DeviceMatch in any case, it can be useful.\n> > \n> >> Reason 2 is valid and a issue discussed at some V4L2 conference and is \n> >> the main blocker for video devices can't be register at probe time. Once \n> >> the kernel have a way to express that a media graph is complete we \n> >> should use that in libcamera.\n> > \n> > It is valid, but not for all devices. For the ImgU for instance, we have\n> > no external entity, so it shouldn't be an issue.\n> > \n> > Furthermore, for the CIO2, we indeed want to make sure all entities are\n> > successfully probed, but we don't know what sensor are present in the\n> > system, so the only entities we can match on anyway are internal to the\n> > CIO2, which seems a bit pointless to me.\n> > \n> >> Reason 3 is a real problem as we don't know what variations of a \n> >> hardware block will be released in a slightly different form in the \n> >> future which a driver would be extended to support.\n> > \n> > Same as for reason 2, this is valid, but not for all devices.\n> > \n> > In the IPU3 case I believe matching on entities would only be useful to\n> > handle the first issue you pointed out, and I wonder if we need to do so\n> > now.\n> \n> I agree with your comments.\n> \n> My point is that we should think about these issues when developing the \n> first pipeline handlers. Even if it's not strictly needed for the IPU3 \n> it might be a good idea to match on all entities we know should be there \n> (or not). Cargo cult programming is areal thing :-)\n\nEven better, we should document the usage guidelines for DeviceMatch :-)\nWould you like to capture this in a documentation patch ?","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 41D1A60B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 16 Jan 2019 16:55:49 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8A2004F8;\n\tWed, 16 Jan 2019 16:55:48 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547654148;\n\tbh=m8eCYceNDdr46XVvXCK7K8YRcLciPBRo9ZVIgUX5a00=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=J2EIF/Q1wq/BPvNZymAJjxz069bt1SzhxwVncrQ7PqHpUgGUVibvdM79M62B31x0O\n\tVctmzedYXAWKAS1wyAVcM2JTmMyI/dke7UuaFrl3h+sFMIhpQErb/FekL9gz48rQde\n\tNQ4ErDWjZI4iV7eBrxad6bJ6d/VkRtOtUC5vSpsM=","Date":"Wed, 16 Jan 2019 17:55:49 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","Message-ID":"<20190116155549.GC20097@pendragon.ideasonboard.com>","References":"<20190111165109.26803-1-jacopo@jmondi.org>\n\t<20190115203824.GA7393@bigcity.dyn.berto.se>\n\t<20190115212507.GA28045@pendragon.ideasonboard.com>\n\t<20190115214349.GC7393@bigcity.dyn.berto.se>\n\t<20190116154230.GB20097@pendragon.ideasonboard.com>\n\t<20190116155358.GJ6484@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190116155358.GJ6484@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Intel IPU3\n\tpipeline skeleton","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 16 Jan 2019 15:55:49 -0000"}}]