| Message ID | 20251209180954.332392-7-isaac.scott@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Tue, Dec 09, 2025 at 06:09:54PM +0000, Isaac Scott wrote: > The rkisp1 supports ISP bypass for RAW formats. There are other formats Please list them. > that are not RAW which can also be used in this mode, which are not > currently supported by the rkisp1 pipeline handler. Don't patches 1 to 5 add support for those formats to the pipeline handler ? > > Add a new IPA context member which tracks formats which are not RAW, so > we can attempt to use them in bypass mode if they are supported. > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> > --- > src/ipa/rkisp1/ipa_context.h | 2 ++ > src/ipa/rkisp1/rkisp1.cpp | 15 ++++++++++++++- > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index b257cee55..28d25af18 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -70,7 +70,9 @@ struct IPASessionConfiguration { > Size size; > } sensor; > > + bool bypass; > bool raw; > + > uint32_t paramFormat; > }; > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 9fee33de2..c195f0287 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -292,6 +292,19 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, > return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW; > }); > > + /* > + * Many formats are supported by the ISP in bypass mode, and are not RAW. Some support Reflow to 80 columns. > + * both RAW and bypass, but for now we can keep track of those that are not RAW, > + * so we can attempt to use bypass for them. > + */ > + context_.configuration.bypass = std::any_of(streamConfig.begin(), streamConfig.end(), > + [](auto &cfg) -> bool { > + PixelFormat pixelFormat{ cfg.second.pixelFormat }; > + const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat); > + return format.colourEncoding != PixelFormatInfo::ColourEncodingRAW; > + }); If I understand this correctly, you're setting bypass to true if any of the stream formats is not raw. That doesn't seem right. Have you tested this series with a raw sensor and the ISP not bypassed ? > + > + Extra blank line. > for (auto const &a : algorithms()) { > Algorithm *algo = static_cast<Algorithm *>(a.get()); > > @@ -374,7 +387,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, > * provided. > */ > const rkisp1_stat_buffer *stats = nullptr; > - if (!context_.configuration.raw) > + if (!context_.configuration.bypass) > stats = reinterpret_cast<rkisp1_stat_buffer *>( > mappedBuffers_.at(bufferId).planes()[0].data()); >
Hi Laurent, Quoting Laurent Pinchart (2025-12-11 02:14:59) > On Tue, Dec 09, 2025 at 06:09:54PM +0000, Isaac Scott wrote: > > The rkisp1 supports ISP bypass for RAW formats. There are other formats > > Please list them. > > > that are not RAW which can also be used in this mode, which are not > > currently supported by the rkisp1 pipeline handler. > > Don't patches 1 to 5 add support for those formats to the pipeline > handler ? > > > > > Add a new IPA context member which tracks formats which are not RAW, so > > we can attempt to use them in bypass mode if they are supported. > > > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> > > --- > > src/ipa/rkisp1/ipa_context.h | 2 ++ > > src/ipa/rkisp1/rkisp1.cpp | 15 ++++++++++++++- > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > index b257cee55..28d25af18 100644 > > --- a/src/ipa/rkisp1/ipa_context.h > > +++ b/src/ipa/rkisp1/ipa_context.h > > @@ -70,7 +70,9 @@ struct IPASessionConfiguration { > > Size size; > > } sensor; > > > > + bool bypass; > > bool raw; > > + > > uint32_t paramFormat; > > }; > > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > > index 9fee33de2..c195f0287 100644 > > --- a/src/ipa/rkisp1/rkisp1.cpp > > +++ b/src/ipa/rkisp1/rkisp1.cpp > > @@ -292,6 +292,19 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, > > return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW; > > }); > > > > + /* > > + * Many formats are supported by the ISP in bypass mode, and are not RAW. Some support > > Reflow to 80 columns. I'll fix that in the next series! > > > + * both RAW and bypass, but for now we can keep track of those that are not RAW, > > + * so we can attempt to use bypass for them. > > + */ > > + context_.configuration.bypass = std::any_of(streamConfig.begin(), streamConfig.end(), > > + [](auto &cfg) -> bool { > > + PixelFormat pixelFormat{ cfg.second.pixelFormat }; > > + const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat); > > + return format.colourEncoding != PixelFormatInfo::ColourEncodingRAW; > > + }); > > If I understand this correctly, you're setting bypass to true if any of > the stream formats is not raw. That doesn't seem right. Have you tested > this series with a raw sensor and the ISP not bypassed ? > You're absolutely right, I've been doing further work on this series and realised that this is not the correct place to do this. This way breaks the use case that already exists and makes it attempt to use bypass. I'll try to fix this issue in the next submission. > > + > > + > > Extra blank line. Ack > > > for (auto const &a : algorithms()) { > > Algorithm *algo = static_cast<Algorithm *>(a.get()); > > > > @@ -374,7 +387,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, > > * provided. > > */ > > const rkisp1_stat_buffer *stats = nullptr; > > - if (!context_.configuration.raw) > > + if (!context_.configuration.bypass) > > stats = reinterpret_cast<rkisp1_stat_buffer *>( > > mappedBuffers_.at(bufferId).planes()[0].data()); > > Thanks, Isaac > > -- > Regards, > > Laurent Pinchart
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index b257cee55..28d25af18 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -70,7 +70,9 @@ struct IPASessionConfiguration { Size size; } sensor; + bool bypass; bool raw; + uint32_t paramFormat; }; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 9fee33de2..c195f0287 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -292,6 +292,19 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW; }); + /* + * Many formats are supported by the ISP in bypass mode, and are not RAW. Some support + * both RAW and bypass, but for now we can keep track of those that are not RAW, + * so we can attempt to use bypass for them. + */ + context_.configuration.bypass = std::any_of(streamConfig.begin(), streamConfig.end(), + [](auto &cfg) -> bool { + PixelFormat pixelFormat{ cfg.second.pixelFormat }; + const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat); + return format.colourEncoding != PixelFormatInfo::ColourEncodingRAW; + }); + + for (auto const &a : algorithms()) { Algorithm *algo = static_cast<Algorithm *>(a.get()); @@ -374,7 +387,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, * provided. */ const rkisp1_stat_buffer *stats = nullptr; - if (!context_.configuration.raw) + if (!context_.configuration.bypass) stats = reinterpret_cast<rkisp1_stat_buffer *>( mappedBuffers_.at(bufferId).planes()[0].data());
The rkisp1 supports ISP bypass for RAW formats. There are other formats that are not RAW which can also be used in this mode, which are not currently supported by the rkisp1 pipeline handler. Add a new IPA context member which tracks formats which are not RAW, so we can attempt to use them in bypass mode if they are supported. Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> --- src/ipa/rkisp1/ipa_context.h | 2 ++ src/ipa/rkisp1/rkisp1.cpp | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-)