On 14/12/15 09:26, tiffany lin wrote:

On Fri, 2015-12-11 at 11:29 -0600, Rob Herring wrote:

quoted

On Fri, Dec 11, 2015 at 05:55:39PM +0800, Tiffany Lin wrote:

quoted

Add a DT binding documentation of Video Encoder for the
MT8173 SoC from Mediatek.

Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>

A question and minor issue below, otherwise:

Acked-by: Rob Herring <robh@kernel.org>

quoted

---
  .../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..510cd81
--- /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.
+- mediatek,larb : must contain the local arbiters in the current Socs.

This looks strange, shouldn't it be "larb" instead of "mediatek,larb".
At least the example does not use the mediatek prefix.

quoted

quoted

+- 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.
+- mediatek,vpu : the node of video processor unit

Same here.

Regards,
Matthias

quoted

quoted

+
+Example:
+vcodec_enc: vcodec at 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>,

Is this the same iommu as the VPU? If so, you can't have a mixed number
of cells.

Yes, its same iommus as the VPU.
Now we use two parameters for iommus.
We will fix this in next version.

quoted

quoted

+             <&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>;

Need to update the example.

Sorry, I didn't get it.
Do you means update VPU binding document "media/mediatek-vpu.txt"?

quoted

quoted

+    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