[RFC,4/6] pipeline: rkisp1: Add YUV bypass formats to list of 'raw' formats
diff mbox series

Message ID 20251209180954.332392-5-isaac.scott@ideasonboard.com
State New
Headers show
Series
  • rkisp1: Add support for YUV bypass
Related show

Commit Message

Isaac Scott Dec. 9, 2025, 6:09 p.m. UTC 
When ISP bypass is enabled, we need to be able to match YUV formats as
YUV and RAW bypass should be handled the same way by the pipeline
handler. Add them.

Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 31 ++++++++++++++++----
 src/libcamera/sensor/camera_sensor_basic.cpp |  1 +
 2 files changed, 26 insertions(+), 6 deletions(-)

Comments

Laurent Pinchart Dec. 11, 2025, 2:06 a.m. UTC | #1 
On Tue, Dec 09, 2025 at 06:09:52PM +0000, Isaac Scott wrote:
> When ISP bypass is enabled, we need to be able to match YUV formats as
> YUV and RAW bypass should be handled the same way by the pipeline
> handler. Add them.
> 
> Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 31 ++++++++++++++++----
>  src/libcamera/sensor/camera_sensor_basic.cpp |  1 +
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index ad0f3af34..0c8c15ea5 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -520,7 +520,9 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
>  namespace {
>  
>  /* Keep in sync with the supported raw formats in rkisp1_path.cpp. */

That seems to be done in patch 5/6. Will the pipeline handler still work
when applying patches 1 to 4 only ? We try to avoid bisection breakages
unless it would be really, really difficult to do otherwise.

> -const std::map<PixelFormat, uint32_t> rawFormats = {
> +const std::map<PixelFormat, uint32_t> bypassFormats = {
> +	{ formats::UYVY, MEDIA_BUS_FMT_UYVY8_1X16 },
> +	{ formats::YUYV, MEDIA_BUS_FMT_YUYV8_1X16 },
>  	{ formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 },
>  	{ formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 },
>  	{ formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 },
> @@ -755,10 +757,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  
>  	std::vector<unsigned int> mbusCodes;
>  
> -	if (isRaw) {
> -		mbusCodes = { rawFormats.at(config_[0].pixelFormat) };
> +	if (isRaw)
> +		mbusCodes = { bypassFormats.at(config_[0].pixelFormat) };
> +
> +	/* Select the sensor format. */
> +	PixelFormat bypassFormat;
> +	Size maxSize;
> +
> +	for (const StreamConfiguration &cfg : config_) {
> +		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> +		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW ||
> +		    info.colourEncoding == PixelFormatInfo::ColourEncodingYUV)
> +			bypassFormat = cfg.pixelFormat;
> +
> +		maxSize = std::max(maxSize, cfg.size);
> +	}
> +
> +	if (bypassFormat.isValid()) {
> +		LOG(RkISP1, Info) << "Using bypass format " << bypassFormat;
> +		mbusCodes = { bypassFormats.at(bypassFormat) };
>  	} else {
> -		std::transform(rawFormats.begin(), rawFormats.end(),
> +		std::transform(bypassFormats.begin(), bypassFormats.end(),
>  			       std::back_inserter(mbusCodes),
>  			       [](const auto &value) { return value.second; });
>  	}
> @@ -919,8 +938,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>  	const PixelFormat &streamFormat = config->at(0).pixelFormat;
>  	const PixelFormatInfo &info = PixelFormatInfo::info(streamFormat);
> -	isRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> -	data->usesDewarper_ = data->canUseDewarper_ && !isRaw_;
> +	isIspBypassed_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> +	data->usesDewarper_ = data->canUseDewarper_ && !isIspBypassed_;
>  
>  	Transform transform = config->combinedTransform();
>  	bool transposeAfterIsp = false;
> diff --git a/src/libcamera/sensor/camera_sensor_basic.cpp b/src/libcamera/sensor/camera_sensor_basic.cpp
> index 57213a1ab..9531fc192 100644
> --- a/src/libcamera/sensor/camera_sensor_basic.cpp
> +++ b/src/libcamera/sensor/camera_sensor_basic.cpp
> @@ -97,6 +97,7 @@ public:
>  	int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override;
>  	const CameraSensorProperties::SensorDelays &sensorDelays() override;
>  	BayerFormat::Order bayerOrder(Transform t) const override;
> +	Orientation mountingOrientation() const override { return mountingOrientation_; }

Seems unrelated to this patch.

>  
>  protected:
>  	std::string logPrefix() const override;
Isaac Scott Dec. 11, 2025, 10:44 a.m. UTC | #2 
Hi Laurent,

Thanks for the review!

Quoting Laurent Pinchart (2025-12-11 02:06:46)
> On Tue, Dec 09, 2025 at 06:09:52PM +0000, Isaac Scott wrote:
> > When ISP bypass is enabled, we need to be able to match YUV formats as
> > YUV and RAW bypass should be handled the same way by the pipeline
> > handler. Add them.
> > 
> > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 31 ++++++++++++++++----
> >  src/libcamera/sensor/camera_sensor_basic.cpp |  1 +
> >  2 files changed, 26 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index ad0f3af34..0c8c15ea5 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -520,7 +520,9 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
> >  namespace {
> >  
> >  /* Keep in sync with the supported raw formats in rkisp1_path.cpp. */
> 
> That seems to be done in patch 5/6. Will the pipeline handler still work
> when applying patches 1 to 4 only ? We try to avoid bisection breakages
> unless it would be really, really difficult to do otherwise.

Yes, that does seem to be an oversight on my part, when I submit the
series out of RFC I'll make sure to not break bisectability.

> 
> > -const std::map<PixelFormat, uint32_t> rawFormats = {
> > +const std::map<PixelFormat, uint32_t> bypassFormats = {
> > +     { formats::UYVY, MEDIA_BUS_FMT_UYVY8_1X16 },
> > +     { formats::YUYV, MEDIA_BUS_FMT_YUYV8_1X16 },
> >       { formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 },
> >       { formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 },
> >       { formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 },
> > @@ -755,10 +757,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> >  
> >       std::vector<unsigned int> mbusCodes;
> >  
> > -     if (isRaw) {
> > -             mbusCodes = { rawFormats.at(config_[0].pixelFormat) };
> > +     if (isRaw)
> > +             mbusCodes = { bypassFormats.at(config_[0].pixelFormat) };
> > +
> > +     /* Select the sensor format. */
> > +     PixelFormat bypassFormat;
> > +     Size maxSize;
> > +
> > +     for (const StreamConfiguration &cfg : config_) {
> > +             const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> > +             if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW ||
> > +                 info.colourEncoding == PixelFormatInfo::ColourEncodingYUV)
> > +                     bypassFormat = cfg.pixelFormat;
> > +
> > +             maxSize = std::max(maxSize, cfg.size);
> > +     }
> > +
> > +     if (bypassFormat.isValid()) {
> > +             LOG(RkISP1, Info) << "Using bypass format " << bypassFormat;
> > +             mbusCodes = { bypassFormats.at(bypassFormat) };
> >       } else {
> > -             std::transform(rawFormats.begin(), rawFormats.end(),
> > +             std::transform(bypassFormats.begin(), bypassFormats.end(),
> >                              std::back_inserter(mbusCodes),
> >                              [](const auto &value) { return value.second; });
> >       }
> > @@ -919,8 +938,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >  
> >       const PixelFormat &streamFormat = config->at(0).pixelFormat;
> >       const PixelFormatInfo &info = PixelFormatInfo::info(streamFormat);
> > -     isRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> > -     data->usesDewarper_ = data->canUseDewarper_ && !isRaw_;
> > +     isIspBypassed_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> > +     data->usesDewarper_ = data->canUseDewarper_ && !isIspBypassed_;
> >  
> >       Transform transform = config->combinedTransform();
> >       bool transposeAfterIsp = false;
> > diff --git a/src/libcamera/sensor/camera_sensor_basic.cpp b/src/libcamera/sensor/camera_sensor_basic.cpp
> > index 57213a1ab..9531fc192 100644
> > --- a/src/libcamera/sensor/camera_sensor_basic.cpp
> > +++ b/src/libcamera/sensor/camera_sensor_basic.cpp
> > @@ -97,6 +97,7 @@ public:
> >       int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override;
> >       const CameraSensorProperties::SensorDelays &sensorDelays() override;
> >       BayerFormat::Order bayerOrder(Transform t) const override;
> > +     Orientation mountingOrientation() const override { return mountingOrientation_; }
> 
> Seems unrelated to this patch.

Indeed, it should go into the patch adding camera_sensor_basic (whatever
it ends up being) :-)

Thanks,
Isaac

> 
> >  
> >  protected:
> >       std::string logPrefix() const override;
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index ad0f3af34..0c8c15ea5 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -520,7 +520,9 @@  void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
 namespace {
 
 /* Keep in sync with the supported raw formats in rkisp1_path.cpp. */
-const std::map<PixelFormat, uint32_t> rawFormats = {
+const std::map<PixelFormat, uint32_t> bypassFormats = {
+	{ formats::UYVY, MEDIA_BUS_FMT_UYVY8_1X16 },
+	{ formats::YUYV, MEDIA_BUS_FMT_YUYV8_1X16 },
 	{ formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 },
 	{ formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 },
 	{ formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 },
@@ -755,10 +757,27 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 
 	std::vector<unsigned int> mbusCodes;
 
-	if (isRaw) {
-		mbusCodes = { rawFormats.at(config_[0].pixelFormat) };
+	if (isRaw)
+		mbusCodes = { bypassFormats.at(config_[0].pixelFormat) };
+
+	/* Select the sensor format. */
+	PixelFormat bypassFormat;
+	Size maxSize;
+
+	for (const StreamConfiguration &cfg : config_) {
+		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
+		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW ||
+		    info.colourEncoding == PixelFormatInfo::ColourEncodingYUV)
+			bypassFormat = cfg.pixelFormat;
+
+		maxSize = std::max(maxSize, cfg.size);
+	}
+
+	if (bypassFormat.isValid()) {
+		LOG(RkISP1, Info) << "Using bypass format " << bypassFormat;
+		mbusCodes = { bypassFormats.at(bypassFormat) };
 	} else {
-		std::transform(rawFormats.begin(), rawFormats.end(),
+		std::transform(bypassFormats.begin(), bypassFormats.end(),
 			       std::back_inserter(mbusCodes),
 			       [](const auto &value) { return value.second; });
 	}
@@ -919,8 +938,8 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 
 	const PixelFormat &streamFormat = config->at(0).pixelFormat;
 	const PixelFormatInfo &info = PixelFormatInfo::info(streamFormat);
-	isRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
-	data->usesDewarper_ = data->canUseDewarper_ && !isRaw_;
+	isIspBypassed_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
+	data->usesDewarper_ = data->canUseDewarper_ && !isIspBypassed_;
 
 	Transform transform = config->combinedTransform();
 	bool transposeAfterIsp = false;
diff --git a/src/libcamera/sensor/camera_sensor_basic.cpp b/src/libcamera/sensor/camera_sensor_basic.cpp
index 57213a1ab..9531fc192 100644
--- a/src/libcamera/sensor/camera_sensor_basic.cpp
+++ b/src/libcamera/sensor/camera_sensor_basic.cpp
@@ -97,6 +97,7 @@  public:
 	int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override;
 	const CameraSensorProperties::SensorDelays &sensorDelays() override;
 	BayerFormat::Order bayerOrder(Transform t) const override;
+	Orientation mountingOrientation() const override { return mountingOrientation_; }
 
 protected:
 	std::string logPrefix() const override;