[libcamera-devel,v2,6/8] cam: Add KMS sink class

13152 diff mbox series

Add a KMSSink class to display framebuffers through the DRM/KMS API.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Use formats:: namespace
---
 src/cam/kms_sink.cpp | 298 +++++++++++++++++++++++++++++++++++++++++++
 src/cam/kms_sink.h   |  76 +++++++++++
 src/cam/meson.build  |   1 +
 3 files changed, 375 insertions(+)
 create mode 100644 src/cam/kms_sink.cpp
 create mode 100644 src/cam/kms_sink.h

Hi Laurent,

On Fri, Jul 30, 2021 at 04:03:04AM +0300, Laurent Pinchart wrote:
> Add a KMSSink class to display framebuffers through the DRM/KMS API.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v1:
> 
> - Use formats:: namespace
> ---
>  src/cam/kms_sink.cpp | 298 +++++++++++++++++++++++++++++++++++++++++++
>  src/cam/kms_sink.h   |  76 +++++++++++
>  src/cam/meson.build  |   1 +
>  3 files changed, 375 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..b8f86dcb6f4e
> --- /dev/null
> +++ b/src/cam/kms_sink.cpp
> @@ -0,0 +1,298 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Ideas on Board Oy

s/2020/2021 ?

Same for the header.

> + *
> + * 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 connector is requested, pick the
> +	 * first connected connector.
> +	 */
> +	for (const DRM::Connector &conn : dev_.connectors()) {
> +		if (conn.name() == connectorName) {
> +			connector_ = &conn;
> +			break;
> +		}
> +
> +		if (conn.status() != DRM::Connector::Disconnected) {
> +			if (!connector_ ||
> +			    (connector_->status() == DRM::Connector::Unknown &&
> +			     conn.status() == DRM::Connector::Connected))
> +				connector_ = &conn;
> +		}

I think this is the only part I'm confused about. If no connector is
requested, then the first candidate connector can be chosen even if its
status is Unknown.


Paul

> +	}
> +
> +	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)
> +{
> +	crtc_ = nullptr;
> +	plane_ = nullptr;
> +	mode_ = nullptr;
> +
> +	const libcamera::StreamConfiguration &cfg = config.at(0);
> +	int ret = configurePipeline(cfg.pixelFormat);
> +	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::consumeRequest(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);
> +
> +	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_)
> +		requestReleased.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..7b6ffcede28c
> --- /dev/null
> +++ b/src/cam/kms_sink.h
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, 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 consumeRequest(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
>

Hi Paul,

On Wed, Aug 04, 2021 at 03:56:18PM +0900, paul.elder@ideasonboard.com wrote:
> On Fri, Jul 30, 2021 at 04:03:04AM +0300, Laurent Pinchart wrote:
> > Add a KMSSink class to display framebuffers through the DRM/KMS API.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Use formats:: namespace
> > ---
> >  src/cam/kms_sink.cpp | 298 +++++++++++++++++++++++++++++++++++++++++++
> >  src/cam/kms_sink.h   |  76 +++++++++++
> >  src/cam/meson.build  |   1 +
> >  3 files changed, 375 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..b8f86dcb6f4e
> > --- /dev/null
> > +++ b/src/cam/kms_sink.cpp
> > @@ -0,0 +1,298 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Ideas on Board Oy
> 
> s/2020/2021 ?
> 
> Same for the header.
> 
> > + *
> > + * 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 connector is requested, pick the
> > +	 * first connected connector.
> > +	 */
> > +	for (const DRM::Connector &conn : dev_.connectors()) {
> > +		if (conn.name() == connectorName) {
> > +			connector_ = &conn;
> > +			break;
> > +		}
> > +
> > +		if (conn.status() != DRM::Connector::Disconnected) {
> > +			if (!connector_ ||
> > +			    (connector_->status() == DRM::Connector::Unknown &&
> > +			     conn.status() == DRM::Connector::Connected))
> > +				connector_ = &conn;
> > +		}
> 
> I think this is the only part I'm confused about. If no connector is
> requested, then the first candidate connector can be chosen even if its
> status is Unknown.

First of all, a bit of background information. DRM/KMS isn't always able
to tell if a connector is connected, for instance VGA often lacks hot
plug detection support. That's why a connector can have three states:
disconnected, unknown or connected.

Then, the logic. If no suitable connector has been found yet
(!connector_), then we pick any connector that is not marked as
disconnected. Otherwise, we replace the currently (tentatively) selected
connector if it has state unknown, and conn.status() is connected. This
results in picking the first connected connector, with a fallback on the
last connector in unknown state if no connector is known to be
connected.

The logic is a bit convoluted, I'll try to do better.

> > +	}
> > +
> > +	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)
> > +{
> > +	crtc_ = nullptr;
> > +	plane_ = nullptr;
> > +	mode_ = nullptr;
> > +
> > +	const libcamera::StreamConfiguration &cfg = config.at(0);
> > +	int ret = configurePipeline(cfg.pixelFormat);
> > +	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::consumeRequest(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);
> > +
> > +	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_)
> > +		requestReleased.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..7b6ffcede28c
> > --- /dev/null
> > +++ b/src/cam/kms_sink.h
> > @@ -0,0 +1,76 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, 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 consumeRequest(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
> >

On Wed, Aug 04, 2021 at 11:53:01AM +0300, Laurent Pinchart wrote:
> On Wed, Aug 04, 2021 at 03:56:18PM +0900, paul.elder@ideasonboard.com wrote:
> > On Fri, Jul 30, 2021 at 04:03:04AM +0300, Laurent Pinchart wrote:
> > > Add a KMSSink class to display framebuffers through the DRM/KMS API.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > Changes since v1:
> > > 
> > > - Use formats:: namespace
> > > ---
> > >  src/cam/kms_sink.cpp | 298 +++++++++++++++++++++++++++++++++++++++++++
> > >  src/cam/kms_sink.h   |  76 +++++++++++
> > >  src/cam/meson.build  |   1 +
> > >  3 files changed, 375 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..b8f86dcb6f4e
> > > --- /dev/null
> > > +++ b/src/cam/kms_sink.cpp
> > > @@ -0,0 +1,298 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/*
> > > + * Copyright (C) 2020, Ideas on Board Oy
> > 
> > s/2020/2021 ?
> > 
> > Same for the header.
> > 
> > > + *
> > > + * 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 connector is requested, pick the
> > > +	 * first connected connector.
> > > +	 */
> > > +	for (const DRM::Connector &conn : dev_.connectors()) {
> > > +		if (conn.name() == connectorName) {
> > > +			connector_ = &conn;
> > > +			break;
> > > +		}
> > > +
> > > +		if (conn.status() != DRM::Connector::Disconnected) {
> > > +			if (!connector_ ||
> > > +			    (connector_->status() == DRM::Connector::Unknown &&
> > > +			     conn.status() == DRM::Connector::Connected))
> > > +				connector_ = &conn;
> > > +		}
> > 
> > I think this is the only part I'm confused about. If no connector is
> > requested, then the first candidate connector can be chosen even if its
> > status is Unknown.
> 
> First of all, a bit of background information. DRM/KMS isn't always able
> to tell if a connector is connected, for instance VGA often lacks hot
> plug detection support. That's why a connector can have three states:
> disconnected, unknown or connected.
> 
> Then, the logic. If no suitable connector has been found yet
> (!connector_), then we pick any connector that is not marked as
> disconnected. Otherwise, we replace the currently (tentatively) selected
> connector if it has state unknown, and conn.status() is connected. This
> results in picking the first connected connector, with a fallback on the
> last connector in unknown state if no connector is known to be
> connected.

Actually the fallback is on the first connector with unknown state. I'll
document this properly.

> The logic is a bit convoluted, I'll try to do better.
> 
> > > +	}
> > > +
> > > +	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)
> > > +{
> > > +	crtc_ = nullptr;
> > > +	plane_ = nullptr;
> > > +	mode_ = nullptr;
> > > +
> > > +	const libcamera::StreamConfiguration &cfg = config.at(0);
> > > +	int ret = configurePipeline(cfg.pixelFormat);
> > > +	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::consumeRequest(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);
> > > +
> > > +	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_)
> > > +		requestReleased.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..7b6ffcede28c
> > > --- /dev/null
> > > +++ b/src/cam/kms_sink.h
> > > @@ -0,0 +1,76 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/*
> > > + * Copyright (C) 2020, 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 consumeRequest(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