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

[Laurent Pinchart]

Hi Helen,

On Thu, Jul 04, 2019 at 04:44:24PM -0300, Helen Koike wrote:
> On 7/3/19 7:42 PM, Laurent Pinchart wrote:
> > 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.

Of course. This makes complete sense.

I would like to delay application 1/2 until the corresponding kernel
patch series gets reviewed, but I think 2/2 could be merged already. It
however depends on 1/2. Would you mind rebasing it as a standalone patch
?

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

-- 
Regards,

Laurent Pinchart