[PATCH 0/4] media: qcom: iris: add support for SA8775P

[PATCH 0/4] media: qcom: iris: add support for SA8775P

[Vikash Garodia]

Add support for video hardware acceleration on SA8775P platform. SA8775P 
is a similar video IP as that in SM8550, except the input to hardware 
being a collapsible MX.

Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
---
Vikash Garodia (4):
      dt-bindings: media: qcom,sm8550-iris: update power domain name
      dt-bindings: media: qcom,sm8550-iris: document SA8775p IRIS accelerator
      arm64: dts: qcom: sa8775p: add support for video node
      media: iris: add compatible string for sa8775p

 .../bindings/media/qcom,sm8550-iris.yaml           |  6 +-
 arch/arm64/boot/dts/qcom/sa8775p.dtsi              | 67 ++++++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
 3 files changed, 75 insertions(+), 2 deletions(-)
---
base-commit: f2151613e040973c868d28c8b00885dfab69eb75
change-id: 20250310-dtbinding-8921bfc151e9

Best regards,
-- 
Vikash Garodia <quic_vgarodia@quicinc.com>

[PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Vikash Garodia]

Not all platforms has a collapsible mx, so use the more generic naming
of mx in the binding.

Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
---
 Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -28,7 +28,7 @@ properties:
     items:
       - const: venus
       - const: vcodec0
-      - const: mxc
+      - const: mx
       - const: mmcx
 
   clocks:

-- 
2.34.1

[PATCH 2/4] dt-bindings: media: qcom,sm8550-iris: document SA8775p IRIS accelerator

[Vikash Garodia]

Document the IRIS video decoder and encoder accelerator found in the
SA8775P platform. SA8775P has collapsible MX compared to SM8550.

Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
---
 Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index 440a0d7cdfe19a1ccedefc207d96b26eed5d6630..20ac596638ba33f49cce9e42d70d31a8aaa7c36e 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -19,7 +19,9 @@ allOf:
 
 properties:
   compatible:
-    const: qcom,sm8550-iris
+    enum:
+      - qcom,sm8550-iris
+      - qcom,sa8775p-iris
 
   power-domains:
     maxItems: 4

-- 
2.34.1

[PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Vikash Garodia]

Video node enables video on Qualcomm SA8775P platform.

Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
 #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
 #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
+#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
 #include <dt-bindings/dma/qcom-gpi.h>
 #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
 #include <dt-bindings/mailbox/qcom-ipcc.h>
@@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
 			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
+		iris: video-codec@aa00000 {
+			compatible = "qcom,sa8775p-iris";
+
+			reg = <0 0x0aa00000 0 0xf0000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
+					<&videocc VIDEO_CC_MVS0_GDSC>,
+					<&rpmhpd SA8775P_MXC>,
+					<&rpmhpd SA8775P_MMCX>;
+			power-domain-names = "venus",
+					     "vcodec0",
+					     "mx",
+					     "mmcx";
+			operating-points-v2 = <&iris_opp_table>;
+
+			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
+				 <&videocc VIDEO_CC_MVS0C_CLK>,
+				 <&videocc VIDEO_CC_MVS0_CLK>;
+			clock-names = "iface",
+				      "core",
+				      "vcodec0_core";
+
+			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
+					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
+					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
+					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+			interconnect-names = "cpu-cfg",
+					     "video-mem";
+
+			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";
+			memory-region = <&pil_video_mem>;
+
+			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
+			reset-names = "bus";
+
+			iommus = <&apps_smmu 0x0880 0x0400>,
+				 <&apps_smmu 0x0887 0x0400>;
+			dma-coherent;
+
+			iris_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-366000000 {
+					opp-hz = /bits/ 64 <366000000>;
+					required-opps = <&rpmhpd_opp_svs_l1>,
+							<&rpmhpd_opp_svs_l1>;
+				};
+				opp-444000000 {
+					opp-hz = /bits/ 64 <444000000>;
+					required-opps = <&rpmhpd_opp_nom>,
+							<&rpmhpd_opp_nom>;
+				};
+				opp-533333334 {
+					opp-hz = /bits/ 64 <533333334>;
+					required-opps = <&rpmhpd_opp_turbo>,
+							<&rpmhpd_opp_turbo>;
+				};
+				opp-560000000 {
+					opp-hz = /bits/ 64 <560000000>;
+					required-opps = <&rpmhpd_opp_turbo_l1>,
+							<&rpmhpd_opp_turbo_l1>;
+				};
+			};
+		};
+
 		videocc: clock-controller@abf0000 {
 			compatible = "qcom,sa8775p-videocc";
 			reg = <0x0 0x0abf0000 0x0 0x10000>;

-- 
2.34.1

[PATCH 4/4] media: iris: add compatible string for sa8775p

[Vikash Garodia]

Add required compatible string to enable video hardware
acceleration on sa8775p.

Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
---
 drivers/media/platform/qcom/iris/iris_probe.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index aca442dcc153830e6252d1dca87afb38c0b9eb8f..23c9e76a68013399b0fc1d68c1ccf0f8b0ddb037 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -324,6 +324,10 @@ static const struct of_device_id iris_dt_match[] = {
 		.compatible = "qcom,sm8550-iris",
 		.data = &sm8550_data,
 	},
+	{
+		.compatible = "qcom,sa8775p-iris",
+		.data = &sm8550_data,
+	},
 #if (!IS_ENABLED(CONFIG_VIDEO_QCOM_VENUS))
 		{
 			.compatible = "qcom,sm8250-venus",

-- 
2.34.1

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Rob Herring (Arm)]

On Tue, 11 Mar 2025 17:33:53 +0530, Vikash Garodia wrote:
> Not all platforms has a collapsible mx, so use the more generic naming
> of mx in the binding.
> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/qcom,sm8550-iris.example.dtb: video-codec@aa00000: power-domain-names:2: 'mx' was expected
	from schema $id: http://devicetree.org/schemas/media/qcom,sm8550-iris.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250311-dtbinding-v1-1-5c807d33f7ae@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
> Not all platforms has a collapsible mx, so use the more generic naming
> of mx in the binding.

I guess, it wasn't even tested...

> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> @@ -28,7 +28,7 @@ properties:
>      items:
>        - const: venus
>        - const: vcodec0
> -      - const: mxc
> +      - const: mx
>        - const: mmcx
>  
>    clocks:
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Vikash Garodia]

On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
>> Not all platforms has a collapsible mx, so use the more generic naming
>> of mx in the binding.
> 
> I guess, it wasn't even tested...
Not sure what made you guess so, let me check why my binding checker did not
catch the bot reported warning.
Regards,
Vikash
> 
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> @@ -28,7 +28,7 @@ properties:
>>      items:
>>        - const: venus
>>        - const: vcodec0
>> -      - const: mxc
>> +      - const: mx
>>        - const: mmcx
>>  
>>    clocks:
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH 2/4] dt-bindings: media: qcom,sm8550-iris: document SA8775p IRIS accelerator

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:33:54PM +0530, Vikash Garodia wrote:
> Document the IRIS video decoder and encoder accelerator found in the
> SA8775P platform. SA8775P has collapsible MX compared to SM8550.

How compatible is SA8775P to SM8550? Should it be using a fallback
compatible?

This kind of comes as a more generic question: is there anything like
'IP version' or 'core version'? It would be really nice to determine the
'baseline' SoCs and make other instances compatible with the baseline.

> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index 440a0d7cdfe19a1ccedefc207d96b26eed5d6630..20ac596638ba33f49cce9e42d70d31a8aaa7c36e 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> @@ -19,7 +19,9 @@ allOf:
>  
>  properties:
>    compatible:
> -    const: qcom,sm8550-iris
> +    enum:
> +      - qcom,sm8550-iris
> +      - qcom,sa8775p-iris
>  
>    power-domains:
>      maxItems: 4
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 4/4] media: iris: add compatible string for sa8775p

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:33:56PM +0530, Vikash Garodia wrote:
> Add required compatible string to enable video hardware
> acceleration on sa8775p.

No, use fallback compatibles instead.

> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  drivers/media/platform/qcom/iris/iris_probe.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index aca442dcc153830e6252d1dca87afb38c0b9eb8f..23c9e76a68013399b0fc1d68c1ccf0f8b0ddb037 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -324,6 +324,10 @@ static const struct of_device_id iris_dt_match[] = {
>  		.compatible = "qcom,sm8550-iris",
>  		.data = &sm8550_data,
>  	},
> +	{
> +		.compatible = "qcom,sa8775p-iris",
> +		.data = &sm8550_data,
> +	},
>  #if (!IS_ENABLED(CONFIG_VIDEO_QCOM_VENUS))
>  		{
>  			.compatible = "qcom,sm8250-venus",
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 08:41:01PM +0530, Vikash Garodia wrote:
> 
> On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
> >> Not all platforms has a collapsible mx, so use the more generic naming
> >> of mx in the binding.
> > 
> > I guess, it wasn't even tested...
> Not sure what made you guess so, let me check why my binding checker did not
> catch the bot reported warning.

Obvious: you are changing the bindings in a non-backwards compatible
way, but you are not changing the example in the same file (and
obviously you are not changing the DTs), which means that this wasn't
tested.

Hint: you can use enum [mx, mxc] instead of const. That would make it
backwards compatible.

> Regards,
> Vikash
> > 
> >>
> >> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> >> ---
> >>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
> >> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> @@ -28,7 +28,7 @@ properties:
> >>      items:
> >>        - const: venus
> >>        - const: vcodec0
> >> -      - const: mxc
> >> +      - const: mx
> >>        - const: mmcx
> >>  
> >>    clocks:
> >>
> >> -- 
> >> 2.34.1
> >>
> > 

-- 
With best wishes
Dmitry

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Konrad Dybcio]

On 3/11/25 4:11 PM, Vikash Garodia wrote:
> 
> On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
>> On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
>>> Not all platforms has a collapsible mx, so use the more generic naming
>>> of mx in the binding.
>>
>> I guess, it wasn't even tested...
> Not sure what made you guess so, let me check why my binding checker did not
> catch the bot reported warning.

You probably checked the compiled DTBs (make dtbs_check / CHECK_DTBS=1), but you
also need to test the YAML (make dt_binding_check)

This change can't be accepted as-is, because there are already expectations
about the naming (and order) of the entries.

Because there's a difference, you would normally be expected to add a whole new
list, but maybe the dt-bindings maintainers will agree to Dmitry's solution of
adding a sneaky enum inside the list

Konrad

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Krzysztof Kozlowski]

On 11/03/2025 13:03, Vikash Garodia wrote:
> Not all platforms has a collapsible mx, so use the more generic naming
> of mx in the binding.
> 

No, neither tested, nor justified. Read the file. How many platforms do
you have there? One. Out of this one platform you claim not all of them
have MX collapsible, so you want MX?

Best regards,
Krzysztof

Re: [PATCH 4/4] media: iris: add compatible string for sa8775p

[Krzysztof Kozlowski]

On 11/03/2025 13:03, Vikash Garodia wrote:
> Add required compatible string to enable video hardware
> acceleration on sa8775p.


Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  drivers/media/platform/qcom/iris/iris_probe.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index aca442dcc153830e6252d1dca87afb38c0b9eb8f..23c9e76a68013399b0fc1d68c1ccf0f8b0ddb037 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -324,6 +324,10 @@ static const struct of_device_id iris_dt_match[] = {
>  		.compatible = "qcom,sm8550-iris",
>  		.data = &sm8550_data,
>  	},
> +	{
> +		.compatible = "qcom,sa8775p-iris",
Drop. Not needed.

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: media: qcom,sm8550-iris: document SA8775p IRIS accelerator

[Krzysztof Kozlowski]

On 11/03/2025 13:03, Vikash Garodia wrote:
> Document the IRIS video decoder and encoder accelerator found in the
> SA8775P platform. SA8775P has collapsible MX compared to SM8550.
Does this make interface incompatible? Does not look like.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Vikash Garodia]

On 3/11/2025 11:03 PM, Krzysztof Kozlowski wrote:
> On 11/03/2025 13:03, Vikash Garodia wrote:
>> Not all platforms has a collapsible mx, so use the more generic naming
>> of mx in the binding.
>>
> 
> No, neither tested, nor justified. Read the file. How many platforms do
> you have there? One. Out of this one platform you claim not all of them
> have MX collapsible, so you want MX?
Let say we have one which is non-collapsible, what should be the way in that
case to use the bindings which differ only in the MX/MXC part ?
Regards,
Vikash
> 
> Best regards,
> Krzysztof

Re: [PATCH 2/4] dt-bindings: media: qcom,sm8550-iris: document SA8775p IRIS accelerator

[Vikash Garodia]

On 3/11/2025 11:05 PM, Krzysztof Kozlowski wrote:
> On 11/03/2025 13:03, Vikash Garodia wrote:
>> Document the IRIS video decoder and encoder accelerator found in the
>> SA8775P platform. SA8775P has collapsible MX compared to SM8550.
> Does this make interface incompatible? Does not look like.
Just the phandle specifier in the DT property, otherwise same.

Regards,
Vikash

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Vikash Garodia]

On 3/11/2025 9:05 PM, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 08:41:01PM +0530, Vikash Garodia wrote:
>>
>> On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
>>> On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
>>>> Not all platforms has a collapsible mx, so use the more generic naming
>>>> of mx in the binding.
>>>
>>> I guess, it wasn't even tested...
>> Not sure what made you guess so, let me check why my binding checker did not
>> catch the bot reported warning.
> 
> Obvious: you are changing the bindings in a non-backwards compatible
> way, but you are not changing the example in the same file (and
> obviously you are not changing the DTs), which means that this wasn't
> tested.
> 
> Hint: you can use enum [mx, mxc] instead of const. That would make it
> backwards compatible.
Currently there are no user of this binding. Given that either of MX or MXC are
same connection to video hardware, just that one is collapsible, it would be
good to replace the existing element instead of enum.

Regards,
Vikash
> 
>> Regards,
>> Vikash
>>>
>>>>
>>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>>> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
>>>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>>>> @@ -28,7 +28,7 @@ properties:
>>>>      items:
>>>>        - const: venus
>>>>        - const: vcodec0
>>>> -      - const: mxc
>>>> +      - const: mx
>>>>        - const: mmcx
>>>>  
>>>>    clocks:
>>>>
>>>> -- 
>>>> 2.34.1
>>>>
>>>
> 

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Dmitry Baryshkov]

On Tue, 11 Mar 2025 at 21:19, Vikash Garodia <quic_vgarodia@quicinc.com> wrote:
>
>
> On 3/11/2025 9:05 PM, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 08:41:01PM +0530, Vikash Garodia wrote:
> >>
> >> On 3/11/2025 8:37 PM, Dmitry Baryshkov wrote:
> >>> On Tue, Mar 11, 2025 at 05:33:53PM +0530, Vikash Garodia wrote:
> >>>> Not all platforms has a collapsible mx, so use the more generic naming
> >>>> of mx in the binding.
> >>>
> >>> I guess, it wasn't even tested...
> >> Not sure what made you guess so, let me check why my binding checker did not
> >> catch the bot reported warning.
> >
> > Obvious: you are changing the bindings in a non-backwards compatible
> > way, but you are not changing the example in the same file (and
> > obviously you are not changing the DTs), which means that this wasn't
> > tested.
> >
> > Hint: you can use enum [mx, mxc] instead of const. That would make it
> > backwards compatible.
> Currently there are no user of this binding. Given that either of MX or MXC are
> same connection to video hardware, just that one is collapsible, it would be
> good to replace the existing element instead of enum.

This obviously should go to the commit message.

>
> Regards,
> Vikash
> >
> >> Regards,
> >> Vikash
> >>>
> >>>>
> >>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >>>> index e424ea84c211f473a799481fd5463a16580187ed..440a0d7cdfe19a1ccedefc207d96b26eed5d6630 100644
> >>>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >>>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >>>> @@ -28,7 +28,7 @@ properties:
> >>>>      items:
> >>>>        - const: venus
> >>>>        - const: vcodec0
> >>>> -      - const: mxc
> >>>> +      - const: mx
> >>>>        - const: mmcx
> >>>>
> >>>>    clocks:
> >>>>
> >>>> --
> >>>> 2.34.1
> >>>>
> >>>
> >



-- 
With best wishes
Dmitry

Re: [PATCH 2/4] dt-bindings: media: qcom,sm8550-iris: document SA8775p IRIS accelerator

[Vikash Garodia]

On 3/11/2025 9:01 PM, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 05:33:54PM +0530, Vikash Garodia wrote:
>> Document the IRIS video decoder and encoder accelerator found in the
>> SA8775P platform. SA8775P has collapsible MX compared to SM8550.
> 
> How compatible is SA8775P to SM8550? Should it be using a fallback
> compatible?
They are quite compatible. At the moment i do not see any reason for not using
fallback compatible. Later, if we want to distinguish the associated platform
data for SA8775P and SM8550, the compatible can be made independent and use it
in driver to associate with its own platform data ?
> 
> This kind of comes as a more generic question: is there anything like
> 'IP version' or 'core version'? It would be really nice to determine the
> 'baseline' SoCs and make other instances compatible with the baseline.
It would be helpful if you can elaborate more on this with some drivers
following it (if any) ?

Regards,
Vikash
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> index 440a0d7cdfe19a1ccedefc207d96b26eed5d6630..20ac596638ba33f49cce9e42d70d31a8aaa7c36e 100644
>> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> @@ -19,7 +19,9 @@ allOf:
>>  
>>  properties:
>>    compatible:
>> -    const: qcom,sm8550-iris
>> +    enum:
>> +      - qcom,sm8550-iris
>> +      - qcom,sa8775p-iris
>>  
>>    power-domains:
>>      maxItems: 4
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH 2/4] dt-bindings: media: qcom,sm8550-iris: document SA8775p IRIS accelerator

[Krzysztof Kozlowski]

On 11/03/2025 18:57, Vikash Garodia wrote:
> 
> 
> On 3/11/2025 11:05 PM, Krzysztof Kozlowski wrote:
>> On 11/03/2025 13:03, Vikash Garodia wrote:
>>> Document the IRIS video decoder and encoder accelerator found in the
>>> SA8775P platform. SA8775P has collapsible MX compared to SM8550.
>> Does this make interface incompatible? Does not look like.
> Just the phandle specifier in the DT property, otherwise same.
I don't understand above. This is about hardware and its interface.
Hardware does not care about DT or its phandles.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Krzysztof Kozlowski]

On 11/03/2025 18:47, Vikash Garodia wrote:
> 
> On 3/11/2025 11:03 PM, Krzysztof Kozlowski wrote:
>> On 11/03/2025 13:03, Vikash Garodia wrote:
>>> Not all platforms has a collapsible mx, so use the more generic naming
>>> of mx in the binding.
>>>
>>
>> No, neither tested, nor justified. Read the file. How many platforms do
>> you have there? One. Out of this one platform you claim not all of them
>> have MX collapsible, so you want MX?
> Let say we have one which is non-collapsible, what should be the way in that
> case to use the bindings which differ only in the MX/MXC part ?


I don't care about imaginary things. Send patches for real hardware. How
does collapsibility of the domain change the real hardware interface?

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: media: qcom,sm8550-iris: document SA8775p IRIS accelerator

[Dmitry Baryshkov]

On Wed, 12 Mar 2025 at 10:07, Vikash Garodia <quic_vgarodia@quicinc.com> wrote:
>
>
> On 3/11/2025 9:01 PM, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 05:33:54PM +0530, Vikash Garodia wrote:
> >> Document the IRIS video decoder and encoder accelerator found in the
> >> SA8775P platform. SA8775P has collapsible MX compared to SM8550.
> >
> > How compatible is SA8775P to SM8550? Should it be using a fallback
> > compatible?
> They are quite compatible. At the moment i do not see any reason for not using
> fallback compatible. Later, if we want to distinguish the associated platform
> data for SA8775P and SM8550, the compatible can be made independent and use it
> in driver to associate with its own platform data ?

It is already independent, even if you have a fallback. Nothing stops
you from using platform-specific compat at any point in the future,
you don't even have to change schema for that.

> >
> > This kind of comes as a more generic question: is there anything like
> > 'IP version' or 'core version'? It would be really nice to determine the
> > 'baseline' SoCs and make other instances compatible with the baseline.
> It would be helpful if you can elaborate more on this with some drivers
> following it (if any) ?

iMX platforms have a lot of examples, see any of the device trees.
Another perfect example is the Qualcomm SMMU, where we stopped adding
new compats to the driver, because all implementations are declared
compatible with "qcom,smmu-500' and "arm,smmu-500".

>
> Regards,
> Vikash
> >>
> >> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> >> ---
> >>  Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> index 440a0d7cdfe19a1ccedefc207d96b26eed5d6630..20ac596638ba33f49cce9e42d70d31a8aaa7c36e 100644
> >> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> >> @@ -19,7 +19,9 @@ allOf:
> >>
> >>  properties:
> >>    compatible:
> >> -    const: qcom,sm8550-iris
> >> +    enum:
> >> +      - qcom,sm8550-iris
> >> +      - qcom,sa8775p-iris
> >>
> >>    power-domains:
> >>      maxItems: 4
> >>
> >> --
> >> 2.34.1
> >>
> >



-- 
With best wishes
Dmitry

Re: [PATCH 1/4] dt-bindings: media: qcom,sm8550-iris: update power domain name

[Vikash Garodia]

On 3/12/2025 2:14 PM, Krzysztof Kozlowski wrote:
> On 11/03/2025 18:47, Vikash Garodia wrote:
>>
>> On 3/11/2025 11:03 PM, Krzysztof Kozlowski wrote:
>>> On 11/03/2025 13:03, Vikash Garodia wrote:
>>>> Not all platforms has a collapsible mx, so use the more generic naming
>>>> of mx in the binding.
>>>>
>>>
>>> No, neither tested, nor justified. Read the file. How many platforms do
>>> you have there? One. Out of this one platform you claim not all of them
>>> have MX collapsible, so you want MX?
>> Let say we have one which is non-collapsible, what should be the way in that
>> case to use the bindings which differ only in the MX/MXC part ?
> 
> 
> I don't care about imaginary things. Send patches for real hardware. How
> does collapsibility of the domain change the real hardware interface?
It does not. I am now thinking to drop this patch altogether, and continue to
use MXC as defined in bindings, irrespective of connection to hardware as MX or
MXC. For ex SM8550/SA8775P have MXC, while QCS8300 have MX, but again, as you
mentioned, these difference just alters some property in DT, binding can remain
same.
Regards,
Vikash

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Konrad Dybcio]

On 3/11/25 1:03 PM, Vikash Garodia wrote:
> Video node enables video on Qualcomm SA8775P platform.
> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> @@ -10,6 +10,7 @@
>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>  #include <dt-bindings/dma/qcom-gpi.h>
>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>  #include <dt-bindings/mailbox/qcom-ipcc.h>
> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>  		};
>  
> +		iris: video-codec@aa00000 {
> +			compatible = "qcom,sa8775p-iris";
> +
> +			reg = <0 0x0aa00000 0 0xf0000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
> +					<&videocc VIDEO_CC_MVS0_GDSC>,
> +					<&rpmhpd SA8775P_MXC>,
> +					<&rpmhpd SA8775P_MMCX>;
> +			power-domain-names = "venus",
> +					     "vcodec0",
> +					     "mx",
> +					     "mmcx";
> +			operating-points-v2 = <&iris_opp_table>;
> +
> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
> +				 <&videocc VIDEO_CC_MVS0_CLK>;
> +			clock-names = "iface",
> +				      "core",
> +				      "vcodec0_core";
> +
> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,

This path should use QCOM_ICC_TAG_ACTIVE_ONLY on both endpoints

> +					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
> +					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> +			interconnect-names = "cpu-cfg",
> +					     "video-mem";
> +
> +			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";

If it needs different firmware, I have my doubts over why 8550's data
would be fully reused. Are you sure everything in iris_platform_sm8550.c
applies?

> +			memory-region = <&pil_video_mem>;
> +
> +			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
> +			reset-names = "bus";
> +
> +			iommus = <&apps_smmu 0x0880 0x0400>,
> +				 <&apps_smmu 0x0887 0x0400>;
> +			dma-coherent;
> +
> +			iris_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				opp-366000000 {
> +					opp-hz = /bits/ 64 <366000000>;
> +					required-opps = <&rpmhpd_opp_svs_l1>,
> +							<&rpmhpd_opp_svs_l1>;
> +				};

Please add a newline between subsequent subnodes

Konrad

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Konrad Dybcio]

On 3/15/25 2:43 PM, Konrad Dybcio wrote:
> On 3/11/25 1:03 PM, Vikash Garodia wrote:
>> Video node enables video on Qualcomm SA8775P platform.
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 67 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> @@ -10,6 +10,7 @@
>>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
>> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>>  #include <dt-bindings/dma/qcom-gpi.h>
>>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>>  #include <dt-bindings/mailbox/qcom-ipcc.h>
>> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>>  		};
>>  
>> +		iris: video-codec@aa00000 {
>> +			compatible = "qcom,sa8775p-iris";
>> +
>> +			reg = <0 0x0aa00000 0 0xf0000>;
>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
>> +					<&videocc VIDEO_CC_MVS0_GDSC>,
>> +					<&rpmhpd SA8775P_MXC>,
>> +					<&rpmhpd SA8775P_MMCX>;
>> +			power-domain-names = "venus",
>> +					     "vcodec0",
>> +					     "mx",
>> +					     "mmcx";
>> +			operating-points-v2 = <&iris_opp_table>;
>> +
>> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
>> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
>> +				 <&videocc VIDEO_CC_MVS0_CLK>;
>> +			clock-names = "iface",
>> +				      "core",
>> +				      "vcodec0_core";
>> +
>> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
> 
> This path should use QCOM_ICC_TAG_ACTIVE_ONLY on both endpoints

Please also align the &s

Konrad

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Dmitry Baryshkov]

On Sat, Mar 15, 2025 at 02:43:30PM +0100, Konrad Dybcio wrote:
> On 3/11/25 1:03 PM, Vikash Garodia wrote:
> > Video node enables video on Qualcomm SA8775P platform.
> > 
> > Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > @@ -10,6 +10,7 @@
> >  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
> >  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
> >  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
> > +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
> >  #include <dt-bindings/dma/qcom-gpi.h>
> >  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
> >  #include <dt-bindings/mailbox/qcom-ipcc.h>
> > @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
> >  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
> >  		};
> >  
> > +		iris: video-codec@aa00000 {
> > +			compatible = "qcom,sa8775p-iris";
> > +
> > +			reg = <0 0x0aa00000 0 0xf0000>;
> > +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
> > +					<&videocc VIDEO_CC_MVS0_GDSC>,
> > +					<&rpmhpd SA8775P_MXC>,
> > +					<&rpmhpd SA8775P_MMCX>;
> > +			power-domain-names = "venus",
> > +					     "vcodec0",
> > +					     "mx",
> > +					     "mmcx";
> > +			operating-points-v2 = <&iris_opp_table>;
> > +
> > +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
> > +				 <&videocc VIDEO_CC_MVS0C_CLK>,
> > +				 <&videocc VIDEO_CC_MVS0_CLK>;
> > +			clock-names = "iface",
> > +				      "core",
> > +				      "vcodec0_core";
> > +
> > +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
> > +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
> 
> This path should use QCOM_ICC_TAG_ACTIVE_ONLY on both endpoints
> 
> > +					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
> > +					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> > +			interconnect-names = "cpu-cfg",
> > +					     "video-mem";
> > +
> > +			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";

Firmware-name should be a part of the board DT file rather than the SoC
DT.

> 
> If it needs different firmware, I have my doubts over why 8550's data
> would be fully reused. Are you sure everything in iris_platform_sm8550.c
> applies?

If I understand correctly, the firmware is different, because the
signature profile is different. The Iris core should be compatible.

> 
> > +			memory-region = <&pil_video_mem>;
> > +
> > +			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
> > +			reset-names = "bus";
> > +
> > +			iommus = <&apps_smmu 0x0880 0x0400>,
> > +				 <&apps_smmu 0x0887 0x0400>;
> > +			dma-coherent;
> > +
> > +			iris_opp_table: opp-table {
> > +				compatible = "operating-points-v2";
> > +
> > +				opp-366000000 {
> > +					opp-hz = /bits/ 64 <366000000>;
> > +					required-opps = <&rpmhpd_opp_svs_l1>,
> > +							<&rpmhpd_opp_svs_l1>;
> > +				};
> 
> Please add a newline between subsequent subnodes
> 
> Konrad

-- 
With best wishes
Dmitry

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Vikash Garodia]

On 3/16/2025 12:02 AM, Dmitry Baryshkov wrote:
> On Sat, Mar 15, 2025 at 02:43:30PM +0100, Konrad Dybcio wrote:
>> On 3/11/25 1:03 PM, Vikash Garodia wrote:
>>> Video node enables video on Qualcomm SA8775P platform.
>>>
>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>>>  1 file changed, 67 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
>>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> @@ -10,6 +10,7 @@
>>>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>>>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>>>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
>>> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>>>  #include <dt-bindings/dma/qcom-gpi.h>
>>>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>>>  #include <dt-bindings/mailbox/qcom-ipcc.h>
>>> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>>>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>>>  		};
>>>  
>>> +		iris: video-codec@aa00000 {
>>> +			compatible = "qcom,sa8775p-iris";
>>> +
>>> +			reg = <0 0x0aa00000 0 0xf0000>;
>>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>>> +
>>> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
>>> +					<&videocc VIDEO_CC_MVS0_GDSC>,
>>> +					<&rpmhpd SA8775P_MXC>,
>>> +					<&rpmhpd SA8775P_MMCX>;
>>> +			power-domain-names = "venus",
>>> +					     "vcodec0",
>>> +					     "mx",
>>> +					     "mmcx";
>>> +			operating-points-v2 = <&iris_opp_table>;
>>> +
>>> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
>>> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
>>> +				 <&videocc VIDEO_CC_MVS0_CLK>;
>>> +			clock-names = "iface",
>>> +				      "core",
>>> +				      "vcodec0_core";
>>> +
>>> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>>> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
>>
>> This path should use QCOM_ICC_TAG_ACTIVE_ONLY on both endpoints
>>
>>> +					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
>>> +					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>>> +			interconnect-names = "cpu-cfg",
>>> +					     "video-mem";
>>> +
>>> +			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";
> 
> Firmware-name should be a part of the board DT file rather than the SoC
> DT.
Given that video ip is independent of any board, remains same for any board
variant of SA8775p, and again for the same reason, PIL, aspects of video driver
was made out of probe, i would prefer to keep it in SOC. Again, this can be
overridden in board specific variant DT, if any case arise, incase.
>>
>> If it needs different firmware, I have my doubts over why 8550's data
>> would be fully reused. Are you sure everything in iris_platform_sm8550.c
>> applies?
> 
> If I understand correctly, the firmware is different, because the
> signature profile is different. The Iris core should be compatible.
That appropriately explains the need for different firmware.
> 
>>
>>> +			memory-region = <&pil_video_mem>;
>>> +
>>> +			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>>> +			reset-names = "bus";
>>> +
>>> +			iommus = <&apps_smmu 0x0880 0x0400>,
>>> +				 <&apps_smmu 0x0887 0x0400>;
>>> +			dma-coherent;
>>> +
>>> +			iris_opp_table: opp-table {
>>> +				compatible = "operating-points-v2";
>>> +
>>> +				opp-366000000 {
>>> +					opp-hz = /bits/ 64 <366000000>;
>>> +					required-opps = <&rpmhpd_opp_svs_l1>,
>>> +							<&rpmhpd_opp_svs_l1>;
>>> +				};
>>
>> Please add a newline between subsequent subnodes
>>
>> Konrad
Regards,
Vikash

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Dmitry Baryshkov]

On Mon, Mar 17, 2025 at 11:30:16AM +0530, Vikash Garodia wrote:
> 
> On 3/16/2025 12:02 AM, Dmitry Baryshkov wrote:
> > On Sat, Mar 15, 2025 at 02:43:30PM +0100, Konrad Dybcio wrote:
> >> On 3/11/25 1:03 PM, Vikash Garodia wrote:
> >>> Video node enables video on Qualcomm SA8775P platform.
> >>>
> >>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 67 insertions(+)
> >>>


> >>
> >>> +					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
> >>> +					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> >>> +			interconnect-names = "cpu-cfg",
> >>> +					     "video-mem";
> >>> +
> >>> +			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";
> > 
> > Firmware-name should be a part of the board DT file rather than the SoC
> > DT.
> Given that video ip is independent of any board, remains same for any board
> variant of SA8775p, and again for the same reason, PIL, aspects of video driver
> was made out of probe, i would prefer to keep it in SOC. Again, this can be
> overridden in board specific variant DT, if any case arise, incase.

This file is also board-specific and will not be accepted by a board
fused to work with a particular set of CA keys. So, no. This needs to
go to the board DT file.


-- 
With best wishes
Dmitry

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:33:55PM +0530, Vikash Garodia wrote:
> Video node enables video on Qualcomm SA8775P platform.
> 
> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> @@ -10,6 +10,7 @@
>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>  #include <dt-bindings/dma/qcom-gpi.h>
>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>  #include <dt-bindings/mailbox/qcom-ipcc.h>
> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>  		};
>  
> +		iris: video-codec@aa00000 {
> +			compatible = "qcom,sa8775p-iris";
> +
> +			reg = <0 0x0aa00000 0 0xf0000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
> +					<&videocc VIDEO_CC_MVS0_GDSC>,
> +					<&rpmhpd SA8775P_MXC>,
> +					<&rpmhpd SA8775P_MMCX>;
> +			power-domain-names = "venus",
> +					     "vcodec0",
> +					     "mx",
> +					     "mmcx";
> +			operating-points-v2 = <&iris_opp_table>;
> +
> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
> +				 <&videocc VIDEO_CC_MVS0_CLK>;
> +			clock-names = "iface",
> +				      "core",
> +				      "vcodec0_core";
> +
> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
> +					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
> +					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> +			interconnect-names = "cpu-cfg",
> +					     "video-mem";
> +
> +			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";
> +			memory-region = <&pil_video_mem>;
> +
> +			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
> +			reset-names = "bus";
> +
> +			iommus = <&apps_smmu 0x0880 0x0400>,
> +				 <&apps_smmu 0x0887 0x0400>;
> +			dma-coherent;

I think almost missed it:

	status = "disabled";

Don't enable the device by default, there is a chance of the firmware
being not present.

-- 
With best wishes
Dmitry

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Konrad Dybcio]

On 3/17/25 12:31 PM, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 05:33:55PM +0530, Vikash Garodia wrote:
>> Video node enables video on Qualcomm SA8775P platform.
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 67 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> @@ -10,6 +10,7 @@
>>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
>> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>>  #include <dt-bindings/dma/qcom-gpi.h>
>>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>>  #include <dt-bindings/mailbox/qcom-ipcc.h>
>> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>>  		};
>>  
>> +		iris: video-codec@aa00000 {
>> +			compatible = "qcom,sa8775p-iris";
>> +
>> +			reg = <0 0x0aa00000 0 0xf0000>;
>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
>> +					<&videocc VIDEO_CC_MVS0_GDSC>,
>> +					<&rpmhpd SA8775P_MXC>,
>> +					<&rpmhpd SA8775P_MMCX>;
>> +			power-domain-names = "venus",
>> +					     "vcodec0",
>> +					     "mx",
>> +					     "mmcx";
>> +			operating-points-v2 = <&iris_opp_table>;
>> +
>> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
>> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
>> +				 <&videocc VIDEO_CC_MVS0_CLK>;
>> +			clock-names = "iface",
>> +				      "core",
>> +				      "vcodec0_core";
>> +
>> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
>> +					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
>> +					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>> +			interconnect-names = "cpu-cfg",
>> +					     "video-mem";
>> +
>> +			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";
>> +			memory-region = <&pil_video_mem>;
>> +
>> +			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>> +			reset-names = "bus";
>> +
>> +			iommus = <&apps_smmu 0x0880 0x0400>,
>> +				 <&apps_smmu 0x0887 0x0400>;
>> +			dma-coherent;
> 
> I think almost missed it:
> 
> 	status = "disabled";
> 
> Don't enable the device by default, there is a chance of the firmware
> being not present.

On a sidenote, venus driver broke sync_state by requesting the firmware at
load time, instead of at the time of first open (though the latter could
introduce some latency on first video playback, but both are debatable) -
what does iris do here?

Konrad> 

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Vikash Garodia]

On 3/18/2025 5:52 PM, Konrad Dybcio wrote:
> On 3/17/25 12:31 PM, Dmitry Baryshkov wrote:
>> On Tue, Mar 11, 2025 at 05:33:55PM +0530, Vikash Garodia wrote:
>>> Video node enables video on Qualcomm SA8775P platform.
>>>
>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>>>  1 file changed, 67 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
>>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> @@ -10,6 +10,7 @@
>>>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>>>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>>>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
>>> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>>>  #include <dt-bindings/dma/qcom-gpi.h>
>>>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>>>  #include <dt-bindings/mailbox/qcom-ipcc.h>
>>> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>>>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>>>  		};
>>>  
>>> +		iris: video-codec@aa00000 {
>>> +			compatible = "qcom,sa8775p-iris";
>>> +
>>> +			reg = <0 0x0aa00000 0 0xf0000>;
>>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>>> +
>>> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
>>> +					<&videocc VIDEO_CC_MVS0_GDSC>,
>>> +					<&rpmhpd SA8775P_MXC>,
>>> +					<&rpmhpd SA8775P_MMCX>;
>>> +			power-domain-names = "venus",
>>> +					     "vcodec0",
>>> +					     "mx",
>>> +					     "mmcx";
>>> +			operating-points-v2 = <&iris_opp_table>;
>>> +
>>> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
>>> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
>>> +				 <&videocc VIDEO_CC_MVS0_CLK>;
>>> +			clock-names = "iface",
>>> +				      "core",
>>> +				      "vcodec0_core";
>>> +
>>> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>>> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
>>> +					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
>>> +					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>>> +			interconnect-names = "cpu-cfg",
>>> +					     "video-mem";
>>> +
>>> +			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";
>>> +			memory-region = <&pil_video_mem>;
>>> +
>>> +			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>>> +			reset-names = "bus";
>>> +
>>> +			iommus = <&apps_smmu 0x0880 0x0400>,
>>> +				 <&apps_smmu 0x0887 0x0400>;
>>> +			dma-coherent;
>>
>> I think almost missed it:
>>
>> 	status = "disabled";
>>
>> Don't enable the device by default, there is a chance of the firmware
>> being not present.
> 
> On a sidenote, venus driver broke sync_state by requesting the firmware at
> load time, instead of at the time of first open (though the latter could
> introduce some latency on first video playback, but both are debatable) -
> what does iris do here?
iris does it on first open
Regards,
Vikash
> 
> Konrad> 

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Vikash Garodia]

On 3/15/2025 7:13 PM, Konrad Dybcio wrote:
> On 3/11/25 1:03 PM, Vikash Garodia wrote:
>> Video node enables video on Qualcomm SA8775P platform.
>>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 67 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> @@ -10,6 +10,7 @@
>>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
>> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>>  #include <dt-bindings/dma/qcom-gpi.h>
>>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>>  #include <dt-bindings/mailbox/qcom-ipcc.h>
>> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>>  		};
>>  
>> +		iris: video-codec@aa00000 {
>> +			compatible = "qcom,sa8775p-iris";
>> +
>> +			reg = <0 0x0aa00000 0 0xf0000>;
>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
>> +					<&videocc VIDEO_CC_MVS0_GDSC>,
>> +					<&rpmhpd SA8775P_MXC>,
>> +					<&rpmhpd SA8775P_MMCX>;
>> +			power-domain-names = "venus",
>> +					     "vcodec0",
>> +					     "mx",
>> +					     "mmcx";
>> +			operating-points-v2 = <&iris_opp_table>;
>> +
>> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
>> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
>> +				 <&videocc VIDEO_CC_MVS0_CLK>;
>> +			clock-names = "iface",
>> +				      "core",
>> +				      "vcodec0_core";
>> +
>> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
> 
> This path should use QCOM_ICC_TAG_ACTIVE_ONLY on both endpoints
What is the advantage of "ALWAYS" vs "ACTIVE_ONLY". Thinking of a possibility of
APSS power collapsed, while video hardware is processing a frame ?

Regards,
Vikash
> 
>> +					<&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
>> +					&mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>> +			interconnect-names = "cpu-cfg",
>> +					     "video-mem";
>> +
>> +			firmware-name = "qcom/vpu/vpu30_p4_s6.mbn";
> 
> If it needs different firmware, I have my doubts over why 8550's data
> would be fully reused. Are you sure everything in iris_platform_sm8550.c
> applies?
> 
>> +			memory-region = <&pil_video_mem>;
>> +
>> +			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>> +			reset-names = "bus";
>> +
>> +			iommus = <&apps_smmu 0x0880 0x0400>,
>> +				 <&apps_smmu 0x0887 0x0400>;
>> +			dma-coherent;
>> +
>> +			iris_opp_table: opp-table {
>> +				compatible = "operating-points-v2";
>> +
>> +				opp-366000000 {
>> +					opp-hz = /bits/ 64 <366000000>;
>> +					required-opps = <&rpmhpd_opp_svs_l1>,
>> +							<&rpmhpd_opp_svs_l1>;
>> +				};
> 
> Please add a newline between subsequent subnodes
> 
> Konrad

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Konrad Dybcio]

On 3/18/25 3:24 PM, Vikash Garodia wrote:
> 
> On 3/15/2025 7:13 PM, Konrad Dybcio wrote:
>> On 3/11/25 1:03 PM, Vikash Garodia wrote:
>>> Video node enables video on Qualcomm SA8775P platform.
>>>
>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>>>  1 file changed, 67 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
>>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> @@ -10,6 +10,7 @@
>>>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>>>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>>>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
>>> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>>>  #include <dt-bindings/dma/qcom-gpi.h>
>>>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>>>  #include <dt-bindings/mailbox/qcom-ipcc.h>
>>> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>>>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>>>  		};
>>>  
>>> +		iris: video-codec@aa00000 {
>>> +			compatible = "qcom,sa8775p-iris";
>>> +
>>> +			reg = <0 0x0aa00000 0 0xf0000>;
>>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>>> +
>>> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
>>> +					<&videocc VIDEO_CC_MVS0_GDSC>,
>>> +					<&rpmhpd SA8775P_MXC>,
>>> +					<&rpmhpd SA8775P_MMCX>;
>>> +			power-domain-names = "venus",
>>> +					     "vcodec0",
>>> +					     "mx",
>>> +					     "mmcx";
>>> +			operating-points-v2 = <&iris_opp_table>;
>>> +
>>> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
>>> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
>>> +				 <&videocc VIDEO_CC_MVS0_CLK>;
>>> +			clock-names = "iface",
>>> +				      "core",
>>> +				      "vcodec0_core";
>>> +
>>> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>>> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
>>
>> This path should use QCOM_ICC_TAG_ACTIVE_ONLY on both endpoints
> What is the advantage of "ALWAYS" vs "ACTIVE_ONLY". Thinking of a possibility of
> APSS power collapsed, while video hardware is processing a frame ?

That's totally okay, I'm requesting ACTIVE_ONLY just on the cpu-cfg path,
which must not be used if APSS if offline anyway

Konrad

Re: [PATCH 3/4] arm64: dts: qcom: sa8775p: add support for video node

[Vikash Garodia]

On 3/18/2025 8:03 PM, Konrad Dybcio wrote:
> On 3/18/25 3:24 PM, Vikash Garodia wrote:
>>
>> On 3/15/2025 7:13 PM, Konrad Dybcio wrote:
>>> On 3/11/25 1:03 PM, Vikash Garodia wrote:
>>>> Video node enables video on Qualcomm SA8775P platform.
>>>>
>>>> Signed-off-by: Vikash Garodia <quic_vgarodia@quicinc.com>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 67 +++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 67 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>>> index 3394ae2d13003417a15e64c9e47833725ec779e6..09db8e2eb578f1cada0f4a15e3f844dc097bd46d 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>>> @@ -10,6 +10,7 @@
>>>>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>>>>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>>>>  #include <dt-bindings/clock/qcom,sa8775p-gpucc.h>
>>>> +#include <dt-bindings/clock/qcom,sa8775p-videocc.h>
>>>>  #include <dt-bindings/dma/qcom-gpi.h>
>>>>  #include <dt-bindings/interconnect/qcom,sa8775p-rpmh.h>
>>>>  #include <dt-bindings/mailbox/qcom-ipcc.h>
>>>> @@ -3783,6 +3784,72 @@ llcc: system-cache-controller@9200000 {
>>>>  			interrupts = <GIC_SPI 580 IRQ_TYPE_LEVEL_HIGH>;
>>>>  		};
>>>>  
>>>> +		iris: video-codec@aa00000 {
>>>> +			compatible = "qcom,sa8775p-iris";
>>>> +
>>>> +			reg = <0 0x0aa00000 0 0xf0000>;
>>>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>>>> +
>>>> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
>>>> +					<&videocc VIDEO_CC_MVS0_GDSC>,
>>>> +					<&rpmhpd SA8775P_MXC>,
>>>> +					<&rpmhpd SA8775P_MMCX>;
>>>> +			power-domain-names = "venus",
>>>> +					     "vcodec0",
>>>> +					     "mx",
>>>> +					     "mmcx";
>>>> +			operating-points-v2 = <&iris_opp_table>;
>>>> +
>>>> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
>>>> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
>>>> +				 <&videocc VIDEO_CC_MVS0_CLK>;
>>>> +			clock-names = "iface",
>>>> +				      "core",
>>>> +				      "vcodec0_core";
>>>> +
>>>> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>>>> +					&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
>>>
>>> This path should use QCOM_ICC_TAG_ACTIVE_ONLY on both endpoints
>> What is the advantage of "ALWAYS" vs "ACTIVE_ONLY". Thinking of a possibility of
>> APSS power collapsed, while video hardware is processing a frame ?
> 
> That's totally okay, I'm requesting ACTIVE_ONLY just on the cpu-cfg path,
> which must not be used if APSS if offline anyway
Agree on the CFG path. Will update in next version.
> 
> Konrad