| Message ID | 20210209145244.25408-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel On Tue, Feb 09, 2021 at 03:52:44PM +0100, Jean-Michel Hautbois wrote: > In order to get the stats back, the imgu subdev needs to have the > V4L2_CID_INTEL_IPU3_MODE control set. > Set it to video mode by default to get the stats at each frame. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 61f7bf43..104cb532 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -552,8 +552,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > /* Apply the "pipe_mode" control to the ImgU subdevice. */ > ControlList ctrls(imgu->imgu_->controls()); > ctrls.set(V4L2_CID_IPU3_PIPE_MODE, > - static_cast<int32_t>(vfCfg ? IPU3PipeModeVideo : > - IPU3PipeModeStillCapture)); > + static_cast<int32_t>(IPU3PipeModeVideo)); I'm not opposed to this, and know we got to know what effect the pipe mode has, at least on statistics generation. I wonder what other features the StillCapture mode provides. Could you record with a todo that the usage of the pipe mode has to be clarified ? Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > ret = imgu->imgu_->setControls(&ctrls); > if (ret) { > LOG(IPU3, Error) << "Unable to set pipe_mode control"; > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo ! On 10/02/2021 18:43, Jacopo Mondi wrote: > Hi Jean-Michel > > On Tue, Feb 09, 2021 at 03:52:44PM +0100, Jean-Michel Hautbois wrote: >> In order to get the stats back, the imgu subdev needs to have the >> V4L2_CID_INTEL_IPU3_MODE control set. >> Set it to video mode by default to get the stats at each frame. >> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> --- >> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >> index 61f7bf43..104cb532 100644 >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >> @@ -552,8 +552,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) >> /* Apply the "pipe_mode" control to the ImgU subdevice. */ >> ControlList ctrls(imgu->imgu_->controls()); >> ctrls.set(V4L2_CID_IPU3_PIPE_MODE, >> - static_cast<int32_t>(vfCfg ? IPU3PipeModeVideo : >> - IPU3PipeModeStillCapture)); >> + static_cast<int32_t>(IPU3PipeModeVideo)); > > I'm not opposed to this, and know we got to know what effect the pipe > mode has, at least on statistics generation. > > I wonder what other features the StillCapture mode provides. > Could you record with a todo that the usage of the pipe mode has to be > clarified ? I don't know if we can know more than what is already in the kernel documentation ? https://linuxtv.org/downloads/v4l-dvb-apis/admin-guide/ipu3.html#running-mode-and-firmware-binary-selection I can add a TODO, in a v2 ? > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks for that ;-). > > Thanks > j >> ret = imgu->imgu_->setControls(&ctrls); >> if (ret) { >> LOG(IPU3, Error) << "Unable to set pipe_mode control"; >> -- >> 2.27.0 >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel >
Hi Jean-Michel On Wed, Feb 10, 2021 at 08:48:15PM +0100, Jean-Michel Hautbois wrote: > Hi Jacopo ! > > On 10/02/2021 18:43, Jacopo Mondi wrote: > > Hi Jean-Michel > > > > On Tue, Feb 09, 2021 at 03:52:44PM +0100, Jean-Michel Hautbois wrote: > >> In order to get the stats back, the imgu subdev needs to have the > >> V4L2_CID_INTEL_IPU3_MODE control set. > >> Set it to video mode by default to get the stats at each frame. > >> > >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > >> --- > >> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > >> index 61f7bf43..104cb532 100644 > >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > >> @@ -552,8 +552,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > >> /* Apply the "pipe_mode" control to the ImgU subdevice. */ > >> ControlList ctrls(imgu->imgu_->controls()); > >> ctrls.set(V4L2_CID_IPU3_PIPE_MODE, > >> - static_cast<int32_t>(vfCfg ? IPU3PipeModeVideo : > >> - IPU3PipeModeStillCapture)); > >> + static_cast<int32_t>(IPU3PipeModeVideo)); > > > > I'm not opposed to this, and know we got to know what effect the pipe > > mode has, at least on statistics generation. > > > > I wonder what other features the StillCapture mode provides. > > Could you record with a todo that the usage of the pipe mode has to be > > clarified ? > > I don't know if we can know more than what is already in the kernel > documentation ? > https://linuxtv.org/downloads/v4l-dvb-apis/admin-guide/ipu3.html#running-mode-and-firmware-binary-selection > > I can add a TODO, in a v2 ? Yes please, I still think we might want to use the StilCapture mode for, no surprise, still capture of raw frames, which, if I'm not mistaken should not involve the IPA and for which we don't need statistics. If my understanding is correct, something along the lines of: /* * \todo Set the ImgU pipe mode to 'Video' unconditionally to have statistics * generated and re-consider if 'Still Capture' should be used for * high quality RAW capture operations that do not involve the IPA. */ ? Thanks j > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > Thanks for that ;-). > > > > > Thanks > > j > >> ret = imgu->imgu_->setControls(&ctrls); > >> if (ret) { > >> LOG(IPU3, Error) << "Unable to set pipe_mode control"; > >> -- > >> 2.27.0 > >> > >> _______________________________________________ > >> libcamera-devel mailing list > >> libcamera-devel@lists.libcamera.org > >> https://lists.libcamera.org/listinfo/libcamera-devel > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > >
Hi Jacopo, On 12/02/2021 13:54, Jacopo Mondi wrote: > Hi Jean-Michel > > On Wed, Feb 10, 2021 at 08:48:15PM +0100, Jean-Michel Hautbois wrote: >> Hi Jacopo ! >> >> On 10/02/2021 18:43, Jacopo Mondi wrote: >>> Hi Jean-Michel >>> >>> On Tue, Feb 09, 2021 at 03:52:44PM +0100, Jean-Michel Hautbois wrote: >>>> In order to get the stats back, the imgu subdev needs to have the >>>> V4L2_CID_INTEL_IPU3_MODE control set. >>>> Set it to video mode by default to get the stats at each frame. >>>> >>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >>>> --- >>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> >>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >>>> index 61f7bf43..104cb532 100644 >>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >>>> @@ -552,8 +552,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) >>>> /* Apply the "pipe_mode" control to the ImgU subdevice. */ >>>> ControlList ctrls(imgu->imgu_->controls()); >>>> ctrls.set(V4L2_CID_IPU3_PIPE_MODE, >>>> - static_cast<int32_t>(vfCfg ? IPU3PipeModeVideo : >>>> - IPU3PipeModeStillCapture)); >>>> + static_cast<int32_t>(IPU3PipeModeVideo)); >>> >>> I'm not opposed to this, and know we got to know what effect the pipe >>> mode has, at least on statistics generation. >>> >>> I wonder what other features the StillCapture mode provides. >>> Could you record with a todo that the usage of the pipe mode has to be >>> clarified ? >> >> I don't know if we can know more than what is already in the kernel >> documentation ? >> https://linuxtv.org/downloads/v4l-dvb-apis/admin-guide/ipu3.html#running-mode-and-firmware-binary-selection >> >> I can add a TODO, in a v2 ? > > Yes please, I still think we might want to use the StilCapture mode > for, no surprise, still capture of raw frames, which, if I'm not > mistaken should not involve the IPA and for which we don't need > statistics. > > If my understanding is correct, something along the lines of: > /* > * \todo Set the ImgU pipe mode to 'Video' unconditionally to have statistics > * generated and re-consider if 'Still Capture' should be used for > * high quality RAW capture operations that do not involve the IPA. > */ Sounds great, v2 in a short time :-) Thanks ! JM
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 61f7bf43..104cb532 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -552,8 +552,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) /* Apply the "pipe_mode" control to the ImgU subdevice. */ ControlList ctrls(imgu->imgu_->controls()); ctrls.set(V4L2_CID_IPU3_PIPE_MODE, - static_cast<int32_t>(vfCfg ? IPU3PipeModeVideo : - IPU3PipeModeStillCapture)); + static_cast<int32_t>(IPU3PipeModeVideo)); ret = imgu->imgu_->setControls(&ctrls); if (ret) { LOG(IPU3, Error) << "Unable to set pipe_mode control";
In order to get the stats back, the imgu subdev needs to have the V4L2_CID_INTEL_IPU3_MODE control set. Set it to video mode by default to get the stats at each frame. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)