[{"id":1082,"web_url":"https://patchwork.libcamera.org/comment/1082/","msgid":"<20190321085005.GF4615@pendragon.ideasonboard.com>","date":"2019-03-21T08:50:05","subject":"Re: [libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure\n\tsensor provides a compatible format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Mar 20, 2019 at 05:30:27PM +0100, Jacopo Mondi wrote:\n> When creating a camera, make sure a the image sensor provides images in\n> a format compatible with IPU3 CIO2 unit requirements.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 43 ++++++++++++++++++++++++++--\n> 1 file changed, 41 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 55489c31df2d..2602f89617a3 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -8,6 +8,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \n> #include \n> #include \n> @@ -78,6 +80,8 @@ private:\n> \t\t\tPipelineHandler::cameraData(camera));\n> \t}\n> \n> +\tint mediaBusToCIO2Format(unsigned int code);\n> +\n> \tvoid registerCameras();\n> \n> \tstd::shared_ptr cio2_;\n> @@ -327,6 +331,22 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> \treturn true;\n> }\n> \n> +int PipelineHandlerIPU3::mediaBusToCIO2Format(unsigned int code)\n> +{\n> +\tswitch (code) {\n> +\tcase MEDIA_BUS_FMT_SBGGR10_1X10:\n> +\t\treturn V4L2_PIX_FMT_IPU3_SBGGR10;\n> +\tcase MEDIA_BUS_FMT_SGBRG10_1X10:\n> +\t\treturn V4L2_PIX_FMT_IPU3_SGBRG10;\n> +\tcase MEDIA_BUS_FMT_SGRBG10_1X10:\n> +\t\treturn V4L2_PIX_FMT_IPU3_SGRBG10;\n> +\tcase MEDIA_BUS_FMT_SRGGB10_1X10:\n> +\t\treturn V4L2_PIX_FMT_IPU3_SRGGB10;\n> +\tdefault:\n> +\t\treturn -EINVAL;\n> +\t}\n> +}\n> +\n> /*\n> * Cameras are created associating an image sensor (represented by a\n> * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> @@ -404,18 +424,37 @@ void PipelineHandlerIPU3::registerCameras()\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> -\t\tdata->cio2_->bufferReady.connect(data.get(), &IPU3CameraData::bufferReady);\n> -\n> +\t\t/*\n> +\t\t * Make sure the sensor produces at least one image format\n> +\t\t * compatible with IPU3 CIO2 requirements.\n> +\t\t */\n> \t\tdata->sensor_ = new V4L2Subdevice(sensor);\n> \t\tret = data->sensor_->open();\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> +\t\tconst FormatEnum formats = data->sensor_->formats(0);\n> +\t\tauto it = formats.begin();\n> +\t\tfor (; it != formats.end(); ++it) {\n\nHow about\n\n\t\tfor (auto it : formats)\n\n> +\t\t\tif (mediaBusToCIO2Format(it->first) != -EINVAL)\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\t\tif (it == formats.end()) {\n> +\t\t\tLOG(IPU3, Info)\n> +\t\t\t\t<< \"Sensor '\" << data->sensor_->deviceName()\n\nHow about printing the entity name instead ?\n\nReviewed-by: Laurent Pinchart \n\n> +\t\t\t\t<< \"' detected, but no supported image format \"\n> +\t\t\t\t<< \" found: skip camera creation\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\tdata->csi2_ = new V4L2Subdevice(csi2);\n> \t\tret = data->csi2_->open();\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> +\t\tdata->cio2_->bufferReady.connect(data.get(),\n> +\t\t\t\t\t\t &IPU3CameraData::bufferReady);\n> +\n> \t\tregisterCamera(std::move(camera), std::move(data));\n> \n> \t\tLOG(IPU3, Info)","headers":{"Return-Path":"","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 7C5CA600F9\n\tfor ;\n\tThu, 21 Mar 2019 09:50:17 +0100 (CET)","from pendragon.ideasonboard.com (30.net042126252.t-com.ne.jp\n\t[42.126.252.30])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2A9B123A;\n\tThu, 21 Mar 2019 09:50:15 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1553158217;\n\tbh=UmElX+XWmD0zG/DanENqs1Hqom9C7x8+i00NxpsTA6s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ADBZlIrwz4h9qjSYG1kElha4kaEz+52J9jg3l2TD59eaxuK6MrU1yZ+8GKqM0kzhh\n\twvTZ/ku2mRNCH8ifnPHVIvYtdSAnUkAuGjyzFhi5PTSHtgtR7dR7TW5I8IX7LS4wFR\n\t/TNBP4AEFXA+kMiMsU3dYxFF4rQs833bSKw1TZgo=","Date":"Thu, 21 Mar 2019 10:50:05 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190321085005.GF4615@pendragon.ideasonboard.com>","References":"<20190320163055.22056-1-jacopo@jmondi.org>\n\t<20190320163055.22056-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190320163055.22056-4-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure\n\tsensor provides a compatible format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 21 Mar 2019 08:50:17 -0000"}},{"id":1095,"web_url":"https://patchwork.libcamera.org/comment/1095/","msgid":"<20190321145016.fxlw47uwk2wg6o2d@uno.localdomain>","date":"2019-03-21T14:50:16","subject":"Re: [libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure\n\tsensor provides a compatible format","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Mar 21, 2019 at 10:50:05AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Mar 20, 2019 at 05:30:27PM +0100, Jacopo Mondi wrote:\n> > When creating a camera, make sure a the image sensor provides images in\n> > a format compatible with IPU3 CIO2 unit requirements.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 43 ++++++++++++++++++++++++++--\n> > 1 file changed, 41 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 55489c31df2d..2602f89617a3 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -8,6 +8,8 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > +\n> > #include \n> > #include \n> > #include \n> > @@ -78,6 +80,8 @@ private:\n> > \t\t\tPipelineHandler::cameraData(camera));\n> > \t}\n> >\n> > +\tint mediaBusToCIO2Format(unsigned int code);\n> > +\n> > \tvoid registerCameras();\n> >\n> > \tstd::shared_ptr cio2_;\n> > @@ -327,6 +331,22 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > \treturn true;\n> > }\n> >\n> > +int PipelineHandlerIPU3::mediaBusToCIO2Format(unsigned int code)\n> > +{\n> > +\tswitch (code) {\n> > +\tcase MEDIA_BUS_FMT_SBGGR10_1X10:\n> > +\t\treturn V4L2_PIX_FMT_IPU3_SBGGR10;\n> > +\tcase MEDIA_BUS_FMT_SGBRG10_1X10:\n> > +\t\treturn V4L2_PIX_FMT_IPU3_SGBRG10;\n> > +\tcase MEDIA_BUS_FMT_SGRBG10_1X10:\n> > +\t\treturn V4L2_PIX_FMT_IPU3_SGRBG10;\n> > +\tcase MEDIA_BUS_FMT_SRGGB10_1X10:\n> > +\t\treturn V4L2_PIX_FMT_IPU3_SRGGB10;\n> > +\tdefault:\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +}\n> > +\n> > /*\n> > * Cameras are created associating an image sensor (represented by a\n> > * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> > @@ -404,18 +424,37 @@ void PipelineHandlerIPU3::registerCameras()\n> > \t\tif (ret)\n> > \t\t\tcontinue;\n> >\n> > -\t\tdata->cio2_->bufferReady.connect(data.get(), &IPU3CameraData::bufferReady);\n> > -\n> > +\t\t/*\n> > +\t\t * Make sure the sensor produces at least one image format\n> > +\t\t * compatible with IPU3 CIO2 requirements.\n> > +\t\t */\n> > \t\tdata->sensor_ = new V4L2Subdevice(sensor);\n> > \t\tret = data->sensor_->open();\n> > \t\tif (ret)\n> > \t\t\tcontinue;\n> >\n> > +\t\tconst FormatEnum formats = data->sensor_->formats(0);\n> > +\t\tauto it = formats.begin();\n> > +\t\tfor (; it != formats.end(); ++it) {\n>\n> How about\n>\n> \t\tfor (auto it : formats)\n>\n\nHere and in other patches in the series I need to check the value of\nit after the for loop, so I cannot use a variable with a scope local\nto the loop only.\n\nYou suggested alternatives offline, but this cose seems quite parsable\nto me.\n\n> > +\t\t\tif (mediaBusToCIO2Format(it->first) != -EINVAL)\n> > +\t\t\t\tbreak;\n> > +\t\t}\n> > +\t\tif (it == formats.end()) {\n> > +\t\t\tLOG(IPU3, Info)\n> > +\t\t\t\t<< \"Sensor '\" << data->sensor_->deviceName()\n>\n> How about printing the entity name instead ?\n\nFor V4L2Subdevice deviceName() prints the entity name.\nConfusing?\n\n>\n> Reviewed-by: Laurent Pinchart \n\nThanks, I'll also consider caching the camera mix and min sizes when\nregistering it, as you suggested in a comment to the next patch.\n\nThanks\n j\n>\n> > +\t\t\t\t<< \"' detected, but no supported image format \"\n> > +\t\t\t\t<< \" found: skip camera creation\";\n> > +\t\t\tcontinue;\n> > +\t\t}\n> > +\n> > \t\tdata->csi2_ = new V4L2Subdevice(csi2);\n> > \t\tret = data->csi2_->open();\n> > \t\tif (ret)\n> > \t\t\tcontinue;\n> >\n> > +\t\tdata->cio2_->bufferReady.connect(data.get(),\n> > +\t\t\t\t\t\t &IPU3CameraData::bufferReady);\n> > +\n> > \t\tregisterCamera(std::move(camera), std::move(data));\n> >\n> > \t\tLOG(IPU3, Info)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 897FF600FD\n\tfor ;\n\tThu, 21 Mar 2019 15:49:37 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 065AC40014;\n\tThu, 21 Mar 2019 14:49:36 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 21 Mar 2019 15:50:16 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190321145016.fxlw47uwk2wg6o2d@uno.localdomain>","References":"<20190320163055.22056-1-jacopo@jmondi.org>\n\t<20190320163055.22056-4-jacopo@jmondi.org>\n\t<20190321085005.GF4615@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"eiem6kld7rrlky3y\"","Content-Disposition":"inline","In-Reply-To":"<20190321085005.GF4615@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure\n\tsensor provides a compatible format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 21 Mar 2019 14:49:37 -0000"}},{"id":1104,"web_url":"https://patchwork.libcamera.org/comment/1104/","msgid":"<20190323125818.GB4587@pendragon.ideasonboard.com>","date":"2019-03-23T12:58:18","subject":"Re: [libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure\n\tsensor provides a compatible format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Mar 21, 2019 at 03:50:16PM +0100, Jacopo Mondi wrote:\n> On Thu, Mar 21, 2019 at 10:50:05AM +0200, Laurent Pinchart wrote:\n> > On Wed, Mar 20, 2019 at 05:30:27PM +0100, Jacopo Mondi wrote:\n> >> When creating a camera, make sure a the image sensor provides images in\n> >> a format compatible with IPU3 CIO2 unit requirements.\n> >>\n> >> Signed-off-by: Jacopo Mondi \n> >> ---\n> >> src/libcamera/pipeline/ipu3/ipu3.cpp | 43 ++++++++++++++++++++++++++--\n> >> 1 file changed, 41 insertions(+), 2 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> index 55489c31df2d..2602f89617a3 100644\n> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> @@ -8,6 +8,8 @@\n> >> #include \n> >> #include \n> >>\n> >> +#include \n> >> +\n> >> #include \n> >> #include \n> >> #include \n> >> @@ -78,6 +80,8 @@ private:\n> >> \t\t\tPipelineHandler::cameraData(camera));\n> >> \t}\n> >>\n> >> +\tint mediaBusToCIO2Format(unsigned int code);\n> >> +\n> >> \tvoid registerCameras();\n> >>\n> >> \tstd::shared_ptr cio2_;\n> >> @@ -327,6 +331,22 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> >> \treturn true;\n> >> }\n> >>\n> >> +int PipelineHandlerIPU3::mediaBusToCIO2Format(unsigned int code)\n> >> +{\n> >> +\tswitch (code) {\n> >> +\tcase MEDIA_BUS_FMT_SBGGR10_1X10:\n> >> +\t\treturn V4L2_PIX_FMT_IPU3_SBGGR10;\n> >> +\tcase MEDIA_BUS_FMT_SGBRG10_1X10:\n> >> +\t\treturn V4L2_PIX_FMT_IPU3_SGBRG10;\n> >> +\tcase MEDIA_BUS_FMT_SGRBG10_1X10:\n> >> +\t\treturn V4L2_PIX_FMT_IPU3_SGRBG10;\n> >> +\tcase MEDIA_BUS_FMT_SRGGB10_1X10:\n> >> +\t\treturn V4L2_PIX_FMT_IPU3_SRGGB10;\n> >> +\tdefault:\n> >> +\t\treturn -EINVAL;\n> >> +\t}\n> >> +}\n> >> +\n> >> /*\n> >> * Cameras are created associating an image sensor (represented by a\n> >> * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> >> @@ -404,18 +424,37 @@ void PipelineHandlerIPU3::registerCameras()\n> >> \t\tif (ret)\n> >> \t\t\tcontinue;\n> >>\n> >> -\t\tdata->cio2_->bufferReady.connect(data.get(), &IPU3CameraData::bufferReady);\n> >> -\n> >> +\t\t/*\n> >> +\t\t * Make sure the sensor produces at least one image format\n> >> +\t\t * compatible with IPU3 CIO2 requirements.\n> >> +\t\t */\n> >> \t\tdata->sensor_ = new V4L2Subdevice(sensor);\n> >> \t\tret = data->sensor_->open();\n> >> \t\tif (ret)\n> >> \t\t\tcontinue;\n> >>\n> >> +\t\tconst FormatEnum formats = data->sensor_->formats(0);\n> >> +\t\tauto it = formats.begin();\n> >> +\t\tfor (; it != formats.end(); ++it) {\n> >\n> > How about\n> >\n> > \t\tfor (auto it : formats)\n> >\n> \n> Here and in other patches in the series I need to check the value of\n> it after the for loop, so I cannot use a variable with a scope local\n> to the loop only.\n> \n> You suggested alternatives offline, but this cose seems quite parsable\n> to me.\n> \n> >> +\t\t\tif (mediaBusToCIO2Format(it->first) != -EINVAL)\n> >> +\t\t\t\tbreak;\n> >> +\t\t}\n> >> +\t\tif (it == formats.end()) {\n> >> +\t\t\tLOG(IPU3, Info)\n> >> +\t\t\t\t<< \"Sensor '\" << data->sensor_->deviceName()\n> >\n> > How about printing the entity name instead ?\n> \n> For V4L2Subdevice deviceName() prints the entity name.\n> Confusing?\n\nI suppose so, given that I got confused :-)\n\n> >\n> > Reviewed-by: Laurent Pinchart \n> \n> Thanks, I'll also consider caching the camera mix and min sizes when\n> registering it, as you suggested in a comment to the next patch.\n> \n> >> +\t\t\t\t<< \"' detected, but no supported image format \"\n> >> +\t\t\t\t<< \" found: skip camera creation\";\n> >> +\t\t\tcontinue;\n> >> +\t\t}\n> >> +\n> >> \t\tdata->csi2_ = new V4L2Subdevice(csi2);\n> >> \t\tret = data->csi2_->open();\n> >> \t\tif (ret)\n> >> \t\t\tcontinue;\n> >>\n> >> +\t\tdata->cio2_->bufferReady.connect(data.get(),\n> >> +\t\t\t\t\t\t &IPU3CameraData::bufferReady);\n> >> +\n> >> \t\tregisterCamera(std::move(camera), std::move(data));\n> >>\n> >> \t\tLOG(IPU3, Info)","headers":{"Return-Path":"","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 5C3D4610B6\n\tfor ;\n\tSat, 23 Mar 2019 13:58:31 +0100 (CET)","from pendragon.ideasonboard.com\n\t(p5269001-ipngn11702marunouchi.tokyo.ocn.ne.jp [114.158.195.1])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A607C2D0;\n\tSat, 23 Mar 2019 13:58:29 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1553345910;\n\tbh=Me5tKaovfv7L72FF0JksjEcZLtve742w7cSZDzQ4SKk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bgYUI32EhoT7cZGp4QwyVpwSK5vI5JfqDU07pJaVwhdBK2hN2QR4ZPLGEbOLDRcLo\n\tdDjA/6Ktcqewc9bkXym3gAFgNcsMCqLqNgOYkK50qOU6qKilfghG7w7E0IsDykb+dX\n\toSRU7ZYxuDCsueXSahLGb0B5EczQ5emimkL9V25M=","Date":"Sat, 23 Mar 2019 14:58:18 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190323125818.GB4587@pendragon.ideasonboard.com>","References":"<20190320163055.22056-1-jacopo@jmondi.org>\n\t<20190320163055.22056-4-jacopo@jmondi.org>\n\t<20190321085005.GF4615@pendragon.ideasonboard.com>\n\t<20190321145016.fxlw47uwk2wg6o2d@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190321145016.fxlw47uwk2wg6o2d@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure\n\tsensor provides a compatible format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 23 Mar 2019 12:58:31 -0000"}},{"id":1106,"web_url":"https://patchwork.libcamera.org/comment/1106/","msgid":"<20190323130628.ub6hmtdzkc4vjsda@uno.localdomain>","date":"2019-03-23T13:06:28","subject":"Re: [libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure\n\tsensor provides a compatible format","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Mar 23, 2019 at 02:58:18PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Thu, Mar 21, 2019 at 03:50:16PM +0100, Jacopo Mondi wrote:\n> > On Thu, Mar 21, 2019 at 10:50:05AM +0200, Laurent Pinchart wrote:\n> > > On Wed, Mar 20, 2019 at 05:30:27PM +0100, Jacopo Mondi wrote:\n> > >> When creating a camera, make sure a the image sensor provides images in\n> > >> a format compatible with IPU3 CIO2 unit requirements.\n> > >>\n> > >> Signed-off-by: Jacopo Mondi \n> > >> ---\n> > >> src/libcamera/pipeline/ipu3/ipu3.cpp | 43 ++++++++++++++++++++++++++--\n> > >> 1 file changed, 41 insertions(+), 2 deletions(-)\n> > >>\n> > >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > >> index 55489c31df2d..2602f89617a3 100644\n> > >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > >> @@ -8,6 +8,8 @@\n> > >> #include \n> > >> #include \n> > >>\n> > >> +#include \n> > >> +\n> > >> #include \n> > >> #include \n> > >> #include \n> > >> @@ -78,6 +80,8 @@ private:\n> > >> \t\t\tPipelineHandler::cameraData(camera));\n> > >> \t}\n> > >>\n> > >> +\tint mediaBusToCIO2Format(unsigned int code);\n> > >> +\n> > >> \tvoid registerCameras();\n> > >>\n> > >> \tstd::shared_ptr cio2_;\n> > >> @@ -327,6 +331,22 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > >> \treturn true;\n> > >> }\n> > >>\n> > >> +int PipelineHandlerIPU3::mediaBusToCIO2Format(unsigned int code)\n> > >> +{\n> > >> +\tswitch (code) {\n> > >> +\tcase MEDIA_BUS_FMT_SBGGR10_1X10:\n> > >> +\t\treturn V4L2_PIX_FMT_IPU3_SBGGR10;\n> > >> +\tcase MEDIA_BUS_FMT_SGBRG10_1X10:\n> > >> +\t\treturn V4L2_PIX_FMT_IPU3_SGBRG10;\n> > >> +\tcase MEDIA_BUS_FMT_SGRBG10_1X10:\n> > >> +\t\treturn V4L2_PIX_FMT_IPU3_SGRBG10;\n> > >> +\tcase MEDIA_BUS_FMT_SRGGB10_1X10:\n> > >> +\t\treturn V4L2_PIX_FMT_IPU3_SRGGB10;\n> > >> +\tdefault:\n> > >> +\t\treturn -EINVAL;\n> > >> +\t}\n> > >> +}\n> > >> +\n> > >> /*\n> > >> * Cameras are created associating an image sensor (represented by a\n> > >> * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> > >> @@ -404,18 +424,37 @@ void PipelineHandlerIPU3::registerCameras()\n> > >> \t\tif (ret)\n> > >> \t\t\tcontinue;\n> > >>\n> > >> -\t\tdata->cio2_->bufferReady.connect(data.get(), &IPU3CameraData::bufferReady);\n> > >> -\n> > >> +\t\t/*\n> > >> +\t\t * Make sure the sensor produces at least one image format\n> > >> +\t\t * compatible with IPU3 CIO2 requirements.\n> > >> +\t\t */\n> > >> \t\tdata->sensor_ = new V4L2Subdevice(sensor);\n> > >> \t\tret = data->sensor_->open();\n> > >> \t\tif (ret)\n> > >> \t\t\tcontinue;\n> > >>\n> > >> +\t\tconst FormatEnum formats = data->sensor_->formats(0);\n> > >> +\t\tauto it = formats.begin();\n> > >> +\t\tfor (; it != formats.end(); ++it) {\n> > >\n> > > How about\n> > >\n> > > \t\tfor (auto it : formats)\n> > >\n> >\n> > Here and in other patches in the series I need to check the value of\n> > it after the for loop, so I cannot use a variable with a scope local\n> > to the loop only.\n> >\n> > You suggested alternatives offline, but this cose seems quite parsable\n> > to me.\n> >\n> > >> +\t\t\tif (mediaBusToCIO2Format(it->first) != -EINVAL)\n> > >> +\t\t\t\tbreak;\n> > >> +\t\t}\n> > >> +\t\tif (it == formats.end()) {\n> > >> +\t\t\tLOG(IPU3, Info)\n> > >> +\t\t\t\t<< \"Sensor '\" << data->sensor_->deviceName()\n> > >\n> > > How about printing the entity name instead ?\n> >\n> > For V4L2Subdevice deviceName() prints the entity name.\n> > Confusing?\n>\n> I suppose so, given that I got confused :-)\n\nV5 will contain a patch which changes this (and makes the\nV4L2Subdevice return a const & to a string in its deviceNode() and\ndeviceName() methods, which they embarassing enough copy back at\nthe moment)\n\nThanks\n j\n>\n> > >\n> > > Reviewed-by: Laurent Pinchart \n> >\n> > Thanks, I'll also consider caching the camera mix and min sizes when\n> > registering it, as you suggested in a comment to the next patch.\n> >\n> > >> +\t\t\t\t<< \"' detected, but no supported image format \"\n> > >> +\t\t\t\t<< \" found: skip camera creation\";\n> > >> +\t\t\tcontinue;\n> > >> +\t\t}\n> > >> +\n> > >> \t\tdata->csi2_ = new V4L2Subdevice(csi2);\n> > >> \t\tret = data->csi2_->open();\n> > >> \t\tif (ret)\n> > >> \t\t\tcontinue;\n> > >>\n> > >> +\t\tdata->cio2_->bufferReady.connect(data.get(),\n> > >> +\t\t\t\t\t\t &IPU3CameraData::bufferReady);\n> > >> +\n> > >> \t\tregisterCamera(std::move(camera), std::move(data));\n> > >>\n> > >> \t\tLOG(IPU3, Info)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4B2A5610B6\n\tfor ;\n\tSat, 23 Mar 2019 14:05:48 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id BDCAC200009;\n\tSat, 23 Mar 2019 13:05:47 +0000 (UTC)"],"Date":"Sat, 23 Mar 2019 14:06:28 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190323130628.ub6hmtdzkc4vjsda@uno.localdomain>","References":"<20190320163055.22056-1-jacopo@jmondi.org>\n\t<20190320163055.22056-4-jacopo@jmondi.org>\n\t<20190321085005.GF4615@pendragon.ideasonboard.com>\n\t<20190321145016.fxlw47uwk2wg6o2d@uno.localdomain>\n\t<20190323125818.GB4587@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"vqmr25q7lfasxiwz\"","Content-Disposition":"inline","In-Reply-To":"<20190323125818.GB4587@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v4 03/31] libcamera: ipu3: Make sure\n\tsensor provides a compatible format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 23 Mar 2019 13:05:48 -0000"}}]