[{"id":1969,"web_url":"https://patchwork.libcamera.org/comment/1969/","msgid":"<20190622210720.GH8156@pendragon.ideasonboard.com>","date":"2019-06-22T21:07:20","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Mickael,\n\nOn Tue, Jun 18, 2019 at 01:34:04PM +0200, Mickael Guene wrote:\n> The pipeline handler for the Qualcomm ISP creates one camera instance per\n> CSI-2 sensor.\n> \n> It supports two CSI-2 sensors. One connected to CSI-2 Rx msm_csiphy0 sub-device\n> and the other one connected to CSI-2 Rx msm_csiphy1 sub-device.\n\nPlease see below for some miscellaneous comments. At the top level, I\nwonder if we really need a qcom-specific pipeline handler for this. It\nlooks like the qcom-camss driver only supports the pass-through mode of\nthe camera ISP, with a linear pipeline that doesn't expose much\nconfiguration options.\n\nBenjamin Gaignard (CC'ed) has submitted a pipeline handler for the\nSTM32 (see \"[PATCH v2] libcamera: pipeline: stm32: add pipeline handler\nfor stm32\"), which also supports linear pipelines only and doesn't have\nmuch dependencies on a particular hardware. We have discussed with him\nthe option of creating instead a \"simple pipeline handler\" that would\nsupport a whole range of simple devices, and I think the qcom-camss\nwould fit in this category.\n\nIs there a chance you could work with Benjamin to merge the two\nimplementations ?\n\n> Signed-off-by: Mickael Guene \n> ---\n> \n> src/libcamera/pipeline/meson.build | 1 +\n> src/libcamera/pipeline/qcom_camss/meson.build | 3 +\n> src/libcamera/pipeline/qcom_camss/qcom_camss.cpp | 543 +++++++++++++++++++++++\n> 3 files changed, 547 insertions(+)\n> create mode 100644 src/libcamera/pipeline/qcom_camss/meson.build\n> create mode 100644 src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n> \n> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build\n> index 0d46622..f559884 100644\n> --- a/src/libcamera/pipeline/meson.build\n> +++ b/src/libcamera/pipeline/meson.build\n> @@ -5,3 +5,4 @@ libcamera_sources += files([\n> \n> subdir('ipu3')\n> subdir('rkisp1')\n> +subdir('qcom_camss')\n\nPlease keep the entries alphabetically sorted.\n\n> diff --git a/src/libcamera/pipeline/qcom_camss/meson.build b/src/libcamera/pipeline/qcom_camss/meson.build\n> new file mode 100644\n> index 0000000..155482f\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/qcom_camss/meson.build\n> @@ -0,0 +1,3 @@\n> +libcamera_sources += files([\n> + 'qcom_camss.cpp',\n> +])\n> diff --git a/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp b/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n> new file mode 100644\n> index 0000000..6382b57\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n> @@ -0,0 +1,543 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) STMicroelectronics SA 2019\n> + *\n> + * qcom_camss.cpp - Pipeline handler for Qualcomm ISP found on 8x16 processors.\n> + */\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"camera_sensor.h\"\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> +#include \"v4l2_subdevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(QcomCamss)\n> +\n> +/* pair of supported media code and pixel format */\n> +static const std::array, 16> codes {\n> +\t{\n> +\t\t{MEDIA_BUS_FMT_UYVY8_2X8, V4L2_PIX_FMT_UYVY},\n> +\t\t{MEDIA_BUS_FMT_VYUY8_2X8, V4L2_PIX_FMT_VYUY},\n> +\t\t{MEDIA_BUS_FMT_YUYV8_2X8, V4L2_PIX_FMT_YUYV},\n> +\t\t{MEDIA_BUS_FMT_YVYU8_2X8, V4L2_PIX_FMT_YVYU},\n> +\t\t{MEDIA_BUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8},\n> +\t\t{MEDIA_BUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8},\n> +\t\t{MEDIA_BUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8},\n> +\t\t{MEDIA_BUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8},\n> +\t\t{MEDIA_BUS_FMT_SGBRG10_1X10, V4L2_PIX_FMT_SGBRG10P},\n> +\t\t{MEDIA_BUS_FMT_SBGGR10_1X10, V4L2_PIX_FMT_SBGGR10P},\n> +\t\t{MEDIA_BUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10P},\n> +\t\t{MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_PIX_FMT_SRGGB10P},\n> +\t\t{MEDIA_BUS_FMT_SGBRG12_1X12, V4L2_PIX_FMT_SGBRG12P},\n> +\t\t{MEDIA_BUS_FMT_SBGGR12_1X12, V4L2_PIX_FMT_SBGGR12P},\n> +\t\t{MEDIA_BUS_FMT_SGRBG12_1X12, V4L2_PIX_FMT_SGRBG12P},\n> +\t\t{MEDIA_BUS_FMT_SRGGB12_1X12, V4L2_PIX_FMT_SRGGB12P},\n> +\t}\n> +};\n> +\n> +static unsigned int mediaCodeFromPixelFormat(unsigned int pixelFormat)\n> +{\n> +\tfor (auto code : codes) {\n> +\t\tif (code[1] != pixelFormat)\n> +\t\t\tcontinue;\n> +\t\treturn code[0];\n> +\t}\n> +\n> +\treturn codes[0][0];\n> +}\n> +\n> +static unsigned int getDefaultPixelFormat(const CameraSensor *sensor)\n> +{\n> +\tconst std::vector< unsigned int > sensorCodes = sensor->mbusCodes();\n> +\n> +\t/* return first matching using codes ordering */\n> +\tfor (auto code : codes) {\n> +\t\tif (std::any_of(sensorCodes.begin(), sensorCodes.end(),\n> +\t\t\t\t[&](unsigned int c) { return c == code[0]; })) {\n> +\t\t\treturn code[1];\n> +\t\t}\n> +\t}\n> +\n> +\treturn codes[0][0];\n> +}\n> +\n> +static bool isPixelFormatSupportedBySensor(const CameraSensor *sensor,\n> +\t\t\t\t\t unsigned int pixelFormat)\n> +{\n> +\tconst std::vector< unsigned int > sensorCodes = sensor->mbusCodes();\n> +\n> +\tfor (auto code : codes) {\n> +\t\tif (code[1] != pixelFormat)\n> +\t\t\tcontinue;\n> +\t\tif (std::any_of(sensorCodes.begin(), sensorCodes.end(),\n> +\t\t\t\t[&](unsigned int c) { return c == code[0]; }))\n> +\t\t\treturn true;\n> +\t}\n> +\n> +\treturn false;\n> +}\n> +\n> +class QcomCamssVideoPipe\n> +{\n> +public:\n> +\tQcomCamssVideoPipe() : video_(nullptr)\n> +\t{\n> +\t}\n> +\t~QcomCamssVideoPipe()\n> +\t{\n> +\t\tfor (V4L2Subdevice *device : subDevicePipe_)\n> +\t\t\tdelete device;\n> +\t}\n> +\n> +\tbool create_and_link(MediaDevice *media,\n> +\t\t\t const std::array subDevNames,\n> +\t\t\t const std::string videoName);\n\nOur coding style uses camelCase for all methods.\n\nThe subDevNames and videoName variables should be passed as const\nreferences to avoid an unnecessary copy.\n\n> +\tint configure(V4L2SubdeviceFormat &format, StreamConfiguration &cfg);\n> +\n> +\tstd::vector subDevicePipe_;\n> +\tV4L2Device *video_;\n> +};\n> +\n> +class QcomCamssCameraData : public CameraData\n> +{\n> +public:\n> +\tQcomCamssCameraData(PipelineHandler *pipe,\n> +\t\t\t QcomCamssVideoPipe *videoPipe)\n> +\t\t: CameraData(pipe), sensor_(nullptr), videoPipe_(videoPipe),\n> +\t\t activeCamera_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\t~QcomCamssCameraData()\n> +\t{\n> +\t\tdelete sensor_;\n> +\t}\n> +\tvoid bufferReady(Buffer *buffer);\n> +\n> +\tStream stream_;\n> +\tCameraSensor *sensor_;\n> +\tQcomCamssVideoPipe *videoPipe_;\n> +\tCamera *activeCamera_;\n> +};\n> +\n> +class QcomCamssCameraConfiguration : public CameraConfiguration\n> +{\n> +public:\n> +\tQcomCamssCameraConfiguration(Camera *camera, QcomCamssCameraData *data);\n> +\n> +\tStatus validate() override;\n> +\n> +\tconst V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; }\n> +\n> +private:\n> +\tstatic constexpr unsigned int QCOMCAMSS_BUFFER_COUNT = 4;\n> +\n> +\t/*\n> +\t * The QcomCamssCameraData instance is guaranteed to be valid as long as\n> +\t * the corresponding Camera instance is valid. In order to borrow a\n> +\t * reference to the camera data, store a new reference to the camera.\n> +\t */\n> +\tstd::shared_ptr camera_;\n> +\tconst QcomCamssCameraData *data_;\n> +\n> +\tV4L2SubdeviceFormat sensorFormat_;\n> +};\n> +\n> +class PipelineHandlerQcomCamss : public PipelineHandler\n> +{\n> +public:\n> +\tPipelineHandlerQcomCamss(CameraManager *manager)\n> +\t\t: PipelineHandler(manager)\n> +\t{\n> +\t}\n> +\t~PipelineHandlerQcomCamss()\n> +\t{\n> +\t}\n> +\n> +\tCameraConfiguration *generateConfiguration(Camera *camera,\n> +\t\t\tconst StreamRoles &roles) override;\n> +\tint configure(Camera *camera, CameraConfiguration *config) override;\n> +\n> +\tint allocateBuffers(Camera *camera,\n> +\t\t\t const std::set &streams) override;\n> +\tint freeBuffers(Camera *camera,\n> +\t\t\tconst std::set &streams) 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) override;\n> +\n> +private:\n> +\tstatic constexpr unsigned int QCOMCAMSS_PIPE_NB = 2;\n> +\n> +\tQcomCamssCameraData *cameraData(const Camera *camera)\n> +\t{\n> +\t\treturn static_cast(\n> +\t\t\tPipelineHandler::cameraData(camera));\n> +\t}\n> +\n> +\tint createCamera(QcomCamssVideoPipe *videoPipe, MediaEntity *sensor);\n> +\n> +\tMediaDevice *media_;\n> +\tQcomCamssVideoPipe pipe_[QCOMCAMSS_PIPE_NB];\n> +};\n> +\n> +bool QcomCamssVideoPipe::create_and_link(MediaDevice *media,\n> +\t\tconst std::array subDevNames,\n> +\t\tconst std::string videoName)\n> +{\n> +\tV4L2Subdevice *subDev;\n> +\tMediaLink *link;\n> +\n> +\tfor (std::string subDevName : subDevNames) {\n\nsubDevName should be a const reference, there's no need to duplicate it.\n\n> +\t\tsubDev = V4L2Subdevice::fromEntityName(media, subDevName);\n> +\t\tif (subDev->open() < 0)\n> +\t\t\treturn false;\n> +\t\tsubDevicePipe_.push_back(subDev);\n> +\t}\n> +\tvideo_ = V4L2Device::fromEntityName(media, videoName);\n> +\tif (video_->open() < 0)\n> +\t\treturn false;\n> +\n> +\t/* enable links */\n\nPlease start all comments with a capital letter and end them with a\nperiod.\n\n> +\tfor (unsigned int i = 0; i < subDevNames.size() - 1; i++) {\n> +\t\tlink = media->link(subDevNames[i], 1, subDevNames[i + 1], 0);\n> +\t\tif (!link)\n> +\t\t\treturn false;\n> +\t\tif (link->setEnabled(true) < 0)\n> +\t\t\treturn false;\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> +int QcomCamssVideoPipe::configure(V4L2SubdeviceFormat &format,\n> +\t\t\t\t StreamConfiguration &cfg)\n> +{\n> +\tint ret;\n> +\n> +\tfor (unsigned int i = 0; i < subDevicePipe_.size() - 1; i++) {\n> +\t\tV4L2Subdevice *subDev = subDevicePipe_[i];\n> +\t\tret = subDev->setFormat(0, &format);\n> +\t\tif (ret < 0) {\n> +\t\t\tLOG(QcomCamss, Debug) << \"Failed to set format for subdev => \"\n> +\t\t\t\t\t << ret;\n> +\t\t\treturn ret;\n> +\t\t}\n> +\t\tret = subDev->getFormat(1, &format);\n> +\t\tif (ret < 0) {\n> +\t\t\tLOG(QcomCamss, Debug) << \"Failed to get format from subdev => \"\n> +\t\t\t\t\t << ret;\n> +\t\t\treturn ret;\n> +\t\t}\n> +\t}\n> +\tret = subDevicePipe_.back()->setFormat(0, &format);\n> +\tif (ret < 0) {\n> +\t\tLOG(QcomCamss, Debug) << \"Failed to set format for subdev => \"\n> +\t\t\t\t << ret;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tV4L2DeviceFormat outputFormat = {};\n> +\toutputFormat.fourcc = cfg.pixelFormat;\n> +\toutputFormat.size = cfg.size;\n> +\t/* our supported formats are single plane only */\n> +\toutputFormat.planesCount = 1;\n> +\n> +\tret = video_->setFormat(&outputFormat);\n> +\tLOG(QcomCamss, Debug) << \" video_ setFormat => \" << ret;\n> +\tif (ret) {\n> +\t\tLOG(QcomCamss, Debug) << \"Failed to set format for video_ => \"\n> +\t\t\t\t << ret;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (outputFormat.size != cfg.size ||\n> +\t outputFormat.fourcc != cfg.pixelFormat) {\n> +\t\tLOG(QcomCamss, Error) << \"Unable to configure capture for \"\n> +\t\t\t\t << cfg.toString();\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +void QcomCamssCameraData::bufferReady(Buffer *buffer)\n> +{\n> +\tASSERT(activeCamera_);\n> +\n> +\tRequest *request = queuedRequests_.front();\n> +\n> +\tpipe_->completeBuffer(activeCamera_, request, buffer);\n> +\tpipe_->completeRequest(activeCamera_, request);\n\nThe base CameraData class provides you with a pointer to the camera_,\nthere's no need for an activeCamera_ field.\n\n> +}\n> +\n> +QcomCamssCameraConfiguration::QcomCamssCameraConfiguration(Camera *camera,\n> +\t\tQcomCamssCameraData *data)\n> +\t: CameraConfiguration()\n> +{\n> +\tcamera_ = camera->shared_from_this();\n> +\tdata_ = data;\n> +}\n> +\n> +CameraConfiguration::Status QcomCamssCameraConfiguration::validate()\n> +{\n> +\tconst CameraSensor *sensor = data_->sensor_;\n> +\tStatus status = Valid;\n> +\n> +\tif (config_.empty())\n> +\t\treturn Invalid;\n> +\n> +\t/* Cap the number of entries to the available streams. */\n> +\tif (config_.size() > 1) {\n> +\t\tconfig_.resize(1);\n> +\t\tstatus = Adjusted;\n> +\t}\n> +\n> +\tStreamConfiguration &cfg = config_[0];\n> +\n> +\t/* Adjust the pixel format if needed */\n> +\tif (!isPixelFormatSupportedBySensor(sensor, cfg.pixelFormat)) {\n> +\t\tLOG(QcomCamss, Debug) << \"Adjusting format to default one\";\n> +\t\tcfg.pixelFormat = getDefaultPixelFormat(sensor);\n> +\t\tstatus = Adjusted;\n> +\t}\n> +\n> +\t/* Select the sensor format. */\n> +\tsensorFormat_ = sensor->getFormat({ mediaCodeFromPixelFormat(cfg.pixelFormat) },\n> +\t\t\t\t\t cfg.size);\n> +\t/* test sensorFormat_.mbus_code is valid */\n> +\tif (!sensorFormat_.mbus_code)\n> +\t\treturn Invalid;\n> +\n> +\t/* applied to cfg.size */\n> +\tconst Size size = cfg.size;\n> +\tcfg.size = sensorFormat_.size;\n> +\t/* clip for hw constraints support */\n> +\tcfg.size.width = std::max(1U, std::min(8191U, cfg.size.width));\n> +\tcfg.size.height = std::max(1U, std::min(8191U, cfg.size.height));\n> +\tif (cfg.size != size) {\n> +\t\tLOG(QcomCamss, Debug)\n> +\t\t\t<< \"Adjusting size from \" << size.toString()\n> +\t\t\t<< \" to \" << cfg.size.toString();\n> +\t\tstatus = Adjusted;\n> +\t}\n> +\n> +\tcfg.bufferCount = QCOMCAMSS_BUFFER_COUNT;\n> +\n> +\treturn status;\n> +}\n> +\n> +/* -----------------------------------------------------------------------------\n> + * Pipeline Operations\n> + */\n> +\n> +CameraConfiguration *PipelineHandlerQcomCamss::generateConfiguration(\n> +\tCamera *camera,\n> +\tconst StreamRoles &roles)\n> +{\n> +\tQcomCamssCameraData *data = cameraData(camera);\n> +\tCameraConfiguration *config =\n> +\t\tnew QcomCamssCameraConfiguration(camera, data);\n> +\n> +\tif (roles.empty())\n> +\t\treturn config;\n> +\n> +\tStreamConfiguration cfg{};\n> +\tcfg.pixelFormat = getDefaultPixelFormat(data->sensor_);\n> +\tcfg.size = data->sensor_->sizes()[0];\n> +\n> +\tconfig->addConfiguration(cfg);\n> +\n> +\tconfig->validate();\n> +\n> +\treturn config;\n> +}\n> +\n> +int PipelineHandlerQcomCamss::configure(Camera *camera, CameraConfiguration *c)\n> +{\n> +\tQcomCamssCameraConfiguration *config =\n> +\t\tstatic_cast(c);\n> +\tQcomCamssCameraData *data = cameraData(camera);\n> +\tStreamConfiguration &cfg = config->at(0);\n> +\tCameraSensor *sensor = data->sensor_;\n> +\tQcomCamssVideoPipe *videoPipe = data->videoPipe_;\n> +\tint ret;\n> +\n> +\t/*\n> +\t * Configure the format on the sensor output and propagate it through\n> +\t * the pipeline.\n> +\t */\n> +\tV4L2SubdeviceFormat format = config->sensorFormat();\n> +\tLOG(QcomCamss, Debug) << \"Configuring sensor with \"\n> +\t\t\t << format.toString();\n> +\n> +\tret = sensor->setFormat(&format);\n> +\tif (ret < 0) {\n> +\t\tLOG(QcomCamss, Error) << \"Failed to applied format for sensor => \"\n> +\t\t\t\t << ret;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = videoPipe->configure(format, cfg);\n> +\tif (ret) {\n> +\t\tLOG(QcomCamss, Debug) << \"Failed to configure format for video pipe => \"\n> +\t\t\t\t << ret;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tcfg.setStream(&data->stream_);\n> +\n> +\treturn 0;\n> +}\n> +\n> +int PipelineHandlerQcomCamss::allocateBuffers(Camera *camera,\n> +\t\t\t\t\t const std::set &streams)\n> +{\n> +\tQcomCamssCameraData *data = cameraData(camera);\n> +\n> +\tStream *stream = *streams.begin();\n> +\n> +\treturn data->videoPipe_->video_->exportBuffers(&stream->bufferPool());\n> +}\n> +\n> +int PipelineHandlerQcomCamss::freeBuffers(Camera *camera,\n> +\t\t\t\t\t const std::set &streams)\n> +{\n> +\tQcomCamssCameraData *data = cameraData(camera);\n> +\n> +\tif (data->videoPipe_->video_->releaseBuffers())\n> +\t\tLOG(QcomCamss, Error) << \"Failed to release buffers\";\n> +\n> +\treturn 0;\n> +}\n> +\n> +int PipelineHandlerQcomCamss::PipelineHandlerQcomCamss::start(Camera *camera)\n> +{\n> +\tQcomCamssCameraData *data = cameraData(camera);\n> +\tint ret;\n> +\n> +\tret = data->videoPipe_->video_->streamOn();\n> +\tif (ret)\n> +\t\tLOG(QcomCamss, Error)\n> +\t\t\t<< \"Failed to start camera \" << camera->name();\n> +\n> +\tdata->activeCamera_ = camera;\n> +\n> +\treturn ret;\n> +}\n> +\n> +void PipelineHandlerQcomCamss::stop(Camera *camera)\n> +{\n> +\tQcomCamssCameraData *data = cameraData(camera);\n> +\tint ret;\n> +\n> +\tret = data->videoPipe_->video_->streamOff();\n> +\tif (ret)\n> +\t\tLOG(QcomCamss, Warning)\n> +\t\t\t<< \"Failed to stop camera \" << camera->name();\n> +\n> +\tPipelineHandler::stop(camera);\n> +\n> +\tdata->activeCamera_ = nullptr;\n> +}\n> +\n> +int PipelineHandlerQcomCamss::queueRequest(Camera *camera, Request *request)\n> +{\n> +\tQcomCamssCameraData *data = cameraData(camera);\n> +\tStream *stream = &data->stream_;\n> +\n> +\tBuffer *buffer = request->findBuffer(stream);\n> +\tif (!buffer) {\n> +\t\tLOG(QcomCamss, Error)\n> +\t\t\t<< \"Attempt to queue request with invalid stream\";\n> +\t\treturn -ENOENT;\n> +\t}\n> +\n> +\tint ret = data->videoPipe_->video_->queueBuffer(buffer);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tPipelineHandler::queueRequest(camera, request);\n> +\n> +\treturn 0;\n> +}\n> +\n> +/* -----------------------------------------------------------------------------\n> + * Match and Setup\n> + */\n\nMissing blank line.\n\n> +int PipelineHandlerQcomCamss::createCamera(QcomCamssVideoPipe *videoPipe,\n> +\t\tMediaEntity *sensor)\n> +{\n> +\tint ret;\n> +\tstd::unique_ptr data =\n> +\t\tutils::make_unique(this, videoPipe);\n> +\n> +\tvideoPipe->video_->bufferReady.connect(data.get(),\n> +\t\t\t&QcomCamssCameraData::bufferReady);\n> +\tdata->sensor_ = new CameraSensor(sensor);\n> +\tret = data->sensor_->init();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstd::set streams{ &data->stream_ };\n> +\tstd::shared_ptr camera =\n> +\t\tCamera::create(this, sensor->name(), streams);\n> +\tregisterCamera(std::move(camera), std::move(data));\n> +\n> +\treturn 0;\n> +}\n> +\n> +bool PipelineHandlerQcomCamss::match(DeviceEnumerator *enumerator)\n> +{\n> +\tconst std::array, 2> subDevNames = {\n> +\t\t{\n> +\t\t\t{\"msm_csiphy0\", \"msm_csid0\", \"msm_ispif0\", \"msm_vfe0_rdi0\"},\n> +\t\t\t{\"msm_csiphy1\", \"msm_csid1\", \"msm_ispif1\", \"msm_vfe0_rdi1\"},\n> +\t\t}\n> +\t};\n> +\tconst std::array videoNames = {\"msm_vfe0_video0\", \"msm_vfe0_video1\"};\n\nThese two variables should be static const.\n\n> +\tconst MediaPad *pad;\n> +\tDeviceMatch dm(\"qcom-camss\");\n> +\n> +\t/*\n> +\t * Code will work with either 8x16 or 8x96 but only expose capabilities\n> +\t * of 8x16.\n> +\t */\n> +\tmedia_ = acquireMediaDevice(enumerator, dm);\n> +\tif (!media_)\n> +\t\treturn false;\n> +\n> +\tfor (unsigned int i = 0; i < QCOMCAMSS_PIPE_NB; i++) {\n> +\t\tif (!pipe_[i].create_and_link(media_, subDevNames[i],\n> +\t\t\t\t\t videoNames[i])) {\n> +\t\t\tLOG(QcomCamss, Error) << \"create_and_link failed\";\n> +\t\t\treturn false;\n> +\t\t}\n> +\n> +\t\tpad = pipe_[i].subDevicePipe_[0]->entity()->getPadByIndex(0);\n> +\t\tif (pad)\n> +\t\t\tcreateCamera(&pipe_[i],\n> +\t\t\t\t pad->links()[0]->source()->entity());\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> +REGISTER_PIPELINE_HANDLER(PipelineHandlerQcomCamss);\n> +\n> +} /* namespace libcamera */\n> \\ No newline at end of file\n\nLet's add one :-)","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2640E6002E\n\tfor ;\n\tSat, 22 Jun 2019 23:07:47 +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 9501367;\n\tSat, 22 Jun 2019 23:07:46 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561237666;\n\tbh=yE1iLoiRr1MtpIzNfvuEF/8iFAJE+6YzI5d9it9YvUY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fL7XWwyqHGrSqahlzHU4DDQxL74NvkxCF6vX+W8F9N0TeoN5omFrWEUGfC1X59102\n\tlEJ5wW0CgDVlScxRYLKjYzj+OPmLx/vUA84K+RIOqs2W5xO6sq2aZyq9DRlAVAxUAz\n\tenJ7p4R9/0E3rLwIg/xlkY1zGNX4WW71NG60T2SU=","Date":"Sun, 23 Jun 2019 00:07:20 +0300","From":"Laurent Pinchart ","To":"Mickael Guene ","Cc":"libcamera-devel@lists.libcamera.org,\n\tBenjamin Gaignard ","Message-ID":"<20190622210720.GH8156@pendragon.ideasonboard.com>","References":"<1560857644-40044-1-git-send-email-mickael.guene@st.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<1560857644-40044-1-git-send-email-mickael.guene@st.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 22 Jun 2019 21:07:47 -0000"}},{"id":2029,"web_url":"https://patchwork.libcamera.org/comment/2029/","msgid":"","date":"2019-06-26T08:34:29","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","submitter":{"id":19,"url":"https://patchwork.libcamera.org/api/people/19/","name":"Mickael GUENE","email":"mickael.guene@st.com"},"content":"Hi Laurent,\n\n Thanks for the review.\n Find my comments below.\n\nRegards\nMickael\n\nOn 6/22/19 23:07, Laurent Pinchart wrote:\n> Hi Mickael,\n> \n> On Tue, Jun 18, 2019 at 01:34:04PM +0200, Mickael Guene wrote:\n>> The pipeline handler for the Qualcomm ISP creates one camera instance per\n>> CSI-2 sensor.\n>>\n>> It supports two CSI-2 sensors. One connected to CSI-2 Rx msm_csiphy0 sub-device\n>> and the other one connected to CSI-2 Rx msm_csiphy1 sub-device.\n> \n> Please see below for some miscellaneous comments. At the top level, I\n> wonder if we really need a qcom-specific pipeline handler for this. It\n> looks like the qcom-camss driver only supports the pass-through mode of\n> the camera ISP, with a linear pipeline that doesn't expose much\n> configuration options.\n> \n> Benjamin Gaignard (CC'ed) has submitted a pipeline handler for the\n> STM32 (see \"[PATCH v2] libcamera: pipeline: stm32: add pipeline handler\n> for stm32\"), which also supports linear pipelines only and doesn't have\n> much dependencies on a particular hardware. We have discussed with him\n> the option of creating instead a \"simple pipeline handler\" that would\n> support a whole range of simple devices, and I think the qcom-camss\n> would fit in this category.\n> \n> Is there a chance you could work with Benjamin to merge the two\n> implementations ?\n>\n I have talked with Benjamin and we decided not to merge our works.\n\n>> Signed-off-by: Mickael Guene \n>> ---\n>>\n>> src/libcamera/pipeline/meson.build | 1 +\n>> src/libcamera/pipeline/qcom_camss/meson.build | 3 +\n>> src/libcamera/pipeline/qcom_camss/qcom_camss.cpp | 543 +++++++++++++++++++++++\n>> 3 files changed, 547 insertions(+)\n>> create mode 100644 src/libcamera/pipeline/qcom_camss/meson.build\n>> create mode 100644 src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n>>\n>> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build\n>> index 0d46622..f559884 100644\n>> --- a/src/libcamera/pipeline/meson.build\n>> +++ b/src/libcamera/pipeline/meson.build\n>> @@ -5,3 +5,4 @@ libcamera_sources += files([\n>> \n>> subdir('ipu3')\n>> subdir('rkisp1')\n>> +subdir('qcom_camss')\n> \n> Please keep the entries alphabetically sorted.\n>\nok\n\n>> diff --git a/src/libcamera/pipeline/qcom_camss/meson.build b/src/libcamera/pipeline/qcom_camss/meson.build\n>> new file mode 100644\n>> index 0000000..155482f\n>> --- /dev/null\n>> +++ b/src/libcamera/pipeline/qcom_camss/meson.build\n>> @@ -0,0 +1,3 @@\n>> +libcamera_sources += files([\n>> + 'qcom_camss.cpp',\n>> +])\n>> diff --git a/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp b/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n>> new file mode 100644\n>> index 0000000..6382b57\n>> --- /dev/null\n>> +++ b/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n>> @@ -0,0 +1,543 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) STMicroelectronics SA 2019\n>> + *\n>> + * qcom_camss.cpp - Pipeline handler for Qualcomm ISP found on 8x16 processors.\n>> + */\n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"camera_sensor.h\"\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>> +#include \"v4l2_subdevice.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DEFINE_CATEGORY(QcomCamss)\n>> +\n>> +/* pair of supported media code and pixel format */\n>> +static const std::array, 16> codes {\n>> +\t{\n>> +\t\t{MEDIA_BUS_FMT_UYVY8_2X8, V4L2_PIX_FMT_UYVY},\n>> +\t\t{MEDIA_BUS_FMT_VYUY8_2X8, V4L2_PIX_FMT_VYUY},\n>> +\t\t{MEDIA_BUS_FMT_YUYV8_2X8, V4L2_PIX_FMT_YUYV},\n>> +\t\t{MEDIA_BUS_FMT_YVYU8_2X8, V4L2_PIX_FMT_YVYU},\n>> +\t\t{MEDIA_BUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8},\n>> +\t\t{MEDIA_BUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8},\n>> +\t\t{MEDIA_BUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8},\n>> +\t\t{MEDIA_BUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8},\n>> +\t\t{MEDIA_BUS_FMT_SGBRG10_1X10, V4L2_PIX_FMT_SGBRG10P},\n>> +\t\t{MEDIA_BUS_FMT_SBGGR10_1X10, V4L2_PIX_FMT_SBGGR10P},\n>> +\t\t{MEDIA_BUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10P},\n>> +\t\t{MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_PIX_FMT_SRGGB10P},\n>> +\t\t{MEDIA_BUS_FMT_SGBRG12_1X12, V4L2_PIX_FMT_SGBRG12P},\n>> +\t\t{MEDIA_BUS_FMT_SBGGR12_1X12, V4L2_PIX_FMT_SBGGR12P},\n>> +\t\t{MEDIA_BUS_FMT_SGRBG12_1X12, V4L2_PIX_FMT_SGRBG12P},\n>> +\t\t{MEDIA_BUS_FMT_SRGGB12_1X12, V4L2_PIX_FMT_SRGGB12P},\n>> +\t}\n>> +};\n>> +\n>> +static unsigned int mediaCodeFromPixelFormat(unsigned int pixelFormat)\n>> +{\n>> +\tfor (auto code : codes) {\n>> +\t\tif (code[1] != pixelFormat)\n>> +\t\t\tcontinue;\n>> +\t\treturn code[0];\n>> +\t}\n>> +\n>> +\treturn codes[0][0];\n>> +}\n>> +\n>> +static unsigned int getDefaultPixelFormat(const CameraSensor *sensor)\n>> +{\n>> +\tconst std::vector< unsigned int > sensorCodes = sensor->mbusCodes();\n>> +\n>> +\t/* return first matching using codes ordering */\n>> +\tfor (auto code : codes) {\n>> +\t\tif (std::any_of(sensorCodes.begin(), sensorCodes.end(),\n>> +\t\t\t\t[&](unsigned int c) { return c == code[0]; })) {\n>> +\t\t\treturn code[1];\n>> +\t\t}\n>> +\t}\n>> +\n>> +\treturn codes[0][0];\n>> +}\n>> +\n>> +static bool isPixelFormatSupportedBySensor(const CameraSensor *sensor,\n>> +\t\t\t\t\t unsigned int pixelFormat)\n>> +{\n>> +\tconst std::vector< unsigned int > sensorCodes = sensor->mbusCodes();\n>> +\n>> +\tfor (auto code : codes) {\n>> +\t\tif (code[1] != pixelFormat)\n>> +\t\t\tcontinue;\n>> +\t\tif (std::any_of(sensorCodes.begin(), sensorCodes.end(),\n>> +\t\t\t\t[&](unsigned int c) { return c == code[0]; }))\n>> +\t\t\treturn true;\n>> +\t}\n>> +\n>> +\treturn false;\n>> +}\n>> +\n>> +class QcomCamssVideoPipe\n>> +{\n>> +public:\n>> +\tQcomCamssVideoPipe() : video_(nullptr)\n>> +\t{\n>> +\t}\n>> +\t~QcomCamssVideoPipe()\n>> +\t{\n>> +\t\tfor (V4L2Subdevice *device : subDevicePipe_)\n>> +\t\t\tdelete device;\n>> +\t}\n>> +\n>> +\tbool create_and_link(MediaDevice *media,\n>> +\t\t\t const std::array subDevNames,\n>> +\t\t\t const std::string videoName);\n> \n> Our coding style uses camelCase for all methods.\n>\nok\n\n> The subDevNames and videoName variables should be passed as const\n> references to avoid an unnecessary copy.\n>\nok\n \n>> +\tint configure(V4L2SubdeviceFormat &format, StreamConfiguration &cfg);\n>> +\n>> +\tstd::vector subDevicePipe_;\n>> +\tV4L2Device *video_;\n>> +};\n>> +\n>> +class QcomCamssCameraData : public CameraData\n>> +{\n>> +public:\n>> +\tQcomCamssCameraData(PipelineHandler *pipe,\n>> +\t\t\t QcomCamssVideoPipe *videoPipe)\n>> +\t\t: CameraData(pipe), sensor_(nullptr), videoPipe_(videoPipe),\n>> +\t\t activeCamera_(nullptr)\n>> +\t{\n>> +\t}\n>> +\n>> +\t~QcomCamssCameraData()\n>> +\t{\n>> +\t\tdelete sensor_;\n>> +\t}\n>> +\tvoid bufferReady(Buffer *buffer);\n>> +\n>> +\tStream stream_;\n>> +\tCameraSensor *sensor_;\n>> +\tQcomCamssVideoPipe *videoPipe_;\n>> +\tCamera *activeCamera_;\n>> +};\n>> +\n>> +class QcomCamssCameraConfiguration : public CameraConfiguration\n>> +{\n>> +public:\n>> +\tQcomCamssCameraConfiguration(Camera *camera, QcomCamssCameraData *data);\n>> +\n>> +\tStatus validate() override;\n>> +\n>> +\tconst V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; }\n>> +\n>> +private:\n>> +\tstatic constexpr unsigned int QCOMCAMSS_BUFFER_COUNT = 4;\n>> +\n>> +\t/*\n>> +\t * The QcomCamssCameraData instance is guaranteed to be valid as long as\n>> +\t * the corresponding Camera instance is valid. In order to borrow a\n>> +\t * reference to the camera data, store a new reference to the camera.\n>> +\t */\n>> +\tstd::shared_ptr camera_;\n>> +\tconst QcomCamssCameraData *data_;\n>> +\n>> +\tV4L2SubdeviceFormat sensorFormat_;\n>> +};\n>> +\n>> +class PipelineHandlerQcomCamss : public PipelineHandler\n>> +{\n>> +public:\n>> +\tPipelineHandlerQcomCamss(CameraManager *manager)\n>> +\t\t: PipelineHandler(manager)\n>> +\t{\n>> +\t}\n>> +\t~PipelineHandlerQcomCamss()\n>> +\t{\n>> +\t}\n>> +\n>> +\tCameraConfiguration *generateConfiguration(Camera *camera,\n>> +\t\t\tconst StreamRoles &roles) override;\n>> +\tint configure(Camera *camera, CameraConfiguration *config) override;\n>> +\n>> +\tint allocateBuffers(Camera *camera,\n>> +\t\t\t const std::set &streams) override;\n>> +\tint freeBuffers(Camera *camera,\n>> +\t\t\tconst std::set &streams) 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) override;\n>> +\n>> +private:\n>> +\tstatic constexpr unsigned int QCOMCAMSS_PIPE_NB = 2;\n>> +\n>> +\tQcomCamssCameraData *cameraData(const Camera *camera)\n>> +\t{\n>> +\t\treturn static_cast(\n>> +\t\t\tPipelineHandler::cameraData(camera));\n>> +\t}\n>> +\n>> +\tint createCamera(QcomCamssVideoPipe *videoPipe, MediaEntity *sensor);\n>> +\n>> +\tMediaDevice *media_;\n>> +\tQcomCamssVideoPipe pipe_[QCOMCAMSS_PIPE_NB];\n>> +};\n>> +\n>> +bool QcomCamssVideoPipe::create_and_link(MediaDevice *media,\n>> +\t\tconst std::array subDevNames,\n>> +\t\tconst std::string videoName)\n>> +{\n>> +\tV4L2Subdevice *subDev;\n>> +\tMediaLink *link;\n>> +\n>> +\tfor (std::string subDevName : subDevNames) {\n> \n> subDevName should be a const reference, there's no need to duplicate it.\n>\nok\n\n>> +\t\tsubDev = V4L2Subdevice::fromEntityName(media, subDevName);\n>> +\t\tif (subDev->open() < 0)\n>> +\t\t\treturn false;\n>> +\t\tsubDevicePipe_.push_back(subDev);\n>> +\t}\n>> +\tvideo_ = V4L2Device::fromEntityName(media, videoName);\n>> +\tif (video_->open() < 0)\n>> +\t\treturn false;\n>> +\n>> +\t/* enable links */\n> \n> Please start all comments with a capital letter and end them with a\n> period.\n>\nok\n\n>> +\tfor (unsigned int i = 0; i < subDevNames.size() - 1; i++) {\n>> +\t\tlink = media->link(subDevNames[i], 1, subDevNames[i + 1], 0);\n>> +\t\tif (!link)\n>> +\t\t\treturn false;\n>> +\t\tif (link->setEnabled(true) < 0)\n>> +\t\t\treturn false;\n>> +\t}\n>> +\n>> +\treturn true;\n>> +}\n>> +\n>> +int QcomCamssVideoPipe::configure(V4L2SubdeviceFormat &format,\n>> +\t\t\t\t StreamConfiguration &cfg)\n>> +{\n>> +\tint ret;\n>> +\n>> +\tfor (unsigned int i = 0; i < subDevicePipe_.size() - 1; i++) {\n>> +\t\tV4L2Subdevice *subDev = subDevicePipe_[i];\n>> +\t\tret = subDev->setFormat(0, &format);\n>> +\t\tif (ret < 0) {\n>> +\t\t\tLOG(QcomCamss, Debug) << \"Failed to set format for subdev => \"\n>> +\t\t\t\t\t << ret;\n>> +\t\t\treturn ret;\n>> +\t\t}\n>> +\t\tret = subDev->getFormat(1, &format);\n>> +\t\tif (ret < 0) {\n>> +\t\t\tLOG(QcomCamss, Debug) << \"Failed to get format from subdev => \"\n>> +\t\t\t\t\t << ret;\n>> +\t\t\treturn ret;\n>> +\t\t}\n>> +\t}\n>> +\tret = subDevicePipe_.back()->setFormat(0, &format);\n>> +\tif (ret < 0) {\n>> +\t\tLOG(QcomCamss, Debug) << \"Failed to set format for subdev => \"\n>> +\t\t\t\t << ret;\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tV4L2DeviceFormat outputFormat = {};\n>> +\toutputFormat.fourcc = cfg.pixelFormat;\n>> +\toutputFormat.size = cfg.size;\n>> +\t/* our supported formats are single plane only */\n>> +\toutputFormat.planesCount = 1;\n>> +\n>> +\tret = video_->setFormat(&outputFormat);\n>> +\tLOG(QcomCamss, Debug) << \" video_ setFormat => \" << ret;\n>> +\tif (ret) {\n>> +\t\tLOG(QcomCamss, Debug) << \"Failed to set format for video_ => \"\n>> +\t\t\t\t << ret;\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tif (outputFormat.size != cfg.size ||\n>> +\t outputFormat.fourcc != cfg.pixelFormat) {\n>> +\t\tLOG(QcomCamss, Error) << \"Unable to configure capture for \"\n>> +\t\t\t\t << cfg.toString();\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +void QcomCamssCameraData::bufferReady(Buffer *buffer)\n>> +{\n>> +\tASSERT(activeCamera_);\n>> +\n>> +\tRequest *request = queuedRequests_.front();\n>> +\n>> +\tpipe_->completeBuffer(activeCamera_, request, buffer);\n>> +\tpipe_->completeRequest(activeCamera_, request);\n> \n> The base CameraData class provides you with a pointer to the camera_,\n> there's no need for an activeCamera_ field.\n>\nok\n\n>> +}\n>> +\n>> +QcomCamssCameraConfiguration::QcomCamssCameraConfiguration(Camera *camera,\n>> +\t\tQcomCamssCameraData *data)\n>> +\t: CameraConfiguration()\n>> +{\n>> +\tcamera_ = camera->shared_from_this();\n>> +\tdata_ = data;\n>> +}\n>> +\n>> +CameraConfiguration::Status QcomCamssCameraConfiguration::validate()\n>> +{\n>> +\tconst CameraSensor *sensor = data_->sensor_;\n>> +\tStatus status = Valid;\n>> +\n>> +\tif (config_.empty())\n>> +\t\treturn Invalid;\n>> +\n>> +\t/* Cap the number of entries to the available streams. */\n>> +\tif (config_.size() > 1) {\n>> +\t\tconfig_.resize(1);\n>> +\t\tstatus = Adjusted;\n>> +\t}\n>> +\n>> +\tStreamConfiguration &cfg = config_[0];\n>> +\n>> +\t/* Adjust the pixel format if needed */\n>> +\tif (!isPixelFormatSupportedBySensor(sensor, cfg.pixelFormat)) {\n>> +\t\tLOG(QcomCamss, Debug) << \"Adjusting format to default one\";\n>> +\t\tcfg.pixelFormat = getDefaultPixelFormat(sensor);\n>> +\t\tstatus = Adjusted;\n>> +\t}\n>> +\n>> +\t/* Select the sensor format. */\n>> +\tsensorFormat_ = sensor->getFormat({ mediaCodeFromPixelFormat(cfg.pixelFormat) },\n>> +\t\t\t\t\t cfg.size);\n>> +\t/* test sensorFormat_.mbus_code is valid */\n>> +\tif (!sensorFormat_.mbus_code)\n>> +\t\treturn Invalid;\n>> +\n>> +\t/* applied to cfg.size */\n>> +\tconst Size size = cfg.size;\n>> +\tcfg.size = sensorFormat_.size;\n>> +\t/* clip for hw constraints support */\n>> +\tcfg.size.width = std::max(1U, std::min(8191U, cfg.size.width));\n>> +\tcfg.size.height = std::max(1U, std::min(8191U, cfg.size.height));\n>> +\tif (cfg.size != size) {\n>> +\t\tLOG(QcomCamss, Debug)\n>> +\t\t\t<< \"Adjusting size from \" << size.toString()\n>> +\t\t\t<< \" to \" << cfg.size.toString();\n>> +\t\tstatus = Adjusted;\n>> +\t}\n>> +\n>> +\tcfg.bufferCount = QCOMCAMSS_BUFFER_COUNT;\n>> +\n>> +\treturn status;\n>> +}\n>> +\n>> +/* -----------------------------------------------------------------------------\n>> + * Pipeline Operations\n>> + */\n>> +\n>> +CameraConfiguration *PipelineHandlerQcomCamss::generateConfiguration(\n>> +\tCamera *camera,\n>> +\tconst StreamRoles &roles)\n>> +{\n>> +\tQcomCamssCameraData *data = cameraData(camera);\n>> +\tCameraConfiguration *config =\n>> +\t\tnew QcomCamssCameraConfiguration(camera, data);\n>> +\n>> +\tif (roles.empty())\n>> +\t\treturn config;\n>> +\n>> +\tStreamConfiguration cfg{};\n>> +\tcfg.pixelFormat = getDefaultPixelFormat(data->sensor_);\n>> +\tcfg.size = data->sensor_->sizes()[0];\n>> +\n>> +\tconfig->addConfiguration(cfg);\n>> +\n>> +\tconfig->validate();\n>> +\n>> +\treturn config;\n>> +}\n>> +\n>> +int PipelineHandlerQcomCamss::configure(Camera *camera, CameraConfiguration *c)\n>> +{\n>> +\tQcomCamssCameraConfiguration *config =\n>> +\t\tstatic_cast(c);\n>> +\tQcomCamssCameraData *data = cameraData(camera);\n>> +\tStreamConfiguration &cfg = config->at(0);\n>> +\tCameraSensor *sensor = data->sensor_;\n>> +\tQcomCamssVideoPipe *videoPipe = data->videoPipe_;\n>> +\tint ret;\n>> +\n>> +\t/*\n>> +\t * Configure the format on the sensor output and propagate it through\n>> +\t * the pipeline.\n>> +\t */\n>> +\tV4L2SubdeviceFormat format = config->sensorFormat();\n>> +\tLOG(QcomCamss, Debug) << \"Configuring sensor with \"\n>> +\t\t\t << format.toString();\n>> +\n>> +\tret = sensor->setFormat(&format);\n>> +\tif (ret < 0) {\n>> +\t\tLOG(QcomCamss, Error) << \"Failed to applied format for sensor => \"\n>> +\t\t\t\t << ret;\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tret = videoPipe->configure(format, cfg);\n>> +\tif (ret) {\n>> +\t\tLOG(QcomCamss, Debug) << \"Failed to configure format for video pipe => \"\n>> +\t\t\t\t << ret;\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tcfg.setStream(&data->stream_);\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +int PipelineHandlerQcomCamss::allocateBuffers(Camera *camera,\n>> +\t\t\t\t\t const std::set &streams)\n>> +{\n>> +\tQcomCamssCameraData *data = cameraData(camera);\n>> +\n>> +\tStream *stream = *streams.begin();\n>> +\n>> +\treturn data->videoPipe_->video_->exportBuffers(&stream->bufferPool());\n>> +}\n>> +\n>> +int PipelineHandlerQcomCamss::freeBuffers(Camera *camera,\n>> +\t\t\t\t\t const std::set &streams)\n>> +{\n>> +\tQcomCamssCameraData *data = cameraData(camera);\n>> +\n>> +\tif (data->videoPipe_->video_->releaseBuffers())\n>> +\t\tLOG(QcomCamss, Error) << \"Failed to release buffers\";\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +int PipelineHandlerQcomCamss::PipelineHandlerQcomCamss::start(Camera *camera)\n>> +{\n>> +\tQcomCamssCameraData *data = cameraData(camera);\n>> +\tint ret;\n>> +\n>> +\tret = data->videoPipe_->video_->streamOn();\n>> +\tif (ret)\n>> +\t\tLOG(QcomCamss, Error)\n>> +\t\t\t<< \"Failed to start camera \" << camera->name();\n>> +\n>> +\tdata->activeCamera_ = camera;\n>> +\n>> +\treturn ret;\n>> +}\n>> +\n>> +void PipelineHandlerQcomCamss::stop(Camera *camera)\n>> +{\n>> +\tQcomCamssCameraData *data = cameraData(camera);\n>> +\tint ret;\n>> +\n>> +\tret = data->videoPipe_->video_->streamOff();\n>> +\tif (ret)\n>> +\t\tLOG(QcomCamss, Warning)\n>> +\t\t\t<< \"Failed to stop camera \" << camera->name();\n>> +\n>> +\tPipelineHandler::stop(camera);\n>> +\n>> +\tdata->activeCamera_ = nullptr;\n>> +}\n>> +\n>> +int PipelineHandlerQcomCamss::queueRequest(Camera *camera, Request *request)\n>> +{\n>> +\tQcomCamssCameraData *data = cameraData(camera);\n>> +\tStream *stream = &data->stream_;\n>> +\n>> +\tBuffer *buffer = request->findBuffer(stream);\n>> +\tif (!buffer) {\n>> +\t\tLOG(QcomCamss, Error)\n>> +\t\t\t<< \"Attempt to queue request with invalid stream\";\n>> +\t\treturn -ENOENT;\n>> +\t}\n>> +\n>> +\tint ret = data->videoPipe_->video_->queueBuffer(buffer);\n>> +\tif (ret < 0)\n>> +\t\treturn ret;\n>> +\n>> +\tPipelineHandler::queueRequest(camera, request);\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +/* -----------------------------------------------------------------------------\n>> + * Match and Setup\n>> + */\n> \n> Missing blank line.\n> \nok\n\n>> +int PipelineHandlerQcomCamss::createCamera(QcomCamssVideoPipe *videoPipe,\n>> +\t\tMediaEntity *sensor)\n>> +{\n>> +\tint ret;\n>> +\tstd::unique_ptr data =\n>> +\t\tutils::make_unique(this, videoPipe);\n>> +\n>> +\tvideoPipe->video_->bufferReady.connect(data.get(),\n>> +\t\t\t&QcomCamssCameraData::bufferReady);\n>> +\tdata->sensor_ = new CameraSensor(sensor);\n>> +\tret = data->sensor_->init();\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> +\tstd::set streams{ &data->stream_ };\n>> +\tstd::shared_ptr camera =\n>> +\t\tCamera::create(this, sensor->name(), streams);\n>> +\tregisterCamera(std::move(camera), std::move(data));\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +bool PipelineHandlerQcomCamss::match(DeviceEnumerator *enumerator)\n>> +{\n>> +\tconst std::array, 2> subDevNames = {\n>> +\t\t{\n>> +\t\t\t{\"msm_csiphy0\", \"msm_csid0\", \"msm_ispif0\", \"msm_vfe0_rdi0\"},\n>> +\t\t\t{\"msm_csiphy1\", \"msm_csid1\", \"msm_ispif1\", \"msm_vfe0_rdi1\"},\n>> +\t\t}\n>> +\t};\n>> +\tconst std::array videoNames = {\"msm_vfe0_video0\", \"msm_vfe0_video1\"};\n> \n> These two variables should be static const.\n> \nok\n\n>> +\tconst MediaPad *pad;\n>> +\tDeviceMatch dm(\"qcom-camss\");\n>> +\n>> +\t/*\n>> +\t * Code will work with either 8x16 or 8x96 but only expose capabilities\n>> +\t * of 8x16.\n>> +\t */\n>> +\tmedia_ = acquireMediaDevice(enumerator, dm);\n>> +\tif (!media_)\n>> +\t\treturn false;\n>> +\n>> +\tfor (unsigned int i = 0; i < QCOMCAMSS_PIPE_NB; i++) {\n>> +\t\tif (!pipe_[i].create_and_link(media_, subDevNames[i],\n>> +\t\t\t\t\t videoNames[i])) {\n>> +\t\t\tLOG(QcomCamss, Error) << \"create_and_link failed\";\n>> +\t\t\treturn false;\n>> +\t\t}\n>> +\n>> +\t\tpad = pipe_[i].subDevicePipe_[0]->entity()->getPadByIndex(0);\n>> +\t\tif (pad)\n>> +\t\t\tcreateCamera(&pipe_[i],\n>> +\t\t\t\t pad->links()[0]->source()->entity());\n>> +\t}\n>> +\n>> +\treturn true;\n>> +}\n>> +\n>> +REGISTER_PIPELINE_HANDLER(PipelineHandlerQcomCamss);\n>> +\n>> +} /* namespace libcamera */\n>> \\ No newline at end of file\n> \n> Let's add one :-)\n> \nok","headers":{"Return-Path":"","Received":["from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com\n\t[62.209.51.94])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5295260BF9\n\tfor ;\n\tWed, 26 Jun 2019 10:34:33 +0200 (CEST)","from pps.filterd (m0046668.ppops.net [127.0.0.1])\n\tby mx07-00178001.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id\n\tx5Q8WlXc028557; Wed, 26 Jun 2019 10:34:31 +0200","from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35])\n\tby mx07-00178001.pphosted.com with ESMTP id 2t9d2geq58-1\n\t(version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT);\n\tWed, 26 Jun 2019 10:34:31 +0200","from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9])\n\tby beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 14F503A;\n\tWed, 26 Jun 2019 08:34:29 +0000 (GMT)","from Webmail-eu.st.com (sfhdag3node1.st.com [10.75.127.7])\n\tby zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id C08F31ACF;\n\tWed, 26 Jun 2019 08:34:29 +0000 (GMT)","from SFHDAG5NODE3.st.com (10.75.127.15) by SFHDAG3NODE1.st.com\n\t(10.75.127.7) with Microsoft SMTP Server (TLS) id 15.0.1347.2;\n\tWed, 26 Jun 2019 10:34:29 +0200","from SFHDAG5NODE3.st.com ([fe80::7c09:5d6b:d2c7:5f47]) by\n\tSFHDAG5NODE3.st.com ([fe80::7c09:5d6b:d2c7:5f47%20]) with mapi id\n\t15.00.1473.003; Wed, 26 Jun 2019 10:34:29 +0200"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=st.com;\n\th=from : to : cc : subject\n\t: date : message-id : references : in-reply-to : content-type :\n\tcontent-id\n\t: content-transfer-encoding : mime-version; s=STMicroelectronics;\n\tbh=945TU8uOQsgCwQlUKkeMyqX6eC3+IlndL7j29C3YJuY=;\n\tb=SsyzhBe8d9LwbtIOhW7IMa/WJ4lEK2hGHWXFpkaHWCMRrr/iif6vJnPY5rWA65VvO3kF\n\tRMqT/hGuK5CAHd2fZ00OoErWzCFTQI89J7Z4k//ZSdkNVThoKyfavrlqZnIVlqKT0HVA\n\t5iRd424TQBu/gAD2q9XDssbaPTMdPbMDTcLefK8I+bAQfqGgrNXUVv4BG2i6pUZK7VAl\n\tHb3Q3O9ChhQXmTUGLCmDKmytGvuxyfQh/OLpzXka3sHUxiM44X8A4jEPPthkhNJLfIBJ\n\txplR/MB5/GGIespIa9tc2fAnX2pDI/P12g57qI6lQfqPxXUaGTux7TPcBU3GZckJtoEP\n\tVw== ","From":"Mickael GUENE ","To":"Laurent Pinchart ","CC":"\"libcamera-devel@lists.libcamera.org\"\n\t, Benjamin GAIGNARD\n\t","Thread-Topic":"[libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","Thread-Index":"AQHVJcm9eS3Ka889s0ueRsyrGrJQuqaoD9oAgAV2/IA=","Date":"Wed, 26 Jun 2019 08:34:29 +0000","Message-ID":"","References":"<1560857644-40044-1-git-send-email-mickael.guene@st.com>\n\t<20190622210720.GH8156@pendragon.ideasonboard.com>","In-Reply-To":"<20190622210720.GH8156@pendragon.ideasonboard.com>","Accept-Language":"fr-FR, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.1","x-ms-exchange-messagesentrepresentingtype":"1","x-ms-exchange-transport-fromentityheader":"Hosted","x-originating-ip":"[10.75.127.50]","Content-Type":"text/plain; charset=\"utf-8\"","Content-ID":"<06A0A0CB813D48478DB560548A760567@st.com>","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10434:, ,\n\tdefinitions=2019-06-26_04:, , signatures=0","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 26 Jun 2019 08:34:33 -0000"}},{"id":2030,"web_url":"https://patchwork.libcamera.org/comment/2030/","msgid":"<20190626083914.GB4776@pendragon.ideasonboard.com>","date":"2019-06-26T08:39:14","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Mickael,\n\nOn Wed, Jun 26, 2019 at 08:34:29AM +0000, Mickael GUENE wrote:\n> Hi Laurent,\n> \n> Thanks for the review.\n> Find my comments below.\n> \n> Regards\n> Mickael\n> \n> On 6/22/19 23:07, Laurent Pinchart wrote:\n> > Hi Mickael,\n> > \n> > On Tue, Jun 18, 2019 at 01:34:04PM +0200, Mickael Guene wrote:\n> >> The pipeline handler for the Qualcomm ISP creates one camera instance per\n> >> CSI-2 sensor.\n> >>\n> >> It supports two CSI-2 sensors. One connected to CSI-2 Rx msm_csiphy0 sub-device\n> >> and the other one connected to CSI-2 Rx msm_csiphy1 sub-device.\n> > \n> > Please see below for some miscellaneous comments. At the top level, I\n> > wonder if we really need a qcom-specific pipeline handler for this. It\n> > looks like the qcom-camss driver only supports the pass-through mode of\n> > the camera ISP, with a linear pipeline that doesn't expose much\n> > configuration options.\n> > \n> > Benjamin Gaignard (CC'ed) has submitted a pipeline handler for the\n> > STM32 (see \"[PATCH v2] libcamera: pipeline: stm32: add pipeline handler\n> > for stm32\"), which also supports linear pipelines only and doesn't have\n> > much dependencies on a particular hardware. We have discussed with him\n> > the option of creating instead a \"simple pipeline handler\" that would\n> > support a whole range of simple devices, and I think the qcom-camss\n> > would fit in this category.\n> > \n> > Is there a chance you could work with Benjamin to merge the two\n> > implementations ?\n>\n> I have talked with Benjamin and we decided not to merge our works.\n\nNo worries, but I'm afraid that a qcom-camss-specific pipeline handler\ndoesn't make much sense without ISP support. It came to my attention\nyesterday that work was ongoing on a pipeline handler for the i.MX SoCs,\nwhich could fall in the same category. We don't want to see a\nmultiplication of nearly identical pipeline handlers for linear\npipelines without an ISP, they should all be supported through the same\ncode.\n\n> >> Signed-off-by: Mickael Guene \n> >> ---\n> >>\n> >> src/libcamera/pipeline/meson.build | 1 +\n> >> src/libcamera/pipeline/qcom_camss/meson.build | 3 +\n> >> src/libcamera/pipeline/qcom_camss/qcom_camss.cpp | 543 +++++++++++++++++++++++\n> >> 3 files changed, 547 insertions(+)\n> >> create mode 100644 src/libcamera/pipeline/qcom_camss/meson.build\n> >> create mode 100644 src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n> >>\n> >> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build\n> >> index 0d46622..f559884 100644\n> >> --- a/src/libcamera/pipeline/meson.build\n> >> +++ b/src/libcamera/pipeline/meson.build\n> >> @@ -5,3 +5,4 @@ libcamera_sources += files([\n> >> \n> >> subdir('ipu3')\n> >> subdir('rkisp1')\n> >> +subdir('qcom_camss')\n> > \n> > Please keep the entries alphabetically sorted.\n> >\n> ok\n> \n> >> diff --git a/src/libcamera/pipeline/qcom_camss/meson.build b/src/libcamera/pipeline/qcom_camss/meson.build\n> >> new file mode 100644\n> >> index 0000000..155482f\n> >> --- /dev/null\n> >> +++ b/src/libcamera/pipeline/qcom_camss/meson.build\n> >> @@ -0,0 +1,3 @@\n> >> +libcamera_sources += files([\n> >> + 'qcom_camss.cpp',\n> >> +])\n> >> diff --git a/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp b/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n> >> new file mode 100644\n> >> index 0000000..6382b57\n> >> --- /dev/null\n> >> +++ b/src/libcamera/pipeline/qcom_camss/qcom_camss.cpp\n> >> @@ -0,0 +1,543 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) STMicroelectronics SA 2019\n> >> + *\n> >> + * qcom_camss.cpp - Pipeline handler for Qualcomm ISP found on 8x16 processors.\n> >> + */\n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \"camera_sensor.h\"\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> >> +#include \"v4l2_subdevice.h\"\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +LOG_DEFINE_CATEGORY(QcomCamss)\n> >> +\n> >> +/* pair of supported media code and pixel format */\n> >> +static const std::array, 16> codes {\n> >> +\t{\n> >> +\t\t{MEDIA_BUS_FMT_UYVY8_2X8, V4L2_PIX_FMT_UYVY},\n> >> +\t\t{MEDIA_BUS_FMT_VYUY8_2X8, V4L2_PIX_FMT_VYUY},\n> >> +\t\t{MEDIA_BUS_FMT_YUYV8_2X8, V4L2_PIX_FMT_YUYV},\n> >> +\t\t{MEDIA_BUS_FMT_YVYU8_2X8, V4L2_PIX_FMT_YVYU},\n> >> +\t\t{MEDIA_BUS_FMT_SGBRG8_1X8, V4L2_PIX_FMT_SGBRG8},\n> >> +\t\t{MEDIA_BUS_FMT_SBGGR8_1X8, V4L2_PIX_FMT_SBGGR8},\n> >> +\t\t{MEDIA_BUS_FMT_SGRBG8_1X8, V4L2_PIX_FMT_SGRBG8},\n> >> +\t\t{MEDIA_BUS_FMT_SRGGB8_1X8, V4L2_PIX_FMT_SRGGB8},\n> >> +\t\t{MEDIA_BUS_FMT_SGBRG10_1X10, V4L2_PIX_FMT_SGBRG10P},\n> >> +\t\t{MEDIA_BUS_FMT_SBGGR10_1X10, V4L2_PIX_FMT_SBGGR10P},\n> >> +\t\t{MEDIA_BUS_FMT_SGRBG10_1X10, V4L2_PIX_FMT_SGRBG10P},\n> >> +\t\t{MEDIA_BUS_FMT_SRGGB10_1X10, V4L2_PIX_FMT_SRGGB10P},\n> >> +\t\t{MEDIA_BUS_FMT_SGBRG12_1X12, V4L2_PIX_FMT_SGBRG12P},\n> >> +\t\t{MEDIA_BUS_FMT_SBGGR12_1X12, V4L2_PIX_FMT_SBGGR12P},\n> >> +\t\t{MEDIA_BUS_FMT_SGRBG12_1X12, V4L2_PIX_FMT_SGRBG12P},\n> >> +\t\t{MEDIA_BUS_FMT_SRGGB12_1X12, V4L2_PIX_FMT_SRGGB12P},\n> >> +\t}\n> >> +};\n> >> +\n> >> +static unsigned int mediaCodeFromPixelFormat(unsigned int pixelFormat)\n> >> +{\n> >> +\tfor (auto code : codes) {\n> >> +\t\tif (code[1] != pixelFormat)\n> >> +\t\t\tcontinue;\n> >> +\t\treturn code[0];\n> >> +\t}\n> >> +\n> >> +\treturn codes[0][0];\n> >> +}\n> >> +\n> >> +static unsigned int getDefaultPixelFormat(const CameraSensor *sensor)\n> >> +{\n> >> +\tconst std::vector< unsigned int > sensorCodes = sensor->mbusCodes();\n> >> +\n> >> +\t/* return first matching using codes ordering */\n> >> +\tfor (auto code : codes) {\n> >> +\t\tif (std::any_of(sensorCodes.begin(), sensorCodes.end(),\n> >> +\t\t\t\t[&](unsigned int c) { return c == code[0]; })) {\n> >> +\t\t\treturn code[1];\n> >> +\t\t}\n> >> +\t}\n> >> +\n> >> +\treturn codes[0][0];\n> >> +}\n> >> +\n> >> +static bool isPixelFormatSupportedBySensor(const CameraSensor *sensor,\n> >> +\t\t\t\t\t unsigned int pixelFormat)\n> >> +{\n> >> +\tconst std::vector< unsigned int > sensorCodes = sensor->mbusCodes();\n> >> +\n> >> +\tfor (auto code : codes) {\n> >> +\t\tif (code[1] != pixelFormat)\n> >> +\t\t\tcontinue;\n> >> +\t\tif (std::any_of(sensorCodes.begin(), sensorCodes.end(),\n> >> +\t\t\t\t[&](unsigned int c) { return c == code[0]; }))\n> >> +\t\t\treturn true;\n> >> +\t}\n> >> +\n> >> +\treturn false;\n> >> +}\n> >> +\n> >> +class QcomCamssVideoPipe\n> >> +{\n> >> +public:\n> >> +\tQcomCamssVideoPipe() : video_(nullptr)\n> >> +\t{\n> >> +\t}\n> >> +\t~QcomCamssVideoPipe()\n> >> +\t{\n> >> +\t\tfor (V4L2Subdevice *device : subDevicePipe_)\n> >> +\t\t\tdelete device;\n> >> +\t}\n> >> +\n> >> +\tbool create_and_link(MediaDevice *media,\n> >> +\t\t\t const std::array subDevNames,\n> >> +\t\t\t const std::string videoName);\n> > \n> > Our coding style uses camelCase for all methods.\n> >\n> ok\n> \n> > The subDevNames and videoName variables should be passed as const\n> > references to avoid an unnecessary copy.\n> >\n> ok\n> \n> >> +\tint configure(V4L2SubdeviceFormat &format, StreamConfiguration &cfg);\n> >> +\n> >> +\tstd::vector subDevicePipe_;\n> >> +\tV4L2Device *video_;\n> >> +};\n> >> +\n> >> +class QcomCamssCameraData : public CameraData\n> >> +{\n> >> +public:\n> >> +\tQcomCamssCameraData(PipelineHandler *pipe,\n> >> +\t\t\t QcomCamssVideoPipe *videoPipe)\n> >> +\t\t: CameraData(pipe), sensor_(nullptr), videoPipe_(videoPipe),\n> >> +\t\t activeCamera_(nullptr)\n> >> +\t{\n> >> +\t}\n> >> +\n> >> +\t~QcomCamssCameraData()\n> >> +\t{\n> >> +\t\tdelete sensor_;\n> >> +\t}\n> >> +\tvoid bufferReady(Buffer *buffer);\n> >> +\n> >> +\tStream stream_;\n> >> +\tCameraSensor *sensor_;\n> >> +\tQcomCamssVideoPipe *videoPipe_;\n> >> +\tCamera *activeCamera_;\n> >> +};\n> >> +\n> >> +class QcomCamssCameraConfiguration : public CameraConfiguration\n> >> +{\n> >> +public:\n> >> +\tQcomCamssCameraConfiguration(Camera *camera, QcomCamssCameraData *data);\n> >> +\n> >> +\tStatus validate() override;\n> >> +\n> >> +\tconst V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; }\n> >> +\n> >> +private:\n> >> +\tstatic constexpr unsigned int QCOMCAMSS_BUFFER_COUNT = 4;\n> >> +\n> >> +\t/*\n> >> +\t * The QcomCamssCameraData instance is guaranteed to be valid as long as\n> >> +\t * the corresponding Camera instance is valid. In order to borrow a\n> >> +\t * reference to the camera data, store a new reference to the camera.\n> >> +\t */\n> >> +\tstd::shared_ptr camera_;\n> >> +\tconst QcomCamssCameraData *data_;\n> >> +\n> >> +\tV4L2SubdeviceFormat sensorFormat_;\n> >> +};\n> >> +\n> >> +class PipelineHandlerQcomCamss : public PipelineHandler\n> >> +{\n> >> +public:\n> >> +\tPipelineHandlerQcomCamss(CameraManager *manager)\n> >> +\t\t: PipelineHandler(manager)\n> >> +\t{\n> >> +\t}\n> >> +\t~PipelineHandlerQcomCamss()\n> >> +\t{\n> >> +\t}\n> >> +\n> >> +\tCameraConfiguration *generateConfiguration(Camera *camera,\n> >> +\t\t\tconst StreamRoles &roles) override;\n> >> +\tint configure(Camera *camera, CameraConfiguration *config) override;\n> >> +\n> >> +\tint allocateBuffers(Camera *camera,\n> >> +\t\t\t const std::set &streams) override;\n> >> +\tint freeBuffers(Camera *camera,\n> >> +\t\t\tconst std::set &streams) 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) override;\n> >> +\n> >> +private:\n> >> +\tstatic constexpr unsigned int QCOMCAMSS_PIPE_NB = 2;\n> >> +\n> >> +\tQcomCamssCameraData *cameraData(const Camera *camera)\n> >> +\t{\n> >> +\t\treturn static_cast(\n> >> +\t\t\tPipelineHandler::cameraData(camera));\n> >> +\t}\n> >> +\n> >> +\tint createCamera(QcomCamssVideoPipe *videoPipe, MediaEntity *sensor);\n> >> +\n> >> +\tMediaDevice *media_;\n> >> +\tQcomCamssVideoPipe pipe_[QCOMCAMSS_PIPE_NB];\n> >> +};\n> >> +\n> >> +bool QcomCamssVideoPipe::create_and_link(MediaDevice *media,\n> >> +\t\tconst std::array subDevNames,\n> >> +\t\tconst std::string videoName)\n> >> +{\n> >> +\tV4L2Subdevice *subDev;\n> >> +\tMediaLink *link;\n> >> +\n> >> +\tfor (std::string subDevName : subDevNames) {\n> > \n> > subDevName should be a const reference, there's no need to duplicate it.\n> >\n> ok\n> \n> >> +\t\tsubDev = V4L2Subdevice::fromEntityName(media, subDevName);\n> >> +\t\tif (subDev->open() < 0)\n> >> +\t\t\treturn false;\n> >> +\t\tsubDevicePipe_.push_back(subDev);\n> >> +\t}\n> >> +\tvideo_ = V4L2Device::fromEntityName(media, videoName);\n> >> +\tif (video_->open() < 0)\n> >> +\t\treturn false;\n> >> +\n> >> +\t/* enable links */\n> > \n> > Please start all comments with a capital letter and end them with a\n> > period.\n> >\n> ok\n> \n> >> +\tfor (unsigned int i = 0; i < subDevNames.size() - 1; i++) {\n> >> +\t\tlink = media->link(subDevNames[i], 1, subDevNames[i + 1], 0);\n> >> +\t\tif (!link)\n> >> +\t\t\treturn false;\n> >> +\t\tif (link->setEnabled(true) < 0)\n> >> +\t\t\treturn false;\n> >> +\t}\n> >> +\n> >> +\treturn true;\n> >> +}\n> >> +\n> >> +int QcomCamssVideoPipe::configure(V4L2SubdeviceFormat &format,\n> >> +\t\t\t\t StreamConfiguration &cfg)\n> >> +{\n> >> +\tint ret;\n> >> +\n> >> +\tfor (unsigned int i = 0; i < subDevicePipe_.size() - 1; i++) {\n> >> +\t\tV4L2Subdevice *subDev = subDevicePipe_[i];\n> >> +\t\tret = subDev->setFormat(0, &format);\n> >> +\t\tif (ret < 0) {\n> >> +\t\t\tLOG(QcomCamss, Debug) << \"Failed to set format for subdev => \"\n> >> +\t\t\t\t\t << ret;\n> >> +\t\t\treturn ret;\n> >> +\t\t}\n> >> +\t\tret = subDev->getFormat(1, &format);\n> >> +\t\tif (ret < 0) {\n> >> +\t\t\tLOG(QcomCamss, Debug) << \"Failed to get format from subdev => \"\n> >> +\t\t\t\t\t << ret;\n> >> +\t\t\treturn ret;\n> >> +\t\t}\n> >> +\t}\n> >> +\tret = subDevicePipe_.back()->setFormat(0, &format);\n> >> +\tif (ret < 0) {\n> >> +\t\tLOG(QcomCamss, Debug) << \"Failed to set format for subdev => \"\n> >> +\t\t\t\t << ret;\n> >> +\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tV4L2DeviceFormat outputFormat = {};\n> >> +\toutputFormat.fourcc = cfg.pixelFormat;\n> >> +\toutputFormat.size = cfg.size;\n> >> +\t/* our supported formats are single plane only */\n> >> +\toutputFormat.planesCount = 1;\n> >> +\n> >> +\tret = video_->setFormat(&outputFormat);\n> >> +\tLOG(QcomCamss, Debug) << \" video_ setFormat => \" << ret;\n> >> +\tif (ret) {\n> >> +\t\tLOG(QcomCamss, Debug) << \"Failed to set format for video_ => \"\n> >> +\t\t\t\t << ret;\n> >> +\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tif (outputFormat.size != cfg.size ||\n> >> +\t outputFormat.fourcc != cfg.pixelFormat) {\n> >> +\t\tLOG(QcomCamss, Error) << \"Unable to configure capture for \"\n> >> +\t\t\t\t << cfg.toString();\n> >> +\t\treturn -EINVAL;\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +void QcomCamssCameraData::bufferReady(Buffer *buffer)\n> >> +{\n> >> +\tASSERT(activeCamera_);\n> >> +\n> >> +\tRequest *request = queuedRequests_.front();\n> >> +\n> >> +\tpipe_->completeBuffer(activeCamera_, request, buffer);\n> >> +\tpipe_->completeRequest(activeCamera_, request);\n> > \n> > The base CameraData class provides you with a pointer to the camera_,\n> > there's no need for an activeCamera_ field.\n> >\n> ok\n> \n> >> +}\n> >> +\n> >> +QcomCamssCameraConfiguration::QcomCamssCameraConfiguration(Camera *camera,\n> >> +\t\tQcomCamssCameraData *data)\n> >> +\t: CameraConfiguration()\n> >> +{\n> >> +\tcamera_ = camera->shared_from_this();\n> >> +\tdata_ = data;\n> >> +}\n> >> +\n> >> +CameraConfiguration::Status QcomCamssCameraConfiguration::validate()\n> >> +{\n> >> +\tconst CameraSensor *sensor = data_->sensor_;\n> >> +\tStatus status = Valid;\n> >> +\n> >> +\tif (config_.empty())\n> >> +\t\treturn Invalid;\n> >> +\n> >> +\t/* Cap the number of entries to the available streams. */\n> >> +\tif (config_.size() > 1) {\n> >> +\t\tconfig_.resize(1);\n> >> +\t\tstatus = Adjusted;\n> >> +\t}\n> >> +\n> >> +\tStreamConfiguration &cfg = config_[0];\n> >> +\n> >> +\t/* Adjust the pixel format if needed */\n> >> +\tif (!isPixelFormatSupportedBySensor(sensor, cfg.pixelFormat)) {\n> >> +\t\tLOG(QcomCamss, Debug) << \"Adjusting format to default one\";\n> >> +\t\tcfg.pixelFormat = getDefaultPixelFormat(sensor);\n> >> +\t\tstatus = Adjusted;\n> >> +\t}\n> >> +\n> >> +\t/* Select the sensor format. */\n> >> +\tsensorFormat_ = sensor->getFormat({ mediaCodeFromPixelFormat(cfg.pixelFormat) },\n> >> +\t\t\t\t\t cfg.size);\n> >> +\t/* test sensorFormat_.mbus_code is valid */\n> >> +\tif (!sensorFormat_.mbus_code)\n> >> +\t\treturn Invalid;\n> >> +\n> >> +\t/* applied to cfg.size */\n> >> +\tconst Size size = cfg.size;\n> >> +\tcfg.size = sensorFormat_.size;\n> >> +\t/* clip for hw constraints support */\n> >> +\tcfg.size.width = std::max(1U, std::min(8191U, cfg.size.width));\n> >> +\tcfg.size.height = std::max(1U, std::min(8191U, cfg.size.height));\n> >> +\tif (cfg.size != size) {\n> >> +\t\tLOG(QcomCamss, Debug)\n> >> +\t\t\t<< \"Adjusting size from \" << size.toString()\n> >> +\t\t\t<< \" to \" << cfg.size.toString();\n> >> +\t\tstatus = Adjusted;\n> >> +\t}\n> >> +\n> >> +\tcfg.bufferCount = QCOMCAMSS_BUFFER_COUNT;\n> >> +\n> >> +\treturn status;\n> >> +}\n> >> +\n> >> +/* -----------------------------------------------------------------------------\n> >> + * Pipeline Operations\n> >> + */\n> >> +\n> >> +CameraConfiguration *PipelineHandlerQcomCamss::generateConfiguration(\n> >> +\tCamera *camera,\n> >> +\tconst StreamRoles &roles)\n> >> +{\n> >> +\tQcomCamssCameraData *data = cameraData(camera);\n> >> +\tCameraConfiguration *config =\n> >> +\t\tnew QcomCamssCameraConfiguration(camera, data);\n> >> +\n> >> +\tif (roles.empty())\n> >> +\t\treturn config;\n> >> +\n> >> +\tStreamConfiguration cfg{};\n> >> +\tcfg.pixelFormat = getDefaultPixelFormat(data->sensor_);\n> >> +\tcfg.size = data->sensor_->sizes()[0];\n> >> +\n> >> +\tconfig->addConfiguration(cfg);\n> >> +\n> >> +\tconfig->validate();\n> >> +\n> >> +\treturn config;\n> >> +}\n> >> +\n> >> +int PipelineHandlerQcomCamss::configure(Camera *camera, CameraConfiguration *c)\n> >> +{\n> >> +\tQcomCamssCameraConfiguration *config =\n> >> +\t\tstatic_cast(c);\n> >> +\tQcomCamssCameraData *data = cameraData(camera);\n> >> +\tStreamConfiguration &cfg = config->at(0);\n> >> +\tCameraSensor *sensor = data->sensor_;\n> >> +\tQcomCamssVideoPipe *videoPipe = data->videoPipe_;\n> >> +\tint ret;\n> >> +\n> >> +\t/*\n> >> +\t * Configure the format on the sensor output and propagate it through\n> >> +\t * the pipeline.\n> >> +\t */\n> >> +\tV4L2SubdeviceFormat format = config->sensorFormat();\n> >> +\tLOG(QcomCamss, Debug) << \"Configuring sensor with \"\n> >> +\t\t\t << format.toString();\n> >> +\n> >> +\tret = sensor->setFormat(&format);\n> >> +\tif (ret < 0) {\n> >> +\t\tLOG(QcomCamss, Error) << \"Failed to applied format for sensor => \"\n> >> +\t\t\t\t << ret;\n> >> +\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tret = videoPipe->configure(format, cfg);\n> >> +\tif (ret) {\n> >> +\t\tLOG(QcomCamss, Debug) << \"Failed to configure format for video pipe => \"\n> >> +\t\t\t\t << ret;\n> >> +\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tcfg.setStream(&data->stream_);\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +int PipelineHandlerQcomCamss::allocateBuffers(Camera *camera,\n> >> +\t\t\t\t\t const std::set &streams)\n> >> +{\n> >> +\tQcomCamssCameraData *data = cameraData(camera);\n> >> +\n> >> +\tStream *stream = *streams.begin();\n> >> +\n> >> +\treturn data->videoPipe_->video_->exportBuffers(&stream->bufferPool());\n> >> +}\n> >> +\n> >> +int PipelineHandlerQcomCamss::freeBuffers(Camera *camera,\n> >> +\t\t\t\t\t const std::set &streams)\n> >> +{\n> >> +\tQcomCamssCameraData *data = cameraData(camera);\n> >> +\n> >> +\tif (data->videoPipe_->video_->releaseBuffers())\n> >> +\t\tLOG(QcomCamss, Error) << \"Failed to release buffers\";\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +int PipelineHandlerQcomCamss::PipelineHandlerQcomCamss::start(Camera *camera)\n> >> +{\n> >> +\tQcomCamssCameraData *data = cameraData(camera);\n> >> +\tint ret;\n> >> +\n> >> +\tret = data->videoPipe_->video_->streamOn();\n> >> +\tif (ret)\n> >> +\t\tLOG(QcomCamss, Error)\n> >> +\t\t\t<< \"Failed to start camera \" << camera->name();\n> >> +\n> >> +\tdata->activeCamera_ = camera;\n> >> +\n> >> +\treturn ret;\n> >> +}\n> >> +\n> >> +void PipelineHandlerQcomCamss::stop(Camera *camera)\n> >> +{\n> >> +\tQcomCamssCameraData *data = cameraData(camera);\n> >> +\tint ret;\n> >> +\n> >> +\tret = data->videoPipe_->video_->streamOff();\n> >> +\tif (ret)\n> >> +\t\tLOG(QcomCamss, Warning)\n> >> +\t\t\t<< \"Failed to stop camera \" << camera->name();\n> >> +\n> >> +\tPipelineHandler::stop(camera);\n> >> +\n> >> +\tdata->activeCamera_ = nullptr;\n> >> +}\n> >> +\n> >> +int PipelineHandlerQcomCamss::queueRequest(Camera *camera, Request *request)\n> >> +{\n> >> +\tQcomCamssCameraData *data = cameraData(camera);\n> >> +\tStream *stream = &data->stream_;\n> >> +\n> >> +\tBuffer *buffer = request->findBuffer(stream);\n> >> +\tif (!buffer) {\n> >> +\t\tLOG(QcomCamss, Error)\n> >> +\t\t\t<< \"Attempt to queue request with invalid stream\";\n> >> +\t\treturn -ENOENT;\n> >> +\t}\n> >> +\n> >> +\tint ret = data->videoPipe_->video_->queueBuffer(buffer);\n> >> +\tif (ret < 0)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tPipelineHandler::queueRequest(camera, request);\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +/* -----------------------------------------------------------------------------\n> >> + * Match and Setup\n> >> + */\n> > \n> > Missing blank line.\n> > \n> ok\n> \n> >> +int PipelineHandlerQcomCamss::createCamera(QcomCamssVideoPipe *videoPipe,\n> >> +\t\tMediaEntity *sensor)\n> >> +{\n> >> +\tint ret;\n> >> +\tstd::unique_ptr data =\n> >> +\t\tutils::make_unique(this, videoPipe);\n> >> +\n> >> +\tvideoPipe->video_->bufferReady.connect(data.get(),\n> >> +\t\t\t&QcomCamssCameraData::bufferReady);\n> >> +\tdata->sensor_ = new CameraSensor(sensor);\n> >> +\tret = data->sensor_->init();\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tstd::set streams{ &data->stream_ };\n> >> +\tstd::shared_ptr camera =\n> >> +\t\tCamera::create(this, sensor->name(), streams);\n> >> +\tregisterCamera(std::move(camera), std::move(data));\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +bool PipelineHandlerQcomCamss::match(DeviceEnumerator *enumerator)\n> >> +{\n> >> +\tconst std::array, 2> subDevNames = {\n> >> +\t\t{\n> >> +\t\t\t{\"msm_csiphy0\", \"msm_csid0\", \"msm_ispif0\", \"msm_vfe0_rdi0\"},\n> >> +\t\t\t{\"msm_csiphy1\", \"msm_csid1\", \"msm_ispif1\", \"msm_vfe0_rdi1\"},\n> >> +\t\t}\n> >> +\t};\n> >> +\tconst std::array videoNames = {\"msm_vfe0_video0\", \"msm_vfe0_video1\"};\n> > \n> > These two variables should be static const.\n> > \n> ok\n> \n> >> +\tconst MediaPad *pad;\n> >> +\tDeviceMatch dm(\"qcom-camss\");\n> >> +\n> >> +\t/*\n> >> +\t * Code will work with either 8x16 or 8x96 but only expose capabilities\n> >> +\t * of 8x16.\n> >> +\t */\n> >> +\tmedia_ = acquireMediaDevice(enumerator, dm);\n> >> +\tif (!media_)\n> >> +\t\treturn false;\n> >> +\n> >> +\tfor (unsigned int i = 0; i < QCOMCAMSS_PIPE_NB; i++) {\n> >> +\t\tif (!pipe_[i].create_and_link(media_, subDevNames[i],\n> >> +\t\t\t\t\t videoNames[i])) {\n> >> +\t\t\tLOG(QcomCamss, Error) << \"create_and_link failed\";\n> >> +\t\t\treturn false;\n> >> +\t\t}\n> >> +\n> >> +\t\tpad = pipe_[i].subDevicePipe_[0]->entity()->getPadByIndex(0);\n> >> +\t\tif (pad)\n> >> +\t\t\tcreateCamera(&pipe_[i],\n> >> +\t\t\t\t pad->links()[0]->source()->entity());\n> >> +\t}\n> >> +\n> >> +\treturn true;\n> >> +}\n> >> +\n> >> +REGISTER_PIPELINE_HANDLER(PipelineHandlerQcomCamss);\n> >> +\n> >> +} /* namespace libcamera */\n> >> \\ No newline at end of file\n> > \n> > Let's add one :-)\n> > \n> ok","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B93BF60BF9\n\tfor ;\n\tWed, 26 Jun 2019 10:39:34 +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 364D1510;\n\tWed, 26 Jun 2019 10:39:34 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561538374;\n\tbh=0w1IBoIrFLdou8vLGoWxpu0cLLGGjB3a5RX21HY9svg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LKyZWvCNpLoMRlHklbM/obXr25gsS6tZoyrKXWP0XcgyNCLmY+JYKKYvk27QP3WFo\n\t+oGgFqaduXhV9QAQ3io0IedrHTtw0AbYOrNJDDQAsbF8DtkSbnlVfxSz1Xqw379mGY\n\tMrsd/kzM9tI485nOioKDlPo6xuVFbHOKvBq6tzfU=","Date":"Wed, 26 Jun 2019 11:39:14 +0300","From":"Laurent Pinchart ","To":"Mickael GUENE ","Cc":"\"libcamera-devel@lists.libcamera.org\"\n\t, \n\tBenjamin GAIGNARD ","Message-ID":"<20190626083914.GB4776@pendragon.ideasonboard.com>","References":"<1560857644-40044-1-git-send-email-mickael.guene@st.com>\n\t<20190622210720.GH8156@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 26 Jun 2019 08:39:35 -0000"}},{"id":2031,"web_url":"https://patchwork.libcamera.org/comment/2031/","msgid":"","date":"2019-06-26T09:17:45","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","submitter":{"id":19,"url":"https://patchwork.libcamera.org/api/people/19/","name":"Mickael GUENE","email":"mickael.guene@st.com"},"content":"Hi Laurent,\n\n>> I have talked with Benjamin and we decided not to merge our works.\n> \n> No worries, but I'm afraid that a qcom-camss-specific pipeline handler\n> doesn't make much sense without ISP support. It came to my attention\n> yesterday that work was ongoing on a pipeline handler for the i.MX SoCs,\n> which could fall in the same category. We don't want to see a\n> multiplication of nearly identical pipeline handlers for linear\n> pipelines without an ISP, they should all be supported through the same\n> code.\n So this means that it's useless to post a v2 since you won't\nupstream it ?\n\nRegards\nMickael","headers":{"Return-Path":"","Received":["from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com\n\t[62.209.51.94])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AE60E60BF9\n\tfor ;\n\tWed, 26 Jun 2019 11:17:48 +0200 (CEST)","from pps.filterd (m0046037.ppops.net [127.0.0.1])\n\tby mx07-00178001.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id\n\tx5Q9D1pO004278; Wed, 26 Jun 2019 11:17:47 +0200","from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35])\n\tby mx07-00178001.pphosted.com with ESMTP id 2tb1f3u4h3-1\n\t(version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT);\n\tWed, 26 Jun 2019 11:17:47 +0200","from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9])\n\tby beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 89DD248;\n\tWed, 26 Jun 2019 09:17:46 +0000 (GMT)","from Webmail-eu.st.com (sfhdag3node1.st.com [10.75.127.7])\n\tby zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 6418124D6;\n\tWed, 26 Jun 2019 09:17:46 +0000 (GMT)","from SFHDAG5NODE3.st.com (10.75.127.15) by SFHDAG3NODE1.st.com\n\t(10.75.127.7) with Microsoft SMTP Server (TLS) id 15.0.1347.2;\n\tWed, 26 Jun 2019 11:17:46 +0200","from SFHDAG5NODE3.st.com ([fe80::7c09:5d6b:d2c7:5f47]) by\n\tSFHDAG5NODE3.st.com ([fe80::7c09:5d6b:d2c7:5f47%20]) with mapi id\n\t15.00.1473.003; Wed, 26 Jun 2019 11:17:46 +0200"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=st.com;\n\th=from : to : cc : subject\n\t: date : message-id : references : in-reply-to : content-type :\n\tcontent-id\n\t: content-transfer-encoding : mime-version; s=STMicroelectronics;\n\tbh=PrH9GM/n73s9KA7cIXtq6gtgsCLcl8zFIQrMNekFCdE=;\n\tb=loxEkkf6TuA3lwkb2OGriFO5SKwjBQLt2rE2DjHL5fB/WXdRZtrB5xsvlLLobkRpTios\n\t/PKr+nQjpCpKa66Yr+pU17sE1M26SlA4x9YQ6kmdfe1wydJojlFis6L/39Hv/vdEn0yn\n\tA6mcQ3ePHXgWiB8sy0/jHyeJE9fCdp4u7jcmfLoTeakzwwKpP6WaKAERZ/nmUopzKof/\n\t9OW4RZSMau9WSGjimnWciPKOlQqgogjvG34ki3tNJFb1OtDe5sGWCU9Yqhc9w1RNnPCv\n\tW3wPI9ouuUU3l+fCRZz95cU+Vjp+3nnFvGvEUMHH33yMrd/GmecZX0Aj8elQgZdt9PCG\n\tlw== ","From":"Mickael GUENE ","To":"Laurent Pinchart ","CC":"\"libcamera-devel@lists.libcamera.org\"\n\t, Benjamin GAIGNARD\n\t","Thread-Topic":"[libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","Thread-Index":"AQHVJcm9eS3Ka889s0ueRsyrGrJQuqaoD9oAgAV2/ICAAAFTAIAACsIA","Date":"Wed, 26 Jun 2019 09:17:45 +0000","Message-ID":"","References":"<1560857644-40044-1-git-send-email-mickael.guene@st.com>\n\t<20190622210720.GH8156@pendragon.ideasonboard.com>\n\t\n\t<20190626083914.GB4776@pendragon.ideasonboard.com>","In-Reply-To":"<20190626083914.GB4776@pendragon.ideasonboard.com>","Accept-Language":"fr-FR, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.1","x-ms-exchange-messagesentrepresentingtype":"1","x-ms-exchange-transport-fromentityheader":"Hosted","x-originating-ip":"[10.75.127.50]","Content-Type":"text/plain; charset=\"utf-8\"","Content-ID":"","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10434:, ,\n\tdefinitions=2019-06-26_05:, , signatures=0","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 26 Jun 2019 09:17:48 -0000"}},{"id":2032,"web_url":"https://patchwork.libcamera.org/comment/2032/","msgid":"<20190626092306.GD4776@pendragon.ideasonboard.com>","date":"2019-06-26T09:23:06","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Mickael,\n\nOn Wed, Jun 26, 2019 at 09:17:45AM +0000, Mickael GUENE wrote:\n> Hi Laurent,\n> \n> >> I have talked with Benjamin and we decided not to merge our works.\n> > \n> > No worries, but I'm afraid that a qcom-camss-specific pipeline handler\n> > doesn't make much sense without ISP support. It came to my attention\n> > yesterday that work was ongoing on a pipeline handler for the i.MX SoCs,\n> > which could fall in the same category. We don't want to see a\n> > multiplication of nearly identical pipeline handlers for linear\n> > pipelines without an ISP, they should all be supported through the same\n> > code.\n> \n> So this means that it's useless to post a v2 since you won't\n> upstream it ?\n\nGiven our current understanding of the issue, yes, we won't merge a\nqcom-camss pipeline handler as the qcom-camss should instead be\nsupported through a simple pipeline handler.","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 382B660BF9\n\tfor ;\n\tWed, 26 Jun 2019 11:23:25 +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 84C60510;\n\tWed, 26 Jun 2019 11:23:24 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561541004;\n\tbh=JvO7hKoaWe0Oo1L8hW750zMFwgELLSdc/yL2zWbaFlI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dHq76lVb80byjM2/hiokmzgTyKv/yPZ2j3c9yBiGmLiSWYOj8KDApSxftelqyFgNh\n\t50eE+/AN+2uYeOoFWMd8hAqmAjQkdtmDxf2EHmMTD33lSFDSSAMZA/SfEyASI7YQoO\n\tw2bLDU13OET/WTa1MS2vZOdelnt7fun75UEZBX3k=","Date":"Wed, 26 Jun 2019 12:23:06 +0300","From":"Laurent Pinchart ","To":"Mickael GUENE ","Cc":"\"libcamera-devel@lists.libcamera.org\"\n\t, \n\tBenjamin GAIGNARD ","Message-ID":"<20190626092306.GD4776@pendragon.ideasonboard.com>","References":"<1560857644-40044-1-git-send-email-mickael.guene@st.com>\n\t<20190622210720.GH8156@pendragon.ideasonboard.com>\n\t\n\t<20190626083914.GB4776@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 26 Jun 2019 09:23:25 -0000"}},{"id":2033,"web_url":"https://patchwork.libcamera.org/comment/2033/","msgid":"<744d4cff-ab22-ee3f-165b-f964be99581b@st.com>","date":"2019-06-26T09:26:20","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","submitter":{"id":19,"url":"https://patchwork.libcamera.org/api/people/19/","name":"Mickael GUENE","email":"mickael.guene@st.com"},"content":"Laurent,\n\nOn 6/26/19 11:23, Laurent Pinchart wrote:\n> Hi Mickael,\n> \n> On Wed, Jun 26, 2019 at 09:17:45AM +0000, Mickael GUENE wrote:\n>> Hi Laurent,\n>>\n>>>> I have talked with Benjamin and we decided not to merge our works.\n>>>\n>>> No worries, but I'm afraid that a qcom-camss-specific pipeline handler\n>>> doesn't make much sense without ISP support. It came to my attention\n>>> yesterday that work was ongoing on a pipeline handler for the i.MX SoCs,\n>>> which could fall in the same category. We don't want to see a\n>>> multiplication of nearly identical pipeline handlers for linear\n>>> pipelines without an ISP, they should all be supported through the same\n>>> code.\n>>\n>> So this means that it's useless to post a v2 since you won't\n>> upstream it ?\n> \n> Given our current understanding of the issue, yes, we won't merge a\n> qcom-camss pipeline handler as the qcom-camss should instead be\n> supported through a simple pipeline handler.\n> \n\n Thanks for the clarification. So I will continue to maintain my own\nset of patches.\n\nRegards\nMickael","headers":{"Return-Path":"","Received":["from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com\n\t[62.209.51.94])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BB9A960BF9\n\tfor ;\n\tWed, 26 Jun 2019 11:26:24 +0200 (CEST)","from pps.filterd (m0046037.ppops.net [127.0.0.1])\n\tby mx07-00178001.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id\n\tx5Q9QOk4014133; Wed, 26 Jun 2019 11:26:24 +0200","from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35])\n\tby mx07-00178001.pphosted.com with ESMTP id 2tb1f3u6bm-1\n\t(version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT);\n\tWed, 26 Jun 2019 11:26:24 +0200","from zeta.dmz-eu.st.com (zeta.dmz-eu.st.com [164.129.230.9])\n\tby beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 97D8643;\n\tWed, 26 Jun 2019 09:26:20 +0000 (GMT)","from Webmail-eu.st.com (sfhdag3node1.st.com [10.75.127.7])\n\tby zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 6E90D24FB;\n\tWed, 26 Jun 2019 09:26:20 +0000 (GMT)","from SFHDAG5NODE3.st.com (10.75.127.15) by SFHDAG3NODE1.st.com\n\t(10.75.127.7) with Microsoft SMTP Server (TLS) id 15.0.1347.2;\n\tWed, 26 Jun 2019 11:26:20 +0200","from SFHDAG5NODE3.st.com ([fe80::7c09:5d6b:d2c7:5f47]) by\n\tSFHDAG5NODE3.st.com ([fe80::7c09:5d6b:d2c7:5f47%20]) with mapi id\n\t15.00.1473.003; Wed, 26 Jun 2019 11:26:20 +0200"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=st.com;\n\th=from : to : cc : subject\n\t: date : message-id : references : in-reply-to : content-type :\n\tcontent-id\n\t: content-transfer-encoding : mime-version; s=STMicroelectronics;\n\tbh=hYv15hv0pYHvSn6s8dPqYJYue6H0x1XjC1cbHdeWKFM=;\n\tb=ja1KezH65HkuRQrKuoEVEv3boiunTuhPf0ax4K9jJ3Jj9TN7qLE21GRy0NMvs1TiCQ8E\n\tUqpCmD3BdKQ0aNkRSn2EZIIZSpkJRabDCErxYhMdpdAKI1VdetVGg2GQGgWPNHmoH4Ri\n\tjQ1vHpo7vfqcOxFBNQjXXf9dOq93UJT1fCIMJ9DHENwsHNwb6dRWglxJTjIbsCBxgTX/\n\tVdaoETWYadHTjxxc+QoCN9o6OAHhXpQo/NUMkoZYDbJzHBEd3xUYw9wGl5sHefJP0SoV\n\tTOjEnq2oRPuOTm3eICUD42gyZZlNiB0XrTbd5MSwGNL/mdWSnqydjImsLXk6vRp0SNCg\n\tOg== ","From":"Mickael GUENE ","To":"Laurent Pinchart ","CC":"\"libcamera-devel@lists.libcamera.org\"\n\t, Benjamin GAIGNARD\n\t","Thread-Topic":"[libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","Thread-Index":"AQHVJcm9eS3Ka889s0ueRsyrGrJQuqaoD9oAgAV2/ICAAAFTAIAACsIAgAABgACAAADmgA==","Date":"Wed, 26 Jun 2019 09:26:20 +0000","Message-ID":"<744d4cff-ab22-ee3f-165b-f964be99581b@st.com>","References":"<1560857644-40044-1-git-send-email-mickael.guene@st.com>\n\t<20190622210720.GH8156@pendragon.ideasonboard.com>\n\t\n\t<20190626083914.GB4776@pendragon.ideasonboard.com>\n\t\n\t<20190626092306.GD4776@pendragon.ideasonboard.com>","In-Reply-To":"<20190626092306.GD4776@pendragon.ideasonboard.com>","Accept-Language":"fr-FR, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","user-agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.1","x-ms-exchange-messagesentrepresentingtype":"1","x-ms-exchange-transport-fromentityheader":"Hosted","x-originating-ip":"[10.75.127.50]","Content-Type":"text/plain; charset=\"utf-8\"","Content-ID":"<6390D9724B6D1E4098C844F3AF926464@st.com>","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10434:, ,\n\tdefinitions=2019-06-26_05:, , signatures=0","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: Add Qualcomm\n\t8x16 pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 26 Jun 2019 09:26:24 -0000"}}]