[PATCH v2 6/6] pipeline: vimc: Don't hardcode scaling factor with recent kernels

[Dan Scally]

Hi Laurent

On 29/05/2024 16:43, Laurent Pinchart wrote:
> Starting in kernel v5.16, the vimc driver stopped hardcoding the scaler
> factor. Use this to lift constraints on the camera configuration, and in
> particular on the exotic output size alignment to a multiple of 6. As a
> result, vimc-based cameras can more easily match common display
> resolutions.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


Looks good to me too: Daniel Scally <dan.scally at ideasonboard.com>

> ---
>   src/libcamera/pipeline/vimc/vimc.cpp | 47 +++++++++++++++++++---------
>   1 file changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index c7650432ddcc..0ec9928eec23 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -114,6 +114,9 @@ static const std::map<PixelFormat, uint32_t> pixelformats{
>   	{ formats::BGR888, MEDIA_BUS_FMT_RGB888_1X24 },
>   };
>   
> +static constexpr Size kMinSize{ 16, 16 };
> +static constexpr Size kMaxSize{ 4096, 2160 };
> +
>   } /* namespace */
>   
>   VimcCameraConfiguration::VimcCameraConfiguration(VimcCameraData *data)
> @@ -153,14 +156,20 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
>   	const Size size = cfg.size;
>   
>   	/*
> -	 * The scaler hardcodes a x3 scale-up ratio, and the sensor output size
> -	 * is aligned to two pixels in both directions. The output width and
> -	 * height thus have to be multiples of 6.
> +	 * The sensor output size is aligned to two pixels in both directions.
> +	 * Additionally, prior to v5.16, the scaler hardcodes a x3 scale-up
> +	 * ratio, requiring the output width and height to be multiples of 6.
>   	 */
> -	cfg.size.width = std::max(48U, std::min(4096U, cfg.size.width));
> -	cfg.size.height = std::max(48U, std::min(2160U, cfg.size.height));
> -	cfg.size.width -= cfg.size.width % 6;
> -	cfg.size.height -= cfg.size.height % 6;
> +	Size minSize{ kMinSize };
> +	unsigned int alignment = 2;
> +
> +	if (data_->media_->version() < KERNEL_VERSION(5, 16, 0)) {
> +		minSize *= 3;
> +		alignment *= 3;
> +	}
> +
> +	cfg.size.expandTo(minSize).boundTo(kMaxSize)
> +		.alignDownTo(alignment, alignment);
>   
>   	if (cfg.size != size) {
>   		LOG(VIMC, Debug)
> @@ -216,10 +225,12 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera,
>   			}
>   		}
>   
> -		/* The scaler hardcodes a x3 scale-up ratio. */
> -		std::vector<SizeRange> sizes{
> -			SizeRange{ { 48, 48 }, { 4096, 2160 } }
> -		};
> +		/* Prior to v5.16, the scaler hardcodes a x3 scale-up ratio. */
> +		Size minSize{ kMinSize };
> +		if (data->media_->version() < KERNEL_VERSION(5, 16, 0))
> +			minSize *= 3;
> +
> +		std::vector<SizeRange> sizes{ { minSize, kMaxSize } };
>   		formats[pixelformat.first] = sizes;
>   	}
>   
> @@ -242,10 +253,18 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
>   	StreamConfiguration &cfg = config->at(0);
>   	int ret;
>   
> -	/* The scaler hardcodes a x3 scale-up ratio. */
> +	/*
> +	 * Prior to v5.16, the scaler hardcodes a x3 scale-up ratio. For newer
> +	 * kernels, use a sensor resolution of 1920x1080 and let the scaler
> +	 * produce the requested stream size.
> +	 */
> +	Size sensorSize{ 1920, 1080 };
> +	if (data->media_->version() < KERNEL_VERSION(5, 16, 0))
> +		sensorSize = { cfg.size.width / 3, cfg.size.height / 3 };
> +
>   	V4L2SubdeviceFormat subformat = {};
>   	subformat.code = MEDIA_BUS_FMT_SGRBG8_1X8;
> -	subformat.size = { cfg.size.width / 3, cfg.size.height / 3 };
> +	subformat.size = sensorSize;
>   
>   	ret = data->sensor_->setFormat(&subformat);
>   	if (ret)
> @@ -293,7 +312,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
>   	 * vimc driver will fail pipeline validation.
>   	 */
>   	format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8);
> -	format.size = { cfg.size.width / 3, cfg.size.height / 3 };
> +	format.size = sensorSize;
>   
>   	ret = data->raw_->setFormat(&format);
>   	if (ret)