[{"id":15056,"web_url":"https://patchwork.libcamera.org/comment/15056/","msgid":"<20210208160825.lydwjw5fshm52s57@uno.localdomain>","date":"2021-02-08T16:08:25","subject":"Re: [libcamera-devel] [PATCH/RFC] libcamera: pipeline: simple:\n\tSupport camera sensors that contain an ISP","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Feb 08, 2021 at 05:15:58PM +0200, Laurent Pinchart wrote:\n> Camera sensors can include an ISP. For instance, the AP1302 external ISP\n> can be connected to up to two raw camera sensors, and the combination of\n> the sensors and ISP is considered as a (smart) camera sensor from\n> libcamera's point of view.\n>\n> The CameraSensor class has limited support for this already. Extend the\n> simple pipeline handler to support such sensors, by using the media\n> entity corresponding to the ISP instead of the raw camera sensor's\n> entity.\n>\n> We don't need to handle the case where an entity in the SoC would expose\n> the MEDIA_ENT_F_PROC_VIDEO_ISP function, as pipeline containing an ISP\n> would have a dedicated pipeline handler.\n>\n> The implementation is limited as it won't support other multi-entity\n> camera sensors (such as CCS). While this would be worth supporting, we\n> don't have a test platform with a CCS-compatible sensor at this point,\n> so let's not over-engineer the solution. Extending support to CCS (and\n> possibly other sensor topologies) will likely involve helpers that can\n> be used by other pipeline handlers (such as generic graph walk helpers\n> for instance) and extensions to the CameraSensor class.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 87 ++++++++++++++++++++----\n> 1 file changed, 74 insertions(+), 13 deletions(-)\n>\n> This patch depends on \"[PATCH/RFC] libcamera: camera_sensor: Accept\n> entities exposing the ISP function\" ([1]). It has been tested on a\n> MediaTek Pumpkin i500 board with an ON Semiconductor AP1302 and AR0330\n> and AR0144 sensors.\n>\n> [1] https://patchwork.libcamera.org/patch/11182/\n\nDoesn't it depend on the introduction of such entity function in\nkernel space too ?\n\nAlso, I'm not sure if the model for sensor that expose multiple\nentities has really been finalized. In example I don't see any driver\nfor those sensors in mainline. Am I mistaken ?\n\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 9d468f7c9cc4..2df33a3a79ea 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -240,6 +240,8 @@ private:\n> \t\t\tPipelineHandler::cameraData(camera));\n> \t}\n>\n> +\tstd::vector locateSensors();\n> +\n> \tvoid bufferReady(FrameBuffer *buffer);\n> \tvoid converterInputDone(FrameBuffer *buffer);\n> \tvoid converterOutputDone(FrameBuffer *buffer);\n> @@ -865,6 +867,77 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> * Match and Setup\n> */\n>\n> +std::vector SimplePipelineHandler::locateSensors()\n> +{\n> +\tstd::vector entities;\n> +\n> +\t/*\n> +\t * Gather all the camera sensor entities based on the function they\n> +\t * expose.\n> +\t */\n> +\tfor (MediaEntity *entity : media_->entities()) {\n> +\t\tif (entity->function() == MEDIA_ENT_F_CAM_SENSOR)\n> +\t\t\tentities.push_back(entity);\n> +\t}\n> +\n> +\tif (entities.empty())\n> +\t\treturn {};\n> +\n> +\t/*\n> +\t * Sensors can be made of multiple entities. For instance, a raw sensor\n> +\t * can be connected to an ISP, and the combination of both should be\n> +\t * treated as one sensor. To support this, as a crude heuristic, check\n> +\t * the downstream entity from the camera sensor, and if it is an ISP,\n> +\t * use it instead of the sensor.\n> +\t */\n> +\tstd::vector sensors;\n> +\n> +\tfor (MediaEntity *entity : entities) {\n> +\t\t/*\n> +\t\t * Locate the downstream entity by following the first enabled\n> +\t\t * link from a source pad.\n> +\t\t */\n> +\t\tconst MediaPad *pad = nullptr;\n> +\t\tconst MediaLink *link = nullptr;\n> +\n> +\t\tfor (const MediaPad *p : entity->pads()) {\n> +\t\t\tif (p->flags() & MEDIA_PAD_FL_SOURCE) {\n> +\t\t\t\tpad = p;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tif (!pad)\n> +\t\t\tcontinue;\n> +\n> +\t\tfor (const MediaLink *l : pad->links()) {\n> +\t\t\tif (l->flags() & MEDIA_LNK_FL_ENABLED) {\n> +\t\t\t\tlink = l;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tif (!link)\n> +\t\t\tcontinue;\n> +\n> +\t\tMediaEntity *remote = link->sink()->entity();\n> +\t\tif (remote->function() == MEDIA_ENT_F_PROC_VIDEO_ISP)\n> +\t\t\tsensors.push_back(remote);\n> +\t\telse\n> +\t\t\tsensors.push_back(entity);\n> +\t}\n> +\n> +\t/*\n> +\t * Remove duplicates, in case multiple sensors are connected to the\n> +\t * same ISP.\n> +\t */\n> +\tstd::sort(sensors.begin(), sensors.end());\n> +\tauto last = std::unique(sensors.begin(), sensors.end());\n> +\tsensors.erase(last, sensors.end());\n> +\n> +\treturn sensors;\n> +}\n> +\n> bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> {\n> \tconst SimplePipelineInfo *info = nullptr;\n> @@ -888,19 +961,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> \t}\n>\n> \t/* Locate the sensors. */\n> -\tstd::vector sensors;\n> -\n> -\tfor (MediaEntity *entity : media_->entities()) {\n> -\t\tswitch (entity->function()) {\n> -\t\tcase MEDIA_ENT_F_CAM_SENSOR:\n> -\t\t\tsensors.push_back(entity);\n> -\t\t\tbreak;\n> -\n> -\t\tdefault:\n> -\t\t\tbreak;\n> -\t\t}\n> -\t}\n> -\n> +\tstd::vector sensors = locateSensors();\n> \tif (sensors.empty()) {\n> \t\tLOG(SimplePipeline, Error) << \"No sensor found\";\n> \t\treturn false;\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 10E3DBD160\n\tfor ;\n\tMon, 8 Feb 2021 16:08:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 99B7B60D33;\n\tMon, 8 Feb 2021 17:08:03 +0100 (CET)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9696A60D2B\n\tfor ;\n\tMon, 8 Feb 2021 17:08:02 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 207551BF217;\n\tMon, 8 Feb 2021 16:08:01 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 8 Feb 2021 17:08:25 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210208160825.lydwjw5fshm52s57@uno.localdomain>","References":"<20210208151558.5697-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210208151558.5697-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH/RFC] libcamera: pipeline: simple:\n\tSupport camera sensors that contain an ISP","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15059,"web_url":"https://patchwork.libcamera.org/comment/15059/","msgid":"","date":"2021-02-08T16:19:31","subject":"Re: [libcamera-devel] [PATCH/RFC] libcamera: pipeline: simple:\n\tSupport camera sensors that contain an ISP","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Feb 08, 2021 at 05:08:25PM +0100, Jacopo Mondi wrote:\n> On Mon, Feb 08, 2021 at 05:15:58PM +0200, Laurent Pinchart wrote:\n> > Camera sensors can include an ISP. For instance, the AP1302 external ISP\n> > can be connected to up to two raw camera sensors, and the combination of\n> > the sensors and ISP is considered as a (smart) camera sensor from\n> > libcamera's point of view.\n> >\n> > The CameraSensor class has limited support for this already. Extend the\n> > simple pipeline handler to support such sensors, by using the media\n> > entity corresponding to the ISP instead of the raw camera sensor's\n> > entity.\n> >\n> > We don't need to handle the case where an entity in the SoC would expose\n> > the MEDIA_ENT_F_PROC_VIDEO_ISP function, as pipeline containing an ISP\n> > would have a dedicated pipeline handler.\n> >\n> > The implementation is limited as it won't support other multi-entity\n> > camera sensors (such as CCS). While this would be worth supporting, we\n> > don't have a test platform with a CCS-compatible sensor at this point,\n> > so let's not over-engineer the solution. Extending support to CCS (and\n> > possibly other sensor topologies) will likely involve helpers that can\n> > be used by other pipeline handlers (such as generic graph walk helpers\n> > for instance) and extensions to the CameraSensor class.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/pipeline/simple/simple.cpp | 87 ++++++++++++++++++++----\n> > 1 file changed, 74 insertions(+), 13 deletions(-)\n> >\n> > This patch depends on \"[PATCH/RFC] libcamera: camera_sensor: Accept\n> > entities exposing the ISP function\" ([1]). It has been tested on a\n> > MediaTek Pumpkin i500 board with an ON Semiconductor AP1302 and AR0330\n> > and AR0144 sensors.\n> >\n> > [1] https://patchwork.libcamera.org/patch/11182/\n> \n> Doesn't it depend on the introduction of such entity function in\n> kernel space too ?\n\nYes it does. That's scheduled for v5.12-rc1.\n\n> Also, I'm not sure if the model for sensor that expose multiple\n> entities has really been finalized. In example I don't see any driver\n> for those sensors in mainline. Am I mistaken ?\n\nYou're not. The AP1302 driver should be posted soon. I wanted to get\nthis patch out as an RFC first to see if there was a general agreement\non the direction and approach, and I'll maintain it out of tree as long\nas needed.\n\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 9d468f7c9cc4..2df33a3a79ea 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -240,6 +240,8 @@ private:\n> > \t\t\tPipelineHandler::cameraData(camera));\n> > \t}\n> >\n> > +\tstd::vector locateSensors();\n> > +\n> > \tvoid bufferReady(FrameBuffer *buffer);\n> > \tvoid converterInputDone(FrameBuffer *buffer);\n> > \tvoid converterOutputDone(FrameBuffer *buffer);\n> > @@ -865,6 +867,77 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> > * Match and Setup\n> > */\n> >\n> > +std::vector SimplePipelineHandler::locateSensors()\n> > +{\n> > +\tstd::vector entities;\n> > +\n> > +\t/*\n> > +\t * Gather all the camera sensor entities based on the function they\n> > +\t * expose.\n> > +\t */\n> > +\tfor (MediaEntity *entity : media_->entities()) {\n> > +\t\tif (entity->function() == MEDIA_ENT_F_CAM_SENSOR)\n> > +\t\t\tentities.push_back(entity);\n> > +\t}\n> > +\n> > +\tif (entities.empty())\n> > +\t\treturn {};\n> > +\n> > +\t/*\n> > +\t * Sensors can be made of multiple entities. For instance, a raw sensor\n> > +\t * can be connected to an ISP, and the combination of both should be\n> > +\t * treated as one sensor. To support this, as a crude heuristic, check\n> > +\t * the downstream entity from the camera sensor, and if it is an ISP,\n> > +\t * use it instead of the sensor.\n> > +\t */\n> > +\tstd::vector sensors;\n> > +\n> > +\tfor (MediaEntity *entity : entities) {\n> > +\t\t/*\n> > +\t\t * Locate the downstream entity by following the first enabled\n> > +\t\t * link from a source pad.\n> > +\t\t */\n> > +\t\tconst MediaPad *pad = nullptr;\n> > +\t\tconst MediaLink *link = nullptr;\n> > +\n> > +\t\tfor (const MediaPad *p : entity->pads()) {\n> > +\t\t\tif (p->flags() & MEDIA_PAD_FL_SOURCE) {\n> > +\t\t\t\tpad = p;\n> > +\t\t\t\tbreak;\n> > +\t\t\t}\n> > +\t\t}\n> > +\n> > +\t\tif (!pad)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tfor (const MediaLink *l : pad->links()) {\n> > +\t\t\tif (l->flags() & MEDIA_LNK_FL_ENABLED) {\n> > +\t\t\t\tlink = l;\n> > +\t\t\t\tbreak;\n> > +\t\t\t}\n> > +\t\t}\n> > +\n> > +\t\tif (!link)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tMediaEntity *remote = link->sink()->entity();\n> > +\t\tif (remote->function() == MEDIA_ENT_F_PROC_VIDEO_ISP)\n> > +\t\t\tsensors.push_back(remote);\n> > +\t\telse\n> > +\t\t\tsensors.push_back(entity);\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * Remove duplicates, in case multiple sensors are connected to the\n> > +\t * same ISP.\n> > +\t */\n> > +\tstd::sort(sensors.begin(), sensors.end());\n> > +\tauto last = std::unique(sensors.begin(), sensors.end());\n> > +\tsensors.erase(last, sensors.end());\n> > +\n> > +\treturn sensors;\n> > +}\n> > +\n> > bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > {\n> > \tconst SimplePipelineInfo *info = nullptr;\n> > @@ -888,19 +961,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> > \t}\n> >\n> > \t/* Locate the sensors. */\n> > -\tstd::vector sensors;\n> > -\n> > -\tfor (MediaEntity *entity : media_->entities()) {\n> > -\t\tswitch (entity->function()) {\n> > -\t\tcase MEDIA_ENT_F_CAM_SENSOR:\n> > -\t\t\tsensors.push_back(entity);\n> > -\t\t\tbreak;\n> > -\n> > -\t\tdefault:\n> > -\t\t\tbreak;\n> > -\t\t}\n> > -\t}\n> > -\n> > +\tstd::vector sensors = locateSensors();\n> > \tif (sensors.empty()) {\n> > \t\tLOG(SimplePipeline, Error) << \"No sensor found\";\n> > \t\treturn false;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B0782BD160\n\tfor ;\n\tMon, 8 Feb 2021 16:19:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3DE8860D3B;\n\tMon, 8 Feb 2021 17:19:58 +0100 (CET)","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 4F65260D2B\n\tfor ;\n\tMon, 8 Feb 2021 17:19:57 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BB5A23D7;\n\tMon, 8 Feb 2021 17:19:56 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JnLiq3vN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612801196;\n\tbh=E/kXkwOw1qSkD7UY8lPjF4/d8iB7n01vzlUCsqyUpT0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JnLiq3vN7WpjFFPcBi+9oV4nOTmmivWqlxdmVIQTgCn6rbMpJiql0tdGREhmB+mfm\n\tZHIFb/mVKQIDMhb5si7F+iKtMnbdFCMKEydqCJW25ECUzThjoBZzGqBJLHRUV21jem\n\tRdCbdbhYLy2Wz/bBMx6VX3POLJ2gQovPp9iXn1NM=","Date":"Mon, 8 Feb 2021 18:19:31 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210208151558.5697-1-laurent.pinchart@ideasonboard.com>\n\t<20210208160825.lydwjw5fshm52s57@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210208160825.lydwjw5fshm52s57@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH/RFC] libcamera: pipeline: simple:\n\tSupport camera sensors that contain an ISP","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]