diff mbox series

[RFC,4/4] dt-bindings: media: add Amlogic Meson Video Decoder Bindings

Message ID 20180801193320.25313-5-maxi.jourdan@wanadoo.fr (mailing list archive)
State New, archived
Headers show
Series media: meson: add video decoder driver | expand

Commit Message

Maxime Jourdan Aug. 1, 2018, 7:33 p.m. UTC
Add documentation for the meson vdec dts node.

Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr>
---
 .../bindings/media/amlogic,meson-vdec.txt     | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt

Comments

Martin Blumenstingl Aug. 1, 2018, 8:13 p.m. UTC | #1
Hi Maxime,

many thanks for your work!

On Wed, Aug 1, 2018 at 9:34 PM Maxime Jourdan <maxi.jourdan@wanadoo.fr> wrote:
>
> Add documentation for the meson vdec dts node.
>
> Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr>
> ---
>  .../bindings/media/amlogic,meson-vdec.txt     | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
>
> diff --git a/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt b/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
> new file mode 100644
> index 000000000000..120b135e6bb5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
> @@ -0,0 +1,60 @@
> +Amlogic Meson Video Decoder
> +================================
> +
> +The VDEC IP is composed of the following blocks :
> +
> +- ESPARSER is a bitstream parser that outputs to a VIFIFO. Further VDEC blocks
> +then feed from this VIFIFO.
> +- VDEC_1 can decode MPEG-1, MPEG-2, MPEG-4 part 2, H.263, H.264.
> +- VDEC_2 is used as a helper for corner cases like H.264 4K on older SoCs.
> +It is not handled by this driver.
is it currently not handled or will it never be?

> +- VDEC_HCODEC is the H.264 encoding block. It is not handled by this driver.
> +- VDEC_HEVC can decode HEVC and VP9.
> +
> +Device Tree Bindings:
> +---------------------
> +
> +VDEC: Video Decoder
> +--------------------------
> +
> +Required properties:
> +- compatible: value should be different for each SoC family as :
> +       - GXBB (S905) : "amlogic,meson-gxbb-vdec"
> +       - GXL (S905X, S905D) : "amlogic,meson-gxl-vdec"
> +       - GXM (S912) : "amlogic,meson-gxm-vdec"
> +- reg: base address and size of he following memory-mapped regions :
> +       - dos
> +       - esparser
> +       - dmc
> +- reg-names: should contain the names of the previous memory regions
any reason why you are not using the DMC syscon (as added in your
patch "dt-bindings: soc: amlogic: add meson-canvas documentation")
instead of mapping the DMC region again?

> +- interrupts: should contain the vdec and esparser IRQs.
are these two IRQs the "currently supported" ones or are there more
for the whole IP block (but just not implemented yet)?

> +- clocks: should contain the following clocks :
> +       - dos_parser
> +       - dos
> +       - vdec_1
> +       - vdec_hevc
> +- clock-names: should contain the names of the previous clocks
> +- resets: should contain the parser reset.
> +- reset-names: should be "esparser".
> +
> +Example:
> +
> +vdec: video-decoder@0xd0050000 {
> +       compatible = "amlogic,meson-gxbb-vdec";
> +       reg = <0x0 0xc8820000 0x0 0x10000
> +              0x0 0xc110a580 0x0 0xe4
> +              0x0 0xc8838000 0x0 0x60>;
AFAIK the "correct" format is (just like you've done for the clocks below):
       reg = <0x0 0xc8820000 0x0 0x10000>,
                 <0x0 0xc110a580 0x0 0xe4>,
                 <0x0 0xc8838000 0x0 0x60>;

> +       reg-names = "dos", "esparser", "dmc";
> +
> +       interrupts = <GIC_SPI 44 IRQ_TYPE_EDGE_RISING
> +                     GIC_SPI 32 IRQ_TYPE_EDGE_RISING>;
AFAIK the "correct" format is (just like you've done for the clocks below):
       interrupts = <GIC_SPI 44 IRQ_TYPE_EDGE_RISING>,
                           <GIC_SPI 32 IRQ_TYPE_EDGE_RISING>;

> +       interrupt-names = "vdec", "esparser";
> +
> +       amlogic,ao-sysctrl = <&sysctrl_AO>;
this is not documented above - is it needed?


Regards
Martin


[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-August/008034.html
Jerome Brunet Aug. 2, 2018, 10:33 a.m. UTC | #2
On Wed, 2018-08-01 at 21:33 +0200, Maxime Jourdan wrote:
> Add documentation for the meson vdec dts node.
> 
> Signed-off-by: Maxime Jourdan <maxi.jourdan@wanadoo.fr>
> ---
>  .../bindings/media/amlogic,meson-vdec.txt     | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt

Maxime, when formatting your patchset, remember to put the bindings
documentation before actually using them. This patch could be the first one of
your series.
Maxime Jourdan Aug. 2, 2018, 12:48 p.m. UTC | #3
Hi Martin & Jerome,

2018-08-02 12:33 GMT+02:00 Jerome Brunet <jbrunet@baylibre.com>:
> Maxime, when formatting your patchset, remember to put the bindings
> documentation before actually using them. This patch could be the first one of
> your series.

Noted, thanks.

2018-08-01 22:13 GMT+02:00 Martin Blumenstingl
<martin.blumenstingl@googlemail.com>:
>> +- VDEC_2 is used as a helper for corner cases like H.264 4K on older SoCs.
>> +It is not handled by this driver.
> is it currently not handled or will it never be?

I don't think it will ever be, at least from me. This VDEC unit is
rarely used and only for a few corner cases on SoCs like meson8b, and
I have no intention of supporting them for now as there are other
limitations.

> any reason why you are not using the DMC syscon (as added in your
> patch "dt-bindings: soc: amlogic: add meson-canvas documentation")
> instead of mapping the DMC region again?

To answer you and Jerome, I didn't use it because I wanted to keep
both patchsets separate in case of testing. In hindsight though, I
should have used the canvas module in the vdec in the RFC.
So yeah, this will definitely be used by the final product.

>> +- interrupts: should contain the vdec and esparser IRQs.
> are these two IRQs the "currently supported" ones or are there more
> for the whole IP block (but just not implemented yet)?

There are more IRQs within the VDEC but they are not used at the
moment. Some are for the demuxer, VDEC_2, etc..

> AFAIK the "correct" format is (just like you've done for the clocks below):
>        reg = <0x0 0xc8820000 0x0 0x10000>,
>                  <0x0 0xc110a580 0x0 0xe4>,
>                  <0x0 0xc8838000 0x0 0x60>;
>

> AFAIK the "correct" format is (just like you've done for the clocks below):
>        interrupts = <GIC_SPI 44 IRQ_TYPE_EDGE_RISING>,
>                            <GIC_SPI 32 IRQ_TYPE_EDGE_RISING>;
>

>> +       amlogic,ao-sysctrl = <&sysctrl_AO>;
> this is not documented above - is it needed?

Duly noted, thanks.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt b/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
new file mode 100644
index 000000000000..120b135e6bb5
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/amlogic,meson-vdec.txt
@@ -0,0 +1,60 @@ 
+Amlogic Meson Video Decoder
+================================
+
+The VDEC IP is composed of the following blocks :
+
+- ESPARSER is a bitstream parser that outputs to a VIFIFO. Further VDEC blocks
+then feed from this VIFIFO.
+- VDEC_1 can decode MPEG-1, MPEG-2, MPEG-4 part 2, H.263, H.264.
+- VDEC_2 is used as a helper for corner cases like H.264 4K on older SoCs.
+It is not handled by this driver.
+- VDEC_HCODEC is the H.264 encoding block. It is not handled by this driver.
+- VDEC_HEVC can decode HEVC and VP9.
+
+Device Tree Bindings:
+---------------------
+
+VDEC: Video Decoder
+--------------------------
+
+Required properties:
+- compatible: value should be different for each SoC family as :
+	- GXBB (S905) : "amlogic,meson-gxbb-vdec"
+	- GXL (S905X, S905D) : "amlogic,meson-gxl-vdec"
+	- GXM (S912) : "amlogic,meson-gxm-vdec"
+- reg: base address and size of he following memory-mapped regions :
+	- dos
+	- esparser
+	- dmc
+- reg-names: should contain the names of the previous memory regions
+- interrupts: should contain the vdec and esparser IRQs.
+- clocks: should contain the following clocks :
+	- dos_parser
+	- dos
+	- vdec_1
+	- vdec_hevc
+- clock-names: should contain the names of the previous clocks
+- resets: should contain the parser reset.
+- reset-names: should be "esparser".
+
+Example:
+
+vdec: video-decoder@0xd0050000 {
+	compatible = "amlogic,meson-gxbb-vdec";
+	reg = <0x0 0xc8820000 0x0 0x10000
+	       0x0 0xc110a580 0x0 0xe4
+	       0x0 0xc8838000 0x0 0x60>;
+	reg-names = "dos", "esparser", "dmc";
+
+	interrupts = <GIC_SPI 44 IRQ_TYPE_EDGE_RISING
+		      GIC_SPI 32 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "vdec", "esparser";
+
+	amlogic,ao-sysctrl = <&sysctrl_AO>;
+
+	clocks = <&clkc CLKID_DOS_PARSER>, <&clkc CLKID_DOS>, <&clkc CLKID_VDEC_1>, <&clkc CLKID_VDEC_HEVC>;
+	clock-names = "dos_parser", "dos", "vdec_1", "vdec_hevc";
+
+	resets = <&reset RESET_PARSER>;
+	reset-names = "esparser";
+};