Re: [PATCH v8 2/4] vcodec: mediatek: Add Mediatek JPEG Decoder Driver

2016-12-13 Thread Ricky Liang
Hi Rick,

Can you upload patchset v9 to address the issue? Thanks!

On Mon, Dec 12, 2016 at 5:07 PM, Rick Chang  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  wrote:
>> > Add v4l2 driver for Mediatek JPEG Decoder
>> >
>> > Signed-off-by: Rick Chang 
>> > Signed-off-by: Minghsiu Tsai 
>>
>> 
>>
>> > +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 = >out_q;
>> > +   if (q_data->w != param->pic_w || q_data->h != param->pic_h) {
>> > +   v4l2_dbg(1, debug, >v4l2_dev, "Picture size 
>> > change\n");
>> > +   return true;
>> > +   }
>> > +
>> > +   q_data = >cap_q;
>> > +   if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc,
>> > +   
>> > MTK_JPEG_FMT_TYPE_CAPTURE)) {
>> > +   v4l2_dbg(1, debug, >v4l2_dev, "format change\n");
>> > +   return true;
>> > +   }
>> > +   return false;
>>
>> 
>>
>> > +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, 
>> > _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, );
>> > +   if (mtk_jpeg_set_dec_dst(ctx, _src_buf->dec_param, dst_buf, 
>> > ))
>> > +   goto dec_end;
>> > +
>> > +   spin_lock_irqsave(>hw_lock, flags);
>> > +   mtk_jpeg_dec_reset(jpeg->dec_reg_base);
>> > +   mtk_jpeg_dec_set_config(jpeg->dec_reg_base,
>> > +   _src_buf->dec_param, , );
>> > +
>> > +   mtk_jpeg_dec_start(jpeg->dec_reg_base);
>> > +   spin_unlock_irqrestore(>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);
>> > +}
>>
>> 
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 2/4] vcodec: mediatek: Add Mediatek JPEG Decoder Driver

2016-12-12 Thread Rick Chang
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  wrote:
> > Add v4l2 driver for Mediatek JPEG Decoder
> >
> > Signed-off-by: Rick Chang 
> > Signed-off-by: Minghsiu Tsai 
> 
> 
> 
> > +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 = >out_q;
> > +   if (q_data->w != param->pic_w || q_data->h != param->pic_h) {
> > +   v4l2_dbg(1, debug, >v4l2_dev, "Picture size 
> > change\n");
> > +   return true;
> > +   }
> > +
> > +   q_data = >cap_q;
> > +   if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc,
> > +   MTK_JPEG_FMT_TYPE_CAPTURE)) 
> > {
> > +   v4l2_dbg(1, debug, >v4l2_dev, "format change\n");
> > +   return true;
> > +   }
> > +   return false;
> 
> 
> 
> > +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, 
> > _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, );
> > +   if (mtk_jpeg_set_dec_dst(ctx, _src_buf->dec_param, dst_buf, 
> > ))
> > +   goto dec_end;
> > +
> > +   spin_lock_irqsave(>hw_lock, flags);
> > +   mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > +   mtk_jpeg_dec_set_config(jpeg->dec_reg_base,
> > +   _src_buf->dec_param, , );
> > +
> > +   mtk_jpeg_dec_start(jpeg->dec_reg_base);
> > +   spin_unlock_irqrestore(>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);
> > +}
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 2/4] vcodec: mediatek: Add Mediatek JPEG Decoder Driver

2016-12-11 Thread Ricky Liang
Hi Rick,

On Wed, Nov 30, 2016 at 11:08 AM, Rick Chang  wrote:
> Add v4l2 driver for Mediatek JPEG Decoder
>
> Signed-off-by: Rick Chang 
> Signed-off-by: Minghsiu Tsai 



> +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 = >out_q;
> +   if (q_data->w != param->pic_w || q_data->h != param->pic_h) {
> +   v4l2_dbg(1, debug, >v4l2_dev, "Picture size change\n");
> +   return true;
> +   }
> +
> +   q_data = >cap_q;
> +   if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc,
> +   MTK_JPEG_FMT_TYPE_CAPTURE)) {
> +   v4l2_dbg(1, debug, >v4l2_dev, "format change\n");
> +   return true;
> +   }
> +   return false;



> +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, _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, );
> +   if (mtk_jpeg_set_dec_dst(ctx, _src_buf->dec_param, dst_buf, ))
> +   goto dec_end;
> +
> +   spin_lock_irqsave(>hw_lock, flags);
> +   mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> +   mtk_jpeg_dec_set_config(jpeg->dec_reg_base,
> +   _src_buf->dec_param, , );
> +
> +   mtk_jpeg_dec_start(jpeg->dec_reg_base);
> +   spin_unlock_irqrestore(>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);
> +}


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v8 2/4] vcodec: mediatek: Add Mediatek JPEG Decoder Driver

2016-11-29 Thread Rick Chang
Add v4l2 driver for Mediatek JPEG Decoder

Signed-off-by: Rick Chang 
Signed-off-by: Minghsiu Tsai 
---
 drivers/media/platform/Kconfig   |   15 +
 drivers/media/platform/Makefile  |2 +
 drivers/media/platform/mtk-jpeg/Makefile |2 +
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c  | 1302 ++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h  |  139 +++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c|  417 +++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h|   91 ++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.c |  160 +++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.h |   25 +
 drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h   |   58 +
 10 files changed, 2211 insertions(+)
 create mode 100644 drivers/media/platform/mtk-jpeg/Makefile
 create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
 create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
 create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c
 create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h
 create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.c
 create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_parse.h
 create mode 100644 drivers/media/platform/mtk-jpeg/mtk_jpeg_reg.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 754edbf1..96c9887 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -162,6 +162,21 @@ config VIDEO_CODA
   Coda is a range of video codec IPs that supports
   H.264, MPEG-4, and other video formats.
 
+config VIDEO_MEDIATEK_JPEG
+   tristate "Mediatek JPEG Codec driver"
+   depends on MTK_IOMMU_V1 || COMPILE_TEST
+   depends on VIDEO_DEV && VIDEO_V4L2
+   depends on ARCH_MEDIATEK || COMPILE_TEST
+   depends on HAS_DMA
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_MEM2MEM_DEV
+   ---help---
+ Mediatek jpeg codec driver provides HW capability to decode
+ JPEG format
+
+ To compile this driver as a module, choose M here: the
+ module will be called mtk-jpeg
+
 config VIDEO_MEDIATEK_VPU
tristate "Mediatek Video Processor Unit"
depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index f842933..cf701e3 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -68,3 +68,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_VPU)  += mtk-vpu/
 obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC)+= mtk-vcodec/
 
 obj-$(CONFIG_VIDEO_MEDIATEK_MDP)   += mtk-mdp/
+
+obj-$(CONFIG_VIDEO_MEDIATEK_JPEG)  += mtk-jpeg/
diff --git a/drivers/media/platform/mtk-jpeg/Makefile 
b/drivers/media/platform/mtk-jpeg/Makefile
new file mode 100644
index 000..b2e6069
--- /dev/null
+++ b/drivers/media/platform/mtk-jpeg/Makefile
@@ -0,0 +1,2 @@
+mtk_jpeg-objs := mtk_jpeg_core.o mtk_jpeg_hw.o mtk_jpeg_parse.o
+obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c 
b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
new file mode 100644
index 000..63a8994
--- /dev/null
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -0,0 +1,1302 @@
+/*
+ * Copyright (c) 2016 MediaTek Inc.
+ * Author: Ming Hsiu Tsai 
+ * Rick Chang 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "mtk_jpeg_hw.h"
+#include "mtk_jpeg_core.h"
+#include "mtk_jpeg_parse.h"
+
+static struct mtk_jpeg_fmt mtk_jpeg_formats[] = {
+   {
+   .fourcc = V4L2_PIX_FMT_JPEG,
+   .colplanes  = 1,
+   .flags  = MTK_JPEG_FMT_FLAG_DEC_OUTPUT,
+   },
+   {
+   .fourcc = V4L2_PIX_FMT_YUV420M,
+   .h_sample   = {4, 2, 2},
+   .v_sample   = {4, 2, 2},
+   .colplanes  = 3,
+   .h_align= 5,
+   .v_align= 4,
+   .flags  = MTK_JPEG_FMT_FLAG_DEC_CAPTURE,
+   },
+   {
+   .fourcc = V4L2_PIX_FMT_YUV422M,
+   .h_sample   = {4, 2, 2},
+   .v_sample   = {4, 4, 4},
+   .colplanes  = 3,
+   .h_align=