[v7,6/8] libcamera: mali-c55: Do not rely on bufferCount
diff mbox series

Message ID 20260331-mali-cru-v7-6-4caedc898a0e@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: mali-c55: Add support for memory-to-memory
Related show

Commit Message

Jacopo Mondi March 31, 2026, 4:36 p.m. UTC 
Do not rely on bufferCount for the allocation of parameters and stats
buffers but add instead kMaliC55BufferCount which is also used to limit
the number of in-flight requests supported by the pipeline handler.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/libcamera/pipeline/mali-c55/mali-c55.cpp | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

Comments

Barnabás Pőcze April 1, 2026, 9:58 a.m. UTC | #1 
Hi

2026. 03. 31. 18:36 keltezéssel, Jacopo Mondi írta:
> Do not rely on bufferCount for the allocation of parameters and stats
> buffers but add instead kMaliC55BufferCount which is also used to limit
> the number of in-flight requests supported by the pipeline handler.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> index 566fa0549675..ce1f0b65cd6e 100644
> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> @@ -48,6 +48,8 @@
>   
>   namespace {
>   
> +static constexpr unsigned int kMaliC55BufferCount = 16;

Isn't 16 a bit too much? In any case, this change itself looks good to me.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


> +
>   bool isFormatRaw(const libcamera::PixelFormat &pixFmt)
>   {
>   	return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==
> @@ -778,7 +780,7 @@ private:
>   };
>   
>   PipelineHandlerMaliC55::PipelineHandlerMaliC55(CameraManager *manager)
> -	: PipelineHandler(manager), dsFitted_(true)
> +	: PipelineHandler(manager, kMaliC55BufferCount), dsFitted_(true)
>   {
>   }
>   
> @@ -1227,14 +1229,8 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
>   {
>   	MaliC55CameraData *data = cameraData(camera);
>   	unsigned int ipaBufferId = 1;
> -	unsigned int bufferCount;
>   	int ret;
>   
> -	bufferCount = std::max({
> -		data->frStream_.configuration().bufferCount,
> -		data->dsStream_.configuration().bufferCount,
> -	});
> -
>   	auto pushBuffers = [&](const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
>   			       std::queue<FrameBuffer *> &queue,
>   			       std::vector<IPABuffer> &ipaBuffers) {
> @@ -1249,14 +1245,14 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
>   		}
>   	};
>   
> -	ret = stats_->allocateBuffers(bufferCount, &statsBuffers_);
> +	ret = stats_->allocateBuffers(kMaliC55BufferCount, &statsBuffers_);
>   	if (ret < 0)
>   		return ret;
>   
>   	pushBuffers(statsBuffers_, availableStatsBuffers_,
>   		    data->ipaStatBuffers_);
>   
> -	ret = params_->allocateBuffers(bufferCount, &paramsBuffers_);
> +	ret = params_->allocateBuffers(kMaliC55BufferCount, &paramsBuffers_);
>   	if (ret < 0)
>   		return ret;
>   
>
Jacopo Mondi April 1, 2026, 1:01 p.m. UTC | #2 
Hi Barnabás

On Wed, Apr 01, 2026 at 11:58:57AM +0200, Barnabás Pőcze wrote:
> Hi
>
> 2026. 03. 31. 18:36 keltezéssel, Jacopo Mondi írta:
> > Do not rely on bufferCount for the allocation of parameters and stats
> > buffers but add instead kMaliC55BufferCount which is also used to limit
> > the number of in-flight requests supported by the pipeline handler.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 14 +++++---------
> >   1 file changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > index 566fa0549675..ce1f0b65cd6e 100644
> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > @@ -48,6 +48,8 @@
> >   namespace {
> > +static constexpr unsigned int kMaliC55BufferCount = 16;
>
> Isn't 16 a bit too much? In any case, this change itself looks good to me.

good question. RkISP1 has only 4 ... Should I make it 4 as well ?

>
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>
>
> > +
> >   bool isFormatRaw(const libcamera::PixelFormat &pixFmt)
> >   {
> >   	return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==
> > @@ -778,7 +780,7 @@ private:
> >   };
> >   PipelineHandlerMaliC55::PipelineHandlerMaliC55(CameraManager *manager)
> > -	: PipelineHandler(manager), dsFitted_(true)
> > +	: PipelineHandler(manager, kMaliC55BufferCount), dsFitted_(true)
> >   {
> >   }
> > @@ -1227,14 +1229,8 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
> >   {
> >   	MaliC55CameraData *data = cameraData(camera);
> >   	unsigned int ipaBufferId = 1;
> > -	unsigned int bufferCount;
> >   	int ret;
> > -	bufferCount = std::max({
> > -		data->frStream_.configuration().bufferCount,
> > -		data->dsStream_.configuration().bufferCount,
> > -	});
> > -
> >   	auto pushBuffers = [&](const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
> >   			       std::queue<FrameBuffer *> &queue,
> >   			       std::vector<IPABuffer> &ipaBuffers) {
> > @@ -1249,14 +1245,14 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
> >   		}
> >   	};
> > -	ret = stats_->allocateBuffers(bufferCount, &statsBuffers_);
> > +	ret = stats_->allocateBuffers(kMaliC55BufferCount, &statsBuffers_);
> >   	if (ret < 0)
> >   		return ret;
> >   	pushBuffers(statsBuffers_, availableStatsBuffers_,
> >   		    data->ipaStatBuffers_);
> > -	ret = params_->allocateBuffers(bufferCount, &paramsBuffers_);
> > +	ret = params_->allocateBuffers(kMaliC55BufferCount, &paramsBuffers_);
> >   	if (ret < 0)
> >   		return ret;
> >
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
index 566fa0549675..ce1f0b65cd6e 100644
--- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
+++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
@@ -48,6 +48,8 @@ 
 
 namespace {
 
+static constexpr unsigned int kMaliC55BufferCount = 16;
+
 bool isFormatRaw(const libcamera::PixelFormat &pixFmt)
 {
 	return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==
@@ -778,7 +780,7 @@  private:
 };
 
 PipelineHandlerMaliC55::PipelineHandlerMaliC55(CameraManager *manager)
-	: PipelineHandler(manager), dsFitted_(true)
+	: PipelineHandler(manager, kMaliC55BufferCount), dsFitted_(true)
 {
 }
 
@@ -1227,14 +1229,8 @@  int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
 {
 	MaliC55CameraData *data = cameraData(camera);
 	unsigned int ipaBufferId = 1;
-	unsigned int bufferCount;
 	int ret;
 
-	bufferCount = std::max({
-		data->frStream_.configuration().bufferCount,
-		data->dsStream_.configuration().bufferCount,
-	});
-
 	auto pushBuffers = [&](const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
 			       std::queue<FrameBuffer *> &queue,
 			       std::vector<IPABuffer> &ipaBuffers) {
@@ -1249,14 +1245,14 @@  int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
 		}
 	};
 
-	ret = stats_->allocateBuffers(bufferCount, &statsBuffers_);
+	ret = stats_->allocateBuffers(kMaliC55BufferCount, &statsBuffers_);
 	if (ret < 0)
 		return ret;
 
 	pushBuffers(statsBuffers_, availableStatsBuffers_,
 		    data->ipaStatBuffers_);
 
-	ret = params_->allocateBuffers(bufferCount, &paramsBuffers_);
+	ret = params_->allocateBuffers(kMaliC55BufferCount, &paramsBuffers_);
 	if (ret < 0)
 		return ret;