[{"id":1518,"web_url":"https://patchwork.libcamera.org/comment/1518/","msgid":"<969a20a3-565b-7ed9-af04-206e271fdab5@ideasonboard.com>","date":"2019-04-27T09:11:26","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: pipeline: stm32: add\n\tpipeline handler for stm32","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Benjamin\n\nThank you for the updated patch.\n\nWe have discussed this topic today in the team, and we are still in the\nmindset that the STM32 is a simple pipeline currently, even with the\nbridge. (It has only a single linear stream) and this perhaps deserves\nsome thought to make it easy to add other 'simple' streams with a single\nmatch line addition.\n\nWe should be able to use a single class to support the majority of the\nfunctionality and prevent the requirement of duplicating lots of code\nand files.\n\n\nIs this a topic you would be able to tackle? We understand that you\nmight not be able to commit such time (but it will be great if you can),\nand if that's the case then we don't want to block this pipeline\nhandler, but we would need to have access to the hardware ourselves to\ndo the simplification later. In that event, would you be able to provide\nus with boards and sensors for testing and development?\n\n\n(comments below in the code relating to the attached updated patches)\n\n\nRegards\n\nKieran\n\n\nOn 19/04/2019 11:03, Benjamin Gaignard wrote:\n> Provide a pipeline handler for the stm32 driver.\n> \n> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>\n> ---\n> version 2:\n> - fix indentation and run checkstyle:\n> ./utils/checkstyle.py \n> ---------------------------------------------------------------------------------------------------\n> 9360626c2158b2c5d865608f4504d203b6d0014f libcamera: pipeline: stm32: add pipeline handler for stm32\n> ---------------------------------------------------------------------------------------------------\n> No style issue detected\n> - Use the first video node instead of using MEDIA_ENT_FL_DEFAULT flag\n> \n>  src/libcamera/pipeline/meson.build       |   2 +\n>  src/libcamera/pipeline/stm32/meson.build |   3 +\n>  src/libcamera/pipeline/stm32/stm32.cpp   | 220 +++++++++++++++++++++++++++++++\n>  3 files changed, 225 insertions(+)\n>  create mode 100644 src/libcamera/pipeline/stm32/meson.build\n>  create mode 100644 src/libcamera/pipeline/stm32/stm32.cpp\n> \n> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build\n> index 40bb264..08d6e1c 100644\n> --- a/src/libcamera/pipeline/meson.build\n> +++ b/src/libcamera/pipeline/meson.build\n> @@ -4,3 +4,5 @@ libcamera_sources += files([\n>  ])\n>  \n>  subdir('ipu3')\n> +\n> +subdir('stm32')\n> diff --git a/src/libcamera/pipeline/stm32/meson.build b/src/libcamera/pipeline/stm32/meson.build\n> new file mode 100644\n> index 0000000..cb6f16b\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/stm32/meson.build\n> @@ -0,0 +1,3 @@\n> +libcamera_sources += files([\n> +    'stm32.cpp',\n> +])\n> diff --git a/src/libcamera/pipeline/stm32/stm32.cpp b/src/libcamera/pipeline/stm32/stm32.cpp\n> new file mode 100644\n> index 0000000..15c7764\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/stm32/stm32.cpp\n> @@ -0,0 +1,220 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * stm32.cpp - Pipeline handler for stm32 devices\n> + */\n> +\n> +#include <libcamera/camera.h>\n> +#include <libcamera/request.h>\n> +#include <libcamera/stream.h>\n> +\n> +#include \"device_enumerator.h\"\n> +#include \"log.h\"\n> +#include \"media_device.h\"\n> +#include \"pipeline_handler.h\"\n> +#include \"utils.h\"\n> +#include \"v4l2_device.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(STM32)\n> +\n> +class PipelineHandlerSTM32 : public PipelineHandler\n> +{\n> +public:\n> +\tPipelineHandlerSTM32(CameraManager *manager);\n> +\t~PipelineHandlerSTM32();\n> +\n> +\tstd::map<Stream *, StreamConfiguration>\n> +\tstreamConfiguration(Camera *camera, std::set<Stream *> &streams) override;\n> +\tint configureStreams(\n> +\t\tCamera *camera, std::map<Stream *, StreamConfiguration> &config) override;\n> +\n> +\tint allocateBuffers(Camera *camera, Stream *stream) override;\n> +\tint freeBuffers(Camera *camera, Stream *stream) override;\n> +\n> +\tint start(Camera *camera) override;\n> +\tvoid stop(Camera *camera) override;\n> +\n> +\tint queueRequest(Camera *camera, Request *request) override;\n> +\n> +\tbool match(DeviceEnumerator *enumerator);\n> +\n> +private:\n> +\tclass STM32CameraData : public CameraData\n> +\t{\n> +\tpublic:\n> +\t\tSTM32CameraData(PipelineHandler *pipe)\n> +\t\t\t: CameraData(pipe), video_(nullptr) {}\n> +\n> +\t\t~STM32CameraData() { delete video_; }\n> +\n> +\t\tvoid bufferReady(Buffer *buffer);\n> +\n> +\t\tV4L2Device *video_;\n> +\t\tStream stream_;\n> +\t};\n> +\n> +\tSTM32CameraData *cameraData(const Camera *camera)\n> +\t{\n> +\t\treturn static_cast<STM32CameraData *>(PipelineHandler::cameraData(camera));\n> +\t}\n> +\n> +\tstd::shared_ptr<MediaDevice> media_;\n> +};\n> +\n> +PipelineHandlerSTM32::PipelineHandlerSTM32(CameraManager *manager)\n> +\t: PipelineHandler(manager), media_(nullptr)\n> +{\n> +}\n> +\n> +PipelineHandlerSTM32::~PipelineHandlerSTM32()\n> +{\n> +\tif (media_)\n> +\t\tmedia_->release();\n> +}\n> +\n> +std::map<Stream *, StreamConfiguration>\n\nI've just learnt when rebasing the RaspberryPi handler to the latest\nmaster that this has now changed, and the Stream configurations use a\nCameraConfiguration type.\n\nIt's easy enough to fix, but you'll hit it when/if you rebase.\n\nI've attached a patch showing the changes needed for mainline, and an\nupdated v3 which applies those changes and compiles on todays master.\n\n\nHowever just talking to Niklas, it is planned within the next week to\nchange some of these API's yet again (likely only minor fixes, but still...)\n\n\n> +PipelineHandlerSTM32::streamConfiguration(Camera *camera,\n> +\t\t\t\t\t  std::set<Stream *> &streams)\n> +{\n> +\tSTM32CameraData *data = cameraData(camera);\n> +\n> +\tstd::map<Stream *, StreamConfiguration> configs;\n> +\tStreamConfiguration config{};\n> +\n> +\tLOG(STM32, Debug) << \"Retrieving default format\";\n> +\tconfig.width = 640;\n> +\tconfig.height = 480;\n> +\tconfig.pixelFormat = V4L2_PIX_FMT_YUYV;\n> +\tconfig.bufferCount = 4;\n> +\n> +\tconfigs[&data->stream_] = config;\n> +\n> +\treturn configs;\n> +}\n> +\n> +int PipelineHandlerSTM32::configureStreams(\n> +\tCamera *camera, std::map<Stream *, StreamConfiguration> &config)\n> +{\n> +\tSTM32CameraData *data = cameraData(camera);\n> +\tStreamConfiguration *cfg = &config[&data->stream_];\n> +\tint ret;\n> +\n> +\tLOG(STM32, Debug) << \"Configure the camera for resolution \" << cfg->width\n> +\t\t\t  << \"x\" << cfg->height;\n> +\n> +\tV4L2DeviceFormat format = {};\n> +\tformat.width = cfg->width;\n> +\tformat.height = cfg->height;\n> +\tformat.fourcc = cfg->pixelFormat;\n> +\n> +\tret = data->video_->setFormat(&format);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tif (format.width != cfg->width || format.height != cfg->height ||\n> +\t    format.fourcc != cfg->pixelFormat)\n> +\t\treturn -EINVAL;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int PipelineHandlerSTM32::allocateBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tSTM32CameraData *data = cameraData(camera);\n> +\tconst StreamConfiguration &cfg = stream->configuration();\n> +\n> +\tLOG(STM32, Debug) << \"Requesting \" << cfg.bufferCount << \" buffers\";\n> +\n> +\treturn data->video_->exportBuffers(&stream->bufferPool());\n> +}\n> +\n> +int PipelineHandlerSTM32::freeBuffers(Camera *camera, Stream *stream)\n> +{\n> +\tSTM32CameraData *data = cameraData(camera);\n> +\treturn data->video_->releaseBuffers();\n> +}\n> +\n> +int PipelineHandlerSTM32::start(Camera *camera)\n> +{\n> +\tSTM32CameraData *data = cameraData(camera);\n> +\treturn data->video_->streamOn();\n> +}\n> +\n> +void PipelineHandlerSTM32::stop(Camera *camera)\n> +{\n> +\tSTM32CameraData *data = cameraData(camera);\n> +\tdata->video_->streamOff();\n> +\tPipelineHandler::stop(camera);\n> +}\n> +\n> +int PipelineHandlerSTM32::queueRequest(Camera *camera, Request *request)\n> +{\n> +\tSTM32CameraData *data = cameraData(camera);\n> +\tBuffer *buffer = request->findBuffer(&data->stream_);\n> +\tif (!buffer) {\n> +\t\tLOG(STM32, Error) << \"Attempt to queue request with invalid stream\";\n> +\n> +\t\treturn -ENOENT;\n> +\t}\n> +\n> +\tint ret = data->video_->queueBuffer(buffer);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tPipelineHandler::queueRequest(camera, request);\n> +\n> +\treturn 0;\n> +}\n> +\n> +bool PipelineHandlerSTM32::match(DeviceEnumerator *enumerator)\n> +{\n> +\tDeviceMatch dm(\"stm32-dcmi\");\n> +\n> +\tmedia_ = enumerator->search(dm);\n> +\tif (!media_)\n> +\t\treturn false;\n> +\n> +\tmedia_->acquire();\n> +\n> +\tstd::unique_ptr<STM32CameraData> data =\n> +\t\tutils::make_unique<STM32CameraData>(this);\n> +\n> +\t/* Open the first video node */\n> +\tMediaEntity *entity = media_->entities()[0];\n> +\tif (!entity) {\n> +\t\tLOG(STM32, Error) << \"No video node\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\tdata->video_ = new V4L2Device(entity);\n> +\tif (!data->video_) {\n> +\t\tLOG(STM32, Error) << \"Could not find a default video device\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\tif (data->video_->open())\n> +\t\treturn false;\n> +\n> +\tdata->video_->bufferReady.connect(data.get(), &STM32CameraData::bufferReady);\n> +\n> +\t/* Create and register the camera. */\n> +\tstd::set<Stream *> streams{ &data->stream_ };\n> +\tstd::shared_ptr<Camera> camera =\n> +\t\tCamera::create(this, media_->model(), streams);\n> +\tregisterCamera(std::move(camera), std::move(data));\n> +\n> +\treturn true;\n> +}\n> +\n> +void PipelineHandlerSTM32::STM32CameraData::bufferReady(Buffer *buffer)\n> +{\n> +\tRequest *request = queuedRequests_.front();\n> +\n> +\tpipe_->completeBuffer(camera_, request, buffer);\n> +\tpipe_->completeRequest(camera_, request);\n> +}\n> +\n> +REGISTER_PIPELINE_HANDLER(PipelineHandlerSTM32);\n> +\n> +} /* namespace libcamera */\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C94A60E99\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Apr 2019 11:11:31 +0200 (CEST)","from [192.168.1.6] (net-37-182-44-227.cust.vodafonedsl.it\n\t[37.182.44.227])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 542525F;\n\tSat, 27 Apr 2019 11:11:30 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1556356290;\n\tbh=0HpIIGUifsNYsITDcB6ddu32io8mWcHv01a4Pemo0i8=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=Jam7sbRklx9c0TTQ//dKfUG3gorkyJA7rNJai7+jQkQV9TZix0abItQBvZmytWg8Q\n\tKgo6uiBDXAb6qJztaZ+B0zUNRTKc/GKrDFTi8raw2zsPhww+960tCqEFWKEu0scu3V\n\tCyXMQiXun6yRKyiIYGyQNfKyP2zZ1E89bdEGUjp8=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Benjamin Gaignard <benjamin.gaignard@st.com>,\n\tlibcamera-devel@lists.libcamera.org","Cc":"peter.griffin@linaro.org, hugues.fruchet@st.com","References":"<20190419090345.18136-1-benjamin.gaignard@st.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<969a20a3-565b-7ed9-af04-206e271fdab5@ideasonboard.com>","Date":"Sat, 27 Apr 2019 11:11:26 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.6.1","MIME-Version":"1.0","In-Reply-To":"<20190419090345.18136-1-benjamin.gaignard@st.com>","Content-Type":"multipart/mixed;\n\tboundary=\"------------EA7291C2FB5CB4B8D77C6D9E\"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: pipeline: stm32: add\n\tpipeline handler for stm32","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":"Sat, 27 Apr 2019 09:11:31 -0000"}}]