[libcamera-devel,3/4] pipeline: ipu3: Set bytesused before queuing parameters buffer
diff mbox series

Message ID 20221002003612.13603-4-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Fix kernel deprecation warning with output buffers
Related show

Commit Message

Laurent Pinchart Oct. 2, 2022, 12:36 a.m. UTC
The bytesused value for the parameters buffer is initialized to 0 and
never set. The V4L2 API specification indicates that, for an output
video device, the driver will set the bytesused value to the size of the
plane in that case. The videobuf2 framework does so, but considers this
as deprecated and prints a warning:

[   54.375534] use of bytesused == 0 is deprecated and will be removed in the future,
[   54.388026] use the actual size instead.

Fix it by setting bytesused to the correct value before queuing the
parameters buffer.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jacopo Mondi Oct. 3, 2022, 8:35 a.m. UTC | #1
Hi Laurent

On Sun, Oct 02, 2022 at 03:36:11AM +0300, Laurent Pinchart via libcamera-devel wrote:
> The bytesused value for the parameters buffer is initialized to 0 and
> never set. The V4L2 API specification indicates that, for an output
> video device, the driver will set the bytesused value to the size of the
> plane in that case. The videobuf2 framework does so, but considers this
> as deprecated and prints a warning:
>
> [   54.375534] use of bytesused == 0 is deprecated and will be removed in the future,
> [   54.388026] use the actual size instead.
>
> Fix it by setting bytesused to the correct value before queuing the
> parameters buffer.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 93219a6c1134..3b892d9671c5 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -11,6 +11,8 @@
>  #include <queue>
>  #include <vector>
>
> +#include <linux/intel-ipu3.h>
> +
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/utils.h>
>
> @@ -1298,6 +1300,8 @@ void IPU3CameraData::paramsBufferReady(unsigned int id)
>  			imgu_->viewfinder_->queueBuffer(outbuffer);
>  	}
>
> +	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
> +		sizeof(struct ipu3_uapi_params);

Do you think we need and helper ?

        FrameBuffer::Private::setBytesUsed(unsigned int plane,
                                           unsigned int bytesused) ?

>  	imgu_->param_->queueBuffer(info->paramBuffer);
>  	imgu_->stat_->queueBuffer(info->statBuffer);
>  	imgu_->input_->queueBuffer(info->rawBuffer);
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart Oct. 3, 2022, 12:24 p.m. UTC | #2
Hi Jacopo,

On Mon, Oct 03, 2022 at 10:35:53AM +0200, Jacopo Mondi wrote:
> On Sun, Oct 02, 2022 at 03:36:11AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > The bytesused value for the parameters buffer is initialized to 0 and
> > never set. The V4L2 API specification indicates that, for an output
> > video device, the driver will set the bytesused value to the size of the
> > plane in that case. The videobuf2 framework does so, but considers this
> > as deprecated and prints a warning:
> >
> > [   54.375534] use of bytesused == 0 is deprecated and will be removed in the future,
> > [   54.388026] use the actual size instead.
> >
> > Fix it by setting bytesused to the correct value before queuing the
> > parameters buffer.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 93219a6c1134..3b892d9671c5 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -11,6 +11,8 @@
> >  #include <queue>
> >  #include <vector>
> >
> > +#include <linux/intel-ipu3.h>
> > +
> >  #include <libcamera/base/log.h>
> >  #include <libcamera/base/utils.h>
> >
> > @@ -1298,6 +1300,8 @@ void IPU3CameraData::paramsBufferReady(unsigned int id)
> >  			imgu_->viewfinder_->queueBuffer(outbuffer);
> >  	}
> >
> > +	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
> > +		sizeof(struct ipu3_uapi_params);
> 
> Do you think we need and helper ?
> 
>         FrameBuffer::Private::setBytesUsed(unsigned int plane,
>                                            unsigned int bytesused) ?

I don't really see what this would bring.

> >  	imgu_->param_->queueBuffer(info->paramBuffer);
> >  	imgu_->stat_->queueBuffer(info->statBuffer);
> >  	imgu_->input_->queueBuffer(info->rawBuffer);
Jacopo Mondi Oct. 3, 2022, 12:49 p.m. UTC | #3
On Mon, Oct 03, 2022 at 03:24:57PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Oct 03, 2022 at 10:35:53AM +0200, Jacopo Mondi wrote:
> > On Sun, Oct 02, 2022 at 03:36:11AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > The bytesused value for the parameters buffer is initialized to 0 and
> > > never set. The V4L2 API specification indicates that, for an output
> > > video device, the driver will set the bytesused value to the size of the
> > > plane in that case. The videobuf2 framework does so, but considers this
> > > as deprecated and prints a warning:
> > >
> > > [   54.375534] use of bytesused == 0 is deprecated and will be removed in the future,
> > > [   54.388026] use the actual size instead.
> > >
> > > Fix it by setting bytesused to the correct value before queuing the
> > > parameters buffer.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 93219a6c1134..3b892d9671c5 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -11,6 +11,8 @@
> > >  #include <queue>
> > >  #include <vector>
> > >
> > > +#include <linux/intel-ipu3.h>
> > > +
> > >  #include <libcamera/base/log.h>
> > >  #include <libcamera/base/utils.h>
> > >
> > > @@ -1298,6 +1300,8 @@ void IPU3CameraData::paramsBufferReady(unsigned int id)
> > >  			imgu_->viewfinder_->queueBuffer(outbuffer);
> > >  	}
> > >
> > > +	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
> > > +		sizeof(struct ipu3_uapi_params);
> >
> > Do you think we need and helper ?
> >
> >         FrameBuffer::Private::setBytesUsed(unsigned int plane,
> >                                            unsigned int bytesused) ?
>
> I don't really see what this would bring.

Less indirections for the callers ? If it's a repeated patter it might
be nice to have.

Not a strict requirement for this series if you think it's not worth
it.

For this and the RkISP equivalent
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j


>
> > >  	imgu_->param_->queueBuffer(info->paramBuffer);
> > >  	imgu_->stat_->queueBuffer(info->statBuffer);
> > >  	imgu_->input_->queueBuffer(info->rawBuffer);
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 93219a6c1134..3b892d9671c5 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -11,6 +11,8 @@ 
 #include <queue>
 #include <vector>
 
+#include <linux/intel-ipu3.h>
+
 #include <libcamera/base/log.h>
 #include <libcamera/base/utils.h>
 
@@ -1298,6 +1300,8 @@  void IPU3CameraData::paramsBufferReady(unsigned int id)
 			imgu_->viewfinder_->queueBuffer(outbuffer);
 	}
 
+	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
+		sizeof(struct ipu3_uapi_params);
 	imgu_->param_->queueBuffer(info->paramBuffer);
 	imgu_->stat_->queueBuffer(info->statBuffer);
 	imgu_->input_->queueBuffer(info->rawBuffer);