[{"id":12175,"web_url":"https://patchwork.libcamera.org/comment/12175/","msgid":"<36c2258d-eded-2b41-b711-4cbb88f759e8@ideasonboard.com>","date":"2020-08-27T11:52:27","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 27/08/2020 09:20, Jacopo Mondi wrote:\n> Provide an RPiCameraData::init() method where the dmaHeaps_ member\n> is opened.\n> \n> This allows to fail earlier in case the allocator fails to open.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 42c9caa03e2e..38da45607d4b 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -291,6 +291,7 @@ public:\n> \t{\n> \t}\n> \n> +\tint init();\n> \tvoid frameStarted(uint32_t sequence);\n> \n> \tint loadIPA();\n> @@ -904,6 +905,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \n> \tstd::unique_ptr data = std::make_unique(this);\n> +\tif (data->init())\n> +\t\treturn false;\n> \n> \t/* Locate and open the unicam video streams. */\n> \tdata->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> @@ -1084,6 +1087,11 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera)\n> \t\tstream->releaseBuffers();\n> }\n> \n> +int RPiCameraData::init()\n> +{\n> +\treturn dmaHeap_.open();\n> +}\n> +\n\nShould we explicitly close in RPiCameraData destructor? Perhaps not in\nfact, as the dmaHeap destructor already does that.\n\nReviewed-by: Kieran Bingham \n\n> void RPiCameraData::frameStarted(uint32_t sequence)\n> {\n> \tLOG(RPI, Debug) << \"frame start \" << sequence;\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 7F554BDC71\n\tfor ;\n\tThu, 27 Aug 2020 11:52:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 179E662901;\n\tThu, 27 Aug 2020 13:52:34 +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 09F44628D4\n\tfor ;\n\tThu, 27 Aug 2020 13:52:33 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 489F91870;\n\tThu, 27 Aug 2020 13:52:30 +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=\"ACqBeg/y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598529150;\n\tbh=IHA97XWQANaP/FiPkG3CUF7YFhToMixOoFsxsCP5eSA=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=ACqBeg/yUG3c0bnVSEAEr1enbuWeD43jQnurFamXPpjveBSr2xvwv7Z9Q0V3vcznd\n\t1fjlg8dMwtIHJnu6GLDo2fkTg7x9kXWRzHsxBc/6OBg1ooL/FVceCS4YG73VdcGY1H\n\tBQ6AEots5iTp2HQN10ZsNk+dI1VmSQkzNeiXwrFI=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org,\n\tDave Stevenson ,\n\tNaushir Patuck ","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-4-jacopo@jmondi.org>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<36c2258d-eded-2b41-b711-4cbb88f759e8@ideasonboard.com>","Date":"Thu, 27 Aug 2020 12:52:27 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200827082038.40758-4-jacopo@jmondi.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12178,"web_url":"https://patchwork.libcamera.org/comment/12178/","msgid":"<20200827120039.vxgoewqdbniaoiip@uno.localdomain>","date":"2020-08-27T12:00:39","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Thu, Aug 27, 2020 at 12:52:27PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 27/08/2020 09:20, Jacopo Mondi wrote:\n> > Provide an RPiCameraData::init() method where the dmaHeaps_ member\n> > is opened.\n> >\n> > This allows to fail earlier in case the allocator fails to open.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 8 ++++++++\n> > 1 file changed, 8 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 42c9caa03e2e..38da45607d4b 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -291,6 +291,7 @@ public:\n> > \t{\n> > \t}\n> >\n> > +\tint init();\n> > \tvoid frameStarted(uint32_t sequence);\n> >\n> > \tint loadIPA();\n> > @@ -904,6 +905,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> >\n> > \tstd::unique_ptr data = std::make_unique(this);\n> > +\tif (data->init())\n> > +\t\treturn false;\n> >\n> > \t/* Locate and open the unicam video streams. */\n> > \tdata->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> > @@ -1084,6 +1087,11 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera)\n> > \t\tstream->releaseBuffers();\n> > }\n> >\n> > +int RPiCameraData::init()\n> > +{\n> > +\treturn dmaHeap_.open();\n> > +}\n> > +\n>\n> Should we explicitly close in RPiCameraData destructor? Perhaps not in\n> fact, as the dmaHeap destructor already does that.\n>\n\nI had added a ~RPiCameraData destructor for that, but since the\nDmaHeaps class destructor does the same, I then removed it.\n\n> Reviewed-by: Kieran Bingham \n>\n\nThanks\n j\n\n> > void RPiCameraData::frameStarted(uint32_t sequence)\n> > {\n> > \tLOG(RPI, Debug) << \"frame start \" << sequence;\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 C6412BF019\n\tfor ;\n\tThu, 27 Aug 2020 11:56:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9EE1E62901;\n\tThu, 27 Aug 2020 13:56:55 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C2402628D4\n\tfor ;\n\tThu, 27 Aug 2020 13:56:54 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id D3A43100002;\n\tThu, 27 Aug 2020 11:56:53 +0000 (UTC)"],"Date":"Thu, 27 Aug 2020 14:00:39 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20200827120039.vxgoewqdbniaoiip@uno.localdomain>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-4-jacopo@jmondi.org>\n\t<36c2258d-eded-2b41-b711-4cbb88f759e8@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<36c2258d-eded-2b41-b711-4cbb88f759e8@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12186,"web_url":"https://patchwork.libcamera.org/comment/12186/","msgid":"<20200827142531.GJ6042@pendragon.ideasonboard.com>","date":"2020-08-27T14:25:31","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Aug 27, 2020 at 10:20:38AM +0200, Jacopo Mondi wrote:\n> Provide an RPiCameraData::init() method where the dmaHeaps_ member\n> is opened.\n> \n> This allows to fail earlier in case the allocator fails to open.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 42c9caa03e2e..38da45607d4b 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -291,6 +291,7 @@ public:\n> \t{\n> \t}\n> \n> +\tint init();\n> \tvoid frameStarted(uint32_t sequence);\n> \n> \tint loadIPA();\n> @@ -904,6 +905,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \n> \tstd::unique_ptr data = std::make_unique(this);\n> +\tif (data->init())\n> +\t\treturn false;\n> \n> \t/* Locate and open the unicam video streams. */\n> \tdata->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> @@ -1084,6 +1087,11 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera)\n> \t\tstream->releaseBuffers();\n> }\n> \n> +int RPiCameraData::init()\n> +{\n> +\treturn dmaHeap_.open();\n\nThis looks goot do me. With the isValid() (or similar) comment from 1/3\naddressed,\n\nReviewed-by: Laurent Pinchart \n\n> +}\n> +\n> void RPiCameraData::frameStarted(uint32_t sequence)\n> {\n> \tLOG(RPI, Debug) << \"frame start \" << sequence;","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 9D72EBF019\n\tfor ;\n\tThu, 27 Aug 2020 14:25:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3304E62901;\n\tThu, 27 Aug 2020 16:25:55 +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 3097B628D4\n\tfor ;\n\tThu, 27 Aug 2020 16:25:53 +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 2F2D11872;\n\tThu, 27 Aug 2020 16:25:52 +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=\"oTanxTKh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598538352;\n\tbh=CSje4mJNKum3Nwm8wDz5Neae0hEYp+yGMMKDglp1y6s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oTanxTKh6jJlhOrrqqM/MkF6lPl4a2SSeic0xEYnls9e94RA7wyqweGGiCToqt4Qv\n\trz2A1XqiIT0mHOQhgVZBYt4piAdPMaq9matMxRMOn/igqRI+GjlM4UoWm7U1EnSbYq\n\t+4zt0q7ttNZogsuFnYEbIy4F7N6+Hi6kbZdy8KQs=","Date":"Thu, 27 Aug 2020 17:25:31 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200827142531.GJ6042@pendragon.ideasonboard.com>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200827082038.40758-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12192,"web_url":"https://patchwork.libcamera.org/comment/12192/","msgid":"<20200828110412.etq7audcazqgn55d@uno.localdomain>","date":"2020-08-28T11:04:12","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Aug 27, 2020 at 05:25:31PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Thu, Aug 27, 2020 at 10:20:38AM +0200, Jacopo Mondi wrote:\n> > Provide an RPiCameraData::init() method where the dmaHeaps_ member\n> > is opened.\n> >\n> > This allows to fail earlier in case the allocator fails to open.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 8 ++++++++\n> > 1 file changed, 8 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 42c9caa03e2e..38da45607d4b 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -291,6 +291,7 @@ public:\n> > \t{\n> > \t}\n> >\n> > +\tint init();\n> > \tvoid frameStarted(uint32_t sequence);\n> >\n> > \tint loadIPA();\n> > @@ -904,6 +905,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> >\n> > \tstd::unique_ptr data = std::make_unique(this);\n> > +\tif (data->init())\n> > +\t\treturn false;\n> >\n> > \t/* Locate and open the unicam video streams. */\n> > \tdata->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> > @@ -1084,6 +1087,11 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera)\n> > \t\tstream->releaseBuffers();\n> > }\n> >\n> > +int RPiCameraData::init()\n> > +{\n> > +\treturn dmaHeap_.open();\n>\n> This looks goot do me. With the isValid() (or similar) comment from 1/3\n> addressed,\n>\n\nConsidering this might move to become a standard component what API do\nyou think it's best ?\n1) One that forces you to open the allocator, and make sure it\nsucceeds\n2) One that allows you to check if the allocator is valid but silently\nfail at construction time ?\n\nI would pick 1, of course I should guard against double opens if\nthat's meant to become a standard component, but opening in\nconstructor + optional isValid() is not the best choice here in my\nopinion.\n\n> Reviewed-by: Laurent Pinchart \n>\n> > +}\n> > +\n> > void RPiCameraData::frameStarted(uint32_t sequence)\n> > {\n> > \tLOG(RPI, Debug) << \"frame start \" << sequence;\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 9A415BDC71\n\tfor ;\n\tFri, 28 Aug 2020 11:00:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 308D261EA0;\n\tFri, 28 Aug 2020 13:00:28 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5AE826037B\n\tfor ;\n\tFri, 28 Aug 2020 13:00:27 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 6EE6D10000C;\n\tFri, 28 Aug 2020 11:00:26 +0000 (UTC)"],"Date":"Fri, 28 Aug 2020 13:04:12 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200828110412.etq7audcazqgn55d@uno.localdomain>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-4-jacopo@jmondi.org>\n\t<20200827142531.GJ6042@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200827142531.GJ6042@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12200,"web_url":"https://patchwork.libcamera.org/comment/12200/","msgid":"<20200828152104.GC10034@pendragon.ideasonboard.com>","date":"2020-08-28T15:21:04","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, Aug 28, 2020 at 01:04:12PM +0200, Jacopo Mondi wrote:\n> On Thu, Aug 27, 2020 at 05:25:31PM +0300, Laurent Pinchart wrote:\n> > On Thu, Aug 27, 2020 at 10:20:38AM +0200, Jacopo Mondi wrote:\n> > > Provide an RPiCameraData::init() method where the dmaHeaps_ member\n> > > is opened.\n> > >\n> > > This allows to fail earlier in case the allocator fails to open.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 8 ++++++++\n> > > 1 file changed, 8 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 42c9caa03e2e..38da45607d4b 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -291,6 +291,7 @@ public:\n> > > \t{\n> > > \t}\n> > >\n> > > +\tint init();\n> > > \tvoid frameStarted(uint32_t sequence);\n> > >\n> > > \tint loadIPA();\n> > > @@ -904,6 +905,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> > > \t\treturn false;\n> > >\n> > > \tstd::unique_ptr data = std::make_unique(this);\n> > > +\tif (data->init())\n> > > +\t\treturn false;\n> > >\n> > > \t/* Locate and open the unicam video streams. */\n> > > \tdata->unicam_[Unicam::Embedded] = RPiStream(\"Unicam Embedded\", unicam_->getEntityByName(\"unicam-embedded\"));\n> > > @@ -1084,6 +1087,11 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera)\n> > > \t\tstream->releaseBuffers();\n> > > }\n> > >\n> > > +int RPiCameraData::init()\n> > > +{\n> > > +\treturn dmaHeap_.open();\n> >\n> > This looks goot do me. With the isValid() (or similar) comment from 1/3\n> > addressed,\n> \n> Considering this might move to become a standard component what API do\n> you think it's best ?\n> 1) One that forces you to open the allocator, and make sure it\n> succeeds\n> 2) One that allows you to check if the allocator is valid but silently\n> fail at construction time ?\n> \n> I would pick 1, of course I should guard against double opens if\n> that's meant to become a standard component, but opening in\n> constructor + optional isValid() is not the best choice here in my\n> opinion.\n\nAs we'll have to refactor it anyway, I'd go for 2 as that's the least\nintrusive change, doesn't require thinking about how to implement the\nopen/close semantics, or guarding against multiple opens. It's up to\nyou, but I don't think now is the time to already think about how it\nwill be refactored, we don't know yet.\n\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > +}\n> > > +\n> > > void RPiCameraData::frameStarted(uint32_t sequence)\n> > > {\n> > > \tLOG(RPI, Debug) << \"frame start \" << sequence;","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 1279CBF019\n\tfor ;\n\tFri, 28 Aug 2020 15:21:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A24C7628F3;\n\tFri, 28 Aug 2020 17:21:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 78FC76037B\n\tfor ;\n\tFri, 28 Aug 2020 17:21:27 +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 7C311303;\n\tFri, 28 Aug 2020 17:21:24 +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=\"mnFa7kp8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598628084;\n\tbh=3Ff84l740t/3+nw44joDWiJWq5HLfxzsIwU0+vMeKcQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mnFa7kp8xCdaU25C8urKvxEax11u50SoU4MaKe+3gocq+w6mN2bG48zJNpwfAouIG\n\tHl4bKc0xq6vQP0lUNcPzdPA7CX26gabdUaJqm0ojiAi2e5H9XeYLACbkTvkpl+whP6\n\t7adxAy+CgVcZg9UTcZOTRukCcN3X0xKMV35wq2Tc=","Date":"Fri, 28 Aug 2020 18:21:04 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200828152104.GC10034@pendragon.ideasonboard.com>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-4-jacopo@jmondi.org>\n\t<20200827142531.GJ6042@pendragon.ideasonboard.com>\n\t<20200828110412.etq7audcazqgn55d@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200828110412.etq7audcazqgn55d@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: raspberrypi: Fail on\n\tdmaHeaps_ open error","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]