Re: [PATCH v2 1/7] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding

[Krzysztof Kozlowski <krzk@kernel.org>]

On 27/02/2026 15:11, Vikash Garodia wrote:
> Kaanapali SOC brings in the new generation of video IP i.e iris4. When
> compared to previous generation, iris3x, it has,
> - separate power domains for stream and pixel processing hardware blocks
>   (bse and vpp).
> - additional power domain for apv codec.
> - power domains for individual pipes (VPPx).
> - different clocks and reset lines.
> 
> iommu-map include all the different stream-ids which can be possibly
> generated by vpu4 hardware as below,
> bitstream stream from vcodec
> non-pixel stream from vcodec
> non-pixel stream from tensilica
> pixel stream from vcodec
> secure bitstream stream from vcodec
> secure non-pixel stream from vcodec
> secure non-pixel stream from tensilica
> secure pixel stream from vcodec
> firmware stream from tensilica (might be handled by the TZ / hyp)

I cannot really parse above.

> 
> This patch is depend on the below dt-schema patch.
> Link: https://github.com/devicetree-org/dt-schema/pull/184/changes/d341298d62805bc972dfba691da6b3b62aa3ff15
> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> ---
>  .../bindings/media/qcom,kaanapali-iris.yaml        | 261 +++++++++++++++++++++
>  include/dt-bindings/media/qcom,iris.h              |  18 ++
>  2 files changed, 279 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,kaanapali-iris.yaml b/Documentation/devicetree/bindings/media/qcom,kaanapali-iris.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..db734c664a0417d8f5ea55b066f63f42583b1c14
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-iris.yaml
> @@ -0,0 +1,261 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,kaanapali-iris.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Kaanapali Iris video encoder and decoder
> +
> +maintainers:
> +  - Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> +  - Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> +
> +description:
> +  The iris video processing unit is a video encode and decode accelerator
> +  present on Qualcomm Kaanapali SoC.
> +
> +definitions:
> +  iommu-types:
> +    items:
> +      - description: Function ID
> +      - description: Phandle to IOMMU
> +      - description: IOMMU stream ID base
> +      - description: IOMMU stream ID mask
> +      - description: Number of stream IDs

You don't need definitions.

> +
> +properties:
> +  compatible:
> +    const: qcom,kaanapali-iris
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 10
> +
> +  clock-names:
> +    items:
> +      - const: iface
> +      - const: core
> +      - const: vcodec0_core
> +      - const: iface1
> +      - const: core_freerun
> +      - const: vcodec0_core_freerun
> +      - const: vcodec_bse
> +      - const: vcodec_vpp0
> +      - const: vcodec_vpp1
> +      - const: vcodec_apv
> +
> +  dma-coherent: true
> +
> +  firmware-name:
> +    maxItems: 1
> +
> +  interconnects:
> +    maxItems: 2
> +
> +  interconnect-names:
> +    items:
> +      - const: cpu-cfg
> +      - const: video-mem
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  iommu-map:
> +    description: |
> +        - bitstream stream from vcodec
> +        - non-pixel stream from vcodec
> +        - non-pixel stream from tensilica
> +        - pixel stream from vcodec
> +        - secure bitstream stream from vcodec
> +        - secure non-pixel stream from vcodec
> +        - secure non-pixel stream from tensilica
> +        - secure pixel stream from vcodec
> +        # firmware might be handled by the TZ / hyp
> +        - firmware stream from tensilica

Drop description, pointless. How does it help anyhow in writing DTS?

> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    items:
> +      $ref: '#/definitions/iommu-types'
> +      minItems: 5
> +    minItems: 8
> +    maxItems: 9

This solves nothing from the discussions we had. I re-iterated multiple
times, plus there were internal threads doing the same, that you need to
define the items to have strict order.

> +
> +  memory-region:
> +    maxItems: 1
> +
> +  operating-points-v2: true
> +  opp-table:
> +    type: object
> +
> +  power-domains:
> +    maxItems: 7
> +
> +  power-domain-names:
> +    items:
> +      - const: venus
> +      - const: vcodec0
> +      - const: mxc
> +      - const: mmcx
> +      - const: vpp0
> +      - const: vpp1
> +      - const: apv
> +
> +  resets:
> +    maxItems: 4
> +
> +  reset-names:
> +    items:
> +      - const: bus0
> +      - const: bus1
> +      - const: core
> +      - const: vcodec0_core
> +

I assume you checked and used last SoC order of all the entries here.


> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - dma-coherent
> +  - interconnects
> +  - interconnect-names
> +  - interrupts
> +  - iommu-map
> +  - memory-region
> +  - power-domains
> +  - power-domain-names
> +  - resets
> +  - reset-names
> +
> +unevaluatedProperties: false

additionalprops, see writing schema for explanation.


...

> +    };
> diff --git a/include/dt-bindings/media/qcom,iris.h b/include/dt-bindings/media/qcom,iris.h

Filename must match binding/compatible. I asked for this multiple times.

> new file mode 100644
> index 0000000000000000000000000000000000000000..beb244289466ca938c7e5fe5cf15526f606a3a6c
> --- /dev/null
> +++ b/include/dt-bindings/media/qcom,iris.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef _DT_BINDINGS_MEDIA_QCOM_IRIS_H
> +#define _DT_BINDINGS_MEDIA_QCOM_IRIS_H
> +
> +/* Function identifiers for iommu-map to attach for the context bank devices */
> +#define IRIS_BITSTREAM		0x100
> +#define IRIS_NON_PIXEL		0x101
> +#define IRIS_PIXEL		0x102
> +#define IRIS_SECURE_BITSTREAM	0x200
> +#define IRIS_SECURE_NON_PIXEL	0x201
> +#define IRIS_SECURE_PIXEL	0x202
> +#define IRIS_FIRMWARE		0x300
> +

Abstract identifiers for ABI start with 0 or 1 and are incremented by 1,
not 0x100. There are also decimal, not hex.

Best regards,
Krzysztof