[libcamera-devel] [PATCH] libcamera: ipu3: Fix RAW sizes selection

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Feb 03, 2021 at 12:23:16PM +0100, Jacopo Mondi wrote:
> Commit 7208e70211a6 ("libcamera: ipu3: Always use sensor full frame size")
> changed the CIO2 configuration procedure to always select the full
> sensor resolution to work around an ImgU configuration issue, as
> reported in the todo item introduced in said commit.
> 
> When capturing a RAW stream only and the ImgU is not involved, the CIO2
> has to be configured with the requested stream size to avoid adjusting
> the stream to the sensor resolution.
> 
> As an example, capturing a raw frame smaller than the sensor resolution
> with this patch applied results in the stream configuration to be
> correctly assigned.
> 
> $ cam -c1 -swidth=1056,height=784,role=raw
> ipu3.cpp:207 CIO2 configuration: 1056x784-SGRBG10_IPU3
> ipu3.cpp:222 Validating stream: 1056x784-SGRBG10_IPU3
> ipu3.cpp:233 Assigned 1056x784-SGRBG10_IPU3 to the raw stream
> 
> Without this patch the same operation results in the stream resolution
> to be adjusted to the sensor resolution.
> 
> $ cam -c1 -swidth=1056,height=784,role=raw
> ipu3.cpp:201 CIO2 configuration: 4224x3136-SGRBG10_IPU3
> ipu3.cpp:216 Validating stream: 1056x784-SGRBG10_IPU3
> ipu3.cpp:227 Assigned 4224x3136-SGRBG10_IPU3 to the raw stream
> ipu3.cpp:297 Stream 0 configuration adjusted to 4224x3136-SGRBG10_IPU3
> Camera configuration adjusted
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index db0d6b91be70..d73dd50e2f1d 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -162,12 +162,14 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  	unsigned int rawCount = 0;
>  	unsigned int yuvCount = 0;
>  	Size maxYuvSize;
> +	Size maxRawSize;
>  
>  	for (const StreamConfiguration &cfg : config_) {
>  		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
>  
>  		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
>  			rawCount++;
> +			maxRawSize.expandTo(cfg.size);

Given that we can't have more than one raw stream you could also write

			maxRawSize = cfg.size;

and possibly event rename the variable to rawSize. Up to you.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  		} else {
>  			yuvCount++;
>  			maxYuvSize.expandTo(cfg.size);
> @@ -190,11 +192,16 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  	 * commit message of the patch that introduced this comment for more
>  	 * failure examples).
>  	 *
> -	 * Until the sensor frame size calculation criteria are clarified,
> -	 * always use the largest possible one which guarantees better results
> -	 * at the expense of the frame rate and CSI-2 bus bandwidth.
> +	 * Until the sensor frame size calculation criteria are clarified, when
> +	 * capturing from ImgU always use the largest possible size which
> +	 * guarantees better results at the expense of the frame rate and CSI-2
> +	 * bus bandwidth. When only a raw stream is requested use the requested
> +	 * size instead, as the ImgU is not involved.
>  	 */
> -	cio2Configuration_ = data_->cio2_.generateConfiguration({});
> +	if (!yuvCount)
> +		cio2Configuration_ = data_->cio2_.generateConfiguration(maxRawSize);
> +	else
> +		cio2Configuration_ = data_->cio2_.generateConfiguration({});
>  	if (!cio2Configuration_.pixelFormat.isValid())
>  		return Invalid;
>  

-- 
Regards,

Laurent Pinchart