| Message ID | 20210804132526.162376-1-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, On 04/08/2021 14:25, Umang Jain wrote: > Ideally to capture the raw frames, ImgU should not be required. > However, we do need to configure the IPA since it shall setup > the sensor controls (exposure, vblank and so on) for the capture. > One cannot simply configure the IPA, without the ImgU as the > parameters and statistics buffer passed to the IPA are actually > managed by the ImgU. > > Until we prepare and setup the ImgU to run an internal queue for > raw-only camera configuration, disallow this configuration and > report it as invalid. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> This fixes lc-compliance at least, which helps unblock testing of Nicolas' series too. > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 76c3bb3d..9f6a6f21 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -248,6 +248,20 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() > if (rawCount > 1 || yuvCount > 2) { > LOG(IPU3, Debug) << "Camera configuration not supported"; > return Invalid; > + } else if (rawCount && !yuvCount) { > + /* > + * Disallow raw-only camera configuration. Currently, ImgU does > + * not get configured for raw-only streams and has early return > + * in configure(). To support raw-only stream, we do need the IPA > + * to get configured since it will setup the sensor controls for > + * the capture. > + * > + * \todo Configure the ImgU with internal buffers which will enable > + * the IPA to get configured, for the raw-only camera configuration. > + */ > + LOG(IPU3, Debug) > + << "Camera configuration cannot support raw-only streams"; > + return Invalid; > } > > /* >
Hi Umang, Thank you for the patch. On Wed, Aug 04, 2021 at 06:55:26PM +0530, Umang Jain wrote: > Ideally to capture the raw frames, ImgU should not be required. > However, we do need to configure the IPA since it shall setup I'd write "To capture raw frames, the ImgU isn't needed. However, to imlpement auto-exposure, we do need ...". There's nothing ideal to not using the ImgU, it's just not possible :-) > the sensor controls (exposure, vblank and so on) for the capture. > One cannot simply configure the IPA, without the ImgU as the > parameters and statistics buffer passed to the IPA are actually > managed by the ImgU. > > Until we prepare and setup the ImgU to run an internal queue for > raw-only camera configuration, disallow this configuration and > report it as invalid. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 76c3bb3d..9f6a6f21 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -248,6 +248,20 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() > if (rawCount > 1 || yuvCount > 2) { > LOG(IPU3, Debug) << "Camera configuration not supported"; > return Invalid; > + } else if (rawCount && !yuvCount) { > + /* > + * Disallow raw-only camera configuration. Currently, ImgU does > + * not get configured for raw-only streams and has early return > + * in configure(). To support raw-only stream, we do need the IPA > + * to get configured since it will setup the sensor controls for > + * the capture. > + * > + * \todo Configure the ImgU with internal buffers which will enable > + * the IPA to get configured, for the raw-only camera configuration. s/,// Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + LOG(IPU3, Debug) > + << "Camera configuration cannot support raw-only streams"; > + return Invalid; > } > > /*
Hi Laurent, On 8/9/21 5:01 AM, Laurent Pinchart wrote: > Hi Umang, > > Thank you for the patch. > > On Wed, Aug 04, 2021 at 06:55:26PM +0530, Umang Jain wrote: >> Ideally to capture the raw frames, ImgU should not be required. >> However, we do need to configure the IPA since it shall setup > I'd write "To capture raw frames, the ImgU isn't needed. However, to > imlpement auto-exposure, we do need ...". There's nothing ideal to not > using the ImgU, it's just not possible :-) haha okay, I'll fixup! while pushing this. > >> the sensor controls (exposure, vblank and so on) for the capture. >> One cannot simply configure the IPA, without the ImgU as the >> parameters and statistics buffer passed to the IPA are actually >> managed by the ImgU. >> >> Until we prepare and setup the ImgU to run an internal queue for >> raw-only camera configuration, disallow this configuration and >> report it as invalid. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> --- >> src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >> index 76c3bb3d..9f6a6f21 100644 >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >> @@ -248,6 +248,20 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() >> if (rawCount > 1 || yuvCount > 2) { >> LOG(IPU3, Debug) << "Camera configuration not supported"; >> return Invalid; >> + } else if (rawCount && !yuvCount) { >> + /* >> + * Disallow raw-only camera configuration. Currently, ImgU does >> + * not get configured for raw-only streams and has early return >> + * in configure(). To support raw-only stream, we do need the IPA >> + * to get configured since it will setup the sensor controls for >> + * the capture. >> + * >> + * \todo Configure the ImgU with internal buffers which will enable >> + * the IPA to get configured, for the raw-only camera configuration. > s/,// > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> + */ >> + LOG(IPU3, Debug) >> + << "Camera configuration cannot support raw-only streams"; >> + return Invalid; >> } >> >> /*
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 76c3bb3d..9f6a6f21 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -248,6 +248,20 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() if (rawCount > 1 || yuvCount > 2) { LOG(IPU3, Debug) << "Camera configuration not supported"; return Invalid; + } else if (rawCount && !yuvCount) { + /* + * Disallow raw-only camera configuration. Currently, ImgU does + * not get configured for raw-only streams and has early return + * in configure(). To support raw-only stream, we do need the IPA + * to get configured since it will setup the sensor controls for + * the capture. + * + * \todo Configure the ImgU with internal buffers which will enable + * the IPA to get configured, for the raw-only camera configuration. + */ + LOG(IPU3, Debug) + << "Camera configuration cannot support raw-only streams"; + return Invalid; } /*
Ideally to capture the raw frames, ImgU should not be required. However, we do need to configure the IPA since it shall setup the sensor controls (exposure, vblank and so on) for the capture. One cannot simply configure the IPA, without the ImgU as the parameters and statistics buffer passed to the IPA are actually managed by the ImgU. Until we prepare and setup the ImgU to run an internal queue for raw-only camera configuration, disallow this configuration and report it as invalid. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+)