[libcamera-devel] [PATCH v2 09/13] libcamera: pipeline: rkisp1: Add self path devices

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Sep 14, 2020 at 04:21:45PM +0200, Niklas Söderlund wrote:
> Add the V4L2 device nodes needed to operate the self path.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 26 ++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 4e1295486c184178..5aa01f87afd71183 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -131,9 +131,11 @@ public:
>  class RkISP1CameraData : public CameraData
>  {
>  public:
> -	RkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)
> +	RkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *mainPathVideo,
> +			 V4L2VideoDevice *selfPathVideo)
>  		: CameraData(pipe), sensor_(nullptr), frame_(0),
> -		  frameInfo_(pipe), mainPathVideo_(video)
> +		  frameInfo_(pipe), mainPathVideo_(mainPathVideo),
> +		  selfPathVideo_(selfPathVideo)
>  	{
>  	}
>  
> @@ -152,6 +154,7 @@ public:
>  	RkISP1Timeline timeline_;
>  
>  	V4L2VideoDevice *mainPathVideo_;
> +	V4L2VideoDevice *selfPathVideo_;
>  
>  private:
>  	void queueFrameAction(unsigned int frame,
> @@ -228,7 +231,9 @@ private:
>  	MediaDevice *media_;
>  	V4L2Subdevice *isp_;
>  	V4L2Subdevice *mainPathResizer_;
> +	V4L2Subdevice *selfPathResizer_;
>  	V4L2VideoDevice *mainPathVideo_;
> +	V4L2VideoDevice *selfPathVideo_;
>  	V4L2VideoDevice *param_;
>  	V4L2VideoDevice *stat_;
>  
> @@ -561,7 +566,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  
>  PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
>  	: PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),
> -	  mainPathVideo_(nullptr), param_(nullptr), stat_(nullptr)
> +	  selfPathResizer_(nullptr), mainPathVideo_(nullptr),
> +	  selfPathVideo_(nullptr), param_(nullptr), stat_(nullptr)
>  {
>  }
>  
> @@ -570,7 +576,9 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
>  	delete param_;
>  	delete stat_;
>  	delete mainPathVideo_;
> +	delete selfPathVideo_;
>  	delete mainPathResizer_;
> +	delete selfPathResizer_;
>  	delete isp_;

Not a candidate for this series, but should we use std::unique_ptr<> ?

With the same assumption as for patch 08/13,

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

>  }
>  
> @@ -975,7 +983,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	int ret;
>  
>  	std::unique_ptr<RkISP1CameraData> data =
> -		std::make_unique<RkISP1CameraData>(this, mainPathVideo_);
> +		std::make_unique<RkISP1CameraData>(this, mainPathVideo_,
> +						   selfPathVideo_);
>  
>  	ControlInfoMap::Map ctrls;
>  	ctrls.emplace(std::piecewise_construct,
> @@ -1030,11 +1039,19 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  	if (mainPathResizer_->open() < 0)
>  		return false;
>  
> +	selfPathResizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_selfpath");
> +	if (selfPathResizer_->open() < 0)
> +		return false;
> +
>  	/* Locate and open the capture video node. */
>  	mainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
>  	if (mainPathVideo_->open() < 0)
>  		return false;
>  
> +	selfPathVideo_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_selfpath");
> +	if (selfPathVideo_->open() < 0)
> +		return false;
> +
>  	stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_stats");
>  	if (stat_->open() < 0)
>  		return false;
> @@ -1044,6 +1061,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  		return false;
>  
>  	mainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> +	selfPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
>  	stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
>  	param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
>  

-- 
Regards,

Laurent Pinchart