From: Kamil Debski <k.debski@samsung.com>
To: 'Kiran AVND' <avnd.kiran@samsung.com>, linux-media@vger.kernel.org
Cc: wuchengli@chromium.org, posciak@chromium.org, arun.m@samsung.com,
ihf@chromium.org, prathyush.k@samsung.com, arun.kk@samsung.com
Subject: RE: [PATCH 16/17] [media] s5p-mfc: fix a race in interrupt flags handling
Date: Mon, 15 Sep 2014 16:42:37 +0200 [thread overview]
Message-ID: <022c01cfd0f3$4b35a170$e1a0e450$%debski@samsung.com> (raw)
In-Reply-To: <1410763393-12183-17-git-send-email-avnd.kiran@samsung.com>
Hi,
The patch does not apply to the newest media_tree branch.
http://git.linuxtv.org/cgit.cgi/media_tree.git/
Please send a rebased version.
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
> -----Original Message-----
> From: Kiran AVND [mailto:avnd.kiran@samsung.com]
> Sent: Monday, September 15, 2014 8:43 AM
> To: linux-media@vger.kernel.org
> Cc: k.debski@samsung.com; wuchengli@chromium.org; posciak@chromium.org;
> arun.m@samsung.com; ihf@chromium.org; prathyush.k@samsung.com;
> arun.kk@samsung.com
> Subject: [PATCH 16/17] [media] s5p-mfc: fix a race in interrupt flags
> handling
>
> From: Pawel Osciak <posciak@chromium.org>
>
> Interrupt result flags have to be cleared before a hardware job is run.
> Otherwise, if they are cleared asynchronously, we may end up clearing
> them after the interrupt for which we wanted to wait has already
> arrived, thus overwriting the job results that we intended to wait for.
>
> To prevent this, clear the flags only under hw_lock and before running
> a hardware job.
>
> Signed-off-by: Pawel Osciak <posciak@chromium.org>
> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 2 --
> drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 3 ---
> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 1 -
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 13 ++-----------
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 12 ++----------
> 5 files changed, 4 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> index 8531c72..7aae7f4 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> @@ -467,7 +467,6 @@ int s5p_mfc_open_mfc_inst(struct s5p_mfc_dev *dev,
> struct s5p_mfc_ctx *ctx)
> }
>
> set_work_bit_irqsave(ctx);
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
> if (s5p_mfc_wait_for_done_ctx(ctx,
> S5P_MFC_R2H_CMD_OPEN_INSTANCE_RET, 0)) { @@ -493,7 +492,6
> @@ void s5p_mfc_close_mfc_inst(struct s5p_mfc_dev *dev, struct
> s5p_mfc_ctx *ctx) {
> ctx->state = MFCINST_RETURN_INST;
> set_work_bit_irqsave(ctx);
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
> /* Wait until instance is returned or timeout occurred */
> if (s5p_mfc_wait_for_done_ctx(ctx,
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> index d0bdbfb..fb8069a 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> @@ -350,7 +350,6 @@ static int vidioc_g_fmt(struct file *file, void
> *priv, struct v4l2_format *f)
> MFCINST_RES_CHANGE_END)) {
> /* If the MFC is parsing the header,
> * so wait until it is finished */
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_wait_for_done_ctx(ctx,
> S5P_MFC_R2H_CMD_SEQ_DONE_RET,
> 0);
> }
> @@ -756,7 +755,6 @@ static int s5p_mfc_dec_g_v_ctrl(struct v4l2_ctrl
> *ctrl)
> return -EINVAL;
> }
> /* Should wait for the header to be parsed */
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_wait_for_done_ctx(ctx,
> S5P_MFC_R2H_CMD_SEQ_DONE_RET, 0);
> if (ctx->state >= MFCINST_HEAD_PARSED && @@ -1070,7 +1068,6
> @@ static void s5p_mfc_stop_streaming(struct vb2_queue *q)
> if (IS_MFCV6_PLUS(dev) && (ctx->state == MFCINST_RUNNING))
> {
> ctx->state = MFCINST_FLUSH;
> set_work_bit_irqsave(ctx);
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
> if (s5p_mfc_wait_for_done_ctx(ctx,
> S5P_MFC_R2H_CMD_DPB_FLUSH_RET, 0))
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> index f7a6f68..04b0467 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> @@ -1682,7 +1682,6 @@ static int s5p_mfc_enc_g_v_ctrl(struct v4l2_ctrl
> *ctrl)
> return -EINVAL;
> }
> /* Should wait for the header to be produced */
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_wait_for_done_ctx(ctx,
> S5P_MFC_R2H_CMD_SEQ_DONE_RET, 0);
> if (ctx->state >= MFCINST_HEAD_PARSED && diff --git
> a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> index e2b2f31..38e79e0 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> @@ -1177,7 +1177,6 @@ static void s5p_mfc_run_res_change(struct
> s5p_mfc_ctx *ctx)
>
> s5p_mfc_set_dec_stream_buffer_v5(ctx, 0, 0, 0);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_decode_one_frame_v5(ctx, MFC_DEC_RES_CHANGE); }
>
> @@ -1192,7 +1191,6 @@ static int s5p_mfc_run_dec_frame(struct
> s5p_mfc_ctx *ctx, int last_frame)
> last_frame = MFC_DEC_LAST_FRAME;
> s5p_mfc_set_dec_stream_buffer_v5(ctx, 0, 0, 0);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_decode_one_frame_v5(ctx, last_frame);
> return 0;
> }
> @@ -1213,7 +1211,6 @@ static int s5p_mfc_run_dec_frame(struct
> s5p_mfc_ctx *ctx, int last_frame)
> spin_unlock_irqrestore(&dev->irqlock, flags);
> index = temp_vb->b->v4l2_buf.index;
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> if (temp_vb->b->v4l2_planes[0].bytesused == 0) {
> last_frame = MFC_DEC_LAST_FRAME;
> mfc_debug(2, "Setting ctx->state to FINISHING\n"); @@ -
> 1274,7 +1271,6 @@ static int s5p_mfc_run_enc_frame(struct s5p_mfc_ctx
> *ctx)
> s5p_mfc_set_enc_stream_buffer_v5(ctx, dst_addr, dst_size);
> spin_unlock_irqrestore(&dev->irqlock, flags);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> mfc_debug(2, "encoding buffer with index=%d state=%d\n",
> src_mb ? src_mb->b->v4l2_buf.index : -1, ctx->state);
> s5p_mfc_encode_one_frame_v5(ctx);
> @@ -1298,7 +1294,6 @@ static void s5p_mfc_run_init_dec(struct
> s5p_mfc_ctx *ctx)
> 0, temp_vb->b->v4l2_planes[0].bytesused);
> spin_unlock_irqrestore(&dev->irqlock, flags);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_init_decode_v5(ctx);
> }
>
> @@ -1318,7 +1313,6 @@ static void s5p_mfc_run_init_enc(struct
> s5p_mfc_ctx *ctx)
> s5p_mfc_set_enc_stream_buffer_v5(ctx, dst_addr, dst_size);
> spin_unlock_irqrestore(&dev->irqlock, flags);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_init_encode_v5(ctx);
> }
>
> @@ -1353,7 +1347,6 @@ static int s5p_mfc_run_init_dec_buffers(struct
> s5p_mfc_ctx *ctx)
> 0, temp_vb->b->v4l2_planes[0].bytesused);
> spin_unlock_irqrestore(&dev->irqlock, flags);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_set_dec_frame_buffer_v5(ctx);
> if (ret) {
> mfc_err("Failed to alloc frame mem\n"); @@ -1403,6 +1396,8
> @@ static void s5p_mfc_try_run_v5(struct s5p_mfc_dev *dev)
> if (test_and_set_bit(0, &dev->clk_flag) == 0)
> s5p_mfc_clock_on();
>
> + s5p_mfc_clean_ctx_int_flags(ctx);
> +
> if (ctx->type == MFCINST_DECODER) {
> s5p_mfc_set_dec_desc_buffer(ctx);
> switch (ctx->state) {
> @@ -1413,12 +1408,10 @@ static void s5p_mfc_try_run_v5(struct
> s5p_mfc_dev *dev)
> ret = s5p_mfc_run_dec_frame(ctx, MFC_DEC_FRAME);
> break;
> case MFCINST_INIT:
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_hw_call(dev->mfc_cmds, open_inst_cmd,
> ctx);
> break;
> case MFCINST_RETURN_INST:
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_hw_call(dev->mfc_cmds, close_inst_cmd,
> ctx);
> break;
> @@ -1451,12 +1444,10 @@ static void s5p_mfc_try_run_v5(struct
> s5p_mfc_dev *dev)
> ret = s5p_mfc_run_enc_frame(ctx);
> break;
> case MFCINST_INIT:
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_hw_call(dev->mfc_cmds, open_inst_cmd,
> ctx);
> break;
> case MFCINST_RETURN_INST:
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_hw_call(dev->mfc_cmds, close_inst_cmd,
> ctx);
> break;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index 4cdea67..92e7f3e 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -1399,7 +1399,6 @@ static inline void s5p_mfc_set_flush(struct
> s5p_mfc_ctx *ctx, int flush)
>
> if (flush) {
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> WRITEL(ctx->inst_no, mfc_regs->instance_id);
> s5p_mfc_hw_call(dev->mfc_cmds, cmd_host2risc, dev,
> S5P_FIMV_H2R_CMD_FLUSH_V6, NULL);
> @@ -1540,7 +1539,6 @@ static inline void
> s5p_mfc_run_dec_last_frames(struct s5p_mfc_ctx *ctx)
>
> s5p_mfc_set_dec_stream_buffer_v6(ctx, 0, 0, 0);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_decode_one_frame_v6(ctx, MFC_DEC_LAST_FRAME); }
>
> @@ -1577,7 +1575,6 @@ static inline int s5p_mfc_run_dec_frame(struct
> s5p_mfc_ctx *ctx)
> spin_unlock_irqrestore(&dev->irqlock, flags);
>
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> if (temp_vb->b->v4l2_planes[0].bytesused == 0) {
> last_frame = 1;
> mfc_debug(2, "Setting ctx->state to FINISHING\n"); @@ -
> 1634,7 +1631,6 @@ static inline int s5p_mfc_run_enc_frame(struct
> s5p_mfc_ctx *ctx)
> spin_unlock_irqrestore(&dev->irqlock, flags);
>
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_encode_one_frame_v6(ctx);
>
> return 0;
> @@ -1656,7 +1652,6 @@ static inline void s5p_mfc_run_init_dec(struct
> s5p_mfc_ctx *ctx)
> temp_vb->b->v4l2_planes[0].bytesused);
> spin_unlock_irqrestore(&dev->irqlock, flags);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_init_decode_v6(ctx);
> }
>
> @@ -1676,7 +1671,6 @@ static inline void s5p_mfc_run_init_enc(struct
> s5p_mfc_ctx *ctx)
> s5p_mfc_set_enc_stream_buffer_v6(ctx, dst_addr, dst_size);
> spin_unlock_irqrestore(&dev->irqlock, flags);
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> s5p_mfc_init_encode_v6(ctx);
> }
>
> @@ -1696,7 +1690,6 @@ static inline int
> s5p_mfc_run_init_dec_buffers(struct s5p_mfc_ctx *ctx)
> }
>
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_set_dec_frame_buffer_v6(ctx);
> if (ret) {
> mfc_err("Failed to alloc frame mem.\n"); @@ -1711,7 +1704,6
> @@ static inline int s5p_mfc_run_init_enc_buffers(struct s5p_mfc_ctx
> *ctx)
> int ret;
>
> dev->curr_ctx = ctx->num;
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_set_enc_ref_buffer_v6(ctx);
> if (ret) {
> mfc_err("Failed to alloc frame mem.\n"); @@ -1771,6 +1763,8
> @@ static void s5p_mfc_try_run_v6(struct s5p_mfc_dev *dev)
> if (test_and_set_bit(0, &dev->clk_flag) == 0)
> s5p_mfc_clock_on();
>
> + s5p_mfc_clean_ctx_int_flags(ctx);
> +
> if (ctx->type == MFCINST_DECODER) {
> switch (ctx->state) {
> case MFCINST_FINISHING:
> @@ -1780,12 +1774,10 @@ static void s5p_mfc_try_run_v6(struct
> s5p_mfc_dev *dev)
> ret = s5p_mfc_run_dec_frame(ctx);
> break;
> case MFCINST_INIT:
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_hw_call(dev->mfc_cmds, open_inst_cmd,
> ctx);
> break;
> case MFCINST_RETURN_INST:
> - s5p_mfc_clean_ctx_int_flags(ctx);
> ret = s5p_mfc_hw_call(dev->mfc_cmds, close_inst_cmd,
> ctx);
> break;
> --
> 1.7.3.rc2
next prev parent reply other threads:[~2014-09-15 14:42 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-15 6:42 [PATCH 00/17] Fixes from Chrome OS tree for MFC driver Kiran AVND
2014-09-15 6:42 ` [PATCH 01/17] [media] s5p-mfc: support MIN_BUFFERS query for encoder Kiran AVND
2014-09-15 6:42 ` [PATCH 02/17] [media] s5p-mfc: Fix REQBUFS(0) " Kiran AVND
2014-09-15 6:42 ` [PATCH 03/17] [media] s5p-mfc: set B-frames as 2 while encoding Kiran AVND
2014-09-15 14:42 ` Kamil Debski
[not found] ` <CAFHkUjjjK4TOSfYj=xS12LPcUAusKrMaDG1WYiVegG+B9K6w3A@mail.gmail.com>
2014-09-16 14:20 ` Kamil Debski
2014-09-15 6:42 ` [PATCH 04/17] [media] s5p-mfc: clear 'enter_suspend' flag if suspend fails Kiran AVND
2014-09-15 6:43 ` [PATCH 05/17] [media] s5p-mfc: don't disable clock when next ctx is pending Kiran AVND
2014-09-16 14:20 ` Kamil Debski
2014-09-17 9:25 ` Kiran Avnd
2014-09-17 9:57 ` Kamil Debski
2014-09-17 10:55 ` Kiran Avnd
2014-09-15 6:43 ` [PATCH 06/17] [media] s5p-mfc: Only set timestamp/timecode for new frames Kiran AVND
2014-09-15 6:43 ` [PATCH 07/17] [media] s5p-mfc: keep RISC ON during reset for V7/V8 Kiran AVND
2014-09-15 6:43 ` [PATCH 08/17] [media] s5p-mfc: check mfc bus ctrl before reset Kiran AVND
2014-09-15 6:43 ` [PATCH 09/17] [media] s5p-mfc: Don't crash the kernel if the watchdog kicks in Kiran AVND
2014-09-15 6:43 ` [PATCH 10/17] [media] s5p-mfc: modify mfc wakeup sequence for V8 Kiran AVND
2014-09-15 14:42 ` Kamil Debski
2014-09-15 6:43 ` [PATCH 11/17] [media] s5p-mfc: De-init MFC when watchdog kicks in Kiran AVND
2014-09-15 6:43 ` [PATCH 12/17] [media] s5p-mfc: flush dpbs when resolution changes Kiran AVND
2014-09-15 6:43 ` [PATCH 13/17] [media] s5p-mfc: Remove unused alloc field from private buffer struct Kiran AVND
2014-09-15 6:43 ` [PATCH 14/17] [media] s5p-mfc: fix V4L2_CID_MIN_BUFFERS_FOR_CAPTURE on resolution change Kiran AVND
2014-09-15 6:43 ` [PATCH 15/17] [media] s5p-mfc: remove reduntant clock on & clock off Kiran AVND
2014-09-15 14:42 ` Kamil Debski
2014-09-15 6:43 ` [PATCH 16/17] [media] s5p-mfc: fix a race in interrupt flags handling Kiran AVND
2014-09-15 14:42 ` Kamil Debski [this message]
2014-09-15 6:43 ` [PATCH 17/17] [media] s5p-mfc: Don't change the image size to smaller than the request Kiran AVND
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='022c01cfd0f3$4b35a170$e1a0e450$%debski@samsung.com' \
--to=k.debski@samsung.com \
--cc=arun.kk@samsung.com \
--cc=arun.m@samsung.com \
--cc=avnd.kiran@samsung.com \
--cc=ihf@chromium.org \
--cc=linux-media@vger.kernel.org \
--cc=posciak@chromium.org \
--cc=prathyush.k@samsung.com \
--cc=wuchengli@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox