[libcamera-devel] [PATCH] libcamera: pipeline: stm32: add pipeline handler for stm32
Benjamin GAIGNARD
benjamin.gaignard at st.com
Tue Apr 16 10:39:25 CEST 2019
On 4/16/19 1:14 AM, Laurent Pinchart wrote:
> Hi Benjamin,
>
> On Tue, Apr 09, 2019 at 09:38:48AM +0000, Benjamin GAIGNARD wrote:
>> On 4/9/19 11:02 AM, Kieran Bingham wrote:
>>> On 08/04/2019 13:43, Benjamin Gaignard wrote:
>>>> Provide a pipeline handler for the stm32 driver.
>>> Excellent :-) Thank you for your contribution.
>>>
>>> Have you been able to test this and successfully list your camera or
>>> capture frames?
>> Yes I have been able to list and capture frames with cam
>>
>>> To list successfully registered cameras on the device:
>>> build/src/cam/cam -l
>>>
>>> I couldn't see any media-controller support in the stm32-dcmi driver...
>>> Perhaps I'm missing something.
>> Hugues has send the patches for media controller support a week ago.
>>
>> https://lkml.org/lkml/2019/4/1/298
>>
>> I have tested libcamera with those patches.
> I think you may have missed the other comments from Kieran further down.
Yes I have completely miss them
>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard at st.com>
>>>> ---
>>>> src/libcamera/pipeline/meson.build | 2 +
>>>> src/libcamera/pipeline/stm32/meson.build | 3 +
>>>> src/libcamera/pipeline/stm32/stm32.cpp | 205 +++++++++++++++++++++++++++++++
>>>> 3 files changed, 210 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',
>>>> +])
>>> Currently the stm32.cpp seems like a fairly simple implementation, which
>>> fits in its own file, much like pipeline/uvcvideo.cpp.
>>>
>>> Do you forsee needing to break the STM32 handler into multiple files
>>> later, or support other components on the STM32?
>>>
>>> I think it's fine if you want to have stm32 as a subdir either way though.
>>>
>>> I wonder also, if it is just a 'simple' device if perhaps we should move
>>> the bulk of the uvcvideo handler to a 'simple.cpp' and allow it to be
>>> subclassed to simply add the device matching, or provide a table of
>>> supported match strings.
>>>
>>>
>>> What hardware is available on the STM32 for processing frames?
>>> Do you have a scaler/resizer? or any further ISP functionality?
No scaler, no resize and no ISP functionality.
It is a basic device to memory driver.
>>>
>>> It would be interesting to see the output of 'media-ctl -p' for your
>>> platform.
The ouput of media-ctl -p (with Hugues's patches)
Media controller API version 4.19.24
Media device information
------------------------
driver stm32-dcmi
model stm32-dcmi
serial
bus info platform:stm32-dcmi
hw revision 0x0
driver version 4.19.24
Device topology
- entity 1: stm32_dcmi (1 pad, 1 link)
type Node subtype V4L flags 1
device node name /dev/video0
pad0: Sink
<- "ov5640 0-003c":0 [ENABLED,IMMUTABLE]
- entity 5: ov5640 0-003c (1 pad, 1 link)
type V4L2 subdev subtype Sensor flags 0
device node name /dev/v4l-subdev0
pad0: Source
[fmt:JPEG_1X8/320x240 at 1/30 field:none colorspace:jpeg
xfer:srgb ycbcr:601 quantization:full-range]
-> "stm32_dcmi":0 [ENABLED,IMMUTABLE]
>>>
>>>> diff --git a/src/libcamera/pipeline/stm32/stm32.cpp b/src/libcamera/pipeline/stm32/stm32.cpp
>>>> new file mode 100644
>>>> index 0000000..301fdfc
>>>> --- /dev/null
>>>> +++ b/src/libcamera/pipeline/stm32/stm32.cpp
>>>> @@ -0,0 +1,205 @@
>>>> +/* 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();
>>> Our coding style uses a tab to indent.
>>>
>>> When you have committed your code, you can run ./utils/checkstyle.py to
>>> check the most recent commit (or you can specify a range suitable for git).
>>>
>>> This requires installing clang-format ideally, although astyle is also
>>> supported to some degree.
I have fix clang-format package version on my side, it will be better in v2.
>>>
>>>> +
>>>> + 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>
>>>> +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");
>>> I see that the stm32-dcmi driver calls down to a subdevice. Is there
>>> ever a need to interact with this subdevice directly?
>>>
>>> Or are all controls handled through the DCMI driver?
>>>
>>> I think it looks like everything simply goes through the stm32-dcmi
>>> V4L2Device interface.
Until today yes but Hugues is working with v4l for media-controller
awareness.
>>>
>>> That said, I can't see if there is any media-controller device in the
>>> DCMI driver. Our DeviceMatch currently only looks at media-controller
>>> enabled devices.
>>>
>>>> +
>>>> + media_ = enumerator->search(dm);
>>>> + if (!media_)
>>>> + return false;
>>>> +
>>>> + media_->acquire();
>>>> +
>>>> + std::unique_ptr<STM32CameraData> data =
>>>> + utils::make_unique<STM32CameraData>(this);
>>>> +
>>>> + /* Locate and open the default video node. */
>>>> + for (MediaEntity *entity : media_->entities()) {
>>>> + if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
>>>> + data->video_ = new V4L2Device(entity);
>>>> + break;
>>>> + }
>>>> + }
>>> I think the MEDIA_ENT_FL_DEFAULT is currently quite specific to the
>>> UVCVideo (and omap3isp) pipelines, so I'm not sure if this will
>>> correctly match your device?
It will because it is in Hugues's patches.
Does an another way of working is posssible ?
Maybe we have misunderstood the mean of MEDIA_ENT_FL_DEFAULT and how to
use it.
Do you have some advices ?
Benjamin
>>>
>>>> +
>>>> + 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 */
>>>>
>>> Thank you for joining the libcamera project! - If there is anything more
>>> we can do to help you - please let us know.
More information about the libcamera-devel
mailing list