From: Sergey Senozhatsky <senozhatsky@chromium.org>
To: Vikash Garodia <quic_vgarodia@quicinc.com>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Stanimir Varbanov <stanimir.k.varbanov@gmail.com>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Tomasz Figa <tfiga@chromium.org>,
linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCHv3] media: venus: provide video device lock
Date: Thu, 25 May 2023 09:53:12 +0900 [thread overview]
Message-ID: <20230525005312.GC30543@google.com> (raw)
In-Reply-To: <83cd3dc7-455d-0f26-d2a8-3ebe92d9e33f@quicinc.com>
On (23/05/24 22:06), Vikash Garodia wrote:
> > Instead the struct v4l2_m2m_ctx q_lock pointer, if set, will use that
> > mutex for all vb2 operations.
> >
> > I think you can set it to the 'lock' mutex in struct venus_inst.
>
> IIUC, the suggestion is to use the 'lock' in struct venus_inst while
> initializing the queue. This might lead to deadlock as the same lock is used
> during vb2 operations in driver. Might be introducing a new lock for this
> purpose in struct venus_inst would do, unless we are trying to serialize at
> video device (or core) context.
Something like this?
Video device has to provide a lock so that __video_do_ioctl()
can serialize IOCTL calls. Introduce a dedicated venus_inst
mutex (which is set a ctx ->q_lock) for the purpose of vb2
operations synchronization.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/media/platform/qcom/venus/core.h | 2 ++
drivers/media/platform/qcom/venus/vdec.c | 4 ++++
drivers/media/platform/qcom/venus/venc.c | 3 +++
3 files changed, 9 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 4f81669986ba..6ac5236d6888 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -389,6 +389,7 @@ enum venus_inst_modes {
* @sequence_out: a sequence counter for output queue
* @m2m_dev: a reference to m2m device structure
* @m2m_ctx: a reference to m2m context structure
+ * @ctx_queue_lock: a lock to serialize video device ioctl calls
* @state: current state of the instance
* @done: a completion for sync HFI operation
* @error: an error returned during last HFI sync operation
@@ -460,6 +461,7 @@ struct venus_inst {
u32 sequence_out;
struct v4l2_m2m_dev *m2m_dev;
struct v4l2_m2m_ctx *m2m_ctx;
+ struct mutex ctx_queue_lock;
unsigned int state;
struct completion done;
unsigned int error;
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 51a53bf82bd3..2caeba5b6378 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -1641,6 +1641,7 @@ static int vdec_open(struct file *file)
INIT_LIST_HEAD(&inst->internalbufs);
INIT_LIST_HEAD(&inst->list);
mutex_init(&inst->lock);
+ mutex_init(&inst->ctx_queue_lock);
inst->core = core;
inst->session_type = VIDC_SESSION_TYPE_DEC;
@@ -1684,8 +1685,10 @@ static int vdec_open(struct file *file)
goto err_m2m_release;
}
+
v4l2_fh_init(&inst->fh, core->vdev_dec);
+ inst->m2m_ctx->q_lock = &inst->ctx_queue_lock;
inst->fh.ctrl_handler = &inst->ctrl_handler;
v4l2_fh_add(&inst->fh);
inst->fh.m2m_ctx = inst->m2m_ctx;
@@ -1716,6 +1719,7 @@ static int vdec_close(struct file *file)
ida_destroy(&inst->dpb_ids);
hfi_session_destroy(inst);
mutex_destroy(&inst->lock);
+ mutex_destroy(&inst->ctx_queue_lock);
v4l2_fh_del(&inst->fh);
v4l2_fh_exit(&inst->fh);
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 4666f42feea3..4292b299f014 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -1443,6 +1443,7 @@ static int venc_open(struct file *file)
INIT_LIST_HEAD(&inst->internalbufs);
INIT_LIST_HEAD(&inst->list);
mutex_init(&inst->lock);
+ mutex_init(&inst->ctx_queue_lock);
inst->core = core;
inst->session_type = VIDC_SESSION_TYPE_ENC;
@@ -1483,6 +1484,7 @@ static int venc_open(struct file *file)
v4l2_fh_init(&inst->fh, core->vdev_enc);
+ inst->m2m_ctx->q_lock = &inst->ctx_queue_lock;
inst->fh.ctrl_handler = &inst->ctrl_handler;
v4l2_fh_add(&inst->fh);
inst->fh.m2m_ctx = inst->m2m_ctx;
@@ -1512,6 +1514,7 @@ static int venc_close(struct file *file)
venc_ctrl_deinit(inst);
hfi_session_destroy(inst);
mutex_destroy(&inst->lock);
+ mutex_destroy(&inst->ctx_queue_lock);
v4l2_fh_del(&inst->fh);
v4l2_fh_exit(&inst->fh);
--
2.40.1.698.g37aff9b760-goog
next prev parent reply other threads:[~2023-05-25 0:53 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-24 1:36 [PATCH] media: venus: provide video device lock Sergey Senozhatsky
2023-05-24 12:40 ` Sergey Senozhatsky
2023-05-24 13:56 ` [PATCHv2] " Sergey Senozhatsky
2023-05-24 14:12 ` [PATCHv3] " Sergey Senozhatsky
2023-05-24 14:29 ` Bryan O'Donoghue
2023-05-24 14:44 ` Hans Verkuil
2023-05-24 16:36 ` Vikash Garodia
2023-05-25 0:53 ` Sergey Senozhatsky [this message]
2023-05-25 11:02 ` Vikash Garodia
2023-05-26 3:35 ` Sergey Senozhatsky
2023-05-25 7:22 ` Hans Verkuil
2023-05-25 8:59 ` Sergey Senozhatsky
2023-05-25 9:03 ` Hans Verkuil
2023-06-01 10:16 ` Vikash Garodia
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230525005312.GC30543@google.com \
--to=senozhatsky@chromium.org \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=bryan.odonoghue@linaro.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=konrad.dybcio@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=quic_vgarodia@quicinc.com \
--cc=stanimir.k.varbanov@gmail.com \
--cc=tfiga@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.