[libcamera-devel] [PATCH v3 6/8] cam: Add KMS sink class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Aug 5 13:13:58 CEST 2021
Hi Umang,
On Thu, Aug 05, 2021 at 12:34:06PM +0530, Umang Jain wrote:
> On 8/4/21 6:13 PM, Laurent Pinchart wrote:
> > Add a KMSSink class to display framebuffers through the DRM/KMS API.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Changes since v2:
> >
> > - Simplify connector selection logic, and fix it when a connector name
> > is specified
> > - Fail configure() if no connector is selected
> > - Update copyright years
> >
> > Changes since v1:
> >
> > - Use formats:: namespace
> >
> > fixup! cam: Add KMS sink class
> > ---
> > src/cam/kms_sink.cpp | 306 +++++++++++++++++++++++++++++++++++++++++++
> > src/cam/kms_sink.h | 76 +++++++++++
> > src/cam/meson.build | 1 +
> > 3 files changed, 383 insertions(+)
> > create mode 100644 src/cam/kms_sink.cpp
> > create mode 100644 src/cam/kms_sink.h
> >
> > diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp
> > new file mode 100644
> > index 000000000000..f708b613f226
> > --- /dev/null
> > +++ b/src/cam/kms_sink.cpp
> > @@ -0,0 +1,306 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2021, Ideas on Board Oy
> > + *
> > + * kms_sink.cpp - KMS Sink
> > + */
> > +
> > +#include "kms_sink.h"
> > +
> > +#include <algorithm>
> > +#include <assert.h>
> > +#include <iostream>
> > +#include <memory>
> > +#include <string.h>
> > +
> > +#include <libcamera/camera.h>
> > +#include <libcamera/formats.h>
> > +#include <libcamera/framebuffer.h>
> > +#include <libcamera/stream.h>
> > +
> > +#include "drm.h"
> > +
> > +KMSSink::KMSSink(const std::string &connectorName)
> > + : connector_(nullptr), crtc_(nullptr), plane_(nullptr), mode_(nullptr)
> > +{
> > + int ret = dev_.init();
> > + if (ret < 0)
> > + return;
> > +
> > + /*
> > + * Find the requested connector. If no specific connector is requested,
> > + * pick the first connected connector or, if no connector is connected,
> > + * the first connector with unknown status.
> > + */
> > + for (const DRM::Connector &conn : dev_.connectors()) {
> > + if (!connectorName.empty()) {
> > + if (conn.name() != connectorName)
> > + continue;
> > +
> > + connector_ = &conn;
> > + break;
> > + }
> > +
> > + if (conn.status() == DRM::Connector::Connected) {
> > + connector_ = &conn;
> > + break;
> > + }
> > +
> > + if (!connector_ && conn.status() == DRM::Connector::Unknown)
> > + connector_ = &conn;
> > + }
> > +
> > + if (!connector_) {
> > + if (!connectorName.empty())
> > + std::cerr
> > + << "Connector " << connectorName << " not found"
> > + << std::endl;
> > + else
> > + std::cerr << "No connected connector found" << std::endl;
> > + return;
> > + }
> > +
> > + dev_.requestComplete.connect(this, &KMSSink::requestComplete);
> > +}
> > +
> > +void KMSSink::mapBuffer(libcamera::FrameBuffer *buffer)
> > +{
> > + std::unique_ptr<DRM::FrameBuffer> drmBuffer =
> > + dev_.createFrameBuffer(*buffer, format_, size_, stride_);
> > + if (!drmBuffer)
> > + return;
> > +
> > + buffers_.emplace(std::piecewise_construct,
> > + std::forward_as_tuple(buffer),
> > + std::forward_as_tuple(std::move(drmBuffer)));
> > +}
> > +
> > +int KMSSink::configure(const libcamera::CameraConfiguration &config)
> > +{
> > + if (!connector_)
> > + return -EINVAL;
>
> I assumed there isValid() for a reason, an alternative would be use to
> that. But this is fine as well probably.
That function is actually unused, so I'll drop it.
> > +
> > + crtc_ = nullptr;
> > + plane_ = nullptr;
> > + mode_ = nullptr;
> > +
> > + const libcamera::StreamConfiguration &cfg = config.at(0);
> > + int ret = configurePipeline(cfg.pixelFormat);
>
> Should we be finding a compatiable connector's mode (as done below)
> before configurePipeline() ? Does not seem logical to error out on
> incompatiable mode after we have configured the pipeline with possible
> crtc and encoder.
>
> (But I maybe wrong here, there might be some implicit operational
> sequence between these two, that I am missing)
Sounds reasonable, I'll swap the two.
> > + if (ret < 0)
> > + return ret;
> > +
> > + const std::vector<DRM::Mode> &modes = connector_->modes();
> > + const auto iter = std::find_if(modes.begin(), modes.end(),
> > + [&](const DRM::Mode &mode) {
> > + return mode.hdisplay == cfg.size.width &&
> > + mode.vdisplay == cfg.size.height;
> > + });
> > + if (iter == modes.end()) {
> > + std::cerr
> > + << "No mode matching " << cfg.size.toString()
> > + << std::endl;
> > + return -EINVAL;
> > + }
> > +
> > + mode_ = &*iter;
> > + size_ = cfg.size;
> > + stride_ = cfg.stride;
> > +
> > + return 0;
> > +}
> > +
> > +int KMSSink::configurePipeline(const libcamera::PixelFormat &format)
> > +{
> > + /*
> > + * If the requested format has an alpha channel, also consider the X
> > + * variant.
> > + */
> > + libcamera::PixelFormat xFormat;
> > +
> > + switch (format) {
> > + case libcamera::formats::ABGR8888:
> > + xFormat = libcamera::formats::XBGR8888;
> > + break;
> > + case libcamera::formats::ARGB8888:
> > + xFormat = libcamera::formats::XRGB8888;
> > + break;
> > + case libcamera::formats::BGRA8888:
> > + xFormat = libcamera::formats::BGRX8888;
> > + break;
> > + case libcamera::formats::RGBA8888:
> > + xFormat = libcamera::formats::RGBX8888;
> > + break;
> > + }
> > +
> > + /*
> > + * Find a CRTC and plane suitable for the request format and the
> > + * connector at the end of the pipeline. Restrict the search to primary
> > + * planes for now.
> > + */
> > + for (const DRM::Encoder *encoder : connector_->encoders()) {
> > + for (const DRM::Crtc *crtc : encoder->possibleCrtcs()) {
> > + for (const DRM::Plane *plane : crtc->planes()) {
> > + if (plane->type() != DRM::Plane::TypePrimary)
> > + continue;
> > +
> > + if (plane->supportsFormat(format)) {
> > + crtc_ = crtc;
> > + plane_ = plane;
> > + format_ = format;
> > + return 0;
> > + }
> > +
> > + if (plane->supportsFormat(xFormat)) {
> > + crtc_ = crtc;
> > + plane_ = plane;
> > + format_ = xFormat;
> > + return 0;
> > + }
> > + }
> > + }
> > + }
> > +
> > + std::cerr
> > + << "Unable to find display pipeline for format "
> > + << format.toString() << std::endl;
> > + return -EPIPE;
> > +}
> > +
> > +int KMSSink::start()
> > +{
> > + std::unique_ptr<DRM::AtomicRequest> request;
> > +
> > + int ret = FrameSink::start();
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Disable all CRTCs and planes to start from a known valid state. */
> > + request = std::make_unique<DRM::AtomicRequest>(&dev_);
> > +
> > + for (const DRM::Crtc &crtc : dev_.crtcs())
> > + request->addProperty(&crtc, "ACTIVE", 0);
> > +
> > + for (const DRM::Plane &plane : dev_.planes()) {
> > + request->addProperty(&plane, "CRTC_ID", 0);
> > + request->addProperty(&plane, "FB_ID", 0);
> > + }
> > +
> > + ret = request->commit(DRM::AtomicRequest::FlagAllowModeset);
> > + if (ret < 0) {
> > + std::cerr
> > + << "Failed to disable CRTCs and planes: "
> > + << strerror(-ret) << std::endl;
> > + return ret;
> > + }
> > +
> > + /* Enable the display pipeline with no plane to start with. */
> > + request = std::make_unique<DRM::AtomicRequest>(&dev_);
> > +
> > + request->addProperty(connector_, "CRTC_ID", crtc_->id());
> > + request->addProperty(crtc_, "ACTIVE", 1);
> > + request->addProperty(crtc_, "MODE_ID", mode_->toBlob(&dev_));
> > +
> > + ret = request->commit(DRM::AtomicRequest::FlagAllowModeset);
> > + if (ret < 0) {
> > + std::cerr
> > + << "Failed to enable display pipeline: "
> > + << strerror(-ret) << std::endl;
> > + return ret;
> > + }
> > +
> > + planeInitialized_ = false;
> > +
> > + return 0;
> > +}
> > +
> > +int KMSSink::stop()
> > +{
> > + /* Display pipeline. */
> > + DRM::AtomicRequest request(&dev_);
> > +
> > + request.addProperty(connector_, "CRTC_ID", 0);
> > + request.addProperty(crtc_, "ACTIVE", 0);
> > + request.addProperty(crtc_, "MODE_ID", 0);
> > + request.addProperty(plane_, "CRTC_ID", 0);
> > + request.addProperty(plane_, "FB_ID", 0);
> > +
> > + int ret = request.commit(DRM::AtomicRequest::FlagAllowModeset);
> > + if (ret < 0) {
> > + std::cerr
> > + << "Failed to stop display pipeline: "
> > + << strerror(-ret) << std::endl;
> > + return ret;
> > + }
> > +
> > + /* Free all buffers. */
> > + pending_.reset();
> > + queued_.reset();
> > + active_.reset();
> > + buffers_.clear();
> > +
> > + return FrameSink::stop();
> > +}
> > +
> > +bool KMSSink::processRequest(libcamera::Request *camRequest)
> > +{
> > + if (pending_)
> > + return true;
> > +
> > + libcamera::FrameBuffer *buffer = camRequest->buffers().begin()->second;
> > + auto iter = buffers_.find(buffer);
> > + if (iter == buffers_.end())
> > + return true;
> > +
> > + DRM::FrameBuffer *drmBuffer = iter->second.get();
> > +
> > + DRM::AtomicRequest *drmRequest = new DRM::AtomicRequest(&dev_);
> > + drmRequest->addProperty(plane_, "FB_ID", drmBuffer->id());
> > +
> > + if (!planeInitialized_) {
> > + drmRequest->addProperty(plane_, "CRTC_ID", crtc_->id());
> > + drmRequest->addProperty(plane_, "SRC_X", 0 << 16);
> > + drmRequest->addProperty(plane_, "SRC_Y", 0 << 16);
> > + drmRequest->addProperty(plane_, "SRC_W", mode_->hdisplay << 16);
> > + drmRequest->addProperty(plane_, "SRC_H", mode_->vdisplay << 16);
> > + drmRequest->addProperty(plane_, "CRTC_X", 0);
> > + drmRequest->addProperty(plane_, "CRTC_Y", 0);
> > + drmRequest->addProperty(plane_, "CRTC_W", mode_->hdisplay);
> > + drmRequest->addProperty(plane_, "CRTC_H", mode_->vdisplay);
> > + planeInitialized_ = true;
> > + }
> > +
> > + pending_ = std::make_unique<Request>(drmRequest, camRequest);
>
> This is neat! :)
Unique pointers are really nice, yes.
> Looks good to me with my limited understanding I gained this morning on
> the subject:
>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>
> > +
> > + std::lock_guard<std::mutex> lock(lock_);
> > +
> > + if (!queued_) {
> > + int ret = drmRequest->commit(DRM::AtomicRequest::FlagAsync);
> > + if (ret < 0)
> > + std::cerr
> > + << "Failed to commit atomic request: "
> > + << strerror(-ret) << std::endl;
> > + queued_ = std::move(pending_);
> > + }
> > +
> > + return false;
> > +}
> > +
> > +void KMSSink::requestComplete(DRM::AtomicRequest *request)
> > +{
> > + std::lock_guard<std::mutex> lock(lock_);
> > +
> > + assert(queued_ && queued_->drmRequest_.get() == request);
> > +
> > + /* Complete the active request, if any. */
> > + if (active_)
> > + requestProcessed.emit(active_->camRequest_);
> > +
> > + /* The queued request becomes active. */
> > + active_ = std::move(queued_);
> > +
> > + /* Queue the pending request, if any. */
> > + if (pending_) {
> > + pending_->drmRequest_->commit(DRM::AtomicRequest::FlagAsync);
> > + queued_ = std::move(pending_);
> > + }
> > +}
> > diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h
> > new file mode 100644
> > index 000000000000..2895e00f84a1
> > --- /dev/null
> > +++ b/src/cam/kms_sink.h
> > @@ -0,0 +1,76 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2021, Ideas on Board Oy
> > + *
> > + * kms_sink.h - KMS Sink
> > + */
> > +#ifndef __CAM_KMS_SINK_H__
> > +#define __CAM_KMS_SINK_H__
> > +
> > +#include <list>
> > +#include <memory>
> > +#include <mutex>
> > +#include <string>
> > +#include <utility>
> > +
> > +#include <libcamera/base/signal.h>
> > +
> > +#include <libcamera/geometry.h>
> > +#include <libcamera/pixel_format.h>
> > +
> > +#include "drm.h"
> > +#include "frame_sink.h"
> > +
> > +class KMSSink : public FrameSink
> > +{
> > +public:
> > + KMSSink(const std::string &connectorName);
> > +
> > + bool isValid() const { return connector_ != nullptr; }
> > +
> > + void mapBuffer(libcamera::FrameBuffer *buffer) override;
> > +
> > + int configure(const libcamera::CameraConfiguration &config) override;
> > + int start() override;
> > + int stop() override;
> > +
> > + bool processRequest(libcamera::Request *request) override;
> > +
> > +private:
> > + class Request
> > + {
> > + public:
> > + Request(DRM::AtomicRequest *drmRequest, libcamera::Request *camRequest)
> > + : drmRequest_(drmRequest), camRequest_(camRequest)
> > + {
> > + }
> > +
> > + std::unique_ptr<DRM::AtomicRequest> drmRequest_;
> > + libcamera::Request *camRequest_;
> > + };
> > +
> > + int configurePipeline(const libcamera::PixelFormat &format);
> > + void requestComplete(DRM::AtomicRequest *request);
> > +
> > + DRM::Device dev_;
> > +
> > + const DRM::Connector *connector_;
> > + const DRM::Crtc *crtc_;
> > + const DRM::Plane *plane_;
> > + const DRM::Mode *mode_;
> > +
> > + libcamera::PixelFormat format_;
> > + libcamera::Size size_;
> > + unsigned int stride_;
> > +
> > + bool planeInitialized_;
> > +
> > + std::map<libcamera::FrameBuffer *, std::unique_ptr<DRM::FrameBuffer>> buffers_;
> > +
> > + std::mutex lock_;
> > + std::unique_ptr<Request> pending_;
> > + std::unique_ptr<Request> queued_;
> > + std::unique_ptr<Request> active_;
> > +};
> > +
> > +#endif /* __CAM_KMS_SINK_H__ */
> > diff --git a/src/cam/meson.build b/src/cam/meson.build
> > index b47add55b0cb..ea36aaa5c514 100644
> > --- a/src/cam/meson.build
> > +++ b/src/cam/meson.build
> > @@ -27,6 +27,7 @@ if libdrm.found()
> > cam_cpp_args += [ '-DHAVE_KMS' ]
> > cam_sources += files([
> > 'drm.cpp',
> > + 'kms_sink.cpp'
> > ])
> > endif
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list