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

[Kieran Bingham]

Quoting Laurent Pinchart (2024-04-25 00:42:24)
> 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.

Now that's helpful!

> Signed-off-by: Laurent Pinchart <laurent.pinchart 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 5e66ee1d26c1..be7a6733472b 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;
>  

Sounds pretty good to me. I haven't tested, but I expect you have.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>         ret = data->raw_->setFormat(&format);
>         if (ret)
> -- 
> Regards,
> 
> Laurent Pinchart
>