[{"id":31818,"web_url":"https://patchwork.libcamera.org/comment/31818/","msgid":"<172937712292.2485972.9882413556210770900@ping.linuxembedded.co.uk>","date":"2024-10-19T22:32:02","subject":"Re: [PATCH] libcamera: Add open() and close() in CameraLens","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Harvey Yang (2024-10-17 08:39:47)\n> From: Han-Lin Chen <hanlinchen@chromium.org>\n> \n> This allows pipeline handlers to save some power when a camera is close.\n\nThis seems reasonable - but makes me wonder - is there similar\nlimitations with the CameraSensor class that has a very similar design?\n\n\n> \n> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>\n> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> ---\n>  include/libcamera/internal/camera_lens.h |  4 ++++\n>  src/libcamera/camera_lens.cpp            | 17 +++++++++++++++++\n>  2 files changed, 21 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n> index 5a4b993bb..095056791 100644\n> --- a/include/libcamera/internal/camera_lens.h\n> +++ b/include/libcamera/internal/camera_lens.h\n> @@ -26,6 +26,10 @@ public:\n>         ~CameraLens();\n>  \n>         int init();\n\nShould we be calling close() at the end of init()? Perhaps with some\nsort of way to make sure if an API call is used on a now closed device\nit gets opened again first ? (i.e. mostly perhaps V4L2Device::setControls)?\n\n> +\n> +       int open();\n> +       void close();\n> +\n>         int setFocusPosition(int32_t position);\n>  \n>         const std::string &model() const { return model_; }\n> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n> index ccc2a6a65..039f5ad2a 100644\n> --- a/src/libcamera/camera_lens.cpp\n> +++ b/src/libcamera/camera_lens.cpp\n> @@ -76,6 +76,23 @@ int CameraLens::init()\n>         return 0;\n>  }\n>  \n> +/**\n> + * \\brief Open the subdev\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int CameraLens::open()\n> +{\n> +       return subdev_->open();\n> +}\n> +\n> +/**\n> + * \\brief Close the subdev\n> + */\n> +void CameraLens::close()\n> +{\n> +       subdev_->close();\n> +}\n> +\n>  /**\n>   * \\brief This function sets the focal point of the lens to a specific position.\n>   * \\param[in] position The focal point of the lens\n> -- \n> 2.47.0.rc1.288.g06298d1525-goog\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1DD48BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 19 Oct 2024 22:32:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EDD356538D;\n\tSun, 20 Oct 2024 00:32:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 69D6B65382\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 20 Oct 2024 00:32:06 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7EA58502;\n\tSun, 20 Oct 2024 00:30:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sABxcUj2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729377021;\n\tbh=Yz1dllCQAmCRM0dVgXgkWgPz7dGFSpkwyOgVU/Gn8BU=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=sABxcUj2yP9W20I4lRl/qb2PHgBV5InjhICLjcPgSIWW3Yr05+oGBk8AC4o0XIfo2\n\tBQmBvcLOKXfmnxGoJLnTZwkwxBw04XzoAjCvlGYXDtAuL7kfh8TgzI/LiVh5+0ktVv\n\tQP4n7iAq4P7Vsl4C1GmVT0ELKU+Emtvsnp4p+7Qw=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241017073948.353346-1-chenghaoyang@chromium.org>","References":"<20241017073948.353346-1-chenghaoyang@chromium.org>","Subject":"Re: [PATCH] libcamera: Add open() and close() in CameraLens","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Han-Lin Chen <hanlinchen@chromium.org>,\n\tHarvey Yang <chenghaoyang@chromium.org>","To":"Harvey Yang <chenghaoyang@chromium.org>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Sat, 19 Oct 2024 23:32:02 +0100","Message-ID":"<172937712292.2485972.9882413556210770900@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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31833,"web_url":"https://patchwork.libcamera.org/comment/31833/","msgid":"<20241020160845.GE7770@pendragon.ideasonboard.com>","date":"2024-10-20T16:08:45","subject":"Re: [PATCH] libcamera: Add open() and close() in CameraLens","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"CC'ing Sakari.\n\nOn Sat, Oct 19, 2024 at 11:32:02PM +0100, Kieran Bingham wrote:\n> Quoting Harvey Yang (2024-10-17 08:39:47)\n> > From: Han-Lin Chen <hanlinchen@chromium.org>\n> > \n> > This allows pipeline handlers to save some power when a camera is close.\n> \n> This seems reasonable - but makes me wonder - is there similar\n> limitations with the CameraSensor class that has a very similar design?\n\nI think it's time to bite the bullet and see how to address the issue of\npower management of lens controllers in the kernel. Tying it to open()\nand close() isn't necessarily a great option.\n\n> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>\n> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> > ---\n> >  include/libcamera/internal/camera_lens.h |  4 ++++\n> >  src/libcamera/camera_lens.cpp            | 17 +++++++++++++++++\n> >  2 files changed, 21 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n> > index 5a4b993bb..095056791 100644\n> > --- a/include/libcamera/internal/camera_lens.h\n> > +++ b/include/libcamera/internal/camera_lens.h\n> > @@ -26,6 +26,10 @@ public:\n> >         ~CameraLens();\n> >  \n> >         int init();\n> \n> Should we be calling close() at the end of init()? Perhaps with some\n> sort of way to make sure if an API call is used on a now closed device\n> it gets opened again first ? (i.e. mostly perhaps V4L2Device::setControls)?\n> \n> > +\n> > +       int open();\n> > +       void close();\n> > +\n> >         int setFocusPosition(int32_t position);\n> >  \n> >         const std::string &model() const { return model_; }\n> > diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n> > index ccc2a6a65..039f5ad2a 100644\n> > --- a/src/libcamera/camera_lens.cpp\n> > +++ b/src/libcamera/camera_lens.cpp\n> > @@ -76,6 +76,23 @@ int CameraLens::init()\n> >         return 0;\n> >  }\n> >  \n> > +/**\n> > + * \\brief Open the subdev\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int CameraLens::open()\n> > +{\n> > +       return subdev_->open();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Close the subdev\n> > + */\n> > +void CameraLens::close()\n> > +{\n> > +       subdev_->close();\n> > +}\n> > +\n> >  /**\n> >   * \\brief This function sets the focal point of the lens to a specific position.\n> >   * \\param[in] position The focal point of the lens","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7B28EC3306\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 20 Oct 2024 16:08:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CA10C6538B;\n\tSun, 20 Oct 2024 18:08:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B51765379\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 20 Oct 2024 18:08:51 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7250E352;\n\tSun, 20 Oct 2024 18:07:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QyIpzE/m\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729440425;\n\tbh=16Zp8LP4EXfNknHoNJqXSKaYcwNmm4Nl+GeMZCW5Tvs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QyIpzE/mimyIPZVMw9PRcECd0kubgfaZYFs5o+OPQD1zVpoHaQ1O5/SN39LGyxi0l\n\tYQMplhOU9DV5KaZs4CRBqrKC6AkCUvLsouC9P2GFNQ35r6ujBZbJ6153n5GkfTWLCP\n\tw5L0+XoLU5IPIzu/jC2AQswF+qc8sFMpJDgc6Lh4=","Date":"Sun, 20 Oct 2024 19:08:45 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Harvey Yang <chenghaoyang@chromium.org>,\n\tlibcamera-devel@lists.libcamera.org,\n\tHan-Lin Chen <hanlinchen@chromium.org>,\n\tSakari Ailus <sakari.ailus@iki.fi>","Subject":"Re: [PATCH] libcamera: Add open() and close() in CameraLens","Message-ID":"<20241020160845.GE7770@pendragon.ideasonboard.com>","References":"<20241017073948.353346-1-chenghaoyang@chromium.org>\n\t<172937712292.2485972.9882413556210770900@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<172937712292.2485972.9882413556210770900@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31888,"web_url":"https://patchwork.libcamera.org/comment/31888/","msgid":"<CAEB1ahvfdUmxyonfBoAZVuN9QUqBuT=jcxsBaZqkfCgaRuD4xw@mail.gmail.com>","date":"2024-10-23T09:20:34","subject":"Re: [PATCH] libcamera: Add open() and close() in CameraLens","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Kieran and Laurent,\n\nOn Mon, Oct 21, 2024 at 12:08 AM Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> CC'ing Sakari.\n>\n> On Sat, Oct 19, 2024 at 11:32:02PM +0100, Kieran Bingham wrote:\n> > Quoting Harvey Yang (2024-10-17 08:39:47)\n> > > From: Han-Lin Chen <hanlinchen@chromium.org>\n> > >\n> > > This allows pipeline handlers to save some power when a camera is close.\n> >\n> > This seems reasonable - but makes me wonder - is there similar\n> > limitations with the CameraSensor class that has a very similar design?\n>\n> I think it's time to bite the bullet and see how to address the issue of\n> power management of lens controllers in the kernel. Tying it to open()\n> and close() isn't necessarily a great option.\n\nHan-lin told me that CameraLens is a special case of\nV4L2Subdevice (at least in mtkisp7), that it's powered on when opened.\nOther ones like CameraSensor's `subdev_` might be powered on when\nthe V4L2VideoDevice that receives buffers are `streamOn`ed. Just FYI.\n\n>\n> > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n> > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>\n> > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n> > > ---\n> > >  include/libcamera/internal/camera_lens.h |  4 ++++\n> > >  src/libcamera/camera_lens.cpp            | 17 +++++++++++++++++\n> > >  2 files changed, 21 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n> > > index 5a4b993bb..095056791 100644\n> > > --- a/include/libcamera/internal/camera_lens.h\n> > > +++ b/include/libcamera/internal/camera_lens.h\n> > > @@ -26,6 +26,10 @@ public:\n> > >         ~CameraLens();\n> > >\n> > >         int init();\n> >\n> > Should we be calling close() at the end of init()? Perhaps with some\n> > sort of way to make sure if an API call is used on a now closed device\n> > it gets opened again first ? (i.e. mostly perhaps V4L2Device::setControls)?\n\nHmm, in the current API, I don't see function calls that \"start\" to use\nCameraLens.\n\nBR,\nHarvey\n\n> >\n> > > +\n> > > +       int open();\n> > > +       void close();\n> > > +\n> > >         int setFocusPosition(int32_t position);\n> > >\n> > >         const std::string &model() const { return model_; }\n> > > diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n> > > index ccc2a6a65..039f5ad2a 100644\n> > > --- a/src/libcamera/camera_lens.cpp\n> > > +++ b/src/libcamera/camera_lens.cpp\n> > > @@ -76,6 +76,23 @@ int CameraLens::init()\n> > >         return 0;\n> > >  }\n> > >\n> > > +/**\n> > > + * \\brief Open the subdev\n> > > + * \\return 0 on success or a negative error code otherwise\n> > > + */\n> > > +int CameraLens::open()\n> > > +{\n> > > +       return subdev_->open();\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Close the subdev\n> > > + */\n> > > +void CameraLens::close()\n> > > +{\n> > > +       subdev_->close();\n> > > +}\n> > > +\n> > >  /**\n> > >   * \\brief This function sets the focal point of the lens to a specific position.\n> > >   * \\param[in] position The focal point of the lens\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 DA956BD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Oct 2024 09:20:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D20065391;\n\tWed, 23 Oct 2024 11:20:48 +0200 (CEST)","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n\t[IPv6:2a00:1450:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4643C6042C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Oct 2024 11:20:46 +0200 (CEST)","by mail-lj1-x22a.google.com with SMTP id\n\t38308e7fff4ca-2fc96f9c41fso7332621fa.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Oct 2024 02:20:46 -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=\"nRFb3Jsq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1729675245; x=1730280045;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=jDNbR5Ff3cA1wvMjPYTzD+i6dK7KknakYxUkLIXz+fQ=;\n\tb=nRFb3Jsqx4zt9GFbTCDLrm1MqFb3WFXwAG+Z7aAQngfd209foG8ZQ+6FaFKp+GNcTY\n\tlU19C0WpPr4vhaoOZWS6Mfy4DMxUjDuAo6qwxkvIikzRea18lj79DQLpAhXclt+VNQDN\n\tYz8+OSRG0hVUamUiChK/zwby829OW/tIUaTmE=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1729675245; x=1730280045;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=jDNbR5Ff3cA1wvMjPYTzD+i6dK7KknakYxUkLIXz+fQ=;\n\tb=dp979B5ZFBxodOhshZs8XpyktWYmU/Lten3gyMqAdVHUgSkNQZQvjZIeD/s79n5UDS\n\tSTr1Gau7kwpGcrZu178vVVZRy0zvuPdTmf7FDOmJRaUwxzeHxLU7ooTpzD/8dASx9bBb\n\tJCViG6aWMnS+yTqTiGW2oEKuwoNaqV4/clXlak3ihM1Mid+k+RExitLsr2V3jcvECgSr\n\tkzXqHoThuBSD4NOe1aeuqf87qGPx423SWIrpqZrwvfjE0eAUB6CWwCly8x+qmCzJyVvE\n\tcgIXnzdv4+wRCvF5S++CRhZSOU4d9dawHpez5XdgpJAYCwynHjkA4IzVWWm7F9I8+A1H\n\t7DQQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVf1XP7AUzBUu+RgK4dFjM7ShogS25wee60MPkfIRrjrdYgD4rnoiwlwkHzOtSAEss48/cMZuubqU3A1M2sVXM=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yy86VJOO7PYvC7pMDMkEvdSeZh5vXwKkiVTzHKNT+BRgf2Y9roC\n\tsdQEiPWjUO3T1+Nyssdr6RXvJ+Fg0wRJEpDTAz+arIaqipzUM1d9b4YhXmkbzlLAmQezLQ7IdDL\n\tb9OoVKKuFiXxe+qwlCYDaf+x5F7NH4OXJKXMe","X-Google-Smtp-Source":"AGHT+IGtuu/PuNTwuwreyBr6dWpben1sTbJl+IEurhn8Vzws1wyGXwKI2YAcd35h8IYbqiWc49fdw4m125EN2fVTcFk=","X-Received":"by 2002:a2e:709:0:b0:2f7:5ca2:6d10 with SMTP id\n\t38308e7fff4ca-2fc9d0b9970mr6223681fa.15.1729675245150;\n\tWed, 23 Oct 2024 02:20:45 -0700 (PDT)","MIME-Version":"1.0","References":"<20241017073948.353346-1-chenghaoyang@chromium.org>\n\t<172937712292.2485972.9882413556210770900@ping.linuxembedded.co.uk>\n\t<20241020160845.GE7770@pendragon.ideasonboard.com>","In-Reply-To":"<20241020160845.GE7770@pendragon.ideasonboard.com>","From":"Cheng-Hao Yang <chenghaoyang@chromium.org>","Date":"Wed, 23 Oct 2024 17:20:34 +0800","Message-ID":"<CAEB1ahvfdUmxyonfBoAZVuN9QUqBuT=jcxsBaZqkfCgaRuD4xw@mail.gmail.com>","Subject":"Re: [PATCH] libcamera: Add open() and close() in CameraLens","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, \n\tHan-Lin Chen <hanlinchen@chromium.org>,\n\tSakari Ailus <sakari.ailus@iki.fi>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31889,"web_url":"https://patchwork.libcamera.org/comment/31889/","msgid":"<62d21368-4ff2-4dae-a905-e35970f30faa@redhat.com>","date":"2024-10-23T10:25:34","subject":"Re: [PATCH] libcamera: Add open() and close() in CameraLens","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 23-Oct-24 11:20 AM, Cheng-Hao Yang wrote:\n> Hi Kieran and Laurent,\n> \n> On Mon, Oct 21, 2024 at 12:08 AM Laurent Pinchart\n> <laurent.pinchart@ideasonboard.com> wrote:\n>>\n>> CC'ing Sakari.\n>>\n>> On Sat, Oct 19, 2024 at 11:32:02PM +0100, Kieran Bingham wrote:\n>>> Quoting Harvey Yang (2024-10-17 08:39:47)\n>>>> From: Han-Lin Chen <hanlinchen@chromium.org>\n>>>>\n>>>> This allows pipeline handlers to save some power when a camera is close.\n>>>\n>>> This seems reasonable - but makes me wonder - is there similar\n>>> limitations with the CameraSensor class that has a very similar design?\n>>\n>> I think it's time to bite the bullet and see how to address the issue of\n>> power management of lens controllers in the kernel. Tying it to open()\n>> and close() isn't necessarily a great option.\n> \n> Han-lin told me that CameraLens is a special case of\n> V4L2Subdevice (at least in mtkisp7), that it's powered on when opened.\n> Other ones like CameraSensor's `subdev_` might be powered on when\n> the V4L2VideoDevice that receives buffers are `streamOn`ed. Just FYI.\n\nRight, but what Laurent is proposing is changing the kernel\nso that just opening the /dev/v4l20-subdev device for the VCM\nwill no longer power it up.\n\nI submitted a possible proposal for this here:\n\nhttps://lore.kernel.org/linux-media/20240901211834.145186-1-hdegoede@redhat.com/\n\nNote that Laurent was not a fan of the approach taken there, so\nthis needs more discussion and unfortunately I have not been able\nto make the time to reply to Laurent about this yet.\n\nI'll try to make time to reply to Laurent about this today.\n\nRegards,\n\nHans\n\n\n\n\n> \n>>\n>>>> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>\n>>>> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>\n>>>> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>\n>>>> ---\n>>>>  include/libcamera/internal/camera_lens.h |  4 ++++\n>>>>  src/libcamera/camera_lens.cpp            | 17 +++++++++++++++++\n>>>>  2 files changed, 21 insertions(+)\n>>>>\n>>>> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n>>>> index 5a4b993bb..095056791 100644\n>>>> --- a/include/libcamera/internal/camera_lens.h\n>>>> +++ b/include/libcamera/internal/camera_lens.h\n>>>> @@ -26,6 +26,10 @@ public:\n>>>>         ~CameraLens();\n>>>>\n>>>>         int init();\n>>>\n>>> Should we be calling close() at the end of init()? Perhaps with some\n>>> sort of way to make sure if an API call is used on a now closed device\n>>> it gets opened again first ? (i.e. mostly perhaps V4L2Device::setControls)?\n> \n> Hmm, in the current API, I don't see function calls that \"start\" to use\n> CameraLens.\n> \n> BR,\n> Harvey\n> \n>>>\n>>>> +\n>>>> +       int open();\n>>>> +       void close();\n>>>> +\n>>>>         int setFocusPosition(int32_t position);\n>>>>\n>>>>         const std::string &model() const { return model_; }\n>>>> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n>>>> index ccc2a6a65..039f5ad2a 100644\n>>>> --- a/src/libcamera/camera_lens.cpp\n>>>> +++ b/src/libcamera/camera_lens.cpp\n>>>> @@ -76,6 +76,23 @@ int CameraLens::init()\n>>>>         return 0;\n>>>>  }\n>>>>\n>>>> +/**\n>>>> + * \\brief Open the subdev\n>>>> + * \\return 0 on success or a negative error code otherwise\n>>>> + */\n>>>> +int CameraLens::open()\n>>>> +{\n>>>> +       return subdev_->open();\n>>>> +}\n>>>> +\n>>>> +/**\n>>>> + * \\brief Close the subdev\n>>>> + */\n>>>> +void CameraLens::close()\n>>>> +{\n>>>> +       subdev_->close();\n>>>> +}\n>>>> +\n>>>>  /**\n>>>>   * \\brief This function sets the focal point of the lens to a specific position.\n>>>>   * \\param[in] position The focal point of the lens\n>>\n>> --\n>> Regards,\n>>\n>> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 EE61CC0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Oct 2024 10:25:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E4A9965393;\n\tWed, 23 Oct 2024 12:25:41 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E99396537E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Oct 2024 12:25:39 +0200 (CEST)","from mail-ej1-f69.google.com (mail-ej1-f69.google.com\n\t[209.85.218.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-9-MXgZ7tHVOZWFn0EMITyBgA-1; Wed, 23 Oct 2024 06:25:37 -0400","by mail-ej1-f69.google.com with SMTP id\n\ta640c23a62f3a-a9a157d028aso451827966b.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Oct 2024 03:25:36 -0700 (PDT)","from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec?\n\t(2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:c32:7800:5bfa:a036:83f0:f9ec])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a9a912f7db1sm453440066b.89.2024.10.23.03.25.34\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 23 Oct 2024 03:25:34 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"FbyuZ3rH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1729679138;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=IViilYZV0SYzPXP2iUEjpLswp0ZDpG8jO9pwysG5Lgg=;\n\tb=FbyuZ3rHJiM2wQbbCmwqELAnUToxy7O2btLUMSxGwPOUo6BmB9edYbsk47fWQ9fDIiND2+\n\thAT0HlrYuYyYfxx0yRfVtwhyW2T3bvPQ5m7fXo45vMxS7S9ivlHcvmpgJggDpfLHzJw0sP\n\tfGW4CypJEfXlOw3tDd0PanVYmf0Gjso=","X-MC-Unique":"MXgZ7tHVOZWFn0EMITyBgA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1729679135; x=1730283935;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=IViilYZV0SYzPXP2iUEjpLswp0ZDpG8jO9pwysG5Lgg=;\n\tb=pMXkWMaNxDsvAPwtcyMW0L6SI5FdcLf/exjGxyIX3K3LFSPWrp0gaCfRjiYtjx3DP5\n\tDrkL/alRJHjQpDXlpDCM0AXDa2FD0xA0rh9U7b5WmXI6gLqmPcAXvciiwbFvtBbZ1KnF\n\txEVDxW2FUsZ7sERAEFbWOJmOM3udRZ4Kg2s5HG9HHBPTWAx0X0SBjpyBwTNM/gLdZN2a\n\tDQvBftMPZ0ocNRrckSmtuOJviGk5Kxy0hQJcloxPScGliQobd+YaCleVxfnoky0Y1uy5\n\tJc/FLzraE/bCy/ScWvaMNdQ+W0xIPclRpMmQZ53TLRHlCseWTq/aE8B5SmXJTdzPKUPF\n\tWf1Q==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUPkviwMC0goVlVPYP50UTqVIczcceUrdSkBMF50O+Bv3J/YcGURZ4m4MFANzIOLLeSl/LYgJk92S8Pp599UvE=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yx5lESSGdIpHpQjwotRuTqeoanoAQqNMjsWDvhN/A0i1849GfgR\n\tyaEywm4EG1DHwjmbegf3yY3YJdy3fleVoP/he+WLB0zw83iBEA00ca1urRtjzCUgZL0C945+ngn\n\tbBTA+CmsWq9Cd568MfLD6vWkBF/x3Vomraoy9RCOrHvvvq/98tkqz9omX2xxl4OkIrFgWN2M=","X-Received":["by 2002:a17:907:72d4:b0:a99:2ab0:d973 with SMTP id\n\ta640c23a62f3a-a9abf96ce13mr187041466b.55.1729679135452; \n\tWed, 23 Oct 2024 03:25:35 -0700 (PDT)","by 2002:a17:907:72d4:b0:a99:2ab0:d973 with SMTP id\n\ta640c23a62f3a-a9abf96ce13mr187039466b.55.1729679135085; \n\tWed, 23 Oct 2024 03:25:35 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGOcgDGtjOALXSCW0pPluuf2xEHlErCq6Y629pKWYi7OvxHJntknFik0s0u6LIyJ+1cFMVSZQ==","Message-ID":"<62d21368-4ff2-4dae-a905-e35970f30faa@redhat.com>","Date":"Wed, 23 Oct 2024 12:25:34 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] libcamera: Add open() and close() in CameraLens","To":"Cheng-Hao Yang <chenghaoyang@chromium.org>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tHan-Lin Chen <hanlinchen@chromium.org>, \n\tSakari Ailus <sakari.ailus@iki.fi>","References":"<20241017073948.353346-1-chenghaoyang@chromium.org>\n\t<172937712292.2485972.9882413556210770900@ping.linuxembedded.co.uk>\n\t<20241020160845.GE7770@pendragon.ideasonboard.com>\n\t<CAEB1ahvfdUmxyonfBoAZVuN9QUqBuT=jcxsBaZqkfCgaRuD4xw@mail.gmail.com>","From":"Hans de Goede <hdegoede@redhat.com>","In-Reply-To":"<CAEB1ahvfdUmxyonfBoAZVuN9QUqBuT=jcxsBaZqkfCgaRuD4xw@mail.gmail.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US, nl","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]