[libcamera-devel,v7,3/3] libcamera: pipeline: ipu3: Apply a requested test pattern mode
diff mbox series

Message ID 20211123190812.69805-4-hiroh@chromium.org
State Superseded
Headers show
Series
  • Enable to apply test pattern mode in IPU3
Related show

Commit Message

Hirokazu Honda Nov. 23, 2021, 7:08 p.m. UTC 
This enables ipu3 pipeline handler to apply a test pattern mode
per frame.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 45 ++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi Nov. 25, 2021, 12:01 a.m. UTC | #1 
Hi Hiro,

On Wed, Nov 24, 2021 at 04:08:12AM +0900, Hirokazu Honda wrote:
> This enables ipu3 pipeline handler to apply a test pattern mode
> per frame.

Yes, and this also  introduces a a way to set controls
immediately, the first (and only one for now) is the test pattern
mode.

Do you think it would be mentioned ?

>
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 45 ++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 25490dcf..a15c6466 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -59,6 +59,7 @@ public:
>  	void statBufferReady(FrameBuffer *buffer);
>  	void queuePendingRequests();
>  	void cancelPendingRequests();
> +	void frameStart(uint32_t sequence);
>
>  	CIO2Device cio2_;
>  	ImgUDevice *imgu_;
> @@ -76,7 +77,10 @@ public:
>
>  	std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
>
> +	/* Requests before queueing cio2 device. */
>  	std::queue<Request *> pendingRequests_;
> +	/* Requests queued in cio2 device and before passing imgu device. */
> +	std::queue<Request *> processingRequests_;
>
>  	ControlInfoMap ipaControls_;
>
> @@ -803,6 +807,7 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>
>  	ret |= data->imgu_->stop();
>  	ret |= data->cio2_.stop();
> +

Intentional ?

>  	if (ret)
>  		LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
>
> @@ -823,6 +828,8 @@ void IPU3CameraData::cancelPendingRequests()
>  		pipe()->completeRequest(request);
>  		pendingRequests_.pop();
>  	}
> +
> +	processingRequests_ = {};

Should processingRequests_ be completed one by one as the pending ones
instead of being dropped ?

>  }
>
>  void IPU3CameraData::queuePendingRequests()
> @@ -853,6 +860,8 @@ void IPU3CameraData::queuePendingRequests()
>
>  		info->rawBuffer = rawBuffer;
>
> +		processingRequests_.push(request);
> +
>  		ipa::ipu3::IPU3Event ev;
>  		ev.op = ipa::ipu3::EventProcessControls;
>  		ev.frame = info->id;
> @@ -1121,8 +1130,8 @@ int PipelineHandlerIPU3::registerCameras()
>  		data->delayedCtrls_ =
>  			std::make_unique<DelayedControls>(cio2->sensor()->device(),
>  							  params);
> -		data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
> -						 &DelayedControls::applyControls);
> +		data->cio2_.frameStart().connect(data.get(),
> +						 &IPU3CameraData::frameStart);
>
>  		/* Convert the sensor rotation to a transformation */
>  		int32_t rotation = 0;
> @@ -1414,6 +1423,38 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>  	ipa_->processEvent(ev);
>  }
>
> +void IPU3CameraData::frameStart(uint32_t sequence)

Maybe a small comment block on the function

/*
 * Handle the start of frame exposure signal.
 *
 * Inspect the list of pending requests waiting for a RAW frame to be
 * produced and apply controls for the 'next' one.
 *
 * Some controls need to be applied immediately, such as the
 * TestPatternMode one. Other controls are handled through the delayed
 * controls class.
 */

This shows that this functions does two things:
- Apply some libcamera::controls from the Request immediately
- Inform the DelayedControl class that a new frame has started to
  advance its internal frame-counting

> +{
> +	if (!processingRequests_.empty()) {
> +		/* Handle controls which are to be set ready for the next frame to start. */

                /* Apply controls that have to take effect immediately. */

I now wonder how did we get to the notion that some controls take
effect 'immediately'. Did we get here simply because we had to way to
apply libcamera::controls to CameraSensor without going through the
IPA ? I then wonder if talking about 'immediate' and 'next frame' is
not misleading. Aren't we just applying some controls which the
pipelinehandler is in charge of bypassing the IPA ?

do we need a notion of immediate controls ?

> +		Request *request = processingRequests_.front();
> +		processingRequests_.pop();
> +
> +		/* Assumes applying the test pattern mode affects immediately. */
> +		if (request->controls().contains(controls::draft::TestPatternMode)) {

		if (!request->controls().contains(controls::draft::TestPatternMode))
                        break;

And reduce the indendation below

> +			const int32_t testPatternMode = request->controls().get(
> +				controls::draft::TestPatternMode);
> +
> +			LOG(IPU3, Debug) << "Apply test pattern mode: "
> +					 << testPatternMode;
> +
> +			int ret = cio2_.sensor()->setTestPatternMode(
> +				static_cast<controls::draft::TestPatternModeEnum>(
> +					testPatternMode));
> +			if (ret) {
> +				LOG(IPU3, Error) << "Failed to set test pattern mode: "
> +						 << ret;
                                break;
And remove the else {} clause

> +			} else {
> +				request->metadata().set(controls::draft::TestPatternMode,
> +							testPatternMode);
> +			}
> +		}
> +	}
> +
> +	/* Controls that don't affect immediately are applied in delayedCtrls. */

I would drop this with the block at the beginning of the function

> +	delayedCtrls_->applyControls(sequence);

Minors apart

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
>
>  } /* namespace libcamera */
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>
Hirokazu Honda Nov. 26, 2021, 4:42 a.m. UTC | #2 
Hi Jacopo, thank you for reviewing.

On Thu, Nov 25, 2021 at 9:00 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Hiro,
>
> On Wed, Nov 24, 2021 at 04:08:12AM +0900, Hirokazu Honda wrote:
> > This enables ipu3 pipeline handler to apply a test pattern mode
> > per frame.
>
> Yes, and this also  introduces a a way to set controls
> immediately, the first (and only one for now) is the test pattern
> mode.
>
> Do you think it would be mentioned ?
>
> >
> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 45 ++++++++++++++++++++++++++--
> >  1 file changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 25490dcf..a15c6466 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -59,6 +59,7 @@ public:
> >       void statBufferReady(FrameBuffer *buffer);
> >       void queuePendingRequests();
> >       void cancelPendingRequests();
> > +     void frameStart(uint32_t sequence);
> >
> >       CIO2Device cio2_;
> >       ImgUDevice *imgu_;
> > @@ -76,7 +77,10 @@ public:
> >
> >       std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
> >
> > +     /* Requests before queueing cio2 device. */
> >       std::queue<Request *> pendingRequests_;
> > +     /* Requests queued in cio2 device and before passing imgu device. */
> > +     std::queue<Request *> processingRequests_;
> >
> >       ControlInfoMap ipaControls_;
> >
> > @@ -803,6 +807,7 @@ void PipelineHandlerIPU3::stop(Camera *camera)
> >
> >       ret |= data->imgu_->stop();
> >       ret |= data->cio2_.stop();
> > +
>
> Intentional ?
>
> >       if (ret)
> >               LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
> >
> > @@ -823,6 +828,8 @@ void IPU3CameraData::cancelPendingRequests()
> >               pipe()->completeRequest(request);
> >               pendingRequests_.pop();
> >       }
> > +
> > +     processingRequests_ = {};
>
> Should processingRequests_ be completed one by one as the pending ones
> instead of being dropped ?
>

Hmm, this is a good question.
I think the metadata for aborted capture doesn't have to be applied to a camera.
So I just discard pending requests.

> >  }
> >
> >  void IPU3CameraData::queuePendingRequests()
> > @@ -853,6 +860,8 @@ void IPU3CameraData::queuePendingRequests()
> >
> >               info->rawBuffer = rawBuffer;
> >
> > +             processingRequests_.push(request);
> > +
> >               ipa::ipu3::IPU3Event ev;
> >               ev.op = ipa::ipu3::EventProcessControls;
> >               ev.frame = info->id;
> > @@ -1121,8 +1130,8 @@ int PipelineHandlerIPU3::registerCameras()
> >               data->delayedCtrls_ =
> >                       std::make_unique<DelayedControls>(cio2->sensor()->device(),
> >                                                         params);
> > -             data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
> > -                                              &DelayedControls::applyControls);
> > +             data->cio2_.frameStart().connect(data.get(),
> > +                                              &IPU3CameraData::frameStart);
> >
> >               /* Convert the sensor rotation to a transformation */
> >               int32_t rotation = 0;
> > @@ -1414,6 +1423,38 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
> >       ipa_->processEvent(ev);
> >  }
> >
> > +void IPU3CameraData::frameStart(uint32_t sequence)
>
> Maybe a small comment block on the function
>
> /*
>  * Handle the start of frame exposure signal.
>  *
>  * Inspect the list of pending requests waiting for a RAW frame to be
>  * produced and apply controls for the 'next' one.
>  *
>  * Some controls need to be applied immediately, such as the
>  * TestPatternMode one. Other controls are handled through the delayed
>  * controls class.
>  */
>
> This shows that this functions does two things:
> - Apply some libcamera::controls from the Request immediately
> - Inform the DelayedControl class that a new frame has started to
>   advance its internal frame-counting
>
> > +{
> > +     if (!processingRequests_.empty()) {
> > +             /* Handle controls which are to be set ready for the next frame to start. */
>
>                 /* Apply controls that have to take effect immediately. */
>
> I now wonder how did we get to the notion that some controls take
> effect 'immediately'. Did we get here simply because we had to way to
> apply libcamera::controls to CameraSensor without going through the
> IPA ? I then wonder if talking about 'immediate' and 'next frame' is
> not misleading. Aren't we just applying some controls which the
> pipelinehandler is in charge of bypassing the IPA ?
>
> do we need a notion of immediate controls ?

It effects immediately that a control is applied to a capture which
associates the control.
I think the time of applying the control is dependent on camera hardware.


>
> > +             Request *request = processingRequests_.front();
> > +             processingRequests_.pop();
> > +
> > +             /* Assumes applying the test pattern mode affects immediately. */
> > +             if (request->controls().contains(controls::draft::TestPatternMode)) {
>
>                 if (!request->controls().contains(controls::draft::TestPatternMode))
>                         break;
>
> And reduce the indendation below
>
> > +                     const int32_t testPatternMode = request->controls().get(
> > +                             controls::draft::TestPatternMode);
> > +
> > +                     LOG(IPU3, Debug) << "Apply test pattern mode: "
> > +                                      << testPatternMode;
> > +
> > +                     int ret = cio2_.sensor()->setTestPatternMode(
> > +                             static_cast<controls::draft::TestPatternModeEnum>(
> > +                                     testPatternMode));
> > +                     if (ret) {
> > +                             LOG(IPU3, Error) << "Failed to set test pattern mode: "
> > +                                              << ret;
>                                 break;
> And remove the else {} clause
>

hmm, break cannot be used because this is neither for nor while.
I think the later delayedCtrls can be invoked in the begging of the
function and then can use "return"?

-Hiro
> > +                     } else {
> > +                             request->metadata().set(controls::draft::TestPatternMode,
> > +                                                     testPatternMode);
> > +                     }
> > +             }
> > +     }
> > +
> > +     /* Controls that don't affect immediately are applied in delayedCtrls. */
>
> I would drop this with the block at the beginning of the function
>
> > +     delayedCtrls_->applyControls(sequence);
>
> Minors apart
>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
>    j
>
> > +}
> > +
> >  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
> >
> >  } /* namespace libcamera */
> > --
> > 2.34.0.rc2.393.gf8c9666880-goog
> >
Jacopo Mondi Nov. 26, 2021, 8:27 a.m. UTC | #3 
Hi Hiro,

On Fri, Nov 26, 2021 at 01:42:05PM +0900, Hirokazu Honda wrote:
> Hi Jacopo, thank you for reviewing.
>
> On Thu, Nov 25, 2021 at 9:00 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > Hi Hiro,
> >
> > On Wed, Nov 24, 2021 at 04:08:12AM +0900, Hirokazu Honda wrote:
> > > This enables ipu3 pipeline handler to apply a test pattern mode
> > > per frame.
> >
> > Yes, and this also  introduces a a way to set controls
> > immediately, the first (and only one for now) is the test pattern
> > mode.
> >
> > Do you think it would be mentioned ?
> >
> > >
> > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> > > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > > ---
> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 45 ++++++++++++++++++++++++++--
> > >  1 file changed, 43 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 25490dcf..a15c6466 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -59,6 +59,7 @@ public:
> > >       void statBufferReady(FrameBuffer *buffer);
> > >       void queuePendingRequests();
> > >       void cancelPendingRequests();
> > > +     void frameStart(uint32_t sequence);
> > >
> > >       CIO2Device cio2_;
> > >       ImgUDevice *imgu_;
> > > @@ -76,7 +77,10 @@ public:
> > >
> > >       std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
> > >
> > > +     /* Requests before queueing cio2 device. */
> > >       std::queue<Request *> pendingRequests_;
> > > +     /* Requests queued in cio2 device and before passing imgu device. */
> > > +     std::queue<Request *> processingRequests_;
> > >
> > >       ControlInfoMap ipaControls_;
> > >
> > > @@ -803,6 +807,7 @@ void PipelineHandlerIPU3::stop(Camera *camera)
> > >
> > >       ret |= data->imgu_->stop();
> > >       ret |= data->cio2_.stop();
> > > +
> >
> > Intentional ?
> >
> > >       if (ret)
> > >               LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
> > >
> > > @@ -823,6 +828,8 @@ void IPU3CameraData::cancelPendingRequests()
> > >               pipe()->completeRequest(request);
> > >               pendingRequests_.pop();
> > >       }
> > > +
> > > +     processingRequests_ = {};
> >
> > Should processingRequests_ be completed one by one as the pending ones
> > instead of being dropped ?
> >
>
> Hmm, this is a good question.
> I think the metadata for aborted capture doesn't have to be applied to a camera.
> So I just discard pending requests.
>

So we'll lose requests when the Camera is stopped. Please have a look
at IPU3CameraData::cancelPendingRequests(). I think we'll have to do
the same for processingRequests_

> > >  }
> > >
> > >  void IPU3CameraData::queuePendingRequests()
> > > @@ -853,6 +860,8 @@ void IPU3CameraData::queuePendingRequests()
> > >
> > >               info->rawBuffer = rawBuffer;
> > >
> > > +             processingRequests_.push(request);
> > > +
> > >               ipa::ipu3::IPU3Event ev;
> > >               ev.op = ipa::ipu3::EventProcessControls;
> > >               ev.frame = info->id;
> > > @@ -1121,8 +1130,8 @@ int PipelineHandlerIPU3::registerCameras()
> > >               data->delayedCtrls_ =
> > >                       std::make_unique<DelayedControls>(cio2->sensor()->device(),
> > >                                                         params);
> > > -             data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
> > > -                                              &DelayedControls::applyControls);
> > > +             data->cio2_.frameStart().connect(data.get(),
> > > +                                              &IPU3CameraData::frameStart);
> > >
> > >               /* Convert the sensor rotation to a transformation */
> > >               int32_t rotation = 0;
> > > @@ -1414,6 +1423,38 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
> > >       ipa_->processEvent(ev);
> > >  }
> > >
> > > +void IPU3CameraData::frameStart(uint32_t sequence)
> >
> > Maybe a small comment block on the function
> >
> > /*
> >  * Handle the start of frame exposure signal.
> >  *
> >  * Inspect the list of pending requests waiting for a RAW frame to be
> >  * produced and apply controls for the 'next' one.
> >  *
> >  * Some controls need to be applied immediately, such as the
> >  * TestPatternMode one. Other controls are handled through the delayed
> >  * controls class.
> >  */
> >
> > This shows that this functions does two things:
> > - Apply some libcamera::controls from the Request immediately
> > - Inform the DelayedControl class that a new frame has started to
> >   advance its internal frame-counting
> >
> > > +{
> > > +     if (!processingRequests_.empty()) {
> > > +             /* Handle controls which are to be set ready for the next frame to start. */
> >
> >                 /* Apply controls that have to take effect immediately. */
> >
> > I now wonder how did we get to the notion that some controls take
> > effect 'immediately'. Did we get here simply because we had to way to
> > apply libcamera::controls to CameraSensor without going through the
> > IPA ? I then wonder if talking about 'immediate' and 'next frame' is
> > not misleading. Aren't we just applying some controls which the
> > pipelinehandler is in charge of bypassing the IPA ?
> >
> > do we need a notion of immediate controls ?
>
> It effects immediately that a control is applied to a capture which
> associates the control.
> I think the time of applying the control is dependent on camera hardware.
>
>
> >
> > > +             Request *request = processingRequests_.front();
> > > +             processingRequests_.pop();
> > > +
> > > +             /* Assumes applying the test pattern mode affects immediately. */
> > > +             if (request->controls().contains(controls::draft::TestPatternMode)) {
> >
> >                 if (!request->controls().contains(controls::draft::TestPatternMode))
> >                         break;
> >
> > And reduce the indendation below
> >
> > > +                     const int32_t testPatternMode = request->controls().get(
> > > +                             controls::draft::TestPatternMode);
> > > +
> > > +                     LOG(IPU3, Debug) << "Apply test pattern mode: "
> > > +                                      << testPatternMode;
> > > +
> > > +                     int ret = cio2_.sensor()->setTestPatternMode(
> > > +                             static_cast<controls::draft::TestPatternModeEnum>(
> > > +                                     testPatternMode));
> > > +                     if (ret) {
> > > +                             LOG(IPU3, Error) << "Failed to set test pattern mode: "
> > > +                                              << ret;
> >                                 break;
> > And remove the else {} clause
> >
>
> hmm, break cannot be used because this is neither for nor while.

Ah ups

> I think the later delayedCtrls can be invoked in the begging of the
> function and then can use "return"?

That would be nice, thanks

>
> -Hiro
> > > +                     } else {
> > > +                             request->metadata().set(controls::draft::TestPatternMode,
> > > +                                                     testPatternMode);
> > > +                     }
> > > +             }
> > > +     }
> > > +
> > > +     /* Controls that don't affect immediately are applied in delayedCtrls. */
> >
> > I would drop this with the block at the beginning of the function
> >
> > > +     delayedCtrls_->applyControls(sequence);
> >
> > Minors apart
> >
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> >
> > Thanks
> >    j
> >
> > > +}
> > > +
> > >  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
> > >
> > >  } /* namespace libcamera */
> > > --
> > > 2.34.0.rc2.393.gf8c9666880-goog
> > >

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 25490dcf..a15c6466 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -59,6 +59,7 @@  public:
 	void statBufferReady(FrameBuffer *buffer);
 	void queuePendingRequests();
 	void cancelPendingRequests();
+	void frameStart(uint32_t sequence);
 
 	CIO2Device cio2_;
 	ImgUDevice *imgu_;
@@ -76,7 +77,10 @@  public:
 
 	std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
 
+	/* Requests before queueing cio2 device. */
 	std::queue<Request *> pendingRequests_;
+	/* Requests queued in cio2 device and before passing imgu device. */
+	std::queue<Request *> processingRequests_;
 
 	ControlInfoMap ipaControls_;
 
@@ -803,6 +807,7 @@  void PipelineHandlerIPU3::stop(Camera *camera)
 
 	ret |= data->imgu_->stop();
 	ret |= data->cio2_.stop();
+
 	if (ret)
 		LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
 
@@ -823,6 +828,8 @@  void IPU3CameraData::cancelPendingRequests()
 		pipe()->completeRequest(request);
 		pendingRequests_.pop();
 	}
+
+	processingRequests_ = {};
 }
 
 void IPU3CameraData::queuePendingRequests()
@@ -853,6 +860,8 @@  void IPU3CameraData::queuePendingRequests()
 
 		info->rawBuffer = rawBuffer;
 
+		processingRequests_.push(request);
+
 		ipa::ipu3::IPU3Event ev;
 		ev.op = ipa::ipu3::EventProcessControls;
 		ev.frame = info->id;
@@ -1121,8 +1130,8 @@  int PipelineHandlerIPU3::registerCameras()
 		data->delayedCtrls_ =
 			std::make_unique<DelayedControls>(cio2->sensor()->device(),
 							  params);
-		data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
-						 &DelayedControls::applyControls);
+		data->cio2_.frameStart().connect(data.get(),
+						 &IPU3CameraData::frameStart);
 
 		/* Convert the sensor rotation to a transformation */
 		int32_t rotation = 0;
@@ -1414,6 +1423,38 @@  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
 	ipa_->processEvent(ev);
 }
 
+void IPU3CameraData::frameStart(uint32_t sequence)
+{
+	if (!processingRequests_.empty()) {
+		/* Handle controls which are to be set ready for the next frame to start. */
+		Request *request = processingRequests_.front();
+		processingRequests_.pop();
+
+		/* Assumes applying the test pattern mode affects immediately. */
+		if (request->controls().contains(controls::draft::TestPatternMode)) {
+			const int32_t testPatternMode = request->controls().get(
+				controls::draft::TestPatternMode);
+
+			LOG(IPU3, Debug) << "Apply test pattern mode: "
+					 << testPatternMode;
+
+			int ret = cio2_.sensor()->setTestPatternMode(
+				static_cast<controls::draft::TestPatternModeEnum>(
+					testPatternMode));
+			if (ret) {
+				LOG(IPU3, Error) << "Failed to set test pattern mode: "
+						 << ret;
+			} else {
+				request->metadata().set(controls::draft::TestPatternMode,
+							testPatternMode);
+			}
+		}
+	}
+
+	/* Controls that don't affect immediately are applied in delayedCtrls. */
+	delayedCtrls_->applyControls(sequence);
+}
+
 REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
 
 } /* namespace libcamera */