Hi Andrzej, Thank you for your patch. Best wishes, -- Kamil Debski Linux Platform Group Samsung Poland R&D Center > From: Andrzej Hajda [mailto:a.hajda@xxxxxxxxxxx] > Sent: 14 August 2012 11:14 > > All code setting/clearing working context bits has been moved > to separate functions. set_bit/clear_bit have been replaced by > non-atomic variants - variable is already guarded by spin_lock. > > Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> Acked-by: Kamil Debski <k.debski@xxxxxxxxxxx> > --- > The patch should be applied after > "[RESEND] v4l/s5p-mfc: added support for end of stream handling in MFC encoder" > http://patchwork.linuxtv.org/patch/13721/ > --- > drivers/media/video/s5p-mfc/s5p_mfc.c | 55 ++++++++++++++++++------- > drivers/media/video/s5p-mfc/s5p_mfc_common.h | 5 ++ > drivers/media/video/s5p-mfc/s5p_mfc_dec.c | 28 +++---------- > drivers/media/video/s5p-mfc/s5p_mfc_enc.c | 34 ++++------------ > 4 files changed, 60 insertions(+), 62 deletions(-) > > diff --git a/drivers/media/video/s5p-mfc/s5p_mfc.c b/drivers/media/video/s5p- > mfc/s5p_mfc.c > index 40cef6b..e5c2b80 100644 > --- a/drivers/media/video/s5p-mfc/s5p_mfc.c > +++ b/drivers/media/video/s5p-mfc/s5p_mfc.c > @@ -41,16 +41,49 @@ module_param(debug, int, S_IRUGO | S_IWUSR); > MODULE_PARM_DESC(debug, "Debug level - higher value produces more verbose > messages"); > > /* Helper functions for interrupt processing */ > + > /* Remove from hw execution round robin */ > -static void clear_work_bit(struct s5p_mfc_ctx *ctx) > +void clear_work_bit(struct s5p_mfc_ctx *ctx) > { > struct s5p_mfc_dev *dev = ctx->dev; > > spin_lock(&dev->condlock); > - clear_bit(ctx->num, &dev->ctx_work_bits); > + __clear_bit(ctx->num, &dev->ctx_work_bits); > spin_unlock(&dev->condlock); > } > > +/* Add to hw execution round robin */ > +void set_work_bit(struct s5p_mfc_ctx *ctx) > +{ > + struct s5p_mfc_dev *dev = ctx->dev; > + > + spin_lock(&dev->condlock); > + __set_bit(ctx->num, &dev->ctx_work_bits); > + spin_unlock(&dev->condlock); > +} > + > +/* Remove from hw execution round robin */ > +void clear_work_bit_irqsave(struct s5p_mfc_ctx *ctx) > +{ > + struct s5p_mfc_dev *dev = ctx->dev; > + unsigned long flags; > + > + spin_lock_irqsave(&dev->condlock, flags); > + __clear_bit(ctx->num, &dev->ctx_work_bits); > + spin_unlock_irqrestore(&dev->condlock, flags); > +} > + > +/* Add to hw execution round robin */ > +void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx) > +{ > + struct s5p_mfc_dev *dev = ctx->dev; > + unsigned long flags; > + > + spin_lock_irqsave(&dev->condlock, flags); > + __set_bit(ctx->num, &dev->ctx_work_bits); > + spin_unlock_irqrestore(&dev->condlock, flags); > +} > + > /* Wake up context wait_queue */ > static void wake_up_ctx(struct s5p_mfc_ctx *ctx, unsigned int reason, > unsigned int err) > @@ -504,9 +537,7 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx > *ctx, > ctx->int_type = reason; > ctx->int_err = err; > ctx->int_cond = 1; > - spin_lock(&dev->condlock); > - clear_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock(&dev->condlock); > + clear_work_bit(ctx); > if (err == 0) { > ctx->state = MFCINST_RUNNING; > if (!ctx->dpb_flush_flag) { > @@ -681,7 +712,6 @@ static int s5p_mfc_open(struct file *file) > struct s5p_mfc_dev *dev = video_drvdata(file); > struct s5p_mfc_ctx *ctx = NULL; > struct vb2_queue *q; > - unsigned long flags; > int ret = 0; > > mfc_debug_enter(); > @@ -714,9 +744,7 @@ static int s5p_mfc_open(struct file *file) > } > } > /* Mark context as idle */ > - spin_lock_irqsave(&dev->condlock, flags); > - clear_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > + clear_work_bit_irqsave(ctx); > dev->ctx[ctx->num] = ctx; > if (s5p_mfc_get_node_type(file) == MFCNODE_DECODER) { > ctx->type = MFCINST_DECODER; > @@ -843,7 +871,6 @@ static int s5p_mfc_release(struct file *file) > { > struct s5p_mfc_ctx *ctx = fh_to_ctx(file->private_data); > struct s5p_mfc_dev *dev = ctx->dev; > - unsigned long flags; > > mfc_debug_enter(); > mutex_lock(&dev->mfc_mutex); > @@ -851,17 +878,13 @@ static int s5p_mfc_release(struct file *file) > vb2_queue_release(&ctx->vq_src); > vb2_queue_release(&ctx->vq_dst); > /* Mark context as idle */ > - spin_lock_irqsave(&dev->condlock, flags); > - clear_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > + clear_work_bit_irqsave(ctx); > /* If instance was initialised then > * return instance and free reosurces */ > if (ctx->inst_no != MFC_NO_INSTANCE_SET) { > mfc_debug(2, "Has to free instance\n"); > ctx->state = MFCINST_RETURN_INST; > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > + set_work_bit_irqsave(ctx); > s5p_mfc_clean_ctx_int_flags(ctx); > s5p_mfc_try_run(dev); > /* Wait until instance is returned or timeout occured */ > diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_common.h > b/drivers/media/video/s5p-mfc/s5p_mfc_common.h > index 8871f0d..519b0d6 100644 > --- a/drivers/media/video/s5p-mfc/s5p_mfc_common.h > +++ b/drivers/media/video/s5p-mfc/s5p_mfc_common.h > @@ -570,4 +570,9 @@ struct mfc_control { > #define ctrl_to_ctx(__ctrl) \ > container_of((__ctrl)->handler, struct s5p_mfc_ctx, ctrl_handler) > > +void clear_work_bit(struct s5p_mfc_ctx *ctx); > +void set_work_bit(struct s5p_mfc_ctx *ctx); > +void clear_work_bit_irqsave(struct s5p_mfc_ctx *ctx); > +void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx); > + > #endif /* S5P_MFC_COMMON_H_ */ > diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_dec.c > b/drivers/media/video/s5p-mfc/s5p_mfc_dec.c > index 65be806..9c6309d 100644 > --- a/drivers/media/video/s5p-mfc/s5p_mfc_dec.c > +++ b/drivers/media/video/s5p-mfc/s5p_mfc_dec.c > @@ -415,7 +415,6 @@ static int vidioc_reqbufs(struct file *file, void *priv, > struct s5p_mfc_dev *dev = video_drvdata(file); > struct s5p_mfc_ctx *ctx = fh_to_ctx(priv); > int ret = 0; > - unsigned long flags; > > if (reqbufs->memory != V4L2_MEMORY_MMAP) { > mfc_err("Only V4L2_MEMORY_MAP is supported\n"); > @@ -497,11 +496,8 @@ static int vidioc_reqbufs(struct file *file, void *priv, > s5p_mfc_clock_off(); > return -ENOMEM; > } > - if (s5p_mfc_ctx_ready(ctx)) { > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > - } > + if (s5p_mfc_ctx_ready(ctx)) > + set_work_bit_irqsave(ctx); > s5p_mfc_try_run(dev); > s5p_mfc_wait_for_done_ctx(ctx, > S5P_FIMV_R2H_CMD_INIT_BUFFERS_RET, 0); > @@ -576,7 +572,6 @@ static int vidioc_streamon(struct file *file, void *priv, > { > struct s5p_mfc_ctx *ctx = fh_to_ctx(priv); > struct s5p_mfc_dev *dev = ctx->dev; > - unsigned long flags; > int ret = -EINVAL; > > mfc_debug_enter(); > @@ -589,9 +584,7 @@ static int vidioc_streamon(struct file *file, void *priv, > ctx->output_state = QUEUE_FREE; > s5p_mfc_alloc_instance_buffer(ctx); > s5p_mfc_alloc_dec_temp_buffers(ctx); > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > + set_work_bit_irqsave(ctx); > s5p_mfc_clean_ctx_int_flags(ctx); > s5p_mfc_try_run(dev); > > @@ -875,18 +868,14 @@ static int s5p_mfc_start_streaming(struct vb2_queue *q, > unsigned int count) > { > struct s5p_mfc_ctx *ctx = fh_to_ctx(q->drv_priv); > struct s5p_mfc_dev *dev = ctx->dev; > - unsigned long flags; > > v4l2_ctrl_handler_setup(&ctx->ctrl_handler); > if (ctx->state == MFCINST_FINISHING || > ctx->state == MFCINST_FINISHED) > ctx->state = MFCINST_RUNNING; > /* If context is ready then dev = work->data;schedule it to run */ > - if (s5p_mfc_ctx_ready(ctx)) { > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > - } > + if (s5p_mfc_ctx_ready(ctx)) > + set_work_bit_irqsave(ctx); > s5p_mfc_try_run(dev); > return 0; > } > @@ -953,11 +942,8 @@ static void s5p_mfc_buf_queue(struct vb2_buffer *vb) > } else { > mfc_err("Unsupported buffer type (%d)\n", vq->type); > } > - if (s5p_mfc_ctx_ready(ctx)) { > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > - } > + if (s5p_mfc_ctx_ready(ctx)) > + set_work_bit_irqsave(ctx); > s5p_mfc_try_run(dev); > } > > diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_enc.c > b/drivers/media/video/s5p-mfc/s5p_mfc_enc.c > index 602bc7d..53c305d 100644 > --- a/drivers/media/video/s5p-mfc/s5p_mfc_enc.c > +++ b/drivers/media/video/s5p-mfc/s5p_mfc_enc.c > @@ -643,11 +643,8 @@ static int enc_post_seq_start(struct s5p_mfc_ctx *ctx) > spin_unlock_irqrestore(&dev->irqlock, flags); > } > ctx->state = MFCINST_RUNNING; > - if (s5p_mfc_ctx_ready(ctx)) { > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > - } > + if (s5p_mfc_ctx_ready(ctx)) > + set_work_bit_irqsave(ctx); > s5p_mfc_try_run(dev); > return 0; > } > @@ -755,11 +752,8 @@ static int enc_post_frame_start(struct s5p_mfc_ctx *ctx) > vb2_buffer_done(mb_entry->b, VB2_BUF_STATE_DONE); > } > spin_unlock_irqrestore(&dev->irqlock, flags); > - if ((ctx->src_queue_cnt == 0) || (ctx->dst_queue_cnt == 0)) { > - spin_lock(&dev->condlock); > - clear_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock(&dev->condlock); > - } > + if ((ctx->src_queue_cnt == 0) || (ctx->dst_queue_cnt == 0)) > + clear_work_bit(ctx); > return 0; > } > > @@ -922,7 +916,6 @@ static int vidioc_s_fmt(struct file *file, void *priv, > struct v4l2_format *f) > struct s5p_mfc_ctx *ctx = fh_to_ctx(priv); > struct s5p_mfc_fmt *fmt; > struct v4l2_pix_format_mplane *pix_fmt_mp = &f->fmt.pix_mp; > - unsigned long flags; > int ret = 0; > > ret = vidioc_try_fmt(file, priv, f); > @@ -947,9 +940,7 @@ static int vidioc_s_fmt(struct file *file, void *priv, > struct v4l2_format *f) > ctx->dst_bufs_cnt = 0; > ctx->capture_state = QUEUE_FREE; > s5p_mfc_alloc_instance_buffer(ctx); > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > + set_work_bit_irqsave(ctx); > s5p_mfc_clean_ctx_int_flags(ctx); > s5p_mfc_try_run(dev); > if (s5p_mfc_wait_for_done_ctx(ctx, \ > @@ -1719,15 +1710,11 @@ static int s5p_mfc_start_streaming(struct vb2_queue *q, > unsigned int count) > { > struct s5p_mfc_ctx *ctx = fh_to_ctx(q->drv_priv); > struct s5p_mfc_dev *dev = ctx->dev; > - unsigned long flags; > > v4l2_ctrl_handler_setup(&ctx->ctrl_handler); > /* If context is ready then dev = work->data;schedule it to run */ > - if (s5p_mfc_ctx_ready(ctx)) { > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > - } > + if (s5p_mfc_ctx_ready(ctx)) > + set_work_bit_irqsave(ctx); > s5p_mfc_try_run(dev); > return 0; > } > @@ -1793,11 +1780,8 @@ static void s5p_mfc_buf_queue(struct vb2_buffer *vb) > } else { > mfc_err("unsupported buffer type (%d)\n", vq->type); > } > - if (s5p_mfc_ctx_ready(ctx)) { > - spin_lock_irqsave(&dev->condlock, flags); > - set_bit(ctx->num, &dev->ctx_work_bits); > - spin_unlock_irqrestore(&dev->condlock, flags); > - } > + if (s5p_mfc_ctx_ready(ctx)) > + set_work_bit_irqsave(ctx); > s5p_mfc_try_run(dev); > } > > -- > 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
