[{"id":1216,"web_url":"https://patchwork.libcamera.org/comment/1216/","msgid":"<20190402171957.GM4805@pendragon.ideasonboard.com>","date":"2019-04-02T17:19:57","subject":"Re: [libcamera-devel] [PATCH v7 03/13] libcamera: ipu3: Create\n\tCIO2Device class","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 Tue, Apr 02, 2019 at 07:12:59PM +0200, Jacopo Mondi wrote:\n> Group CIO2 components (cio2, csi2 and image sensor) in a class\n> associated with the CameraData, to ease management and initialization of\n> CIO2 unit at camera registration time. A CIO2 unit will always be\n> associated with a single Camera only.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 247 +++++++++++++++------------\n>  1 file changed, 138 insertions(+), 109 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 55489c31df2d..a870b325f4b3 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -24,6 +24,28 @@ namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(IPU3)\n>  \n> +class CIO2Device\n> +{\n> +public:\n> +\tCIO2Device()\n> +\t\t: output_(nullptr), csi2_(nullptr), sensor_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\t~CIO2Device()\n> +\t{\n> +\t\tdelete output_;\n> +\t\tdelete csi2_;\n> +\t\tdelete sensor_;\n> +\t}\n> +\n> +\tint init(const MediaDevice *media, unsigned int index);\n> +\n> +\tV4L2Device *output_;\n> +\tV4L2Subdevice *csi2_;\n> +\tV4L2Subdevice *sensor_;\n> +};\n> +\n>  class PipelineHandlerIPU3 : public PipelineHandler\n>  {\n>  public:\n> @@ -51,23 +73,13 @@ private:\n>  \t{\n>  \tpublic:\n>  \t\tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t\t: CameraData(pipe), cio2_(nullptr), csi2_(nullptr),\n> -\t\t\t  sensor_(nullptr)\n> -\t\t{\n> -\t\t}\n> -\n> -\t\t~IPU3CameraData()\n> +\t\t\t: CameraData(pipe)\n>  \t\t{\n> -\t\t\tdelete cio2_;\n> -\t\t\tdelete csi2_;\n> -\t\t\tdelete sensor_;\n>  \t\t}\n>  \n>  \t\tvoid bufferReady(Buffer *buffer);\n>  \n> -\t\tV4L2Device *cio2_;\n> -\t\tV4L2Subdevice *csi2_;\n> -\t\tV4L2Subdevice *sensor_;\n> +\t\tCIO2Device cio2_;\n>  \n>  \t\tStream stream_;\n>  \t};\n> @@ -80,22 +92,22 @@ private:\n>  \n>  \tvoid registerCameras();\n>  \n> -\tstd::shared_ptr<MediaDevice> cio2_;\n> -\tstd::shared_ptr<MediaDevice> imgu_;\n> +\tstd::shared_ptr<MediaDevice> cio2MediaDev_;\n> +\tstd::shared_ptr<MediaDevice> imguMediaDev_;\n>  };\n>  \n>  PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n> -\t: PipelineHandler(manager), cio2_(nullptr), imgu_(nullptr)\n> +\t: PipelineHandler(manager), cio2MediaDev_(nullptr), imguMediaDev_(nullptr)\n>  {\n>  }\n>  \n>  PipelineHandlerIPU3::~PipelineHandlerIPU3()\n>  {\n> -\tif (cio2_)\n> -\t\tcio2_->release();\n> +\tif (cio2MediaDev_)\n> +\t\tcio2MediaDev_->release();\n>  \n> -\tif (imgu_)\n> -\t\timgu_->release();\n> +\tif (imguMediaDev_)\n> +\t\timguMediaDev_->release();\n>  }\n>  \n>  std::map<Stream *, StreamConfiguration>\n> @@ -110,7 +122,7 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n>  \t * FIXME: As of now, return the image format reported by the sensor.\n>  \t * In future good defaults should be provided for each stream.\n>  \t */\n> -\tif (data->sensor_->getFormat(0, &format)) {\n> +\tif (data->cio2_.sensor_->getFormat(0, &format)) {\n>  \t\tLOG(IPU3, Error) << \"Failed to create stream configurations\";\n>  \t\treturn configs;\n>  \t}\n> @@ -131,9 +143,9 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n>  \tStreamConfiguration *cfg = &config[&data->stream_];\n> -\tV4L2Subdevice *sensor = data->sensor_;\n> -\tV4L2Subdevice *csi2 = data->csi2_;\n> -\tV4L2Device *cio2 = data->cio2_;\n> +\tV4L2Subdevice *sensor = data->cio2_.sensor_;\n> +\tV4L2Subdevice *csi2 = data->cio2_.csi2_;\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \tV4L2SubdeviceFormat subdevFormat = {};\n>  \tV4L2DeviceFormat devFormat = {};\n>  \tint ret;\n> @@ -190,13 +202,14 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,\n>  \n>  int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)\n>  {\n> -\tIPU3CameraData *data = cameraData(camera);\n>  \tconst StreamConfiguration &cfg = stream->configuration();\n> +\tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \n>  \tif (!cfg.bufferCount)\n>  \t\treturn -EINVAL;\n>  \n> -\tint ret = data->cio2_->exportBuffers(&stream->bufferPool());\n> +\tint ret = cio2->exportBuffers(&stream->bufferPool());\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Error) << \"Failed to request memory\";\n>  \t\treturn ret;\n> @@ -208,8 +221,9 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)\n>  int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \n> -\tint ret = data->cio2_->releaseBuffers();\n> +\tint ret = cio2->releaseBuffers();\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Error) << \"Failed to release memory\";\n>  \t\treturn ret;\n> @@ -221,9 +235,10 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)\n>  int PipelineHandlerIPU3::start(Camera *camera)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \tint ret;\n>  \n> -\tret = data->cio2_->streamOn();\n> +\tret = cio2->streamOn();\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Info) << \"Failed to start camera \" << camera->name();\n>  \t\treturn ret;\n> @@ -235,8 +250,9 @@ int PipelineHandlerIPU3::start(Camera *camera)\n>  void PipelineHandlerIPU3::stop(Camera *camera)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \n> -\tif (data->cio2_->streamOff())\n> +\tif (cio2->streamOff())\n>  \t\tLOG(IPU3, Info) << \"Failed to stop camera \" << camera->name();\n>  \n>  \tPipelineHandler::stop(camera);\n> @@ -245,6 +261,7 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n>  int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \tStream *stream = &data->stream_;\n>  \n>  \tBuffer *buffer = request->findBuffer(stream);\n> @@ -254,7 +271,7 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n>  \t\treturn -ENOENT;\n>  \t}\n>  \n> -\tint ret = data->cio2_->queueBuffer(buffer);\n> +\tint ret = cio2->queueBuffer(buffer);\n>  \tif (ret < 0)\n>  \t\treturn ret;\n>  \n> @@ -293,17 +310,17 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n>  \t * It is safe to acquire both media devices at this point as\n>  \t * DeviceEnumerator::search() skips the busy ones for us.\n>  \t */\n> -\tcio2_ = enumerator->search(cio2_dm);\n> -\tif (!cio2_)\n> +\tcio2MediaDev_ = enumerator->search(cio2_dm);\n> +\tif (!cio2MediaDev_)\n>  \t\treturn false;\n>  \n> -\tcio2_->acquire();\n> +\tcio2MediaDev_->acquire();\n>  \n> -\timgu_ = enumerator->search(imgu_dm);\n> -\tif (!imgu_)\n> +\timguMediaDev_ = enumerator->search(imgu_dm);\n> +\tif (!imguMediaDev_)\n>  \t\treturn false;\n>  \n> -\timgu_->acquire();\n> +\timguMediaDev_->acquire();\n>  \n>  \t/*\n>  \t * Disable all links that are enabled by default on CIO2, as camera\n> @@ -312,17 +329,17 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n>  \t * Close the CIO2 media device after, as links are enabled and should\n>  \t * not need to be changed after.\n>  \t */\n> -\tif (cio2_->open())\n> +\tif (cio2MediaDev_->open())\n>  \t\treturn false;\n>  \n> -\tif (cio2_->disableLinks()) {\n> -\t\tcio2_->close();\n> +\tif (cio2MediaDev_->disableLinks()) {\n> +\t\tcio2MediaDev_->close();\n>  \t\treturn false;\n>  \t}\n>  \n>  \tregisterCameras();\n>  \n> -\tcio2_->close();\n> +\tcio2MediaDev_->close();\n>  \n>  \treturn true;\n>  }\n> @@ -336,85 +353,28 @@ void PipelineHandlerIPU3::registerCameras()\n>  {\n>  \t/*\n>  \t * For each CSI-2 receiver on the IPU3, create a Camera if an\n> -\t * image sensor is connected to it.\n> +\t * image sensor is connected to it and the sensor can produce images\n> +\t * in a compatible format.\n>  \t */\n>  \tunsigned int numCameras = 0;\n>  \tfor (unsigned int id = 0; id < 4; ++id) {\n> -\t\tstd::string csi2Name = \"ipu3-csi2 \" + std::to_string(id);\n> -\t\tMediaEntity *csi2 = cio2_->getEntityByName(csi2Name);\n> -\t\tint ret;\n> -\n> -\t\t/*\n> -\t\t * This shall not happen, as the device enumerator matched\n> -\t\t * all entities described in the cio2_dm DeviceMatch.\n> -\t\t *\n> -\t\t * As this check is basically free, better stay safe than sorry.\n> -\t\t */\n> -\t\tif (!csi2)\n> -\t\t\tcontinue;\n> -\n> -\t\tconst std::vector<MediaPad *> &pads = csi2->pads();\n> -\t\tif (pads.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\t/* IPU3 CSI-2 receivers have a single sink pad at index 0. */\n> -\t\tMediaPad *sink = pads[0];\n> -\t\tconst std::vector<MediaLink *> &links = sink->links();\n> -\t\tif (links.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\t/*\n> -\t\t * Verify that the receiver is connected to a sensor, enable\n> -\t\t * the media link between the two, and create a Camera with\n> -\t\t * a unique name.\n> -\t\t */\n> -\t\tMediaLink *link = links[0];\n> -\t\tMediaEntity *sensor = link->source()->entity();\n> -\t\tif (sensor->function() != MEDIA_ENT_F_CAM_SENSOR)\n> -\t\t\tcontinue;\n> -\n> -\t\tif (link->setEnabled(true))\n> -\t\t\tcontinue;\n> -\n> -\t\tstd::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>(this);\n> -\n> -\t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> +\t\tstd::unique_ptr<IPU3CameraData> data =\n> +\t\t\tutils::make_unique<IPU3CameraData>(this);\n>  \t\tstd::set<Stream *> streams{ &data->stream_ };\n> -\t\tstd::shared_ptr<Camera> camera = Camera::create(this, cameraName, streams);\n> -\n> -\t\t/*\n> -\t\t * Create and open video devices and subdevices associated with\n> -\t\t * the camera.\n> -\t\t *\n> -\t\t * If any of these operations fails, the Camera instance won't\n> -\t\t * be registered. The 'camera' shared pointer and the 'data'\n> -\t\t * unique pointers go out of scope and delete the objects they\n> -\t\t * manage.\n> -\t\t */\n> -\t\tstd::string cio2Name = \"ipu3-cio2 \" + std::to_string(id);\n> -\t\tMediaEntity *cio2 = cio2_->getEntityByName(cio2Name);\n> -\t\tif (!cio2) {\n> -\t\t\tLOG(IPU3, Error)\n> -\t\t\t\t<< \"Failed to get entity '\" << cio2Name << \"'\";\n> -\t\t\tcontinue;\n> -\t\t}\n> +\t\tCIO2Device *cio2 = &data->cio2_;\n>  \n> -\t\tdata->cio2_ = new V4L2Device(cio2);\n> -\t\tret = data->cio2_->open();\n> +\t\tint ret = cio2->init(cio2MediaDev_.get(), id);\n>  \t\tif (ret)\n>  \t\t\tcontinue;\n>  \n> -\t\tdata->cio2_->bufferReady.connect(data.get(), &IPU3CameraData::bufferReady);\n> +\t\tstd::string cameraName = cio2->sensor_->entityName() + \" \"\n> +\t\t\t\t       + std::to_string(id);\n> +\t\tstd::shared_ptr<Camera> camera = Camera::create(this,\n> +\t\t\t\t\t\t\t\tcameraName,\n> +\t\t\t\t\t\t\t\tstreams);\n>  \n> -\t\tdata->sensor_ = new V4L2Subdevice(sensor);\n> -\t\tret = data->sensor_->open();\n> -\t\tif (ret)\n> -\t\t\tcontinue;\n> -\n> -\t\tdata->csi2_ = new V4L2Subdevice(csi2);\n> -\t\tret = data->csi2_->open();\n> -\t\tif (ret)\n> -\t\t\tcontinue;\n> +\t\tcio2->output_->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> @@ -435,6 +395,75 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n>  \tpipe_->completeRequest(camera_, request);\n>  }\n>  \n> +/*------------------------------------------------------------------------------\n> + * CIO2 Device\n> + */\n> +\n> +/**\n> + * \\brief Initialize components of the CIO2 device with \\a index\n> + * \\param[in] media The CIO2 media device\n> + * \\param[in] index The CIO2 device index\n> + *\n> + * Create and open the video device and subdevices in the CIO2 instance at \\a\n> + * index, if an image sensor is connected to the CSI-2 receiver of this CIO2\n> + * instance.  Enable the media links connecting the CIO2 components to prepare\n> + * for capture operations and cache the sensor maximum size.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + * \\retval -ENODEV No supported image sensor is connected to this CIO2 instance\n> + */\n> +int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> +{\n> +\tint ret;\n> +\n> +\t/*\n> +\t * Verify that a sensor subdevice is connected to this CIO2 instance\n> +\t * and enable link between the two.\n\ns/link/the link/\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t */\n> +\tstd::string csi2Name = \"ipu3-csi2 \" + std::to_string(index);\n> +\tMediaEntity *csi2Entity = media->getEntityByName(csi2Name);\n> +\tconst std::vector<MediaPad *> &pads = csi2Entity->pads();\n> +\tif (pads.empty())\n> +\t\treturn -ENODEV;\n> +\n> +\t/* IPU3 CSI-2 receivers have a single sink pad at index 0. */\n> +\tMediaPad *sink = pads[0];\n> +\tconst std::vector<MediaLink *> &links = sink->links();\n> +\tif (links.empty())\n> +\t\treturn -ENODEV;\n> +\n> +\tMediaLink *link = links[0];\n> +\tMediaEntity *sensorEntity = link->source()->entity();\n> +\tif (sensorEntity->function() != MEDIA_ENT_F_CAM_SENSOR)\n> +\t\treturn -ENODEV;\n> +\n> +\tret = link->setEnabled(true);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/*\n> +\t * \\todo Define when to open and close video device nodes, as they\n> +\t * might impact on power consumption.\n> +\t */\n> +\tsensor_ = new V4L2Subdevice(sensorEntity);\n> +\tret = sensor_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tcsi2_ = new V4L2Subdevice(csi2Entity);\n> +\tret = csi2_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstd::string cio2Name = \"ipu3-cio2 \" + std::to_string(index);\n> +\toutput_ = V4L2Device::fromEntityName(media, cio2Name);\n> +\tret = output_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn 0;\n> +}\n> +\n>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);\n>  \n>  } /* namespace libcamera */","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60E88610B6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Apr 2019 19:20:08 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B5AB92F9;\n\tTue,  2 Apr 2019 19:20:07 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1554225607;\n\tbh=GHBUqwOPsANFXlhb1CCLCNw59sPIFdGJJQkARTWsHrs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tPWdUP0JbFKofWR/53s6toUqA/pKv8otz/8RIx7z6sXlLMsF1vEw6y27dEnICeLyw\n\thnx+NuQWJQ7T9okIJqxIHzTxFb/1RQLF53OOldDlaXNiSc1jhxMmM3CIzFirk9yXHk\n\tjKHKR1yntisHtltA5lNTbooHbOltVtMmrlK6vjeg=","Date":"Tue, 2 Apr 2019 20:19:57 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402171957.GM4805@pendragon.ideasonboard.com>","References":"<20190402171309.6447-1-jacopo@jmondi.org>\n\t<20190402171309.6447-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190402171309.6447-4-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v7 03/13] libcamera: ipu3: Create\n\tCIO2Device class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 02 Apr 2019 17:20:08 -0000"}},{"id":1226,"web_url":"https://patchwork.libcamera.org/comment/1226/","msgid":"<20190402180920.GA23466@bigcity.dyn.berto.se>","date":"2019-04-02T18:09:20","subject":"Re: [libcamera-devel] [PATCH v7 03/13] libcamera: ipu3: Create\n\tCIO2Device class","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2019-04-02 19:12:59 +0200, Jacopo Mondi wrote:\n> Group CIO2 components (cio2, csi2 and image sensor) in a class\n> associated with the CameraData, to ease management and initialization of\n> CIO2 unit at camera registration time. A CIO2 unit will always be\n> associated with a single Camera only.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 247 +++++++++++++++------------\n>  1 file changed, 138 insertions(+), 109 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 55489c31df2d..a870b325f4b3 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -24,6 +24,28 @@ namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(IPU3)\n>  \n> +class CIO2Device\n> +{\n> +public:\n> +\tCIO2Device()\n> +\t\t: output_(nullptr), csi2_(nullptr), sensor_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\t~CIO2Device()\n> +\t{\n> +\t\tdelete output_;\n> +\t\tdelete csi2_;\n> +\t\tdelete sensor_;\n> +\t}\n> +\n> +\tint init(const MediaDevice *media, unsigned int index);\n> +\n> +\tV4L2Device *output_;\n> +\tV4L2Subdevice *csi2_;\n> +\tV4L2Subdevice *sensor_;\n> +};\n> +\n>  class PipelineHandlerIPU3 : public PipelineHandler\n>  {\n>  public:\n> @@ -51,23 +73,13 @@ private:\n>  \t{\n>  \tpublic:\n>  \t\tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t\t: CameraData(pipe), cio2_(nullptr), csi2_(nullptr),\n> -\t\t\t  sensor_(nullptr)\n> -\t\t{\n> -\t\t}\n> -\n> -\t\t~IPU3CameraData()\n> +\t\t\t: CameraData(pipe)\n>  \t\t{\n> -\t\t\tdelete cio2_;\n> -\t\t\tdelete csi2_;\n> -\t\t\tdelete sensor_;\n>  \t\t}\n>  \n>  \t\tvoid bufferReady(Buffer *buffer);\n>  \n> -\t\tV4L2Device *cio2_;\n> -\t\tV4L2Subdevice *csi2_;\n> -\t\tV4L2Subdevice *sensor_;\n> +\t\tCIO2Device cio2_;\n>  \n>  \t\tStream stream_;\n>  \t};\n> @@ -80,22 +92,22 @@ private:\n>  \n>  \tvoid registerCameras();\n>  \n> -\tstd::shared_ptr<MediaDevice> cio2_;\n> -\tstd::shared_ptr<MediaDevice> imgu_;\n> +\tstd::shared_ptr<MediaDevice> cio2MediaDev_;\n> +\tstd::shared_ptr<MediaDevice> imguMediaDev_;\n>  };\n>  \n>  PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n> -\t: PipelineHandler(manager), cio2_(nullptr), imgu_(nullptr)\n> +\t: PipelineHandler(manager), cio2MediaDev_(nullptr), imguMediaDev_(nullptr)\n>  {\n>  }\n>  \n>  PipelineHandlerIPU3::~PipelineHandlerIPU3()\n>  {\n> -\tif (cio2_)\n> -\t\tcio2_->release();\n> +\tif (cio2MediaDev_)\n> +\t\tcio2MediaDev_->release();\n>  \n> -\tif (imgu_)\n> -\t\timgu_->release();\n> +\tif (imguMediaDev_)\n> +\t\timguMediaDev_->release();\n>  }\n>  \n>  std::map<Stream *, StreamConfiguration>\n> @@ -110,7 +122,7 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n>  \t * FIXME: As of now, return the image format reported by the sensor.\n>  \t * In future good defaults should be provided for each stream.\n>  \t */\n> -\tif (data->sensor_->getFormat(0, &format)) {\n> +\tif (data->cio2_.sensor_->getFormat(0, &format)) {\n>  \t\tLOG(IPU3, Error) << \"Failed to create stream configurations\";\n>  \t\treturn configs;\n>  \t}\n> @@ -131,9 +143,9 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n>  \tStreamConfiguration *cfg = &config[&data->stream_];\n> -\tV4L2Subdevice *sensor = data->sensor_;\n> -\tV4L2Subdevice *csi2 = data->csi2_;\n> -\tV4L2Device *cio2 = data->cio2_;\n> +\tV4L2Subdevice *sensor = data->cio2_.sensor_;\n> +\tV4L2Subdevice *csi2 = data->cio2_.csi2_;\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \tV4L2SubdeviceFormat subdevFormat = {};\n>  \tV4L2DeviceFormat devFormat = {};\n>  \tint ret;\n> @@ -190,13 +202,14 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,\n>  \n>  int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)\n>  {\n> -\tIPU3CameraData *data = cameraData(camera);\n>  \tconst StreamConfiguration &cfg = stream->configuration();\n> +\tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \n>  \tif (!cfg.bufferCount)\n>  \t\treturn -EINVAL;\n>  \n> -\tint ret = data->cio2_->exportBuffers(&stream->bufferPool());\n> +\tint ret = cio2->exportBuffers(&stream->bufferPool());\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Error) << \"Failed to request memory\";\n>  \t\treturn ret;\n> @@ -208,8 +221,9 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)\n>  int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \n> -\tint ret = data->cio2_->releaseBuffers();\n> +\tint ret = cio2->releaseBuffers();\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Error) << \"Failed to release memory\";\n>  \t\treturn ret;\n> @@ -221,9 +235,10 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)\n>  int PipelineHandlerIPU3::start(Camera *camera)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \tint ret;\n>  \n> -\tret = data->cio2_->streamOn();\n> +\tret = cio2->streamOn();\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Info) << \"Failed to start camera \" << camera->name();\n>  \t\treturn ret;\n> @@ -235,8 +250,9 @@ int PipelineHandlerIPU3::start(Camera *camera)\n>  void PipelineHandlerIPU3::stop(Camera *camera)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \n> -\tif (data->cio2_->streamOff())\n> +\tif (cio2->streamOff())\n>  \t\tLOG(IPU3, Info) << \"Failed to stop camera \" << camera->name();\n>  \n>  \tPipelineHandler::stop(camera);\n> @@ -245,6 +261,7 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n>  int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2_.output_;\n>  \tStream *stream = &data->stream_;\n>  \n>  \tBuffer *buffer = request->findBuffer(stream);\n> @@ -254,7 +271,7 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n>  \t\treturn -ENOENT;\n>  \t}\n>  \n> -\tint ret = data->cio2_->queueBuffer(buffer);\n> +\tint ret = cio2->queueBuffer(buffer);\n>  \tif (ret < 0)\n>  \t\treturn ret;\n>  \n> @@ -293,17 +310,17 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n>  \t * It is safe to acquire both media devices at this point as\n>  \t * DeviceEnumerator::search() skips the busy ones for us.\n>  \t */\n> -\tcio2_ = enumerator->search(cio2_dm);\n> -\tif (!cio2_)\n> +\tcio2MediaDev_ = enumerator->search(cio2_dm);\n> +\tif (!cio2MediaDev_)\n>  \t\treturn false;\n>  \n> -\tcio2_->acquire();\n> +\tcio2MediaDev_->acquire();\n>  \n> -\timgu_ = enumerator->search(imgu_dm);\n> -\tif (!imgu_)\n> +\timguMediaDev_ = enumerator->search(imgu_dm);\n> +\tif (!imguMediaDev_)\n>  \t\treturn false;\n>  \n> -\timgu_->acquire();\n> +\timguMediaDev_->acquire();\n>  \n>  \t/*\n>  \t * Disable all links that are enabled by default on CIO2, as camera\n> @@ -312,17 +329,17 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n>  \t * Close the CIO2 media device after, as links are enabled and should\n>  \t * not need to be changed after.\n>  \t */\n> -\tif (cio2_->open())\n> +\tif (cio2MediaDev_->open())\n>  \t\treturn false;\n>  \n> -\tif (cio2_->disableLinks()) {\n> -\t\tcio2_->close();\n> +\tif (cio2MediaDev_->disableLinks()) {\n> +\t\tcio2MediaDev_->close();\n>  \t\treturn false;\n>  \t}\n>  \n>  \tregisterCameras();\n>  \n> -\tcio2_->close();\n> +\tcio2MediaDev_->close();\n>  \n>  \treturn true;\n>  }\n> @@ -336,85 +353,28 @@ void PipelineHandlerIPU3::registerCameras()\n>  {\n>  \t/*\n>  \t * For each CSI-2 receiver on the IPU3, create a Camera if an\n> -\t * image sensor is connected to it.\n> +\t * image sensor is connected to it and the sensor can produce images\n> +\t * in a compatible format.\n>  \t */\n>  \tunsigned int numCameras = 0;\n>  \tfor (unsigned int id = 0; id < 4; ++id) {\n> -\t\tstd::string csi2Name = \"ipu3-csi2 \" + std::to_string(id);\n> -\t\tMediaEntity *csi2 = cio2_->getEntityByName(csi2Name);\n> -\t\tint ret;\n> -\n> -\t\t/*\n> -\t\t * This shall not happen, as the device enumerator matched\n> -\t\t * all entities described in the cio2_dm DeviceMatch.\n> -\t\t *\n> -\t\t * As this check is basically free, better stay safe than sorry.\n> -\t\t */\n> -\t\tif (!csi2)\n> -\t\t\tcontinue;\n> -\n> -\t\tconst std::vector<MediaPad *> &pads = csi2->pads();\n> -\t\tif (pads.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\t/* IPU3 CSI-2 receivers have a single sink pad at index 0. */\n> -\t\tMediaPad *sink = pads[0];\n> -\t\tconst std::vector<MediaLink *> &links = sink->links();\n> -\t\tif (links.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\t/*\n> -\t\t * Verify that the receiver is connected to a sensor, enable\n> -\t\t * the media link between the two, and create a Camera with\n> -\t\t * a unique name.\n> -\t\t */\n> -\t\tMediaLink *link = links[0];\n> -\t\tMediaEntity *sensor = link->source()->entity();\n> -\t\tif (sensor->function() != MEDIA_ENT_F_CAM_SENSOR)\n> -\t\t\tcontinue;\n> -\n> -\t\tif (link->setEnabled(true))\n> -\t\t\tcontinue;\n> -\n> -\t\tstd::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>(this);\n> -\n> -\t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> +\t\tstd::unique_ptr<IPU3CameraData> data =\n> +\t\t\tutils::make_unique<IPU3CameraData>(this);\n>  \t\tstd::set<Stream *> streams{ &data->stream_ };\n> -\t\tstd::shared_ptr<Camera> camera = Camera::create(this, cameraName, streams);\n> -\n> -\t\t/*\n> -\t\t * Create and open video devices and subdevices associated with\n> -\t\t * the camera.\n> -\t\t *\n> -\t\t * If any of these operations fails, the Camera instance won't\n> -\t\t * be registered. The 'camera' shared pointer and the 'data'\n> -\t\t * unique pointers go out of scope and delete the objects they\n> -\t\t * manage.\n> -\t\t */\n> -\t\tstd::string cio2Name = \"ipu3-cio2 \" + std::to_string(id);\n> -\t\tMediaEntity *cio2 = cio2_->getEntityByName(cio2Name);\n> -\t\tif (!cio2) {\n> -\t\t\tLOG(IPU3, Error)\n> -\t\t\t\t<< \"Failed to get entity '\" << cio2Name << \"'\";\n> -\t\t\tcontinue;\n> -\t\t}\n> +\t\tCIO2Device *cio2 = &data->cio2_;\n>  \n> -\t\tdata->cio2_ = new V4L2Device(cio2);\n> -\t\tret = data->cio2_->open();\n> +\t\tint ret = cio2->init(cio2MediaDev_.get(), id);\n>  \t\tif (ret)\n>  \t\t\tcontinue;\n>  \n> -\t\tdata->cio2_->bufferReady.connect(data.get(), &IPU3CameraData::bufferReady);\n> +\t\tstd::string cameraName = cio2->sensor_->entityName() + \" \"\n> +\t\t\t\t       + std::to_string(id);\n> +\t\tstd::shared_ptr<Camera> camera = Camera::create(this,\n> +\t\t\t\t\t\t\t\tcameraName,\n> +\t\t\t\t\t\t\t\tstreams);\n>  \n> -\t\tdata->sensor_ = new V4L2Subdevice(sensor);\n> -\t\tret = data->sensor_->open();\n> -\t\tif (ret)\n> -\t\t\tcontinue;\n> -\n> -\t\tdata->csi2_ = new V4L2Subdevice(csi2);\n> -\t\tret = data->csi2_->open();\n> -\t\tif (ret)\n> -\t\t\tcontinue;\n> +\t\tcio2->output_->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> @@ -435,6 +395,75 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n>  \tpipe_->completeRequest(camera_, request);\n>  }\n>  \n> +/*------------------------------------------------------------------------------\n> + * CIO2 Device\n> + */\n> +\n> +/**\n> + * \\brief Initialize components of the CIO2 device with \\a index\n> + * \\param[in] media The CIO2 media device\n> + * \\param[in] index The CIO2 device index\n> + *\n> + * Create and open the video device and subdevices in the CIO2 instance at \\a\n> + * index, if an image sensor is connected to the CSI-2 receiver of this CIO2\n> + * instance.  Enable the media links connecting the CIO2 components to prepare\n> + * for capture operations and cache the sensor maximum size.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + * \\retval -ENODEV No supported image sensor is connected to this CIO2 instance\n> + */\n> +int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> +{\n> +\tint ret;\n> +\n> +\t/*\n> +\t * Verify that a sensor subdevice is connected to this CIO2 instance\n> +\t * and enable link between the two.\n> +\t */\n> +\tstd::string csi2Name = \"ipu3-csi2 \" + std::to_string(index);\n> +\tMediaEntity *csi2Entity = media->getEntityByName(csi2Name);\n> +\tconst std::vector<MediaPad *> &pads = csi2Entity->pads();\n> +\tif (pads.empty())\n> +\t\treturn -ENODEV;\n> +\n> +\t/* IPU3 CSI-2 receivers have a single sink pad at index 0. */\n> +\tMediaPad *sink = pads[0];\n> +\tconst std::vector<MediaLink *> &links = sink->links();\n> +\tif (links.empty())\n> +\t\treturn -ENODEV;\n> +\n> +\tMediaLink *link = links[0];\n> +\tMediaEntity *sensorEntity = link->source()->entity();\n> +\tif (sensorEntity->function() != MEDIA_ENT_F_CAM_SENSOR)\n> +\t\treturn -ENODEV;\n> +\n> +\tret = link->setEnabled(true);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/*\n> +\t * \\todo Define when to open and close video device nodes, as they\n> +\t * might impact on power consumption.\n> +\t */\n> +\tsensor_ = new V4L2Subdevice(sensorEntity);\n> +\tret = sensor_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tcsi2_ = new V4L2Subdevice(csi2Entity);\n> +\tret = csi2_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstd::string cio2Name = \"ipu3-cio2 \" + std::to_string(index);\n> +\toutput_ = V4L2Device::fromEntityName(media, cio2Name);\n> +\tret = output_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn 0;\n> +}\n> +\n>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);\n>  \n>  } /* namespace libcamera */\n> -- \n> 2.21.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90531610B6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Apr 2019 20:09:22 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id y6so12425011ljd.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 02 Apr 2019 11:09:22 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tj6sm1257964ljc.0.2019.04.02.11.09.20\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 02 Apr 2019 11:09:20 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=OB/rSlc0YZcKcG5/i6K3tsLhupKZRepRNKTN8L8SlP4=;\n\tb=lS4keE9CNp21hsGnT85YPlf8EyEWFX/m0o+Ccnd8uzzy7/Uw7Trp7deniU2lQGbDRk\n\tBY6qap6I3U50nvylTPB0hovtEUBLuMo+w/xVP+9s8p305UxCnQhKTwMlzJsv6ZJqk7iL\n\tZI7xmcZPtjWvaFp0iga8Ww4gWNaWztnUA3L2w+YQnkCNanAPle0Xrq/V9SY0Ps/I4iQy\n\tiuIw+D1sNEbncs9q/NnYb3yLL2vBTI/ZEohXrl4Mr9s3Lo6J0Vqf4kSkABdppbM4qN9N\n\t3qmC/BNzr2qYpYUxFzigajh5hJlaMkB52YjLTVgP9Udi1aGg0M305ph1Bd/VJK6uwY0J\n\tVTvA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=OB/rSlc0YZcKcG5/i6K3tsLhupKZRepRNKTN8L8SlP4=;\n\tb=dN5Tyq3Ih+cnbfr5CjxpFjD4mMLXXBhD6I9HLPknYnKFJ7LiGSte+CXuSZLJNGX9Oq\n\tmmXVQVScpTKno9CDGMwsJPdUfNP7yagZ1NKjtAWmbFLSB3OcvPSE9YjNSJNE+Uw2bsnH\n\tw52iz+cnyFgGLeyh8bZ9IzEN6m2CoWhFumZ1G/YuQGjBtrlqXiLJRQAc/wncp5llZ/O0\n\tWd2mShSMfJ/xyvriyVciSzNg+ba8Xpdde71/ds5Cr1Pk8CKEN3wiJBxt02Xizqlgt6+9\n\tvkjsWgIMbigOChqjD7HlOTu1f70WMS8WMv1ZQOe+JWtpjV+Ik0Q37JLjVgjO33Y/sUbd\n\tAHYw==","X-Gm-Message-State":"APjAAAWKcowm1oz5gUKZnD/ACVVElKPf/wXOd4PeAsOhF1lmudluNfkt\n\t6QJYVV4tVvacUH2CIx5eq+nujg==","X-Google-Smtp-Source":"APXvYqx6NVRbncy1dEAuNOJRUoqumiyvQbY7Su/yZG6MOlTliKEen/hHSSjNdHOWfkI2rbNB3lzWFg==","X-Received":"by 2002:a2e:9a49:: with SMTP id k9mr40468393ljj.84.1554228561819;\n\tTue, 02 Apr 2019 11:09:21 -0700 (PDT)","Date":"Tue, 2 Apr 2019 20:09:20 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402180920.GA23466@bigcity.dyn.berto.se>","References":"<20190402171309.6447-1-jacopo@jmondi.org>\n\t<20190402171309.6447-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190402171309.6447-4-jacopo@jmondi.org>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH v7 03/13] libcamera: ipu3: Create\n\tCIO2Device class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 02 Apr 2019 18:09:22 -0000"}}]