| Message ID | 20190304232530.4427-3-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Mon, Mar 04, 2019 at 11:25:30PM +0000, Kieran Bingham wrote: > Before buffers can be released back to V4L2 all mappings must be unmapped. I don't think "released back to v4L2" is the right terminology. Maybe "Before buffers can be freed by the V4L2 device that has allocated them, all mappings must be destroyed" ? Same for the comment below. Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Ensure that buffers are released correctly by destroying the buffer pool > contents before freeing the resources on the stream. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/camera.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 8e17085bfb50..875e1e46c89c 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -488,8 +488,13 @@ int Camera::freeBuffers() > if (!stream->bufferPool().count()) > continue; > > - pipe_->freeBuffers(this, stream); > + /* > + * Destroy Buffers will empty the pool, and unmap any existing > + * memory mapping. This needs to be done before releasing > + * buffers back to the V4L2Device through freeBuffers(). > + */ > stream->bufferPool().destroyBuffers(); > + pipe_->freeBuffers(this, stream); > } > > state_ = CameraConfigured;
Hi Laurent, On 05/03/2019 12:54, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Mon, Mar 04, 2019 at 11:25:30PM +0000, Kieran Bingham wrote: >> Before buffers can be released back to V4L2 all mappings must be unmapped. > > I don't think "released back to v4L2" is the right terminology. Maybe > "Before buffers can be freed by the V4L2 device that has allocated them, > all mappings must be destroyed" ? Same for the comment below. Apart from > that, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Agreed, I'll reword as suggested and push with 1/2. -- Kieran > >> Ensure that buffers are released correctly by destroying the buffer pool >> contents before freeing the resources on the stream. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/libcamera/camera.cpp | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp >> index 8e17085bfb50..875e1e46c89c 100644 >> --- a/src/libcamera/camera.cpp >> +++ b/src/libcamera/camera.cpp >> @@ -488,8 +488,13 @@ int Camera::freeBuffers() >> if (!stream->bufferPool().count()) >> continue; >> >> - pipe_->freeBuffers(this, stream); >> + /* >> + * Destroy Buffers will empty the pool, and unmap any existing >> + * memory mapping. This needs to be done before releasing >> + * buffers back to the V4L2Device through freeBuffers(). >> + */ >> stream->bufferPool().destroyBuffers(); >> + pipe_->freeBuffers(this, stream); >> } >> >> state_ = CameraConfigured; >
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 8e17085bfb50..875e1e46c89c 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -488,8 +488,13 @@ int Camera::freeBuffers() if (!stream->bufferPool().count()) continue; - pipe_->freeBuffers(this, stream); + /* + * Destroy Buffers will empty the pool, and unmap any existing + * memory mapping. This needs to be done before releasing + * buffers back to the V4L2Device through freeBuffers(). + */ stream->bufferPool().destroyBuffers(); + pipe_->freeBuffers(this, stream); } state_ = CameraConfigured;
Before buffers can be released back to V4L2 all mappings must be unmapped. Ensure that buffers are released correctly by destroying the buffer pool contents before freeing the resources on the stream. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/libcamera/camera.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)