[libcamera-devel] [PATCH] ipa: rkisp1: Do not set controls during configure

[Kieran Bingham]

On 25/03/2021 17:42, Sebastian Fricke wrote:
> The configure operation is synchronous and should not send events back
> to the pipeline handler.
> 
> If information needs to be returned from configure it should be handled
> through the interface directly.
> 
> Move the initial call to setControls() out of configure() and into the
> start() method which is called after the IPA running_ state is updated.
> 
> Signed-off-by: Sebastian Fricke <sebastian.fricke at posteo.net>

Given that this matches the IPU3 implementation, and it's known that we
need to update this soon, I think this should be merged.

With the whitespace issues corrected.

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

Niklas, as this has just hit you, could you apply this and fix the
whitespace and apply after testing please?


> ---
>  src/ipa/rkisp1/rkisp1.cpp | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index d2a10bb9..b0698903 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -32,7 +32,7 @@ class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface
>  {
>  public:
>  	int init(unsigned int hwRevision) override;
> -	int start() override { return 0; }
> +	int start() override;
>  	void stop() override {}
>  
>  	int configure(const CameraSensorInfo &info,
> @@ -80,6 +80,13 @@ int IPARkISP1::init(unsigned int hwRevision)
>  	return 0;
>  }
>  
> +int IPARkISP1::start()
> +{
> +        setControls(0);
> +
> +        return 0;
> +}
> +
>  /**
>   * \todo The RkISP1 pipeline currently provides an empty CameraSensorInfo
>   * if the connected sensor does not provide enough information to properly
> @@ -121,7 +128,6 @@ int IPARkISP1::configure([[maybe_unused]] const CameraSensorInfo &info,
>  		<< "Exposure: " << minExposure_ << "-" << maxExposure_
>  		<< " Gain: " << minGain_ << "-" << maxGain_;
>  
> -	setControls(0);
>  	return 0;
>  }
>  
> 

-- 
Regards
--
Kieran