Re: [PATCH v9 3/6] media: dt-bindings: Add Amlogic V4L2 video decoder

[Zhentao Guo]

Hi Krzysztof,

> On Mon, Sep 14, 2026 at 04:10:20PM +0800, Zhentao Guo wrote:
> > Describe the initial support for the V4L2 stateless video decoder
> > driver used with the Amlogic S4 (S805X2) platform.
> Drop driver. Describe the hardware instead.
OK，got it.
> > Signed-off-by: Zhentao Guo <zhentao.guo@xxxxxxxxxxx>
> > ---
> >   .../devicetree/bindings/media/amlogic,s4-vdec.yaml | 114 +++++++++++++++++++++
> >   1 file changed, 114 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml b/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
> > new file mode 100644
> > index 000000000000..d350b3e68a38
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
> > @@ -0,0 +1,114 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2025 Amlogic, Inc. All rights reserved
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/amlogic,s4-vdec.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Amlogic Video Decode Accelerator
> > +
> > +maintainers:
> > +  - Zhentao Guo <zhentao.guo@xxxxxxxxxxx>
> > +
> > +description:
> > +  The Video Decoder Accelerator present on Amlogic SOCs.
> > +  It supports stateless h264 decoding.
> > +
> > +properties:
> > +  compatible:
> > +    const: amlogic,s4-vdec
> > +
> > +  reg:
> > +    maxItems: 2
> > +
> > +  reg-names:
> > +    items:
> > +      - const: dos
> > +      - const: dmc
> > +
> > +  interrupts:
> > +    maxItems: 2
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: core_hvdec
> > +      - const: core_vdec
> Drop "core_" from both... or rather some consistency would be more
> useful. Below you call them core_v and core_h, but there is also:
>
> > +
> > +  clocks:
> > +    items:
> > +      - description: Clock for dos core
> core dos?

To explain this, I would like to show you the structure diagram of the 
Amlogic hardware decoder IP:

+-----------+------------------------+
|           |                        |
|           |        vdec            |
|           |                        |
|    dos    +------------------------+
|           |                        |
|           |        hvdec           |
|           |                        |
+-----------+------------------------+

The IP consists of three cores. Dos core is in the front of the IP, 
which controls the rest two cores. The actual hardware decoding is 
handled by the two cores vdec and hvdec.

Each of the three cores has its own independent clock source.

> > +      - description: Clock for vdec core
> > +      - description: Clock for hvdec core
> > +
> > +  clock-names:
> > +    items:
> > +      - const: dos
> > +      - const: core_v
> hvdec?
>
> > +      - const: core_h
> vdec? to keep the naming and order of interrupts?
Actually, in the interrupt vector table they are arranged exactly this 
way: hvdec comes first, followed by vdec. This is indeed the reverse of 
the order used for clk and power-domain.
> > +
> > +  power-domains:
> > +    maxItems: 2
> > +
> > +  power-domain-names:
> > +    items:
> > +      - const: dos_v
> > +      - const: dos_h
> So there is also dos variant v and h?
>
> Pretty confusing.

That is indeed a bit odd, I'll give them new names.

Also, I'd like to ask your advice on these names. I remember that in 
your review comments on v7, you mentioned that having "vdec" as the 
module name and then reusing it for the clock/interrupt/power-domain 
name felt repetitive. But as shown in the diagram above, the decoder 
hardware does have a sub-core called vdec, so do you think it is 
appropriate if I'm also using "vdec" to name its clock, power-domain, 
and interrupt, to indicate that these all used by vdec core? I would be 
grateful for your feedback on this.
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  amlogic,canvas:
> > +    description: Canvas maintains an index-to-physical-address
> > +     mapping table that allows the 16-bit AMRISC decoder core
> > +     to access 32-bit or 64-bit physical address.
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +
> > +  amlogic,secure-monitor:
> > +    description: Used to decrypt and load the signed decoder
> > +     firmware through ARM SMC (Secure Monitor Call).
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - power-domains
> > +  - power-domain-names
> > +  - resets
> > +  - amlogic,canvas
> > +  - amlogic,secure-monitor
> > +
> > +additionalProperties: false
> Best regards,
> Krzysztof

BRs

Zhentao