[libcamera-devel] [PATCH 1/1] pipeline: rkisp1: Fix sensor-ISP format mismatch

[Jacopo Mondi]

Hi Sebastian,

On Thu, Nov 19, 2020 at 02:13:31PM +0100, Sebastian Fricke wrote:
> Make sure to use a sensor format resolution that is smaller or equal to
> the maximum allowed resolution for the RkISP1. The maximum resolution
> is defined within the `rkisp1-common.h` file as:
> define RKISP1_ISP_MAX_WIDTH		4032
> define RKISP1_ISP_MAX_HEIGHT		3024
>
> Change the order of setting the formats, in order to first check if the
> requested resolution exceeds the maximum and search for the next smaller
> available sensor resolution if that is the case.
> Fail if no viable sensor format was located.
>
> This means that some camera sensors can never operate with their maximum
> resolution, for example on my OV13850 camera sensor, there are two
> possible resolutions: 4224x3136 & 2112x1568, the first of those two will
> never be picked as it surpasses the maximum of the ISP.
>
> Signed-off-by: Sebastian Fricke <sebastian.fricke.linux at gmail.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 36 ++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 1b1922a..621e9bf 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -671,6 +671,9 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	RkISP1CameraData *data = cameraData(camera);
>  	CameraSensor *sensor = data->sensor_;
>  	int ret;
> +	Size original_format_size = Size(0, 0);
> +	Size max_size = Size(0, 0);
> +
>
>  	ret = initLinks(camera, sensor, *config);
>  	if (ret)
> @@ -682,17 +685,44 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	 */
>  	V4L2SubdeviceFormat format = config->sensorFormat();
>  	LOG(RkISP1, Debug) << "Configuring sensor with " << format.toString();
> +	// format is changed in setFormat, keep the resolution for comparison

Just a style note skimming through the patch.

libcamera enforces a common code style
http://libcamera.org/coding-style.html#coding-style-guidelines

and provides a style checker tool in utils/checkstyle.py

Make sure to use it before submitting patches.

Thanks
  j

> +	original_format_size = format.size;
>
> -	ret = sensor->setFormat(&format);
> +	ret = isp_->setFormat(0, &format);
>  	if (ret < 0)
>  		return ret;
> +	LOG(RkISP1, Debug) << "ISP configured with " << format.toString();
> +
> +	if (original_format_size > format.size) {
> +		LOG(RkISP1, Info) << "Configured resolution is greater than "
> +				     "the maximum resolution for the ISP, "
> +				     "trying to re-configure to a smaller "
> +				     "valid sensor format";
> +		for (const Size &size : sensor->sizes()) {
> +			if (size <= format.size && size > max_size)
> +				max_size = size;
> +		}
> +		if (max_size == Size(0, 0)) {
> +			LOG(RkISP1, Error) << "No available sensor resolution"
> +					      "that is smaller or equal to "
> +					   << format.toString();
> +			return -1;
> +		}
> +		format = sensor->getFormat(sensor->mbusCodes(), max_size);
>
> -	LOG(RkISP1, Debug) << "Sensor configured with " << format.toString();
> +		ret = isp_->setFormat(0, &format);
> +		if (ret < 0)
> +			return ret;
> +		LOG(RkISP1, Debug) << "ISP re-configured with "
> +				   << format.toString();
> +	}
>
> -	ret = isp_->setFormat(0, &format);
> +	ret = sensor->setFormat(&format);
>  	if (ret < 0)
>  		return ret;
>
> +	LOG(RkISP1, Debug) << "Sensor configured with " << format.toString();
> +
>  	Rectangle rect(0, 0, format.size);
>  	ret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &rect);
>  	if (ret < 0)
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel