Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1271154 > unrolled thread
| Started by | Tiffany Lin <tiffany.lin@mediatek.com> |
|---|---|
| First post | 2015-11-17 14:00 +0100 |
| Last post | 2015-11-30 16:00 +0100 |
| Articles | 16 — 7 participants |
Back to article view | Back to linux.kernel
[RESEND RFC/PATCH 0/8] Add MT8173 Video Encoder Driver and VPU Driver Tiffany Lin <tiffany.lin@mediatek.com> - 2015-11-17 14:00 +0100
[RESEND RFC/PATCH 1/8] dt-bindings: Add a binding for Mediatek Video Processor Unit Tiffany Lin <tiffany.lin@mediatek.com> - 2015-11-17 14:00 +0100
Re: [RESEND RFC/PATCH 1/8] dt-bindings: Add a binding for Mediatek Video Processor Unit Mark Rutland <mark.rutland@arm.com> - 2015-11-17 15:20 +0100
Re: [RESEND RFC/PATCH 1/8] dt-bindings: Add a binding for Mediatek Video Processor Unit andrew-ct chen <andrew-ct.chen@mediatek.com> - 2015-11-19 03:50 +0100
[RESEND RFC/PATCH 2/8] arm64: dts: mediatek: Add node for Mediatek Video Processor Unit Tiffany Lin <tiffany.lin@mediatek.com> - 2015-11-17 14:00 +0100
[RESEND RFC/PATCH 8/8] media: platform: mtk-vcodec: Add Mediatek H264 Video Encoder Driver Tiffany Lin <tiffany.lin@mediatek.com> - 2015-11-17 14:00 +0100
[RESEND RFC/PATCH 4/8] dt-bindings: Add a binding for Mediatek Video Encoder Tiffany Lin <tiffany.lin@mediatek.com> - 2015-11-17 14:00 +0100
Re: [RESEND RFC/PATCH 4/8] dt-bindings: Add a binding for Mediatek Video Encoder Rob Herring <robh@kernel.org> - 2015-11-17 20:50 +0100
Re: [RESEND RFC/PATCH 4/8] dt-bindings: Add a binding for Mediatek Video Encoder tiffany lin <tiffany.lin@mediatek.com> - 2015-11-18 08:20 +0100
[RESEND RFC/PATCH 5/8] arm64: dts: mediatek: Add Video Encoder for MT8173 Tiffany Lin <tiffany.lin@mediatek.com> - 2015-11-17 14:00 +0100
Re: [RESEND RFC/PATCH 0/8] Add MT8173 Video Encoder Driver and VPU Driver Hans Verkuil <hverkuil@xs4all.nl> - 2015-11-19 08:50 +0100
Re: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU Daniel Thompson <daniel.thompson@linaro.org> - 2015-11-27 13:30 +0100
Re: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU andrew-ct chen <andrew-ct.chen@mediatek.com> - 2015-11-30 12:50 +0100
Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver Daniel Thompson <daniel.thompson@linaro.org> - 2015-11-27 17:40 +0100
Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver tiffany lin <tiffany.lin@mediatek.com> - 2015-11-30 12:50 +0100
Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver Daniel Thompson <daniel.thompson@linaro.org> - 2015-11-30 16:00 +0100
| From | Tiffany Lin <tiffany.lin@mediatek.com> |
|---|---|
| Date | 2015-11-17 14:00 +0100 |
| Subject | [RESEND RFC/PATCH 0/8] Add MT8173 Video Encoder Driver and VPU Driver |
| Message-ID | <qvO1A-58e-13@gated-at.bofh.it> |
============== Introduction ============== The purpose of this RFC is to discuss the driver for a hw video codec embedded in the Mediatek's MT8173 SoCs. Mediatek Video Codec is able to handle video encoding of in a range of formats. This RFC also include VPU driver. Mediatek Video Codec driver rely on VPU driver to load, communicate with VPU. Internally the driver uses videobuf2 framework and MTK IOMMU and MTK SMI. MTK IOMMU and MTK SMI have not yet been merged, but we wanted to start discussion about the driver earlier so it could be merged sooner. The driver posted here is the initial version, so I suppose it will require more work. [1]http://lists.infradead.org/pipermail/linux-mediatek/2015-October/002525.html ================== Device interface ================== In principle the driver bases on memory-to-memory framework: it provides a single video node and each opened file handle gets its own private context with separate buffer queues. Each context consist of 2 buffer queues: OUTPUT (for source buffers, i.e. raw video frames) and CAPTURE (for destination buffers, i.e. encoded video frames). The process of encoding video data from stream is a bit more complicated than typical memory-to-memory processing. We base on memory-to-memory framework and add the complicated part in our vb2 and v4l2 callback functionss. So we can base on well done m2m memory-to-memory framework, reduce duplicate code and make our driver code simple. ============================== VPU (Video Processor Unit) ============================== The VPU driver for hw video codec embedded in Mediatek's MT8173 SOCs. It is able to handle video decoding/encoding of in a range of formats. The driver provides with VPU firmware download, memory management and the communication interface between CPU and VPU. For VPU initialization, it will create virtual memory for CPU access and IOMMU address for vcodec hw device access. When a decode/encode instance opens a device node, vpu driver will download vpu firmware to the device. A decode/encode instant will decode/encode a frame using VPU interface to interrupt vpu to handle decoding/encoding jobs. Please have a look at the code and comments will be very much appreciated. Andrew-CT Chen (3): dt-bindings: Add a binding for Mediatek Video Processor Unit arm64: dts: mediatek: Add node for Mediatek Video Processor Unit media: platform: mtk-vpu: Support Mediatek VPU Daniel Hsiao (1): media: platform: mtk-vcodec: Add Mediatek VP8 Video Encoder Driver Tiffany Lin (4): dt-bindings: Add a binding for Mediatek Video Encoder arm64: dts: mediatek: Add Video Encoder for MT8173 media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver media: platform: mtk-vcodec: Add Mediatek H264 Video Encoder Driver .../devicetree/bindings/media/mediatek-vcodec.txt | 58 + .../devicetree/bindings/media/mediatek-vpu.txt | 27 + arch/arm64/boot/dts/mediatek/mt8173.dtsi | 58 + drivers/media/platform/Kconfig | 19 + drivers/media/platform/Makefile | 5 + drivers/media/platform/mtk-vcodec/Kconfig | 5 + drivers/media/platform/mtk-vcodec/Makefile | 12 + drivers/media/platform/mtk-vcodec/common/Makefile | 12 + .../media/platform/mtk-vcodec/common/venc_drv_if.c | 159 ++ .../media/platform/mtk-vcodec/h264_enc/Makefile | 9 + .../platform/mtk-vcodec/h264_enc/venc_h264_if.c | 529 ++++++ .../platform/mtk-vcodec/h264_enc/venc_h264_if.h | 53 + .../platform/mtk-vcodec/h264_enc/venc_h264_vpu.c | 341 ++++ .../platform/mtk-vcodec/include/venc_drv_base.h | 68 + .../platform/mtk-vcodec/include/venc_drv_if.h | 187 +++ .../platform/mtk-vcodec/include/venc_ipi_msg.h | 212 +++ drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 441 +++++ drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 1773 ++++++++++++++++++++ drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.h | 28 + .../media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 535 ++++++ .../media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 122 ++ .../media/platform/mtk-vcodec/mtk_vcodec_intr.c | 110 ++ .../media/platform/mtk-vcodec/mtk_vcodec_intr.h | 30 + drivers/media/platform/mtk-vcodec/mtk_vcodec_pm.h | 26 + .../media/platform/mtk-vcodec/mtk_vcodec_util.c | 106 ++ .../media/platform/mtk-vcodec/mtk_vcodec_util.h | 66 + drivers/media/platform/mtk-vcodec/vp8_enc/Makefile | 9 + .../platform/mtk-vcodec/vp8_enc/venc_vp8_if.c | 371 ++++ .../platform/mtk-vcodec/vp8_enc/venc_vp8_if.h | 48 + .../platform/mtk-vcodec/vp8_enc/venc_vp8_vpu.c | 245 +++ drivers/media/platform/mtk-vpu/Makefile | 1 + .../platform/mtk-vpu/h264_enc/venc_h264_vpu.h | 127 ++ .../media/platform/mtk-vpu/include/venc_ipi_msg.h | 212 +++ drivers/media/platform/mtk-vpu/mtk_vpu_core.c | 823 +++++++++ drivers/media/platform/mtk-vpu/mtk_vpu_core.h | 161 ++ .../media/platform/mtk-vpu/vp8_enc/venc_vp8_vpu.h | 119 ++ 36 files changed, 7107 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec.txt create mode 100644 Documentation/devicetree/bindings/media/mediatek-vpu.txt create mode 100644 drivers/media/platform/mtk-vcodec/Kconfig create mode 100644 drivers/media/platform/mtk-vcodec/Makefile create mode 100644 drivers/media/platform/mtk-vcodec/common/Makefile create mode 100644 drivers/media/platform/mtk-vcodec/common/venc_drv_if.c create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/Makefile create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.c create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.h create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_vpu.c create mode 100644 drivers/media/platform/mtk-vcodec/include/venc_drv_base.h create mode 100644 drivers/media/platform/mtk-vcodec/include/venc_drv_if.h create mode 100644 drivers/media/platform/mtk-vcodec/include/venc_ipi_msg.h create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.h create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.c create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.h create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_pm.h create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_util.c create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h create mode 100644 drivers/media/platform/mtk-vcodec/vp8_enc/Makefile create mode 100644 drivers/media/platform/mtk-vcodec/vp8_enc/venc_vp8_if.c create mode 100644 drivers/media/platform/mtk-vcodec/vp8_enc/venc_vp8_if.h create mode 100644 drivers/media/platform/mtk-vcodec/vp8_enc/venc_vp8_vpu.c create mode 100644 drivers/media/platform/mtk-vpu/Makefile create mode 100644 drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h create mode 100644 drivers/media/platform/mtk-vpu/include/venc_ipi_msg.h create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu_core.c create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu_core.h create mode 100644 drivers/media/platform/mtk-vpu/vp8_enc/venc_vp8_vpu.h -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Tiffany Lin <tiffany.lin@mediatek.com> |
|---|---|
| Date | 2015-11-17 14:00 +0100 |
| Subject | [RESEND RFC/PATCH 1/8] dt-bindings: Add a binding for Mediatek Video Processor Unit |
| Message-ID | <qvO1A-58e-15@gated-at.bofh.it> |
| In reply to | #1271154 |
From: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
Add a DT binding documentation of Video Processor Unit for the
MT8173 SoC from Mediatek.
Signed-off-by: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
---
.../devicetree/bindings/media/mediatek-vpu.txt | 27 ++++++++++++++++++++
1 file changed, 27 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/mediatek-vpu.txt
diff --git a/Documentation/devicetree/bindings/media/mediatek-vpu.txt b/Documentation/devicetree/bindings/media/mediatek-vpu.txt
new file mode 100644
index 0000000..99a4e5e
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek-vpu.txt
@@ -0,0 +1,27 @@
+* Mediatek Video Processor Unit
+
+Video Processor Unit is a HW video controller. It controls HW Codec including
+H.264/VP8/VP9 Decode, H.264/VP8 Encode and Image Processor (scale/rotate/color convert).
+
+Required properties:
+ - compatible: "mediatek,mt8173-vpu"
+ - reg: Must contain an entry for each entry in reg-names.
+ - reg-names: Must include the following entries:
+ "sram": SRAM base
+ "cfg_reg": Main configuration registers base
+ - interrupts: interrupt number to the cpu.
+ - clocks : clock name from clock manager
+ - clock-names: the clocks of the VPU H/W
+ - iommus : phandle and IOMMU spcifier for the IOMMU that serves the VPU.
+
+Example:
+ vpu: vpu@10020000 {
+ compatible = "mediatek,mt8173-vpu";
+ reg = <0 0x10020000 0 0x30000>,
+ <0 0x10050000 0 0x100>;
+ reg-names = "sram", "cfg_reg";
+ interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&topckgen TOP_SCP_SEL>;
+ clock-names = "main";
+ iommus = <&iommu M4U_PORT_VENC_RCPU>;
+ };
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2015-11-17 15:20 +0100 |
| Subject | Re: [RESEND RFC/PATCH 1/8] dt-bindings: Add a binding for Mediatek Video Processor Unit |
| Message-ID | <qvPh0-67E-5@gated-at.bofh.it> |
| In reply to | #1271155 |
On Tue, Nov 17, 2015 at 08:54:38PM +0800, Tiffany Lin wrote:
> From: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
>
> Add a DT binding documentation of Video Processor Unit for the
> MT8173 SoC from Mediatek.
>
> Signed-off-by: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
> ---
> .../devicetree/bindings/media/mediatek-vpu.txt | 27 ++++++++++++++++++++
> 1 file changed, 27 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/mediatek-vpu.txt
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek-vpu.txt b/Documentation/devicetree/bindings/media/mediatek-vpu.txt
> new file mode 100644
> index 0000000..99a4e5e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek-vpu.txt
> @@ -0,0 +1,27 @@
> +* Mediatek Video Processor Unit
> +
> +Video Processor Unit is a HW video controller. It controls HW Codec including
> +H.264/VP8/VP9 Decode, H.264/VP8 Encode and Image Processor (scale/rotate/color convert).
> +
> +Required properties:
> + - compatible: "mediatek,mt8173-vpu"
> + - reg: Must contain an entry for each entry in reg-names.
> + - reg-names: Must include the following entries:
> + "sram": SRAM base
> + "cfg_reg": Main configuration registers base
> + - interrupts: interrupt number to the cpu.
> + - clocks : clock name from clock manager
> + - clock-names: the clocks of the VPU H/W
You need to explicitly define the set of clock-names you expect here.
Mark.
> + - iommus : phandle and IOMMU spcifier for the IOMMU that serves the VPU.
> +
> +Example:
> + vpu: vpu@10020000 {
> + compatible = "mediatek,mt8173-vpu";
> + reg = <0 0x10020000 0 0x30000>,
> + <0 0x10050000 0 0x100>;
> + reg-names = "sram", "cfg_reg";
> + interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&topckgen TOP_SCP_SEL>;
> + clock-names = "main";
> + iommus = <&iommu M4U_PORT_VENC_RCPU>;
> + };
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | andrew-ct chen <andrew-ct.chen@mediatek.com> |
|---|---|
| Date | 2015-11-19 03:50 +0100 |
| Subject | Re: [RESEND RFC/PATCH 1/8] dt-bindings: Add a binding for Mediatek Video Processor Unit |
| Message-ID | <qwnsl-3hD-7@gated-at.bofh.it> |
| In reply to | #1271261 |
On Tue, 2015-11-17 at 14:13 +0000, Mark Rutland wrote:
> On Tue, Nov 17, 2015 at 08:54:38PM +0800, Tiffany Lin wrote:
> > From: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
> >
> > Add a DT binding documentation of Video Processor Unit for the
> > MT8173 SoC from Mediatek.
> >
> > Signed-off-by: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
> > ---
> > .../devicetree/bindings/media/mediatek-vpu.txt | 27 ++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/mediatek-vpu.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/mediatek-vpu.txt b/Documentation/devicetree/bindings/media/mediatek-vpu.txt
> > new file mode 100644
> > index 0000000..99a4e5e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/mediatek-vpu.txt
> > @@ -0,0 +1,27 @@
> > +* Mediatek Video Processor Unit
> > +
> > +Video Processor Unit is a HW video controller. It controls HW Codec including
> > +H.264/VP8/VP9 Decode, H.264/VP8 Encode and Image Processor (scale/rotate/color convert).
> > +
> > +Required properties:
> > + - compatible: "mediatek,mt8173-vpu"
> > + - reg: Must contain an entry for each entry in reg-names.
> > + - reg-names: Must include the following entries:
> > + "sram": SRAM base
> > + "cfg_reg": Main configuration registers base
> > + - interrupts: interrupt number to the cpu.
> > + - clocks : clock name from clock manager
> > + - clock-names: the clocks of the VPU H/W
>
> You need to explicitly define the set of clock-names you expect here.
>
> Mark.
Sorry, only one clock to enable VPU hardware.
We will modify to
- clocks : must contain one entry for each clock-names.
- clock-names : must be "main", It is the main clock of VPU.
Thanks..
>
> > + - iommus : phandle and IOMMU spcifier for the IOMMU that serves the VPU.
> > +
> > +Example:
> > + vpu: vpu@10020000 {
> > + compatible = "mediatek,mt8173-vpu";
> > + reg = <0 0x10020000 0 0x30000>,
> > + <0 0x10050000 0 0x100>;
> > + reg-names = "sram", "cfg_reg";
> > + interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&topckgen TOP_SCP_SEL>;
> > + clock-names = "main";
> > + iommus = <&iommu M4U_PORT_VENC_RCPU>;
> > + };
> > --
> > 1.7.9.5
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tiffany Lin <tiffany.lin@mediatek.com> |
|---|---|
| Date | 2015-11-17 14:00 +0100 |
| Subject | [RESEND RFC/PATCH 2/8] arm64: dts: mediatek: Add node for Mediatek Video Processor Unit |
| Message-ID | <qvO1A-58e-23@gated-at.bofh.it> |
| In reply to | #1271154 |
From: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
add VPU drivers for MT8173
Signed-off-by: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 4dd5f93..098c15e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -258,6 +258,17 @@
clock-names = "spi", "wrap";
};
+ vpu: vpu@10020000 {
+ compatible = "mediatek,mt8173-vpu";
+ reg = <0 0x10020000 0 0x30000>,
+ <0 0x10050000 0 0x100>;
+ reg-names = "sram", "cfg_reg";
+ interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&topckgen CLK_TOP_SCP_SEL>;
+ clock-names = "main";
+ iommus = <&iommu M4U_LARB3_ID M4U_PORT_VENC_RCPU>;
+ };
+
sysirq: intpol-controller@10200620 {
compatible = "mediatek,mt8173-sysirq",
"mediatek,mt6577-sysirq";
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tiffany Lin <tiffany.lin@mediatek.com> |
|---|---|
| Date | 2015-11-17 14:00 +0100 |
| Subject | [RESEND RFC/PATCH 8/8] media: platform: mtk-vcodec: Add Mediatek H264 Video Encoder Driver |
| Message-ID | <qvO1A-58e-29@gated-at.bofh.it> |
| In reply to | #1271154 |
Signed-off-by: Daniel Hsiao <daniel.hsiao@mediatek.com>
---
drivers/media/platform/mtk-vcodec/Makefile | 2 +-
drivers/media/platform/mtk-vcodec/common/Makefile | 4 +-
.../media/platform/mtk-vcodec/common/venc_drv_if.c | 3 +
.../media/platform/mtk-vcodec/h264_enc/Makefile | 9 +
.../platform/mtk-vcodec/h264_enc/venc_h264_if.c | 529 ++++++++++++++++++++
.../platform/mtk-vcodec/h264_enc/venc_h264_if.h | 53 ++
.../platform/mtk-vcodec/h264_enc/venc_h264_vpu.c | 341 +++++++++++++
7 files changed, 939 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/Makefile
create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.c
create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.h
create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_vpu.c
diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile
index b881a8b..d2189f7 100644
--- a/drivers/media/platform/mtk-vcodec/Makefile
+++ b/drivers/media/platform/mtk-vcodec/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk_vcodec_intr.o \
mtk_vcodec_enc.o \
mtk_vcodec_enc_pm.o
-obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += common/ vp8_enc/
+obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += common/ vp8_enc/ h264_enc/
ccflags-y += -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
-I$(srctree)/drivers/media/platform/mtk-vcodec \
diff --git a/drivers/media/platform/mtk-vcodec/common/Makefile b/drivers/media/platform/mtk-vcodec/common/Makefile
index 71ae856..b33d48d 100644
--- a/drivers/media/platform/mtk-vcodec/common/Makefile
+++ b/drivers/media/platform/mtk-vcodec/common/Makefile
@@ -6,5 +6,7 @@ ccflags-y += \
-I$(srctree)/drivers/media/platform/mtk-vcodec \
-I$(srctree)/drivers/media/platform/mtk-vcodec/include \
-I$(srctree)/drivers/media/platform/mtk-vcodec/vp8_enc \
+ -I$(srctree)/drivers/media/platform/mtk-vcodec/h264_enc \
-I$(srctree)/drivers/media/platform/mtk-vpu \
- -I$(srctree)/drivers/media/platform/mtk-vpu/vp8_enc
+ -I$(srctree)/drivers/media/platform/mtk-vpu/vp8_enc \
+ -I$(srctree)/drivers/media/platform/mtk-vpu/h264_enc
diff --git a/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
index e9be186..930254b 100644
--- a/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
+++ b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
@@ -27,6 +27,7 @@
#include "venc_drv_if.h"
#include "venc_drv_base.h"
#include "venc_vp8_if.h"
+#include "venc_h264_if.h"
int venc_if_create(void *ctx, unsigned int fourcc, unsigned long *handle)
{
@@ -48,6 +49,8 @@ int venc_if_create(void *ctx, unsigned int fourcc, unsigned long *handle)
h->enc_if = get_vp8_enc_comm_if();
break;
case V4L2_PIX_FMT_H264:
+ h->enc_if = get_h264_enc_comm_if();
+ break;
default:
mtk_vcodec_err(h, "invalid format %s", str);
goto err_out;
diff --git a/drivers/media/platform/mtk-vcodec/h264_enc/Makefile b/drivers/media/platform/mtk-vcodec/h264_enc/Makefile
new file mode 100644
index 0000000..6559908
--- /dev/null
+++ b/drivers/media/platform/mtk-vcodec/h264_enc/Makefile
@@ -0,0 +1,9 @@
+obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += venc_h264_if.o venc_h264_vpu.o
+
+ccflags-y += \
+ -I$(srctree)/include/ \
+ -I$(srctree)/drivers/media/platform/mtk-vcodec/ \
+ -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
+ -I$(srctree)/drivers/media/platform/mtk-vcodec/h264_enc \
+ -I$(srctree)/drivers/media/platform/mtk-vpu \
+ -I$(srctree)/drivers/media/platform/mtk-vpu/h264_enc
diff --git a/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.c b/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.c
new file mode 100644
index 0000000..c880865
--- /dev/null
+++ b/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.c
@@ -0,0 +1,529 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Jungchang Tsao <jungchang.tsao@mediatek.com>
+ * Daniel Hsiao <daniel.hsiao@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+
+#include "mtk_vcodec_drv.h"
+#include "mtk_vcodec_util.h"
+#include "mtk_vcodec_intr.h"
+#include "mtk_vcodec_enc.h"
+#include "mtk_vcodec_pm.h"
+#include "mtk_vpu_core.h"
+
+#include "venc_h264_if.h"
+#include "venc_h264_vpu.h"
+
+#define h264_write_reg(h, addr, val) writel(val, h->hw_base + addr)
+#define h264_read_reg(h, addr) readl(h->hw_base + addr)
+
+#define VENC_PIC_BITSTREAM_BYTE_CNT 0x0098
+
+enum venc_h264_irq_status {
+ H264_IRQ_STATUS_ENC_SPS_INT = (1 << 0),
+ H264_IRQ_STATUS_ENC_PPS_INT = (1 << 1),
+ H264_IRQ_STATUS_ENC_FRM_INT = (1 << 2),
+};
+
+static int h264_enc_alloc_work_buf(struct venc_h264_handle *handle)
+{
+ int i, j;
+ int ret = 0;
+ struct venc_h264_vpu_buf *wb = handle->vpu_inst.drv->work_bufs;
+
+ mtk_vcodec_debug_enter(handle);
+
+ for (i = 0; i < VENC_H264_VPU_WORK_BUF_MAX; i++) {
+ /*
+ * This 'wb' structure is set by VPU side and shared to AP for
+ * buffer allocation and physical addr mapping. For most of
+ * the buffers, AP will allocate the buffer according to 'size'
+ * field and store the physical addr in 'pa' field. There are two
+ * exceptions:
+ * (1) RC_CODE buffer, it's pre-allocated in the VPU side, and
+ * save the VPU addr in the 'vpua' field. The AP will translate
+ * the VPU addr to the corresponding physical addr and store
+ * in 'pa' field for reg setting in VPU side.
+ * (2) SKIP_FRAME buffer, it's pre-allocated in the VPU side, and
+ * save the VPU addr in the 'vpua' field. The AP will translate
+ * the VPU addr to the corresponding AP side virtual address and
+ * do some memcpy access to move to bitstream buffer assigned
+ * by v4l2 layer.
+ */
+ if (i == VENC_H264_VPU_WORK_BUF_RC_CODE) {
+ handle->work_bufs[i].size = wb[i].size;
+ handle->work_bufs[i].va = vpu_mapping_dm_addr(
+ handle->dev, (uintptr_t *)(unsigned long)
+ wb[i].vpua);
+ handle->work_bufs[i].dma_addr =
+ (dma_addr_t)vpu_mapping_iommu_dm_addr(
+ handle->dev, (uintptr_t *)(unsigned long)
+ wb[i].vpua);
+ wb[i].pa = handle->work_bufs[i].dma_addr;
+ } else if (i == VENC_H264_VPU_WORK_BUF_SKIP_FRAME) {
+ handle->work_bufs[i].size = wb[i].size;
+ handle->work_bufs[i].va = vpu_mapping_dm_addr(
+ handle->dev, (uintptr_t *)(unsigned long)
+ wb[i].vpua);
+ handle->work_bufs[i].dma_addr = 0;
+ wb[i].pa = handle->work_bufs[i].dma_addr;
+ } else {
+ handle->work_bufs[i].size = wb[i].size;
+ if (mtk_vcodec_mem_alloc(handle->ctx,
+ &handle->work_bufs[i])) {
+ mtk_vcodec_err(handle, "cannot allocate buf %d", i);
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+ wb[i].pa = handle->work_bufs[i].dma_addr;
+ }
+ mtk_vcodec_debug(handle, "buf[%d] va=0x%p pa=0x%p size=0x%lx", i,
+ handle->work_bufs[i].va,
+ (void *)handle->work_bufs[i].dma_addr,
+ handle->work_bufs[i].size);
+ }
+
+ /* the pps_buf is used by AP side only */
+ handle->pps_buf.size = 128;
+ if (mtk_vcodec_mem_alloc(handle->ctx,
+ &handle->pps_buf)) {
+ mtk_vcodec_err(handle, "cannot allocate pps_buf");
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+ mtk_vcodec_debug_leave(handle);
+
+ return ret;
+
+err_alloc:
+ for (j = 0; j < i; j++) {
+ if ((j != VENC_H264_VPU_WORK_BUF_RC_CODE) &&
+ (j != VENC_H264_VPU_WORK_BUF_SKIP_FRAME))
+ mtk_vcodec_mem_free(handle->ctx, &handle->work_bufs[j]);
+ }
+
+ return ret;
+}
+
+static void h264_enc_free_work_buf(struct venc_h264_handle *handle)
+{
+ int i;
+
+ mtk_vcodec_debug_enter(handle);
+ for (i = 0; i < VENC_H264_VPU_WORK_BUF_MAX; i++) {
+ if ((i != VENC_H264_VPU_WORK_BUF_RC_CODE) &&
+ (i != VENC_H264_VPU_WORK_BUF_SKIP_FRAME))
+ mtk_vcodec_mem_free(handle->ctx, &handle->work_bufs[i]);
+ }
+ mtk_vcodec_mem_free(handle->ctx, &handle->pps_buf);
+ mtk_vcodec_debug_leave(handle);
+}
+
+static unsigned int h264_enc_wait_venc_done(struct venc_h264_handle *handle)
+{
+ unsigned int irq_status = 0;
+ struct mtk_vcodec_ctx *pctx = handle->ctx;
+
+ mtk_vcodec_debug_enter(handle);
+ mtk_vcodec_wait_for_done_ctx(pctx, MTK_INST_IRQ_RECEIVED, 1000, true);
+ irq_status = pctx->irq_status;
+ mtk_vcodec_debug(handle, "irq_status %x <-", irq_status);
+
+ return irq_status;
+}
+
+static int h264_encode_sps(struct venc_h264_handle *handle,
+ struct mtk_vcodec_mem *bs_buf,
+ unsigned int *bs_size)
+{
+ unsigned int irq_status;
+
+ mtk_vcodec_debug_enter(handle);
+
+ if (h264_enc_vpu_encode(handle, H264_BS_MODE_SPS, NULL,
+ bs_buf, bs_size)) {
+ mtk_vcodec_err(handle, "h264_enc_vpu_encode sps failed");
+ return -EINVAL;
+ }
+
+ irq_status = h264_enc_wait_venc_done(handle);
+ if (irq_status != H264_IRQ_STATUS_ENC_SPS_INT) {
+ mtk_vcodec_err(handle, "expect irq status %d",
+ H264_IRQ_STATUS_ENC_SPS_INT);
+ return -EINVAL;
+ }
+
+ *bs_size = h264_read_reg(handle, VENC_PIC_BITSTREAM_BYTE_CNT);
+ mtk_vcodec_debug(handle, "bs size %d <-", *bs_size);
+
+ return 0;
+}
+
+static int h264_encode_pps(struct venc_h264_handle *handle,
+ struct mtk_vcodec_mem *bs_buf,
+ unsigned int *bs_size)
+{
+ unsigned int irq_status;
+
+ mtk_vcodec_debug_enter(handle);
+
+ if (h264_enc_vpu_encode(handle, H264_BS_MODE_PPS, NULL,
+ bs_buf, bs_size)) {
+ mtk_vcodec_err(handle, "h264_enc_vpu_encode pps failed");
+ return -EINVAL;
+ }
+
+ irq_status = h264_enc_wait_venc_done(handle);
+ if (irq_status != H264_IRQ_STATUS_ENC_PPS_INT) {
+ mtk_vcodec_err(handle, "expect irq status %d",
+ H264_IRQ_STATUS_ENC_PPS_INT);
+ return -EINVAL;
+ }
+
+ *bs_size = h264_read_reg(handle, VENC_PIC_BITSTREAM_BYTE_CNT);
+ mtk_vcodec_debug(handle, "bs size %d <-", *bs_size);
+
+ return 0;
+}
+
+static int h264_encode_frame(struct venc_h264_handle *handle,
+ struct venc_frm_buf *frm_buf,
+ struct mtk_vcodec_mem *bs_buf,
+ unsigned int *bs_size)
+{
+ unsigned int irq_status;
+
+ mtk_vcodec_debug_enter(handle);
+
+ if (h264_enc_vpu_encode(handle, H264_BS_MODE_FRAME, frm_buf,
+ bs_buf, bs_size)) {
+ mtk_vcodec_err(handle, "h264_enc_vpu_encode frame failed");
+ return -EINVAL;
+ }
+
+ /*
+ * skip frame case: The skip frame buffer is composed by vpu side only,
+ * it does not trigger the hw, so skip the wait interrupt operation.
+ */
+ if (!handle->vpu_inst.wait_int) {
+ ++handle->frm_cnt;
+ return 0;
+ }
+
+ irq_status = h264_enc_wait_venc_done(handle);
+ if (irq_status != H264_IRQ_STATUS_ENC_FRM_INT) {
+ mtk_vcodec_err(handle, "irq_status=%d failed", irq_status);
+ return -EINVAL;
+ }
+
+ *bs_size = h264_read_reg(handle,
+ VENC_PIC_BITSTREAM_BYTE_CNT);
+ ++handle->frm_cnt;
+ mtk_vcodec_debug(handle, "frm %d bs size %d key_frm %d <-",
+ handle->frm_cnt,
+ *bs_size, handle->is_key_frm);
+
+ return 0;
+}
+
+static void h264_encode_filler(struct venc_h264_handle *handle, void *buf, int size)
+{
+ unsigned char *p = buf;
+
+ *p++ = 0x0;
+ *p++ = 0x0;
+ *p++ = 0x0;
+ *p++ = 0x1;
+ *p++ = 0xc;
+ size -= 5;
+ while (size) {
+ *p++ = 0xff;
+ size -= 1;
+ }
+}
+
+int h264_enc_init(struct mtk_vcodec_ctx *ctx, unsigned long *handle)
+{
+ struct venc_h264_handle *h;
+
+ h = kzalloc(sizeof(*h), GFP_KERNEL);
+ if (!h)
+ return -ENOMEM;
+
+ h->ctx = ctx;
+ h->dev = mtk_vcodec_get_plat_dev(ctx);
+ h->hw_base = mtk_vcodec_get_reg_addr(h->ctx, VENC_SYS);
+
+ if (h264_enc_vpu_init(h)) {
+ mtk_vcodec_err(h, "h264_enc_init failed");
+ return -EINVAL;
+ }
+
+ (*handle) = (unsigned long)h;
+
+ return 0;
+}
+
+int h264_enc_encode(unsigned long handle,
+ enum venc_start_opt opt,
+ struct venc_frm_buf *frm_buf,
+ struct mtk_vcodec_mem *bs_buf,
+ struct venc_done_result *result)
+{
+ int ret = 0;
+ struct venc_h264_handle *h = (struct venc_h264_handle *)handle;
+
+ mtk_vcodec_debug(h, "opt %d ->", opt);
+
+ switch (opt) {
+ case VENC_START_OPT_ENCODE_SEQUENCE_HEADER: {
+ unsigned int bs_size_sps;
+ unsigned int bs_size_pps;
+
+ memset(bs_buf->va, 0x38, 20);
+ if (h264_encode_sps(h, bs_buf, &bs_size_sps)) {
+ mtk_vcodec_err(h, "h264_encode_sps failed");
+ ret = -EINVAL;
+ goto encode_err;
+ }
+ memset(h->pps_buf.va, 0x49, 20);
+
+ if (h264_encode_pps(h, &h->pps_buf, &bs_size_pps)) {
+ mtk_vcodec_err(h, "h264_encode_pps failed");
+ ret = -EINVAL;
+ goto encode_err;
+ }
+
+ memcpy(bs_buf->va + bs_size_sps,
+ h->pps_buf.va,
+ bs_size_pps);
+ result->bs_size = bs_size_sps + bs_size_pps;
+ result->is_key_frm = false;
+ }
+ break;
+
+ case VENC_START_OPT_ENCODE_FRAME:
+ if (h->prepend_hdr) {
+ int hdr_sz;
+ int hdr_sz_ext;
+ int bs_alignment = 128;
+ int filler_sz = 0;
+ struct mtk_vcodec_mem tmp_bs_buf;
+ unsigned int bs_size_sps;
+ unsigned int bs_size_pps;
+ unsigned int bs_size_frm;
+
+ mtk_vcodec_debug(h,
+ "h264_encode_frame prepend SPS/PPS");
+ if (h264_encode_sps(h, bs_buf, &bs_size_sps)) {
+ mtk_vcodec_err(h,
+ "h264_encode_sps failed");
+ ret = -EINVAL;
+ goto encode_err;
+ }
+
+ if (h264_encode_pps(h, &h->pps_buf, &bs_size_pps)) {
+ mtk_vcodec_err(h,
+ "h264_encode_pps failed");
+ ret = -EINVAL;
+ goto encode_err;
+ }
+ memcpy(bs_buf->va + bs_size_sps,
+ h->pps_buf.va,
+ bs_size_pps);
+
+ hdr_sz = bs_size_sps + bs_size_pps;
+ hdr_sz_ext = (hdr_sz & (bs_alignment - 1));
+ if (hdr_sz_ext) {
+ filler_sz = bs_alignment - hdr_sz_ext;
+ if (hdr_sz_ext + 5 > bs_alignment)
+ filler_sz += bs_alignment;
+ h264_encode_filler(
+ h, bs_buf->va + hdr_sz,
+ filler_sz);
+ }
+
+ tmp_bs_buf.va = bs_buf->va + hdr_sz +
+ filler_sz;
+ tmp_bs_buf.dma_addr = bs_buf->dma_addr + hdr_sz +
+ filler_sz;
+ tmp_bs_buf.size = bs_buf->size -
+ (hdr_sz + filler_sz);
+
+ if (h264_encode_frame(h, frm_buf,
+ &tmp_bs_buf,
+ &bs_size_frm)) {
+ mtk_vcodec_err(h,
+ "h264_encode_frame failed");
+ ret = -EINVAL;
+ goto encode_err;
+ }
+
+ result->bs_size = hdr_sz + filler_sz + bs_size_frm;
+ mtk_vcodec_debug(h,
+ "hdr %d filler %d frame %d bs %d",
+ hdr_sz, filler_sz, bs_size_frm,
+ result->bs_size);
+
+ h->prepend_hdr = 0;
+ } else {
+ if (h264_encode_frame(h, frm_buf, bs_buf,
+ &result->bs_size)) {
+ mtk_vcodec_err(h,
+ "h264_encode_frame failed");
+ ret = -EINVAL;
+ goto encode_err;
+ }
+ }
+ result->is_key_frm = h->is_key_frm;
+ break;
+
+ default:
+ mtk_vcodec_err(h, "venc_start_opt %d not supported",
+ opt);
+ ret = -EINVAL;
+ break;
+ }
+
+encode_err:
+ if (ret)
+ result->msg = VENC_MESSAGE_ERR;
+ else
+ result->msg = VENC_MESSAGE_OK;
+
+ mtk_vcodec_debug(h, "opt %d <-", opt);
+ return ret;
+}
+
+int h264_enc_set_param(unsigned long handle,
+ enum venc_set_param_type type, void *in)
+{
+ int ret = 0;
+ struct venc_h264_handle *h = (struct venc_h264_handle *)handle;
+ struct venc_enc_prm *enc_prm;
+
+ mtk_vcodec_debug(h, "->type=%d", type);
+
+ switch (type) {
+ case VENC_SET_PARAM_ENC:
+ enc_prm = in;
+ if (h264_enc_vpu_set_param(h,
+ VENC_SET_PARAM_ENC,
+ enc_prm)) {
+ ret = -EINVAL;
+ break;
+ }
+ if (h->work_buf_alloc == 0) {
+ if (h264_enc_alloc_work_buf(h)) {
+ mtk_vcodec_err(h,
+ "h264_enc_alloc_work_buf failed");
+ ret = -ENOMEM;
+ } else {
+ h->work_buf_alloc = 1;
+ }
+ }
+ break;
+
+ case VENC_SET_PARAM_FORCE_INTRA:
+ if (h264_enc_vpu_set_param(h,
+ VENC_SET_PARAM_FORCE_INTRA, 0)) {
+ mtk_vcodec_err(h, "force intra failed");
+ ret = -EINVAL;
+ }
+ break;
+
+ case VENC_SET_PARAM_ADJUST_BITRATE:
+ enc_prm = in;
+ if (h264_enc_vpu_set_param(h, VENC_SET_PARAM_ADJUST_BITRATE,
+ &enc_prm->bitrate)) {
+ mtk_vcodec_err(h, "adjust bitrate failed");
+ ret = -EINVAL;
+ }
+ break;
+
+ case VENC_SET_PARAM_ADJUST_FRAMERATE:
+ enc_prm = in;
+ if (h264_enc_vpu_set_param(h, VENC_SET_PARAM_ADJUST_FRAMERATE,
+ &enc_prm->frm_rate)) {
+ mtk_vcodec_err(h, "adjust frame rate failed");
+ ret = -EINVAL;
+ }
+ break;
+
+ case VENC_SET_PARAM_I_FRAME_INTERVAL:
+ if (h264_enc_vpu_set_param(h,
+ VENC_SET_PARAM_I_FRAME_INTERVAL,
+ in)) {
+ mtk_vcodec_err(h, "set I frame interval failed");
+ ret = -EINVAL;
+ }
+ break;
+
+ case VENC_SET_PARAM_SKIP_FRAME:
+ if (h264_enc_vpu_set_param(h, VENC_SET_PARAM_SKIP_FRAME, 0)) {
+ mtk_vcodec_err(h, "skip frame failed");
+ ret = -EINVAL;
+ }
+ break;
+
+ case VENC_SET_PARAM_PREPEND_HEADER:
+ h->prepend_hdr = 1;
+ mtk_vcodec_debug(h, "set prepend header mode");
+ break;
+
+ default:
+ mtk_vcodec_err(h, "type %d not supported", type);
+ ret = -EINVAL;
+ break;
+ }
+
+ mtk_vcodec_debug_leave(h);
+ return ret;
+}
+
+int h264_enc_deinit(unsigned long handle)
+{
+ int ret = 0;
+ struct venc_h264_handle *h = (struct venc_h264_handle *)handle;
+
+ mtk_vcodec_debug_enter(h);
+
+ if (h264_enc_vpu_deinit(h)) {
+ mtk_vcodec_err(h, "h264_enc_vpu_deinit failed");
+ ret = -EINVAL;
+ }
+
+ if (h->work_buf_alloc)
+ h264_enc_free_work_buf(h);
+
+ mtk_vcodec_debug_leave(h);
+ kfree(h);
+
+ return ret;
+}
+
+struct venc_common_if venc_h264_if = {
+ h264_enc_init,
+ h264_enc_encode,
+ h264_enc_set_param,
+ h264_enc_deinit,
+};
+
+struct venc_common_if *get_h264_enc_comm_if(void)
+{
+ return &venc_h264_if;
+}
diff --git a/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.h b/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.h
new file mode 100644
index 0000000..4deb7d4
--- /dev/null
+++ b/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Jungchang Tsao <jungchang.tsao@mediatek.com>
+ * Daniel Hsiao <daniel.hsiao@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _VENC_H264_IF_H_
+#define _VENC_H264_IF_H_
+
+#include "venc_drv_base.h"
+#include "venc_h264_vpu.h"
+
+/*
+ * struct venc_h264_handle - h264 encoder AP driver handle
+ * @hw_base: h264 encoder hardware register base
+ * @work_bufs: working buffer
+ * @pps_buf: buffer to store the pps bitstream
+ * @work_buf_alloc: working buffer allocated flag
+ * @frm_cnt: encoded frame count
+ * @prepend_hdr: when the v4l2 layer send VENC_SET_PARAM_PREPEND_HEADER cmd
+ * through h264_enc_set_param interface, it will set this flag and prepend the
+ * sps/pps in h264_enc_encode function.
+ * @is_key_frm: key frame flag
+ * @vpu_inst: VPU instance to exchange information between AP and VPU
+ * @ctx: context for v4l2 layer integration
+ * @dev: device for v4l2 layer integration
+ */
+struct venc_h264_handle {
+ void __iomem *hw_base;
+ struct mtk_vcodec_mem work_bufs[VENC_H264_VPU_WORK_BUF_MAX];
+ struct mtk_vcodec_mem pps_buf;
+ unsigned int work_buf_alloc;
+ unsigned int frm_cnt;
+ unsigned int prepend_hdr;
+ unsigned int is_key_frm;
+ struct venc_h264_vpu_inst vpu_inst;
+ void *ctx;
+ struct platform_device *dev;
+};
+
+struct venc_common_if *get_h264_enc_comm_if(void);
+
+#endif
diff --git a/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_vpu.c b/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_vpu.c
new file mode 100644
index 0000000..5715971
--- /dev/null
+++ b/drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_vpu.c
@@ -0,0 +1,341 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Jungchang Tsao <jungchang.tsao@mediatek.com>
+ * Daniel Hsiao <daniel.hsiao@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "mtk_vpu_core.h"
+
+#include "venc_h264_if.h"
+#include "venc_h264_vpu.h"
+#include "venc_ipi_msg.h"
+
+static unsigned int h264_get_profile(unsigned int profile)
+{
+ /* (Baseline=66, Main=77, High=100) */
+ switch (profile) {
+ case V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE:
+ return 66;
+ case V4L2_MPEG_VIDEO_H264_PROFILE_MAIN:
+ return 77;
+ case V4L2_MPEG_VIDEO_H264_PROFILE_HIGH:
+ return 100;
+ default:
+ return 100;
+ }
+}
+
+static unsigned int h264_get_level(unsigned int level)
+{
+ /* (UpTo4.1(HighProfile)) */
+ switch (level) {
+ case V4L2_MPEG_VIDEO_H264_LEVEL_1_0:
+ return 10;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_1_1:
+ return 11;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_1_2:
+ return 12;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_1_3:
+ return 13;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_2_0:
+ return 20;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_2_1:
+ return 21;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_2_2:
+ return 22;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_3_0:
+ return 30;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_3_1:
+ return 31;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_3_2:
+ return 32;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_4_0:
+ return 40;
+ case V4L2_MPEG_VIDEO_H264_LEVEL_4_1:
+ return 41;
+ default:
+ return 31;
+ }
+}
+
+static void handle_h264_enc_init_msg(struct venc_h264_handle *hndl, void *data)
+{
+ struct venc_vpu_ipi_msg_init *msg = data;
+
+ hndl->vpu_inst.id = msg->inst_id;
+ hndl->vpu_inst.drv = (struct venc_h264_vpu_drv *)vpu_mapping_dm_addr(
+ hndl->dev, (uintptr_t *)(unsigned long)msg->inst_id);
+}
+
+static void handle_h264_enc_encode_msg(struct venc_h264_handle *hndl,
+ void *data)
+{
+ struct venc_vpu_ipi_msg_enc *msg = data;
+
+ hndl->vpu_inst.state = msg->state;
+ hndl->vpu_inst.bs_size = msg->bs_size;
+ hndl->is_key_frm = msg->key_frame;
+}
+
+static void h264_enc_vpu_ipi_handler(void *data, unsigned int len, void *priv)
+{
+ struct venc_vpu_ipi_msg_common *msg = data;
+ struct venc_h264_handle *hndl = (struct venc_h264_handle *)msg->venc_inst;
+
+ mtk_vcodec_debug_enter(hndl);
+
+ mtk_vcodec_debug(hndl, "msg_id %x hndl %p status %d",
+ msg->msg_id, hndl, msg->status);
+
+ switch (msg->msg_id) {
+ case VPU_IPIMSG_H264_ENC_INIT_DONE:
+ handle_h264_enc_init_msg(hndl, data);
+ break;
+ case VPU_IPIMSG_H264_ENC_SET_PARAM_DONE:
+ break;
+ case VPU_IPIMSG_H264_ENC_ENCODE_DONE:
+ handle_h264_enc_encode_msg(hndl, data);
+ break;
+ case VPU_IPIMSG_H264_ENC_DEINIT_DONE:
+ break;
+ default:
+ mtk_vcodec_err(hndl, "unknown msg id %x", msg->msg_id);
+ break;
+ }
+
+ hndl->vpu_inst.signaled = 1;
+ hndl->vpu_inst.failure = (msg->status != VENC_IPI_MSG_STATUS_OK);
+ wake_up_interruptible(&hndl->vpu_inst.wq_hd);
+
+ mtk_vcodec_debug_leave(hndl);
+}
+
+static int h264_enc_vpu_wait_ack(struct venc_h264_handle *hndl,
+ unsigned int timeout_ms)
+{
+ int ret;
+
+ mtk_vcodec_debug_enter(hndl);
+
+ ret = wait_event_interruptible_timeout(hndl->vpu_inst.wq_hd,
+ hndl->vpu_inst.signaled == 1,
+ msecs_to_jiffies(timeout_ms));
+ if (0 == ret) {
+ mtk_vcodec_err(hndl, "wait vpu ack time out !");
+ return -EINVAL;
+ }
+ if (-ERESTARTSYS == ret) {
+ mtk_vcodec_err(hndl,
+ "wait vpu ack interrupted by a signal");
+ return -EINVAL;
+ }
+
+ hndl->vpu_inst.signaled = 0;
+
+ mtk_vcodec_debug_leave(hndl);
+ return 0;
+}
+
+static int h264_enc_vpu_send_msg(struct venc_h264_handle *hndl, void *msg,
+ int len, int wait_ack)
+{
+ int status;
+
+ mtk_vcodec_debug_enter(hndl);
+
+ status = vpu_ipi_send(hndl->dev, IPI_VENC_H264, msg, len, 1);
+ if (status) {
+ mtk_vcodec_err(hndl, "vpu_ipi_send msg %x len %d fail %d",
+ *(unsigned int *)msg, len, status);
+ return -EINVAL;
+ }
+ mtk_vcodec_debug(hndl, "vpu_ipi_send msg %x success",
+ *(unsigned int *)msg);
+
+ if (wait_ack && h264_enc_vpu_wait_ack(hndl, 2000)) {
+ mtk_vcodec_err(hndl, "h264_enc_vpu_wait_ack failed");
+ return -EINVAL;
+ }
+
+ mtk_vcodec_debug_leave(hndl);
+ return 0;
+}
+
+int h264_enc_vpu_init(void *handle)
+{
+ int status;
+ struct venc_h264_handle *hndl = handle;
+ struct venc_ap_ipi_msg_init out;
+
+ mtk_vcodec_debug_enter(hndl);
+
+ init_waitqueue_head(&hndl->vpu_inst.wq_hd);
+ hndl->vpu_inst.signaled = 0;
+ hndl->vpu_inst.failure = 0;
+
+ status = vpu_ipi_register(hndl->dev, IPI_VENC_H264,
+ h264_enc_vpu_ipi_handler,
+ "h264_enc", NULL);
+ if (status) {
+ mtk_vcodec_err(hndl, "vpu_ipi_register fail %d", status);
+ return -EINVAL;
+ }
+ mtk_vcodec_debug(hndl, "vpu_ipi_register success");
+
+ out.msg_id = AP_IPIMSG_H264_ENC_INIT;
+ out.venc_inst = (unsigned long)hndl;
+ if (h264_enc_vpu_send_msg(hndl, &out, sizeof(out), 1) ||
+ hndl->vpu_inst.failure) {
+ mtk_vcodec_err(hndl, "AP_IPIMSG_H264_ENC_INIT failed");
+ return -EINVAL;
+ }
+
+ mtk_vcodec_debug_leave(hndl);
+
+ return 0;
+}
+
+int h264_enc_vpu_set_param(void *handle, unsigned int id, void *param)
+{
+ struct venc_h264_handle *hndl = handle;
+ struct venc_ap_ipi_msg_set_param out;
+
+ mtk_vcodec_debug(hndl, "id %d ->", id);
+
+ out.msg_id = AP_IPIMSG_H264_ENC_SET_PARAM;
+ out.inst_id = hndl->vpu_inst.id;
+ out.param_id = id;
+ switch (id) {
+ case VENC_SET_PARAM_ENC: {
+ struct venc_enc_prm *enc_param = param;
+
+ hndl->vpu_inst.drv->config.input_fourcc = enc_param->input_fourcc;
+ hndl->vpu_inst.drv->config.bitrate = enc_param->bitrate;
+ hndl->vpu_inst.drv->config.pic_w = enc_param->width;
+ hndl->vpu_inst.drv->config.pic_h = enc_param->height;
+ hndl->vpu_inst.drv->config.buf_w = enc_param->buf_width;
+ hndl->vpu_inst.drv->config.buf_h = enc_param->buf_height;
+ hndl->vpu_inst.drv->config.intra_period =
+ enc_param->intra_period;
+ hndl->vpu_inst.drv->config.framerate = enc_param->frm_rate;
+ hndl->vpu_inst.drv->config.profile =
+ h264_get_profile(enc_param->h264_profile);
+ hndl->vpu_inst.drv->config.level =
+ h264_get_level(enc_param->h264_level);
+ hndl->vpu_inst.drv->config.wfd = 0;
+ out.data_item = 0;
+ break;
+ }
+ case VENC_SET_PARAM_FORCE_INTRA:
+ out.data_item = 0;
+ break;
+ case VENC_SET_PARAM_ADJUST_BITRATE:
+ out.data_item = 1;
+ out.data[0] = *(unsigned int *)param;
+ break;
+ case VENC_SET_PARAM_ADJUST_FRAMERATE:
+ out.data_item = 1;
+ out.data[0] = *(unsigned int *)param;
+ break;
+ case VENC_SET_PARAM_I_FRAME_INTERVAL:
+ out.data_item = 1;
+ out.data[0] = *(unsigned int *)param;
+ break;
+ case VENC_SET_PARAM_SKIP_FRAME:
+ out.data_item = 0;
+ break;
+ }
+ if (h264_enc_vpu_send_msg(hndl, &out, sizeof(out), 1) ||
+ hndl->vpu_inst.failure) {
+ mtk_vcodec_err(hndl,
+ "AP_IPIMSG_H264_ENC_SET_PARAM %d fail", id);
+ return -EINVAL;
+ }
+
+ mtk_vcodec_debug(hndl, "id %d <-", id);
+
+ return 0;
+}
+
+int h264_enc_vpu_encode(void *handle, unsigned int bs_mode,
+ struct venc_frm_buf *frm_buf,
+ struct mtk_vcodec_mem *bs_buf,
+ unsigned int *bs_size)
+{
+ struct venc_h264_handle *hndl = handle;
+ struct venc_ap_ipi_msg_enc out;
+
+ mtk_vcodec_debug(hndl, "bs_mode %d ->", bs_mode);
+
+ out.msg_id = AP_IPIMSG_H264_ENC_ENCODE;
+ out.inst_id = hndl->vpu_inst.id;
+ out.bs_mode = bs_mode;
+ if (frm_buf) {
+ out.input_addr[0] = frm_buf->fb_addr.dma_addr;
+ out.input_addr[1] = frm_buf->fb_addr1.dma_addr;
+ out.input_addr[2] = frm_buf->fb_addr2.dma_addr;
+ } else {
+ out.input_addr[0] = 0;
+ out.input_addr[1] = 0;
+ out.input_addr[2] = 0;
+ }
+ if (bs_buf) {
+ out.bs_addr = bs_buf->dma_addr;
+ out.bs_size = bs_buf->size;
+ } else {
+ out.bs_addr = 0;
+ out.bs_size = 0;
+ }
+ if (h264_enc_vpu_send_msg(hndl, &out, sizeof(out), 1) ||
+ hndl->vpu_inst.failure) {
+ mtk_vcodec_err(hndl, "AP_IPIMSG_H264_ENC_ENCODE %d fail",
+ bs_mode);
+ return -EINVAL;
+ }
+
+ mtk_vcodec_debug(hndl, "state %d size %d key_frm %d",
+ hndl->vpu_inst.state, hndl->vpu_inst.bs_size,
+ hndl->is_key_frm);
+ hndl->vpu_inst.wait_int = 1;
+ if (hndl->vpu_inst.state == VEN_IPI_MSG_ENC_STATE_SKIP) {
+ *bs_size = hndl->vpu_inst.bs_size;
+ memcpy(bs_buf->va,
+ hndl->work_bufs[VENC_H264_VPU_WORK_BUF_SKIP_FRAME].va,
+ *bs_size);
+ hndl->vpu_inst.wait_int = 0;
+ }
+
+ mtk_vcodec_debug(hndl, "bs_mode %d ->", bs_mode);
+
+ return 0;
+}
+
+int h264_enc_vpu_deinit(void *handle)
+{
+ struct venc_h264_handle *hndl = handle;
+ struct venc_ap_ipi_msg_deinit out;
+
+ mtk_vcodec_debug_enter(hndl);
+
+ out.msg_id = AP_IPIMSG_H264_ENC_DEINIT;
+ out.inst_id = hndl->vpu_inst.id;
+ if (h264_enc_vpu_send_msg(hndl, &out, sizeof(out), 1) ||
+ hndl->vpu_inst.failure) {
+ mtk_vcodec_err(hndl, "AP_IPIMSG_H264_ENC_DEINIT fail");
+ return -EINVAL;
+ }
+
+ mtk_vcodec_debug_leave(hndl);
+
+ return 0;
+}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tiffany Lin <tiffany.lin@mediatek.com> |
|---|---|
| Date | 2015-11-17 14:00 +0100 |
| Subject | [RESEND RFC/PATCH 4/8] dt-bindings: Add a binding for Mediatek Video Encoder |
| Message-ID | <qvO1A-58e-35@gated-at.bofh.it> |
| In reply to | #1271154 |
add a DT binding documentation of Video Encoder for the
MT8173 SoC from Mediatek.
Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
---
.../devicetree/bindings/media/mediatek-vcodec.txt | 58 ++++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec.txt
diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
new file mode 100644
index 0000000..fea4d7c
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
@@ -0,0 +1,58 @@
+Mediatek Video Codec
+
+Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
+supports high resolution encoding functionalities.
+
+Required properties:
+- compatible : "mediatek,mt8173-vcodec-enc" for encoder
+- reg : Physical base address of the video codec registers and length of
+ memory mapped region.
+- interrupts : interrupt number to the cpu.
+- larb : must contain the larbes of current platform
+- clocks : list of clock specifiers, corresponding to entries in
+ the clock-names property;
+- clock-names: must contain "vencpll", "venc_lt_sel", "vcodecpll_370p5_ck"
+- iommus : list of iommus specifiers should be enabled for hw encode.
+ There are 2 cells needed to enable/disable iommu.
+ The first one is local arbiter index(larbid), and the other is port
+ index(portid) within local arbiter. Specifies the larbid and portid
+ as defined in dt-binding/memory/mt8173-larb-port.h.
+- vpu : the node of video processor unit
+
+Example:
+vcodec_enc: vcodec@0x18002000 {
+ 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>;
+ larb = <&larb3>,
+ <&larb5>;
+ iommus = <&iommu M4U_LARB3_ID M4U_PORT_VENC_RCPU>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_REC>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_BSDMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_SV_COMV>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_RD_COMV>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_CUR_LUMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_CUR_CHROMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_REF_LUMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_REF_CHROMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_NBM_RDMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_NBM_WDMA>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_RCPU_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_REC_FRM_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_BSDMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_SV_COMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_RD_COMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_CUR_LUMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_CUR_CHROMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_REF_LUMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_REC_CHROMA_SET2>;
+ vpu = <&vpu>;
+ clocks = <&apmixedsys CLK_APMIXED_VENCPLL>,
+ <&topckgen CLK_TOP_VENC_LT_SEL>,
+ <&topckgen CLK_TOP_VCODECPLL_370P5>;
+ clock-names = "vencpll",
+ "venc_lt_sel",
+ "vcodecpll_370p5_ck";
+ };
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2015-11-17 20:50 +0100 |
| Subject | Re: [RESEND RFC/PATCH 4/8] dt-bindings: Add a binding for Mediatek Video Encoder |
| Message-ID | <qvUqm-TY-21@gated-at.bofh.it> |
| In reply to | #1271162 |
On Tue, Nov 17, 2015 at 08:54:41PM +0800, Tiffany Lin wrote:
> add a DT binding documentation of Video Encoder for the
> MT8173 SoC from Mediatek.
>
> Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
> ---
> .../devicetree/bindings/media/mediatek-vcodec.txt | 58 ++++++++++++++++++++
> 1 file changed, 58 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec.txt
>
> diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> new file mode 100644
> index 0000000..fea4d7c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> @@ -0,0 +1,58 @@
> +Mediatek Video Codec
> +
> +Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
> +supports high resolution encoding functionalities.
> +
> +Required properties:
> +- compatible : "mediatek,mt8173-vcodec-enc" for encoder
> +- reg : Physical base address of the video codec registers and length of
> + memory mapped region.
> +- interrupts : interrupt number to the cpu.
> +- larb : must contain the larbes of current platform
What is this?
> +- clocks : list of clock specifiers, corresponding to entries in
> + the clock-names property;
> +- clock-names: must contain "vencpll", "venc_lt_sel", "vcodecpll_370p5_ck"
> +- iommus : list of iommus specifiers should be enabled for hw encode.
> + There are 2 cells needed to enable/disable iommu.
> + The first one is local arbiter index(larbid), and the other is port
> + index(portid) within local arbiter. Specifies the larbid and portid
> + as defined in dt-binding/memory/mt8173-larb-port.h.
> +- vpu : the node of video processor unit
This should be prefixed with mediatek.
> +
> +Example:
> +vcodec_enc: vcodec@0x18002000 {
> + 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>;
> + larb = <&larb3>,
> + <&larb5>;
> + iommus = <&iommu M4U_LARB3_ID M4U_PORT_VENC_RCPU>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_REC>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_BSDMA>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_SV_COMV>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_RD_COMV>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_CUR_LUMA>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_CUR_CHROMA>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_REF_LUMA>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_REF_CHROMA>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_NBM_RDMA>,
> + <&iommu M4U_LARB3_ID M4U_PORT_VENC_NBM_WDMA>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_RCPU_SET2>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_REC_FRM_SET2>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_BSDMA_SET2>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_SV_COMA_SET2>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_RD_COMA_SET2>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_CUR_LUMA_SET2>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_CUR_CHROMA_SET2>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_REF_LUMA_SET2>,
> + <&iommu M4U_LARB5_ID M4U_PORT_VENC_REC_CHROMA_SET2>;
> + vpu = <&vpu>;
> + clocks = <&apmixedsys CLK_APMIXED_VENCPLL>,
> + <&topckgen CLK_TOP_VENC_LT_SEL>,
> + <&topckgen CLK_TOP_VCODECPLL_370P5>;
> + clock-names = "vencpll",
> + "venc_lt_sel",
> + "vcodecpll_370p5_ck";
> + };
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | tiffany lin <tiffany.lin@mediatek.com> |
|---|---|
| Date | 2015-11-18 08:20 +0100 |
| Subject | Re: [RESEND RFC/PATCH 4/8] dt-bindings: Add a binding for Mediatek Video Encoder |
| Message-ID | <qw5c5-89v-1@gated-at.bofh.it> |
| In reply to | #1271619 |
Hi Rob,
On Tue, 2015-11-17 at 13:41 -0600, Rob Herring wrote:
> On Tue, Nov 17, 2015 at 08:54:41PM +0800, Tiffany Lin wrote:
> > add a DT binding documentation of Video Encoder for the
> > MT8173 SoC from Mediatek.
> >
> > Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
> > ---
> > .../devicetree/bindings/media/mediatek-vcodec.txt | 58 ++++++++++++++++++++
> > 1 file changed, 58 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > new file mode 100644
> > index 0000000..fea4d7c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > @@ -0,0 +1,58 @@
> > +Mediatek Video Codec
> > +
> > +Mediatek Video Codec is the video codec hw present in Mediatek SoCs which
> > +supports high resolution encoding functionalities.
> > +
> > +Required properties:
> > +- compatible : "mediatek,mt8173-vcodec-enc" for encoder
> > +- reg : Physical base address of the video codec registers and length of
> > + memory mapped region.
> > +- interrupts : interrupt number to the cpu.
> > +- larb : must contain the larbes of current platform
>
> What is this?
Resend due to previous mail has html format and reject by some mail
servers.
This is SMI (Smart Multimedia Interface) Local Arbiter.
MT8173 has different local arbiters (larb).
Please see
http://lists.linuxfoundation.org/pipermail/iommu/2015-October/014587.html
Video Encoder HW has it's local arbiter (larb), by configure ports in
this larb, we can have encoder HW go through the m4u to talk with EMI.
We will change it to mediatek,larb in next version.
>
> > +- clocks : list of clock specifiers, corresponding to entries in
> > + the clock-names property;
> > +- clock-names: must contain "vencpll", "venc_lt_sel", "vcodecpll_370p5_ck"
> > +- iommus : list of iommus specifiers should be enabled for hw encode.
> > + There are 2 cells needed to enable/disable iommu.
> > + The first one is local arbiter index(larbid), and the other is port
> > + index(portid) within local arbiter. Specifies the larbid and portid
> > + as defined in dt-binding/memory/mt8173-larb-port.h.
> > +- vpu : the node of video processor unit
>
> This should be prefixed with mediatek.
We will prefix it with mediatek in next version.
>
> > +
> > +Example:
> > +vcodec_enc: vcodec@0x18002000 {
> > + 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>;
> > + larb = <&larb3>,
> > + <&larb5>;
> > + iommus = <&iommu M4U_LARB3_ID M4U_PORT_VENC_RCPU>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_REC>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_BSDMA>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_SV_COMV>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_RD_COMV>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_CUR_LUMA>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_CUR_CHROMA>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_REF_LUMA>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_REF_CHROMA>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_NBM_RDMA>,
> > + <&iommu M4U_LARB3_ID M4U_PORT_VENC_NBM_WDMA>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_RCPU_SET2>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_REC_FRM_SET2>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_BSDMA_SET2>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_SV_COMA_SET2>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_RD_COMA_SET2>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_CUR_LUMA_SET2>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_CUR_CHROMA_SET2>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_REF_LUMA_SET2>,
> > + <&iommu M4U_LARB5_ID M4U_PORT_VENC_REC_CHROMA_SET2>;
> > + vpu = <&vpu>;
> > + clocks = <&apmixedsys CLK_APMIXED_VENCPLL>,
> > + <&topckgen CLK_TOP_VENC_LT_SEL>,
> > + <&topckgen CLK_TOP_VCODECPLL_370P5>;
> > + clock-names = "vencpll",
> > + "venc_lt_sel",
> > + "vcodecpll_370p5_ck";
> > + };
> > --
> > 1.7.9.5
> >
best regards,
Tiffany
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Tiffany Lin <tiffany.lin@mediatek.com> |
|---|---|
| Date | 2015-11-17 14:00 +0100 |
| Subject | [RESEND RFC/PATCH 5/8] arm64: dts: mediatek: Add Video Encoder for MT8173 |
| Message-ID | <qvO1B-58e-41@gated-at.bofh.it> |
| In reply to | #1271154 |
add video encoder driver for MT8173
Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 47 ++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 098c15e..85ba167 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -545,6 +545,53 @@
#clock-cells = <1>;
};
+ larb3: larb@18001000 {
+ compatible = "mediatek,mt8173-smi-larb";
+ reg = <0 0x18001000 0 0x1000>;
+ mediatek,smi = <&smi_common>;
+ power-domains = <&scpsys MT8173_POWER_DOMAIN_VENC>;
+ clocks = <&vencsys CLK_VENC_CKE1>,
+ <&vencsys CLK_VENC_CKE0>;
+ clock-names = "apb", "smi";
+ };
+
+ 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>;
+ larb = <&larb3>,
+ <&larb5>;
+ iommus = <&iommu M4U_LARB3_ID M4U_PORT_VENC_RCPU>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_REC>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_BSDMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_SV_COMV>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_RD_COMV>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_CUR_LUMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_CUR_CHROMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_REF_LUMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_REF_CHROMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_NBM_RDMA>,
+ <&iommu M4U_LARB3_ID M4U_PORT_VENC_NBM_WDMA>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_RCPU_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_REC_FRM_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_BSDMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_SV_COMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_RD_COMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_CUR_LUMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_CUR_CHROMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_REF_LUMA_SET2>,
+ <&iommu M4U_LARB5_ID M4U_PORT_VENC_REC_CHROMA_SET2>;
+ vpu = <&vpu>;
+ clocks = <&apmixedsys CLK_APMIXED_VENCPLL>,
+ <&topckgen CLK_TOP_VENC_LT_SEL>,
+ <&topckgen CLK_TOP_VCODECPLL_370P5>;
+ clock-names = "vencpll",
+ "venc_lt_sel",
+ "vcodecpll_370p5_ck";
+ };
+
vencltsys: clock-controller@19000000 {
compatible = "mediatek,mt8173-vencltsys", "syscon";
reg = <0 0x19000000 0 0x1000>;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2015-11-19 08:50 +0100 |
| Subject | Re: [RESEND RFC/PATCH 0/8] Add MT8173 Video Encoder Driver and VPU Driver |
| Message-ID | <qws8G-6mi-5@gated-at.bofh.it> |
| In reply to | #1271154 |
On 11/17/2015 01:54 PM, Tiffany Lin wrote: > ============== > Introduction > ============== > > The purpose of this RFC is to discuss the driver for a hw video codec > embedded in the Mediatek's MT8173 SoCs. Mediatek Video Codec is able to > handle video encoding of in a range of formats. > > This RFC also include VPU driver. Mediatek Video Codec driver rely on > VPU driver to load, communicate with VPU. > > Internally the driver uses videobuf2 framework and MTK IOMMU and MTK SMI. > MTK IOMMU and MTK SMI have not yet been merged, but we wanted to start > discussion about the driver earlier so it could be merged sooner. The > driver posted here is the initial version, so I suppose it will require > more work. I plan on reviewing this patch series (at least the non-dt parts). It's busy, though, and I don't know exactly when I get the chance. But just so you know that someone will be reviewing it. Regards, Hans > > [1]http://lists.infradead.org/pipermail/linux-mediatek/2015-October/002525.html > > ================== > Device interface > ================== > > In principle the driver bases on memory-to-memory framework: > it provides a single video node and each opened file handle gets its own > private context with separate buffer queues. Each context consist of 2 > buffer queues: OUTPUT (for source buffers, i.e. raw video frames) > and CAPTURE (for destination buffers, i.e. encoded video frames). > > The process of encoding video data from stream is a bit more complicated > than typical memory-to-memory processing. We base on memory-to-memory > framework and add the complicated part in our vb2 and v4l2 callback > functionss. So we can base on well done m2m memory-to-memory framework, > reduce duplicate code and make our driver code simple. > > ============================== > VPU (Video Processor Unit) > ============================== > The VPU driver for hw video codec embedded in Mediatek's MT8173 SOCs. > It is able to handle video decoding/encoding of in a range of formats. > The driver provides with VPU firmware download, memory management and > the communication interface between CPU and VPU. > For VPU initialization, it will create virtual memory for CPU access and > IOMMU address for vcodec hw device access. When a decode/encode instance > opens a device node, vpu driver will download vpu firmware to the device. > A decode/encode instant will decode/encode a frame using VPU > interface to interrupt vpu to handle decoding/encoding jobs. > > Please have a look at the code and comments will be very much appreciated. > > Andrew-CT Chen (3): > dt-bindings: Add a binding for Mediatek Video Processor Unit > arm64: dts: mediatek: Add node for Mediatek Video Processor Unit > media: platform: mtk-vpu: Support Mediatek VPU > > Daniel Hsiao (1): > media: platform: mtk-vcodec: Add Mediatek VP8 Video Encoder Driver > > Tiffany Lin (4): > dt-bindings: Add a binding for Mediatek Video Encoder > arm64: dts: mediatek: Add Video Encoder for MT8173 > media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver > media: platform: mtk-vcodec: Add Mediatek H264 Video Encoder Driver > > .../devicetree/bindings/media/mediatek-vcodec.txt | 58 + > .../devicetree/bindings/media/mediatek-vpu.txt | 27 + > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 58 + > drivers/media/platform/Kconfig | 19 + > drivers/media/platform/Makefile | 5 + > drivers/media/platform/mtk-vcodec/Kconfig | 5 + > drivers/media/platform/mtk-vcodec/Makefile | 12 + > drivers/media/platform/mtk-vcodec/common/Makefile | 12 + > .../media/platform/mtk-vcodec/common/venc_drv_if.c | 159 ++ > .../media/platform/mtk-vcodec/h264_enc/Makefile | 9 + > .../platform/mtk-vcodec/h264_enc/venc_h264_if.c | 529 ++++++ > .../platform/mtk-vcodec/h264_enc/venc_h264_if.h | 53 + > .../platform/mtk-vcodec/h264_enc/venc_h264_vpu.c | 341 ++++ > .../platform/mtk-vcodec/include/venc_drv_base.h | 68 + > .../platform/mtk-vcodec/include/venc_drv_if.h | 187 +++ > .../platform/mtk-vcodec/include/venc_ipi_msg.h | 212 +++ > drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 441 +++++ > drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 1773 ++++++++++++++++++++ > drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.h | 28 + > .../media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 535 ++++++ > .../media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 122 ++ > .../media/platform/mtk-vcodec/mtk_vcodec_intr.c | 110 ++ > .../media/platform/mtk-vcodec/mtk_vcodec_intr.h | 30 + > drivers/media/platform/mtk-vcodec/mtk_vcodec_pm.h | 26 + > .../media/platform/mtk-vcodec/mtk_vcodec_util.c | 106 ++ > .../media/platform/mtk-vcodec/mtk_vcodec_util.h | 66 + > drivers/media/platform/mtk-vcodec/vp8_enc/Makefile | 9 + > .../platform/mtk-vcodec/vp8_enc/venc_vp8_if.c | 371 ++++ > .../platform/mtk-vcodec/vp8_enc/venc_vp8_if.h | 48 + > .../platform/mtk-vcodec/vp8_enc/venc_vp8_vpu.c | 245 +++ > drivers/media/platform/mtk-vpu/Makefile | 1 + > .../platform/mtk-vpu/h264_enc/venc_h264_vpu.h | 127 ++ > .../media/platform/mtk-vpu/include/venc_ipi_msg.h | 212 +++ > drivers/media/platform/mtk-vpu/mtk_vpu_core.c | 823 +++++++++ > drivers/media/platform/mtk-vpu/mtk_vpu_core.h | 161 ++ > .../media/platform/mtk-vpu/vp8_enc/venc_vp8_vpu.h | 119 ++ > 36 files changed, 7107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/mediatek-vcodec.txt > create mode 100644 Documentation/devicetree/bindings/media/mediatek-vpu.txt > create mode 100644 drivers/media/platform/mtk-vcodec/Kconfig > create mode 100644 drivers/media/platform/mtk-vcodec/Makefile > create mode 100644 drivers/media/platform/mtk-vcodec/common/Makefile > create mode 100644 drivers/media/platform/mtk-vcodec/common/venc_drv_if.c > create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/Makefile > create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.c > create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_if.h > create mode 100644 drivers/media/platform/mtk-vcodec/h264_enc/venc_h264_vpu.c > create mode 100644 drivers/media/platform/mtk-vcodec/include/venc_drv_base.h > create mode 100644 drivers/media/platform/mtk-vcodec/include/venc_drv_if.h > create mode 100644 drivers/media/platform/mtk-vcodec/include/venc_ipi_msg.h > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.h > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.c > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_intr.h > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_pm.h > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_util.c > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h > create mode 100644 drivers/media/platform/mtk-vcodec/vp8_enc/Makefile > create mode 100644 drivers/media/platform/mtk-vcodec/vp8_enc/venc_vp8_if.c > create mode 100644 drivers/media/platform/mtk-vcodec/vp8_enc/venc_vp8_if.h > create mode 100644 drivers/media/platform/mtk-vcodec/vp8_enc/venc_vp8_vpu.c > create mode 100644 drivers/media/platform/mtk-vpu/Makefile > create mode 100644 drivers/media/platform/mtk-vpu/h264_enc/venc_h264_vpu.h > create mode 100644 drivers/media/platform/mtk-vpu/include/venc_ipi_msg.h > create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu_core.c > create mode 100644 drivers/media/platform/mtk-vpu/mtk_vpu_core.h > create mode 100644 drivers/media/platform/mtk-vpu/vp8_enc/venc_vp8_vpu.h > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Daniel Thompson <daniel.thompson@linaro.org> |
|---|---|
| Date | 2015-11-27 13:30 +0100 |
| Subject | Re: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU |
| Message-ID | <qzqk2-2Nt-15@gated-at.bofh.it> |
| In reply to | #1271154 |
On 27/11/15 12:10, andrew-ct chen wrote:
>>> +
>>> > >+ memcpy((void *)send_obj->share_buf, buf, len);
>>> > >+ send_obj->len = len;
>>> > >+ send_obj->id = id;
>>> > >+ vpu_cfg_writel(vpu, 0x1, HOST_TO_VPU);
>>> > >+
>>> > >+ /* Wait until VPU receives the command */
>>> > >+ timeout = jiffies + msecs_to_jiffies(IPI_TIMEOUT_MS);
>>> > >+ do {
>>> > >+ if (time_after(jiffies, timeout)) {
>>> > >+ dev_err(vpu->dev, "vpu_ipi_send: IPI timeout!\n");
>>> > >+ return -EIO;
>>> > >+ }
>>> > >+ } while (vpu_cfg_readl(vpu, HOST_TO_VPU));
>> >
>> >Do we need to busy wait every time we communicate with the co-processor?
>> >Couldn't we put this wait*before* we write to HOST_TO_VPU above.
>> >
>> >That way we only spin when there is a need to.
>> >
> Since the hardware VPU only allows that one client sends the command to
> it each time.
> We need the wait to make sure VPU accepted the command and cleared the
> interrupt and then the next command would be served.
I understand that the VPU can only have on message outstanding at once.
I just wonder why we busy wait *after* sending the first command rather
than *before* sending the second one.
Streamed decode/encode typically ends up being rate controlled by
capture or display meaning that in these cases we don't need to busy
wait at all (because by the time we send the next frame the VPU has
already accepted the previous message).
Daniel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | andrew-ct chen <andrew-ct.chen@mediatek.com> |
|---|---|
| Date | 2015-11-30 12:50 +0100 |
| Subject | Re: [RESEND RFC/PATCH 3/8] media: platform: mtk-vpu: Support Mediatek VPU |
| Message-ID | <qAv7Z-33k-31@gated-at.bofh.it> |
| In reply to | #1278784 |
On Fri, 2015-11-27 at 12:21 +0000, Daniel Thompson wrote:
> On 27/11/15 12:10, andrew-ct chen wrote:
> >>> +
> >>> > >+ memcpy((void *)send_obj->share_buf, buf, len);
> >>> > >+ send_obj->len = len;
> >>> > >+ send_obj->id = id;
> >>> > >+ vpu_cfg_writel(vpu, 0x1, HOST_TO_VPU);
> >>> > >+
> >>> > >+ /* Wait until VPU receives the command */
> >>> > >+ timeout = jiffies + msecs_to_jiffies(IPI_TIMEOUT_MS);
> >>> > >+ do {
> >>> > >+ if (time_after(jiffies, timeout)) {
> >>> > >+ dev_err(vpu->dev, "vpu_ipi_send: IPI timeout!\n");
> >>> > >+ return -EIO;
> >>> > >+ }
> >>> > >+ } while (vpu_cfg_readl(vpu, HOST_TO_VPU));
> >> >
> >> >Do we need to busy wait every time we communicate with the co-processor?
> >> >Couldn't we put this wait*before* we write to HOST_TO_VPU above.
> >> >
> >> >That way we only spin when there is a need to.
> >> >
> > Since the hardware VPU only allows that one client sends the command to
> > it each time.
> > We need the wait to make sure VPU accepted the command and cleared the
> > interrupt and then the next command would be served.
>
> I understand that the VPU can only have on message outstanding at once.
>
> I just wonder why we busy wait *after* sending the first command rather
> than *before* sending the second one.
No other special reasons. Just send one command and wait until VPU gets
the command. Then, I think this wait also can be put before we write to
HOST_TO_VPU.Is this better than former? May I know the reason?
>
> Streamed decode/encode typically ends up being rate controlled by
> capture or display meaning that in these cases we don't need to busy
> wait at all (because by the time we send the next frame the VPU has
> already accepted the previous message).
For now, only one device "encoder" exists, it is true.
But, we'll have encoder and decoder devices, the decode and encode
requested to VPU are simultaneous.
Is this supposed to be removed for this patches and we can add it back
if the another device(decoder) is ready for review?
Andrew
>
>
> Daniel.
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Daniel Thompson <daniel.thompson@linaro.org> |
|---|---|
| Date | 2015-11-27 17:40 +0100 |
| Subject | Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver |
| Message-ID | <qzudX-5jX-5@gated-at.bofh.it> |
| In reply to | #1271154 |
Hi Tiffany/Andrew
This review is a rather more superficial than my previous one. Mostly
I'm just commenting on some of the bits I spotted whilst trying to find
my way around the patchset.
I hope to another more detailed review for v2 (and feel free to add me
to Cc:).
On 17/11/15 12:54, Tiffany Lin wrote:
> Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
> Signed-off-by: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
There is no description of what this patch does. Its not enough to have
it on the cover letter (because that won't end up in version control).
You need something here.
> diff --git a/drivers/media/platform/mtk-vcodec/Kconfig b/drivers/media/platform/mtk-vcodec/Kconfig
> new file mode 100644
> index 0000000..1c0b935
> --- /dev/null
> +++ b/drivers/media/platform/mtk-vcodec/Kconfig
> @@ -0,0 +1,5 @@
> +config MEDIATEK_VPU
> + bool
> + ---help---
> + This driver provides downloading firmware vpu (video processor unit)
> + and communicating with vpu.
Haven't I seen this before (in patch 3)? Why is it being added to
another Kconfig file?
> diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile
> new file mode 100644
> index 0000000..c7f7174
> --- /dev/null
> +++ b/drivers/media/platform/mtk-vcodec/Makefile
> @@ -0,0 +1,12 @@
> +obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk_vcodec_intr.o \
> + mtk_vcodec_util.o \
> + mtk_vcodec_enc_drv.o \
> + mtk_vcodec_enc.o \
> + mtk_vcodec_enc_pm.o
> +
> +obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += common/
> +
> +ccflags-y += -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
> + -I$(srctree)/drivers/media/platform/mtk-vcodec \
> + -I$(srctree)/drivers/media/platform/mtk-vpu
Seems like there's a lot of directories here. Are these files
(framework, common, vcodec, etc) so unrelated they really need to live
in separate directories?
Why not just drivers/media/platform/mediatek?
> diff --git a/drivers/media/platform/mtk-vcodec/common/Makefile b/drivers/media/platform/mtk-vcodec/common/Makefile
> new file mode 100644
> index 0000000..477ab80
> --- /dev/null
> +++ b/drivers/media/platform/mtk-vcodec/common/Makefile
> @@ -0,0 +1,8 @@
> +obj-y += \
> + venc_drv_if.o
> +
> +ccflags-y += \
> + -I$(srctree)/include/ \
> + -I$(srctree)/drivers/media/platform/mtk-vcodec \
> + -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
> + -I$(srctree)/drivers/media/platform/mtk-vpu
As above, this appears to be a directory to hold just one file.
> diff --git a/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
> new file mode 100644
> index 0000000..9b3f025
> --- /dev/null
> +++ b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
> @@ -0,0 +1,152 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + * Author: Daniel Hsiao <daniel.hsiao@mediatek.com>
> + * Jungchang Tsao <jungchang.tsao@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +
> +#include "mtk_vcodec_drv.h"
> +#include "mtk_vcodec_enc.h"
> +#include "mtk_vcodec_pm.h"
> +#include "mtk_vcodec_util.h"
> +#include "mtk_vpu_core.h"
> +
> +#include "venc_drv_if.h"
> +#include "venc_drv_base.h"
> +
> +
> +int venc_if_create(void *ctx, unsigned int fourcc, unsigned long
*handle)
> +{
> + struct venc_handle *h;
> + char str[10];
> +
> + mtk_vcodec_fmt2str(fourcc, str);
> +
> + h = kzalloc(sizeof(*h), GFP_KERNEL);
> + if (!h)
> + return -ENOMEM;
> +
> + h->fourcc = fourcc;
> + h->ctx = ctx;
> + mtk_vcodec_debug(h, "fmt = %s handle = %p", str, h);
> +
> + switch (fourcc) {
> + default:
> + mtk_vcodec_err(h, "invalid format %s", str);
> + goto err_out;
> + }
> +
> + *handle = (unsigned long)h;
> + return 0;
> +
> +err_out:
> + kfree(h);
> + return -EINVAL;
> +}
> +
> +int venc_if_init(unsigned long handle)
> +{
> + int ret = 0;
> + struct venc_handle *h = (struct venc_handle *)handle;
> +
> + mtk_vcodec_debug_enter(h);
> +
> + mtk_venc_lock(h->ctx);
> + mtk_vcodec_enc_clock_on();
> + vpu_enable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
> + ret = h->enc_if->init(h->ctx, (unsigned long *)&h->drv_handle);
> + vpu_disable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
> + mtk_vcodec_enc_clock_off();
> + mtk_venc_unlock(h->ctx);
> +
> + return ret;
> +}
To me this looks more like an obfuscation layer rather than a
abstraction layer. I don't understand why we need to hide things from
the V4L2 implementation that this code forms part of.
More importantly, if this code was included somewhere where it could be
properly integrated with the device model you might be able to use the
pm_runtime system to avoid this sort of "heroics" to manage the clocks
anyway.
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> new file mode 100644
> index 0000000..22239f8
> --- /dev/null
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> @@ -0,0 +1,441 @@
> +/*
> +* Copyright (c) 2015 MediaTek Inc.
> +* Author: PC Chen <pc.chen@mediatek.com>
> +* Tiffany Lin <tiffany.lin@mediatek.com>
> +*
> +* This program is free software; you can redistribute it and/or modify
> +* it under the terms of the GNU General Public License version 2 as
> +* published by the Free Software Foundation.
> +*
> +* This program is distributed in the hope that it will be useful,
> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +* GNU General Public License for more details.
> +*/
> +
> +#ifndef _MTK_VCODEC_DRV_H_
> +#define _MTK_VCODEC_DRV_H_
> +
> +#include <linux/platform_device.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-core.h>
> +#include <media/videobuf2-v4l2.h>
> +
> +#include "venc_drv_if.h"
> +
> +#define MTK_VCODEC_MAX_INSTANCES 32
> +#define MTK_VCODEC_MAX_FRAME_SIZE 0x800000
> +#define MTK_VIDEO_MAX_FRAME 32
> +#define MTK_MAX_CTRLS 10
> +
> +#define MTK_VCODEC_DRV_NAME "mtk_vcodec_drv"
> +#define MTK_VCODEC_ENC_NAME "mtk-vcodec-enc"
> +
> +#define MTK_VENC_IRQ_STATUS_SPS 0x1
> +#define MTK_VENC_IRQ_STATUS_PPS 0x2
> +#define MTK_VENC_IRQ_STATUS_FRM 0x4
> +#define MTK_VENC_IRQ_STATUS_DRAM 0x8
> +#define MTK_VENC_IRQ_STATUS_PAUSE 0x10
> +#define MTK_VENC_IRQ_STATUS_SWITCH 0x20
Probably better to use BIT(0) .. BIT(5).
> +#define MTK_VENC_IRQ_STATUS_OFFSET 0x05C
> +#define MTK_VENC_IRQ_ACK_OFFSET 0x060
> +
> +#define MTK_VCODEC_MAX_PLANES 3
> +
> +#define VDEC_HW_ACTIVE 0x10
> +#define VDEC_IRQ_CFG 0x11
> +#define VDEC_IRQ_CLR 0x10
> +
> +#define VDEC_IRQ_CFG_REG 0xa4
> +#define NUM_MAX_ALLOC_CTX 4
> +#define MTK_V4L2_BENCHMARK 0
> +#define USE_ENCODE_THREAD 1
> +
> +/**
> + * enum mtk_hw_reg_idx - MTK hw register base index
> + */
> +enum mtk_hw_reg_idx {
> + VDEC_SYS,
> + VDEC_MISC,
> + VDEC_LD,
> + VDEC_TOP,
> + VDEC_CM,
> + VDEC_AD,
> + VDEC_AV,
> + VDEC_PP,
> + VDEC_HWD,
> + VDEC_HWQ,
> + VDEC_HWB,
> + VDEC_HWG,
> + NUM_MAX_VDEC_REG_BASE,
> + VENC_SYS = NUM_MAX_VDEC_REG_BASE,
> + VENC_LT_SYS,
> + NUM_MAX_VCODEC_REG_BASE
> +};
> +
> +/**
> + * enum mtk_instance_type - The type of an MTK Vcodec instance.
> + */
> +enum mtk_instance_type {
> + MTK_INST_DECODER = 0,
> + MTK_INST_ENCODER = 1,
> +};
> +
> +/**
> + * enum mtk_instance_state - The state of an MTK Vcodec instance.
> + * @MTK_STATE_FREE - default state when instance create
> + * @MTK_STATE_CREATE - vdec instance is create
> + * @MTK_STATE_INIT - vdec instance is init
> + * @MTK_STATE_CONFIG - reserved for encoder
> + * @MTK_STATE_HEADER - vdec had sps/pps header parsed
> + * @MTK_STATE_RUNNING - vdec is decoding
> + * @MTK_STATE_FLUSH - vdec is flushing
> + * @MTK_STATE_RES_CHANGE - vdec detect resolution change
> + * @MTK_STATE_FINISH - ctx instance is stopped streaming
> + * @MTK_STATE_DEINIT - before release ctx instance
> + * @MTK_STATE_ERROR - vdec has something wrong
> + * @MTK_STATE_ABORT - abort work in working thread
> + */
> +enum mtk_instance_state {
> + MTK_STATE_FREE = 0,
> + MTK_STATE_CREATE = (1 << 0),
> + MTK_STATE_INIT = (1 << 1),
> + MTK_STATE_CONFIG = (1 << 2),
> + MTK_STATE_HEADER = (1 << 3),
> + MTK_STATE_RUNNING = (1 << 4),
> + MTK_STATE_FLUSH = (1 << 5),
> + MTK_STATE_RES_CHANGE = (1 << 6),
> + MTK_STATE_FINISH = (1 << 7),
> + MTK_STATE_DEINIT = (1 << 8),
> + MTK_STATE_ERROR = (1 << 9),
> + MTK_STATE_ABORT = (1 << 10),
This looks like it started as a state machine and somehow turned into
flags, resulting in a state machine with 2048 states or, to give it a
different name, a debugging nightmare.
If the start streaming operation implemented cleanup-on-error properly
then there would only be two useful states: Started and stopped. Even
the "sticky" error behavior looks unnecessary to me (meaning we don't
need to track its state).
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> new file mode 100644
> index 0000000..8e1b6f0
> --- /dev/null
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> @@ -0,0 +1,1773 @@
> [...]
> +static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> +{
> + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(q);
> + struct v4l2_device *v4l2_dev = &ctx->dev->v4l2_dev;
> + int ret;
> +#if MTK_V4L2_BENCHMARK
> + struct timeval begin, end;
> +
> + do_gettimeofday(&begin);
> +#endif
> +
> + if (!(vb2_start_streaming_called(&ctx->m2m_ctx->out_q_ctx.q) &
> + vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q))) {
> + mtk_v4l2_debug(1, "[%d]-> out=%d cap=%d",
> + ctx->idx,
> + vb2_start_streaming_called(&ctx->m2m_ctx->out_q_ctx.q),
> + vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q));
> + return 0;
> + }
> +
> + if ((ctx->state & (MTK_STATE_ERROR | MTK_STATE_ABORT)))
> + return -EINVAL;
This is the sort of thing I mean.
This sticky error behaviour means that every subsequent call to
vb2ops_venc_start_streaming() will fail. Note also that the user will
never try to stop streaming (which can clear the error state) because
according to the return code it got when it tried to start streaming we
never actually started.
This is what I mean about having two many states. From the user's
perspective there are only two states. There needs to be a good reason
for the driver to manage so many extra secret states internally.
> +
> + if (ctx->state == MTK_STATE_FREE) {
> + ret = venc_if_create(ctx,
> + ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc,
> + &ctx->h_enc);
> +
> + if (ret != 0) {
> + ctx->state |= MTK_STATE_ERROR;
> + v4l2_err(v4l2_dev, "invalid codec type=%x\n",
> + ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc);
> + v4l2_err(v4l2_dev, "venc_if_create failed=%d\n", ret);
> + return -EINVAL;
> + }
> +
> + if (ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc ==
> + V4L2_PIX_FMT_H264)
> + ctx->hdr = 1;
> +
> + ctx->state |= MTK_STATE_CREATE;
> + }
> +
> + if ((ctx->state & MTK_STATE_CREATE) && !(ctx->state & MTK_STATE_INIT)) {
> + ret = venc_if_init(ctx->h_enc);
> + if (ret != 0) {
> + ctx->state |= MTK_STATE_ERROR;
> + v4l2_err(v4l2_dev, "venc_if_init failed=%d\n", ret);
> + return -EINVAL;
This error path leaves the encoder partially constructed and relies on
something else to tidy things up. It would be much better to tidy things
up from this function and
Also I don't think both venc_if_create and venc_if_init are needed. They
are only ever called one after the other and thus they only serve to
complicate the error handling code.
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
> new file mode 100644
> index 0000000..a8e683a
> --- /dev/null
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
> @@ -0,0 +1,66 @@
> +/*
> +* Copyright (c) 2015 MediaTek Inc.
> +* Author: PC Chen <pc.chen@mediatek.com>
> +* Tiffany Lin <tiffany.lin@mediatek.com>
> +*
> +* This program is free software; you can redistribute it and/or modify
> +* it under the terms of the GNU General Public License version 2 as
> +* published by the Free Software Foundation.
> +*
> +* This program is distributed in the hope that it will be useful,
> +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +* GNU General Public License for more details.
> +*/
> +
> +#ifndef _MTK_VCODEC_UTIL_H_
> +#define _MTK_VCODEC_UTIL_H_
> +
> +#include <linux/types.h>
> +#include <linux/dma-direction.h>
> +
> +struct mtk_vcodec_mem {
> + size_t size;
> + void *va;
> + dma_addr_t dma_addr;
> +};
> +
> +extern int mtk_v4l2_dbg_level;
> +extern bool mtk_vcodec_dbg;
> +
> +#define mtk_v4l2_debug(level, fmt, args...) \
> + do { \
> + if (mtk_v4l2_dbg_level >= level) \
> + pr_info("[MTK_V4L2] level=%d %s(),%d: " fmt "\n",\
> + level, __func__, __LINE__, ##args); \
> + } while (0)
> +
> +#define mtk_v4l2_err(fmt, args...) \
> + pr_err("[MTK_V4L2][ERROR] %s:%d: " fmt "\n", __func__, __LINE__, \
> + ##args)
Obviously the code should be structured to make use of dev_dbg/dev_err
possible.
However where this won't work do you really need special macros for
this. Assuming your error messages are well written 'git grep' and the
following should be enough:
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#define mtk_v4l2_debug_enter() mtk_v4l2_debug(5, "+\n")
> +#define mtk_v4l2_debug_leave() mtk_v4l2_debug(5, "-\n")
Remove these. If you care about function entry and exit for debugging
you should be able to use ftrace.
> +#define mtk_vcodec_debug(h, fmt, args...) \
> + do { \
> + if (mtk_vcodec_dbg) \
> + pr_info("[MTK_VCODEC][%d]: %s() " fmt "\n", \
> + ((struct mtk_vcodec_ctx *)h->ctx)->idx, \
> + __func__, ##args); \
> + } while (0)
> +
> +#define mtk_vcodec_err(h, fmt, args...) \
> + pr_err("[MTK_VCODEC][ERROR][%d]: %s() " fmt "\n", \
> + ((struct mtk_vcodec_ctx *)h->ctx)->idx, __func__, ##args)
> +
> +#define mtk_vcodec_debug_enter(h) mtk_vcodec_debug(h, "+\n")
> +#define mtk_vcodec_debug_leave(h) mtk_vcodec_debug(h, "-\n")
All above comments apply to these too.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | tiffany lin <tiffany.lin@mediatek.com> |
|---|---|
| Date | 2015-11-30 12:50 +0100 |
| Subject | Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver |
| Message-ID | <qAv7Y-33k-21@gated-at.bofh.it> |
| In reply to | #1278878 |
On Fri, 2015-11-27 at 16:34 +0000, Daniel Thompson wrote:
> Hi Tiffany/Andrew
>
> This review is a rather more superficial than my previous one. Mostly
> I'm just commenting on some of the bits I spotted whilst trying to find
> my way around the patchset.
>
> I hope to another more detailed review for v2 (and feel free to add me
> to Cc:).
>
Thanks for your comments.
>
> On 17/11/15 12:54, Tiffany Lin wrote:
> > Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
> > Signed-off-by: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
>
> There is no description of what this patch does. Its not enough to have
> it on the cover letter (because that won't end up in version control).
> You need something here.
>
Got it, We will add description for each patch in next version.
>
> > diff --git a/drivers/media/platform/mtk-vcodec/Kconfig b/drivers/media/platform/mtk-vcodec/Kconfig
> > new file mode 100644
> > index 0000000..1c0b935
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vcodec/Kconfig
> > @@ -0,0 +1,5 @@
> > +config MEDIATEK_VPU
> > + bool
> > + ---help---
> > + This driver provides downloading firmware vpu (video processor unit)
> > + and communicating with vpu.
>
> Haven't I seen this before (in patch 3)? Why is it being added to
> another Kconfig file?
>
We will remove this in next version.
>
> > diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile
> > new file mode 100644
> > index 0000000..c7f7174
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vcodec/Makefile
> > @@ -0,0 +1,12 @@
> > +obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk_vcodec_intr.o \
> > + mtk_vcodec_util.o \
> > + mtk_vcodec_enc_drv.o \
> > + mtk_vcodec_enc.o \
> > + mtk_vcodec_enc_pm.o
> > +
> > +obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += common/
> > +
> > +ccflags-y += -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
> > + -I$(srctree)/drivers/media/platform/mtk-vcodec \
> > + -I$(srctree)/drivers/media/platform/mtk-vpu
>
> Seems like there's a lot of directories here. Are these files
> (framework, common, vcodec, etc) so unrelated they really need to live
> in separate directories?
>
> Why not just drivers/media/platform/mediatek?
This is because VPU and Vcodec are two different drivers.
Driver in mtk-vpu is for controlling VPU device and provide
communication API to VPU.
Driver in mtk-vcodec is for control different encoder (vp8, h264), it
include v4l2 driver layer, glue layer between encoders and vp8 and h264
encoder.
>
>
> > diff --git a/drivers/media/platform/mtk-vcodec/common/Makefile b/drivers/media/platform/mtk-vcodec/common/Makefile
> > new file mode 100644
> > index 0000000..477ab80
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vcodec/common/Makefile
> > @@ -0,0 +1,8 @@
> > +obj-y += \
> > + venc_drv_if.o
> > +
> > +ccflags-y += \
> > + -I$(srctree)/include/ \
> > + -I$(srctree)/drivers/media/platform/mtk-vcodec \
> > + -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
> > + -I$(srctree)/drivers/media/platform/mtk-vpu
>
> As above, this appears to be a directory to hold just one file.
>
Sorry, I didn't get it. Could you explain more?
>
> > diff --git a/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
> b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
> > new file mode 100644
> > index 0000000..9b3f025
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
> > @@ -0,0 +1,152 @@
> > +/*
> > + * Copyright (c) 2015 MediaTek Inc.
> > + * Author: Daniel Hsiao <daniel.hsiao@mediatek.com>
> > + * Jungchang Tsao <jungchang.tsao@mediatek.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +
> > +#include "mtk_vcodec_drv.h"
> > +#include "mtk_vcodec_enc.h"
> > +#include "mtk_vcodec_pm.h"
> > +#include "mtk_vcodec_util.h"
> > +#include "mtk_vpu_core.h"
> > +
> > +#include "venc_drv_if.h"
> > +#include "venc_drv_base.h"
> > +
> > +
> > +int venc_if_create(void *ctx, unsigned int fourcc, unsigned long
> *handle)
> > +{
> > + struct venc_handle *h;
> > + char str[10];
> > +
> > + mtk_vcodec_fmt2str(fourcc, str);
> > +
> > + h = kzalloc(sizeof(*h), GFP_KERNEL);
> > + if (!h)
> > + return -ENOMEM;
> > +
> > + h->fourcc = fourcc;
> > + h->ctx = ctx;
> > + mtk_vcodec_debug(h, "fmt = %s handle = %p", str, h);
> > +
> > + switch (fourcc) {
> > + default:
> > + mtk_vcodec_err(h, "invalid format %s", str);
> > + goto err_out;
> > + }
> > +
> > + *handle = (unsigned long)h;
> > + return 0;
> > +
> > +err_out:
> > + kfree(h);
> > + return -EINVAL;
> > +}
> > +
> > +int venc_if_init(unsigned long handle)
> > +{
> > + int ret = 0;
> > + struct venc_handle *h = (struct venc_handle *)handle;
> > +
> > + mtk_vcodec_debug_enter(h);
> > +
> > + mtk_venc_lock(h->ctx);
> > + mtk_vcodec_enc_clock_on();
> > + vpu_enable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
> > + ret = h->enc_if->init(h->ctx, (unsigned long *)&h->drv_handle);
> > + vpu_disable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
> > + mtk_vcodec_enc_clock_off();
> > + mtk_venc_unlock(h->ctx);
> > +
> > + return ret;
> > +}
>
> To me this looks more like an obfuscation layer rather than a
> abstraction layer. I don't understand why we need to hide things from
> the V4L2 implementation that this code forms part of.
>
> More importantly, if this code was included somewhere where it could be
> properly integrated with the device model you might be able to use the
> pm_runtime system to avoid this sort of "heroics" to manage the clocks
> anyway.
>
We want to abstract common part from encoder driver.
Every encoder driver follow same calling flow and only need to take care
about how to communicate with vpu to encode specific format.
Encoder driver do not need to take care clock and multiple instance
issue.
>
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > new file mode 100644
> > index 0000000..22239f8
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> > @@ -0,0 +1,441 @@
> > +/*
> > +* Copyright (c) 2015 MediaTek Inc.
> > +* Author: PC Chen <pc.chen@mediatek.com>
> > +* Tiffany Lin <tiffany.lin@mediatek.com>
> > +*
> > +* This program is free software; you can redistribute it and/or modify
> > +* it under the terms of the GNU General Public License version 2 as
> > +* published by the Free Software Foundation.
> > +*
> > +* This program is distributed in the hope that it will be useful,
> > +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +* GNU General Public License for more details.
> > +*/
> > +
> > +#ifndef _MTK_VCODEC_DRV_H_
> > +#define _MTK_VCODEC_DRV_H_
> > +
> > +#include <linux/platform_device.h>
> > +#include <linux/videodev2.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-ioctl.h>
> > +#include <media/videobuf2-core.h>
> > +#include <media/videobuf2-v4l2.h>
> > +
> > +#include "venc_drv_if.h"
> > +
> > +#define MTK_VCODEC_MAX_INSTANCES 32
> > +#define MTK_VCODEC_MAX_FRAME_SIZE 0x800000
> > +#define MTK_VIDEO_MAX_FRAME 32
> > +#define MTK_MAX_CTRLS 10
> > +
> > +#define MTK_VCODEC_DRV_NAME "mtk_vcodec_drv"
> > +#define MTK_VCODEC_ENC_NAME "mtk-vcodec-enc"
> > +
> > +#define MTK_VENC_IRQ_STATUS_SPS 0x1
> > +#define MTK_VENC_IRQ_STATUS_PPS 0x2
> > +#define MTK_VENC_IRQ_STATUS_FRM 0x4
> > +#define MTK_VENC_IRQ_STATUS_DRAM 0x8
> > +#define MTK_VENC_IRQ_STATUS_PAUSE 0x10
> > +#define MTK_VENC_IRQ_STATUS_SWITCH 0x20
>
> Probably better to use BIT(0) .. BIT(5).
Will change to use BIT(0) in next version.
>
>
> > +#define MTK_VENC_IRQ_STATUS_OFFSET 0x05C
> > +#define MTK_VENC_IRQ_ACK_OFFSET 0x060
> > +
> > +#define MTK_VCODEC_MAX_PLANES 3
> > +
> > +#define VDEC_HW_ACTIVE 0x10
> > +#define VDEC_IRQ_CFG 0x11
> > +#define VDEC_IRQ_CLR 0x10
> > +
> > +#define VDEC_IRQ_CFG_REG 0xa4
> > +#define NUM_MAX_ALLOC_CTX 4
> > +#define MTK_V4L2_BENCHMARK 0
> > +#define USE_ENCODE_THREAD 1
> > +
> > +/**
> > + * enum mtk_hw_reg_idx - MTK hw register base index
> > + */
> > +enum mtk_hw_reg_idx {
> > + VDEC_SYS,
> > + VDEC_MISC,
> > + VDEC_LD,
> > + VDEC_TOP,
> > + VDEC_CM,
> > + VDEC_AD,
> > + VDEC_AV,
> > + VDEC_PP,
> > + VDEC_HWD,
> > + VDEC_HWQ,
> > + VDEC_HWB,
> > + VDEC_HWG,
> > + NUM_MAX_VDEC_REG_BASE,
> > + VENC_SYS = NUM_MAX_VDEC_REG_BASE,
> > + VENC_LT_SYS,
> > + NUM_MAX_VCODEC_REG_BASE
> > +};
> > +
> > +/**
> > + * enum mtk_instance_type - The type of an MTK Vcodec instance.
> > + */
> > +enum mtk_instance_type {
> > + MTK_INST_DECODER = 0,
> > + MTK_INST_ENCODER = 1,
> > +};
> > +
> > +/**
> > + * enum mtk_instance_state - The state of an MTK Vcodec instance.
> > + * @MTK_STATE_FREE - default state when instance create
> > + * @MTK_STATE_CREATE - vdec instance is create
> > + * @MTK_STATE_INIT - vdec instance is init
> > + * @MTK_STATE_CONFIG - reserved for encoder
> > + * @MTK_STATE_HEADER - vdec had sps/pps header parsed
> > + * @MTK_STATE_RUNNING - vdec is decoding
> > + * @MTK_STATE_FLUSH - vdec is flushing
> > + * @MTK_STATE_RES_CHANGE - vdec detect resolution change
> > + * @MTK_STATE_FINISH - ctx instance is stopped streaming
> > + * @MTK_STATE_DEINIT - before release ctx instance
> > + * @MTK_STATE_ERROR - vdec has something wrong
> > + * @MTK_STATE_ABORT - abort work in working thread
> > + */
> > +enum mtk_instance_state {
> > + MTK_STATE_FREE = 0,
> > + MTK_STATE_CREATE = (1 << 0),
> > + MTK_STATE_INIT = (1 << 1),
> > + MTK_STATE_CONFIG = (1 << 2),
> > + MTK_STATE_HEADER = (1 << 3),
> > + MTK_STATE_RUNNING = (1 << 4),
> > + MTK_STATE_FLUSH = (1 << 5),
> > + MTK_STATE_RES_CHANGE = (1 << 6),
> > + MTK_STATE_FINISH = (1 << 7),
> > + MTK_STATE_DEINIT = (1 << 8),
> > + MTK_STATE_ERROR = (1 << 9),
> > + MTK_STATE_ABORT = (1 << 10),
>
> This looks like it started as a state machine and somehow turned into
> flags, resulting in a state machine with 2048 states or, to give it a
> different name, a debugging nightmare.
>
It's define some state happened rather than state machine.
Though some states are for v4l2 decoder driver and not used in encoder
driver.
> If the start streaming operation implemented cleanup-on-error properly
> then there would only be two useful states: Started and stopped. Even
> the "sticky" error behavior looks unnecessary to me (meaning we don't
> need to track its state).
>
We cannot guaranteed that IOCTLs called from the user space follow
required sequence.
We need states to know if our driver could accept IOCTL command.
>
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > new file mode 100644
> > index 0000000..8e1b6f0
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> > @@ -0,0 +1,1773 @@
> > [...]
> > +static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
> > +{
> > + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(q);
> > + struct v4l2_device *v4l2_dev = &ctx->dev->v4l2_dev;
> > + int ret;
> > +#if MTK_V4L2_BENCHMARK
> > + struct timeval begin, end;
> > +
> > + do_gettimeofday(&begin);
> > +#endif
> > +
> > + if (!(vb2_start_streaming_called(&ctx->m2m_ctx->out_q_ctx.q) &
> > + vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q))) {
> > + mtk_v4l2_debug(1, "[%d]-> out=%d cap=%d",
> > + ctx->idx,
> > + vb2_start_streaming_called(&ctx->m2m_ctx->out_q_ctx.q),
> > + vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q));
> > + return 0;
> > + }
> > +
> > + if ((ctx->state & (MTK_STATE_ERROR | MTK_STATE_ABORT)))
> > + return -EINVAL;
>
> This is the sort of thing I mean.
>
> This sticky error behaviour means that every subsequent call to
> vb2ops_venc_start_streaming() will fail. Note also that the user will
> never try to stop streaming (which can clear the error state) because
> according to the return code it got when it tried to start streaming we
> never actually started.
>
> This is what I mean about having two many states. From the user's
> perspective there are only two states. There needs to be a good reason
> for the driver to manage so many extra secret states internally.
>
For my understanding, that vb2ops_venc_start_streaming cannot fail.
If it fail, user space will close and release this encoder instance
(fd).
We really need to state driver to see what it should do when receive
current IOCTL.
>
> > +
> > + if (ctx->state == MTK_STATE_FREE) {
> > + ret = venc_if_create(ctx,
> > + ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc,
> > + &ctx->h_enc);
> > +
> > + if (ret != 0) {
> > + ctx->state |= MTK_STATE_ERROR;
> > + v4l2_err(v4l2_dev, "invalid codec type=%x\n",
> > + ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc);
> > + v4l2_err(v4l2_dev, "venc_if_create failed=%d\n", ret);
> > + return -EINVAL;
> > + }
> > +
> > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc ==
> > + V4L2_PIX_FMT_H264)
> > + ctx->hdr = 1;
> > +
> > + ctx->state |= MTK_STATE_CREATE;
> > + }
> > +
> > + if ((ctx->state & MTK_STATE_CREATE) && !(ctx->state & MTK_STATE_INIT)) {
> > + ret = venc_if_init(ctx->h_enc);
> > + if (ret != 0) {
> > + ctx->state |= MTK_STATE_ERROR;
> > + v4l2_err(v4l2_dev, "venc_if_init failed=%d\n", ret);
> > + return -EINVAL;
>
> This error path leaves the encoder partially constructed and relies on
> something else to tidy things up. It would be much better to tidy things
> up from this function and
>
> Also I don't think both venc_if_create and venc_if_init are needed. They
> are only ever called one after the other and thus they only serve to
> complicate the error handling code.
>
venc_if_create is for creating instance in arm side and base on encode
format hook corresponding encoder driver interface.
venc_if_init is trying to init encoder instance in VPU side.
Failures from two functions should have different error handling.
We will enhance this part in next version.
>
> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
> > new file mode 100644
> > index 0000000..a8e683a
> > --- /dev/null
> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
> > @@ -0,0 +1,66 @@
> > +/*
> > +* Copyright (c) 2015 MediaTek Inc.
> > +* Author: PC Chen <pc.chen@mediatek.com>
> > +* Tiffany Lin <tiffany.lin@mediatek.com>
> > +*
> > +* This program is free software; you can redistribute it and/or modify
> > +* it under the terms of the GNU General Public License version 2 as
> > +* published by the Free Software Foundation.
> > +*
> > +* This program is distributed in the hope that it will be useful,
> > +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +* GNU General Public License for more details.
> > +*/
> > +
> > +#ifndef _MTK_VCODEC_UTIL_H_
> > +#define _MTK_VCODEC_UTIL_H_
> > +
> > +#include <linux/types.h>
> > +#include <linux/dma-direction.h>
> > +
> > +struct mtk_vcodec_mem {
> > + size_t size;
> > + void *va;
> > + dma_addr_t dma_addr;
> > +};
> > +
> > +extern int mtk_v4l2_dbg_level;
> > +extern bool mtk_vcodec_dbg;
> > +
> > +#define mtk_v4l2_debug(level, fmt, args...) \
> > + do { \
> > + if (mtk_v4l2_dbg_level >= level) \
> > + pr_info("[MTK_V4L2] level=%d %s(),%d: " fmt "\n",\
> > + level, __func__, __LINE__, ##args); \
> > + } while (0)
> > +
> > +#define mtk_v4l2_err(fmt, args...) \
> > + pr_err("[MTK_V4L2][ERROR] %s:%d: " fmt "\n", __func__, __LINE__, \
> > + ##args)
>
> Obviously the code should be structured to make use of dev_dbg/dev_err
> possible.
>
> However where this won't work do you really need special macros for
> this. Assuming your error messages are well written 'git grep' and the
> following should be enough:
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
Thanks.
For pr_err case, we will try to use "#define pr_fmt(fmt) KBUILD_MODNAME
": " fmt" in next version.
For pr_info case, we still need debug level to control output messages.
>
> > +#define mtk_v4l2_debug_enter() mtk_v4l2_debug(5, "+\n")
> > +#define mtk_v4l2_debug_leave() mtk_v4l2_debug(5, "-\n")
>
> Remove these. If you care about function entry and exit for debugging
> you should be able to use ftrace.
>
I am not familiar with ftrace.
What if we only want to trace v4l2 video encoder driver called flow not
called stack? And only for functions we are interested not all
functions.
I will check if it is convenience for us using ftrace.
>
> > +#define mtk_vcodec_debug(h, fmt, args...) \
> > + do { \
> > + if (mtk_vcodec_dbg) \
> > + pr_info("[MTK_VCODEC][%d]: %s() " fmt "\n", \
> > + ((struct mtk_vcodec_ctx *)h->ctx)->idx, \
> > + __func__, ##args); \
> > + } while (0)
> > +
> > +#define mtk_vcodec_err(h, fmt, args...) \
> > + pr_err("[MTK_VCODEC][ERROR][%d]: %s() " fmt "\n", \
> > + ((struct mtk_vcodec_ctx *)h->ctx)->idx, __func__, ##args)
> > +
> > +#define mtk_vcodec_debug_enter(h) mtk_vcodec_debug(h, "+\n")
> > +#define mtk_vcodec_debug_leave(h) mtk_vcodec_debug(h, "-\n")
>
> All above comments apply to these too.
>
best regards,
Tiffany
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Daniel Thompson <daniel.thompson@linaro.org> |
|---|---|
| Date | 2015-11-30 16:00 +0100 |
| Subject | Re: [RESEND RFC/PATCH 6/8] media: platform: mtk-vcodec: Add Mediatek V4L2 Video Encoder Driver |
| Message-ID | <qAy5Q-4Vq-9@gated-at.bofh.it> |
| In reply to | #1279798 |
On 30 November 2015 at 11:39, tiffany lin <tiffany.lin@mediatek.com> wrote:
>> > diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile
>> > new file mode 100644
>> > index 0000000..c7f7174
>> > --- /dev/null
>> > +++ b/drivers/media/platform/mtk-vcodec/Makefile
>> > @@ -0,0 +1,12 @@
>> > +obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk_vcodec_intr.o \
>> > + mtk_vcodec_util.o \
>> > + mtk_vcodec_enc_drv.o \
>> > + mtk_vcodec_enc.o \
>> > + mtk_vcodec_enc_pm.o
>> > +
>> > +obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += common/
>> > +
>> > +ccflags-y += -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
>> > + -I$(srctree)/drivers/media/platform/mtk-vcodec \
>> > + -I$(srctree)/drivers/media/platform/mtk-vpu
>>
>> Seems like there's a lot of directories here. Are these files
>> (framework, common, vcodec, etc) so unrelated they really need to live
>> in separate directories?
>>
>> Why not just drivers/media/platform/mediatek?
> This is because VPU and Vcodec are two different drivers.
> Driver in mtk-vpu is for controlling VPU device and provide
> communication API to VPU.
> Driver in mtk-vcodec is for control different encoder (vp8, h264), it
> include v4l2 driver layer, glue layer between encoders and vp8 and h264
> encoder.
They may be separate pieces of hardware the drivers for them are very
clearly interlinked. This is obvious because the Makefiles are having
to set ccflags to pick up the headers of the other drivers.
No other V4L2 driver uses ccflags-y in this manner.
>> > diff --git a/drivers/media/platform/mtk-vcodec/common/Makefile b/drivers/media/platform/mtk-vcodec/common/Makefile
>> > new file mode 100644
>> > index 0000000..477ab80
>> > --- /dev/null
>> > +++ b/drivers/media/platform/mtk-vcodec/common/Makefile
>> > @@ -0,0 +1,8 @@
>> > +obj-y += \
>> > + venc_drv_if.o
>> > +
>> > +ccflags-y += \
>> > + -I$(srctree)/include/ \
>> > + -I$(srctree)/drivers/media/platform/mtk-vcodec \
>> > + -I$(srctree)/drivers/media/platform/mtk-vcodec/include \
>> > + -I$(srctree)/drivers/media/platform/mtk-vpu
>>
>> As above, this appears to be a directory to hold just one file.
>>
> Sorry, I didn't get it. Could you explain more?
Just that this is another example of the excessive directory structure.
A directory that contains only one source file is a strong indication
that the splitting of the V4L2 implementation into directories is
excessive.
>> > diff --git a/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
>> b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
>> > new file mode 100644
>> > index 0000000..9b3f025
>> > --- /dev/null
>> > +++ b/drivers/media/platform/mtk-vcodec/common/venc_drv_if.c
>> > @@ -0,0 +1,152 @@
>> > +/*
>> > + * Copyright (c) 2015 MediaTek Inc.
>> > + * Author: Daniel Hsiao <daniel.hsiao@mediatek.com>
>> > + * Jungchang Tsao <jungchang.tsao@mediatek.com>
>> > + *
>> > + * This program is free software; you can redistribute it and/or
>> > + * modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> > + * GNU General Public License for more details.
>> > + */
>> > +
>> > +#include <linux/interrupt.h>
>> > +#include <linux/kernel.h>
>> > +#include <linux/slab.h>
>> > +
>> > +#include "mtk_vcodec_drv.h"
>> > +#include "mtk_vcodec_enc.h"
>> > +#include "mtk_vcodec_pm.h"
>> > +#include "mtk_vcodec_util.h"
>> > +#include "mtk_vpu_core.h"
>> > +
>> > +#include "venc_drv_if.h"
>> > +#include "venc_drv_base.h"
>> > +
>> > +
>> > +int venc_if_create(void *ctx, unsigned int fourcc, unsigned long
>> *handle)
>> > +{
>> > + struct venc_handle *h;
>> > + char str[10];
>> > +
>> > + mtk_vcodec_fmt2str(fourcc, str);
>> > +
>> > + h = kzalloc(sizeof(*h), GFP_KERNEL);
>> > + if (!h)
>> > + return -ENOMEM;
>> > +
>> > + h->fourcc = fourcc;
>> > + h->ctx = ctx;
>> > + mtk_vcodec_debug(h, "fmt = %s handle = %p", str, h);
>> > +
>> > + switch (fourcc) {
>> > + default:
>> > + mtk_vcodec_err(h, "invalid format %s", str);
>> > + goto err_out;
>> > + }
>> > +
>> > + *handle = (unsigned long)h;
>> > + return 0;
>> > +
>> > +err_out:
>> > + kfree(h);
>> > + return -EINVAL;
>> > +}
>> > +
>> > +int venc_if_init(unsigned long handle)
>> > +{
>> > + int ret = 0;
>> > + struct venc_handle *h = (struct venc_handle *)handle;
>> > +
>> > + mtk_vcodec_debug_enter(h);
>> > +
>> > + mtk_venc_lock(h->ctx);
>> > + mtk_vcodec_enc_clock_on();
>> > + vpu_enable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
>> > + ret = h->enc_if->init(h->ctx, (unsigned long *)&h->drv_handle);
>> > + vpu_disable_clock(vpu_get_plat_device(h->ctx->dev->plat_dev));
>> > + mtk_vcodec_enc_clock_off();
>> > + mtk_venc_unlock(h->ctx);
>> > +
>> > + return ret;
>> > +}
>>
>> To me this looks more like an obfuscation layer rather than a
>> abstraction layer. I don't understand why we need to hide things from
>> the V4L2 implementation that this code forms part of.
>>
>> More importantly, if this code was included somewhere where it could be
>> properly integrated with the device model you might be able to use the
>> pm_runtime system to avoid this sort of "heroics" to manage the clocks
>> anyway.
>>
> We want to abstract common part from encoder driver.
> Every encoder driver follow same calling flow and only need to take care
> about how to communicate with vpu to encode specific format.
> Encoder driver do not need to take care clock and multiple instance
> issue.
Looking at each of those stages:
mtk_venc_lock():
Why isn't one of the existing V4L2 locking strategies ok for you?
mtk_vcodec_enc_clock_on():
This does seem like something a sub-driver *should* be doing for itself
vpu_enable_clock():
Why can't the VPU driver manage this internally using pm_runtime?
That is why I described this as an obfuscation layer. It is collecting
a bunch of stuff that can be handled using the kernel driver model and
clumping them together in a special middle layer.
>> > +/**
>> > + * enum mtk_instance_type - The type of an MTK Vcodec instance.
>> > + */
>> > +enum mtk_instance_type {
>> > + MTK_INST_DECODER = 0,
>> > + MTK_INST_ENCODER = 1,
>> > +};
>> > +
>> > +/**
>> > + * enum mtk_instance_state - The state of an MTK Vcodec instance.
>> > + * @MTK_STATE_FREE - default state when instance create
>> > + * @MTK_STATE_CREATE - vdec instance is create
>> > + * @MTK_STATE_INIT - vdec instance is init
>> > + * @MTK_STATE_CONFIG - reserved for encoder
>> > + * @MTK_STATE_HEADER - vdec had sps/pps header parsed
>> > + * @MTK_STATE_RUNNING - vdec is decoding
>> > + * @MTK_STATE_FLUSH - vdec is flushing
>> > + * @MTK_STATE_RES_CHANGE - vdec detect resolution change
>> > + * @MTK_STATE_FINISH - ctx instance is stopped streaming
>> > + * @MTK_STATE_DEINIT - before release ctx instance
>> > + * @MTK_STATE_ERROR - vdec has something wrong
>> > + * @MTK_STATE_ABORT - abort work in working thread
>> > + */
>> > +enum mtk_instance_state {
>> > + MTK_STATE_FREE = 0,
>> > + MTK_STATE_CREATE = (1 << 0),
>> > + MTK_STATE_INIT = (1 << 1),
>> > + MTK_STATE_CONFIG = (1 << 2),
>> > + MTK_STATE_HEADER = (1 << 3),
>> > + MTK_STATE_RUNNING = (1 << 4),
>> > + MTK_STATE_FLUSH = (1 << 5),
>> > + MTK_STATE_RES_CHANGE = (1 << 6),
>> > + MTK_STATE_FINISH = (1 << 7),
>> > + MTK_STATE_DEINIT = (1 << 8),
>> > + MTK_STATE_ERROR = (1 << 9),
>> > + MTK_STATE_ABORT = (1 << 10),
>>
>> This looks like it started as a state machine and somehow turned into
>> flags, resulting in a state machine with 2048 states or, to give it a
>> different name, a debugging nightmare.
>>
> It's define some state happened rather than state machine.
> Though some states are for v4l2 decoder driver and not used in encoder
> driver.
Saying the flags track when "something happened" doesn't stop this
from being an extremely complex (and poorly documented) state machine.
There are way too many states compared to what is needed to implement
V4L2 correctly. To make clear why I am raising this point: with the
current driver state management code it is close to impossible to
properly review the error paths in this driver. The cause of error and
the recovery after error are too decoupled.
>> If the start streaming operation implemented cleanup-on-error properly
>> then there would only be two useful states: Started and stopped. Even
>> the "sticky" error behavior looks unnecessary to me (meaning we don't
>> need to track its state).
>>
> We cannot guaranteed that IOCTLs called from the user space follow
> required sequence.
> We need states to know if our driver could accept IOCTL command.
I believe that knowing whether the streaming is started or stopped
(e.g. two states) is sufficient for a driver to correctly handle
abitrary ioctls from userspace and even then, the core code tracks
this state for you so there's no need for you do it.
The queue/dequeue ioctls succeed or fail based on the length of the
queue (i.e. is the buffer queue overflowing or not) and have no need
to check the streaming state.
If you are absolutely sure that the other states are needed then
please provide an example of an ioctl() sequence where the additional
state is needed.
>> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
>> > new file mode 100644
>> > index 0000000..8e1b6f0
>> > --- /dev/null
>> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
>> > @@ -0,0 +1,1773 @@
>> > [...]
>> > +static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
>> > +{
>> > + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(q);
>> > + struct v4l2_device *v4l2_dev = &ctx->dev->v4l2_dev;
>> > + int ret;
>> > +#if MTK_V4L2_BENCHMARK
>> > + struct timeval begin, end;
>> > +
>> > + do_gettimeofday(&begin);
>> > +#endif
>> > +
>> > + if (!(vb2_start_streaming_called(&ctx->m2m_ctx->out_q_ctx.q) &
>> > + vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q))) {
>> > + mtk_v4l2_debug(1, "[%d]-> out=%d cap=%d",
>> > + ctx->idx,
>> > + vb2_start_streaming_called(&ctx->m2m_ctx->out_q_ctx.q),
>> > + vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q));
>> > + return 0;
>> > + }
>> > +
>> > + if ((ctx->state & (MTK_STATE_ERROR | MTK_STATE_ABORT)))
>> > + return -EINVAL;
>>
>> This is the sort of thing I mean.
>>
>> This sticky error behaviour means that every subsequent call to
>> vb2ops_venc_start_streaming() will fail. Note also that the user will
>> never try to stop streaming (which can clear the error state) because
>> according to the return code it got when it tried to start streaming we
>> never actually started.
>>
>> This is what I mean about having two many states. From the user's
>> perspective there are only two states. There needs to be a good reason
>> for the driver to manage so many extra secret states internally.
>>
> For my understanding, that vb2ops_venc_start_streaming cannot fail.
I disagree: See
http://lxr.free-electrons.com/source/include/media/videobuf2-core.h#L288
How did you confirm your understanding before replying?
When this function returns an error the simplest (and easiest to
review) error recovery strategy is simply to undo any actions which
have already been performed (like resource allocation) and return an
error code.#
There is no need for the driver to remember that it has already
reported an error. If the userspace tries again then its OK for us to
fail again.
> If it fail, user space will close and release this encoder instance
> (fd).
The userspace is not required to do this and the driver must not
assume that it will. It could attempt some kind of reconfiguration and
retry.
> We really need to state driver to see what it should do when receive
> current IOCTL.
I think you'll find that the v4l2-core does this for you.
>> > +
>> > + if (ctx->state == MTK_STATE_FREE) {
>> > + ret = venc_if_create(ctx,
>> > + ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc,
>> > + &ctx->h_enc);
>> > +
>> > + if (ret != 0) {
>> > + ctx->state |= MTK_STATE_ERROR;
>> > + v4l2_err(v4l2_dev, "invalid codec type=%x\n",
>> > + ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc);
>> > + v4l2_err(v4l2_dev, "venc_if_create failed=%d\n", ret);
>> > + return -EINVAL;
>> > + }
>> > +
>> > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->fourcc ==
>> > + V4L2_PIX_FMT_H264)
>> > + ctx->hdr = 1;
>> > +
>> > + ctx->state |= MTK_STATE_CREATE;
>> > + }
>> > +
>> > + if ((ctx->state & MTK_STATE_CREATE) && !(ctx->state & MTK_STATE_INIT)) {
>> > + ret = venc_if_init(ctx->h_enc);
>> > + if (ret != 0) {
>> > + ctx->state |= MTK_STATE_ERROR;
>> > + v4l2_err(v4l2_dev, "venc_if_init failed=%d\n", ret);
>> > + return -EINVAL;
>>
>> This error path leaves the encoder partially constructed and relies on
>> something else to tidy things up. It would be much better to tidy things
>> up from this function and
>>
>> Also I don't think both venc_if_create and venc_if_init are needed. They
>> are only ever called one after the other and thus they only serve to
>> complicate the error handling code.
>>
> venc_if_create is for creating instance in arm side and base on encode
> format hook corresponding encoder driver interface.
> venc_if_init is trying to init encoder instance in VPU side.
> Failures from two functions should have different error handling.
> We will enhance this part in next version.
As mentioned above, I'm very uncomfortable about this API in its
entirety and think it should be reconsidered.
So whilst I disagree here (the caller does not have any significant
difference in error handling so using -ENOMEM/-EINVAL/-EIO should be
quite sufficient to distringuish between errors) I rather you spent
some time trying to eliminate this API.
>> > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
>> > new file mode 100644
>> > index 0000000..a8e683a
>> > --- /dev/null
>> > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
>> > @@ -0,0 +1,66 @@
>> > +/*
>> > +* Copyright (c) 2015 MediaTek Inc.
>> > +* Author: PC Chen <pc.chen@mediatek.com>
>> > +* Tiffany Lin <tiffany.lin@mediatek.com>
>> > +*
>> > +* This program is free software; you can redistribute it and/or modify
>> > +* it under the terms of the GNU General Public License version 2 as
>> > +* published by the Free Software Foundation.
>> > +*
>> > +* This program is distributed in the hope that it will be useful,
>> > +* but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> > +* GNU General Public License for more details.
>> > +*/
>> > +
>> > +#ifndef _MTK_VCODEC_UTIL_H_
>> > +#define _MTK_VCODEC_UTIL_H_
>> > +
>> > +#include <linux/types.h>
>> > +#include <linux/dma-direction.h>
>> > +
>> > +struct mtk_vcodec_mem {
>> > + size_t size;
>> > + void *va;
>> > + dma_addr_t dma_addr;
>> > +};
>> > +
>> > +extern int mtk_v4l2_dbg_level;
>> > +extern bool mtk_vcodec_dbg;
>> > +
>> > +#define mtk_v4l2_debug(level, fmt, args...) \
>> > + do { \
>> > + if (mtk_v4l2_dbg_level >= level) \
>> > + pr_info("[MTK_V4L2] level=%d %s(),%d: " fmt "\n",\
>> > + level, __func__, __LINE__, ##args); \
>> > + } while (0)
>> > +
>> > +#define mtk_v4l2_err(fmt, args...) \
>> > + pr_err("[MTK_V4L2][ERROR] %s:%d: " fmt "\n", __func__, __LINE__, \
>> > + ##args)
>>
>> Obviously the code should be structured to make use of dev_dbg/dev_err
>> possible.
>>
>> However where this won't work do you really need special macros for
>> this. Assuming your error messages are well written 'git grep' and the
>> following should be enough:
>>
>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>
> Thanks.
> For pr_err case, we will try to use "#define pr_fmt(fmt) KBUILD_MODNAME
> ": " fmt" in next version.
> For pr_info case, we still need debug level to control output messages.
To be honest I expect new code to be able to rely on -DDEBUG and/or
CONFIG_DYNAMIC_DEBUG.
I really can't see why a single V4L2 driver needs to hand roll a six
level debug message framework. If it really, really, really needs it
then it should at least have the good manners to copy the prior art in
the existing V4L2 drivers.
>
>>
>> > +#define mtk_v4l2_debug_enter() mtk_v4l2_debug(5, "+\n")
>> > +#define mtk_v4l2_debug_leave() mtk_v4l2_debug(5, "-\n")
>>
>> Remove these. If you care about function entry and exit for debugging
>> you should be able to use ftrace.
>>
> I am not familiar with ftrace.
> What if we only want to trace v4l2 video encoder driver called flow not
> called stack? And only for functions we are interested not all
> functions.
> I will check if it is convenience for us using ftrace.
It is find for ftrace to only track a subset of functions.
Daniel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web