Existing definition limits the IOVA to an addressable range of 4GiB, and
even within that range, some of the space is used by IO registers,
thereby limiting the available IOVA to even lesser. Video hardware is
designed to emit different stream-ID for pixel and non-pixel buffers,
thereby introduce a non-pixel sub node to handle non-pixel stream-ID.
With this, both iris and non-pixel device can have IOVA range of 0-4GiB
individually. Certain video usecases like higher video concurrency needs
IOVA higher than 4GiB.
Add reference to the reserve-memory schema, which defines reserved IOVA
regions that are *excluded* from addressable range. Video hardware
generates different stream IDs based on the predefined range of IOVA
addresses. Thereby IOVA addresses for firmware and data buffers need to
be non overlapping. For ex. 0x0-0x25800000 address range is reserved for
firmware stream-ID, while non-pixel (bitstream) stream-ID can be
generated by hardware only when bitstream buffers IOVA address is from
0x25800000-0xe0000000.
Non-pixel stream-ID can now be part of the new sub-node, hence iommus in
iris node can have either 1 entry for pixel stream-id or 2 entries for
pixel and non-pixel stream-ids.
Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
---
.../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -65,10 +65,31 @@ properties:
- const: core
iommus:
+ minItems: 1
maxItems: 2
dma-coherent: true
+ non-pixel:
+ type: object
+ additionalProperties: false
+
+ description:
+ Non pixel context bank is needed when video hardware have distinct iommus
+ for non pixel buffers. Non pixel buffers are mainly compressed and
+ internal buffers.
+
+ properties:
+ iommus:
+ maxItems: 1
+
+ memory-region:
+ maxItems: 1
+
+ required:
+ - iommus
+ - memory-region
+
operating-points-v2: true
opp-table:
@@ -86,6 +107,7 @@ required:
allOf:
- $ref: qcom,venus-common.yaml#
+ - $ref: /schemas/reserved-memory/reserved-memory.yaml
- if:
properties:
compatible:
@@ -117,6 +139,16 @@ examples:
#include <dt-bindings/power/qcom-rpmpd.h>
#include <dt-bindings/power/qcom,rpmhpd.h>
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ iris_resv: reservation-iris {
+ iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>,
+ <&iris_non_pixel 0x0 0xe0000000 0x0 0x20000000>;
+ };
+ };
+
video-codec@aa00000 {
compatible = "qcom,sm8550-iris";
reg = <0x0aa00000 0xf0000>;
@@ -144,12 +176,16 @@ examples:
resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
reset-names = "bus";
- iommus = <&apps_smmu 0x1940 0x0000>,
- <&apps_smmu 0x1947 0x0000>;
+ iommus = <&apps_smmu 0x1947 0x0000>;
dma-coherent;
operating-points-v2 = <&iris_opp_table>;
+ iris_non_pixel: non-pixel {
+ iommus = <&apps_smmu 0x1940 0x0000>;
+ memory-region = <&iris_resv>;
+ };
+
iris_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.34.1On 27/06/2025 17:48, Vikash Garodia wrote:
> +
> video-codec@aa00000 {
> compatible = "qcom,sm8550-iris";
> reg = <0x0aa00000 0xf0000>;
> @@ -144,12 +176,16 @@ examples:
> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
> reset-names = "bus";
>
> - iommus = <&apps_smmu 0x1940 0x0000>,
> - <&apps_smmu 0x1947 0x0000>;
> + iommus = <&apps_smmu 0x1947 0x0000>;
I missed, that's technically ABI break and nothing in commit msg
explains that. You need to clearly explain the reasons and impact.
Best regards,
Krzysztof
On 7/2/2025 4:53 PM, Krzysztof Kozlowski wrote:
> On 27/06/2025 17:48, Vikash Garodia wrote:
>> +
>> video-codec@aa00000 {
>> compatible = "qcom,sm8550-iris";
>> reg = <0x0aa00000 0xf0000>;
>> @@ -144,12 +176,16 @@ examples:
>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>> reset-names = "bus";
>>
>> - iommus = <&apps_smmu 0x1940 0x0000>,
>> - <&apps_smmu 0x1947 0x0000>;
>> + iommus = <&apps_smmu 0x1947 0x0000>;
>
> I missed, that's technically ABI break and nothing in commit msg
> explains that. You need to clearly explain the reasons and impact.
No, it is not. Interface works well with old or new approach.
>
> Best regards,
> Krzysztof
On 02/07/2025 13:45, Vikash Garodia wrote:
>
> On 7/2/2025 4:53 PM, Krzysztof Kozlowski wrote:
>> On 27/06/2025 17:48, Vikash Garodia wrote:
>>> +
>>> video-codec@aa00000 {
>>> compatible = "qcom,sm8550-iris";
>>> reg = <0x0aa00000 0xf0000>;
>>> @@ -144,12 +176,16 @@ examples:
>>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>>> reset-names = "bus";
>>>
>>> - iommus = <&apps_smmu 0x1940 0x0000>,
>>> - <&apps_smmu 0x1947 0x0000>;
>>> + iommus = <&apps_smmu 0x1947 0x0000>;
>>
>> I missed, that's technically ABI break and nothing in commit msg
>> explains that. You need to clearly explain the reasons and impact.
> No, it is not. Interface works well with old or new approach.
You changed the order of IOMMUs, so yes it is. Which interface works
well - FreeBSD? Or other? You are changing ABI for every user.
Best regards,
Krzysztof
On 7/2/2025 5:17 PM, Krzysztof Kozlowski wrote:
> On 02/07/2025 13:45, Vikash Garodia wrote:
>>
>> On 7/2/2025 4:53 PM, Krzysztof Kozlowski wrote:
>>> On 27/06/2025 17:48, Vikash Garodia wrote:
>>>> +
>>>> video-codec@aa00000 {
>>>> compatible = "qcom,sm8550-iris";
>>>> reg = <0x0aa00000 0xf0000>;
>>>> @@ -144,12 +176,16 @@ examples:
>>>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>>>> reset-names = "bus";
>>>>
>>>> - iommus = <&apps_smmu 0x1940 0x0000>,
>>>> - <&apps_smmu 0x1947 0x0000>;
>>>> + iommus = <&apps_smmu 0x1947 0x0000>;
>>>
>>> I missed, that's technically ABI break and nothing in commit msg
>>> explains that. You need to clearly explain the reasons and impact.
>> No, it is not. Interface works well with old or new approach.
>
>
> You changed the order of IOMMUs, so yes it is. Which interface works
> well - FreeBSD? Or other? You are changing ABI for every user.
Why do i need to change, when without changing would work as well ?
>
> Best regards,
> Krzysztof
On 02/07/2025 13:55, Vikash Garodia wrote:
>
>
> On 7/2/2025 5:17 PM, Krzysztof Kozlowski wrote:
>> On 02/07/2025 13:45, Vikash Garodia wrote:
>>>
>>> On 7/2/2025 4:53 PM, Krzysztof Kozlowski wrote:
>>>> On 27/06/2025 17:48, Vikash Garodia wrote:
>>>>> +
>>>>> video-codec@aa00000 {
>>>>> compatible = "qcom,sm8550-iris";
>>>>> reg = <0x0aa00000 0xf0000>;
>>>>> @@ -144,12 +176,16 @@ examples:
>>>>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>>>>> reset-names = "bus";
>>>>>
>>>>> - iommus = <&apps_smmu 0x1940 0x0000>,
>>>>> - <&apps_smmu 0x1947 0x0000>;
>>>>> + iommus = <&apps_smmu 0x1947 0x0000>;
>>>>
>>>> I missed, that's technically ABI break and nothing in commit msg
>>>> explains that. You need to clearly explain the reasons and impact.
>>> No, it is not. Interface works well with old or new approach.
>>
>>
>> You changed the order of IOMMUs, so yes it is. Which interface works
>> well - FreeBSD? Or other? You are changing ABI for every user.
> Why do i need to change, when without changing would work as well ?
? I don't understand. I made a statement, not a question. You are doing
this - you are changing the ABI.
Which item was the first IOMMU before and which was second?
Which item is the first IOMMU now?
Best regards,
Krzysztof
On 7/2/2025 5:28 PM, Krzysztof Kozlowski wrote:
> On 02/07/2025 13:55, Vikash Garodia wrote:
>>
>>
>> On 7/2/2025 5:17 PM, Krzysztof Kozlowski wrote:
>>> On 02/07/2025 13:45, Vikash Garodia wrote:
>>>>
>>>> On 7/2/2025 4:53 PM, Krzysztof Kozlowski wrote:
>>>>> On 27/06/2025 17:48, Vikash Garodia wrote:
>>>>>> +
>>>>>> video-codec@aa00000 {
>>>>>> compatible = "qcom,sm8550-iris";
>>>>>> reg = <0x0aa00000 0xf0000>;
>>>>>> @@ -144,12 +176,16 @@ examples:
>>>>>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>>>>>> reset-names = "bus";
>>>>>>
>>>>>> - iommus = <&apps_smmu 0x1940 0x0000>,
>>>>>> - <&apps_smmu 0x1947 0x0000>;
>>>>>> + iommus = <&apps_smmu 0x1947 0x0000>;
>>>>>
>>>>> I missed, that's technically ABI break and nothing in commit msg
>>>>> explains that. You need to clearly explain the reasons and impact.
>>>> No, it is not. Interface works well with old or new approach.
>>>
>>>
>>> You changed the order of IOMMUs, so yes it is. Which interface works
>>> well - FreeBSD? Or other? You are changing ABI for every user.
>> Why do i need to change, when without changing would work as well ?
> ? I don't understand. I made a statement, not a question. You are doing
> this - you are changing the ABI.
>
> Which item was the first IOMMU before and which was second?
>
> Which item is the first IOMMU now?
Old approach - max 2 iommus interface - <SID-A, SID-B>
New approach - min 1/max 2, iommu interface - <SID-B>, child - <SID-A>
If both works, how is interchanging impacting any existing hardware OR breaking
ABI ?
>
> Best regards,
> Krzysztof
On 02/07/2025 14:08, Vikash Garodia wrote:
>
> On 7/2/2025 5:28 PM, Krzysztof Kozlowski wrote:
>> On 02/07/2025 13:55, Vikash Garodia wrote:
>>>
>>>
>>> On 7/2/2025 5:17 PM, Krzysztof Kozlowski wrote:
>>>> On 02/07/2025 13:45, Vikash Garodia wrote:
>>>>>
>>>>> On 7/2/2025 4:53 PM, Krzysztof Kozlowski wrote:
>>>>>> On 27/06/2025 17:48, Vikash Garodia wrote:
>>>>>>> +
>>>>>>> video-codec@aa00000 {
>>>>>>> compatible = "qcom,sm8550-iris";
>>>>>>> reg = <0x0aa00000 0xf0000>;
>>>>>>> @@ -144,12 +176,16 @@ examples:
>>>>>>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>>>>>>> reset-names = "bus";
>>>>>>>
>>>>>>> - iommus = <&apps_smmu 0x1940 0x0000>,
>>>>>>> - <&apps_smmu 0x1947 0x0000>;
>>>>>>> + iommus = <&apps_smmu 0x1947 0x0000>;
>>>>>>
>>>>>> I missed, that's technically ABI break and nothing in commit msg
>>>>>> explains that. You need to clearly explain the reasons and impact.
>>>>> No, it is not. Interface works well with old or new approach.
>>>>
>>>>
>>>> You changed the order of IOMMUs, so yes it is. Which interface works
>>>> well - FreeBSD? Or other? You are changing ABI for every user.
>>> Why do i need to change, when without changing would work as well ?
>> ? I don't understand. I made a statement, not a question. You are doing
>> this - you are changing the ABI.
>>
>> Which item was the first IOMMU before and which was second?
>>
>> Which item is the first IOMMU now?
> Old approach - max 2 iommus interface - <SID-A, SID-B>
> New approach - min 1/max 2, iommu interface - <SID-B>, child - <SID-A>
So you changed first IOMMU entry, first IOMMU master and that is
technically ABI break.
>
> If both works, how is interchanging impacting any existing hardware OR breaking
> ABI ?
Because you change the entries. The ordering of lists - not iommus which
do not matter for Linux here - was discussed many times, so just refer
to that discussions.
Best regards,
Krzysztof
On 27/06/2025 17:48, Vikash Garodia wrote:
> Existing definition limits the IOVA to an addressable range of 4GiB, and
> even within that range, some of the space is used by IO registers,
> thereby limiting the available IOVA to even lesser. Video hardware is
> designed to emit different stream-ID for pixel and non-pixel buffers,
> thereby introduce a non-pixel sub node to handle non-pixel stream-ID.
>
> With this, both iris and non-pixel device can have IOVA range of 0-4GiB
> individually. Certain video usecases like higher video concurrency needs
> IOVA higher than 4GiB.
>
> Add reference to the reserve-memory schema, which defines reserved IOVA
No. That schema is always selected. This makes no sense at all.
> regions that are *excluded* from addressable range. Video hardware
> generates different stream IDs based on the predefined range of IOVA
> addresses. Thereby IOVA addresses for firmware and data buffers need to
> be non overlapping. For ex. 0x0-0x25800000 address range is reserved for
> firmware stream-ID, while non-pixel (bitstream) stream-ID can be
> generated by hardware only when bitstream buffers IOVA address is from
> 0x25800000-0xe0000000.
> Non-pixel stream-ID can now be part of the new sub-node, hence iommus in
> iris node can have either 1 entry for pixel stream-id or 2 entries for
> pixel and non-pixel stream-ids.
>
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
> .../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> @@ -65,10 +65,31 @@ properties:
> - const: core
>
> iommus:
> + minItems: 1
> maxItems: 2
No, why hardware suddenly has different amount?
>
> dma-coherent: true
>
> + non-pixel:
Why EXISTING hardware grows?
> + type: object
> + additionalProperties: false
> +
> + description:
> + Non pixel context bank is needed when video hardware have distinct iommus
> + for non pixel buffers. Non pixel buffers are mainly compressed and
> + internal buffers.
> +
> + properties:
> + iommus:
> + maxItems: 1
> +
> + memory-region:
> + maxItems: 1
> +
> + required:
> + - iommus
> + - memory-region
> +
> operating-points-v2: true
>
> opp-table:
> @@ -86,6 +107,7 @@ required:
>
> allOf:
> - $ref: qcom,venus-common.yaml#
> + - $ref: /schemas/reserved-memory/reserved-memory.yaml
This makes no sense. how is this device a reserved memory?
> - if:
> properties:
> compatible:
> @@ -117,6 +139,16 @@ examples:
> #include <dt-bindings/power/qcom-rpmpd.h>
> #include <dt-bindings/power/qcom,rpmhpd.h>
>
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
Why do you need this?
> +
> + iris_resv: reservation-iris {
Mixing MMIO and non-MMIO is not the way to go. This is also not relevant
here. Don't embed other things into your binding example.
> + iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>,
> + <&iris_non_pixel 0x0 0xe0000000 0x0 0x20000000>;
> + };
> + };
> +
> video-codec@aa00000 {
> compatible = "qcom,sm8550-iris";
> reg = <0x0aa00000 0xf0000>;
> @@ -144,12 +176,16 @@ examples:
> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
> reset-names = "bus";
>
> - iommus = <&apps_smmu 0x1940 0x0000>,
> - <&apps_smmu 0x1947 0x0000>;
> + iommus = <&apps_smmu 0x1947 0x0000>;
Why did the device or hardware change? Nothing explains in commit msg
what is wrong with existing device and existing binding.
> dma-coherent;
>
> operating-points-v2 = <&iris_opp_table>;
>
> + iris_non_pixel: non-pixel {
> + iommus = <&apps_smmu 0x1940 0x0000>;
> + memory-region = <&iris_resv>;
> + };
> +
> iris_opp_table: opp-table {
> compatible = "operating-points-v2";
>
>
Best regards,
Krzysztof
On 02/07/2025 13:13, Krzysztof Kozlowski wrote: >> --- >> .../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++-- >> 1 file changed, 38 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml >> index c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644 >> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml >> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml >> @@ -65,10 +65,31 @@ properties: >> - const: core >> >> iommus: >> + minItems: 1 >> maxItems: 2 > > No, why hardware suddenly has different amount? > >> >> dma-coherent: true >> >> + non-pixel: > > Why EXISTING hardware grows? One more comment here - this entire node is not needed, there is no device here being described. Just solve the problem in the main device node. Best regards, Krzysztof
On 7/2/2025 4:43 PM, Krzysztof Kozlowski wrote:
> On 27/06/2025 17:48, Vikash Garodia wrote:
>> Existing definition limits the IOVA to an addressable range of 4GiB, and
>> even within that range, some of the space is used by IO registers,
>> thereby limiting the available IOVA to even lesser. Video hardware is
>> designed to emit different stream-ID for pixel and non-pixel buffers,
>> thereby introduce a non-pixel sub node to handle non-pixel stream-ID.
>>
>> With this, both iris and non-pixel device can have IOVA range of 0-4GiB
>> individually. Certain video usecases like higher video concurrency needs
>> IOVA higher than 4GiB.
>>
>> Add reference to the reserve-memory schema, which defines reserved IOVA
>
> No. That schema is always selected. This makes no sense at all.
I could not get this, are you suggesting to drop this reference ?
>
>> regions that are *excluded* from addressable range. Video hardware
>> generates different stream IDs based on the predefined range of IOVA
>> addresses. Thereby IOVA addresses for firmware and data buffers need to
>> be non overlapping. For ex. 0x0-0x25800000 address range is reserved for
>> firmware stream-ID, while non-pixel (bitstream) stream-ID can be
>> generated by hardware only when bitstream buffers IOVA address is from
>> 0x25800000-0xe0000000.
>> Non-pixel stream-ID can now be part of the new sub-node, hence iommus in
>> iris node can have either 1 entry for pixel stream-id or 2 entries for
>> pixel and non-pixel stream-ids.
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>> .../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++--
>> 1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> index c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644
>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> @@ -65,10 +65,31 @@ properties:
>> - const: core
>>
>> iommus:
>> + minItems: 1
>> maxItems: 2
>
> No, why hardware suddenly has different amount?
Its not about hardware started to have a new stream-ID. You can look for the
description in the commit which explains the need for a new device and hence the
split of stream-IDs in iris device OR non-pixel device.
>
>>
>> dma-coherent: true
>>
>> + non-pixel:
>
> Why EXISTING hardware grows?
Same here, the commit describes the limitation of existing design and also
explains the need for having the non-pixel device. Its not that the hardware is
growing here, rather the hardware stream-IDs are utilized differently to get
higher device addressable range.
>
>> + type: object
>> + additionalProperties: false
>> +
>> + description:
>> + Non pixel context bank is needed when video hardware have distinct iommus
>> + for non pixel buffers. Non pixel buffers are mainly compressed and
>> + internal buffers.
>> +
>> + properties:
>> + iommus:
>> + maxItems: 1
>> +
>> + memory-region:
>> + maxItems: 1
>> +
>> + required:
>> + - iommus
>> + - memory-region
>> +
>> operating-points-v2: true
>>
>> opp-table:
>> @@ -86,6 +107,7 @@ required:
>>
>> allOf:
>> - $ref: qcom,venus-common.yaml#
>> + - $ref: /schemas/reserved-memory/reserved-memory.yaml
>
> This makes no sense. how is this device a reserved memory?
Again, explained the "excluded" portion from IOVA part in commit description.
For such excluded region, reserved memory would be needed. I have followed the
adsp example in the reserved-memory schema[1], its same for iris.
[1]
https://github.com/devicetree-org/dt-schema/blame/main/dtschema/schemas/reserved-memory/reserved-memory.yaml
>
>> - if:
>> properties:
>> compatible:
>> @@ -117,6 +139,16 @@ examples:
>> #include <dt-bindings/power/qcom-rpmpd.h>
>> #include <dt-bindings/power/qcom,rpmhpd.h>
>>
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>
> Why do you need this?
Was planning to drop this, as the reserved-memory region have it defined.
>
>> +
>> + iris_resv: reservation-iris {
>
> Mixing MMIO and non-MMIO is not the way to go. This is also not relevant
> here. Don't embed other things into your binding example.
>
>
>> + iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>,
>> + <&iris_non_pixel 0x0 0xe0000000 0x0 0x20000000>;
>> + };
>> + };
>> +
>> video-codec@aa00000 {
>> compatible = "qcom,sm8550-iris";
>> reg = <0x0aa00000 0xf0000>;
>> @@ -144,12 +176,16 @@ examples:
>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>> reset-names = "bus";
>>
>> - iommus = <&apps_smmu 0x1940 0x0000>,
>> - <&apps_smmu 0x1947 0x0000>;
>> + iommus = <&apps_smmu 0x1947 0x0000>;
>
> Why did the device or hardware change? Nothing explains in commit msg
> what is wrong with existing device and existing binding.
Same query here, the commit well explains the limitation with existing device
and how adding a new sub node mitigates the situation.
Regards,
Vikash
>
>> dma-coherent;
>>
>> operating-points-v2 = <&iris_opp_table>;
>>
>> + iris_non_pixel: non-pixel {
>> + iommus = <&apps_smmu 0x1940 0x0000>;
>> + memory-region = <&iris_resv>;
>> + };
>> +
>> iris_opp_table: opp-table {
>> compatible = "operating-points-v2";
>>
>>
>
>
> Best regards,
> Krzysztof
On 02/07/2025 13:32, Vikash Garodia wrote:
>
> On 7/2/2025 4:43 PM, Krzysztof Kozlowski wrote:
>> On 27/06/2025 17:48, Vikash Garodia wrote:
>>> Existing definition limits the IOVA to an addressable range of 4GiB, and
>>> even within that range, some of the space is used by IO registers,
>>> thereby limiting the available IOVA to even lesser. Video hardware is
>>> designed to emit different stream-ID for pixel and non-pixel buffers,
>>> thereby introduce a non-pixel sub node to handle non-pixel stream-ID.
>>>
>>> With this, both iris and non-pixel device can have IOVA range of 0-4GiB
>>> individually. Certain video usecases like higher video concurrency needs
>>> IOVA higher than 4GiB.
>>>
>>> Add reference to the reserve-memory schema, which defines reserved IOVA
>>
>> No. That schema is always selected. This makes no sense at all.
> I could not get this, are you suggesting to drop this reference ?
What does the schema says?
7 title: /reserved-memory Child Node Common
Is this the binding for reserved-memory node? Not sure, your subject
does not have proper prefix, but diff suggested that not.
Maybe I missed something.
>>
>>> regions that are *excluded* from addressable range. Video hardware
>>> generates different stream IDs based on the predefined range of IOVA
>>> addresses. Thereby IOVA addresses for firmware and data buffers need to
>>> be non overlapping. For ex. 0x0-0x25800000 address range is reserved for
>>> firmware stream-ID, while non-pixel (bitstream) stream-ID can be
>>> generated by hardware only when bitstream buffers IOVA address is from
>>> 0x25800000-0xe0000000.
>>> Non-pixel stream-ID can now be part of the new sub-node, hence iommus in
>>> iris node can have either 1 entry for pixel stream-id or 2 entries for
>>> pixel and non-pixel stream-ids.
>>>
>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>>> ---
>>> .../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++--
>>> 1 file changed, 38 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>> index c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644
>>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>> @@ -65,10 +65,31 @@ properties:
>>> - const: core
>>>
>>> iommus:
>>> + minItems: 1
>>> maxItems: 2
>>
>> No, why hardware suddenly has different amount?
> Its not about hardware started to have a new stream-ID. You can look for the
> description in the commit which explains the need for a new device and hence the
> split of stream-IDs in iris device OR non-pixel device.
But this is not a new device! This is sm8550. Existing device.
>>
>>>
>>> dma-coherent: true
>>>
>>> + non-pixel:
>>
>> Why EXISTING hardware grows?
> Same here, the commit describes the limitation of existing design and also
> explains the need for having the non-pixel device. Its not that the hardware is
> growing here, rather the hardware stream-IDs are utilized differently to get
> higher device addressable range.
You are not doing this for a new device. There is no new device here at
all. Nowhere here is a new device.
Changes for a new device COME TOGETHER with the new device.
What you are doing here is changing existing hardware without any
explanation why.
>>
>>> + type: object
>>> + additionalProperties: false
>>> +
>>> + description:
>>> + Non pixel context bank is needed when video hardware have distinct iommus
>>> + for non pixel buffers. Non pixel buffers are mainly compressed and
>>> + internal buffers.
>>> +
>>> + properties:
>>> + iommus:
>>> + maxItems: 1
>>> +
>>> + memory-region:
>>> + maxItems: 1
>>> +
>>> + required:
>>> + - iommus
>>> + - memory-region
>>> +
>>> operating-points-v2: true
>>>
>>> opp-table:
>>> @@ -86,6 +107,7 @@ required:
>>>
>>> allOf:
>>> - $ref: qcom,venus-common.yaml#
>>> + - $ref: /schemas/reserved-memory/reserved-memory.yaml
>>
>> This makes no sense. how is this device a reserved memory?
> Again, explained the "excluded" portion from IOVA part in commit description.
> For such excluded region, reserved memory would be needed. I have followed the
> adsp example in the reserved-memory schema[1], its same for iris.
>
> [1]
> https://github.com/devicetree-org/dt-schema/blame/main/dtschema/schemas/reserved-memory/reserved-memory.yaml
Read the title there.
>>
>>> - if:
>>> properties:
>>> compatible:
>>> @@ -117,6 +139,16 @@ examples:
>>> #include <dt-bindings/power/qcom-rpmpd.h>
>>> #include <dt-bindings/power/qcom,rpmhpd.h>
>>>
>>> + reserved-memory {
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>
>> Why do you need this?
> Was planning to drop this, as the reserved-memory region have it defined.
>>
>>> +
>>> + iris_resv: reservation-iris {
>>
>> Mixing MMIO and non-MMIO is not the way to go. This is also not relevant
>> here. Don't embed other things into your binding example.
>>
>>
>>> + iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>,
>>> + <&iris_non_pixel 0x0 0xe0000000 0x0 0x20000000>;
>>> + };
>>> + };
>>> +
>>> video-codec@aa00000 {
>>> compatible = "qcom,sm8550-iris";
>>> reg = <0x0aa00000 0xf0000>;
>>> @@ -144,12 +176,16 @@ examples:
>>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>>> reset-names = "bus";
>>>
>>> - iommus = <&apps_smmu 0x1940 0x0000>,
>>> - <&apps_smmu 0x1947 0x0000>;
>>> + iommus = <&apps_smmu 0x1947 0x0000>;
>>
>> Why did the device or hardware change? Nothing explains in commit msg
>> what is wrong with existing device and existing binding.
> Same query here, the commit well explains the limitation with existing device
> and how adding a new sub node mitigates the situation.
I read it and still do not get what is wrong with existing device. Which
hardware emits different stream-ID? How does it affect users? How can I
reproduce the problem?
Remember, that you are now affecting ABI.
Best regards,
Krzysztof
On 7/2/25 1:46 PM, Krzysztof Kozlowski wrote: > On 02/07/2025 13:32, Vikash Garodia wrote: >> >> On 7/2/2025 4:43 PM, Krzysztof Kozlowski wrote: >>> On 27/06/2025 17:48, Vikash Garodia wrote: >>>> Existing definition limits the IOVA to an addressable range of 4GiB, and >>>> even within that range, some of the space is used by IO registers, >>>> thereby limiting the available IOVA to even lesser. Video hardware is >>>> designed to emit different stream-ID for pixel and non-pixel buffers, >>>> thereby introduce a non-pixel sub node to handle non-pixel stream-ID. >>>> >>>> With this, both iris and non-pixel device can have IOVA range of 0-4GiB >>>> individually. Certain video usecases like higher video concurrency needs >>>> IOVA higher than 4GiB. >>>> >>>> Add reference to the reserve-memory schema, which defines reserved IOVA [...] >>>> dma-coherent: true >>>> >>>> + non-pixel: >>> >>> Why EXISTING hardware grows? >> Same here, the commit describes the limitation of existing design and also >> explains the need for having the non-pixel device. Its not that the hardware is >> growing here, rather the hardware stream-IDs are utilized differently to get >> higher device addressable range. > > You are not doing this for a new device. There is no new device here at > all. Nowhere here is a new device. > > Changes for a new device COME TOGETHER with the new device. > > What you are doing here is changing existing hardware without any > explanation why. This is bindings getting a reality check.. this goes as far back as Venus existed (msm8974/2012) We unfortunately have to expect a number of similar updates for all multimedia peripherals (GPU/Camera/Display etc.), as certain mappings must be done through certain SIDs (which are deemed 'secure') and some hardware has general addressing limitations that may have been causing silent issues all along Konrad
On 02/07/2025 15:11, Konrad Dybcio wrote: > On 7/2/25 1:46 PM, Krzysztof Kozlowski wrote: >> On 02/07/2025 13:32, Vikash Garodia wrote: >>> >>> On 7/2/2025 4:43 PM, Krzysztof Kozlowski wrote: >>>> On 27/06/2025 17:48, Vikash Garodia wrote: >>>>> Existing definition limits the IOVA to an addressable range of 4GiB, and >>>>> even within that range, some of the space is used by IO registers, >>>>> thereby limiting the available IOVA to even lesser. Video hardware is >>>>> designed to emit different stream-ID for pixel and non-pixel buffers, >>>>> thereby introduce a non-pixel sub node to handle non-pixel stream-ID. >>>>> >>>>> With this, both iris and non-pixel device can have IOVA range of 0-4GiB >>>>> individually. Certain video usecases like higher video concurrency needs >>>>> IOVA higher than 4GiB. >>>>> >>>>> Add reference to the reserve-memory schema, which defines reserved IOVA > > [...] > >>>>> dma-coherent: true >>>>> >>>>> + non-pixel: >>>> >>>> Why EXISTING hardware grows? >>> Same here, the commit describes the limitation of existing design and also >>> explains the need for having the non-pixel device. Its not that the hardware is >>> growing here, rather the hardware stream-IDs are utilized differently to get >>> higher device addressable range. >> >> You are not doing this for a new device. There is no new device here at >> all. Nowhere here is a new device. >> >> Changes for a new device COME TOGETHER with the new device. >> >> What you are doing here is changing existing hardware without any >> explanation why. > > This is bindings getting a reality check.. this goes as far back as Venus > existed (msm8974/2012) This won't fly. This is a new binding after long reviews and discussions, why Qualcomm does not want to extend existing Venus but needs completely new Iris. Well, if you get completely new Iris, you cannot use argument of "legacy". We insisted on growing existing solution, but choice of abandoning it and coming with a new one means you were supposed to do it right since there is no legacy. > > We unfortunately have to expect a number of similar updates for all > multimedia peripherals (GPU/Camera/Display etc.), as certain mappings > must be done through certain SIDs (which are deemed 'secure') and some > hardware has general addressing limitations that may have been causing > silent issues all along > Isn't all this commit msg here about non-pixel stuff just not really describing the real problem at all? This commit msg is very vague and silent on actual use cases and actual firmware, so even multiple readings of commit msg did not help me. Stephan Gerhold now nicely summarized what was never told in commit msg - there is a gap in address space which is reserved for firmware and no allocations can be done from that? Also commit msg says "Existing definition limits the IOVA to an addressable range of 4GiB, and" but I do not see such definition in the binding at all. So what does it refer to? Best regards, Krzysztof
On 02-Jul-25 15:59, Krzysztof Kozlowski wrote: > On 02/07/2025 15:11, Konrad Dybcio wrote: >> On 7/2/25 1:46 PM, Krzysztof Kozlowski wrote: >>> On 02/07/2025 13:32, Vikash Garodia wrote: >>>> >>>> On 7/2/2025 4:43 PM, Krzysztof Kozlowski wrote: >>>>> On 27/06/2025 17:48, Vikash Garodia wrote: >>>>>> Existing definition limits the IOVA to an addressable range of 4GiB, and >>>>>> even within that range, some of the space is used by IO registers, >>>>>> thereby limiting the available IOVA to even lesser. Video hardware is >>>>>> designed to emit different stream-ID for pixel and non-pixel buffers, >>>>>> thereby introduce a non-pixel sub node to handle non-pixel stream-ID. >>>>>> >>>>>> With this, both iris and non-pixel device can have IOVA range of 0-4GiB >>>>>> individually. Certain video usecases like higher video concurrency needs >>>>>> IOVA higher than 4GiB. >>>>>> >>>>>> Add reference to the reserve-memory schema, which defines reserved IOVA >> >> [...] >> >>>>>> dma-coherent: true >>>>>> >>>>>> + non-pixel: >>>>> >>>>> Why EXISTING hardware grows? >>>> Same here, the commit describes the limitation of existing design and also >>>> explains the need for having the non-pixel device. Its not that the hardware is >>>> growing here, rather the hardware stream-IDs are utilized differently to get >>>> higher device addressable range. >>> >>> You are not doing this for a new device. There is no new device here at >>> all. Nowhere here is a new device. >>> >>> Changes for a new device COME TOGETHER with the new device. >>> >>> What you are doing here is changing existing hardware without any >>> explanation why. >> >> This is bindings getting a reality check.. this goes as far back as Venus >> existed (msm8974/2012) > > This won't fly. This is a new binding after long reviews and > discussions, why Qualcomm does not want to extend existing Venus but > needs completely new Iris. Well, if you get completely new Iris, you > cannot use argument of "legacy". We insisted on growing existing > solution, but choice of abandoning it and coming with a new one means > you were supposed to do it right since there is no legacy. So maybe I worded this in an unfortunate way.. I meant Venus the HW block, not Venus the driver. Even the ancient msm8974 has the same addressing limitations and a separate IOMMU domain for non_pixel/secure/etc. >> We unfortunately have to expect a number of similar updates for all >> multimedia peripherals (GPU/Camera/Display etc.), as certain mappings >> must be done through certain SIDs (which are deemed 'secure') and some >> hardware has general addressing limitations that may have been causing >> silent issues all along >> > Isn't all this commit msg here about non-pixel stuff just not really > describing the real problem at all? This commit msg is very vague and > silent on actual use cases and actual firmware, so even multiple > readings of commit msg did not help me. Stephan Gerhold now nicely > summarized what was never told in commit msg - there is a gap in address > space which is reserved for firmware and no allocations can be done from > that? > > Also commit msg says "Existing definition limits the IOVA to an > addressable range of 4GiB, and" but I do not see such definition in the > binding at all. So what does it refer to? Yeah, there's many parts to this. The solution that this patchset proposes will essentially need to be copypasted a couple times if this is going to go in as-is.. see qcom,msm-vidc,context-bank (lol) subnodes, each describing some combination of the above problems: https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/tags/android-11.0.0_r0.56/qcom/scuba-vidc.dtsi#58 Konrad
On 7/2/2025 7:29 PM, Krzysztof Kozlowski wrote: > On 02/07/2025 15:11, Konrad Dybcio wrote: >> On 7/2/25 1:46 PM, Krzysztof Kozlowski wrote: >>> On 02/07/2025 13:32, Vikash Garodia wrote: >>>> >>>> On 7/2/2025 4:43 PM, Krzysztof Kozlowski wrote: >>>>> On 27/06/2025 17:48, Vikash Garodia wrote: >>>>>> Existing definition limits the IOVA to an addressable range of 4GiB, and >>>>>> even within that range, some of the space is used by IO registers, >>>>>> thereby limiting the available IOVA to even lesser. Video hardware is >>>>>> designed to emit different stream-ID for pixel and non-pixel buffers, >>>>>> thereby introduce a non-pixel sub node to handle non-pixel stream-ID. >>>>>> >>>>>> With this, both iris and non-pixel device can have IOVA range of 0-4GiB >>>>>> individually. Certain video usecases like higher video concurrency needs >>>>>> IOVA higher than 4GiB. >>>>>> >>>>>> Add reference to the reserve-memory schema, which defines reserved IOVA >> >> [...] >> >>>>>> dma-coherent: true >>>>>> >>>>>> + non-pixel: >>>>> >>>>> Why EXISTING hardware grows? >>>> Same here, the commit describes the limitation of existing design and also >>>> explains the need for having the non-pixel device. Its not that the hardware is >>>> growing here, rather the hardware stream-IDs are utilized differently to get >>>> higher device addressable range. >>> >>> You are not doing this for a new device. There is no new device here at >>> all. Nowhere here is a new device. >>> >>> Changes for a new device COME TOGETHER with the new device. >>> >>> What you are doing here is changing existing hardware without any >>> explanation why. >> >> This is bindings getting a reality check.. this goes as far back as Venus >> existed (msm8974/2012) > > This won't fly. This is a new binding after long reviews and > discussions, why Qualcomm does not want to extend existing Venus but > needs completely new Iris. Well, if you get completely new Iris, you > cannot use argument of "legacy". We insisted on growing existing > solution, but choice of abandoning it and coming with a new one means > you were supposed to do it right since there is no legacy. > >> >> We unfortunately have to expect a number of similar updates for all >> multimedia peripherals (GPU/Camera/Display etc.), as certain mappings >> must be done through certain SIDs (which are deemed 'secure') and some >> hardware has general addressing limitations that may have been causing >> silent issues all along >> > Isn't all this commit msg here about non-pixel stuff just not really > describing the real problem at all? This commit msg is very vague and > silent on actual use cases and actual firmware, so even multiple > readings of commit msg did not help me. Stephan Gerhold now nicely > summarized what was never told in commit msg - there is a gap in address > space which is reserved for firmware and no allocations can be done from > that? Yes precisely that. Thanks to Stephan for clarifying. An existing example which is defined in reserve-memory schema here https://github.com/devicetree-org/dt-schema/blame/main/dtschema/schemas/reserved-memory/reserved-memory.yaml#L149 > > Also commit msg says "Existing definition limits the IOVA to an > addressable range of 4GiB, and" but I do not see such definition in the > binding at all. So what does it refer to? Processors based out of 32 bit OS, can serve addresses in range 0-31, which implies 4GiB (2pow31). > > > > Best regards, > Krzysztof
On 02/07/2025 18:36, Vikash Garodia wrote: >> >> Also commit msg says "Existing definition limits the IOVA to an >> addressable range of 4GiB, and" but I do not see such definition in the >> binding at all. So what does it refer to? > Processors based out of 32 bit OS, can serve addresses in range 0-31, which > implies 4GiB (2pow31). You are not replying to statements. Your commit msg said "existing definition". Point me to the binding part saying that. Best regards, Krzysztof
On 27/06/2025 17:48, Vikash Garodia wrote:
> Existing definition limits the IOVA to an addressable range of 4GiB, and
> even within that range, some of the space is used by IO registers,
> thereby limiting the available IOVA to even lesser. Video hardware is
> designed to emit different stream-ID for pixel and non-pixel buffers,
> thereby introduce a non-pixel sub node to handle non-pixel stream-ID.
>
> With this, both iris and non-pixel device can have IOVA range of 0-4GiB
> individually. Certain video usecases like higher video concurrency needs
> IOVA higher than 4GiB.
>
> Add reference to the reserve-memory schema, which defines reserved IOVA
> regions that are *excluded* from addressable range. Video hardware
> generates different stream IDs based on the predefined range of IOVA
> addresses. Thereby IOVA addresses for firmware and data buffers need to
> be non overlapping. For ex. 0x0-0x25800000 address range is reserved for
> firmware stream-ID, while non-pixel (bitstream) stream-ID can be
> generated by hardware only when bitstream buffers IOVA address is from
> 0x25800000-0xe0000000.
> Non-pixel stream-ID can now be part of the new sub-node, hence iommus in
> iris node can have either 1 entry for pixel stream-id or 2 entries for
> pixel and non-pixel stream-ids.
>
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
> .../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> @@ -65,10 +65,31 @@ properties:
> - const: core
>
> iommus:
> + minItems: 1
> maxItems: 2
>
> dma-coherent: true
>
> + non-pixel:
> + type: object
> + additionalProperties: false
> +
> + description:
> + Non pixel context bank is needed when video hardware have distinct iommus
> + for non pixel buffers. Non pixel buffers are mainly compressed and
> + internal buffers.
> +
> + properties:
> + iommus:
> + maxItems: 1
> +
> + memory-region:
> + maxItems: 1
> +
> + required:
> + - iommus
> + - memory-region
> +
> operating-points-v2: true
>
> opp-table:
> @@ -86,6 +107,7 @@ required:
>
> allOf:
> - $ref: qcom,venus-common.yaml#
> + - $ref: /schemas/reserved-memory/reserved-memory.yaml
> - if:
> properties:
> compatible:
> @@ -117,6 +139,16 @@ examples:
> #include <dt-bindings/power/qcom-rpmpd.h>
> #include <dt-bindings/power/qcom,rpmhpd.h>
>
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + iris_resv: reservation-iris {
> + iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>,
> + <&iris_non_pixel 0x0 0xe0000000 0x0 0x20000000>;
> + };
> + };
> +
> video-codec@aa00000 {
> compatible = "qcom,sm8550-iris";
> reg = <0x0aa00000 0xf0000>;
> @@ -144,12 +176,16 @@ examples:
> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
> reset-names = "bus";
>
> - iommus = <&apps_smmu 0x1940 0x0000>,
> - <&apps_smmu 0x1947 0x0000>;
> + iommus = <&apps_smmu 0x1947 0x0000>;
> dma-coherent;
>
> operating-points-v2 = <&iris_opp_table>;
>
> + iris_non_pixel: non-pixel {
You can drop the label for this node.
Neil
> + iommus = <&apps_smmu 0x1940 0x0000>;
> + memory-region = <&iris_resv>;
> + };
> +
> iris_opp_table: opp-table {
> compatible = "operating-points-v2";
>
>
On 30/06/2025 17:48, neil.armstrong@linaro.org wrote:
> On 27/06/2025 17:48, Vikash Garodia wrote:
>> Existing definition limits the IOVA to an addressable range of 4GiB, and
>> even within that range, some of the space is used by IO registers,
>> thereby limiting the available IOVA to even lesser. Video hardware is
>> designed to emit different stream-ID for pixel and non-pixel buffers,
>> thereby introduce a non-pixel sub node to handle non-pixel stream-ID.
>>
>> With this, both iris and non-pixel device can have IOVA range of 0-4GiB
>> individually. Certain video usecases like higher video concurrency needs
>> IOVA higher than 4GiB.
>>
>> Add reference to the reserve-memory schema, which defines reserved IOVA
>> regions that are *excluded* from addressable range. Video hardware
>> generates different stream IDs based on the predefined range of IOVA
>> addresses. Thereby IOVA addresses for firmware and data buffers need to
>> be non overlapping. For ex. 0x0-0x25800000 address range is reserved for
>> firmware stream-ID, while non-pixel (bitstream) stream-ID can be
>> generated by hardware only when bitstream buffers IOVA address is from
>> 0x25800000-0xe0000000.
>> Non-pixel stream-ID can now be part of the new sub-node, hence iommus in
>> iris node can have either 1 entry for pixel stream-id or 2 entries for
>> pixel and non-pixel stream-ids.
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>> .../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++--
>> 1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> index c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644
>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> @@ -65,10 +65,31 @@ properties:
>> - const: core
>> iommus:
>> + minItems: 1
>> maxItems: 2
>> dma-coherent: true
>> + non-pixel:
>> + type: object
>> + additionalProperties: false
>> +
>> + description:
>> + Non pixel context bank is needed when video hardware have distinct iommus
>> + for non pixel buffers. Non pixel buffers are mainly compressed and
>> + internal buffers.
>> +
>> + properties:
>> + iommus:
>> + maxItems: 1
>> +
>> + memory-region:
>> + maxItems: 1
>> +
>> + required:
>> + - iommus
>> + - memory-region
>> +
>> operating-points-v2: true
>> opp-table:
>> @@ -86,6 +107,7 @@ required:
>> allOf:
>> - $ref: qcom,venus-common.yaml#
>> + - $ref: /schemas/reserved-memory/reserved-memory.yaml
>> - if:
>> properties:
>> compatible:
>> @@ -117,6 +139,16 @@ examples:
>> #include <dt-bindings/power/qcom-rpmpd.h>
>> #include <dt-bindings/power/qcom,rpmhpd.h>
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + iris_resv: reservation-iris {
>> + iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>,
>> + <&iris_non_pixel 0x0 0xe0000000 0x0 0x20000000>;
>> + };
>> + };
>> +
>> video-codec@aa00000 {
>> compatible = "qcom,sm8550-iris";
>> reg = <0x0aa00000 0xf0000>;
>> @@ -144,12 +176,16 @@ examples:
>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>> reset-names = "bus";
>> - iommus = <&apps_smmu 0x1940 0x0000>,
>> - <&apps_smmu 0x1947 0x0000>;
>> + iommus = <&apps_smmu 0x1947 0x0000>;
>> dma-coherent;
>> operating-points-v2 = <&iris_opp_table>;
>> + iris_non_pixel: non-pixel {
>
> You can drop the label for this node.
Sorry forget this....
>
> Neil
>
>> + iommus = <&apps_smmu 0x1940 0x0000>;
>> + memory-region = <&iris_resv>;
>> + };
>> +
>> iris_opp_table: opp-table {
>> compatible = "operating-points-v2";
>>
>
On 27/06/2025 16:48, Vikash Garodia wrote:
> Existing definition limits the IOVA to an addressable range of 4GiB, and
> even within that range, some of the space is used by IO registers,
> thereby limiting the available IOVA to even lesser. Video hardware is
> designed to emit different stream-ID for pixel and non-pixel buffers,
> thereby introduce a non-pixel sub node to handle non-pixel stream-ID.
>
> With this, both iris and non-pixel device can have IOVA range of 0-4GiB
> individually. Certain video usecases like higher video concurrency needs
> IOVA higher than 4GiB.
>
> Add reference to the reserve-memory schema, which defines reserved IOVA
> regions that are *excluded* from addressable range. Video hardware
> generates different stream IDs based on the predefined range of IOVA
> addresses. Thereby IOVA addresses for firmware and data buffers need to
> be non overlapping. For ex. 0x0-0x25800000 address range is reserved for
> firmware stream-ID, while non-pixel (bitstream) stream-ID can be
> generated by hardware only when bitstream buffers IOVA address is from
> 0x25800000-0xe0000000.
> Non-pixel stream-ID can now be part of the new sub-node, hence iommus in
> iris node can have either 1 entry for pixel stream-id or 2 entries for
> pixel and non-pixel stream-ids.
>
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
> .../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> @@ -65,10 +65,31 @@ properties:
> - const: core
>
> iommus:
> + minItems: 1
> maxItems: 2
>
> dma-coherent: true
>
> + non-pixel:
> + type: object
> + additionalProperties: false
> +
> + description:
> + Non pixel context bank is needed when video hardware have distinct iommus
> + for non pixel buffers. Non pixel buffers are mainly compressed and
> + internal buffers.
You do a better job in the cover letter of describing what this is, why
its needed etc.
Not asking for this verbatim but its clearer:
"All non pixel buffers, i.e bitstream, HFI queues
and internal buffers related to bitstream processing, would be managed
by this non_pixel device."
Where does the term "non-pixel" come from if its a meaningful name wrt
to the firmware then non-pixel is fine but, consider a name such as
"out-of-band" or "oob"
out-of-band is a common term as is "sideband" but sideband I think has a
different meaning, really this non-data/non-pixel data stuff is out-of-band.
At least for the way the language pack I have installed in my brain
right now, "oob" or "out-of-band" is a more intuitive name. Its really
up to you though the main point would be to enumerate the description
here with some of the detail you've put into the cover letter.
> +
> + properties:
> + iommus:
> + maxItems: 1
> +
> + memory-region:
> + maxItems: 1
> +
> + required:
> + - iommus
> + - memory-region
> +
> operating-points-v2: true
> opp-table:
> @@ -86,6 +107,7 @@ required:
>
> allOf:
> - $ref: qcom,venus-common.yaml#
> + - $ref: /schemas/reserved-memory/reserved-memory.yaml
> - if:
> properties:
> compatible:
> @@ -117,6 +139,16 @@ examples:
> #include <dt-bindings/power/qcom-rpmpd.h>
> #include <dt-bindings/power/qcom,rpmhpd.h>
>
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + iris_resv: reservation-iris {
> + iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>,
> + <&iris_non_pixel 0x0 0xe0000000 0x0 0x20000000>;
> + };
> + };
iris_oob would be less text in the end.
> +
> video-codec@aa00000 {
> compatible = "qcom,sm8550-iris";
> reg = <0x0aa00000 0xf0000>;
> @@ -144,12 +176,16 @@ examples:
> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
> reset-names = "bus";
>
> - iommus = <&apps_smmu 0x1940 0x0000>,
> - <&apps_smmu 0x1947 0x0000>;
> + iommus = <&apps_smmu 0x1947 0x0000>;
> dma-coherent;
>
> operating-points-v2 = <&iris_opp_table>;
>
> + iris_non_pixel: non-pixel {
> + iommus = <&apps_smmu 0x1940 0x0000>;
> + memory-region = <&iris_resv>;
> + };
> +
> iris_opp_table: opp-table {
> compatible = "operating-points-v2";
>
>
So I was trying to think of a way to catch you out with an ABI break
but, I don't see how you add minItems: 1 to the iommus declaration above
so dts prior to this change should still be valid.
I think this adds up but, consider oob instead of non-pixel.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
bod
On 6/27/2025 10:01 PM, Bryan O'Donoghue wrote:
> On 27/06/2025 16:48, Vikash Garodia wrote:
>> Existing definition limits the IOVA to an addressable range of 4GiB, and
>> even within that range, some of the space is used by IO registers,
>> thereby limiting the available IOVA to even lesser. Video hardware is
>> designed to emit different stream-ID for pixel and non-pixel buffers,
>> thereby introduce a non-pixel sub node to handle non-pixel stream-ID.
>>
>> With this, both iris and non-pixel device can have IOVA range of 0-4GiB
>> individually. Certain video usecases like higher video concurrency needs
>> IOVA higher than 4GiB.
>>
>> Add reference to the reserve-memory schema, which defines reserved IOVA
>> regions that are *excluded* from addressable range. Video hardware
>> generates different stream IDs based on the predefined range of IOVA
>> addresses. Thereby IOVA addresses for firmware and data buffers need to
>> be non overlapping. For ex. 0x0-0x25800000 address range is reserved for
>> firmware stream-ID, while non-pixel (bitstream) stream-ID can be
>> generated by hardware only when bitstream buffers IOVA address is from
>> 0x25800000-0xe0000000.
>> Non-pixel stream-ID can now be part of the new sub-node, hence iommus in
>> iris node can have either 1 entry for pixel stream-id or 2 entries for
>> pixel and non-pixel stream-ids.
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>> .../bindings/media/qcom,sm8550-iris.yaml | 40 ++++++++++++++++++++--
>> 1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> index
>> c79bf2101812d83b99704f38b7348a9f728dff44..4dda2c9ca1293baa7aee3b9ee10aff38d280fe05 100644
>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> @@ -65,10 +65,31 @@ properties:
>> - const: core
>> iommus:
>> + minItems: 1
>> maxItems: 2
>> dma-coherent: true
>> + non-pixel:
>> + type: object
>> + additionalProperties: false
>> +
>> + description:
>> + Non pixel context bank is needed when video hardware have distinct iommus
>> + for non pixel buffers. Non pixel buffers are mainly compressed and
>> + internal buffers.
>
> You do a better job in the cover letter of describing what this is, why its
> needed etc.
>
> Not asking for this verbatim but its clearer:
>
> "All non pixel buffers, i.e bitstream, HFI queues
> and internal buffers related to bitstream processing, would be managed
> by this non_pixel device."
>
> Where does the term "non-pixel" come from if its a meaningful name wrt to the
> firmware then non-pixel is fine but, consider a name such as "out-of-band" or "oob"
It is in-sync with firmware and we would be seeing it more when we extend it for
secure usecases.
Regards,
Vikash
> out-of-band is a common term as is "sideband" but sideband I think has a
> different meaning, really this non-data/non-pixel data stuff is out-of-band.
>
> At least for the way the language pack I have installed in my brain right now,
> "oob" or "out-of-band" is a more intuitive name. Its really up to you though the
> main point would be to enumerate the description here with some of the detail
> you've put into the cover letter.
>
>> +
>> + properties:
>> + iommus:
>> + maxItems: 1
>> +
>> + memory-region:
>> + maxItems: 1
>> +
>> + required:
>> + - iommus
>> + - memory-region
>> +
>> operating-points-v2: true
>
>> opp-table:
>> @@ -86,6 +107,7 @@ required:
>> allOf:
>> - $ref: qcom,venus-common.yaml#
>> + - $ref: /schemas/reserved-memory/reserved-memory.yaml
>> - if:
>> properties:
>> compatible:
>> @@ -117,6 +139,16 @@ examples:
>> #include <dt-bindings/power/qcom-rpmpd.h>
>> #include <dt-bindings/power/qcom,rpmhpd.h>
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + iris_resv: reservation-iris {
>> + iommu-addresses = <&iris_non_pixel 0x0 0x0 0x0 0x25800000>,
>> + <&iris_non_pixel 0x0 0xe0000000 0x0 0x20000000>;
>> + };
>> + };
>
> iris_oob would be less text in the end.
>
>> +
>> video-codec@aa00000 {
>> compatible = "qcom,sm8550-iris";
>> reg = <0x0aa00000 0xf0000>;
>> @@ -144,12 +176,16 @@ examples:
>> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>> reset-names = "bus";
>> - iommus = <&apps_smmu 0x1940 0x0000>,
>> - <&apps_smmu 0x1947 0x0000>;
>> + iommus = <&apps_smmu 0x1947 0x0000>;
>> dma-coherent;
>> operating-points-v2 = <&iris_opp_table>;
>> + iris_non_pixel: non-pixel {
>> + iommus = <&apps_smmu 0x1940 0x0000>;
>> + memory-region = <&iris_resv>;
>> + };
>> +
>> iris_opp_table: opp-table {
>> compatible = "operating-points-v2";
>>
>
> So I was trying to think of a way to catch you out with an ABI break but, I
> don't see how you add minItems: 1 to the iommus declaration above so dts prior
> to this change should still be valid.
>
> I think this adds up but, consider oob instead of non-pixel.
>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>
> ---
> bod
© 2016 - 2025 Red Hat, Inc.