Patch Detail
Show a patch.
GET /api/patches/11071/?format=api
{ "id": 11071, "url": "https://patchwork.libcamera.org/api/patches/11071/?format=api", "web_url": "https://patchwork.libcamera.org/patch/11071/", "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": "<20210130111651.1073981-1-naush@raspberrypi.com>", "date": "2021-01-30T11:16:51", "name": "[libcamera-devel] pipeline: ipa: raspberrypi: Rationalise use of RPi::ConfigParameters", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "22b41c57acf487b1a10b991c14dbea155acc21a6", "submitter": { "id": 34, "url": "https://patchwork.libcamera.org/api/people/34/?format=api", "name": "Naushir Patuck", "email": "naush@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/11071/mbox/", "series": [ { "id": 1630, "url": "https://patchwork.libcamera.org/api/series/1630/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1630", "date": "2021-01-30T11:16:51", "name": "[libcamera-devel] pipeline: ipa: raspberrypi: Rationalise use of RPi::ConfigParameters", "version": 1, "mbox": "https://patchwork.libcamera.org/series/1630/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/11071/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/11071/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 A16EDBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 30 Jan 2021 11:16:58 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 125C9683D0;\n\tSat, 30 Jan 2021 12:16:58 +0100 (CET)", "from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com\n\t[IPv6:2a00:1450:4864:20::42c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 21EE968386\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Jan 2021 12:16:57 +0100 (CET)", "by mail-wr1-x42c.google.com with SMTP id s7so8406508wru.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 30 Jan 2021 03:16:56 -0800 (PST)", "from naushir-VirtualBox.patuck.local ([88.97.76.4])\n\tby smtp.gmail.com with ESMTPSA id\n\ts4sm13853126wme.38.2021.01.30.03.16.54\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 30 Jan 2021 03:16:55 -0800 (PST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"XTqYNGiV\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:mime-version\n\t:content-transfer-encoding;\n\tbh=F0HlmwnKvLLPjxoJ4oXCK8Gr9GJEr3oIi44/tsZyqwk=;\n\tb=XTqYNGiVQfY9tiar4ScdH1GPLKp4MpHMU7VT26K/0/rZRRp7vJsuSwGTdMrRUOyseN\n\tYrb/Jp2rV6wnmprvZbE+t2rnc7hdz51RFg2OfcBSlGk3UO+t2YUh8g4LBWWEDuNdyx4J\n\tap4Dvo0B8Jv4wd1rz2cVfpdFLAE4GxtRGvEZ6BuAmg5jgy44czvHw/7oPRmuv1oHR3Ry\n\teOxFTbQWU9Csg+TOf60XYMy72zJgvTZ4m/cpddmh7jOEEyn5RsmR7t2+9F1OCu9Ue0Rp\n\tCUzWF6j1aITxnq4vmwvVAjCSjPzRQsD4PVjShFU4ZXSRcV/26OMgdHqHU190c6SNOX/p\n\tWWZw==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version\n\t:content-transfer-encoding;\n\tbh=F0HlmwnKvLLPjxoJ4oXCK8Gr9GJEr3oIi44/tsZyqwk=;\n\tb=MEYTB7LoB1um5aVQblHzIfRKFk98sklYdm/DO33Q+UleM6vomsXqV1Crm4RLltFTYy\n\tAPrxm7jKRWVvcnaLgCEnfj9lJiCuBpU9yXtZjhgEFs44JMJVsg96RisqUQpRrR39yx38\n\tj0MN6RgN5ZEvheZ96YN7PNaXh6o6u5jvF/7xVMx9+rc8dkUqF/h8luolMHNdLwpxKZLV\n\tSvGEG/VO9/HqeDtzF/I3RYrt7mchh3U4u+ioYyH+nDrn2Hqd4vfzp18LsLciv6YWOsct\n\tppsCzKmvRXm0h0GwP6dLQ1hNGiaXQBKtu/I6AxAQtdyegrbFvUyg11TPW/SWmaRP9ELD\n\tCqwQ==", "X-Gm-Message-State": "AOAM531n0/hC3WIrTJ9O8VjHzcMcw7o7G5Quxw8Q9f4bZr9AzSx1Kuiz\n\tCt7nAzRNYMAK5yW9y7KPy56H6+ruPRsI868X", "X-Google-Smtp-Source": "ABdhPJyg6qVPccQGap9rzCuzONOsjsgGuF86hcpK0axHghg70wGwLH3GVceDb+a95JFy+YSvZA/mZA==", "X-Received": "by 2002:adf:fa8b:: with SMTP id\n\th11mr9376595wrr.114.1612005416193; \n\tSat, 30 Jan 2021 03:16:56 -0800 (PST)", "From": "Naushir Patuck <naush@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 30 Jan 2021 11:16:51 +0000", "Message-Id": "<20210130111651.1073981-1-naush@raspberrypi.com>", "X-Mailer": "git-send-email 2.25.1", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Rationalise\n\tuse of RPi::ConfigParameters", "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>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Rename the enum values to indicate pipeline handler -> IPA actions\n(IPA_CONFIG_*) and IPA -> pipeline handler return results (IPA_RESULT_*).\nAdditionally, provide more descriptive names for these values.\n\nFixup logic when handling IPA_RESULT_SENSOR_PARAMS where we must\noverwrite the parameters if provided by IPA. In the current codebase,\nthis only happens once on startup, so there is no effective functional\ndifference. But this change allows the option for the IPA to request new\nsensor parameters per-mode if required.\n\nSigned-off-by: Naushir Patuck <naush@raspberrypi.com>\n---\n include/libcamera/ipa/raspberrypi.h | 10 +++---\n src/ipa/raspberrypi/raspberrypi.cpp | 18 +++++------\n .../pipeline/raspberrypi/raspberrypi.cpp | 31 +++++++++----------\n 3 files changed, 28 insertions(+), 31 deletions(-)", "diff": "diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\nindex 5a9433825d5a..970b9e931188 100644\n--- a/include/libcamera/ipa/raspberrypi.h\n+++ b/include/libcamera/ipa/raspberrypi.h\n@@ -20,11 +20,11 @@ namespace RPi {\n \n enum ConfigParameters {\n \tIPA_CONFIG_LS_TABLE = (1 << 0),\n-\tIPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n-\tIPA_CONFIG_SENSOR = (1 << 2),\n-\tIPA_CONFIG_DROP_FRAMES = (1 << 3),\n-\tIPA_CONFIG_FAILED = (1 << 4),\n-\tIPA_CONFIG_STARTUP = (1 << 5),\n+\tIPA_CONFIG_STARTUP_CTRLS = (1 << 1),\n+\tIPA_RESULT_CONFIG_FAILED = (1 << 2),\n+\tIPA_RESULT_SENSOR_PARAMS = (1 << 3),\n+\tIPA_RESULT_SENSOR_CTRLS = (1 << 4),\n+\tIPA_RESULT_DROP_FRAMES = (1 << 5),\n };\n \n enum Operations {\ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex 681ab9211b7c..fea1ea3957bb 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -170,7 +170,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)\n \n \tASSERT(result);\n \tresult->operation = 0;\n-\tif (data.operation & RPi::IPA_CONFIG_STARTUP) {\n+\tif (data.operation & RPi::IPA_CONFIG_STARTUP_CTRLS) {\n \t\t/* We have been given some controls to action before start. */\n \t\tqueueRequest(data.controls[0]);\n \t}\n@@ -188,7 +188,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)\n \t\tControlList ctrls(sensorCtrls_);\n \t\tapplyAGC(&agcStatus, ctrls);\n \t\tresult->controls.emplace_back(ctrls);\n-\t\tresult->operation |= RPi::IPA_CONFIG_SENSOR;\n+\t\tresult->operation |= RPi::IPA_RESULT_SENSOR_CTRLS;\n \t}\n \n \t/*\n@@ -236,7 +236,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result)\n \t}\n \n \tresult->data.push_back(dropFrame);\n-\tresult->operation |= RPi::IPA_CONFIG_DROP_FRAMES;\n+\tresult->operation |= RPi::IPA_RESULT_DROP_FRAMES;\n \n \tfirstStart_ = false;\n \n@@ -289,7 +289,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n {\n \tif (entityControls.size() != 2) {\n \t\tLOG(IPARPI, Error) << \"No ISP or sensor controls found.\";\n-\t\tresult->operation = RPi::IPA_CONFIG_FAILED;\n+\t\tresult->operation = RPi::IPA_RESULT_CONFIG_FAILED;\n \t\treturn;\n \t}\n \n@@ -300,13 +300,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \n \tif (!validateSensorControls()) {\n \t\tLOG(IPARPI, Error) << \"Sensor control validation failed.\";\n-\t\tresult->operation = RPi::IPA_CONFIG_FAILED;\n+\t\tresult->operation = RPi::IPA_RESULT_CONFIG_FAILED;\n \t\treturn;\n \t}\n \n \tif (!validateIspControls()) {\n \t\tLOG(IPARPI, Error) << \"ISP control validation failed.\";\n-\t\tresult->operation = RPi::IPA_CONFIG_FAILED;\n+\t\tresult->operation = RPi::IPA_RESULT_CONFIG_FAILED;\n \t\treturn;\n \t}\n \n@@ -325,7 +325,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \t\tif (!helper_) {\n \t\t\tLOG(IPARPI, Error) << \"Could not create camera helper for \"\n \t\t\t\t\t << cameraName;\n-\t\t\tresult->operation = RPi::IPA_CONFIG_FAILED;\n+\t\t\tresult->operation = RPi::IPA_RESULT_CONFIG_FAILED;\n \t\t\treturn;\n \t\t}\n \n@@ -342,7 +342,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \t\tresult->data.push_back(exposureDelay); /* For VBLANK ctrl */\n \t\tresult->data.push_back(sensorMetadata);\n \n-\t\tresult->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;\n+\t\tresult->operation |= RPi::IPA_RESULT_SENSOR_PARAMS;\n \t}\n \n \t/* Re-assemble camera mode using the sensor info. */\n@@ -395,7 +395,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \t\tapplyAGC(&agcStatus, ctrls);\n \n \t\tresult->controls.emplace_back(ctrls);\n-\t\tresult->operation |= RPi::IPA_CONFIG_SENSOR;\n+\t\tresult->operation |= RPi::IPA_RESULT_SENSOR_CTRLS;\n \t}\n \n \tlastMode_ = mode_;\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 5ad12d99638f..c44cb437a596 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -753,7 +753,7 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont\n \tIPAOperationData ipaData = {};\n \tIPAOperationData result = {};\n \tif (controls) {\n-\t\tipaData.operation = RPi::IPA_CONFIG_STARTUP;\n+\t\tipaData.operation = RPi::IPA_CONFIG_STARTUP_CTRLS;\n \t\tipaData.controls.emplace_back(*controls);\n \t}\n \tret = data->ipa_->start(ipaData, &result);\n@@ -765,12 +765,12 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont\n \t}\n \n \t/* Apply any gain/exposure settings that the IPA may have passed back. */\n-\tif (result.operation & RPi::IPA_CONFIG_SENSOR) {\n+\tif (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) {\n \t\tControlList &ctrls = result.controls[0];\n \t\tdata->unicam_[Unicam::Image].dev()->setControls(&ctrls);\n \t}\n \n-\tif (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) {\n+\tif (result.operation & RPi::IPA_RESULT_DROP_FRAMES) {\n \t\t/* Configure the number of dropped frames required on startup. */\n \t\tdata->dropFrameCount_ = result.data[0];\n \t}\n@@ -1213,31 +1213,28 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n \tipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,\n \t\t\t&result);\n \n-\tif (result.operation & RPi::IPA_CONFIG_FAILED) {\n+\tif (result.operation & RPi::IPA_RESULT_CONFIG_FAILED) {\n \t\tLOG(RPI, Error) << \"IPA configuration failed!\";\n \t\treturn -EPIPE;\n \t}\n \n \tunsigned int resultIdx = 0;\n-\tif (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) {\n+\tif (result.operation & RPi::IPA_RESULT_SENSOR_PARAMS) {\n \t\t/*\n \t\t * Setup our delayed control writer with the sensor default\n \t\t * gain and exposure delays.\n \t\t */\n-\t\tif (!delayedCtrls_) {\n-\t\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n-\t\t\t\t{ V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },\n-\t\t\t\t{ V4L2_CID_EXPOSURE, result.data[resultIdx++] },\n-\t\t\t\t{ V4L2_CID_VBLANK, result.data[resultIdx++] }\n-\t\t\t};\n-\n-\t\t\tdelayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);\n-\n-\t\t\tsensorMetadata_ = result.data[resultIdx++];\n-\t\t}\n+\t\tstd::unordered_map<uint32_t, unsigned int> delays = {\n+\t\t\t{ V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },\n+\t\t\t{ V4L2_CID_EXPOSURE, result.data[resultIdx++] },\n+\t\t\t{ V4L2_CID_VBLANK, result.data[resultIdx++] }\n+\t\t};\n+\n+\t\tdelayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);\n+\t\tsensorMetadata_ = result.data[resultIdx++];\n \t}\n \n-\tif (result.operation & RPi::IPA_CONFIG_SENSOR) {\n+\tif (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) {\n \t\tControlList &ctrls = result.controls[0];\n \t\tunicam_[Unicam::Image].dev()->setControls(&ctrls);\n \t}\n", "prefixes": [ "libcamera-devel" ] }