[PATCH V1, 0/6] Support multi-hardware jpeg decoding using of_platform_populate

[PATCH V1, 0/6] Support multi-hardware jpeg decoding using of_platform_populate

[kyrie.wu]

This series adds support for multi hardware jpeg decoding,
by first adding use of_platform_populate to manage each hardware
information: interrupt, clock, register bases and power.
Secondly add decoding work queue to deal with the decoding requests
of multi-hardware at the same time. Lastly, add output picture
reorder function interface to eliminate the out of order images.

This series has been tested with both MT8195.
Decoding worked for this chip.

Patches 1~2 use of_platform_populate to manage multi-hardware.

Patch 3 add jpeg decoding timeout function to judge hardware timeout.

Patch 4 add decoding work queue to deal with multi-hardware decoding
at the same time.

Patch 5 add output picture reorder function to order images.

Patch 6 refactor jpegdec func interface for HW working.
---
kyrie.wu (6):
  dt-bindings: mediatek: Add mediatek, mt8195-jpgdec compatible
  media: mtk-jpegdec: manage jpegdec multi-hardware
  media: mtk-jpegdec: add jpegdec timeout func interface
  media: mtk-jpegdec: add jpeg decode worker interface
  media: mtk-jpegdec: add output pic reorder interface
  media: mtk-jpegdec: refactor jpegdec func interface

 .../bindings/media/mediatek-jpeg-decoder.yaml      |   4 +
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c    | 295 ++++++++++++----
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h    |  64 ++++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c  | 369 +++++++++++++++++++--
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h  |   7 +-
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_reg.h |   1 +
 6 files changed, 640 insertions(+), 100 deletions(-)

-- 
2.6.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V1, 1/6] dt-bindings: mediatek: Add mediatek, mt8195-jpgdec compatible

[kyrie.wu]

Add mediatek, mt8195-jpgdec compatible to binding document

Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
---
 Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
index 9b87f03..cc7ec4d 100644
--- a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
@@ -19,6 +19,10 @@ properties:
           - enum:
               - mediatek,mt8173-jpgdec
               - mediatek,mt2701-jpgdec
+              - mediatek,mt8195-jpgdec
+              - mediatek,mt8195-jpgdec0
+              - mediatek,mt8195-jpgdec1
+              - mediatek,mt8195-jpgdec2
       - items:
           - enum:
               - mediatek,mt7623-jpgdec
-- 
2.6.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V1, 2/6] media: mtk-jpegdec: manage jpegdec multi-hardware

[kyrie.wu]

manage each hardware information, including irq/clk/power.

Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   |  77 +++----
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |  56 ++++++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c | 232 ++++++++++++++++++++++
 3 files changed, 318 insertions(+), 47 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index 9e89629..f2a5e84 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -1403,6 +1403,11 @@ static struct clk_bulk_data mtk_jpeg_clocks[] = {
 	{ .id = "jpgenc" },
 };
 
+static struct clk_bulk_data mtk_jpeg_dec_clocks[] = {
+	{ .id = "jpgdec-smi" },
+	{ .id = "jpgdec" },
+};
+
 static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
 {
 	struct device_node *node;
@@ -1460,8 +1465,6 @@ static inline void mtk_jpeg_clk_release(struct mtk_jpeg_dev *jpeg)
 static int mtk_jpeg_probe(struct platform_device *pdev)
 {
 	struct mtk_jpeg_dev *jpeg;
-	struct resource *res;
-	int jpeg_irq;
 	int ret;
 
 	jpeg = devm_kzalloc(&pdev->dev, sizeof(*jpeg), GFP_KERNEL);
@@ -1473,40 +1476,7 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 	jpeg->dev = &pdev->dev;
 	jpeg->variant = of_device_get_match_data(jpeg->dev);
 
-	if (!jpeg->variant->is_encoder) {
-		INIT_DELAYED_WORK(&jpeg->job_timeout_work,
-				mtk_jpeg_job_timeout_work);
-
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-		jpeg->reg_base = devm_ioremap_resource(&pdev->dev, res);
-		if (IS_ERR(jpeg->reg_base)) {
-			ret = PTR_ERR(jpeg->reg_base);
-			return ret;
-		}
-
-		jpeg_irq = platform_get_irq(pdev, 0);
-		if (jpeg_irq < 0) {
-			dev_err(&pdev->dev, "Failed to get jpeg_irq.\n");
-			return jpeg_irq;
-		}
-
-		ret = devm_request_irq(&pdev->dev, jpeg_irq,
-				       jpeg->variant->irq_handler,
-					   0, pdev->name, jpeg);
-		if (ret) {
-			dev_err(&pdev->dev, "Failed to request jpeg_irq %d (%d)\n",
-				jpeg_irq, ret);
-			goto err_req_irq;
-		}
-
-		ret = mtk_jpeg_clk_init(jpeg);
-		if (ret) {
-			dev_err(&pdev->dev, "Failed to init clk(%d)\n", ret);
-			goto err_clk_init;
-		}
-
-		pm_runtime_enable(&pdev->dev);
-	} else {
+	if (jpeg->variant->is_encoder) {
 		init_waitqueue_head(&jpeg->enc_hw_wq);
 		jpeg->workqueue = alloc_ordered_workqueue(MTK_JPEG_NAME,
 			WQ_MEM_RECLAIM | WQ_FREEZABLE);
@@ -1561,13 +1531,11 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 		  jpeg->variant->dev_name, jpeg->vdev->num,
 		  VIDEO_MAJOR, jpeg->vdev->minor);
 
-	if (jpeg->variant->is_encoder) {
-		ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
-			&pdev->dev);
-		if (ret) {
-			v4l2_err(&jpeg->v4l2_dev, "Master of platform populate failed.");
-			goto err_vfd_jpeg_register;
-		}
+	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
+		&pdev->dev);
+	if (ret) {
+		v4l2_err(&jpeg->v4l2_dev, "Master of platform populate failed.");
+		goto err_vfd_jpeg_register;
 	}
 
 	platform_set_drvdata(pdev, jpeg);
@@ -1586,12 +1554,8 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 err_dev_register:
 	mtk_jpeg_clk_release(jpeg);
 
-err_clk_init:
-
 err_alloc_workqueue:
 
-err_req_irq:
-
 	return ret;
 }
 
@@ -1697,6 +1661,20 @@ static const struct mtk_jpeg_variant mtk_jpegenc_drvdata = {
 	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
 };
 
+static const struct mtk_jpeg_variant mtk_jpegdec_drvdata = {
+	.is_encoder	= false,
+	.clks = mtk_jpeg_dec_clocks,
+	.num_clks = ARRAY_SIZE(mtk_jpeg_dec_clocks),
+	.formats = mtk_jpeg_dec_formats,
+	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
+	.qops = &mtk_jpeg_dec_qops,
+	.m2m_ops = &mtk_jpeg_dec_m2m_ops,
+	.dev_name = "mtk-jpeg-dec",
+	.ioctl_ops = &mtk_jpeg_dec_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
+};
+
 #if defined(CONFIG_OF)
 static const struct of_device_id mtk_jpeg_match[] = {
 	{
@@ -1715,6 +1693,10 @@ static const struct of_device_id mtk_jpeg_match[] = {
 		.compatible = "mediatek,mt8195-jpgenc",
 		.data = &mtk_jpegenc_drvdata,
 	},
+	{
+		.compatible = "mediatek,mt8195-jpgdec",
+		.data = &mtk_jpegdec_drvdata,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_jpeg_match);
@@ -1732,6 +1714,7 @@ static struct platform_driver mtk_jpeg_driver = {
 
 static struct platform_driver * const mtk_jpeg_source_drivers[] = {
 	&mtk_jpegenc_hw_driver,
+	&mtk_jpegdec_hw_driver,
 	&mtk_jpeg_driver,
 };
 static int __init mtk_jpeg_init(void)
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index f276221..20243d4 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
@@ -102,6 +102,13 @@ enum mtk_jpegenc_hw_id {
 	MTK_JPEGENC_HW_MAX,
 };
 
+enum mtk_jpegdec_hw_id {
+	MTK_JPEGDEC_HW0,
+	MTK_JPEGDEC_HW1,
+	MTK_JPEGDEC_HW2,
+	MTK_JPEGDEC_HW_MAX,
+};
+
 /**
  * struct mtk_jpegenc_clk_info - Structure used to store clock name
  */
@@ -151,6 +158,51 @@ struct mtk_jpegenc_comp_dev {
 };
 
 /**
+ * struct mtk_jpegdec_clk_info - Structure used to store clock name
+ */
+struct mtk_jpegdec_clk_info {
+	const char	*clk_name;
+	struct clk
+	*jpegdec_clk;
+};
+
+/**
+ * struct mtk_jpegdec_clk - Structure used to
+ * store vcodec clock information
+ */
+struct mtk_jpegdec_clk {
+	struct mtk_jpegdec_clk_info	*clk_info;
+	int	clk_num;
+};
+
+/**
+ * struct mtk_jpegdec_pm - Power management data structure
+ */
+struct mtk_jpegdec_pm {
+	struct mtk_jpegdec_clk	dec_clk;
+	struct device	*dev;
+	struct mtk_jpegdec_comp_dev	*mtkdev;
+};
+
+/**
+ * struct mtk_jpegdec_comp_dev - JPEG COREX abstraction
+ * @dev:		        JPEG device
+ * @plat_dev:		    platform device data
+ * @reg_base:		    JPEG registers mapping
+ * @master_dev:		    mtk_jpeg_dev device
+ * @pm:	                mtk_jpegdec_pm
+ * @jpegdec_irq:	    jpeg decode irq num
+ */
+struct mtk_jpegdec_comp_dev {
+	struct device *dev;
+	struct platform_device *plat_dev;
+	void __iomem *reg_base;
+	struct mtk_jpeg_dev *master_dev;
+	struct mtk_jpegdec_pm pm;
+	int jpegdec_irq;
+};
+
+/**
  * struct mtk_jpeg_dev - JPEG IP abstraction
  * @lock:		the mutex protecting this structure
  * @hw_lock:		spinlock protecting the hw device resource
@@ -182,6 +234,9 @@ struct mtk_jpeg_dev {
 	void __iomem *reg_encbase[MTK_JPEGENC_HW_MAX];
 	struct mtk_jpegenc_comp_dev *enc_hw_dev[MTK_JPEGENC_HW_MAX];
 	wait_queue_head_t enc_hw_wq;
+
+	void __iomem *reg_decbase[MTK_JPEGDEC_HW_MAX];
+	struct mtk_jpegdec_comp_dev *dec_hw_dev[MTK_JPEGDEC_HW_MAX];
 };
 
 /**
@@ -248,5 +303,6 @@ struct mtk_jpeg_ctx {
 };
 
 extern struct platform_driver mtk_jpegenc_hw_driver;
+extern struct platform_driver mtk_jpegdec_hw_driver;
 
 #endif /* _MTK_JPEG_CORE_H */
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
index 1e38522..86f12bd 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
@@ -5,9 +5,24 @@
  *         Rick Chang <rick.chang@mediatek.com>
  */
 
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <media/media-device.h>
 #include <media/videobuf2-core.h>
+#include <media/videobuf2-v4l2.h>
+#include <media/v4l2-mem2mem.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
 
 #include "mtk_jpeg_core.h"
 #include "mtk_jpeg_dec_hw.h"
@@ -24,6 +39,25 @@ enum mtk_jpeg_color {
 	MTK_JPEG_COLOR_400		= 0x00110000
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id mtk_jpegdec_hw_ids[] = {
+	{
+		.compatible = "mediatek,mt8195-jpgdec0",
+		.data = (void *)MTK_JPEGDEC_HW0,
+	},
+	{
+		.compatible = "mediatek,mt8195-jpgdec1",
+		.data = (void *)MTK_JPEGDEC_HW1,
+	},
+	{
+		.compatible = "mediatek,mt8195-jpgdec2",
+		.data = (void *)MTK_JPEGDEC_HW2,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mtk_jpegdec_hw_ids);
+#endif
+
 static inline int mtk_jpeg_verify_align(u32 val, int align, u32 reg)
 {
 	if (val & (align - 1)) {
@@ -408,3 +442,201 @@ void mtk_jpeg_dec_set_config(void __iomem *base,
 				   config->dma_last_mcu);
 	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
 }
+
+int mtk_jpegdec_init_pm(struct mtk_jpegdec_comp_dev *mtkdev)
+{
+	struct mtk_jpegdec_clk_info *clk_info;
+	struct mtk_jpegdec_clk *jpegdec_clk;
+	struct platform_device *pdev;
+	struct mtk_jpegdec_pm *pm;
+	int i, ret;
+
+	pdev = mtkdev->plat_dev;
+	pm = &mtkdev->pm;
+	pm->dev = &pdev->dev;
+	pm->mtkdev = mtkdev;
+	jpegdec_clk = &pm->dec_clk;
+	jpegdec_clk->clk_num =
+		of_property_count_strings(pdev->dev.of_node, "clock-names");
+	if (!jpegdec_clk->clk_num) {
+		dev_err(&pdev->dev, "Failed to get jpegenc clock count\n");
+		return -EINVAL;
+	}
+
+	jpegdec_clk->clk_info = devm_kcalloc(&pdev->dev,
+		jpegdec_clk->clk_num,
+		sizeof(*clk_info),
+		GFP_KERNEL);
+	if (!jpegdec_clk->clk_info)
+		return -ENOMEM;
+
+	for (i = 0; i < jpegdec_clk->clk_num; i++) {
+		clk_info = &jpegdec_clk->clk_info[i];
+		ret = of_property_read_string_index(pdev->dev.of_node,
+			"clock-names", i,
+			&clk_info->clk_name);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to get jpegenc clock name\n");
+			return ret;
+		}
+
+		clk_info->jpegdec_clk = devm_clk_get(&pdev->dev,
+			clk_info->clk_name);
+		if (IS_ERR(clk_info->jpegdec_clk)) {
+			dev_err(&pdev->dev, "devm_clk_get (%d)%s fail",
+				i, clk_info->clk_name);
+			return PTR_ERR(clk_info->jpegdec_clk);
+		}
+	}
+
+	pm_runtime_enable(&pdev->dev);
+
+	return ret;
+}
+
+static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
+{
+	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+	struct mtk_jpeg_src_buf *jpeg_src_buf;
+	enum vb2_buffer_state buf_state;
+	struct mtk_jpeg_ctx *ctx;
+	u32 dec_irq_ret;
+	u32 irq_status;
+	int i;
+
+	struct mtk_jpegdec_comp_dev *jpeg = priv;
+	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
+
+	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
+	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
+	if (dec_irq_ret >= MTK_JPEG_DEC_RESULT_UNDERFLOW)
+		mtk_jpeg_dec_reset(jpeg->reg_base);
+	if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE) {
+		dev_err(jpeg->dev, "decode failed\n");
+		goto dec_end;
+	}
+
+	ctx = v4l2_m2m_get_curr_priv(master_jpeg->m2m_dev);
+	if (!ctx) {
+		dev_err(jpeg->dev, "Context is NULL\n");
+		return IRQ_HANDLED;
+		}
+
+	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+	jpeg_src_buf =
+		container_of(src_buf, struct mtk_jpeg_src_buf, b);
+
+	for (i = 0; i < dst_buf->vb2_buf.num_planes; i++)
+		vb2_set_plane_payload(&dst_buf->vb2_buf, i,
+		jpeg_src_buf->dec_param.comp_size[i]);
+
+	buf_state = VB2_BUF_STATE_DONE;
+
+dec_end:
+	v4l2_m2m_buf_done(src_buf, buf_state);
+	v4l2_m2m_buf_done(dst_buf, buf_state);
+	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
+	pm_runtime_put(ctx->jpeg->dev);
+
+	return IRQ_HANDLED;
+}
+
+static int mtk_jpegdec_hw_init_irq(struct mtk_jpegdec_comp_dev *dev)
+{
+	struct platform_device *pdev = dev->plat_dev;
+	int ret;
+
+	dev->jpegdec_irq = platform_get_irq(pdev, 0);
+	if (dev->jpegdec_irq < 0) {
+		dev_err(&pdev->dev, "Failed to get irq resource");
+		return dev->jpegdec_irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, dev->jpegdec_irq,
+		mtk_jpegdec_hw_irq_handler, 0, pdev->name, dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to devm_request_irq %d (%d)",
+			dev->jpegdec_irq, ret);
+		return -ENOENT;
+	}
+
+	return 0;
+}
+
+void mtk_jpegdec_release_pm(struct mtk_jpegdec_comp_dev *mtkdev)
+{
+	struct platform_device *pdev = mtkdev->plat_dev;
+
+	pm_runtime_disable(&pdev->dev);
+}
+
+static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
+{
+	struct mtk_jpeg_dev *master_dev;
+	struct mtk_jpegdec_comp_dev *dev;
+	const struct of_device_id *of_id;
+	int ret, comp_idx;
+
+	struct device *decs = &pdev->dev;
+
+	if (!decs->parent)
+		return -EPROBE_DEFER;
+
+	master_dev = dev_get_drvdata(decs->parent);
+	if (!master_dev)
+		return -EPROBE_DEFER;
+
+	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	dev->plat_dev = pdev;
+	ret = mtk_jpegdec_init_pm(dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to get jpeg enc clock source");
+		return ret;
+	}
+
+	dev->reg_base =
+		devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(dev->reg_base)) {
+		ret = PTR_ERR(dev->reg_base);
+		goto err;
+	}
+
+	ret = mtk_jpegdec_hw_init_irq(dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register JPEGDEC irq handler.\n");
+		goto err;
+	}
+
+	of_id = of_match_device(mtk_jpegdec_hw_ids, decs);
+	if (!of_id) {
+		dev_err(&pdev->dev, "Can't get vdec comp device id.\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	comp_idx = (enum mtk_jpegdec_hw_id)of_id->data;
+	if (comp_idx < MTK_JPEGDEC_HW_MAX) {
+		master_dev->dec_hw_dev[comp_idx] = dev;
+		master_dev->reg_decbase[comp_idx] = dev->reg_base;
+		dev->master_dev = master_dev;
+	}
+
+	platform_set_drvdata(pdev, dev);
+	return 0;
+
+err:
+	mtk_jpegdec_release_pm(dev);
+	return ret;
+}
+
+struct platform_driver mtk_jpegdec_hw_driver = {
+	.probe	= mtk_jpegdec_hw_probe,
+	.driver	= {
+		.name	= "mtk-jpegdec-hw",
+		.of_match_table = mtk_jpegdec_hw_ids,
+	},
+};
-- 
2.6.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V1, 2/6] media: mtk-jpegdec: manage jpegdec multi-hardware

[AngeloGioacchino Del Regno]

Il 03/12/21 06:34, kyrie.wu ha scritto:
> manage each hardware information, including irq/clk/power.
> 
> Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
> ---
>   drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   |  77 +++----
>   drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |  56 ++++++
>   drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c | 232 ++++++++++++++++++++++
>   3 files changed, 318 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index 9e89629..f2a5e84 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -1403,6 +1403,11 @@ static struct clk_bulk_data mtk_jpeg_clocks[] = {
>   	{ .id = "jpgenc" },
>   };
>   
> +static struct clk_bulk_data mtk_jpeg_dec_clocks[] = {
> +	{ .id = "jpgdec-smi" },
> +	{ .id = "jpgdec" },
> +};
> +

This is the exact same thing as "mt8173_jpeg_dec_clocks", hence it's redundant and
should not be added.
Please reuse that one instead.


>   static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
>   {
>   	struct device_node *node;
> @@ -1460,8 +1465,6 @@ static inline void mtk_jpeg_clk_release(struct mtk_jpeg_dev *jpeg)
>   static int mtk_jpeg_probe(struct platform_device *pdev)
>   {
>   	struct mtk_jpeg_dev *jpeg;
> -	struct resource *res;
> -	int jpeg_irq;
>   	int ret;
>   
>   	jpeg = devm_kzalloc(&pdev->dev, sizeof(*jpeg), GFP_KERNEL);
> @@ -1473,40 +1476,7 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
>   	jpeg->dev = &pdev->dev;
>   	jpeg->variant = of_device_get_match_data(jpeg->dev);
>   
> -	if (!jpeg->variant->is_encoder) {
> -		INIT_DELAYED_WORK(&jpeg->job_timeout_work,
> -				mtk_jpeg_job_timeout_work);

You are removing this worker, so mt8173, mt8183 and the others will be broken.
Please make sure to not break older platforms.

> -
> -		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -		jpeg->reg_base = devm_ioremap_resource(&pdev->dev, res);
> -		if (IS_ERR(jpeg->reg_base)) {
> -			ret = PTR_ERR(jpeg->reg_base);
> -			return ret;
> -		}
> -
> -		jpeg_irq = platform_get_irq(pdev, 0);
> -		if (jpeg_irq < 0) {
> -			dev_err(&pdev->dev, "Failed to get jpeg_irq.\n");
> -			return jpeg_irq;
> -		}
> -
> -		ret = devm_request_irq(&pdev->dev, jpeg_irq,
> -				       jpeg->variant->irq_handler,
> -					   0, pdev->name, jpeg);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Failed to request jpeg_irq %d (%d)\n",
> -				jpeg_irq, ret);
> -			goto err_req_irq;
> -		}
> -
> -		ret = mtk_jpeg_clk_init(jpeg);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Failed to init clk(%d)\n", ret);
> -			goto err_clk_init;
> -		}
> -
> -		pm_runtime_enable(&pdev->dev);
> -	} else {
> +	if (jpeg->variant->is_encoder) {
>   		init_waitqueue_head(&jpeg->enc_hw_wq);
>   		jpeg->workqueue = alloc_ordered_workqueue(MTK_JPEG_NAME,
>   			WQ_MEM_RECLAIM | WQ_FREEZABLE);
> @@ -1561,13 +1531,11 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
>   		  jpeg->variant->dev_name, jpeg->vdev->num,
>   		  VIDEO_MAJOR, jpeg->vdev->minor);
>   
> -	if (jpeg->variant->is_encoder) {
> -		ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
> -			&pdev->dev);
> -		if (ret) {
> -			v4l2_err(&jpeg->v4l2_dev, "Master of platform populate failed.");
> -			goto err_vfd_jpeg_register;
> -		}
> +	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
> +		&pdev->dev);
> +	if (ret) {
> +		v4l2_err(&jpeg->v4l2_dev, "Master of platform populate failed.");
> +		goto err_vfd_jpeg_register;
>   	}
>   
>   	platform_set_drvdata(pdev, jpeg);
> @@ -1586,12 +1554,8 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
>   err_dev_register:
>   	mtk_jpeg_clk_release(jpeg);
>   
> -err_clk_init:
> -
>   err_alloc_workqueue:
>   
> -err_req_irq:
> -
>   	return ret;
>   }
>   
> @@ -1697,6 +1661,20 @@ static const struct mtk_jpeg_variant mtk_jpegenc_drvdata = {
>   	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
>   };
>   
> +static const struct mtk_jpeg_variant mtk_jpegdec_drvdata = {
> +	.is_encoder	= false,
> +	.clks = mtk_jpeg_dec_clocks,
> +	.num_clks = ARRAY_SIZE(mtk_jpeg_dec_clocks),
> +	.formats = mtk_jpeg_dec_formats,
> +	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
> +	.qops = &mtk_jpeg_dec_qops,
> +	.m2m_ops = &mtk_jpeg_dec_m2m_ops,
> +	.dev_name = "mtk-jpeg-dec",
> +	.ioctl_ops = &mtk_jpeg_dec_ioctl_ops,
> +	.out_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> +	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
> +};
> +
>   #if defined(CONFIG_OF)
>   static const struct of_device_id mtk_jpeg_match[] = {
>   	{
> @@ -1715,6 +1693,10 @@ static const struct of_device_id mtk_jpeg_match[] = {
>   		.compatible = "mediatek,mt8195-jpgenc",
>   		.data = &mtk_jpegenc_drvdata,
>   	},
> +	{
> +		.compatible = "mediatek,mt8195-jpgdec",
> +		.data = &mtk_jpegdec_drvdata,
> +	},
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, mtk_jpeg_match);
> @@ -1732,6 +1714,7 @@ static struct platform_driver mtk_jpeg_driver = {
>   
>   static struct platform_driver * const mtk_jpeg_source_drivers[] = {
>   	&mtk_jpegenc_hw_driver,
> +	&mtk_jpegdec_hw_driver,
>   	&mtk_jpeg_driver,
>   };
>   static int __init mtk_jpeg_init(void)
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> index f276221..20243d4 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> @@ -102,6 +102,13 @@ enum mtk_jpegenc_hw_id {
>   	MTK_JPEGENC_HW_MAX,
>   };
>   
> +enum mtk_jpegdec_hw_id {
> +	MTK_JPEGDEC_HW0,
> +	MTK_JPEGDEC_HW1,
> +	MTK_JPEGDEC_HW2,
> +	MTK_JPEGDEC_HW_MAX,
> +};
> +
>   /**
>    * struct mtk_jpegenc_clk_info - Structure used to store clock name
>    */
> @@ -151,6 +158,51 @@ struct mtk_jpegenc_comp_dev {
>   };
>   
>   /**
> + * struct mtk_jpegdec_clk_info - Structure used to store clock name
> + */
> +struct mtk_jpegdec_clk_info {
> +	const char	*clk_name;
> +	struct clk
> +	*jpegdec_clk;
> +};
> +
> +/**
> + * struct mtk_jpegdec_clk - Structure used to
> + * store vcodec clock information
> + */
> +struct mtk_jpegdec_clk {
> +	struct mtk_jpegdec_clk_info	*clk_info;
> +	int	clk_num;
> +};
> +
> +/**
> + * struct mtk_jpegdec_pm - Power management data structure
> + */
> +struct mtk_jpegdec_pm {
> +	struct mtk_jpegdec_clk	dec_clk;
> +	struct device	*dev;
> +	struct mtk_jpegdec_comp_dev	*mtkdev;
> +};
> +
> +/**
> + * struct mtk_jpegdec_comp_dev - JPEG COREX abstraction
> + * @dev:		        JPEG device
> + * @plat_dev:		    platform device data
> + * @reg_base:		    JPEG registers mapping
> + * @master_dev:		    mtk_jpeg_dev device
> + * @pm:	                mtk_jpegdec_pm
> + * @jpegdec_irq:	    jpeg decode irq num
> + */
> +struct mtk_jpegdec_comp_dev {
> +	struct device *dev;
> +	struct platform_device *plat_dev;
> +	void __iomem *reg_base;
> +	struct mtk_jpeg_dev *master_dev;
> +	struct mtk_jpegdec_pm pm;
> +	int jpegdec_irq;
> +};
> +
> +/**
>    * struct mtk_jpeg_dev - JPEG IP abstraction
>    * @lock:		the mutex protecting this structure
>    * @hw_lock:		spinlock protecting the hw device resource
> @@ -182,6 +234,9 @@ struct mtk_jpeg_dev {
>   	void __iomem *reg_encbase[MTK_JPEGENC_HW_MAX];
>   	struct mtk_jpegenc_comp_dev *enc_hw_dev[MTK_JPEGENC_HW_MAX];
>   	wait_queue_head_t enc_hw_wq;
> +
> +	void __iomem *reg_decbase[MTK_JPEGDEC_HW_MAX];
> +	struct mtk_jpegdec_comp_dev *dec_hw_dev[MTK_JPEGDEC_HW_MAX];
>   };
>   
>   /**
> @@ -248,5 +303,6 @@ struct mtk_jpeg_ctx {
>   };
>   
>   extern struct platform_driver mtk_jpegenc_hw_driver;
> +extern struct platform_driver mtk_jpegdec_hw_driver;
>   
>   #endif /* _MTK_JPEG_CORE_H */
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> index 1e38522..86f12bd 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> @@ -5,9 +5,24 @@
>    *         Rick Chang <rick.chang@mediatek.com>
>    */
>   
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
>   #include <linux/io.h>
>   #include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <media/media-device.h>
>   #include <media/videobuf2-core.h>
> +#include <media/videobuf2-v4l2.h>
> +#include <media/v4l2-mem2mem.h>
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-fh.h>
> +#include <media/v4l2-event.h>
>   
>   #include "mtk_jpeg_core.h"
>   #include "mtk_jpeg_dec_hw.h"
> @@ -24,6 +39,25 @@ enum mtk_jpeg_color {
>   	MTK_JPEG_COLOR_400		= 0x00110000
>   };
>   
> +#if defined(CONFIG_OF)
> +static const struct of_device_id mtk_jpegdec_hw_ids[] = {
> +	{
> +		.compatible = "mediatek,mt8195-jpgdec0",
> +		.data = (void *)MTK_JPEGDEC_HW0,
> +	},
> +	{
> +		.compatible = "mediatek,mt8195-jpgdec1",
> +		.data = (void *)MTK_JPEGDEC_HW1,
> +	},
> +	{
> +		.compatible = "mediatek,mt8195-jpgdec2",
> +		.data = (void *)MTK_JPEGDEC_HW2,
> +	},
> +	{},
> +};

Please look at the comment/proposal that I've made on the jpeg encoder
series: the same applies here to the decoder part.
So, in this case, it'd be just "mediatek,mt8195-jpgdec-hw".

> +MODULE_DEVICE_TABLE(of, mtk_jpegdec_hw_ids);
> +#endif
> +
>   static inline int mtk_jpeg_verify_align(u32 val, int align, u32 reg)
>   {
>   	if (val & (align - 1)) {
> @@ -408,3 +442,201 @@ void mtk_jpeg_dec_set_config(void __iomem *base,
>   				   config->dma_last_mcu);
>   	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
>   }
> +
> +int mtk_jpegdec_init_pm(struct mtk_jpegdec_comp_dev *mtkdev)
> +{
> +	struct mtk_jpegdec_clk_info *clk_info;
> +	struct mtk_jpegdec_clk *jpegdec_clk;
> +	struct platform_device *pdev;
> +	struct mtk_jpegdec_pm *pm;
> +	int i, ret;
> +
> +	pdev = mtkdev->plat_dev;
> +	pm = &mtkdev->pm;
> +	pm->dev = &pdev->dev;
> +	pm->mtkdev = mtkdev;
> +	jpegdec_clk = &pm->dec_clk;
> +	jpegdec_clk->clk_num =
> +		of_property_count_strings(pdev->dev.of_node, "clock-names");
> +	if (!jpegdec_clk->clk_num) {
> +		dev_err(&pdev->dev, "Failed to get jpegenc clock count\n");
> +		return -EINVAL;
> +	}
> +
> +	jpegdec_clk->clk_info = devm_kcalloc(&pdev->dev,
> +		jpegdec_clk->clk_num,
> +		sizeof(*clk_info),
> +		GFP_KERNEL);
> +	if (!jpegdec_clk->clk_info)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < jpegdec_clk->clk_num; i++) {
> +		clk_info = &jpegdec_clk->clk_info[i];
> +		ret = of_property_read_string_index(pdev->dev.of_node,
> +			"clock-names", i,
> +			&clk_info->clk_name);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to get jpegenc clock name\n");
> +			return ret;
> +		}
> +
> +		clk_info->jpegdec_clk = devm_clk_get(&pdev->dev,
> +			clk_info->clk_name);
> +		if (IS_ERR(clk_info->jpegdec_clk)) {
> +			dev_err(&pdev->dev, "devm_clk_get (%d)%s fail",
> +				i, clk_info->clk_name);
> +			return PTR_ERR(clk_info->jpegdec_clk);
> +		}
> +	}
> +
> +	pm_runtime_enable(&pdev->dev);

To shorten the code, you should call pm_runtime_enable() at the end of the
probe function instead... check the comments below.

> +
> +	return ret;
> +}
> +
> +static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> +{
> +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +	struct mtk_jpeg_src_buf *jpeg_src_buf;
> +	enum vb2_buffer_state buf_state;
> +	struct mtk_jpeg_ctx *ctx;
> +	u32 dec_irq_ret;
> +	u32 irq_status;
> +	int i;
> +
> +	struct mtk_jpegdec_comp_dev *jpeg = priv;
> +	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
> +
> +	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
> +	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
> +	if (dec_irq_ret >= MTK_JPEG_DEC_RESULT_UNDERFLOW)
> +		mtk_jpeg_dec_reset(jpeg->reg_base);
> +	if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE) {
> +		dev_err(jpeg->dev, "decode failed\n");
> +		goto dec_end;
> +	}
> +
> +	ctx = v4l2_m2m_get_curr_priv(master_jpeg->m2m_dev);
> +	if (!ctx) {
> +		dev_err(jpeg->dev, "Context is NULL\n");
> +		return IRQ_HANDLED;
> +		}
> +
> +	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> +	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> +	jpeg_src_buf =
> +		container_of(src_buf, struct mtk_jpeg_src_buf, b);
> +
> +	for (i = 0; i < dst_buf->vb2_buf.num_planes; i++)
> +		vb2_set_plane_payload(&dst_buf->vb2_buf, i,
> +		jpeg_src_buf->dec_param.comp_size[i]);
> +
> +	buf_state = VB2_BUF_STATE_DONE;
> +
> +dec_end:
> +	v4l2_m2m_buf_done(src_buf, buf_state);
> +	v4l2_m2m_buf_done(dst_buf, buf_state);
> +	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
> +	pm_runtime_put(ctx->jpeg->dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mtk_jpegdec_hw_init_irq(struct mtk_jpegdec_comp_dev *dev)
> +{
> +	struct platform_device *pdev = dev->plat_dev;
> +	int ret;
> +
> +	dev->jpegdec_irq = platform_get_irq(pdev, 0);
> +	if (dev->jpegdec_irq < 0) {
> +		dev_err(&pdev->dev, "Failed to get irq resource");
> +		return dev->jpegdec_irq;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, dev->jpegdec_irq,
> +		mtk_jpegdec_hw_irq_handler, 0, pdev->name, dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to devm_request_irq %d (%d)",
> +			dev->jpegdec_irq, ret);
> +		return -ENOENT;
> +	}
> +
> +	return 0;
> +}
> +
> +void mtk_jpegdec_release_pm(struct mtk_jpegdec_comp_dev *mtkdev)
> +{
> +	struct platform_device *pdev = mtkdev->plat_dev;
> +

You're using this function only in this file... and then, this is just one line.
This helper is not needed: please simply call pm_runtime_disable where needed.

> +	pm_runtime_disable(&pdev->dev);
> +}
> +
> +static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
> +{
> +	struct mtk_jpeg_dev *master_dev;
> +	struct mtk_jpegdec_comp_dev *dev;
> +	const struct of_device_id *of_id;
> +	int ret, comp_idx;
> +
> +	struct device *decs = &pdev->dev;
> +
> +	if (!decs->parent)
> +		return -EPROBE_DEFER;
> +
> +	master_dev = dev_get_drvdata(decs->parent);
> +	if (!master_dev)
> +		return -EPROBE_DEFER;
> +
> +	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	dev->plat_dev = pdev;
> +	ret = mtk_jpegdec_init_pm(dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to get jpeg enc clock source");

	if (ret)
		return dev_err_probe(&pdev->dev, ret, ".....blurb");

That's shorter, and it is recommended to use dev_err_probe() even if the
function cannot return -EPROBE_DEFER.

> +		return ret;
> +	}
> +
> +	dev->reg_base =
> +		devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(dev->reg_base)) {

By moving the pm_runtime_enable() call to the end of the probe function, you
won't need this goto, hence you'll be able to simply

		return PTR_ERR(dev->reg_base);

> +		ret = PTR_ERR(dev->reg_base);
> +		goto err;
> +	}
> +
> +	ret = mtk_jpegdec_hw_init_irq(dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to register JPEGDEC irq handler.\n");

...and it's the same here, with dev_err_probe().

> +		goto err;
> +	}
> +
> +	of_id = of_match_device(mtk_jpegdec_hw_ids, decs);
> +	if (!of_id) {
> +		dev_err(&pdev->dev, "Can't get vdec comp device id.\n");

.... and here too, but if you follow what I said in the encoder review, this call
may not even be necessary, as you'd be simply checking if "mediatek,hw-leader" is
true (meaning that it's device 0), otherwise it's device number x (where x > 0).

> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	comp_idx = (enum mtk_jpegdec_hw_id)of_id->data;
> +	if (comp_idx < MTK_JPEGDEC_HW_MAX) {
> +		master_dev->dec_hw_dev[comp_idx] = dev;
> +		master_dev->reg_decbase[comp_idx] = dev->reg_base;
> +		dev->master_dev = master_dev;
> +	}
> +

	pm_runtime_enable(&pdev->dev);

> +	platform_set_drvdata(pdev, dev);
> +	return 0;
> +
> +err:
> +	mtk_jpegdec_release_pm(dev);
> +	return ret;
> +}
> +
> +struct platform_driver mtk_jpegdec_hw_driver = {
> +	.probe	= mtk_jpegdec_hw_probe,
> +	.driver	= {
> +		.name	= "mtk-jpegdec-hw",
> +		.of_match_table = mtk_jpegdec_hw_ids,
> +	},
> +};

Regards,
Angelo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V1, 2/6] media: mtk-jpegdec: manage jpegdec multi-hardware

[kyrie.wu]

On Mon, 2022-02-07 at 15:50 +0100, AngeloGioacchino Del Regno wrote:
> Il 03/12/21 06:34, kyrie.wu ha scritto:
> > manage each hardware information, including irq/clk/power.
> > 
> > Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
> > ---
> >   drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   |  77 +++----
> >   drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |  56 ++++++
> >   drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c | 232
> > ++++++++++++++++++++++
> >   3 files changed, 318 insertions(+), 47 deletions(-)
> > 
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 9e89629..f2a5e84 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -1403,6 +1403,11 @@ static struct clk_bulk_data
> > mtk_jpeg_clocks[] = {
> >   	{ .id = "jpgenc" },
> >   };
> >   
> > +static struct clk_bulk_data mtk_jpeg_dec_clocks[] = {
> > +	{ .id = "jpgdec-smi" },
> > +	{ .id = "jpgdec" },
> > +};
> > +
> 
> This is the exact same thing as "mt8173_jpeg_dec_clocks", hence it's
> redundant and
> should not be added.
> Please reuse that one instead.
Thanks for your reminding, I will reconstruct my patch.
> 
> 
> >   static int mtk_jpeg_clk_init(struct mtk_jpeg_dev *jpeg)
> >   {
> >   	struct device_node *node;
> > @@ -1460,8 +1465,6 @@ static inline void
> > mtk_jpeg_clk_release(struct mtk_jpeg_dev *jpeg)
> >   static int mtk_jpeg_probe(struct platform_device *pdev)
> >   {
> >   	struct mtk_jpeg_dev *jpeg;
> > -	struct resource *res;
> > -	int jpeg_irq;
> >   	int ret;
> >   
> >   	jpeg = devm_kzalloc(&pdev->dev, sizeof(*jpeg), GFP_KERNEL);
> > @@ -1473,40 +1476,7 @@ static int mtk_jpeg_probe(struct
> > platform_device *pdev)
> >   	jpeg->dev = &pdev->dev;
> >   	jpeg->variant = of_device_get_match_data(jpeg->dev);
> >   
> > -	if (!jpeg->variant->is_encoder) {
> > -		INIT_DELAYED_WORK(&jpeg->job_timeout_work,
> > -				mtk_jpeg_job_timeout_work);
> 
> You are removing this worker, so mt8173, mt8183 and the others will
> be broken.
> Please make sure to not break older platforms.
Thanks. I will make sure it.
> 
> > -
> > -		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -		jpeg->reg_base = devm_ioremap_resource(&pdev->dev,
> > res);
> > -		if (IS_ERR(jpeg->reg_base)) {
> > -			ret = PTR_ERR(jpeg->reg_base);
> > -			return ret;
> > -		}
> > -
> > -		jpeg_irq = platform_get_irq(pdev, 0);
> > -		if (jpeg_irq < 0) {
> > -			dev_err(&pdev->dev, "Failed to get
> > jpeg_irq.\n");
> > -			return jpeg_irq;
> > -		}
> > -
> > -		ret = devm_request_irq(&pdev->dev, jpeg_irq,
> > -				       jpeg->variant->irq_handler,
> > -					   0, pdev->name, jpeg);
> > -		if (ret) {
> > -			dev_err(&pdev->dev, "Failed to request jpeg_irq
> > %d (%d)\n",
> > -				jpeg_irq, ret);
> > -			goto err_req_irq;
> > -		}
> > -
> > -		ret = mtk_jpeg_clk_init(jpeg);
> > -		if (ret) {
> > -			dev_err(&pdev->dev, "Failed to init clk(%d)\n",
> > ret);
> > -			goto err_clk_init;
> > -		}
> > -
> > -		pm_runtime_enable(&pdev->dev);
> > -	} else {
> > +	if (jpeg->variant->is_encoder) {
> >   		init_waitqueue_head(&jpeg->enc_hw_wq);
> >   		jpeg->workqueue =
> > alloc_ordered_workqueue(MTK_JPEG_NAME,
> >   			WQ_MEM_RECLAIM | WQ_FREEZABLE);
> > @@ -1561,13 +1531,11 @@ static int mtk_jpeg_probe(struct
> > platform_device *pdev)
> >   		  jpeg->variant->dev_name, jpeg->vdev->num,
> >   		  VIDEO_MAJOR, jpeg->vdev->minor);
> >   
> > -	if (jpeg->variant->is_encoder) {
> > -		ret = of_platform_populate(pdev->dev.of_node, NULL,
> > NULL,
> > -			&pdev->dev);
> > -		if (ret) {
> > -			v4l2_err(&jpeg->v4l2_dev, "Master of platform
> > populate failed.");
> > -			goto err_vfd_jpeg_register;
> > -		}
> > +	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
> > +		&pdev->dev);
> > +	if (ret) {
> > +		v4l2_err(&jpeg->v4l2_dev, "Master of platform populate
> > failed.");
> > +		goto err_vfd_jpeg_register;
> >   	}
> >   
> >   	platform_set_drvdata(pdev, jpeg);
> > @@ -1586,12 +1554,8 @@ static int mtk_jpeg_probe(struct
> > platform_device *pdev)
> >   err_dev_register:
> >   	mtk_jpeg_clk_release(jpeg);
> >   
> > -err_clk_init:
> > -
> >   err_alloc_workqueue:
> >   
> > -err_req_irq:
> > -
> >   	return ret;
> >   }
> >   
> > @@ -1697,6 +1661,20 @@ static const struct mtk_jpeg_variant
> > mtk_jpegenc_drvdata = {
> >   	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> >   };
> >   
> > +static const struct mtk_jpeg_variant mtk_jpegdec_drvdata = {
> > +	.is_encoder	= false,
> > +	.clks = mtk_jpeg_dec_clocks,
> > +	.num_clks = ARRAY_SIZE(mtk_jpeg_dec_clocks),
> > +	.formats = mtk_jpeg_dec_formats,
> > +	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
> > +	.qops = &mtk_jpeg_dec_qops,
> > +	.m2m_ops = &mtk_jpeg_dec_m2m_ops,
> > +	.dev_name = "mtk-jpeg-dec",
> > +	.ioctl_ops = &mtk_jpeg_dec_ioctl_ops,
> > +	.out_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> > +	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
> > +};
> > +
> >   #if defined(CONFIG_OF)
> >   static const struct of_device_id mtk_jpeg_match[] = {
> >   	{
> > @@ -1715,6 +1693,10 @@ static const struct of_device_id
> > mtk_jpeg_match[] = {
> >   		.compatible = "mediatek,mt8195-jpgenc",
> >   		.data = &mtk_jpegenc_drvdata,
> >   	},
> > +	{
> > +		.compatible = "mediatek,mt8195-jpgdec",
> > +		.data = &mtk_jpegdec_drvdata,
> > +	},
> >   	{},
> >   };
> >   MODULE_DEVICE_TABLE(of, mtk_jpeg_match);
> > @@ -1732,6 +1714,7 @@ static struct platform_driver mtk_jpeg_driver
> > = {
> >   
> >   static struct platform_driver * const mtk_jpeg_source_drivers[] =
> > {
> >   	&mtk_jpegenc_hw_driver,
> > +	&mtk_jpegdec_hw_driver,
> >   	&mtk_jpeg_driver,
> >   };
> >   static int __init mtk_jpeg_init(void)
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index f276221..20243d4 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -102,6 +102,13 @@ enum mtk_jpegenc_hw_id {
> >   	MTK_JPEGENC_HW_MAX,
> >   };
> >   
> > +enum mtk_jpegdec_hw_id {
> > +	MTK_JPEGDEC_HW0,
> > +	MTK_JPEGDEC_HW1,
> > +	MTK_JPEGDEC_HW2,
> > +	MTK_JPEGDEC_HW_MAX,
> > +};
> > +
> >   /**
> >    * struct mtk_jpegenc_clk_info - Structure used to store clock
> > name
> >    */
> > @@ -151,6 +158,51 @@ struct mtk_jpegenc_comp_dev {
> >   };
> >   
> >   /**
> > + * struct mtk_jpegdec_clk_info - Structure used to store clock
> > name
> > + */
> > +struct mtk_jpegdec_clk_info {
> > +	const char	*clk_name;
> > +	struct clk
> > +	*jpegdec_clk;
> > +};
> > +
> > +/**
> > + * struct mtk_jpegdec_clk - Structure used to
> > + * store vcodec clock information
> > + */
> > +struct mtk_jpegdec_clk {
> > +	struct mtk_jpegdec_clk_info	*clk_info;
> > +	int	clk_num;
> > +};
> > +
> > +/**
> > + * struct mtk_jpegdec_pm - Power management data structure
> > + */
> > +struct mtk_jpegdec_pm {
> > +	struct mtk_jpegdec_clk	dec_clk;
> > +	struct device	*dev;
> > +	struct mtk_jpegdec_comp_dev	*mtkdev;
> > +};
> > +
> > +/**
> > + * struct mtk_jpegdec_comp_dev - JPEG COREX abstraction
> > + * @dev:		        JPEG device
> > + * @plat_dev:		    platform device data
> > + * @reg_base:		    JPEG registers mapping
> > + * @master_dev:		    mtk_jpeg_dev device
> > + * @pm:	                mtk_jpegdec_pm
> > + * @jpegdec_irq:	    jpeg decode irq num
> > + */
> > +struct mtk_jpegdec_comp_dev {
> > +	struct device *dev;
> > +	struct platform_device *plat_dev;
> > +	void __iomem *reg_base;
> > +	struct mtk_jpeg_dev *master_dev;
> > +	struct mtk_jpegdec_pm pm;
> > +	int jpegdec_irq;
> > +};
> > +
> > +/**
> >    * struct mtk_jpeg_dev - JPEG IP abstraction
> >    * @lock:		the mutex protecting this structure
> >    * @hw_lock:		spinlock protecting the hw device
> > resource
> > @@ -182,6 +234,9 @@ struct mtk_jpeg_dev {
> >   	void __iomem *reg_encbase[MTK_JPEGENC_HW_MAX];
> >   	struct mtk_jpegenc_comp_dev *enc_hw_dev[MTK_JPEGENC_HW_MAX];
> >   	wait_queue_head_t enc_hw_wq;
> > +
> > +	void __iomem *reg_decbase[MTK_JPEGDEC_HW_MAX];
> > +	struct mtk_jpegdec_comp_dev *dec_hw_dev[MTK_JPEGDEC_HW_MAX];
> >   };
> >   
> >   /**
> > @@ -248,5 +303,6 @@ struct mtk_jpeg_ctx {
> >   };
> >   
> >   extern struct platform_driver mtk_jpegenc_hw_driver;
> > +extern struct platform_driver mtk_jpegdec_hw_driver;
> >   
> >   #endif /* _MTK_JPEG_CORE_H */
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> > index 1e38522..86f12bd 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> > @@ -5,9 +5,24 @@
> >    *         Rick Chang <rick.chang@mediatek.com>
> >    */
> >   
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irq.h>
> >   #include <linux/io.h>
> >   #include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/slab.h>
> > +#include <media/media-device.h>
> >   #include <media/videobuf2-core.h>
> > +#include <media/videobuf2-v4l2.h>
> > +#include <media/v4l2-mem2mem.h>
> > +#include <media/v4l2-dev.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-fh.h>
> > +#include <media/v4l2-event.h>
> >   
> >   #include "mtk_jpeg_core.h"
> >   #include "mtk_jpeg_dec_hw.h"
> > @@ -24,6 +39,25 @@ enum mtk_jpeg_color {
> >   	MTK_JPEG_COLOR_400		= 0x00110000
> >   };
> >   
> > +#if defined(CONFIG_OF)
> > +static const struct of_device_id mtk_jpegdec_hw_ids[] = {
> > +	{
> > +		.compatible = "mediatek,mt8195-jpgdec0",
> > +		.data = (void *)MTK_JPEGDEC_HW0,
> > +	},
> > +	{
> > +		.compatible = "mediatek,mt8195-jpgdec1",
> > +		.data = (void *)MTK_JPEGDEC_HW1,
> > +	},
> > +	{
> > +		.compatible = "mediatek,mt8195-jpgdec2",
> > +		.data = (void *)MTK_JPEGDEC_HW2,
> > +	},
> > +	{},
> > +};
> 
> Please look at the comment/proposal that I've made on the jpeg
> encoder
> series: the same applies here to the decoder part.
> So, in this case, it'd be just "mediatek,mt8195-jpgdec-hw".
Thanks. I will repair this code.
> 
> > +MODULE_DEVICE_TABLE(of, mtk_jpegdec_hw_ids);
> > +#endif
> > +
> >   static inline int mtk_jpeg_verify_align(u32 val, int align, u32
> > reg)
> >   {
> >   	if (val & (align - 1)) {
> > @@ -408,3 +442,201 @@ void mtk_jpeg_dec_set_config(void __iomem
> > *base,
> >   				   config->dma_last_mcu);
> >   	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
> >   }
> > +
> > +int mtk_jpegdec_init_pm(struct mtk_jpegdec_comp_dev *mtkdev)
> > +{
> > +	struct mtk_jpegdec_clk_info *clk_info;
> > +	struct mtk_jpegdec_clk *jpegdec_clk;
> > +	struct platform_device *pdev;
> > +	struct mtk_jpegdec_pm *pm;
> > +	int i, ret;
> > +
> > +	pdev = mtkdev->plat_dev;
> > +	pm = &mtkdev->pm;
> > +	pm->dev = &pdev->dev;
> > +	pm->mtkdev = mtkdev;
> > +	jpegdec_clk = &pm->dec_clk;
> > +	jpegdec_clk->clk_num =
> > +		of_property_count_strings(pdev->dev.of_node, "clock-
> > names");
> > +	if (!jpegdec_clk->clk_num) {
> > +		dev_err(&pdev->dev, "Failed to get jpegenc clock
> > count\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	jpegdec_clk->clk_info = devm_kcalloc(&pdev->dev,
> > +		jpegdec_clk->clk_num,
> > +		sizeof(*clk_info),
> > +		GFP_KERNEL);
> > +	if (!jpegdec_clk->clk_info)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < jpegdec_clk->clk_num; i++) {
> > +		clk_info = &jpegdec_clk->clk_info[i];
> > +		ret = of_property_read_string_index(pdev->dev.of_node,
> > +			"clock-names", i,
> > +			&clk_info->clk_name);
> > +		if (ret) {
> > +			dev_err(&pdev->dev, "Failed to get jpegenc
> > clock name\n");
> > +			return ret;
> > +		}
> > +
> > +		clk_info->jpegdec_clk = devm_clk_get(&pdev->dev,
> > +			clk_info->clk_name);
> > +		if (IS_ERR(clk_info->jpegdec_clk)) {
> > +			dev_err(&pdev->dev, "devm_clk_get (%d)%s fail",
> > +				i, clk_info->clk_name);
> > +			return PTR_ERR(clk_info->jpegdec_clk);
> > +		}
> > +	}
> > +
> > +	pm_runtime_enable(&pdev->dev);
> 
> To shorten the code, you should call pm_runtime_enable() at the end
> of the
> probe function instead... check the comments below.
Dear AngeloGioacchino,
Thanks. Your suggestion would be took in the version.
> 
> > +
> > +	return ret;
> > +}
> > +
> > +static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> > +{
> > +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > +	struct mtk_jpeg_src_buf *jpeg_src_buf;
> > +	enum vb2_buffer_state buf_state;
> > +	struct mtk_jpeg_ctx *ctx;
> > +	u32 dec_irq_ret;
> > +	u32 irq_status;
> > +	int i;
> > +
> > +	struct mtk_jpegdec_comp_dev *jpeg = priv;
> > +	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
> > +
> > +	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
> > +	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
> > +	if (dec_irq_ret >= MTK_JPEG_DEC_RESULT_UNDERFLOW)
> > +		mtk_jpeg_dec_reset(jpeg->reg_base);
> > +	if (dec_irq_ret != MTK_JPEG_DEC_RESULT_EOF_DONE) {
> > +		dev_err(jpeg->dev, "decode failed\n");
> > +		goto dec_end;
> > +	}
> > +
> > +	ctx = v4l2_m2m_get_curr_priv(master_jpeg->m2m_dev);
> > +	if (!ctx) {
> > +		dev_err(jpeg->dev, "Context is NULL\n");
> > +		return IRQ_HANDLED;
> > +		}
> > +
> > +	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > +	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > +	jpeg_src_buf =
> > +		container_of(src_buf, struct mtk_jpeg_src_buf, b);
> > +
> > +	for (i = 0; i < dst_buf->vb2_buf.num_planes; i++)
> > +		vb2_set_plane_payload(&dst_buf->vb2_buf, i,
> > +		jpeg_src_buf->dec_param.comp_size[i]);
> > +
> > +	buf_state = VB2_BUF_STATE_DONE;
> > +
> > +dec_end:
> > +	v4l2_m2m_buf_done(src_buf, buf_state);
> > +	v4l2_m2m_buf_done(dst_buf, buf_state);
> > +	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +	pm_runtime_put(ctx->jpeg->dev);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int mtk_jpegdec_hw_init_irq(struct mtk_jpegdec_comp_dev
> > *dev)
> > +{
> > +	struct platform_device *pdev = dev->plat_dev;
> > +	int ret;
> > +
> > +	dev->jpegdec_irq = platform_get_irq(pdev, 0);
> > +	if (dev->jpegdec_irq < 0) {
> > +		dev_err(&pdev->dev, "Failed to get irq resource");
> > +		return dev->jpegdec_irq;
> > +	}
> > +
> > +	ret = devm_request_irq(&pdev->dev, dev->jpegdec_irq,
> > +		mtk_jpegdec_hw_irq_handler, 0, pdev->name, dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to devm_request_irq %d
> > (%d)",
> > +			dev->jpegdec_irq, ret);
> > +		return -ENOENT;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +void mtk_jpegdec_release_pm(struct mtk_jpegdec_comp_dev *mtkdev)
> > +{
> > +	struct platform_device *pdev = mtkdev->plat_dev;
> > +
> 
> You're using this function only in this file... and then, this is
> just one line.
> This helper is not needed: please simply call pm_runtime_disable
> where needed.
I will take your suggestion to shorten code.
Thanks.
> 
> > +	pm_runtime_disable(&pdev->dev);
> > +}
> > +
> > +static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
> > +{
> > +	struct mtk_jpeg_dev *master_dev;
> > +	struct mtk_jpegdec_comp_dev *dev;
> > +	const struct of_device_id *of_id;
> > +	int ret, comp_idx;
> > +
> > +	struct device *decs = &pdev->dev;
> > +
> > +	if (!decs->parent)
> > +		return -EPROBE_DEFER;
> > +
> > +	master_dev = dev_get_drvdata(decs->parent);
> > +	if (!master_dev)
> > +		return -EPROBE_DEFER;
> > +
> > +	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> > +	if (!dev)
> > +		return -ENOMEM;
> > +
> > +	dev->plat_dev = pdev;
> > +	ret = mtk_jpegdec_init_pm(dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to get jpeg enc clock
> > source");
> 
> 	if (ret)
> 		return dev_err_probe(&pdev->dev, ret, ".....blurb");
> 
> That's shorter, and it is recommended to use dev_err_probe() even if
> the
> function cannot return -EPROBE_DEFER.
Thanks. I will use dev_err_probe() to replace it.
> 
> > +		return ret;
> > +	}
> > +
> > +	dev->reg_base =
> > +		devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(dev->reg_base)) {
> 
> By moving the pm_runtime_enable() call to the end of the probe
> function, you
> won't need this goto, hence you'll be able to simply
Thanks. I will simply this code.
> 
> 		return PTR_ERR(dev->reg_base);
> 
> > +		ret = PTR_ERR(dev->reg_base);
> > +		goto err;
> > +	}
> > +
> > +	ret = mtk_jpegdec_hw_init_irq(dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to register JPEGDEC irq
> > handler.\n");
> 
> ...and it's the same here, with dev_err_probe().
Thanks.
> 
> > +		goto err;
> > +	}
> > +
> > +	of_id = of_match_device(mtk_jpegdec_hw_ids, decs);
> > +	if (!of_id) {
> > +		dev_err(&pdev->dev, "Can't get vdec comp device
> > id.\n");
> 
> .... and here too, but if you follow what I said in the encoder
> review, this call
> may not even be necessary, as you'd be simply checking if
> "mediatek,hw-leader" is
> true (meaning that it's device 0), otherwise it's device number x
> (where x > 0).
Thanks.
The code logic will be simplified.
> 
> > +		ret = -EINVAL;
> > +		goto err;
> > +	}
> > +
> > +	comp_idx = (enum mtk_jpegdec_hw_id)of_id->data;
> > +	if (comp_idx < MTK_JPEGDEC_HW_MAX) {
> > +		master_dev->dec_hw_dev[comp_idx] = dev;
> > +		master_dev->reg_decbase[comp_idx] = dev->reg_base;
> > +		dev->master_dev = master_dev;
> > +	}
> > +
> 
> 	pm_runtime_enable(&pdev->dev);
> 
> > +	platform_set_drvdata(pdev, dev);
> > +	return 0;
> > +
> > +err:
> > +	mtk_jpegdec_release_pm(dev);
> > +	return ret;
> > +}
> > +
> > +struct platform_driver mtk_jpegdec_hw_driver = {
> > +	.probe	= mtk_jpegdec_hw_probe,
> > +	.driver	= {
> > +		.name	= "mtk-jpegdec-hw",
> > +		.of_match_table = mtk_jpegdec_hw_ids,
> > +	},
> > +};
> 
> Regards,
> Angelo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V1, 3/6] media: mtk-jpegdec: add jpegdec timeout func interface

[kyrie.wu]

Generalizes jpegdec timeout func interfaces to handle HW timeout.

Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |  2 ++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index 20243d4..b7102db 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
@@ -200,6 +200,8 @@ struct mtk_jpegdec_comp_dev {
 	struct mtk_jpeg_dev *master_dev;
 	struct mtk_jpegdec_pm pm;
 	int jpegdec_irq;
+	struct delayed_work job_timeout_work;
+	struct mtk_jpeg_hw_param hw_param;
 };
 
 /**
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
index 86f12bd..e295576 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
@@ -443,6 +443,24 @@ void mtk_jpeg_dec_set_config(void __iomem *base,
 	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
 }
 
+static void mtk_jpegdec_timeout_work(struct work_struct *work)
+{
+	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
+	struct mtk_jpegdec_comp_dev *cjpeg =
+		container_of(work, struct mtk_jpegdec_comp_dev,
+		job_timeout_work.work);
+	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+
+	src_buf = cjpeg->hw_param.src_buffer;
+	dst_buf = cjpeg->hw_param.dst_buffer;
+
+	mtk_jpeg_dec_reset(cjpeg->reg_base);
+	clk_disable_unprepare(cjpeg->pm.dec_clk.clk_info->jpegdec_clk);
+	pm_runtime_put(cjpeg->pm.dev);
+	v4l2_m2m_buf_done(src_buf, buf_state);
+	v4l2_m2m_buf_done(dst_buf, buf_state);
+}
+
 int mtk_jpegdec_init_pm(struct mtk_jpegdec_comp_dev *mtkdev)
 {
 	struct mtk_jpegdec_clk_info *clk_info;
@@ -507,6 +525,8 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	struct mtk_jpegdec_comp_dev *jpeg = priv;
 	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
 
+	cancel_delayed_work(&jpeg->job_timeout_work);
+
 	irq_status = mtk_jpeg_dec_get_int_status(jpeg->reg_base);
 	dec_irq_ret = mtk_jpeg_dec_enum_result(irq_status);
 	if (dec_irq_ret >= MTK_JPEG_DEC_RESULT_UNDERFLOW)
@@ -592,6 +612,9 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dev->plat_dev = pdev;
+
+	INIT_DELAYED_WORK(&dev->job_timeout_work, mtk_jpegdec_timeout_work);
+
 	ret = mtk_jpegdec_init_pm(dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to get jpeg enc clock source");
-- 
2.6.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V1, 4/6] media: mtk-jpegdec: add jpeg decode worker interface

[kyrie.wu]

Add jpeg decoding worker to ensure that three HWs
run in parallel in MT8195.

Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   | 190 +++++++++++++++++++---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |   5 +
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c |  17 ++
 3 files changed, 189 insertions(+), 23 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index f2a5e84..2518660 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -1065,57 +1065,187 @@ static void mtk_jpeg_enc_device_run(void *priv)
 	queue_work(jpeg->workqueue, &ctx->jpeg_work);
 }
 
-static void mtk_jpeg_dec_device_run(void *priv)
+static int mtk_jpegdec_select_hw(struct mtk_jpeg_ctx *ctx)
 {
-	struct mtk_jpeg_ctx *ctx = priv;
+	struct mtk_jpegdec_comp_dev *comp_jpeg;
 	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
-	struct vb2_v4l2_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;
+	int hw_id = -1;
+	int i;
+
+	spin_lock_irqsave(&jpeg->hw_lock, flags);
+	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
+		comp_jpeg = jpeg->dec_hw_dev[i];
+		if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {
+			hw_id = i;
+			comp_jpeg->hw_state = MTK_JPEG_HW_BUSY;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
+
+	return hw_id;
+}
+
+static int mtk_jpegdec_deselect_hw(struct mtk_jpeg_dev *jpeg, int hw_id)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&jpeg->hw_lock, flags);
+	jpeg->dec_hw_dev[hw_id]->hw_state =
+		MTK_JPEG_HW_IDLE;
+	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
+
+	return 0;
+}
+
+static int mtk_jpegdec_set_hw_param(struct mtk_jpeg_ctx *ctx,
+	int hw_id,
+	struct vb2_v4l2_buffer *src_buf,
+	struct vb2_v4l2_buffer *dst_buf)
+{
+	struct mtk_jpegdec_comp_dev *jpeg =
+		ctx->jpeg->dec_hw_dev[hw_id];
+
+	jpeg->hw_param.curr_ctx = ctx;
+	jpeg->hw_param.src_buffer = src_buf;
+	jpeg->hw_param.dst_buffer = dst_buf;
+
+	return 0;
+}
+
+static void mtk_jpegdec_worker(struct work_struct *work)
+{
+	struct mtk_jpeg_ctx *ctx = container_of(work, struct mtk_jpeg_ctx,
+		jpeg_work);
+	struct mtk_jpegdec_comp_dev *comp_jpeg[MTK_JPEGDEC_HW_MAX];
+	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
+	struct mtk_jpeg_src_buf *jpeg_src_buf, *jpeg_dst_buf;
+	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
+	atomic_t *hw_rdy[MTK_JPEGDEC_HW_MAX];
+	struct clk *jpegdec_clk;
+	int ret, i, hw_id = 0;
 	struct mtk_jpeg_bs bs;
 	struct mtk_jpeg_fb fb;
-	int ret;
+	unsigned long flags;
+
+	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
+		comp_jpeg[i] = jpeg->dec_hw_dev[i];
+		hw_rdy[i] = &comp_jpeg[i]->hw_rdy;
+	}
+
+retry_select:
+	hw_id = mtk_jpegdec_select_hw(ctx);
+	if (hw_id < 0) {
+		ret = wait_event_interruptible(jpeg->dec_hw_wq,
+			(atomic_read(hw_rdy[0]) ||
+			atomic_read(hw_rdy[1]) ||
+			atomic_read(hw_rdy[2])) > 0);
+		if (ret != 0) {
+			dev_err(jpeg->dev, "%s : %d, all HW are busy\n",
+				__func__, __LINE__);
+			v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
+			return;
+		}
+
+		dev_err(jpeg->dev, "%s : %d, NEW HW IDLE, please retry selcet!!!\n",
+			__func__, __LINE__);
+		goto retry_select;
+	}
 
+	atomic_dec(&comp_jpeg[hw_id]->hw_rdy);
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+	if (!src_buf) {
+		dev_err(jpeg->dev, "%s : %d, get src_buf fail !!!\n",
+			__func__, __LINE__);
+		goto getbuf_fail;
+	}
 	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+	if (!dst_buf) {
+		dev_err(jpeg->dev, "%s : %d, get dst_buf fail !!!\n",
+			__func__, __LINE__);
+		goto getbuf_fail;
+	}
+
 	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
+	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
 
-	if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) {
+	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;
+		goto getbuf_fail;
 	}
 
-	ret = pm_runtime_resume_and_get(jpeg->dev);
-	if (ret < 0)
+	jpeg_src_buf->curr_ctx = ctx;
+	jpeg_src_buf->frame_num = ctx->total_frame_num;
+	jpeg_dst_buf->curr_ctx = ctx;
+	jpeg_dst_buf->frame_num = ctx->total_frame_num;
+	ctx->total_frame_num++;
+
+	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
+	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
+
+	mtk_jpegdec_set_hw_param(ctx, hw_id, src_buf, dst_buf);
+	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->pm.dev);
+	if (ret < 0) {
+		dev_err(jpeg->dev, "%s : %d, pm_runtime_get_sync fail !!!\n",
+			__func__, __LINE__);
 		goto dec_end;
+	}
 
-	schedule_delayed_work(&jpeg->job_timeout_work,
-			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
+	jpegdec_clk = comp_jpeg[hw_id]->pm.dec_clk.clk_info->jpegdec_clk;
+	ret = clk_prepare_enable(jpegdec_clk);
+	if (ret) {
+		dev_err(jpeg->dev, "%s : %d, jpegdec clk_prepare_enable fail\n",
+			__func__, __LINE__);
+		goto clk_end;
+	}
+
+	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
+		msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
 
 	mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
-	if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
-		goto dec_end;
+	if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param,
+		&dst_buf->vb2_buf, &fb)) {
+		dev_err(jpeg->dev, "%s : %d, mtk_jpeg_set_dec_dst fail\n",
+			__func__, __LINE__);
+		goto setdst_end;
+	}
 
-	spin_lock_irqsave(&jpeg->hw_lock, flags);
-	mtk_jpeg_dec_reset(jpeg->reg_base);
+	spin_lock_irqsave(&comp_jpeg[hw_id]->hw_lock, flags);
+	mtk_jpeg_dec_reset(comp_jpeg[hw_id]->reg_base);
 	mtk_jpeg_dec_set_config(jpeg->reg_base,
-				&jpeg_src_buf->dec_param, &bs, &fb);
+		&jpeg_src_buf->dec_param,
+		&bs, &fb);
+	mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
+	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
+	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
 
-	mtk_jpeg_dec_start(jpeg->reg_base);
-	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
 	return;
 
+setdst_end:
+	clk_disable_unprepare(jpegdec_clk);
+clk_end:
+	pm_runtime_put(comp_jpeg[hw_id]->pm.dev);
 dec_end:
-	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
-	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	v4l2_m2m_buf_done(dst_buf, buf_state);
+getbuf_fail:
+	atomic_inc(&comp_jpeg[hw_id]->hw_rdy);
+	mtk_jpegdec_deselect_hw(jpeg, hw_id);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
 }
 
+static void mtk_jpeg_dec_device_run(void *priv)
+{
+	struct mtk_jpeg_ctx *ctx = priv;
+	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
+
+	queue_work(jpeg->dec_workqueue, &ctx->jpeg_work);
+}
+
 static int mtk_jpeg_dec_job_ready(void *priv)
 {
 	struct mtk_jpeg_ctx *ctx = priv;
@@ -1334,6 +1464,8 @@ static int mtk_jpeg_open(struct file *file)
 
 	if (jpeg->variant->is_encoder)
 		INIT_WORK(&ctx->jpeg_work, mtk_jpegenc_worker);
+	else
+		INIT_WORK(&ctx->jpeg_work, mtk_jpegdec_worker);
 
 	INIT_LIST_HEAD(&ctx->dst_done_queue);
 	v4l2_fh_init(&ctx->fh, vfd);
@@ -1481,7 +1613,17 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 		jpeg->workqueue = alloc_ordered_workqueue(MTK_JPEG_NAME,
 			WQ_MEM_RECLAIM | WQ_FREEZABLE);
 		if (!jpeg->workqueue) {
-			dev_err(&pdev->dev, "Failed to create jpeg workqueue!\n");
+			dev_err(&pdev->dev, "Failed to create jpegenc workqueue!\n");
+			ret = -EINVAL;
+			goto err_alloc_workqueue;
+		}
+	} else {
+		init_waitqueue_head(&jpeg->dec_hw_wq);
+
+		jpeg->dec_workqueue = alloc_ordered_workqueue(MTK_JPEG_NAME,
+			WQ_MEM_RECLAIM | WQ_FREEZABLE);
+		if (!jpeg->dec_workqueue) {
+			dev_err(&pdev->dev, "Failed to create jpegdec workqueue!\n");
 			ret = -EINVAL;
 			goto err_alloc_workqueue;
 		}
@@ -1571,6 +1713,8 @@ static int mtk_jpeg_remove(struct platform_device *pdev)
 	mtk_jpeg_clk_release(jpeg);
 	flush_workqueue(jpeg->workqueue);
 	destroy_workqueue(jpeg->workqueue);
+	flush_workqueue(jpeg->dec_workqueue);
+	destroy_workqueue(jpeg->dec_workqueue);
 
 	return 0;
 }
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index b7102db..d67c49f 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
@@ -202,6 +202,9 @@ struct mtk_jpegdec_comp_dev {
 	int jpegdec_irq;
 	struct delayed_work job_timeout_work;
 	struct mtk_jpeg_hw_param hw_param;
+	atomic_t hw_rdy;
+	enum mtk_jpeg_hw_state hw_state;
+	spinlock_t hw_lock;
 };
 
 /**
@@ -239,6 +242,8 @@ struct mtk_jpeg_dev {
 
 	void __iomem *reg_decbase[MTK_JPEGDEC_HW_MAX];
 	struct mtk_jpegdec_comp_dev *dec_hw_dev[MTK_JPEGDEC_HW_MAX];
+	wait_queue_head_t dec_hw_wq;
+	struct workqueue_struct	*dec_workqueue;
 };
 
 /**
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
index e295576..9138ecb 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
@@ -449,6 +449,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 	struct mtk_jpegdec_comp_dev *cjpeg =
 		container_of(work, struct mtk_jpegdec_comp_dev,
 		job_timeout_work.work);
+	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 
 	src_buf = cjpeg->hw_param.src_buffer;
@@ -457,6 +458,9 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 	mtk_jpeg_dec_reset(cjpeg->reg_base);
 	clk_disable_unprepare(cjpeg->pm.dec_clk.clk_info->jpegdec_clk);
 	pm_runtime_put(cjpeg->pm.dev);
+	cjpeg->hw_state = MTK_JPEG_HW_IDLE;
+	atomic_inc(&cjpeg->hw_rdy);
+	wake_up(&master_jpeg->dec_hw_wq);
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	v4l2_m2m_buf_done(dst_buf, buf_state);
 }
@@ -557,8 +561,18 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 	v4l2_m2m_buf_done(src_buf, buf_state);
 	v4l2_m2m_buf_done(dst_buf, buf_state);
 	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
+	clk_disable_unprepare(jpeg->pm.dec_clk.clk_info->jpegdec_clk);
 	pm_runtime_put(ctx->jpeg->dev);
 
+	if (ctx->fh.m2m_ctx &&
+		(!list_empty(&ctx->fh.m2m_ctx->out_q_ctx.rdy_queue) ||
+		!list_empty(&ctx->fh.m2m_ctx->cap_q_ctx.rdy_queue)))
+		queue_work(master_jpeg->workqueue, &ctx->jpeg_work);
+
+	jpeg->hw_state = MTK_JPEG_HW_IDLE;
+	wake_up(&master_jpeg->dec_hw_wq);
+	atomic_inc(&jpeg->hw_rdy);
+
 	return IRQ_HANDLED;
 }
 
@@ -612,6 +626,9 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dev->plat_dev = pdev;
+	atomic_set(&dev->hw_rdy, 1U);
+	spin_lock_init(&dev->hw_lock);
+	dev->hw_state = MTK_JPEG_HW_IDLE;
 
 	INIT_DELAYED_WORK(&dev->job_timeout_work, mtk_jpegdec_timeout_work);
 
-- 
2.6.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V1, 4/6] media: mtk-jpegdec: add jpeg decode worker interface

[Ricardo Ribalda]

kyrie.wu wrote:

> Add jpeg decoding worker to ensure that three HWs
> run in parallel in MT8195.
> 
> Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
> ---
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   | 190 +++++++++++++++++++---
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |   5 +
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c |  17 ++
>  3 files changed, 189 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index f2a5e84..2518660 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -1065,57 +1065,187 @@ static void mtk_jpeg_enc_device_run(void *priv)
>  	queue_work(jpeg->workqueue, &ctx->jpeg_work);
>  }
>  
> -static void mtk_jpeg_dec_device_run(void *priv)
> +static int mtk_jpegdec_select_hw(struct mtk_jpeg_ctx *ctx)
>  {
> -	struct mtk_jpeg_ctx *ctx = priv;
> +	struct mtk_jpegdec_comp_dev *comp_jpeg;
>  	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> -	struct vb2_v4l2_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;
> +	int hw_id = -1;
> +	int i;
> +
> +	spin_lock_irqsave(&jpeg->hw_lock, flags);
> +	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
> +		comp_jpeg = jpeg->dec_hw_dev[i];
> +		if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {
> +			hw_id = i;
> +			comp_jpeg->hw_state = MTK_JPEG_HW_BUSY;
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> +
> +	return hw_id;
> +}
> +
> +static int mtk_jpegdec_deselect_hw(struct mtk_jpeg_dev *jpeg, int hw_id)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&jpeg->hw_lock, flags);
> +	jpeg->dec_hw_dev[hw_id]->hw_state =
> +		MTK_JPEG_HW_IDLE;
> +	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> +
> +	return 0;
> +}
> +
> +static int mtk_jpegdec_set_hw_param(struct mtk_jpeg_ctx *ctx,
> +	int hw_id,
> +	struct vb2_v4l2_buffer *src_buf,
> +	struct vb2_v4l2_buffer *dst_buf)
> +{
> +	struct mtk_jpegdec_comp_dev *jpeg =
> +		ctx->jpeg->dec_hw_dev[hw_id];
> +
> +	jpeg->hw_param.curr_ctx = ctx;
> +	jpeg->hw_param.src_buffer = src_buf;
> +	jpeg->hw_param.dst_buffer = dst_buf;
> +
> +	return 0;
> +}
> +
> +static void mtk_jpegdec_worker(struct work_struct *work)
> +{
> +	struct mtk_jpeg_ctx *ctx = container_of(work, struct mtk_jpeg_ctx,
> +		jpeg_work);
> +	struct mtk_jpegdec_comp_dev *comp_jpeg[MTK_JPEGDEC_HW_MAX];
> +	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> +	struct mtk_jpeg_src_buf *jpeg_src_buf, *jpeg_dst_buf;
> +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> +	atomic_t *hw_rdy[MTK_JPEGDEC_HW_MAX];
> +	struct clk *jpegdec_clk;
> +	int ret, i, hw_id = 0;
>  	struct mtk_jpeg_bs bs;
>  	struct mtk_jpeg_fb fb;
> -	int ret;
> +	unsigned long flags;
> +
> +	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
> +		comp_jpeg[i] = jpeg->dec_hw_dev[i];
> +		hw_rdy[i] = &comp_jpeg[i]->hw_rdy;
> +	}
> +
> +retry_select:
> +	hw_id = mtk_jpegdec_select_hw(ctx);
> +	if (hw_id < 0) {
> +		ret = wait_event_interruptible(jpeg->dec_hw_wq,
> +			(atomic_read(hw_rdy[0]) ||
> +			atomic_read(hw_rdy[1]) ||
> +			atomic_read(hw_rdy[2])) > 0);
> +		if (ret != 0) {
> +			dev_err(jpeg->dev, "%s : %d, all HW are busy\n",
> +				__func__, __LINE__);
> +			v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> +			return;
> +		}
> +
> +		dev_err(jpeg->dev, "%s : %d, NEW HW IDLE, please retry selcet!!!\n",
> +			__func__, __LINE__);
> +		goto retry_select;
> +	}
>  
> +	atomic_dec(&comp_jpeg[hw_id]->hw_rdy);
>  	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> +	if (!src_buf) {
> +		dev_err(jpeg->dev, "%s : %d, get src_buf fail !!!\n",
> +			__func__, __LINE__);
> +		goto getbuf_fail;
> +	}
>  	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> +	if (!dst_buf) {
> +		dev_err(jpeg->dev, "%s : %d, get dst_buf fail !!!\n",
> +			__func__, __LINE__);
> +		goto getbuf_fail;
> +	}
> +
>  	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> +	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
>  
> -	if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) {
> +	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;
> +		goto getbuf_fail;
>  	}
>  
> -	ret = pm_runtime_resume_and_get(jpeg->dev);
> -	if (ret < 0)
> +	jpeg_src_buf->curr_ctx = ctx;
> +	jpeg_src_buf->frame_num = ctx->total_frame_num;
> +	jpeg_dst_buf->curr_ctx = ctx;
> +	jpeg_dst_buf->frame_num = ctx->total_frame_num;
> +	ctx->total_frame_num++;
> +
> +	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> +	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> +
> +	mtk_jpegdec_set_hw_param(ctx, hw_id, src_buf, dst_buf);
> +	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->pm.dev);
> +	if (ret < 0) {
> +		dev_err(jpeg->dev, "%s : %d, pm_runtime_get_sync fail !!!\n",
> +			__func__, __LINE__);
>  		goto dec_end;
> +	}
>  
> -	schedule_delayed_work(&jpeg->job_timeout_work,
> -			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
> +	jpegdec_clk = comp_jpeg[hw_id]->pm.dec_clk.clk_info->jpegdec_clk;
> +	ret = clk_prepare_enable(jpegdec_clk);
> +	if (ret) {
> +		dev_err(jpeg->dev, "%s : %d, jpegdec clk_prepare_enable fail\n",
> +			__func__, __LINE__);
> +		goto clk_end;
> +	}
> +
> +	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
> +		msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
>  
>  	mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
> -	if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
> -		goto dec_end;
> +	if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param,
> +		&dst_buf->vb2_buf, &fb)) {
> +		dev_err(jpeg->dev, "%s : %d, mtk_jpeg_set_dec_dst fail\n",
> +			__func__, __LINE__);
> +		goto setdst_end;
> +	}
>  
> -	spin_lock_irqsave(&jpeg->hw_lock, flags);
> -	mtk_jpeg_dec_reset(jpeg->reg_base);
> +	spin_lock_irqsave(&comp_jpeg[hw_id]->hw_lock, flags);
> +	mtk_jpeg_dec_reset(comp_jpeg[hw_id]->reg_base);
>  	mtk_jpeg_dec_set_config(jpeg->reg_base,
> -				&jpeg_src_buf->dec_param, &bs, &fb);
> +		&jpeg_src_buf->dec_param,
> +		&bs, &fb);
> +	mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
> +	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> +	spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
>  
> -	mtk_jpeg_dec_start(jpeg->reg_base);
> -	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
>  	return;
>  
> +setdst_end:
> +	clk_disable_unprepare(jpegdec_clk);
> +clk_end:
> +	pm_runtime_put(comp_jpeg[hw_id]->pm.dev);
>  dec_end:
> -	v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> -	v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	v4l2_m2m_buf_done(dst_buf, buf_state);
> +getbuf_fail:
> +	atomic_inc(&comp_jpeg[hw_id]->hw_rdy);
> +	mtk_jpegdec_deselect_hw(jpeg, hw_id);
>  	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
>  }
>  
> +static void mtk_jpeg_dec_device_run(void *priv)
> +{
> +	struct mtk_jpeg_ctx *ctx = priv;
> +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> +
> +	queue_work(jpeg->dec_workqueue, &ctx->jpeg_work);
> +}
> +
>  static int mtk_jpeg_dec_job_ready(void *priv)
>  {
>  	struct mtk_jpeg_ctx *ctx = priv;
> @@ -1334,6 +1464,8 @@ static int mtk_jpeg_open(struct file *file)
>  
>  	if (jpeg->variant->is_encoder)
>  		INIT_WORK(&ctx->jpeg_work, mtk_jpegenc_worker);
> +	else
> +		INIT_WORK(&ctx->jpeg_work, mtk_jpegdec_worker);
>  
>  	INIT_LIST_HEAD(&ctx->dst_done_queue);
>  	v4l2_fh_init(&ctx->fh, vfd);
> @@ -1481,7 +1613,17 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
>  		jpeg->workqueue = alloc_ordered_workqueue(MTK_JPEG_NAME,
>  			WQ_MEM_RECLAIM | WQ_FREEZABLE);
>  		if (!jpeg->workqueue) {
> -			dev_err(&pdev->dev, "Failed to create jpeg workqueue!\n");
> +			dev_err(&pdev->dev, "Failed to create jpegenc workqueue!\n");
> +			ret = -EINVAL;
> +			goto err_alloc_workqueue;
> +		}
> +	} else {
> +		init_waitqueue_head(&jpeg->dec_hw_wq);
> +
> +		jpeg->dec_workqueue = alloc_ordered_workqueue(MTK_JPEG_NAME,
> +			WQ_MEM_RECLAIM | WQ_FREEZABLE);
> +		if (!jpeg->dec_workqueue) {
> +			dev_err(&pdev->dev, "Failed to create jpegdec workqueue!\n");
>  			ret = -EINVAL;
>  			goto err_alloc_workqueue;
>  		}
> @@ -1571,6 +1713,8 @@ static int mtk_jpeg_remove(struct platform_device *pdev)
>  	mtk_jpeg_clk_release(jpeg);
>  	flush_workqueue(jpeg->workqueue);
>  	destroy_workqueue(jpeg->workqueue);
> +	flush_workqueue(jpeg->dec_workqueue);
> +	destroy_workqueue(jpeg->dec_workqueue);
>  
>  	return 0;
>  }
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> index b7102db..d67c49f 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> @@ -202,6 +202,9 @@ struct mtk_jpegdec_comp_dev {
>  	int jpegdec_irq;
>  	struct delayed_work job_timeout_work;
>  	struct mtk_jpeg_hw_param hw_param;
> +	atomic_t hw_rdy;
> +	enum mtk_jpeg_hw_state hw_state;
> +	spinlock_t hw_lock;
>  };
>  
>  /**
> @@ -239,6 +242,8 @@ struct mtk_jpeg_dev {
>  
>  	void __iomem *reg_decbase[MTK_JPEGDEC_HW_MAX];
>  	struct mtk_jpegdec_comp_dev *dec_hw_dev[MTK_JPEGDEC_HW_MAX];
> +	wait_queue_head_t dec_hw_wq;
> +	struct workqueue_struct	*dec_workqueue;
>  };
>  
>  /**
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> index e295576..9138ecb 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> @@ -449,6 +449,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  	struct mtk_jpegdec_comp_dev *cjpeg =
>  		container_of(work, struct mtk_jpegdec_comp_dev,
>  		job_timeout_work.work);
> +	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
>  	struct vb2_v4l2_buffer *src_buf, *dst_buf;
>  
>  	src_buf = cjpeg->hw_param.src_buffer;
> @@ -457,6 +458,9 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  	mtk_jpeg_dec_reset(cjpeg->reg_base);
>  	clk_disable_unprepare(cjpeg->pm.dec_clk.clk_info->jpegdec_clk);
>  	pm_runtime_put(cjpeg->pm.dev);
> +	cjpeg->hw_state = MTK_JPEG_HW_IDLE;
> +	atomic_inc(&cjpeg->hw_rdy);
> +	wake_up(&master_jpeg->dec_hw_wq);
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	v4l2_m2m_buf_done(dst_buf, buf_state);
>  }
> @@ -557,8 +561,18 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>  	v4l2_m2m_buf_done(src_buf, buf_state);
>  	v4l2_m2m_buf_done(dst_buf, buf_state);
>  	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
> +	clk_disable_unprepare(jpeg->pm.dec_clk.clk_info->jpegdec_clk);
>  	pm_runtime_put(ctx->jpeg->dev);
>  
> +	if (ctx->fh.m2m_ctx &&
> +		(!list_empty(&ctx->fh.m2m_ctx->out_q_ctx.rdy_queue) ||
> +		!list_empty(&ctx->fh.m2m_ctx->cap_q_ctx.rdy_queue)))
> +		queue_work(master_jpeg->workqueue, &ctx->jpeg_work);
> +
> +	jpeg->hw_state = MTK_JPEG_HW_IDLE;
> +	wake_up(&master_jpeg->dec_hw_wq);
> +	atomic_inc(&jpeg->hw_rdy);
> +
>  	return IRQ_HANDLED;
>  }
>  
> @@ -612,6 +626,9 @@ static int mtk_jpegdec_hw_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	dev->plat_dev = pdev;
> +	atomic_set(&dev->hw_rdy, 1U);
> +	spin_lock_init(&dev->hw_lock);
> +	dev->hw_state = MTK_JPEG_HW_IDLE;
>  
>  	INIT_DELAYED_WORK(&dev->job_timeout_work, mtk_jpegdec_timeout_work);
>  
> -- 
> 2.6.4
> 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V1, 4/6] media: mtk-jpegdec: add jpeg decode worker interface

[AngeloGioacchino Del Regno]

Il 03/12/21 06:34, kyrie.wu ha scritto:
> Add jpeg decoding worker to ensure that three HWs
> run in parallel in MT8195.
> 
> Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
> ---
>   drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   | 190 +++++++++++++++++++---
>   drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |   5 +
>   drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c |  17 ++
>   3 files changed, 189 insertions(+), 23 deletions(-)
> 

Hello Kyrie,
thanks for the patch! However, there are some things to improve...

> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index f2a5e84..2518660 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -1065,57 +1065,187 @@ static void mtk_jpeg_enc_device_run(void *priv)
>   	queue_work(jpeg->workqueue, &ctx->jpeg_work);
>   }
>   
> -static void mtk_jpeg_dec_device_run(void *priv)
> +static int mtk_jpegdec_select_hw(struct mtk_jpeg_ctx *ctx)
>   {
> -	struct mtk_jpeg_ctx *ctx = priv;
> +	struct mtk_jpegdec_comp_dev *comp_jpeg;
>   	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> -	struct vb2_v4l2_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;
> +	int hw_id = -1;
> +	int i;
> +
> +	spin_lock_irqsave(&jpeg->hw_lock, flags);
> +	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
> +		comp_jpeg = jpeg->dec_hw_dev[i];
> +		if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {
> +			hw_id = i;
> +			comp_jpeg->hw_state = MTK_JPEG_HW_BUSY;
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> +
> +	return hw_id;
> +}
> +
> +static int mtk_jpegdec_deselect_hw(struct mtk_jpeg_dev *jpeg, int hw_id)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&jpeg->hw_lock, flags);
> +	jpeg->dec_hw_dev[hw_id]->hw_state =
> +		MTK_JPEG_HW_IDLE;
> +	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> +
> +	return 0;
> +}
> +
> +static int mtk_jpegdec_set_hw_param(struct mtk_jpeg_ctx *ctx,
> +	int hw_id,
> +	struct vb2_v4l2_buffer *src_buf,
> +	struct vb2_v4l2_buffer *dst_buf)
> +{
> +	struct mtk_jpegdec_comp_dev *jpeg =
> +		ctx->jpeg->dec_hw_dev[hw_id];
> +
> +	jpeg->hw_param.curr_ctx = ctx;
> +	jpeg->hw_param.src_buffer = src_buf;
> +	jpeg->hw_param.dst_buffer = dst_buf;
> +
> +	return 0;
> +}
> +
> +static void mtk_jpegdec_worker(struct work_struct *work)
> +{
> +	struct mtk_jpeg_ctx *ctx = container_of(work, struct mtk_jpeg_ctx,
> +		jpeg_work);
> +	struct mtk_jpegdec_comp_dev *comp_jpeg[MTK_JPEGDEC_HW_MAX];
> +	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> +	struct mtk_jpeg_src_buf *jpeg_src_buf, *jpeg_dst_buf;
> +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> +	atomic_t *hw_rdy[MTK_JPEGDEC_HW_MAX];
> +	struct clk *jpegdec_clk;
> +	int ret, i, hw_id = 0;
>   	struct mtk_jpeg_bs bs;
>   	struct mtk_jpeg_fb fb;
> -	int ret;
> +	unsigned long flags;
> +
> +	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
> +		comp_jpeg[i] = jpeg->dec_hw_dev[i];
> +		hw_rdy[i] = &comp_jpeg[i]->hw_rdy;
> +	}
> +

This entire retry_select block should go to a helper function instead
of being inside of here.

Also, there's a big issue with this snippet that has to be solved: you're
unconditionally calling "goto retry_select" at the end of the if branch,
but please consider the following scenario:

- mtk_jpegdec_select_hw() returns a hw_id < 0
- wait_event_interruptible returns 0
... then we redo the same, and we still get the same result.

This may generate an infinite loop!!

After putting this into a separate function, please use a controlled loop
with a well thought maximum number of retries, as to avoid this situation.

> +retry_select:
> +	hw_id = mtk_jpegdec_select_hw(ctx);
> +	if (hw_id < 0) {
> +		ret = wait_event_interruptible(jpeg->dec_hw_wq,
> +			(atomic_read(hw_rdy[0]) ||
> +			atomic_read(hw_rdy[1]) ||
> +			atomic_read(hw_rdy[2])) > 0);
> +		if (ret != 0) {
> +			dev_err(jpeg->dev, "%s : %d, all HW are busy\n",
> +				__func__, __LINE__);
> +			v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> +			return;
> +		}
> +
> +		dev_err(jpeg->dev, "%s : %d, NEW HW IDLE, please retry selcet!!!\n",
> +			__func__, __LINE__);
> +		goto retry_select;
> +	}
>   
> +	atomic_dec(&comp_jpeg[hw_id]->hw_rdy);
>   	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> +	if (!src_buf) {
> +		dev_err(jpeg->dev, "%s : %d, get src_buf fail !!!\n",
> +			__func__, __LINE__);
> +		goto getbuf_fail;
> +	}
>   	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> +	if (!dst_buf) {
> +		dev_err(jpeg->dev, "%s : %d, get dst_buf fail !!!\n",
> +			__func__, __LINE__);
> +		goto getbuf_fail;
> +	}
> +
>   	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> +	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
>   
> -	if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) {
> +	if (mtk_jpeg_check_resolution_change(ctx,
> +		&jpeg_src_buf->dec_param)) {

Why are you breaking this line? There's no need to.

>   		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;
> +		goto getbuf_fail;
>   	}
>   


Regards,
- Angelo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V1, 4/6] media: mtk-jpegdec: add jpeg decode worker interface

[kyrie.wu]

On Mon, 2021-12-06 at 17:26 +0100, AngeloGioacchino Del Regno wrote:
> Il 03/12/21 06:34, kyrie.wu ha scritto:
> > Add jpeg decoding worker to ensure that three HWs
> > run in parallel in MT8195.
> > 
> > Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
> > ---
> >   drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   | 190
> > +++++++++++++++++++---
> >   drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |   5 +
> >   drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c |  17 ++
> >   3 files changed, 189 insertions(+), 23 deletions(-)
> > 
> 
> Hello Kyrie,
> thanks for the patch! However, there are some things to improve...
> 
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index f2a5e84..2518660 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -1065,57 +1065,187 @@ static void mtk_jpeg_enc_device_run(void
> > *priv)
> >   	queue_work(jpeg->workqueue, &ctx->jpeg_work);
> >   }
> >   
> > -static void mtk_jpeg_dec_device_run(void *priv)
> > +static int mtk_jpegdec_select_hw(struct mtk_jpeg_ctx *ctx)
> >   {
> > -	struct mtk_jpeg_ctx *ctx = priv;
> > +	struct mtk_jpegdec_comp_dev *comp_jpeg;
> >   	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > -	struct vb2_v4l2_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;
> > +	int hw_id = -1;
> > +	int i;
> > +
> > +	spin_lock_irqsave(&jpeg->hw_lock, flags);
> > +	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
> > +		comp_jpeg = jpeg->dec_hw_dev[i];
> > +		if (comp_jpeg->hw_state == MTK_JPEG_HW_IDLE) {
> > +			hw_id = i;
> > +			comp_jpeg->hw_state = MTK_JPEG_HW_BUSY;
> > +			break;
> > +		}
> > +	}
> > +	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> > +
> > +	return hw_id;
> > +}
> > +
> > +static int mtk_jpegdec_deselect_hw(struct mtk_jpeg_dev *jpeg, int
> > hw_id)
> > +{
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&jpeg->hw_lock, flags);
> > +	jpeg->dec_hw_dev[hw_id]->hw_state =
> > +		MTK_JPEG_HW_IDLE;
> > +	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_jpegdec_set_hw_param(struct mtk_jpeg_ctx *ctx,
> > +	int hw_id,
> > +	struct vb2_v4l2_buffer *src_buf,
> > +	struct vb2_v4l2_buffer *dst_buf)
> > +{
> > +	struct mtk_jpegdec_comp_dev *jpeg =
> > +		ctx->jpeg->dec_hw_dev[hw_id];
> > +
> > +	jpeg->hw_param.curr_ctx = ctx;
> > +	jpeg->hw_param.src_buffer = src_buf;
> > +	jpeg->hw_param.dst_buffer = dst_buf;
> > +
> > +	return 0;
> > +}
> > +
> > +static void mtk_jpegdec_worker(struct work_struct *work)
> > +{
> > +	struct mtk_jpeg_ctx *ctx = container_of(work, struct
> > mtk_jpeg_ctx,
> > +		jpeg_work);
> > +	struct mtk_jpegdec_comp_dev *comp_jpeg[MTK_JPEGDEC_HW_MAX];
> > +	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > +	struct mtk_jpeg_src_buf *jpeg_src_buf, *jpeg_dst_buf;
> > +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > +	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > +	atomic_t *hw_rdy[MTK_JPEGDEC_HW_MAX];
> > +	struct clk *jpegdec_clk;
> > +	int ret, i, hw_id = 0;
> >   	struct mtk_jpeg_bs bs;
> >   	struct mtk_jpeg_fb fb;
> > -	int ret;
> > +	unsigned long flags;
> > +
> > +	for (i = 0; i < MTK_JPEGDEC_HW_MAX; i++) {
> > +		comp_jpeg[i] = jpeg->dec_hw_dev[i];
> > +		hw_rdy[i] = &comp_jpeg[i]->hw_rdy;
> > +	}
> > +
> 
> This entire retry_select block should go to a helper function instead
> of being inside of here.
> 
> Also, there's a big issue with this snippet that has to be solved:
> you're
> unconditionally calling "goto retry_select" at the end of the if
> branch,
> but please consider the following scenario:
> 
> - mtk_jpegdec_select_hw() returns a hw_id < 0
> - wait_event_interruptible returns 0
> ... then we redo the same, and we still get the same result.
> 
> This may generate an infinite loop!!
> 
> After putting this into a separate function, please use a controlled
> loop
> with a well thought maximum number of retries, as to avoid this
> situation.
Dear Angelo,
I will use wait_event_interruptible_timeout to replace
wait_event_interruptible to avoid the situation you memtioned above and
use a helper function instead those code.
> 
> > +retry_select:
> > +	hw_id = mtk_jpegdec_select_hw(ctx);
> > +	if (hw_id < 0) {
> > +		ret = wait_event_interruptible(jpeg->dec_hw_wq,
> > +			(atomic_read(hw_rdy[0]) ||
> > +			atomic_read(hw_rdy[1]) ||
> > +			atomic_read(hw_rdy[2])) > 0);
> > +		if (ret != 0) {
> > +			dev_err(jpeg->dev, "%s : %d, all HW are
> > busy\n",
> > +				__func__, __LINE__);
> > +			v4l2_m2m_job_finish(jpeg->m2m_dev, ctx-
> > >fh.m2m_ctx);
> > +			return;
> > +		}
> > +
> > +		dev_err(jpeg->dev, "%s : %d, NEW HW IDLE, please retry
> > selcet!!!\n",
> > +			__func__, __LINE__);
> > +		goto retry_select;
> > +	}
> >   
> > +	atomic_dec(&comp_jpeg[hw_id]->hw_rdy);
> >   	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > +	if (!src_buf) {
> > +		dev_err(jpeg->dev, "%s : %d, get src_buf fail !!!\n",
> > +			__func__, __LINE__);
> > +		goto getbuf_fail;
> > +	}
> >   	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > +	if (!dst_buf) {
> > +		dev_err(jpeg->dev, "%s : %d, get dst_buf fail !!!\n",
> > +			__func__, __LINE__);
> > +		goto getbuf_fail;
> > +	}
> > +
> >   	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
> > +	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
> >   
> > -	if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf-
> > >dec_param)) {
> > +	if (mtk_jpeg_check_resolution_change(ctx,
> > +		&jpeg_src_buf->dec_param)) {
> 
> Why are you breaking this line? There's no need to.
If the resolution changed, all input and output buffer would be free
and the new size buffer would be malloc to match the lastest resolution
for jpeg decode.
> 
> >   		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;
> > +		goto getbuf_fail;
> >   	}
> >   
> 
> 
> Regards,
> - Angelo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V1, 5/6] media: mtk-jpegdec: add output pic reorder interface

[kyrie.wu]

add output reorder func to reorder the output images
to ensure the output pic is consistent with the input images.

Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c | 50 +++++++++++++++++++++--
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
index 9138ecb..fad5bf1c 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
@@ -443,6 +443,49 @@ void mtk_jpeg_dec_set_config(void __iomem *base,
 	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
 }
 
+void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)
+{
+	struct mtk_jpeg_src_buf *dst_done_buf, *tmp_dst_done_buf;
+	struct vb2_v4l2_buffer *dst_buffer;
+	struct list_head *temp_entry;
+	struct list_head *pos = NULL;
+	struct mtk_jpeg_ctx *ctx;
+	unsigned long flags;
+
+	ctx = jpeg->hw_param.curr_ctx;
+	if (!ctx) {
+		dev_err(jpeg->dev, "comp_jpeg ctx fail !!!\n");
+		return;
+	}
+
+	dst_buffer = jpeg->hw_param.dst_buffer;
+	if (!dst_buffer) {
+		dev_err(jpeg->dev, "comp_jpeg dst_buffer fail !!!\n");
+		return;
+	}
+
+	dst_done_buf = container_of(dst_buffer, struct mtk_jpeg_src_buf, b);
+
+	spin_lock_irqsave(&ctx->done_queue_lock, flags);
+	list_add_tail(&dst_done_buf->list, &ctx->dst_done_queue);
+	while (!list_empty(&ctx->dst_done_queue) &&
+		(pos != &ctx->dst_done_queue)) {
+		list_for_each_prev_safe(pos, temp_entry,
+			(&ctx->dst_done_queue)) {
+			tmp_dst_done_buf = list_entry(pos,
+				struct mtk_jpeg_src_buf, list);
+			if (tmp_dst_done_buf->frame_num ==
+				ctx->last_done_frame_num) {
+				list_del(&tmp_dst_done_buf->list);
+				v4l2_m2m_buf_done(&tmp_dst_done_buf->b,
+					VB2_BUF_STATE_DONE);
+				ctx->last_done_frame_num++;
+			}
+		}
+	}
+	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
+}
+
 static void mtk_jpegdec_timeout_work(struct work_struct *work)
 {
 	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
@@ -450,10 +493,9 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 		container_of(work, struct mtk_jpegdec_comp_dev,
 		job_timeout_work.work);
 	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
-	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+	struct vb2_v4l2_buffer *src_buf;
 
 	src_buf = cjpeg->hw_param.src_buffer;
-	dst_buf = cjpeg->hw_param.dst_buffer;
 
 	mtk_jpeg_dec_reset(cjpeg->reg_base);
 	clk_disable_unprepare(cjpeg->pm.dec_clk.clk_info->jpegdec_clk);
@@ -462,7 +504,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
 	atomic_inc(&cjpeg->hw_rdy);
 	wake_up(&master_jpeg->dec_hw_wq);
 	v4l2_m2m_buf_done(src_buf, buf_state);
-	v4l2_m2m_buf_done(dst_buf, buf_state);
+	mtk_jpegdec_put_buf(cjpeg);
 }
 
 int mtk_jpegdec_init_pm(struct mtk_jpegdec_comp_dev *mtkdev)
@@ -559,7 +601,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
 
 dec_end:
 	v4l2_m2m_buf_done(src_buf, buf_state);
-	v4l2_m2m_buf_done(dst_buf, buf_state);
+	mtk_jpegdec_put_buf(jpeg);
 	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
 	clk_disable_unprepare(jpeg->pm.dec_clk.clk_info->jpegdec_clk);
 	pm_runtime_put(ctx->jpeg->dev);
-- 
2.6.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V1, 5/6] media: mtk-jpegdec: add output pic reorder interface

[Ricardo Ribalda]

kyrie.wu wrote:

> add output reorder func to reorder the output images
> to ensure the output pic is consistent with the input images.
> 
> Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
> ---
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c | 50 +++++++++++++++++++++--
>  1 file changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> index 9138ecb..fad5bf1c 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> @@ -443,6 +443,49 @@ void mtk_jpeg_dec_set_config(void __iomem *base,
>  	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
>  }
>  
> +void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)
> +{
> +	struct mtk_jpeg_src_buf *dst_done_buf, *tmp_dst_done_buf;
> +	struct vb2_v4l2_buffer *dst_buffer;
> +	struct list_head *temp_entry;
> +	struct list_head *pos = NULL;
> +	struct mtk_jpeg_ctx *ctx;
> +	unsigned long flags;
> +
> +	ctx = jpeg->hw_param.curr_ctx;
> +	if (!ctx) {
> +		dev_err(jpeg->dev, "comp_jpeg ctx fail !!!\n");
> +		return;
> +	}
> +
> +	dst_buffer = jpeg->hw_param.dst_buffer;
> +	if (!dst_buffer) {
> +		dev_err(jpeg->dev, "comp_jpeg dst_buffer fail !!!\n");
> +		return;
> +	}
> +
> +	dst_done_buf = container_of(dst_buffer, struct mtk_jpeg_src_buf, b);
> +
> +	spin_lock_irqsave(&ctx->done_queue_lock, flags);
> +	list_add_tail(&dst_done_buf->list, &ctx->dst_done_queue);
> +	while (!list_empty(&ctx->dst_done_queue) &&
> +		(pos != &ctx->dst_done_queue)) {
> +		list_for_each_prev_safe(pos, temp_entry,
> +			(&ctx->dst_done_queue)) {
> +			tmp_dst_done_buf = list_entry(pos,
> +				struct mtk_jpeg_src_buf, list);
> +			if (tmp_dst_done_buf->frame_num ==
> +				ctx->last_done_frame_num) {
> +				list_del(&tmp_dst_done_buf->list);
> +				v4l2_m2m_buf_done(&tmp_dst_done_buf->b,
> +					VB2_BUF_STATE_DONE);
> +				ctx->last_done_frame_num++;
> +			}
> +		}
> +	}
> +	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
> +}
> +
>  static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  {
>  	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> @@ -450,10 +493,9 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  		container_of(work, struct mtk_jpegdec_comp_dev,
>  		job_timeout_work.work);
>  	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
> -	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +	struct vb2_v4l2_buffer *src_buf;
>  
>  	src_buf = cjpeg->hw_param.src_buffer;
> -	dst_buf = cjpeg->hw_param.dst_buffer;
>  
>  	mtk_jpeg_dec_reset(cjpeg->reg_base);
>  	clk_disable_unprepare(cjpeg->pm.dec_clk.clk_info->jpegdec_clk);
> @@ -462,7 +504,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>  	atomic_inc(&cjpeg->hw_rdy);
>  	wake_up(&master_jpeg->dec_hw_wq);
>  	v4l2_m2m_buf_done(src_buf, buf_state);
> -	v4l2_m2m_buf_done(dst_buf, buf_state);
> +	mtk_jpegdec_put_buf(cjpeg);
>  }
>  
>  int mtk_jpegdec_init_pm(struct mtk_jpegdec_comp_dev *mtkdev)
> @@ -559,7 +601,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>  
>  dec_end:
>  	v4l2_m2m_buf_done(src_buf, buf_state);
> -	v4l2_m2m_buf_done(dst_buf, buf_state);
> +	mtk_jpegdec_put_buf(jpeg);
>  	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
>  	clk_disable_unprepare(jpeg->pm.dec_clk.clk_info->jpegdec_clk);
>  	pm_runtime_put(ctx->jpeg->dev);
> -- 
> 2.6.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V1, 5/6] media: mtk-jpegdec: add output pic reorder interface

[AngeloGioacchino Del Regno]

Il 03/12/21 06:34, kyrie.wu ha scritto:
> add output reorder func to reorder the output images
> to ensure the output pic is consistent with the input images.
> 
> Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
> ---
>   drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c | 50 +++++++++++++++++++++--
>   1 file changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> index 9138ecb..fad5bf1c 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> @@ -443,6 +443,49 @@ void mtk_jpeg_dec_set_config(void __iomem *base,
>   	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
>   }
>   
> +void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)

This function is used only in this file, hence it should be static.

> +{
> +	struct mtk_jpeg_src_buf *dst_done_buf, *tmp_dst_done_buf;
> +	struct vb2_v4l2_buffer *dst_buffer;
> +	struct list_head *temp_entry;
> +	struct list_head *pos = NULL;
> +	struct mtk_jpeg_ctx *ctx;
> +	unsigned long flags;
> +
> +	ctx = jpeg->hw_param.curr_ctx;
> +	if (!ctx) {
> +		dev_err(jpeg->dev, "comp_jpeg ctx fail !!!\n");

Since this is unlikely to happen (or should be unlikely anyway!!), this print
should then be a dev_dbg()

> +		return;
> +	}
> +
> +	dst_buffer = jpeg->hw_param.dst_buffer;
> +	if (!dst_buffer) {
> +		dev_err(jpeg->dev, "comp_jpeg dst_buffer fail !!!\n");
> +		return;
> +	}
> +
> +	dst_done_buf = container_of(dst_buffer, struct mtk_jpeg_src_buf, b);
> +
> +	spin_lock_irqsave(&ctx->done_queue_lock, flags);
> +	list_add_tail(&dst_done_buf->list, &ctx->dst_done_queue);
> +	while (!list_empty(&ctx->dst_done_queue) &&
> +		(pos != &ctx->dst_done_queue)) {
> +		list_for_each_prev_safe(pos, temp_entry,
> +			(&ctx->dst_done_queue)) {
> +			tmp_dst_done_buf = list_entry(pos,
> +				struct mtk_jpeg_src_buf, list);
> +			if (tmp_dst_done_buf->frame_num ==
> +				ctx->last_done_frame_num) {
> +				list_del(&tmp_dst_done_buf->list);
> +				v4l2_m2m_buf_done(&tmp_dst_done_buf->b,
> +					VB2_BUF_STATE_DONE);
> +				ctx->last_done_frame_num++;
> +			}
> +		}
> +	}
> +	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
> +}
> +
>   static void mtk_jpegdec_timeout_work(struct work_struct *work)
>   {
>   	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> @@ -450,10 +493,9 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>   		container_of(work, struct mtk_jpegdec_comp_dev,
>   		job_timeout_work.work);
>   	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
> -	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +	struct vb2_v4l2_buffer *src_buf;
>   
>   	src_buf = cjpeg->hw_param.src_buffer;
> -	dst_buf = cjpeg->hw_param.dst_buffer;
>   
>   	mtk_jpeg_dec_reset(cjpeg->reg_base);
>   	clk_disable_unprepare(cjpeg->pm.dec_clk.clk_info->jpegdec_clk);
> @@ -462,7 +504,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
>   	atomic_inc(&cjpeg->hw_rdy);
>   	wake_up(&master_jpeg->dec_hw_wq);
>   	v4l2_m2m_buf_done(src_buf, buf_state);
> -	v4l2_m2m_buf_done(dst_buf, buf_state);
> +	mtk_jpegdec_put_buf(cjpeg);
>   }
>   
>   int mtk_jpegdec_init_pm(struct mtk_jpegdec_comp_dev *mtkdev)
> @@ -559,7 +601,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
>   
>   dec_end:
>   	v4l2_m2m_buf_done(src_buf, buf_state);
> -	v4l2_m2m_buf_done(dst_buf, buf_state);
> +	mtk_jpegdec_put_buf(jpeg);
>   	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
>   	clk_disable_unprepare(jpeg->pm.dec_clk.clk_info->jpegdec_clk);
>   	pm_runtime_put(ctx->jpeg->dev);



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V1, 5/6] media: mtk-jpegdec: add output pic reorder interface

[kyrie.wu]

On Mon, 2022-02-07 at 15:50 +0100, AngeloGioacchino Del Regno wrote:
> Il 03/12/21 06:34, kyrie.wu ha scritto:
> > add output reorder func to reorder the output images
> > to ensure the output pic is consistent with the input images.
> > 
> > Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
> > ---
> >   drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c | 50
> > +++++++++++++++++++++--
> >   1 file changed, 46 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> > index 9138ecb..fad5bf1c 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
> > @@ -443,6 +443,49 @@ void mtk_jpeg_dec_set_config(void __iomem
> > *base,
> >   	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
> >   }
> >   
> > +void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)
> 
> This function is used only in this file, hence it should be static.
Thanks. I will fix it.
> 
> > +{
> > +	struct mtk_jpeg_src_buf *dst_done_buf, *tmp_dst_done_buf;
> > +	struct vb2_v4l2_buffer *dst_buffer;
> > +	struct list_head *temp_entry;
> > +	struct list_head *pos = NULL;
> > +	struct mtk_jpeg_ctx *ctx;
> > +	unsigned long flags;
> > +
> > +	ctx = jpeg->hw_param.curr_ctx;
> > +	if (!ctx) {
> > +		dev_err(jpeg->dev, "comp_jpeg ctx fail !!!\n");
> 
> Since this is unlikely to happen (or should be unlikely anyway!!),
> this print
> should then be a dev_dbg()
Thanks. I will consider your suggestion.
> 
> > +		return;
> > +	}
> > +
> > +	dst_buffer = jpeg->hw_param.dst_buffer;
> > +	if (!dst_buffer) {
> > +		dev_err(jpeg->dev, "comp_jpeg dst_buffer fail !!!\n");
> > +		return;
> > +	}
> > +
> > +	dst_done_buf = container_of(dst_buffer, struct
> > mtk_jpeg_src_buf, b);
> > +
> > +	spin_lock_irqsave(&ctx->done_queue_lock, flags);
> > +	list_add_tail(&dst_done_buf->list, &ctx->dst_done_queue);
> > +	while (!list_empty(&ctx->dst_done_queue) &&
> > +		(pos != &ctx->dst_done_queue)) {
> > +		list_for_each_prev_safe(pos, temp_entry,
> > +			(&ctx->dst_done_queue)) {
> > +			tmp_dst_done_buf = list_entry(pos,
> > +				struct mtk_jpeg_src_buf, list);
> > +			if (tmp_dst_done_buf->frame_num ==
> > +				ctx->last_done_frame_num) {
> > +				list_del(&tmp_dst_done_buf->list);
> > +				v4l2_m2m_buf_done(&tmp_dst_done_buf->b,
> > +					VB2_BUF_STATE_DONE);
> > +				ctx->last_done_frame_num++;
> > +			}
> > +		}
> > +	}
> > +	spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
> > +}
> > +
> >   static void mtk_jpegdec_timeout_work(struct work_struct *work)
> >   {
> >   	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > @@ -450,10 +493,9 @@ static void mtk_jpegdec_timeout_work(struct
> > work_struct *work)
> >   		container_of(work, struct mtk_jpegdec_comp_dev,
> >   		job_timeout_work.work);
> >   	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
> > -	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> > +	struct vb2_v4l2_buffer *src_buf;
> >   
> >   	src_buf = cjpeg->hw_param.src_buffer;
> > -	dst_buf = cjpeg->hw_param.dst_buffer;
> >   
> >   	mtk_jpeg_dec_reset(cjpeg->reg_base);
> >   	clk_disable_unprepare(cjpeg->pm.dec_clk.clk_info->jpegdec_clk);
> > @@ -462,7 +504,7 @@ static void mtk_jpegdec_timeout_work(struct
> > work_struct *work)
> >   	atomic_inc(&cjpeg->hw_rdy);
> >   	wake_up(&master_jpeg->dec_hw_wq);
> >   	v4l2_m2m_buf_done(src_buf, buf_state);
> > -	v4l2_m2m_buf_done(dst_buf, buf_state);
> > +	mtk_jpegdec_put_buf(cjpeg);
> >   }
> >   
> >   int mtk_jpegdec_init_pm(struct mtk_jpegdec_comp_dev *mtkdev)
> > @@ -559,7 +601,7 @@ static irqreturn_t
> > mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> >   
> >   dec_end:
> >   	v4l2_m2m_buf_done(src_buf, buf_state);
> > -	v4l2_m2m_buf_done(dst_buf, buf_state);
> > +	mtk_jpegdec_put_buf(jpeg);
> >   	v4l2_m2m_job_finish(master_jpeg->m2m_dev, ctx->fh.m2m_ctx);
> >   	clk_disable_unprepare(jpeg->pm.dec_clk.clk_info->jpegdec_clk);
> >   	pm_runtime_put(ctx->jpeg->dev);
> 
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V1, 6/6] media: mtk-jpegdec: refactor jpegdec func interface

[kyrie.wu]

refactor the func interface of mtk_jpeg_dec_set_config
for decode

Signed-off-by: kyrie.wu <kyrie.wu@mediatek.com>
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c    | 28 ++++++++++-
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h    |  1 +
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c  | 55 ++++++++++++----------
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h  |  7 +--
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_reg.h |  1 +
 5 files changed, 62 insertions(+), 30 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index 2518660..22b6a4c 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -296,6 +296,31 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_pix_format_mplane *pix_mp,
 	return 0;
 }
 
+static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
+{
+	struct v4l2_fh *fh = file->private_data;
+	struct mtk_jpeg_ctx *ctx = mtk_jpeg_fh_to_ctx(priv);
+	struct vb2_queue *vq;
+	struct vb2_buffer *vb;
+	struct mtk_jpeg_src_buf *jpeg_src_buf;
+
+	if (buf->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		goto end;
+
+	vq = v4l2_m2m_get_vq(fh->m2m_ctx, buf->type);
+	if (buf->index >= vq->num_buffers) {
+		dev_err(ctx->jpeg->dev, "buffer index out of range\n");
+		return -EINVAL;
+	}
+
+	vb = vq->bufs[buf->index];
+	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb);
+	jpeg_src_buf->bs_size = buf->m.planes[0].bytesused;
+
+end:
+	return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
+}
+
 static int mtk_jpeg_g_fmt_vid_mplane(struct file *file, void *priv,
 				     struct v4l2_format *f)
 {
@@ -620,7 +645,7 @@ static const struct v4l2_ioctl_ops mtk_jpeg_dec_ioctl_ops = {
 	.vidioc_g_fmt_vid_out_mplane    = mtk_jpeg_g_fmt_vid_mplane,
 	.vidioc_s_fmt_vid_cap_mplane    = mtk_jpeg_s_fmt_vid_cap_mplane,
 	.vidioc_s_fmt_vid_out_mplane    = mtk_jpeg_s_fmt_vid_out_mplane,
-	.vidioc_qbuf                    = v4l2_m2m_ioctl_qbuf,
+	.vidioc_qbuf                    = mtk_jpeg_qbuf,
 	.vidioc_subscribe_event         = mtk_jpeg_subscribe_event,
 	.vidioc_g_selection		= mtk_jpeg_dec_g_selection,
 
@@ -1218,6 +1243,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
 	mtk_jpeg_dec_reset(comp_jpeg[hw_id]->reg_base);
 	mtk_jpeg_dec_set_config(jpeg->reg_base,
 		&jpeg_src_buf->dec_param,
+		jpeg_src_buf->bs_size,
 		&bs, &fb);
 	mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index d67c49f..90cdb3c 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
@@ -79,6 +79,7 @@ struct mtk_jpeg_variant {
 struct mtk_jpeg_src_buf {
 	struct vb2_v4l2_buffer b;
 	struct list_head list;
+	u32 bs_size;
 	struct mtk_jpeg_dec_param dec_param;
 
 	struct mtk_jpeg_ctx *curr_ctx;
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
index fad5bf1c..ec23e61 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
@@ -334,12 +334,14 @@ static void mtk_jpeg_dec_set_bs_write_ptr(void __iomem *base, u32 ptr)
 	writel(ptr, base + JPGDEC_REG_FILE_BRP);
 }
 
-static void mtk_jpeg_dec_set_bs_info(void __iomem *base, u32 addr, u32 size)
+static void mtk_jpeg_dec_set_bs_info(void __iomem *base, u32 addr, u32 size,
+				     u32 bitstream_size)
 {
 	mtk_jpeg_verify_align(addr, 16, JPGDEC_REG_FILE_ADDR);
 	mtk_jpeg_verify_align(size, 128, JPGDEC_REG_FILE_TOTAL_SIZE);
 	writel(addr, base + JPGDEC_REG_FILE_ADDR);
 	writel(size, base + JPGDEC_REG_FILE_TOTAL_SIZE);
+	writel(bitstream_size, base + JPGDEC_REG_BIT_STREAM_SIZE);
 }
 
 static void mtk_jpeg_dec_set_comp_id(void __iomem *base, u32 id_y, u32 id_u,
@@ -408,39 +410,40 @@ static void mtk_jpeg_dec_set_sampling_factor(void __iomem *base, u32 comp_num,
 }
 
 void mtk_jpeg_dec_set_config(void __iomem *base,
-			     struct mtk_jpeg_dec_param *config,
+			     struct mtk_jpeg_dec_param *cfg,
+			      u32 bitstream_size,
 			     struct mtk_jpeg_bs *bs,
 			     struct mtk_jpeg_fb *fb)
 {
-	mtk_jpeg_dec_set_brz_factor(base, 0, 0, config->uv_brz_w, 0);
+	mtk_jpeg_dec_set_brz_factor(base, 0, 0, cfg->uv_brz_w, 0);
 	mtk_jpeg_dec_set_dec_mode(base, 0);
-	mtk_jpeg_dec_set_comp0_du(base, config->unit_num);
-	mtk_jpeg_dec_set_total_mcu(base, config->total_mcu);
-	mtk_jpeg_dec_set_bs_info(base, bs->str_addr, bs->size);
+	mtk_jpeg_dec_set_comp0_du(base, cfg->unit_num);
+	mtk_jpeg_dec_set_total_mcu(base, cfg->total_mcu);
+	mtk_jpeg_dec_set_bs_info(base, bs->str_addr, bs->size, bitstream_size);
 	mtk_jpeg_dec_set_bs_write_ptr(base, bs->end_addr);
-	mtk_jpeg_dec_set_du_membership(base, config->membership, 1,
-				       (config->comp_num == 1) ? 1 : 0);
-	mtk_jpeg_dec_set_comp_id(base, config->comp_id[0], config->comp_id[1],
-				 config->comp_id[2]);
-	mtk_jpeg_dec_set_q_table(base, config->qtbl_num[0],
-				 config->qtbl_num[1], config->qtbl_num[2]);
-	mtk_jpeg_dec_set_sampling_factor(base, config->comp_num,
-					 config->sampling_w[0],
-					 config->sampling_h[0],
-					 config->sampling_w[1],
-					 config->sampling_h[1],
-					 config->sampling_w[2],
-					 config->sampling_h[2]);
-	mtk_jpeg_dec_set_mem_stride(base, config->mem_stride[0],
-				    config->mem_stride[1]);
-	mtk_jpeg_dec_set_img_stride(base, config->img_stride[0],
-				    config->img_stride[1]);
+	mtk_jpeg_dec_set_du_membership(base, cfg->membership, 1,
+				       (cfg->comp_num == 1) ? 1 : 0);
+	mtk_jpeg_dec_set_comp_id(base, cfg->comp_id[0], cfg->comp_id[1],
+				 cfg->comp_id[2]);
+	mtk_jpeg_dec_set_q_table(base, cfg->qtbl_num[0],
+				 cfg->qtbl_num[1], cfg->qtbl_num[2]);
+	mtk_jpeg_dec_set_sampling_factor(base, cfg->comp_num,
+					 cfg->sampling_w[0],
+					 cfg->sampling_h[0],
+					 cfg->sampling_w[1],
+					 cfg->sampling_h[1],
+					 cfg->sampling_w[2],
+					 cfg->sampling_h[2]);
+	mtk_jpeg_dec_set_mem_stride(base, cfg->mem_stride[0],
+				    cfg->mem_stride[1]);
+	mtk_jpeg_dec_set_img_stride(base, cfg->img_stride[0],
+				    cfg->img_stride[1]);
 	mtk_jpeg_dec_set_dst_bank0(base, fb->plane_addr[0],
 				   fb->plane_addr[1], fb->plane_addr[2]);
 	mtk_jpeg_dec_set_dst_bank1(base, 0, 0, 0);
-	mtk_jpeg_dec_set_dma_group(base, config->dma_mcu, config->dma_group,
-				   config->dma_last_mcu);
-	mtk_jpeg_dec_set_pause_mcu_idx(base, config->total_mcu);
+	mtk_jpeg_dec_set_dma_group(base, cfg->dma_mcu, cfg->dma_group,
+				   cfg->dma_last_mcu);
+	mtk_jpeg_dec_set_pause_mcu_idx(base, cfg->total_mcu);
 }
 
 void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
index 87aaa5c..e7a6c0b 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
@@ -71,9 +71,10 @@ int mtk_jpeg_dec_fill_param(struct mtk_jpeg_dec_param *param);
 u32 mtk_jpeg_dec_get_int_status(void __iomem *dec_reg_base);
 u32 mtk_jpeg_dec_enum_result(u32 irq_result);
 void mtk_jpeg_dec_set_config(void __iomem *base,
-			     struct mtk_jpeg_dec_param *config,
-			     struct mtk_jpeg_bs *bs,
-			     struct mtk_jpeg_fb *fb);
+			    struct mtk_jpeg_dec_param *cfg,
+			    u32 bitstream_size,
+			    struct mtk_jpeg_bs *bs,
+			    struct mtk_jpeg_fb *fb);
 void mtk_jpeg_dec_reset(void __iomem *dec_reg_base);
 void mtk_jpeg_dec_start(void __iomem *dec_reg_base);
 
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_reg.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_reg.h
index 21ec8f9..27b7711 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_reg.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_reg.h
@@ -45,5 +45,6 @@
 #define JPGDEC_REG_QT_ID		0x0270
 #define JPGDEC_REG_INTERRUPT_STATUS	0x0274
 #define JPGDEC_REG_STATUS		0x0278
+#define JPGDEC_REG_BIT_STREAM_SIZE	0x0344
 
 #endif /* _MTK_JPEG_REG_H */
-- 
2.6.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel