[{"id":17692,"web_url":"https://patchwork.libcamera.org/comment/17692/","msgid":"<20210622103440.kob4vei3us5eapx6@uno.localdomain>","date":"2021-06-22T10:34:40","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> Turns off a sensor test pattern mode at the initialization of the\n> sensor. Without this, the camera sensor is configured with the last\n> test pattern mode that has been set.\n>\n> Signed-off-by: Hirokazu Honda \n> ---\n> src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 1be2cbcd..8548f749 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -9,6 +9,7 @@\n>\n> #include \n>\n> +#include \n> #include \n> #include \n> #include \n> @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n>\n> \tLOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n>\n> +\tret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error)\n> +\t\t\t<< \"Failed to reset test pattern mode: \" << ret;\n> +\t\treturn ret;\n> +\t}\n> +\n\nNow I see why you need all the checks in the previous patch. But, do\nwe need to do so ? Isn't the application which is in control of the\ntest pattern ? Shouldn't the app explicitly disable it if they want to\n?\n\n> \treturn 0;\n> }\n>\n> --\n> 2.32.0.288.g62a8d224e6-goog\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 B7131C321B\n\tfor ;\n\tTue, 22 Jun 2021 10:33:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CC0568932;\n\tTue, 22 Jun 2021 12:33:52 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7E60F60292\n\tfor ;\n\tTue, 22 Jun 2021 12:33:51 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id DB0D6E0003;\n\tTue, 22 Jun 2021 10:33:50 +0000 (UTC)"],"Date":"Tue, 22 Jun 2021 12:34:40 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210622103440.kob4vei3us5eapx6@uno.localdomain>","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210622023654.969162-3-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17704,"web_url":"https://patchwork.libcamera.org/comment/17704/","msgid":"","date":"2021-06-23T03:43:22","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo,\n\nOn Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n>\n> Hi Hiro,\n>\n> On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > Turns off a sensor test pattern mode at the initialization of the\n> > sensor. Without this, the camera sensor is configured with the last\n> > test pattern mode that has been set.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > 1 file changed, 8 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index 1be2cbcd..8548f749 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > @@ -9,6 +9,7 @@\n> >\n> > #include \n> >\n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> >\n> > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> >\n> > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > + if (ret) {\n> > + LOG(IPU3, Error)\n> > + << \"Failed to reset test pattern mode: \" << ret;\n> > + return ret;\n> > + }\n> > +\n>\n> Now I see why you need all the checks in the previous patch. But, do\n> we need to do so ? Isn't the application which is in control of the\n> test pattern ? Shouldn't the app explicitly disable it if they want to\n> ?\n>\n\nAn application implementation is not controlled by us.\nIt can specify an invalid test pattern mode. I would like to have the guard.\n\n> Shouldn't the app explicitly disable it if they want to\n\nI am not sure honestly. I *think* most apps don't reset the test\npattern (i.e. disable) in either start up and termination, because a\nv4l2 sensor driver resets the test pattern in open() or close().\nBut the libcamera implementation keeps opening the node and in\nlibcamera we should reset in configure().\n\n-Hiro\n> > return 0;\n> > }\n> >\n> > --\n> > 2.32.0.288.g62a8d224e6-goog\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 6FCE7C321A\n\tfor ;\n\tWed, 23 Jun 2021 03:43:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EAFBD68932;\n\tWed, 23 Jun 2021 05:43:33 +0200 (CEST)","from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com\n\t[IPv6:2a00:1450:4864:20::62e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B96116050A\n\tfor ;\n\tWed, 23 Jun 2021 05:43:32 +0200 (CEST)","by mail-ej1-x62e.google.com with SMTP id my49so1675636ejc.7\n\tfor ;\n\tTue, 22 Jun 2021 20:43:32 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"T3u90OOO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=UeJ+tb85gTJ89iHQhmLtIf67s/0Jlj0+rrfjw4DfUf0=;\n\tb=T3u90OOOkN5+pFnRcTU7df1SuvcZ7H44UvX2uyJqrXf+1ttYtcWwWUPa4IAHSYlkZH\n\t/ZCnQa7QHlMX7byjhGSOPhyEbzeYQuglUTVWIYi1kD0NM1AhwXoxHb3APc73rrAtNJr8\n\tkhjwMMpjffDWVaCfhtGgz7VGyxqlOO1arJ9+Q=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=UeJ+tb85gTJ89iHQhmLtIf67s/0Jlj0+rrfjw4DfUf0=;\n\tb=hMjQX+pvpKJBfXtEaA05WMVf5MFMTEZSqDU0GfTrQLc7QWmz2GsqeIBwuo7T4Etl+g\n\tWvdX0kEeIdi/lcdQHuQGiS41z6TjChzSm+j8QLqAJ1m/wag2UGyIvNigx+xgrk4Nc+ug\n\tpwOCnm8zu7AbLY51R0fJdxo0fHw/BCXY64TGGarjJ8URR+bALaeM83QtUl5HDsNQd4GM\n\tQAZfSTlLGFo9Bm4gbjxYDgGt7/nGvnmpE4HwIb4LTmNVvWXr11qHj0eaZBz0dswd+pbY\n\toY7JkpIZrFXGaM5wWqJTNFvvMPQH2dBrzzQI8G7wYGkv59G/GEmGTSikiC4/jZjoa4++\n\txaMg==","X-Gm-Message-State":"AOAM532UKu0c1nG/1ZTtjTiAdxThqYXSxCzyL3eZHn0aD9VjWHNlmkLF\n\tI+FUbZaNqfjNgvOabWeZozTXxNxLfz8dqzZk2jbhyw==","X-Google-Smtp-Source":"ABdhPJwmE5zBTI+/I58OezwAt2gwlSjao6qBNFOh6R10V+5UlqSBMM0RczJqNKmLiy1TFgvR7J3GIWI3LwaE5syo8PQ=","X-Received":"by 2002:a17:906:19cc:: with SMTP id\n\th12mr7408182ejd.306.1624419812382; \n\tTue, 22 Jun 2021 20:43:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>","In-Reply-To":"<20210622103440.kob4vei3us5eapx6@uno.localdomain>","From":"Hirokazu Honda ","Date":"Wed, 23 Jun 2021 12:43:22 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17707,"web_url":"https://patchwork.libcamera.org/comment/17707/","msgid":"<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>","date":"2021-06-23T07:34:32","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Wed, Jun 23, 2021 at 12:43:22PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo,\n>\n> On Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n> >\n> > Hi Hiro,\n> >\n> > On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > > Turns off a sensor test pattern mode at the initialization of the\n> > > sensor. Without this, the camera sensor is configured with the last\n> > > test pattern mode that has been set.\n> > >\n> > > Signed-off-by: Hirokazu Honda \n> > > ---\n> > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > 1 file changed, 8 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > index 1be2cbcd..8548f749 100644\n> > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > @@ -9,6 +9,7 @@\n> > >\n> > > #include \n> > >\n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > >\n> > > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> > >\n> > > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > > + if (ret) {\n> > > + LOG(IPU3, Error)\n> > > + << \"Failed to reset test pattern mode: \" << ret;\n> > > + return ret;\n> > > + }\n> > > +\n> >\n> > Now I see why you need all the checks in the previous patch. But, do\n> > we need to do so ? Isn't the application which is in control of the\n> > test pattern ? Shouldn't the app explicitly disable it if they want to\n> > ?\n> >\n>\n> An application implementation is not controlled by us.\n> It can specify an invalid test pattern mode. I would like to have the guard.\n>\n\nWhat I meant was summarized in the below here question\n \"Shouldn't the app explicitly disable it if they want to ?\"\n\nRearding the value of the test pattern to apply, I was almost sure\nthat the control validator verified the provided value is in the range\nof supported ones. But it does only verify that the control id is\nvalid for the camera (see camera_controls.cpp). It would be rather\nstraightforward to add a .validate() to ControlInfoMap though and make\nsure all the values we receive are valid in the current ranges...\n\n> > Shouldn't the app explicitly disable it if they want to\n>\n> I am not sure honestly. I *think* most apps don't reset the test\n> pattern (i.e. disable) in either start up and termination, because a\n> v4l2 sensor driver resets the test pattern in open() or close().\n\nI think that's part of the API semantic we have to better define,\nthat's why I wanted to rope Laurent in.\n\nWhat v4l2 does should not strictly drive our design though.\n\nThanks\n j\n\n> But the libcamera implementation keeps opening the node and in\n> libcamera we should reset in configure().\n>\n> -Hiro\n> > > return 0;\n> > > }\n> > >\n> > > --\n> > > 2.32.0.288.g62a8d224e6-goog\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 9C431C321B\n\tfor ;\n\tWed, 23 Jun 2021 07:33:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2CBB468935;\n\tWed, 23 Jun 2021 09:33:45 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C42F68931\n\tfor ;\n\tWed, 23 Jun 2021 09:33:44 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 908E22000C;\n\tWed, 23 Jun 2021 07:33:43 +0000 (UTC)"],"Date":"Wed, 23 Jun 2021 09:34:32 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17711,"web_url":"https://patchwork.libcamera.org/comment/17711/","msgid":"","date":"2021-06-23T08:00:30","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo,\n\nOn Wed, Jun 23, 2021 at 4:33 PM Jacopo Mondi wrote:\n>\n> Hi Hiro,\n>\n> On Wed, Jun 23, 2021 at 12:43:22PM +0900, Hirokazu Honda wrote:\n> > Hi Jacopo,\n> >\n> > On Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n> > >\n> > > Hi Hiro,\n> > >\n> > > On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > > > Turns off a sensor test pattern mode at the initialization of the\n> > > > sensor. Without this, the camera sensor is configured with the last\n> > > > test pattern mode that has been set.\n> > > >\n> > > > Signed-off-by: Hirokazu Honda \n> > > > ---\n> > > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > > 1 file changed, 8 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > index 1be2cbcd..8548f749 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > @@ -9,6 +9,7 @@\n> > > >\n> > > > #include \n> > > >\n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > #include \n> > > > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > > >\n> > > > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> > > >\n> > > > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > > > + if (ret) {\n> > > > + LOG(IPU3, Error)\n> > > > + << \"Failed to reset test pattern mode: \" << ret;\n> > > > + return ret;\n> > > > + }\n> > > > +\n> > >\n> > > Now I see why you need all the checks in the previous patch. But, do\n> > > we need to do so ? Isn't the application which is in control of the\n> > > test pattern ? Shouldn't the app explicitly disable it if they want to\n> > > ?\n> > >\n> >\n> > An application implementation is not controlled by us.\n> > It can specify an invalid test pattern mode. I would like to have the guard.\n> >\n>\n> What I meant was summarized in the below here question\n> \"Shouldn't the app explicitly disable it if they want to ?\"\n>\n> Rearding the value of the test pattern to apply, I was almost sure\n> that the control validator verified the provided value is in the range\n> of supported ones. But it does only verify that the control id is\n> valid for the camera (see camera_controls.cpp). It would be rather\n> straightforward to add a .validate() to ControlInfoMap though and make\n> sure all the values we receive are valid in the current ranges...\n>\n\nI assume the task is orthogonal to this patch series, although it is\nbetter to do before this series.\nAdditionally I am so puzzled by the control classes that I couldn't\nproperly make it.\nMay I ask you to do that?\n\n> > > Shouldn't the app explicitly disable it if they want to\n> >\n> > I am not sure honestly. I *think* most apps don't reset the test\n> > pattern (i.e. disable) in either start up and termination, because a\n> > v4l2 sensor driver resets the test pattern in open() or close().\n>\n> I think that's part of the API semantic we have to better define,\n> that's why I wanted to rope Laurent in.\n>\n> What v4l2 does should not strictly drive our design though.\n>\n> Thanks\n> j\n>\n> > But the libcamera implementation keeps opening the node and in\n> > libcamera we should reset in configure().\n> >\n> > -Hiro\n> > > > return 0;\n> > > > }\n> > > >\n> > > > --\n> > > > 2.32.0.288.g62a8d224e6-goog\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 22AC0C321A\n\tfor ;\n\tWed, 23 Jun 2021 08:00:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8634C6893A;\n\tWed, 23 Jun 2021 10:00:41 +0200 (CEST)","from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com\n\t[IPv6:2a00:1450:4864:20::62e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C316B68932\n\tfor ;\n\tWed, 23 Jun 2021 10:00:40 +0200 (CEST)","by mail-ej1-x62e.google.com with SMTP id nd37so2625091ejc.3\n\tfor ;\n\tWed, 23 Jun 2021 01:00:40 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"kogVP3cg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=zf02UiPOnMIOrEVLK3Lavz3T6Wob7LhbdDrmXj/cxpg=;\n\tb=kogVP3cgncyhl4cVt/wDAPX7OLoJcUBh9sSE2Xpoyvh7lgifX5bIjSeB4V7DUxngqW\n\tzMQf/J/3C1JsXoRgXekSdt/2yI/W5+EN1uEKaLDkTaRWnKyBdlt2q6scnPZ0SSoedbPL\n\tdQYNkOxIHBfLptkPCRPpyX/To633NhPMc83uQ=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=zf02UiPOnMIOrEVLK3Lavz3T6Wob7LhbdDrmXj/cxpg=;\n\tb=ratKBW7d+zR9IvAbSwe4W9yFVzF74FPeAd7FfCflxANfi+UHkLkQ3vhH2dVKWEI3n2\n\tIgx4CZ1ThMpCWiG/HgDaMQ7ENKzslRoBjvjP1c0oF3AKqnE+AbFxvPD4AarWTkxJQSoj\n\t/QfTVYw28WJimJI+Z+imAlqpVsW+NYk98P2Leh8CMfK+r4WlLQVqgESmlyqnj8qSKqGY\n\t9ac1j95h8WhvVy3yaY3NTS3hFV9T46Yoxy1zYEo3osWORqzacpLAJGLYAhsFoxtP48B9\n\tSSCLzDpBSZ2T0WjNlP35Chhy0yE2NgzdPkUOwgEq7VeC2DpCL0d90vvv10EYpMhefYEc\n\tb6TQ==","X-Gm-Message-State":"AOAM532TUTDdCDmaJJkkACNIv0FQTsa/ow1UtffTXcM0u987h97he/5z\n\tRDZUp/3MefhRPQkcIYYJvLNVq0++zj6uq6xezuC/Vg==","X-Google-Smtp-Source":"ABdhPJxj+Hy8IU8ZTL2/KCm1j6B6XpNpbAsfDoMa4LcApQ/BM09IlfFUntp1RvAzH+HFsnvVbAQbkRpQdxmDY+wKnjc=","X-Received":"by 2002:a17:906:ccc3:: with SMTP id\n\tot3mr8623690ejb.475.1624435240388; \n\tWed, 23 Jun 2021 01:00:40 -0700 (PDT)","MIME-Version":"1.0","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>\n\t\n\t<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>","In-Reply-To":"<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>","From":"Hirokazu Honda ","Date":"Wed, 23 Jun 2021 17:00:30 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17712,"web_url":"https://patchwork.libcamera.org/comment/17712/","msgid":"","date":"2021-06-23T08:06:02","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"+Laurent Pinchart\n\nOn Wed, Jun 23, 2021 at 5:00 PM Hirokazu Honda wrote:\n>\n> Hi Jacopo,\n>\n> On Wed, Jun 23, 2021 at 4:33 PM Jacopo Mondi wrote:\n> >\n> > Hi Hiro,\n> >\n> > On Wed, Jun 23, 2021 at 12:43:22PM +0900, Hirokazu Honda wrote:\n> > > Hi Jacopo,\n> > >\n> > > On Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n> > > >\n> > > > Hi Hiro,\n> > > >\n> > > > On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > > > > Turns off a sensor test pattern mode at the initialization of the\n> > > > > sensor. Without this, the camera sensor is configured with the last\n> > > > > test pattern mode that has been set.\n> > > > >\n> > > > > Signed-off-by: Hirokazu Honda \n> > > > > ---\n> > > > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > > > 1 file changed, 8 insertions(+)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > index 1be2cbcd..8548f749 100644\n> > > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > @@ -9,6 +9,7 @@\n> > > > >\n> > > > > #include \n> > > > >\n> > > > > +#include \n> > > > > #include \n> > > > > #include \n> > > > > #include \n> > > > > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > > > >\n> > > > > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> > > > >\n> > > > > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > > > > + if (ret) {\n> > > > > + LOG(IPU3, Error)\n> > > > > + << \"Failed to reset test pattern mode: \" << ret;\n> > > > > + return ret;\n> > > > > + }\n> > > > > +\n> > > >\n> > > > Now I see why you need all the checks in the previous patch. But, do\n> > > > we need to do so ? Isn't the application which is in control of the\n> > > > test pattern ? Shouldn't the app explicitly disable it if they want to\n> > > > ?\n> > > >\n> > >\n> > > An application implementation is not controlled by us.\n> > > It can specify an invalid test pattern mode. I would like to have the guard.\n> > >\n> >\n> > What I meant was summarized in the below here question\n> > \"Shouldn't the app explicitly disable it if they want to ?\"\n> >\n> > Rearding the value of the test pattern to apply, I was almost sure\n> > that the control validator verified the provided value is in the range\n> > of supported ones. But it does only verify that the control id is\n> > valid for the camera (see camera_controls.cpp). It would be rather\n> > straightforward to add a .validate() to ControlInfoMap though and make\n> > sure all the values we receive are valid in the current ranges...\n> >\n>\n> I assume the task is orthogonal to this patch series, although it is\n> better to do before this series.\n> Additionally I am so puzzled by the control classes that I couldn't\n> properly make it.\n> May I ask you to do that?\n>\n> > > > Shouldn't the app explicitly disable it if they want to\n> > >\n> > > I am not sure honestly. I *think* most apps don't reset the test\n> > > pattern (i.e. disable) in either start up and termination, because a\n> > > v4l2 sensor driver resets the test pattern in open() or close().\n> >\n> > I think that's part of the API semantic we have to better define,\n> > that's why I wanted to rope Laurent in.\n> >\n> > What v4l2 does should not strictly drive our design though.\n> >\n> > Thanks\n> > j\n> >\n> > > But the libcamera implementation keeps opening the node and in\n> > > libcamera we should reset in configure().\n> > >\n> > > -Hiro\n> > > > > return 0;\n> > > > > }\n> > > > >\n> > > > > --\n> > > > > 2.32.0.288.g62a8d224e6-goog\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 9713DC321B\n\tfor ;\n\tWed, 23 Jun 2021 08:06:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E41E868935;\n\tWed, 23 Jun 2021 10:06:13 +0200 (CEST)","from mail-ed1-x532.google.com (mail-ed1-x532.google.com\n\t[IPv6:2a00:1450:4864:20::532])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C4F868931\n\tfor ;\n\tWed, 23 Jun 2021 10:06:12 +0200 (CEST)","by mail-ed1-x532.google.com with SMTP id h17so2158385edw.11\n\tfor ;\n\tWed, 23 Jun 2021 01:06:12 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"L5WtUwV/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=tSW2Nk/oDSTNnh0FSNTM8KbfE85SAt0MaOZIG4wIVaY=;\n\tb=L5WtUwV/s/V62t++a2q/KrtZZMFn7vh5kjZ4Z04UTi+jBlt36TWM3zEGZ8Il3F29hV\n\t5MuvglRj+GD/SyhgTEDSKNv2TABnkhzoSNk/QMJ9XVrQ0hBv0ShvCln8R3PhKRMSowuS\n\tVgkI/FsL17Kc4T6pY14v91+Qey+wWb1W5RF4M=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=tSW2Nk/oDSTNnh0FSNTM8KbfE85SAt0MaOZIG4wIVaY=;\n\tb=dl5mlg/MVqLx13H+tpT84MIIGS09IKMGcu20N2Hqmuvb3NsP7J+n+AXjMBonKiQOrh\n\t4jITEDyBhcFXuMbhE8JHbCfKtt3m8uGEAs/ifeMf0jrlHJZpO3exntN+nuXJx64/7IhJ\n\ta2niEzlsuFCeaidxTS0tsMSnX3TPCPR3/lrPbK0WHUWgSfpL+v0NCh/XzY73dSrlx4ts\n\to2Ojy8tH1fOuH7UfzbVkgMSHEH76xJmmzbYeO+40OxfxAk7L1lh45BZ4HZNUZp9jWYKx\n\tCV0EmshVFObMXD+1fS01wXAVrE0iYhgmTIeprf5X4PA6kWNbdTWBgdppyZp5kJIIl/kn\n\tl6aA==","X-Gm-Message-State":"AOAM5324xmLKk4RjYkP22G//Vs2rVl5rGWkBd7yhW7kyIbe5c3gD+LFL\n\tpRVZUWEIRlUudsuKH/rhrmqc//7ytm5M7FLYDI7VAg==","X-Google-Smtp-Source":"ABdhPJx+2M1VKSM30lyy42JM6K985ol/LnUx3mDwYuTZzjRu4q8nGSAGgp7zkeSHUTlOg8wJWLU6uzvXGFGGCXrfiY8=","X-Received":"by 2002:a05:6402:3548:: with SMTP id\n\tf8mr8993434edd.116.1624435572279; \n\tWed, 23 Jun 2021 01:06:12 -0700 (PDT)","MIME-Version":"1.0","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>\n\t\n\t<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Wed, 23 Jun 2021 17:06:02 +0900","Message-ID":"","To":"Jacopo Mondi ,\n\tLaurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17714,"web_url":"https://patchwork.libcamera.org/comment/17714/","msgid":"<20210623080825.fnt7mgnj5chxvruf@uno.localdomain>","date":"2021-06-23T08:08:25","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro\n\nOn Wed, Jun 23, 2021 at 05:00:30PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo,\n>\n> On Wed, Jun 23, 2021 at 4:33 PM Jacopo Mondi wrote:\n> >\n> > Hi Hiro,\n> >\n> > On Wed, Jun 23, 2021 at 12:43:22PM +0900, Hirokazu Honda wrote:\n> > > Hi Jacopo,\n> > >\n> > > On Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n> > > >\n> > > > Hi Hiro,\n> > > >\n> > > > On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > > > > Turns off a sensor test pattern mode at the initialization of the\n> > > > > sensor. Without this, the camera sensor is configured with the last\n> > > > > test pattern mode that has been set.\n> > > > >\n> > > > > Signed-off-by: Hirokazu Honda \n> > > > > ---\n> > > > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > > > 1 file changed, 8 insertions(+)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > index 1be2cbcd..8548f749 100644\n> > > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > @@ -9,6 +9,7 @@\n> > > > >\n> > > > > #include \n> > > > >\n> > > > > +#include \n> > > > > #include \n> > > > > #include \n> > > > > #include \n> > > > > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > > > >\n> > > > > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> > > > >\n> > > > > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > > > > + if (ret) {\n> > > > > + LOG(IPU3, Error)\n> > > > > + << \"Failed to reset test pattern mode: \" << ret;\n> > > > > + return ret;\n> > > > > + }\n> > > > > +\n> > > >\n> > > > Now I see why you need all the checks in the previous patch. But, do\n> > > > we need to do so ? Isn't the application which is in control of the\n> > > > test pattern ? Shouldn't the app explicitly disable it if they want to\n> > > > ?\n> > > >\n> > >\n> > > An application implementation is not controlled by us.\n> > > It can specify an invalid test pattern mode. I would like to have the guard.\n> > >\n> >\n> > What I meant was summarized in the below here question\n> > \"Shouldn't the app explicitly disable it if they want to ?\"\n> >\n> > Rearding the value of the test pattern to apply, I was almost sure\n> > that the control validator verified the provided value is in the range\n> > of supported ones. But it does only verify that the control id is\n> > valid for the camera (see camera_controls.cpp). It would be rather\n> > straightforward to add a .validate() to ControlInfoMap though and make\n> > sure all the values we receive are valid in the current ranges...\n> >\n>\n> I assume the task is orthogonal to this patch series, although it is\n> better to do before this series.\n> Additionally I am so puzzled by the control classes that I couldn't\n> properly make it.\n> May I ask you to do that?\n\nI wasn't certainly asking you do so as part of this series :)\nI was suprised it was not there, that's all :)","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 BADF7C321B\n\tfor ;\n\tWed, 23 Jun 2021 08:07:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8171268936;\n\tWed, 23 Jun 2021 10:07:37 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BADAB68932\n\tfor ;\n\tWed, 23 Jun 2021 10:07:36 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 1F453FF815;\n\tWed, 23 Jun 2021 08:07:35 +0000 (UTC)"],"Date":"Wed, 23 Jun 2021 10:08:25 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210623080825.fnt7mgnj5chxvruf@uno.localdomain>","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>\n\t\n\t<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17716,"web_url":"https://patchwork.libcamera.org/comment/17716/","msgid":"","date":"2021-06-23T08:10:07","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo\n\nOn Wed, Jun 23, 2021 at 5:07 PM Jacopo Mondi wrote:\n>\n> Hi Hiro\n>\n> On Wed, Jun 23, 2021 at 05:00:30PM +0900, Hirokazu Honda wrote:\n> > Hi Jacopo,\n> >\n> > On Wed, Jun 23, 2021 at 4:33 PM Jacopo Mondi wrote:\n> > >\n> > > Hi Hiro,\n> > >\n> > > On Wed, Jun 23, 2021 at 12:43:22PM +0900, Hirokazu Honda wrote:\n> > > > Hi Jacopo,\n> > > >\n> > > > On Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n> > > > >\n> > > > > Hi Hiro,\n> > > > >\n> > > > > On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > > > > > Turns off a sensor test pattern mode at the initialization of the\n> > > > > > sensor. Without this, the camera sensor is configured with the last\n> > > > > > test pattern mode that has been set.\n> > > > > >\n> > > > > > Signed-off-by: Hirokazu Honda \n> > > > > > ---\n> > > > > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > > > > 1 file changed, 8 insertions(+)\n> > > > > >\n> > > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > > index 1be2cbcd..8548f749 100644\n> > > > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > > @@ -9,6 +9,7 @@\n> > > > > >\n> > > > > > #include \n> > > > > >\n> > > > > > +#include \n> > > > > > #include \n> > > > > > #include \n> > > > > > #include \n> > > > > > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > > > > >\n> > > > > > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> > > > > >\n> > > > > > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > > > > > + if (ret) {\n> > > > > > + LOG(IPU3, Error)\n> > > > > > + << \"Failed to reset test pattern mode: \" << ret;\n> > > > > > + return ret;\n> > > > > > + }\n> > > > > > +\n> > > > >\n> > > > > Now I see why you need all the checks in the previous patch. But, do\n> > > > > we need to do so ? Isn't the application which is in control of the\n> > > > > test pattern ? Shouldn't the app explicitly disable it if they want to\n> > > > > ?\n> > > > >\n> > > >\n> > > > An application implementation is not controlled by us.\n> > > > It can specify an invalid test pattern mode. I would like to have the guard.\n> > > >\n> > >\n> > > What I meant was summarized in the below here question\n> > > \"Shouldn't the app explicitly disable it if they want to ?\"\n> > >\n> > > Rearding the value of the test pattern to apply, I was almost sure\n> > > that the control validator verified the provided value is in the range\n> > > of supported ones. But it does only verify that the control id is\n> > > valid for the camera (see camera_controls.cpp). It would be rather\n> > > straightforward to add a .validate() to ControlInfoMap though and make\n> > > sure all the values we receive are valid in the current ranges...\n> > >\n> >\n> > I assume the task is orthogonal to this patch series, although it is\n> > better to do before this series.\n> > Additionally I am so puzzled by the control classes that I couldn't\n> > properly make it.\n> > May I ask you to do that?\n>\n> I wasn't certainly asking you do so as part of this series :)\n> I was suprised it was not there, that's all :)\n\nOkay, thanks!\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 8CDC0C321A\n\tfor ;\n\tWed, 23 Jun 2021 08:10:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5178268936;\n\tWed, 23 Jun 2021 10:10:19 +0200 (CEST)","from mail-ed1-x529.google.com (mail-ed1-x529.google.com\n\t[IPv6:2a00:1450:4864:20::529])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C066C68932\n\tfor ;\n\tWed, 23 Jun 2021 10:10:17 +0200 (CEST)","by mail-ed1-x529.google.com with SMTP id s6so2173640edu.10\n\tfor ;\n\tWed, 23 Jun 2021 01:10:17 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"luqgveVC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=9tCbiH9qWCYjjK6+Tej308NAopL0l8qpUGL6sq+zB70=;\n\tb=luqgveVCbi4n4+oEfC3dPLTl80W8XX1vvagoGxiLgK8Cqat4NSfnk+1G1/IUQlxRfY\n\tVZy17guooGMNimLR9ftW3pIfO1eTTwmX1vAoc+rEeZrcCwlLFHV98xCPN/YPMmoq5RBs\n\tH0F7B+Dg7xngGCPD1kOrpI4K0plMV3WiU/FiA=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=9tCbiH9qWCYjjK6+Tej308NAopL0l8qpUGL6sq+zB70=;\n\tb=PuePWOnx3+4vc+VMGBfvQZeeOC23t22k0XTs2TNoIrID3F51ePotLqrxSUWiu8J9z8\n\tdT2LjBirvT44y3uIAQ2rHk4Li7aTtUbIobyARGbl96f+GCxKYtI4Doe/r1ZqQdK+RIvx\n\tTr3mrZX+l/MCBoymVJsoQKZ/noUI1qqZJIgGKNG7czoIbKwIo8WUIXAR50uoByX7uAJj\n\tvD+ecuxMzxCffgRfGPnQLvDzgKcr1zpeMEZTNVdwgCIuGdlg9tn00lLf9pFdEcyV1H7e\n\tYyzfS2mHd7ykmVHRSkcd68CYHQv62HTp+lhfFiJuxqeDmBCowObkBqrMP4he1s9NVxuT\n\tdtXg==","X-Gm-Message-State":"AOAM5300M7zIuTKGdUSdo8XC0VEqzkhGZWW1Kbhuc0xWquXzNmHFrUwR\n\tRoMhbbbaKKtRWHU5OZ/F7OJUq1qxuXZBiRnMhDJO8Q==","X-Google-Smtp-Source":"ABdhPJwV72b9vUgRMKiFl2a3VUxC/T82kdgHaSaln8nEBjo5de4rUX/I9DLYmTJTiklNmpyDKxdejbUfZBQEXEOjFGE=","X-Received":"by 2002:a05:6402:151:: with SMTP id\n\ts17mr10455823edu.233.1624435817485; \n\tWed, 23 Jun 2021 01:10:17 -0700 (PDT)","MIME-Version":"1.0","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>\n\t\n\t<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>\n\t\n\t<20210623080825.fnt7mgnj5chxvruf@uno.localdomain>","In-Reply-To":"<20210623080825.fnt7mgnj5chxvruf@uno.localdomain>","From":"Hirokazu Honda ","Date":"Wed, 23 Jun 2021 17:10:07 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17844,"web_url":"https://patchwork.libcamera.org/comment/17844/","msgid":"","date":"2021-06-28T03:28:50","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Wed, Jun 23, 2021 at 09:34:32AM +0200, Jacopo Mondi wrote:\n> On Wed, Jun 23, 2021 at 12:43:22PM +0900, Hirokazu Honda wrote:\n> > On Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n> > > On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > > > Turns off a sensor test pattern mode at the initialization of the\n> > > > sensor. Without this, the camera sensor is configured with the last\n> > > > test pattern mode that has been set.\n> > > >\n> > > > Signed-off-by: Hirokazu Honda \n> > > > ---\n> > > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > > 1 file changed, 8 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > index 1be2cbcd..8548f749 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > @@ -9,6 +9,7 @@\n> > > >\n> > > > #include \n> > > >\n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > #include \n> > > > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > > >\n> > > > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> > > >\n> > > > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > > > + if (ret) {\n> > > > + LOG(IPU3, Error)\n> > > > + << \"Failed to reset test pattern mode: \" << ret;\n> > > > + return ret;\n> > > > + }\n> > > > +\n> > >\n> > > Now I see why you need all the checks in the previous patch. But, do\n> > > we need to do so ? Isn't the application which is in control of the\n> > > test pattern ? Shouldn't the app explicitly disable it if they want to\n> > > ?\n> >\n> > An application implementation is not controlled by us.\n> > It can specify an invalid test pattern mode. I would like to have the guard.\n> \n> What I meant was summarized in the below here question\n> \"Shouldn't the app explicitly disable it if they want to ?\"\n> \n> Rearding the value of the test pattern to apply, I was almost sure\n> that the control validator verified the provided value is in the range\n> of supported ones. But it does only verify that the control id is\n> valid for the camera (see camera_controls.cpp). It would be rather\n> straightforward to add a .validate() to ControlInfoMap though and make\n> sure all the values we receive are valid in the current ranges...\n> \n> > > Shouldn't the app explicitly disable it if they want to\n> >\n> > I am not sure honestly. I *think* most apps don't reset the test\n> > pattern (i.e. disable) in either start up and termination, because a\n> > v4l2 sensor driver resets the test pattern in open() or close().\n> \n> I think that's part of the API semantic we have to better define,\n> that's why I wanted to rope Laurent in.\n> \n> What v4l2 does should not strictly drive our design though.\n\nWe need to ensure a consistent startup state, regardless of how the\ncamera has been used before. There are (at least) two ways to do so:\n\n- Force all applications to set all controls in the first request\n- Set default values internally (at the latest) at start time for any\n control that isn't specified by the application\n\nThe first option would require implementing something similar to the\nAndroid's request templates, as we really can't let applications to this\nmanually, they won't get it right.\n\nIn both options, it should be the pipeline handler and IPA that control\nwhat default values to set (either directly in the second option, or\nindirectly through the provided request template in the first option).\nHowever, it would be nice to avoid having to duplicate the above code in\nall pipeline handlers, so some involvement of the CameraSensor class\nwould help. It also needs need to be done in a way that let the pipeline\nhandler override any decision made by the CameraSensor class.\n\n> > But the libcamera implementation keeps opening the node and in\n> > libcamera we should reset in configure().\n> >\n> > > > return 0;\n> > > > }\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 AF26AC321D\n\tfor ;\n\tMon, 28 Jun 2021 03:28:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6FCE9684D5;\n\tMon, 28 Jun 2021 05:28:53 +0200 (CEST)","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 53BA36028C\n\tfor ;\n\tMon, 28 Jun 2021 05:28:52 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D57F157E;\n\tMon, 28 Jun 2021 05:28:51 +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=\"t1u/wS9m\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624850932;\n\tbh=GoBPNOfz1IZyrVjOZJC/2zzTmpIC2efYznEZYpgdkLo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t1u/wS9mP+5Uz9LHPFSxRRq0tIeLwhV2QkEkW+xhqP16UE5CiQpBz7oKNEcu+gXt5\n\tDsehmWyY/QKR3g5S/1/IJhYSm7nX2r/hkRcc5CNtbm6A5raWjnwKVPGL2966mLeGYo\n\teWjdoD9hrS3zqcVM5PCRfjQMnQy0Yyolc7d1ZhzI=","Date":"Mon, 28 Jun 2021 06:28:50 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>\n\t\n\t<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17862,"web_url":"https://patchwork.libcamera.org/comment/17862/","msgid":"","date":"2021-06-28T05:27:59","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent,\n\nOn Mon, Jun 28, 2021 at 12:28 PM Laurent Pinchart\n wrote:\n>\n> Hello,\n>\n> On Wed, Jun 23, 2021 at 09:34:32AM +0200, Jacopo Mondi wrote:\n> > On Wed, Jun 23, 2021 at 12:43:22PM +0900, Hirokazu Honda wrote:\n> > > On Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n> > > > On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > > > > Turns off a sensor test pattern mode at the initialization of the\n> > > > > sensor. Without this, the camera sensor is configured with the last\n> > > > > test pattern mode that has been set.\n> > > > >\n> > > > > Signed-off-by: Hirokazu Honda \n> > > > > ---\n> > > > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > > > 1 file changed, 8 insertions(+)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > index 1be2cbcd..8548f749 100644\n> > > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > @@ -9,6 +9,7 @@\n> > > > >\n> > > > > #include \n> > > > >\n> > > > > +#include \n> > > > > #include \n> > > > > #include \n> > > > > #include \n> > > > > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > > > >\n> > > > > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> > > > >\n> > > > > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > > > > + if (ret) {\n> > > > > + LOG(IPU3, Error)\n> > > > > + << \"Failed to reset test pattern mode: \" << ret;\n> > > > > + return ret;\n> > > > > + }\n> > > > > +\n> > > >\n> > > > Now I see why you need all the checks in the previous patch. But, do\n> > > > we need to do so ? Isn't the application which is in control of the\n> > > > test pattern ? Shouldn't the app explicitly disable it if they want to\n> > > > ?\n> > >\n> > > An application implementation is not controlled by us.\n> > > It can specify an invalid test pattern mode. I would like to have the guard.\n> >\n> > What I meant was summarized in the below here question\n> > \"Shouldn't the app explicitly disable it if they want to ?\"\n> >\n> > Rearding the value of the test pattern to apply, I was almost sure\n> > that the control validator verified the provided value is in the range\n> > of supported ones. But it does only verify that the control id is\n> > valid for the camera (see camera_controls.cpp). It would be rather\n> > straightforward to add a .validate() to ControlInfoMap though and make\n> > sure all the values we receive are valid in the current ranges...\n> >\n> > > > Shouldn't the app explicitly disable it if they want to\n> > >\n> > > I am not sure honestly. I *think* most apps don't reset the test\n> > > pattern (i.e. disable) in either start up and termination, because a\n> > > v4l2 sensor driver resets the test pattern in open() or close().\n> >\n> > I think that's part of the API semantic we have to better define,\n> > that's why I wanted to rope Laurent in.\n> >\n> > What v4l2 does should not strictly drive our design though.\n>\n> We need to ensure a consistent startup state, regardless of how the\n> camera has been used before. There are (at least) two ways to do so:\n>\n> - Force all applications to set all controls in the first request\n> - Set default values internally (at the latest) at start time for any\n> control that isn't specified by the application\n>\n> The first option would require implementing something similar to the\n> Android's request templates, as we really can't let applications to this\n> manually, they won't get it right.\n>\n> In both options, it should be the pipeline handler and IPA that control\n> what default values to set (either directly in the second option, or\n> indirectly through the provided request template in the first option).\n> However, it would be nice to avoid having to duplicate the above code in\n> all pipeline handlers, so some involvement of the CameraSensor class\n> would help. It also needs need to be done in a way that let the pipeline\n> handler override any decision made by the CameraSensor class.\n>\n\nCould you elaborate on the improvement of the CameraSensor class?\n\n> > > But the libcamera implementation keeps opening the node and in\n> > > libcamera we should reset in configure().\n> > >\n> > > > > return 0;\n> > > > > }\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 A2CDCC321A\n\tfor ;\n\tMon, 28 Jun 2021 05:28:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 55534684D5;\n\tMon, 28 Jun 2021 07:28:13 +0200 (CEST)","from mail-ed1-x536.google.com (mail-ed1-x536.google.com\n\t[IPv6:2a00:1450:4864:20::536])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 415F26028C\n\tfor ;\n\tMon, 28 Jun 2021 07:28:11 +0200 (CEST)","by mail-ed1-x536.google.com with SMTP id q14so23893898eds.5\n\tfor ;\n\tSun, 27 Jun 2021 22:28:11 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"YYMIwh+C\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=pLFJEnazt+6oIIAgRPxQ9nO5+0Tso588H2yxbJ+BDXU=;\n\tb=YYMIwh+CxGgq7yFMv904XGXEE3rfGBphdS6XOnGXO7otTWBGBr/aPfT3xw5MqHsyIA\n\t1Ytqqpo3fEjhC5tmpT6cMtgwpR5NmkT8bBCCDvVkFaRaW0AF8MdNhxs5MiGAGJc298TS\n\tZyFC2mxLKh1XPGEdggQwVam8la+kI4Kda/1tY=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=pLFJEnazt+6oIIAgRPxQ9nO5+0Tso588H2yxbJ+BDXU=;\n\tb=DHL6MC/PU0H5V4aCMR074bp7AqhUTzVx/nDQgWeQRyT9QIWMQT1F3AsZbGBazmz6+K\n\tQ3fmNy7Rno2C8YdduTyxp9aHp8bLLF3Y3JoPWHc1DOVqEmELnbGhvLcpNmt2XmSKYaLL\n\tfwJMp/ZbWw8mWIOc7I5jjBXIrTI6G5+vZL6UiJzmaA5fcGZPXX+Zr6OHRc00eYw2gH81\n\tgJkqwUZsegYSh6fQIIjIQBGXCXxj8WrEHOAcPN9nbfPDUrBpaiQplgOOlU694nWN9uLc\n\tRgiJsDJQtwlsQ2kYU8Ge9aKk+nSUtJtJUpTDsYpzUtJOKw6fHIlE0UODZiULEmpHKc1O\n\tEWKQ==","X-Gm-Message-State":"AOAM5326EzZunz9UxCTOevl9OyyvyBhjThop8CVkMfUAcji7ZVnL+J8Q\n\tgwsbE4LQ4ktMslErfCvHzCQZ3fXJf1QagC2FGgiHiA==","X-Google-Smtp-Source":"ABdhPJya/zUl4Ua4uimDTR8SFgr/Lvo+rsr7Zg4yuwjp7iatCZ415GY4QMfS+7Dx/AfHRDmfnxHf6rgYfdg/1eZJxzk=","X-Received":"by 2002:a05:6402:290b:: with SMTP id\n\tee11mr29999801edb.325.1624858090989; \n\tSun, 27 Jun 2021 22:28:10 -0700 (PDT)","MIME-Version":"1.0","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>\n\t\n\t<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Mon, 28 Jun 2021 14:27:59 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17868,"web_url":"https://patchwork.libcamera.org/comment/17868/","msgid":"","date":"2021-06-28T06:28:09","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nOn Mon, Jun 28, 2021 at 02:27:59PM +0900, Hirokazu Honda wrote:\n> On Mon, Jun 28, 2021 at 12:28 PM Laurent Pinchart wrote:\n> > On Wed, Jun 23, 2021 at 09:34:32AM +0200, Jacopo Mondi wrote:\n> > > On Wed, Jun 23, 2021 at 12:43:22PM +0900, Hirokazu Honda wrote:\n> > > > On Tue, Jun 22, 2021 at 7:33 PM Jacopo Mondi wrote:\n> > > > > On Tue, Jun 22, 2021 at 11:36:52AM +0900, Hirokazu Honda wrote:\n> > > > > > Turns off a sensor test pattern mode at the initialization of the\n> > > > > > sensor. Without this, the camera sensor is configured with the last\n> > > > > > test pattern mode that has been set.\n> > > > > >\n> > > > > > Signed-off-by: Hirokazu Honda \n> > > > > > ---\n> > > > > > src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++\n> > > > > > 1 file changed, 8 insertions(+)\n> > > > > >\n> > > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > > index 1be2cbcd..8548f749 100644\n> > > > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > > > @@ -9,6 +9,7 @@\n> > > > > >\n> > > > > > #include \n> > > > > >\n> > > > > > +#include \n> > > > > > #include \n> > > > > > #include \n> > > > > > #include \n> > > > > > @@ -192,6 +193,13 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > > > > >\n> > > > > > LOG(IPU3, Debug) << \"CIO2 output format \" << outputFormat->toString();\n> > > > > >\n> > > > > > + ret = sensor_->setTestPatternMode(controls::draft::TestPatternModeOff);\n> > > > > > + if (ret) {\n> > > > > > + LOG(IPU3, Error)\n> > > > > > + << \"Failed to reset test pattern mode: \" << ret;\n> > > > > > + return ret;\n> > > > > > + }\n> > > > > > +\n> > > > >\n> > > > > Now I see why you need all the checks in the previous patch. But, do\n> > > > > we need to do so ? Isn't the application which is in control of the\n> > > > > test pattern ? Shouldn't the app explicitly disable it if they want to\n> > > > > ?\n> > > >\n> > > > An application implementation is not controlled by us.\n> > > > It can specify an invalid test pattern mode. I would like to have the guard.\n> > >\n> > > What I meant was summarized in the below here question\n> > > \"Shouldn't the app explicitly disable it if they want to ?\"\n> > >\n> > > Rearding the value of the test pattern to apply, I was almost sure\n> > > that the control validator verified the provided value is in the range\n> > > of supported ones. But it does only verify that the control id is\n> > > valid for the camera (see camera_controls.cpp). It would be rather\n> > > straightforward to add a .validate() to ControlInfoMap though and make\n> > > sure all the values we receive are valid in the current ranges...\n> > >\n> > > > > Shouldn't the app explicitly disable it if they want to\n> > > >\n> > > > I am not sure honestly. I *think* most apps don't reset the test\n> > > > pattern (i.e. disable) in either start up and termination, because a\n> > > > v4l2 sensor driver resets the test pattern in open() or close().\n> > >\n> > > I think that's part of the API semantic we have to better define,\n> > > that's why I wanted to rope Laurent in.\n> > >\n> > > What v4l2 does should not strictly drive our design though.\n> >\n> > We need to ensure a consistent startup state, regardless of how the\n> > camera has been used before. There are (at least) two ways to do so:\n> >\n> > - Force all applications to set all controls in the first request\n> > - Set default values internally (at the latest) at start time for any\n> > control that isn't specified by the application\n> >\n> > The first option would require implementing something similar to the\n> > Android's request templates, as we really can't let applications to this\n> > manually, they won't get it right.\n> >\n> > In both options, it should be the pipeline handler and IPA that control\n> > what default values to set (either directly in the second option, or\n> > indirectly through the provided request template in the first option).\n> > However, it would be nice to avoid having to duplicate the above code in\n> > all pipeline handlers, so some involvement of the CameraSensor class\n> > would help. It also needs need to be done in a way that let the pipeline\n> > handler override any decision made by the CameraSensor class.\n> \n> Could you elaborate on the improvement of the CameraSensor class?\n\nI'd like to avoid duplicating the sensor_->setTestPatternMode() in the\nconfigure() function of every pipeline handler. If it was just for the\ntest pattern, the call could simply move to the CameraSensor class.\nHowever, there can be other controls too, and for some of them, the\npipeline handler and/or IPA will need to select or at least influence\nthe default value. I'd like the CameraSensor class to help as much as\npossible to avoid code duplication in pipeline handlers. How this could\nbe done, I have no idea, it needs to be researched and designed.\n\n> > > > But the libcamera implementation keeps opening the node and in\n> > > > libcamera we should reset in configure().\n> > > >\n> > > > > > return 0;\n> > > > > > }\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 9E07AC321A\n\tfor ;\n\tMon, 28 Jun 2021 06:28:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2F06684D5;\n\tMon, 28 Jun 2021 08:28:12 +0200 (CEST)","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 843BE6028C\n\tfor ;\n\tMon, 28 Jun 2021 08:28:11 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E7884B8A;\n\tMon, 28 Jun 2021 08:28:10 +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=\"uSGWnj+D\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624861691;\n\tbh=XPzO4FotBrDC/hVk6wIXdfR9hEJcmIclUVh4JPoWU0I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uSGWnj+DyOLouWDL/7UXSmo1D3N5cuWoTtUPkrUF3aFyPCXPxo5g0E7DUiaaa5C0P\n\taePJWrRVTRiSi9GlmspCMiFZztI69DCW945SLq37pLj7ckiNoCPFndXoFA9UG1hVxq\n\th7nraHfBhu9kdmKGKIsyFSt27Gm2+hKmV3RTy12Q=","Date":"Mon, 28 Jun 2021 09:28:09 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210622023654.969162-1-hiroh@chromium.org>\n\t<20210622023654.969162-3-hiroh@chromium.org>\n\t<20210622103440.kob4vei3us5eapx6@uno.localdomain>\n\t\n\t<20210623073432.75fiqm2g4thqrs7m@uno.localdomain>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/5] libcamera: ipu3: Disable a\n\tsensor test pattern mode at initialization","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]