Re: [PATCH v15 30/56] media: test-drivers: Stop direct calls to queue num_buffers field
From: Andrzej Pietrasiewicz
Date: Fri Nov 10 2023 - 14:22:00 EST
Hi Benjamin,
W dniu 10.11.2023 o 10:55, Benjamin Gaignard pisze:
Le 10/11/2023 à 10:35, Andrzej Pietrasiewicz a écrit :
Hi Benjamin,
W dniu 9.11.2023 o 17:34, Benjamin Gaignard pisze:
Use vb2_get_num_buffers() to avoid using queue num_buffers field directly.
This allows us to change how the number of buffers is computed in the
future.
If 'min_buffers_needed' is set remove useless checks in queue setup
functions.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
CC: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx>
---
drivers/media/test-drivers/visl/visl-dec.c | 4 ++--
drivers/media/test-drivers/vivid/vivid-meta-cap.c | 3 ---
drivers/media/test-drivers/vivid/vivid-meta-out.c | 5 +++--
drivers/media/test-drivers/vivid/vivid-touch-cap.c | 5 +++--
drivers/media/test-drivers/vivid/vivid-vbi-cap.c | 3 ---
drivers/media/test-drivers/vivid/vivid-vbi-out.c | 3 ---
drivers/media/test-drivers/vivid/vivid-vid-cap.c | 3 ---
drivers/media/test-drivers/vivid/vivid-vid-out.c | 5 +----
8 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/media/test-drivers/visl/visl-dec.c
b/drivers/media/test-drivers/visl/visl-dec.c
index ba20ea998d19..4672dc5e52bb 100644
--- a/drivers/media/test-drivers/visl/visl-dec.c
+++ b/drivers/media/test-drivers/visl/visl-dec.c
@@ -287,7 +287,7 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct
visl_run *run)
frame_dprintk(ctx->dev, run->dst->sequence, "%s\n", buf);
len = 0;
- for (i = 0; i < out_q->num_buffers; i++) {
+ for (i = 0; i < vb2_get_num_buffers(out_q); i++) {
char entry[] = "index: %u, state: %s, request_fd: %d, ";
u32 old_len = len;
struct vb2_buffer *vb2;
@@ -347,7 +347,7 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct
visl_run *run)
frame_dprintk(ctx->dev, run->dst->sequence, "%s\n", buf);
len = 0;
- for (i = 0; i < cap_q->num_buffers; i++) {
+ for (i = 0; i < vb2_get_num_buffers(cap_q); i++) {
u32 old_len = len;
struct vb2_buffer *vb2;
char *q_status;
diff --git a/drivers/media/test-drivers/vivid/vivid-meta-cap.c
b/drivers/media/test-drivers/vivid/vivid-meta-cap.c
index 780f96860a6d..0a718d037e59 100644
--- a/drivers/media/test-drivers/vivid/vivid-meta-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-meta-cap.c
@@ -30,9 +30,6 @@ static int meta_cap_queue_setup(struct vb2_queue *vq,
unsigned int *nbuffers,
sizes[0] = size;
}
- if (vq->num_buffers + *nbuffers < 2)
- *nbuffers = 2 - vq->num_buffers;
-
*nplanes = 1;
return 0;
}
diff --git a/drivers/media/test-drivers/vivid/vivid-meta-out.c
b/drivers/media/test-drivers/vivid/vivid-meta-out.c
index 95835b52b58f..4a569a6e58be 100644
--- a/drivers/media/test-drivers/vivid/vivid-meta-out.c
+++ b/drivers/media/test-drivers/vivid/vivid-meta-out.c
@@ -18,6 +18,7 @@ static int meta_out_queue_setup(struct vb2_queue *vq,
unsigned int *nbuffers,
struct device *alloc_devs[])
{
struct vivid_dev *dev = vb2_get_drv_priv(vq);
+ unsigned int q_num_bufs = vb2_get_num_buffers(vq);
unsigned int size = sizeof(struct vivid_meta_out_buf);
if (!vivid_is_webcam(dev))
@@ -30,8 +31,8 @@ static int meta_out_queue_setup(struct vb2_queue *vq,
unsigned int *nbuffers,
sizes[0] = size;
}
- if (vq->num_buffers + *nbuffers < 2)
- *nbuffers = 2 - vq->num_buffers;
+ if (q_num_bufs + *nbuffers < 2)
+ *nbuffers = 2 - q_num_bufs;
*nplanes = 1;
return 0;
diff --git a/drivers/media/test-drivers/vivid/vivid-touch-cap.c
b/drivers/media/test-drivers/vivid/vivid-touch-cap.c
index c7f6e23df51e..4b3c6ea0afde 100644
--- a/drivers/media/test-drivers/vivid/vivid-touch-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-touch-cap.c
@@ -13,6 +13,7 @@ static int touch_cap_queue_setup(struct vb2_queue *vq,
unsigned int *nbuffers,
struct device *alloc_devs[])
{
struct vivid_dev *dev = vb2_get_drv_priv(vq);
+ unsigned int q_num_bufs = vb2_get_num_buffers(vq);
struct v4l2_pix_format *f = &dev->tch_format;
unsigned int size = f->sizeimage;
@@ -23,8 +24,8 @@ static int touch_cap_queue_setup(struct vb2_queue *vq,
unsigned int *nbuffers,
sizes[0] = size;
}
- if (vq->num_buffers + *nbuffers < 2)
- *nbuffers = 2 - vq->num_buffers;
+ if (q_num_bufs + *nbuffers < 2)
+ *nbuffers = 2 - q_num_bufs;
*nplanes = 1;
return 0;
diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c
b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c
index b65b02eeeb97..3840b3a664ac 100644
--- a/drivers/media/test-drivers/vivid/vivid-vbi-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vbi-cap.c
@@ -134,9 +134,6 @@ static int vbi_cap_queue_setup(struct vb2_queue *vq,
sizes[0] = size;
- if (vq->num_buffers + *nbuffers < 2)
- *nbuffers = 2 - vq->num_buffers;
-
*nplanes = 1;
return 0;
}
diff --git a/drivers/media/test-drivers/vivid/vivid-vbi-out.c
b/drivers/media/test-drivers/vivid/vivid-vbi-out.c
index cd56476902a2..434a10676417 100644
--- a/drivers/media/test-drivers/vivid/vivid-vbi-out.c
+++ b/drivers/media/test-drivers/vivid/vivid-vbi-out.c
@@ -30,9 +30,6 @@ static int vbi_out_queue_setup(struct vb2_queue *vq,
sizes[0] = size;
- if (vq->num_buffers + *nbuffers < 2)
- *nbuffers = 2 - vq->num_buffers;
-
*nplanes = 1;
return 0;
}
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index 3a06df35a2d7..2804975fe278 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -117,9 +117,6 @@ static int vid_cap_queue_setup(struct vb2_queue *vq,
dev->fmt_cap->data_offset[p];
}
- if (vq->num_buffers + *nbuffers < 2)
- *nbuffers = 2 - vq->num_buffers;
-
*nplanes = buffers;
dprintk(dev, 1, "%s: count=%d\n", __func__, *nbuffers);
here the format specifier for "*nbuffers" is "%d"...
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-out.c
b/drivers/media/test-drivers/vivid/vivid-vid-out.c
index 184a6df2c29f..1653b2988f7e 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-out.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-out.c
@@ -73,12 +73,9 @@ static int vid_out_queue_setup(struct vb2_queue *vq,
vfmt->data_offset[p] : size;
}
- if (vq->num_buffers + *nbuffers < 2)
- *nbuffers = 2 - vq->num_buffers;
-
*nplanes = planes;
- dprintk(dev, 1, "%s: count=%d\n", __func__, *nbuffers);
+ dprintk(dev, 1, "%s: count=%u\n", __func__, *nbuffers);
... but here you change it to "%u". Is there a reason for these two to be
different? I didn't notice it in the previous version but now it stands out
clearly. Probably you changed to %u because of the type returned by
vb2_get_num_buffers(). And, actually, *nbuffers _is_ unsigned, too.
Like you said *nbuffers is unsigned so %u sound for me.
The goal of this patch was to avoid vq->num_buffers usage not to clean up all the
file.
That could be done in other patch since there is lot to do here.
Arguably, if the purpose of this patch is to avoid using vq->num_buffers,
the line we're discussing could be left unpatched, which minimizes the diffstat.
Otherwise, you _are_ changing vid_cap_queue_setup() anyway.
Regards,
Andrzej
Regards,
Benjamin
Regards,
Andrzej
for (p = 0; p < planes; p++)
dprintk(dev, 1, "%s: size[%u]=%u\n", __func__, p, sizes[p]);
return 0;