| Message ID | 20240516122102.16379-3-yunfei.dong@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: mediatek: add driver to support secure video decoder | expand |
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; > >
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;