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

[Laurent Pinchart]

Hi Helen,

Thank you for the patch, and sorry for the late reply.

On Tue, Jul 30, 2019 at 04:10:07PM -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

s/need/needs/

> libcamera.
> 
> Signed-off-by: Helen Koike <helen.koike at collabora.com>
> 
> ---
> 
> Hi,
> 
> I'm re-sending this patch standalone as Laurent suggested.
> 
> Thanks
> Helen
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index efa9604..bc7cb3f 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -286,6 +286,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	if (ret < 0)
>  		return ret;
>  
> +	LOG(RkISP1, Debug) << "Configuring ISP input pad with " << format.toString();
> +
>  	ret = dphy_->getFormat(1, &format);
>  	if (ret < 0)
>  		return ret;
> @@ -294,6 +296,18 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	if (ret < 0)
>  		return ret;
>  
> +	LOG(RkISP1, Debug) << "ISP input pad configured with " << format.toString();
> +
> +	/* YUYV8_2X8 is required in ISP pad 1 for non-bayer output */

s/output/output./

According to the code below, isn't this pad 2 ? I would write "YUYV8_2X8
is required on the ISP source path pad for YUV output.".

Apart from that,

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

Please let me know if you are fine with those small changes, I will then
add them when applying the patch.

Niklas, I know you have a running environment for the Scarlet tablet,
would you be able to test this patch ?

> +	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();
> +
>  	V4L2DeviceFormat outputFormat = {};
>  	outputFormat.fourcc = cfg.pixelFormat;
>  	outputFormat.size = cfg.size;

-- 
Regards,

Laurent Pinchart