From patchwork Fri Jul 31 15:33:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 9098 X-Patchwork-Delegate: jacopo@jmondi.org 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 B5D11BD86F for ; Fri, 31 Jul 2020 15:29:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 42A7461A14; Fri, 31 Jul 2020 17:29:48 +0200 (CEST) Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 40F9B60398 for ; Fri, 31 Jul 2020 17:29:46 +0200 (CEST) Received: from uno.lan (93-34-118-233.ip49.fastwebnet.it [93.34.118.233]) (Authenticated sender: jacopo@jmondi.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id B4A02240004; Fri, 31 Jul 2020 15:29:45 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Fri, 31 Jul 2020 17:33:01 +0200 Message-Id: <20200731153320.58107-1-jacopo@jmondi.org> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v5 00/19] ipu3: rework pipe confiuguration 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" I have addressed minor comments and added a few more details in comment and documentation here and there. I have simplified a bit the ImgU pipe configuration by removing not-required casts, by making the code a bit less dense, and with a small rename. The diff between v4 and v5 is so minimal I reported the full diff below. I tested results against the IPU3 pipe configuration tool by instrumenting a version of it to run on a set of known values and compare the results between its calculation and libcamera: https://jmondi.org/cgit/intel-ipu3-pipe-config/commit/?id=78bc59f8bfe8c14908f612008a62edc621660cf3 All patches reviewed but I'm not pushing yet as I would like Kieran's ack, as he's working on JPEG and moving the ground under his feet while developing seems not nice. Thanks j Jacopo Mondi (19): libcamera: ipu3: Rename mbusCodesToInfo libcamera: utils: Add alignUp and alignDown functions libcamera: geometry: Add isNull() function to Rectangle class libcamera: ipu3: Remove streams from generateConfiguration libcamera: ipu3: Make sure the config is valid libcamera: ipu3: cio2: Report format and sizes libcamera: ipu3: Do not overwrite StreamConfiguration libcamera: ipu3: Report StreamFormats libcamera: ipu3: Remove initialization of Size libcamera: ipu3: Validate the stream combination libcamera: camera: Zero streams before validate() libcamera: ipu3: cio2: Add a const sensor() method libcamera: ipu3: Adjust and assign streams in validate() libcamera: ipu3: Remove streams from IPU3CameraConfiguration libcamera: ipu3: Remove camera_ from IPU3CameraConfiguration libcamera: ipu3: imgu: Calculate ImgU pipe configuration libcamera: ipu3: Validate the pipe configuration libcamera: ipu3: Configure ImgU with the computed parameters libcamera: ipu3: imgu: Rename configureInput() include/libcamera/geometry.h | 1 + include/libcamera/internal/utils.h | 10 + src/libcamera/camera.cpp | 4 +- src/libcamera/geometry.cpp | 6 + src/libcamera/pipeline/ipu3/cio2.cpp | 54 +++- src/libcamera/pipeline/ipu3/cio2.h | 6 + src/libcamera/pipeline/ipu3/imgu. | 0 src/libcamera/pipeline/ipu3/imgu.cpp | 386 +++++++++++++++++++++++- src/libcamera/pipeline/ipu3/imgu.h | 22 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 424 +++++++++++++-------------- src/libcamera/utils.cpp | 16 + test/geometry.cpp | 14 + test/utils.cpp | 11 + 13 files changed, 714 insertions(+), 240 deletions(-) create mode 100644 src/libcamera/pipeline/ipu3/imgu. --- 2.27.0 diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index a3ea3eabe318..cf26980c75f1 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -25,6 +25,14 @@ LOG_DECLARE_CATEGORY(IPU3) namespace { +/* + * The procedure to calculate the ImgU pipe configuration has been ported + * from the pipe_config.py python script, available at: + * https://github.com/intel/intel-ipu3-pipecfg + * at revision: 61e83f2f7606 ("Add more information into README") + */ + static constexpr unsigned int FILTER_H = 4; static constexpr unsigned int IF_ALIGN_W = 2; @@ -103,10 +111,8 @@ float findScaleFactor(float sf, const std::vector &range, bool isSameRatio(const Size &in, const Size &out) { - float inRatio = static_cast(in.width) / - static_cast(in.height); - float outRatio = static_cast(out.width) / - static_cast(out.height); + float inRatio = static_cast(in.width) / in.height; + float outRatio = static_cast(out.width) / out.height; if (std::abs(inRatio - outRatio) > 0.1) return false; @@ -123,16 +129,18 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc float bdsHeight; if (!isSameRatio(pipe->input, gdc)) { - bool found = false; - float estIFHeight = static_cast(iif.width * gdc.height) / + float estIFHeight = (iif.width * gdc.height) / static_cast(gdc.width); estIFHeight = utils::clamp(estIFHeight, minIFHeight, iif.height); + bool found = false; + ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H); - while (ifHeight >= minIFHeight && - static_cast(ifHeight) / bdsSF >= minBDSHeight) { - bdsHeight = static_cast(ifHeight) / bdsSF; + while (ifHeight >= minIFHeight && ifHeight / bdsSF >= minBDSHeight) { + + bdsHeight = ifHeight / bdsSF; if (std::fmod(bdsHeight, 1.0) == 0) { unsigned int bdsIntHeight = static_cast(bdsHeight); + if (!(bdsIntHeight % BDS_ALIGN_H)) { found = true; break; @@ -143,11 +151,12 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc } ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H); - while (ifHeight <= iif.height && - static_cast(ifHeight) / bdsSF >= minBDSHeight) { - bdsHeight = static_cast(ifHeight) / bdsSF; + while (ifHeight <= iif.height && ifHeight / bdsSF >= minBDSHeight) { + + bdsHeight = ifHeight / bdsSF; if (std::fmod(bdsHeight, 1.0) == 0) { unsigned int bdsIntHeight = static_cast(bdsHeight); + if (!(bdsIntHeight % BDS_ALIGN_H)) { found = true; break; @@ -159,14 +168,15 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc if (found) { unsigned int bdsIntHeight = static_cast(bdsHeight); + pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight }, { bdsWidth, bdsIntHeight }, gdc }); return; } } else { ifHeight = utils::alignUp(iif.height, IF_ALIGN_H); - while (ifHeight > minIFHeight && - static_cast(ifHeight) / bdsSF >= minBDSHeight) { + while (ifHeight > minIFHeight && ifHeight / bdsSF >= minBDSHeight) { + bdsHeight = ifHeight / bdsSF; if (std::fmod(ifHeight, 1.0) == 0 && std::fmod(bdsHeight, 1.0) == 0) { unsigned int bdsIntHeight = static_cast(bdsHeight); @@ -183,8 +193,7 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc } } -void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, - float bdsSF) +void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, float bdsSF) { unsigned int minBDSWidth = gdc.width + FILTER_H * 2; @@ -218,15 +227,14 @@ void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, Size calculateGDC(ImgUDevice::Pipe *pipe) { const Size &in = pipe->input; - const Size &main = pipe->output; + const Size &main = pipe->main; const Size &vf = pipe->viewfinder; Size gdc; if (!vf.isNull()) { gdc.width = main.width; - float ratio = static_cast(main.width * vf.height) / - static_cast(vf.width); + float ratio = (main.width * vf.height) / static_cast(vf.width); gdc.height = std::max(static_cast(main.height), ratio); return gdc; @@ -237,14 +245,13 @@ Size calculateGDC(ImgUDevice::Pipe *pipe) return gdc; } - float totalSF = static_cast(in.width) / - static_cast(main.width); + float totalSF = static_cast(in.width) / main.width; float bdsSF = totalSF > 2 ? 2 : 1; float yuvSF = totalSF / bdsSF; float sf = findScaleFactor(yuvSF, gdcScalingFactors); - gdc.width = static_cast(main.width) * sf; - gdc.height = static_cast(main.height) * sf; + gdc.width = main.width * sf; + gdc.height = main.height * sf; return gdc; } @@ -259,6 +266,7 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe) float ifCropH = static_cast(in.height - pipe.iif.height); float gdcCropW = static_cast(pipe.bds.width - pipe.gdc.width) * pipe.bds_sf; float gdcCropH = static_cast(pipe.bds.height - pipe.gdc.height) * pipe.bds_sf; + fov.w = (inW - (ifCropW + gdcCropW)) / inW; fov.h = (inH - (ifCropH + gdcCropH)) / inH; @@ -304,11 +312,11 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe) * \var Pipe::input * \brief The input image size * - * \var Pipe::output + * \var Pipe::main * \brief The requested main output size * * \var Pipe::viewfinder - * \brief The requested viewfinder size + * \brief The requested viewfinder output size */ /** @@ -378,7 +386,7 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe) LOG(IPU3, Debug) << "Calculating pipe configuration for: "; LOG(IPU3, Debug) << "input: " << pipe->input.toString(); - LOG(IPU3, Debug) << "main: " << pipe->output.toString(); + LOG(IPU3, Debug) << "main: " << pipe->main.toString(); LOG(IPU3, Debug) << "vf: " << pipe->viewfinder.toString(); const Size &in = pipe->input; @@ -387,9 +395,9 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe) unsigned int ifWidth = utils::alignUp(in.width, IF_ALIGN_W); unsigned int ifHeight = in.height; unsigned int minIfWidth = in.width - IF_CROP_MAX_W; - float bdsSF = static_cast(in.width) / - static_cast(gdc.width); + float bdsSF = static_cast(in.width) / gdc.width; float sf = findScaleFactor(bdsSF, bdsScalingFactors, true); + while (ifWidth >= minIfWidth) { Size iif{ ifWidth, ifHeight }; calculateBDS(pipe, iif, gdc, sf); diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h index ff8dab7d645c..c73ac5a5a37c 100644 --- a/src/libcamera/pipeline/ipu3/imgu.h +++ b/src/libcamera/pipeline/ipu3/imgu.h @@ -37,7 +37,7 @@ public: struct Pipe { Size input; - Size output; + Size main; Size viewfinder; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 06afc7d79dac..161a88da8497 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -225,11 +225,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() * margins from the CIO2 output frame size. * * The ImgU outputs needs to be strictly smaller than - * the CIO2 output frame and rounded down to 64 - * pixels in width and 32 pixels in height. This comes - * from inspecting the pipe configuration script results - * and the available suggested configurations in the - * ChromeOS BSP .xml camera tuning files. + * the CIO2 output frame and rounded down to 64 pixels + * in width and 32 pixels in height. This assumption + * comes from inspecting the pipe configuration script + * results and the available suggested configurations in + * the ChromeOS BSP .xml camera tuning files and shall + * be validated. * * \todo Clarify what are the hardware constraints * that require this alignements, if any. It might @@ -267,9 +268,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() cfg->setStream(const_cast(&data_->outStream_)); mainOutputAvailable = false; - pipe.output = cfg->size; + pipe.main = cfg->size; if (yuvCount == 1) - pipe.viewfinder = pipe.output; + pipe.viewfinder = pipe.main; LOG(IPU3, Debug) << "Assigned " << cfg->toString() << " to the main output"; @@ -333,8 +334,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, * to the ImgU maximum output size) and aligned down to * the required frame margin. * - * The alignment constraints should be clarified - * as per the todo item in validate(). + * \todo Clarify the alignment constraints as exaplained + * in validate() */ size = sensorResolution.boundedTo(IMGU_OUTPUT_MAX_SIZE); size.width = utils::alignDown(size.width - 1,