[libcamera-devel] [PATCH] libcamera: pipeline: stm32: add pipeline handler for stm32

[Benjamin GAIGNARD]

On 4/9/19 11:02 AM, Kieran Bingham wrote:
> Hi Benjamin,
>
> 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.

Benjamin

>
>> 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?
>
> It would be interesting to see the output of 'media-ctl -p' for your
> platform.
>
>
>> 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.
>
>> +
>> +  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.
>
> 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?
>
>
>> +
>> +  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.
>