[v2,4/8] ipa: rpi: Allow sharing params buffer to the IPA
diff mbox series

Message ID 20260722-pi4-upstream-v2-4-eb44d3c6fd92@ideasonboard.com
State Superseded, archived
Headers show
Series
  • Raspberry Pi: Update VC4 pipeline for mainline
Related show

Commit Message

Jai Luthra July 22, 2026, 11:23 a.m. UTC 
While the VC4 IPA currently writes control values for configuration, the
mainline driver expects a parameter buffer.

In preparation for the migration, add plumbing to pass the parameter
buffer to the IPA, while reporting back how many bytes were filled
through the prepareIspComplete() callback.

This is unused for now, VC4 will use it in subsequent commits.

Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
---
 include/libcamera/ipa/raspberrypi.mojom  |  3 ++-
 src/ipa/rpi/common/ipa_base.cpp          | 11 ++++++++++-
 src/ipa/rpi/common/ipa_base.h            |  4 ++++
 src/libcamera/pipeline/rpi/pisp/pisp.cpp |  6 ++++--
 src/libcamera/pipeline/rpi/vc4/vc4.cpp   |  6 ++++--
 5 files changed, 24 insertions(+), 6 deletions(-)

Comments

Paul Elder Sept. 4, 2026, 9:41 a.m. UTC | #1 
Quoting Jai Luthra (2026-07-22 20:23:20)
> While the VC4 IPA currently writes control values for configuration, the
> mainline driver expects a parameter buffer.
> 
> In preparation for the migration, add plumbing to pass the parameter
> buffer to the IPA, while reporting back how many bytes were filled
> through the prepareIspComplete() callback.
> 
> This is unused for now, VC4 will use it in subsequent commits.
> 
> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
>  include/libcamera/ipa/raspberrypi.mojom  |  3 ++-
>  src/ipa/rpi/common/ipa_base.cpp          | 11 ++++++++++-
>  src/ipa/rpi/common/ipa_base.h            |  4 ++++
>  src/libcamera/pipeline/rpi/pisp/pisp.cpp |  6 ++++--
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp   |  6 ++++--
>  5 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index 1b7e03582..dcd3b2c66 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -33,6 +33,7 @@ struct BufferIds {
>         uint32 bayer;
>         uint32 embedded;
>         uint32 stats;
> +       uint32 params;
>  };
>  
>  struct ConfigParams {
> @@ -227,7 +228,7 @@ interface IPARPiEventInterface {
>          * processing the frame. The embedded data buffer may be recycled after
>          * this event.
>          */
> -       prepareIspComplete(BufferIds buffers, bool stitchSwapBuffers);
> +       prepareIspComplete(BufferIds buffers, bool stitchSwapBuffers, uint32 paramsBytesUsed);

Documentation needs to be updated.


With that fixed,

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

>  
>         /**
>          * \fn processStatsComplete()
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index e3f7b99a8..b3313f577 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -427,6 +427,7 @@ void IpaBase::prepareIsp(const PrepareParams &params)
>         unsigned int ipaContext = params.ipaContext % rpiMetadata_.size();
>         RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext];
>         Span<uint8_t> embeddedBuffer;
> +       Span<uint8_t> paramsBuffer;
>  
>         rpiMetadata.clear();
>         fillDeviceStatus(params.sensorControls, ipaContext);
> @@ -450,6 +451,13 @@ void IpaBase::prepareIsp(const PrepareParams &params)
>                 embeddedBuffer = it->second.planes()[0];
>         }
>  
> +       if (params.buffers.params) {
> +               auto it = buffers_.find(params.buffers.params);
> +               ASSERT(it != buffers_.end());
> +               paramsBuffer = it->second.planes()[0];
> +               platformParamsBufferInit(paramsBuffer);
> +       }
> +
>         /*
>          * AGC wants to know the algorithm status from the time it actioned the
>          * sensor exposure/gain changes. So fetch it from the metadata list
> @@ -519,7 +527,8 @@ void IpaBase::prepareIsp(const PrepareParams &params)
>                 reportMetadata(ipaContext);
>  
>         /* Ready to push the input buffer into the ISP. */
> -       prepareIspComplete.emit(params.buffers, stitchSwapBuffers_);
> +       prepareIspComplete.emit(params.buffers, stitchSwapBuffers_,
> +                               platformParamsBytesUsed());
>  }
>  
>  void IpaBase::processStats(const ProcessParams &params)
> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h
> index f4157e9ec..1e9b0e57e 100644
> --- a/src/ipa/rpi/common/ipa_base.h
> +++ b/src/ipa/rpi/common/ipa_base.h
> @@ -79,6 +79,8 @@ protected:
>         /* Whether the stitch block (if available) needs to swap buffers. */
>         bool stitchSwapBuffers_;
>  
> +       virtual size_t platformParamsBytesUsed() const { return 0; }
> +
>  private:
>         /* Number of metadata objects available in the context list. */
>         static constexpr unsigned int numMetadataContexts = 16;
> @@ -87,7 +89,9 @@ private:
>         virtual int32_t platformStart(const ControlList &controls, StartResult *result) = 0;
>         virtual int32_t platformConfigure(const ConfigParams &params, ConfigResult *result) = 0;
>  
> +       virtual void platformParamsBufferInit([[maybe_unused]] Span<uint8_t> paramsBuffer) {}
>         virtual void platformPrepareIsp(RPiController::Metadata &rpiMetadata) = 0;
> +
>         virtual void platformPrepareAgc(RPiController::Metadata &rpiMetadata) = 0;
>         virtual RPiController::StatisticsPtr platformProcessStats(Span<uint8_t> mem) = 0;
>  
> diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> index 406d683dc..616014729 100644
> --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> @@ -759,7 +759,8 @@ public:
>         void beOutputDequeue(FrameBuffer *buffer);
>  
>         void processStatsComplete(const ipa::RPi::BufferIds &buffers);
> -       void prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers);
> +       void prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers,
> +                               unsigned int paramsBytesUsed);
>         void setCameraTimeout(uint32_t maxFrameLengthMs);
>  
>         /* Array of CFE and ISP device streams and associated buffers/streams. */
> @@ -1885,7 +1886,8 @@ void PiSPCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)
>         cfe_[Cfe::Output0].dev()->setDequeueTimeout(timeout);
>  }
>  
> -void PiSPCameraData::prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers)
> +void PiSPCameraData::prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers,
> +                                       [[maybe_unused]] unsigned int paramsBytesUsed)
>  {
>         unsigned int embeddedId = buffers.embedded & RPi::MaskID;
>         unsigned int bayerId = buffers.bayer & RPi::MaskID;
> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> index 3e9a49058..c18358876 100644
> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> @@ -79,7 +79,8 @@ public:
>         void ispOutputDequeue(FrameBuffer *buffer);
>  
>         void processStatsComplete(const ipa::RPi::BufferIds &buffers);
> -       void prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers);
> +       void prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers,
> +                               unsigned int paramsBytesUsed);
>         void setIspControls(const ControlList &controls);
>         void setCameraTimeout(uint32_t maxFrameLengthMs);
>  
> @@ -863,7 +864,8 @@ void Vc4CameraData::processStatsComplete(const ipa::RPi::BufferIds &buffers)
>  }
>  
>  void Vc4CameraData::prepareIspComplete(const ipa::RPi::BufferIds &buffers,
> -                                      [[maybe_unused]] bool stitchSwapBuffers)
> +                                      [[maybe_unused]] bool stitchSwapBuffers,
> +                                      [[maybe_unused]] unsigned int paramsBytesUsed)
>  {
>         unsigned int embeddedId = buffers.embedded & RPi::MaskID;
>         unsigned int bayer = buffers.bayer & RPi::MaskID;
> 
> -- 
> 2.54.0
>

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
index 1b7e03582..dcd3b2c66 100644
--- a/include/libcamera/ipa/raspberrypi.mojom
+++ b/include/libcamera/ipa/raspberrypi.mojom
@@ -33,6 +33,7 @@  struct BufferIds {
 	uint32 bayer;
 	uint32 embedded;
 	uint32 stats;
+	uint32 params;
 };
 
 struct ConfigParams {
@@ -227,7 +228,7 @@  interface IPARPiEventInterface {
 	 * processing the frame. The embedded data buffer may be recycled after
 	 * this event.
 	 */
-	prepareIspComplete(BufferIds buffers, bool stitchSwapBuffers);
+	prepareIspComplete(BufferIds buffers, bool stitchSwapBuffers, uint32 paramsBytesUsed);
 
 	/**
 	 * \fn processStatsComplete()
diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index e3f7b99a8..b3313f577 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -427,6 +427,7 @@  void IpaBase::prepareIsp(const PrepareParams &params)
 	unsigned int ipaContext = params.ipaContext % rpiMetadata_.size();
 	RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext];
 	Span<uint8_t> embeddedBuffer;
+	Span<uint8_t> paramsBuffer;
 
 	rpiMetadata.clear();
 	fillDeviceStatus(params.sensorControls, ipaContext);
@@ -450,6 +451,13 @@  void IpaBase::prepareIsp(const PrepareParams &params)
 		embeddedBuffer = it->second.planes()[0];
 	}
 
+	if (params.buffers.params) {
+		auto it = buffers_.find(params.buffers.params);
+		ASSERT(it != buffers_.end());
+		paramsBuffer = it->second.planes()[0];
+		platformParamsBufferInit(paramsBuffer);
+	}
+
 	/*
 	 * AGC wants to know the algorithm status from the time it actioned the
 	 * sensor exposure/gain changes. So fetch it from the metadata list
@@ -519,7 +527,8 @@  void IpaBase::prepareIsp(const PrepareParams &params)
 		reportMetadata(ipaContext);
 
 	/* Ready to push the input buffer into the ISP. */
-	prepareIspComplete.emit(params.buffers, stitchSwapBuffers_);
+	prepareIspComplete.emit(params.buffers, stitchSwapBuffers_,
+				platformParamsBytesUsed());
 }
 
 void IpaBase::processStats(const ProcessParams &params)
diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h
index f4157e9ec..1e9b0e57e 100644
--- a/src/ipa/rpi/common/ipa_base.h
+++ b/src/ipa/rpi/common/ipa_base.h
@@ -79,6 +79,8 @@  protected:
 	/* Whether the stitch block (if available) needs to swap buffers. */
 	bool stitchSwapBuffers_;
 
+	virtual size_t platformParamsBytesUsed() const { return 0; }
+
 private:
 	/* Number of metadata objects available in the context list. */
 	static constexpr unsigned int numMetadataContexts = 16;
@@ -87,7 +89,9 @@  private:
 	virtual int32_t platformStart(const ControlList &controls, StartResult *result) = 0;
 	virtual int32_t platformConfigure(const ConfigParams &params, ConfigResult *result) = 0;
 
+	virtual void platformParamsBufferInit([[maybe_unused]] Span<uint8_t> paramsBuffer) {}
 	virtual void platformPrepareIsp(RPiController::Metadata &rpiMetadata) = 0;
+
 	virtual void platformPrepareAgc(RPiController::Metadata &rpiMetadata) = 0;
 	virtual RPiController::StatisticsPtr platformProcessStats(Span<uint8_t> mem) = 0;
 
diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
index 406d683dc..616014729 100644
--- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
+++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
@@ -759,7 +759,8 @@  public:
 	void beOutputDequeue(FrameBuffer *buffer);
 
 	void processStatsComplete(const ipa::RPi::BufferIds &buffers);
-	void prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers);
+	void prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers,
+				unsigned int paramsBytesUsed);
 	void setCameraTimeout(uint32_t maxFrameLengthMs);
 
 	/* Array of CFE and ISP device streams and associated buffers/streams. */
@@ -1885,7 +1886,8 @@  void PiSPCameraData::setCameraTimeout(uint32_t maxFrameLengthMs)
 	cfe_[Cfe::Output0].dev()->setDequeueTimeout(timeout);
 }
 
-void PiSPCameraData::prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers)
+void PiSPCameraData::prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers,
+					[[maybe_unused]] unsigned int paramsBytesUsed)
 {
 	unsigned int embeddedId = buffers.embedded & RPi::MaskID;
 	unsigned int bayerId = buffers.bayer & RPi::MaskID;
diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
index 3e9a49058..c18358876 100644
--- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
+++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
@@ -79,7 +79,8 @@  public:
 	void ispOutputDequeue(FrameBuffer *buffer);
 
 	void processStatsComplete(const ipa::RPi::BufferIds &buffers);
-	void prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers);
+	void prepareIspComplete(const ipa::RPi::BufferIds &buffers, bool stitchSwapBuffers,
+				unsigned int paramsBytesUsed);
 	void setIspControls(const ControlList &controls);
 	void setCameraTimeout(uint32_t maxFrameLengthMs);
 
@@ -863,7 +864,8 @@  void Vc4CameraData::processStatsComplete(const ipa::RPi::BufferIds &buffers)
 }
 
 void Vc4CameraData::prepareIspComplete(const ipa::RPi::BufferIds &buffers,
-				       [[maybe_unused]] bool stitchSwapBuffers)
+				       [[maybe_unused]] bool stitchSwapBuffers,
+				       [[maybe_unused]] unsigned int paramsBytesUsed)
 {
 	unsigned int embeddedId = buffers.embedded & RPi::MaskID;
 	unsigned int bayer = buffers.bayer & RPi::MaskID;