Hi Ricky, Thanks for your feedback. We will fix the problem in another patch. On Mon, 2016-12-12 at 12:34 +0800, Ricky Liang wrote: > Hi Rick, > > On Wed, Nov 30, 2016 at 11:08 AM, Rick Chang <rick.chang@xxxxxxxxxxxx> wrote: > > Add v4l2 driver for Mediatek JPEG Decoder > > > > Signed-off-by: Rick Chang <rick.chang@xxxxxxxxxxxx> > > Signed-off-by: Minghsiu Tsai <minghsiu.tsai@xxxxxxxxxxxx> > > <snip...> > > > +static bool mtk_jpeg_check_resolution_change(struct mtk_jpeg_ctx *ctx, > > + struct mtk_jpeg_dec_param *param) > > +{ > > + struct mtk_jpeg_dev *jpeg = ctx->jpeg; > > + struct mtk_jpeg_q_data *q_data; > > + > > + q_data = &ctx->out_q; > > + if (q_data->w != param->pic_w || q_data->h != param->pic_h) { > > + v4l2_dbg(1, debug, &jpeg->v4l2_dev, "Picture size change\n"); > > + return true; > > + } > > + > > + q_data = &ctx->cap_q; > > + if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc, > > + MTK_JPEG_FMT_TYPE_CAPTURE)) { > > + v4l2_dbg(1, debug, &jpeg->v4l2_dev, "format change\n"); > > + return true; > > + } > > + return false; > > <snip...> > > > +static void mtk_jpeg_device_run(void *priv) > > +{ > > + struct mtk_jpeg_ctx *ctx = priv; > > + struct mtk_jpeg_dev *jpeg = ctx->jpeg; > > + struct vb2_buffer *src_buf, *dst_buf; > > + enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR; > > + unsigned long flags; > > + struct mtk_jpeg_src_buf *jpeg_src_buf; > > + struct mtk_jpeg_bs bs; > > + struct mtk_jpeg_fb fb; > > + int i; > > + > > + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx); > > + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx); > > + jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(src_buf); > > + > > + if (jpeg_src_buf->flags & MTK_JPEG_BUF_FLAGS_LAST_FRAME) { > > + for (i = 0; i < dst_buf->num_planes; i++) > > + vb2_set_plane_payload(dst_buf, i, 0); > > + buf_state = VB2_BUF_STATE_DONE; > > + goto dec_end; > > + } > > + > > + if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) { > > + mtk_jpeg_queue_src_chg_event(ctx); > > + ctx->state = MTK_JPEG_SOURCE_CHANGE; > > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx); > > + return; > > + } > > This only detects source change if multiple OUPUT buffers are queued. > It does not catch the source change in the following scenario: > > - OUPUT buffers for jpeg1 enqueued > - OUTPUT queue STREAMON > - userspace creates CAPTURE buffers > - CAPTURE buffers enqueued > - CAPTURE queue STREAMON > - decode > - OUTPUT queue STREAMOFF > - userspace recreates OUTPUT buffers for jpeg2 > - OUTPUT buffers for jpeg2 enqueued > - OUTPUT queue STREAMON > > In the above sequence if jpeg2's decoded size is larger than jpeg1 the > function fails to detect that the existing CAPTURE buffers are not big > enough to hold the decoded data. > > A possible fix is to pass *dst_buf to > mtk_jpeg_check_resolution_change(), and check in the function that all > the dst_buf planes are large enough to hold the decoded data. > > > + > > + mtk_jpeg_set_dec_src(ctx, src_buf, &bs); > > + if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, dst_buf, &fb)) > > + goto dec_end; > > + > > + spin_lock_irqsave(&jpeg->hw_lock, flags); > > + mtk_jpeg_dec_reset(jpeg->dec_reg_base); > > + mtk_jpeg_dec_set_config(jpeg->dec_reg_base, > > + &jpeg_src_buf->dec_param, &bs, &fb); > > + > > + mtk_jpeg_dec_start(jpeg->dec_reg_base); > > + spin_unlock_irqrestore(&jpeg->hw_lock, flags); > > + return; > > + > > +dec_end: > > + v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx); > > + v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx); > > + v4l2_m2m_buf_done(to_vb2_v4l2_buffer(src_buf), buf_state); > > + v4l2_m2m_buf_done(to_vb2_v4l2_buffer(dst_buf), buf_state); > > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx); > > +} > > <snip...> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html