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

[Helen Koike]

On 7/3/19 7:42 PM, Laurent Pinchart wrote:
> 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 ?

It works without this patch if you don't touch the default configuration
in the topology, but if you set the output pad to be bayer with
media-ctl for instance, then cam tool won't work anymore.

Thanks
Helen

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