From: Yunke Cao <yunkec@chromium.org>
To: Tomasz Figa <tfiga@chromium.org>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org, Yunke Cao <yunkec@chromium.org>
Subject: [PATCH v2 1/3] media: videobuf2-core: release all planes first in __prepare_dmabuf()
Date: Wed, 3 Apr 2024 18:13:04 +0900 [thread overview]
Message-ID: <20240403091306.1308878-2-yunkec@chromium.org> (raw)
In-Reply-To: <20240403091306.1308878-1-yunkec@chromium.org>
The existing implementation, validating planes, checking if the planes
changed, releasing previous planes and reaquiring new planes all happens in
the same for loop.
Split the for loop into 3 parts
1. In the first for loop, validate planes and check if planes changed.
2. Call __vb2_buf_dmabuf_put() to release all planes.
3. In the second for loop, reaquire new planes.
Signed-off-by: Yunke Cao <yunkec@chromium.org>
---
.../media/common/videobuf2/videobuf2-core.c | 64 ++++++++++---------
1 file changed, 34 insertions(+), 30 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index b6bf8f232f48..702f7b6f783a 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1341,11 +1341,13 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
for (plane = 0; plane < vb->num_planes; ++plane) {
struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
+ planes[plane].dbuf = dbuf;
+
if (IS_ERR_OR_NULL(dbuf)) {
dprintk(q, 1, "invalid dmabuf fd for plane %d\n",
plane);
ret = -EINVAL;
- goto err;
+ goto err_put_dbuf;
}
/* use DMABUF size if length is not provided */
@@ -1356,17 +1358,14 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
dprintk(q, 1, "invalid dmabuf length %u for plane %d, minimum length %u\n",
planes[plane].length, plane,
vb->planes[plane].min_length);
- dma_buf_put(dbuf);
ret = -EINVAL;
- goto err;
+ goto err_put_dbuf;
}
/* Skip the plane if already verified */
if (dbuf == vb->planes[plane].dbuf &&
- vb->planes[plane].length == planes[plane].length) {
- dma_buf_put(dbuf);
+ vb->planes[plane].length == planes[plane].length)
continue;
- }
dprintk(q, 3, "buffer for plane %d changed\n", plane);
@@ -1375,29 +1374,30 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
vb->copied_timestamp = 0;
call_void_vb_qop(vb, buf_cleanup, vb);
}
+ }
- /* Release previously acquired memory if present */
- __vb2_plane_dmabuf_put(vb, &vb->planes[plane]);
- vb->planes[plane].bytesused = 0;
- vb->planes[plane].length = 0;
- vb->planes[plane].m.fd = 0;
- vb->planes[plane].data_offset = 0;
+ if (reacquired) {
+ __vb2_buf_dmabuf_put(vb);
+
+ for (plane = 0; plane < vb->num_planes; ++plane) {
+ /* Acquire each plane's memory */
+ mem_priv = call_ptr_memop(attach_dmabuf,
+ vb,
+ q->alloc_devs[plane] ? : q->dev,
+ planes[plane].dbuf,
+ planes[plane].length);
+ if (IS_ERR(mem_priv)) {
+ dprintk(q, 1, "failed to attach dmabuf\n");
+ ret = PTR_ERR(mem_priv);
+ goto err_put_dbuf;
+ }
- /* Acquire each plane's memory */
- mem_priv = call_ptr_memop(attach_dmabuf,
- vb,
- q->alloc_devs[plane] ? : q->dev,
- dbuf,
- planes[plane].length);
- if (IS_ERR(mem_priv)) {
- dprintk(q, 1, "failed to attach dmabuf\n");
- ret = PTR_ERR(mem_priv);
- dma_buf_put(dbuf);
- goto err;
+ vb->planes[plane].dbuf = planes[plane].dbuf;
+ vb->planes[plane].mem_priv = mem_priv;
}
-
- vb->planes[plane].dbuf = dbuf;
- vb->planes[plane].mem_priv = mem_priv;
+ } else {
+ for (plane = 0; plane < vb->num_planes; ++plane)
+ dma_buf_put(planes[plane].dbuf);
}
/*
@@ -1413,7 +1413,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
if (ret) {
dprintk(q, 1, "failed to map dmabuf for plane %d\n",
plane);
- goto err;
+ goto err_put_vb2_buf;
}
vb->planes[plane].dbuf_mapped = 1;
}
@@ -1437,7 +1437,7 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
ret = call_vb_qop(vb, buf_init, vb);
if (ret) {
dprintk(q, 1, "buffer initialization failed\n");
- goto err;
+ goto err_put_vb2_buf;
}
}
@@ -1445,11 +1445,15 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
if (ret) {
dprintk(q, 1, "buffer preparation failed\n");
call_void_vb_qop(vb, buf_cleanup, vb);
- goto err;
+ goto err_put_vb2_buf;
}
return 0;
-err:
+
+err_put_dbuf:
+ for (plane = 0; plane < vb->num_planes; ++plane)
+ dma_buf_put(planes[plane].dbuf);
+err_put_vb2_buf:
/* In case of errors, release planes that were already acquired */
__vb2_buf_dmabuf_put(vb);
--
2.44.0.478.gd926399ef9-goog
next prev parent reply other threads:[~2024-04-03 9:13 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-03 9:13 [PATCH v2 0/3] media: videobuf2-core: attach once if multiple planes share the same dbuf Yunke Cao
2024-04-03 9:13 ` Yunke Cao [this message]
2024-05-17 11:11 ` [PATCH v2 1/3] media: videobuf2-core: release all planes first in __prepare_dmabuf() Tomasz Figa
2024-05-30 4:13 ` Yunke Cao
2024-04-03 9:13 ` [PATCH v2 2/3] media: videobuf2-core: reverse the iteration order in __vb2_buf_dmabuf_put Yunke Cao
2024-04-24 10:24 ` Hans Verkuil
2024-05-17 11:14 ` Tomasz Figa
2024-04-03 9:13 ` [PATCH v2 3/3] media: videobuf2-core: attach once if multiple planes share the same dbuf Yunke Cao
2024-05-17 11:23 ` Tomasz Figa
2024-05-20 2:19 ` Yunke Cao
2024-04-24 10:25 ` [PATCH v2 0/3] " Hans Verkuil
2024-05-08 16:02 ` Tomasz Figa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240403091306.1308878-2-yunkec@chromium.org \
--to=yunkec@chromium.org \
--cc=linux-media@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=mchehab@kernel.org \
--cc=tfiga@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).