[{"id":12171,"web_url":"https://patchwork.libcamera.org/comment/12171/","msgid":"<50d2139e-def0-23ee-261a-464a0e7fa986@ideasonboard.com>","date":"2020-08-27T11:21:24","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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> Be a tad more verbose on failures in opening the dma_heaps allocator\n> devices.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/pipeline/raspberrypi/dma_heaps.cpp | 7 ++++++-\n>  1 file changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> index 739f05d3d4d8..500f1eac2eb8 100644\n> --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> @@ -45,9 +45,14 @@ int DmaHeap::open()\n>  {\n>  \tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_NAME, O_RDWR, 0);\n>  \tif (dmaHeapHandle_ == -1) {\n> +\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n> +\t\t\t\t<< DMA_HEAP_CMA_NAME << \": \"\n> +\t\t\t\t<< strerror(errno);\n\nThis will report an error, even if the ALT_NAME is successful...\n\n\n>  \t\tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_ALT_NAME, O_RDWR, 0);\n>  \t\tif (dmaHeapHandle_ == -1) {\n> -\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device\";\n> +\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n> +\t\t\t\t\t<< DMA_HEAP_CMA_ALT_NAME << \": \"\n> +\t\t\t\t\t<< strerror(errno);\n>  \t\t\treturn dmaHeapHandle_;\n>  \t\t}\n>  \t}\n> \n\nI wonder if this is a good opportunity to refactor this code:\n\n(untested/uncompiled psuedo code to follow)\n\n\nstd::array<const char *, 2> heap_names {\n\t\"/dev/dma_heap/linux,cma\",\n\t\"/dev/dma_heap/reserved\",\n}\n\nint DmaHeap::open()\n{\n\tfor (const char *heap : heap_names) {\n\t\tif (!libcamera::File::exists(heap))\n\t\t\tcontinue;\n\n\t\tdmaHeapHandle_ = ::open(heap, O_RDWR, 0);\n\n\t\tif (dmaHeapHandle_ != -1) /* Success! */\n\t\t\treturn 0;\n\n\t\tLOG(RPI, Warning)\n\t\t\t<< \"Could not open dmaHeap \"\n\t\t\t<< heap << \": \"\t<< strerror(errno);\n\t}\n\n\t/* No DMA Heap was available */\n\tLOG(RPI, Error) << \"Could not open any dmaHeap device\";\n\n\treturn -ENODEV;\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 95B85BF019\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 27 Aug 2020 11:21:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C48162901;\n\tThu, 27 Aug 2020 13:21:29 +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 208A9628D4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 Aug 2020 13:21:28 +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 104459B1;\n\tThu, 27 Aug 2020 13:21:27 +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=\"vPnw5K7n\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598527287;\n\tbh=nzietQaWPKcmBMYvOUQrKh46olL09gh7tbHiCsEwICQ=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=vPnw5K7n3ClsvWpTE6g3EtWbVTvvfwdoPgeSamSCOhFaG7ry4AtU1IgVIDOlTVO6A\n\tnzFZm9DYgN5ronii55ujLoRuRJJ8wYwrPnjtry0FgJbUXklwzpVU7B4e7MWIwgxT3/\n\ttlD7ShXNG/Bd9tvh1JTbhDDRVkb0BzCllAha1ga8=","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org,\n\tDave Stevenson <dave.stevenson@raspberrypi.com>,\n\tNaushir Patuck <naush@raspberrypi.com>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-3-jacopo@jmondi.org>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<50d2139e-def0-23ee-261a-464a0e7fa986@ideasonboard.com>","Date":"Thu, 27 Aug 2020 12:21:24 +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-3-jacopo@jmondi.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12172,"web_url":"https://patchwork.libcamera.org/comment/12172/","msgid":"<20200827114604.kez3nz2n3h6gp6lu@uno.localdomain>","date":"2020-08-27T11:46:04","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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:21:24PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 27/08/2020 09:20, Jacopo Mondi wrote:\n> > Be a tad more verbose on failures in opening the dma_heaps allocator\n> > devices.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/libcamera/pipeline/raspberrypi/dma_heaps.cpp | 7 ++++++-\n> >  1 file changed, 6 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> > index 739f05d3d4d8..500f1eac2eb8 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> > @@ -45,9 +45,14 @@ int DmaHeap::open()\n> >  {\n> >  \tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_NAME, O_RDWR, 0);\n> >  \tif (dmaHeapHandle_ == -1) {\n> > +\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n> > +\t\t\t\t<< DMA_HEAP_CMA_NAME << \": \"\n> > +\t\t\t\t<< strerror(errno);\n>\n> This will report an error, even if the ALT_NAME is successful...\n>\n\nTrue, but I think that's ok as the DMA_HEAP_CMA_ALT_NAME device is\njust a fallback and there shouldn't be a need to poke it\n\n>\n> >  \t\tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_ALT_NAME, O_RDWR, 0);\n> >  \t\tif (dmaHeapHandle_ == -1) {\n> > -\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device\";\n> > +\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n> > +\t\t\t\t\t<< DMA_HEAP_CMA_ALT_NAME << \": \"\n> > +\t\t\t\t\t<< strerror(errno);\n> >  \t\t\treturn dmaHeapHandle_;\n> >  \t\t}\n> >  \t}\n> >\n>\n> I wonder if this is a good opportunity to refactor this code:\n>\n> (untested/uncompiled psuedo code to follow)\n>\n>\n> std::array<const char *, 2> heap_names {\n> \t\"/dev/dma_heap/linux,cma\",\n> \t\"/dev/dma_heap/reserved\",\n> }\n>\n> int DmaHeap::open()\n> {\n> \tfor (const char *heap : heap_names) {\n> \t\tif (!libcamera::File::exists(heap))\n> \t\t\tcontinue;\n>\n> \t\tdmaHeapHandle_ = ::open(heap, O_RDWR, 0);\n>\n> \t\tif (dmaHeapHandle_ != -1) /* Success! */\n> \t\t\treturn 0;\n>\n> \t\tLOG(RPI, Warning)\n> \t\t\t<< \"Could not open dmaHeap \"\n> \t\t\t<< heap << \": \"\t<< strerror(errno);\n> \t}\n>\n> \t/* No DMA Heap was available */\n> \tLOG(RPI, Error) << \"Could not open any dmaHeap device\";\n>\n> \treturn -ENODEV;\n\n\nWe could work on something similar indeed if erroring on the first\nfailure is not welcome.\n\nThanks\n  j\n\n> }\n>\n> --\n> Regards\n> --\n> Kieran","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 7E55DBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 27 Aug 2020 11:42:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2305B62901;\n\tThu, 27 Aug 2020 13:42:21 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE1D9628D4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 Aug 2020 13:42:19 +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 relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 0BACD60009;\n\tThu, 27 Aug 2020 11:42:18 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 27 Aug 2020 13:46:04 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200827114604.kez3nz2n3h6gp6lu@uno.localdomain>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-3-jacopo@jmondi.org>\n\t<50d2139e-def0-23ee-261a-464a0e7fa986@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<50d2139e-def0-23ee-261a-464a0e7fa986@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12173,"web_url":"https://patchwork.libcamera.org/comment/12173/","msgid":"<4d0d3bf8-9cb3-a1e5-6609-1f0fcbfc7283@ideasonboard.com>","date":"2020-08-27T11:45:17","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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 12:46, Jacopo Mondi wrote:\n> On Thu, Aug 27, 2020 at 12:21:24PM +0100, Kieran Bingham wrote:\n>> Hi Jacopo,\n>>\n>> On 27/08/2020 09:20, Jacopo Mondi wrote:\n>>> Be a tad more verbose on failures in opening the dma_heaps allocator\n>>> devices.\n>>>\n>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>>> ---\n>>>  src/libcamera/pipeline/raspberrypi/dma_heaps.cpp | 7 ++++++-\n>>>  1 file changed, 6 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n>>> index 739f05d3d4d8..500f1eac2eb8 100644\n>>> --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n>>> +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n>>> @@ -45,9 +45,14 @@ int DmaHeap::open()\n>>>  {\n>>>  \tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_NAME, O_RDWR, 0);\n>>>  \tif (dmaHeapHandle_ == -1) {\n>>> +\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n>>> +\t\t\t\t<< DMA_HEAP_CMA_NAME << \": \"\n>>> +\t\t\t\t<< strerror(errno);\n>>\n>> This will report an error, even if the ALT_NAME is successful...\n>>\n> \n> True, but I think that's ok as the DMA_HEAP_CMA_ALT_NAME device is\n> just a fallback and there shouldn't be a need to poke it\n\nOh, that's not how I interpret the comment up at the #defines.\n\nIt says if the cma heap size is specified on the kernel command line,\nthen the heap gets named as reserved instead.\n\nThat therefore implies to me that with your patch, when the user\nspecifies a cma heap size on the kernel, they will always be presented\nwith an error - in a situation which is not an error if I understand it\ncorrectly.\n\n\n\n>>\n>>>  \t\tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_ALT_NAME, O_RDWR, 0);\n>>>  \t\tif (dmaHeapHandle_ == -1) {\n>>> -\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device\";\n>>> +\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n>>> +\t\t\t\t\t<< DMA_HEAP_CMA_ALT_NAME << \": \"\n>>> +\t\t\t\t\t<< strerror(errno);\n>>>  \t\t\treturn dmaHeapHandle_;\n>>>  \t\t}\n>>>  \t}\n>>>\n>>\n>> I wonder if this is a good opportunity to refactor this code:\n>>\n>> (untested/uncompiled psuedo code to follow)\n>>\n>>\n>> std::array<const char *, 2> heap_names {\n>> \t\"/dev/dma_heap/linux,cma\",\n>> \t\"/dev/dma_heap/reserved\",\n>> }\n>>\n>> int DmaHeap::open()\n>> {\n>> \tfor (const char *heap : heap_names) {\n>> \t\tif (!libcamera::File::exists(heap))\n>> \t\t\tcontinue;\n>>\n>> \t\tdmaHeapHandle_ = ::open(heap, O_RDWR, 0);\n>>\n>> \t\tif (dmaHeapHandle_ != -1) /* Success! */\n>> \t\t\treturn 0;\n>>\n>> \t\tLOG(RPI, Warning)\n>> \t\t\t<< \"Could not open dmaHeap \"\n>> \t\t\t<< heap << \": \"\t<< strerror(errno);\n>> \t}\n>>\n>> \t/* No DMA Heap was available */\n>> \tLOG(RPI, Error) << \"Could not open any dmaHeap device\";\n>>\n>> \treturn -ENODEV;\n> \n> \n> We could work on something similar indeed if erroring on the first\n> failure is not welcome.\n> \n> Thanks\n>   j\n> \n>> }\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","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 0F074BF019\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 27 Aug 2020 11:45:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8BE3862901;\n\tThu, 27 Aug 2020 13:45: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 26E8E628D4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 Aug 2020 13:45:27 +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 739461870;\n\tThu, 27 Aug 2020 13:45:20 +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=\"NcusFgbu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598528722;\n\tbh=jWk2ZbwsUHs6B9HSvSIEgdq263YLhQapYWi++a14vjo=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=NcusFgbuuU6Ttk47F30eP8AcXknj5X52Wve/fBRhUXJGryQHMqaSsLvmAuGGgriJP\n\twg/KanOYhfWoKXZgH/Hv2i301L2IuunONapLjiIV/r9Oomtkw7TmlDkoad7HtaQuyu\n\tUlizD+TsTc/isuoYO9PngQinQybVF7NK4rYgzK8Y=","To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-3-jacopo@jmondi.org>\n\t<50d2139e-def0-23ee-261a-464a0e7fa986@ideasonboard.com>\n\t<20200827114604.kez3nz2n3h6gp6lu@uno.localdomain>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<4d0d3bf8-9cb3-a1e5-6609-1f0fcbfc7283@ideasonboard.com>","Date":"Thu, 27 Aug 2020 12:45:17 +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":"<20200827114604.kez3nz2n3h6gp6lu@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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>","Reply-To":"kieran.bingham@ideasonboard.com","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12177,"web_url":"https://patchwork.libcamera.org/comment/12177/","msgid":"<20200827115945.gqwupbytp5fngthq@uno.localdomain>","date":"2020-08-27T11:59:45","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Thu, Aug 27, 2020 at 12:45:17PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 27/08/2020 12:46, Jacopo Mondi wrote:\n> > On Thu, Aug 27, 2020 at 12:21:24PM +0100, Kieran Bingham wrote:\n> >> Hi Jacopo,\n> >>\n> >> On 27/08/2020 09:20, Jacopo Mondi wrote:\n> >>> Be a tad more verbose on failures in opening the dma_heaps allocator\n> >>> devices.\n> >>>\n> >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> >>> ---\n> >>>  src/libcamera/pipeline/raspberrypi/dma_heaps.cpp | 7 ++++++-\n> >>>  1 file changed, 6 insertions(+), 1 deletion(-)\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> >>> index 739f05d3d4d8..500f1eac2eb8 100644\n> >>> --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> >>> @@ -45,9 +45,14 @@ int DmaHeap::open()\n> >>>  {\n> >>>  \tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_NAME, O_RDWR, 0);\n> >>>  \tif (dmaHeapHandle_ == -1) {\n> >>> +\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n> >>> +\t\t\t\t<< DMA_HEAP_CMA_NAME << \": \"\n> >>> +\t\t\t\t<< strerror(errno);\n> >>\n> >> This will report an error, even if the ALT_NAME is successful...\n> >>\n> >\n> > True, but I think that's ok as the DMA_HEAP_CMA_ALT_NAME device is\n> > just a fallback and there shouldn't be a need to poke it\n>\n> Oh, that's not how I interpret the comment up at the #defines.\n>\n> It says if the cma heap size is specified on the kernel command line,\n> then the heap gets named as reserved instead.\n>\n> That therefore implies to me that with your patch, when the user\n> specifies a cma heap size on the kernel, they will always be presented\n> with an error - in a situation which is not an error if I understand it\n> correctly.\n>\n\n * Annoyingly, should the cma heap size be specified on the kernel command line\n * instead of DT, the heap gets named \"reserved\" instead.\n\nYou're right indeed, even if that's just a printout, it should be\navoided.\n\nI'll re-work this one!\n\nThanks\n   j\n\n>\n>\n> >>\n> >>>  \t\tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_ALT_NAME, O_RDWR, 0);\n> >>>  \t\tif (dmaHeapHandle_ == -1) {\n> >>> -\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device\";\n> >>> +\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n> >>> +\t\t\t\t\t<< DMA_HEAP_CMA_ALT_NAME << \": \"\n> >>> +\t\t\t\t\t<< strerror(errno);\n> >>>  \t\t\treturn dmaHeapHandle_;\n> >>>  \t\t}\n> >>>  \t}\n> >>>\n> >>\n> >> I wonder if this is a good opportunity to refactor this code:\n> >>\n> >> (untested/uncompiled psuedo code to follow)\n> >>\n> >>\n> >> std::array<const char *, 2> heap_names {\n> >> \t\"/dev/dma_heap/linux,cma\",\n> >> \t\"/dev/dma_heap/reserved\",\n> >> }\n> >>\n> >> int DmaHeap::open()\n> >> {\n> >> \tfor (const char *heap : heap_names) {\n> >> \t\tif (!libcamera::File::exists(heap))\n> >> \t\t\tcontinue;\n> >>\n> >> \t\tdmaHeapHandle_ = ::open(heap, O_RDWR, 0);\n> >>\n> >> \t\tif (dmaHeapHandle_ != -1) /* Success! */\n> >> \t\t\treturn 0;\n> >>\n> >> \t\tLOG(RPI, Warning)\n> >> \t\t\t<< \"Could not open dmaHeap \"\n> >> \t\t\t<< heap << \": \"\t<< strerror(errno);\n> >> \t}\n> >>\n> >> \t/* No DMA Heap was available */\n> >> \tLOG(RPI, Error) << \"Could not open any dmaHeap device\";\n> >>\n> >> \treturn -ENODEV;\n> >\n> >\n> > We could work on something similar indeed if erroring on the first\n> > failure is not welcome.\n> >\n> > Thanks\n> >   j\n> >\n> >> }\n> >>\n> >> --\n> >> Regards\n> >> --\n> >> Kieran\n>\n> --\n> Regards\n> --\n> Kieran","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 8BE07BF019\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 27 Aug 2020 11:56:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68E5062901;\n\tThu, 27 Aug 2020 13:56:02 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D156628D4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 Aug 2020 13:56:01 +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 relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 30D7940003;\n\tThu, 27 Aug 2020 11:55:59 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 27 Aug 2020 13:59:45 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200827115945.gqwupbytp5fngthq@uno.localdomain>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-3-jacopo@jmondi.org>\n\t<50d2139e-def0-23ee-261a-464a0e7fa986@ideasonboard.com>\n\t<20200827114604.kez3nz2n3h6gp6lu@uno.localdomain>\n\t<4d0d3bf8-9cb3-a1e5-6609-1f0fcbfc7283@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<4d0d3bf8-9cb3-a1e5-6609-1f0fcbfc7283@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12185,"web_url":"https://patchwork.libcamera.org/comment/12185/","msgid":"<20200827141958.GI6042@pendragon.ideasonboard.com>","date":"2020-08-27T14:19:58","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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 01:59:45PM +0200, Jacopo Mondi wrote:\n> On Thu, Aug 27, 2020 at 12:45:17PM +0100, Kieran Bingham wrote:\n> > On 27/08/2020 12:46, Jacopo Mondi wrote:\n> > > On Thu, Aug 27, 2020 at 12:21:24PM +0100, Kieran Bingham wrote:\n> > >> On 27/08/2020 09:20, Jacopo Mondi wrote:\n> > >>> Be a tad more verbose on failures in opening the dma_heaps allocator\n> > >>> devices.\n> > >>>\n> > >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > >>> ---\n> > >>>  src/libcamera/pipeline/raspberrypi/dma_heaps.cpp | 7 ++++++-\n> > >>>  1 file changed, 6 insertions(+), 1 deletion(-)\n> > >>>\n> > >>> diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> > >>> index 739f05d3d4d8..500f1eac2eb8 100644\n> > >>> --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> > >>> +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp\n> > >>> @@ -45,9 +45,14 @@ int DmaHeap::open()\n> > >>>  {\n> > >>>  \tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_NAME, O_RDWR, 0);\n> > >>>  \tif (dmaHeapHandle_ == -1) {\n> > >>> +\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n> > >>> +\t\t\t\t<< DMA_HEAP_CMA_NAME << \": \"\n> > >>> +\t\t\t\t<< strerror(errno);\n> > >>\n> > >> This will report an error, even if the ALT_NAME is successful...\n> > >>\n> > >\n> > > True, but I think that's ok as the DMA_HEAP_CMA_ALT_NAME device is\n> > > just a fallback and there shouldn't be a need to poke it\n> >\n> > Oh, that's not how I interpret the comment up at the #defines.\n> >\n> > It says if the cma heap size is specified on the kernel command line,\n> > then the heap gets named as reserved instead.\n> >\n> > That therefore implies to me that with your patch, when the user\n> > specifies a cma heap size on the kernel, they will always be presented\n> > with an error - in a situation which is not an error if I understand it\n> > correctly.\n> \n>  * Annoyingly, should the cma heap size be specified on the kernel command line\n>  * instead of DT, the heap gets named \"reserved\" instead.\n> \n> You're right indeed, even if that's just a printout, it should be\n> avoided.\n\nAgreed. Let's avoid printing any non-debug message for the case where we\nfall back to the alternative name. An error message at the end to report\nthat no heap could be found should be enough.\n\n> I'll re-work this one!\n> \n> > >>\n> > >>>  \t\tdmaHeapHandle_ = ::open(DMA_HEAP_CMA_ALT_NAME, O_RDWR, 0);\n> > >>>  \t\tif (dmaHeapHandle_ == -1) {\n> > >>> -\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device\";\n> > >>> +\t\t\tLOG(RPI, Error) << \"Could not open dmaHeap device \"\n> > >>> +\t\t\t\t\t<< DMA_HEAP_CMA_ALT_NAME << \": \"\n> > >>> +\t\t\t\t\t<< strerror(errno);\n> > >>>  \t\t\treturn dmaHeapHandle_;\n> > >>>  \t\t}\n> > >>>  \t}\n> > >>>\n> > >>\n> > >> I wonder if this is a good opportunity to refactor this code:\n> > >>\n> > >> (untested/uncompiled psuedo code to follow)\n> > >>\n> > >>\n> > >> std::array<const char *, 2> heap_names {\n> > >> \t\"/dev/dma_heap/linux,cma\",\n> > >> \t\"/dev/dma_heap/reserved\",\n> > >> }\n> > >>\n> > >> int DmaHeap::open()\n> > >> {\n> > >> \tfor (const char *heap : heap_names) {\n> > >> \t\tif (!libcamera::File::exists(heap))\n> > >> \t\t\tcontinue;\n> > >>\n> > >> \t\tdmaHeapHandle_ = ::open(heap, O_RDWR, 0);\n> > >>\n> > >> \t\tif (dmaHeapHandle_ != -1) /* Success! */\n> > >> \t\t\treturn 0;\n> > >>\n> > >> \t\tLOG(RPI, Warning)\n> > >> \t\t\t<< \"Could not open dmaHeap \"\n> > >> \t\t\t<< heap << \": \"\t<< strerror(errno);\n> > >> \t}\n> > >>\n> > >> \t/* No DMA Heap was available */\n> > >> \tLOG(RPI, Error) << \"Could not open any dmaHeap device\";\n> > >>\n> > >> \treturn -ENODEV;\n> > >\n> > > We could work on something similar indeed if erroring on the first\n> > > failure is not welcome.","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 92B44BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 27 Aug 2020 14:20:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 159F662912;\n\tThu, 27 Aug 2020 16:20:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D441628D4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 Aug 2020 16:20:19 +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 F03FF1872;\n\tThu, 27 Aug 2020 16:20:17 +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=\"nlgj1kmt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598538018;\n\tbh=L9O5e30xxNZvG8G5WxrY60Ir/eWKhSstMFK1VLhRuS4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nlgj1kmttzHsH0+mlRN8bRFbEhn8C+jTPFwvHryu32Wz59jAUy7lg8hYxV+DdHOmV\n\tuD55ARVJY3zRKFGukKI3JCb7GcultggVER/n/FZtwfAWENSEgT1o395Cv52Y0d8zZh\n\trTDkArAZjPGsyLsEYZrf6932JbouGo10krjJ5gQs=","Date":"Thu, 27 Aug 2020 17:19:58 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200827141958.GI6042@pendragon.ideasonboard.com>","References":"<20200827082038.40758-1-jacopo@jmondi.org>\n\t<20200827082038.40758-3-jacopo@jmondi.org>\n\t<50d2139e-def0-23ee-261a-464a0e7fa986@ideasonboard.com>\n\t<20200827114604.kez3nz2n3h6gp6lu@uno.localdomain>\n\t<4d0d3bf8-9cb3-a1e5-6609-1f0fcbfc7283@ideasonboard.com>\n\t<20200827115945.gqwupbytp5fngthq@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200827115945.gqwupbytp5fngthq@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: raspberrypi:\n\tdma_heaps: Be verbose on errors","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]