[PATCH] [RFC] libcamera: pipeline: Add h/v blanking delays

[Kieran Bingham]

Quoting Paul Elder (2025-05-13 20:07:21)
> Quoting Kieran Bingham (2025-05-13 19:43:12)
> > The HBLANK and VBLANK control delays are not being accounted for in all
> > of the libipa platforms. Update their delayed controls instantiation
> > accordingly.
> 
> I thought hblank was a generally a read-only control?

Generally perhaps - but not always - it can be a read-only or a
writeable control for devices that can set it. So we should consider it
when possible. It will give a better configuration range on exposure and
frame duration if we have sensors with hblank controls.

But yes, some sensors are read only here.

> > In particular, update the RKISP1 VBLANK control delay to be marked as a
> > priority control as well as setting the correct delay to align with the
> > existing Raspberry Pi implementation.
> 
> Given that the priorityWrite docs says 
> 
>  * Typically set for the \a V4L2_CID_VBLANK control so that the device driver
>  * does not reject \a V4L2_CID_EXPOSURE control values that may be outside of
>  * the existing vertical blanking specified bounds, but are within the new
>  * blanking bounds.
> 
> I suppose we do want this.
> 
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> > 
> > Marking this as RFC because I haven't tested this or explored it yet -
> > but as I was reviewing other parts of the code base - I noted that these
> > are inconsistent across pipeline handlers - so lets figure out why and
> > make sure all the pipeline handlers operate in the same way where
> > possible.
> > 
> >  src/libcamera/pipeline/ipu3/ipu3.cpp         | 2 ++
> >  src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 ++
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 3 ++-
> >  src/libcamera/pipeline/simple/simple.cpp     | 2 ++
> >  4 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index e31e3879dcc9..4cec22b01940 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -1082,6 +1082,8 @@ int PipelineHandlerIPU3::registerCameras()
> >                 std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> >                         { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> >                         { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> > +                       { V4L2_CID_HBLANK, { delays.hblankDelay, false } },
> > +                       { V4L2_CID_VBLANK, { delays.vblankDelay, true } },
> >                 };
> >  
> >                 data->delayedCtrls_ =
> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > index a05e11fccf8d..7d052263b34d 100644
> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > @@ -1607,6 +1607,8 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink)
> >                 std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> >                         { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> >                         { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> > +                       { V4L2_CID_HBLANK, { delays.hblankDelay, false } },
> > +                       { V4L2_CID_VBLANK, { delays.vblankDelay, true } },
> >                 };
> >  
> >                 data->delayedCtrls_ =
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 52633fe3cb85..6c5b81b9a2ba 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -1325,7 +1325,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >         std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> >                 { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> >                 { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> > -               { V4L2_CID_VBLANK, { 1, false } },
> 
> (This probably won't apply upstream as it was recently fixed to
> { V4L2_CID_VBLANK, { delays.vblankDelay, false } }, )
> 

Ooops - I hadn't realised I wasn't on a mainline branch...

So I'll rebase and update - but seems like it is the right thing to do
for each pipeline ... which makes me wonder if we should just put a
helper somewhere common (V4L2Device given it's V4L2Specific? or pipeline
handler base, or common utils ?) that can parse the delayed controls
delays in the same way for each...

> 
> Paul
> 
> > +               { V4L2_CID_HBLANK, { delays.hblankDelay, false } },
> > +               { V4L2_CID_VBLANK, { delays.vblankDelay, true } },
> >         };
> >  
> >         data->delayedCtrls_ =
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index efb07051b175..fc88efc7a716 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -546,6 +546,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> >         std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> >                 { V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> >                 { V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> > +               { V4L2_CID_HBLANK, { delays.hblankDelay, false } },
> > +               { V4L2_CID_VBLANK, { delays.vblankDelay, true } },
> >         };
> >         delayedCtrls_ = std::make_unique<DelayedControls>(sensor_->device(), params);
> >  
> > -- 
> > 2.49.0
> >