From patchwork Sat Jan 30 11:16:51 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11071 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A16EDBD808 for ; Sat, 30 Jan 2021 11:16:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 125C9683D0; Sat, 30 Jan 2021 12:16:58 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="XTqYNGiV"; dkim-atps=neutral Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 21EE968386 for ; Sat, 30 Jan 2021 12:16:57 +0100 (CET) Received: by mail-wr1-x42c.google.com with SMTP id s7so8406508wru.5 for ; Sat, 30 Jan 2021 03:16:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=F0HlmwnKvLLPjxoJ4oXCK8Gr9GJEr3oIi44/tsZyqwk=; b=XTqYNGiVQfY9tiar4ScdH1GPLKp4MpHMU7VT26K/0/rZRRp7vJsuSwGTdMrRUOyseN Yrb/Jp2rV6wnmprvZbE+t2rnc7hdz51RFg2OfcBSlGk3UO+t2YUh8g4LBWWEDuNdyx4J ap4Dvo0B8Jv4wd1rz2cVfpdFLAE4GxtRGvEZ6BuAmg5jgy44czvHw/7oPRmuv1oHR3Ry eOxFTbQWU9Csg+TOf60XYMy72zJgvTZ4m/cpddmh7jOEEyn5RsmR7t2+9F1OCu9Ue0Rp CUzWF6j1aITxnq4vmwvVAjCSjPzRQsD4PVjShFU4ZXSRcV/26OMgdHqHU190c6SNOX/p WWZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=F0HlmwnKvLLPjxoJ4oXCK8Gr9GJEr3oIi44/tsZyqwk=; b=MEYTB7LoB1um5aVQblHzIfRKFk98sklYdm/DO33Q+UleM6vomsXqV1Crm4RLltFTYy APrxm7jKRWVvcnaLgCEnfj9lJiCuBpU9yXtZjhgEFs44JMJVsg96RisqUQpRrR39yx38 j0MN6RgN5ZEvheZ96YN7PNaXh6o6u5jvF/7xVMx9+rc8dkUqF/h8luolMHNdLwpxKZLV SvGEG/VO9/HqeDtzF/I3RYrt7mchh3U4u+ioYyH+nDrn2Hqd4vfzp18LsLciv6YWOsct ppsCzKmvRXm0h0GwP6dLQ1hNGiaXQBKtu/I6AxAQtdyegrbFvUyg11TPW/SWmaRP9ELD CqwQ== X-Gm-Message-State: AOAM531n0/hC3WIrTJ9O8VjHzcMcw7o7G5Quxw8Q9f4bZr9AzSx1Kuiz Ct7nAzRNYMAK5yW9y7KPy56H6+ruPRsI868X X-Google-Smtp-Source: ABdhPJyg6qVPccQGap9rzCuzONOsjsgGuF86hcpK0axHghg70wGwLH3GVceDb+a95JFy+YSvZA/mZA== X-Received: by 2002:adf:fa8b:: with SMTP id h11mr9376595wrr.114.1612005416193; Sat, 30 Jan 2021 03:16:56 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id s4sm13853126wme.38.2021.01.30.03.16.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 30 Jan 2021 03:16:55 -0800 (PST) From: Naushir Patuck 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 use of RPi::ConfigParameters X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Rename the enum values to indicate pipeline handler -> IPA actions (IPA_CONFIG_*) and IPA -> pipeline handler return results (IPA_RESULT_*). Additionally, provide more descriptive names for these values. Fixup logic when handling IPA_RESULT_SENSOR_PARAMS where we must overwrite the parameters if provided by IPA. In the current codebase, this only happens once on startup, so there is no effective functional difference. But this change allows the option for the IPA to request new sensor parameters per-mode if required. Signed-off-by: Naushir Patuck Reviewed-by: Jacopo Mondi --- include/libcamera/ipa/raspberrypi.h | 10 +++--- src/ipa/raspberrypi/raspberrypi.cpp | 18 +++++------ .../pipeline/raspberrypi/raspberrypi.cpp | 31 +++++++++---------- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h index 5a9433825d5a..970b9e931188 100644 --- a/include/libcamera/ipa/raspberrypi.h +++ b/include/libcamera/ipa/raspberrypi.h @@ -20,11 +20,11 @@ namespace RPi { enum ConfigParameters { IPA_CONFIG_LS_TABLE = (1 << 0), - IPA_CONFIG_STAGGERED_WRITE = (1 << 1), - IPA_CONFIG_SENSOR = (1 << 2), - IPA_CONFIG_DROP_FRAMES = (1 << 3), - IPA_CONFIG_FAILED = (1 << 4), - IPA_CONFIG_STARTUP = (1 << 5), + IPA_CONFIG_STARTUP_CTRLS = (1 << 1), + IPA_RESULT_CONFIG_FAILED = (1 << 2), + IPA_RESULT_SENSOR_PARAMS = (1 << 3), + IPA_RESULT_SENSOR_CTRLS = (1 << 4), + IPA_RESULT_DROP_FRAMES = (1 << 5), }; enum Operations { diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 681ab9211b7c..fea1ea3957bb 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -170,7 +170,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result) ASSERT(result); result->operation = 0; - if (data.operation & RPi::IPA_CONFIG_STARTUP) { + if (data.operation & RPi::IPA_CONFIG_STARTUP_CTRLS) { /* We have been given some controls to action before start. */ queueRequest(data.controls[0]); } @@ -188,7 +188,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result) ControlList ctrls(sensorCtrls_); applyAGC(&agcStatus, ctrls); result->controls.emplace_back(ctrls); - result->operation |= RPi::IPA_CONFIG_SENSOR; + result->operation |= RPi::IPA_RESULT_SENSOR_CTRLS; } /* @@ -236,7 +236,7 @@ int IPARPi::start(const IPAOperationData &data, IPAOperationData *result) } result->data.push_back(dropFrame); - result->operation |= RPi::IPA_CONFIG_DROP_FRAMES; + result->operation |= RPi::IPA_RESULT_DROP_FRAMES; firstStart_ = false; @@ -289,7 +289,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, { if (entityControls.size() != 2) { LOG(IPARPI, Error) << "No ISP or sensor controls found."; - result->operation = RPi::IPA_CONFIG_FAILED; + result->operation = RPi::IPA_RESULT_CONFIG_FAILED; return; } @@ -300,13 +300,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, if (!validateSensorControls()) { LOG(IPARPI, Error) << "Sensor control validation failed."; - result->operation = RPi::IPA_CONFIG_FAILED; + result->operation = RPi::IPA_RESULT_CONFIG_FAILED; return; } if (!validateIspControls()) { LOG(IPARPI, Error) << "ISP control validation failed."; - result->operation = RPi::IPA_CONFIG_FAILED; + result->operation = RPi::IPA_RESULT_CONFIG_FAILED; return; } @@ -325,7 +325,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, if (!helper_) { LOG(IPARPI, Error) << "Could not create camera helper for " << cameraName; - result->operation = RPi::IPA_CONFIG_FAILED; + result->operation = RPi::IPA_RESULT_CONFIG_FAILED; return; } @@ -342,7 +342,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, result->data.push_back(exposureDelay); /* For VBLANK ctrl */ result->data.push_back(sensorMetadata); - result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE; + result->operation |= RPi::IPA_RESULT_SENSOR_PARAMS; } /* Re-assemble camera mode using the sensor info. */ @@ -395,7 +395,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, applyAGC(&agcStatus, ctrls); result->controls.emplace_back(ctrls); - result->operation |= RPi::IPA_CONFIG_SENSOR; + result->operation |= RPi::IPA_RESULT_SENSOR_CTRLS; } lastMode_ = mode_; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 5ad12d99638f..c44cb437a596 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -753,7 +753,7 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont IPAOperationData ipaData = {}; IPAOperationData result = {}; if (controls) { - ipaData.operation = RPi::IPA_CONFIG_STARTUP; + ipaData.operation = RPi::IPA_CONFIG_STARTUP_CTRLS; ipaData.controls.emplace_back(*controls); } ret = data->ipa_->start(ipaData, &result); @@ -765,12 +765,12 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont } /* Apply any gain/exposure settings that the IPA may have passed back. */ - if (result.operation & RPi::IPA_CONFIG_SENSOR) { + if (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) { ControlList &ctrls = result.controls[0]; data->unicam_[Unicam::Image].dev()->setControls(&ctrls); } - if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) { + if (result.operation & RPi::IPA_RESULT_DROP_FRAMES) { /* Configure the number of dropped frames required on startup. */ data->dropFrameCount_ = result.data[0]; } @@ -1213,31 +1213,28 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig, &result); - if (result.operation & RPi::IPA_CONFIG_FAILED) { + if (result.operation & RPi::IPA_RESULT_CONFIG_FAILED) { LOG(RPI, Error) << "IPA configuration failed!"; return -EPIPE; } unsigned int resultIdx = 0; - if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) { + if (result.operation & RPi::IPA_RESULT_SENSOR_PARAMS) { /* * Setup our delayed control writer with the sensor default * gain and exposure delays. */ - if (!delayedCtrls_) { - std::unordered_map delays = { - { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] }, - { V4L2_CID_EXPOSURE, result.data[resultIdx++] }, - { V4L2_CID_VBLANK, result.data[resultIdx++] } - }; - - delayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), delays); - - sensorMetadata_ = result.data[resultIdx++]; - } + std::unordered_map delays = { + { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] }, + { V4L2_CID_EXPOSURE, result.data[resultIdx++] }, + { V4L2_CID_VBLANK, result.data[resultIdx++] } + }; + + delayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), delays); + sensorMetadata_ = result.data[resultIdx++]; } - if (result.operation & RPi::IPA_CONFIG_SENSOR) { + if (result.operation & RPi::IPA_RESULT_SENSOR_CTRLS) { ControlList &ctrls = result.controls[0]; unicam_[Unicam::Image].dev()->setControls(&ctrls); }