[21/35] libcamera: software_isp: gbm: Add in a GBM helper class for GPU surface access

23527 diff mbox series

A helper class to interact with GBM. This will allow us to specify the
internal storage format of the CPU when making a texture for the Debayer
vertext/fragment shaders and thus ensure we receive an uncompressed and
untiled output buffer.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 include/libcamera/internal/gbm.h       |  55 ++++++++++
 include/libcamera/internal/meson.build |   1 +
 src/libcamera/gbm.cpp                  | 137 +++++++++++++++++++++++++
 src/libcamera/meson.build              |  11 ++
 4 files changed, 204 insertions(+)
 create mode 100644 include/libcamera/internal/gbm.h
 create mode 100644 src/libcamera/gbm.cpp

On 11/06/2025 02:32, Bryan O'Donoghue wrote:
> A helper class to interact with GBM. This will allow us to specify the
> internal storage format of the CPU when making a texture for the Debayer
> vertext/fragment shaders and thus ensure we receive an uncompressed and
> untiled output buffer.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   include/libcamera/internal/gbm.h       |  55 ++++++++++
>   include/libcamera/internal/meson.build |   1 +
>   src/libcamera/gbm.cpp                  | 137 +++++++++++++++++++++++++
>   src/libcamera/meson.build              |  11 ++
>   4 files changed, 204 insertions(+)
>   create mode 100644 include/libcamera/internal/gbm.h
>   create mode 100644 src/libcamera/gbm.cpp
> 
> diff --git a/include/libcamera/internal/gbm.h b/include/libcamera/internal/gbm.h
> new file mode 100644
> index 00000000..a5486cc9
> --- /dev/null
> +++ b/include/libcamera/internal/gbm.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Linaro Ltd.
> + *
> + * Authors:
> + * Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> + *
> + * gbm.h - Helper class for managing GBM interactions.
> + */
> +
> +#pragma once
> +
> +#include <gbm.h>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/formats.h>
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(GBM)
> +
> +class GBM
> +{
> +public:
> +	GBM();
> +	~GBM();
> +
> +	int initSurface(uint32_t width, uint32_t height);
> +	int mapSurface();
> +	int getFrameBufferData(uint8_t *data_out, size_t data_len);
> +	struct gbm_device *getDevice() { return gbm_device_; }
> +	struct gbm_surface *getSurface() { return gbm_surface_; }
> +	uint32_t getFrameSize() { return framesize_; }
> +	uint32_t getStride() { return stride_; }
> +	PixelFormat getPixelFormat() { return format_; }
> +
> +private:
> +	int fd_;
> +	struct gbm_device *gbm_device_;
> +	struct gbm_surface *gbm_surface_;
> +
> +	struct gbm_bo *gbm_bo_;
> +	uint32_t width_;
> +	uint32_t height_;
> +	uint32_t stride_;
> +	uint32_t offset_;
> +	uint32_t framesize_;
> +	void *map_;
> +	int bo_fd_;
> +
> +	PixelFormat format_;
> +};
> +
> +} // namespace libcamera
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index c0b593bf..4a2919f6 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -23,6 +23,7 @@ libcamera_internal_headers = files([
>       'dma_buf_allocator.h',
>       'formats.h',
>       'framebuffer.h',
> +    'gbm.h',
>       'ipa_data_serializer.h',
>       'ipa_manager.h',
>       'ipa_module.h',
> diff --git a/src/libcamera/gbm.cpp b/src/libcamera/gbm.cpp
> new file mode 100644
> index 00000000..43032093
> --- /dev/null
> +++ b/src/libcamera/gbm.cpp
> @@ -0,0 +1,137 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Linaro Ltd.
> + *
> + * Authors:
> + * Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> + *
> + * egl.cpp - Helper class for managing GBM interactions.
> + */
> +
> +#include "libcamera/internal/gbm.h"
> +
> +#include <fcntl.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(GBM)
> +
> +GBM::GBM()
> +{
> +	fd_ = 0;
> +}
> +
> +GBM::~GBM()
> +{
> +	if (gbm_surface_)
> +		gbm_surface_destroy(gbm_surface_);
> +
> +	if (gbm_device_)
> +		gbm_device_destroy(gbm_device_);
> +
> +	if (fd_ >= 0)
> +		close(fd_);
> +}
> +
> +// this should probably go into its own class to deal with the
> +// allocation and deletion of frambuffers attached to GBM devices/objects
> +int GBM::initSurface(uint32_t width, uint32_t height)
> +{
> +	const char *dri_node = "/dev/dri/renderD128"; //TODO: get from an env or config setting
> +
> +	fd_ = open(dri_node, O_RDWR | O_CLOEXEC); //TODO: CLOEXEC ?
> +	if (fd_ < 0) {
> +		LOG(GBM, Error) << "Open " << dri_node << " fail " << fd_;
> +		return fd_;
> +	}
> +
> +	gbm_device_ = gbm_create_device(fd_);
> +	if (!gbm_device_) {
> +		LOG(GBM, Error) << "gbm_crate_device fail";
> +		goto fail;
> +	}
> +
> +	// GBM_FORMAT_RGBA8888 is not supported mesa::src/gbm/dri/gbm_dri.c::gbm_dri_visuals_table[]
> +	// This means we need to choose XRGB8888 or ARGB8888 as the raw buffer format
> +	gbm_surface_ = gbm_surface_create(gbm_device_, width, height, GBM_FORMAT_ARGB8888,
> +					  GBM_BO_USE_RENDERING | GBM_BO_USE_LINEAR);
TODO:

Pass BPP and select between GBM_FORMAT_ARGB8888 and GBM_FORMAT_RGB8888.

Do this because the surface the shader uses - i.e. the number of pixels 
the shader writes matters.

Once BPP is passed into the above, we can also amend the shaders with 
some kind of passed in environment define to allow differentation between.

gl_FragColor = rgba

and

gl_FragColor = rgb

Thus allowing feature parity with CPUISP wrt supported output pixel formats.

---
bod

Hi Bryan,

due to lack of expertise, I cannot comment on the factual correctness;
and I don't see any obvious flaw, just commenting on some style issues.

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> A helper class to interact with GBM. This will allow us to specify the
> internal storage format of the CPU when making a texture for the Debayer
> vertext/fragment shaders and thus ensure we receive an uncompressed and
> untiled output buffer.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  include/libcamera/internal/gbm.h       |  55 ++++++++++
>  include/libcamera/internal/meson.build |   1 +
>  src/libcamera/gbm.cpp                  | 137 +++++++++++++++++++++++++
>  src/libcamera/meson.build              |  11 ++
>  4 files changed, 204 insertions(+)
>  create mode 100644 include/libcamera/internal/gbm.h
>  create mode 100644 src/libcamera/gbm.cpp
>
> diff --git a/include/libcamera/internal/gbm.h b/include/libcamera/internal/gbm.h
> new file mode 100644
> index 00000000..a5486cc9
> --- /dev/null
> +++ b/include/libcamera/internal/gbm.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Linaro Ltd.
> + *
> + * Authors:
> + * Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> + *
> + * gbm.h - Helper class for managing GBM interactions.

No file name and no final dot:

  * Helper class for managing GBM interactions

> + */
> +
> +#pragma once
> +
> +#include <gbm.h>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/formats.h>
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(GBM)
> +
> +class GBM
> +{
> +public:
> +	GBM();
> +	~GBM();
> +
> +	int initSurface(uint32_t width, uint32_t height);
> +	int mapSurface();
> +	int getFrameBufferData(uint8_t *data_out, size_t data_len);

libcamera uses camelCase :-( ; this applies also to the identifiers
below.

> +	struct gbm_device *getDevice() { return gbm_device_; }
> +	struct gbm_surface *getSurface() { return gbm_surface_; }
> +	uint32_t getFrameSize() { return framesize_; }
> +	uint32_t getStride() { return stride_; }
> +	PixelFormat getPixelFormat() { return format_; }
> +
> +private:
> +	int fd_;
> +	struct gbm_device *gbm_device_;
> +	struct gbm_surface *gbm_surface_;
> +
> +	struct gbm_bo *gbm_bo_;
> +	uint32_t width_;
> +	uint32_t height_;
> +	uint32_t stride_;
> +	uint32_t offset_;
> +	uint32_t framesize_;
> +	void *map_;
> +	int bo_fd_;
> +
> +	PixelFormat format_;
> +};
> +
> +} // namespace libcamera

Should be

  } /* namespace libcamera */

> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index c0b593bf..4a2919f6 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -23,6 +23,7 @@ libcamera_internal_headers = files([
>      'dma_buf_allocator.h',
>      'formats.h',
>      'framebuffer.h',
> +    'gbm.h',
>      'ipa_data_serializer.h',
>      'ipa_manager.h',
>      'ipa_module.h',
> diff --git a/src/libcamera/gbm.cpp b/src/libcamera/gbm.cpp
> new file mode 100644
> index 00000000..43032093
> --- /dev/null
> +++ b/src/libcamera/gbm.cpp
> @@ -0,0 +1,137 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Linaro Ltd.
> + *
> + * Authors:
> + * Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> + *
> + * egl.cpp - Helper class for managing GBM interactions.

No file name and no final dot.

> + */
> +
> +#include "libcamera/internal/gbm.h"
> +
> +#include <fcntl.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(GBM)
> +
> +GBM::GBM()
> +{
> +	fd_ = 0;
> +}
> +
> +GBM::~GBM()
> +{
> +	if (gbm_surface_)
> +		gbm_surface_destroy(gbm_surface_);
> +
> +	if (gbm_device_)
> +		gbm_device_destroy(gbm_device_);
> +
> +	if (fd_ >= 0)
> +		close(fd_);
> +}
> +
> +// this should probably go into its own class to deal with the
> +// allocation and deletion of frambuffers attached to GBM devices/objects
> +int GBM::initSurface(uint32_t width, uint32_t height)
> +{
> +	const char *dri_node = "/dev/dri/renderD128"; //TODO: get from an env or config setting
> +
> +	fd_ = open(dri_node, O_RDWR | O_CLOEXEC); //TODO: CLOEXEC ?
> +	if (fd_ < 0) {
> +		LOG(GBM, Error) << "Open " << dri_node << " fail " << fd_;
> +		return fd_;
> +	}
> +
> +	gbm_device_ = gbm_create_device(fd_);
> +	if (!gbm_device_) {
> +		LOG(GBM, Error) << "gbm_crate_device fail";

s/crate/create/

> +		goto fail;
> +	}
> +
> +	// GBM_FORMAT_RGBA8888 is not supported mesa::src/gbm/dri/gbm_dri.c::gbm_dri_visuals_table[]
> +	// This means we need to choose XRGB8888 or ARGB8888 as the raw buffer format

/* ... */ comment style.

> +	gbm_surface_ = gbm_surface_create(gbm_device_, width, height, GBM_FORMAT_ARGB8888,
> +					  GBM_BO_USE_RENDERING | GBM_BO_USE_LINEAR);
> +	if (!gbm_surface_) {
> +		LOG(GBM, Error) << "Unable to create linear gbm surface";
> +		goto fail;
> +	}
> +
> +	format_ = libcamera::formats::ARGB8888;
> +
> +	return 0;
> +fail:
> +	return -ENODEV;
> +}
> +
> +int GBM::mapSurface()
> +{
> +	gbm_bo_ = gbm_surface_lock_front_buffer(gbm_surface_);
> +	if (!gbm_bo_) {
> +		LOG(GBM, Error) << "GBM input buffer object create fail";
> +		return -ENODEV;
> +	}
> +	gbm_surface_release_buffer(gbm_surface_, gbm_bo_);
> +
> +	bo_fd_ = gbm_bo_get_fd(gbm_bo_);
> +
> +	if (!bo_fd_) {
> +		gbm_surface_release_buffer(gbm_surface_, gbm_bo_);
> +		LOG(GBM, Error) << "Unable to get fd for bo: " << bo_fd_;
> +		return -ENODEV;
> +	}
> +
> +	stride_ = gbm_bo_get_stride(gbm_bo_);
> +	width_ = gbm_bo_get_width(gbm_bo_);
> +	height_ = gbm_bo_get_height(gbm_bo_);
> +	offset_ = gbm_bo_get_offset(gbm_bo_, 0);
> +	framesize_ = height_ * stride_;
> +
> +	map_ = mmap(NULL, height_ * stride_, PROT_READ, MAP_SHARED, bo_fd_, 0);
> +	if (map_ == MAP_FAILED) {
> +		LOG(GBM, Error) << "mmap gbm_bo_ fail";
> +		return -ENODEV;
> +	}
> +
> +	LOG(GBM, Debug) << " stride " << stride_
> +			<< " width " << width_
> +			<< " height " << height_
> +			<< " offset " << offset_
> +			<< " framesize " << framesize_;
> +
> +	return 0;
> +}
> +
> +int GBM::getFrameBufferData(uint8_t *data, size_t data_len)
> +{
> +	struct dma_buf_sync sync;
> +
> +	gbm_surface_lock_front_buffer(gbm_surface_);
> +
> +	sync.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_READ;
> +	ioctl(bo_fd_, DMA_BUF_IOCTL_SYNC, &sync);
> +
> +	if (data_len > framesize_) {
> +		LOG(GBM, Error) << "Invalid read size " << data_len << " max is " << framesize_;
> +		return -EINVAL;
> +	}
> +
> +	memcpy(data, map_, data_len);
> +
> +	sync.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_READ;
> +	ioctl(bo_fd_, DMA_BUF_IOCTL_SYNC, &sync);

A comment/todo should be added about the slow stuff on some archs
(memcpy, ioctl's).

> +
> +	gbm_surface_release_buffer(gbm_surface_, gbm_bo_);
> +
> +	return 0;
> +}
> +} //namespace libcamera

  } /* namespace libcamera */

> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 202db1ef..0d004694 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -67,6 +67,16 @@ libcamera_deps = []
>  libatomic = cc.find_library('atomic', required : false)
>  libthreads = dependency('threads')
>  
> +libgbm = cc.find_library('gbm', required: false)
> +gbm_works = cc.check_header('gbm.h', required: false) 

An extra space add the end of the line.

> +
> +if libgbm.found() and gbm_works
> +    config_h.set('HAVE_GBM', 1)
> +    libcamera_internal_sources += files([
> +        'gbm.cpp',
> +    ])
> +endif
> +
>  subdir('base')
>  subdir('converter')
>  subdir('ipa')
> @@ -188,6 +198,7 @@ libcamera_deps += [
>      libcamera_base_private,
>      libcrypto,
>      libdl,
> +    libgbm,
>      liblttng,
>      libudev,
>      libyaml,

Hi

2025. 06. 11. 3:32 keltezéssel, Bryan O'Donoghue írta:
> A helper class to interact with GBM. This will allow us to specify the
> internal storage format of the CPU when making a texture for the Debayer
> vertext/fragment shaders and thus ensure we receive an uncompressed and
> untiled output buffer.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   include/libcamera/internal/gbm.h       |  55 ++++++++++
>   include/libcamera/internal/meson.build |   1 +
>   src/libcamera/gbm.cpp                  | 137 +++++++++++++++++++++++++
>   src/libcamera/meson.build              |  11 ++
>   4 files changed, 204 insertions(+)
>   create mode 100644 include/libcamera/internal/gbm.h
>   create mode 100644 src/libcamera/gbm.cpp
> 
> diff --git a/include/libcamera/internal/gbm.h b/include/libcamera/internal/gbm.h
> new file mode 100644
> index 00000000..a5486cc9
> --- /dev/null
> +++ b/include/libcamera/internal/gbm.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Linaro Ltd.
> + *
> + * Authors:
> + * Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> + *
> + * gbm.h - Helper class for managing GBM interactions.
> + */
> +
> +#pragma once
> +
> +#include <gbm.h>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/formats.h>
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(GBM)
> +
> +class GBM
> +{
> +public:
> +	GBM();
> +	~GBM();
> +
> +	int initSurface(uint32_t width, uint32_t height);

`Size` type from geometry.h ?


> +	int mapSurface();
> +	int getFrameBufferData(uint8_t *data_out, size_t data_len);
> +	struct gbm_device *getDevice() { return gbm_device_; }
> +	struct gbm_surface *getSurface() { return gbm_surface_; }
> +	uint32_t getFrameSize() { return framesize_; }
> +	uint32_t getStride() { return stride_; }
> +	PixelFormat getPixelFormat() { return format_; }
> +
> +private:
> +	int fd_;

UniqueFD fd_;

> +	struct gbm_device *gbm_device_;
> +	struct gbm_surface *gbm_surface_;
> +
> +	struct gbm_bo *gbm_bo_;

   = nullptr; for the three members above.

> +	uint32_t width_;
> +	uint32_t height_;
> +	uint32_t stride_;
> +	uint32_t offset_;
> +	uint32_t framesize_;
> +	void *map_;
> +	int bo_fd_;
> +
> +	PixelFormat format_;
> +};

Copy ctor/assignment should be deleted; move ctor/assignment should be deleted,
or implemented if necessary.


> +
> +} // namespace libcamera
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index c0b593bf..4a2919f6 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -23,6 +23,7 @@ libcamera_internal_headers = files([
>       'dma_buf_allocator.h',
>       'formats.h',
>       'framebuffer.h',
> +    'gbm.h',
>       'ipa_data_serializer.h',
>       'ipa_manager.h',
>       'ipa_module.h',
> diff --git a/src/libcamera/gbm.cpp b/src/libcamera/gbm.cpp
> new file mode 100644
> index 00000000..43032093
> --- /dev/null
> +++ b/src/libcamera/gbm.cpp
> @@ -0,0 +1,137 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Linaro Ltd.
> + *
> + * Authors:
> + * Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> + *
> + * egl.cpp - Helper class for managing GBM interactions.
> + */
> +
> +#include "libcamera/internal/gbm.h"
> +
> +#include <fcntl.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(GBM)
> +
> +GBM::GBM()
> +{
> +	fd_ = 0;

Not needed if `fd_` is `UniqueFD`. Just remove the constructor or ` = default;`.


> +}
> +
> +GBM::~GBM()
> +{

Should `map_` be unmapped?


> +	if (gbm_surface_)
> +		gbm_surface_destroy(gbm_surface_);
> +
> +	if (gbm_device_)
> +		gbm_device_destroy(gbm_device_);
> +
> +	if (fd_ >= 0)
> +		close(fd_);
> +}
> +
> +// this should probably go into its own class to deal with the
> +// allocation and deletion of frambuffers attached to GBM devices/objects
> +int GBM::initSurface(uint32_t width, uint32_t height)

Multiple calls leak `gbm_device_` and `gbm_surface_`. Either
return an error/assert if these are already set, or destroy
them in case of an error.


> +{
> +	const char *dri_node = "/dev/dri/renderD128"; //TODO: get from an env or config setting
> +
> +	fd_ = open(dri_node, O_RDWR | O_CLOEXEC); //TODO: CLOEXEC ?
> +	if (fd_ < 0) {
> +		LOG(GBM, Error) << "Open " << dri_node << " fail " << fd_;
> +		return fd_;
> +	}
> +
> +	gbm_device_ = gbm_create_device(fd_);
> +	if (!gbm_device_) {
> +		LOG(GBM, Error) << "gbm_crate_device fail";
> +		goto fail;
> +	}
> +
> +	// GBM_FORMAT_RGBA8888 is not supported mesa::src/gbm/dri/gbm_dri.c::gbm_dri_visuals_table[]
> +	// This means we need to choose XRGB8888 or ARGB8888 as the raw buffer format
> +	gbm_surface_ = gbm_surface_create(gbm_device_, width, height, GBM_FORMAT_ARGB8888,
> +					  GBM_BO_USE_RENDERING | GBM_BO_USE_LINEAR);
> +	if (!gbm_surface_) {
> +		LOG(GBM, Error) << "Unable to create linear gbm surface";
> +		goto fail;
> +	}> +
> +	format_ = libcamera::formats::ARGB8888;
> +
> +	return 0;
> +fail:
> +	return -ENODEV;
> +}
> +
> +int GBM::mapSurface()
> +{

ASSERT(gbm_surface_); // or similar


> +	gbm_bo_ = gbm_surface_lock_front_buffer(gbm_surface_);
> +	if (!gbm_bo_) {
> +		LOG(GBM, Error) << "GBM input buffer object create fail";
> +		return -ENODEV;
> +	}
> +	gbm_surface_release_buffer(gbm_surface_, gbm_bo_);
> +
> +	bo_fd_ = gbm_bo_get_fd(gbm_bo_);
> +
> +	if (!bo_fd_) {
> +		gbm_surface_release_buffer(gbm_surface_, gbm_bo_);

Is the above call needed? It was already returned to the surface, no?


> +		LOG(GBM, Error) << "Unable to get fd for bo: " << bo_fd_;
> +		return -ENODEV;
> +	}
> +
> +	stride_ = gbm_bo_get_stride(gbm_bo_);
> +	width_ = gbm_bo_get_width(gbm_bo_);
> +	height_ = gbm_bo_get_height(gbm_bo_);
> +	offset_ = gbm_bo_get_offset(gbm_bo_, 0);
> +	framesize_ = height_ * stride_;
> +
> +	map_ = mmap(NULL, height_ * stride_, PROT_READ, MAP_SHARED, bo_fd_, 0);

nullptr


> +	if (map_ == MAP_FAILED) {
> +		LOG(GBM, Error) << "mmap gbm_bo_ fail";
> +		return -ENODEV;
> +	}
> +
> +	LOG(GBM, Debug) << " stride " << stride_
> +			<< " width " << width_
> +			<< " height " << height_
> +			<< " offset " << offset_
> +			<< " framesize " << framesize_;
> +
> +	return 0;
> +}
> +
> +int GBM::getFrameBufferData(uint8_t *data, size_t data_len)

`void *data` ?


> +{

ASSERT(gbm_surface_); // or similar?


> +	struct dma_buf_sync sync;
> +
> +	gbm_surface_lock_front_buffer(gbm_surface_);
> +
> +	sync.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_READ;
> +	ioctl(bo_fd_, DMA_BUF_IOCTL_SYNC, &sync);
> +
> +	if (data_len > framesize_) {
> +		LOG(GBM, Error) << "Invalid read size " << data_len << " max is " << framesize_;
> +		return -EINVAL;

This does not finish the sync, and keeps the front buffer locked.
Are those issues? Could this be moved to the top?


> +	}
> +
> +	memcpy(data, map_, data_len);
> +
> +	sync.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_READ;
> +	ioctl(bo_fd_, DMA_BUF_IOCTL_SYNC, &sync);
> +
> +	gbm_surface_release_buffer(gbm_surface_, gbm_bo_);
> +
> +	return 0;
> +}
> +} //namespace libcamera
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 202db1ef..0d004694 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -67,6 +67,16 @@ libcamera_deps = []
>   libatomic = cc.find_library('atomic', required : false)
>   libthreads = dependency('threads')
>   
> +libgbm = cc.find_library('gbm', required: false)

gbm_dep = dependency('gbm', required: false) ?


> +gbm_works = cc.check_header('gbm.h', required: false)

gbm_works = cc.check_header(..., dependency: gbm_dep) ?


Regards,
Barnabás Pőcze

> +
> +if libgbm.found() and gbm_works
> +    config_h.set('HAVE_GBM', 1)
> +    libcamera_internal_sources += files([
> +        'gbm.cpp',
> +    ])
> +endif
> +
>   subdir('base')
>   subdir('converter')
>   subdir('ipa')
> @@ -188,6 +198,7 @@ libcamera_deps += [
>       libcamera_base_private,
>       libcrypto,
>       libdl,
> +    libgbm,
>       liblttng,
>       libudev,
>       libyaml,