[{"id":28576,"web_url":"https://patchwork.libcamera.org/comment/28576/","msgid":"<20240123020319.GH22880@pendragon.ideasonboard.com>","date":"2024-01-23T02:03:19","subject":"Re: [PATCH v2] libcamera: camera_sensor: Cache pixel rate","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Mon, Jan 22, 2024 at 09:13:58PM +0900, Paul Elder wrote:\n> Cache the pixel rate at set format time instead of fetching it from the\n> v4l2 subdev every time it's needed.\n\nCould you please explain in the commit message why this is needed ? Is\nit \"just\" a small optimization, or does it fix an issue ?\n\n> It needs to be cached at set format time as opposed to initialization\n> time as some sensor drives (eg. imx708) change the pixel rate depending\n\ns/drives/drivers/\n\n> on the mode.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> ---\n> Changes in v2:\n> - Cache the pixel rate at set format time instead of at initialization\n>   time\n> ---\n>  include/libcamera/internal/camera_sensor.h | 2 ++\n>  src/libcamera/camera_sensor.cpp            | 8 +++++++-\n>  2 files changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 60a8b106d..da3bf12b3 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -105,6 +105,8 @@ private:\n>  \tstd::string model_;\n>  \tstd::string id_;\n>  \n> +\tuint64_t pixelRate_;\n> +\n>  \tV4L2Subdevice::Formats formats_;\n>  \tstd::vector<unsigned int> mbusCodes_;\n>  \tstd::vector<Size> sizes_;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 0ef78d9c8..127610321 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -515,6 +515,9 @@ int CameraSensor::initProperties()\n>  \t\tproperties_.set(properties::draft::ColorFilterArrangement, cfa);\n>  \t}\n>  \n> +\tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });\n> +\tpixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> +\n>  \treturn 0;\n>  }\n>  \n> @@ -814,6 +817,9 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format, Transform transform)\n>  \tif (ret)\n>  \t\treturn ret;\n>  \n> +\tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });\n> +\tpixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> +\n>  \tupdateControlInfo();\n>  \treturn 0;\n>  }\n> @@ -1080,7 +1086,7 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n>  \t\treturn -EINVAL;\n>  \t}\n>  \n> -\tinfo->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n\nThis avoids fetching the pixel rate from the ControlList, but it's still\nfetched from the driver when populating the control list. I'm thus\ncurious to know the reason for this patch.\n\n> +\tinfo->pixelRate = pixelRate_;\n>  \n>  \tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n>  \tinfo->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();","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 EFC00C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Jan 2024 02:03:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 185FF6291F;\n\tTue, 23 Jan 2024 03:03:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5FD1461D46\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Jan 2024 03:03:16 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AB2211593;\n\tTue, 23 Jan 2024 03:02:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RMDm80sY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705975322;\n\tbh=AZxKaQCjsTTeTCjPZdbfotgyEnAPFJuT++TuWyof+tk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RMDm80sY4kL//hAVc2HKZK659p1P+Xa1iMmL9lPAzvqSfAIaL2QobxVgBvf0DGXlV\n\tHCkf22mZHwZ/lrsnL55O7vJ+yRvQW4IBAsSPt+3abCeJn+SfEKpgB5Nv28wkh5NEon\n\tQVfFK1FCdtenGc9BPT/U1h6ajRfGCW6GhMWaOduU=","Date":"Tue, 23 Jan 2024 04:03:19 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v2] libcamera: camera_sensor: Cache pixel rate","Message-ID":"<20240123020319.GH22880@pendragon.ideasonboard.com>","References":"<20240122121358.3426521-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240122121358.3426521-1-paul.elder@ideasonboard.com>","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28577,"web_url":"https://patchwork.libcamera.org/comment/28577/","msgid":"<b82a85d6-624e-d477-96cb-7486f5ac6c66@ideasonboard.com>","date":"2024-01-23T05:51:55","subject":"Re: [PATCH v2] libcamera: camera_sensor: Cache pixel rate","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul\n\nOn 1/22/24 5:43 PM, Paul Elder wrote:\n> Cache the pixel rate at set format time instead of fetching it from the\n> v4l2 subdev every time it's needed.\n>\n> It needs to be cached at set format time as opposed to initialization\n> time as some sensor drives (eg. imx708) change the pixel rate depending\n> on the mode.\n>\n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n>\n> ---\n> Changes in v2:\n> - Cache the pixel rate at set format time instead of at initialization\n>    time\n> ---\n>   include/libcamera/internal/camera_sensor.h | 2 ++\n>   src/libcamera/camera_sensor.cpp            | 8 +++++++-\n>   2 files changed, 9 insertions(+), 1 deletion(-)\n>\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 60a8b106d..da3bf12b3 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -105,6 +105,8 @@ private:\n>   \tstd::string model_;\n>   \tstd::string id_;\n>   \n> +\tuint64_t pixelRate_;\n> +\n>   \tV4L2Subdevice::Formats formats_;\n>   \tstd::vector<unsigned int> mbusCodes_;\n>   \tstd::vector<Size> sizes_;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 0ef78d9c8..127610321 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -515,6 +515,9 @@ int CameraSensor::initProperties()\n>   \t\tproperties_.set(properties::draft::ColorFilterArrangement, cfa);\n>   \t}\n>   \n> +\tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });\n> +\tpixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> +\n\nDo we have to cache it at two places? One initProperties() and other in \nsetFormat() ?\n\nIn the change log, you have used ' instead of '  so I am a bit confused..\n>   \treturn 0;\n>   }\n>   \n> @@ -814,6 +817,9 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format, Transform transform)\n>   \tif (ret)\n>   \t\treturn ret;\n>   \n> +\tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });\n> +\tpixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> +\n>   \tupdateControlInfo();\n>   \treturn 0;\n>   }\n> @@ -1080,7 +1086,7 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n>   \t\treturn -EINVAL;\n>   \t}\n>   \n> -\tinfo->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> +\tinfo->pixelRate = pixelRate_;\n>   \n>   \tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n>   \tinfo->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();","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 57DC5C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Jan 2024 05:52:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7BDBC6291F;\n\tTue, 23 Jan 2024 06:52:02 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E3FA061D28\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Jan 2024 06:52:00 +0100 (CET)","from [192.168.1.106] (unknown [103.86.18.249])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B5829541;\n\tTue, 23 Jan 2024 06:50:46 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kayZnSij\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705989047;\n\tbh=1xEx5bRGYmILbMhAvy+yHzizk9sMg34CInQxDvjGKTk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=kayZnSijKmw+eO3aXOo+EwH/4neury87V+e8MgU1lLXh6Fm75SxvwP85bX0X+HxFr\n\tBnRPhcmNBKgukOrJRzaODgdrtXNC0/bx6+s3zWEywkNjG8seLJVynnhuMUfGyAIEcx\n\t1FAtc9ghrpqt5xPhfZHhAz1gn2szEDyVqM/Y1WPw=","Message-ID":"<b82a85d6-624e-d477-96cb-7486f5ac6c66@ideasonboard.com>","Date":"Tue, 23 Jan 2024 11:21:55 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.10.0","Subject":"Re: [PATCH v2] libcamera: camera_sensor: Cache pixel rate","Content-Language":"en-US","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240122121358.3426521-1-paul.elder@ideasonboard.com>","From":"Umang Jain <umang.jain@ideasonboard.com>","In-Reply-To":"<20240122121358.3426521-1-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","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>"}},{"id":28578,"web_url":"https://patchwork.libcamera.org/comment/28578/","msgid":"<20240123081822.cofdk5gekxsijwwg@pyrite.rasen.tech>","date":"2024-01-23T08:18:22","subject":"Re: [PATCH v2] libcamera: camera_sensor: Cache pixel rate","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Jan 23, 2024 at 04:03:19AM +0200, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Mon, Jan 22, 2024 at 09:13:58PM +0900, Paul Elder wrote:\n> > Cache the pixel rate at set format time instead of fetching it from the\n> > v4l2 subdev every time it's needed.\n> \n> Could you please explain in the commit message why this is needed ? Is\n> it \"just\" a small optimization, or does it fix an issue ?\n\nI want it for adding libcamera controls to CameraSensor because it's\nneeded for both getting and setting the hblank and vblank controls and I\nwant to avoid subdev_->getControls() every time we do that.\n\n> > It needs to be cached at set format time as opposed to initialization\n> > time as some sensor drives (eg. imx708) change the pixel rate depending\n> \n> s/drives/drivers/\n> \n> > on the mode.\n> > \n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > \n> > ---\n> > Changes in v2:\n> > - Cache the pixel rate at set format time instead of at initialization\n> >   time\n> > ---\n> >  include/libcamera/internal/camera_sensor.h | 2 ++\n> >  src/libcamera/camera_sensor.cpp            | 8 +++++++-\n> >  2 files changed, 9 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index 60a8b106d..da3bf12b3 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -105,6 +105,8 @@ private:\n> >  \tstd::string model_;\n> >  \tstd::string id_;\n> >  \n> > +\tuint64_t pixelRate_;\n> > +\n> >  \tV4L2Subdevice::Formats formats_;\n> >  \tstd::vector<unsigned int> mbusCodes_;\n> >  \tstd::vector<Size> sizes_;\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 0ef78d9c8..127610321 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -515,6 +515,9 @@ int CameraSensor::initProperties()\n> >  \t\tproperties_.set(properties::draft::ColorFilterArrangement, cfa);\n> >  \t}\n> >  \n> > +\tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });\n> > +\tpixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> > +\n> >  \treturn 0;\n> >  }\n> >  \n> > @@ -814,6 +817,9 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format, Transform transform)\n> >  \tif (ret)\n> >  \t\treturn ret;\n> >  \n> > +\tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });\n> > +\tpixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> > +\n> >  \tupdateControlInfo();\n> >  \treturn 0;\n> >  }\n> > @@ -1080,7 +1086,7 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n> >  \t\treturn -EINVAL;\n> >  \t}\n> >  \n> > -\tinfo->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> \n> This avoids fetching the pixel rate from the ControlList, but it's still\n> fetched from the driver when populating the control list. I'm thus\n> curious to know the reason for this patch.\n\nRight, I should remove fetching the pixel rate.\n\n\nThanks,\n\nPaul\n\n> \n> > +\tinfo->pixelRate = pixelRate_;\n> >  \n> >  \tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> >  \tinfo->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();","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 C2B16BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Jan 2024 08:18:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF86A61D3A;\n\tTue, 23 Jan 2024 09:18:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 463D161D3A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Jan 2024 09:18:30 +0100 (CET)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 99E1D6E0;\n\tTue, 23 Jan 2024 09:17:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"P3+JGQ6U\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705997836;\n\tbh=qGG4zqMqbjph3RT8eL+Oxk9J5cO75MmO9y/j5rBJ+zY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=P3+JGQ6UscHRa4X+eC8n55v2n7tFnemvQfenJkzF+pKYm1AsPjpQCnkT2/oaKwJUQ\n\tSmfdmKoqC+41DClm5d02p1PCDa2Ap2yzQScAcU6JDcLeSOq/elIdcifHzW8dw63W9G\n\tu6Xae2wObNGBWhhcEwRrDswmHXci7WpuMsoge67A=","Date":"Tue, 23 Jan 2024 17:18:22 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [PATCH v2] libcamera: camera_sensor: Cache pixel rate","Message-ID":"<20240123081822.cofdk5gekxsijwwg@pyrite.rasen.tech>","References":"<20240122121358.3426521-1-paul.elder@ideasonboard.com>\n\t<20240123020319.GH22880@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240123020319.GH22880@pendragon.ideasonboard.com>","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28579,"web_url":"https://patchwork.libcamera.org/comment/28579/","msgid":"<20240123082718.p46mgbbn4n6f7fwg@pyrite.rasen.tech>","date":"2024-01-23T08:27:18","subject":"Re: [PATCH v2] libcamera: camera_sensor: Cache pixel rate","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Jan 23, 2024 at 11:21:55AM +0530, Umang Jain wrote:\n> Hi Paul\n> \n> On 1/22/24 5:43 PM, Paul Elder wrote:\n> > Cache the pixel rate at set format time instead of fetching it from the\n> > v4l2 subdev every time it's needed.\n> > \n> > It needs to be cached at set format time as opposed to initialization\n> > time as some sensor drives (eg. imx708) change the pixel rate depending\n> > on the mode.\n> > \n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > \n> > ---\n> > Changes in v2:\n> > - Cache the pixel rate at set format time instead of at initialization\n> >    time\n> > ---\n> >   include/libcamera/internal/camera_sensor.h | 2 ++\n> >   src/libcamera/camera_sensor.cpp            | 8 +++++++-\n> >   2 files changed, 9 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index 60a8b106d..da3bf12b3 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -105,6 +105,8 @@ private:\n> >   \tstd::string model_;\n> >   \tstd::string id_;\n> > +\tuint64_t pixelRate_;\n> > +\n> >   \tV4L2Subdevice::Formats formats_;\n> >   \tstd::vector<unsigned int> mbusCodes_;\n> >   \tstd::vector<Size> sizes_;\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 0ef78d9c8..127610321 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -515,6 +515,9 @@ int CameraSensor::initProperties()\n> >   \t\tproperties_.set(properties::draft::ColorFilterArrangement, cfa);\n> >   \t}\n> > +\tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });\n> > +\tpixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> > +\n> \n> Do we have to cache it at two places? One initProperties() and other in\n> setFormat() ?\n> \n> In the change log, you have used ' instead of '  so I am a bit confused..\n\nOops. Yeah we need to cache it in both places.\n\n\nPaul\n\n> >   \treturn 0;\n> >   }\n> > @@ -814,6 +817,9 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format, Transform transform)\n> >   \tif (ret)\n> >   \t\treturn ret;\n> > +\tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE });\n> > +\tpixelRate_ = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> > +\n> >   \tupdateControlInfo();\n> >   \treturn 0;\n> >   }\n> > @@ -1080,7 +1086,7 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n> >   \t\treturn -EINVAL;\n> >   \t}\n> > -\tinfo->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>();\n> > +\tinfo->pixelRate = pixelRate_;\n> >   \tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> >   \tinfo->minLineLength = info->outputSize.width + hblank.min().get<int32_t>();\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 DC27DC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Jan 2024 08:27:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2626C62945;\n\tTue, 23 Jan 2024 09:27:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90A4C61D3A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Jan 2024 09:27:26 +0100 (CET)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D5649FA2;\n\tTue, 23 Jan 2024 09:26:11 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"EFh73wDd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705998373;\n\tbh=JyNLy8Ys9E5OxaEoaSmEAQ33Jdeb6hfA7/z1aqfDgBI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EFh73wDd2KfzsJ2+3A+qgy0vcykEeu69ON7tcJoS9Tk6pYUvPLrra7Tqs2zQYWGOM\n\tJgF8lzJZfhApoOO0c8nccQ3phkDkbo+/0zK/OgR6Xe6PT4lkl9OXxYxSbnS2ZdH5VY\n\tj/0KG8e44n+tzjIPHbaoVtZAg9+JmK4G7A9BZ/38=","Date":"Tue, 23 Jan 2024 17:27:18 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Subject":"Re: [PATCH v2] libcamera: camera_sensor: Cache pixel rate","Message-ID":"<20240123082718.p46mgbbn4n6f7fwg@pyrite.rasen.tech>","References":"<20240122121358.3426521-1-paul.elder@ideasonboard.com>\n\t<b82a85d6-624e-d477-96cb-7486f5ac6c66@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<b82a85d6-624e-d477-96cb-7486f5ac6c66@ideasonboard.com>","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]