[{"id":25848,"web_url":"https://patchwork.libcamera.org/comment/25848/","msgid":"<20221122130750.4mdoftcuagqwik4w@uno.localdomain>","date":"2022-11-22T13:07:50","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: camera_sensor: Add a\n\tfunction to reset the HBLANK interval","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Tue, Nov 22, 2022 at 12:03:41PM +0000, David Plowman via libcamera-devel wrote:\n> The existing code to reset the HBLANK interval to the minimum value\n> (in the init method) is moved into a separate function that pipeline\n> handlers will be able to call.\n>\n> We no longer reset the HBLANK in init because we may not own the\n> camera and the operation may fail, so pipeline handlers will have to\n> call the new method for themselves. These calls will be added in\n> subsequent commits.\n>\n\nThis was again part of a discussion with Dave on a recent series for a\ncamera sensor\nhttps://patchwork.linuxtv.org/project/linux-media/patch/20221005190613.394277-8-jacopo@jmondi.org/#140629\n\nTo sum it up: I would have liked to standardize on the idea that when\na new mode is applied on a sensor, the blankings are reset to their\nminimum by the driver.\n\nAs nowadays every driver behaves as it likes the most, and changing\nmode might eventually change the blankings value anyway, I considered\na known behaviour to be more predictable for applications.\n\nTo be honest, I overlooked the fact we reset hblank unconditionally\n(but only at init() time).\n\nCurious to know what you think on this and what would your PH expect\n(I presume it doesn't make much difference, as the IPA wants to\ncontrol HBLANK anyway).\n\n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/internal/camera_sensor.h | 2 ++\n> src/libcamera/camera_sensor.cpp | 10 +++++++---\n> 2 files changed, 9 insertions(+), 3 deletions(-)\n>\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index b9f4d786..c97b5698 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -68,6 +68,8 @@ public:\n>\n> \tCameraLens *focusLens() { return focusLens_.get(); }\n>\n> +\tint resetHblank();\n> +\n> protected:\n> \tstd::string logPrefix() const override;\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 572a313a..589e9736 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -176,6 +176,11 @@ int CameraSensor::init()\n> \tif (ret)\n> \t\treturn ret;\n>\n> +\treturn applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> +}\n> +\n> +int CameraSensor::resetHblank()\n\nI'm not super excited by this, it's a very ad-hoc call for PH, if they\ndon't know they have to call it explicitly it will be easily missed.\n\nIf my idea of \"starting in a known state in drivers\" doesn't fly, this\ncould be moved to CameraSensor::setFormat() maybe ? IPAs that control\nHBLANK will override it anyway.\n\n> +{\n> \t/*\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> @@ -192,17 +197,16 @@ int CameraSensor::init()\n> \tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> \tconst int32_t hblankMin = hblank.min().get();\n> \tconst int32_t hblankMax = hblank.max().get();\n> +\tint ret = 0;\n>\n> \tif (hblankMin != hblankMax) {\n> \t\tControlList ctrl(subdev_->controls());\n>\n> \t\tctrl.set(V4L2_CID_HBLANK, hblankMin);\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> +\treturn ret;\n> }\n>\n> int CameraSensor::validateSensorDriver()\n> --\n> 2.30.2\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 9E9B5BDE6B\n\tfor ;\n\tTue, 22 Nov 2022 13:07:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 126E561F2B;\n\tTue, 22 Nov 2022 14:07:54 +0100 (CET)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 76D5F603CE\n\tfor ;\n\tTue, 22 Nov 2022 14:07:52 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 91C1724000C;\n\tTue, 22 Nov 2022 13:07:51 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669122474;\n\tbh=hMLmEKAVdJKm4VlsGHa0pn7Be5P0duK4paucZxqqcLk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=e/ep6uh5qGa9UYyucTgRZonsM0g6n9dJCsjzr3ZiIHaW1GtztGFD4502WtNZAG/8l\n\tMfPJhxsuMd0BjJkBUn1DKxfmQoDSxpL7HEmDu/smv7tkvrE5XKd7EX61yRjIoTwPgg\n\tlOjQSHxIuvmj8Srgv+X0ZfIQKGs3auh0crRpTqzxEgC0GgmlaZGtDjMYbNMkfGfLMl\n\tJk5lwweDEVhh/dmZgGK64g2GCLv3VyMA+KMrWOauNGxSuv0U+VjGvTHum3qJ99gdr6\n\tUyYdYm1MeWubW6ztBzCVAgnF0CC5znFnpUQ1clWx6HErLyZWAenXONEkxxaY+UYOGE\n\tCE8tQmsBFVPWA==","Date":"Tue, 22 Nov 2022 14:07:50 +0100","To":"David Plowman ","Message-ID":"<20221122130750.4mdoftcuagqwik4w@uno.localdomain>","References":"<20221122120343.4825-1-david.plowman@raspberrypi.com>\n\t<20221122120343.4825-2-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221122120343.4825-2-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: camera_sensor: Add a\n\tfunction to reset the HBLANK interval","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25849,"web_url":"https://patchwork.libcamera.org/comment/25849/","msgid":"","date":"2022-11-22T13:41:56","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: camera_sensor: Add a\n\tfunction to reset the HBLANK interval","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for the comments. Indeed I defer to folks who spend much more\ntime on camera drivers than I do!\n\nOn Tue, 22 Nov 2022 at 13:07, Jacopo Mondi wrote:\n>\n> Hi David,\n>\n> On Tue, Nov 22, 2022 at 12:03:41PM +0000, David Plowman via libcamera-devel wrote:\n> > The existing code to reset the HBLANK interval to the minimum value\n> > (in the init method) is moved into a separate function that pipeline\n> > handlers will be able to call.\n> >\n> > We no longer reset the HBLANK in init because we may not own the\n> > camera and the operation may fail, so pipeline handlers will have to\n> > call the new method for themselves. These calls will be added in\n> > subsequent commits.\n> >\n>\n> This was again part of a discussion with Dave on a recent series for a\n> camera sensor\n> https://patchwork.linuxtv.org/project/linux-media/patch/20221005190613.394277-8-jacopo@jmondi.org/#140629\n>\n> To sum it up: I would have liked to standardize on the idea that when\n> a new mode is applied on a sensor, the blankings are reset to their\n> minimum by the driver.\n>\n> As nowadays every driver behaves as it likes the most, and changing\n> mode might eventually change the blankings value anyway, I considered\n> a known behaviour to be more predictable for applications.\n>\n> To be honest, I overlooked the fact we reset hblank unconditionally\n> (but only at init() time).\n>\n> Curious to know what you think on this and what would your PH expect\n> (I presume it doesn't make much difference, as the IPA wants to\n> control HBLANK anyway).\n>\n> > Signed-off-by: David Plowman \n> > ---\n> > include/libcamera/internal/camera_sensor.h | 2 ++\n> > src/libcamera/camera_sensor.cpp | 10 +++++++---\n> > 2 files changed, 9 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index b9f4d786..c97b5698 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -68,6 +68,8 @@ public:\n> >\n> > CameraLens *focusLens() { return focusLens_.get(); }\n> >\n> > + int resetHblank();\n> > +\n> > protected:\n> > std::string logPrefix() const override;\n> >\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 572a313a..589e9736 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -176,6 +176,11 @@ int CameraSensor::init()\n> > if (ret)\n> > return ret;\n> >\n> > + return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> > +}\n> > +\n> > +int CameraSensor::resetHblank()\n>\n> I'm not super excited by this, it's a very ad-hoc call for PH, if they\n> don't know they have to call it explicitly it will be easily missed.\n>\n> If my idea of \"starting in a known state in drivers\" doesn't fly, this\n> could be moved to CameraSensor::setFormat() maybe ? IPAs that control\n> HBLANK will override it anyway.\n\nThat sounds pretty convincing to me. What do other folks think?\n\nThanks\nDavid\n\n>\n> > +{\n> > /*\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> > @@ -192,17 +197,16 @@ int CameraSensor::init()\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> > + int ret = 0;\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> >\n> > - return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> > + return ret;\n> > }\n> >\n> > int CameraSensor::validateSensorDriver()\n> > --\n> > 2.30.2\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 BF5CCBE08B\n\tfor ;\n\tTue, 22 Nov 2022 13:42:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 20D3363314;\n\tTue, 22 Nov 2022 14:42:11 +0100 (CET)","from mail-pl1-x631.google.com (mail-pl1-x631.google.com\n\t[IPv6:2607:f8b0:4864:20::631])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 34C77603CE\n\tfor ;\n\tTue, 22 Nov 2022 14:42:09 +0100 (CET)","by mail-pl1-x631.google.com with SMTP id y4so13680529plb.2\n\tfor ;\n\tTue, 22 Nov 2022 05:42:09 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669124531;\n\tbh=y79qlXTKWVY/WiX7uXweC2JQe7+ZsWiq/7NQqdnR4k8=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pYih0RbIfhFJzmFM/9P0qWfrXAlBmi3PIuyO1LNs0vmwS5Ui3FbkbZMcyiHZmzvxT\n\tVVUIblLvPdELWRDUMgOYzhyW02Go1KUwRdB7AzqDhgI+K9mK5vnE9QSyfnyJYpGUiK\n\tgADIjSRC7OgE2kkQo5hWV1CxsrEKijqwnwYk0T7cRhmF97SapFf60YG1eViP9a1P+6\n\tW/z1PO7Td2TwaMZ136IJCopNK9A9TguDxYaZ4NlvgPVPACVYItWvxLZVrPQA3nh5Sh\n\tTTHVHrbuz7Ys3NZtZaglv3QLpRhGVI40ZUZFyy8OHxvJrDMBUOszh6beKZgLIgrm4q\n\tzDnkDOBwj60bA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\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=5tDSFYQIhBecWKM9U1p76DwIhxTYtxlZeJqZsLMGSWs=;\n\tb=jdP1qaPVnarXuEK+Mvtc0SDPMsbsSC1A2mGv3BKhUbphYA4WCZU7d1A0gzUQIlPT5V\n\tBKh/U2kvb4SqRajqVQTGMbXodjE4pHYgavZB6p/X6eLSWjt6GSuGTZeohRLIsk5ysXHu\n\tNurJYQLYT+IrgowNQkoOH0JCNf0G5jiuSMr6kMtHbs8rb9tCNeVUyytZ+TRQqWAKPY65\n\trcqqP6NYvmXxgLhRScevv8Y6H1GXN458BOqvXc29FibXJSHETccfFBvfuJEhRFMqMtD4\n\tprEKdc45i0TG/bq/KT7dj0shYMgisiuWUbXfdX6bG3aq46MX7kSeZLkcaG9Fn3Ot/QOy\n\tPPdg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"jdP1qaPV\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\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=5tDSFYQIhBecWKM9U1p76DwIhxTYtxlZeJqZsLMGSWs=;\n\tb=aug6Qx+Xk87MbZzy8is+tF9HbrSt2Qhjvd8Bkd+V1HDtgY6AuKfCN+PIrS91Nz61tG\n\tUzLmEM0WCedbmMGAqqr/+78HbidfOdkJStVCFXKeaZyja8rd3ikRZ4od/IVdA2D7fZ4H\n\tsd5TY2Qzk+cn+AGT74WT35kpRYGQUNODcKEq0j5vZn70dz+Mv7mH+vBwKwG2bi+q9JWv\n\tBbIds9U5c7hcdWJVLYmR8Vd43zpnHfK32OVZab1jWALoOeX9xF8JuKgtjLRInAhp56UD\n\t28BDlglyRn1UTb7t4iqqCsZDCKpAn7BbhIQSOGWxsfUPQscIDqURIEVcqzdnZjJL8j7b\n\tZJHg==","X-Gm-Message-State":"ANoB5pmiEVMFeO8CGnaomf94BYjy88sxylvOeuf3ZMdaL12SkScd5M6N\n\tCvFGq/aZztMkRlZG8LP00/5Zw7zXEED5PGUgMzFjGde2dpc=","X-Google-Smtp-Source":"AA0mqf7GT+gINhKywKNLZLRDWPoh9TAyv771w1JM7W+beDBdEYU5Pn+4aFIV0UvDIVJ/NRonkMnI4drWu7+76U6UtRs=","X-Received":"by 2002:a17:90a:a588:b0:218:8eb8:8502 with SMTP id\n\tb8-20020a17090aa58800b002188eb88502mr18908840pjq.179.1669124527592;\n\tTue, 22 Nov 2022 05:42:07 -0800 (PST)","MIME-Version":"1.0","References":"<20221122120343.4825-1-david.plowman@raspberrypi.com>\n\t<20221122120343.4825-2-david.plowman@raspberrypi.com>\n\t<20221122130750.4mdoftcuagqwik4w@uno.localdomain>","In-Reply-To":"<20221122130750.4mdoftcuagqwik4w@uno.localdomain>","Date":"Tue, 22 Nov 2022 13:41:56 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: camera_sensor: Add a\n\tfunction to reset the HBLANK interval","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","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25850,"web_url":"https://patchwork.libcamera.org/comment/25850/","msgid":"","date":"2022-11-22T14:13:09","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: camera_sensor: Add a\n\tfunction to reset the HBLANK interval","submitter":{"id":27,"url":"https://patchwork.libcamera.org/api/people/27/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.com"},"content":"Hi David & Jacopo\n\nOn Tue, 22 Nov 2022 at 13:42, David Plowman via libcamera-devel\n wrote:\n>\n> Hi Jacopo\n>\n> Thanks for the comments. Indeed I defer to folks who spend much more\n> time on camera drivers than I do!\n>\n> On Tue, 22 Nov 2022 at 13:07, Jacopo Mondi wrote:\n> >\n> > Hi David,\n> >\n> > On Tue, Nov 22, 2022 at 12:03:41PM +0000, David Plowman via libcamera-devel wrote:\n> > > The existing code to reset the HBLANK interval to the minimum value\n> > > (in the init method) is moved into a separate function that pipeline\n> > > handlers will be able to call.\n> > >\n> > > We no longer reset the HBLANK in init because we may not own the\n> > > camera and the operation may fail, so pipeline handlers will have to\n> > > call the new method for themselves. These calls will be added in\n> > > subsequent commits.\n> > >\n> >\n> > This was again part of a discussion with Dave on a recent series for a\n> > camera sensor\n> > https://patchwork.linuxtv.org/project/linux-media/patch/20221005190613.394277-8-jacopo@jmondi.org/#140629\n> >\n> > To sum it up: I would have liked to standardize on the idea that when\n> > a new mode is applied on a sensor, the blankings are reset to their\n> > minimum by the driver.\n\nWe really need Sakari to respond on this one as the maintainer of\nimage sensor drivers.\n\nEven if the documentation is updated to state that the expected\nbehaviour is to reset the blankings, you then have a fair task to\nvalidate and potentially correct that behaviour on all mainline sensor\ndrivers. I guess you could restrict it to just those that have tunings\nor other config within libcamera, but that's still a moderate number.\n\n> > As nowadays every driver behaves as it likes the most, and changing\n> > mode might eventually change the blankings value anyway, I considered\n> > a known behaviour to be more predictable for applications.\n> >\n> > To be honest, I overlooked the fact we reset hblank unconditionally\n> > (but only at init() time).\n> >\n> > Curious to know what you think on this and what would your PH expect\n> > (I presume it doesn't make much difference, as the IPA wants to\n> > control HBLANK anyway).\n\nThis is the flip side - if the PH already knows how to handle setting\nall the blankings, what does it matter what the sensor driver does?\n\n> > > Signed-off-by: David Plowman \n> > > ---\n> > > include/libcamera/internal/camera_sensor.h | 2 ++\n> > > src/libcamera/camera_sensor.cpp | 10 +++++++---\n> > > 2 files changed, 9 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > > index b9f4d786..c97b5698 100644\n> > > --- a/include/libcamera/internal/camera_sensor.h\n> > > +++ b/include/libcamera/internal/camera_sensor.h\n> > > @@ -68,6 +68,8 @@ public:\n> > >\n> > > CameraLens *focusLens() { return focusLens_.get(); }\n> > >\n> > > + int resetHblank();\n> > > +\n> > > protected:\n> > > std::string logPrefix() const override;\n> > >\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index 572a313a..589e9736 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -176,6 +176,11 @@ int CameraSensor::init()\n> > > if (ret)\n> > > return ret;\n> > >\n> > > + return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> > > +}\n> > > +\n> > > +int CameraSensor::resetHblank()\n> >\n> > I'm not super excited by this, it's a very ad-hoc call for PH, if they\n> > don't know they have to call it explicitly it will be easily missed.\n> >\n> > If my idea of \"starting in a known state in drivers\" doesn't fly, this\n> > could be moved to CameraSensor::setFormat() maybe ? IPAs that control\n> > HBLANK will override it anyway.\n>\n> That sounds pretty convincing to me. What do other folks think?\n\nI'm not developing libcamera so can't really comment. I will say that\nyou've added another userspace/kernel context switch unless controls\nare all batched together.\nThen again setFormat is having to call VIDIOC_QUERT_EXT_CTRL on all\ncontrols anyway, so that swamps the one extra VIDIOC_S_CTRL.\n\n Dave\n\n> Thanks\n> David\n>\n> >\n> > > +{\n> > > /*\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> > > @@ -192,17 +197,16 @@ int CameraSensor::init()\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> > > + int ret = 0;\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> > >\n> > > - return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> > > + return ret;\n> > > }\n> > >\n> > > int CameraSensor::validateSensorDriver()\n> > > --\n> > > 2.30.2\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 E1107BDE6B\n\tfor ;\n\tTue, 22 Nov 2022 14:13:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3F0B463314;\n\tTue, 22 Nov 2022 15:13:28 +0100 (CET)","from mail-io1-xd34.google.com (mail-io1-xd34.google.com\n\t[IPv6:2607:f8b0:4864:20::d34])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 24E1F603CE\n\tfor ;\n\tTue, 22 Nov 2022 15:13:27 +0100 (CET)","by mail-io1-xd34.google.com with SMTP id p184so10971002iof.11\n\tfor ;\n\tTue, 22 Nov 2022 06:13:27 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669126408;\n\tbh=5NGGFh/nq/+aA7B7XtqVewbKBEPg6rTvuSb/6kEwa8s=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=sugC6iKHbfS8LUei4uNZVs/6soaxjnesgO4seTvBap6SiAekGy7SAEmEABPUMf7M9\n\tyeiWRboju6werorDsk0fqnisO0inAmz0Oc+M1frIQ+u6c+uNaXhax21g9OIER7A6nK\n\txAwMCMpw88DNBvClTzi6/pKPwXiR7eSEGszcX9qdtbweiEDU1txms6KQvuZJ11FvwB\n\tITytxOfQTA7a4slM8IDvddd1PXW29TnvAbmY4FtNjHz7FqRam4A4P8JaavATdXo1hz\n\ttt90tKnJmb85hEm76xMTvfl8xObS4w4KboLnbt9l95ICSYktUCuY3PbGiXTPNI1lfk\n\tEP1KfFqcebkFA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\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=whDw1TU0Lj5zUE2jiop7yz5Mjhau4+OXNf+Jv4Zv+xw=;\n\tb=e8KXpYN8OHQNBk6Q1Od+dLC85ezGKQNINrd7wzphnSYP3IhiKSMvLssscHw9T2sWFD\n\tv6d2ZOtLYsl3UDnsLkpA+ByCGhKIbSjjEM1lGYvKecXc8vdafIaRwgrjy+TWb4trDGti\n\t+Z1P0Jsc85Ms5GjCEQ0N7F5Hr//IgJpBJEgb46v7nRA0XQnvrnXZ+lur2MYx6M8G8YZI\n\tUH3KUaOSLAD7mAHz3f8JNBSkdhrfsj7DLQ50W+5pxJC8BKewNuSWOl8q+dYNbcuTpFfh\n\ttau0+eFm8NwTIvLDfNNzEm309nJUa7hklZiMrGYS8nQ5SU21Wbupnm8Xf2VK5SAcnxty\n\tpz+g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"e8KXpYN8\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\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=whDw1TU0Lj5zUE2jiop7yz5Mjhau4+OXNf+Jv4Zv+xw=;\n\tb=ABwejp5aiYS1ADmddxfRtvXhCZB1E5PKy/UCLSpFoK2LuOWznVbTtwqhYW/51jCwJ0\n\t63edrhmQ+YVtrtt0ocb7GLkFLmu8BPqXu7CAGrrkRXOiIvtU+XhWV37iEwvJg5X07Sb0\n\tcQcwm2V6iFRA/2xML6oA8a1YY8qPpCHaGzGHH8ZBgWzNYW/gWboFRgXznlrHgmUSTXKg\n\tznvEyVOsHJcrGLiP6bWZIrl9NP6PK6yj0jHLcxLgetDLQ8z0SGRUg83bIK02i9b1IZBZ\n\tj44fbsAA2kYSLKCBMtapmi+gbJ8Uu1hvfy5fEPdzGWjrNMSvdgQPSFH+UuaBHVsVUzP1\n\t+emg==","X-Gm-Message-State":"ANoB5pnPdVM5UE257/PEkFyxqxccmoYPNMbbKo9Im1zXFRGSpDxiqfNt\n\tIVesh1LLW3Fs0G7vZf+m5a3Fp+14pziqqSO2d0aunw==","X-Google-Smtp-Source":"AA0mqf7Hc48C0y1wMrtV3Zl8BPwr4wnEA9U1+MfZY9ErfT/ujB/uayAoTdjYmTdsJu/6E/mqPqvanoqoPBgfwISRd0M=","X-Received":"by 2002:a05:6602:2814:b0:6de:ca95:45d8 with SMTP id\n\td20-20020a056602281400b006deca9545d8mr5188635ioe.26.1669126405757;\n\tTue, 22 Nov 2022 06:13:25 -0800 (PST)","MIME-Version":"1.0","References":"<20221122120343.4825-1-david.plowman@raspberrypi.com>\n\t<20221122120343.4825-2-david.plowman@raspberrypi.com>\n\t<20221122130750.4mdoftcuagqwik4w@uno.localdomain>\n\t","In-Reply-To":"","Date":"Tue, 22 Nov 2022 14:13:09 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: camera_sensor: Add a\n\tfunction to reset the HBLANK interval","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","From":"Dave Stevenson via libcamera-devel\n\t","Reply-To":"Dave Stevenson ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]