* [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node
@ 2021-01-21 6:18 Irui Wang
2021-01-21 6:18 ` [PATCH 2/3] arm64: dts: mt8173: Separating mtk-vcodec-enc device node Irui Wang
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Irui Wang @ 2021-01-21 6:18 UTC (permalink / raw)
To: Alexandre Courbot, Hans Verkuil, Tiffany Lin, Andrew-CT Chen,
Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Tomasz Figa, Hsin-Yi Wang, Maoguang Meng, Longfei Wang,
Yunfei Dong
Cc: Irui Wang, linux-media, devicetree, linux-kernel,
linux-arm-kernel, srv_heupstream, linux-mediatek
Updates binding document since the avc and vp8 hardware encoder in
MT8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
"mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
.../bindings/media/mediatek-vcodec.txt | 58 ++++++++++---------
1 file changed, 31 insertions(+), 27 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
index 8217424fd4bd..f85276e629bf 100644
--- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
+++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
@@ -4,7 +4,9 @@ Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
supports high resolution encoding and decoding functionalities.
Required properties:
-- compatible : "mediatek,mt8173-vcodec-enc" for MT8173 encoder
+- compatible : must be one of the following string:
+ "mediatek,mt8173-vcodec-vp8-enc" for mt8173 vp8 encoder.
+ "mediatek,mt8173-vcodec-avc-enc" for mt8173 avc encoder.
"mediatek,mt8183-vcodec-enc" for MT8183 encoder.
"mediatek,mt8173-vcodec-dec" for MT8173 decoder.
- reg : Physical base address of the video codec registers and length of
@@ -13,10 +15,11 @@ Required properties:
- mediatek,larb : must contain the local arbiters in the current Socs.
- clocks : list of clock specifiers, corresponding to entries in
the clock-names property.
-- clock-names: encoder must contain "venc_sel_src", "venc_sel",,
- "venc_lt_sel_src", "venc_lt_sel", decoder must contain "vcodecpll",
- "univpll_d2", "clk_cci400_sel", "vdec_sel", "vdecpll", "vencpll",
- "venc_lt_sel", "vdec_bus_clk_src".
+- clock-names:
+ avc venc must contain "venc_sel";
+ vp8 venc must contain "venc_lt_sel";
+ decoder must contain "vcodecpll", "univpll_d2", "clk_cci400_sel",
+ "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src".
- iommus : should point to the respective IOMMU block with master port as
argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
for details.
@@ -80,14 +83,10 @@ vcodec_dec: vcodec@16000000 {
assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
};
- vcodec_enc: vcodec@18002000 {
- compatible = "mediatek,mt8173-vcodec-enc";
- reg = <0 0x18002000 0 0x1000>, /*VENC_SYS*/
- <0 0x19002000 0 0x1000>; /*VENC_LT_SYS*/
- interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>,
- <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
- mediatek,larb = <&larb3>,
- <&larb5>;
+vcodec_enc: vcodec@18002000 {
+ compatible = "mediatek,mt8173-vcodec-avc-enc";
+ reg = <0 0x18002000 0 0x1000>;
+ interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
iommus = <&iommu M4U_PORT_VENC_RCPU>,
<&iommu M4U_PORT_VENC_REC>,
<&iommu M4U_PORT_VENC_BSDMA>,
@@ -98,8 +97,20 @@ vcodec_dec: vcodec@16000000 {
<&iommu M4U_PORT_VENC_REF_LUMA>,
<&iommu M4U_PORT_VENC_REF_CHROMA>,
<&iommu M4U_PORT_VENC_NBM_RDMA>,
- <&iommu M4U_PORT_VENC_NBM_WDMA>,
- <&iommu M4U_PORT_VENC_RCPU_SET2>,
+ <&iommu M4U_PORT_VENC_NBM_WDMA>;
+ mediatek,larb = <&larb3>;
+ mediatek,vpu = <&vpu>;
+ clocks = <&topckgen CLK_TOP_VENC_SEL>;
+ clock-names = "venc_sel";
+ assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;
+ assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;
+ };
+
+vcodec_enc_lt: vcodec@19002000 {
+ compatible = "mediatek,mt8173-vcodec-vp8-enc";
+ reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
+ interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
+ iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,
<&iommu M4U_PORT_VENC_REC_FRM_SET2>,
<&iommu M4U_PORT_VENC_BSDMA_SET2>,
<&iommu M4U_PORT_VENC_SV_COMA_SET2>,
@@ -108,17 +119,10 @@ vcodec_dec: vcodec@16000000 {
<&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
<&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
<&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
+ mediatek,larb = <&larb5>;
mediatek,vpu = <&vpu>;
- clocks = <&topckgen CLK_TOP_VENCPLL_D2>,
- <&topckgen CLK_TOP_VENC_SEL>,
- <&topckgen CLK_TOP_UNIVPLL1_D2>,
- <&topckgen CLK_TOP_VENC_LT_SEL>;
- clock-names = "venc_sel_src",
- "venc_sel",
- "venc_lt_sel_src",
- "venc_lt_sel";
- assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>,
- <&topckgen CLK_TOP_VENC_LT_SEL>;
- assigned-clock-parents = <&topckgen CLK_TOP_VENCPLL_D2>,
- <&topckgen CLK_TOP_UNIVPLL1_D2>;
+ clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
+ clock-names = "venc_lt_sel";
+ assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
+ assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>;
};
--
2.18.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] arm64: dts: mt8173: Separating mtk-vcodec-enc device node
2021-01-21 6:18 [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Irui Wang
@ 2021-01-21 6:18 ` Irui Wang
2021-01-26 8:43 ` Tiffany Lin
2021-01-21 6:18 ` [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver Irui Wang
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Irui Wang @ 2021-01-21 6:18 UTC (permalink / raw)
To: Alexandre Courbot, Hans Verkuil, Tiffany Lin, Andrew-CT Chen,
Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Tomasz Figa, Hsin-Yi Wang, Maoguang Meng, Longfei Wang,
Yunfei Dong
Cc: Irui Wang, linux-media, devicetree, linux-kernel,
linux-arm-kernel, srv_heupstream, linux-mediatek
There are two separate hardware encoder blocks inside MT8173.
Split the current mtk-vcodec-enc node to match the hardware architecture.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 60 ++++++++++++------------
1 file changed, 31 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 7fa870e4386a..d667b296c512 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -1459,13 +1459,10 @@
};
vcodec_enc: vcodec@18002000 {
- compatible = "mediatek,mt8173-vcodec-enc";
- reg = <0 0x18002000 0 0x1000>, /* VENC_SYS */
- <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
- interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>,
- <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
- mediatek,larb = <&larb3>,
- <&larb5>;
+ compatible = "mediatek,mt8173-vcodec-avc-enc";
+ reg = <0 0x18002000 0 0x1000>; /* VENC_SYS */
+ interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
+ mediatek,larb = <&larb3>;
iommus = <&iommu M4U_PORT_VENC_RCPU>,
<&iommu M4U_PORT_VENC_REC>,
<&iommu M4U_PORT_VENC_BSDMA>,
@@ -1476,29 +1473,12 @@
<&iommu M4U_PORT_VENC_REF_LUMA>,
<&iommu M4U_PORT_VENC_REF_CHROMA>,
<&iommu M4U_PORT_VENC_NBM_RDMA>,
- <&iommu M4U_PORT_VENC_NBM_WDMA>,
- <&iommu M4U_PORT_VENC_RCPU_SET2>,
- <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
- <&iommu M4U_PORT_VENC_BSDMA_SET2>,
- <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
- <&iommu M4U_PORT_VENC_RD_COMA_SET2>,
- <&iommu M4U_PORT_VENC_CUR_LUMA_SET2>,
- <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
- <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
- <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
+ <&iommu M4U_PORT_VENC_NBM_WDMA>;
mediatek,vpu = <&vpu>;
- clocks = <&topckgen CLK_TOP_VENCPLL_D2>,
- <&topckgen CLK_TOP_VENC_SEL>,
- <&topckgen CLK_TOP_UNIVPLL1_D2>,
- <&topckgen CLK_TOP_VENC_LT_SEL>;
- clock-names = "venc_sel_src",
- "venc_sel",
- "venc_lt_sel_src",
- "venc_lt_sel";
- assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>,
- <&topckgen CLK_TOP_VENC_LT_SEL>;
- assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>,
- <&topckgen CLK_TOP_VCODECPLL_370P5>;
+ clocks = <&topckgen CLK_TOP_VENC_SEL>;
+ clock-names = "venc_sel";
+ assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;
+ assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;
};
jpegdec: jpegdec@18004000 {
@@ -1530,5 +1510,27 @@
<&vencltsys CLK_VENCLT_CKE0>;
clock-names = "apb", "smi";
};
+
+ vcodec_enc_lt: vcodec@19002000 {
+ compatible = "mediatek,mt8173-vcodec-vp8-enc";
+ reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
+ interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
+ iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,
+ <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
+ <&iommu M4U_PORT_VENC_BSDMA_SET2>,
+ <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
+ <&iommu M4U_PORT_VENC_RD_COMA_SET2>,
+ <&iommu M4U_PORT_VENC_CUR_LUMA_SET2>,
+ <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
+ <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
+ <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
+ mediatek,larb = <&larb5>;
+ mediatek,vpu = <&vpu>;
+ clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
+ clock-names = "venc_lt_sel";
+ assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
+ assigned-clock-parents =
+ <&topckgen CLK_TOP_VCODECPLL_370P5>;
+ };
};
};
--
2.18.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver
2021-01-21 6:18 [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Irui Wang
2021-01-21 6:18 ` [PATCH 2/3] arm64: dts: mt8173: Separating mtk-vcodec-enc device node Irui Wang
@ 2021-01-21 6:18 ` Irui Wang
2021-01-26 8:44 ` Tiffany Lin
2021-02-03 10:44 ` Alexandre Courbot
2021-01-26 8:42 ` [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Tiffany Lin
` (2 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Irui Wang @ 2021-01-21 6:18 UTC (permalink / raw)
To: Alexandre Courbot, Hans Verkuil, Tiffany Lin, Andrew-CT Chen,
Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Tomasz Figa, Hsin-Yi Wang, Maoguang Meng, Longfei Wang,
Yunfei Dong
Cc: Irui Wang, linux-media, devicetree, linux-kernel,
linux-arm-kernel, srv_heupstream, linux-mediatek
MTK H264 Encoder(VENC_SYS) and VP8 Encoder(VENC_LT_SYS) are two
independent hardware instance. They have their owner interrupt,
register mapping, and special clocks.
This patch seperates them into two drivers:
User Call "VIDIOC_QUERYCAP":
H264 Encoder return driver name "mtk-vcodec-enc";
VP8 Encoder return driver name "mtk-venc-vp8.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
.../platform/mtk-vcodec/mtk_vcodec_drv.h | 10 +-
.../platform/mtk-vcodec/mtk_vcodec_enc.c | 23 +++-
.../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 121 +++++++-----------
.../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 40 +-----
.../platform/mtk-vcodec/venc/venc_vp8_if.c | 4 +-
5 files changed, 82 insertions(+), 116 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
index 3dd010cba23e..1594edcc706d 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
@@ -19,6 +19,7 @@
#define MTK_VCODEC_DRV_NAME "mtk_vcodec_drv"
#define MTK_VCODEC_DEC_NAME "mtk-vcodec-dec"
#define MTK_VCODEC_ENC_NAME "mtk-vcodec-enc"
+#define MTK_VENC_VP8_NAME "mtk-venc-vp8"
#define MTK_PLATFORM_STR "platform:mt8173"
#define MTK_VCODEC_MAX_PLANES 3
@@ -193,7 +194,6 @@ struct mtk_vcodec_pm {
struct mtk_vcodec_clk venc_clk;
struct device *larbvenc;
- struct device *larbvenclt;
struct device *dev;
struct mtk_vcodec_dev *mtkdev;
};
@@ -311,25 +311,27 @@ enum mtk_chip {
* @chip: chip this encoder is compatible with
*
* @uses_ext: whether the encoder uses the extended firmware messaging format
- * @has_lt_irq: whether the encoder uses the LT irq
+ * @name: whether the encoder core is vp8
* @min_birate: minimum supported encoding bitrate
* @max_bitrate: maximum supported encoding bitrate
* @capture_formats: array of supported capture formats
* @num_capture_formats: number of entries in capture_formats
* @output_formats: array of supported output formats
* @num_output_formats: number of entries in output_formats
+ * @core_id: stand for h264 or vp8 encode index
*/
struct mtk_vcodec_enc_pdata {
enum mtk_chip chip;
bool uses_ext;
- bool has_lt_irq;
+ const char *name;
unsigned long min_bitrate;
unsigned long max_bitrate;
const struct mtk_video_fmt *capture_formats;
size_t num_capture_formats;
const struct mtk_video_fmt *output_formats;
size_t num_output_formats;
+ int core_id;
};
#define MTK_ENC_CTX_IS_EXT(ctx) ((ctx)->dev->venc_pdata->uses_ext)
@@ -361,7 +363,6 @@ struct mtk_vcodec_enc_pdata {
*
* @dec_irq: decoder irq resource
* @enc_irq: h264 encoder irq resource
- * @enc_lt_irq: vp8 encoder irq resource
*
* @dec_mutex: decoder hardware lock
* @enc_mutex: encoder hardware lock.
@@ -397,7 +398,6 @@ struct mtk_vcodec_dev {
int dec_irq;
int enc_irq;
- int enc_lt_irq;
struct mutex dec_mutex;
struct mutex enc_mutex;
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index 21de1431cfcb..0da6871b4b39 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -9,6 +9,7 @@
#include <media/v4l2-mem2mem.h>
#include <media/videobuf2-dma-contig.h>
#include <soc/mediatek/smi.h>
+#include <linux/pm_runtime.h>
#include "mtk_vcodec_drv.h"
#include "mtk_vcodec_enc.h"
@@ -189,7 +190,10 @@ static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
static int vidioc_venc_querycap(struct file *file, void *priv,
struct v4l2_capability *cap)
{
- strscpy(cap->driver, MTK_VCODEC_ENC_NAME, sizeof(cap->driver));
+ const struct mtk_vcodec_enc_pdata *pdata =
+ fh_to_ctx(priv)->dev->venc_pdata;
+
+ strscpy(cap->driver, pdata->name, sizeof(cap->driver));
strscpy(cap->bus_info, MTK_PLATFORM_STR, sizeof(cap->bus_info));
strscpy(cap->card, MTK_PLATFORM_STR, sizeof(cap->card));
@@ -797,7 +801,7 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
*/
if ((ctx->state == MTK_STATE_ABORT) || (ctx->state == MTK_STATE_FREE)) {
ret = -EIO;
- goto err_set_param;
+ goto err_start_stream;
}
/* Do the initialization when both start_streaming have been called */
@@ -809,6 +813,12 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
return 0;
}
+ ret = pm_runtime_get_sync(&ctx->dev->plat_dev->dev);
+ if (ret < 0) {
+ mtk_v4l2_err("pm_runtime_get_sync fail %d", ret);
+ goto err_start_stream;
+ }
+
mtk_venc_set_param(ctx, ¶m);
ret = venc_if_set_param(ctx, VENC_SET_PARAM_ENC, ¶m);
if (ret) {
@@ -835,6 +845,11 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
return 0;
err_set_param:
+ ret = pm_runtime_put(&ctx->dev->plat_dev->dev);
+ if (ret < 0)
+ mtk_v4l2_err("pm_runtime_put fail %d", ret);
+
+err_start_stream:
for (i = 0; i < q->num_buffers; ++i) {
struct vb2_buffer *buf = vb2_get_buffer(q, i);
@@ -888,6 +903,10 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
if (ret)
mtk_v4l2_err("venc_if_deinit failed=%d", ret);
+ ret = pm_runtime_put(&ctx->dev->plat_dev->dev);
+ if (ret < 0)
+ mtk_v4l2_err("pm_runtime_put fail %d", ret);
+
ctx->state = MTK_STATE_FREE;
}
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
index dfb42e19bf81..4bee42454253 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
@@ -49,12 +49,15 @@ static const struct mtk_video_fmt mtk_video_formats_output_mt8173[] = {
},
};
-static const struct mtk_video_fmt mtk_video_formats_capture_mt8173[] = {
+static const struct mtk_video_fmt mtk_video_formats_capture_mt8173_h264[] = {
{
.fourcc = V4L2_PIX_FMT_H264,
.type = MTK_FMT_ENC,
.num_planes = 1,
},
+};
+
+static const struct mtk_video_fmt mtk_video_formats_capture_mt8173_vp8[] = {
{
.fourcc = V4L2_PIX_FMT_VP8,
.type = MTK_FMT_ENC,
@@ -110,35 +113,13 @@ static irqreturn_t mtk_vcodec_enc_irq_handler(int irq, void *priv)
ctx = dev->curr_ctx;
spin_unlock_irqrestore(&dev->irqlock, flags);
- mtk_v4l2_debug(1, "id=%d", ctx->id);
- addr = dev->reg_base[VENC_SYS] + MTK_VENC_IRQ_ACK_OFFSET;
-
- ctx->irq_status = readl(dev->reg_base[VENC_SYS] +
- (MTK_VENC_IRQ_STATUS_OFFSET));
-
- clean_irq_status(ctx->irq_status, addr);
-
- wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED);
- return IRQ_HANDLED;
-}
-
-static irqreturn_t mtk_vcodec_enc_lt_irq_handler(int irq, void *priv)
-{
- struct mtk_vcodec_dev *dev = priv;
- struct mtk_vcodec_ctx *ctx;
- unsigned long flags;
- void __iomem *addr;
-
- spin_lock_irqsave(&dev->irqlock, flags);
- ctx = dev->curr_ctx;
- spin_unlock_irqrestore(&dev->irqlock, flags);
+ mtk_v4l2_debug(1, "id=%d coreid:%d", ctx->id, dev->venc_pdata->core_id);
+ addr = dev->reg_base[dev->venc_pdata->core_id] +
+ MTK_VENC_IRQ_ACK_OFFSET;
- mtk_v4l2_debug(1, "id=%d", ctx->id);
- ctx->irq_status = readl(dev->reg_base[VENC_LT_SYS] +
+ ctx->irq_status = readl(dev->reg_base[dev->venc_pdata->core_id] +
(MTK_VENC_IRQ_STATUS_OFFSET));
- addr = dev->reg_base[VENC_LT_SYS] + MTK_VENC_IRQ_ACK_OFFSET;
-
clean_irq_status(ctx->irq_status, addr);
wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED);
@@ -293,17 +274,21 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
dev->venc_pdata = of_device_get_match_data(&pdev->dev);
ret = mtk_vcodec_init_enc_pm(dev);
if (ret < 0) {
- dev_err(&pdev->dev, "Failed to get mt vcodec clock source!");
+ dev_err(&pdev->dev, "Failed to get mtk vcodec clock source!");
goto err_enc_pm;
}
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- dev->reg_base[VENC_SYS] = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR((__force void *)dev->reg_base[VENC_SYS])) {
- ret = PTR_ERR((__force void *)dev->reg_base[VENC_SYS]);
+ pm_runtime_enable(&pdev->dev);
+
+ snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
+ dev->venc_pdata->name);
+
+ dev->reg_base[dev->venc_pdata->core_id] =
+ devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(dev->reg_base[dev->venc_pdata->core_id])) {
+ ret = PTR_ERR(dev->reg_base[dev->venc_pdata->core_id]);
goto err_res;
}
- mtk_v4l2_debug(2, "reg[%d] base=0x%p", i, dev->reg_base[VENC_SYS]);
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (res == NULL) {
@@ -318,44 +303,17 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
mtk_vcodec_enc_irq_handler,
0, pdev->name, dev);
if (ret) {
- dev_err(&pdev->dev, "Failed to install dev->enc_irq %d (%d)",
- dev->enc_irq,
- ret);
+ dev_err(&pdev->dev,
+ "Failed to install dev->enc_irq %d (%d) core_id:%d",
+ dev->enc_irq, ret, dev->venc_pdata->core_id);
ret = -EINVAL;
goto err_res;
}
- if (dev->venc_pdata->has_lt_irq) {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- dev->reg_base[VENC_LT_SYS] = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR((__force void *)dev->reg_base[VENC_LT_SYS])) {
- ret = PTR_ERR((__force void *)dev->reg_base[VENC_LT_SYS]);
- goto err_res;
- }
- mtk_v4l2_debug(2, "reg[%d] base=0x%p", i, dev->reg_base[VENC_LT_SYS]);
-
- dev->enc_lt_irq = platform_get_irq(pdev, 1);
- irq_set_status_flags(dev->enc_lt_irq, IRQ_NOAUTOEN);
- ret = devm_request_irq(&pdev->dev,
- dev->enc_lt_irq,
- mtk_vcodec_enc_lt_irq_handler,
- 0, pdev->name, dev);
- if (ret) {
- dev_err(&pdev->dev,
- "Failed to install dev->enc_lt_irq %d (%d)",
- dev->enc_lt_irq, ret);
- ret = -EINVAL;
- goto err_res;
- }
- }
-
mutex_init(&dev->enc_mutex);
mutex_init(&dev->dev_mutex);
spin_lock_init(&dev->irqlock);
- snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
- "[MTK_V4L2_VENC]");
-
ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
if (ret) {
mtk_v4l2_err("v4l2_device_register err=%d", ret);
@@ -381,7 +339,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
V4L2_CAP_STREAMING;
snprintf(vfd_enc->name, sizeof(vfd_enc->name), "%s",
- MTK_VCODEC_ENC_NAME);
+ dev->venc_pdata->name);
video_set_drvdata(vfd_enc, dev);
dev->vfd_enc = vfd_enc;
platform_set_drvdata(pdev, dev);
@@ -409,8 +367,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
goto err_enc_reg;
}
- mtk_v4l2_debug(0, "encoder registered as /dev/video%d",
- vfd_enc->num);
+ mtk_v4l2_debug(0, "encoder %d registered as /dev/video%d",
+ dev->venc_pdata->core_id, vfd_enc->num);
return 0;
@@ -429,20 +387,33 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
return ret;
}
-static const struct mtk_vcodec_enc_pdata mt8173_pdata = {
+static const struct mtk_vcodec_enc_pdata mt8173_avc_pdata = {
.chip = MTK_MT8173,
- .has_lt_irq = true,
- .capture_formats = mtk_video_formats_capture_mt8173,
- .num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_mt8173),
+ .name = MTK_VCODEC_ENC_NAME,
+ .capture_formats = mtk_video_formats_capture_mt8173_h264,
+ .num_capture_formats = 1,
.output_formats = mtk_video_formats_output_mt8173,
.num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
- .min_bitrate = 1,
+ .min_bitrate = 64,
+ .max_bitrate = 4000000,
+ .core_id = VENC_SYS,
+};
+
+static const struct mtk_vcodec_enc_pdata mt8173_vp8_pdata = {
+ .chip = MTK_MT8173,
+ .name = MTK_VENC_VP8_NAME,
+ .capture_formats = mtk_video_formats_capture_mt8173_vp8,
+ .num_capture_formats = 1,
+ .output_formats = mtk_video_formats_output_mt8173,
+ .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
+ .min_bitrate = 64,
.max_bitrate = 4000000,
+ .core_id = VENC_LT_SYS,
};
static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
.chip = MTK_MT8183,
- .has_lt_irq = false,
+ .name = MTK_VCODEC_ENC_NAME,
.uses_ext = true,
.capture_formats = mtk_video_formats_capture_mt8183,
.num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_mt8183),
@@ -451,10 +422,14 @@ static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
.num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
.min_bitrate = 64,
.max_bitrate = 40000000,
+ .core_id = VENC_SYS,
};
static const struct of_device_id mtk_vcodec_enc_match[] = {
- {.compatible = "mediatek,mt8173-vcodec-enc", .data = &mt8173_pdata},
+ {.compatible = "mediatek,mt8173-vcodec-avc-enc",
+ .data = &mt8173_avc_pdata},
+ {.compatible = "mediatek,mt8173-vcodec-vp8-enc",
+ .data = &mt8173_vp8_pdata},
{.compatible = "mediatek,mt8183-vcodec-enc", .data = &mt8183_pdata},
{},
};
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
index 3b7c54d6aa8f..1b2e4930ed27 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
@@ -43,23 +43,6 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
return -ENODEV;
}
pm->larbvenc = &pdev->dev;
-
- node = of_parse_phandle(dev->of_node, "mediatek,larb", 1);
- if (!node) {
- mtk_v4l2_err("no mediatek,larb found");
- ret = -ENODEV;
- goto put_larbvenc;
- }
-
- pdev = of_find_device_by_node(node);
- of_node_put(node);
- if (!pdev) {
- mtk_v4l2_err("no mediatek,larb device found");
- ret = -ENODEV;
- goto put_larbvenc;
- }
-
- pm->larbvenclt = &pdev->dev;
pdev = mtkdev->plat_dev;
pm->dev = &pdev->dev;
@@ -71,12 +54,12 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
GFP_KERNEL);
if (!enc_clk->clk_info) {
ret = -ENOMEM;
- goto put_larbvenclt;
+ goto put_larbvenc;
}
} else {
mtk_v4l2_err("Failed to get venc clock count");
ret = -EINVAL;
- goto put_larbvenclt;
+ goto put_larbvenc;
}
for (i = 0; i < enc_clk->clk_num; i++) {
@@ -85,7 +68,7 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
"clock-names", i, &clk_info->clk_name);
if (ret) {
mtk_v4l2_err("venc failed to get clk name %d", i);
- goto put_larbvenclt;
+ goto put_larbvenc;
}
clk_info->vcodec_clk = devm_clk_get(&pdev->dev,
clk_info->clk_name);
@@ -93,14 +76,12 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
mtk_v4l2_err("venc devm_clk_get (%d)%s fail", i,
clk_info->clk_name);
ret = PTR_ERR(clk_info->vcodec_clk);
- goto put_larbvenclt;
+ goto put_larbvenc;
}
}
return 0;
-put_larbvenclt:
- put_device(pm->larbvenclt);
put_larbvenc:
put_device(pm->larbvenc);
return ret;
@@ -108,7 +89,7 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
void mtk_vcodec_release_enc_pm(struct mtk_vcodec_dev *mtkdev)
{
- put_device(mtkdev->pm.larbvenclt);
+ pm_runtime_disable(mtkdev->pm.dev);
put_device(mtkdev->pm.larbvenc);
}
@@ -130,18 +111,10 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm)
ret = mtk_smi_larb_get(pm->larbvenc);
if (ret) {
mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret);
- goto larbvencerr;
- }
- ret = mtk_smi_larb_get(pm->larbvenclt);
- if (ret) {
- mtk_v4l2_err("mtk_smi_larb_get larb4 fail %d", ret);
- goto larbvenclterr;
+ goto clkerr;
}
return;
-larbvenclterr:
- mtk_smi_larb_put(pm->larbvenc);
-larbvencerr:
clkerr:
for (i -= 1; i >= 0; i--)
clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
@@ -153,7 +126,6 @@ void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm)
int i = 0;
mtk_smi_larb_put(pm->larbvenc);
- mtk_smi_larb_put(pm->larbvenclt);
for (i = enc_clk->clk_num - 1; i >= 0; i--)
clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
}
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
index 11abb191ada5..8267a9c4fd25 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
@@ -367,7 +367,7 @@ static int vp8_enc_encode(void *handle,
mtk_vcodec_debug_enter(inst);
- enable_irq(ctx->dev->enc_lt_irq);
+ enable_irq(ctx->dev->enc_irq);
switch (opt) {
case VENC_START_OPT_ENCODE_FRAME:
@@ -386,7 +386,7 @@ static int vp8_enc_encode(void *handle,
encode_err:
- disable_irq(ctx->dev->enc_lt_irq);
+ disable_irq(ctx->dev->enc_irq);
mtk_vcodec_debug_leave(inst);
return ret;
--
2.18.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node
2021-01-21 6:18 [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Irui Wang
2021-01-21 6:18 ` [PATCH 2/3] arm64: dts: mt8173: Separating mtk-vcodec-enc device node Irui Wang
2021-01-21 6:18 ` [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver Irui Wang
@ 2021-01-26 8:42 ` Tiffany Lin
2021-02-03 10:44 ` Alexandre Courbot
2021-02-09 15:53 ` Rob Herring
4 siblings, 0 replies; 13+ messages in thread
From: Tiffany Lin @ 2021-01-26 8:42 UTC (permalink / raw)
To: Irui Wang
Cc: Alexandre Courbot, Hans Verkuil, Andrew-CT Chen,
Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Tomasz Figa, Hsin-Yi Wang, Maoguang Meng, Longfei Wang,
Yunfei Dong, linux-media, devicetree, linux-kernel,
linux-arm-kernel, srv_heupstream, linux-mediatek
On Thu, 2021-01-21 at 14:18 +0800, Irui Wang wrote:
> Updates binding document since the avc and vp8 hardware encoder in
> MT8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
>
Acked-by: Tiffany Lin <tiffany.lin@mediatek.com>
> ---
> .../bindings/media/mediatek-vcodec.txt | 58 ++++++++++---------
> 1 file changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> index 8217424fd4bd..f85276e629bf 100644
> --- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> @@ -4,7 +4,9 @@ Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
> supports high resolution encoding and decoding functionalities.
>
> Required properties:
> -- compatible : "mediatek,mt8173-vcodec-enc" for MT8173 encoder
> +- compatible : must be one of the following string:
> + "mediatek,mt8173-vcodec-vp8-enc" for mt8173 vp8 encoder.
> + "mediatek,mt8173-vcodec-avc-enc" for mt8173 avc encoder.
> "mediatek,mt8183-vcodec-enc" for MT8183 encoder.
> "mediatek,mt8173-vcodec-dec" for MT8173 decoder.
> - reg : Physical base address of the video codec registers and length of
> @@ -13,10 +15,11 @@ Required properties:
> - mediatek,larb : must contain the local arbiters in the current Socs.
> - clocks : list of clock specifiers, corresponding to entries in
> the clock-names property.
> -- clock-names: encoder must contain "venc_sel_src", "venc_sel",,
> - "venc_lt_sel_src", "venc_lt_sel", decoder must contain "vcodecpll",
> - "univpll_d2", "clk_cci400_sel", "vdec_sel", "vdecpll", "vencpll",
> - "venc_lt_sel", "vdec_bus_clk_src".
> +- clock-names:
> + avc venc must contain "venc_sel";
> + vp8 venc must contain "venc_lt_sel";
> + decoder must contain "vcodecpll", "univpll_d2", "clk_cci400_sel",
> + "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src".
> - iommus : should point to the respective IOMMU block with master port as
> argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> for details.
> @@ -80,14 +83,10 @@ vcodec_dec: vcodec@16000000 {
> assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
> };
>
> - vcodec_enc: vcodec@18002000 {
> - compatible = "mediatek,mt8173-vcodec-enc";
> - reg = <0 0x18002000 0 0x1000>, /*VENC_SYS*/
> - <0 0x19002000 0 0x1000>; /*VENC_LT_SYS*/
> - interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> - mediatek,larb = <&larb3>,
> - <&larb5>;
> +vcodec_enc: vcodec@18002000 {
> + compatible = "mediatek,mt8173-vcodec-avc-enc";
> + reg = <0 0x18002000 0 0x1000>;
> + interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
> iommus = <&iommu M4U_PORT_VENC_RCPU>,
> <&iommu M4U_PORT_VENC_REC>,
> <&iommu M4U_PORT_VENC_BSDMA>,
> @@ -98,8 +97,20 @@ vcodec_dec: vcodec@16000000 {
> <&iommu M4U_PORT_VENC_REF_LUMA>,
> <&iommu M4U_PORT_VENC_REF_CHROMA>,
> <&iommu M4U_PORT_VENC_NBM_RDMA>,
> - <&iommu M4U_PORT_VENC_NBM_WDMA>,
> - <&iommu M4U_PORT_VENC_RCPU_SET2>,
> + <&iommu M4U_PORT_VENC_NBM_WDMA>;
> + mediatek,larb = <&larb3>;
> + mediatek,vpu = <&vpu>;
> + clocks = <&topckgen CLK_TOP_VENC_SEL>;
> + clock-names = "venc_sel";
> + assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;
> + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;
> + };
> +
> +vcodec_enc_lt: vcodec@19002000 {
> + compatible = "mediatek,mt8173-vcodec-vp8-enc";
> + reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
> + interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> + iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,
> <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
> <&iommu M4U_PORT_VENC_BSDMA_SET2>,
> <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
> @@ -108,17 +119,10 @@ vcodec_dec: vcodec@16000000 {
> <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
> <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
> <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
> + mediatek,larb = <&larb5>;
> mediatek,vpu = <&vpu>;
> - clocks = <&topckgen CLK_TOP_VENCPLL_D2>,
> - <&topckgen CLK_TOP_VENC_SEL>,
> - <&topckgen CLK_TOP_UNIVPLL1_D2>,
> - <&topckgen CLK_TOP_VENC_LT_SEL>;
> - clock-names = "venc_sel_src",
> - "venc_sel",
> - "venc_lt_sel_src",
> - "venc_lt_sel";
> - assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>,
> - <&topckgen CLK_TOP_VENC_LT_SEL>;
> - assigned-clock-parents = <&topckgen CLK_TOP_VENCPLL_D2>,
> - <&topckgen CLK_TOP_UNIVPLL1_D2>;
> + clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> + clock-names = "venc_lt_sel";
> + assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>;
> };
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] arm64: dts: mt8173: Separating mtk-vcodec-enc device node
2021-01-21 6:18 ` [PATCH 2/3] arm64: dts: mt8173: Separating mtk-vcodec-enc device node Irui Wang
@ 2021-01-26 8:43 ` Tiffany Lin
0 siblings, 0 replies; 13+ messages in thread
From: Tiffany Lin @ 2021-01-26 8:43 UTC (permalink / raw)
To: Irui Wang
Cc: Alexandre Courbot, Hans Verkuil, Andrew-CT Chen,
Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Tomasz Figa, Hsin-Yi Wang, Maoguang Meng, Longfei Wang,
Yunfei Dong, linux-media, devicetree, linux-kernel,
linux-arm-kernel, srv_heupstream, linux-mediatek
On Thu, 2021-01-21 at 14:18 +0800, Irui Wang wrote:
> There are two separate hardware encoder blocks inside MT8173.
> Split the current mtk-vcodec-enc node to match the hardware architecture.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
>
Acked-by: Tiffany Lin <tiffany.lin@mediatek.com>
> ---
> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 60 ++++++++++++------------
> 1 file changed, 31 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index 7fa870e4386a..d667b296c512 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -1459,13 +1459,10 @@
> };
>
> vcodec_enc: vcodec@18002000 {
> - compatible = "mediatek,mt8173-vcodec-enc";
> - reg = <0 0x18002000 0 0x1000>, /* VENC_SYS */
> - <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
> - interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> - mediatek,larb = <&larb3>,
> - <&larb5>;
> + compatible = "mediatek,mt8173-vcodec-avc-enc";
> + reg = <0 0x18002000 0 0x1000>; /* VENC_SYS */
> + interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
> + mediatek,larb = <&larb3>;
> iommus = <&iommu M4U_PORT_VENC_RCPU>,
> <&iommu M4U_PORT_VENC_REC>,
> <&iommu M4U_PORT_VENC_BSDMA>,
> @@ -1476,29 +1473,12 @@
> <&iommu M4U_PORT_VENC_REF_LUMA>,
> <&iommu M4U_PORT_VENC_REF_CHROMA>,
> <&iommu M4U_PORT_VENC_NBM_RDMA>,
> - <&iommu M4U_PORT_VENC_NBM_WDMA>,
> - <&iommu M4U_PORT_VENC_RCPU_SET2>,
> - <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
> - <&iommu M4U_PORT_VENC_BSDMA_SET2>,
> - <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
> - <&iommu M4U_PORT_VENC_RD_COMA_SET2>,
> - <&iommu M4U_PORT_VENC_CUR_LUMA_SET2>,
> - <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
> - <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
> - <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
> + <&iommu M4U_PORT_VENC_NBM_WDMA>;
> mediatek,vpu = <&vpu>;
> - clocks = <&topckgen CLK_TOP_VENCPLL_D2>,
> - <&topckgen CLK_TOP_VENC_SEL>,
> - <&topckgen CLK_TOP_UNIVPLL1_D2>,
> - <&topckgen CLK_TOP_VENC_LT_SEL>;
> - clock-names = "venc_sel_src",
> - "venc_sel",
> - "venc_lt_sel_src",
> - "venc_lt_sel";
> - assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>,
> - <&topckgen CLK_TOP_VENC_LT_SEL>;
> - assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>,
> - <&topckgen CLK_TOP_VCODECPLL_370P5>;
> + clocks = <&topckgen CLK_TOP_VENC_SEL>;
> + clock-names = "venc_sel";
> + assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;
> + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;
> };
>
> jpegdec: jpegdec@18004000 {
> @@ -1530,5 +1510,27 @@
> <&vencltsys CLK_VENCLT_CKE0>;
> clock-names = "apb", "smi";
> };
> +
> + vcodec_enc_lt: vcodec@19002000 {
> + compatible = "mediatek,mt8173-vcodec-vp8-enc";
> + reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
> + interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> + iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,
> + <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
> + <&iommu M4U_PORT_VENC_BSDMA_SET2>,
> + <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
> + <&iommu M4U_PORT_VENC_RD_COMA_SET2>,
> + <&iommu M4U_PORT_VENC_CUR_LUMA_SET2>,
> + <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
> + <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
> + <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
> + mediatek,larb = <&larb5>;
> + mediatek,vpu = <&vpu>;
> + clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> + clock-names = "venc_lt_sel";
> + assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> + assigned-clock-parents =
> + <&topckgen CLK_TOP_VCODECPLL_370P5>;
> + };
> };
> };
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver
2021-01-21 6:18 ` [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver Irui Wang
@ 2021-01-26 8:44 ` Tiffany Lin
2021-02-03 10:44 ` Alexandre Courbot
1 sibling, 0 replies; 13+ messages in thread
From: Tiffany Lin @ 2021-01-26 8:44 UTC (permalink / raw)
To: Irui Wang
Cc: Alexandre Courbot, Hans Verkuil, Andrew-CT Chen,
Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
Tomasz Figa, Hsin-Yi Wang, Maoguang Meng, Longfei Wang,
Yunfei Dong, linux-media, devicetree, linux-kernel,
linux-arm-kernel, srv_heupstream, linux-mediatek
On Thu, 2021-01-21 at 14:18 +0800, Irui Wang wrote:
> MTK H264 Encoder(VENC_SYS) and VP8 Encoder(VENC_LT_SYS) are two
> independent hardware instance. They have their owner interrupt,
> register mapping, and special clocks.
>
> This patch seperates them into two drivers:
> User Call "VIDIOC_QUERYCAP":
> H264 Encoder return driver name "mtk-vcodec-enc";
> VP8 Encoder return driver name "mtk-venc-vp8.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
>
Acked-by: Tiffany Lin <tiffany.lin@mediatek.com>
> ---
> .../platform/mtk-vcodec/mtk_vcodec_drv.h | 10 +-
> .../platform/mtk-vcodec/mtk_vcodec_enc.c | 23 +++-
> .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 121 +++++++-----------
> .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 40 +-----
> .../platform/mtk-vcodec/venc/venc_vp8_if.c | 4 +-
> 5 files changed, 82 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> index 3dd010cba23e..1594edcc706d 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> @@ -19,6 +19,7 @@
> #define MTK_VCODEC_DRV_NAME "mtk_vcodec_drv"
> #define MTK_VCODEC_DEC_NAME "mtk-vcodec-dec"
> #define MTK_VCODEC_ENC_NAME "mtk-vcodec-enc"
> +#define MTK_VENC_VP8_NAME "mtk-venc-vp8"
> #define MTK_PLATFORM_STR "platform:mt8173"
>
> #define MTK_VCODEC_MAX_PLANES 3
> @@ -193,7 +194,6 @@ struct mtk_vcodec_pm {
>
> struct mtk_vcodec_clk venc_clk;
> struct device *larbvenc;
> - struct device *larbvenclt;
> struct device *dev;
> struct mtk_vcodec_dev *mtkdev;
> };
> @@ -311,25 +311,27 @@ enum mtk_chip {
> * @chip: chip this encoder is compatible with
> *
> * @uses_ext: whether the encoder uses the extended firmware messaging format
> - * @has_lt_irq: whether the encoder uses the LT irq
> + * @name: whether the encoder core is vp8
> * @min_birate: minimum supported encoding bitrate
> * @max_bitrate: maximum supported encoding bitrate
> * @capture_formats: array of supported capture formats
> * @num_capture_formats: number of entries in capture_formats
> * @output_formats: array of supported output formats
> * @num_output_formats: number of entries in output_formats
> + * @core_id: stand for h264 or vp8 encode index
> */
> struct mtk_vcodec_enc_pdata {
> enum mtk_chip chip;
>
> bool uses_ext;
> - bool has_lt_irq;
> + const char *name;
> unsigned long min_bitrate;
> unsigned long max_bitrate;
> const struct mtk_video_fmt *capture_formats;
> size_t num_capture_formats;
> const struct mtk_video_fmt *output_formats;
> size_t num_output_formats;
> + int core_id;
> };
>
> #define MTK_ENC_CTX_IS_EXT(ctx) ((ctx)->dev->venc_pdata->uses_ext)
> @@ -361,7 +363,6 @@ struct mtk_vcodec_enc_pdata {
> *
> * @dec_irq: decoder irq resource
> * @enc_irq: h264 encoder irq resource
> - * @enc_lt_irq: vp8 encoder irq resource
> *
> * @dec_mutex: decoder hardware lock
> * @enc_mutex: encoder hardware lock.
> @@ -397,7 +398,6 @@ struct mtk_vcodec_dev {
>
> int dec_irq;
> int enc_irq;
> - int enc_lt_irq;
>
> struct mutex dec_mutex;
> struct mutex enc_mutex;
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> index 21de1431cfcb..0da6871b4b39 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> @@ -9,6 +9,7 @@
> #include <media/v4l2-mem2mem.h>
> #include <media/videobuf2-dma-contig.h>
> #include <soc/mediatek/smi.h>
> +#include <linux/pm_runtime.h>
>
> #include "mtk_vcodec_drv.h"
> #include "mtk_vcodec_enc.h"
> @@ -189,7 +190,10 @@ static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
> static int vidioc_venc_querycap(struct file *file, void *priv,
> struct v4l2_capability *cap)
> {
> - strscpy(cap->driver, MTK_VCODEC_ENC_NAME, sizeof(cap->driver));
> + const struct mtk_vcodec_enc_pdata *pdata =
> + fh_to_ctx(priv)->dev->venc_pdata;
> +
> + strscpy(cap->driver, pdata->name, sizeof(cap->driver));
> strscpy(cap->bus_info, MTK_PLATFORM_STR, sizeof(cap->bus_info));
> strscpy(cap->card, MTK_PLATFORM_STR, sizeof(cap->card));
>
> @@ -797,7 +801,7 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> */
> if ((ctx->state == MTK_STATE_ABORT) || (ctx->state == MTK_STATE_FREE)) {
> ret = -EIO;
> - goto err_set_param;
> + goto err_start_stream;
> }
>
> /* Do the initialization when both start_streaming have been called */
> @@ -809,6 +813,12 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> return 0;
> }
>
> + ret = pm_runtime_get_sync(&ctx->dev->plat_dev->dev);
> + if (ret < 0) {
> + mtk_v4l2_err("pm_runtime_get_sync fail %d", ret);
> + goto err_start_stream;
> + }
> +
> mtk_venc_set_param(ctx, ¶m);
> ret = venc_if_set_param(ctx, VENC_SET_PARAM_ENC, ¶m);
> if (ret) {
> @@ -835,6 +845,11 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> return 0;
>
> err_set_param:
> + ret = pm_runtime_put(&ctx->dev->plat_dev->dev);
> + if (ret < 0)
> + mtk_v4l2_err("pm_runtime_put fail %d", ret);
> +
> +err_start_stream:
> for (i = 0; i < q->num_buffers; ++i) {
> struct vb2_buffer *buf = vb2_get_buffer(q, i);
>
> @@ -888,6 +903,10 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
> if (ret)
> mtk_v4l2_err("venc_if_deinit failed=%d", ret);
>
> + ret = pm_runtime_put(&ctx->dev->plat_dev->dev);
> + if (ret < 0)
> + mtk_v4l2_err("pm_runtime_put fail %d", ret);
> +
> ctx->state = MTK_STATE_FREE;
> }
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> index dfb42e19bf81..4bee42454253 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> @@ -49,12 +49,15 @@ static const struct mtk_video_fmt mtk_video_formats_output_mt8173[] = {
> },
> };
>
> -static const struct mtk_video_fmt mtk_video_formats_capture_mt8173[] = {
> +static const struct mtk_video_fmt mtk_video_formats_capture_mt8173_h264[] = {
> {
> .fourcc = V4L2_PIX_FMT_H264,
> .type = MTK_FMT_ENC,
> .num_planes = 1,
> },
> +};
> +
> +static const struct mtk_video_fmt mtk_video_formats_capture_mt8173_vp8[] = {
> {
> .fourcc = V4L2_PIX_FMT_VP8,
> .type = MTK_FMT_ENC,
> @@ -110,35 +113,13 @@ static irqreturn_t mtk_vcodec_enc_irq_handler(int irq, void *priv)
> ctx = dev->curr_ctx;
> spin_unlock_irqrestore(&dev->irqlock, flags);
>
> - mtk_v4l2_debug(1, "id=%d", ctx->id);
> - addr = dev->reg_base[VENC_SYS] + MTK_VENC_IRQ_ACK_OFFSET;
> -
> - ctx->irq_status = readl(dev->reg_base[VENC_SYS] +
> - (MTK_VENC_IRQ_STATUS_OFFSET));
> -
> - clean_irq_status(ctx->irq_status, addr);
> -
> - wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED);
> - return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t mtk_vcodec_enc_lt_irq_handler(int irq, void *priv)
> -{
> - struct mtk_vcodec_dev *dev = priv;
> - struct mtk_vcodec_ctx *ctx;
> - unsigned long flags;
> - void __iomem *addr;
> -
> - spin_lock_irqsave(&dev->irqlock, flags);
> - ctx = dev->curr_ctx;
> - spin_unlock_irqrestore(&dev->irqlock, flags);
> + mtk_v4l2_debug(1, "id=%d coreid:%d", ctx->id, dev->venc_pdata->core_id);
> + addr = dev->reg_base[dev->venc_pdata->core_id] +
> + MTK_VENC_IRQ_ACK_OFFSET;
>
> - mtk_v4l2_debug(1, "id=%d", ctx->id);
> - ctx->irq_status = readl(dev->reg_base[VENC_LT_SYS] +
> + ctx->irq_status = readl(dev->reg_base[dev->venc_pdata->core_id] +
> (MTK_VENC_IRQ_STATUS_OFFSET));
>
> - addr = dev->reg_base[VENC_LT_SYS] + MTK_VENC_IRQ_ACK_OFFSET;
> -
> clean_irq_status(ctx->irq_status, addr);
>
> wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED);
> @@ -293,17 +274,21 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> dev->venc_pdata = of_device_get_match_data(&pdev->dev);
> ret = mtk_vcodec_init_enc_pm(dev);
> if (ret < 0) {
> - dev_err(&pdev->dev, "Failed to get mt vcodec clock source!");
> + dev_err(&pdev->dev, "Failed to get mtk vcodec clock source!");
> goto err_enc_pm;
> }
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - dev->reg_base[VENC_SYS] = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR((__force void *)dev->reg_base[VENC_SYS])) {
> - ret = PTR_ERR((__force void *)dev->reg_base[VENC_SYS]);
> + pm_runtime_enable(&pdev->dev);
> +
> + snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
> + dev->venc_pdata->name);
> +
> + dev->reg_base[dev->venc_pdata->core_id] =
> + devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(dev->reg_base[dev->venc_pdata->core_id])) {
> + ret = PTR_ERR(dev->reg_base[dev->venc_pdata->core_id]);
> goto err_res;
> }
> - mtk_v4l2_debug(2, "reg[%d] base=0x%p", i, dev->reg_base[VENC_SYS]);
>
> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> if (res == NULL) {
> @@ -318,44 +303,17 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> mtk_vcodec_enc_irq_handler,
> 0, pdev->name, dev);
> if (ret) {
> - dev_err(&pdev->dev, "Failed to install dev->enc_irq %d (%d)",
> - dev->enc_irq,
> - ret);
> + dev_err(&pdev->dev,
> + "Failed to install dev->enc_irq %d (%d) core_id:%d",
> + dev->enc_irq, ret, dev->venc_pdata->core_id);
> ret = -EINVAL;
> goto err_res;
> }
>
> - if (dev->venc_pdata->has_lt_irq) {
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - dev->reg_base[VENC_LT_SYS] = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR((__force void *)dev->reg_base[VENC_LT_SYS])) {
> - ret = PTR_ERR((__force void *)dev->reg_base[VENC_LT_SYS]);
> - goto err_res;
> - }
> - mtk_v4l2_debug(2, "reg[%d] base=0x%p", i, dev->reg_base[VENC_LT_SYS]);
> -
> - dev->enc_lt_irq = platform_get_irq(pdev, 1);
> - irq_set_status_flags(dev->enc_lt_irq, IRQ_NOAUTOEN);
> - ret = devm_request_irq(&pdev->dev,
> - dev->enc_lt_irq,
> - mtk_vcodec_enc_lt_irq_handler,
> - 0, pdev->name, dev);
> - if (ret) {
> - dev_err(&pdev->dev,
> - "Failed to install dev->enc_lt_irq %d (%d)",
> - dev->enc_lt_irq, ret);
> - ret = -EINVAL;
> - goto err_res;
> - }
> - }
> -
> mutex_init(&dev->enc_mutex);
> mutex_init(&dev->dev_mutex);
> spin_lock_init(&dev->irqlock);
>
> - snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
> - "[MTK_V4L2_VENC]");
> -
> ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> if (ret) {
> mtk_v4l2_err("v4l2_device_register err=%d", ret);
> @@ -381,7 +339,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> V4L2_CAP_STREAMING;
>
> snprintf(vfd_enc->name, sizeof(vfd_enc->name), "%s",
> - MTK_VCODEC_ENC_NAME);
> + dev->venc_pdata->name);
> video_set_drvdata(vfd_enc, dev);
> dev->vfd_enc = vfd_enc;
> platform_set_drvdata(pdev, dev);
> @@ -409,8 +367,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> goto err_enc_reg;
> }
>
> - mtk_v4l2_debug(0, "encoder registered as /dev/video%d",
> - vfd_enc->num);
> + mtk_v4l2_debug(0, "encoder %d registered as /dev/video%d",
> + dev->venc_pdata->core_id, vfd_enc->num);
>
> return 0;
>
> @@ -429,20 +387,33 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> return ret;
> }
>
> -static const struct mtk_vcodec_enc_pdata mt8173_pdata = {
> +static const struct mtk_vcodec_enc_pdata mt8173_avc_pdata = {
> .chip = MTK_MT8173,
> - .has_lt_irq = true,
> - .capture_formats = mtk_video_formats_capture_mt8173,
> - .num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_mt8173),
> + .name = MTK_VCODEC_ENC_NAME,
> + .capture_formats = mtk_video_formats_capture_mt8173_h264,
> + .num_capture_formats = 1,
> .output_formats = mtk_video_formats_output_mt8173,
> .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> - .min_bitrate = 1,
> + .min_bitrate = 64,
> + .max_bitrate = 4000000,
> + .core_id = VENC_SYS,
> +};
> +
> +static const struct mtk_vcodec_enc_pdata mt8173_vp8_pdata = {
> + .chip = MTK_MT8173,
> + .name = MTK_VENC_VP8_NAME,
> + .capture_formats = mtk_video_formats_capture_mt8173_vp8,
> + .num_capture_formats = 1,
> + .output_formats = mtk_video_formats_output_mt8173,
> + .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> + .min_bitrate = 64,
> .max_bitrate = 4000000,
> + .core_id = VENC_LT_SYS,
> };
>
> static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
> .chip = MTK_MT8183,
> - .has_lt_irq = false,
> + .name = MTK_VCODEC_ENC_NAME,
> .uses_ext = true,
> .capture_formats = mtk_video_formats_capture_mt8183,
> .num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_mt8183),
> @@ -451,10 +422,14 @@ static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
> .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> .min_bitrate = 64,
> .max_bitrate = 40000000,
> + .core_id = VENC_SYS,
> };
>
> static const struct of_device_id mtk_vcodec_enc_match[] = {
> - {.compatible = "mediatek,mt8173-vcodec-enc", .data = &mt8173_pdata},
> + {.compatible = "mediatek,mt8173-vcodec-avc-enc",
> + .data = &mt8173_avc_pdata},
> + {.compatible = "mediatek,mt8173-vcodec-vp8-enc",
> + .data = &mt8173_vp8_pdata},
> {.compatible = "mediatek,mt8183-vcodec-enc", .data = &mt8183_pdata},
> {},
> };
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> index 3b7c54d6aa8f..1b2e4930ed27 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> @@ -43,23 +43,6 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> return -ENODEV;
> }
> pm->larbvenc = &pdev->dev;
> -
> - node = of_parse_phandle(dev->of_node, "mediatek,larb", 1);
> - if (!node) {
> - mtk_v4l2_err("no mediatek,larb found");
> - ret = -ENODEV;
> - goto put_larbvenc;
> - }
> -
> - pdev = of_find_device_by_node(node);
> - of_node_put(node);
> - if (!pdev) {
> - mtk_v4l2_err("no mediatek,larb device found");
> - ret = -ENODEV;
> - goto put_larbvenc;
> - }
> -
> - pm->larbvenclt = &pdev->dev;
> pdev = mtkdev->plat_dev;
> pm->dev = &pdev->dev;
>
> @@ -71,12 +54,12 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> GFP_KERNEL);
> if (!enc_clk->clk_info) {
> ret = -ENOMEM;
> - goto put_larbvenclt;
> + goto put_larbvenc;
> }
> } else {
> mtk_v4l2_err("Failed to get venc clock count");
> ret = -EINVAL;
> - goto put_larbvenclt;
> + goto put_larbvenc;
> }
>
> for (i = 0; i < enc_clk->clk_num; i++) {
> @@ -85,7 +68,7 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> "clock-names", i, &clk_info->clk_name);
> if (ret) {
> mtk_v4l2_err("venc failed to get clk name %d", i);
> - goto put_larbvenclt;
> + goto put_larbvenc;
> }
> clk_info->vcodec_clk = devm_clk_get(&pdev->dev,
> clk_info->clk_name);
> @@ -93,14 +76,12 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> mtk_v4l2_err("venc devm_clk_get (%d)%s fail", i,
> clk_info->clk_name);
> ret = PTR_ERR(clk_info->vcodec_clk);
> - goto put_larbvenclt;
> + goto put_larbvenc;
> }
> }
>
> return 0;
>
> -put_larbvenclt:
> - put_device(pm->larbvenclt);
> put_larbvenc:
> put_device(pm->larbvenc);
> return ret;
> @@ -108,7 +89,7 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
>
> void mtk_vcodec_release_enc_pm(struct mtk_vcodec_dev *mtkdev)
> {
> - put_device(mtkdev->pm.larbvenclt);
> + pm_runtime_disable(mtkdev->pm.dev);
> put_device(mtkdev->pm.larbvenc);
> }
>
> @@ -130,18 +111,10 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm)
> ret = mtk_smi_larb_get(pm->larbvenc);
> if (ret) {
> mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret);
> - goto larbvencerr;
> - }
> - ret = mtk_smi_larb_get(pm->larbvenclt);
> - if (ret) {
> - mtk_v4l2_err("mtk_smi_larb_get larb4 fail %d", ret);
> - goto larbvenclterr;
> + goto clkerr;
> }
> return;
>
> -larbvenclterr:
> - mtk_smi_larb_put(pm->larbvenc);
> -larbvencerr:
> clkerr:
> for (i -= 1; i >= 0; i--)
> clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
> @@ -153,7 +126,6 @@ void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm)
> int i = 0;
>
> mtk_smi_larb_put(pm->larbvenc);
> - mtk_smi_larb_put(pm->larbvenclt);
> for (i = enc_clk->clk_num - 1; i >= 0; i--)
> clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
> }
> diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> index 11abb191ada5..8267a9c4fd25 100644
> --- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> @@ -367,7 +367,7 @@ static int vp8_enc_encode(void *handle,
>
> mtk_vcodec_debug_enter(inst);
>
> - enable_irq(ctx->dev->enc_lt_irq);
> + enable_irq(ctx->dev->enc_irq);
>
> switch (opt) {
> case VENC_START_OPT_ENCODE_FRAME:
> @@ -386,7 +386,7 @@ static int vp8_enc_encode(void *handle,
>
> encode_err:
>
> - disable_irq(ctx->dev->enc_lt_irq);
> + disable_irq(ctx->dev->enc_irq);
> mtk_vcodec_debug_leave(inst);
>
> return ret;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node
2021-01-21 6:18 [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Irui Wang
` (2 preceding siblings ...)
2021-01-26 8:42 ` [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Tiffany Lin
@ 2021-02-03 10:44 ` Alexandre Courbot
2021-02-20 6:26 ` Irui Wang
2021-02-09 15:53 ` Rob Herring
4 siblings, 1 reply; 13+ messages in thread
From: Alexandre Courbot @ 2021-02-03 10:44 UTC (permalink / raw)
To: Irui Wang
Cc: Hans Verkuil, Tiffany Lin, Andrew-CT Chen, Mauro Carvalho Chehab,
Rob Herring, Matthias Brugger, Tomasz Figa, Hsin-Yi Wang,
Maoguang Meng, Longfei Wang, Yunfei Dong,
Linux Media Mailing List, devicetree, LKML,
moderated list:ARM/Mediatek SoC support, srv_heupstream,
moderated list:ARM/Mediatek SoC support
On Thu, Jan 21, 2021 at 3:18 PM Irui Wang <irui.wang@mediatek.com> wrote:
>
> Updates binding document since the avc and vp8 hardware encoder in
> MT8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
>
> ---
> .../bindings/media/mediatek-vcodec.txt | 58 ++++++++++---------
> 1 file changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> index 8217424fd4bd..f85276e629bf 100644
> --- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> @@ -4,7 +4,9 @@ Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
> supports high resolution encoding and decoding functionalities.
>
> Required properties:
> -- compatible : "mediatek,mt8173-vcodec-enc" for MT8173 encoder
> +- compatible : must be one of the following string:
> + "mediatek,mt8173-vcodec-vp8-enc" for mt8173 vp8 encoder.
> + "mediatek,mt8173-vcodec-avc-enc" for mt8173 avc encoder.
IMHO "mediatek,mt8173-vcodec-enc-vp8" and
"mediatek,mt8173-vcodec-enc-avc" would be more logical. Also to keep a
bit of backward compatibility, shall we also allow
"mediatek,mt8173-vcodec-enc" to be an alias for
"mediatek,mt8173-vcodec-enc-avc"? The line above would become
"mediatek,mt8173-vcodec-enc-avc" or "mediatek,mt8173-vcodec-enc" for
mt8173 avc encoder.
> "mediatek,mt8183-vcodec-enc" for MT8183 encoder.
> "mediatek,mt8173-vcodec-dec" for MT8173 decoder.
> - reg : Physical base address of the video codec registers and length of
> @@ -13,10 +15,11 @@ Required properties:
> - mediatek,larb : must contain the local arbiters in the current Socs.
> - clocks : list of clock specifiers, corresponding to entries in
> the clock-names property.
> -- clock-names: encoder must contain "venc_sel_src", "venc_sel",,
> - "venc_lt_sel_src", "venc_lt_sel", decoder must contain "vcodecpll",
> - "univpll_d2", "clk_cci400_sel", "vdec_sel", "vdecpll", "vencpll",
> - "venc_lt_sel", "vdec_bus_clk_src".
> +- clock-names:
> + avc venc must contain "venc_sel";
> + vp8 venc must contain "venc_lt_sel";
Can't we use "venc_sel" for both avc and vp8, since they are different
nodes now? That way we can just say
encoder must contain "venc_sel"
which is clearer and also simpler on the code side.
> + decoder must contain "vcodecpll", "univpll_d2", "clk_cci400_sel",
> + "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src".
> - iommus : should point to the respective IOMMU block with master port as
> argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> for details.
> @@ -80,14 +83,10 @@ vcodec_dec: vcodec@16000000 {
> assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
> };
>
> - vcodec_enc: vcodec@18002000 {
> - compatible = "mediatek,mt8173-vcodec-enc";
> - reg = <0 0x18002000 0 0x1000>, /*VENC_SYS*/
> - <0 0x19002000 0 0x1000>; /*VENC_LT_SYS*/
> - interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> - mediatek,larb = <&larb3>,
> - <&larb5>;
> +vcodec_enc: vcodec@18002000 {
Let's use vcodec_enc_avc as a label?
> + compatible = "mediatek,mt8173-vcodec-avc-enc";
> + reg = <0 0x18002000 0 0x1000>;
> + interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
> iommus = <&iommu M4U_PORT_VENC_RCPU>,
> <&iommu M4U_PORT_VENC_REC>,
> <&iommu M4U_PORT_VENC_BSDMA>,
> @@ -98,8 +97,20 @@ vcodec_dec: vcodec@16000000 {
> <&iommu M4U_PORT_VENC_REF_LUMA>,
> <&iommu M4U_PORT_VENC_REF_CHROMA>,
> <&iommu M4U_PORT_VENC_NBM_RDMA>,
> - <&iommu M4U_PORT_VENC_NBM_WDMA>,
> - <&iommu M4U_PORT_VENC_RCPU_SET2>,
> + <&iommu M4U_PORT_VENC_NBM_WDMA>;
> + mediatek,larb = <&larb3>;
> + mediatek,vpu = <&vpu>;
> + clocks = <&topckgen CLK_TOP_VENC_SEL>;
> + clock-names = "venc_sel";
> + assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;
> + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;
> + };
> +
> +vcodec_enc_lt: vcodec@19002000 {
And here the label should probably be "vcodec_enc_vp8" for consistency.
> + compatible = "mediatek,mt8173-vcodec-vp8-enc";
> + reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
> + interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> + iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,
> <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
> <&iommu M4U_PORT_VENC_BSDMA_SET2>,
> <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
> @@ -108,17 +119,10 @@ vcodec_dec: vcodec@16000000 {
> <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
> <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
> <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
> + mediatek,larb = <&larb5>;
> mediatek,vpu = <&vpu>;
> - clocks = <&topckgen CLK_TOP_VENCPLL_D2>,
> - <&topckgen CLK_TOP_VENC_SEL>,
> - <&topckgen CLK_TOP_UNIVPLL1_D2>,
> - <&topckgen CLK_TOP_VENC_LT_SEL>;
> - clock-names = "venc_sel_src",
> - "venc_sel",
> - "venc_lt_sel_src",
> - "venc_lt_sel";
> - assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>,
> - <&topckgen CLK_TOP_VENC_LT_SEL>;
> - assigned-clock-parents = <&topckgen CLK_TOP_VENCPLL_D2>,
> - <&topckgen CLK_TOP_UNIVPLL1_D2>;
> + clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> + clock-names = "venc_lt_sel";
> + assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>;
> };
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver
2021-01-21 6:18 ` [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver Irui Wang
2021-01-26 8:44 ` Tiffany Lin
@ 2021-02-03 10:44 ` Alexandre Courbot
2021-02-20 6:55 ` Irui Wang
1 sibling, 1 reply; 13+ messages in thread
From: Alexandre Courbot @ 2021-02-03 10:44 UTC (permalink / raw)
To: Irui Wang
Cc: Hans Verkuil, Tiffany Lin, Andrew-CT Chen, Mauro Carvalho Chehab,
Rob Herring, Matthias Brugger, Tomasz Figa, Hsin-Yi Wang,
Maoguang Meng, Longfei Wang, Yunfei Dong,
Linux Media Mailing List, devicetree, LKML,
moderated list:ARM/Mediatek SoC support, srv_heupstream,
moderated list:ARM/Mediatek SoC support
Hi Irui,
Thanks for pushing this forward. I had two small conflicts when
applying this patch to the media tree, so you may want to rebase
before sending the next version. Please see the comments inline.
On Thu, Jan 21, 2021 at 3:18 PM Irui Wang <irui.wang@mediatek.com> wrote:
>
> MTK H264 Encoder(VENC_SYS) and VP8 Encoder(VENC_LT_SYS) are two
> independent hardware instance. They have their owner interrupt,
> register mapping, and special clocks.
>
> This patch seperates them into two drivers:
seperates -> separates
Also the patch does not result in two drivers, but two devices.
> User Call "VIDIOC_QUERYCAP":
> H264 Encoder return driver name "mtk-vcodec-enc";
> VP8 Encoder return driver name "mtk-venc-vp8.
I wonder if we need to use two different names? The driver is the
same, so it makes sense to me that both devices return
"mtk-vcodec-enc". Userspace can then list the formats on the CAPTURE
queue in order to query the supported codecs.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
>
> ---
> .../platform/mtk-vcodec/mtk_vcodec_drv.h | 10 +-
> .../platform/mtk-vcodec/mtk_vcodec_enc.c | 23 +++-
> .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 121 +++++++-----------
> .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 40 +-----
> .../platform/mtk-vcodec/venc/venc_vp8_if.c | 4 +-
> 5 files changed, 82 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> index 3dd010cba23e..1594edcc706d 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> @@ -19,6 +19,7 @@
> #define MTK_VCODEC_DRV_NAME "mtk_vcodec_drv"
> #define MTK_VCODEC_DEC_NAME "mtk-vcodec-dec"
> #define MTK_VCODEC_ENC_NAME "mtk-vcodec-enc"
> +#define MTK_VENC_VP8_NAME "mtk-venc-vp8"
> #define MTK_PLATFORM_STR "platform:mt8173"
>
> #define MTK_VCODEC_MAX_PLANES 3
> @@ -193,7 +194,6 @@ struct mtk_vcodec_pm {
>
> struct mtk_vcodec_clk venc_clk;
> struct device *larbvenc;
> - struct device *larbvenclt;
> struct device *dev;
> struct mtk_vcodec_dev *mtkdev;
> };
> @@ -311,25 +311,27 @@ enum mtk_chip {
> * @chip: chip this encoder is compatible with
> *
> * @uses_ext: whether the encoder uses the extended firmware messaging format
> - * @has_lt_irq: whether the encoder uses the LT irq
> + * @name: whether the encoder core is vp8
> * @min_birate: minimum supported encoding bitrate
> * @max_bitrate: maximum supported encoding bitrate
> * @capture_formats: array of supported capture formats
> * @num_capture_formats: number of entries in capture_formats
> * @output_formats: array of supported output formats
> * @num_output_formats: number of entries in output_formats
> + * @core_id: stand for h264 or vp8 encode index
> */
> struct mtk_vcodec_enc_pdata {
> enum mtk_chip chip;
>
> bool uses_ext;
> - bool has_lt_irq;
> + const char *name;
This new member can be removed if we use the same name for both devices.
> unsigned long min_bitrate;
> unsigned long max_bitrate;
> const struct mtk_video_fmt *capture_formats;
> size_t num_capture_formats;
> const struct mtk_video_fmt *output_formats;
> size_t num_output_formats;
> + int core_id;
> };
>
> #define MTK_ENC_CTX_IS_EXT(ctx) ((ctx)->dev->venc_pdata->uses_ext)
> @@ -361,7 +363,6 @@ struct mtk_vcodec_enc_pdata {
> *
> * @dec_irq: decoder irq resource
> * @enc_irq: h264 encoder irq resource
> - * @enc_lt_irq: vp8 encoder irq resource
> *
> * @dec_mutex: decoder hardware lock
> * @enc_mutex: encoder hardware lock.
> @@ -397,7 +398,6 @@ struct mtk_vcodec_dev {
>
> int dec_irq;
> int enc_irq;
> - int enc_lt_irq;
>
> struct mutex dec_mutex;
> struct mutex enc_mutex;
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> index 21de1431cfcb..0da6871b4b39 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> @@ -9,6 +9,7 @@
> #include <media/v4l2-mem2mem.h>
> #include <media/videobuf2-dma-contig.h>
> #include <soc/mediatek/smi.h>
> +#include <linux/pm_runtime.h>
>
> #include "mtk_vcodec_drv.h"
> #include "mtk_vcodec_enc.h"
> @@ -189,7 +190,10 @@ static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
> static int vidioc_venc_querycap(struct file *file, void *priv,
> struct v4l2_capability *cap)
> {
> - strscpy(cap->driver, MTK_VCODEC_ENC_NAME, sizeof(cap->driver));
> + const struct mtk_vcodec_enc_pdata *pdata =
> + fh_to_ctx(priv)->dev->venc_pdata;
> +
> + strscpy(cap->driver, pdata->name, sizeof(cap->driver));
> strscpy(cap->bus_info, MTK_PLATFORM_STR, sizeof(cap->bus_info));
> strscpy(cap->card, MTK_PLATFORM_STR, sizeof(cap->card));
>
> @@ -797,7 +801,7 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> */
> if ((ctx->state == MTK_STATE_ABORT) || (ctx->state == MTK_STATE_FREE)) {
> ret = -EIO;
> - goto err_set_param;
> + goto err_start_stream;
> }
>
> /* Do the initialization when both start_streaming have been called */
> @@ -809,6 +813,12 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> return 0;
> }
>
> + ret = pm_runtime_get_sync(&ctx->dev->plat_dev->dev);
> + if (ret < 0) {
> + mtk_v4l2_err("pm_runtime_get_sync fail %d", ret);
> + goto err_start_stream;
> + }
This does not seem to be related to the split ; why is this necessary?
> +
> mtk_venc_set_param(ctx, ¶m);
> ret = venc_if_set_param(ctx, VENC_SET_PARAM_ENC, ¶m);
> if (ret) {
> @@ -835,6 +845,11 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> return 0;
>
> err_set_param:
> + ret = pm_runtime_put(&ctx->dev->plat_dev->dev);
> + if (ret < 0)
> + mtk_v4l2_err("pm_runtime_put fail %d", ret);
> +
> +err_start_stream:
> for (i = 0; i < q->num_buffers; ++i) {
> struct vb2_buffer *buf = vb2_get_buffer(q, i);
>
> @@ -888,6 +903,10 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
> if (ret)
> mtk_v4l2_err("venc_if_deinit failed=%d", ret);
>
> + ret = pm_runtime_put(&ctx->dev->plat_dev->dev);
> + if (ret < 0)
> + mtk_v4l2_err("pm_runtime_put fail %d", ret);
> +
> ctx->state = MTK_STATE_FREE;
> }
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> index dfb42e19bf81..4bee42454253 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> @@ -49,12 +49,15 @@ static const struct mtk_video_fmt mtk_video_formats_output_mt8173[] = {
> },
> };
>
> -static const struct mtk_video_fmt mtk_video_formats_capture_mt8173[] = {
> +static const struct mtk_video_fmt mtk_video_formats_capture_mt8173_h264[] = {
> {
> .fourcc = V4L2_PIX_FMT_H264,
> .type = MTK_FMT_ENC,
> .num_planes = 1,
> },
> +};
> +
> +static const struct mtk_video_fmt mtk_video_formats_capture_mt8173_vp8[] = {
> {
> .fourcc = V4L2_PIX_FMT_VP8,
> .type = MTK_FMT_ENC,
> @@ -110,35 +113,13 @@ static irqreturn_t mtk_vcodec_enc_irq_handler(int irq, void *priv)
> ctx = dev->curr_ctx;
> spin_unlock_irqrestore(&dev->irqlock, flags);
>
> - mtk_v4l2_debug(1, "id=%d", ctx->id);
> - addr = dev->reg_base[VENC_SYS] + MTK_VENC_IRQ_ACK_OFFSET;
> -
> - ctx->irq_status = readl(dev->reg_base[VENC_SYS] +
> - (MTK_VENC_IRQ_STATUS_OFFSET));
> -
> - clean_irq_status(ctx->irq_status, addr);
> -
> - wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED);
> - return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t mtk_vcodec_enc_lt_irq_handler(int irq, void *priv)
> -{
> - struct mtk_vcodec_dev *dev = priv;
> - struct mtk_vcodec_ctx *ctx;
> - unsigned long flags;
> - void __iomem *addr;
> -
> - spin_lock_irqsave(&dev->irqlock, flags);
> - ctx = dev->curr_ctx;
> - spin_unlock_irqrestore(&dev->irqlock, flags);
> + mtk_v4l2_debug(1, "id=%d coreid:%d", ctx->id, dev->venc_pdata->core_id);
> + addr = dev->reg_base[dev->venc_pdata->core_id] +
> + MTK_VENC_IRQ_ACK_OFFSET;
>
> - mtk_v4l2_debug(1, "id=%d", ctx->id);
> - ctx->irq_status = readl(dev->reg_base[VENC_LT_SYS] +
> + ctx->irq_status = readl(dev->reg_base[dev->venc_pdata->core_id] +
> (MTK_VENC_IRQ_STATUS_OFFSET));
>
> - addr = dev->reg_base[VENC_LT_SYS] + MTK_VENC_IRQ_ACK_OFFSET;
> -
> clean_irq_status(ctx->irq_status, addr);
>
> wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED);
> @@ -293,17 +274,21 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> dev->venc_pdata = of_device_get_match_data(&pdev->dev);
> ret = mtk_vcodec_init_enc_pm(dev);
> if (ret < 0) {
> - dev_err(&pdev->dev, "Failed to get mt vcodec clock source!");
> + dev_err(&pdev->dev, "Failed to get mtk vcodec clock source!");
> goto err_enc_pm;
> }
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - dev->reg_base[VENC_SYS] = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR((__force void *)dev->reg_base[VENC_SYS])) {
> - ret = PTR_ERR((__force void *)dev->reg_base[VENC_SYS]);
> + pm_runtime_enable(&pdev->dev);
> +
> + snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
> + dev->venc_pdata->name);
> +
> + dev->reg_base[dev->venc_pdata->core_id] =
> + devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(dev->reg_base[dev->venc_pdata->core_id])) {
> + ret = PTR_ERR(dev->reg_base[dev->venc_pdata->core_id]);
> goto err_res;
> }
> - mtk_v4l2_debug(2, "reg[%d] base=0x%p", i, dev->reg_base[VENC_SYS]);
>
> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> if (res == NULL) {
> @@ -318,44 +303,17 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> mtk_vcodec_enc_irq_handler,
> 0, pdev->name, dev);
> if (ret) {
> - dev_err(&pdev->dev, "Failed to install dev->enc_irq %d (%d)",
> - dev->enc_irq,
> - ret);
> + dev_err(&pdev->dev,
> + "Failed to install dev->enc_irq %d (%d) core_id:%d",
> + dev->enc_irq, ret, dev->venc_pdata->core_id);
> ret = -EINVAL;
> goto err_res;
> }
>
> - if (dev->venc_pdata->has_lt_irq) {
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - dev->reg_base[VENC_LT_SYS] = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR((__force void *)dev->reg_base[VENC_LT_SYS])) {
> - ret = PTR_ERR((__force void *)dev->reg_base[VENC_LT_SYS]);
> - goto err_res;
> - }
> - mtk_v4l2_debug(2, "reg[%d] base=0x%p", i, dev->reg_base[VENC_LT_SYS]);
> -
> - dev->enc_lt_irq = platform_get_irq(pdev, 1);
> - irq_set_status_flags(dev->enc_lt_irq, IRQ_NOAUTOEN);
> - ret = devm_request_irq(&pdev->dev,
> - dev->enc_lt_irq,
> - mtk_vcodec_enc_lt_irq_handler,
> - 0, pdev->name, dev);
> - if (ret) {
> - dev_err(&pdev->dev,
> - "Failed to install dev->enc_lt_irq %d (%d)",
> - dev->enc_lt_irq, ret);
> - ret = -EINVAL;
> - goto err_res;
> - }
> - }
> -
> mutex_init(&dev->enc_mutex);
> mutex_init(&dev->dev_mutex);
> spin_lock_init(&dev->irqlock);
>
> - snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
> - "[MTK_V4L2_VENC]");
> -
> ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> if (ret) {
> mtk_v4l2_err("v4l2_device_register err=%d", ret);
> @@ -381,7 +339,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> V4L2_CAP_STREAMING;
>
> snprintf(vfd_enc->name, sizeof(vfd_enc->name), "%s",
> - MTK_VCODEC_ENC_NAME);
> + dev->venc_pdata->name);
Let's keep the indendation at the same level as before (if we use the
same driver name for both devices this change is unnecessary though).
> video_set_drvdata(vfd_enc, dev);
> dev->vfd_enc = vfd_enc;
> platform_set_drvdata(pdev, dev);
> @@ -409,8 +367,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> goto err_enc_reg;
> }
>
> - mtk_v4l2_debug(0, "encoder registered as /dev/video%d",
> - vfd_enc->num);
> + mtk_v4l2_debug(0, "encoder %d registered as /dev/video%d",
> + dev->venc_pdata->core_id, vfd_enc->num);
>
> return 0;
>
> @@ -429,20 +387,33 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> return ret;
> }
>
> -static const struct mtk_vcodec_enc_pdata mt8173_pdata = {
> +static const struct mtk_vcodec_enc_pdata mt8173_avc_pdata = {
> .chip = MTK_MT8173,
> - .has_lt_irq = true,
> - .capture_formats = mtk_video_formats_capture_mt8173,
> - .num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_mt8173),
> + .name = MTK_VCODEC_ENC_NAME,
> + .capture_formats = mtk_video_formats_capture_mt8173_h264,
> + .num_capture_formats = 1,
Can't this be ARRAY_SIZE(mtk_video_formats_capture_mt8173_h264)
> .output_formats = mtk_video_formats_output_mt8173,
> .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> - .min_bitrate = 1,
> + .min_bitrate = 64,
I'm fine with changing the value of min_bitrate, but this should be a
separate patch.
> + .max_bitrate = 4000000,
> + .core_id = VENC_SYS,
> +};
> +
> +static const struct mtk_vcodec_enc_pdata mt8173_vp8_pdata = {
> + .chip = MTK_MT8173,
> + .name = MTK_VENC_VP8_NAME,
> + .capture_formats = mtk_video_formats_capture_mt8173_vp8,
> + .num_capture_formats = 1,
Same here, ARRAY_SIZE(mtk_video_formats_capture_mt8173_vp8)
> + .output_formats = mtk_video_formats_output_mt8173,
> + .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> + .min_bitrate = 64,
> .max_bitrate = 4000000,
> + .core_id = VENC_LT_SYS,
> };
>
> static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
> .chip = MTK_MT8183,
> - .has_lt_irq = false,
> + .name = MTK_VCODEC_ENC_NAME,
> .uses_ext = true,
> .capture_formats = mtk_video_formats_capture_mt8183,
> .num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_mt8183),
> @@ -451,10 +422,14 @@ static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
> .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> .min_bitrate = 64,
> .max_bitrate = 40000000,
> + .core_id = VENC_SYS,
> };
>
> static const struct of_device_id mtk_vcodec_enc_match[] = {
> - {.compatible = "mediatek,mt8173-vcodec-enc", .data = &mt8173_pdata},
> + {.compatible = "mediatek,mt8173-vcodec-avc-enc",
> + .data = &mt8173_avc_pdata},
For backward compatibility let's also match
"mediatek,mt8173-vcodec-enc" against this platform data.
> + {.compatible = "mediatek,mt8173-vcodec-vp8-enc",
> + .data = &mt8173_vp8_pdata},
> {.compatible = "mediatek,mt8183-vcodec-enc", .data = &mt8183_pdata},
> {},
> };
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> index 3b7c54d6aa8f..1b2e4930ed27 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> @@ -43,23 +43,6 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> return -ENODEV;
> }
> pm->larbvenc = &pdev->dev;
> -
> - node = of_parse_phandle(dev->of_node, "mediatek,larb", 1);
> - if (!node) {
> - mtk_v4l2_err("no mediatek,larb found");
> - ret = -ENODEV;
> - goto put_larbvenc;
> - }
> -
> - pdev = of_find_device_by_node(node);
> - of_node_put(node);
> - if (!pdev) {
> - mtk_v4l2_err("no mediatek,larb device found");
> - ret = -ENODEV;
> - goto put_larbvenc;
> - }
> -
> - pm->larbvenclt = &pdev->dev;
> pdev = mtkdev->plat_dev;
> pm->dev = &pdev->dev;
>
> @@ -71,12 +54,12 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> GFP_KERNEL);
> if (!enc_clk->clk_info) {
> ret = -ENOMEM;
> - goto put_larbvenclt;
> + goto put_larbvenc;
> }
> } else {
> mtk_v4l2_err("Failed to get venc clock count");
> ret = -EINVAL;
> - goto put_larbvenclt;
> + goto put_larbvenc;
> }
>
> for (i = 0; i < enc_clk->clk_num; i++) {
> @@ -85,7 +68,7 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> "clock-names", i, &clk_info->clk_name);
> if (ret) {
> mtk_v4l2_err("venc failed to get clk name %d", i);
> - goto put_larbvenclt;
> + goto put_larbvenc;
> }
> clk_info->vcodec_clk = devm_clk_get(&pdev->dev,
> clk_info->clk_name);
> @@ -93,14 +76,12 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> mtk_v4l2_err("venc devm_clk_get (%d)%s fail", i,
> clk_info->clk_name);
> ret = PTR_ERR(clk_info->vcodec_clk);
> - goto put_larbvenclt;
> + goto put_larbvenc;
> }
> }
>
> return 0;
>
> -put_larbvenclt:
> - put_device(pm->larbvenclt);
> put_larbvenc:
> put_device(pm->larbvenc);
> return ret;
> @@ -108,7 +89,7 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
>
> void mtk_vcodec_release_enc_pm(struct mtk_vcodec_dev *mtkdev)
> {
> - put_device(mtkdev->pm.larbvenclt);
> + pm_runtime_disable(mtkdev->pm.dev);
> put_device(mtkdev->pm.larbvenc);
> }
>
> @@ -130,18 +111,10 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm)
> ret = mtk_smi_larb_get(pm->larbvenc);
> if (ret) {
> mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret);
> - goto larbvencerr;
> - }
> - ret = mtk_smi_larb_get(pm->larbvenclt);
> - if (ret) {
> - mtk_v4l2_err("mtk_smi_larb_get larb4 fail %d", ret);
> - goto larbvenclterr;
> + goto clkerr;
> }
> return;
>
> -larbvenclterr:
> - mtk_smi_larb_put(pm->larbvenc);
> -larbvencerr:
> clkerr:
> for (i -= 1; i >= 0; i--)
> clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
> @@ -153,7 +126,6 @@ void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm)
> int i = 0;
>
> mtk_smi_larb_put(pm->larbvenc);
> - mtk_smi_larb_put(pm->larbvenclt);
> for (i = enc_clk->clk_num - 1; i >= 0; i--)
> clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
> }
> diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> index 11abb191ada5..8267a9c4fd25 100644
> --- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> @@ -367,7 +367,7 @@ static int vp8_enc_encode(void *handle,
>
> mtk_vcodec_debug_enter(inst);
>
> - enable_irq(ctx->dev->enc_lt_irq);
> + enable_irq(ctx->dev->enc_irq);
>
> switch (opt) {
> case VENC_START_OPT_ENCODE_FRAME:
> @@ -386,7 +386,7 @@ static int vp8_enc_encode(void *handle,
>
> encode_err:
>
> - disable_irq(ctx->dev->enc_lt_irq);
> + disable_irq(ctx->dev->enc_irq);
> mtk_vcodec_debug_leave(inst);
>
> return ret;
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node
2021-01-21 6:18 [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Irui Wang
` (3 preceding siblings ...)
2021-02-03 10:44 ` Alexandre Courbot
@ 2021-02-09 15:53 ` Rob Herring
2021-02-20 6:33 ` Irui Wang
4 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2021-02-09 15:53 UTC (permalink / raw)
To: Irui Wang
Cc: Alexandre Courbot, Hans Verkuil, Tiffany Lin, Andrew-CT Chen,
Mauro Carvalho Chehab, Matthias Brugger, Tomasz Figa,
Hsin-Yi Wang, Maoguang Meng, Longfei Wang, Yunfei Dong,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
srv_heupstream, linux-mediatek
On Thu, Jan 21, 2021 at 02:18:02PM +0800, Irui Wang wrote:
> Updates binding document since the avc and vp8 hardware encoder in
> MT8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
This is not a compatible change. You need to detail that and why that's
okay (assuming it is).
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
>
> ---
> .../bindings/media/mediatek-vcodec.txt | 58 ++++++++++---------
> 1 file changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> index 8217424fd4bd..f85276e629bf 100644
> --- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> @@ -4,7 +4,9 @@ Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
> supports high resolution encoding and decoding functionalities.
>
> Required properties:
> -- compatible : "mediatek,mt8173-vcodec-enc" for MT8173 encoder
> +- compatible : must be one of the following string:
> + "mediatek,mt8173-vcodec-vp8-enc" for mt8173 vp8 encoder.
> + "mediatek,mt8173-vcodec-avc-enc" for mt8173 avc encoder.
> "mediatek,mt8183-vcodec-enc" for MT8183 encoder.
> "mediatek,mt8173-vcodec-dec" for MT8173 decoder.
> - reg : Physical base address of the video codec registers and length of
> @@ -13,10 +15,11 @@ Required properties:
> - mediatek,larb : must contain the local arbiters in the current Socs.
> - clocks : list of clock specifiers, corresponding to entries in
> the clock-names property.
> -- clock-names: encoder must contain "venc_sel_src", "venc_sel",,
> - "venc_lt_sel_src", "venc_lt_sel", decoder must contain "vcodecpll",
> - "univpll_d2", "clk_cci400_sel", "vdec_sel", "vdecpll", "vencpll",
> - "venc_lt_sel", "vdec_bus_clk_src".
> +- clock-names:
> + avc venc must contain "venc_sel";
> + vp8 venc must contain "venc_lt_sel";
> + decoder must contain "vcodecpll", "univpll_d2", "clk_cci400_sel",
> + "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src".
> - iommus : should point to the respective IOMMU block with master port as
> argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> for details.
> @@ -80,14 +83,10 @@ vcodec_dec: vcodec@16000000 {
> assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
> };
>
> - vcodec_enc: vcodec@18002000 {
> - compatible = "mediatek,mt8173-vcodec-enc";
> - reg = <0 0x18002000 0 0x1000>, /*VENC_SYS*/
> - <0 0x19002000 0 0x1000>; /*VENC_LT_SYS*/
> - interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>,
> - <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> - mediatek,larb = <&larb3>,
> - <&larb5>;
> +vcodec_enc: vcodec@18002000 {
> + compatible = "mediatek,mt8173-vcodec-avc-enc";
> + reg = <0 0x18002000 0 0x1000>;
> + interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
> iommus = <&iommu M4U_PORT_VENC_RCPU>,
> <&iommu M4U_PORT_VENC_REC>,
> <&iommu M4U_PORT_VENC_BSDMA>,
> @@ -98,8 +97,20 @@ vcodec_dec: vcodec@16000000 {
> <&iommu M4U_PORT_VENC_REF_LUMA>,
> <&iommu M4U_PORT_VENC_REF_CHROMA>,
> <&iommu M4U_PORT_VENC_NBM_RDMA>,
> - <&iommu M4U_PORT_VENC_NBM_WDMA>,
> - <&iommu M4U_PORT_VENC_RCPU_SET2>,
> + <&iommu M4U_PORT_VENC_NBM_WDMA>;
> + mediatek,larb = <&larb3>;
> + mediatek,vpu = <&vpu>;
> + clocks = <&topckgen CLK_TOP_VENC_SEL>;
> + clock-names = "venc_sel";
> + assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;
> + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;
> + };
> +
> +vcodec_enc_lt: vcodec@19002000 {
> + compatible = "mediatek,mt8173-vcodec-vp8-enc";
> + reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
> + interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> + iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,
> <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
> <&iommu M4U_PORT_VENC_BSDMA_SET2>,
> <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
> @@ -108,17 +119,10 @@ vcodec_dec: vcodec@16000000 {
> <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
> <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
> <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
> + mediatek,larb = <&larb5>;
> mediatek,vpu = <&vpu>;
> - clocks = <&topckgen CLK_TOP_VENCPLL_D2>,
> - <&topckgen CLK_TOP_VENC_SEL>,
> - <&topckgen CLK_TOP_UNIVPLL1_D2>,
> - <&topckgen CLK_TOP_VENC_LT_SEL>;
> - clock-names = "venc_sel_src",
> - "venc_sel",
> - "venc_lt_sel_src",
> - "venc_lt_sel";
> - assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>,
> - <&topckgen CLK_TOP_VENC_LT_SEL>;
> - assigned-clock-parents = <&topckgen CLK_TOP_VENCPLL_D2>,
> - <&topckgen CLK_TOP_UNIVPLL1_D2>;
> + clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> + clock-names = "venc_lt_sel";
> + assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>;
> };
> --
> 2.18.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node
2021-02-03 10:44 ` Alexandre Courbot
@ 2021-02-20 6:26 ` Irui Wang
0 siblings, 0 replies; 13+ messages in thread
From: Irui Wang @ 2021-02-20 6:26 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Hans Verkuil, Tiffany Lin, Andrew-CT Chen, Mauro Carvalho Chehab,
Rob Herring, Matthias Brugger, Tomasz Figa, Hsin-Yi Wang,
Maoguang Meng, Longfei Wang, Yunfei Dong,
Linux Media Mailing List, devicetree, LKML,
moderated list:ARM/Mediatek SoC support, srv_heupstream,
moderated list:ARM/Mediatek SoC support
On Wed, 2021-02-03 at 19:44 +0900, Alexandre Courbot wrote:
> On Thu, Jan 21, 2021 at 3:18 PM Irui Wang <irui.wang@mediatek.com> wrote:
> >
> > Updates binding document since the avc and vp8 hardware encoder in
> > MT8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> > "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> >
> > ---
> > .../bindings/media/mediatek-vcodec.txt | 58 ++++++++++---------
> > 1 file changed, 31 insertions(+), 27 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > index 8217424fd4bd..f85276e629bf 100644
> > --- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > @@ -4,7 +4,9 @@ Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
> > supports high resolution encoding and decoding functionalities.
> >
> > Required properties:
> > -- compatible : "mediatek,mt8173-vcodec-enc" for MT8173 encoder
> > +- compatible : must be one of the following string:
> > + "mediatek,mt8173-vcodec-vp8-enc" for mt8173 vp8 encoder.
> > + "mediatek,mt8173-vcodec-avc-enc" for mt8173 avc encoder.
>
> IMHO "mediatek,mt8173-vcodec-enc-vp8" and
> "mediatek,mt8173-vcodec-enc-avc" would be more logical. Also to keep a
> bit of backward compatibility, shall we also allow
> "mediatek,mt8173-vcodec-enc" to be an alias for
> "mediatek,mt8173-vcodec-enc-avc"? The line above would become
>
> "mediatek,mt8173-vcodec-enc-avc" or "mediatek,mt8173-vcodec-enc" for
> mt8173 avc encoder.
>
will be updated in the next version.
> > "mediatek,mt8183-vcodec-enc" for MT8183 encoder.
> > "mediatek,mt8173-vcodec-dec" for MT8173 decoder.
> > - reg : Physical base address of the video codec registers and length of
> > @@ -13,10 +15,11 @@ Required properties:
> > - mediatek,larb : must contain the local arbiters in the current Socs.
> > - clocks : list of clock specifiers, corresponding to entries in
> > the clock-names property.
> > -- clock-names: encoder must contain "venc_sel_src", "venc_sel",,
> > - "venc_lt_sel_src", "venc_lt_sel", decoder must contain "vcodecpll",
> > - "univpll_d2", "clk_cci400_sel", "vdec_sel", "vdecpll", "vencpll",
> > - "venc_lt_sel", "vdec_bus_clk_src".
> > +- clock-names:
> > + avc venc must contain "venc_sel";
> > + vp8 venc must contain "venc_lt_sel";
>
> Can't we use "venc_sel" for both avc and vp8, since they are different
> nodes now? That way we can just say
>
> encoder must contain "venc_sel"
>
> which is clearer and also simpler on the code side.
>
ditto
> > + decoder must contain "vcodecpll", "univpll_d2", "clk_cci400_sel",
> > + "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src".
> > - iommus : should point to the respective IOMMU block with master port as
> > argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> > for details.
> > @@ -80,14 +83,10 @@ vcodec_dec: vcodec@16000000 {
> > assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
> > };
> >
> > - vcodec_enc: vcodec@18002000 {
> > - compatible = "mediatek,mt8173-vcodec-enc";
> > - reg = <0 0x18002000 0 0x1000>, /*VENC_SYS*/
> > - <0 0x19002000 0 0x1000>; /*VENC_LT_SYS*/
> > - interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>,
> > - <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> > - mediatek,larb = <&larb3>,
> > - <&larb5>;
> > +vcodec_enc: vcodec@18002000 {
>
> Let's use vcodec_enc_avc as a label?
ditto
>
> > + compatible = "mediatek,mt8173-vcodec-avc-enc";
> > + reg = <0 0x18002000 0 0x1000>;
> > + interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
> > iommus = <&iommu M4U_PORT_VENC_RCPU>,
> > <&iommu M4U_PORT_VENC_REC>,
> > <&iommu M4U_PORT_VENC_BSDMA>,
> > @@ -98,8 +97,20 @@ vcodec_dec: vcodec@16000000 {
> > <&iommu M4U_PORT_VENC_REF_LUMA>,
> > <&iommu M4U_PORT_VENC_REF_CHROMA>,
> > <&iommu M4U_PORT_VENC_NBM_RDMA>,
> > - <&iommu M4U_PORT_VENC_NBM_WDMA>,
> > - <&iommu M4U_PORT_VENC_RCPU_SET2>,
> > + <&iommu M4U_PORT_VENC_NBM_WDMA>;
> > + mediatek,larb = <&larb3>;
> > + mediatek,vpu = <&vpu>;
> > + clocks = <&topckgen CLK_TOP_VENC_SEL>;
> > + clock-names = "venc_sel";
> > + assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;
> > + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;
> > + };
> > +
> > +vcodec_enc_lt: vcodec@19002000 {
>
> And here the label should probably be "vcodec_enc_vp8" for consistency.
ditto
>
>
>
> > + compatible = "mediatek,mt8173-vcodec-vp8-enc";
> > + reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
> > + interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> > + iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,
> > <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
> > <&iommu M4U_PORT_VENC_BSDMA_SET2>,
> > <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
> > @@ -108,17 +119,10 @@ vcodec_dec: vcodec@16000000 {
> > <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
> > <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
> > <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
> > + mediatek,larb = <&larb5>;
> > mediatek,vpu = <&vpu>;
> > - clocks = <&topckgen CLK_TOP_VENCPLL_D2>,
> > - <&topckgen CLK_TOP_VENC_SEL>,
> > - <&topckgen CLK_TOP_UNIVPLL1_D2>,
> > - <&topckgen CLK_TOP_VENC_LT_SEL>;
> > - clock-names = "venc_sel_src",
> > - "venc_sel",
> > - "venc_lt_sel_src",
> > - "venc_lt_sel";
> > - assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>,
> > - <&topckgen CLK_TOP_VENC_LT_SEL>;
> > - assigned-clock-parents = <&topckgen CLK_TOP_VENCPLL_D2>,
> > - <&topckgen CLK_TOP_UNIVPLL1_D2>;
> > + clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> > + clock-names = "venc_lt_sel";
> > + assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> > + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>;
> > };
> > --
> > 2.18.0
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node
2021-02-09 15:53 ` Rob Herring
@ 2021-02-20 6:33 ` Irui Wang
0 siblings, 0 replies; 13+ messages in thread
From: Irui Wang @ 2021-02-20 6:33 UTC (permalink / raw)
To: Rob Herring
Cc: Alexandre Courbot, Hans Verkuil, Tiffany Lin, Andrew-CT Chen,
Mauro Carvalho Chehab, Matthias Brugger, Tomasz Figa,
Hsin-Yi Wang, Maoguang Meng, Longfei Wang, Yunfei Dong,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
srv_heupstream, linux-mediatek
On Tue, 2021-02-09 at 09:53 -0600, Rob Herring wrote:
> On Thu, Jan 21, 2021 at 02:18:02PM +0800, Irui Wang wrote:
> > Updates binding document since the avc and vp8 hardware encoder in
> > MT8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> > "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
>
> This is not a compatible change. You need to detail that and why that's
> okay (assuming it is).
>
this patch separates the two devices, it's a preparing patch for adding
device_link between the larbs and venc-device. It's mainly for fixing
the problem:
https://lkml.org/lkml/2019/9/3/316
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> >
> > ---
> > .../bindings/media/mediatek-vcodec.txt | 58 ++++++++++---------
> > 1 file changed, 31 insertions(+), 27 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > index 8217424fd4bd..f85276e629bf 100644
> > --- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > @@ -4,7 +4,9 @@ Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
> > supports high resolution encoding and decoding functionalities.
> >
> > Required properties:
> > -- compatible : "mediatek,mt8173-vcodec-enc" for MT8173 encoder
> > +- compatible : must be one of the following string:
> > + "mediatek,mt8173-vcodec-vp8-enc" for mt8173 vp8 encoder.
> > + "mediatek,mt8173-vcodec-avc-enc" for mt8173 avc encoder.
> > "mediatek,mt8183-vcodec-enc" for MT8183 encoder.
> > "mediatek,mt8173-vcodec-dec" for MT8173 decoder.
> > - reg : Physical base address of the video codec registers and length of
> > @@ -13,10 +15,11 @@ Required properties:
> > - mediatek,larb : must contain the local arbiters in the current Socs.
> > - clocks : list of clock specifiers, corresponding to entries in
> > the clock-names property.
> > -- clock-names: encoder must contain "venc_sel_src", "venc_sel",,
> > - "venc_lt_sel_src", "venc_lt_sel", decoder must contain "vcodecpll",
> > - "univpll_d2", "clk_cci400_sel", "vdec_sel", "vdecpll", "vencpll",
> > - "venc_lt_sel", "vdec_bus_clk_src".
> > +- clock-names:
> > + avc venc must contain "venc_sel";
> > + vp8 venc must contain "venc_lt_sel";
> > + decoder must contain "vcodecpll", "univpll_d2", "clk_cci400_sel",
> > + "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src".
> > - iommus : should point to the respective IOMMU block with master port as
> > argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> > for details.
> > @@ -80,14 +83,10 @@ vcodec_dec: vcodec@16000000 {
> > assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
> > };
> >
> > - vcodec_enc: vcodec@18002000 {
> > - compatible = "mediatek,mt8173-vcodec-enc";
> > - reg = <0 0x18002000 0 0x1000>, /*VENC_SYS*/
> > - <0 0x19002000 0 0x1000>; /*VENC_LT_SYS*/
> > - interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>,
> > - <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> > - mediatek,larb = <&larb3>,
> > - <&larb5>;
> > +vcodec_enc: vcodec@18002000 {
> > + compatible = "mediatek,mt8173-vcodec-avc-enc";
> > + reg = <0 0x18002000 0 0x1000>;
> > + interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>;
> > iommus = <&iommu M4U_PORT_VENC_RCPU>,
> > <&iommu M4U_PORT_VENC_REC>,
> > <&iommu M4U_PORT_VENC_BSDMA>,
> > @@ -98,8 +97,20 @@ vcodec_dec: vcodec@16000000 {
> > <&iommu M4U_PORT_VENC_REF_LUMA>,
> > <&iommu M4U_PORT_VENC_REF_CHROMA>,
> > <&iommu M4U_PORT_VENC_NBM_RDMA>,
> > - <&iommu M4U_PORT_VENC_NBM_WDMA>,
> > - <&iommu M4U_PORT_VENC_RCPU_SET2>,
> > + <&iommu M4U_PORT_VENC_NBM_WDMA>;
> > + mediatek,larb = <&larb3>;
> > + mediatek,vpu = <&vpu>;
> > + clocks = <&topckgen CLK_TOP_VENC_SEL>;
> > + clock-names = "venc_sel";
> > + assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>;
> > + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>;
> > + };
> > +
> > +vcodec_enc_lt: vcodec@19002000 {
> > + compatible = "mediatek,mt8173-vcodec-vp8-enc";
> > + reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */
> > + interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>;
> > + iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>,
> > <&iommu M4U_PORT_VENC_REC_FRM_SET2>,
> > <&iommu M4U_PORT_VENC_BSDMA_SET2>,
> > <&iommu M4U_PORT_VENC_SV_COMA_SET2>,
> > @@ -108,17 +119,10 @@ vcodec_dec: vcodec@16000000 {
> > <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>,
> > <&iommu M4U_PORT_VENC_REF_LUMA_SET2>,
> > <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;
> > + mediatek,larb = <&larb5>;
> > mediatek,vpu = <&vpu>;
> > - clocks = <&topckgen CLK_TOP_VENCPLL_D2>,
> > - <&topckgen CLK_TOP_VENC_SEL>,
> > - <&topckgen CLK_TOP_UNIVPLL1_D2>,
> > - <&topckgen CLK_TOP_VENC_LT_SEL>;
> > - clock-names = "venc_sel_src",
> > - "venc_sel",
> > - "venc_lt_sel_src",
> > - "venc_lt_sel";
> > - assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>,
> > - <&topckgen CLK_TOP_VENC_LT_SEL>;
> > - assigned-clock-parents = <&topckgen CLK_TOP_VENCPLL_D2>,
> > - <&topckgen CLK_TOP_UNIVPLL1_D2>;
> > + clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> > + clock-names = "venc_lt_sel";
> > + assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>;
> > + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>;
> > };
> > --
> > 2.18.0
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver
2021-02-03 10:44 ` Alexandre Courbot
@ 2021-02-20 6:55 ` Irui Wang
2021-02-22 13:03 ` Alexandre Courbot
0 siblings, 1 reply; 13+ messages in thread
From: Irui Wang @ 2021-02-20 6:55 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Hans Verkuil, Tiffany Lin, Andrew-CT Chen, Mauro Carvalho Chehab,
Rob Herring, Matthias Brugger, Tomasz Figa, Hsin-Yi Wang,
Maoguang Meng, Longfei Wang, Yunfei Dong,
Linux Media Mailing List, devicetree, LKML,
moderated list:ARM/Mediatek SoC support, srv_heupstream,
moderated list:ARM/Mediatek SoC support
On Wed, 2021-02-03 at 19:44 +0900, Alexandre Courbot wrote:
> Hi Irui,
>
> Thanks for pushing this forward. I had two small conflicts when
> applying this patch to the media tree, so you may want to rebase
> before sending the next version. Please see the comments inline.
>
> On Thu, Jan 21, 2021 at 3:18 PM Irui Wang <irui.wang@mediatek.com> wrote:
> >
> > MTK H264 Encoder(VENC_SYS) and VP8 Encoder(VENC_LT_SYS) are two
> > independent hardware instance. They have their owner interrupt,
> > register mapping, and special clocks.
> >
> > This patch seperates them into two drivers:
>
> seperates -> separates
>
> Also the patch does not result in two drivers, but two devices.
>
> > User Call "VIDIOC_QUERYCAP":
> > H264 Encoder return driver name "mtk-vcodec-enc";
> > VP8 Encoder return driver name "mtk-venc-vp8.
>
> I wonder if we need to use two different names? The driver is the
> same, so it makes sense to me that both devices return
> "mtk-vcodec-enc". Userspace can then list the formats on the CAPTURE
> queue in order to query the supported codecs.
>
I'm afraid we can't, there is a symlink when chrome use the
encoder(50-media.rules):
ATTR{name} == "mtk-vcodec-enc", SYMLINK+="video-enc"
ATTR{name} == "mtk-venc-vp8", SYMLINK+="video-enc0"
if we use the same name,how userspace access the encoder? maybe there
will be some modifications are needed in VEA(for example)?
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> >
> > ---
> > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 10 +-
> > .../platform/mtk-vcodec/mtk_vcodec_enc.c | 23 +++-
> > .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 121 +++++++-----------
> > .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 40 +-----
> > .../platform/mtk-vcodec/venc/venc_vp8_if.c | 4 +-
> > 5 files changed, 82 insertions(+), 116 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > index 3dd010cba23e..1594edcc706d 100644
> > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > @@ -19,6 +19,7 @@
> > #define MTK_VCODEC_DRV_NAME "mtk_vcodec_drv"
> > #define MTK_VCODEC_DEC_NAME "mtk-vcodec-dec"
> > #define MTK_VCODEC_ENC_NAME "mtk-vcodec-enc"
> > +#define MTK_VENC_VP8_NAME "mtk-venc-vp8"
> > #define MTK_PLATFORM_STR "platform:mt8173"
> >
> > #define MTK_VCODEC_MAX_PLANES 3
> > @@ -193,7 +194,6 @@ struct mtk_vcodec_pm {
> >
> > struct mtk_vcodec_clk venc_clk;
> > struct device *larbvenc;
> > - struct device *larbvenclt;
> > struct device *dev;
> > struct mtk_vcodec_dev *mtkdev;
> > };
> > @@ -311,25 +311,27 @@ enum mtk_chip {
> > * @chip: chip this encoder is compatible with
> > *
> > * @uses_ext: whether the encoder uses the extended firmware messaging format
> > - * @has_lt_irq: whether the encoder uses the LT irq
> > + * @name: whether the encoder core is vp8
> > * @min_birate: minimum supported encoding bitrate
> > * @max_bitrate: maximum supported encoding bitrate
> > * @capture_formats: array of supported capture formats
> > * @num_capture_formats: number of entries in capture_formats
> > * @output_formats: array of supported output formats
> > * @num_output_formats: number of entries in output_formats
> > + * @core_id: stand for h264 or vp8 encode index
> > */
> > struct mtk_vcodec_enc_pdata {
> > enum mtk_chip chip;
> >
> > bool uses_ext;
> > - bool has_lt_irq;
> > + const char *name;
>
> This new member can be removed if we use the same name for both devices.
>
> > unsigned long min_bitrate;
> > unsigned long max_bitrate;
> > const struct mtk_video_fmt *capture_formats;
> > size_t num_capture_formats;
> > const struct mtk_video_fmt *output_formats;
> > size_t num_output_formats;
> > + int core_id;
> > };
> >
> > #define MTK_ENC_CTX_IS_EXT(ctx) ((ctx)->dev->venc_pdata->uses_ext)
> > @@ -361,7 +363,6 @@ struct mtk_vcodec_enc_pdata {
> > *
> > * @dec_irq: decoder irq resource
> > * @enc_irq: h264 encoder irq resource
> > - * @enc_lt_irq: vp8 encoder irq resource
> > *
> > * @dec_mutex: decoder hardware lock
> > * @enc_mutex: encoder hardware lock.
> > @@ -397,7 +398,6 @@ struct mtk_vcodec_dev {
> >
> > int dec_irq;
> > int enc_irq;
> > - int enc_lt_irq;
> >
> > struct mutex dec_mutex;
> > struct mutex enc_mutex;
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > index 21de1431cfcb..0da6871b4b39 100644
> > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > @@ -9,6 +9,7 @@
> > #include <media/v4l2-mem2mem.h>
> > #include <media/videobuf2-dma-contig.h>
> > #include <soc/mediatek/smi.h>
> > +#include <linux/pm_runtime.h>
> >
> > #include "mtk_vcodec_drv.h"
> > #include "mtk_vcodec_enc.h"
> > @@ -189,7 +190,10 @@ static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
> > static int vidioc_venc_querycap(struct file *file, void *priv,
> > struct v4l2_capability *cap)
> > {
> > - strscpy(cap->driver, MTK_VCODEC_ENC_NAME, sizeof(cap->driver));
> > + const struct mtk_vcodec_enc_pdata *pdata =
> > + fh_to_ctx(priv)->dev->venc_pdata;
> > +
> > + strscpy(cap->driver, pdata->name, sizeof(cap->driver));
> > strscpy(cap->bus_info, MTK_PLATFORM_STR, sizeof(cap->bus_info));
> > strscpy(cap->card, MTK_PLATFORM_STR, sizeof(cap->card));
> >
> > @@ -797,7 +801,7 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> > */
> > if ((ctx->state == MTK_STATE_ABORT) || (ctx->state == MTK_STATE_FREE)) {
> > ret = -EIO;
> > - goto err_set_param;
> > + goto err_start_stream;
> > }
> >
> > /* Do the initialization when both start_streaming have been called */
> > @@ -809,6 +813,12 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> > return 0;
> > }
> >
> > + ret = pm_runtime_get_sync(&ctx->dev->plat_dev->dev);
> > + if (ret < 0) {
> > + mtk_v4l2_err("pm_runtime_get_sync fail %d", ret);
> > + goto err_start_stream;
> > + }
>
> This does not seem to be related to the split ; why is this necessary?
It's a prepare patch for the larbs, after separating, we need call
pm_runtime_ API additionally.
>
> > +
> > mtk_venc_set_param(ctx, ¶m);
> > ret = venc_if_set_param(ctx, VENC_SET_PARAM_ENC, ¶m);
> > if (ret) {
> > @@ -835,6 +845,11 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> > return 0;
> >
> > err_set_param:
> > + ret = pm_runtime_put(&ctx->dev->plat_dev->dev);
> > + if (ret < 0)
> > + mtk_v4l2_err("pm_runtime_put fail %d", ret);
> > +
> > +err_start_stream:
> > for (i = 0; i < q->num_buffers; ++i) {
> > struct vb2_buffer *buf = vb2_get_buffer(q, i);
> >
> > @@ -888,6 +903,10 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
> > if (ret)
> > mtk_v4l2_err("venc_if_deinit failed=%d", ret);
> >
> > + ret = pm_runtime_put(&ctx->dev->plat_dev->dev);
> > + if (ret < 0)
> > + mtk_v4l2_err("pm_runtime_put fail %d", ret);
> > +
> > ctx->state = MTK_STATE_FREE;
> > }
> >
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> > index dfb42e19bf81..4bee42454253 100644
> > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> > @@ -49,12 +49,15 @@ static const struct mtk_video_fmt mtk_video_formats_output_mt8173[] = {
> > },
> > };
> >
> > -static const struct mtk_video_fmt mtk_video_formats_capture_mt8173[] = {
> > +static const struct mtk_video_fmt mtk_video_formats_capture_mt8173_h264[] = {
> > {
> > .fourcc = V4L2_PIX_FMT_H264,
> > .type = MTK_FMT_ENC,
> > .num_planes = 1,
> > },
> > +};
> > +
> > +static const struct mtk_video_fmt mtk_video_formats_capture_mt8173_vp8[] = {
> > {
> > .fourcc = V4L2_PIX_FMT_VP8,
> > .type = MTK_FMT_ENC,
> > @@ -110,35 +113,13 @@ static irqreturn_t mtk_vcodec_enc_irq_handler(int irq, void *priv)
> > ctx = dev->curr_ctx;
> > spin_unlock_irqrestore(&dev->irqlock, flags);
> >
> > - mtk_v4l2_debug(1, "id=%d", ctx->id);
> > - addr = dev->reg_base[VENC_SYS] + MTK_VENC_IRQ_ACK_OFFSET;
> > -
> > - ctx->irq_status = readl(dev->reg_base[VENC_SYS] +
> > - (MTK_VENC_IRQ_STATUS_OFFSET));
> > -
> > - clean_irq_status(ctx->irq_status, addr);
> > -
> > - wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED);
> > - return IRQ_HANDLED;
> > -}
> > -
> > -static irqreturn_t mtk_vcodec_enc_lt_irq_handler(int irq, void *priv)
> > -{
> > - struct mtk_vcodec_dev *dev = priv;
> > - struct mtk_vcodec_ctx *ctx;
> > - unsigned long flags;
> > - void __iomem *addr;
> > -
> > - spin_lock_irqsave(&dev->irqlock, flags);
> > - ctx = dev->curr_ctx;
> > - spin_unlock_irqrestore(&dev->irqlock, flags);
> > + mtk_v4l2_debug(1, "id=%d coreid:%d", ctx->id, dev->venc_pdata->core_id);
> > + addr = dev->reg_base[dev->venc_pdata->core_id] +
> > + MTK_VENC_IRQ_ACK_OFFSET;
> >
> > - mtk_v4l2_debug(1, "id=%d", ctx->id);
> > - ctx->irq_status = readl(dev->reg_base[VENC_LT_SYS] +
> > + ctx->irq_status = readl(dev->reg_base[dev->venc_pdata->core_id] +
> > (MTK_VENC_IRQ_STATUS_OFFSET));
> >
> > - addr = dev->reg_base[VENC_LT_SYS] + MTK_VENC_IRQ_ACK_OFFSET;
> > -
> > clean_irq_status(ctx->irq_status, addr);
> >
> > wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED);
> > @@ -293,17 +274,21 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> > dev->venc_pdata = of_device_get_match_data(&pdev->dev);
> > ret = mtk_vcodec_init_enc_pm(dev);
> > if (ret < 0) {
> > - dev_err(&pdev->dev, "Failed to get mt vcodec clock source!");
> > + dev_err(&pdev->dev, "Failed to get mtk vcodec clock source!");
> > goto err_enc_pm;
> > }
> >
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - dev->reg_base[VENC_SYS] = devm_ioremap_resource(&pdev->dev, res);
> > - if (IS_ERR((__force void *)dev->reg_base[VENC_SYS])) {
> > - ret = PTR_ERR((__force void *)dev->reg_base[VENC_SYS]);
> > + pm_runtime_enable(&pdev->dev);
> > +
> > + snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
> > + dev->venc_pdata->name);
> > +
> > + dev->reg_base[dev->venc_pdata->core_id] =
> > + devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(dev->reg_base[dev->venc_pdata->core_id])) {
> > + ret = PTR_ERR(dev->reg_base[dev->venc_pdata->core_id]);
> > goto err_res;
> > }
> > - mtk_v4l2_debug(2, "reg[%d] base=0x%p", i, dev->reg_base[VENC_SYS]);
> >
> > res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > if (res == NULL) {
> > @@ -318,44 +303,17 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> > mtk_vcodec_enc_irq_handler,
> > 0, pdev->name, dev);
> > if (ret) {
> > - dev_err(&pdev->dev, "Failed to install dev->enc_irq %d (%d)",
> > - dev->enc_irq,
> > - ret);
> > + dev_err(&pdev->dev,
> > + "Failed to install dev->enc_irq %d (%d) core_id:%d",
> > + dev->enc_irq, ret, dev->venc_pdata->core_id);
> > ret = -EINVAL;
> > goto err_res;
> > }
> >
> > - if (dev->venc_pdata->has_lt_irq) {
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > - dev->reg_base[VENC_LT_SYS] = devm_ioremap_resource(&pdev->dev, res);
> > - if (IS_ERR((__force void *)dev->reg_base[VENC_LT_SYS])) {
> > - ret = PTR_ERR((__force void *)dev->reg_base[VENC_LT_SYS]);
> > - goto err_res;
> > - }
> > - mtk_v4l2_debug(2, "reg[%d] base=0x%p", i, dev->reg_base[VENC_LT_SYS]);
> > -
> > - dev->enc_lt_irq = platform_get_irq(pdev, 1);
> > - irq_set_status_flags(dev->enc_lt_irq, IRQ_NOAUTOEN);
> > - ret = devm_request_irq(&pdev->dev,
> > - dev->enc_lt_irq,
> > - mtk_vcodec_enc_lt_irq_handler,
> > - 0, pdev->name, dev);
> > - if (ret) {
> > - dev_err(&pdev->dev,
> > - "Failed to install dev->enc_lt_irq %d (%d)",
> > - dev->enc_lt_irq, ret);
> > - ret = -EINVAL;
> > - goto err_res;
> > - }
> > - }
> > -
> > mutex_init(&dev->enc_mutex);
> > mutex_init(&dev->dev_mutex);
> > spin_lock_init(&dev->irqlock);
> >
> > - snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
> > - "[MTK_V4L2_VENC]");
> > -
> > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> > if (ret) {
> > mtk_v4l2_err("v4l2_device_register err=%d", ret);
> > @@ -381,7 +339,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> > V4L2_CAP_STREAMING;
> >
> > snprintf(vfd_enc->name, sizeof(vfd_enc->name), "%s",
> > - MTK_VCODEC_ENC_NAME);
> > + dev->venc_pdata->name);
>
> Let's keep the indendation at the same level as before (if we use the
> same driver name for both devices this change is unnecessary though).
>
Need to confirm before send the next version.
> > video_set_drvdata(vfd_enc, dev);
> > dev->vfd_enc = vfd_enc;
> > platform_set_drvdata(pdev, dev);
> > @@ -409,8 +367,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> > goto err_enc_reg;
> > }
> >
> > - mtk_v4l2_debug(0, "encoder registered as /dev/video%d",
> > - vfd_enc->num);
> > + mtk_v4l2_debug(0, "encoder %d registered as /dev/video%d",
> > + dev->venc_pdata->core_id, vfd_enc->num);
> >
> > return 0;
> >
> > @@ -429,20 +387,33 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
> > return ret;
> > }
> >
> > -static const struct mtk_vcodec_enc_pdata mt8173_pdata = {
> > +static const struct mtk_vcodec_enc_pdata mt8173_avc_pdata = {
> > .chip = MTK_MT8173,
> > - .has_lt_irq = true,
> > - .capture_formats = mtk_video_formats_capture_mt8173,
> > - .num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_mt8173),
> > + .name = MTK_VCODEC_ENC_NAME,
> > + .capture_formats = mtk_video_formats_capture_mt8173_h264,
> > + .num_capture_formats = 1,
>
> Can't this be ARRAY_SIZE(mtk_video_formats_capture_mt8173_h264)
>
> > .output_formats = mtk_video_formats_output_mt8173,
> > .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> > - .min_bitrate = 1,
> > + .min_bitrate = 64,
>
> I'm fine with changing the value of min_bitrate, but this should be a
> separate patch.
>
> > + .max_bitrate = 4000000,
> > + .core_id = VENC_SYS,
> > +};
> > +
> > +static const struct mtk_vcodec_enc_pdata mt8173_vp8_pdata = {
> > + .chip = MTK_MT8173,
> > + .name = MTK_VENC_VP8_NAME,
> > + .capture_formats = mtk_video_formats_capture_mt8173_vp8,
> > + .num_capture_formats = 1,
>
> Same here, ARRAY_SIZE(mtk_video_formats_capture_mt8173_vp8)
>
> > + .output_formats = mtk_video_formats_output_mt8173,
> > + .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> > + .min_bitrate = 64,
> > .max_bitrate = 4000000,
> > + .core_id = VENC_LT_SYS,
> > };
> >
> > static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
> > .chip = MTK_MT8183,
> > - .has_lt_irq = false,
> > + .name = MTK_VCODEC_ENC_NAME,
> > .uses_ext = true,
> > .capture_formats = mtk_video_formats_capture_mt8183,
> > .num_capture_formats = ARRAY_SIZE(mtk_video_formats_capture_mt8183),
> > @@ -451,10 +422,14 @@ static const struct mtk_vcodec_enc_pdata mt8183_pdata = {
> > .num_output_formats = ARRAY_SIZE(mtk_video_formats_output_mt8173),
> > .min_bitrate = 64,
> > .max_bitrate = 40000000,
> > + .core_id = VENC_SYS,
> > };
> >
> > static const struct of_device_id mtk_vcodec_enc_match[] = {
> > - {.compatible = "mediatek,mt8173-vcodec-enc", .data = &mt8173_pdata},
> > + {.compatible = "mediatek,mt8173-vcodec-avc-enc",
> > + .data = &mt8173_avc_pdata},
>
> For backward compatibility let's also match
> "mediatek,mt8173-vcodec-enc" against this platform data.
>
>
> > + {.compatible = "mediatek,mt8173-vcodec-vp8-enc",
> > + .data = &mt8173_vp8_pdata},
> > {.compatible = "mediatek,mt8183-vcodec-enc", .data = &mt8183_pdata},
> > {},
> > };
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> > index 3b7c54d6aa8f..1b2e4930ed27 100644
> > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c
> > @@ -43,23 +43,6 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> > return -ENODEV;
> > }
> > pm->larbvenc = &pdev->dev;
> > -
> > - node = of_parse_phandle(dev->of_node, "mediatek,larb", 1);
> > - if (!node) {
> > - mtk_v4l2_err("no mediatek,larb found");
> > - ret = -ENODEV;
> > - goto put_larbvenc;
> > - }
> > -
> > - pdev = of_find_device_by_node(node);
> > - of_node_put(node);
> > - if (!pdev) {
> > - mtk_v4l2_err("no mediatek,larb device found");
> > - ret = -ENODEV;
> > - goto put_larbvenc;
> > - }
> > -
> > - pm->larbvenclt = &pdev->dev;
> > pdev = mtkdev->plat_dev;
> > pm->dev = &pdev->dev;
> >
> > @@ -71,12 +54,12 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> > GFP_KERNEL);
> > if (!enc_clk->clk_info) {
> > ret = -ENOMEM;
> > - goto put_larbvenclt;
> > + goto put_larbvenc;
> > }
> > } else {
> > mtk_v4l2_err("Failed to get venc clock count");
> > ret = -EINVAL;
> > - goto put_larbvenclt;
> > + goto put_larbvenc;
> > }
> >
> > for (i = 0; i < enc_clk->clk_num; i++) {
> > @@ -85,7 +68,7 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> > "clock-names", i, &clk_info->clk_name);
> > if (ret) {
> > mtk_v4l2_err("venc failed to get clk name %d", i);
> > - goto put_larbvenclt;
> > + goto put_larbvenc;
> > }
> > clk_info->vcodec_clk = devm_clk_get(&pdev->dev,
> > clk_info->clk_name);
> > @@ -93,14 +76,12 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> > mtk_v4l2_err("venc devm_clk_get (%d)%s fail", i,
> > clk_info->clk_name);
> > ret = PTR_ERR(clk_info->vcodec_clk);
> > - goto put_larbvenclt;
> > + goto put_larbvenc;
> > }
> > }
> >
> > return 0;
> >
> > -put_larbvenclt:
> > - put_device(pm->larbvenclt);
> > put_larbvenc:
> > put_device(pm->larbvenc);
> > return ret;
> > @@ -108,7 +89,7 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev)
> >
> > void mtk_vcodec_release_enc_pm(struct mtk_vcodec_dev *mtkdev)
> > {
> > - put_device(mtkdev->pm.larbvenclt);
> > + pm_runtime_disable(mtkdev->pm.dev);
> > put_device(mtkdev->pm.larbvenc);
> > }
> >
> > @@ -130,18 +111,10 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm)
> > ret = mtk_smi_larb_get(pm->larbvenc);
> > if (ret) {
> > mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret);
> > - goto larbvencerr;
> > - }
> > - ret = mtk_smi_larb_get(pm->larbvenclt);
> > - if (ret) {
> > - mtk_v4l2_err("mtk_smi_larb_get larb4 fail %d", ret);
> > - goto larbvenclterr;
> > + goto clkerr;
> > }
> > return;
> >
> > -larbvenclterr:
> > - mtk_smi_larb_put(pm->larbvenc);
> > -larbvencerr:
> > clkerr:
> > for (i -= 1; i >= 0; i--)
> > clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
> > @@ -153,7 +126,6 @@ void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm)
> > int i = 0;
> >
> > mtk_smi_larb_put(pm->larbvenc);
> > - mtk_smi_larb_put(pm->larbvenclt);
> > for (i = enc_clk->clk_num - 1; i >= 0; i--)
> > clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
> > }
> > diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> > index 11abb191ada5..8267a9c4fd25 100644
> > --- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> > +++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> > @@ -367,7 +367,7 @@ static int vp8_enc_encode(void *handle,
> >
> > mtk_vcodec_debug_enter(inst);
> >
> > - enable_irq(ctx->dev->enc_lt_irq);
> > + enable_irq(ctx->dev->enc_irq);
> >
> > switch (opt) {
> > case VENC_START_OPT_ENCODE_FRAME:
> > @@ -386,7 +386,7 @@ static int vp8_enc_encode(void *handle,
> >
> > encode_err:
> >
> > - disable_irq(ctx->dev->enc_lt_irq);
> > + disable_irq(ctx->dev->enc_irq);
> > mtk_vcodec_debug_leave(inst);
> >
> > return ret;
> > --
> > 2.18.0
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver
2021-02-20 6:55 ` Irui Wang
@ 2021-02-22 13:03 ` Alexandre Courbot
0 siblings, 0 replies; 13+ messages in thread
From: Alexandre Courbot @ 2021-02-22 13:03 UTC (permalink / raw)
To: Irui Wang
Cc: Hans Verkuil, Tiffany Lin, Andrew-CT Chen, Mauro Carvalho Chehab,
Rob Herring, Matthias Brugger, Tomasz Figa, Hsin-Yi Wang,
Maoguang Meng, Longfei Wang, Yunfei Dong,
Linux Media Mailing List, devicetree, LKML,
moderated list:ARM/Mediatek SoC support, srv_heupstream,
moderated list:ARM/Mediatek SoC support
On Sat, Feb 20, 2021 at 3:56 PM Irui Wang <irui.wang@mediatek.com> wrote:
>
> On Wed, 2021-02-03 at 19:44 +0900, Alexandre Courbot wrote:
> > Hi Irui,
> >
> > Thanks for pushing this forward. I had two small conflicts when
> > applying this patch to the media tree, so you may want to rebase
> > before sending the next version. Please see the comments inline.
> >
> > On Thu, Jan 21, 2021 at 3:18 PM Irui Wang <irui.wang@mediatek.com> wrote:
> > >
> > > MTK H264 Encoder(VENC_SYS) and VP8 Encoder(VENC_LT_SYS) are two
> > > independent hardware instance. They have their owner interrupt,
> > > register mapping, and special clocks.
> > >
> > > This patch seperates them into two drivers:
> >
> > seperates -> separates
> >
> > Also the patch does not result in two drivers, but two devices.
> >
> > > User Call "VIDIOC_QUERYCAP":
> > > H264 Encoder return driver name "mtk-vcodec-enc";
> > > VP8 Encoder return driver name "mtk-venc-vp8.
> >
> > I wonder if we need to use two different names? The driver is the
> > same, so it makes sense to me that both devices return
> > "mtk-vcodec-enc". Userspace can then list the formats on the CAPTURE
> > queue in order to query the supported codecs.
> >
> I'm afraid we can't, there is a symlink when chrome use the
> encoder(50-media.rules):
> ATTR{name} == "mtk-vcodec-enc", SYMLINK+="video-enc"
> ATTR{name} == "mtk-venc-vp8", SYMLINK+="video-enc0"
> if we use the same name,how userspace access the encoder? maybe there
> will be some modifications are needed in VEA(for example)?
Chrome OS can use a different udev rule to differentiate the two
nodes. Actually I already have a CL to support this:
https://chromium-review.googlesource.com/c/chromiumos/overlays/board-overlays/+/2673592
So both nodes being named the same won't be a problem for Chrome OS,
and makes more sense for an upstream merge anyway.
Cheers,
Alex.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-02-22 14:03 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 6:18 [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Irui Wang
2021-01-21 6:18 ` [PATCH 2/3] arm64: dts: mt8173: Separating mtk-vcodec-enc device node Irui Wang
2021-01-26 8:43 ` Tiffany Lin
2021-01-21 6:18 ` [PATCH 3/3] media: mtk-vcodec: Separating mtk encoder driver Irui Wang
2021-01-26 8:44 ` Tiffany Lin
2021-02-03 10:44 ` Alexandre Courbot
2021-02-20 6:55 ` Irui Wang
2021-02-22 13:03 ` Alexandre Courbot
2021-01-26 8:42 ` [PATCH 1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node Tiffany Lin
2021-02-03 10:44 ` Alexandre Courbot
2021-02-20 6:26 ` Irui Wang
2021-02-09 15:53 ` Rob Herring
2021-02-20 6:33 ` Irui Wang
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).