[libcamera-devel] [PATCH 2/2] libcamera: pipeline: RKISP1 configure isp output pad

[Laurent Pinchart]

Hi Helen,

Thank you for the patch.

On Wed, Jul 03, 2019 at 05:21:54PM -0300, Helen Koike wrote:
> ISP output pad should be set to YUYV8_2X8 for non-bayer output format.
> Bayer formats are not listed in RkISP1CameraConfiguration::validate(),
> only non-bayer are listed, so we can set YUYV8_2X8 directly.
> This need to be changed if we add support for bayer output with
> libcamera.
> 
> Signed-off-by: Helen Koike <helen.koike at collabora.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 358e2c8..fc04cf8 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -279,13 +279,23 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>  	LOG(RkISP1, Debug) << "Sensor configured with " << format.toString();
>  
> -	LOG(RkISP1, Debug) << "Configuring ISP with " << format.toString();
> +	LOG(RkISP1, Debug) << "Configuring ISP input pad with " << format.toString();
>  
>  	ret = isp_->setFormat(0, &format);
>  	if (ret < 0)
>  		return ret;
>  
> -	LOG(RkISP1, Debug) << "ISP configured with " << format.toString();
> +	LOG(RkISP1, Debug) << "ISP input pad configured with " << format.toString();
> +
> +	/* YUYV8_2X8 is required in ISP pad 1 for non-bayer output */
> +	format.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;
> +	LOG(RkISP1, Debug) << "Configuring ISP output pad with " << format.toString();
> +
> +	ret = isp_->setFormat(2, &format);
> +	if (ret < 0)
> +		return ret;
> +
> +	LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();

This looks reasonable to me, but I wonder why the current code works :-)
What happens on your test system without this patch ? Or is this needed
due to a change in your rkisp1 patch series ? If so, could you briefly
explain what that change is ?

>  
>  	V4L2DeviceFormat outputFormat = {};
>  	outputFormat.fourcc = cfg.pixelFormat;

-- 
Regards,

Laurent Pinchart