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

[Helen Koike]

Hi Laurent,

On 8/8/19 7:01 PM, Laurent Pinchart wrote:
> Hi Helen,
> 
> Thank you for the patch, and sorry for the late reply.

no problem, thanks for reviewing.

> 
> 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.".

Yes, it is pad 1, sorry about that

> 
> 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.

Yes, I'm fine with those changes, thanks!

Regards,
Helen

> 
> 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;
>