[libcamera-devel,15/15] libcamera: ipu3: Pass cio2 fourcc to ImgUDevice::configure()
diff mbox series

Message ID 20200701123036.51922-16-jacopo@jmondi.org
State Superseded, archived
Delegated to: Jacopo Mondi
Headers show
Series
  • libcamera: ipu3: Rework streams configuration
Related show

Commit Message

Jacopo Mondi July 1, 2020, 12:30 p.m. UTC 
Only the CIO2 4cc code is required for ImgUDevice::configure(), as the
input frame sizes are now reported through the ImgUDevice::Pipe
structure.

Add the CIO2 pixel format to ImguDevice::Pipe instead of passing the
whole CIO2 V4L2DeviceFormat to ImgUDevice::configure().

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/ipu3/imgu.cpp | 19 ++++++++++++++-----
 src/libcamera/pipeline/ipu3/imgu.h   |  3 ++-
 src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
 3 files changed, 18 insertions(+), 7 deletions(-)

Comments

Niklas Söderlund July 1, 2020, 4:56 p.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

On 2020-07-01 14:30:36 +0200, Jacopo Mondi wrote:
> Only the CIO2 4cc code is required for ImgUDevice::configure(), as the
> input frame sizes are now reported through the ImgUDevice::Pipe
> structure.
> 
> Add the CIO2 pixel format to ImguDevice::Pipe instead of passing the
> whole CIO2 V4L2DeviceFormat to ImgUDevice::configure().
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/imgu.cpp | 19 ++++++++++++++-----
>  src/libcamera/pipeline/ipu3/imgu.h   |  3 ++-
>  src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
>  3 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index f580c1d2ff6c..59e63644d22f 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -26,11 +26,16 @@ LOG_DECLARE_CATEGORY(IPU3)
>   * The ImgU unit processes images through several components, which have
>   * to be properly configured inspecting the input image size and the desired
>   * output sizes. This structure collects the ImgU input configuration and the
> - * requested main output and viewfinder configurations.
> + * requested main output and viewfinder configurations, along with the
> + * pixel format to be applied to the ImgU input, which is the same as the
> + * one used by the CIO2 unit.
>   *
>   * \var Pipe::input
>   * \brief The input image size
>   *
> + * \var Pipe::cio2Fourcc
> + * \brief The CIO2 pixel format to be applied to ImgU input
> + *
>   * \var Pipe::output
>   * \brief The requested main output size
>   *
> @@ -96,17 +101,21 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
>  /**
>   * \brief Configure the ImgU unit input
>   * \param[in] pipe The ImgU pipe configuration
> - * \param[in] inputFormat The format to be applied to ImgU input
>   * \return 0 on success or a negative error code otherwise
>   */
> -int ImgUDevice::configure(struct Pipe *pipe, V4L2DeviceFormat *inputFormat)
> +int ImgUDevice::configure(struct Pipe *pipe)
>  {
>  	/* Configure the ImgU input video device with the requested sizes. */
> -	int ret = input_->setFormat(inputFormat);
> +	V4L2DeviceFormat inputFormat = {
> +		.fourcc = pipe->cio2Fourcc,
> +		.size = pipe->input,
> +		.planesCount = 1,
> +	};
> +	int ret = input_->setFormat(&inputFormat);
>  	if (ret)
>  		return ret;
>  
> -	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat->toString();
> +	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat.toString();
>  
>  	/*
>  	 * \todo The IPU3 driver implementation shall be changed to use the
> diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
> index 45c5cc1ce8e6..b0ed228756c7 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.h
> +++ b/src/libcamera/pipeline/ipu3/imgu.h
> @@ -25,13 +25,14 @@ class ImgUDevice
>  public:
>  	struct Pipe {
>  		Size input;
> +		V4L2PixelFormat cio2Fourcc;

Would it make sens to name this inputFourcc ?

>  		Size output;
>  		Size viewfinder;
>  	};
>  
>  	int init(MediaDevice *media, unsigned int index);
>  
> -	int configure(struct Pipe *pipe, V4L2DeviceFormat *inputFormat);
> +	int configure(struct Pipe *pipe);
>  
>  	int configureOutput(const StreamConfiguration &cfg,
>  			    V4L2DeviceFormat *outputFormat)
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 317e78155ebb..79ec492341b2 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -410,6 +410,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  	ImgUDevice::Pipe imguPipe{};
>  	imguPipe.input.width = cio2Format.size.width;
>  	imguPipe.input.height = cio2Format.size.height;
> +	imguPipe.cio2Fourcc = cio2Format.fourcc;
>  
>  	for (unsigned int i = 0; i < config->size(); ++i) {
>  		StreamConfiguration &cfg = (*config)[i];
> @@ -459,7 +460,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  	 * Configure the ImgU with the collected pipe configuration and the
>  	 * CIO2 unit format.
>  	 */
> -	ret = imgu->configure(&imguPipe, &cio2Format);
> +	ret = imgu->configure(&imguPipe);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi July 2, 2020, 7:36 a.m. UTC | #2 
Hi Niklas,

On Wed, Jul 01, 2020 at 06:56:41PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-07-01 14:30:36 +0200, Jacopo Mondi wrote:
> > Only the CIO2 4cc code is required for ImgUDevice::configure(), as the
> > input frame sizes are now reported through the ImgUDevice::Pipe
> > structure.
> >
> > Add the CIO2 pixel format to ImguDevice::Pipe instead of passing the
> > whole CIO2 V4L2DeviceFormat to ImgUDevice::configure().
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/pipeline/ipu3/imgu.cpp | 19 ++++++++++++++-----
> >  src/libcamera/pipeline/ipu3/imgu.h   |  3 ++-
> >  src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
> >  3 files changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> > index f580c1d2ff6c..59e63644d22f 100644
> > --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> > @@ -26,11 +26,16 @@ LOG_DECLARE_CATEGORY(IPU3)
> >   * The ImgU unit processes images through several components, which have
> >   * to be properly configured inspecting the input image size and the desired
> >   * output sizes. This structure collects the ImgU input configuration and the
> > - * requested main output and viewfinder configurations.
> > + * requested main output and viewfinder configurations, along with the
> > + * pixel format to be applied to the ImgU input, which is the same as the
> > + * one used by the CIO2 unit.
> >   *
> >   * \var Pipe::input
> >   * \brief The input image size
> >   *
> > + * \var Pipe::cio2Fourcc
> > + * \brief The CIO2 pixel format to be applied to ImgU input
> > + *
> >   * \var Pipe::output
> >   * \brief The requested main output size
> >   *
> > @@ -96,17 +101,21 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
> >  /**
> >   * \brief Configure the ImgU unit input
> >   * \param[in] pipe The ImgU pipe configuration
> > - * \param[in] inputFormat The format to be applied to ImgU input
> >   * \return 0 on success or a negative error code otherwise
> >   */
> > -int ImgUDevice::configure(struct Pipe *pipe, V4L2DeviceFormat *inputFormat)
> > +int ImgUDevice::configure(struct Pipe *pipe)
> >  {
> >  	/* Configure the ImgU input video device with the requested sizes. */
> > -	int ret = input_->setFormat(inputFormat);
> > +	V4L2DeviceFormat inputFormat = {
> > +		.fourcc = pipe->cio2Fourcc,
> > +		.size = pipe->input,
> > +		.planesCount = 1,
> > +	};
> > +	int ret = input_->setFormat(&inputFormat);
> >  	if (ret)
> >  		return ret;
> >
> > -	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat->toString();
> > +	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat.toString();
> >
> >  	/*
> >  	 * \todo The IPU3 driver implementation shall be changed to use the
> > diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
> > index 45c5cc1ce8e6..b0ed228756c7 100644
> > --- a/src/libcamera/pipeline/ipu3/imgu.h
> > +++ b/src/libcamera/pipeline/ipu3/imgu.h
> > @@ -25,13 +25,14 @@ class ImgUDevice
> >  public:
> >  	struct Pipe {
> >  		Size input;
> > +		V4L2PixelFormat cio2Fourcc;
>
> Would it make sens to name this inputFourcc ?
>

To pair it with "Size input" ? I really think so, yes!

> >  		Size output;
> >  		Size viewfinder;
> >  	};
> >
> >  	int init(MediaDevice *media, unsigned int index);
> >
> > -	int configure(struct Pipe *pipe, V4L2DeviceFormat *inputFormat);
> > +	int configure(struct Pipe *pipe);
> >
> >  	int configureOutput(const StreamConfiguration &cfg,
> >  			    V4L2DeviceFormat *outputFormat)
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 317e78155ebb..79ec492341b2 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -410,6 +410,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >  	ImgUDevice::Pipe imguPipe{};
> >  	imguPipe.input.width = cio2Format.size.width;
> >  	imguPipe.input.height = cio2Format.size.height;
> > +	imguPipe.cio2Fourcc = cio2Format.fourcc;
> >
> >  	for (unsigned int i = 0; i < config->size(); ++i) {
> >  		StreamConfiguration &cfg = (*config)[i];
> > @@ -459,7 +460,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >  	 * Configure the ImgU with the collected pipe configuration and the
> >  	 * CIO2 unit format.
> >  	 */
> > -	ret = imgu->configure(&imguPipe, &cio2Format);
> > +	ret = imgu->configure(&imguPipe);
> >  	if (ret)
> >  		return ret;
> >
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index f580c1d2ff6c..59e63644d22f 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -26,11 +26,16 @@  LOG_DECLARE_CATEGORY(IPU3)
  * The ImgU unit processes images through several components, which have
  * to be properly configured inspecting the input image size and the desired
  * output sizes. This structure collects the ImgU input configuration and the
- * requested main output and viewfinder configurations.
+ * requested main output and viewfinder configurations, along with the
+ * pixel format to be applied to the ImgU input, which is the same as the
+ * one used by the CIO2 unit.
  *
  * \var Pipe::input
  * \brief The input image size
  *
+ * \var Pipe::cio2Fourcc
+ * \brief The CIO2 pixel format to be applied to ImgU input
+ *
  * \var Pipe::output
  * \brief The requested main output size
  *
@@ -96,17 +101,21 @@  int ImgUDevice::init(MediaDevice *media, unsigned int index)
 /**
  * \brief Configure the ImgU unit input
  * \param[in] pipe The ImgU pipe configuration
- * \param[in] inputFormat The format to be applied to ImgU input
  * \return 0 on success or a negative error code otherwise
  */
-int ImgUDevice::configure(struct Pipe *pipe, V4L2DeviceFormat *inputFormat)
+int ImgUDevice::configure(struct Pipe *pipe)
 {
 	/* Configure the ImgU input video device with the requested sizes. */
-	int ret = input_->setFormat(inputFormat);
+	V4L2DeviceFormat inputFormat = {
+		.fourcc = pipe->cio2Fourcc,
+		.size = pipe->input,
+		.planesCount = 1,
+	};
+	int ret = input_->setFormat(&inputFormat);
 	if (ret)
 		return ret;
 
-	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat->toString();
+	LOG(IPU3, Debug) << "ImgU input format = " << inputFormat.toString();
 
 	/*
 	 * \todo The IPU3 driver implementation shall be changed to use the
diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
index 45c5cc1ce8e6..b0ed228756c7 100644
--- a/src/libcamera/pipeline/ipu3/imgu.h
+++ b/src/libcamera/pipeline/ipu3/imgu.h
@@ -25,13 +25,14 @@  class ImgUDevice
 public:
 	struct Pipe {
 		Size input;
+		V4L2PixelFormat cio2Fourcc;
 		Size output;
 		Size viewfinder;
 	};
 
 	int init(MediaDevice *media, unsigned int index);
 
-	int configure(struct Pipe *pipe, V4L2DeviceFormat *inputFormat);
+	int configure(struct Pipe *pipe);
 
 	int configureOutput(const StreamConfiguration &cfg,
 			    V4L2DeviceFormat *outputFormat)
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 317e78155ebb..79ec492341b2 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -410,6 +410,7 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	ImgUDevice::Pipe imguPipe{};
 	imguPipe.input.width = cio2Format.size.width;
 	imguPipe.input.height = cio2Format.size.height;
+	imguPipe.cio2Fourcc = cio2Format.fourcc;
 
 	for (unsigned int i = 0; i < config->size(); ++i) {
 		StreamConfiguration &cfg = (*config)[i];
@@ -459,7 +460,7 @@  int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
 	 * Configure the ImgU with the collected pipe configuration and the
 	 * CIO2 unit format.
 	 */
-	ret = imgu->configure(&imguPipe, &cio2Format);
+	ret = imgu->configure(&imguPipe);
 	if (ret)
 		return ret;