[{"id":18534,"web_url":"https://patchwork.libcamera.org/comment/18534/","msgid":"<20210804065618.GE2167@pyrite.rasen.tech>","date":"2021-08-04T06:56:18","subject":"Re: [libcamera-devel] [PATCH v2 6/8] cam: Add KMS sink class","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Fri, Jul 30, 2021 at 04:03:04AM +0300, Laurent Pinchart wrote:\n> Add a KMSSink class to display framebuffers through the DRM/KMS API.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n> Changes since v1:\n> \n> - Use formats:: namespace\n> ---\n>  src/cam/kms_sink.cpp | 298 +++++++++++++++++++++++++++++++++++++++++++\n>  src/cam/kms_sink.h   |  76 +++++++++++\n>  src/cam/meson.build  |   1 +\n>  3 files changed, 375 insertions(+)\n>  create mode 100644 src/cam/kms_sink.cpp\n>  create mode 100644 src/cam/kms_sink.h\n> \n> diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp\n> new file mode 100644\n> index 000000000000..b8f86dcb6f4e\n> --- /dev/null\n> +++ b/src/cam/kms_sink.cpp\n> @@ -0,0 +1,298 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Ideas on Board Oy\n\ns/2020/2021 ?\n\nSame for the header.\n\n> + *\n> + * kms_sink.cpp - KMS Sink\n> + */\n> +\n> +#include \"kms_sink.h\"\n> +\n> +#include <algorithm>\n> +#include <assert.h>\n> +#include <iostream>\n> +#include <memory>\n> +#include <string.h>\n> +\n> +#include <libcamera/camera.h>\n> +#include <libcamera/formats.h>\n> +#include <libcamera/framebuffer.h>\n> +#include <libcamera/stream.h>\n> +\n> +#include \"drm.h\"\n> +\n> +KMSSink::KMSSink(const std::string &connectorName)\n> +\t: connector_(nullptr), crtc_(nullptr), plane_(nullptr), mode_(nullptr)\n> +{\n> +\tint ret = dev_.init();\n> +\tif (ret < 0)\n> +\t\treturn;\n> +\n> +\t/*\n> +\t * Find the requested connector. If no connector is requested, pick the\n> +\t * first connected connector.\n> +\t */\n> +\tfor (const DRM::Connector &conn : dev_.connectors()) {\n> +\t\tif (conn.name() == connectorName) {\n> +\t\t\tconnector_ = &conn;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tif (conn.status() != DRM::Connector::Disconnected) {\n> +\t\t\tif (!connector_ ||\n> +\t\t\t    (connector_->status() == DRM::Connector::Unknown &&\n> +\t\t\t     conn.status() == DRM::Connector::Connected))\n> +\t\t\t\tconnector_ = &conn;\n> +\t\t}\n\nI think this is the only part I'm confused about. If no connector is\nrequested, then the first candidate connector can be chosen even if its\nstatus is Unknown.\n\n\nPaul\n\n> +\t}\n> +\n> +\tif (!connector_) {\n> +\t\tif (!connectorName.empty())\n> +\t\t\tstd::cerr\n> +\t\t\t\t<< \"Connector \" << connectorName << \" not found\"\n> +\t\t\t\t<< std::endl;\n> +\t\telse\n> +\t\t\tstd::cerr << \"No connected connector found\" << std::endl;\n> +\t\treturn;\n> +\t}\n> +\n> +\tdev_.requestComplete.connect(this, &KMSSink::requestComplete);\n> +}\n> +\n> +void KMSSink::mapBuffer(libcamera::FrameBuffer *buffer)\n> +{\n> +\tstd::unique_ptr<DRM::FrameBuffer> drmBuffer =\n> +\t\tdev_.createFrameBuffer(*buffer, format_, size_, stride_);\n> +\tif (!drmBuffer)\n> +\t\treturn;\n> +\n> +\tbuffers_.emplace(std::piecewise_construct,\n> +\t\t\t std::forward_as_tuple(buffer),\n> +\t\t\t std::forward_as_tuple(std::move(drmBuffer)));\n> +}\n> +\n> +int KMSSink::configure(const libcamera::CameraConfiguration &config)\n> +{\n> +\tcrtc_ = nullptr;\n> +\tplane_ = nullptr;\n> +\tmode_ = nullptr;\n> +\n> +\tconst libcamera::StreamConfiguration &cfg = config.at(0);\n> +\tint ret = configurePipeline(cfg.pixelFormat);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tconst std::vector<DRM::Mode> &modes = connector_->modes();\n> +\tconst auto iter = std::find_if(modes.begin(), modes.end(),\n> +\t\t\t\t       [&](const DRM::Mode &mode) {\n> +\t\t\t\t\t       return mode.hdisplay == cfg.size.width &&\n> +\t\t\t\t\t\t      mode.vdisplay == cfg.size.height;\n> +\t\t\t\t       });\n> +\tif (iter == modes.end()) {\n> +\t\tstd::cerr\n> +\t\t\t<< \"No mode matching \" << cfg.size.toString()\n> +\t\t\t<< std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tmode_ = &*iter;\n> +\tsize_ = cfg.size;\n> +\tstride_ = cfg.stride;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> +{\n> +\t/*\n> +\t * If the requested format has an alpha channel, also consider the X\n> +\t * variant.\n> +\t */\n> +\tlibcamera::PixelFormat xFormat;\n> +\n> +\tswitch (format) {\n> +\tcase libcamera::formats::ABGR8888:\n> +\t\txFormat = libcamera::formats::XBGR8888;\n> +\t\tbreak;\n> +\tcase libcamera::formats::ARGB8888:\n> +\t\txFormat = libcamera::formats::XRGB8888;\n> +\t\tbreak;\n> +\tcase libcamera::formats::BGRA8888:\n> +\t\txFormat = libcamera::formats::BGRX8888;\n> +\t\tbreak;\n> +\tcase libcamera::formats::RGBA8888:\n> +\t\txFormat = libcamera::formats::RGBX8888;\n> +\t\tbreak;\n> +\t}\n> +\n> +\t/*\n> +\t * Find a CRTC and plane suitable for the request format and the\n> +\t * connector at the end of the pipeline. Restrict the search to primary\n> +\t * planes for now.\n> +\t */\n> +\tfor (const DRM::Encoder *encoder : connector_->encoders()) {\n> +\t\tfor (const DRM::Crtc *crtc : encoder->possibleCrtcs()) {\n> +\t\t\tfor (const DRM::Plane *plane : crtc->planes()) {\n> +\t\t\t\tif (plane->type() != DRM::Plane::TypePrimary)\n> +\t\t\t\t\tcontinue;\n> +\n> +\t\t\t\tif (plane->supportsFormat(format)) {\n> +\t\t\t\t\tcrtc_ = crtc;\n> +\t\t\t\t\tplane_ = plane;\n> +\t\t\t\t\tformat_ = format;\n> +\t\t\t\t\treturn 0;\n> +\t\t\t\t}\n> +\n> +\t\t\t\tif (plane->supportsFormat(xFormat)) {\n> +\t\t\t\t\tcrtc_ = crtc;\n> +\t\t\t\t\tplane_ = plane;\n> +\t\t\t\t\tformat_ = xFormat;\n> +\t\t\t\t\treturn 0;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\tstd::cerr\n> +\t\t<< \"Unable to find display pipeline for format \"\n> +\t\t<< format.toString() << std::endl;\n> +\treturn -EPIPE;\n> +}\n> +\n> +int KMSSink::start()\n> +{\n> +\tstd::unique_ptr<DRM::AtomicRequest> request;\n> +\n> +\tint ret = FrameSink::start();\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\t/* Disable all CRTCs and planes to start from a known valid state. */\n> +\trequest = std::make_unique<DRM::AtomicRequest>(&dev_);\n> +\n> +\tfor (const DRM::Crtc &crtc : dev_.crtcs())\n> +\t\trequest->addProperty(&crtc, \"ACTIVE\", 0);\n> +\n> +\tfor (const DRM::Plane &plane : dev_.planes()) {\n> +\t\trequest->addProperty(&plane, \"CRTC_ID\", 0);\n> +\t\trequest->addProperty(&plane, \"FB_ID\", 0);\n> +\t}\n> +\n> +\tret = request->commit(DRM::AtomicRequest::FlagAllowModeset);\n> +\tif (ret < 0) {\n> +\t\tstd::cerr\n> +\t\t\t<< \"Failed to disable CRTCs and planes: \"\n> +\t\t\t<< strerror(-ret) << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\t/* Enable the display pipeline with no plane to start with. */\n> +\trequest = std::make_unique<DRM::AtomicRequest>(&dev_);\n> +\n> +\trequest->addProperty(connector_, \"CRTC_ID\", crtc_->id());\n> +\trequest->addProperty(crtc_, \"ACTIVE\", 1);\n> +\trequest->addProperty(crtc_, \"MODE_ID\", mode_->toBlob(&dev_));\n> +\n> +\tret = request->commit(DRM::AtomicRequest::FlagAllowModeset);\n> +\tif (ret < 0) {\n> +\t\tstd::cerr\n> +\t\t\t<< \"Failed to enable display pipeline: \"\n> +\t\t\t<< strerror(-ret) << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tplaneInitialized_ = false;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int KMSSink::stop()\n> +{\n> +\t/* Display pipeline. */\n> +\tDRM::AtomicRequest request(&dev_);\n> +\n> +\trequest.addProperty(connector_, \"CRTC_ID\", 0);\n> +\trequest.addProperty(crtc_, \"ACTIVE\", 0);\n> +\trequest.addProperty(crtc_, \"MODE_ID\", 0);\n> +\trequest.addProperty(plane_, \"CRTC_ID\", 0);\n> +\trequest.addProperty(plane_, \"FB_ID\", 0);\n> +\n> +\tint ret = request.commit(DRM::AtomicRequest::FlagAllowModeset);\n> +\tif (ret < 0) {\n> +\t\tstd::cerr\n> +\t\t\t<< \"Failed to stop display pipeline: \"\n> +\t\t\t<< strerror(-ret) << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\t/* Free all buffers. */\n> +\tpending_.reset();\n> +\tqueued_.reset();\n> +\tactive_.reset();\n> +\tbuffers_.clear();\n> +\n> +\treturn FrameSink::stop();\n> +}\n> +\n> +bool KMSSink::consumeRequest(libcamera::Request *camRequest)\n> +{\n> +\tif (pending_)\n> +\t\treturn true;\n> +\n> +\tlibcamera::FrameBuffer *buffer = camRequest->buffers().begin()->second;\n> +\tauto iter = buffers_.find(buffer);\n> +\tif (iter == buffers_.end())\n> +\t\treturn true;\n> +\n> +\tDRM::FrameBuffer *drmBuffer = iter->second.get();\n> +\n> +\tDRM::AtomicRequest *drmRequest = new DRM::AtomicRequest(&dev_);\n> +\tdrmRequest->addProperty(plane_, \"FB_ID\", drmBuffer->id());\n> +\n> +\tif (!planeInitialized_) {\n> +\t\tdrmRequest->addProperty(plane_, \"CRTC_ID\", crtc_->id());\n> +\t\tdrmRequest->addProperty(plane_, \"SRC_X\", 0 << 16);\n> +\t\tdrmRequest->addProperty(plane_, \"SRC_Y\", 0 << 16);\n> +\t\tdrmRequest->addProperty(plane_, \"SRC_W\", mode_->hdisplay << 16);\n> +\t\tdrmRequest->addProperty(plane_, \"SRC_H\", mode_->vdisplay << 16);\n> +\t\tdrmRequest->addProperty(plane_, \"CRTC_X\", 0);\n> +\t\tdrmRequest->addProperty(plane_, \"CRTC_Y\", 0);\n> +\t\tdrmRequest->addProperty(plane_, \"CRTC_W\", mode_->hdisplay);\n> +\t\tdrmRequest->addProperty(plane_, \"CRTC_H\", mode_->vdisplay);\n> +\t\tplaneInitialized_ = true;\n> +\t}\n> +\n> +\tpending_ = std::make_unique<Request>(drmRequest, camRequest);\n> +\n> +\tstd::lock_guard<std::mutex> lock(lock_);\n> +\n> +\tif (!queued_) {\n> +\t\tint ret = drmRequest->commit(DRM::AtomicRequest::FlagAsync);\n> +\t\tif (ret < 0)\n> +\t\t\tstd::cerr\n> +\t\t\t\t<< \"Failed to commit atomic request: \"\n> +\t\t\t\t<< strerror(-ret) << std::endl;\n> +\t\tqueued_ = std::move(pending_);\n> +\t}\n> +\n> +\treturn false;\n> +}\n> +\n> +void KMSSink::requestComplete(DRM::AtomicRequest *request)\n> +{\n> +\tstd::lock_guard<std::mutex> lock(lock_);\n> +\n> +\tassert(queued_ && queued_->drmRequest_.get() == request);\n> +\n> +\t/* Complete the active request, if any. */\n> +\tif (active_)\n> +\t\trequestReleased.emit(active_->camRequest_);\n> +\n> +\t/* The queued request becomes active. */\n> +\tactive_ = std::move(queued_);\n> +\n> +\t/* Queue the pending request, if any. */\n> +\tif (pending_) {\n> +\t\tpending_->drmRequest_->commit(DRM::AtomicRequest::FlagAsync);\n> +\t\tqueued_ = std::move(pending_);\n> +\t}\n> +}\n> diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h\n> new file mode 100644\n> index 000000000000..7b6ffcede28c\n> --- /dev/null\n> +++ b/src/cam/kms_sink.h\n> @@ -0,0 +1,76 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Ideas on Board Oy\n> + *\n> + * kms_sink.h - KMS Sink\n> + */\n> +#ifndef __CAM_KMS_SINK_H__\n> +#define __CAM_KMS_SINK_H__\n> +\n> +#include <list>\n> +#include <memory>\n> +#include <mutex>\n> +#include <string>\n> +#include <utility>\n> +\n> +#include <libcamera/base/signal.h>\n> +\n> +#include <libcamera/geometry.h>\n> +#include <libcamera/pixel_format.h>\n> +\n> +#include \"drm.h\"\n> +#include \"frame_sink.h\"\n> +\n> +class KMSSink : public FrameSink\n> +{\n> +public:\n> +\tKMSSink(const std::string &connectorName);\n> +\n> +\tbool isValid() const { return connector_ != nullptr; }\n> +\n> +\tvoid mapBuffer(libcamera::FrameBuffer *buffer) override;\n> +\n> +\tint configure(const libcamera::CameraConfiguration &config) override;\n> +\tint start() override;\n> +\tint stop() override;\n> +\n> +\tbool consumeRequest(libcamera::Request *request) override;\n> +\n> +private:\n> +\tclass Request\n> +\t{\n> +\tpublic:\n> +\t\tRequest(DRM::AtomicRequest *drmRequest, libcamera::Request *camRequest)\n> +\t\t\t: drmRequest_(drmRequest), camRequest_(camRequest)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tstd::unique_ptr<DRM::AtomicRequest> drmRequest_;\n> +\t\tlibcamera::Request *camRequest_;\n> +\t};\n> +\n> +\tint configurePipeline(const libcamera::PixelFormat &format);\n> +\tvoid requestComplete(DRM::AtomicRequest *request);\n> +\n> +\tDRM::Device dev_;\n> +\n> +\tconst DRM::Connector *connector_;\n> +\tconst DRM::Crtc *crtc_;\n> +\tconst DRM::Plane *plane_;\n> +\tconst DRM::Mode *mode_;\n> +\n> +\tlibcamera::PixelFormat format_;\n> +\tlibcamera::Size size_;\n> +\tunsigned int stride_;\n> +\n> +\tbool planeInitialized_;\n> +\n> +\tstd::map<libcamera::FrameBuffer *, std::unique_ptr<DRM::FrameBuffer>> buffers_;\n> +\n> +\tstd::mutex lock_;\n> +\tstd::unique_ptr<Request> pending_;\n> +\tstd::unique_ptr<Request> queued_;\n> +\tstd::unique_ptr<Request> active_;\n> +};\n> +\n> +#endif /* __CAM_KMS_SINK_H__ */\n> diff --git a/src/cam/meson.build b/src/cam/meson.build\n> index b47add55b0cb..ea36aaa5c514 100644\n> --- a/src/cam/meson.build\n> +++ b/src/cam/meson.build\n> @@ -27,6 +27,7 @@ if libdrm.found()\n>  cam_cpp_args += [ '-DHAVE_KMS' ]\n>  cam_sources += files([\n>      'drm.cpp',\n> +    'kms_sink.cpp'\n>  ])\n>  endif\n>  \n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7ED68C3232\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Aug 2021 06:56:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C6DC76880F;\n\tWed,  4 Aug 2021 08:56:28 +0200 (CEST)","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 0989760268\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Aug 2021 08:56:27 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 82FFC24F;\n\tWed,  4 Aug 2021 08:56:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cPDFI1kO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628060186;\n\tbh=vCzM3VNr6SCc3f853/hZSMqFJiMITDa4EuZg+fiUuTA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cPDFI1kO0A8uGgMCWbPcSoHZJZvflFnCwWKj9QQurHJWqju0zEdMC9vn4+6EpkfsR\n\t1CqzaqypA6ZkSCT+LeuJC6VmpqiZ6g05z1rnIwM+zR1t06Ko0HzeZOTB8dVfcEhMG6\n\ttfI1Pg/wPdi51Zada2s6IrvXZtoS+IKuszsHqBKA=","Date":"Wed, 4 Aug 2021 15:56:18 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210804065618.GE2167@pyrite.rasen.tech>","References":"<20210730010306.19956-1-laurent.pinchart@ideasonboard.com>\n\t<20210730010306.19956-7-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210730010306.19956-7-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 6/8] cam: Add KMS sink class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18542,"web_url":"https://patchwork.libcamera.org/comment/18542/","msgid":"<YQpVbGONd5/F+Uu7@pendragon.ideasonboard.com>","date":"2021-08-04T08:53:00","subject":"Re: [libcamera-devel] [PATCH v2 6/8] cam: Add KMS sink class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Wed, Aug 04, 2021 at 03:56:18PM +0900, paul.elder@ideasonboard.com wrote:\n> On Fri, Jul 30, 2021 at 04:03:04AM +0300, Laurent Pinchart wrote:\n> > Add a KMSSink class to display framebuffers through the DRM/KMS API.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> > Changes since v1:\n> > \n> > - Use formats:: namespace\n> > ---\n> >  src/cam/kms_sink.cpp | 298 +++++++++++++++++++++++++++++++++++++++++++\n> >  src/cam/kms_sink.h   |  76 +++++++++++\n> >  src/cam/meson.build  |   1 +\n> >  3 files changed, 375 insertions(+)\n> >  create mode 100644 src/cam/kms_sink.cpp\n> >  create mode 100644 src/cam/kms_sink.h\n> > \n> > diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp\n> > new file mode 100644\n> > index 000000000000..b8f86dcb6f4e\n> > --- /dev/null\n> > +++ b/src/cam/kms_sink.cpp\n> > @@ -0,0 +1,298 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Ideas on Board Oy\n> \n> s/2020/2021 ?\n> \n> Same for the header.\n> \n> > + *\n> > + * kms_sink.cpp - KMS Sink\n> > + */\n> > +\n> > +#include \"kms_sink.h\"\n> > +\n> > +#include <algorithm>\n> > +#include <assert.h>\n> > +#include <iostream>\n> > +#include <memory>\n> > +#include <string.h>\n> > +\n> > +#include <libcamera/camera.h>\n> > +#include <libcamera/formats.h>\n> > +#include <libcamera/framebuffer.h>\n> > +#include <libcamera/stream.h>\n> > +\n> > +#include \"drm.h\"\n> > +\n> > +KMSSink::KMSSink(const std::string &connectorName)\n> > +\t: connector_(nullptr), crtc_(nullptr), plane_(nullptr), mode_(nullptr)\n> > +{\n> > +\tint ret = dev_.init();\n> > +\tif (ret < 0)\n> > +\t\treturn;\n> > +\n> > +\t/*\n> > +\t * Find the requested connector. If no connector is requested, pick the\n> > +\t * first connected connector.\n> > +\t */\n> > +\tfor (const DRM::Connector &conn : dev_.connectors()) {\n> > +\t\tif (conn.name() == connectorName) {\n> > +\t\t\tconnector_ = &conn;\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\n> > +\t\tif (conn.status() != DRM::Connector::Disconnected) {\n> > +\t\t\tif (!connector_ ||\n> > +\t\t\t    (connector_->status() == DRM::Connector::Unknown &&\n> > +\t\t\t     conn.status() == DRM::Connector::Connected))\n> > +\t\t\t\tconnector_ = &conn;\n> > +\t\t}\n> \n> I think this is the only part I'm confused about. If no connector is\n> requested, then the first candidate connector can be chosen even if its\n> status is Unknown.\n\nFirst of all, a bit of background information. DRM/KMS isn't always able\nto tell if a connector is connected, for instance VGA often lacks hot\nplug detection support. That's why a connector can have three states:\ndisconnected, unknown or connected.\n\nThen, the logic. If no suitable connector has been found yet\n(!connector_), then we pick any connector that is not marked as\ndisconnected. Otherwise, we replace the currently (tentatively) selected\nconnector if it has state unknown, and conn.status() is connected. This\nresults in picking the first connected connector, with a fallback on the\nlast connector in unknown state if no connector is known to be\nconnected.\n\nThe logic is a bit convoluted, I'll try to do better.\n\n> > +\t}\n> > +\n> > +\tif (!connector_) {\n> > +\t\tif (!connectorName.empty())\n> > +\t\t\tstd::cerr\n> > +\t\t\t\t<< \"Connector \" << connectorName << \" not found\"\n> > +\t\t\t\t<< std::endl;\n> > +\t\telse\n> > +\t\t\tstd::cerr << \"No connected connector found\" << std::endl;\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\tdev_.requestComplete.connect(this, &KMSSink::requestComplete);\n> > +}\n> > +\n> > +void KMSSink::mapBuffer(libcamera::FrameBuffer *buffer)\n> > +{\n> > +\tstd::unique_ptr<DRM::FrameBuffer> drmBuffer =\n> > +\t\tdev_.createFrameBuffer(*buffer, format_, size_, stride_);\n> > +\tif (!drmBuffer)\n> > +\t\treturn;\n> > +\n> > +\tbuffers_.emplace(std::piecewise_construct,\n> > +\t\t\t std::forward_as_tuple(buffer),\n> > +\t\t\t std::forward_as_tuple(std::move(drmBuffer)));\n> > +}\n> > +\n> > +int KMSSink::configure(const libcamera::CameraConfiguration &config)\n> > +{\n> > +\tcrtc_ = nullptr;\n> > +\tplane_ = nullptr;\n> > +\tmode_ = nullptr;\n> > +\n> > +\tconst libcamera::StreamConfiguration &cfg = config.at(0);\n> > +\tint ret = configurePipeline(cfg.pixelFormat);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\n> > +\tconst std::vector<DRM::Mode> &modes = connector_->modes();\n> > +\tconst auto iter = std::find_if(modes.begin(), modes.end(),\n> > +\t\t\t\t       [&](const DRM::Mode &mode) {\n> > +\t\t\t\t\t       return mode.hdisplay == cfg.size.width &&\n> > +\t\t\t\t\t\t      mode.vdisplay == cfg.size.height;\n> > +\t\t\t\t       });\n> > +\tif (iter == modes.end()) {\n> > +\t\tstd::cerr\n> > +\t\t\t<< \"No mode matching \" << cfg.size.toString()\n> > +\t\t\t<< std::endl;\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tmode_ = &*iter;\n> > +\tsize_ = cfg.size;\n> > +\tstride_ = cfg.stride;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> > +{\n> > +\t/*\n> > +\t * If the requested format has an alpha channel, also consider the X\n> > +\t * variant.\n> > +\t */\n> > +\tlibcamera::PixelFormat xFormat;\n> > +\n> > +\tswitch (format) {\n> > +\tcase libcamera::formats::ABGR8888:\n> > +\t\txFormat = libcamera::formats::XBGR8888;\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::ARGB8888:\n> > +\t\txFormat = libcamera::formats::XRGB8888;\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::BGRA8888:\n> > +\t\txFormat = libcamera::formats::BGRX8888;\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::RGBA8888:\n> > +\t\txFormat = libcamera::formats::RGBX8888;\n> > +\t\tbreak;\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * Find a CRTC and plane suitable for the request format and the\n> > +\t * connector at the end of the pipeline. Restrict the search to primary\n> > +\t * planes for now.\n> > +\t */\n> > +\tfor (const DRM::Encoder *encoder : connector_->encoders()) {\n> > +\t\tfor (const DRM::Crtc *crtc : encoder->possibleCrtcs()) {\n> > +\t\t\tfor (const DRM::Plane *plane : crtc->planes()) {\n> > +\t\t\t\tif (plane->type() != DRM::Plane::TypePrimary)\n> > +\t\t\t\t\tcontinue;\n> > +\n> > +\t\t\t\tif (plane->supportsFormat(format)) {\n> > +\t\t\t\t\tcrtc_ = crtc;\n> > +\t\t\t\t\tplane_ = plane;\n> > +\t\t\t\t\tformat_ = format;\n> > +\t\t\t\t\treturn 0;\n> > +\t\t\t\t}\n> > +\n> > +\t\t\t\tif (plane->supportsFormat(xFormat)) {\n> > +\t\t\t\t\tcrtc_ = crtc;\n> > +\t\t\t\t\tplane_ = plane;\n> > +\t\t\t\t\tformat_ = xFormat;\n> > +\t\t\t\t\treturn 0;\n> > +\t\t\t\t}\n> > +\t\t\t}\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tstd::cerr\n> > +\t\t<< \"Unable to find display pipeline for format \"\n> > +\t\t<< format.toString() << std::endl;\n> > +\treturn -EPIPE;\n> > +}\n> > +\n> > +int KMSSink::start()\n> > +{\n> > +\tstd::unique_ptr<DRM::AtomicRequest> request;\n> > +\n> > +\tint ret = FrameSink::start();\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\n> > +\t/* Disable all CRTCs and planes to start from a known valid state. */\n> > +\trequest = std::make_unique<DRM::AtomicRequest>(&dev_);\n> > +\n> > +\tfor (const DRM::Crtc &crtc : dev_.crtcs())\n> > +\t\trequest->addProperty(&crtc, \"ACTIVE\", 0);\n> > +\n> > +\tfor (const DRM::Plane &plane : dev_.planes()) {\n> > +\t\trequest->addProperty(&plane, \"CRTC_ID\", 0);\n> > +\t\trequest->addProperty(&plane, \"FB_ID\", 0);\n> > +\t}\n> > +\n> > +\tret = request->commit(DRM::AtomicRequest::FlagAllowModeset);\n> > +\tif (ret < 0) {\n> > +\t\tstd::cerr\n> > +\t\t\t<< \"Failed to disable CRTCs and planes: \"\n> > +\t\t\t<< strerror(-ret) << std::endl;\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\t/* Enable the display pipeline with no plane to start with. */\n> > +\trequest = std::make_unique<DRM::AtomicRequest>(&dev_);\n> > +\n> > +\trequest->addProperty(connector_, \"CRTC_ID\", crtc_->id());\n> > +\trequest->addProperty(crtc_, \"ACTIVE\", 1);\n> > +\trequest->addProperty(crtc_, \"MODE_ID\", mode_->toBlob(&dev_));\n> > +\n> > +\tret = request->commit(DRM::AtomicRequest::FlagAllowModeset);\n> > +\tif (ret < 0) {\n> > +\t\tstd::cerr\n> > +\t\t\t<< \"Failed to enable display pipeline: \"\n> > +\t\t\t<< strerror(-ret) << std::endl;\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tplaneInitialized_ = false;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int KMSSink::stop()\n> > +{\n> > +\t/* Display pipeline. */\n> > +\tDRM::AtomicRequest request(&dev_);\n> > +\n> > +\trequest.addProperty(connector_, \"CRTC_ID\", 0);\n> > +\trequest.addProperty(crtc_, \"ACTIVE\", 0);\n> > +\trequest.addProperty(crtc_, \"MODE_ID\", 0);\n> > +\trequest.addProperty(plane_, \"CRTC_ID\", 0);\n> > +\trequest.addProperty(plane_, \"FB_ID\", 0);\n> > +\n> > +\tint ret = request.commit(DRM::AtomicRequest::FlagAllowModeset);\n> > +\tif (ret < 0) {\n> > +\t\tstd::cerr\n> > +\t\t\t<< \"Failed to stop display pipeline: \"\n> > +\t\t\t<< strerror(-ret) << std::endl;\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\t/* Free all buffers. */\n> > +\tpending_.reset();\n> > +\tqueued_.reset();\n> > +\tactive_.reset();\n> > +\tbuffers_.clear();\n> > +\n> > +\treturn FrameSink::stop();\n> > +}\n> > +\n> > +bool KMSSink::consumeRequest(libcamera::Request *camRequest)\n> > +{\n> > +\tif (pending_)\n> > +\t\treturn true;\n> > +\n> > +\tlibcamera::FrameBuffer *buffer = camRequest->buffers().begin()->second;\n> > +\tauto iter = buffers_.find(buffer);\n> > +\tif (iter == buffers_.end())\n> > +\t\treturn true;\n> > +\n> > +\tDRM::FrameBuffer *drmBuffer = iter->second.get();\n> > +\n> > +\tDRM::AtomicRequest *drmRequest = new DRM::AtomicRequest(&dev_);\n> > +\tdrmRequest->addProperty(plane_, \"FB_ID\", drmBuffer->id());\n> > +\n> > +\tif (!planeInitialized_) {\n> > +\t\tdrmRequest->addProperty(plane_, \"CRTC_ID\", crtc_->id());\n> > +\t\tdrmRequest->addProperty(plane_, \"SRC_X\", 0 << 16);\n> > +\t\tdrmRequest->addProperty(plane_, \"SRC_Y\", 0 << 16);\n> > +\t\tdrmRequest->addProperty(plane_, \"SRC_W\", mode_->hdisplay << 16);\n> > +\t\tdrmRequest->addProperty(plane_, \"SRC_H\", mode_->vdisplay << 16);\n> > +\t\tdrmRequest->addProperty(plane_, \"CRTC_X\", 0);\n> > +\t\tdrmRequest->addProperty(plane_, \"CRTC_Y\", 0);\n> > +\t\tdrmRequest->addProperty(plane_, \"CRTC_W\", mode_->hdisplay);\n> > +\t\tdrmRequest->addProperty(plane_, \"CRTC_H\", mode_->vdisplay);\n> > +\t\tplaneInitialized_ = true;\n> > +\t}\n> > +\n> > +\tpending_ = std::make_unique<Request>(drmRequest, camRequest);\n> > +\n> > +\tstd::lock_guard<std::mutex> lock(lock_);\n> > +\n> > +\tif (!queued_) {\n> > +\t\tint ret = drmRequest->commit(DRM::AtomicRequest::FlagAsync);\n> > +\t\tif (ret < 0)\n> > +\t\t\tstd::cerr\n> > +\t\t\t\t<< \"Failed to commit atomic request: \"\n> > +\t\t\t\t<< strerror(-ret) << std::endl;\n> > +\t\tqueued_ = std::move(pending_);\n> > +\t}\n> > +\n> > +\treturn false;\n> > +}\n> > +\n> > +void KMSSink::requestComplete(DRM::AtomicRequest *request)\n> > +{\n> > +\tstd::lock_guard<std::mutex> lock(lock_);\n> > +\n> > +\tassert(queued_ && queued_->drmRequest_.get() == request);\n> > +\n> > +\t/* Complete the active request, if any. */\n> > +\tif (active_)\n> > +\t\trequestReleased.emit(active_->camRequest_);\n> > +\n> > +\t/* The queued request becomes active. */\n> > +\tactive_ = std::move(queued_);\n> > +\n> > +\t/* Queue the pending request, if any. */\n> > +\tif (pending_) {\n> > +\t\tpending_->drmRequest_->commit(DRM::AtomicRequest::FlagAsync);\n> > +\t\tqueued_ = std::move(pending_);\n> > +\t}\n> > +}\n> > diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h\n> > new file mode 100644\n> > index 000000000000..7b6ffcede28c\n> > --- /dev/null\n> > +++ b/src/cam/kms_sink.h\n> > @@ -0,0 +1,76 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Ideas on Board Oy\n> > + *\n> > + * kms_sink.h - KMS Sink\n> > + */\n> > +#ifndef __CAM_KMS_SINK_H__\n> > +#define __CAM_KMS_SINK_H__\n> > +\n> > +#include <list>\n> > +#include <memory>\n> > +#include <mutex>\n> > +#include <string>\n> > +#include <utility>\n> > +\n> > +#include <libcamera/base/signal.h>\n> > +\n> > +#include <libcamera/geometry.h>\n> > +#include <libcamera/pixel_format.h>\n> > +\n> > +#include \"drm.h\"\n> > +#include \"frame_sink.h\"\n> > +\n> > +class KMSSink : public FrameSink\n> > +{\n> > +public:\n> > +\tKMSSink(const std::string &connectorName);\n> > +\n> > +\tbool isValid() const { return connector_ != nullptr; }\n> > +\n> > +\tvoid mapBuffer(libcamera::FrameBuffer *buffer) override;\n> > +\n> > +\tint configure(const libcamera::CameraConfiguration &config) override;\n> > +\tint start() override;\n> > +\tint stop() override;\n> > +\n> > +\tbool consumeRequest(libcamera::Request *request) override;\n> > +\n> > +private:\n> > +\tclass Request\n> > +\t{\n> > +\tpublic:\n> > +\t\tRequest(DRM::AtomicRequest *drmRequest, libcamera::Request *camRequest)\n> > +\t\t\t: drmRequest_(drmRequest), camRequest_(camRequest)\n> > +\t\t{\n> > +\t\t}\n> > +\n> > +\t\tstd::unique_ptr<DRM::AtomicRequest> drmRequest_;\n> > +\t\tlibcamera::Request *camRequest_;\n> > +\t};\n> > +\n> > +\tint configurePipeline(const libcamera::PixelFormat &format);\n> > +\tvoid requestComplete(DRM::AtomicRequest *request);\n> > +\n> > +\tDRM::Device dev_;\n> > +\n> > +\tconst DRM::Connector *connector_;\n> > +\tconst DRM::Crtc *crtc_;\n> > +\tconst DRM::Plane *plane_;\n> > +\tconst DRM::Mode *mode_;\n> > +\n> > +\tlibcamera::PixelFormat format_;\n> > +\tlibcamera::Size size_;\n> > +\tunsigned int stride_;\n> > +\n> > +\tbool planeInitialized_;\n> > +\n> > +\tstd::map<libcamera::FrameBuffer *, std::unique_ptr<DRM::FrameBuffer>> buffers_;\n> > +\n> > +\tstd::mutex lock_;\n> > +\tstd::unique_ptr<Request> pending_;\n> > +\tstd::unique_ptr<Request> queued_;\n> > +\tstd::unique_ptr<Request> active_;\n> > +};\n> > +\n> > +#endif /* __CAM_KMS_SINK_H__ */\n> > diff --git a/src/cam/meson.build b/src/cam/meson.build\n> > index b47add55b0cb..ea36aaa5c514 100644\n> > --- a/src/cam/meson.build\n> > +++ b/src/cam/meson.build\n> > @@ -27,6 +27,7 @@ if libdrm.found()\n> >  cam_cpp_args += [ '-DHAVE_KMS' ]\n> >  cam_sources += files([\n> >      'drm.cpp',\n> > +    'kms_sink.cpp'\n> >  ])\n> >  endif\n> >  \n> > -- \n> > Regards,\n> > \n> > Laurent Pinchart\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2B400C3235\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Aug 2021 08:53:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4DA7C68811;\n\tWed,  4 Aug 2021 10:53:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0284C6026C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Aug 2021 10:53:12 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A83C24F;\n\tWed,  4 Aug 2021 10:53:12 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WAgZx4AL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628067192;\n\tbh=b3y15kURvXkwIMvqR2zM63lUfc+iuS98tGKfYa/ufjs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WAgZx4ALVsZyjAttQwFk4BhG9mzwQljP0+09un72uBLygy7r3sjzdkoumdw9PbOsU\n\txXQEAIk6DS5B6MOg1G/BfcOaTQHY+rJzVHRbloU1rK76BMj1lx/BIP3CohVKyiQJLo\n\tVXu3XM+cJyG7QNDKPc99izl+derFkbgrbKzazfs8=","Date":"Wed, 4 Aug 2021 11:53:00 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"paul.elder@ideasonboard.com","Message-ID":"<YQpVbGONd5/F+Uu7@pendragon.ideasonboard.com>","References":"<20210730010306.19956-1-laurent.pinchart@ideasonboard.com>\n\t<20210730010306.19956-7-laurent.pinchart@ideasonboard.com>\n\t<20210804065618.GE2167@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210804065618.GE2167@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v2 6/8] cam: Add KMS sink class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18549,"web_url":"https://patchwork.libcamera.org/comment/18549/","msgid":"<YQpbV4yOC1dhfOlu@pendragon.ideasonboard.com>","date":"2021-08-04T09:18:15","subject":"Re: [libcamera-devel] [PATCH v2 6/8] cam: Add KMS sink class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Aug 04, 2021 at 11:53:01AM +0300, Laurent Pinchart wrote:\n> On Wed, Aug 04, 2021 at 03:56:18PM +0900, paul.elder@ideasonboard.com wrote:\n> > On Fri, Jul 30, 2021 at 04:03:04AM +0300, Laurent Pinchart wrote:\n> > > Add a KMSSink class to display framebuffers through the DRM/KMS API.\n> > > \n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > > Changes since v1:\n> > > \n> > > - Use formats:: namespace\n> > > ---\n> > >  src/cam/kms_sink.cpp | 298 +++++++++++++++++++++++++++++++++++++++++++\n> > >  src/cam/kms_sink.h   |  76 +++++++++++\n> > >  src/cam/meson.build  |   1 +\n> > >  3 files changed, 375 insertions(+)\n> > >  create mode 100644 src/cam/kms_sink.cpp\n> > >  create mode 100644 src/cam/kms_sink.h\n> > > \n> > > diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp\n> > > new file mode 100644\n> > > index 000000000000..b8f86dcb6f4e\n> > > --- /dev/null\n> > > +++ b/src/cam/kms_sink.cpp\n> > > @@ -0,0 +1,298 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2020, Ideas on Board Oy\n> > \n> > s/2020/2021 ?\n> > \n> > Same for the header.\n> > \n> > > + *\n> > > + * kms_sink.cpp - KMS Sink\n> > > + */\n> > > +\n> > > +#include \"kms_sink.h\"\n> > > +\n> > > +#include <algorithm>\n> > > +#include <assert.h>\n> > > +#include <iostream>\n> > > +#include <memory>\n> > > +#include <string.h>\n> > > +\n> > > +#include <libcamera/camera.h>\n> > > +#include <libcamera/formats.h>\n> > > +#include <libcamera/framebuffer.h>\n> > > +#include <libcamera/stream.h>\n> > > +\n> > > +#include \"drm.h\"\n> > > +\n> > > +KMSSink::KMSSink(const std::string &connectorName)\n> > > +\t: connector_(nullptr), crtc_(nullptr), plane_(nullptr), mode_(nullptr)\n> > > +{\n> > > +\tint ret = dev_.init();\n> > > +\tif (ret < 0)\n> > > +\t\treturn;\n> > > +\n> > > +\t/*\n> > > +\t * Find the requested connector. If no connector is requested, pick the\n> > > +\t * first connected connector.\n> > > +\t */\n> > > +\tfor (const DRM::Connector &conn : dev_.connectors()) {\n> > > +\t\tif (conn.name() == connectorName) {\n> > > +\t\t\tconnector_ = &conn;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > +\n> > > +\t\tif (conn.status() != DRM::Connector::Disconnected) {\n> > > +\t\t\tif (!connector_ ||\n> > > +\t\t\t    (connector_->status() == DRM::Connector::Unknown &&\n> > > +\t\t\t     conn.status() == DRM::Connector::Connected))\n> > > +\t\t\t\tconnector_ = &conn;\n> > > +\t\t}\n> > \n> > I think this is the only part I'm confused about. If no connector is\n> > requested, then the first candidate connector can be chosen even if its\n> > status is Unknown.\n> \n> First of all, a bit of background information. DRM/KMS isn't always able\n> to tell if a connector is connected, for instance VGA often lacks hot\n> plug detection support. That's why a connector can have three states:\n> disconnected, unknown or connected.\n> \n> Then, the logic. If no suitable connector has been found yet\n> (!connector_), then we pick any connector that is not marked as\n> disconnected. Otherwise, we replace the currently (tentatively) selected\n> connector if it has state unknown, and conn.status() is connected. This\n> results in picking the first connected connector, with a fallback on the\n> last connector in unknown state if no connector is known to be\n> connected.\n\nActually the fallback is on the first connector with unknown state. I'll\ndocument this properly.\n\n> The logic is a bit convoluted, I'll try to do better.\n> \n> > > +\t}\n> > > +\n> > > +\tif (!connector_) {\n> > > +\t\tif (!connectorName.empty())\n> > > +\t\t\tstd::cerr\n> > > +\t\t\t\t<< \"Connector \" << connectorName << \" not found\"\n> > > +\t\t\t\t<< std::endl;\n> > > +\t\telse\n> > > +\t\t\tstd::cerr << \"No connected connector found\" << std::endl;\n> > > +\t\treturn;\n> > > +\t}\n> > > +\n> > > +\tdev_.requestComplete.connect(this, &KMSSink::requestComplete);\n> > > +}\n> > > +\n> > > +void KMSSink::mapBuffer(libcamera::FrameBuffer *buffer)\n> > > +{\n> > > +\tstd::unique_ptr<DRM::FrameBuffer> drmBuffer =\n> > > +\t\tdev_.createFrameBuffer(*buffer, format_, size_, stride_);\n> > > +\tif (!drmBuffer)\n> > > +\t\treturn;\n> > > +\n> > > +\tbuffers_.emplace(std::piecewise_construct,\n> > > +\t\t\t std::forward_as_tuple(buffer),\n> > > +\t\t\t std::forward_as_tuple(std::move(drmBuffer)));\n> > > +}\n> > > +\n> > > +int KMSSink::configure(const libcamera::CameraConfiguration &config)\n> > > +{\n> > > +\tcrtc_ = nullptr;\n> > > +\tplane_ = nullptr;\n> > > +\tmode_ = nullptr;\n> > > +\n> > > +\tconst libcamera::StreamConfiguration &cfg = config.at(0);\n> > > +\tint ret = configurePipeline(cfg.pixelFormat);\n> > > +\tif (ret < 0)\n> > > +\t\treturn ret;\n> > > +\n> > > +\tconst std::vector<DRM::Mode> &modes = connector_->modes();\n> > > +\tconst auto iter = std::find_if(modes.begin(), modes.end(),\n> > > +\t\t\t\t       [&](const DRM::Mode &mode) {\n> > > +\t\t\t\t\t       return mode.hdisplay == cfg.size.width &&\n> > > +\t\t\t\t\t\t      mode.vdisplay == cfg.size.height;\n> > > +\t\t\t\t       });\n> > > +\tif (iter == modes.end()) {\n> > > +\t\tstd::cerr\n> > > +\t\t\t<< \"No mode matching \" << cfg.size.toString()\n> > > +\t\t\t<< std::endl;\n> > > +\t\treturn -EINVAL;\n> > > +\t}\n> > > +\n> > > +\tmode_ = &*iter;\n> > > +\tsize_ = cfg.size;\n> > > +\tstride_ = cfg.stride;\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> > > +{\n> > > +\t/*\n> > > +\t * If the requested format has an alpha channel, also consider the X\n> > > +\t * variant.\n> > > +\t */\n> > > +\tlibcamera::PixelFormat xFormat;\n> > > +\n> > > +\tswitch (format) {\n> > > +\tcase libcamera::formats::ABGR8888:\n> > > +\t\txFormat = libcamera::formats::XBGR8888;\n> > > +\t\tbreak;\n> > > +\tcase libcamera::formats::ARGB8888:\n> > > +\t\txFormat = libcamera::formats::XRGB8888;\n> > > +\t\tbreak;\n> > > +\tcase libcamera::formats::BGRA8888:\n> > > +\t\txFormat = libcamera::formats::BGRX8888;\n> > > +\t\tbreak;\n> > > +\tcase libcamera::formats::RGBA8888:\n> > > +\t\txFormat = libcamera::formats::RGBX8888;\n> > > +\t\tbreak;\n> > > +\t}\n> > > +\n> > > +\t/*\n> > > +\t * Find a CRTC and plane suitable for the request format and the\n> > > +\t * connector at the end of the pipeline. Restrict the search to primary\n> > > +\t * planes for now.\n> > > +\t */\n> > > +\tfor (const DRM::Encoder *encoder : connector_->encoders()) {\n> > > +\t\tfor (const DRM::Crtc *crtc : encoder->possibleCrtcs()) {\n> > > +\t\t\tfor (const DRM::Plane *plane : crtc->planes()) {\n> > > +\t\t\t\tif (plane->type() != DRM::Plane::TypePrimary)\n> > > +\t\t\t\t\tcontinue;\n> > > +\n> > > +\t\t\t\tif (plane->supportsFormat(format)) {\n> > > +\t\t\t\t\tcrtc_ = crtc;\n> > > +\t\t\t\t\tplane_ = plane;\n> > > +\t\t\t\t\tformat_ = format;\n> > > +\t\t\t\t\treturn 0;\n> > > +\t\t\t\t}\n> > > +\n> > > +\t\t\t\tif (plane->supportsFormat(xFormat)) {\n> > > +\t\t\t\t\tcrtc_ = crtc;\n> > > +\t\t\t\t\tplane_ = plane;\n> > > +\t\t\t\t\tformat_ = xFormat;\n> > > +\t\t\t\t\treturn 0;\n> > > +\t\t\t\t}\n> > > +\t\t\t}\n> > > +\t\t}\n> > > +\t}\n> > > +\n> > > +\tstd::cerr\n> > > +\t\t<< \"Unable to find display pipeline for format \"\n> > > +\t\t<< format.toString() << std::endl;\n> > > +\treturn -EPIPE;\n> > > +}\n> > > +\n> > > +int KMSSink::start()\n> > > +{\n> > > +\tstd::unique_ptr<DRM::AtomicRequest> request;\n> > > +\n> > > +\tint ret = FrameSink::start();\n> > > +\tif (ret < 0)\n> > > +\t\treturn ret;\n> > > +\n> > > +\t/* Disable all CRTCs and planes to start from a known valid state. */\n> > > +\trequest = std::make_unique<DRM::AtomicRequest>(&dev_);\n> > > +\n> > > +\tfor (const DRM::Crtc &crtc : dev_.crtcs())\n> > > +\t\trequest->addProperty(&crtc, \"ACTIVE\", 0);\n> > > +\n> > > +\tfor (const DRM::Plane &plane : dev_.planes()) {\n> > > +\t\trequest->addProperty(&plane, \"CRTC_ID\", 0);\n> > > +\t\trequest->addProperty(&plane, \"FB_ID\", 0);\n> > > +\t}\n> > > +\n> > > +\tret = request->commit(DRM::AtomicRequest::FlagAllowModeset);\n> > > +\tif (ret < 0) {\n> > > +\t\tstd::cerr\n> > > +\t\t\t<< \"Failed to disable CRTCs and planes: \"\n> > > +\t\t\t<< strerror(-ret) << std::endl;\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\t/* Enable the display pipeline with no plane to start with. */\n> > > +\trequest = std::make_unique<DRM::AtomicRequest>(&dev_);\n> > > +\n> > > +\trequest->addProperty(connector_, \"CRTC_ID\", crtc_->id());\n> > > +\trequest->addProperty(crtc_, \"ACTIVE\", 1);\n> > > +\trequest->addProperty(crtc_, \"MODE_ID\", mode_->toBlob(&dev_));\n> > > +\n> > > +\tret = request->commit(DRM::AtomicRequest::FlagAllowModeset);\n> > > +\tif (ret < 0) {\n> > > +\t\tstd::cerr\n> > > +\t\t\t<< \"Failed to enable display pipeline: \"\n> > > +\t\t\t<< strerror(-ret) << std::endl;\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\tplaneInitialized_ = false;\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +int KMSSink::stop()\n> > > +{\n> > > +\t/* Display pipeline. */\n> > > +\tDRM::AtomicRequest request(&dev_);\n> > > +\n> > > +\trequest.addProperty(connector_, \"CRTC_ID\", 0);\n> > > +\trequest.addProperty(crtc_, \"ACTIVE\", 0);\n> > > +\trequest.addProperty(crtc_, \"MODE_ID\", 0);\n> > > +\trequest.addProperty(plane_, \"CRTC_ID\", 0);\n> > > +\trequest.addProperty(plane_, \"FB_ID\", 0);\n> > > +\n> > > +\tint ret = request.commit(DRM::AtomicRequest::FlagAllowModeset);\n> > > +\tif (ret < 0) {\n> > > +\t\tstd::cerr\n> > > +\t\t\t<< \"Failed to stop display pipeline: \"\n> > > +\t\t\t<< strerror(-ret) << std::endl;\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\t/* Free all buffers. */\n> > > +\tpending_.reset();\n> > > +\tqueued_.reset();\n> > > +\tactive_.reset();\n> > > +\tbuffers_.clear();\n> > > +\n> > > +\treturn FrameSink::stop();\n> > > +}\n> > > +\n> > > +bool KMSSink::consumeRequest(libcamera::Request *camRequest)\n> > > +{\n> > > +\tif (pending_)\n> > > +\t\treturn true;\n> > > +\n> > > +\tlibcamera::FrameBuffer *buffer = camRequest->buffers().begin()->second;\n> > > +\tauto iter = buffers_.find(buffer);\n> > > +\tif (iter == buffers_.end())\n> > > +\t\treturn true;\n> > > +\n> > > +\tDRM::FrameBuffer *drmBuffer = iter->second.get();\n> > > +\n> > > +\tDRM::AtomicRequest *drmRequest = new DRM::AtomicRequest(&dev_);\n> > > +\tdrmRequest->addProperty(plane_, \"FB_ID\", drmBuffer->id());\n> > > +\n> > > +\tif (!planeInitialized_) {\n> > > +\t\tdrmRequest->addProperty(plane_, \"CRTC_ID\", crtc_->id());\n> > > +\t\tdrmRequest->addProperty(plane_, \"SRC_X\", 0 << 16);\n> > > +\t\tdrmRequest->addProperty(plane_, \"SRC_Y\", 0 << 16);\n> > > +\t\tdrmRequest->addProperty(plane_, \"SRC_W\", mode_->hdisplay << 16);\n> > > +\t\tdrmRequest->addProperty(plane_, \"SRC_H\", mode_->vdisplay << 16);\n> > > +\t\tdrmRequest->addProperty(plane_, \"CRTC_X\", 0);\n> > > +\t\tdrmRequest->addProperty(plane_, \"CRTC_Y\", 0);\n> > > +\t\tdrmRequest->addProperty(plane_, \"CRTC_W\", mode_->hdisplay);\n> > > +\t\tdrmRequest->addProperty(plane_, \"CRTC_H\", mode_->vdisplay);\n> > > +\t\tplaneInitialized_ = true;\n> > > +\t}\n> > > +\n> > > +\tpending_ = std::make_unique<Request>(drmRequest, camRequest);\n> > > +\n> > > +\tstd::lock_guard<std::mutex> lock(lock_);\n> > > +\n> > > +\tif (!queued_) {\n> > > +\t\tint ret = drmRequest->commit(DRM::AtomicRequest::FlagAsync);\n> > > +\t\tif (ret < 0)\n> > > +\t\t\tstd::cerr\n> > > +\t\t\t\t<< \"Failed to commit atomic request: \"\n> > > +\t\t\t\t<< strerror(-ret) << std::endl;\n> > > +\t\tqueued_ = std::move(pending_);\n> > > +\t}\n> > > +\n> > > +\treturn false;\n> > > +}\n> > > +\n> > > +void KMSSink::requestComplete(DRM::AtomicRequest *request)\n> > > +{\n> > > +\tstd::lock_guard<std::mutex> lock(lock_);\n> > > +\n> > > +\tassert(queued_ && queued_->drmRequest_.get() == request);\n> > > +\n> > > +\t/* Complete the active request, if any. */\n> > > +\tif (active_)\n> > > +\t\trequestReleased.emit(active_->camRequest_);\n> > > +\n> > > +\t/* The queued request becomes active. */\n> > > +\tactive_ = std::move(queued_);\n> > > +\n> > > +\t/* Queue the pending request, if any. */\n> > > +\tif (pending_) {\n> > > +\t\tpending_->drmRequest_->commit(DRM::AtomicRequest::FlagAsync);\n> > > +\t\tqueued_ = std::move(pending_);\n> > > +\t}\n> > > +}\n> > > diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h\n> > > new file mode 100644\n> > > index 000000000000..7b6ffcede28c\n> > > --- /dev/null\n> > > +++ b/src/cam/kms_sink.h\n> > > @@ -0,0 +1,76 @@\n> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > > +/*\n> > > + * Copyright (C) 2020, Ideas on Board Oy\n> > > + *\n> > > + * kms_sink.h - KMS Sink\n> > > + */\n> > > +#ifndef __CAM_KMS_SINK_H__\n> > > +#define __CAM_KMS_SINK_H__\n> > > +\n> > > +#include <list>\n> > > +#include <memory>\n> > > +#include <mutex>\n> > > +#include <string>\n> > > +#include <utility>\n> > > +\n> > > +#include <libcamera/base/signal.h>\n> > > +\n> > > +#include <libcamera/geometry.h>\n> > > +#include <libcamera/pixel_format.h>\n> > > +\n> > > +#include \"drm.h\"\n> > > +#include \"frame_sink.h\"\n> > > +\n> > > +class KMSSink : public FrameSink\n> > > +{\n> > > +public:\n> > > +\tKMSSink(const std::string &connectorName);\n> > > +\n> > > +\tbool isValid() const { return connector_ != nullptr; }\n> > > +\n> > > +\tvoid mapBuffer(libcamera::FrameBuffer *buffer) override;\n> > > +\n> > > +\tint configure(const libcamera::CameraConfiguration &config) override;\n> > > +\tint start() override;\n> > > +\tint stop() override;\n> > > +\n> > > +\tbool consumeRequest(libcamera::Request *request) override;\n> > > +\n> > > +private:\n> > > +\tclass Request\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tRequest(DRM::AtomicRequest *drmRequest, libcamera::Request *camRequest)\n> > > +\t\t\t: drmRequest_(drmRequest), camRequest_(camRequest)\n> > > +\t\t{\n> > > +\t\t}\n> > > +\n> > > +\t\tstd::unique_ptr<DRM::AtomicRequest> drmRequest_;\n> > > +\t\tlibcamera::Request *camRequest_;\n> > > +\t};\n> > > +\n> > > +\tint configurePipeline(const libcamera::PixelFormat &format);\n> > > +\tvoid requestComplete(DRM::AtomicRequest *request);\n> > > +\n> > > +\tDRM::Device dev_;\n> > > +\n> > > +\tconst DRM::Connector *connector_;\n> > > +\tconst DRM::Crtc *crtc_;\n> > > +\tconst DRM::Plane *plane_;\n> > > +\tconst DRM::Mode *mode_;\n> > > +\n> > > +\tlibcamera::PixelFormat format_;\n> > > +\tlibcamera::Size size_;\n> > > +\tunsigned int stride_;\n> > > +\n> > > +\tbool planeInitialized_;\n> > > +\n> > > +\tstd::map<libcamera::FrameBuffer *, std::unique_ptr<DRM::FrameBuffer>> buffers_;\n> > > +\n> > > +\tstd::mutex lock_;\n> > > +\tstd::unique_ptr<Request> pending_;\n> > > +\tstd::unique_ptr<Request> queued_;\n> > > +\tstd::unique_ptr<Request> active_;\n> > > +};\n> > > +\n> > > +#endif /* __CAM_KMS_SINK_H__ */\n> > > diff --git a/src/cam/meson.build b/src/cam/meson.build\n> > > index b47add55b0cb..ea36aaa5c514 100644\n> > > --- a/src/cam/meson.build\n> > > +++ b/src/cam/meson.build\n> > > @@ -27,6 +27,7 @@ if libdrm.found()\n> > >  cam_cpp_args += [ '-DHAVE_KMS' ]\n> > >  cam_sources += files([\n> > >      'drm.cpp',\n> > > +    'kms_sink.cpp'\n> > >  ])\n> > >  endif","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A3116C3232\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Aug 2021 09:18:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C31468811;\n\tWed,  4 Aug 2021 11:18:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E44016026C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Aug 2021 11:18:27 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 828A124F;\n\tWed,  4 Aug 2021 11:18:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"wXWMMocM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628068707;\n\tbh=r6UOESFyMD14jp8e/R5IOYlegCA2Pw6E4xdtrg2QUFQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wXWMMocMbQQeGuxi6BGCIbQeI0A9EIw+swUtjX1AALLNhqcwTTWPzuMsDiG5wsecK\n\tHlIbTRhOSm69ZQzksijvLFDA1HPnoUUmQ3UkdOouTc4/PnrbSLgSNCpn/QzJdY4iyg\n\tOwmkBTEj6s971W+7j1SuvM/NvHrRZxzsdtYcYHAs=","Date":"Wed, 4 Aug 2021 12:18:15 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"paul.elder@ideasonboard.com","Message-ID":"<YQpbV4yOC1dhfOlu@pendragon.ideasonboard.com>","References":"<20210730010306.19956-1-laurent.pinchart@ideasonboard.com>\n\t<20210730010306.19956-7-laurent.pinchart@ideasonboard.com>\n\t<20210804065618.GE2167@pyrite.rasen.tech>\n\t<YQpVbGONd5/F+Uu7@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<YQpVbGONd5/F+Uu7@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 6/8] cam: Add KMS sink class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]