| Message ID | 20190320163055.22056-10-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Wed, Mar 20, 2019 at 05:30:33PM +0100, Jacopo Mondi wrote: > Implement buffer allocation in IPU3 pipeline handlers. > As the pipeline handler supports a single stream, preprare two buffer s/preprare/prepare/ > pools for 'viewfinder' and 'stat' video devices, and export the 'output' > video device buffers to the Stream's pool. > > Share buffers between the CIO2 output and the ImgU input video devices, > as the output of the former should immediately be provided to the > latter for further processing. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 48 +++++++++++++++++++++++++--- > 1 file changed, 43 insertions(+), 5 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 994c95692dd4..659a7ca4829f 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -63,6 +63,9 @@ public: > V4L2Device *viewfinder; > V4L2Device *stat; > /* \todo Add param video device for 3A tuning */ > + > + BufferPool vfPool; > + BufferPool statPool; > }; > > struct CIO2Device { > @@ -81,6 +84,8 @@ struct CIO2Device { > V4L2Device *output; > V4L2Subdevice *csi2; > V4L2Subdevice *sensor; > + > + BufferPool pool; > }; > > class PipelineHandlerIPU3 : public PipelineHandler > @@ -108,6 +113,8 @@ public: > private: > static constexpr unsigned int IPU3_IMGU_COUNT = 2; > static constexpr unsigned int IPU3_BUFFER_COUNT = 4; > + static constexpr unsigned int IPU3_CIO2_BUFFER_COUNT = 4; > + static constexpr unsigned int IPU3_IMGU_BUFFER_COUNT = 4; > > class IPU3CameraData : public CameraData > { > @@ -301,16 +308,47 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera, > > int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream) > { > - const StreamConfiguration &cfg = stream->configuration(); > IPU3CameraData *data = cameraData(camera); > + V4L2Device *viewfinder = data->imgu->viewfinder; > + V4L2Device *output = data->imgu->output; > + V4L2Device *input = data->imgu->input; > V4L2Device *cio2 = data->cio2.output; > + V4L2Device *stat = data->imgu->stat; > + int ret; > > - if (!cfg.bufferCount) > - return -EINVAL; > + /* Share buffers between CIO2 output and ImgU input. */ > + data->cio2.pool.createBuffers(IPU3_CIO2_BUFFER_COUNT); > + ret = cio2->exportBuffers(&data->cio2.pool); > + if (ret) { > + LOG(IPU3, Error) << "Failed to reserve CIO2 memory"; > + return ret; > + } > + > + ret = input->importBuffers(&data->cio2.pool); > + if (ret) { > + LOG(IPU3, Error) << "Failed to import ImgU memory"; > + return ret; > + } > + > + /* Prepare the buffer pools for viewfinder and stat. */ > + data->imgu->vfPool.createBuffers(IPU3_IMGU_BUFFER_COUNT); Don't you need the same number of buffers for viewfinder and stat than for output ? > + ret = viewfinder->exportBuffers(&data->imgu->vfPool); > + if (ret) { > + LOG(IPU3, Error) << "Failed to reserve ImgU viewfinder memory"; > + return ret; > + } > + > + data->imgu->statPool.createBuffers(IPU3_IMGU_BUFFER_COUNT); > + ret = stat->exportBuffers(&data->imgu->statPool); > + if (ret) { > + LOG(IPU3, Error) << "Failed to reserve ImgU stat memory"; > + return ret; > + } > > - int ret = cio2->exportBuffers(&stream->bufferPool()); > + /* Export ImgU output buffers to the stream's pool. */ > + ret = output->exportBuffers(&stream->bufferPool()); > if (ret) { > - LOG(IPU3, Error) << "Failed to request memory"; > + LOG(IPU3, Error) << "Failed to reserve ImgU output memory"; > return ret; > } This looks good overall, I just wonder if it would make sense to move code to the CIO2Device and ImgUDevice classes (creating allocateBuffers() methods there). >
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 994c95692dd4..659a7ca4829f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -63,6 +63,9 @@ public: V4L2Device *viewfinder; V4L2Device *stat; /* \todo Add param video device for 3A tuning */ + + BufferPool vfPool; + BufferPool statPool; }; struct CIO2Device { @@ -81,6 +84,8 @@ struct CIO2Device { V4L2Device *output; V4L2Subdevice *csi2; V4L2Subdevice *sensor; + + BufferPool pool; }; class PipelineHandlerIPU3 : public PipelineHandler @@ -108,6 +113,8 @@ public: private: static constexpr unsigned int IPU3_IMGU_COUNT = 2; static constexpr unsigned int IPU3_BUFFER_COUNT = 4; + static constexpr unsigned int IPU3_CIO2_BUFFER_COUNT = 4; + static constexpr unsigned int IPU3_IMGU_BUFFER_COUNT = 4; class IPU3CameraData : public CameraData { @@ -301,16 +308,47 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera, int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream) { - const StreamConfiguration &cfg = stream->configuration(); IPU3CameraData *data = cameraData(camera); + V4L2Device *viewfinder = data->imgu->viewfinder; + V4L2Device *output = data->imgu->output; + V4L2Device *input = data->imgu->input; V4L2Device *cio2 = data->cio2.output; + V4L2Device *stat = data->imgu->stat; + int ret; - if (!cfg.bufferCount) - return -EINVAL; + /* Share buffers between CIO2 output and ImgU input. */ + data->cio2.pool.createBuffers(IPU3_CIO2_BUFFER_COUNT); + ret = cio2->exportBuffers(&data->cio2.pool); + if (ret) { + LOG(IPU3, Error) << "Failed to reserve CIO2 memory"; + return ret; + } + + ret = input->importBuffers(&data->cio2.pool); + if (ret) { + LOG(IPU3, Error) << "Failed to import ImgU memory"; + return ret; + } + + /* Prepare the buffer pools for viewfinder and stat. */ + data->imgu->vfPool.createBuffers(IPU3_IMGU_BUFFER_COUNT); + ret = viewfinder->exportBuffers(&data->imgu->vfPool); + if (ret) { + LOG(IPU3, Error) << "Failed to reserve ImgU viewfinder memory"; + return ret; + } + + data->imgu->statPool.createBuffers(IPU3_IMGU_BUFFER_COUNT); + ret = stat->exportBuffers(&data->imgu->statPool); + if (ret) { + LOG(IPU3, Error) << "Failed to reserve ImgU stat memory"; + return ret; + } - int ret = cio2->exportBuffers(&stream->bufferPool()); + /* Export ImgU output buffers to the stream's pool. */ + ret = output->exportBuffers(&stream->bufferPool()); if (ret) { - LOG(IPU3, Error) << "Failed to request memory"; + LOG(IPU3, Error) << "Failed to reserve ImgU output memory"; return ret; }
Implement buffer allocation in IPU3 pipeline handlers. As the pipeline handler supports a single stream, preprare two buffer pools for 'viewfinder' and 'stat' video devices, and export the 'output' video device buffers to the Stream's pool. Share buffers between the CIO2 output and the ImgU input video devices, as the output of the former should immediately be provided to the latter for further processing. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 48 +++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 5 deletions(-)