[libcamera-devel] [PATCH v4 7/7] libcamera: pipeline: rkisp1: Use the media link to track if a path is enabled

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tue, Sep 29, 2020 at 03:43:34AM +0200, Niklas Söderlund wrote:
> Instead of manually tracking if a path is enable or not use the media
> graph link status. There is no functional change.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> * Changes since v3
> - Update spelling in commit message.
> - Do not check path status in RkISP1Path::start() instead keep check in
>   pipeline start().
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp    | 26 +++++++--------------
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h |  1 +
>  2 files changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 72593a06b0f502b0..48d00970aba3beda 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -122,8 +122,7 @@ public:
>  	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
>  			 RkISP1SelfPath *selfPath)
>  		: CameraData(pipe), sensor_(nullptr), frame_(0),
> -		  frameInfo_(pipe), mainPath_(mainPath), selfPath_(selfPath),
> -		  mainPathActive_(false), selfPathActive_(false)
> +		  frameInfo_(pipe), mainPath_(mainPath), selfPath_(selfPath)
>  	{
>  	}
>  
> @@ -145,9 +144,6 @@ public:
>  	RkISP1MainPath *mainPath_;
>  	RkISP1SelfPath *selfPath_;
>  
> -	bool mainPathActive_;
> -	bool selfPathActive_;
> -
>  private:
>  	void queueFrameAction(unsigned int frame,
>  			      const IPAOperationData &action);
> @@ -709,20 +705,14 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>  	LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
>  
> -	data->mainPathActive_ = false;
> -	data->selfPathActive_ = false;
>  	for (const StreamConfiguration &cfg : *config) {
> -		if (cfg.stream() == &data->mainPathStream_) {
> +		if (cfg.stream() == &data->mainPathStream_)
>  			ret = mainPath_.configure(cfg, format);
> -			if (ret)
> -				return ret;
> -			data->mainPathActive_ = true;
> -		} else {
> +		else
>  			ret = selfPath_.configure(cfg, format);
> -			if (ret)
> -				return ret;
> -			data->selfPathActive_ = true;
> -		}
> +
> +		if (ret)
> +			return ret;
>  	}
>  
>  	V4L2DeviceFormat paramFormat = {};
> @@ -868,7 +858,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  
>  	std::map<unsigned int, IPAStream> streamConfig;
>  
> -	if (data->mainPathActive_) {
> +	if (data->mainPath_->isEnabled()) {
>  		ret = mainPath_.start();
>  		if (ret) {
>  			param_->streamOff();
> @@ -884,7 +874,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		};
>  	}
>  
> -	if (data->selfPathActive_) {
> +	if (data->selfPath_->isEnabled()) {
>  		ret = selfPath_.start();
>  		if (ret) {
>  			mainPath_.stop();
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index 98863a2e693729be..f34d6bacd49c8f02 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -35,6 +35,7 @@ public:
>  	bool init(MediaDevice *media);
>  
>  	int setEnabled(bool enable) { return link_->setEnabled(enable); }
> +	bool isEnabled() { return link_->flags() & MEDIA_LNK_FL_ENABLED; }

	bool isEnabled() const

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

>  
>  	StreamConfiguration generateConfiguration(const Size &resolution);
>  	CameraConfiguration::Status validate(StreamConfiguration *cfg);

-- 
Regards,

Laurent Pinchart