Skip to content

Commit 95af7c0

Browse files
Yunke CaoHans Verkuil
authored andcommitted
media: videobuf2-core: release all planes first in __prepare_dmabuf()
In 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 <[email protected]> Acked-by: Tomasz Figa <[email protected]> Signed-off-by: Hans Verkuil <[email protected]>
1 parent 6a9c97a commit 95af7c0

File tree

1 file changed

+59
-56
lines changed

1 file changed

+59
-56
lines changed

drivers/media/common/videobuf2/videobuf2-core.c

Lines changed: 59 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,11 +1387,13 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
13871387
for (plane = 0; plane < vb->num_planes; ++plane) {
13881388
struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
13891389

1390+
planes[plane].dbuf = dbuf;
1391+
13901392
if (IS_ERR_OR_NULL(dbuf)) {
13911393
dprintk(q, 1, "invalid dmabuf fd for plane %d\n",
13921394
plane);
13931395
ret = -EINVAL;
1394-
goto err;
1396+
goto err_put_planes;
13951397
}
13961398

13971399
/* use DMABUF size if length is not provided */
@@ -1402,96 +1404,97 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
14021404
dprintk(q, 1, "invalid dmabuf length %u for plane %d, minimum length %u\n",
14031405
planes[plane].length, plane,
14041406
vb->planes[plane].min_length);
1405-
dma_buf_put(dbuf);
14061407
ret = -EINVAL;
1407-
goto err;
1408+
goto err_put_planes;
14081409
}
14091410

14101411
/* Skip the plane if already verified */
14111412
if (dbuf == vb->planes[plane].dbuf &&
1412-
vb->planes[plane].length == planes[plane].length) {
1413-
dma_buf_put(dbuf);
1413+
vb->planes[plane].length == planes[plane].length)
14141414
continue;
1415-
}
14161415

14171416
dprintk(q, 3, "buffer for plane %d changed\n", plane);
14181417

1419-
if (!reacquired) {
1420-
reacquired = true;
1418+
reacquired = true;
1419+
}
1420+
1421+
if (reacquired) {
1422+
if (vb->planes[0].mem_priv) {
14211423
vb->copied_timestamp = 0;
14221424
call_void_vb_qop(vb, buf_cleanup, vb);
1425+
__vb2_buf_dmabuf_put(vb);
14231426
}
14241427

1425-
/* Release previously acquired memory if present */
1426-
__vb2_plane_dmabuf_put(vb, &vb->planes[plane]);
1427-
1428-
/* Acquire each plane's memory */
1429-
mem_priv = call_ptr_memop(attach_dmabuf,
1430-
vb,
1431-
q->alloc_devs[plane] ? : q->dev,
1432-
dbuf,
1433-
planes[plane].length);
1434-
if (IS_ERR(mem_priv)) {
1435-
dprintk(q, 1, "failed to attach dmabuf\n");
1436-
ret = PTR_ERR(mem_priv);
1437-
dma_buf_put(dbuf);
1438-
goto err;
1439-
}
1440-
1441-
vb->planes[plane].dbuf = dbuf;
1442-
vb->planes[plane].mem_priv = mem_priv;
1443-
}
1428+
for (plane = 0; plane < vb->num_planes; ++plane) {
1429+
/* Acquire each plane's memory */
1430+
mem_priv = call_ptr_memop(attach_dmabuf,
1431+
vb,
1432+
q->alloc_devs[plane] ? : q->dev,
1433+
planes[plane].dbuf,
1434+
planes[plane].length);
1435+
if (IS_ERR(mem_priv)) {
1436+
dprintk(q, 1, "failed to attach dmabuf\n");
1437+
ret = PTR_ERR(mem_priv);
1438+
goto err_put_vb2_buf;
1439+
}
14441440

1445-
/*
1446-
* This pins the buffer(s) with dma_buf_map_attachment()). It's done
1447-
* here instead just before the DMA, while queueing the buffer(s) so
1448-
* userspace knows sooner rather than later if the dma-buf map fails.
1449-
*/
1450-
for (plane = 0; plane < vb->num_planes; ++plane) {
1451-
if (vb->planes[plane].dbuf_mapped)
1452-
continue;
1441+
vb->planes[plane].dbuf = planes[plane].dbuf;
1442+
vb->planes[plane].mem_priv = mem_priv;
14531443

1454-
ret = call_memop(vb, map_dmabuf, vb->planes[plane].mem_priv);
1455-
if (ret) {
1456-
dprintk(q, 1, "failed to map dmabuf for plane %d\n",
1457-
plane);
1458-
goto err;
1444+
/*
1445+
* This pins the buffer(s) with dma_buf_map_attachment()). It's done
1446+
* here instead just before the DMA, while queueing the buffer(s) so
1447+
* userspace knows sooner rather than later if the dma-buf map fails.
1448+
*/
1449+
ret = call_memop(vb, map_dmabuf, vb->planes[plane].mem_priv);
1450+
if (ret) {
1451+
dprintk(q, 1, "failed to map dmabuf for plane %d\n",
1452+
plane);
1453+
goto err_put_vb2_buf;
1454+
}
1455+
vb->planes[plane].dbuf_mapped = 1;
14591456
}
1460-
vb->planes[plane].dbuf_mapped = 1;
1461-
}
14621457

1463-
/*
1464-
* Now that everything is in order, copy relevant information
1465-
* provided by userspace.
1466-
*/
1467-
for (plane = 0; plane < vb->num_planes; ++plane) {
1468-
vb->planes[plane].bytesused = planes[plane].bytesused;
1469-
vb->planes[plane].length = planes[plane].length;
1470-
vb->planes[plane].m.fd = planes[plane].m.fd;
1471-
vb->planes[plane].data_offset = planes[plane].data_offset;
1472-
}
1458+
/*
1459+
* Now that everything is in order, copy relevant information
1460+
* provided by userspace.
1461+
*/
1462+
for (plane = 0; plane < vb->num_planes; ++plane) {
1463+
vb->planes[plane].bytesused = planes[plane].bytesused;
1464+
vb->planes[plane].length = planes[plane].length;
1465+
vb->planes[plane].m.fd = planes[plane].m.fd;
1466+
vb->planes[plane].data_offset = planes[plane].data_offset;
1467+
}
14731468

1474-
if (reacquired) {
14751469
/*
14761470
* Call driver-specific initialization on the newly acquired buffer,
14771471
* if provided.
14781472
*/
14791473
ret = call_vb_qop(vb, buf_init, vb);
14801474
if (ret) {
14811475
dprintk(q, 1, "buffer initialization failed\n");
1482-
goto err;
1476+
goto err_put_vb2_buf;
14831477
}
1478+
} else {
1479+
for (plane = 0; plane < vb->num_planes; ++plane)
1480+
dma_buf_put(planes[plane].dbuf);
14841481
}
14851482

14861483
ret = call_vb_qop(vb, buf_prepare, vb);
14871484
if (ret) {
14881485
dprintk(q, 1, "buffer preparation failed\n");
14891486
call_void_vb_qop(vb, buf_cleanup, vb);
1490-
goto err;
1487+
goto err_put_vb2_buf;
14911488
}
14921489

14931490
return 0;
1494-
err:
1491+
1492+
err_put_planes:
1493+
for (plane = 0; plane < vb->num_planes; ++plane) {
1494+
if (!IS_ERR_OR_NULL(planes[plane].dbuf))
1495+
dma_buf_put(planes[plane].dbuf);
1496+
}
1497+
err_put_vb2_buf:
14951498
/* In case of errors, release planes that were already acquired */
14961499
__vb2_buf_dmabuf_put(vb);
14971500

0 commit comments

Comments
 (0)