[libcamera-devel] [PATCH] rkisp1: Add support for sensor test pattern control

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-10-20 00:25:33)
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Tue, Oct 18, 2022 at 05:34:38PM +0900, Paul Elder via libcamera-devel wrote:
> > Add support for the TestPatternMode control.
> > 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/rkisp1.cpp                |  7 +++++
> 
> For IPU3, we implements TPG support in the pipeline handler, without
> involving the IPA module (see commit acf8d028edda "libcamera: pipeline:
> ipu3: Apply a requested test pattern mode"). Is there a reason to do
> differently here ?

I actually suspect we might be better going through the IPA, as I recall
someone reported that if we enable the TPG on IPU3, then the AE/AGC can
end up breaking... So we might need to let the IPA know that the gains
applied will not correlate to the statistics of the frame received.

It would also mean we can align how we handle per-frame control for
'which' frame to apply the test-pattern on, along with other sensor
controls.

> 
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 +++++++++++++++++++++++-
> >  2 files changed, 44 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index 3f5c1a58..1aac8884 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -183,6 +183,13 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
> >  
> >       /* Return the controls handled by the IPA. */
> >       ControlInfoMap::Map ctrlMap = rkisp1Controls;
> > +
> > +     auto tpgInfo = sensorControls.find(V4L2_CID_TEST_PATTERN);
> > +     if (tpgInfo != sensorControls.end()) {
> > +             ctrlMap.emplace(&controls::draft::TestPatternMode,
> > +                             ControlInfo(tpgInfo->second.values()));
> > +     }
> > +
> >       *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> >  
> >       return 0;
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 455ee2a0..42803266 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -93,6 +93,8 @@ public:
> >       PipelineHandlerRkISP1 *pipe();
> >       int loadIPA(unsigned int hwRevision);
> >  
> > +     int setSensorTestPattern(const ControlList *controls);
> > +
> >       Stream mainPathStream_;
> >       Stream selfPathStream_;
> >       std::unique_ptr<CameraSensor> sensor_;
> > @@ -104,6 +106,8 @@ public:
> >       RkISP1MainPath *mainPath_;
> >       RkISP1SelfPath *selfPath_;
> >  
> > +     controls::draft::TestPatternModeEnum testPatternMode_;
> > +
> >       std::unique_ptr<ipa::rkisp1::IPAProxyRkISP1> ipa_;
> >  
> >  private:
> > @@ -357,6 +361,30 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
> >       return 0;
> >  }
> >  
> > +int RkISP1CameraData::setSensorTestPattern(const ControlList *controls)
> > +{
> > +     if (!controls)
> > +             return 0;
> > +
> > +     const auto &testPattern = controls->get(controls::draft::TestPatternMode);
> > +
> > +     if (!testPattern)
> > +             return 0;
> > +
> > +     if (testPattern && *testPattern == testPatternMode_)
> > +             return 0;
> > +
> > +     testPatternMode_ = static_cast<controls::draft::TestPatternModeEnum>(*testPattern);
> > +
> > +     int ret = sensor_->setTestPatternMode(testPatternMode_);
> > +     if (ret && !sensor_->testPatternModes().empty())
> > +             return ret;
> > +
> > +     LOG(RkISP1, Debug) << "Set test pattern to " << static_cast<int>(testPatternMode_);
> > +
> > +     return 0;
> > +}
> > +
> >  void RkISP1CameraData::paramFilled(unsigned int frame)
> >  {
> >       PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
> > @@ -820,11 +848,15 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
> >       return 0;
> >  }
> >  
> > -int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
> > +int PipelineHandlerRkISP1::start(Camera *camera, const ControlList *controls)
> >  {
> >       RkISP1CameraData *data = cameraData(camera);
> >       int ret;
> >  
> > +     ret = data->setSensorTestPattern(controls);
> > +     if (ret)
> > +             return ret;
> > +
> >       /* Allocate buffers for internal pipeline usage. */
> >       ret = allocateBuffers(camera);
> >       if (ret)
> > @@ -923,6 +955,10 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
> >  {
> >       RkISP1CameraData *data = cameraData(camera);
> >  
> > +     int ret = data->setSensorTestPattern(&request->controls());
> > +     if (ret)
> > +             return ret;
> > +
> >       RkISP1FrameInfo *info = data->frameInfo_.create(data, request);
> >       if (!info)
> >               return -ENOENT;
> 
> -- 
> Regards,
> 
> Laurent Pinchart