[{"id":29662,"web_url":"https://patchwork.libcamera.org/comment/29662/","msgid":"<171699275271.191612.10554668055418267557@ping.linuxembedded.co.uk>","date":"2024-05-29T14:25:52","subject":"Re: [PATCH v1 6/6] pipeline: vimc: Don't hardcode scaling factor\n\twith recent kernels","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-04-25 00:42:24)\n> Starting in kernel v5.16, the vimc driver stopped hardcoding the scaler\n> factor. Use this to lift constraints on the camera configuration, and in\n> particular on the exotic output size alignment to a multiple of 6. As a\n> result, vimc-based cameras can more easily match common display\n> resolutions.\n\nNow that's helpful!\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/vimc/vimc.cpp | 47 +++++++++++++++++++---------\n> 1 file changed, 33 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 5e66ee1d26c1..be7a6733472b 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -114,6 +114,9 @@ static const std::map pixelformats{\n> { formats::BGR888, MEDIA_BUS_FMT_RGB888_1X24 },\n> };\n> \n> +static constexpr Size kMinSize{ 16, 16 };\n> +static constexpr Size kMaxSize{ 4096, 2160 };\n> +\n> } /* namespace */\n> \n> VimcCameraConfiguration::VimcCameraConfiguration(VimcCameraData *data)\n> @@ -153,14 +156,20 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n> const Size size = cfg.size;\n> \n> /*\n> - * The scaler hardcodes a x3 scale-up ratio, and the sensor output size\n> - * is aligned to two pixels in both directions. The output width and\n> - * height thus have to be multiples of 6.\n> + * The sensor output size is aligned to two pixels in both directions.\n> + * Additionally, prior to v5.16, the scaler hardcodes a x3 scale-up\n> + * ratio, requiring the output width and height to be multiples of 6.\n> */\n> - cfg.size.width = std::max(48U, std::min(4096U, cfg.size.width));\n> - cfg.size.height = std::max(48U, std::min(2160U, cfg.size.height));\n> - cfg.size.width -= cfg.size.width % 6;\n> - cfg.size.height -= cfg.size.height % 6;\n> + Size minSize{ kMinSize };\n> + unsigned int alignment = 2;\n> +\n> + if (data_->media_->version() < KERNEL_VERSION(5, 16, 0)) {\n> + minSize *= 3;\n> + alignment *= 3;\n> + }\n> +\n> + cfg.size.expandTo(minSize).boundTo(kMaxSize)\n> + .alignDownTo(alignment, alignment);\n> \n> if (cfg.size != size) {\n> LOG(VIMC, Debug)\n> @@ -216,10 +225,12 @@ PipelineHandlerVimc::generateConfiguration(Camera *camera,\n> }\n> }\n> \n> - /* The scaler hardcodes a x3 scale-up ratio. */\n> - std::vector sizes{\n> - SizeRange{ { 48, 48 }, { 4096, 2160 } }\n> - };\n> + /* Prior to v5.16, the scaler hardcodes a x3 scale-up ratio. */\n> + Size minSize{ kMinSize };\n> + if (data->media_->version() < KERNEL_VERSION(5, 16, 0))\n> + minSize *= 3;\n> +\n> + std::vector sizes{ { minSize, kMaxSize } };\n> formats[pixelformat.first] = sizes;\n> }\n> \n> @@ -242,10 +253,18 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> StreamConfiguration &cfg = config->at(0);\n> int ret;\n> \n> - /* The scaler hardcodes a x3 scale-up ratio. */\n> + /*\n> + * Prior to v5.16, the scaler hardcodes a x3 scale-up ratio. For newer\n> + * kernels, use a sensor resolution of 1920x1080 and let the scaler\n> + * produce the requested stream size.\n> + */\n> + Size sensorSize{ 1920, 1080 };\n> + if (data->media_->version() < KERNEL_VERSION(5, 16, 0))\n> + sensorSize = { cfg.size.width / 3, cfg.size.height / 3 };\n> +\n> V4L2SubdeviceFormat subformat = {};\n> subformat.code = MEDIA_BUS_FMT_SGRBG8_1X8;\n> - subformat.size = { cfg.size.width / 3, cfg.size.height / 3 };\n> + subformat.size = sensorSize;\n> \n> ret = data->sensor_->setFormat(&subformat);\n> if (ret)\n> @@ -293,7 +312,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> * vimc driver will fail pipeline validation.\n> */\n> format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8);\n> - format.size = { cfg.size.width / 3, cfg.size.height / 3 };\n> + format.size = sensorSize;\n> \n\nSounds pretty good to me. I haven't tested, but I expect you have.\n\n\nReviewed-by: Kieran Bingham \n\n> ret = data->raw_->setFormat(&format);\n> if (ret)\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EEC23BD87C\n\tfor ;\n\tWed, 29 May 2024 14:25:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 347F3634B0;\n\tWed, 29 May 2024 16:25:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8165A634AF\n\tfor ;\n\tWed, 29 May 2024 16:25:55 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C7DEFA27;\n\tWed, 29 May 2024 16:25:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lGu7HbQi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716992751;\n\tbh=bp7cz0BCSP8ftI3KPO01FF+6+GiFGuerv/YIiB60Xr8=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=lGu7HbQiHEQCa+68f8OE/s63kX+nCK1xQuw0LmUwl/5ddl5Cr/dclUcwK7OkjF3NQ\n\tmv7BRzF8+nqfEzSyyrW4W/xKgi1gaLJW6TUjcLVxCTJ3j4gr51p/t0tbrjyNdszlLx\n\tddHrhqeFmNRXlloVGHLII/3Bzd8rcky9EPgcZHIQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240424234224.9658-7-laurent.pinchart@ideasonboard.com>","References":"<20240424234224.9658-1-laurent.pinchart@ideasonboard.com>\n\t<20240424234224.9658-7-laurent.pinchart@ideasonboard.com>","Subject":"Re: [PATCH v1 6/6] pipeline: vimc: Don't hardcode scaling factor\n\twith recent kernels","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 29 May 2024 15:25:52 +0100","Message-ID":"<171699275271.191612.10554668055418267557@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]