Hi Rick, Can you upload patchset v9 to address the issue? Thanks! On Mon, Dec 12, 2016 at 5:07 PM, Rick Chang <rick.chang@xxxxxxxxxxxx> wrote: > 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