| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4e70c8b8-d459-2b79-2b3d-40875f701d97@synaptics.com>
Date: Mon, 17 Jul 2023 15:14:56 +0800
From: Hsia-Jun Li <Randy.Li@...aptics.com>
To: Tomasz Figa <tfiga@...omium.org>
Cc: linux-media@...r.kernel.org, ayaka@...lik.info,
hans.verkuil@...co.com, mchehab@...nel.org,
laurent.pinchart@...asonboard.com, hiroh@...omium.org,
hverkuil@...all.nl, linux-kernel@...r.kernel.org,
nicolas@...fresne.ca
Subject: Re: [PATCH 2/2] media: v4l2-mem2mem: add a list for buf used by hw
On 7/12/23 17:33, Tomasz Figa wrote:
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On Tue, Jul 04, 2023 at 12:00:38PM +0800, Hsia-Jun Li wrote:
>> From: "Hsia-Jun(Randy) Li" <randy.li@...aptics.com>
>>
>> Many drivers have to create its own buf_struct for a
>> vb2_queue to track such a state. Also driver has to
>> iterate over rdy_queue every times to find out a buffer
>> which is not sent to hardware(or firmware), this new
>> list just offers the driver a place to store the buffer
>> that hardware(firmware) has acknowledged.
>>
>> One important advance about this list, it doesn't like
>> rdy_queue which both bottom half of the user calling
>> could operate it, while the v4l2 worker would as well.
>> The v4l2 core could only operate this queue when its
>> v4l2_context is not running, the driver would only
>> access this new hw_queue in its own worker.
> Could you describe in what case such a list would be useful for a
> mem2mem driver?
This list, as its description, just for saving us from creating a
private buffer struct to track buffer state.
The queue in the kernel is not the queue that hardware(codec firmware)
are using.
>> Signed-off-by: Hsia-Jun(Randy) Li <randy.li@...aptics.com>
>> ---
>> drivers/media/v4l2-core/v4l2-mem2mem.c | 25 +++++++++++++++++--------
>> include/media/v4l2-mem2mem.h | 10 +++++++++-
>> 2 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
>> index c771aba42015..b4151147d5bd 100644
>> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
>> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
>> @@ -321,15 +321,21 @@ static void __v4l2_m2m_try_queue(struct v4l2_m2m_dev *m2m_dev,
>> goto job_unlock;
>> }
>>
>> - src = v4l2_m2m_next_src_buf(m2m_ctx);
>> - dst = v4l2_m2m_next_dst_buf(m2m_ctx);
>> - if (!src && !m2m_ctx->out_q_ctx.buffered) {
>> - dprintk("No input buffers available\n");
>> - goto job_unlock;
>> + if (list_empty(&m2m_ctx->out_q_ctx.hw_queue)) {
>> + src = v4l2_m2m_next_src_buf(m2m_ctx);
>> +
>> + if (!src && !m2m_ctx->out_q_ctx.buffered) {
>> + dprintk("No input buffers available\n");
>> + goto job_unlock;
>> + }
>> }
>> - if (!dst && !m2m_ctx->cap_q_ctx.buffered) {
>> - dprintk("No output buffers available\n");
>> - goto job_unlock;
>> +
>> + if (list_empty(&m2m_ctx->cap_q_ctx.hw_queue)) {
>> + dst = v4l2_m2m_next_dst_buf(m2m_ctx);
>> + if (!dst && !m2m_ctx->cap_q_ctx.buffered) {
>> + dprintk("No output buffers available\n");
>> + goto job_unlock;
>> + }
>> }
> src and dst would be referenced unitialized below if neither of the
> above ifs hits...
I think they have been initialized at v4l2_m2m_ctx_init()
>
> Best regards,
> Tomasz
>
>> m2m_ctx->new_frame = true;
>> @@ -896,6 +902,7 @@ int v4l2_m2m_streamoff(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>> INIT_LIST_HEAD(&q_ctx->rdy_queue);
>> q_ctx->num_rdy = 0;
>> spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
>> + INIT_LIST_HEAD(&q_ctx->hw_queue);
>>
>> if (m2m_dev->curr_ctx == m2m_ctx) {
>> m2m_dev->curr_ctx = NULL;
>> @@ -1234,6 +1241,8 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>>
>> INIT_LIST_HEAD(&out_q_ctx->rdy_queue);
>> INIT_LIST_HEAD(&cap_q_ctx->rdy_queue);
>> + INIT_LIST_HEAD(&out_q_ctx->hw_queue);
>> + INIT_LIST_HEAD(&cap_q_ctx->hw_queue);
>> spin_lock_init(&out_q_ctx->rdy_spinlock);
>> spin_lock_init(&cap_q_ctx->rdy_spinlock);
>>
>> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
>> index d6c8eb2b5201..2342656e582d 100644
>> --- a/include/media/v4l2-mem2mem.h
>> +++ b/include/media/v4l2-mem2mem.h
>> @@ -53,9 +53,16 @@ struct v4l2_m2m_dev;
>> * processed
>> *
>> * @q: pointer to struct &vb2_queue
>> - * @rdy_queue: List of V4L2 mem-to-mem queues
>> + * @rdy_queue: List of V4L2 mem-to-mem queues. If v4l2_m2m_buf_queue() is
>> + * called in struct vb2_ops->buf_queue(), the buffer enqueued
>> + * by user would be added to this list.
>> * @rdy_spinlock: spin lock to protect the struct usage
>> * @num_rdy: number of buffers ready to be processed
>> + * @hw_queue: A list for tracking the buffer is occupied by the hardware
>> + * (or device's firmware). A buffer could only be in either
>> + * this list or @rdy_queue.
>> + * Driver may choose not to use this list while uses its own
>> + * private data to do this work.
>> * @buffered: is the queue buffered?
>> *
>> * Queue for buffers ready to be processed as soon as this
>> @@ -68,6 +75,7 @@ struct v4l2_m2m_queue_ctx {
>> struct list_head rdy_queue;
>> spinlock_t rdy_spinlock;
>> u8 num_rdy;
>> + struct list_head hw_queue;
>> bool buffered;
>> };
>>
>> --
>> 2.17.1
>>
--
Hsia-Jun(Randy) Li
Powered by blists - more mailing lists