.../media/common/videobuf2/videobuf2-core.c | 21 +++++++++++++++++++ .../media/common/videobuf2/videobuf2-v4l2.c | 4 +++- 2 files changed, 24 insertions(+), 1 deletion(-)
From: Jeffrey Kardatzke <jkardatzke@google.com>
Validates the restricted memory flags when setting up a queue and
ensures the queue has the proper capability.
Signed-off-by: Jeffrey Kardatzke <jkardatzke@google.com>
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../media/common/videobuf2/videobuf2-core.c | 21 +++++++++++++++++++
.../media/common/videobuf2/videobuf2-v4l2.c | 4 +++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 358f1fe42975..fe4c0594ab81 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -831,6 +831,15 @@ static bool verify_coherency_flags(struct vb2_queue *q, bool non_coherent_mem)
return true;
}
+static bool verify_restricted_mem_flags(struct vb2_queue *q, bool restricted_mem)
+{
+ if (restricted_mem != q->restricted_mem) {
+ dprintk(q, 1, "restricted memory model mismatch\n");
+ return false;
+ }
+ return true;
+}
+
static int vb2_core_allocated_buffers_storage(struct vb2_queue *q)
{
if (!q->bufs)
@@ -864,6 +873,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int q_num_bufs = vb2_get_num_buffers(q);
unsigned plane_sizes[VB2_MAX_PLANES] = { };
bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
+ bool restricted_mem = flags & V4L2_MEMORY_FLAG_RESTRICTED;
unsigned int i, first_index;
int ret = 0;
@@ -907,6 +917,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
return 0;
}
+ if (restricted_mem && (!q->allow_restricted_mem || memory != VB2_MEMORY_DMABUF))
+ return -EINVAL;
+
/*
* Make sure the requested values and current defaults are sane.
*/
@@ -924,6 +937,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
if (ret)
return ret;
set_queue_coherency(q, non_coherent_mem);
+ q->restricted_mem = restricted_mem;
/*
* Ask the driver how many buffers and planes per buffer it requires.
@@ -1032,6 +1046,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned plane_sizes[VB2_MAX_PLANES] = { };
bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
unsigned int q_num_bufs = vb2_get_num_buffers(q);
+ bool restricted_mem = flags & V4L2_MEMORY_FLAG_RESTRICTED;
bool no_previous_buffers = !q_num_bufs;
int ret = 0;
@@ -1040,6 +1055,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
return -ENOBUFS;
}
+ if (restricted_mem && (!q->allow_restricted_mem || memory != VB2_MEMORY_DMABUF))
+ return -EINVAL;
+
if (no_previous_buffers) {
if (q->waiting_in_dqbuf && *count) {
dprintk(q, 1, "another dup()ped fd is waiting for a buffer\n");
@@ -1058,6 +1076,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
return ret;
q->waiting_for_buffers = !q->is_output;
set_queue_coherency(q, non_coherent_mem);
+ q->restricted_mem = restricted_mem;
} else {
if (q->memory != memory) {
dprintk(q, 1, "memory model mismatch\n");
@@ -1065,6 +1084,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
}
if (!verify_coherency_flags(q, non_coherent_mem))
return -EINVAL;
+ if (!verify_restricted_mem_flags(q, restricted_mem))
+ return -EINVAL;
}
num_buffers = min(*count, q->max_num_buffers - q_num_bufs);
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 293f3d5f1c4e..9ee24e537e0c 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -682,7 +682,7 @@ static void vb2_set_flags_and_caps(struct vb2_queue *q, u32 memory,
*flags = 0;
} else {
/* Clear all unknown flags. */
- *flags &= V4L2_MEMORY_FLAG_NON_COHERENT;
+ *flags &= V4L2_MEMORY_FLAG_NON_COHERENT | V4L2_MEMORY_FLAG_RESTRICTED;
}
*caps |= V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS;
@@ -698,6 +698,8 @@ static void vb2_set_flags_and_caps(struct vb2_queue *q, u32 memory,
*caps |= V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS;
if (q->supports_requests)
*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
+ if (q->allow_restricted_mem && q->io_modes & VB2_DMABUF)
+ *caps |= V4L2_BUF_CAP_SUPPORTS_RESTRICTED_MEM;
if (max_num_bufs) {
*max_num_bufs = q->max_num_buffers;
*caps |= V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS;
--
2.25.1
Hi Yunfei & Jeffrey, W dniu 16.05.2024 o 14:20, Yunfei Dong pisze: > From: Jeffrey Kardatzke <jkardatzke@google.com> > > Validates the restricted memory flags when setting up a queue and > ensures the queue has the proper capability. > > Signed-off-by: Jeffrey Kardatzke <jkardatzke@google.com> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > .../media/common/videobuf2/videobuf2-core.c | 21 +++++++++++++++++++ > .../media/common/videobuf2/videobuf2-v4l2.c | 4 +++- > 2 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index 358f1fe42975..fe4c0594ab81 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -831,6 +831,15 @@ static bool verify_coherency_flags(struct vb2_queue *q, bool non_coherent_mem) > return true; > } > > +static bool verify_restricted_mem_flags(struct vb2_queue *q, bool restricted_mem) > +{ > + if (restricted_mem != q->restricted_mem) { > + dprintk(q, 1, "restricted memory model mismatch\n"); > + return false; > + } > + return true; > +} > + > static int vb2_core_allocated_buffers_storage(struct vb2_queue *q) > { > if (!q->bufs) > @@ -864,6 +873,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int q_num_bufs = vb2_get_num_buffers(q); > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT; > + bool restricted_mem = flags & V4L2_MEMORY_FLAG_RESTRICTED; > unsigned int i, first_index; > int ret = 0; > > @@ -907,6 +917,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > return 0; > } > > + if (restricted_mem && (!q->allow_restricted_mem || memory != VB2_MEMORY_DMABUF)) > + return -EINVAL; > + > /* > * Make sure the requested values and current defaults are sane. > */ > @@ -924,6 +937,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > if (ret) > return ret; > set_queue_coherency(q, non_coherent_mem); > + q->restricted_mem = restricted_mem; > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -1032,6 +1046,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT; > unsigned int q_num_bufs = vb2_get_num_buffers(q); > + bool restricted_mem = flags & V4L2_MEMORY_FLAG_RESTRICTED; > bool no_previous_buffers = !q_num_bufs; > int ret = 0; > > @@ -1040,6 +1055,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > return -ENOBUFS; > } > > + if (restricted_mem && (!q->allow_restricted_mem || memory != VB2_MEMORY_DMABUF)) > + return -EINVAL; > + This condition is repeated in another place. If it is ever to be changed, the person changing it must remember to look at both places. Maybe: static inline int restricted_mem_mismatch(bool restricted_mem, struct vb2_queue *q, enum vb2_memory memory) { return restricted_mem && (!q->allow_restricted_mem || memory != VB2_MEMORY_DMABUF) ? -1 : 0; } (you probably want to clean up line breaks) and: if (restricted_mem_mismatch(restricted_mem, q, memory)) return -EINVAL; Regards, Andrzej > if (no_previous_buffers) { > if (q->waiting_in_dqbuf && *count) { > dprintk(q, 1, "another dup()ped fd is waiting for a buffer\n"); > @@ -1058,6 +1076,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > return ret; > q->waiting_for_buffers = !q->is_output; > set_queue_coherency(q, non_coherent_mem); > + q->restricted_mem = restricted_mem; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > @@ -1065,6 +1084,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > if (!verify_coherency_flags(q, non_coherent_mem)) > return -EINVAL; > + if (!verify_restricted_mem_flags(q, restricted_mem)) > + return -EINVAL; > } > > num_buffers = min(*count, q->max_num_buffers - q_num_bufs); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 293f3d5f1c4e..9ee24e537e0c 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -682,7 +682,7 @@ static void vb2_set_flags_and_caps(struct vb2_queue *q, u32 memory, > *flags = 0; > } else { > /* Clear all unknown flags. */ > - *flags &= V4L2_MEMORY_FLAG_NON_COHERENT; > + *flags &= V4L2_MEMORY_FLAG_NON_COHERENT | V4L2_MEMORY_FLAG_RESTRICTED; > } > > *caps |= V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS; > @@ -698,6 +698,8 @@ static void vb2_set_flags_and_caps(struct vb2_queue *q, u32 memory, > *caps |= V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS; > if (q->supports_requests) > *caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS; > + if (q->allow_restricted_mem && q->io_modes & VB2_DMABUF) > + *caps |= V4L2_BUF_CAP_SUPPORTS_RESTRICTED_MEM; > if (max_num_bufs) { > *max_num_bufs = q->max_num_buffers; > *caps |= V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS;
Hi Andrzej, Thanks for your help to review this patch. Your suggestion is very reasonable, I will change it later. Best Regards, Yunfei Dong On Wed, 2024-05-22 at 14:20 +0200, Andrzej Pietrasiewicz wrote: > Hi Yunfei & Jeffrey, > > W dniu 16.05.2024 o 14:20, Yunfei Dong pisze: > > From: Jeffrey Kardatzke <jkardatzke@google.com> > > > > Validates the restricted memory flags when setting up a queue and > > ensures the queue has the proper capability. > > > > Signed-off-by: Jeffrey Kardatzke <jkardatzke@google.com> > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > --- > > .../media/common/videobuf2/videobuf2-core.c | 21 > > +++++++++++++++++++ > > .../media/common/videobuf2/videobuf2-v4l2.c | 4 +++- > > 2 files changed, 24 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > > b/drivers/media/common/videobuf2/videobuf2-core.c > > index 358f1fe42975..fe4c0594ab81 100644 > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > @@ -831,6 +831,15 @@ static bool verify_coherency_flags(struct > > vb2_queue *q, bool non_coherent_mem) > > return true; > > } > > > > +static bool verify_restricted_mem_flags(struct vb2_queue *q, bool > > restricted_mem) > > +{ > > + if (restricted_mem != q->restricted_mem) { > > + dprintk(q, 1, "restricted memory model mismatch\n"); > > + return false; > > + } > > + return true; > > +} > > + > > static int vb2_core_allocated_buffers_storage(struct vb2_queue > > *q) > > { > > if (!q->bufs) > > @@ -864,6 +873,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum > > vb2_memory memory, > > unsigned int q_num_bufs = vb2_get_num_buffers(q); > > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > > bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT; > > + bool restricted_mem = flags & V4L2_MEMORY_FLAG_RESTRICTED; > > unsigned int i, first_index; > > int ret = 0; > > > > @@ -907,6 +917,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum > > vb2_memory memory, > > return 0; > > } > > > > + if (restricted_mem && (!q->allow_restricted_mem || memory != > > VB2_MEMORY_DMABUF)) > > + return -EINVAL; > > + > > /* > > * Make sure the requested values and current defaults are > > sane. > > */ > > @@ -924,6 +937,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum > > vb2_memory memory, > > if (ret) > > return ret; > > set_queue_coherency(q, non_coherent_mem); > > + q->restricted_mem = restricted_mem; > > > > /* > > * Ask the driver how many buffers and planes per buffer it > > requires. > > @@ -1032,6 +1046,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, > > enum vb2_memory memory, > > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > > bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT; > > unsigned int q_num_bufs = vb2_get_num_buffers(q); > > + bool restricted_mem = flags & V4L2_MEMORY_FLAG_RESTRICTED; > > bool no_previous_buffers = !q_num_bufs; > > int ret = 0; > > > > @@ -1040,6 +1055,9 @@ int vb2_core_create_bufs(struct vb2_queue *q, > > enum vb2_memory memory, > > return -ENOBUFS; > > } > > > > + if (restricted_mem && (!q->allow_restricted_mem || memory != > > VB2_MEMORY_DMABUF)) > > + return -EINVAL; > > + > > This condition is repeated in another place. If it is ever to be > changed, the person changing it must remember to look at both > places. Maybe: > > static inline int restricted_mem_mismatch(bool restricted_mem, > struct vb2_queue *q, enum vb2_memory > memory) > { > return restricted_mem && > (!q->allow_restricted_mem || memory != > VB2_MEMORY_DMABUF) ? > -1 : 0; > } > > (you probably want to clean up line breaks) > > and: > > if (restricted_mem_mismatch(restricted_mem, q, memory)) > return -EINVAL; > > Regards, > > Andrzej > > > if (no_previous_buffers) { > > if (q->waiting_in_dqbuf && *count) { > > dprintk(q, 1, "another dup()ped fd is waiting > > for a buffer\n"); > > @@ -1058,6 +1076,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, > > enum vb2_memory memory, > > return ret; > > q->waiting_for_buffers = !q->is_output; > > set_queue_coherency(q, non_coherent_mem); > > + q->restricted_mem = restricted_mem; > > } else { > > if (q->memory != memory) { > > dprintk(q, 1, "memory model mismatch\n"); > > @@ -1065,6 +1084,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, > > enum vb2_memory memory, > > } > > if (!verify_coherency_flags(q, non_coherent_mem)) > > return -EINVAL; > > + if (!verify_restricted_mem_flags(q, restricted_mem)) > > + return -EINVAL; > > } > > > > num_buffers = min(*count, q->max_num_buffers - q_num_bufs); > > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c > > b/drivers/media/common/videobuf2/videobuf2-v4l2.c > > index 293f3d5f1c4e..9ee24e537e0c 100644 > > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > > @@ -682,7 +682,7 @@ static void vb2_set_flags_and_caps(struct > > vb2_queue *q, u32 memory, > > *flags = 0; > > } else { > > /* Clear all unknown flags. */ > > - *flags &= V4L2_MEMORY_FLAG_NON_COHERENT; > > + *flags &= V4L2_MEMORY_FLAG_NON_COHERENT | > > V4L2_MEMORY_FLAG_RESTRICTED; > > } > > > > *caps |= V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS; > > @@ -698,6 +698,8 @@ static void vb2_set_flags_and_caps(struct > > vb2_queue *q, u32 memory, > > *caps |= V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS; > > if (q->supports_requests) > > *caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS; > > + if (q->allow_restricted_mem && q->io_modes & VB2_DMABUF) > > + *caps |= V4L2_BUF_CAP_SUPPORTS_RESTRICTED_MEM; > > if (max_num_bufs) { > > *max_num_bufs = q->max_num_buffers; > > *caps |= V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS; > >
© 2016 - 2024 Red Hat, Inc.