[{"id":11291,"web_url":"https://patchwork.libcamera.org/comment/11291/","msgid":"<20200709140823.GS3875643@oden.dyn.berto.se>","date":"2020-07-09T14:08:23","subject":"Re: [libcamera-devel] [PATCH v2 19/20] lbcamera: ipu3: Configure\n\tImgU with the computed parameters","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your patch.\n\nOn 2020-07-09 10:41:27 +0200, Jacopo Mondi wrote:\n> Instrument the ImgUDevice::configureInput() function to use the provided\n> pipe configuration parameters to configure the IF, BDS and GDC\n> rectangles.\n> \n> Signed-off-by: Jacopo Mondi \n\ns/lbcamera/libcamera/ in Subject.\n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/pipeline/ipu3/imgu. | 0\n> src/libcamera/pipeline/ipu3/imgu.cpp | 35 ++++++++++++++++------------\n> src/libcamera/pipeline/ipu3/imgu.h | 2 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++-\n> 4 files changed, 23 insertions(+), 17 deletions(-)\n> create mode 100644 src/libcamera/pipeline/ipu3/imgu.\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu. b/src/libcamera/pipeline/ipu3/imgu.\n> new file mode 100644\n> index 000000000000..e69de29bb2d1\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index d1addb7d84d1..69bcc4f30962 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -441,11 +441,11 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> \n> /**\n> * \\brief Configure the ImgU unit input\n> - * \\param[in] size The ImgU input frame size\n> + * \\param[in] config The ImgU pipe configuration parameters\n> * \\param[in] inputFormat The format to be applied to ImgU input\n> * \\return 0 on success or a negative error code otherwise\n> */\n> -int ImgUDevice::configureInput(const Size &size,\n> +int ImgUDevice::configureInput(const PipeConfig &pipeConfig,\n> \t\t\t V4L2DeviceFormat *inputFormat)\n> {\n> \t/* Configure the ImgU input video device with the requested sizes. */\n> @@ -465,32 +465,37 @@ int ImgUDevice::configureInput(const Size &size,\n> \t * to configure the crop/compose rectangles, contradicting the\n> \t * V4L2 specification.\n> \t */\n> -\tRectangle rect = {\n> +\tRectangle iif = {\n> \t\t.x = 0,\n> \t\t.y = 0,\n> -\t\t.width = inputFormat->size.width,\n> -\t\t.height = inputFormat->size.height,\n> +\t\t.width = pipeConfig.iif.width,\n> +\t\t.height = pipeConfig.iif.height,\n> \t};\n> -\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &rect);\n> +\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &iif);\n> \tif (ret)\n> \t\treturn ret;\n> +\tLOG(IPU3, Debug) << \"ImgU input feeder rectangle = \" << iif.toString();\n> \n> -\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_COMPOSE, &rect);\n> +\tRectangle bds = {\n> +\t\t.x = 0,\n> +\t\t.y = 0,\n> +\t\t.width = pipeConfig.bds.width,\n> +\t\t.height = pipeConfig.bds.height,\n> +\t};\n> +\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_COMPOSE, &bds);\n> \tif (ret)\n> \t\treturn ret;\n> +\tLOG(IPU3, Debug) << \"ImgU BDS rectangle = \" << bds.toString();\n> \n> -\tLOG(IPU3, Debug) << \"ImgU input feeder and BDS rectangle = \"\n> -\t\t\t << rect.toString();\n> -\n> -\tV4L2SubdeviceFormat imguFormat = {};\n> -\timguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> -\timguFormat.size = size;\n> +\tV4L2SubdeviceFormat gdcFormat = {};\n> +\tgdcFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> +\tgdcFormat.size = pipeConfig.gdc;\n> \n> -\tret = imgu_->setFormat(PAD_INPUT, &imguFormat);\n> +\tret = imgu_->setFormat(PAD_INPUT, &gdcFormat);\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tLOG(IPU3, Debug) << \"ImgU GDC format = \" << imguFormat.toString();\n> +\tLOG(IPU3, Debug) << \"ImgU GDC format = \" << gdcFormat.toString();\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> index 15ee9a7f5698..6193c84bf35d 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.h\n> +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> @@ -45,7 +45,7 @@ public:\n> \n> \tPipeConfig calculatePipeConfig(Pipe *pipe);\n> \n> -\tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n> +\tint configureInput(const PipeConfig &pipeConfig, V4L2DeviceFormat *inputFormat);\n> \n> \tint configureOutput(const StreamConfiguration &cfg,\n> \t\t\t V4L2DeviceFormat *outputFormat)\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 72261d16e9f8..d33aa7aee5e5 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -66,6 +66,7 @@ public:\n> \tStatus validate() override;\n> \n> \tconst StreamConfiguration &cio2Format() const { return cio2Configuration_; };\n> +\tconst ImgUDevice::PipeConfig imguConfig() const { return pipeConfig_; }\n> \n> private:\n> \tIPU3CameraData *data_;\n> @@ -444,7 +445,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tret = imgu->configureInput(sensorSize, &cio2Format);\n> +\tret = imgu->configureInput(config->imguConfig(), &cio2Format);\n> \tif (ret)\n> \t\treturn ret;\n> \n> -- \n> 2.27.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 99FCABD792\n\tfor ;\n\tThu, 9 Jul 2020 14:08:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 12CF5612EA;\n\tThu, 9 Jul 2020 16:08:27 +0200 (CEST)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D144B611B2\n\tfor ;\n\tThu, 9 Jul 2020 16:08:25 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id t9so1225102lfl.5\n\tfor ;\n\tThu, 09 Jul 2020 07:08:25 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tn10sm959250lfb.82.2020.07.09.07.08.24\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 09 Jul 2020 07:08:24 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"j1wNm1ni\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=wMwZEjIIRovK7cKInuULbVGrnL0e3PomFO9QL9XtZak=;\n\tb=j1wNm1nivMc5EdQTxbvYDcYi82h+XBbh0Hl3wGX0ktp3ajgNYBVPo7iFfHoIYI0qYi\n\tkMF9ma0j6jhkWfHMYTbrY1dNOthnRUS3TJRePDZDjgHpT3B1SeHkCdkfaUuj9CcPP+Rv\n\tSeH2JF4WY1yHn90yLHoxQJETSb3scUCQbM61ogroldKOGi2QCM0c3S6N4WSCtCDBRK77\n\tp7Wn6AfxbGrEEvjznzT3A5yuIZ/ttcySMh4Xh27DLxVGmctmb69vprZGsrh5exMWDtLc\n\tRg1Rucaoqyys52CdCOyFSGn5S0M+liAfm7uvDPipaGqUw45Yak+Dag95axz85xbLzDqp\n\tTWtw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=wMwZEjIIRovK7cKInuULbVGrnL0e3PomFO9QL9XtZak=;\n\tb=op5Cx1r6HMPDGy2mGFWrAXClarsp4SpZxTt6CaaD2IZd46JatTVH5CVg+J5dUbxu6G\n\tmNxip+RyGZPwUFCcYpLAX4hPMd/iNgcdBa71vlu5jLjxxAJgYrdLVAlavozvaFzkaq8e\n\t8mmIqS+lElQEgBxBESO/Oxtvh7ZBF18XIUe9/1b2alTCiMgVfRpU3INlzmifpe5mvSCi\n\t7faclDFGaBOsdsoxdG7oCq2V9MyISFKvz6gK4V2H5aCuz7A1n0ufLkuG65fAHRoEtL9Q\n\tRuYJJajj741Jzab8EWzP5HwPVKo5C5XfJwUJlSHdW6Q/svpuP/v8LTlop6YVB4zK4mmD\n\tdANg==","X-Gm-Message-State":"AOAM533iegAYjLCzRABGHKX+FTarh/iLyQT2h2/1C15IpSABLjbBtz2B\n\tBjsfSazdxYs11vdQFa6YbUY9aA==","X-Google-Smtp-Source":"ABdhPJyjiKZNqEzj531jd4rFHJtSsqUfCxVy79UJX1YviHNmGEYC4eI2DIb9VXygnV9VPt8YJ4PBLA==","X-Received":"by 2002:ac2:5a01:: with SMTP id\n\tq1mr40357400lfn.182.1594303704909; \n\tThu, 09 Jul 2020 07:08:24 -0700 (PDT)","Date":"Thu, 9 Jul 2020 16:08:23 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"<20200709140823.GS3875643@oden.dyn.berto.se>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-20-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200709084128.5316-20-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 19/20] lbcamera: ipu3: Configure\n\tImgU with the computed parameters","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11335,"web_url":"https://patchwork.libcamera.org/comment/11335/","msgid":"<20200710125524.GW5964@pendragon.ideasonboard.com>","date":"2020-07-10T12:55:24","subject":"Re: [libcamera-devel] [PATCH v2 19/20] lbcamera: ipu3: Configure\n\tImgU with the computed parameters","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Jul 09, 2020 at 10:41:27AM +0200, Jacopo Mondi wrote:\n> Instrument the ImgUDevice::configureInput() function to use the provided\n> pipe configuration parameters to configure the IF, BDS and GDC\n> rectangles.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/imgu. | 0\n> src/libcamera/pipeline/ipu3/imgu.cpp | 35 ++++++++++++++++------------\n> src/libcamera/pipeline/ipu3/imgu.h | 2 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++-\n> 4 files changed, 23 insertions(+), 17 deletions(-)\n> create mode 100644 src/libcamera/pipeline/ipu3/imgu.\n> \n> diff --git a/src/libcamera/pipeline/ipu3/imgu. b/src/libcamera/pipeline/ipu3/imgu.\n> new file mode 100644\n> index 000000000000..e69de29bb2d1\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index d1addb7d84d1..69bcc4f30962 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -441,11 +441,11 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> \n> /**\n> * \\brief Configure the ImgU unit input\n> - * \\param[in] size The ImgU input frame size\n> + * \\param[in] config The ImgU pipe configuration parameters\n> * \\param[in] inputFormat The format to be applied to ImgU input\n> * \\return 0 on success or a negative error code otherwise\n> */\n> -int ImgUDevice::configureInput(const Size &size,\n> +int ImgUDevice::configureInput(const PipeConfig &pipeConfig,\n> \t\t\t V4L2DeviceFormat *inputFormat)\n> {\n> \t/* Configure the ImgU input video device with the requested sizes. */\n> @@ -465,32 +465,37 @@ int ImgUDevice::configureInput(const Size &size,\n> \t * to configure the crop/compose rectangles, contradicting the\n> \t * V4L2 specification.\n> \t */\n> -\tRectangle rect = {\n> +\tRectangle iif = {\n\nThe documentation mentions the IF module (Image Feeder), but the code\noften uses iif. What does iif stand for ?\n\n> \t\t.x = 0,\n> \t\t.y = 0,\n> -\t\t.width = inputFormat->size.width,\n> -\t\t.height = inputFormat->size.height,\n> +\t\t.width = pipeConfig.iif.width,\n> +\t\t.height = pipeConfig.iif.height,\n> \t};\n> -\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &rect);\n> +\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &iif);\n> \tif (ret)\n> \t\treturn ret;\n> +\tLOG(IPU3, Debug) << \"ImgU input feeder rectangle = \" << iif.toString();\n> \n> -\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_COMPOSE, &rect);\n> +\tRectangle bds = {\n> +\t\t.x = 0,\n> +\t\t.y = 0,\n> +\t\t.width = pipeConfig.bds.width,\n> +\t\t.height = pipeConfig.bds.height,\n> +\t};\n> +\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_COMPOSE, &bds);\n> \tif (ret)\n> \t\treturn ret;\n> +\tLOG(IPU3, Debug) << \"ImgU BDS rectangle = \" << bds.toString();\n> \n> -\tLOG(IPU3, Debug) << \"ImgU input feeder and BDS rectangle = \"\n> -\t\t\t << rect.toString();\n> -\n> -\tV4L2SubdeviceFormat imguFormat = {};\n> -\timguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> -\timguFormat.size = size;\n> +\tV4L2SubdeviceFormat gdcFormat = {};\n> +\tgdcFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> +\tgdcFormat.size = pipeConfig.gdc;\n> \n> -\tret = imgu_->setFormat(PAD_INPUT, &imguFormat);\n> +\tret = imgu_->setFormat(PAD_INPUT, &gdcFormat);\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tLOG(IPU3, Debug) << \"ImgU GDC format = \" << imguFormat.toString();\n> +\tLOG(IPU3, Debug) << \"ImgU GDC format = \" << gdcFormat.toString();\n\nThis change appears small, but knowing how much work went into the\ncalculation behind the scene, it's very nice to see the pieces fitting\ntogether. Great work !\n\nReviewed-by: Laurent Pinchart \n\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> index 15ee9a7f5698..6193c84bf35d 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.h\n> +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> @@ -45,7 +45,7 @@ public:\n> \n> \tPipeConfig calculatePipeConfig(Pipe *pipe);\n> \n> -\tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n> +\tint configureInput(const PipeConfig &pipeConfig, V4L2DeviceFormat *inputFormat);\n> \n> \tint configureOutput(const StreamConfiguration &cfg,\n> \t\t\t V4L2DeviceFormat *outputFormat)\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 72261d16e9f8..d33aa7aee5e5 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -66,6 +66,7 @@ public:\n> \tStatus validate() override;\n> \n> \tconst StreamConfiguration &cio2Format() const { return cio2Configuration_; };\n> +\tconst ImgUDevice::PipeConfig imguConfig() const { return pipeConfig_; }\n> \n> private:\n> \tIPU3CameraData *data_;\n> @@ -444,7 +445,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tret = imgu->configureInput(sensorSize, &cio2Format);\n> +\tret = imgu->configureInput(config->imguConfig(), &cio2Format);\n> \tif (ret)\n> \t\treturn ret;\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 2F3DBBD790\n\tfor ;\n\tFri, 10 Jul 2020 12:55:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3F74603AD;\n\tFri, 10 Jul 2020 14:55:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C9CF613AF\n\tfor ;\n\tFri, 10 Jul 2020 14:55:31 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 76A592C0;\n\tFri, 10 Jul 2020 14:55:30 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tHaBe6iQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594385730;\n\tbh=C3T3wz86qG8DWIjGDZN8qSCfF8naVyQ7ww/RYQyRDfI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tHaBe6iQRwQ10HctSrngg+RCX5MzzodQYKr+md6AfGoryPy4oi4JOGnSx9aFfS8zw\n\tzTEymCXvh+39TyVB/KFOnp6nC2P5tV8Cv9mboBZONvW4ZPJwCNlSsVup4af1q0QM7L\n\tI8d1PERh5QvrKM5hO9dBcYotCsOu0I0gXbf8Rgoc=","Date":"Fri, 10 Jul 2020 15:55:24 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200710125524.GW5964@pendragon.ideasonboard.com>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-20-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200709084128.5316-20-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 19/20] lbcamera: ipu3: Configure\n\tImgU with the computed parameters","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11371,"web_url":"https://patchwork.libcamera.org/comment/11371/","msgid":"<20200713131107.4bt4dvgym3bxqryy@uno.localdomain>","date":"2020-07-13T13:11:07","subject":"Re: [libcamera-devel] [PATCH v2 19/20] lbcamera: ipu3: Configure\n\tImgU with the computed parameters","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, Jul 10, 2020 at 03:55:24PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Thu, Jul 09, 2020 at 10:41:27AM +0200, Jacopo Mondi wrote:\n> > Instrument the ImgUDevice::configureInput() function to use the provided\n> > pipe configuration parameters to configure the IF, BDS and GDC\n> > rectangles.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/imgu. | 0\n> > src/libcamera/pipeline/ipu3/imgu.cpp | 35 ++++++++++++++++------------\n> > src/libcamera/pipeline/ipu3/imgu.h | 2 +-\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++-\n> > 4 files changed, 23 insertions(+), 17 deletions(-)\n> > create mode 100644 src/libcamera/pipeline/ipu3/imgu.\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/imgu. b/src/libcamera/pipeline/ipu3/imgu.\n> > new file mode 100644\n> > index 000000000000..e69de29bb2d1\n> > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > index d1addb7d84d1..69bcc4f30962 100644\n> > --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> > @@ -441,11 +441,11 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)\n> >\n> > /**\n> > * \\brief Configure the ImgU unit input\n> > - * \\param[in] size The ImgU input frame size\n> > + * \\param[in] config The ImgU pipe configuration parameters\n> > * \\param[in] inputFormat The format to be applied to ImgU input\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > -int ImgUDevice::configureInput(const Size &size,\n> > +int ImgUDevice::configureInput(const PipeConfig &pipeConfig,\n> > \t\t\t V4L2DeviceFormat *inputFormat)\n> > {\n> > \t/* Configure the ImgU input video device with the requested sizes. */\n> > @@ -465,32 +465,37 @@ int ImgUDevice::configureInput(const Size &size,\n> > \t * to configure the crop/compose rectangles, contradicting the\n> > \t * V4L2 specification.\n> > \t */\n> > -\tRectangle rect = {\n> > +\tRectangle iif = {\n>\n> The documentation mentions the IF module (Image Feeder), but the code\n> often uses iif. What does iif stand for ?\n\nif is a keyword, iif seems the most similar term I could find, and\nit's nicely aligned to the other two components names (bds and gdc)\nbeing 3 letters in lenght.\n\n>\n> > \t\t.x = 0,\n> > \t\t.y = 0,\n> > -\t\t.width = inputFormat->size.width,\n> > -\t\t.height = inputFormat->size.height,\n> > +\t\t.width = pipeConfig.iif.width,\n> > +\t\t.height = pipeConfig.iif.height,\n> > \t};\n> > -\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &rect);\n> > +\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_CROP, &iif);\n> > \tif (ret)\n> > \t\treturn ret;\n> > +\tLOG(IPU3, Debug) << \"ImgU input feeder rectangle = \" << iif.toString();\n> >\n> > -\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_COMPOSE, &rect);\n> > +\tRectangle bds = {\n> > +\t\t.x = 0,\n> > +\t\t.y = 0,\n> > +\t\t.width = pipeConfig.bds.width,\n> > +\t\t.height = pipeConfig.bds.height,\n> > +\t};\n> > +\tret = imgu_->setSelection(PAD_INPUT, V4L2_SEL_TGT_COMPOSE, &bds);\n> > \tif (ret)\n> > \t\treturn ret;\n> > +\tLOG(IPU3, Debug) << \"ImgU BDS rectangle = \" << bds.toString();\n> >\n> > -\tLOG(IPU3, Debug) << \"ImgU input feeder and BDS rectangle = \"\n> > -\t\t\t << rect.toString();\n> > -\n> > -\tV4L2SubdeviceFormat imguFormat = {};\n> > -\timguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> > -\timguFormat.size = size;\n> > +\tV4L2SubdeviceFormat gdcFormat = {};\n> > +\tgdcFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n> > +\tgdcFormat.size = pipeConfig.gdc;\n> >\n> > -\tret = imgu_->setFormat(PAD_INPUT, &imguFormat);\n> > +\tret = imgu_->setFormat(PAD_INPUT, &gdcFormat);\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n> > -\tLOG(IPU3, Debug) << \"ImgU GDC format = \" << imguFormat.toString();\n> > +\tLOG(IPU3, Debug) << \"ImgU GDC format = \" << gdcFormat.toString();\n>\n> This change appears small, but knowing how much work went into the\n> calculation behind the scene, it's very nice to see the pieces fitting\n> together. Great work !\n\nSlowly getting there! thanks!\n\n>\n> Reviewed-by: Laurent Pinchart \n>\n> >\n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> > index 15ee9a7f5698..6193c84bf35d 100644\n> > --- a/src/libcamera/pipeline/ipu3/imgu.h\n> > +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> > @@ -45,7 +45,7 @@ public:\n> >\n> > \tPipeConfig calculatePipeConfig(Pipe *pipe);\n> >\n> > -\tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n> > +\tint configureInput(const PipeConfig &pipeConfig, V4L2DeviceFormat *inputFormat);\n> >\n> > \tint configureOutput(const StreamConfiguration &cfg,\n> > \t\t\t V4L2DeviceFormat *outputFormat)\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 72261d16e9f8..d33aa7aee5e5 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -66,6 +66,7 @@ public:\n> > \tStatus validate() override;\n> >\n> > \tconst StreamConfiguration &cio2Format() const { return cio2Configuration_; };\n> > +\tconst ImgUDevice::PipeConfig imguConfig() const { return pipeConfig_; }\n> >\n> > private:\n> > \tIPU3CameraData *data_;\n> > @@ -444,7 +445,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n> > -\tret = imgu->configureInput(sensorSize, &cio2Format);\n> > +\tret = imgu->configureInput(config->imguConfig(), &cio2Format);\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 A5E92BD790\n\tfor ;\n\tMon, 13 Jul 2020 13:07:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 45E466057E;\n\tMon, 13 Jul 2020 15:07:38 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 863046057E\n\tfor ;\n\tMon, 13 Jul 2020 15:07:36 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id F09DAC000C;\n\tMon, 13 Jul 2020 13:07:35 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 13 Jul 2020 15:11:07 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200713131107.4bt4dvgym3bxqryy@uno.localdomain>","References":"<20200709084128.5316-1-jacopo@jmondi.org>\n\t<20200709084128.5316-20-jacopo@jmondi.org>\n\t<20200710125524.GW5964@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200710125524.GW5964@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 19/20] lbcamera: ipu3: Configure\n\tImgU with the computed parameters","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]