linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195
@ 2022-02-24  9:07 kyrie.wu
  2022-02-24  9:07 ` [RESEND, V7, 1/6] dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible kyrie.wu
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: kyrie.wu @ 2022-02-24  9:07 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring, kyrie wu,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

From: kyrie wu <kyrie.wu@mediatek.com>

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 Adds jpeg encoder dt-bindings for mt8195

Patches 2 jpeg encoder builds two module for using Multi-HW,
export some functions to make them visible by other modules.

Patches 3 use devm_of_platform_populate to manage multi-hardware.

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

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

Patch 6 add output picture reorder function to order images.
---
Changes compared with v6:
- new yaml file for mt8195 jpeg encoder.
- some modifications for patch v5's review comments.

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 (6):
  dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible
  media: mtk-jpegenc: export jpeg encoder functions
  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

 .../media/mediatek,mt8195-jpegenc.yaml        | 174 +++++++++++
 drivers/media/platform/mtk-jpeg/Makefile      |  11 +-
 .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 292 +++++++++++++++---
 .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |  81 ++++-
 .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c |   1 +
 .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h |   3 +-
 .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 261 ++++++++++++++++
 7 files changed, 769 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml

-- 
2.18.0


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RESEND, V7, 1/6] dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible
  2022-02-24  9:07 [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 kyrie.wu
@ 2022-02-24  9:07 ` kyrie.wu
  2022-03-16 12:58   ` Rex-BC Chen
  2022-02-24  9:07 ` [RESEND,V7,2/6] media: mtk-jpegenc: export jpeg encoder functions kyrie.wu
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: kyrie.wu @ 2022-02-24  9:07 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring, kyrie wu,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

From: kyrie wu <kyrie.wu@mediatek.com>

Add mediatek,mt8195-jpgenc compatible to binding document.

Signed-off-by: kyrie wu <kyrie.wu@mediatek.com>
---
 .../media/mediatek,mt8195-jpegenc.yaml        | 174 ++++++++++++++++++
 1 file changed, 174 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml

diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
new file mode 100644
index 000000000000..efb0d843edb6
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
@@ -0,0 +1,174 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/mediatek,mt8195-jpegenc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek JPEG Encoder Device Tree Bindings
+
+maintainers:
+  - kyrie wu <kyrie.wu@mediatek.corp-partner.google.com>
+
+description: |-
+  MediaTek JPEG Encoder is the JPEG encode hardware present in MediaTek SoCs
+
+properties:
+  compatible:
+    items:
+      - const: mediatek,mt8195-jpgenc
+
+  power-domains:
+    maxItems: 1
+
+  iommus:
+    maxItems: 4
+    description: |
+      Points to the respective IOMMU block with master port as argument, see
+      Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.
+      Ports are according to the HW.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+  ranges: true
+
+# Required child node:
+patternProperties:
+  "jpgenc@1a030000":
+    type: object
+
+    properties:
+      compatible:
+        const: mediatek,mt8195-jpgenc0
+
+      reg:
+        maxItems: 1
+
+      iommus:
+        minItems: 1
+        maxItems: 32
+        description: |
+          List of the hardware port in respective IOMMU block for current Socs.
+          Refer to bindings/iommu/mediatek,iommu.yaml.
+
+      interrupts:
+        maxItems: 1
+
+      clocks:
+        maxItems: 1
+
+      clock-names:
+        items:
+          - const: jpgenc
+
+      power-domains:
+        maxItems: 1
+
+    required:
+      - compatible
+      - reg
+      - iommus
+      - interrupts
+      - clocks
+      - clock-names
+      - power-domains
+
+    additionalProperties: false
+
+  "jpgenc@1b030000":
+    type: object
+
+    properties:
+      compatible:
+        const: mediatek,mt8195-jpgenc1
+
+      reg:
+        maxItems: 1
+
+      iommus:
+        minItems: 1
+        maxItems: 32
+        description: |
+          List of the hardware port in respective IOMMU block for current Socs.
+          Refer to bindings/iommu/mediatek,iommu.yaml.
+
+      interrupts:
+        maxItems: 1
+
+      clocks:
+        maxItems: 1
+
+      clock-names:
+        items:
+          - const: jpgenc
+
+      power-domains:
+        maxItems: 1
+
+    required:
+      - compatible
+      - reg
+      - iommus
+      - interrupts
+      - clocks
+      - clock-names
+      - power-domains
+
+    additionalProperties: false
+
+
+required:
+  - compatible
+  - power-domains
+  - iommus
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/memory/mt8195-memory-port.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/mt8195-clk.h>
+    #include <dt-bindings/power/mt8195-power.h>
+
+    jpgenc_master {
+            compatible = "mediatek,mt8195-jpgenc";
+            power-domains = <&spm MT8195_POWER_DOMAIN_VENC_CORE1>;
+            iommus = <&iommu_vpp M4U_PORT_L20_JPGENC_Y_RDMA>,
+            <&iommu_vpp M4U_PORT_L20_JPGENC_C_RDMA>,
+            <&iommu_vpp M4U_PORT_L20_JPGENC_Q_TABLE>,
+            <&iommu_vpp M4U_PORT_L20_JPGENC_BSDMA>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges;
+
+            jpgenc@1a030000 {
+                    compatible = "mediatek,mt8195-jpgenc0";
+                    reg = <0x1a030000 0x10000>;
+                    iommus = <&iommu_vdo M4U_PORT_L19_JPGENC_Y_RDMA>,
+                    <&iommu_vdo M4U_PORT_L19_JPGENC_C_RDMA>,
+                    <&iommu_vdo M4U_PORT_L19_JPGENC_Q_TABLE>,
+                    <&iommu_vdo M4U_PORT_L19_JPGENC_BSDMA>;
+                    interrupts = <GIC_SPI 342 IRQ_TYPE_LEVEL_HIGH 0>;
+                    clocks = <&vencsys CLK_VENC_JPGENC>;
+                    clock-names = "jpgenc";
+                    power-domains = <&spm MT8195_POWER_DOMAIN_VENC>;
+            };
+
+            jpgenc@1b030000 {
+                    compatible = "mediatek,mt8195-jpgenc1";
+                    reg = <0x1b030000 0x10000>;
+                    iommus = <&iommu_vpp M4U_PORT_L20_JPGENC_Y_RDMA>,
+                    <&iommu_vpp M4U_PORT_L20_JPGENC_C_RDMA>,
+                    <&iommu_vpp M4U_PORT_L20_JPGENC_Q_TABLE>,
+                    <&iommu_vpp M4U_PORT_L20_JPGENC_BSDMA>;
+                    interrupts = <GIC_SPI 347 IRQ_TYPE_LEVEL_HIGH 0>;
+                    clocks = <&vencsys_core1 CLK_VENC_CORE1_JPGENC>;
+                    clock-names = "jpgenc";
+                    power-domains = <&spm MT8195_POWER_DOMAIN_VENC_CORE1>;
+            };
+    };
-- 
2.18.0


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND,V7,2/6] media: mtk-jpegenc: export jpeg encoder functions
  2022-02-24  9:07 [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 kyrie.wu
  2022-02-24  9:07 ` [RESEND, V7, 1/6] dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible kyrie.wu
@ 2022-02-24  9:07 ` kyrie.wu
  2022-02-24  9:07 ` [RESEND,V7,3/6] media: mtk-jpegenc: manage jpegenc multi-hardware kyrie.wu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: kyrie.wu @ 2022-02-24  9:07 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring, kyrie wu,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

From: kyrie wu <kyrie.wu@mediatek.com>

mtk jpeg encoder is built as a module, export some functions to make them
visible by other modules.

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

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 1cf037bf72dd..a2b6e1f85c2d 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
@@ -36,12 +36,14 @@ void mtk_jpeg_enc_reset(void __iomem *base)
 	writel(JPEG_ENC_RESET_BIT, base + JPEG_ENC_RSTB);
 	writel(0, base + JPEG_ENC_CODEC_SEL);
 }
+EXPORT_SYMBOL_GPL(mtk_jpeg_enc_reset);
 
 u32 mtk_jpeg_enc_get_file_size(void __iomem *base)
 {
 	return readl(base + JPEG_ENC_DMA_ADDR0) -
 	       readl(base + JPEG_ENC_DST_ADDR0);
 }
+EXPORT_SYMBOL_GPL(mtk_jpeg_enc_get_file_size);
 
 void mtk_jpeg_enc_start(void __iomem *base)
 {
@@ -51,6 +53,7 @@ void mtk_jpeg_enc_start(void __iomem *base)
 	value |= JPEG_ENC_CTRL_INT_EN_BIT | JPEG_ENC_CTRL_ENABLE_BIT;
 	writel(value, base + JPEG_ENC_CTRL);
 }
+EXPORT_SYMBOL_GPL(mtk_jpeg_enc_start);
 
 void mtk_jpeg_set_enc_src(struct mtk_jpeg_ctx *ctx,  void __iomem *base,
 			  struct vb2_buffer *src_buf)
@@ -67,6 +70,7 @@ void mtk_jpeg_set_enc_src(struct mtk_jpeg_ctx *ctx,  void __iomem *base,
 			writel(dma_addr, base + JPEG_ENC_SRC_CHROMA_ADDR);
 	}
 }
+EXPORT_SYMBOL_GPL(mtk_jpeg_set_enc_src);
 
 void mtk_jpeg_set_enc_dst(struct mtk_jpeg_ctx *ctx, void __iomem *base,
 			  struct vb2_buffer *dst_buf)
@@ -86,6 +90,7 @@ void mtk_jpeg_set_enc_dst(struct mtk_jpeg_ctx *ctx, void __iomem *base,
 	writel(dma_addr & ~0xf, base + JPEG_ENC_DST_ADDR0);
 	writel((dma_addr + size) & ~0xf, base + JPEG_ENC_STALL_ADDR0);
 }
+EXPORT_SYMBOL_GPL(mtk_jpeg_set_enc_dst);
 
 void mtk_jpeg_set_enc_params(struct mtk_jpeg_ctx *ctx,  void __iomem *base)
 {
@@ -152,3 +157,5 @@ void mtk_jpeg_set_enc_params(struct mtk_jpeg_ctx *ctx,  void __iomem *base)
 
 	writel(ctx->restart_interval, base + JPEG_ENC_RST_MCU_NUM);
 }
+EXPORT_SYMBOL_GPL(mtk_jpeg_set_enc_params);
+
-- 
2.18.0


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND,V7,3/6] media: mtk-jpegenc: manage jpegenc multi-hardware
  2022-02-24  9:07 [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 kyrie.wu
  2022-02-24  9:07 ` [RESEND, V7, 1/6] dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible kyrie.wu
  2022-02-24  9:07 ` [RESEND,V7,2/6] media: mtk-jpegenc: export jpeg encoder functions kyrie.wu
@ 2022-02-24  9:07 ` kyrie.wu
  2022-03-02  9:45   ` [RESEND, V7, 3/6] " AngeloGioacchino Del Regno
  2022-02-24  9:07 ` [RESEND, V7, 4/6] media: mtk-jpegenc: add jpegenc timeout func interface kyrie.wu
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: kyrie.wu @ 2022-02-24  9:07 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring, kyrie wu,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

From: kyrie wu <kyrie.wu@mediatek.com>

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

Signed-off-by: kyrie wu <kyrie.wu@mediatek.com>
---
 drivers/media/platform/mtk-jpeg/Makefile      |  11 +-
 .../media/platform/mtk-jpeg/mtk_jpeg_core.c   |  76 +++++---
 .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |  37 ++++
 .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 168 ++++++++++++++++++
 4 files changed, 267 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/Makefile b/drivers/media/platform/mtk-jpeg/Makefile
index 76c33aad0f3f..69703db4b0a5 100644
--- a/drivers/media/platform/mtk-jpeg/Makefile
+++ b/drivers/media/platform/mtk-jpeg/Makefile
@@ -1,6 +1,9 @@
 # SPDX-License-Identifier: GPL-2.0-only
-mtk_jpeg-objs := mtk_jpeg_core.o \
+obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o \
+	mtk-jpeg-enc-hw.o
+
+mtk_jpeg-y := mtk_jpeg_core.o \
 		 mtk_jpeg_dec_hw.o \
-		 mtk_jpeg_dec_parse.o \
-		 mtk_jpeg_enc_hw.o
-obj-$(CONFIG_VIDEO_MEDIATEK_JPEG) += mtk_jpeg.o
+		 mtk_jpeg_dec_parse.o
+
+mtk-jpeg-enc-hw-y := mtk_jpeg_enc_hw.o
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index d532f86e826e..35fd2b41f446 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -1312,31 +1312,39 @@ 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);
 
-	jpeg->reg_base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(jpeg->reg_base)) {
-		ret = PTR_ERR(jpeg->reg_base);
-		return ret;
-	}
+	if (!jpeg->variant->is_multihw) {
+		INIT_DELAYED_WORK(&jpeg->job_timeout_work,
+				  mtk_jpeg_job_timeout_work);
 
-	jpeg_irq = platform_get_irq(pdev, 0);
-	if (jpeg_irq < 0)
-		return jpeg_irq;
+		jpeg->reg_base = devm_platform_ioremap_resource(pdev, 0);
+		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)
+			return jpeg_irq;
 
-	ret = devm_clk_bulk_get(jpeg->dev, jpeg->variant->num_clks,
-				jpeg->variant->clks);
-	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 = devm_clk_bulk_get(jpeg->dev,
+					jpeg->variant->num_clks,
+					jpeg->variant->clks);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to init clk\n");
+			goto err_clk_init;
+		}
 	}
 
 	ret = v4l2_device_register(&pdev->dev, &jpeg->v4l2_dev);
@@ -1383,6 +1391,14 @@ 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_multihw) {
+		ret = devm_of_platform_populate(&pdev->dev);
+		if (ret) {
+			v4l2_err(&jpeg->v4l2_dev, "Master of platform populate failed.");
+			goto err_vfd_jpeg_register;
+		}
+	}
+
 	platform_set_drvdata(pdev, jpeg);
 
 	pm_runtime_enable(&pdev->dev);
@@ -1494,6 +1510,19 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = {
 	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
 };
 
+static const struct mtk_jpeg_variant mtk8195_jpegenc_drvdata = {
+	.is_multihw = 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",
@@ -1507,10 +1536,15 @@ static const struct of_device_id mtk_jpeg_match[] = {
 		.compatible = "mediatek,mtk-jpgenc",
 		.data = &mtk_jpeg_drvdata,
 	},
+	{
+		.compatible = "mediatek,mt8195-jpgenc",
+		.data = &mtk8195_jpegenc_drvdata,
+	},
 	{},
 };
 
 MODULE_DEVICE_TABLE(of, mtk_jpeg_match);
+#endif
 
 static struct platform_driver mtk_jpeg_driver = {
 	.probe = mtk_jpeg_probe,
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index 3e4811a41ba2..31e941ef84bd 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
@@ -9,6 +9,7 @@
 #ifndef _MTK_JPEG_CORE_H
 #define _MTK_JPEG_CORE_H
 
+#include <linux/clk.h>
 #include <linux/interrupt.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
@@ -60,6 +61,7 @@ enum mtk_jpeg_ctx_state {
  * @cap_q_default_fourcc:	capture queue default fourcc
  */
 struct mtk_jpeg_variant {
+	bool is_multihw;
 	struct clk_bulk_data *clks;
 	int num_clks;
 	struct mtk_jpeg_fmt *formats;
@@ -74,6 +76,38 @@ 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_vcodec_clk - Structure used to store vcodec clock information
+ */
+struct mtk_jpegenc_clk {
+	struct clk_bulk_data *clks;
+	int	clk_num;
+};
+
+/**
+ * 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_clk	venc_clk;
+	int jpegenc_irq;
+};
+
 /**
  * struct mtk_jpeg_dev - JPEG IP abstraction
  * @lock:		the mutex protecting this structure
@@ -100,6 +134,9 @@ struct mtk_jpeg_dev {
 	void __iomem		*reg_base;
 	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];
 };
 
 /**
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 a2b6e1f85c2d..3d967bff1352 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
@@ -5,11 +5,27 @@
  *
  */
 
+#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 +46,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);
@@ -159,3 +190,140 @@ void mtk_jpeg_set_enc_params(struct mtk_jpeg_ctx *ctx,  void __iomem *base)
 }
 EXPORT_SYMBOL_GPL(mtk_jpeg_set_enc_params);
 
+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;
+}
+
+static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
+{
+	struct mtk_jpegenc_clk *jpegenc_clk;
+	struct mtk_jpeg_dev *master_dev;
+	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;
+
+	jpegenc_clk = &dev->venc_clk;
+
+	jpegenc_clk->clk_num = devm_clk_bulk_get_all(&pdev->dev,
+						     &jpegenc_clk->clks);
+	if (jpegenc_clk->clk_num < 0) {
+		dev_err(&pdev->dev, "Failed to get jpegenc clock count\n");
+		return jpegenc_clk->clk_num;
+	}
+
+	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;
+	}
+
+	comp_idx = (enum mtk_jpegenc_hw_id)of_device_get_match_data(decs);
+	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;
+	} else {
+		dev_err(&pdev->dev, "Failed to get_match_data.\n");
+		goto err;
+	}
+
+	platform_set_drvdata(pdev, dev);
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+
+err:
+	return ret;
+}
+
+struct platform_driver mtk_jpegenc_hw_driver = {
+	.probe = mtk_jpegenc_hw_probe,
+	.driver = {
+		.name = "mtk-jpegenc-hw",
+		.of_match_table = of_match_ptr(mtk_jpegenc_drv_ids),
+	},
+};
+
+module_platform_driver(mtk_jpegenc_hw_driver);
+
+MODULE_DESCRIPTION("MediaTek JPEG encode HW driver");
+MODULE_LICENSE("GPL v2");
-- 
2.18.0


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND, V7, 4/6] media: mtk-jpegenc: add jpegenc timeout func interface
  2022-02-24  9:07 [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 kyrie.wu
                   ` (2 preceding siblings ...)
  2022-02-24  9:07 ` [RESEND,V7,3/6] media: mtk-jpegenc: manage jpegenc multi-hardware kyrie.wu
@ 2022-02-24  9:07 ` kyrie.wu
  2022-02-24  9:07 ` [RESEND,V7,5/6] media: mtk-jpegenc: add jpeg encode worker interface kyrie.wu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: kyrie.wu @ 2022-02-24  9:07 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring, kyrie wu,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

From: kyrie wu <kyrie.wu@mediatek.com>

Generalizes jpegenc timeout func interfaces to handle HW timeout.

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

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index 31e941ef84bd..edd4213a47de 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
@@ -76,6 +76,12 @@ struct mtk_jpeg_variant {
 	u32 cap_q_default_fourcc;
 };
 
+struct mtk_jpeg_hw_param {
+	struct vb2_v4l2_buffer *src_buffer;
+	struct vb2_v4l2_buffer *dst_buffer;
+	struct mtk_jpeg_ctx *curr_ctx;
+};
+
 enum mtk_jpegenc_hw_id {
 	MTK_JPEGENC_HW0,
 	MTK_JPEGENC_HW1,
@@ -106,6 +112,8 @@ struct mtk_jpegenc_comp_dev {
 	struct mtk_jpeg_dev *master_dev;
 	struct mtk_jpegenc_clk	venc_clk;
 	int jpegenc_irq;
+	struct delayed_work job_timeout_work;
+	struct mtk_jpeg_hw_param hw_param;
 };
 
 /**
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 3d967bff1352..32de95e4ab3d 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
@@ -190,6 +190,24 @@ void mtk_jpeg_set_enc_params(struct mtk_jpeg_ctx *ctx,  void __iomem *base)
 }
 EXPORT_SYMBOL_GPL(mtk_jpeg_set_enc_params);
 
+static void mtk_jpegenc_timeout_work(struct work_struct *work)
+{
+	struct delayed_work *dly_work = to_delayed_work(work);
+	struct mtk_jpegenc_comp_dev *cjpeg =
+		container_of(dly_work,
+			     struct mtk_jpegenc_comp_dev,
+			     job_timeout_work);
+	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
+	struct vb2_v4l2_buffer *src_buf;
+
+	src_buf = cjpeg->hw_param.src_buffer;
+
+	mtk_jpeg_enc_reset(cjpeg->reg_base);
+	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
+	pm_runtime_put(cjpeg->dev);
+	v4l2_m2m_buf_done(src_buf, buf_state);
+}
+
 static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 {
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
@@ -201,6 +219,8 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
 	struct mtk_jpegenc_comp_dev *jpeg = priv;
 	struct mtk_jpeg_dev *master_jpeg = jpeg->master_dev;
 
+	cancel_delayed_work(&jpeg->job_timeout_work);
+
 	irq_status = readl(jpeg->reg_base + JPEG_ENC_INT_STS) &
 		JPEG_ENC_INT_STATUS_MASK_ALLIRQ;
 	if (irq_status)
@@ -274,6 +294,9 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 
 	dev->plat_dev = pdev;
 
+	INIT_DELAYED_WORK(&dev->job_timeout_work,
+			  mtk_jpegenc_timeout_work);
+
 	jpegenc_clk = &dev->venc_clk;
 
 	jpegenc_clk->clk_num = devm_clk_bulk_get_all(&pdev->dev,
-- 
2.18.0


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND,V7,5/6] media: mtk-jpegenc: add jpeg encode worker interface
  2022-02-24  9:07 [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 kyrie.wu
                   ` (3 preceding siblings ...)
  2022-02-24  9:07 ` [RESEND, V7, 4/6] media: mtk-jpegenc: add jpegenc timeout func interface kyrie.wu
@ 2022-02-24  9:07 ` kyrie.wu
  2022-02-24  9:07 ` [RESEND,V7,6/6] media: mtk-jpegenc: add output pic reorder interface kyrie.wu
  2022-02-24 10:04 ` [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 Hans Verkuil
  6 siblings, 0 replies; 11+ messages in thread
From: kyrie.wu @ 2022-02-24  9:07 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring, kyrie wu,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

From: kyrie wu <kyrie.wu@mediatek.com>

Add jpeg encoding worker to ensure that two HWs
run in parallel in MT8195.

Signed-off-by: kyrie wu <kyrie.wu@mediatek.com>
---
 .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 212 ++++++++++++++++--
 .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |  30 ++-
 .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c |  18 ++
 3 files changed, 234 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index 35fd2b41f446..6f161a3d9935 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -109,6 +109,9 @@ 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;
@@ -907,48 +910,203 @@ static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
 	return 0;
 }
 
-static void mtk_jpeg_enc_device_run(void *priv)
+static int mtk_jpegenc_select_hw(struct mtk_jpeg_ctx *ctx)
 {
-	struct mtk_jpeg_ctx *ctx = priv;
+	struct mtk_jpegenc_comp_dev *comp_jpeg;
 	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
-	struct vb2_v4l2_buffer *src_buf, *dst_buf;
+	unsigned long flags;
+	int hw_id = -1;
+	int i;
+
+	spin_lock_irqsave(&jpeg->hw_lock, flags);
+	for (i = 0; i < MTK_JPEGENC_HW_MAX; i++) {
+		comp_jpeg = jpeg->enc_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_jpegenc_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_jpegenc_comp_dev *jpeg = ctx->jpeg->enc_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 int mtk_jpegenc_deselect_hw(struct mtk_jpeg_dev *jpeg, int hw_id)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&jpeg->hw_lock, flags);
+	jpeg->enc_hw_dev[hw_id]->hw_state = MTK_JPEG_HW_IDLE;
+	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
+
+	return 0;
+}
+
+static void mtk_jpegenc_worker(struct work_struct *work)
+{
+	struct mtk_jpegenc_comp_dev *comp_jpeg[MTK_JPEGENC_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;
+	atomic_t *hw_rdy[MTK_JPEGENC_HW_MAX];
+	int ret, i, hw_id = 0;
 	unsigned long flags;
-	int ret;
 
+	struct mtk_jpeg_ctx *ctx = container_of(work,
+		struct mtk_jpeg_ctx,
+		jpeg_work);
+	struct mtk_jpeg_dev *jpeg = ctx->jpeg;
+
+	for (i = 0; i < MTK_JPEGENC_HW_MAX; i++) {
+		comp_jpeg[i] = jpeg->enc_hw_dev[i];
+		hw_rdy[i] = &comp_jpeg[i]->hw_rdy;
+	}
+
+retry_select:
+	hw_id = mtk_jpegenc_select_hw(ctx);
+	if (hw_id < 0) {
+		ret = wait_event_interruptible(jpeg->enc_hw_wq,
+					       (atomic_read(hw_rdy[0]) ||
+						atomic_read(hw_rdy[1])) > 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;
+		}
+
+		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) {
+		pr_info("%s : %d, get dst_buf fail !!!\n",
+			__func__, __LINE__);
+		goto getbuf_fail;
+	}
 
-	ret = pm_runtime_resume_and_get(jpeg->dev);
-	if (ret < 0)
+	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(&src_buf->vb2_buf);
+	jpeg_dst_buf = mtk_jpeg_vb2_to_srcbuf(&dst_buf->vb2_buf);
+	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_jpegenc_set_hw_param(ctx, hw_id, src_buf, dst_buf);
+	ret = pm_runtime_get_sync(comp_jpeg[hw_id]->dev);
+	if (ret < 0) {
+		dev_err(jpeg->dev, "%s : %d, pm_runtime_get_sync fail !!!\n",
+			__func__, __LINE__);
 		goto enc_end;
+	}
 
-	schedule_delayed_work(&jpeg->job_timeout_work,
-			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
+	ret = clk_prepare_enable(comp_jpeg[hw_id]->venc_clk.clks->clk);
+	if (ret) {
+		dev_err(jpeg->dev, "%s : %d, jpegenc clk_prepare_enable fail\n",
+			__func__, __LINE__);
+		goto enc_end;
+	}
 
-	spin_lock_irqsave(&jpeg->hw_lock, flags);
+	schedule_delayed_work(&comp_jpeg[hw_id]->job_timeout_work,
+			      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
 
-	/*
-	 * Resetting the hardware every frame is to ensure that all the
-	 * registers are cleared. This is a hardware requirement.
-	 */
-	mtk_jpeg_enc_reset(jpeg->reg_base);
+	spin_lock_irqsave(&comp_jpeg[hw_id]->hw_lock, flags);
+	mtk_jpeg_enc_reset(comp_jpeg[hw_id]->reg_base);
+	mtk_jpeg_set_enc_dst(ctx,
+			     comp_jpeg[hw_id]->reg_base,
+			     &dst_buf->vb2_buf);
+	mtk_jpeg_set_enc_src(ctx,
+			     comp_jpeg[hw_id]->reg_base,
+			     &src_buf->vb2_buf);
+	mtk_jpeg_set_enc_params(ctx, comp_jpeg[hw_id]->reg_base);
+	mtk_jpeg_enc_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_set_enc_src(ctx, jpeg->reg_base, &src_buf->vb2_buf);
-	mtk_jpeg_set_enc_dst(ctx, jpeg->reg_base, &dst_buf->vb2_buf);
-	mtk_jpeg_set_enc_params(ctx, jpeg->reg_base);
-	mtk_jpeg_enc_start(jpeg->reg_base);
-	spin_unlock_irqrestore(&jpeg->hw_lock, flags);
 	return;
 
 enc_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_jpegenc_deselect_hw(jpeg, hw_id);
 	v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
 }
 
+static void mtk_jpeg_enc_device_run(void *priv)
+{
+	struct mtk_jpeg_ctx *ctx = priv;
+	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;
+	int ret;
+
+	if (!jpeg->variant->is_multihw) {
+		src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+		dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+
+		ret = pm_runtime_resume_and_get(jpeg->dev);
+		if (ret < 0)
+			goto enc_end;
+
+		schedule_delayed_work(&jpeg->job_timeout_work,
+				      msecs_to_jiffies(MTK_JPEG_HW_TIMEOUT_MSEC));
+
+		spin_lock_irqsave(&jpeg->hw_lock, flags);
+
+		/*
+		 * Resetting the hardware every frame is to ensure that all the
+		 * registers are cleared. This is a hardware requirement.
+		 */
+		mtk_jpeg_enc_reset(jpeg->reg_base);
+
+		mtk_jpeg_set_enc_src(ctx, jpeg->reg_base, &src_buf->vb2_buf);
+		mtk_jpeg_set_enc_dst(ctx, jpeg->reg_base, &dst_buf->vb2_buf);
+		mtk_jpeg_set_enc_params(ctx, jpeg->reg_base);
+		mtk_jpeg_enc_start(jpeg->reg_base);
+		spin_unlock_irqrestore(&jpeg->hw_lock, flags);
+		return;
+
+enc_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);
+		v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
+	} else {
+		queue_work(jpeg->workqueue, &ctx->jpeg_work);
+	}
+}
+
 static void mtk_jpeg_dec_device_run(void *priv)
 {
 	struct mtk_jpeg_ctx *ctx = priv;
@@ -1211,6 +1369,9 @@ static int mtk_jpeg_open(struct file *file)
 		goto free;
 	}
 
+	if (jpeg->variant->is_multihw)
+		INIT_WORK(&ctx->jpeg_work, mtk_jpegenc_worker);
+
 	v4l2_fh_init(&ctx->fh, vfd);
 	file->private_data = &ctx->fh;
 	v4l2_fh_add(&ctx->fh);
@@ -1345,6 +1506,15 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "Failed to init clk\n");
 			goto err_clk_init;
 		}
+	} else {
+		init_waitqueue_head(&jpeg->enc_hw_wq);
+		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");
+			ret = -EINVAL;
+			goto err_alloc_workqueue;
+		}
 	}
 
 	ret = v4l2_device_register(&pdev->dev, &jpeg->v4l2_dev);
@@ -1418,6 +1588,8 @@ static int mtk_jpeg_probe(struct platform_device *pdev)
 
 err_clk_init:
 
+err_alloc_workqueue:
+
 err_req_irq:
 
 	return ret;
diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
index edd4213a47de..aab0e23d1db9 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
@@ -76,6 +76,11 @@ struct mtk_jpeg_variant {
 	u32 cap_q_default_fourcc;
 };
 
+enum mtk_jpeg_hw_state {
+	MTK_JPEG_HW_IDLE = 0,
+	MTK_JPEG_HW_BUSY = 1,
+};
+
 struct mtk_jpeg_hw_param {
 	struct vb2_v4l2_buffer *src_buffer;
 	struct vb2_v4l2_buffer *dst_buffer;
@@ -98,12 +103,17 @@ struct mtk_jpegenc_clk {
 
 /**
  * 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
+ * @dev:		JPEG device
+ * @plat_dev:		platform device data
+ * @reg_base:		JPEG registers mapping
+ * @master_dev:		mtk_jpeg_dev device
+ * @venc_clk:	        mtk_jpegenc_clk
+ * @jpegenc_irq:	jpeg encode irq num
+ * @job_timeout_work:	handle jpeg encode work
+ * @hw_param:		record hw param
+ * @hw_rdy:		record hw ready
+ * @hw_state:		record hw state
+ * @hw_lock:		spinlock protecting the hw device resource
  */
 struct mtk_jpegenc_comp_dev {
 	struct device		*dev;
@@ -114,6 +124,10 @@ struct mtk_jpegenc_comp_dev {
 	int jpegenc_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 protecting the hw device resource
+	spinlock_t hw_lock;
 };
 
 /**
@@ -145,6 +159,7 @@ 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;
 };
 
 /**
@@ -203,6 +218,9 @@ struct mtk_jpeg_ctx {
 	u8 enc_quality;
 	u8 restart_interval;
 	struct v4l2_ctrl_handler ctrl_hdl;
+
+	struct work_struct jpeg_work;
+	u32 total_frame_num;
 };
 
 #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 32de95e4ab3d..88ababf1a059 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
@@ -197,6 +197,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 		container_of(dly_work,
 			     struct mtk_jpegenc_comp_dev,
 			     job_timeout_work);
+	struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
 	enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
 	struct vb2_v4l2_buffer *src_buf;
 
@@ -205,6 +206,9 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
 	mtk_jpeg_enc_reset(cjpeg->reg_base);
 	clk_disable_unprepare(cjpeg->venc_clk.clks->clk);
 	pm_runtime_put(cjpeg->dev);
+	cjpeg->hw_state = MTK_JPEG_HW_IDLE;
+	atomic_inc(&cjpeg->hw_rdy);
+	wake_up(&master_jpeg->enc_hw_wq);
 	v4l2_m2m_buf_done(src_buf, buf_state);
 }
 
@@ -242,7 +246,17 @@ static irqreturn_t mtk_jpegenc_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->venc_clk.clks->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->enc_hw_wq);
+	atomic_inc(&jpeg->hw_rdy);
 
 	return IRQ_HANDLED;
 }
@@ -294,6 +308,10 @@ static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
 
 	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_jpegenc_timeout_work);
 
-- 
2.18.0


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND,V7,6/6] media: mtk-jpegenc: add output pic reorder interface
  2022-02-24  9:07 [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 kyrie.wu
                   ` (4 preceding siblings ...)
  2022-02-24  9:07 ` [RESEND,V7,5/6] media: mtk-jpegenc: add jpeg encode worker interface kyrie.wu
@ 2022-02-24  9:07 ` kyrie.wu
  2022-02-24 10:04 ` [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 Hans Verkuil
  6 siblings, 0 replies; 11+ messages in thread
From: kyrie.wu @ 2022-02-24  9:07 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring, kyrie wu,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

From: kyrie wu <kyrie.wu@mediatek.com>

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 <kyrie.wu@mediatek.com>
---
 .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 10 +---
 .../media/platform/mtk-jpeg/mtk_jpeg_core.h   | 18 ++++++-
 .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c |  1 +
 .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h |  3 +-
 .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 49 ++++++++++++++++++-
 5 files changed, 67 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 6f161a3d9935..77d303e8bdea 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -105,15 +105,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);
 
@@ -1372,6 +1363,7 @@ static int mtk_jpeg_open(struct file *file)
 	if (jpeg->variant->is_multihw)
 		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 aab0e23d1db9..8eb21f0e7124 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
@@ -14,10 +14,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)
@@ -76,6 +77,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,
@@ -221,6 +231,10 @@ struct mtk_jpeg_ctx {
 
 	struct work_struct jpeg_work;
 	u32 total_frame_num;
+	struct list_head dst_done_queue;
+	//spinlock protecting the encode done buffer
+	spinlock_t done_queue_lock;
+	u32 last_done_frame_num;
 };
 
 #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 afbbfd5d02bc..1e3852295f2f 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 fa0d45fd7c34..87aaa5c9082b 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 88ababf1a059..d4ded3def1bf 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
@@ -190,6 +190,51 @@ void mtk_jpeg_set_enc_params(struct mtk_jpeg_ctx *ctx,  void __iomem *base)
 }
 EXPORT_SYMBOL_GPL(mtk_jpeg_set_enc_params);
 
+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 *dly_work = to_delayed_work(work);
@@ -210,6 +255,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)
@@ -244,8 +290,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->venc_clk.clks->clk);
 	pm_runtime_put(ctx->jpeg->dev);
 	if (ctx->fh.m2m_ctx &&
-- 
2.18.0


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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195
  2022-02-24  9:07 [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 kyrie.wu
                   ` (5 preceding siblings ...)
  2022-02-24  9:07 ` [RESEND,V7,6/6] media: mtk-jpegenc: add output pic reorder interface kyrie.wu
@ 2022-02-24 10:04 ` Hans Verkuil
  6 siblings, 0 replies; 11+ messages in thread
From: Hans Verkuil @ 2022-02-24 10:04 UTC (permalink / raw)
  To: kyrie.wu, Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

Hi Kyrie Wu,

I just noticed that none of your patches ever arrived at the linux-media mailinglist.
And since they never arrived there, then they also won't appear in our patchwork
instance (https://patchwork.linuxtv.org/).

That might explain why your media patches aren't picked up since I rely on patchwork
to keep track of new patches.

I've no idea why they do not arrive, I see nothing wrong with them.

Perhaps you should try to post a simple test email to the mailinglist to see if it
arrives.

Looking for mails from you at lore.kernel.org:

https://lore.kernel.org/linux-media/?q=kyrie.wu

only shows replies *to* you, and not a single email actually *from* you.

Regards,

	Hans

On 2/24/22 10:07, kyrie.wu wrote:
> From: kyrie wu <kyrie.wu@mediatek.com>
> 
> 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 Adds jpeg encoder dt-bindings for mt8195
> 
> Patches 2 jpeg encoder builds two module for using Multi-HW,
> export some functions to make them visible by other modules.
> 
> Patches 3 use devm_of_platform_populate to manage multi-hardware.
> 
> Patch 4 add jpeg encoding timeout function to judge hardware timeout.
> 
> Patch 5 add encoding work queue to deal with multi-hardware encoding
> at the same time.
> 
> Patch 6 add output picture reorder function to order images.
> ---
> Changes compared with v6:
> - new yaml file for mt8195 jpeg encoder.
> - some modifications for patch v5's review comments.
> 
> 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 (6):
>   dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible
>   media: mtk-jpegenc: export jpeg encoder functions
>   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
> 
>  .../media/mediatek,mt8195-jpegenc.yaml        | 174 +++++++++++
>  drivers/media/platform/mtk-jpeg/Makefile      |  11 +-
>  .../media/platform/mtk-jpeg/mtk_jpeg_core.c   | 292 +++++++++++++++---
>  .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |  81 ++++-
>  .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.c |   1 +
>  .../media/platform/mtk-jpeg/mtk_jpeg_dec_hw.h |   3 +-
>  .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 261 ++++++++++++++++
>  7 files changed, 769 insertions(+), 54 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> 

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RESEND, V7, 3/6] media: mtk-jpegenc: manage jpegenc multi-hardware
  2022-02-24  9:07 ` [RESEND,V7,3/6] media: mtk-jpegenc: manage jpegenc multi-hardware kyrie.wu
@ 2022-03-02  9:45   ` AngeloGioacchino Del Regno
  2022-03-07  1:46     ` [RESEND,V7,3/6] " kyrie.wu
  0 siblings, 1 reply; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-03-02  9:45 UTC (permalink / raw)
  To: kyrie.wu, Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

Il 24/02/22 10:07, kyrie.wu ha scritto:
> From: kyrie wu <kyrie.wu@mediatek.com>
> 
> manage each hardware information, including irq/clk/power.
> the hardware includes HW0 and HW1.
> 
> Signed-off-by: kyrie wu <kyrie.wu@mediatek.com>
> ---
>   drivers/media/platform/mtk-jpeg/Makefile      |  11 +-
>   .../media/platform/mtk-jpeg/mtk_jpeg_core.c   |  76 +++++---
>   .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |  37 ++++
>   .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 168 ++++++++++++++++++
>   4 files changed, 267 insertions(+), 25 deletions(-)
> 

Hello Kyrie,

despite my v6 review, where I also gave you solutions for an issue with
more than one example, this v7 still didn't get one out of the many
requested fixes.

I'm sure that this was not intentional, so it's not a problem...

In any case, this gave me the opportunity to see some more issues inside
of this patch: let's get it perfect!


...snip...

> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> index 3e4811a41ba2..31e941ef84bd 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> @@ -9,6 +9,7 @@
>   #ifndef _MTK_JPEG_CORE_H
>   #define _MTK_JPEG_CORE_H
>   
> +#include <linux/clk.h>
>   #include <linux/interrupt.h>
>   #include <media/v4l2-ctrls.h>
>   #include <media/v4l2-device.h>
> @@ -60,6 +61,7 @@ enum mtk_jpeg_ctx_state {
>    * @cap_q_default_fourcc:	capture queue default fourcc
>    */
>   struct mtk_jpeg_variant {
> +	bool is_multihw;

Thanks for this fix, this name makes it way clearer!

>   	struct clk_bulk_data *clks;
>   	int num_clks;
>   	struct mtk_jpeg_fmt *formats;
> @@ -74,6 +76,38 @@ 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_vcodec_clk - Structure used to store vcodec clock information
> + */
> +struct mtk_jpegenc_clk {
> +	struct clk_bulk_data *clks;
> +	int	clk_num;

Why is clk_num tabbed?

> +};
> +
> +/**
> + * 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

You're using tabulations *and* spaces.... please use either, not both, as it's
not necessary. Besides, this is also producing bad indentation.

> + */
> +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_clk	venc_clk;
> +	int jpegenc_irq;
> +};
> +
>   /**
>    * struct mtk_jpeg_dev - JPEG IP abstraction
>    * @lock:		the mutex protecting this structure
> @@ -100,6 +134,9 @@ struct mtk_jpeg_dev {
>   	void __iomem		*reg_base;
>   	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];
>   };
>   
>   /**
> 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 a2b6e1f85c2d..3d967bff1352 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> @@ -5,11 +5,27 @@
>    *
>    */
>   
> +#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 +46,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,
> +	},

I've already pointed out an issue with this in your v6 series:

https://patchwork.kernel.org/comment/24726607/

Besides, I want to add up that the SoC distinction is already done in the
parent node which, in MT8195's case, is named "mediatek,mt8195-jpgenc", so
you really don't have to redo this distinction "from scratch" here in the
sub-driver, as you can just get your information from the parent device/node.

So, just "mediatek,jpgenc-hw" should be totally enough here.

Please fix this for v8.


> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids);
> +#endif
> +
>   void mtk_jpeg_enc_reset(void __iomem *base)
>   {
>   	writel(0, base + JPEG_ENC_RSTB);

...snip...

> +
> +static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
> +{
> +	struct mtk_jpegenc_clk *jpegenc_clk;
> +	struct mtk_jpeg_dev *master_dev;
> +	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;
> +
> +	jpegenc_clk = &dev->venc_clk;
> +
> +	jpegenc_clk->clk_num = devm_clk_bulk_get_all(&pdev->dev,
> +						     &jpegenc_clk->clks);

Using dev_err_probe() looks more appropriate here:

	if (jpegenc_clk->clk_num < 0)
		return dev_err_probe(&pdev->dev, jpegenc_clk->clk_num,
				     "Failed to get jpegenc clocks\n");


> +	if (jpegenc_clk->clk_num < 0) {
> +		dev_err(&pdev->dev, "Failed to get jpegenc clock count\n");
> +		return jpegenc_clk->clk_num;
> +	}
> +
> +	dev->reg_base =
> +		devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(dev->reg_base)) {
> +		ret = PTR_ERR(dev->reg_base);
> +		goto err;

There's no need for any goto here, as you're not reverting any operation.

Hence, you can just:

	if (IS_ERR(dev->reg_base))
		return PTR_ERR(dev->reg_base);

> +	}
> +
> +	ret = mtk_jpegenc_hw_init_irq(dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to register JPEGENC irq handler.\n");

You are already printing an error inside of mtk_jpegenc_hw_init_irq(), so printing
another one here is redundant.
Either remove the prints in the function or, more appropriately, remove this print.

Also, same "goto" comment applies here, you can simply return ret.

> +		goto err;
> +	}
> +
> +	comp_idx = (enum mtk_jpegenc_hw_id)of_device_get_match_data(decs);
> +	if (comp_idx < MTK_JPEGENC_HW_MAX) {

`comp_idx` is a bit misleading, this is not using the component framework.

....but this will probably be refactored after following the suggestion that
I gave you in v6 and again now.

> +		master_dev->enc_hw_dev[comp_idx] = dev;
> +		master_dev->reg_encbase[comp_idx] = dev->reg_base;
> +		dev->master_dev = master_dev;
> +	} else {
> +		dev_err(&pdev->dev, "Failed to get_match_data.\n");
> +		goto err;
> +	}
> +
> +	platform_set_drvdata(pdev, dev);
> +	pm_runtime_enable(&pdev->dev);
> +
> +	return 0;
> +
> +err:

This label serves no real purpose: please remove.

> +	return ret;
> +}
> +



Regards,
Angelo

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RESEND,V7,3/6] media: mtk-jpegenc: manage jpegenc multi-hardware
  2022-03-02  9:45   ` [RESEND, V7, 3/6] " AngeloGioacchino Del Regno
@ 2022-03-07  1:46     ` kyrie.wu
  0 siblings, 0 replies; 11+ messages in thread
From: kyrie.wu @ 2022-03-07  1:46 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno, Hans Verkuil, Mauro Carvalho Chehab,
	Rob Herring, Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

On Wed, 2022-03-02 at 10:45 +0100, AngeloGioacchino Del Regno wrote:
> Il 24/02/22 10:07, kyrie.wu ha scritto:
> > From: kyrie wu <kyrie.wu@mediatek.com>
> > 
> > manage each hardware information, including irq/clk/power.
> > the hardware includes HW0 and HW1.
> > 
> > Signed-off-by: kyrie wu <kyrie.wu@mediatek.com>
> > ---
> >   drivers/media/platform/mtk-jpeg/Makefile      |  11 +-
> >   .../media/platform/mtk-jpeg/mtk_jpeg_core.c   |  76 +++++---
> >   .../media/platform/mtk-jpeg/mtk_jpeg_core.h   |  37 ++++
> >   .../media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c | 168
> > ++++++++++++++++++
> >   4 files changed, 267 insertions(+), 25 deletions(-)
> > 
> 
> Hello Kyrie,
> 
> despite my v6 review, where I also gave you solutions for an issue
> with
> more than one example, this v7 still didn't get one out of the many
> requested fixes.
> 
> I'm sure that this was not intentional, so it's not a problem...
> 
> In any case, this gave me the opportunity to see some more issues
> inside
> of this patch: let's get it perfect!
> 
> 
> ...snip...
> 
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 3e4811a41ba2..31e941ef84bd 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -9,6 +9,7 @@
> >   #ifndef _MTK_JPEG_CORE_H
> >   #define _MTK_JPEG_CORE_H
> >   
> > +#include <linux/clk.h>
> >   #include <linux/interrupt.h>
> >   #include <media/v4l2-ctrls.h>
> >   #include <media/v4l2-device.h>
> > @@ -60,6 +61,7 @@ enum mtk_jpeg_ctx_state {
> >    * @cap_q_default_fourcc:	capture queue default fourcc
> >    */
> >   struct mtk_jpeg_variant {
> > +	bool is_multihw;
> 
> Thanks for this fix, this name makes it way clearer!
> 
> >   	struct clk_bulk_data *clks;
> >   	int num_clks;
> >   	struct mtk_jpeg_fmt *formats;
> > @@ -74,6 +76,38 @@ 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_vcodec_clk - Structure used to store vcodec clock
> > information
> > + */
> > +struct mtk_jpegenc_clk {
> > +	struct clk_bulk_data *clks;
> > +	int	clk_num;
> 
> Why is clk_num tabbed?
> 
> > +};
> > +
> > +/**
> > + * 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
> 
> You're using tabulations *and* spaces.... please use either, not
> both, as it's
> not necessary. Besides, this is also producing bad indentation.
> 
> > + */
> > +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_clk	venc_clk;
> > +	int jpegenc_irq;
> > +};
> > +
> >   /**
> >    * struct mtk_jpeg_dev - JPEG IP abstraction
> >    * @lock:		the mutex protecting this structure
> > @@ -100,6 +134,9 @@ struct mtk_jpeg_dev {
> >   	void __iomem		*reg_base;
> >   	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];
> >   };
> >   
> >   /**
> > 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 a2b6e1f85c2d..3d967bff1352 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_enc_hw.c
> > @@ -5,11 +5,27 @@
> >    *
> >    */
> >   
> > +#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 +46,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,
> > +	},
> 
> I've already pointed out an issue with this in your v6 series:
> 
> https://patchwork.kernel.org/comment/24726607/
> 
> Besides, I want to add up that the SoC distinction is already done in
> the
> parent node which, in MT8195's case, is named "mediatek,mt8195-
> jpgenc", so
> you really don't have to redo this distinction "from scratch" here in
> the
> sub-driver, as you can just get your information from the parent
> device/node.
> 
> So, just "mediatek,jpgenc-hw" should be totally enough here.
> 
> Please fix this for v8.
Dear Angelo,
Thank you for your prompt reply.
I'm terrible sorry for that I got a misunderstanding from your preview
comments. The device node have fixed  in [RESEND,V7,1/6], base on this
patch, I thought that your suggestion is adopt properly.
Thanks again for your advice, I will fix driver data in the next
version.
> 
> 
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_jpegenc_drv_ids);
> > +#endif
> > +
> >   void mtk_jpeg_enc_reset(void __iomem *base)
> >   {
> >   	writel(0, base + JPEG_ENC_RSTB);
> 
> ...snip...
> 
> > +
> > +static int mtk_jpegenc_hw_probe(struct platform_device *pdev)
> > +{
> > +	struct mtk_jpegenc_clk *jpegenc_clk;
> > +	struct mtk_jpeg_dev *master_dev;
> > +	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;
> > +
> > +	jpegenc_clk = &dev->venc_clk;
> > +
> > +	jpegenc_clk->clk_num = devm_clk_bulk_get_all(&pdev->dev,
> > +						     &jpegenc_clk-
> > >clks);
> 
> Using dev_err_probe() looks more appropriate here:
Dear Angelo,
The follow suggestions are same as jpegdec patch in the preview
version. I wiil fix all of them.
Thanks.

Regards,
Kyrie
> 
> 	if (jpegenc_clk->clk_num < 0)
> 		return dev_err_probe(&pdev->dev, jpegenc_clk->clk_num,
> 				     "Failed to get jpegenc clocks\n");
> 
> 
> > +	if (jpegenc_clk->clk_num < 0) {
> > +		dev_err(&pdev->dev, "Failed to get jpegenc clock
> > count\n");
> > +		return jpegenc_clk->clk_num;
> > +	}
> > +
> > +	dev->reg_base =
> > +		devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(dev->reg_base)) {
> > +		ret = PTR_ERR(dev->reg_base);
> > +		goto err;
> 
> There's no need for any goto here, as you're not reverting any
> operation.
> 
> Hence, you can just:
> 
> 	if (IS_ERR(dev->reg_base))
> 		return PTR_ERR(dev->reg_base);
> 
> > +	}
> > +
> > +	ret = mtk_jpegenc_hw_init_irq(dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to register JPEGENC irq
> > handler.\n");
> 
> You are already printing an error inside of
> mtk_jpegenc_hw_init_irq(), so printing
> another one here is redundant.
> Either remove the prints in the function or, more appropriately,
> remove this print.
> 
> Also, same "goto" comment applies here, you can simply return ret.
> 
> > +		goto err;
> > +	}
> > +
> > +	comp_idx = (enum
> > mtk_jpegenc_hw_id)of_device_get_match_data(decs);
> > +	if (comp_idx < MTK_JPEGENC_HW_MAX) {
> 
> `comp_idx` is a bit misleading, this is not using the component
> framework.
> 
> ....but this will probably be refactored after following the
> suggestion that
> I gave you in v6 and again now.
> 
> > +		master_dev->enc_hw_dev[comp_idx] = dev;
> > +		master_dev->reg_encbase[comp_idx] = dev->reg_base;
> > +		dev->master_dev = master_dev;
> > +	} else {
> > +		dev_err(&pdev->dev, "Failed to get_match_data.\n");
> > +		goto err;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, dev);
> > +	pm_runtime_enable(&pdev->dev);
> > +
> > +	return 0;
> > +
> > +err:
> 
> This label serves no real purpose: please remove.
> 
> > +	return ret;
> > +}
> > +
> 
> 
> 
> Regards,
> Angelo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RESEND, V7, 1/6] dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible
  2022-02-24  9:07 ` [RESEND, V7, 1/6] dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible kyrie.wu
@ 2022-03-16 12:58   ` Rex-BC Chen
  0 siblings, 0 replies; 11+ messages in thread
From: Rex-BC Chen @ 2022-03-16 12:58 UTC (permalink / raw)
  To: kyrie.wu, Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
	Matthias Brugger, Tzung-Bi Shih
  Cc: Project_Global_Chrome_Upstream_Group, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek, Tomasz Figa,
	xia.jiang, maoguang.meng, srv_heupstream

On Thu, 2022-02-24 at 17:07 +0800, kyrie.wu wrote:
> From: kyrie wu <kyrie.wu@mediatek.com>
> 
> Add mediatek,mt8195-jpgenc compatible to binding document.
> 
> Signed-off-by: kyrie wu <kyrie.wu@mediatek.com>
> ---
>  .../media/mediatek,mt8195-jpegenc.yaml        | 174
> ++++++++++++++++++
>  1 file changed, 174 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/media/mediatek,mt8195-jpegenc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8195-
> jpegenc.yaml
> b/Documentation/devicetree/bindings/media/mediatek,mt8195-
> jpegenc.yaml
> new file mode 100644
> index 000000000000..efb0d843edb6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mt8195-
> jpegenc.yaml
> @@ -0,0 +1,174 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/media/mediatek,mt8195-jpegenc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek JPEG Encoder Device Tree Bindings
> +
> +maintainers:
> +  - kyrie wu <kyrie.wu@mediatek.corp-partner.google.com>

Hello Kyrie,

I think you should use kyrie.wu@mediatek.com instead of pd account.

BRs,
Rex

> +
> +description: |-
> +  MediaTek JPEG Encoder is the JPEG encode hardware present in
> MediaTek SoCs
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: mediatek,mt8195-jpgenc
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  iommus:
> +    maxItems: 4
> +    description: |
> +      Points to the respective IOMMU block with master port as
> argument, see
> +      Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> for details.
> +      Ports are according to the HW.
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  ranges: true
> +
> +# Required child node:
> +patternProperties:
> +  "jpgenc@1a030000":
> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: mediatek,mt8195-jpgenc0
> +
> +      reg:
> +        maxItems: 1
> +
> +      iommus:
> +        minItems: 1
> +        maxItems: 32
> +        description: |
> +          List of the hardware port in respective IOMMU block for
> current Socs.
> +          Refer to bindings/iommu/mediatek,iommu.yaml.
> +
> +      interrupts:
> +        maxItems: 1
> +
> +      clocks:
> +        maxItems: 1
> +
> +      clock-names:
> +        items:
> +          - const: jpgenc
> +
> +      power-domains:
> +        maxItems: 1
> +
> +    required:
> +      - compatible
> +      - reg
> +      - iommus
> +      - interrupts
> +      - clocks
> +      - clock-names
> +      - power-domains
> +
> +    additionalProperties: false
> +
> +  "jpgenc@1b030000":
> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: mediatek,mt8195-jpgenc1
> +
> +      reg:
> +        maxItems: 1
> +
> +      iommus:
> +        minItems: 1
> +        maxItems: 32
> +        description: |
> +          List of the hardware port in respective IOMMU block for
> current Socs.
> +          Refer to bindings/iommu/mediatek,iommu.yaml.
> +
> +      interrupts:
> +        maxItems: 1
> +
> +      clocks:
> +        maxItems: 1
> +
> +      clock-names:
> +        items:
> +          - const: jpgenc
> +
> +      power-domains:
> +        maxItems: 1
> +
> +    required:
> +      - compatible
> +      - reg
> +      - iommus
> +      - interrupts
> +      - clocks
> +      - clock-names
> +      - power-domains
> +
> +    additionalProperties: false
> +
> +
> +required:
> +  - compatible
> +  - power-domains
> +  - iommus
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/memory/mt8195-memory-port.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/power/mt8195-power.h>
> +
> +    jpgenc_master {
> +            compatible = "mediatek,mt8195-jpgenc";
> +            power-domains = <&spm MT8195_POWER_DOMAIN_VENC_CORE1>;
> +            iommus = <&iommu_vpp M4U_PORT_L20_JPGENC_Y_RDMA>,
> +            <&iommu_vpp M4U_PORT_L20_JPGENC_C_RDMA>,
> +            <&iommu_vpp M4U_PORT_L20_JPGENC_Q_TABLE>,
> +            <&iommu_vpp M4U_PORT_L20_JPGENC_BSDMA>;
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            ranges;
> +
> +            jpgenc@1a030000 {
> +                    compatible = "mediatek,mt8195-jpgenc0";
> +                    reg = <0x1a030000 0x10000>;
> +                    iommus = <&iommu_vdo
> M4U_PORT_L19_JPGENC_Y_RDMA>,
> +                    <&iommu_vdo M4U_PORT_L19_JPGENC_C_RDMA>,
> +                    <&iommu_vdo M4U_PORT_L19_JPGENC_Q_TABLE>,
> +                    <&iommu_vdo M4U_PORT_L19_JPGENC_BSDMA>;
> +                    interrupts = <GIC_SPI 342 IRQ_TYPE_LEVEL_HIGH
> 0>;
> +                    clocks = <&vencsys CLK_VENC_JPGENC>;
> +                    clock-names = "jpgenc";
> +                    power-domains = <&spm MT8195_POWER_DOMAIN_VENC>;
> +            };
> +
> +            jpgenc@1b030000 {
> +                    compatible = "mediatek,mt8195-jpgenc1";
> +                    reg = <0x1b030000 0x10000>;
> +                    iommus = <&iommu_vpp
> M4U_PORT_L20_JPGENC_Y_RDMA>,
> +                    <&iommu_vpp M4U_PORT_L20_JPGENC_C_RDMA>,
> +                    <&iommu_vpp M4U_PORT_L20_JPGENC_Q_TABLE>,
> +                    <&iommu_vpp M4U_PORT_L20_JPGENC_BSDMA>;
> +                    interrupts = <GIC_SPI 347 IRQ_TYPE_LEVEL_HIGH
> 0>;
> +                    clocks = <&vencsys_core1 CLK_VENC_CORE1_JPGENC>;
> +                    clock-names = "jpgenc";
> +                    power-domains = <&spm
> MT8195_POWER_DOMAIN_VENC_CORE1>;
> +            };
> +    };


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-03-16 13:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-24  9:07 [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 kyrie.wu
2022-02-24  9:07 ` [RESEND, V7, 1/6] dt-bindings: mediatek: Add mediatek, mt8195-jpgenc compatible kyrie.wu
2022-03-16 12:58   ` Rex-BC Chen
2022-02-24  9:07 ` [RESEND,V7,2/6] media: mtk-jpegenc: export jpeg encoder functions kyrie.wu
2022-02-24  9:07 ` [RESEND,V7,3/6] media: mtk-jpegenc: manage jpegenc multi-hardware kyrie.wu
2022-03-02  9:45   ` [RESEND, V7, 3/6] " AngeloGioacchino Del Regno
2022-03-07  1:46     ` [RESEND,V7,3/6] " kyrie.wu
2022-02-24  9:07 ` [RESEND, V7, 4/6] media: mtk-jpegenc: add jpegenc timeout func interface kyrie.wu
2022-02-24  9:07 ` [RESEND,V7,5/6] media: mtk-jpegenc: add jpeg encode worker interface kyrie.wu
2022-02-24  9:07 ` [RESEND,V7,6/6] media: mtk-jpegenc: add output pic reorder interface kyrie.wu
2022-02-24 10:04 ` [RESEND,V7,0/6] Enable two hardware jpeg encoder for MT8195 Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).