| Message ID | 20220111201102.58256-1-vedantparanjape160201@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Vedant, Thank you for the patch. On Wed, Jan 12, 2022 at 01:41:02AM +0530, Vedant Paranjape wrote: > Add support for PREPARE_BUF as one of the ioctl. Since this is a compat > layer, there doesn't seem to be an equivalent to the "transfer ownership > of the buffer to kernel driver" in V4L2Camera class. > > To match the error checks done by kernel implementation, we'd have to > check if dmabuf fd is valid and that the buffer size is large enough. > Doing so will not add any particular value to the program as > applications most likely don't depend on these conditions being > handled correctly. > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/v4l2/v4l2_camera_proxy.cpp | 32 ++++++++++++++++++++++++++++++++ > src/v4l2/v4l2_camera_proxy.h | 1 + > 2 files changed, 33 insertions(+) > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp > index 4d529bc29a4d..695af5be5c69 100644 > --- a/src/v4l2/v4l2_camera_proxy.cpp > +++ b/src/v4l2/v4l2_camera_proxy.cpp > @@ -544,6 +544,37 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a > return 0; > } > > +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg) > +{ > + LOG(V4L2Compat, Debug) << "Servicing vidioc_prepare_buf, index = " > + << arg->index << " fd = " << file->efd(); We can replace this with LOG(V4L2Compat, Debug) << "[" << file->description() << "] " << __func__ << "(index=" << arg->index << ")"; now that the V4L2 compat debugging improvements have been merged. > + > + if (!hasOwnership(file)) > + return -EBUSY; > + > + if (arg->index >= bufferCount_) > + return -EINVAL; > + > + if (arg->flags & V4L2_BUF_FLAG_REQUEST_FD) > + return -EINVAL; > + > + if (!validateBufferType(arg->type) || > + !validateMemoryType(arg->memory)) > + return -EINVAL; > + > + struct v4l2_buffer &buffer = buffers_[arg->index]; > + > + if (buffer.flags & V4L2_BUF_FLAG_QUEUED || > + buffer.flags & V4L2_BUF_FLAG_PREPARED) > + return -EINVAL; > + > + buffer.flags |= V4L2_BUF_FLAG_PREPARED; This looks good, but there's one missing thing: V4L2CameraProxy::vidioc_dqbuf() should now clear the V4L2_BUF_FLAG_PREPARED flag. Can you send a v3 that addresses those two small issues ? > + > + arg->flags = buffer.flags; > + > + return 0; > +} > + > int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) > { > LOG(V4L2Compat, Debug) << "Servicing vidioc_qbuf, index = " > @@ -709,6 +740,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = { > VIDIOC_S_INPUT, > VIDIOC_REQBUFS, > VIDIOC_QUERYBUF, > + VIDIOC_PREPARE_BUF, > VIDIOC_QBUF, > VIDIOC_DQBUF, > VIDIOC_EXPBUF, > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h > index 14e027c3e7d1..6baba94262a9 100644 > --- a/src/v4l2/v4l2_camera_proxy.h > +++ b/src/v4l2/v4l2_camera_proxy.h > @@ -57,6 +57,7 @@ private: > int vidioc_s_input(V4L2CameraFile *file, int *arg); > int vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg); > int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg); > + int vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg); > int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg); > int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg, > libcamera::Mutex *lock) LIBCAMERA_TSA_REQUIRES(*lock);
Sure, will do that. On Sat, 15 Jan, 2022, 21:57 Laurent Pinchart, < laurent.pinchart@ideasonboard.com> wrote: > Hi Vedant, > > Thank you for the patch. > > On Wed, Jan 12, 2022 at 01:41:02AM +0530, Vedant Paranjape wrote: > > Add support for PREPARE_BUF as one of the ioctl. Since this is a compat > > layer, there doesn't seem to be an equivalent to the "transfer ownership > > of the buffer to kernel driver" in V4L2Camera class. > > > > To match the error checks done by kernel implementation, we'd have to > > check if dmabuf fd is valid and that the buffer size is large enough. > > Doing so will not add any particular value to the program as > > applications most likely don't depend on these conditions being > > handled correctly. > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/v4l2/v4l2_camera_proxy.cpp | 32 ++++++++++++++++++++++++++++++++ > > src/v4l2/v4l2_camera_proxy.h | 1 + > > 2 files changed, 33 insertions(+) > > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp > b/src/v4l2/v4l2_camera_proxy.cpp > > index 4d529bc29a4d..695af5be5c69 100644 > > --- a/src/v4l2/v4l2_camera_proxy.cpp > > +++ b/src/v4l2/v4l2_camera_proxy.cpp > > @@ -544,6 +544,37 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile > *file, struct v4l2_buffer *a > > return 0; > > } > > > > +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct > v4l2_buffer *arg) > > +{ > > + LOG(V4L2Compat, Debug) << "Servicing vidioc_prepare_buf, index = " > > + << arg->index << " fd = " << file->efd(); > > We can replace this with > > LOG(V4L2Compat, Debug) > << "[" << file->description() << "] " << __func__ > << "(index=" << arg->index << ")"; > > now that the V4L2 compat debugging improvements have been merged. > > > + > > + if (!hasOwnership(file)) > > + return -EBUSY; > > + > > + if (arg->index >= bufferCount_) > > + return -EINVAL; > > + > > + if (arg->flags & V4L2_BUF_FLAG_REQUEST_FD) > > + return -EINVAL; > > + > > + if (!validateBufferType(arg->type) || > > + !validateMemoryType(arg->memory)) > > + return -EINVAL; > > + > > + struct v4l2_buffer &buffer = buffers_[arg->index]; > > + > > + if (buffer.flags & V4L2_BUF_FLAG_QUEUED || > > + buffer.flags & V4L2_BUF_FLAG_PREPARED) > > + return -EINVAL; > > + > > + buffer.flags |= V4L2_BUF_FLAG_PREPARED; > > This looks good, but there's one missing thing: > V4L2CameraProxy::vidioc_dqbuf() should now clear the > V4L2_BUF_FLAG_PREPARED flag. Can you send a v3 that addresses those two > small issues ? > > > + > > + arg->flags = buffer.flags; > > + > > + return 0; > > +} > > + > > int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct > v4l2_buffer *arg) > > { > > LOG(V4L2Compat, Debug) << "Servicing vidioc_qbuf, index = " > > @@ -709,6 +740,7 @@ const std::set<unsigned long> > V4L2CameraProxy::supportedIoctls_ = { > > VIDIOC_S_INPUT, > > VIDIOC_REQBUFS, > > VIDIOC_QUERYBUF, > > + VIDIOC_PREPARE_BUF, > > VIDIOC_QBUF, > > VIDIOC_DQBUF, > > VIDIOC_EXPBUF, > > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h > > index 14e027c3e7d1..6baba94262a9 100644 > > --- a/src/v4l2/v4l2_camera_proxy.h > > +++ b/src/v4l2/v4l2_camera_proxy.h > > @@ -57,6 +57,7 @@ private: > > int vidioc_s_input(V4L2CameraFile *file, int *arg); > > int vidioc_reqbufs(V4L2CameraFile *file, struct > v4l2_requestbuffers *arg); > > int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg); > > + int vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer > *arg); > > int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg); > > int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg, > > libcamera::Mutex *lock) > LIBCAMERA_TSA_REQUIRES(*lock); > > -- > Regards, > > Laurent Pinchart >
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index 4d529bc29a4d..695af5be5c69 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -544,6 +544,37 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a return 0; } +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg) +{ + LOG(V4L2Compat, Debug) << "Servicing vidioc_prepare_buf, index = " + << arg->index << " fd = " << file->efd(); + + if (!hasOwnership(file)) + return -EBUSY; + + if (arg->index >= bufferCount_) + return -EINVAL; + + if (arg->flags & V4L2_BUF_FLAG_REQUEST_FD) + return -EINVAL; + + if (!validateBufferType(arg->type) || + !validateMemoryType(arg->memory)) + return -EINVAL; + + struct v4l2_buffer &buffer = buffers_[arg->index]; + + if (buffer.flags & V4L2_BUF_FLAG_QUEUED || + buffer.flags & V4L2_BUF_FLAG_PREPARED) + return -EINVAL; + + buffer.flags |= V4L2_BUF_FLAG_PREPARED; + + arg->flags = buffer.flags; + + return 0; +} + int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg) { LOG(V4L2Compat, Debug) << "Servicing vidioc_qbuf, index = " @@ -709,6 +740,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = { VIDIOC_S_INPUT, VIDIOC_REQBUFS, VIDIOC_QUERYBUF, + VIDIOC_PREPARE_BUF, VIDIOC_QBUF, VIDIOC_DQBUF, VIDIOC_EXPBUF, diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h index 14e027c3e7d1..6baba94262a9 100644 --- a/src/v4l2/v4l2_camera_proxy.h +++ b/src/v4l2/v4l2_camera_proxy.h @@ -57,6 +57,7 @@ private: int vidioc_s_input(V4L2CameraFile *file, int *arg); int vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg); int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg); + int vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg); int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg); int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg, libcamera::Mutex *lock) LIBCAMERA_TSA_REQUIRES(*lock);