Patch Detail
Show a patch.
GET /api/patches/21843/?format=api
{ "id": 21843, "url": "https://patchwork.libcamera.org/api/patches/21843/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21843/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20241107105846.52287-11-dan.scally@ideasonboard.com>", "date": "2024-11-07T10:58:43", "name": "[v5,10/13] libcamera: mali-c55: Correct input/output format representation", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "c48a613cdf8ae76bcabb40f6e31482fdfb9d2c65", "submitter": { "id": 156, "url": "https://patchwork.libcamera.org/api/people/156/?format=api", "name": "Dan Scally", "email": "dan.scally@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/21843/mbox/", "series": [ { "id": 4776, "url": "https://patchwork.libcamera.org/api/series/4776/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4776", "date": "2024-11-07T10:58:33", "name": "Miscellaneous Mali-C55 Pipeline Fixes", "version": 5, "mbox": "https://patchwork.libcamera.org/series/4776/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21843/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21843/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 DDBD4C324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 7 Nov 2024 10:59:12 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7895E65495;\n\tThu, 7 Nov 2024 11:59:08 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5439E6547D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 7 Nov 2024 11:58:57 +0100 (CET)", "from mail.ideasonboard.com\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 602B11870;\n\tThu, 7 Nov 2024 11:58:48 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TnKzcJ+q\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730977128;\n\tbh=5TIRCs1LJFZRvKUfyRoNHkRuFY2ZpNhQ2ASRspN45OI=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=TnKzcJ+qRGBpsKX9wS1YPg8Nwhe4XSCwZf7jCrEeoHGKbwf/hdVUjcbOh3qOOJmTR\n\tApk8GAXNwq9NC1eACeqQ7A+JKzDqpkV5b0nYPIQtFUCZ7VhzulDHWI03sU/S6gULr0\n\tKn3QQPsRDFN87auOzOQdg2OhJLPUkqPNFKRRhasY=", "From": "Daniel Scally <dan.scally@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Daniel Scally <dan.scally@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>", "Subject": "[PATCH v5 10/13] libcamera: mali-c55: Correct input/output format\n\trepresentation", "Date": "Thu, 7 Nov 2024 10:58:43 +0000", "Message-Id": "<20241107105846.52287-11-dan.scally@ideasonboard.com>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20241107105846.52287-1-dan.scally@ideasonboard.com>", "References": "<20241107105846.52287-1-dan.scally@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "At present we configure raw streams by looping through the pixel\nformats we support and finding one with an associated media bus\nformat code that the sensor can produce. In the new representation\nof raw data from the kernel driver this will not work - the sensor\ncould produce 8, 10, 12, 14 or 16 bit data and the ISP will force\nit to RAW16, which is the only actually supported output.\n\nTo fix the issue move to simply finding a pixel format with a bayer\norder that matches that of the media bus format produced by the\nsensor. If the sensor can produce multiple formats with the same\nbayer order use the one with the largest bitdepth.\n\nFinally, remove the claim to support RAW formats of less than 16 bits.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nSigned-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n---\nChanges in v5:\n\n\t- Dropped the Fatal log.\n\n src/libcamera/pipeline/mali-c55/mali-c55.cpp | 128 ++++++++++++-------\n 1 file changed, 80 insertions(+), 48 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\nindex 156560c1..97827abd 100644\n--- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n+++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n@@ -57,22 +57,6 @@ const std::map<libcamera::PixelFormat, unsigned int> maliC55FmtToCode = {\n \t{ formats::NV21, MEDIA_BUS_FMT_YUV10_1X30 },\n \n \t/* RAW formats, FR pipe only. */\n-\t{ formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 },\n-\t{ formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 },\n-\t{ formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 },\n-\t{ formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 },\n-\t{ formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 },\n-\t{ formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 },\n-\t{ formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 },\n-\t{ formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 },\n-\t{ formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 },\n-\t{ formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 },\n-\t{ formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 },\n-\t{ formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 },\n-\t{ formats::SGBRG14, MEDIA_BUS_FMT_SGBRG14_1X14 },\n-\t{ formats::SRGGB14, MEDIA_BUS_FMT_SRGGB14_1X14 },\n-\t{ formats::SBGGR14, MEDIA_BUS_FMT_SBGGR14_1X14 },\n-\t{ formats::SGRBG14, MEDIA_BUS_FMT_SGRBG14_1X14 },\n \t{ formats::SGBRG16, MEDIA_BUS_FMT_SGBRG16_1X16 },\n \t{ formats::SRGGB16, MEDIA_BUS_FMT_SRGGB16_1X16 },\n \t{ formats::SBGGR16, MEDIA_BUS_FMT_SBGGR16_1X16 },\n@@ -98,7 +82,8 @@ public:\n \tconst std::vector<Size> sizes(unsigned int mbusCode) const;\n \tconst Size resolution() const;\n \n-\tPixelFormat bestRawFormat() const;\n+\tint pixfmtToMbusCode(const PixelFormat &pixFmt) const;\n+\tconst PixelFormat &bestRawFormat() const;\n \n \tPixelFormat adjustRawFormat(const PixelFormat &pixFmt) const;\n \tSize adjustRawSizes(const PixelFormat &pixFmt, const Size &rawSize) const;\n@@ -208,33 +193,75 @@ const Size MaliC55CameraData::resolution() const\n \treturn tpgResolution_;\n }\n \n-PixelFormat MaliC55CameraData::bestRawFormat() const\n+/*\n+ * The Mali C55 ISP can only produce 16-bit RAW output in bypass modes, but the\n+ * sensors connected to it might produce 8/10/12/16 bits. We simply search the\n+ * sensor's supported formats for the one with a matching bayer order and the\n+ * greatest bitdepth.\n+ */\n+int MaliC55CameraData::pixfmtToMbusCode(const PixelFormat &pixFmt) const\n {\n+\tauto it = maliC55FmtToCode.find(pixFmt);\n+\tif (it == maliC55FmtToCode.end())\n+\t\treturn -EINVAL;\n+\n+\tBayerFormat bayerFormat = BayerFormat::fromMbusCode(it->second);\n+\tif (!bayerFormat.isValid())\n+\t\treturn -EINVAL;\n+\n+\tV4L2Subdevice::Formats formats = sd_->formats(0);\n+\tunsigned int sensorMbusCode = 0;\n \tunsigned int bitDepth = 0;\n-\tPixelFormat rawFormat;\n \n-\t/*\n-\t * Iterate over all the supported PixelFormat and find the one\n-\t * supported by the camera with the largest bitdepth.\n-\t */\n-\tfor (const auto &maliFormat : maliC55FmtToCode) {\n-\t\tPixelFormat pixFmt = maliFormat.first;\n-\t\tif (!isFormatRaw(pixFmt))\n+\tfor (const auto &[code, sizes] : formats) {\n+\t\tBayerFormat sdBayerFormat = BayerFormat::fromMbusCode(code);\n+\t\tif (!sdBayerFormat.isValid())\n \t\t\tcontinue;\n \n-\t\tunsigned int rawCode = maliFormat.second;\n-\t\tconst auto rawSizes = sizes(rawCode);\n-\t\tif (rawSizes.empty())\n+\t\tif (sdBayerFormat.order != bayerFormat.order)\n \t\t\tcontinue;\n \n-\t\tBayerFormat bayer = BayerFormat::fromMbusCode(rawCode);\n-\t\tif (bayer.bitDepth > bitDepth) {\n-\t\t\tbitDepth = bayer.bitDepth;\n-\t\t\trawFormat = pixFmt;\n+\t\tif (sdBayerFormat.bitDepth > bitDepth) {\n+\t\t\tbitDepth = sdBayerFormat.bitDepth;\n+\t\t\tsensorMbusCode = code;\n+\t\t}\n+\t}\n+\n+\tif (!sensorMbusCode)\n+\t\treturn -EINVAL;\n+\n+\treturn sensorMbusCode;\n+}\n+\n+/*\n+ * Find a RAW PixelFormat supported by both the ISP and the sensor.\n+ *\n+ * The situation is mildly complicated by the fact that we expect the sensor to\n+ * output something like RAW8/10/12/16, but the ISP can only accept as input\n+ * RAW20 and can only produce as output RAW16. The one constant in that is the\n+ * bayer order of the data, so we'll simply check that the sensor produces a\n+ * format with a bayer order that matches that of one of the formats we support,\n+ * and select that.\n+ */\n+const PixelFormat &MaliC55CameraData::bestRawFormat() const\n+{\n+\tstatic const PixelFormat invalidPixFmt = {};\n+\n+\tfor (const auto &fmt : sd_->formats(0)) {\n+\t\tBayerFormat sensorBayer = BayerFormat::fromMbusCode(fmt.first);\n+\n+\t\tfor (const auto &[pixFmt, rawCode] : maliC55FmtToCode) {\n+\t\t\tif (!isFormatRaw(pixFmt))\n+\t\t\t\tcontinue;\n+\n+\t\t\tBayerFormat bayer = BayerFormat::fromMbusCode(rawCode);\n+\t\t\tif (bayer.order == sensorBayer.order)\n+\t\t\t\treturn pixFmt;\n \t\t}\n \t}\n \n-\treturn rawFormat;\n+\tLOG(MaliC55, Error) << \"Sensor doesn't provide a compatible format\";\n+\treturn invalidPixFmt;\n }\n \n /*\n@@ -243,13 +270,11 @@ PixelFormat MaliC55CameraData::bestRawFormat() const\n */\n PixelFormat MaliC55CameraData::adjustRawFormat(const PixelFormat &rawFmt) const\n {\n-\t/* Make sure the provided raw format is supported by the pipeline. */\n-\tauto it = maliC55FmtToCode.find(rawFmt);\n-\tif (it == maliC55FmtToCode.end())\n+\t/* Make sure the RAW mbus code is supported by the image source. */\n+\tint rawCode = pixfmtToMbusCode(rawFmt);\n+\tif (rawCode < 0)\n \t\treturn bestRawFormat();\n \n-\t/* Now make sure the RAW mbus code is supported by the image source. */\n-\tunsigned int rawCode = it->second;\n \tconst auto rawSizes = sizes(rawCode);\n \tif (rawSizes.empty())\n \t\treturn bestRawFormat();\n@@ -259,16 +284,14 @@ PixelFormat MaliC55CameraData::adjustRawFormat(const PixelFormat &rawFmt) const\n \n Size MaliC55CameraData::adjustRawSizes(const PixelFormat &rawFmt, const Size &size) const\n {\n-\t/* Just make sure the format is supported. */\n-\tauto it = maliC55FmtToCode.find(rawFmt);\n-\tif (it == maliC55FmtToCode.end())\n-\t\treturn {};\n-\n \t/* Expand the RAW size to the minimum ISP input size. */\n \tSize rawSize = size.expandedTo(kMaliC55MinInputSize);\n \n \t/* Check if the size is natively supported. */\n-\tunsigned int rawCode = it->second;\n+\tint rawCode = pixfmtToMbusCode(rawFmt);\n+\tif (rawCode < 0)\n+\t\treturn {};\n+\n \tconst auto rawSizes = sizes(rawCode);\n \tauto sizeIt = std::find(rawSizes.begin(), rawSizes.end(), rawSize);\n \tif (sizeIt != rawSizes.end())\n@@ -349,6 +372,10 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()\n \t\t */\n \t\tPixelFormat rawFormat =\n \t\t\tdata_->adjustRawFormat(rawConfig->pixelFormat);\n+\n+\t\tif (!rawFormat.isValid())\n+\t\t\treturn Invalid;\n+\n \t\tif (rawFormat != rawConfig->pixelFormat) {\n \t\t\tLOG(MaliC55, Debug)\n \t\t\t\t<< \"RAW format adjusted to \" << rawFormat;\n@@ -418,8 +445,7 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()\n \n \t/* If there's a RAW config, sensor configuration follows it. */\n \tif (rawConfig) {\n-\t\tconst auto it = maliC55FmtToCode.find(rawConfig->pixelFormat);\n-\t\tsensorFormat_.code = it->second;\n+\t\tsensorFormat_.code = data_->pixfmtToMbusCode(rawConfig->pixelFormat);\n \t\tsensorFormat_.size = rawConfig->size.expandedTo(minSensorSize);\n \n \t\treturn status;\n@@ -427,6 +453,9 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()\n \n \t/* If there's no RAW config, compute the sensor configuration here. */\n \tPixelFormat rawFormat = data_->bestRawFormat();\n+\tif (!rawFormat.isValid())\n+\t\treturn Invalid;\n+\n \tconst auto it = maliC55FmtToCode.find(rawFormat);\n \tsensorFormat_.code = it->second;\n \n@@ -614,7 +643,10 @@ PipelineHandlerMaliC55::generateConfiguration(Camera *camera,\n \n \t\t\tif (isRaw) {\n \t\t\t\t/* Make sure the mbus code is supported. */\n-\t\t\t\tunsigned int rawCode = maliFormat.second;\n+\t\t\t\tint rawCode = data->pixfmtToMbusCode(pixFmt);\n+\t\t\t\tif (rawCode < 0)\n+\t\t\t\t\tcontinue;\n+\n \t\t\t\tconst auto sizes = data->sizes(rawCode);\n \t\t\t\tif (sizes.empty())\n \t\t\t\t\tcontinue;\n", "prefixes": [ "v5", "10/13" ] }