[{"id":28833,"web_url":"https://patchwork.libcamera.org/comment/28833/","msgid":"","date":"2024-03-04T17:41:03","subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote:\n> The CameraSensor class tests if the sensor HBLANK control is read-only\n> by comparing the minimum and maximum values, and documents this as being\n> a workaround for the lack of a read-only control flag in V4L2. This is\n> incorrect, as the V4L2 API provides such a flag. Use it to replace the\n> workaround.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/sensor/camera_sensor.cpp | 27 +++++++-------------------\n> 1 file changed, 7 insertions(+), 20 deletions(-)\n>\n> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> index 86ad9a85371c..402025566544 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -188,28 +188,15 @@ int CameraSensor::init()\n> \t * Set HBLANK to the minimum to start with a well-defined line length,\n> \t * allowing IPA modules that do not modify HBLANK to use the sensor\n> \t * minimum line length in their calculations.\n> -\t *\n> -\t * At present, there is no way of knowing if a control is read-only.\n> -\t * As a workaround, assume that if the minimum and maximum values of\n> -\t * the V4L2_CID_HBLANK control are the same, it implies the control\n> -\t * is read-only.\n> -\t *\n> -\t * \\todo The control API ought to have a flag to specify if a control\n> -\t * is read-only which could be used below.\n> \t */\n\nWeird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this\nhas been introduced. Maybe the API we had to check flags was (or is)\nnot the best one and we decided to compare values ?\n\n> -\tif (ctrls.infoMap()->find(V4L2_CID_HBLANK) != ctrls.infoMap()->end()) {\n> -\t\tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> -\t\tconst int32_t hblankMin = hblank.min().get();\n> -\t\tconst int32_t hblankMax = hblank.max().get();\n> +\tconst struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK);\n> +\tif (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {\n> +\t\tControlList ctrl(subdev_->controls());\n>\n\nThis could be fast-tracked too!\n\nReviewed-by: Jacopo Mondi \n\n> -\t\tif (hblankMin != hblankMax) {\n> -\t\t\tControlList ctrl(subdev_->controls());\n> -\n> -\t\t\tctrl.set(V4L2_CID_HBLANK, hblankMin);\n> -\t\t\tret = subdev_->setControls(&ctrl);\n> -\t\t\tif (ret)\n> -\t\t\t\treturn ret;\n> -\t\t}\n> +\t\tctrl.set(V4L2_CID_HBLANK, static_cast(hblankInfo->minimum));\n> +\t\tret = subdev_->setControls(&ctrl);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> \t}\n>\n> \treturn applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> --\n> Regards,\n>\n> Laurent Pinchart\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 B9C81C326B\n\tfor ;\n\tMon, 4 Mar 2024 17:41:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F01F762871;\n\tMon, 4 Mar 2024 18:41: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 9A912627FC\n\tfor ;\n\tMon, 4 Mar 2024 18:41:07 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1F0D62E7;\n\tMon, 4 Mar 2024 18:40:51 +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=\"bfuokPN6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709574051;\n\tbh=0StuWs/MHKa28+rvTvRt7+QZM3I/YoiKZOshjFtflLU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bfuokPN6AFAZnSP24eQ5TxmIh98KlS3aOeIyZyjUxs8hl8MYbFBdjRvZJvlwUU3M5\n\taipcj98vwnTGyCAhILC/J50AT8TKgCMIUPrs9XF3bB1KQfSLUzxkMgbQL1xsH/8CyB\n\tE/RUBCoH87/+QaSnE8MDtlOlvZiQ89HiOPFIqXl8=","Date":"Mon, 4 Mar 2024 18:41:03 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","Message-ID":"","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-18-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240301212121.9072-18-laurent.pinchart@ideasonboard.com>","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, Sakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28841,"web_url":"https://patchwork.libcamera.org/comment/28841/","msgid":"<20240304224901.GG9233@pendragon.ideasonboard.com>","date":"2024-03-04T22:49:01","subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote:\n> Hi Laurent\n> \n> On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote:\n> > The CameraSensor class tests if the sensor HBLANK control is read-only\n> > by comparing the minimum and maximum values, and documents this as being\n> > a workaround for the lack of a read-only control flag in V4L2. This is\n> > incorrect, as the V4L2 API provides such a flag. Use it to replace the\n> > workaround.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++-------------------\n> > 1 file changed, 7 insertions(+), 20 deletions(-)\n> >\n> > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> > index 86ad9a85371c..402025566544 100644\n> > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > @@ -188,28 +188,15 @@ int CameraSensor::init()\n> > \t * Set HBLANK to the minimum to start with a well-defined line length,\n> > \t * allowing IPA modules that do not modify HBLANK to use the sensor\n> > \t * minimum line length in their calculations.\n> > -\t *\n> > -\t * At present, there is no way of knowing if a control is read-only.\n> > -\t * As a workaround, assume that if the minimum and maximum values of\n> > -\t * the V4L2_CID_HBLANK control are the same, it implies the control\n> > -\t * is read-only.\n> > -\t *\n> > -\t * \\todo The control API ought to have a flag to specify if a control\n> > -\t * is read-only which could be used below.\n> > \t */\n> \n> Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this\n> has been introduced. Maybe the API we had to check flags was (or is)\n> not the best one and we decided to compare values ?\n\nIt puzzled me too, I really don't recall why we didn't use it. Naush,\ndoes it ring a bell ?\n\n> > -\tif (ctrls.infoMap()->find(V4L2_CID_HBLANK) != ctrls.infoMap()->end()) {\n> > -\t\tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> > -\t\tconst int32_t hblankMin = hblank.min().get();\n> > -\t\tconst int32_t hblankMax = hblank.max().get();\n> > +\tconst struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK);\n> > +\tif (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {\n> > +\t\tControlList ctrl(subdev_->controls());\n> \n> This could be fast-tracked too!\n> \n> Reviewed-by: Jacopo Mondi \n> \n> > -\t\tif (hblankMin != hblankMax) {\n> > -\t\t\tControlList ctrl(subdev_->controls());\n> > -\n> > -\t\t\tctrl.set(V4L2_CID_HBLANK, hblankMin);\n> > -\t\t\tret = subdev_->setControls(&ctrl);\n> > -\t\t\tif (ret)\n> > -\t\t\t\treturn ret;\n> > -\t\t}\n> > +\t\tctrl.set(V4L2_CID_HBLANK, static_cast(hblankInfo->minimum));\n> > +\t\tret = subdev_->setControls(&ctrl);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > \t}\n> >\n> > \treturn applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);","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 7850DBD160\n\tfor ;\n\tMon, 4 Mar 2024 22:49:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 27A276286C;\n\tMon, 4 Mar 2024 23:49:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 029D562867\n\tfor ;\n\tMon, 4 Mar 2024 23:49:00 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6212D22E8;\n\tMon, 4 Mar 2024 23:48:43 +0100 (CET)"],"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=\"BsmLwEWO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709592523;\n\tbh=s6v7cKNzjaaYGfDet5Cf72M02hJTw3ZldIdFC2nhHO4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BsmLwEWOzRhsPP3zDLn5vxFl6zlt3lK50DfB8u+ss9v5bqZ4vyuMhTNXAkBAARWCz\n\tXJ1xbBgP/znj7zBtvwfmMXjK4XWQKhOHKZBV0WA5QD3Ro8lwQ6qq8LvGSal1yxBQTE\n\tmRiypy7ba0Up8RoGyArsUi2paT2cMtDjnOhqhrqQ=","Date":"Tue, 5 Mar 2024 00:49:01 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","Message-ID":"<20240304224901.GG9233@pendragon.ideasonboard.com>","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-18-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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, Sakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28845,"web_url":"https://patchwork.libcamera.org/comment/28845/","msgid":"","date":"2024-03-05T07:09:58","subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Mon, 4 Mar 2024, 10:49 pm Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote:\n> > Hi Laurent\n> >\n> > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote:\n> > > The CameraSensor class tests if the sensor HBLANK control is read-only\n> > > by comparing the minimum and maximum values, and documents this as\n> being\n> > > a workaround for the lack of a read-only control flag in V4L2. This is\n> > > incorrect, as the V4L2 API provides such a flag. Use it to replace the\n> > > workaround.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++-------------------\n> > > 1 file changed, 7 insertions(+), 20 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/sensor/camera_sensor.cpp\n> b/src/libcamera/sensor/camera_sensor.cpp\n> > > index 86ad9a85371c..402025566544 100644\n> > > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > > @@ -188,28 +188,15 @@ int CameraSensor::init()\n> > > * Set HBLANK to the minimum to start with a well-defined line\n> length,\n> > > * allowing IPA modules that do not modify HBLANK to use the sensor\n> > > * minimum line length in their calculations.\n> > > - *\n> > > - * At present, there is no way of knowing if a control is\n> read-only.\n> > > - * As a workaround, assume that if the minimum and maximum values\n> of\n> > > - * the V4L2_CID_HBLANK control are the same, it implies the control\n> > > - * is read-only.\n> > > - *\n> > > - * \\todo The control API ought to have a flag to specify if a\n> control\n> > > - * is read-only which could be used below.\n> > > */\n> >\n> > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this\n> > has been introduced. Maybe the API we had to check flags was (or is)\n> > not the best one and we decided to compare values ?\n>\n> It puzzled me too, I really don't recall why we didn't use it. Naush,\n> does it ring a bell ?\n>\n\nMy patch at https://patchwork.libcamera.org/patch/17936/ did make use of\nthis flag. But it never got merged.\n\n\n\n> > > - if (ctrls.infoMap()->find(V4L2_CID_HBLANK) !=\n> ctrls.infoMap()->end()) {\n> > > - const ControlInfo hblank =\n> ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> > > - const int32_t hblankMin = hblank.min().get();\n> > > - const int32_t hblankMax = hblank.max().get();\n> > > + const struct v4l2_query_ext_ctrl *hblankInfo =\n> subdev_->controlInfo(V4L2_CID_HBLANK);\n> > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY))\n> {\n> > > + ControlList ctrl(subdev_->controls());\n> >\n> > This could be fast-tracked too!\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > > - if (hblankMin != hblankMax) {\n> > > - ControlList ctrl(subdev_->controls());\n> > > -\n> > > - ctrl.set(V4L2_CID_HBLANK, hblankMin);\n> > > - ret = subdev_->setControls(&ctrl);\n> > > - if (ret)\n> > > - return ret;\n> > > - }\n> > > + ctrl.set(V4L2_CID_HBLANK,\n> static_cast(hblankInfo->minimum));\n> > > + ret = subdev_->setControls(&ctrl);\n> > > + if (ret)\n> > > + return ret;\n> > > }\n> > >\n> > > return\n> applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 BA145C326B\n\tfor ;\n\tTue, 5 Mar 2024 07:10:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9D0196286F;\n\tTue, 5 Mar 2024 08:10:12 +0100 (CET)","from mail-yw1-x112e.google.com (mail-yw1-x112e.google.com\n\t[IPv6:2607:f8b0:4864:20::112e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 12F2B61C83\n\tfor ;\n\tTue, 5 Mar 2024 08:10:11 +0100 (CET)","by mail-yw1-x112e.google.com with SMTP id\n\t00721157ae682-608ccac1899so56956107b3.1\n\tfor ;\n\tMon, 04 Mar 2024 23:10:10 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"iLi6BlPq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1709622610; x=1710227410;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=Ykf8SCNJdTTNPik5wiRcMEs7D/EIs3CNO8d5RhNSMkA=;\n\tb=iLi6BlPqcX22L2W8i6rlFrX8Ox2wBAe53FBTwu7Bk77JMd09zvUDdjqnXWH6WTFBa7\n\tTnbrsMIaLXIq1lTc/qDcwlBpLeaiIdp5jL1djwZt0WaZrRRiKqCdtajrKB4dnLvRtvHB\n\tX/Kvb6YmQJ3ieTeFNdWHSGyT55gye2cLTJ6hYNuR48Vw+Vc4g6wkMpVRtsdPUjxbtaUL\n\tSZAy7pfiDBIG+V/9tMAiqZbbd5NEJyqMib1FD6aakCcODv+KA8Aw8hcqyXAklTTnZ14v\n\tgf2AXg0diVbi1Jr6BPFl7LTWsoDmraHGIXEfHtM9e7bkCBiS2PFgQN1rgh+OhKUEC6al\n\tCVyg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1709622610; x=1710227410;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=Ykf8SCNJdTTNPik5wiRcMEs7D/EIs3CNO8d5RhNSMkA=;\n\tb=j6iBivD3Ab/xEwehnGUJ9flQhHH6diToMLljPCWCwO8Sh/rsPiArRty9d1XnHFGz0v\n\tz4d8lkZhyNpUUb0jg4qNMvB7dCbqqFWYo6xiF4vT+nuQlImJbzMgwtRVljQTAHmzxlp+\n\taQPDAnqemAOWcyR82p32hWYIEMW48CP0ffItSBX+kjf2kBhv8LUrfn7F4uppTFsbFpL1\n\tlQlgki0FGsn6rLlNUz9Meddz+heSalFdlQ1ufyuXAHu9SUR7W0V3ugsAWBsNSg77q/Gx\n\tPzktZDgYIzCYjjb3Q4SLYwxEaddSc+u9k16n59ifjuBkhu896QVwEa9Vw5k7rzgFiLMU\n\t1HIQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXyNVywkFZpnfNO+9YMkT2qQ7aHZLImgfeC/wRpciap7SWKRsLvgSomyxxU+esXPlZnE5GtJPG2TwyhhjCRSN/8YqnulthgApmdkHhJJ00Bymc4Bg==","X-Gm-Message-State":"AOJu0YzzYIFOeYe6wESKAvSdZlxgQn67scTedyrDe3BaTWfb77QKUO3W\n\tre+O4/3Q+mXMFW5eci94HhiHggAHigKwCCz0ALYrijZ8oaLh2NhRUIOtSpEvRObeNiAMwIEeSel\n\tMgreY/Y+2n6P8hqR/OsrZSVhs2sRy8vtpuY5seA==","X-Google-Smtp-Source":"AGHT+IGVszeDYxXgnDARHxLXTEitJtGX3HiBoucDkeQhwoWc0kflk3efxh5YyeMDhw+eU+B0aWG8kHx8zKhquhtQaD4=","X-Received":"by 2002:a81:5c88:0:b0:5ff:7cca:a434 with SMTP id\n\tq130-20020a815c88000000b005ff7ccaa434mr10777227ywb.51.1709622609729;\n\tMon, 04 Mar 2024 23:10:09 -0800 (PST)","MIME-Version":"1.0","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-18-laurent.pinchart@ideasonboard.com>\n\t\n\t<20240304224901.GG9233@pendragon.ideasonboard.com>","In-Reply-To":"<20240304224901.GG9233@pendragon.ideasonboard.com>","From":"Naushir Patuck ","Date":"Tue, 5 Mar 2024 07:09:58 +0000","Message-ID":"","Subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"0000000000005a6b070612e488d5\"","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":"Jacopo Mondi ,\n\tlibcamera devel ,\n\tSakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28855,"web_url":"https://patchwork.libcamera.org/comment/28855/","msgid":"<170962910524.1676185.9204840442726282996@ping.linuxembedded.co.uk>","date":"2024-03-05T08:58:25","subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2024-03-05 07:09:58)\n> On Mon, 4 Mar 2024, 10:49 pm Laurent Pinchart, <\n> laurent.pinchart@ideasonboard.com> wrote:\n> \n> > On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote:\n> > > Hi Laurent\n> > >\n> > > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote:\n> > > > The CameraSensor class tests if the sensor HBLANK control is read-only\n> > > > by comparing the minimum and maximum values, and documents this as\n> > being\n> > > > a workaround for the lack of a read-only control flag in V4L2. This is\n> > > > incorrect, as the V4L2 API provides such a flag. Use it to replace the\n> > > > workaround.\n> > > >\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++-------------------\n> > > > 1 file changed, 7 insertions(+), 20 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp\n> > b/src/libcamera/sensor/camera_sensor.cpp\n> > > > index 86ad9a85371c..402025566544 100644\n> > > > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > > > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > > > @@ -188,28 +188,15 @@ int CameraSensor::init()\n> > > > * Set HBLANK to the minimum to start with a well-defined line\n> > length,\n> > > > * allowing IPA modules that do not modify HBLANK to use the sensor\n> > > > * minimum line length in their calculations.\n> > > > - *\n> > > > - * At present, there is no way of knowing if a control is\n> > read-only.\n> > > > - * As a workaround, assume that if the minimum and maximum values\n> > of\n> > > > - * the V4L2_CID_HBLANK control are the same, it implies the control\n> > > > - * is read-only.\n> > > > - *\n> > > > - * \\todo The control API ought to have a flag to specify if a\n> > control\n> > > > - * is read-only which could be used below.\n> > > > */\n> > >\n> > > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this\n> > > has been introduced. Maybe the API we had to check flags was (or is)\n> > > not the best one and we decided to compare values ?\n> >\n> > It puzzled me too, I really don't recall why we didn't use it. Naush,\n> > does it ring a bell ?\n> >\n> \n> My patch at https://patchwork.libcamera.org/patch/17936/ did make use of\n> this flag. But it never got merged.\n\nIndeed, and I rebased Naush' patches to:\n\n - https://patchwork.libcamera.org/cover/19187/\n\nWhich seemed overall to be rejected, and Alain came along with:\n\n - https://patchwork.libcamera.org/patch/19214/\n\nwhich I thought would get merged, but Alain hasn't been able to submit a\nnew version with the comments handled.\n\n\nDoes something in this series handle the equivalant change at\nsrc/ipa/rpi/common/ipa_base.cpp as well?\n\n--\nKieran\n\n\n\n\n> \n> \n> \n> > > > - if (ctrls.infoMap()->find(V4L2_CID_HBLANK) !=\n> > ctrls.infoMap()->end()) {\n> > > > - const ControlInfo hblank =\n> > ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> > > > - const int32_t hblankMin = hblank.min().get();\n> > > > - const int32_t hblankMax = hblank.max().get();\n> > > > + const struct v4l2_query_ext_ctrl *hblankInfo =\n> > subdev_->controlInfo(V4L2_CID_HBLANK);\n> > > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY))\n> > {\n> > > > + ControlList ctrl(subdev_->controls());\n> > >\n> > > This could be fast-tracked too!\n> > >\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > > - if (hblankMin != hblankMax) {\n> > > > - ControlList ctrl(subdev_->controls());\n> > > > -\n> > > > - ctrl.set(V4L2_CID_HBLANK, hblankMin);\n> > > > - ret = subdev_->setControls(&ctrl);\n> > > > - if (ret)\n> > > > - return ret;\n> > > > - }\n> > > > + ctrl.set(V4L2_CID_HBLANK,\n> > static_cast(hblankInfo->minimum));\n> > > > + ret = subdev_->setControls(&ctrl);\n> > > > + if (ret)\n> > > > + return ret;\n> > > > }\n> > > >\n> > > > return\n> > applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\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 A540EBD160\n\tfor ;\n\tTue, 5 Mar 2024 08:58:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 615B36286C;\n\tTue, 5 Mar 2024 09:58:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3477D62865\n\tfor ;\n\tTue, 5 Mar 2024 09:58:28 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 693F51225;\n\tTue, 5 Mar 2024 09:58:11 +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=\"XFFItdoh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709629091;\n\tbh=kC+EORhW9Y0WfP/HCKn9jsga0cgPaRSsznY4O3hjuvQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=XFFItdohhWpsuI9dAXwDQJJGA+hqJ5HFgN6GyiLrYrmnwgteG/MF29QBbnmnrx7Sh\n\tqkG+9QP5pHQ8ajeJG5LBx1YvFzBNSuMxrSN5ZWAwcJqVheJVTlfmkDW1hHP+asnPCi\n\tlFy8Ej4Lr5Jo+QEE6fx3YjyXDUc2unh3lpW+MJno=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-18-laurent.pinchart@ideasonboard.com>\n\t\n\t<20240304224901.GG9233@pendragon.ideasonboard.com>\n\t","Subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tNaushir Patuck ","Date":"Tue, 05 Mar 2024 08:58:25 +0000","Message-ID":"<170962910524.1676185.9204840442726282996@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":"Jacopo Mondi ,\n\tlibcamera devel ,\n\tSakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28863,"web_url":"https://patchwork.libcamera.org/comment/28863/","msgid":"<20240305111517.GK12503@pendragon.ideasonboard.com>","date":"2024-03-05T11:15:17","subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Mar 05, 2024 at 08:58:25AM +0000, Kieran Bingham wrote:\n> Quoting Naushir Patuck (2024-03-05 07:09:58)\n> > On Mon, 4 Mar 2024, 10:49 pm Laurent Pinchart wrote:\n> > > On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote:\n> > > > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote:\n> > > > > The CameraSensor class tests if the sensor HBLANK control is read-only\n> > > > > by comparing the minimum and maximum values, and documents this as being\n> > > > > a workaround for the lack of a read-only control flag in V4L2. This is\n> > > > > incorrect, as the V4L2 API provides such a flag. Use it to replace the\n> > > > > workaround.\n> > > > >\n> > > > > Signed-off-by: Laurent Pinchart \n> > > > > ---\n> > > > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++-------------------\n> > > > > 1 file changed, 7 insertions(+), 20 deletions(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> > > > > index 86ad9a85371c..402025566544 100644\n> > > > > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > > > > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > > > > @@ -188,28 +188,15 @@ int CameraSensor::init()\n> > > > > * Set HBLANK to the minimum to start with a well-defined line length,\n> > > > > * allowing IPA modules that do not modify HBLANK to use the sensor\n> > > > > * minimum line length in their calculations.\n> > > > > - *\n> > > > > - * At present, there is no way of knowing if a control is read-only.\n> > > > > - * As a workaround, assume that if the minimum and maximum values of\n> > > > > - * the V4L2_CID_HBLANK control are the same, it implies the control\n> > > > > - * is read-only.\n> > > > > - *\n> > > > > - * \\todo The control API ought to have a flag to specify if a control\n> > > > > - * is read-only which could be used below.\n> > > > > */\n> > > >\n> > > > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this\n> > > > has been introduced. Maybe the API we had to check flags was (or is)\n> > > > not the best one and we decided to compare values ?\n> > >\n> > > It puzzled me too, I really don't recall why we didn't use it. Naush,\n> > > does it ring a bell ?\n> > >\n> > \n> > My patch at https://patchwork.libcamera.org/patch/17936/ did make use of\n> > this flag. But it never got merged.\n> \n> Indeed, and I rebased Naush' patches to:\n> \n> - https://patchwork.libcamera.org/cover/19187/\n> \n> Which seemed overall to be rejected, and Alain came along with:\n> \n> - https://patchwork.libcamera.org/patch/19214/\n> \n> which I thought would get merged, but Alain hasn't been able to submit a\n> new version with the comments handled.\n> \n> \n> Does something in this series handle the equivalant change at\n> src/ipa/rpi/common/ipa_base.cpp as well?\n\nNo it doesn't, I've only addressed the CameraSensor side. I've just\nreplied to Alain's patch, which had fallen through the cracks (sorry\nabout that).\n\n> > > > > - if (ctrls.infoMap()->find(V4L2_CID_HBLANK) != ctrls.infoMap()->end()) {\n> > > > > - const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> > > > > - const int32_t hblankMin = hblank.min().get();\n> > > > > - const int32_t hblankMax = hblank.max().get();\n> > > > > + const struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK);\n> > > > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {\n> > > > > + ControlList ctrl(subdev_->controls());\n> > > >\n> > > > This could be fast-tracked too!\n> > > >\n> > > > Reviewed-by: Jacopo Mondi \n> > > >\n> > > > > - if (hblankMin != hblankMax) {\n> > > > > - ControlList ctrl(subdev_->controls());\n> > > > > -\n> > > > > - ctrl.set(V4L2_CID_HBLANK, hblankMin);\n> > > > > - ret = subdev_->setControls(&ctrl);\n> > > > > - if (ret)\n> > > > > - return ret;\n> > > > > - }\n> > > > > + ctrl.set(V4L2_CID_HBLANK, static_cast(hblankInfo->minimum));\n> > > > > + ret = subdev_->setControls(&ctrl);\n> > > > > + if (ret)\n> > > > > + return ret;\n> > > > > }\n> > > > >\n> > > > > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);","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 5B4F2BD160\n\tfor ;\n\tTue, 5 Mar 2024 11:15:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 12FE661C92;\n\tTue, 5 Mar 2024 12:15:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A837E61C8D\n\tfor ;\n\tTue, 5 Mar 2024 12:15:16 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6BEF62B3;\n\tTue, 5 Mar 2024 12:14:59 +0100 (CET)"],"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=\"v1A8Jpxi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709637299;\n\tbh=5jAitMmTT756t+e9RzDSH+U1fW/LAeVSMpv9aKPoqjM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=v1A8JpxiG/U1UuL/Z7zp3xAvqkIE2cDqFD5rDv4BEDbakbHDrzesx68R8CugKmoz0\n\tKk8d5Cu7ob4FV5RFP41UpGCnMOg23ptg93JowsgSOGz450lKZeLQKXdFy14A5vpns1\n\tTaBlCLyeO6HhJDiN+Lkv/bQlCDK/y7kxaPZ+E/hI=","Date":"Tue, 5 Mar 2024 13:15:17 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","Message-ID":"<20240305111517.GK12503@pendragon.ideasonboard.com>","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-18-laurent.pinchart@ideasonboard.com>\n\t\n\t<20240304224901.GG9233@pendragon.ideasonboard.com>\n\t\n\t<170962910524.1676185.9204840442726282996@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<170962910524.1676185.9204840442726282996@ping.linuxembedded.co.uk>","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":"Jacopo Mondi ,\n\tlibcamera devel ,\n\tSakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28875,"web_url":"https://patchwork.libcamera.org/comment/28875/","msgid":"<170966076951.566498.6167936608203903215@ping.linuxembedded.co.uk>","date":"2024-03-05T17:46:09","subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-03-05 11:15:17)\n> On Tue, Mar 05, 2024 at 08:58:25AM +0000, Kieran Bingham wrote:\n> > Quoting Naushir Patuck (2024-03-05 07:09:58)\n> > > On Mon, 4 Mar 2024, 10:49 pm Laurent Pinchart wrote:\n> > > > On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote:\n> > > > > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote:\n> > > > > > The CameraSensor class tests if the sensor HBLANK control is read-only\n> > > > > > by comparing the minimum and maximum values, and documents this as being\n> > > > > > a workaround for the lack of a read-only control flag in V4L2. This is\n> > > > > > incorrect, as the V4L2 API provides such a flag. Use it to replace the\n> > > > > > workaround.\n> > > > > >\n> > > > > > Signed-off-by: Laurent Pinchart \n> > > > > > ---\n> > > > > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++-------------------\n> > > > > > 1 file changed, 7 insertions(+), 20 deletions(-)\n> > > > > >\n> > > > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> > > > > > index 86ad9a85371c..402025566544 100644\n> > > > > > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > > > > > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > > > > > @@ -188,28 +188,15 @@ int CameraSensor::init()\n> > > > > > * Set HBLANK to the minimum to start with a well-defined line length,\n> > > > > > * allowing IPA modules that do not modify HBLANK to use the sensor\n> > > > > > * minimum line length in their calculations.\n> > > > > > - *\n> > > > > > - * At present, there is no way of knowing if a control is read-only.\n> > > > > > - * As a workaround, assume that if the minimum and maximum values of\n> > > > > > - * the V4L2_CID_HBLANK control are the same, it implies the control\n> > > > > > - * is read-only.\n> > > > > > - *\n> > > > > > - * \\todo The control API ought to have a flag to specify if a control\n> > > > > > - * is read-only which could be used below.\n> > > > > > */\n> > > > >\n> > > > > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this\n> > > > > has been introduced. Maybe the API we had to check flags was (or is)\n> > > > > not the best one and we decided to compare values ?\n> > > >\n> > > > It puzzled me too, I really don't recall why we didn't use it. Naush,\n> > > > does it ring a bell ?\n> > > >\n> > > \n> > > My patch at https://patchwork.libcamera.org/patch/17936/ did make use of\n> > > this flag. But it never got merged.\n> > \n> > Indeed, and I rebased Naush' patches to:\n> > \n> > - https://patchwork.libcamera.org/cover/19187/\n> > \n> > Which seemed overall to be rejected, and Alain came along with:\n> > \n> > - https://patchwork.libcamera.org/patch/19214/\n> > \n> > which I thought would get merged, but Alain hasn't been able to submit a\n> > new version with the comments handled.\n> > \n> > \n> > Does something in this series handle the equivalant change at\n> > src/ipa/rpi/common/ipa_base.cpp as well?\n> \n> No it doesn't, I've only addressed the CameraSensor side. I've just\n> replied to Alain's patch, which had fallen through the cracks (sorry\n> about that).\n> \n> > > > > > - if (ctrls.infoMap()->find(V4L2_CID_HBLANK) != ctrls.infoMap()->end()) {\n> > > > > > - const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> > > > > > - const int32_t hblankMin = hblank.min().get();\n> > > > > > - const int32_t hblankMax = hblank.max().get();\n> > > > > > + const struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK);\n> > > > > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) {\n> > > > > > + ControlList ctrl(subdev_->controls());\n\n\nI certainly prefer this:\n\n const struct v4l2_query_ext_ctrl *hblankInfo =\n\t\t subdev_->controlInfo(V4L2_CID_HBLANK);\n if (hblankInfo ...) {\n }\n\nstyle over\n\nif (ctrls.infoMap()->find(V4L2_CID_HBLANK) != ctrls.infoMap()->end())\n{\n\tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n\t..\n}\n\nas it only does the search once. But it would be good to not lose sight\nof the equivalent issue in the RPi IPA.\n\n\nReviewed-by: Kieran Bingham \n\n\n> > > > >\n> > > > > This could be fast-tracked too!\n> > > > >\n> > > > > Reviewed-by: Jacopo Mondi \n> > > > >\n> > > > > > - if (hblankMin != hblankMax) {\n> > > > > > - ControlList ctrl(subdev_->controls());\n> > > > > > -\n> > > > > > - ctrl.set(V4L2_CID_HBLANK, hblankMin);\n> > > > > > - ret = subdev_->setControls(&ctrl);\n> > > > > > - if (ret)\n> > > > > > - return ret;\n> > > > > > - }\n> > > > > > + ctrl.set(V4L2_CID_HBLANK, static_cast(hblankInfo->minimum));\n> > > > > > + ret = subdev_->setControls(&ctrl);\n> > > > > > + if (ret)\n> > > > > > + return ret;\n> > > > > > }\n> > > > > >\n> > > > > > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\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 E739FBD160\n\tfor ;\n\tTue, 5 Mar 2024 17:46:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 086906286F;\n\tTue, 5 Mar 2024 18:46:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9DD8261C8D\n\tfor ;\n\tTue, 5 Mar 2024 18:46:12 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 712E28D0;\n\tTue, 5 Mar 2024 18:45:55 +0100 (CET)"],"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=\"JsCWFw3d\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709660755;\n\tbh=szv55voesJRbTF2+Jpp8yhyOFanpryqxFkKMc6i2wDg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=JsCWFw3d1VHYEGyaASE7UnsonNVJlHyxoqNHSJ5JR+NV9wnMxuOcQ/3tYpilQroFV\n\timGFLMudkXPekq4pZVbfc+KzO25KhqUhfZuTz6JVD/Uh2MfrnHZSyp9LctVtpIQw3L\n\tjlxBdZ+Vutvm8suwQU6uMn+L2ezLizNHCpMmCVeM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240305111517.GK12503@pendragon.ideasonboard.com>","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-18-laurent.pinchart@ideasonboard.com>\n\t\n\t<20240304224901.GG9233@pendragon.ideasonboard.com>\n\t\n\t<170962910524.1676185.9204840442726282996@ping.linuxembedded.co.uk>\n\t<20240305111517.GK12503@pendragon.ideasonboard.com>","Subject":"Re: [PATCH/RFC 17/32] libcamera: camera_sensor: Test for read-only\n\tHBLANK with READ_ONLY flag","From":"Kieran Bingham ","To":"Laurent Pinchart ","Date":"Tue, 05 Mar 2024 17:46:09 +0000","Message-ID":"<170966076951.566498.6167936608203903215@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":"Jacopo Mondi ,\n\tlibcamera devel ,\n\tSakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]