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

Vikash Garodia posted 8 patches 3 months, 3 weeks ago
[PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Vikash Garodia 3 months, 3 weeks ago
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.

Iommus include all the different stream-ids which can be possibly
generated by vpu4 video hardware in both secure and non secure execution
mode.
The vpu have reserved iova, i.e some portion of the addressable range is
reserved for IO registers. Iris_resv would describe the acceptable
address range.

Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
 .../bindings/media/qcom,kaanapali-iris.yaml        | 231 +++++++++++++++++++++
 1 file changed, 231 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..39e9ac9dad2212e5ae8dc3d45e764418fe7d358d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,kaanapali-iris.yaml
@@ -0,0 +1,231 @@
+# 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.
+
+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
+
+  iommus:
+    minItems: 3
+    maxItems: 8
+
+  memory-region:
+    minItems: 1
+    maxItems: 2
+
+  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_freerun_reset
+      - const: vcodec0_core_freerun_reset
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - dma-coherent
+  - interconnects
+  - interconnect-names
+  - interrupts
+  - iommus
+  - power-domains
+  - power-domain-names
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/qcom,rpmhpd.h>
+
+    video-codec@2000000 {
+        compatible = "qcom,kaanapali-iris";
+        reg = <0x02000000 0xf0000>;
+
+        clocks = <&gcc_video_axi0_clk>,
+                 <&video_cc_mvs0c_clk>,
+                 <&video_cc_mvs0_clk>,
+                 <&gcc_video_axi1_clk>,
+                 <&video_cc_mvs0c_freerun_clk>,
+                 <&video_cc_mvs0_freerun_clk>,
+                 <&video_cc_mvs0b_clk>,
+                 <&video_cc_mvs0_vpp0_clk>,
+                 <&video_cc_mvs0_vpp1_clk>,
+                 <&video_cc_mvs0a_clk>;
+        clock-names = "iface",
+                      "core",
+                      "vcodec0_core",
+                      "iface1",
+                      "core_freerun",
+                      "vcodec0_core_freerun",
+                      "vcodec_bse",
+                      "vcodec_vpp0",
+                      "vcodec_vpp1",
+                      "vcodec_apv";
+
+        dma-coherent;
+
+        interconnects = <&gem_noc_master_appss_proc &config_noc_slave_venus_cfg>,
+                        <&mmss_noc_master_video_mvp &mc_virt_slave_ebi1>;
+        interconnect-names = "cpu-cfg",
+                             "video-mem";
+
+        interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+        iommus = <&apps_smmu 0x1940 0x0>,
+                 <&apps_smmu 0x1944 0x0>,
+                 <&apps_smmu 0x1a20 0x0>,
+                 <&apps_smmu 0x1943 0x0>;
+
+        operating-points-v2 = <&iris_opp_table>;
+
+        memory-region = <&video_mem>, <&iris_resv>;
+
+        power-domains = <&video_cc_mvs0c_gdsc>,
+                        <&video_cc_mvs0_gdsc>,
+                        <&rpmhpd RPMHPD_MXC>,
+                        <&rpmhpd RPMHPD_MMCX>,
+                        <&video_cc_mvs0_vpp0_gdsc>,
+                        <&video_cc_mvs0_vpp1_gdsc>,
+                        <&video_cc_mvs0a_gdsc>;
+        power-domain-names = "venus",
+                             "vcodec0",
+                             "mxc",
+                             "mmcx",
+                             "vpp0",
+                             "vpp1",
+                             "apv";
+
+        resets = <&gcc_video_axi0_clk_ares>,
+                 <&gcc_video_axi1_clk_ares>,
+                 <&video_cc_mvs0c_freerun_clk_ares>,
+                 <&video_cc_mvs0_freerun_clk_ares>;
+        reset-names = "bus0",
+                      "bus1",
+                      "core_freerun_reset",
+                      "vcodec0_core_freerun_reset";
+
+        iris_opp_table: opp-table {
+            compatible = "operating-points-v2";
+
+            opp-240000000 {
+                opp-hz = /bits/ 64 <240000000 240000000 240000000 360000000>;
+                required-opps = <&rpmhpd_opp_low_svs_d1>,
+                                <&rpmhpd_opp_low_svs_d1>;
+            };
+
+            opp-338000000 {
+                opp-hz = /bits/ 64 <338000000 338000000 338000000 507000000>;
+                required-opps = <&rpmhpd_opp_low_svs>,
+                                <&rpmhpd_opp_low_svs>;
+            };
+
+            opp-420000000 {
+                opp-hz = /bits/ 64 <420000000 420000000 420000000 630000000>;
+                required-opps = <&rpmhpd_opp_svs>,
+                                <&rpmhpd_opp_svs>;
+            };
+
+            opp-444000000 {
+                opp-hz = /bits/ 64 <444000000 444000000 444000000 666000000>;
+                required-opps = <&rpmhpd_opp_svs_l1>,
+                                <&rpmhpd_opp_svs_l1>;
+            };
+
+            opp-533000000 {
+                opp-hz = /bits/ 64 <533000000 533000000 533000000 800000000>;
+                required-opps = <&rpmhpd_opp_nom>,
+                                <&rpmhpd_opp_nom>;
+            };
+
+            opp-630000000 {
+                opp-hz = /bits/ 64 <630000000 630000000 630000000 1104000000>;
+                required-opps = <&rpmhpd_opp_turbo>,
+                                <&rpmhpd_opp_turbo>;
+            };
+
+            opp-800000000 {
+                opp-hz = /bits/ 64 <800000000 630000000 630000000 1260000000>;
+                required-opps = <&rpmhpd_opp_turbo_l0>,
+                                <&rpmhpd_opp_turbo_l0>;
+            };
+
+            opp-1000000000 {
+                opp-hz = /bits/ 64 <1000000000 630000000 850000000 1260000000>;
+                required-opps = <&rpmhpd_opp_turbo_l1>,
+                                <&rpmhpd_opp_turbo_l1>;
+            };
+        };
+    };

-- 
2.34.1
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Krzysztof Kozlowski 3 months, 3 weeks ago
On 17/10/2025 16:16, Vikash Garodia wrote:
> +  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
> +
> +  iommus:
> +    minItems: 3
> +    maxItems: 8

I don't understand why this is flexible. Make it fixed size and anyway -
list the items.

I already asked this.

> +
> +  memory-region:
> +    minItems: 1
> +    maxItems: 2

Same comment. I already asked this about iommus.

NAK, we should not repeat the same comment.

Best regards,
Krzysztof
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Vikash Garodia 3 months, 3 weeks ago
On 10/18/2025 9:28 PM, Krzysztof Kozlowski wrote:
> On 17/10/2025 16:16, Vikash Garodia wrote:
>> +  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
>> +
>> +  iommus:
>> +    minItems: 3
>> +    maxItems: 8
> 
> I don't understand why this is flexible. Make it fixed size and anyway -
> list the items.

kaanapali vpu generates 8 different stream-ids. Now, boards running kernel in
EL2 mode can list all of them, while boards running in EL1 can have only non
secure stream IDs. Min have the list of stream ids which can be enabled for all
type of boards, while max is for boards which can list all in HLOS given kernel
is in EL2 mode.

Below crash would be seen if boards running kernel in EL1 mode lists the secure
ones.

[    1.361157] pc : qcom_smmu_write_s2cr+0x64/0xa4
[    1.361165] lr : arm_smmu_write_s2cr+0x2c/0xbc
[    1.361168] sp : ffff80008005b8f0
[    1.361169] x29: ffff80008005b8f0 x28: 0000000000000000 x27: ffffc7f252f45320
....
[    1.361195] x2 : ffff800081200c48 x1 : 0000000000000048 x0 : ffff800081200000
[    1.361198] Call trace:
[    1.361199]  qcom_smmu_write_s2cr+0x64/0xa4 (P)
[    1.361203]  arm_smmu_master_install_s2crs+0x7c/0xac
[    1.361207]  arm_smmu_attach_dev+0xb0/0x1d4

Could you please suggest on listing the iommu items ? I did not find the
relevant references in other bindings where flexible iommus is being listed.

> 
> I already asked this.
> 
>> +
>> +  memory-region:
>> +    minItems: 1
>> +    maxItems: 2
> 
> Same comment. I already asked this about iommus.

Same here, there aren't any bindings which lists for flexible memory-region.
Please suggest if there are any such references.

Regards,
Vikash
> 
> NAK, we should not repeat the same comment.
> 
> Best regards,
> Krzysztof
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Konrad Dybcio 3 months, 2 weeks ago
On 10/21/25 8:55 PM, Vikash Garodia wrote:
> 
> On 10/18/2025 9:28 PM, Krzysztof Kozlowski wrote:
>> On 17/10/2025 16:16, Vikash Garodia wrote:
>>> +  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
>>> +
>>> +  iommus:
>>> +    minItems: 3
>>> +    maxItems: 8
>>
>> I don't understand why this is flexible. Make it fixed size and anyway -
>> list the items.
> 
> kaanapali vpu generates 8 different stream-ids. Now, boards running kernel in
> EL2 mode can list all of them, while boards running in EL1 can have only non
> secure stream IDs. Min have the list of stream ids which can be enabled for all
> type of boards, while max is for boards which can list all in HLOS given kernel
> is in EL2 mode.
> 
> Below crash would be seen if boards running kernel in EL1 mode lists the secure
> ones.
> 
> [    1.361157] pc : qcom_smmu_write_s2cr+0x64/0xa4
> [    1.361165] lr : arm_smmu_write_s2cr+0x2c/0xbc
> [    1.361168] sp : ffff80008005b8f0
> [    1.361169] x29: ffff80008005b8f0 x28: 0000000000000000 x27: ffffc7f252f45320
> ....
> [    1.361195] x2 : ffff800081200c48 x1 : 0000000000000048 x0 : ffff800081200000
> [    1.361198] Call trace:
> [    1.361199]  qcom_smmu_write_s2cr+0x64/0xa4 (P)
> [    1.361203]  arm_smmu_master_install_s2crs+0x7c/0xac
> [    1.361207]  arm_smmu_attach_dev+0xb0/0x1d4
> 
> Could you please suggest on listing the iommu items ? I did not find the
> relevant references in other bindings where flexible iommus is being listed.

Krzysztof would probably like to see what I believe someone else somewhere
sometime suggested in the iommus discussions (sorry it's not possible to
keep track of it all), where the DT can list every possible required iommu
sid, but the driver ensures only the ones that are necessary are utilized.

This will require big changes to the iommu framework though, I'm afraid

>> I already asked this.
>>
>>> +
>>> +  memory-region:
>>> +    minItems: 1
>>> +    maxItems: 2
>>
>> Same comment. I already asked this about iommus.
> 
> Same here, there aren't any bindings which lists for flexible memory-region.
> Please suggest if there are any such references.

Similarly, we can define the additional memory region that's necessary
for $reasons and leave it unused in the driver (actually I don't know
why there may be two, but let's assume it's a QTEE/noQTEE detail), because
for the hw to operate, it must be set up by some entity in the system
either way (i.e. the memory is reserved even if it's not done by Linux)

Konrad
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Krzysztof Kozlowski 3 months, 2 weeks ago
On 22/10/2025 17:36, Konrad Dybcio wrote:
>>
>> [    1.361157] pc : qcom_smmu_write_s2cr+0x64/0xa4
>> [    1.361165] lr : arm_smmu_write_s2cr+0x2c/0xbc
>> [    1.361168] sp : ffff80008005b8f0
>> [    1.361169] x29: ffff80008005b8f0 x28: 0000000000000000 x27: ffffc7f252f45320
>> ....
>> [    1.361195] x2 : ffff800081200c48 x1 : 0000000000000048 x0 : ffff800081200000
>> [    1.361198] Call trace:
>> [    1.361199]  qcom_smmu_write_s2cr+0x64/0xa4 (P)
>> [    1.361203]  arm_smmu_master_install_s2crs+0x7c/0xac
>> [    1.361207]  arm_smmu_attach_dev+0xb0/0x1d4
>>
>> Could you please suggest on listing the iommu items ? I did not find the
>> relevant references in other bindings where flexible iommus is being listed.
> 
> Krzysztof would probably like to see what I believe someone else somewhere
> sometime suggested in the iommus discussions (sorry it's not possible to
> keep track of it all), where the DT can list every possible required iommu
> sid, but the driver ensures only the ones that are necessary are utilized.
> 
> This will require big changes to the iommu framework though, I'm afraid
> 
>>> I already asked this.
>>>
>>>> +
>>>> +  memory-region:
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>
>>> Same comment. I already asked this about iommus.
>>
>> Same here, there aren't any bindings which lists for flexible memory-region.
>> Please suggest if there are any such references.
> 
> Similarly, we can define the additional memory region that's necessary
> for $reasons and leave it unused in the driver (actually I don't know
> why there may be two, but let's assume it's a QTEE/noQTEE detail), because
> for the hw to operate, it must be set up by some entity in the system
> either way (i.e. the memory is reserved even if it's not done by Linux)


Another point is pretty obvious: if one claims that
iommus/memory-regions list is flexible - some elements are optional -
then clearly there is a distinction which elements are mandatory and
which are optional. So there is difference between elements of the
array. If there is a difference, they all must be explicitly listed,
like every other list (clocks, resets etc) property. Writing bindings
doc also defines this rule.

Best regards,
Krzysztof
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Vikash Garodia 3 months ago
On 10/22/2025 9:58 PM, Krzysztof Kozlowski wrote:
> On 22/10/2025 17:36, Konrad Dybcio wrote:
>>>
>>> [    1.361157] pc : qcom_smmu_write_s2cr+0x64/0xa4
>>> [    1.361165] lr : arm_smmu_write_s2cr+0x2c/0xbc
>>> [    1.361168] sp : ffff80008005b8f0
>>> [    1.361169] x29: ffff80008005b8f0 x28: 0000000000000000 x27: ffffc7f252f45320
>>> ....
>>> [    1.361195] x2 : ffff800081200c48 x1 : 0000000000000048 x0 : ffff800081200000
>>> [    1.361198] Call trace:
>>> [    1.361199]  qcom_smmu_write_s2cr+0x64/0xa4 (P)
>>> [    1.361203]  arm_smmu_master_install_s2crs+0x7c/0xac
>>> [    1.361207]  arm_smmu_attach_dev+0xb0/0x1d4
>>>
>>> Could you please suggest on listing the iommu items ? I did not find the
>>> relevant references in other bindings where flexible iommus is being listed.
>>
>> Krzysztof would probably like to see what I believe someone else somewhere
>> sometime suggested in the iommus discussions (sorry it's not possible to
>> keep track of it all), where the DT can list every possible required iommu
>> sid, but the driver ensures only the ones that are necessary are utilized.
>>
>> This will require big changes to the iommu framework though, I'm afraid
>>
>>>> I already asked this.
>>>>
>>>>> +
>>>>> +  memory-region:
>>>>> +    minItems: 1
>>>>> +    maxItems: 2
>>>>
>>>> Same comment. I already asked this about iommus.
>>>
>>> Same here, there aren't any bindings which lists for flexible memory-region.
>>> Please suggest if there are any such references.
>>
>> Similarly, we can define the additional memory region that's necessary
>> for $reasons and leave it unused in the driver (actually I don't know
>> why there may be two, but let's assume it's a QTEE/noQTEE detail), because
>> for the hw to operate, it must be set up by some entity in the system
>> either way (i.e. the memory is reserved even if it's not done by Linux)
> 
> 
> Another point is pretty obvious: if one claims that
> iommus/memory-regions list is flexible - some elements are optional -
> then clearly there is a distinction which elements are mandatory and
> which are optional. So there is difference between elements of the
> array. If there is a difference, they all must be explicitly listed,
> like every other list (clocks, resets etc) property. Writing bindings
> doc also defines this rule.
> 

I would like to describe the video bindings covering all the interfaces, 
including the secure stream ids. For this to do, i would have to wait 
for [1] to conclude. Will put up a new revision on this series, to 
exclude the binding patch and the one which enables kaanapali. That way 
we can have the driver prepared for vpu4, while kaanapali binding and 
patch to enable it in driver can be raised separately later once [1] is 
concluded.

[1] 
https://lore.kernel.org/all/cover.1762235099.git.charan.kalla@oss.qualcomm.com/

Regards,
Vikash
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Krzysztof Kozlowski 3 months, 3 weeks ago
On 21/10/2025 20:55, Vikash Garodia wrote:
> 
> On 10/18/2025 9:28 PM, Krzysztof Kozlowski wrote:
>> On 17/10/2025 16:16, Vikash Garodia wrote:
>>> +  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
>>> +
>>> +  iommus:
>>> +    minItems: 3
>>> +    maxItems: 8
>>
>> I don't understand why this is flexible. Make it fixed size and anyway -
>> list the items.
> 
> kaanapali vpu generates 8 different stream-ids. Now, boards running kernel in
> EL2 mode can list all of them, while boards running in EL1 can have only non
> secure stream IDs. Min have the list of stream ids which can be enabled for all
> type of boards, while max is for boards which can list all in HLOS given kernel
> is in EL2 mode.
> 
> Below crash would be seen if boards running kernel in EL1 mode lists the secure
> ones.


That has to be explained somewhere, e.g. comment, and still we need then
EL2 DTS in the kernel. I did not see such so far, but maybe I missed it
- can you link it?

> 
> [    1.361157] pc : qcom_smmu_write_s2cr+0x64/0xa4
> [    1.361165] lr : arm_smmu_write_s2cr+0x2c/0xbc
> [    1.361168] sp : ffff80008005b8f0
> [    1.361169] x29: ffff80008005b8f0 x28: 0000000000000000 x27: ffffc7f252f45320
> ....
> [    1.361195] x2 : ffff800081200c48 x1 : 0000000000000048 x0 : ffff800081200000
> [    1.361198] Call trace:
> [    1.361199]  qcom_smmu_write_s2cr+0x64/0xa4 (P)
> [    1.361203]  arm_smmu_master_install_s2crs+0x7c/0xac
> [    1.361207]  arm_smmu_attach_dev+0xb0/0x1d4
> 
> Could you please suggest on listing the iommu items ? I did not find the
> relevant references in other bindings where flexible iommus is being listed.


Just like every other list property - clocks, resets, power-domains.

> 
>>
>> I already asked this.
>>
>>> +
>>> +  memory-region:
>>> +    minItems: 1
>>> +    maxItems: 2
>>
>> Same comment. I already asked this about iommus.
> 
> Same here, there aren't any bindings which lists for flexible memory-region.
> Please suggest if there are any such references.

Because they do not matter for all other bindings, but it turned out
recently it might matter for this device.


Best regards,
Krzysztof
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Vikash Garodia 3 months, 3 weeks ago
On 10/22/2025 12:45 AM, Krzysztof Kozlowski wrote:
> On 21/10/2025 20:55, Vikash Garodia wrote:
>>
>> On 10/18/2025 9:28 PM, Krzysztof Kozlowski wrote:
>>> On 17/10/2025 16:16, Vikash Garodia wrote:
>>>> +  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
>>>> +
>>>> +  iommus:
>>>> +    minItems: 3
>>>> +    maxItems: 8
>>>
>>> I don't understand why this is flexible. Make it fixed size and anyway -
>>> list the items.
>>
>> kaanapali vpu generates 8 different stream-ids. Now, boards running kernel in
>> EL2 mode can list all of them, while boards running in EL1 can have only non
>> secure stream IDs. Min have the list of stream ids which can be enabled for all
>> type of boards, while max is for boards which can list all in HLOS given kernel
>> is in EL2 mode.
>>
>> Below crash would be seen if boards running kernel in EL1 mode lists the secure
>> ones.
> 
> 
> That has to be explained somewhere, e.g. comment, 

Sure, will add a description for iommus property explaining the same.

and still we need then
> EL2 DTS in the kernel. I did not see such so far, but maybe I missed it
> - can you link it?
> 

EL2 DTS for kaanapali is not yet posted to handle secure SIDs. While it is in
development, describing the secure stream-ids would ensure to cover all the
hardware generated IDs.

>>
>> [    1.361157] pc : qcom_smmu_write_s2cr+0x64/0xa4
>> [    1.361165] lr : arm_smmu_write_s2cr+0x2c/0xbc
>> [    1.361168] sp : ffff80008005b8f0
>> [    1.361169] x29: ffff80008005b8f0 x28: 0000000000000000 x27: ffffc7f252f45320
>> ....
>> [    1.361195] x2 : ffff800081200c48 x1 : 0000000000000048 x0 : ffff800081200000
>> [    1.361198] Call trace:
>> [    1.361199]  qcom_smmu_write_s2cr+0x64/0xa4 (P)
>> [    1.361203]  arm_smmu_master_install_s2crs+0x7c/0xac
>> [    1.361207]  arm_smmu_attach_dev+0xb0/0x1d4
>>
>> Could you please suggest on listing the iommu items ? I did not find the
>> relevant references in other bindings where flexible iommus is being listed.
> 
> 
> Just like every other list property - clocks, resets, power-domains.
> 
something like

iommu-names:
  items:
    - const: 0x1943
    - const: 0x1940
...

given that one of vpu sub hardware generates multiple SIDs, if we go with sub
hardware name in the list, the names would be repeated.

>>
>>>
>>> I already asked this.
>>>
>>>> +
>>>> +  memory-region:
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>
>>> Same comment. I already asked this about iommus.
>>
>> Same here, there aren't any bindings which lists for flexible memory-region.
>> Please suggest if there are any such references.
> 
> Because they do not matter for all other bindings, but it turned out
> recently it might matter for this device.

memory-region:
  minItems: 1
  maxItems: 2

memory-region-names:
  items:
    - const video_mem
    - const iris_resv

Regards,
Vikash

> 
> 
> Best regards,
> Krzysztof
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Dmitry Baryshkov 3 months, 2 weeks ago
On Wed, Oct 22, 2025 at 02:37:59AM +0530, Vikash Garodia wrote:
> 
> On 10/22/2025 12:45 AM, Krzysztof Kozlowski wrote:
> > On 21/10/2025 20:55, Vikash Garodia wrote:
> >>
> >> On 10/18/2025 9:28 PM, Krzysztof Kozlowski wrote:
> >>> On 17/10/2025 16:16, Vikash Garodia wrote:
> >>>> +  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
> >>>> +
> >>>> +  iommus:
> >>>> +    minItems: 3
> >>>> +    maxItems: 8
> >>>
> >>> I don't understand why this is flexible. Make it fixed size and anyway -
> >>> list the items.
> >>
> >> kaanapali vpu generates 8 different stream-ids. Now, boards running kernel in
> >> EL2 mode can list all of them, while boards running in EL1 can have only non
> >> secure stream IDs. Min have the list of stream ids which can be enabled for all
> >> type of boards, while max is for boards which can list all in HLOS given kernel
> >> is in EL2 mode.
> >>
> >> Below crash would be seen if boards running kernel in EL1 mode lists the secure
> >> ones.
> > 
> > 
> > That has to be explained somewhere, e.g. comment, 
> 
> Sure, will add a description for iommus property explaining the same.
> 
> and still we need then
> > EL2 DTS in the kernel. I did not see such so far, but maybe I missed it
> > - can you link it?
> > 
> 
> EL2 DTS for kaanapali is not yet posted to handle secure SIDs. While it is in
> development, describing the secure stream-ids would ensure to cover all the
> hardware generated IDs.

EL2 is a slightly different topic, it most likely requires additional
changes, etc. I'd suggest focusing on a normal usecase first and getting
the EL2 sorted out separately.

-- 
With best wishes
Dmitry
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Bryan O'Donoghue 3 months, 2 weeks ago
On 22/10/2025 10:37, Dmitry Baryshkov wrote:
>> EL2 DTS for kaanapali is not yet posted to handle secure SIDs. While it is in
>> development, describing the secure stream-ids would ensure to cover all the
>> hardware generated IDs.
> EL2 is a slightly different topic, it most likely requires additional
> changes, etc. I'd suggest focusing on a normal usecase first and getting
> the EL2 sorted out separately.

Is the conversion to EL2 only for compute then, Kanaapali is a gunyah 
system - Linux running @ EL1 here ?

---
bod
Re: [PATCH v2 1/8] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
Posted by Krzysztof Kozlowski 3 months, 2 weeks ago
On 21/10/2025 23:07, Vikash Garodia wrote:
> 
> On 10/22/2025 12:45 AM, Krzysztof Kozlowski wrote:
>> On 21/10/2025 20:55, Vikash Garodia wrote:
>>>
>>> On 10/18/2025 9:28 PM, Krzysztof Kozlowski wrote:
>>>> On 17/10/2025 16:16, Vikash Garodia wrote:
>>>>> +  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
>>>>> +
>>>>> +  iommus:
>>>>> +    minItems: 3
>>>>> +    maxItems: 8
>>>>
>>>> I don't understand why this is flexible. Make it fixed size and anyway -
>>>> list the items.
>>>
>>> kaanapali vpu generates 8 different stream-ids. Now, boards running kernel in
>>> EL2 mode can list all of them, while boards running in EL1 can have only non
>>> secure stream IDs. Min have the list of stream ids which can be enabled for all
>>> type of boards, while max is for boards which can list all in HLOS given kernel
>>> is in EL2 mode.
>>>
>>> Below crash would be seen if boards running kernel in EL1 mode lists the secure
>>> ones.
>>
>>
>> That has to be explained somewhere, e.g. comment, 
> 
> Sure, will add a description for iommus property explaining the same.
> 
> and still we need then
>> EL2 DTS in the kernel. I did not see such so far, but maybe I missed it
>> - can you link it?
>>
> 
> EL2 DTS for kaanapali is not yet posted to handle secure SIDs. While it is in
> development, describing the secure stream-ids would ensure to cover all the
> hardware generated IDs.


Then maybe this binding should wait till we see entire picture of hardware.

> 
>>>
>>> [    1.361157] pc : qcom_smmu_write_s2cr+0x64/0xa4
>>> [    1.361165] lr : arm_smmu_write_s2cr+0x2c/0xbc
>>> [    1.361168] sp : ffff80008005b8f0
>>> [    1.361169] x29: ffff80008005b8f0 x28: 0000000000000000 x27: ffffc7f252f45320
>>> ....
>>> [    1.361195] x2 : ffff800081200c48 x1 : 0000000000000048 x0 : ffff800081200000
>>> [    1.361198] Call trace:
>>> [    1.361199]  qcom_smmu_write_s2cr+0x64/0xa4 (P)
>>> [    1.361203]  arm_smmu_master_install_s2crs+0x7c/0xac
>>> [    1.361207]  arm_smmu_attach_dev+0xb0/0x1d4
>>>
>>> Could you please suggest on listing the iommu items ? I did not find the
>>> relevant references in other bindings where flexible iommus is being listed.
>>
>>
>> Just like every other list property - clocks, resets, power-domains.
>>
> something like
> 
> iommu-names:
>   items:
>     - const: 0x1943
>     - const: 0x1940
> ...
> 
> given that one of vpu sub hardware generates multiple SIDs, if we go with sub
> hardware name in the list, the names would be repeated.

No, we describe items, what are their meaning and purpose. In case of
clock you say what sort of clock input is that. In case of here, you
have IOMMUs for different purpose, you say which purpose is that.


Best regards,
Krzysztof