[{"id":21847,"web_url":"https://patchwork.libcamera.org/comment/21847/","msgid":"","date":"2021-12-21T10:53:06","subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for this patch!\n\nOn Tue, 14 Dec 2021 at 14:00, Naushir Patuck wrote:\n>\n> This change will allow the pipeline handler to enumerate and control Video\n> Mux or Bridge devices that may be attached between sensors and a particular\n> Unicam instance. Cascaded mux or bridge devices are also handled.\n>\n> A new member function RPiCameraData::enumerateVideoDevices(), called from\n> PipelineHandlerRPi::registerCamera(), is used to identify and open all mux and\n> bridge subdevices present in the sensor -> Unicam link.\n>\n> Relevent links are enabled/disabled and pad formats correctly set in\n\nMaybe s/Relevent/Relevant/ ? But I could be wrong. It is also the most\nextreme hair-splitting!\n\n> PipelineHandlerRPi::configure() before the camera is started.\n>\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Kieran Bingham \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 139 ++++++++++++++++++\n> 1 file changed, 139 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 2a2fb5273eb8..4ec646d3c7a3 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n> #include \n> @@ -191,6 +192,8 @@ public:\n> int loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> int configureIPA(const CameraConfiguration *config);\n>\n> + void enumerateVideoDevices(MediaLink *link);\n> +\n> void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n> void runIsp(uint32_t bufferId);\n> void embeddedComplete(uint32_t bufferId);\n> @@ -220,6 +223,11 @@ public:\n> std::vector streams_;\n> /* Stores the ids of the buffers mapped in the IPA. */\n> std::unordered_set ipaBuffers_;\n> + /*\n> + * Stores a cascade of Video Mux or Bridge devices between the sensor and\n> + * Unicam together with media link across the entities.\n> + */\n> + std::vector, MediaLink *>> bridgeDevices_;\n>\n> /* DMAHEAP allocation helper. */\n> RPi::DmaHeap dmaHeap_;\n> @@ -868,6 +876,38 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> */\n> data->properties_.set(properties::ScalerCropMaximum, data->sensorInfo_.analogCrop);\n>\n> + /* Setup the Video Mux/Bridge entities. */\n> + for (auto &[device, link] : data->bridgeDevices_) {\n> + /*\n> + * Start by disabling all the sink pad links on the devices in the\n> + * cascade, with the exception of the link connecting the device.\n> + */\n> + for (const MediaPad *p : device->entity()->pads()) {\n> + if (!(p->flags() & MEDIA_PAD_FL_SINK))\n> + continue;\n> +\n> + for (MediaLink *l : p->links()) {\n> + if (l != link)\n> + l->setEnabled(false);\n> + }\n> + }\n> +\n> + /* Next, enable the entity -> entity links, and setup the pad format. */\n> + link->setEnabled(true);\n> + const MediaPad *srcPad = link->sink();\n> + ret = device->setFormat(srcPad->index(), &sensorFormat);\n> + if (ret) {\n> + LOG(RPI, Error) << \"Failed to set format on \" << device->entity()->name()\n> + << \" pad \" << srcPad->index()\n> + << \" with format \" << format.toString()\n> + << \": \" << ret;\n> + return ret;\n> + }\n> +\n> + LOG(RPI, Info) << \"Configured media link on device \" << device->entity()->name()\n> + << \" on pad \" << srcPad->index();\n> + }\n> +\n> return ret;\n> }\n>\n> @@ -1098,6 +1138,13 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> if (data->sensor_->init())\n> return -EINVAL;\n>\n> + /*\n> + * Enumerate all the Video Mux/Bridge devices across the sensor -> unicam\n> + * link. There may be a cascade of devices in this link!\n> + */\n> + MediaLink *link = sensorEntity->getPadByIndex(0)->links()[0];\n> + data->enumerateVideoDevices(link);\n> +\n> data->sensorFormats_ = populateSensorFormats(data->sensor_);\n>\n> ipa::RPi::SensorConfig sensorConfig;\n> @@ -1447,6 +1494,98 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> return 0;\n> }\n>\n> +/*\n> + * enumerateVideoDevices() iterates over the Media Controller topology, starting\n> + * at the sensor and finishing at Unicam. For each sensor, RPiCameraData stores\n> + * a unique list of any intermediate video mux or bridge devices connected in a\n> + * cascade, together with the entity to entity link.\n> + *\n> + * Entity pad configuration and link enabling happens at the end of configure().\n> + * We first disables all pad links on each entity device in the chain, and then\n\ns/disables/disable/\n\n> + * selectively enabling the specific links to link sensor to Unicam across all\n> + * intermediate muxes and bridges.\n> + *\n> + * In the cascaded topology below, if Sensor1 is used, the Mux2 -> Mux1 link\n> + * will be disabled, and Sensor1 -> Mux1 -> Unicam links enabled. Alternatively,\n> + * if Sensor3 is used, the Sensor2 -> Mux2 and Sensor1 -> Mux1 links are disabled,\n> + * and Sensor3 -> Mux2 -> Mux1 -> Unicam links are enabled. All other links will\n> + * remain unchanged.\n> + *\n> + * +----------+\n> + * | Unicam |\n> + * +-----^----+\n> + * |\n> + * +---+---+\n> + * | Mux1 <-------+\n> + * +--^----+ |\n> + * | |\n> + * +-----+---+ +---+---+\n> + * | Sensor1 | | Mux2 |<--+\n> + * +---------+ +-^-----+ |\n> + * | |\n> + * +-------+-+ +---+-----+\n> + * | Sensor2 | | Sensor3 |\n> + * +---------+ +---------+\n> + */\n> +void RPiCameraData::enumerateVideoDevices(MediaLink *link)\n> +{\n> + const MediaPad *sinkPad = link->sink();\n> + const MediaEntity *entity = sinkPad->entity();\n> + bool unicamFound = false;\n> +\n> + /* We only deal with Video Mux and Bridge devices in cascade. */\n> + if (entity->function() != MEDIA_ENT_F_VID_MUX &&\n> + entity->function() != MEDIA_ENT_F_VID_IF_BRIDGE)\n> + return;\n> +\n> + /* Find the source pad for this Video Mux or Bridge device. */\n> + const MediaPad *entitySourcePad = nullptr;\n> + for (const MediaPad *pad : entity->pads()) {\n> + if (pad->flags() & MEDIA_PAD_FL_SOURCE) {\n> + /*\n> + * We can only deal with devices that have a single source\n> + * pad. If this device has multple source pads, ignore it\n> + * and this branch in the cascade.\n> + */\n> + if (entitySourcePad)\n> + return;\n> +\n> + entitySourcePad = pad;\n> + }\n> + }\n> +\n> + LOG(RPI, Info) << \"Found video mux device \" << entity->name()\n> + << \" linked to sink pad \" << sinkPad->index();\n\nI can see the value during development, but might \"Info\" otherwise get\na bit wordy? Don't really mind though, I guess it won't bother\n\"regular\" users.\n\nBut it all LGTM, so:\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> +\n> + bridgeDevices_.emplace_back(std::make_unique(entity), link);\n> + bridgeDevices_.back().first->open();\n> +\n> + /*\n> + * Iterate through all the sink pad links down the cascade to find any\n> + * other Video Mux and Bridge devices.\n> + */\n> + for (MediaLink *l : entitySourcePad->links()) {\n> + enumerateVideoDevices(l);\n> + /* Once we reach the Unicam entity, we are done. */\n> + if (l->sink()->entity()->name() == \"unicam-image\") {\n> + unicamFound = true;\n> + break;\n> + }\n> + }\n> +\n> + /* This identifies the end of our entity enumeration recursion. */\n> + if (link->source()->entity()->function() == MEDIA_ENT_F_CAM_SENSOR) {\n> + /*\n> + * If Unicam is not at the end of this cascade, we cannot configure\n> + * this topology automatically, so remove all entity references.\n> + */\n> + if (!unicamFound) {\n> + LOG(RPI, Warning) << \"Cannot automatically configure this MC topology!\";\n> + bridgeDevices_.clear();\n> + }\n> + }\n> +}\n> +\n> void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &controls)\n> {\n> if (state_ == State::Stopped)\n> --\n> 2.25.1\n>","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 EC082BF415\n\tfor ;\n\tTue, 21 Dec 2021 10:53:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E021608A2;\n\tTue, 21 Dec 2021 11:53:19 +0100 (CET)","from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com\n\t[IPv6:2a00:1450:4864:20::42b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 56A1D60113\n\tfor ;\n\tTue, 21 Dec 2021 11:53:17 +0100 (CET)","by mail-wr1-x42b.google.com with SMTP id i22so26014960wrb.13\n\tfor ;\n\tTue, 21 Dec 2021 02:53:17 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"V8f1Mwhx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=WwPhG2/IO2ty037aR36fOGaecx7J0I1GUAHHzmNw580=;\n\tb=V8f1MwhxqG833Yo5znrY5cMzj2rQi3Oq/o7SQEWzXzPp2Wh2pArIkLqSiojS62obMR\n\tcTllacxRp0WhgcvyirjgjYS8nRo02pnZchUG5VdBWEVYb8ESTDsCe/L3RMgqz8r0lTOF\n\tJTaPZOxGYyaF7ZEKmE9D0gUN+gqFySHBIQYTFid+h4NjeiBUjvyBgC1ogx6zB73EjUGI\n\tr0iIumQjOG78i7wIi5viyN9gV9yDRMQhKblw4KwMxRvnZtN6dKgMXxS/Cpr9xDSEhNjG\n\tOaH/sDoAGyC1gXpOLxjLJ9O0a3eHlp74JHhX3jt8We1mBS50izPbvUnUswgll1ihm2fP\n\tLLxA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=WwPhG2/IO2ty037aR36fOGaecx7J0I1GUAHHzmNw580=;\n\tb=omSjLrZPsEtAQcGwRuyfW9m+iHuSSxgEn43xY3yoMF5/ZDZ0PfqD6bvG29G64cgBs4\n\tsij4VzGIjlZTBxBpJ5UtkjMBO7Yi1jzoUQO2+mdTLRGBJ1SNJZ3WBEFT6kYD3vR/5mua\n\t2zBa/aTZ3Bf9HvhaM2uXGBjyreRHJYHw2Zt6AxERdOE81FNwHaZhenZEJr3HJrEVNDTD\n\t7LJBkEAPOH0jq3U4QHYaiKHAOTDRkuv53406wvR8PeO+d6NzvTD8OdiZl6JjZVCmk8TW\n\tbBDgSU64zbLlabNMQGxaDf29BDBwQFGOUwhxx3uKoj259tH7X7zcmRxDIXLnX/ADo3n5\n\t3QQQ==","X-Gm-Message-State":"AOAM5314DY9TbgvJmzCi7/n9J7kH7RlwhBgLn5w4Z7SUmztoIWufb/VC\n\t0g7DyxNA/ehPdEnHZmQFeTySZ8UnBp+JQwb7XA0K4uii7w4=","X-Google-Smtp-Source":"ABdhPJzzJYqO76G5FIcn92GUqZkG1mtz4gTmvEsHHXtGF8xK37q7qzCgcFtcpxrTTGV3Mex+QNXtvU37KvHLTn9X8v8=","X-Received":"by 2002:adf:f801:: with SMTP id s1mr2163391wrp.3.1640083996795; \n\tTue, 21 Dec 2021 02:53:16 -0800 (PST)","MIME-Version":"1.0","References":"<20211214140002.3552445-1-naush@raspberrypi.com>\n\t<20211214140002.3552445-3-naush@raspberrypi.com>","In-Reply-To":"<20211214140002.3552445-3-naush@raspberrypi.com>","From":"David Plowman ","Date":"Tue, 21 Dec 2021 10:53:06 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21856,"web_url":"https://patchwork.libcamera.org/comment/21856/","msgid":"","date":"2021-12-21T16:59:22","subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Tue, Dec 14, 2021 at 02:00:02PM +0000, Naushir Patuck wrote:\n> This change will allow the pipeline handler to enumerate and control Video\n> Mux or Bridge devices that may be attached between sensors and a particular\n> Unicam instance. Cascaded mux or bridge devices are also handled.\n> \n> A new member function RPiCameraData::enumerateVideoDevices(), called from\n> PipelineHandlerRPi::registerCamera(), is used to identify and open all mux and\n> bridge subdevices present in the sensor -> Unicam link.\n> \n> Relevent links are enabled/disabled and pad formats correctly set in\n> PipelineHandlerRPi::configure() before the camera is started.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Kieran Bingham \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 139 ++++++++++++++++++\n> 1 file changed, 139 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 2a2fb5273eb8..4ec646d3c7a3 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -191,6 +192,8 @@ public:\n> \tint loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> \tint configureIPA(const CameraConfiguration *config);\n> \n> +\tvoid enumerateVideoDevices(MediaLink *link);\n> +\n> \tvoid statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n> \tvoid runIsp(uint32_t bufferId);\n> \tvoid embeddedComplete(uint32_t bufferId);\n> @@ -220,6 +223,11 @@ public:\n> \tstd::vector streams_;\n> \t/* Stores the ids of the buffers mapped in the IPA. */\n> \tstd::unordered_set ipaBuffers_;\n> +\t/*\n> +\t * Stores a cascade of Video Mux or Bridge devices between the sensor and\n> +\t * Unicam together with media link across the entities.\n> +\t */\n> +\tstd::vector, MediaLink *>> bridgeDevices_;\n> \n> \t/* DMAHEAP allocation helper. */\n> \tRPi::DmaHeap dmaHeap_;\n> @@ -868,6 +876,38 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t */\n> \tdata->properties_.set(properties::ScalerCropMaximum, data->sensorInfo_.analogCrop);\n> \n> +\t/* Setup the Video Mux/Bridge entities. */\n> +\tfor (auto &[device, link] : data->bridgeDevices_) {\n> +\t\t/*\n> +\t\t * Start by disabling all the sink pad links on the devices in the\n> +\t\t * cascade, with the exception of the link connecting the device.\n> +\t\t */\n> +\t\tfor (const MediaPad *p : device->entity()->pads()) {\n> +\t\t\tif (!(p->flags() & MEDIA_PAD_FL_SINK))\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tfor (MediaLink *l : p->links()) {\n> +\t\t\t\tif (l != link)\n> +\t\t\t\t\tl->setEnabled(false);\n> +\t\t\t}\n> +\t\t}\n\nOnce the subdev routing API lands it would be great if the muxes could\nimplement it...\n\n> +\n> +\t\t/* Next, enable the entity -> entity links, and setup the pad format. */\n> +\t\tlink->setEnabled(true);\n> +\t\tconst MediaPad *srcPad = link->sink();\n\nThat looks weird, *source* pad = link->*sink*() ?\n\n> +\t\tret = device->setFormat(srcPad->index(), &sensorFormat);\n> +\t\tif (ret) {\n> +\t\t\tLOG(RPI, Error) << \"Failed to set format on \" << device->entity()->name()\n> +\t\t\t\t\t<< \" pad \" << srcPad->index()\n> +\t\t\t\t\t<< \" with format \" << format.toString()\n> +\t\t\t\t\t<< \": \" << ret;\n> +\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tLOG(RPI, Info) << \"Configured media link on device \" << device->entity()->name()\n> +\t\t\t << \" on pad \" << srcPad->index();\n\nMaybe Debug instead of Info ? Let's not be too verbose by default.\n\nWhen propagating formats you're supposed to then read the format from\nthe source pad and propagate it. Some bridges may change the media bus\ncode for instance.\n\nThis may be handled later if needed, but it would then be nice to record\nit in a comment in the code.\n\n> +\t}\n> +\n> \treturn ret;\n> }\n> \n> @@ -1098,6 +1138,13 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> \tif (data->sensor_->init())\n> \t\treturn -EINVAL;\n> \n> +\t/*\n> +\t * Enumerate all the Video Mux/Bridge devices across the sensor -> unicam\n> +\t * link. There may be a cascade of devices in this link!\n> +\t */\n> +\tMediaLink *link = sensorEntity->getPadByIndex(0)->links()[0];\n> +\tdata->enumerateVideoDevices(link);\n\nWouldn't it be clearer to pass the sensor pointer to this function\ninstead of the link ? You mention \"sensor -> unicam link\" in the\ncomment, which doesn't match what MediaLink models which is a\npoint-to-point link, not a connection through multiple entities.\n\n> +\n> \tdata->sensorFormats_ = populateSensorFormats(data->sensor_);\n> \n> \tipa::RPi::SensorConfig sensorConfig;\n> @@ -1447,6 +1494,98 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \treturn 0;\n> }\n> \n> +/*\n> + * enumerateVideoDevices() iterates over the Media Controller topology, starting\n> + * at the sensor and finishing at Unicam. For each sensor, RPiCameraData stores\n> + * a unique list of any intermediate video mux or bridge devices connected in a\n> + * cascade, together with the entity to entity link.\n> + *\n> + * Entity pad configuration and link enabling happens at the end of configure().\n> + * We first disables all pad links on each entity device in the chain, and then\n> + * selectively enabling the specific links to link sensor to Unicam across all\n> + * intermediate muxes and bridges.\n> + *\n> + * In the cascaded topology below, if Sensor1 is used, the Mux2 -> Mux1 link\n> + * will be disabled, and Sensor1 -> Mux1 -> Unicam links enabled. Alternatively,\n> + * if Sensor3 is used, the Sensor2 -> Mux2 and Sensor1 -> Mux1 links are disabled,\n> + * and Sensor3 -> Mux2 -> Mux1 -> Unicam links are enabled. All other links will\n> + * remain unchanged.\n> + *\n> + * +----------+\n> + * | Unicam |\n> + * +-----^----+\n> + * |\n> + * +---+---+\n> + * | Mux1 <-------+\n> + * +--^----+ |\n> + * | |\n> + * +-----+---+ +---+---+\n> + * | Sensor1 | | Mux2 |<--+\n> + * +---------+ +-^-----+ |\n> + * | |\n> + * +-------+-+ +---+-----+\n> + * | Sensor2 | | Sensor3 |\n> + * +---------+ +---------+\n> + */\n> +void RPiCameraData::enumerateVideoDevices(MediaLink *link)\n> +{\n> +\tconst MediaPad *sinkPad = link->sink();\n> +\tconst MediaEntity *entity = sinkPad->entity();\n> +\tbool unicamFound = false;\n> +\n> +\t/* We only deal with Video Mux and Bridge devices in cascade. */\n> +\tif (entity->function() != MEDIA_ENT_F_VID_MUX &&\n> +\t entity->function() != MEDIA_ENT_F_VID_IF_BRIDGE)\n> +\t\treturn;\n> +\n> +\t/* Find the source pad for this Video Mux or Bridge device. */\n> +\tconst MediaPad *entitySourcePad = nullptr;\n> +\tfor (const MediaPad *pad : entity->pads()) {\n> +\t\tif (pad->flags() & MEDIA_PAD_FL_SOURCE) {\n> +\t\t\t/*\n> +\t\t\t * We can only deal with devices that have a single source\n> +\t\t\t * pad. If this device has multple source pads, ignore it\n> +\t\t\t * and this branch in the cascade.\n> +\t\t\t */\n> +\t\t\tif (entitySourcePad)\n> +\t\t\t\treturn;\n> +\n> +\t\t\tentitySourcePad = pad;\n> +\t\t}\n> +\t}\n> +\n> +\tLOG(RPI, Info) << \"Found video mux device \" << entity->name()\n> +\t\t << \" linked to sink pad \" << sinkPad->index();\n\nSame here, Debug ?\n\n> +\n> +\tbridgeDevices_.emplace_back(std::make_unique(entity), link);\n> +\tbridgeDevices_.back().first->open();\n> +\n> +\t/*\n> +\t * Iterate through all the sink pad links down the cascade to find any\n> +\t * other Video Mux and Bridge devices.\n> +\t */\n> +\tfor (MediaLink *l : entitySourcePad->links()) {\n> +\t\tenumerateVideoDevices(l);\n> +\t\t/* Once we reach the Unicam entity, we are done. */\n> +\t\tif (l->sink()->entity()->name() == \"unicam-image\") {\n> +\t\t\tunicamFound = true;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\t/* This identifies the end of our entity enumeration recursion. */\n> +\tif (link->source()->entity()->function() == MEDIA_ENT_F_CAM_SENSOR) {\n> +\t\t/*\n> +\t\t* If Unicam is not at the end of this cascade, we cannot configure\n> +\t\t* this topology automatically, so remove all entity references.\n> +\t\t*/\n> +\t\tif (!unicamFound) {\n> +\t\t\tLOG(RPI, Warning) << \"Cannot automatically configure this MC topology!\";\n> +\t\t\tbridgeDevices_.clear();\n> +\t\t}\n> +\t}\n> +}\n> +\n> void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &controls)\n> {\n> \tif (state_ == State::Stopped)","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 4DB19BE080\n\tfor ;\n\tTue, 21 Dec 2021 16:59:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A09F4608F9;\n\tTue, 21 Dec 2021 17:59:27 +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 7FA07605A8\n\tfor ;\n\tTue, 21 Dec 2021 17:59:26 +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 EC68C881;\n\tTue, 21 Dec 2021 17:59:25 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mIGIxKhi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1640105966;\n\tbh=TdhjTboKE2VWBtqMYL4wjjtaCEI6JMeOXYehayUov88=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mIGIxKhiiVhll/FmeGSGxlt7SyOujC3mcA8UehGotKMd/s6Hp1mIc6aXgZmW1d/uG\n\tVnHOncMy9CmXi16v2gIfPzjO4BbNAoawyUjPuLVkSL5noZ2dVGaitLu8T3nut+ESbG\n\tMyuMosm8gZ0M5C1iB1NRPsiJV9LoguTee/6h7ONI=","Date":"Tue, 21 Dec 2021 18:59:22 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20211214140002.3552445-1-naush@raspberrypi.com>\n\t<20211214140002.3552445-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211214140002.3552445-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21860,"web_url":"https://patchwork.libcamera.org/comment/21860/","msgid":"<20211221180501.hyjtauom32vi3bq3@uno.localdomain>","date":"2021-12-21T18:05:01","subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Naush\n\nOn Tue, Dec 14, 2021 at 02:00:02PM +0000, Naushir Patuck wrote:\n> This change will allow the pipeline handler to enumerate and control Video\n> Mux or Bridge devices that may be attached between sensors and a particular\n> Unicam instance. Cascaded mux or bridge devices are also handled.\n>\n> A new member function RPiCameraData::enumerateVideoDevices(), called from\n> PipelineHandlerRPi::registerCamera(), is used to identify and open all mux and\n> bridge subdevices present in the sensor -> Unicam link.\n>\n> Relevent links are enabled/disabled and pad formats correctly set in\n> PipelineHandlerRPi::configure() before the camera is started.\n>\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Kieran Bingham \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 139 ++++++++++++++++++\n> 1 file changed, 139 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 2a2fb5273eb8..4ec646d3c7a3 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n> #include \n> @@ -191,6 +192,8 @@ public:\n> \tint loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> \tint configureIPA(const CameraConfiguration *config);\n>\n> +\tvoid enumerateVideoDevices(MediaLink *link);\n> +\n> \tvoid statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n> \tvoid runIsp(uint32_t bufferId);\n> \tvoid embeddedComplete(uint32_t bufferId);\n> @@ -220,6 +223,11 @@ public:\n> \tstd::vector streams_;\n> \t/* Stores the ids of the buffers mapped in the IPA. */\n> \tstd::unordered_set ipaBuffers_;\n> +\t/*\n> +\t * Stores a cascade of Video Mux or Bridge devices between the sensor and\n> +\t * Unicam together with media link across the entities.\n> +\t */\n> +\tstd::vector, MediaLink *>> bridgeDevices_;\n>\n> \t/* DMAHEAP allocation helper. */\n> \tRPi::DmaHeap dmaHeap_;\n> @@ -868,6 +876,38 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t */\n> \tdata->properties_.set(properties::ScalerCropMaximum, data->sensorInfo_.analogCrop);\n>\n> +\t/* Setup the Video Mux/Bridge entities. */\n> +\tfor (auto &[device, link] : data->bridgeDevices_) {\n> +\t\t/*\n> +\t\t * Start by disabling all the sink pad links on the devices in the\n> +\t\t * cascade, with the exception of the link connecting the device.\n> +\t\t */\n> +\t\tfor (const MediaPad *p : device->entity()->pads()) {\n> +\t\t\tif (!(p->flags() & MEDIA_PAD_FL_SINK))\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tfor (MediaLink *l : p->links()) {\n> +\t\t\t\tif (l != link)\n> +\t\t\t\t\tl->setEnabled(false);\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\t/* Next, enable the entity -> entity links, and setup the pad format. */\n> +\t\tlink->setEnabled(true);\n> +\t\tconst MediaPad *srcPad = link->sink();\n\nsinkPad ?\n\n> +\t\tret = device->setFormat(srcPad->index(), &sensorFormat);\n> +\t\tif (ret) {\n> +\t\t\tLOG(RPI, Error) << \"Failed to set format on \" << device->entity()->name()\n> +\t\t\t\t\t<< \" pad \" << srcPad->index()\n> +\t\t\t\t\t<< \" with format \" << format.toString()\n> +\t\t\t\t\t<< \": \" << ret;\n> +\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tLOG(RPI, Info) << \"Configured media link on device \" << device->entity()->name()\n> +\t\t\t << \" on pad \" << srcPad->index();\n> +\t}\n> +\n> \treturn ret;\n> }\n>\n> @@ -1098,6 +1138,13 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> \tif (data->sensor_->init())\n> \t\treturn -EINVAL;\n>\n> +\t/*\n> +\t * Enumerate all the Video Mux/Bridge devices across the sensor -> unicam\n> +\t * link. There may be a cascade of devices in this link!\n> +\t */\n> +\tMediaLink *link = sensorEntity->getPadByIndex(0)->links()[0];\n> +\tdata->enumerateVideoDevices(link);\n> +\n> \tdata->sensorFormats_ = populateSensorFormats(data->sensor_);\n>\n> \tipa::RPi::SensorConfig sensorConfig;\n> @@ -1447,6 +1494,98 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \treturn 0;\n> }\n>\n> +/*\n> + * enumerateVideoDevices() iterates over the Media Controller topology, starting\n> + * at the sensor and finishing at Unicam. For each sensor, RPiCameraData stores\n> + * a unique list of any intermediate video mux or bridge devices connected in a\n> + * cascade, together with the entity to entity link.\n> + *\n> + * Entity pad configuration and link enabling happens at the end of configure().\n> + * We first disables all pad links on each entity device in the chain, and then\n> + * selectively enabling the specific links to link sensor to Unicam across all\n> + * intermediate muxes and bridges.\n> + *\n> + * In the cascaded topology below, if Sensor1 is used, the Mux2 -> Mux1 link\n> + * will be disabled, and Sensor1 -> Mux1 -> Unicam links enabled. Alternatively,\n> + * if Sensor3 is used, the Sensor2 -> Mux2 and Sensor1 -> Mux1 links are disabled,\n> + * and Sensor3 -> Mux2 -> Mux1 -> Unicam links are enabled. All other links will\n> + * remain unchanged.\n> + *\n> + * +----------+\n> + * | Unicam |\n> + * +-----^----+\n> + * |\n> + * +---+---+\n> + * | Mux1 <-------+\n> + * +--^----+ |\n> + * | |\n> + * +-----+---+ +---+---+\n> + * | Sensor1 | | Mux2 |<--+\n> + * +---------+ +-^-----+ |\n> + * | |\n> + * +-------+-+ +---+-----+\n> + * | Sensor2 | | Sensor3 |\n> + * +---------+ +---------+\n> + */\n> +void RPiCameraData::enumerateVideoDevices(MediaLink *link)\n> +{\n> +\tconst MediaPad *sinkPad = link->sink();\n> +\tconst MediaEntity *entity = sinkPad->entity();\n> +\tbool unicamFound = false;\n> +\n> +\t/* We only deal with Video Mux and Bridge devices in cascade. */\n> +\tif (entity->function() != MEDIA_ENT_F_VID_MUX &&\n> +\t entity->function() != MEDIA_ENT_F_VID_IF_BRIDGE)\n> +\t\treturn;\n> +\n> +\t/* Find the source pad for this Video Mux or Bridge device. */\n> +\tconst MediaPad *entitySourcePad = nullptr;\n> +\tfor (const MediaPad *pad : entity->pads()) {\n> +\t\tif (pad->flags() & MEDIA_PAD_FL_SOURCE) {\n> +\t\t\t/*\n> +\t\t\t * We can only deal with devices that have a single source\n> +\t\t\t * pad. If this device has multple source pads, ignore it\n> +\t\t\t * and this branch in the cascade.\n> +\t\t\t */\n> +\t\t\tif (entitySourcePad)\n> +\t\t\t\treturn;\n\nProbably failing loud and saying that a bridge/mux with multiple\nsource pads cannot be handled is more explicit ?\nAh you will fail later though at the end of recursion...\n\n\n> +\n> +\t\t\tentitySourcePad = pad;\n\nCan this just be sourcePad ? Or were you trying to express that this\nlives in the 'next' entity and it's not the link's source pad ?\n\n> +\t\t}\n> +\t}\n> +\n> +\tLOG(RPI, Info) << \"Found video mux device \" << entity->name()\n> +\t\t << \" linked to sink pad \" << sinkPad->index();\n> +\n> +\tbridgeDevices_.emplace_back(std::make_unique(entity), link);\n> +\tbridgeDevices_.back().first->open();\n> +\n> +\t/*\n> +\t * Iterate through all the sink pad links down the cascade to find any\n> +\t * other Video Mux and Bridge devices.\n> +\t */\n> +\tfor (MediaLink *l : entitySourcePad->links()) {\n> +\t\tenumerateVideoDevices(l);\n> +\t\t/* Once we reach the Unicam entity, we are done. */\n> +\t\tif (l->sink()->entity()->name() == \"unicam-image\") {\n> +\t\t\tunicamFound = true;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\t/* This identifies the end of our entity enumeration recursion. */\n> +\tif (link->source()->entity()->function() == MEDIA_ENT_F_CAM_SENSOR) {\n> +\t\t/*\n> +\t\t* If Unicam is not at the end of this cascade, we cannot configure\n> +\t\t* this topology automatically, so remove all entity references.\n> +\t\t*/\n> +\t\tif (!unicamFound) {\n> +\t\t\tLOG(RPI, Warning) << \"Cannot automatically configure this MC topology!\";\n> +\t\t\tbridgeDevices_.clear();\n> +\t\t}\n> +\t}\n> +}\n> +\n> void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &controls)\n> {\n> \tif (state_ == State::Stopped)\n> --\n> 2.25.1\n>","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 DC00FBE080\n\tfor ;\n\tTue, 21 Dec 2021 18:04:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 159FC608F9;\n\tTue, 21 Dec 2021 19:04:09 +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 C1BAD605A8\n\tfor ;\n\tTue, 21 Dec 2021 19:04:07 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 2F6161BF211;\n\tTue, 21 Dec 2021 18:04:06 +0000 (UTC)"],"Date":"Tue, 21 Dec 2021 19:05:01 +0100","From":"Jacopo Mondi ","To":"Naushir Patuck ","Message-ID":"<20211221180501.hyjtauom32vi3bq3@uno.localdomain>","References":"<20211214140002.3552445-1-naush@raspberrypi.com>\n\t<20211214140002.3552445-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211214140002.3552445-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21940,"web_url":"https://patchwork.libcamera.org/comment/21940/","msgid":"","date":"2022-01-04T11:15:16","subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your feedback.\n\nOn Tue, 21 Dec 2021 at 10:53, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for this patch!\n>\n> On Tue, 14 Dec 2021 at 14:00, Naushir Patuck \n> wrote:\n> >\n> > This change will allow the pipeline handler to enumerate and control\n> Video\n> > Mux or Bridge devices that may be attached between sensors and a\n> particular\n> > Unicam instance. Cascaded mux or bridge devices are also handled.\n> >\n> > A new member function RPiCameraData::enumerateVideoDevices(), called from\n> > PipelineHandlerRPi::registerCamera(), is used to identify and open all\n> mux and\n> > bridge subdevices present in the sensor -> Unicam link.\n> >\n> > Relevent links are enabled/disabled and pad formats correctly set in\n>\n> Maybe s/Relevent/Relevant/ ? But I could be wrong. It is also the most\n> extreme hair-splitting!\n>\n\nAck.\n\n\n>\n> > PipelineHandlerRPi::configure() before the camera is started.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 139 ++++++++++++++++++\n> > 1 file changed, 139 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 2a2fb5273eb8..4ec646d3c7a3 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> >\n> > #include \n> > #include \n> > @@ -191,6 +192,8 @@ public:\n> > int loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> > int configureIPA(const CameraConfiguration *config);\n> >\n> > + void enumerateVideoDevices(MediaLink *link);\n> > +\n> > void statsMetadataComplete(uint32_t bufferId, const ControlList\n> &controls);\n> > void runIsp(uint32_t bufferId);\n> > void embeddedComplete(uint32_t bufferId);\n> > @@ -220,6 +223,11 @@ public:\n> > std::vector streams_;\n> > /* Stores the ids of the buffers mapped in the IPA. */\n> > std::unordered_set ipaBuffers_;\n> > + /*\n> > + * Stores a cascade of Video Mux or Bridge devices between the\n> sensor and\n> > + * Unicam together with media link across the entities.\n> > + */\n> > + std::vector, MediaLink\n> *>> bridgeDevices_;\n> >\n> > /* DMAHEAP allocation helper. */\n> > RPi::DmaHeap dmaHeap_;\n> > @@ -868,6 +876,38 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > */\n> > data->properties_.set(properties::ScalerCropMaximum,\n> data->sensorInfo_.analogCrop);\n> >\n> > + /* Setup the Video Mux/Bridge entities. */\n> > + for (auto &[device, link] : data->bridgeDevices_) {\n> > + /*\n> > + * Start by disabling all the sink pad links on the\n> devices in the\n> > + * cascade, with the exception of the link connecting\n> the device.\n> > + */\n> > + for (const MediaPad *p : device->entity()->pads()) {\n> > + if (!(p->flags() & MEDIA_PAD_FL_SINK))\n> > + continue;\n> > +\n> > + for (MediaLink *l : p->links()) {\n> > + if (l != link)\n> > + l->setEnabled(false);\n> > + }\n> > + }\n> > +\n> > + /* Next, enable the entity -> entity links, and setup\n> the pad format. */\n> > + link->setEnabled(true);\n> > + const MediaPad *srcPad = link->sink();\n> > + ret = device->setFormat(srcPad->index(), &sensorFormat);\n> > + if (ret) {\n> > + LOG(RPI, Error) << \"Failed to set format on \" <<\n> device->entity()->name()\n> > + << \" pad \" << srcPad->index()\n> > + << \" with format \" <<\n> format.toString()\n> > + << \": \" << ret;\n> > + return ret;\n> > + }\n> > +\n> > + LOG(RPI, Info) << \"Configured media link on device \" <<\n> device->entity()->name()\n> > + << \" on pad \" << srcPad->index();\n> > + }\n> > +\n> > return ret;\n> > }\n> >\n> > @@ -1098,6 +1138,13 @@ int\n> PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> > if (data->sensor_->init())\n> > return -EINVAL;\n> >\n> > + /*\n> > + * Enumerate all the Video Mux/Bridge devices across the sensor\n> -> unicam\n> > + * link. There may be a cascade of devices in this link!\n> > + */\n> > + MediaLink *link = sensorEntity->getPadByIndex(0)->links()[0];\n> > + data->enumerateVideoDevices(link);\n> > +\n> > data->sensorFormats_ = populateSensorFormats(data->sensor_);\n> >\n> > ipa::RPi::SensorConfig sensorConfig;\n> > @@ -1447,6 +1494,98 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> > return 0;\n> > }\n> >\n> > +/*\n> > + * enumerateVideoDevices() iterates over the Media Controller topology,\n> starting\n> > + * at the sensor and finishing at Unicam. For each sensor,\n> RPiCameraData stores\n> > + * a unique list of any intermediate video mux or bridge devices\n> connected in a\n> > + * cascade, together with the entity to entity link.\n> > + *\n> > + * Entity pad configuration and link enabling happens at the end of\n> configure().\n> > + * We first disables all pad links on each entity device in the chain,\n> and then\n>\n> s/disables/disable/\n>\n> > + * selectively enabling the specific links to link sensor to Unicam\n> across all\n> > + * intermediate muxes and bridges.\n> > + *\n> > + * In the cascaded topology below, if Sensor1 is used, the Mux2 -> Mux1\n> link\n> > + * will be disabled, and Sensor1 -> Mux1 -> Unicam links enabled.\n> Alternatively,\n> > + * if Sensor3 is used, the Sensor2 -> Mux2 and Sensor1 -> Mux1 links\n> are disabled,\n> > + * and Sensor3 -> Mux2 -> Mux1 -> Unicam links are enabled. All other\n> links will\n> > + * remain unchanged.\n> > + *\n> > + * +----------+\n> > + * | Unicam |\n> > + * +-----^----+\n> > + * |\n> > + * +---+---+\n> > + * | Mux1 <-------+\n> > + * +--^----+ |\n> > + * | |\n> > + * +-----+---+ +---+---+\n> > + * | Sensor1 | | Mux2 |<--+\n> > + * +---------+ +-^-----+ |\n> > + * | |\n> > + * +-------+-+ +---+-----+\n> > + * | Sensor2 | | Sensor3 |\n> > + * +---------+ +---------+\n> > + */\n> > +void RPiCameraData::enumerateVideoDevices(MediaLink *link)\n> > +{\n> > + const MediaPad *sinkPad = link->sink();\n> > + const MediaEntity *entity = sinkPad->entity();\n> > + bool unicamFound = false;\n> > +\n> > + /* We only deal with Video Mux and Bridge devices in cascade. */\n> > + if (entity->function() != MEDIA_ENT_F_VID_MUX &&\n> > + entity->function() != MEDIA_ENT_F_VID_IF_BRIDGE)\n> > + return;\n> > +\n> > + /* Find the source pad for this Video Mux or Bridge device. */\n> > + const MediaPad *entitySourcePad = nullptr;\n> > + for (const MediaPad *pad : entity->pads()) {\n> > + if (pad->flags() & MEDIA_PAD_FL_SOURCE) {\n> > + /*\n> > + * We can only deal with devices that have a\n> single source\n> > + * pad. If this device has multple source pads,\n> ignore it\n> > + * and this branch in the cascade.\n> > + */\n> > + if (entitySourcePad)\n> > + return;\n> > +\n> > + entitySourcePad = pad;\n> > + }\n> > + }\n> > +\n> > + LOG(RPI, Info) << \"Found video mux device \" << entity->name()\n> > + << \" linked to sink pad \" << sinkPad->index();\n>\n> I can see the value during development, but might \"Info\" otherwise get\n> a bit wordy? Don't really mind though, I guess it won't bother\n> \"regular\" users.\n>\n\nWill do!\n\n\n\n> But it all LGTM, so:\n>\n> Reviewed-by: David Plowman \n>\n\nThanks!\nNaush\n\n\n\n>\n> Thanks!\n> David\n>\n> > +\n> > +\n> bridgeDevices_.emplace_back(std::make_unique(entity), link);\n> > + bridgeDevices_.back().first->open();\n> > +\n> > + /*\n> > + * Iterate through all the sink pad links down the cascade to\n> find any\n> > + * other Video Mux and Bridge devices.\n> > + */\n> > + for (MediaLink *l : entitySourcePad->links()) {\n> > + enumerateVideoDevices(l);\n> > + /* Once we reach the Unicam entity, we are done. */\n> > + if (l->sink()->entity()->name() == \"unicam-image\") {\n> > + unicamFound = true;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + /* This identifies the end of our entity enumeration recursion.\n> */\n> > + if (link->source()->entity()->function() ==\n> MEDIA_ENT_F_CAM_SENSOR) {\n> > + /*\n> > + * If Unicam is not at the end of this cascade, we cannot\n> configure\n> > + * this topology automatically, so remove all entity\n> references.\n> > + */\n> > + if (!unicamFound) {\n> > + LOG(RPI, Warning) << \"Cannot automatically\n> configure this MC topology!\";\n> > + bridgeDevices_.clear();\n> > + }\n> > + }\n> > +}\n> > +\n> > void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const\n> ControlList &controls)\n> > {\n> > if (state_ == State::Stopped)\n> > --\n> > 2.25.1\n> >\n>","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 16618BE080\n\tfor ;\n\tTue, 4 Jan 2022 11:15:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70CE560927;\n\tTue, 4 Jan 2022 12:15:34 +0100 (CET)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 10FDF604F4\n\tfor ;\n\tTue, 4 Jan 2022 12:15:33 +0100 (CET)","by mail-lf1-x12b.google.com with SMTP id o12so81062020lfk.1\n\tfor ;\n\tTue, 04 Jan 2022 03:15:33 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"PAFVNTGC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=aKbsWx09ZX3awxQxill+LF86U45Ybyg0f+p1AsN+Yow=;\n\tb=PAFVNTGCr67sxbazf1G4a3aT/q9ampXLLiVHINbrer9gNggpzqo5XM1uDFLWH+cGfq\n\tcJ5GowGd8NzMFtz6o5yYMgsBSpBABAg4rSmMjnMB1HPCHg4u647CdQLJ3ZhsqXMFiWkR\n\tkcXxn+2nlDQ4YVzh+nGc4O041wvKPDfzfYPNxDp43uGVrELqVt+YMhWTEtS9dMkiJm3t\n\twK35E7z4opnQIgqVtcanLNReQYy9qNqwY28tIkU3M+Ji49K3J5saYkiT448s9aBe2Zl0\n\tixLfgg/lDDi888W9wNyw66zLxzI8fvvPs9LPh/bbqqhA50mtyflUqEPpGPabOPsTo5vR\n\tXwvA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=aKbsWx09ZX3awxQxill+LF86U45Ybyg0f+p1AsN+Yow=;\n\tb=JBdf2K+VzSseDj1UHBkLLyAJobhwlRGxrQ9bZ4aWAYGkfuatmSFWJSYjyUqiQqVKaP\n\tEOUjQBt2c88vo+yNMhjh0NSI6bV+jXcHPd1Z5MtajBnPCF+HaPgAty38WXE0GIUl00Gs\n\tCt5tyGF63b209J67ltflaqfqenkGsmJpsl9EGnyNrrp6EwFN/7RyK5NZUA4djpEc9wB5\n\thtbsUrfnoN4f/4roFowabL1FlD+w9HqFOlfgs6lQytaWtY3QIm7q+rSoQPmzQOTihFtH\n\tHO6uNppzjB2vs367owpz0E57alvQRk8ELnpArYjG5pb4ZLIIvsbO+xv2B9xJyrjJ6TB/\n\t2P0Q==","X-Gm-Message-State":"AOAM532NbbE4nHaD+OhZKfYZo9wxe90oY7EPrBcOz4Ox4bsvwH9mRcrd\n\tQ/CrKWvCJM7kSxVn3Gd2EV/nJf0BS2pXfgkXZo9lpQ==","X-Google-Smtp-Source":"ABdhPJy6Zwrmq2MlsZCzb3KFMzbdxx2mjcovK/Ne9774q7W2KZeXH5cH8JoWaaXoDTnTLAcTpPBDzFQxl7bVUbAFlNg=","X-Received":"by 2002:a05:6512:22c7:: with SMTP id\n\tg7mr42741297lfu.315.1641294932155; \n\tTue, 04 Jan 2022 03:15:32 -0800 (PST)","MIME-Version":"1.0","References":"<20211214140002.3552445-1-naush@raspberrypi.com>\n\t<20211214140002.3552445-3-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 4 Jan 2022 11:15:16 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000006733f905d4bfc158\"","Subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21941,"web_url":"https://patchwork.libcamera.org/comment/21941/","msgid":"","date":"2022-01-04T11:18:05","subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your feedback.\n\nOn Tue, 21 Dec 2021 at 16:59, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Tue, Dec 14, 2021 at 02:00:02PM +0000, Naushir Patuck wrote:\n> > This change will allow the pipeline handler to enumerate and control\n> Video\n> > Mux or Bridge devices that may be attached between sensors and a\n> particular\n> > Unicam instance. Cascaded mux or bridge devices are also handled.\n> >\n> > A new member function RPiCameraData::enumerateVideoDevices(), called from\n> > PipelineHandlerRPi::registerCamera(), is used to identify and open all\n> mux and\n> > bridge subdevices present in the sensor -> Unicam link.\n> >\n> > Relevent links are enabled/disabled and pad formats correctly set in\n> > PipelineHandlerRPi::configure() before the camera is started.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 139 ++++++++++++++++++\n> > 1 file changed, 139 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 2a2fb5273eb8..4ec646d3c7a3 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> >\n> > #include \n> > #include \n> > @@ -191,6 +192,8 @@ public:\n> > int loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> > int configureIPA(const CameraConfiguration *config);\n> >\n> > + void enumerateVideoDevices(MediaLink *link);\n> > +\n> > void statsMetadataComplete(uint32_t bufferId, const ControlList\n> &controls);\n> > void runIsp(uint32_t bufferId);\n> > void embeddedComplete(uint32_t bufferId);\n> > @@ -220,6 +223,11 @@ public:\n> > std::vector streams_;\n> > /* Stores the ids of the buffers mapped in the IPA. */\n> > std::unordered_set ipaBuffers_;\n> > + /*\n> > + * Stores a cascade of Video Mux or Bridge devices between the\n> sensor and\n> > + * Unicam together with media link across the entities.\n> > + */\n> > + std::vector, MediaLink\n> *>> bridgeDevices_;\n> >\n> > /* DMAHEAP allocation helper. */\n> > RPi::DmaHeap dmaHeap_;\n> > @@ -868,6 +876,38 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > */\n> > data->properties_.set(properties::ScalerCropMaximum,\n> data->sensorInfo_.analogCrop);\n> >\n> > + /* Setup the Video Mux/Bridge entities. */\n> > + for (auto &[device, link] : data->bridgeDevices_) {\n> > + /*\n> > + * Start by disabling all the sink pad links on the\n> devices in the\n> > + * cascade, with the exception of the link connecting the\n> device.\n> > + */\n> > + for (const MediaPad *p : device->entity()->pads()) {\n> > + if (!(p->flags() & MEDIA_PAD_FL_SINK))\n> > + continue;\n> > +\n> > + for (MediaLink *l : p->links()) {\n> > + if (l != link)\n> > + l->setEnabled(false);\n> > + }\n> > + }\n>\n> Once the subdev routing API lands it would be great if the muxes could\n> implement it...\n>\n> > +\n> > + /* Next, enable the entity -> entity links, and setup the\n> pad format. */\n> > + link->setEnabled(true);\n> > + const MediaPad *srcPad = link->sink();\n>\n> That looks weird, *source* pad = link->*sink*() ?\n>\n\nYes, you are correct, will fix.\n\n\n>\n> > + ret = device->setFormat(srcPad->index(), &sensorFormat);\n> > + if (ret) {\n> > + LOG(RPI, Error) << \"Failed to set format on \" <<\n> device->entity()->name()\n> > + << \" pad \" << srcPad->index()\n> > + << \" with format \" <<\n> format.toString()\n> > + << \": \" << ret;\n> > + return ret;\n> > + }\n> > +\n> > + LOG(RPI, Info) << \"Configured media link on device \" <<\n> device->entity()->name()\n> > + << \" on pad \" << srcPad->index();\n>\n> Maybe Debug instead of Info ? Let's not be too verbose by default.\n>\n\nAck.\n\n\n>\n> When propagating formats you're supposed to then read the format from\n> the source pad and propagate it. Some bridges may change the media bus\n> code for instance.\n>\n> This may be handled later if needed, but it would then be nice to record\n> it in a comment in the code.\n>\n\nI'll add a \\todo in the comment to say this.\n\n\n>\n> > + }\n> > +\n> > return ret;\n> > }\n> >\n> > @@ -1098,6 +1138,13 @@ int\n> PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> > if (data->sensor_->init())\n> > return -EINVAL;\n> >\n> > + /*\n> > + * Enumerate all the Video Mux/Bridge devices across the sensor ->\n> unicam\n> > + * link. There may be a cascade of devices in this link!\n> > + */\n> > + MediaLink *link = sensorEntity->getPadByIndex(0)->links()[0];\n> > + data->enumerateVideoDevices(link);\n>\n> Wouldn't it be clearer to pass the sensor pointer to this function\n> instead of the link ? You mention \"sensor -> unicam link\" in the\n> comment, which doesn't match what MediaLink models which is a\n> point-to-point link, not a connection through multiple entities.\n>\n\nI need to pass in the link pointer as this is what is needed to recurse\ndown the chain.\n\nYou are right about the comment being wrong, perhaps s/link/chain/?\n\n\n>\n> > +\n> > data->sensorFormats_ = populateSensorFormats(data->sensor_);\n> >\n> > ipa::RPi::SensorConfig sensorConfig;\n> > @@ -1447,6 +1494,98 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> > return 0;\n> > }\n> >\n> > +/*\n> > + * enumerateVideoDevices() iterates over the Media Controller topology,\n> starting\n> > + * at the sensor and finishing at Unicam. For each sensor,\n> RPiCameraData stores\n> > + * a unique list of any intermediate video mux or bridge devices\n> connected in a\n> > + * cascade, together with the entity to entity link.\n> > + *\n> > + * Entity pad configuration and link enabling happens at the end of\n> configure().\n> > + * We first disables all pad links on each entity device in the chain,\n> and then\n> > + * selectively enabling the specific links to link sensor to Unicam\n> across all\n> > + * intermediate muxes and bridges.\n> > + *\n> > + * In the cascaded topology below, if Sensor1 is used, the Mux2 -> Mux1\n> link\n> > + * will be disabled, and Sensor1 -> Mux1 -> Unicam links enabled.\n> Alternatively,\n> > + * if Sensor3 is used, the Sensor2 -> Mux2 and Sensor1 -> Mux1 links\n> are disabled,\n> > + * and Sensor3 -> Mux2 -> Mux1 -> Unicam links are enabled. All other\n> links will\n> > + * remain unchanged.\n> > + *\n> > + * +----------+\n> > + * | Unicam |\n> > + * +-----^----+\n> > + * |\n> > + * +---+---+\n> > + * | Mux1 <-------+\n> > + * +--^----+ |\n> > + * | |\n> > + * +-----+---+ +---+---+\n> > + * | Sensor1 | | Mux2 |<--+\n> > + * +---------+ +-^-----+ |\n> > + * | |\n> > + * +-------+-+ +---+-----+\n> > + * | Sensor2 | | Sensor3 |\n> > + * +---------+ +---------+\n> > + */\n> > +void RPiCameraData::enumerateVideoDevices(MediaLink *link)\n> > +{\n> > + const MediaPad *sinkPad = link->sink();\n> > + const MediaEntity *entity = sinkPad->entity();\n> > + bool unicamFound = false;\n> > +\n> > + /* We only deal with Video Mux and Bridge devices in cascade. */\n> > + if (entity->function() != MEDIA_ENT_F_VID_MUX &&\n> > + entity->function() != MEDIA_ENT_F_VID_IF_BRIDGE)\n> > + return;\n> > +\n> > + /* Find the source pad for this Video Mux or Bridge device. */\n> > + const MediaPad *entitySourcePad = nullptr;\n> > + for (const MediaPad *pad : entity->pads()) {\n> > + if (pad->flags() & MEDIA_PAD_FL_SOURCE) {\n> > + /*\n> > + * We can only deal with devices that have a\n> single source\n> > + * pad. If this device has multple source pads,\n> ignore it\n> > + * and this branch in the cascade.\n> > + */\n> > + if (entitySourcePad)\n> > + return;\n> > +\n> > + entitySourcePad = pad;\n> > + }\n> > + }\n> > +\n> > + LOG(RPI, Info) << \"Found video mux device \" << entity->name()\n> > + << \" linked to sink pad \" << sinkPad->index();\n>\n> Same here, Debug ?\n>\n\nAck.\n\nRegards,\nNaush\n\n\n>\n> > +\n> > +\n> bridgeDevices_.emplace_back(std::make_unique(entity), link);\n> > + bridgeDevices_.back().first->open();\n> > +\n> > + /*\n> > + * Iterate through all the sink pad links down the cascade to find\n> any\n> > + * other Video Mux and Bridge devices.\n> > + */\n> > + for (MediaLink *l : entitySourcePad->links()) {\n> > + enumerateVideoDevices(l);\n> > + /* Once we reach the Unicam entity, we are done. */\n> > + if (l->sink()->entity()->name() == \"unicam-image\") {\n> > + unicamFound = true;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + /* This identifies the end of our entity enumeration recursion. */\n> > + if (link->source()->entity()->function() ==\n> MEDIA_ENT_F_CAM_SENSOR) {\n> > + /*\n> > + * If Unicam is not at the end of this cascade, we cannot\n> configure\n> > + * this topology automatically, so remove all entity\n> references.\n> > + */\n> > + if (!unicamFound) {\n> > + LOG(RPI, Warning) << \"Cannot automatically\n> configure this MC topology!\";\n> > + bridgeDevices_.clear();\n> > + }\n> > + }\n> > +}\n> > +\n> > void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const\n> ControlList &controls)\n> > {\n> > if (state_ == State::Stopped)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","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 3917EBE080\n\tfor ;\n\tTue, 4 Jan 2022 11:18:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 80BFA60927;\n\tTue, 4 Jan 2022 12:18:23 +0100 (CET)","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C05E0604F4\n\tfor ;\n\tTue, 4 Jan 2022 12:18:21 +0100 (CET)","by mail-lj1-x232.google.com with SMTP id q8so44358278ljp.9\n\tfor ;\n\tTue, 04 Jan 2022 03:18:21 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"ei19NcV6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=ef7eiYLntY/WbGWmy62wPPnkMWwNCOTjom/JYY47dXY=;\n\tb=ei19NcV6MOFjzY/3cVDodyvY1YtHZi8t7Wc2CaNd+3J/NZ7JqDm1Mwq4plTcqNT75K\n\tbzNHZkudFcI/V8hQICaqHTT/Seld8Di6D2g9dpuaQ7OR2q/ZSXBzQ/sPzhFPaHHGKO/E\n\toeAt7EaHACGUYWTWBozpQRdYxU9ZcFSLsnDHupq1eMpjBh3MjJip45+riCCCtUkM5wcO\n\tPaGQmDPYQ968qBHVHoF13c726utWrDnWwfnFLOotGILSkuh5aOT25sR4Bm0DjWPrk0MM\n\th2F3bIB+7u1FH6nnUDov5g8k6WhibRnsKEr/LyasyN+T1IBErZad7d71g9GWJu803HnF\n\te5+A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ef7eiYLntY/WbGWmy62wPPnkMWwNCOTjom/JYY47dXY=;\n\tb=HgGb2jxyOhTS/PAlNYK/usGeGdz4hnGPo5KxJc+KlOfCb85hWrDPk5oKx7QegIwfD8\n\tbzs6Lt9yiSwhdpGwXMsAk9zAo5/ufyRBwaJp/hEPFuTjBixBosh3/9CoUG50d3WP+u86\n\tcE8q5ZFUhO1oi0bnH7DK2/BiUVNCy6gPmbuZycmt+DTI2Pbt3wdWTQpxwlLtCOhOHRFl\n\tELF/W2rVIyUvngu++SjLbND+B2WABzgR7M5zK5NZFDR2ADls8H5Am3ge3B0inq8Xp283\n\t4IKiLi7oDnuQ92DrcDQ3Lqah2+HfD76I5bSno+LOoxQHGTt0395l3nIKhM6yNnYimyWz\n\t3lzw==","X-Gm-Message-State":"AOAM5311eymvuP/1iRNl5i3qV4dtZHau9DkhnLgn+j6P0kuHHKeYso2Q\n\tbM6lT9tfSaXj/KY7QAfDHcIhNoXs7mSLwW4upDxeOA==","X-Google-Smtp-Source":"ABdhPJzHVPRhi9GI22ORQMTJRh+y17KkwX8WKxSdaSgEr7JNmhrcvSzEdth+AML+jgw1eRZpXjlVxZlvA/pufDPxAEo=","X-Received":"by 2002:a2e:7007:: with SMTP id\n\tl7mr35311368ljc.280.1641295101100; \n\tTue, 04 Jan 2022 03:18:21 -0800 (PST)","MIME-Version":"1.0","References":"<20211214140002.3552445-1-naush@raspberrypi.com>\n\t<20211214140002.3552445-3-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 4 Jan 2022 11:18:05 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"0000000000007916fe05d4bfcb21\"","Subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21942,"web_url":"https://patchwork.libcamera.org/comment/21942/","msgid":"","date":"2022-01-04T11:22:54","subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThanks for the feedback!\n\nOn Tue, 21 Dec 2021 at 18:04, Jacopo Mondi wrote:\n\n> Hi Naush\n>\n> On Tue, Dec 14, 2021 at 02:00:02PM +0000, Naushir Patuck wrote:\n> > This change will allow the pipeline handler to enumerate and control\n> Video\n> > Mux or Bridge devices that may be attached between sensors and a\n> particular\n> > Unicam instance. Cascaded mux or bridge devices are also handled.\n> >\n> > A new member function RPiCameraData::enumerateVideoDevices(), called from\n> > PipelineHandlerRPi::registerCamera(), is used to identify and open all\n> mux and\n> > bridge subdevices present in the sensor -> Unicam link.\n> >\n> > Relevent links are enabled/disabled and pad formats correctly set in\n> > PipelineHandlerRPi::configure() before the camera is started.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 139 ++++++++++++++++++\n> > 1 file changed, 139 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 2a2fb5273eb8..4ec646d3c7a3 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> >\n> > #include \n> > #include \n> > @@ -191,6 +192,8 @@ public:\n> > int loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> > int configureIPA(const CameraConfiguration *config);\n> >\n> > + void enumerateVideoDevices(MediaLink *link);\n> > +\n> > void statsMetadataComplete(uint32_t bufferId, const ControlList\n> &controls);\n> > void runIsp(uint32_t bufferId);\n> > void embeddedComplete(uint32_t bufferId);\n> > @@ -220,6 +223,11 @@ public:\n> > std::vector streams_;\n> > /* Stores the ids of the buffers mapped in the IPA. */\n> > std::unordered_set ipaBuffers_;\n> > + /*\n> > + * Stores a cascade of Video Mux or Bridge devices between the\n> sensor and\n> > + * Unicam together with media link across the entities.\n> > + */\n> > + std::vector, MediaLink\n> *>> bridgeDevices_;\n> >\n> > /* DMAHEAP allocation helper. */\n> > RPi::DmaHeap dmaHeap_;\n> > @@ -868,6 +876,38 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > */\n> > data->properties_.set(properties::ScalerCropMaximum,\n> data->sensorInfo_.analogCrop);\n> >\n> > + /* Setup the Video Mux/Bridge entities. */\n> > + for (auto &[device, link] : data->bridgeDevices_) {\n> > + /*\n> > + * Start by disabling all the sink pad links on the\n> devices in the\n> > + * cascade, with the exception of the link connecting the\n> device.\n> > + */\n> > + for (const MediaPad *p : device->entity()->pads()) {\n> > + if (!(p->flags() & MEDIA_PAD_FL_SINK))\n> > + continue;\n> > +\n> > + for (MediaLink *l : p->links()) {\n> > + if (l != link)\n> > + l->setEnabled(false);\n> > + }\n> > + }\n> > +\n> > + /* Next, enable the entity -> entity links, and setup the\n> pad format. */\n> > + link->setEnabled(true);\n> > + const MediaPad *srcPad = link->sink();\n>\n> sinkPad ?\n>\n\nAck.\n\n\n>\n> > + ret = device->setFormat(srcPad->index(), &sensorFormat);\n> > + if (ret) {\n> > + LOG(RPI, Error) << \"Failed to set format on \" <<\n> device->entity()->name()\n> > + << \" pad \" << srcPad->index()\n> > + << \" with format \" <<\n> format.toString()\n> > + << \": \" << ret;\n> > + return ret;\n> > + }\n> > +\n> > + LOG(RPI, Info) << \"Configured media link on device \" <<\n> device->entity()->name()\n> > + << \" on pad \" << srcPad->index();\n> > + }\n> > +\n> > return ret;\n> > }\n> >\n> > @@ -1098,6 +1138,13 @@ int\n> PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> > if (data->sensor_->init())\n> > return -EINVAL;\n> >\n> > + /*\n> > + * Enumerate all the Video Mux/Bridge devices across the sensor ->\n> unicam\n> > + * link. There may be a cascade of devices in this link!\n> > + */\n> > + MediaLink *link = sensorEntity->getPadByIndex(0)->links()[0];\n> > + data->enumerateVideoDevices(link);\n> > +\n> > data->sensorFormats_ = populateSensorFormats(data->sensor_);\n> >\n> > ipa::RPi::SensorConfig sensorConfig;\n> > @@ -1447,6 +1494,98 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> > return 0;\n> > }\n> >\n> > +/*\n> > + * enumerateVideoDevices() iterates over the Media Controller topology,\n> starting\n> > + * at the sensor and finishing at Unicam. For each sensor,\n> RPiCameraData stores\n> > + * a unique list of any intermediate video mux or bridge devices\n> connected in a\n> > + * cascade, together with the entity to entity link.\n> > + *\n> > + * Entity pad configuration and link enabling happens at the end of\n> configure().\n> > + * We first disables all pad links on each entity device in the chain,\n> and then\n> > + * selectively enabling the specific links to link sensor to Unicam\n> across all\n> > + * intermediate muxes and bridges.\n> > + *\n> > + * In the cascaded topology below, if Sensor1 is used, the Mux2 -> Mux1\n> link\n> > + * will be disabled, and Sensor1 -> Mux1 -> Unicam links enabled.\n> Alternatively,\n> > + * if Sensor3 is used, the Sensor2 -> Mux2 and Sensor1 -> Mux1 links\n> are disabled,\n> > + * and Sensor3 -> Mux2 -> Mux1 -> Unicam links are enabled. All other\n> links will\n> > + * remain unchanged.\n> > + *\n> > + * +----------+\n> > + * | Unicam |\n> > + * +-----^----+\n> > + * |\n> > + * +---+---+\n> > + * | Mux1 <-------+\n> > + * +--^----+ |\n> > + * | |\n> > + * +-----+---+ +---+---+\n> > + * | Sensor1 | | Mux2 |<--+\n> > + * +---------+ +-^-----+ |\n> > + * | |\n> > + * +-------+-+ +---+-----+\n> > + * | Sensor2 | | Sensor3 |\n> > + * +---------+ +---------+\n> > + */\n> > +void RPiCameraData::enumerateVideoDevices(MediaLink *link)\n> > +{\n> > + const MediaPad *sinkPad = link->sink();\n> > + const MediaEntity *entity = sinkPad->entity();\n> > + bool unicamFound = false;\n> > +\n> > + /* We only deal with Video Mux and Bridge devices in cascade. */\n> > + if (entity->function() != MEDIA_ENT_F_VID_MUX &&\n> > + entity->function() != MEDIA_ENT_F_VID_IF_BRIDGE)\n> > + return;\n> > +\n> > + /* Find the source pad for this Video Mux or Bridge device. */\n> > + const MediaPad *entitySourcePad = nullptr;\n> > + for (const MediaPad *pad : entity->pads()) {\n> > + if (pad->flags() & MEDIA_PAD_FL_SOURCE) {\n> > + /*\n> > + * We can only deal with devices that have a\n> single source\n> > + * pad. If this device has multple source pads,\n> ignore it\n> > + * and this branch in the cascade.\n> > + */\n> > + if (entitySourcePad)\n> > + return;\n>\n> Probably failing loud and saying that a bridge/mux with multiple\n> source pads cannot be handled is more explicit ?\n> Ah you will fail later though at the end of recursion...\n>\n>\n> > +\n> > + entitySourcePad = pad;\n>\n> Can this just be sourcePad ? Or were you trying to express that this\n> lives in the 'next' entity and it's not the link's source pad ?\n>\n\nI can rename this to sourcePad.\n\nRegards,\nNaush\n\n\n>\n> > + }\n> > + }\n> > +\n> > + LOG(RPI, Info) << \"Found video mux device \" << entity->name()\n> > + << \" linked to sink pad \" << sinkPad->index();\n> > +\n> > +\n> bridgeDevices_.emplace_back(std::make_unique(entity), link);\n> > + bridgeDevices_.back().first->open();\n> > +\n> > + /*\n> > + * Iterate through all the sink pad links down the cascade to find\n> any\n> > + * other Video Mux and Bridge devices.\n> > + */\n> > + for (MediaLink *l : entitySourcePad->links()) {\n> > + enumerateVideoDevices(l);\n> > + /* Once we reach the Unicam entity, we are done. */\n> > + if (l->sink()->entity()->name() == \"unicam-image\") {\n> > + unicamFound = true;\n> > + break;\n> > + }\n> > + }\n> > +\n> > + /* This identifies the end of our entity enumeration recursion. */\n> > + if (link->source()->entity()->function() ==\n> MEDIA_ENT_F_CAM_SENSOR) {\n> > + /*\n> > + * If Unicam is not at the end of this cascade, we cannot\n> configure\n> > + * this topology automatically, so remove all entity\n> references.\n> > + */\n> > + if (!unicamFound) {\n> > + LOG(RPI, Warning) << \"Cannot automatically\n> configure this MC topology!\";\n> > + bridgeDevices_.clear();\n> > + }\n> > + }\n> > +}\n> > +\n> > void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const\n> ControlList &controls)\n> > {\n> > if (state_ == State::Stopped)\n> > --\n> > 2.25.1\n> >\n>","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 6F576BF415\n\tfor ;\n\tTue, 4 Jan 2022 11:23:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC47E60926;\n\tTue, 4 Jan 2022 12:23:12 +0100 (CET)","from mail-lf1-x130.google.com (mail-lf1-x130.google.com\n\t[IPv6:2a00:1450:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42CA3604F4\n\tfor ;\n\tTue, 4 Jan 2022 12:23:11 +0100 (CET)","by mail-lf1-x130.google.com with SMTP id j11so79250124lfg.3\n\tfor ;\n\tTue, 04 Jan 2022 03:23:11 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"rfLCcg12\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=5eWdBXdJIxIyBcCNFT4sM3E5rvhkjxYNWH1ImlCJmNc=;\n\tb=rfLCcg12QDSK3vVtV9l2JdyzTa1qNES6+/MOTktajpn0mZsZUafGobVD9btbShovGD\n\tg+pYhWPbj5un25q61Kl0crvlA6FCCA+lZ1LbSxaK/0Ge0eat8RF8IXbW/Ij4MqswGYqJ\n\tg5Z64RZVyrBBONxSHWSKup6F4DjIZiBD5I+oPiXQgytYU0PcLjpIvGFhYfExStO+lA4y\n\tCL6pVI7oWvhFrxblyWO9oe1f8lgxvBl92En0Wict3+z3jIvPUS0LulnY9GHlI52RPxGS\n\tv2Fu80FYAMAPG2LvTueJZH1Xz8CScp2aB7BRzEZjEKynkfri6qj2hlniHf58dqWl1mUO\n\tELAQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=5eWdBXdJIxIyBcCNFT4sM3E5rvhkjxYNWH1ImlCJmNc=;\n\tb=FYl6J7TTmCgjsQqccx/VkixHTelue491xay5npeBs04qz8F9JqMqpSpmh6WeaVXom5\n\t7dNWXATHzLvFeJkhJllqO1VcdYGPilRgHwJp1bQ5w8WmZUNJckiXO0KfnCW6py0j4YjT\n\t4G0X595WegufPF6C0rrMxUIMIklC96mvMRdJO7zp/+xkm13rrr6E0LxyDml9M3jr8mT9\n\tEnWoRFDOivqx/6K8oMv+grapLtccuK11BJ4+Pbcs1JdJcOgyudqfaf+/BLujjD8tt4N7\n\tN5ONcK5WCqrNFodnclXX1VanX48JfmM3m9O7WJdtCl4rDxoLu3oQ6BlCgP8/Owgh7SVB\n\tYEWw==","X-Gm-Message-State":"AOAM533wphXnn3XzAwkaklqwZfJnZTMudgQ91MeziP+0Smaf066A0aHW\n\tIwknry9uZAImK5V4JhxgCdQV8y5OfYLFTV+BbRl6yz6j4Qhdog==","X-Google-Smtp-Source":"ABdhPJz9VaLx91r9kSR/F63CniYAvZHvhnD5XsLVjCAyO2K6jTy9o6h+RDVBgNz223Nk/jel9qeUd5Pz5Xm5Agov4aY=","X-Received":"by 2002:a19:4402:: with SMTP id\n\tr2mr44963013lfa.161.1641295390562; \n\tTue, 04 Jan 2022 03:23:10 -0800 (PST)","MIME-Version":"1.0","References":"<20211214140002.3552445-1-naush@raspberrypi.com>\n\t<20211214140002.3552445-3-naush@raspberrypi.com>\n\t<20211221180501.hyjtauom32vi3bq3@uno.localdomain>","In-Reply-To":"<20211221180501.hyjtauom32vi3bq3@uno.localdomain>","From":"Naushir Patuck ","Date":"Tue, 4 Jan 2022 11:22:54 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"000000000000b9f0be05d4bfdc98\"","Subject":"Re: [libcamera-devel] [PATCH v4 2/2] pipeline: raspberrypi: Add\n\tsupport for Video Mux and Bridge devices","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]