[libcamera-devel] [PATCH v2 03/25] ipa: Switch to FrameBuffer interface
Niklas Söderlund
niklas.soderlund at ragnatech.se
Mon Dec 30 13:04:48 CET 2019
Switch the IPA interfaces and implementations to use the Framebuffer
interface.
- The IPA interface is switch to use the simpler FrameBuffer::Plane
container when carrying dmabuf descriptions (fd and length) over the
pipeline/IPA boundary.
- The RkISP1 IPA implementation takes advantage of the new simpler and
safer (better control over file descriptors) FrameBuffer interface.
The biggest change can be observed in the test case where it is
demonstrated that FrameBuffer objects only carry the necessary number of
planes and not a fixed number. As a consequence of this the test needs
to verify the file descriptors it injects and not depend on the library
to handle uninitialized fields.
Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
---
include/ipa/ipa_interface.h | 2 +-
src/ipa/libipa/ipa_interface_wrapper.cpp | 9 ++----
src/ipa/rkisp1/rkisp1.cpp | 30 +++++++++++++----
src/libcamera/ipa_context_wrapper.cpp | 8 ++---
src/libcamera/ipa_interface.cpp | 7 ++--
src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++++--
test/ipa/ipa_wrappers_test.cpp | 41 +++++++++++++-----------
7 files changed, 66 insertions(+), 43 deletions(-)
diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
index 0ea53d120fe10acf..229d1124b19ec1c9 100644
--- a/include/ipa/ipa_interface.h
+++ b/include/ipa/ipa_interface.h
@@ -105,7 +105,7 @@ struct IPAStream {
struct IPABuffer {
unsigned int id;
- BufferMemory memory;
+ std::vector<FrameBuffer::Plane> planes;
};
struct IPAOperationData {
diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
index 6a389dfa714ab535..3628a785dc3e63f4 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.cpp
+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
@@ -144,17 +144,14 @@ void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,
for (unsigned int i = 0; i < num_buffers; ++i) {
const struct ipa_buffer &_buffer = _buffers[i];
IPABuffer &buffer = buffers[i];
- std::vector<Plane> &planes = buffer.memory.planes();
+ std::vector<FrameBuffer::Plane> &planes = buffer.planes;
buffer.id = _buffer.id;
planes.resize(_buffer.num_planes);
for (unsigned int j = 0; j < _buffer.num_planes; ++j) {
- if (_buffer.planes[j].dmabuf != -1)
- planes[j].setDmabuf(_buffer.planes[j].dmabuf,
- _buffer.planes[j].length);
- /** \todo Create a Dmabuf class to implement RAII. */
- ::close(_buffer.planes[j].dmabuf);
+ planes[j].fd = FileDescriptor(_buffer.planes[j].dmabuf);
+ planes[j].length = _buffer.planes[j].length;
}
}
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 744a16ae5b9aa420..63776563b149f7ed 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -10,6 +10,7 @@
#include <queue>
#include <stdint.h>
#include <string.h>
+#include <sys/mman.h>
#include <linux/rkisp1-config.h>
@@ -48,7 +49,8 @@ private:
void setControls(unsigned int frame);
void metadataReady(unsigned int frame, unsigned int aeState);
- std::map<unsigned int, BufferMemory> bufferInfo_;
+ std::map<unsigned int, FrameBuffer> buffers_;
+ std::map<unsigned int, void *> buffersMemory_;
ControlInfoMap ctrls_;
@@ -102,15 +104,29 @@ void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
void IPARkISP1::mapBuffers(const std::vector<IPABuffer> &buffers)
{
for (const IPABuffer &buffer : buffers) {
- bufferInfo_[buffer.id] = buffer.memory;
- bufferInfo_[buffer.id].planes()[0].mem();
+ buffers_.emplace(buffer.id, FrameBuffer(buffer.planes));
+ buffersMemory_[buffer.id] = mmap(NULL,
+ buffers_[buffer.id].planes()[0].length,
+ PROT_READ | PROT_WRITE,
+ MAP_SHARED,
+ buffers_[buffer.id].planes()[0].fd.fd(),
+ 0);
+
+ if (buffersMemory_[buffer.id] == MAP_FAILED) {
+ int ret = -errno;
+ LOG(IPARkISP1, Fatal) << "Failed to mmap buffer: "
+ << strerror(-ret);
+ }
}
}
void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
{
- for (unsigned int id : ids)
- bufferInfo_.erase(id);
+ for (unsigned int id : ids) {
+ munmap(buffersMemory_[id], buffers_[id].planes()[0].length);
+ buffersMemory_.erase(id);
+ buffers_.erase(id);
+ }
}
void IPARkISP1::processEvent(const IPAOperationData &event)
@@ -121,7 +137,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event)
unsigned int bufferId = event.data[1];
const rkisp1_stat_buffer *stats =
- static_cast<rkisp1_stat_buffer *>(bufferInfo_[bufferId].planes()[0].mem());
+ static_cast<rkisp1_stat_buffer *>(buffersMemory_[bufferId]);
updateStatistics(frame, stats);
break;
@@ -131,7 +147,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event)
unsigned int bufferId = event.data[1];
rkisp1_isp_params_cfg *params =
- static_cast<rkisp1_isp_params_cfg *>(bufferInfo_[bufferId].planes()[0].mem());
+ static_cast<rkisp1_isp_params_cfg *>(buffersMemory_[bufferId]);
queueRequest(frame, params, event.controls[0]);
break;
diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
index 9603fdac87150aa0..946a2fd8cab1782a 100644
--- a/src/libcamera/ipa_context_wrapper.cpp
+++ b/src/libcamera/ipa_context_wrapper.cpp
@@ -149,15 +149,15 @@ void IPAContextWrapper::mapBuffers(const std::vector<IPABuffer> &buffers)
for (unsigned int i = 0; i < buffers.size(); ++i) {
struct ipa_buffer &c_buffer = c_buffers[i];
const IPABuffer &buffer = buffers[i];
- const std::vector<Plane> &planes = buffer.memory.planes();
+ const std::vector<FrameBuffer::Plane> &planes = buffer.planes;
c_buffer.id = buffer.id;
c_buffer.num_planes = planes.size();
for (unsigned int j = 0; j < planes.size(); ++j) {
- const Plane &plane = planes[j];
- c_buffer.planes[j].dmabuf = plane.dmabuf();
- c_buffer.planes[j].length = plane.length();
+ const FrameBuffer::Plane &plane = planes[j];
+ c_buffer.planes[j].dmabuf = plane.fd.fd();
+ c_buffer.planes[j].length = plane.length;
}
}
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index ee3e3622f39ae85f..2f86087ee4aa414f 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -334,11 +334,10 @@ namespace libcamera {
*/
/**
- * \var IPABuffer::memory
- * \brief The buffer memory description
+ * \var IPABuffer::planes
+ * \brief The buffer planes description
*
- * The memory field stores the dmabuf handle and size for each plane of the
- * buffer.
+ * Stores the dmabuf handle and length for each plane of the buffer.
*/
/**
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 7e41222e3d01a475..d7ee95ded0f76027 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -687,14 +687,22 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
}
for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {
+ FrameBuffer::Plane plane;
+ plane.fd = FileDescriptor(paramPool_.buffers()[i].planes()[0].dmabuf());
+ plane.length = paramPool_.buffers()[i].planes()[0].length();
+
data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i,
- .memory = paramPool_.buffers()[i] });
+ .planes = { plane } });
paramBuffers_.push(new Buffer(i));
}
for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {
+ FrameBuffer::Plane plane;
+ plane.fd = FileDescriptor(statPool_.buffers()[i].planes()[0].dmabuf());
+ plane.length = statPool_.buffers()[i].planes()[0].length();
+
data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i,
- .memory = statPool_.buffers()[i] });
+ .planes = { plane } });
statBuffers_.push(new Buffer(i));
}
diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
index a1e34ad52317f47b..e711e4fe318d8179 100644
--- a/test/ipa/ipa_wrappers_test.cpp
+++ b/test/ipa/ipa_wrappers_test.cpp
@@ -115,28 +115,28 @@ public:
return report(Op_mapBuffers, TestFail);
}
- if (buffers[0].memory.planes().size() != 3 ||
- buffers[1].memory.planes().size() != 3) {
+ if (buffers[0].planes.size() != 3 ||
+ buffers[1].planes.size() != 3) {
cerr << "mapBuffers(): Invalid number of planes" << endl;
return report(Op_mapBuffers, TestFail);
}
- if (buffers[0].memory.planes()[0].length() != 4096 ||
- buffers[0].memory.planes()[1].length() != 0 ||
- buffers[0].memory.planes()[2].length() != 0 ||
- buffers[0].memory.planes()[0].length() != 4096 ||
- buffers[1].memory.planes()[1].length() != 4096 ||
- buffers[1].memory.planes()[2].length() != 0) {
+ if (buffers[0].planes[0].length != 4096 ||
+ buffers[0].planes[1].length != 0 ||
+ buffers[0].planes[2].length != 0 ||
+ buffers[0].planes[0].length != 4096 ||
+ buffers[1].planes[1].length != 4096 ||
+ buffers[1].planes[2].length != 0) {
cerr << "mapBuffers(): Invalid length" << endl;
return report(Op_mapBuffers, TestFail);
}
- if (buffers[0].memory.planes()[0].dmabuf() == -1 ||
- buffers[0].memory.planes()[1].dmabuf() != -1 ||
- buffers[0].memory.planes()[2].dmabuf() != -1 ||
- buffers[0].memory.planes()[0].dmabuf() == -1 ||
- buffers[1].memory.planes()[1].dmabuf() == -1 ||
- buffers[1].memory.planes()[2].dmabuf() != -1) {
+ if (buffers[0].planes[0].fd.fd() == -1 ||
+ buffers[0].planes[1].fd.fd() != -1 ||
+ buffers[0].planes[2].fd.fd() != -1 ||
+ buffers[0].planes[0].fd.fd() == -1 ||
+ buffers[1].planes[1].fd.fd() == -1 ||
+ buffers[1].planes[2].fd.fd() != -1) {
cerr << "mapBuffers(): Invalid dmabuf" << endl;
return report(Op_mapBuffers, TestFail);
}
@@ -287,13 +287,16 @@ protected:
/* Test mapBuffers(). */
std::vector<IPABuffer> buffers(2);
- buffers[0].memory.planes().resize(3);
+ buffers[0].planes.resize(3);
buffers[0].id = 10;
- buffers[0].memory.planes()[0].setDmabuf(fd_, 4096);
+ buffers[0].planes[0].fd = FileDescriptor(fd_);
+ buffers[0].planes[0].length = 4096;
buffers[1].id = 11;
- buffers[1].memory.planes().resize(3);
- buffers[1].memory.planes()[0].setDmabuf(fd_, 4096);
- buffers[1].memory.planes()[1].setDmabuf(fd_, 4096);
+ buffers[1].planes.resize(3);
+ buffers[1].planes[0].fd = FileDescriptor(fd_);
+ buffers[1].planes[0].length = 4096;
+ buffers[1].planes[1].fd = FileDescriptor(fd_);
+ buffers[1].planes[1].length = 4096;
ret = INVOKE(mapBuffers, buffers);
if (ret == TestFail)
--
2.24.1
More information about the libcamera-devel
mailing list