[libcamera-devel,v2] libcamera: pipeline: stm32: add pipeline handler for stm32

1072 diff mbox series

Provide a pipeline handler for the stm32 driver.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 2:
- fix indentation and run checkstyle:
./utils/checkstyle.py 
---------------------------------------------------------------------------------------------------
9360626c2158b2c5d865608f4504d203b6d0014f libcamera: pipeline: stm32: add pipeline handler for stm32
---------------------------------------------------------------------------------------------------
No style issue detected
- Use the first video node instead of using MEDIA_ENT_FL_DEFAULT flag

 src/libcamera/pipeline/meson.build       |   2 +
 src/libcamera/pipeline/stm32/meson.build |   3 +
 src/libcamera/pipeline/stm32/stm32.cpp   | 220 +++++++++++++++++++++++++++++++
 3 files changed, 225 insertions(+)
 create mode 100644 src/libcamera/pipeline/stm32/meson.build
 create mode 100644 src/libcamera/pipeline/stm32/stm32.cpp

Hi Benjamin

Thank you for the updated patch.

We have discussed this topic today in the team, and we are still in the
mindset that the STM32 is a simple pipeline currently, even with the
bridge. (It has only a single linear stream) and this perhaps deserves
some thought to make it easy to add other 'simple' streams with a single
match line addition.

We should be able to use a single class to support the majority of the
functionality and prevent the requirement of duplicating lots of code
and files.


Is this a topic you would be able to tackle? We understand that you
might not be able to commit such time (but it will be great if you can),
and if that's the case then we don't want to block this pipeline
handler, but we would need to have access to the hardware ourselves to
do the simplification later. In that event, would you be able to provide
us with boards and sensors for testing and development?


(comments below in the code relating to the attached updated patches)


Regards

Kieran


On 19/04/2019 11:03, Benjamin Gaignard wrote:
> Provide a pipeline handler for the stm32 driver.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> version 2:
> - fix indentation and run checkstyle:
> ./utils/checkstyle.py 
> ---------------------------------------------------------------------------------------------------
> 9360626c2158b2c5d865608f4504d203b6d0014f libcamera: pipeline: stm32: add pipeline handler for stm32
> ---------------------------------------------------------------------------------------------------
> No style issue detected
> - Use the first video node instead of using MEDIA_ENT_FL_DEFAULT flag
> 
>  src/libcamera/pipeline/meson.build       |   2 +
>  src/libcamera/pipeline/stm32/meson.build |   3 +
>  src/libcamera/pipeline/stm32/stm32.cpp   | 220 +++++++++++++++++++++++++++++++
>  3 files changed, 225 insertions(+)
>  create mode 100644 src/libcamera/pipeline/stm32/meson.build
>  create mode 100644 src/libcamera/pipeline/stm32/stm32.cpp
> 
> diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
> index 40bb264..08d6e1c 100644
> --- a/src/libcamera/pipeline/meson.build
> +++ b/src/libcamera/pipeline/meson.build
> @@ -4,3 +4,5 @@ libcamera_sources += files([
>  ])
>  
>  subdir('ipu3')
> +
> +subdir('stm32')
> diff --git a/src/libcamera/pipeline/stm32/meson.build b/src/libcamera/pipeline/stm32/meson.build
> new file mode 100644
> index 0000000..cb6f16b
> --- /dev/null
> +++ b/src/libcamera/pipeline/stm32/meson.build
> @@ -0,0 +1,3 @@
> +libcamera_sources += files([
> +    'stm32.cpp',
> +])
> diff --git a/src/libcamera/pipeline/stm32/stm32.cpp b/src/libcamera/pipeline/stm32/stm32.cpp
> new file mode 100644
> index 0000000..15c7764
> --- /dev/null
> +++ b/src/libcamera/pipeline/stm32/stm32.cpp
> @@ -0,0 +1,220 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * stm32.cpp - Pipeline handler for stm32 devices
> + */
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/request.h>
> +#include <libcamera/stream.h>
> +
> +#include "device_enumerator.h"
> +#include "log.h"
> +#include "media_device.h"
> +#include "pipeline_handler.h"
> +#include "utils.h"
> +#include "v4l2_device.h"
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(STM32)
> +
> +class PipelineHandlerSTM32 : public PipelineHandler
> +{
> +public:
> +	PipelineHandlerSTM32(CameraManager *manager);
> +	~PipelineHandlerSTM32();
> +
> +	std::map<Stream *, StreamConfiguration>
> +	streamConfiguration(Camera *camera, std::set<Stream *> &streams) override;
> +	int configureStreams(
> +		Camera *camera, std::map<Stream *, StreamConfiguration> &config) override;
> +
> +	int allocateBuffers(Camera *camera, Stream *stream) override;
> +	int freeBuffers(Camera *camera, Stream *stream) override;
> +
> +	int start(Camera *camera) override;
> +	void stop(Camera *camera) override;
> +
> +	int queueRequest(Camera *camera, Request *request) override;
> +
> +	bool match(DeviceEnumerator *enumerator);
> +
> +private:
> +	class STM32CameraData : public CameraData
> +	{
> +	public:
> +		STM32CameraData(PipelineHandler *pipe)
> +			: CameraData(pipe), video_(nullptr) {}
> +
> +		~STM32CameraData() { delete video_; }
> +
> +		void bufferReady(Buffer *buffer);
> +
> +		V4L2Device *video_;
> +		Stream stream_;
> +	};
> +
> +	STM32CameraData *cameraData(const Camera *camera)
> +	{
> +		return static_cast<STM32CameraData *>(PipelineHandler::cameraData(camera));
> +	}
> +
> +	std::shared_ptr<MediaDevice> media_;
> +};
> +
> +PipelineHandlerSTM32::PipelineHandlerSTM32(CameraManager *manager)
> +	: PipelineHandler(manager), media_(nullptr)
> +{
> +}
> +
> +PipelineHandlerSTM32::~PipelineHandlerSTM32()
> +{
> +	if (media_)
> +		media_->release();
> +}
> +
> +std::map<Stream *, StreamConfiguration>

I've just learnt when rebasing the RaspberryPi handler to the latest
master that this has now changed, and the Stream configurations use a
CameraConfiguration type.

It's easy enough to fix, but you'll hit it when/if you rebase.

I've attached a patch showing the changes needed for mainline, and an
updated v3 which applies those changes and compiles on todays master.


However just talking to Niklas, it is planned within the next week to
change some of these API's yet again (likely only minor fixes, but still...)


> +PipelineHandlerSTM32::streamConfiguration(Camera *camera,
> +					  std::set<Stream *> &streams)
> +{
> +	STM32CameraData *data = cameraData(camera);
> +
> +	std::map<Stream *, StreamConfiguration> configs;
> +	StreamConfiguration config{};
> +
> +	LOG(STM32, Debug) << "Retrieving default format";
> +	config.width = 640;
> +	config.height = 480;
> +	config.pixelFormat = V4L2_PIX_FMT_YUYV;
> +	config.bufferCount = 4;
> +
> +	configs[&data->stream_] = config;
> +
> +	return configs;
> +}
> +
> +int PipelineHandlerSTM32::configureStreams(
> +	Camera *camera, std::map<Stream *, StreamConfiguration> &config)
> +{
> +	STM32CameraData *data = cameraData(camera);
> +	StreamConfiguration *cfg = &config[&data->stream_];
> +	int ret;
> +
> +	LOG(STM32, Debug) << "Configure the camera for resolution " << cfg->width
> +			  << "x" << cfg->height;
> +
> +	V4L2DeviceFormat format = {};
> +	format.width = cfg->width;
> +	format.height = cfg->height;
> +	format.fourcc = cfg->pixelFormat;
> +
> +	ret = data->video_->setFormat(&format);
> +	if (ret)
> +		return ret;
> +
> +	if (format.width != cfg->width || format.height != cfg->height ||
> +	    format.fourcc != cfg->pixelFormat)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +int PipelineHandlerSTM32::allocateBuffers(Camera *camera, Stream *stream)
> +{
> +	STM32CameraData *data = cameraData(camera);
> +	const StreamConfiguration &cfg = stream->configuration();
> +
> +	LOG(STM32, Debug) << "Requesting " << cfg.bufferCount << " buffers";
> +
> +	return data->video_->exportBuffers(&stream->bufferPool());
> +}
> +
> +int PipelineHandlerSTM32::freeBuffers(Camera *camera, Stream *stream)
> +{
> +	STM32CameraData *data = cameraData(camera);
> +	return data->video_->releaseBuffers();
> +}
> +
> +int PipelineHandlerSTM32::start(Camera *camera)
> +{
> +	STM32CameraData *data = cameraData(camera);
> +	return data->video_->streamOn();
> +}
> +
> +void PipelineHandlerSTM32::stop(Camera *camera)
> +{
> +	STM32CameraData *data = cameraData(camera);
> +	data->video_->streamOff();
> +	PipelineHandler::stop(camera);
> +}
> +
> +int PipelineHandlerSTM32::queueRequest(Camera *camera, Request *request)
> +{
> +	STM32CameraData *data = cameraData(camera);
> +	Buffer *buffer = request->findBuffer(&data->stream_);
> +	if (!buffer) {
> +		LOG(STM32, Error) << "Attempt to queue request with invalid stream";
> +
> +		return -ENOENT;
> +	}
> +
> +	int ret = data->video_->queueBuffer(buffer);
> +	if (ret < 0)
> +		return ret;
> +
> +	PipelineHandler::queueRequest(camera, request);
> +
> +	return 0;
> +}
> +
> +bool PipelineHandlerSTM32::match(DeviceEnumerator *enumerator)
> +{
> +	DeviceMatch dm("stm32-dcmi");
> +
> +	media_ = enumerator->search(dm);
> +	if (!media_)
> +		return false;
> +
> +	media_->acquire();
> +
> +	std::unique_ptr<STM32CameraData> data =
> +		utils::make_unique<STM32CameraData>(this);
> +
> +	/* Open the first video node */
> +	MediaEntity *entity = media_->entities()[0];
> +	if (!entity) {
> +		LOG(STM32, Error) << "No video node";
> +		return false;
> +	}
> +
> +	data->video_ = new V4L2Device(entity);
> +	if (!data->video_) {
> +		LOG(STM32, Error) << "Could not find a default video device";
> +		return false;
> +	}
> +
> +	if (data->video_->open())
> +		return false;
> +
> +	data->video_->bufferReady.connect(data.get(), &STM32CameraData::bufferReady);
> +
> +	/* Create and register the camera. */
> +	std::set<Stream *> streams{ &data->stream_ };
> +	std::shared_ptr<Camera> camera =
> +		Camera::create(this, media_->model(), streams);
> +	registerCamera(std::move(camera), std::move(data));
> +
> +	return true;
> +}
> +
> +void PipelineHandlerSTM32::STM32CameraData::bufferReady(Buffer *buffer)
> +{
> +	Request *request = queuedRequests_.front();
> +
> +	pipe_->completeBuffer(camera_, request, buffer);
> +	pipe_->completeRequest(camera_, request);
> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerSTM32);
> +
> +} /* namespace libcamera */
>