| Message ID | 20200908074913.109244-12-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Naushir, Thanks for your patch. On 2020-09-08 08:49:11 +0100, Naushir Patuck wrote: > By using a set container, we can easily insert/remove buffer ids that have > been mmaped by the IPA. This will be required to track buffers allocated > externally and passed to the pipeline handler through a Request. > > Move the IPA buffer mapping code into a function to remove duplicated > code. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 48 +++++++++++-------- > 1 file changed, 28 insertions(+), 20 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 6aa13034..5621f182 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -10,6 +10,7 @@ > #include <mutex> > #include <queue> > #include <sys/mman.h> > +#include <unordered_set> > > #include <libcamera/camera.h> > #include <libcamera/control_ids.h> > @@ -158,8 +159,8 @@ public: > RPi::RPiDevice<Isp, 4> isp_; > /* The vector below is just for convenience when iterating over all streams. */ > std::vector<RPi::RPiStream *> streams_; > - /* Buffers passed to the IPA. */ > - std::vector<IPABuffer> ipaBuffers_; > + /* Stores the ids of the buffers mapped in the IPA. */ > + std::unordered_set<unsigned int> ipaBuffers_; > > /* DMAHEAP allocation helper. */ > RPi::DmaHeap dmaHeap_; > @@ -230,6 +231,7 @@ private: > int queueAllBuffers(Camera *camera); > int prepareBuffers(Camera *camera); > void freeBuffers(Camera *camera); > + void mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask); > > MediaDevice *unicam_; > MediaDevice *isp_; > @@ -910,36 +912,42 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) > } > > /* > - * Link the FrameBuffers with the index of their position in the vector > - * stored in the RPi stream object. > + * Pass the stats and embedded data buffers to the IPA. No other > + * buffers need to be passed. > + */ > + mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(), RPiIpaMask::STATS); > + mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), RPiIpaMask::EMBEDDED_DATA); > + > + return 0; > +} > + > +void PipelineHandlerRPi::mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask) > +{ > + RPiCameraData *data = cameraData(camera); > + std::vector<IPABuffer> ipaBuffers; > + /* > + * Link the FrameBuffers with the id (key value) in the map stored in > + * the RPi stream object - along with an identifer mask. > * > * This will allow us to identify buffers passed between the pipeline > * handler and the IPA. > */ > - for (auto const &b : data->isp_[Isp::Stats].getBuffers()) { > - data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b.first, > - .planes = b.second->planes() }); > - } > - > - for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) { > - data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b.first, > - .planes = b.second->planes() }); > + for (auto const &it : buffers) { > + ipaBuffers.push_back({ .id = mask | it.first, > + .planes = it.second->planes() }); > + data->ipaBuffers_.insert(mask | it.first); > } > > - data->ipa_->mapBuffers(data->ipaBuffers_); > - > - return 0; > + data->ipa_->mapBuffers(ipaBuffers); > } > > void PipelineHandlerRPi::freeBuffers(Camera *camera) > { > RPiCameraData *data = cameraData(camera); > > - std::vector<unsigned int> ids; > - for (IPABuffer &ipabuf : data->ipaBuffers_) > - ids.push_back(ipabuf.id); > - > - data->ipa_->unmapBuffers(ids); > + /* Copy the buffer ids from the unordered_set to a vector to pass to the IPA. */ > + std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end()); > + data->ipa_->unmapBuffers(ipaBuffers); > data->ipaBuffers_.clear(); > > for (auto const stream : data->streams_) > -- > 2.25.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Naush, On 08/09/2020 08:49, Naushir Patuck wrote: > By using a set container, we can easily insert/remove buffer ids that have > been mmaped by the IPA. This will be required to track buffers allocated > externally and passed to the pipeline handler through a Request. > > Move the IPA buffer mapping code into a function to remove duplicated > code. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 48 +++++++++++-------- > 1 file changed, 28 insertions(+), 20 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 6aa13034..5621f182 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -10,6 +10,7 @@ > #include <mutex> > #include <queue> > #include <sys/mman.h> > +#include <unordered_set> > > #include <libcamera/camera.h> > #include <libcamera/control_ids.h> > @@ -158,8 +159,8 @@ public: > RPi::RPiDevice<Isp, 4> isp_; > /* The vector below is just for convenience when iterating over all streams. */ > std::vector<RPi::RPiStream *> streams_; > - /* Buffers passed to the IPA. */ > - std::vector<IPABuffer> ipaBuffers_; > + /* Stores the ids of the buffers mapped in the IPA. */ > + std::unordered_set<unsigned int> ipaBuffers_; > > /* DMAHEAP allocation helper. */ > RPi::DmaHeap dmaHeap_; > @@ -230,6 +231,7 @@ private: > int queueAllBuffers(Camera *camera); > int prepareBuffers(Camera *camera); > void freeBuffers(Camera *camera); > + void mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask); > > MediaDevice *unicam_; > MediaDevice *isp_; > @@ -910,36 +912,42 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) > } > > /* > - * Link the FrameBuffers with the index of their position in the vector > - * stored in the RPi stream object. > + * Pass the stats and embedded data buffers to the IPA. No other > + * buffers need to be passed. > + */ > + mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(), RPiIpaMask::STATS); > + mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), RPiIpaMask::EMBEDDED_DATA); > + > + return 0; > +} > + > +void PipelineHandlerRPi::mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask) > +{ > + RPiCameraData *data = cameraData(camera); > + std::vector<IPABuffer> ipaBuffers; > + /* > + * Link the FrameBuffers with the id (key value) in the map stored in > + * the RPi stream object - along with an identifer mask. Trivial nit: s/identifer/identifier/ Hopefully that can be fixed while applying. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > * > * This will allow us to identify buffers passed between the pipeline > * handler and the IPA. > */ > - for (auto const &b : data->isp_[Isp::Stats].getBuffers()) { > - data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b.first, > - .planes = b.second->planes() }); > - } > - > - for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) { > - data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b.first, > - .planes = b.second->planes() }); > + for (auto const &it : buffers) { > + ipaBuffers.push_back({ .id = mask | it.first, > + .planes = it.second->planes() }); > + data->ipaBuffers_.insert(mask | it.first); > } > > - data->ipa_->mapBuffers(data->ipaBuffers_); > - > - return 0; > + data->ipa_->mapBuffers(ipaBuffers); > } > > void PipelineHandlerRPi::freeBuffers(Camera *camera) > { > RPiCameraData *data = cameraData(camera); > > - std::vector<unsigned int> ids; > - for (IPABuffer &ipabuf : data->ipaBuffers_) > - ids.push_back(ipabuf.id); > - > - data->ipa_->unmapBuffers(ids); > + /* Copy the buffer ids from the unordered_set to a vector to pass to the IPA. */ > + std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end()); > + data->ipa_->unmapBuffers(ipaBuffers); > data->ipaBuffers_.clear(); > > for (auto const stream : data->streams_) >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 6aa13034..5621f182 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -10,6 +10,7 @@ #include <mutex> #include <queue> #include <sys/mman.h> +#include <unordered_set> #include <libcamera/camera.h> #include <libcamera/control_ids.h> @@ -158,8 +159,8 @@ public: RPi::RPiDevice<Isp, 4> isp_; /* The vector below is just for convenience when iterating over all streams. */ std::vector<RPi::RPiStream *> streams_; - /* Buffers passed to the IPA. */ - std::vector<IPABuffer> ipaBuffers_; + /* Stores the ids of the buffers mapped in the IPA. */ + std::unordered_set<unsigned int> ipaBuffers_; /* DMAHEAP allocation helper. */ RPi::DmaHeap dmaHeap_; @@ -230,6 +231,7 @@ private: int queueAllBuffers(Camera *camera); int prepareBuffers(Camera *camera); void freeBuffers(Camera *camera); + void mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask); MediaDevice *unicam_; MediaDevice *isp_; @@ -910,36 +912,42 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) } /* - * Link the FrameBuffers with the index of their position in the vector - * stored in the RPi stream object. + * Pass the stats and embedded data buffers to the IPA. No other + * buffers need to be passed. + */ + mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(), RPiIpaMask::STATS); + mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), RPiIpaMask::EMBEDDED_DATA); + + return 0; +} + +void PipelineHandlerRPi::mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask) +{ + RPiCameraData *data = cameraData(camera); + std::vector<IPABuffer> ipaBuffers; + /* + * Link the FrameBuffers with the id (key value) in the map stored in + * the RPi stream object - along with an identifer mask. * * This will allow us to identify buffers passed between the pipeline * handler and the IPA. */ - for (auto const &b : data->isp_[Isp::Stats].getBuffers()) { - data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b.first, - .planes = b.second->planes() }); - } - - for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) { - data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b.first, - .planes = b.second->planes() }); + for (auto const &it : buffers) { + ipaBuffers.push_back({ .id = mask | it.first, + .planes = it.second->planes() }); + data->ipaBuffers_.insert(mask | it.first); } - data->ipa_->mapBuffers(data->ipaBuffers_); - - return 0; + data->ipa_->mapBuffers(ipaBuffers); } void PipelineHandlerRPi::freeBuffers(Camera *camera) { RPiCameraData *data = cameraData(camera); - std::vector<unsigned int> ids; - for (IPABuffer &ipabuf : data->ipaBuffers_) - ids.push_back(ipabuf.id); - - data->ipa_->unmapBuffers(ids); + /* Copy the buffer ids from the unordered_set to a vector to pass to the IPA. */ + std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end()); + data->ipa_->unmapBuffers(ipaBuffers); data->ipaBuffers_.clear(); for (auto const stream : data->streams_)
By using a set container, we can easily insert/remove buffer ids that have been mmaped by the IPA. This will be required to track buffers allocated externally and passed to the pipeline handler through a Request. Move the IPA buffer mapping code into a function to remove duplicated code. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- .../pipeline/raspberrypi/raspberrypi.cpp | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-)