[libcamera-devel,v2,3/3] libcamera: raspberrypi: Check dma heap allocator

Message ID 20200828155136.541696-3-jacopo@jmondi.org
State Accepted
Headers show
Series
  • [libcamera-devel,v2,1/3] libcamera: raspberrypi: dmaheaps: Improve device open
Related show

Commit Message

Jacopo Mondi Aug. 28, 2020, 3:51 p.m. UTC
Check if the dmaHeap_ allocator is valid at match() time to fail
earlier if its construction failed.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 ++
 1 file changed, 2 insertions(+)

Comments

Laurent Pinchart Aug. 28, 2020, 4:07 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Fri, Aug 28, 2020 at 05:51:36PM +0200, Jacopo Mondi wrote:
> Check if the dmaHeap_ allocator is valid at match() time to fail
> earlier if its construction failed.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index c1451e71b587..ce43af345f53 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -904,6 +904,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  		return false;
>  
>  	std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
> +	if (!data->dmaHeap_.isValid())
> +		return false;
>  
>  	/* Locate and open the unicam video streams. */
>  	data->unicam_[Unicam::Embedded] = RPiStream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));
Kieran Bingham Aug. 28, 2020, 4:11 p.m. UTC | #2
On 28/08/2020 16:51, Jacopo Mondi wrote:
> Check if the dmaHeap_ allocator is valid at match() time to fail
> earlier if its construction failed.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index c1451e71b587..ce43af345f53 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -904,6 +904,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>  		return false;
>  
>  	std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
> +	if (!data->dmaHeap_.isValid())
> +		return false;

It feels a bit awkward the way we're returning here, but an error will
have been reported, so I think it's ok, and it's not really a fault of
this patch.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  
>  	/* Locate and open the unicam video streams. */
>  	data->unicam_[Unicam::Embedded] = RPiStream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));
>

Patch

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index c1451e71b587..ce43af345f53 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -904,6 +904,8 @@  bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
 		return false;
 
 	std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
+	if (!data->dmaHeap_.isValid())
+		return false;
 
 	/* Locate and open the unicam video streams. */
 	data->unicam_[Unicam::Embedded] = RPiStream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));