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

2018-08-02 Thread Maxime Jourdan
Hi Martin & Jerome,

2018-08-02 12:33 GMT+02:00 Jerome Brunet :
> 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
:
>> +- 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 0xc882 0x0 0x1>,
>  <0x0 0xc110a580 0x0 0xe4>,
>  <0x0 0xc8838000 0x0 0x60>;
>

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

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

Duly noted, thanks.


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

2018-08-02 Thread Jerome Brunet
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 
> ---
>  .../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.





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

2018-08-01 Thread Martin Blumenstingl
Hi Maxime,

many thanks for your work!

On Wed, Aug 1, 2018 at 9:34 PM Maxime Jourdan  wrote:
>
> Add documentation for the meson vdec dts node.
>
> Signed-off-by: Maxime Jourdan 
> ---
>  .../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 ..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@0xd005 {
> +   compatible = "amlogic,meson-gxbb-vdec";
> +   reg = <0x0 0xc882 0x0 0x1
> +  0x0 0xc110a580 0x0 0xe4
> +  0x0 0xc8838000 0x0 0x60>;
AFAIK the "correct" format is (just like you've done for the clocks below):
   reg = <0x0 0xc882 0x0 0x1>,
 <0x0 0xc110a580 0x0 0xe4>,
 <0x0 0xc8838000 0x0 0x60>;

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

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


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

2018-08-01 Thread Maxime Jourdan
Add documentation for the meson vdec dts node.

Signed-off-by: Maxime Jourdan 
---
 .../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 ..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@0xd005 {
+   compatible = "amlogic,meson-gxbb-vdec";
+   reg = <0x0 0xc882 0x0 0x1
+  0x0 0xc110a580 0x0 0xe4
+  0x0 0xc8838000 0x0 0x60>;
+   reg-names = "dos", "esparser", "dmc";
+
+   interrupts = ;
+   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";
+};
-- 
2.17.1