[PATCH V6, 0/5] Support multi-hardware jpeg encoding using of_platform_populate

This series adds support for multi hardware jpeg encoding, by first
adding use of_platform_populate to manage each hardware information:
interrupt, clock, register bases and power. Secondly add encoding 
work queue to deal with the encoding requestsof 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.
Encoding worked for this chip.

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

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

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

Patch 5 add output picture reorder function to order images.
---
Changes compared with v5:
- use of_platform_populate to replace component framework to
manage multi-hardware in patch 2.

Changes compared with v4:
--No change compaered with v4

Changes compared with v3:
--Structure patches for consistency, non-backward
  compatible and do not break any existing functionality

Changes compared with v2:
--Split the last two patches into several patches
  to enhance readability
--Correct some syntax errors
--Explain why the component framework is used

Changes compared with v1:
--Add jpeg encoder dt-bindings for MT8195
--Use component framework to manage jpegenc HW
--Add jpegenc output pic reorder function interface

kyrie.wu (5):
  dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible
  media: mtk-jpegenc: manage jpegenc multi-hardware
  media: mtk-jpegenc: add jpegenc timeout func interface
  media: mtk-jpegenc: add jpeg encode worker interface
  media: mtk-jpegenc: add output pic reorder interface

 .../bindings/media/mediatek-jpeg-encoder.yaml      |   3 +
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c    | 287 +++++++++++++++----
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h    |  91 +++++-
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c  |   1 +
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h  |   3 +-
 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c  | 316 ++++++++++++++++++++-
 6 files changed, 644 insertions(+), 57 deletions(-)

-- 
2.6.4


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Add mediatek, mt8195-jpgenc compatible to binding document

Signed-off-by: kyrie.wu <redacted>
---
V6: no change
---
 Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
index fcd9b82..e93ccf5 100644
--- a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml

@@ -18,6 +18,9 @@ properties:
       - enum:
           - mediatek,mt2701-jpgenc
           - mediatek,mt8183-jpgenc
+          - mediatek,mt8195-jpgenc
+          - mediatek,mt8195-jpgenc0
+          - mediatek,mt8195-jpgenc1
       - const: mediatek,mtk-jpgenc
   reg:
     maxItems: 1

-- 
2.6.4


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

kyrie.wu wrote:

Add mediatek, mt8195-jpgenc compatible to binding document

Signed-off-by: kyrie.wu <redacted>
---
V6: no change

Hi Kyrie

Your patch does not seem to apply on top of linus or linus/next.

In linus/next they still have the .txt file:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt?id=7afeac307a9561e3a93682c1e7eb22f918aa1187

What are you using?

Thanks

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

On Fri, 2021-12-03 at 14:43 +0100, Ricardo Ribalda wrote:

kyrie.wu wrote:

quoted

Add mediatek, mt8195-jpgenc compatible to binding document

Signed-off-by: kyrie.wu <redacted>
---
V6: no change

Hi Kyrie

Your patch does not seem to apply on top of linus or linus/next.

In linus/next they still have the .txt file:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.txt?id=7afeac307a9561e3a93682c1e7eb22f918aa1187

What are you using?

Thanks

I will fix this issue in the next code version, thanks.

quoted

---
 Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml 
| 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-

encoder.yaml b/Documentation/devicetree/bindings/media/mediatek-
jpeg-encoder.yaml
index fcd9b82..e93ccf5 100644

--- a/Documentation/devicetree/bindings/media/mediatek-jpeg-

encoder.yaml

+++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-

encoder.yaml

@@ -18,6 +18,9 @@ properties:
       - enum:
           - mediatek,mt2701-jpgenc
           - mediatek,mt8183-jpgenc
+          - mediatek,mt8195-jpgenc
+          - mediatek,mt8195-jpgenc0
+          - mediatek,mt8195-jpgenc1
       - const: mediatek,mtk-jpgenc
   reg:
     maxItems: 1

-- 
2.6.4


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

manage each hardware information, including irq/clk/power.
the hardware includes HW0 and HW1.

Signed-off-by: kyrie.wu <redacted>
---
V6: use of_platform_populate to replace component framework
to manage multi-hardware
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   | 103 +++++++---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |  55 +++++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 232 +++++++++++++++++++++-
 3 files changed, 362 insertions(+), 28 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index a89c7b2..da7eb84 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c

@@ -1353,33 +1353,40 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 	spin_lock_init(&jpeg->hw_lock);
 	jpeg->dev = &pdev->dev;
 	jpeg->variant = of_device_get_match_data(jpeg->dev);
-	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;
-	}
+	if (!jpeg->variant->is_encoder) {
+		INIT_DELAYED_WORK(&jpeg->job_timeout_work,
+				mtk_jpeg_job_timeout_work);
 
-	jpeg_irq = platform_get_irq(pdev, 0);
-	if (jpeg_irq < 0) {
-		dev_err(&pdev->dev, "Failed to get jpeg_irq %d.\n", jpeg_irq);
-		return jpeg_irq;
-	}
+		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;
+		}
 
-	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;
-	}
+		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 = mtk_jpeg_clk_init(jpeg);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to init clk, err %d\n", ret);
-		goto err_clk_init;
+		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);
 	}
 
 	ret = v4l2_device_register(&pdev->dev, &jpeg->v4l2_dev);

@@ -1426,9 +1433,16 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 		  jpeg->variant->dev_name, jpeg->vdev->num,
 		  VIDEO_MAJOR, jpeg->vdev->minor);
 
-	platform_set_drvdata(pdev, jpeg);
+	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;
+		}
+	}
 
-	pm_runtime_enable(&pdev->dev);
+	platform_set_drvdata(pdev, jpeg);
 
 	return 0;
 

@@ -1539,6 +1553,19 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = {
 	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
 };
 
+static const struct mtk_jpeg_variant mtk_jpegenc_drvdata = {
+	.is_encoder = true,
+	.formats = mtk_jpeg_enc_formats,
+	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
+	.qops = &mtk_jpeg_enc_qops,
+	.m2m_ops = &mtk_jpeg_enc_m2m_ops,
+	.dev_name = "mtk-jpeg-enc",
+	.ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+};
+
+#if defined(CONFIG_OF)
 static const struct of_device_id mtk_jpeg_match[] = {
 	{
 		.compatible = "mediatek,mt8173-jpgdec",

@@ -1552,10 +1579,14 @@ static const struct of_device_id mtk_jpeg_match[] = {
 		.compatible = "mediatek,mtk-jpgenc",
 		.data = &mtk_jpeg_drvdata,
 	},
+	{
+		.compatible = "mediatek,mt8195-jpgenc",
+		.data = &mtk_jpegenc_drvdata,
+	},
 	{},
 };
-
 MODULE_DEVICE_TABLE(of, mtk_jpeg_match);
+#endif
 
 static struct platform_driver mtk_jpeg_driver = {
 	.probe = mtk_jpeg_probe,

@@ -1567,7 +1598,25 @@ static struct platform_driver mtk_jpeg_driver = {
 	},
 };
 
-module_platform_driver(mtk_jpeg_driver);
+static struct platform_driver * const mtk_jpeg_source_drivers[] = {
+	&mtk_jpegenc_hw_driver,
+	&mtk_jpeg_driver,
+};
+static int __init mtk_jpeg_init(void)
+{
+	return platform_register_drivers(mtk_jpeg_source_drivers,
+		ARRAY_SIZE(mtk_jpeg_source_drivers));
+}
+
+static void __exit mtk_jpeg_exit(void)
+{
+	platform_unregister_drivers(mtk_jpeg_source_drivers,
+		ARRAY_SIZE(mtk_jpeg_source_drivers));
+}
+
+module_init(mtk_jpeg_init);
+module_exit(mtk_jpeg_exit);
+
 
 MODULE_DESCRIPTION("MediaTek JPEG codec driver");
 MODULE_LICENSE("GPL v2");

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index 595f7f1..7d013de 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h

@@ -60,6 +60,7 @@ enum mtk_jpeg_ctx_state {
  * @cap_q_default_fourcc:	capture queue default fourcc
  */
 struct mtk_jpeg_variant {
+	bool is_encoder;
 	struct clk_bulk_data *clks;
 	int num_clks;
 	struct mtk_jpeg_fmt *formats;

@@ -74,6 +75,55 @@ struct mtk_jpeg_variant {
 	u32 cap_q_default_fourcc;
 };
 
+enum mtk_jpegenc_hw_id {
+	MTK_JPEGENC_HW0,
+	MTK_JPEGENC_HW1,
+	MTK_JPEGENC_HW_MAX,
+};
+
+/**
+ * struct mtk_jpegenc_clk_info - Structure used to store clock name
+ */
+struct mtk_jpegenc_clk_info {
+	const char	*clk_name;
+	struct clk	*jpegenc_clk;
+};
+
+/**
+ * struct mtk_vcodec_clk - Structure used to store vcodec clock information
+ */
+struct mtk_jpegenc_clk {
+	struct mtk_jpegenc_clk_info	*clk_info;
+	int	clk_num;
+};
+
+/**
+ * struct mtk_vcodec_pm - Power management data structure
+ */
+struct mtk_jpegenc_pm {
+	struct mtk_jpegenc_clk	venc_clk;
+	struct device	*dev;
+	struct mtk_jpegenc_comp_dev	*mtkdev;
+};
+
+/**
+ * struct mtk_jpegenc_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_jpegenc_pm
+ * @jpegenc_irq:	    jpeg encode irq num
+ */
+struct mtk_jpegenc_comp_dev {
+	struct device		*dev;
+	struct platform_device *plat_dev;
+	void __iomem		*reg_base;
+	struct mtk_jpeg_dev *master_dev;
+	struct mtk_jpegenc_pm pm;
+	int jpegenc_irq;
+};
+
 /**
  * struct mtk_jpeg_dev - JPEG IP abstraction
  * @lock:		the mutex protecting this structure

@@ -102,6 +152,9 @@ struct mtk_jpeg_dev {
 	struct device		*larb;
 	struct delayed_work job_timeout_work;
 	const struct mtk_jpeg_variant *variant;
+
+	void __iomem *reg_encbase[MTK_JPEGENC_HW_MAX];
+	struct mtk_jpegenc_comp_dev *enc_hw_dev[MTK_JPEGENC_HW_MAX];
 };
 
 /**

@@ -162,4 +215,6 @@ struct mtk_jpeg_ctx {
 	struct v4l2_ctrl_handler ctrl_hdl;
 };
 
+extern struct platform_driver mtk_jpegenc_hw_driver;
+
 #endif /* _MTK_JPEG_CORE_H */

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
index 1cf037b..4ccda1d 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c

@@ -4,12 +4,27 @@
  * Author: Xia Jiang <xia.jiang@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-dma-contig.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_enc_hw.h"
 
 static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = {

@@ -30,6 +45,21 @@ static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = {
 	{.quality_param = 97, .hardware_value = JPEG_ENC_QUALITY_Q97},
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id mtk_jpegenc_drv_ids[] = {
+	{
+		.compatible = "mediatek,mt8195-jpgenc0",
+		.data = (void *)MTK_JPEGENC_HW0,
+	},
+	{
+		.compatible = "mediatek,mt8195-jpgenc1",
+		.data = (void *)MTK_JPEGENC_HW1,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids);
+#endif
+
 void mtk_jpeg_enc_reset(void __iomem *base)
 {
 	writel(0, base + JPEG_ENC_RSTB);

@@ -152,3 +182,203 @@ void mtk_jpeg_set_enc_params(struct mtk_jpeg_ctx *ctx,  void __iomem *base)
 
 	writel(ctx->restart_interval, base + JPEG_ENC_RST_MCU_NUM);
 }
+
+static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
+{
+	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+	enum vb2_buffer_state buf_state;
+	struct mtk_jpeg_ctx *ctx;
+	u32 result_size;
+	u32 irq_status;
+
+	struct mtk_jpegenc_comp_dev *jpeg = priv;
+	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
+
+	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
+		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
+	if (irq_status)
+		writel(0, jpeg->reg_base + JPEG_ENC_INT_STS);
+	if (!(irq_status & JPEG_ENC_INT_STATUS_DONE))
+		return IRQ_NONE;
+
+	ctx = v4l2_m2m_get_curr_priv(master_jpeg->m2m_dev);
+	if (!ctx) {
+		v4l2_err(&master_jpeg->v4l2_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);
+	result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base);
+	vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
+	buf_state = VB2_BUF_STATE_DONE;
+	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_jpegenc_hw_init_irq(struct mtk_jpegenc_comp_dev *dev)
+{
+	struct platform_device *pdev = dev->plat_dev;
+	int ret;
+
+	dev->jpegenc_irq = platform_get_irq(pdev, 0);
+	if (dev->jpegenc_irq < 0) {
+		dev_err(&pdev->dev, "Failed to get irq resource");
+		return dev->jpegenc_irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, dev->jpegenc_irq,
+		mtk_jpegenc_hw_irq_handler, 0, pdev->name, dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to devm_request_irq %d (%d)",
+			dev->jpegenc_irq, ret);
+		return -ENOENT;
+	}
+
+	return 0;
+}
+
+int mtk_jpegenc_init_pm(struct mtk_jpegenc_comp_dev *mtkdev)
+{
+	struct mtk_jpegenc_clk_info *clk_info;
+	struct mtk_jpegenc_clk *jpegenc_clk;
+	struct platform_device *pdev;
+	struct mtk_jpegenc_pm *pm;
+	int i, ret;
+
+	pdev = mtkdev->plat_dev;
+	pm = &mtkdev->pm;
+	pm->dev = &pdev->dev;
+	pm->mtkdev = mtkdev;
+	jpegenc_clk = &pm->venc_clk;
+	jpegenc_clk->clk_num =
+		of_property_count_strings(pdev->dev.of_node, "clock-names");
+	if (!jpegenc_clk->clk_num) {
+		dev_err(&pdev->dev, "Failed to get jpegenc clock count\n");
+		return -EINVAL;
+	}
+
+	jpegenc_clk->clk_info = devm_kcalloc(&pdev->dev,
+		jpegenc_clk->clk_num,
+		sizeof(*clk_info),
+		GFP_KERNEL);
+	if (!jpegenc_clk->clk_info)
+		return -ENOMEM;
+
+	for (i = 0; i < jpegenc_clk->clk_num; i++) {
+		clk_info = &jpegenc_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->jpegenc_clk = devm_clk_get(&pdev->dev,
+			clk_info->clk_name);
+		if (IS_ERR(clk_info->jpegenc_clk)) {
+			dev_err(&pdev->dev, "devm_clk_get (%d)%s fail",
+				i, clk_info->clk_name);
+			return PTR_ERR(clk_info->jpegenc_clk);
+		}
+	}
+
+	pm_runtime_enable(&pdev->dev);
+
+	return ret;
+}
+
+void mtk_jpegenc_release_pm(struct mtk_jpegenc_comp_dev *mtkdev)
+{
+	struct platform_device *pdev = mtkdev->plat_dev;
+
+	pm_runtime_disable(&pdev->dev);
+}
+
+static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
+{
+	struct mtk_jpeg_dev *master_dev;
+	const struct of_device_id *of_id;
+	struct mtk_jpegenc_comp_dev *dev;
+	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_jpegenc_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_jpegenc_hw_init_irq(dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register JPEGENC irq handler.\n");
+		goto err;
+	}
+
+	of_id = of_match_device(mtk_jpegenc_drv_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_jpegenc_hw_id)of_id->data;
+	if (comp_idx < MTK_JPEGENC_HW_MAX) {
+		master_dev->enc_hw_dev[comp_idx] = dev;
+		master_dev->reg_encbase[comp_idx] = dev->reg_base;
+		dev->master_dev = master_dev;
+	}
+
+	platform_set_drvdata(pdev, dev);
+
+	return 0;
+
+err:
+	mtk_jpegenc_release_pm(dev);
+	return ret;
+}
+
+
+static int mtk_jpegenc_remove(struct platform_device *pdev)
+{
+	struct mtk_jpegenc_comp_dev *dev = platform_get_drvdata(pdev);
+
+	mtk_jpegenc_release_pm(dev);
+
+	return 0;
+}
+
+struct platform_driver mtk_jpegenc_hw_driver = {
+	.probe = mtk_jpegenc_hw_probe,
+	.remove = mtk_jpegenc_remove,
+	.driver = {
+		.name = "mtk-jpegenc-hw",
+		.of_match_table = of_match_ptr(mtk_jpegenc_drv_ids),
+	},
+};

-- 
2.6.4


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

kyrie.wu wrote:

Maybe better use  devm_platform_ioremap_resource()

+		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;
+		}
 
-	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;
-	}
+		jpeg_irq = platform_get_irq(pdev, 0);

Can you share a device tree that uses this? 

You can't you have all the device in same level, instead of making them
childs of this device?

+		ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
+			&pdev->dev);

Maybe you want devm_of_platform_populate()?

This is not very clear. It would be better to refactor the change and
have a module_platform_driver() macro for jpegenc_hw_driver, where its
probe and remove exists.

Please no extern here:

Have you tried with a threaded_irq?

+	ret = devm_request_irq(&pdev->dev, dev->jpegenc_irq,
+		mtk_jpegenc_hw_irq_handler, 0, pdev->name, dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to devm_request_irq %d (%d)",
+			dev->jpegenc_irq, ret);
+		return -ENOENT;
+	}
+
+	return 0;
+}
+
+int mtk_jpegenc_init_pm(struct mtk_jpegenc_comp_dev *mtkdev)
+{
+	struct mtk_jpegenc_clk_info *clk_info;
+	struct mtk_jpegenc_clk *jpegenc_clk;
+	struct platform_device *pdev;
+	struct mtk_jpegenc_pm *pm;
+	int i, ret;
+
+	pdev = mtkdev->plat_dev;
+	pm = &mtkdev->pm;
+	pm->dev = &pdev->dev;
+	pm->mtkdev = mtkdev;

Can't you simplify with devm_clk_bulk_get_all

+	jpegenc_clk = &pm->venc_clk;
+	jpegenc_clk->clk_num =
+		of_property_count_strings(pdev->dev.of_node, "clock-names");
+	if (!jpegenc_clk->clk_num) {
+		dev_err(&pdev->dev, "Failed to get jpegenc clock count\n");
+		return -EINVAL;
+	}
+
+	jpegenc_clk->clk_info = devm_kcalloc(&pdev->dev,
+		jpegenc_clk->clk_num,
+		sizeof(*clk_info),
+		GFP_KERNEL);
+	if (!jpegenc_clk->clk_info)
+		return -ENOMEM;
+
+	for (i = 0; i < jpegenc_clk->clk_num; i++) {
+		clk_info = &jpegenc_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->jpegenc_clk = devm_clk_get(&pdev->dev,
+			clk_info->clk_name);
+		if (IS_ERR(clk_info->jpegenc_clk)) {
+			dev_err(&pdev->dev, "devm_clk_get (%d)%s fail",
+				i, clk_info->clk_name);
+			return PTR_ERR(clk_info->jpegenc_clk);
+		}
+	}
+

Can't you enable the pm after all the probing is completed to simplify
the error handling?

+	pm_runtime_enable(&pdev->dev);
+
+	return ret;
+}
+
+void mtk_jpegenc_release_pm(struct mtk_jpegenc_comp_dev *mtkdev)
+{
+	struct platform_device *pdev = mtkdev->plat_dev;
+
+	pm_runtime_disable(&pdev->dev);
+}
+
+static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
+{
+	struct mtk_jpeg_dev *master_dev;
+	const struct of_device_id *of_id;
+	struct mtk_jpegenc_comp_dev *dev;
+	int ret, comp_idx;
+
+	struct device *decs = &pdev->dev;
+

When is the parent NULL?

+	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_jpegenc_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_jpegenc_hw_init_irq(dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register JPEGENC irq handler.\n");
+		goto err;
+	}
+

please use of_device_get_match_data()

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

What if the drvdata is freed before you set it?

+	comp_idx = (enum mtk_jpegenc_hw_id)of_id->data;
+	if (comp_idx < MTK_JPEGENC_HW_MAX) {
+		master_dev->enc_hw_dev[comp_idx] = dev;
+		master_dev->reg_encbase[comp_idx] = dev->reg_base;
+		dev->master_dev = master_dev;
+	}
+
+	platform_set_drvdata(pdev, dev);
+
+	return 0;
+
+err:
+	mtk_jpegenc_release_pm(dev);
+	return ret;
+}
+
+
+static int mtk_jpegenc_remove(struct platform_device *pdev)
+{
+	struct mtk_jpegenc_comp_dev *dev = platform_get_drvdata(pdev);
+
+	mtk_jpegenc_release_pm(dev);
+
+	return 0;
+}
+
+struct platform_driver mtk_jpegenc_hw_driver = {
+	.probe = mtk_jpegenc_hw_probe,
+	.remove = mtk_jpegenc_remove,
+	.driver = {
+		.name = "mtk-jpegenc-hw",
+		.of_match_table = of_match_ptr(mtk_jpegenc_drv_ids),
+	},
+};
-- 
2.6.4


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

On Fri, 2021-12-03 at 15:54 +0100, Ricardo Ribalda wrote:

kyrie.wu wrote:

quoted

manage each hardware information, including irq/clk/power.
the hardware includes HW0 and HW1.

Signed-off-by: kyrie.wu <redacted>
---
V6: use of_platform_populate to replace component framework
to manage multi-hardware
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   | 103 +++++++---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |  55 +++++
 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 232
+++++++++++++++++++++-
 3 files changed, 362 insertions(+), 28 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c

b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index a89c7b2..da7eb84 100644

--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c

@@ -1353,33 +1353,40 @@ static int mtk_jpeg_probe(struct

platform_device *pdev)
 	spin_lock_init(&jpeg->hw_lock);
 	jpeg->dev = &pdev->dev;
 	jpeg->variant = of_device_get_match_data(jpeg->dev);
-	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;
-	}
+	if (!jpeg->variant->is_encoder) {
+		INIT_DELAYED_WORK(&jpeg->job_timeout_work,
+				mtk_jpeg_job_timeout_work);
 
-	jpeg_irq = platform_get_irq(pdev, 0);
-	if (jpeg_irq < 0) {
-		dev_err(&pdev->dev, "Failed to get jpeg_irq %d.\n",
jpeg_irq);
-		return jpeg_irq;
-	}

Maybe better use  devm_platform_ioremap_resource()

It would used in the next code version.
thanks.

quoted

+		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;
+		}
 
-	ret = devm_request_irq(&pdev->dev, jpeg_irq,
-			       jpeg->variant->irq_handler, 0, pdev-

quoted

name, jpeg);

-	if (ret) {
-		dev_err(&pdev->dev, "Failed to request jpeg_irq %d
(%d)\n",
-			jpeg_irq, ret);
-		goto err_req_irq;
-	}
+		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 = mtk_jpeg_clk_init(jpeg);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to init clk, err %d\n",
ret);
-		goto err_clk_init;
+		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);
 	}
 
 	ret = v4l2_device_register(&pdev->dev, &jpeg->v4l2_dev);

@@ -1426,9 +1433,16 @@ static int mtk_jpeg_probe(struct

platform_device *pdev)
 		  jpeg->variant->dev_name, jpeg->vdev->num,
 		  VIDEO_MAJOR, jpeg->vdev->minor);
 
-	platform_set_drvdata(pdev, jpeg);
+	if (jpeg->variant->is_encoder) {

Can you share a device tree that uses this?

I will supply in the next code version.
thanks.

 

You can't you have all the device in same level, instead of making
them
childs of this device?

MT8195 has two encode hw, we wanna use one device node to use them, so
they can't set in same level.

quoted

+		ret = of_platform_populate(pdev->dev.of_node, NULL,
NULL,
+			&pdev->dev);

Maybe you want devm_of_platform_populate()?

It would fixed in the next code version.
thanks.

quoted

+		if (ret) {
+			v4l2_err(&jpeg->v4l2_dev, "Master of platform
populate failed.");
+			goto err_vfd_jpeg_register;
+		}
+	}
 
-	pm_runtime_enable(&pdev->dev);
+	platform_set_drvdata(pdev, jpeg);
 
 	return 0;
 

@@ -1539,6 +1553,19 @@ static const struct mtk_jpeg_variant

mtk_jpeg_drvdata = {
 	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
 };
 
+static const struct mtk_jpeg_variant mtk_jpegenc_drvdata = {
+	.is_encoder = true,
+	.formats = mtk_jpeg_enc_formats,
+	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
+	.qops = &mtk_jpeg_enc_qops,
+	.m2m_ops = &mtk_jpeg_enc_m2m_ops,
+	.dev_name = "mtk-jpeg-enc",
+	.ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+};
+
+#if defined(CONFIG_OF)
 static const struct of_device_id mtk_jpeg_match[] = {
 	{
 		.compatible = "mediatek,mt8173-jpgdec",

@@ -1552,10 +1579,14 @@ static const struct of_device_id

mtk_jpeg_match[] = {
 		.compatible = "mediatek,mtk-jpgenc",
 		.data = &mtk_jpeg_drvdata,
 	},
+	{
+		.compatible = "mediatek,mt8195-jpgenc",
+		.data = &mtk_jpegenc_drvdata,
+	},
 	{},
 };
-
 MODULE_DEVICE_TABLE(of, mtk_jpeg_match);
+#endif
 
 static struct platform_driver mtk_jpeg_driver = {
 	.probe = mtk_jpeg_probe,

@@ -1567,7 +1598,25 @@ static struct platform_driver

mtk_jpeg_driver = {
 	},
 };

This is not very clear. It would be better to refactor the change and
have a module_platform_driver() macro for jpegenc_hw_driver, where
its
probe and remove exists.

This suggestion would used in the next code version.
thanks.

quoted

-module_platform_driver(mtk_jpeg_driver);
+static struct platform_driver * const mtk_jpeg_source_drivers[] =
{
+	&mtk_jpegenc_hw_driver,
+	&mtk_jpeg_driver,
+};
+static int __init mtk_jpeg_init(void)
+{
+	return platform_register_drivers(mtk_jpeg_source_drivers,
+		ARRAY_SIZE(mtk_jpeg_source_drivers));
+}
+
+static void __exit mtk_jpeg_exit(void)
+{
+	platform_unregister_drivers(mtk_jpeg_source_drivers,
+		ARRAY_SIZE(mtk_jpeg_source_drivers));
+}
+
+module_init(mtk_jpeg_init);
+module_exit(mtk_jpeg_exit);
+
 
 MODULE_DESCRIPTION("MediaTek JPEG codec driver");
 MODULE_LICENSE("GPL v2");

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h

b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index 595f7f1..7d013de 100644

--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h

@@ -60,6 +60,7 @@ enum mtk_jpeg_ctx_state {
  * @cap_q_default_fourcc:	capture queue default fourcc
  */
 struct mtk_jpeg_variant {
+	bool is_encoder;
 	struct clk_bulk_data *clks;
 	int num_clks;
 	struct mtk_jpeg_fmt *formats;

@@ -74,6 +75,55 @@ struct mtk_jpeg_variant {
 	u32 cap_q_default_fourcc;
 };
 
+enum mtk_jpegenc_hw_id {
+	MTK_JPEGENC_HW0,
+	MTK_JPEGENC_HW1,
+	MTK_JPEGENC_HW_MAX,
+};
+
+/**
+ * struct mtk_jpegenc_clk_info - Structure used to store clock

name
+ */
+struct mtk_jpegenc_clk_info {
+	const char	*clk_name;
+	struct clk	*jpegenc_clk;
+};
+
+/**
+ * struct mtk_vcodec_clk - Structure used to store vcodec clock
information
+ */
+struct mtk_jpegenc_clk {
+	struct mtk_jpegenc_clk_info	*clk_info;
+	int	clk_num;
+};
+
+/**
+ * struct mtk_vcodec_pm - Power management data structure
+ */
+struct mtk_jpegenc_pm {
+	struct mtk_jpegenc_clk	venc_clk;
+	struct device	*dev;
+	struct mtk_jpegenc_comp_dev	*mtkdev;
+};
+
+/**
+ * struct mtk_jpegenc_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_jpegenc_pm
+ * @jpegenc_irq:	    jpeg encode irq num
+ */
+struct mtk_jpegenc_comp_dev {
+	struct device		*dev;
+	struct platform_device *plat_dev;
+	void __iomem		*reg_base;
+	struct mtk_jpeg_dev *master_dev;
+	struct mtk_jpegenc_pm pm;
+	int jpegenc_irq;
+};
+
 /**
  * struct mtk_jpeg_dev - JPEG IP abstraction
  * @lock:		the mutex protecting this structure

@@ -102,6 +152,9 @@ struct mtk_jpeg_dev {
 	struct device		*larb;
 	struct delayed_work job_timeout_work;
 	const struct mtk_jpeg_variant *variant;
+
+	void __iomem *reg_encbase[MTK_JPEGENC_HW_MAX];
+	struct mtk_jpegenc_comp_dev *enc_hw_dev[MTK_JPEGENC_HW_MAX];
 };
 
 /**

@@ -162,4 +215,6 @@ struct mtk_jpeg_ctx {
 	struct v4l2_ctrl_handler ctrl_hdl;
 };

Please no extern here:

It would fixed in the next code version.
thanks.

quoted

+extern struct platform_driver mtk_jpegenc_hw_driver;
+
 #endif /* _MTK_JPEG_CORE_H */

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c

b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
index 1cf037b..4ccda1d 100644

--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c

@@ -4,12 +4,27 @@
  * Author: Xia Jiang <xia.jiang@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-dma-contig.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_enc_hw.h"
 
 static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = {

@@ -30,6 +45,21 @@ static const struct mtk_jpeg_enc_qlt

mtk_jpeg_enc_quality[] = {
 	{.quality_param = 97, .hardware_value = JPEG_ENC_QUALITY_Q97},
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id mtk_jpegenc_drv_ids[] = {
+	{
+		.compatible = "mediatek,mt8195-jpgenc0",
+		.data = (void *)MTK_JPEGENC_HW0,
+	},
+	{
+		.compatible = "mediatek,mt8195-jpgenc1",
+		.data = (void *)MTK_JPEGENC_HW1,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids);
+#endif
+
 void mtk_jpeg_enc_reset(void __iomem *base)
 {
 	writel(0, base + JPEG_ENC_RSTB);

@@ -152,3 +182,203 @@ void mtk_jpeg_set_enc_params(struct

mtk_jpeg_ctx *ctx,  void __iomem *base)
 
 	writel(ctx->restart_interval, base + JPEG_ENC_RST_MCU_NUM);
 }
+
+static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
+{
+	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+	enum vb2_buffer_state buf_state;
+	struct mtk_jpeg_ctx *ctx;
+	u32 result_size;
+	u32 irq_status;
+
+	struct mtk_jpegenc_comp_dev *jpeg = priv;
+	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
+
+	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
+		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
+	if (irq_status)
+		writel(0, jpeg->reg_base + JPEG_ENC_INT_STS);
+	if (!(irq_status & JPEG_ENC_INT_STATUS_DONE))
+		return IRQ_NONE;
+
+	ctx = v4l2_m2m_get_curr_priv(master_jpeg->m2m_dev);
+	if (!ctx) {
+		v4l2_err(&master_jpeg->v4l2_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);
+	result_size = mtk_jpeg_enc_get_file_size(jpeg->reg_base);
+	vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
+	buf_state = VB2_BUF_STATE_DONE;
+	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_jpegenc_hw_init_irq(struct mtk_jpegenc_comp_dev
*dev)
+{
+	struct platform_device *pdev = dev->plat_dev;
+	int ret;
+
+	dev->jpegenc_irq = platform_get_irq(pdev, 0);
+	if (dev->jpegenc_irq < 0) {
+		dev_err(&pdev->dev, "Failed to get irq resource");
+		return dev->jpegenc_irq;
+	}
+

Have you tried with a threaded_irq?

quoted

+	ret = devm_request_irq(&pdev->dev, dev->jpegenc_irq,
+		mtk_jpegenc_hw_irq_handler, 0, pdev->name, dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to devm_request_irq %d
(%d)",
+			dev->jpegenc_irq, ret);
+		return -ENOENT;
+	}
+
+	return 0;
+}
+
+int mtk_jpegenc_init_pm(struct mtk_jpegenc_comp_dev *mtkdev)
+{
+	struct mtk_jpegenc_clk_info *clk_info;
+	struct mtk_jpegenc_clk *jpegenc_clk;
+	struct platform_device *pdev;
+	struct mtk_jpegenc_pm *pm;
+	int i, ret;
+
+	pdev = mtkdev->plat_dev;
+	pm = &mtkdev->pm;
+	pm->dev = &pdev->dev;
+	pm->mtkdev = mtkdev;

Can't you simplify with devm_clk_bulk_get_all

It would fixed in the next code version.
thanks.

quoted

+	jpegenc_clk = &pm->venc_clk;
+	jpegenc_clk->clk_num =
+		of_property_count_strings(pdev->dev.of_node, "clock-
names");
+	if (!jpegenc_clk->clk_num) {
+		dev_err(&pdev->dev, "Failed to get jpegenc clock
count\n");
+		return -EINVAL;
+	}
+
+	jpegenc_clk->clk_info = devm_kcalloc(&pdev->dev,
+		jpegenc_clk->clk_num,
+		sizeof(*clk_info),
+		GFP_KERNEL);
+	if (!jpegenc_clk->clk_info)
+		return -ENOMEM;
+
+	for (i = 0; i < jpegenc_clk->clk_num; i++) {
+		clk_info = &jpegenc_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->jpegenc_clk = devm_clk_get(&pdev->dev,
+			clk_info->clk_name);
+		if (IS_ERR(clk_info->jpegenc_clk)) {
+			dev_err(&pdev->dev, "devm_clk_get (%d)%s fail",
+				i, clk_info->clk_name);
+			return PTR_ERR(clk_info->jpegenc_clk);
+		}
+	}
+

Can't you enable the pm after all the probing is completed to
simplify
the error handling?

It would fixed in the next code version.
thanks.

quoted

+	pm_runtime_enable(&pdev->dev);
+
+	return ret;
+}
+
+void mtk_jpegenc_release_pm(struct mtk_jpegenc_comp_dev *mtkdev)
+{
+	struct platform_device *pdev = mtkdev->plat_dev;
+
+	pm_runtime_disable(&pdev->dev);
+}
+
+static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
+{
+	struct mtk_jpeg_dev *master_dev;
+	const struct of_device_id *of_id;
+	struct mtk_jpegenc_comp_dev *dev;
+	int ret, comp_idx;
+
+	struct device *decs = &pdev->dev;
+

When is the parent NULL?

parent delegates the master_dev, if mtk_jpeg_probe is called after
mtk_jpegenc_hw_probe, the parent is NULL. If return -EPROBE_DEFER,
the system will call mtk_jpegenc_hw_probe later.

quoted

+	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_jpegenc_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_jpegenc_hw_init_irq(dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register JPEGENC irq
handler.\n");
+		goto err;
+	}
+

please use of_device_get_match_data()

It would fixed in the next code version.
thanks.

quoted

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

What if the drvdata is freed before you set it?

I will supply a device tree to explain it in the next version.
thanks.

quoted

+	comp_idx = (enum mtk_jpegenc_hw_id)of_id->data;
+	if (comp_idx < MTK_JPEGENC_HW_MAX) {
+		master_dev->enc_hw_dev[comp_idx] = dev;
+		master_dev->reg_encbase[comp_idx] = dev->reg_base;
+		dev->master_dev = master_dev;
+	}
+
+	platform_set_drvdata(pdev, dev);
+
+	return 0;
+
+err:
+	mtk_jpegenc_release_pm(dev);
+	return ret;
+}
+
+
+static int mtk_jpegenc_remove(struct platform_device *pdev)
+{
+	struct mtk_jpegenc_comp_dev *dev = platform_get_drvdata(pdev);
+
+	mtk_jpegenc_release_pm(dev);
+
+	return 0;
+}
+
+struct platform_driver mtk_jpegenc_hw_driver = {
+	.probe = mtk_jpegenc_hw_probe,
+	.remove = mtk_jpegenc_remove,
+	.driver = {
+		.name = "mtk-jpegenc-hw",
+		.of_match_table = of_match_ptr(mtk_jpegenc_drv_ids),
+	},
+};
-- 
2.6.4


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Il 03/12/21 04:13, kyrie.wu ha scritto:

manage each hardware information, including irq/clk/power.
the hardware includes HW0 and HW1.

Signed-off-by: kyrie.wu <redacted>

Hello Kyrie,
thanks for the patch!

However, there are a few things to improve...

This has to be rebased... new versions are using devm_platform_ioremap_resource(),
which is shorter and cleaner...

-	if (IS_ERR(jpeg->reg_base)) {
-		ret = PTR_ERR(jpeg->reg_base);
-		return ret;
-	}
+	if (!jpeg->variant->is_encoder) {

What about mediatek,mtk-jpgenc? That's also an encoder... this "is_encoder"
variable is a bit confusing... Is that a newer version of the IP?
Please, let's find a better name for this variable to avoid confusion.

+		INIT_DELAYED_WORK(&jpeg->job_timeout_work,
+				mtk_jpeg_job_timeout_work);
  
-	jpeg_irq = platform_get_irq(pdev, 0);
-	if (jpeg_irq < 0) {
-		dev_err(&pdev->dev, "Failed to get jpeg_irq %d.\n", jpeg_irq);
-		return jpeg_irq;
-	}
+		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;
+		}
  
-	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;
-	}
+		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 = mtk_jpeg_clk_init(jpeg);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to init clk, err %d\n", ret);
-		goto err_clk_init;
+		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);
  	}
  
  	ret = v4l2_device_register(&pdev->dev, &jpeg->v4l2_dev);

...snip...

/

There's nothing special in jpgenc1 or, at least, nothing that really needs
us to differentiate between jpgenc0 and jpgenc1, apart knowing which core is
the "main" one, hence, you don't need a special compatible string for each core.

Here's my proposal:
- Use one compatible "mediatek,mt8195-jpgenc-hw"
- Add either of:
   - A bool "mediatek,hw-leader" on core 0; or
   - A u8 "mediatek,instance" (0, 1, 2 ... for core number)

A comment is required on "mediatek,instance" though... this way should only be
chosen if it is expected, in the future, to have the following situation on
newer SoCs:
- More than two cores, and
- non-interchangeable cores (meaning that, for example, frame 1 *shall* go to
   core 1, frame 2 shall go to core 2, frame 3 to core 3, BUT core 2/3 are not
   interchangeable, as in there are reasons to never process frame 2 on core 3),
   so this means that it's not important if Linux labels core 3 as core 2.

Otherwise, if it's not expected to have non-interchangeable "hw-follower" cores,
or if more than two cores are not ever expected, "mediatek,hw-leader" is the best
choice for this.

Example:

jpegenc@address {
	compatible = "mediatek,mt8195-jpgenc";
	reg = < .... >;
	ranges = < ....... >;
	.... other properties here ....

	jpegenc-hw0@relative-address {
		compatible = "mediatek,mt8195-jpgenc-hw", "mediatek,jpgenc-hw";
		iommus, interrupts, other properties here ...
		mediatek,hw-leader;
	};

	jpegenc-hw1@relative-address {
		compatible = "mediatek,mt8195-jpgenc-hw", "mediatek,jpgenc-hw";
		iommus, interrupts, etc.....
	};
};

+	{
+		.compatible = "mediatek,mt8195-jpgenc0",
+		.data = (void *)MTK_JPEGENC_HW0,
+	},
+	{
+		.compatible = "mediatek,mt8195-jpgenc1",
+		.data = (void *)MTK_JPEGENC_HW1,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids);
+#endif
+
  void mtk_jpeg_enc_reset(void __iomem *base)
  {
  	writel(0, base + JPEG_ENC_RSTB);

Thanks,
Angelo


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

On Mon, 2022-02-07 at 15:50 +0100, AngeloGioacchino Del Regno wrote:

Il 03/12/21 04:13, kyrie.wu ha scritto:

quoted

manage each hardware information, including irq/clk/power.
the hardware includes HW0 and HW1.

Signed-off-by: kyrie.wu <redacted>

Hello Kyrie,
thanks for the patch!

However, there are a few things to improve...

quoted

---
V6: use of_platform_populate to replace component framework
to manage multi-hardware
---
  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   | 103 +++++++
---
  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   |  55 +++++
  drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 232
+++++++++++++++++++++-
  3 files changed, 362 insertions(+), 28 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c

b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index a89c7b2..da7eb84 100644

--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c

@@ -1353,33 +1353,40 @@ static int mtk_jpeg_probe(struct

platform_device *pdev)
  	spin_lock_init(&jpeg->hw_lock);
  	jpeg->dev = &pdev->dev;
  	jpeg->variant = of_device_get_match_data(jpeg->dev);
-	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);

This has to be rebased... new versions are using
devm_platform_ioremap_resource(),
which is shorter and cleaner...

Hi AngeloGioacchino,
Thanks for your kindly reply, I will fix this problem in the next
version.

quoted

-	if (IS_ERR(jpeg->reg_base)) {
-		ret = PTR_ERR(jpeg->reg_base);
-		return ret;
-	}
+	if (!jpeg->variant->is_encoder) {

What about mediatek,mtk-jpgenc? That's also an encoder... this
"is_encoder"
variable is a bit confusing... Is that a newer version of the IP?
Please, let's find a better name for this variable to avoid
confusion.

The confusion would modify in the next version.
Thanks.

quoted

+		INIT_DELAYED_WORK(&jpeg->job_timeout_work,
+				mtk_jpeg_job_timeout_work);
  
-	jpeg_irq = platform_get_irq(pdev, 0);
-	if (jpeg_irq < 0) {
-		dev_err(&pdev->dev, "Failed to get jpeg_irq %d.\n",
jpeg_irq);
-		return jpeg_irq;
-	}
+		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;
+		}
  
-	ret = devm_request_irq(&pdev->dev, jpeg_irq,
-			       jpeg->variant->irq_handler, 0, pdev-

quoted

name, jpeg);

-	if (ret) {
-		dev_err(&pdev->dev, "Failed to request jpeg_irq %d
(%d)\n",
-			jpeg_irq, ret);
-		goto err_req_irq;
-	}
+		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 = mtk_jpeg_clk_init(jpeg);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to init clk, err %d\n",
ret);
-		goto err_clk_init;
+		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);
  	}
  
  	ret = v4l2_device_register(&pdev->dev, &jpeg->v4l2_dev);

...snip...

/

quoted

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c

b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
index 1cf037b..4ccda1d 100644

--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c

@@ -4,12 +4,27 @@
   * Author: Xia Jiang <xia.jiang@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-dma-contig.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_enc_hw.h"
  
  static const struct mtk_jpeg_enc_qlt mtk_jpeg_enc_quality[] = {

@@ -30,6 +45,21 @@ static const struct mtk_jpeg_enc_qlt

mtk_jpeg_enc_quality[] = {
  	{.quality_param = 97, .hardware_value = JPEG_ENC_QUALITY_Q97},
  };
  
+#if defined(CONFIG_OF)
+static const struct of_device_id mtk_jpegenc_drv_ids[] = {

There's nothing special in jpgenc1 or, at least, nothing that really
needs
us to differentiate between jpgenc0 and jpgenc1, apart knowing which
core is
the "main" one, hence, you don't need a special compatible string for
each core.

Here's my proposal:
- Use one compatible "mediatek,mt8195-jpgenc-hw"
- Add either of:
   - A bool "mediatek,hw-leader" on core 0; or
   - A u8 "mediatek,instance" (0, 1, 2 ... for core number)

A comment is required on "mediatek,instance" though... this way
should only be
chosen if it is expected, in the future, to have the following
situation on
newer SoCs:
- More than two cores, and
- non-interchangeable cores (meaning that, for example, frame 1
*shall* go to
   core 1, frame 2 shall go to core 2, frame 3 to core 3, BUT core
2/3 are not
   interchangeable, as in there are reasons to never process frame 2
on core 3),
   so this means that it's not important if Linux labels core 3 as
core 2.

Otherwise, if it's not expected to have non-interchangeable "hw-
follower" cores,
or if more than two cores are not ever expected, "mediatek,hw-leader" 
is the best
choice for this.

Example:

jpegenc@address {
	compatible = "mediatek,mt8195-jpgenc";
	reg = < .... >;
	ranges = < ....... >;
	.... other properties here ....

	jpegenc-hw0@relative-address {
		compatible = "mediatek,mt8195-jpgenc-hw",
"mediatek,jpgenc-hw";
		iommus, interrupts, other properties here ...
		mediatek,hw-leader;
	};

	jpegenc-hw1@relative-address {
		compatible = "mediatek,mt8195-jpgenc-hw",
"mediatek,jpgenc-hw";
		iommus, interrupts, etc.....
	};
};

I will take your suggestion in the next version.
Thanks.

quoted

+	{
+		.compatible = "mediatek,mt8195-jpgenc0",
+		.data = (void *)MTK_JPEGENC_HW0,
+	},
+	{
+		.compatible = "mediatek,mt8195-jpgenc1",
+		.data = (void *)MTK_JPEGENC_HW1,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids);
+#endif
+
  void mtk_jpeg_enc_reset(void __iomem *base)
  {
  	writel(0, base + JPEG_ENC_RSTB);

Thanks,
Angelo

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

There are two HWs in mt8195. Since the two HWs run
in parallel, it is necessary to reorder the output images
to ensure that the order is consistent with the input images.

Signed-off-by: kyrie.wu <redacted>
---
V6: no change
---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c   | 10 +----
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h   | 17 +++++++-
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c |  1 +
 drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h |  3 +-
 drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 48 ++++++++++++++++++++++-
 5 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index 80a6c1a..9e89629 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c

@@ -106,15 +106,6 @@ static struct mtk_jpeg_fmt mtk_jpeg_dec_formats[] = {
 #define MTK_JPEG_ENC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_enc_formats)
 #define MTK_JPEG_DEC_NUM_FORMATS ARRAY_SIZE(mtk_jpeg_dec_formats)
 
-struct mtk_jpeg_src_buf {
-	struct vb2_v4l2_buffer b;
-	struct list_head list;
-	struct mtk_jpeg_dec_param dec_param;
-
-	struct mtk_jpeg_ctx *curr_ctx;
-	u32 frame_num;
-};
-
 static int debug;
 module_param(debug, int, 0644);
 

@@ -1344,6 +1335,7 @@ static int mtk_jpeg_open(struct file *file)
 	if (jpeg->variant->is_encoder)
 		INIT_WORK(&ctx->jpeg_work, mtk_jpegenc_worker);
 
+	INIT_LIST_HEAD(&ctx->dst_done_queue);
 	v4l2_fh_init(&ctx->fh, vfd);
 	file->private_data = &ctx->fh;
 	v4l2_fh_add(&ctx->fh);

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index 4c669af..f276221 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h

@@ -13,10 +13,11 @@
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fh.h>
+#include <media/videobuf2-v4l2.h>
 
-#define MTK_JPEG_NAME		"mtk-jpeg"
+#include "mtk_jpeg_dec_hw.h"
 
-#define MTK_JPEG_COMP_MAX		3
+#define MTK_JPEG_NAME		"mtk-jpeg"
 
 #define MTK_JPEG_FMT_FLAG_OUTPUT	BIT(0)
 #define MTK_JPEG_FMT_FLAG_CAPTURE	BIT(1)

@@ -75,6 +76,15 @@ struct mtk_jpeg_variant {
 	u32 cap_q_default_fourcc;
 };
 
+struct mtk_jpeg_src_buf {
+	struct vb2_v4l2_buffer b;
+	struct list_head list;
+	struct mtk_jpeg_dec_param dec_param;
+
+	struct mtk_jpeg_ctx *curr_ctx;
+	u32 frame_num;
+};
+
 enum mtk_jpeg_hw_state {
 	MTK_JPEG_HW_IDLE = 0,
 	MTK_JPEG_HW_BUSY = 1,

@@ -232,6 +242,9 @@ struct mtk_jpeg_ctx {
 	struct v4l2_ctrl_handler ctrl_hdl;
 	struct work_struct jpeg_work;
 	u32 total_frame_num;
+	struct list_head dst_done_queue;
+	spinlock_t done_queue_lock;
+	u32 last_done_frame_num;
 };
 
 extern struct platform_driver mtk_jpegenc_hw_driver;

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 afbbfd5..1e38522 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c

@@ -9,6 +9,7 @@
 #include <linux/kernel.h>
 #include <media/videobuf2-core.h>
 
+#include "mtk_jpeg_core.h"
 #include "mtk_jpeg_dec_hw.h"
 
 #define MTK_JPEG_DUNUM_MASK(val)	(((val) - 1) & 0x3)

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 fa0d45f..87aaa5c 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h

@@ -11,9 +11,10 @@
 
 #include <media/videobuf2-core.h>
 
-#include "mtk_jpeg_core.h"
 #include "mtk_jpeg_dec_reg.h"
 
+#define MTK_JPEG_COMP_MAX		3
+
 enum {
 	MTK_JPEG_DEC_RESULT_EOF_DONE		= 0,
 	MTK_JPEG_DEC_RESULT_PAUSE		= 1,

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
index 244f9f7..3383dc0 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c

@@ -183,6 +183,50 @@ void mtk_jpeg_set_enc_params(struct mtk_jpeg_ctx *ctx,  void __iomem *base)
 	writel(ctx->restart_interval, base + JPEG_ENC_RST_MCU_NUM);
 }
 
+void mtk_jpegenc_put_buf(struct mtk_jpegenc_comp_dev *jpeg)
+{
+	struct mtk_jpeg_ctx *ctx;
+	struct vb2_v4l2_buffer *dst_buffer;
+	struct list_head *temp_entry;
+	struct list_head *pos = NULL;
+	struct mtk_jpeg_src_buf *dst_done_buf, *tmp_dst_done_buf;
+	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_jpegenc_timeout_work(struct work_struct *work)
 {
 	struct delayed_work *Pwork =

@@ -203,6 +247,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 	atomic_inc(&cjpeg->hw_rdy);
 	wake_up(&master_jpeg->enc_hw_wq);
 	v4l2_m2m_buf_done(src_buf, buf_state);
+	mtk_jpegenc_put_buf(cjpeg);
 }
 
 static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)

@@ -237,8 +282,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 	vb2_set_plane_payload(&dst_buf->vb2_buf, 0, result_size);
 	buf_state = VB2_BUF_STATE_DONE;
 	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);
+	mtk_jpegenc_put_buf(jpeg);
 	clk_disable_unprepare(jpeg->pm.venc_clk.clk_info->jpegenc_clk);
 	pm_runtime_put(ctx->jpeg->dev);
 	if (ctx->fh.m2m_ctx &&

-- 
2.6.4


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

kyrie.wu wrote: