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

[Dan Scally]

On 01/06/2024 23:44, Dan Scally wrote:
> 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>


Err, supposed to be a "Reviewed-by: " in there somewhere...

>
>> ---
>>   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)