[PATCH v2,0/3] Enable video on sc7180

[PATCH v2,0/3] Enable video on sc7180

[Dikshita Agarwal]

Hello,

This patch series enables video functionality for SC7180.
Here is v2 with the following changes:

 - updated the place of venus DT node to have it sorted by address

 - added dt-binding schema

 - added DT compatible string and resource structure

These changes depend on patch series [1] and [2].

Regards,
Dikshita

[1] https://patchwork.kernel.org/project/linux-media/list/?series=219021
[2] https://patchwork.kernel.org/project/linux-media/list/?series=214797

Dikshita Agarwal (3):
  arm64: dts: sc7180: Add Venus video codec DT node
  dt-bindings: media: venus: Add sc7180 DT schema
  venus: core: add sc7180 DT compatible and resource struct

 .../bindings/media/qcom,venus-sc7180.yaml          | 131 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7180.dtsi               |  36 ++++++
 drivers/media/platform/qcom/venus/core.c           |  58 ++++++++-
 3 files changed, 224 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml

-- 
1.9.1

[PATCH 1/3] arm64: dts: sc7180: Add Venus video codec DT node

[Dikshita Agarwal]

This adds Venus video codec DT node for sc7180.

Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 6876aae2..42c70f5 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/phy/phy-qcom-qusb2.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
+#include <dt-bindings/clock/qcom,videocc-sc7180.h>
 
 / {
 	interrupt-parent = <&intc>;
@@ -66,6 +67,11 @@
 			compatible = "qcom,cmd-db";
 			no-map;
 		};
+
+		venus_mem: memory@8F600000 {
+			reg = <0 0x8F600000 0 0x500000>;
+			no-map;
+		};
 	};
 
 	cpus {
@@ -1042,6 +1048,36 @@
 			};
 		};
 
+		venus: video-codec@aa00000 {
+			compatible = "qcom,sc7180-venus";
+			reg = <0 0x0aa00000 0 0xff000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&videocc VENUS_GDSC>,
+					<&videocc VCODEC0_GDSC>;
+			power-domain-names = "venus", "vcodec0";
+			clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>,
+				<&videocc VIDEO_CC_VENUS_AHB_CLK>,
+				<&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>,
+				<&videocc VIDEO_CC_VCODEC0_CORE_CLK>,
+				<&videocc VIDEO_CC_VCODEC0_AXI_CLK>;
+			clock-names = "core", "iface", "bus",
+					"vcodec0_core", "vcodec0_bus";
+			iommus = <&apps_smmu 0x0C00 0x60>;
+			memory-region = <&venus_mem>;
+
+			video-core0 {
+					compatible = "venus-decoder";
+			};
+
+			video-core1 {
+					compatible = "venus-encoder";
+			};
+
+			video-firmware {
+					iommus = <&apps_smmu 0x0C42 0x0>;
+			};
+		};
+
 		pdc: interrupt-controller@b220000 {
 			compatible = "qcom,sc7180-pdc", "qcom,pdc";
 			reg = <0 0x0b220000 0 0x30000>;
-- 
1.9.1

[PATCH 2/3] dt-bindings: media: venus: Add sc7180 DT schema

[Dikshita Agarwal]

Add new qcom,sc7180-venus DT binding schema.

Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
---
 .../bindings/media/qcom,venus-sc7180.yaml          | 131 +++++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml

diff --git a/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml b/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml
new file mode 100644
index 0000000..a609739
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/media/qcom,venus-sc7180.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm Venus video encode and decode accelerators
+
+maintainers:
+  - Stanimir Varbanov <stanimir.varbanov@linaro.org>
+
+description: |
+  The Venus IP is a video encode and decode accelerator present
+  on Qualcomm platforms
+
+properties:
+  compatible:
+    enum:
+      - qcom,sc7180-venus
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 2
+
+  power-domain-names:
+    maxItems: 2
+    items:
+      - const: venus
+      - const: vcodec0
+
+  clocks:
+    maxItems: 5
+
+  clock-names:
+    maxItems: 5
+    items:
+      - const: core
+      - const: iface
+      - const: bus
+      - const: vcodec0_core
+      - const: vcodec0_bus
+
+  iommus:
+    minItems: 1
+    maxItems: 20
+
+  memory-region:
+    maxItems: 1
+
+  video-core0:
+    type: object
+
+    properties:
+      compatible:
+        const: "venus-decoder"
+
+    required:
+      - compatible
+
+  video-core1:
+    type: object
+
+    properties:
+      compatible:
+        const: "venus-encoder"
+
+    required:
+      - compatible
+
+  video-firmware:
+    type: object
+
+    description: |
+      Firmware subnode is needed when the platform does not
+      have TrustZone.
+
+    properties:
+      iommus:
+        minItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - power-domains
+  - power-domain-names
+  - clocks
+  - clock-names
+  - iommus
+  - memory-region
+  - video-core0
+  - video-core1
+
+examples:
+  - |
+        #include <dt-bindings/interrupt-controller/arm-gic.h>
+        #include <dt-bindings/clock/qcom,videocc-sc7180.h>
+
+		venus: video-codec@aa00000 {
+			compatible = "qcom,sc7180-venus";
+			reg = <0 0x0aa00000 0 0xff000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&videocc VENUS_GDSC>,
+					<&videocc VCODEC0_GDSC>;
+
+			power-domain-names = "venus", "vcodec0";
+			clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>,
+				<&videocc VIDEO_CC_VENUS_AHB_CLK>,
+				<&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>,
+				<&videocc VIDEO_CC_VCODEC0_CORE_CLK>,
+				<&videocc VIDEO_CC_VCODEC0_AXI_CLK>;
+			clock-names = "core", "iface", "bus",
+					"vcodec0_core", "vcodec0_bus";
+			iommus = <&apps_smmu 0x0C00 0x60>;
+			memory-region = <&venus_mem>;
+			video-core0 {
+					compatible = "venus-decoder";
+			};
+			video-core1 {
+					compatible = "venus-encoder";
+			};
+			video-firmware {
+					iommus = <&apps_smmu 0x0C42 0x0>;
+			};
+		};
-- 
1.9.1

[PATCH 3/3] venus: core: add sc7180 DT compatible and resource struct

[Dikshita Agarwal]

This add DT compatible string and resource structure for sc7180.

Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
---
 drivers/media/platform/qcom/venus/core.c | 58 +++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index c7525d9..e8c8b28 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -469,7 +469,7 @@ static __maybe_unused int venus_runtime_resume(struct device *dev)
 	{ 2073600, 3929000, 0, 5551000, 0 },	/* 4096x2160@60 */
 	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
 	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
-	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
+	{  244800,  442000, 0,  659000, 0 },	/* 1920x1080@30 */
 };
 
 static const struct venus_resources sdm845_res = {
@@ -521,11 +521,67 @@ static __maybe_unused int venus_runtime_resume(struct device *dev)
 	.fwname = "qcom/venus-5.2/venus.mdt",
 };
 
+static const struct freq_tbl sc7180_freq_table[] = {
+	{  0, 380000000 },
+	{  0, 340000000 },
+	{  0, 270000000 },
+	{  0, 150000000 },
+};
+
+static struct codec_freq_data sc7180_codec_freq_data[] =  {
+	{ V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_ENC, 675, 10 },
+	{ V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_ENC, 675, 10 },
+	{ V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_ENC, 675, 10 },
+	{ V4L2_PIX_FMT_MPEG2, VIDC_SESSION_TYPE_DEC, 200, 10 },
+	{ V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_DEC, 200, 10 },
+	{ V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_DEC, 200, 10 },
+	{ V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_DEC, 200, 10 },
+	{ V4L2_PIX_FMT_VP9, VIDC_SESSION_TYPE_DEC, 200, 10 },
+};
+
+static const struct bw_tbl sc7180_bw_table_enc[] = {
+	{  972000,  750000, 0, 0, 0 },	/* 3840x2160@30 */
+	{  489600,  451000, 0, 0, 0 },	/* 1920x1080@60 */
+	{  244800,  234000, 0, 0, 0 },	/* 1920x1080@30 */
+};
+
+static const struct bw_tbl sc7180_bw_table_dec[] = {
+	{ 1036800, 1386000, 0, 1875000, 0 },	/* 4096x2160@30 */
+	{  489600,  865000, 0, 1146000, 0 },	/* 1920x1080@60 */
+	{  244800,  530000, 0,  583000, 0 },	/* 1920x1080@30 */
+};
+
+static const struct venus_resources sc7180_res = {
+	.freq_tbl = sc7180_freq_table,
+	.freq_tbl_size = ARRAY_SIZE(sc7180_freq_table),
+	.bw_tbl_enc = sc7180_bw_table_enc,
+	.bw_tbl_enc_size = ARRAY_SIZE(sc7180_bw_table_enc),
+	.bw_tbl_dec = sc7180_bw_table_dec,
+	.bw_tbl_dec_size = ARRAY_SIZE(sc7180_bw_table_dec),
+	.codec_freq_data = sc7180_codec_freq_data,
+	.codec_freq_data_size = ARRAY_SIZE(sc7180_codec_freq_data),
+	.clks = {"core", "iface", "bus" },
+	.clks_num = 3,
+	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
+	.vcodec_clks_num = 2,
+	.vcodec_pmdomains = { "venus", "vcodec0" },
+	.vcodec_pmdomains_num = 2,
+	.vcodec_num = 1,
+	.max_load = 3110400,	/* 4096x2160@90 */
+	.hfi_version = HFI_VERSION_4XX,
+	.vmem_id = VIDC_RESOURCE_NONE,
+	.vmem_size = 0,
+	.vmem_addr = 0,
+	.dma_mask = 0xe0000000 - 1,
+	.fwname = "qcom/venus-5.4/venus.mdt",
+};
+
 static const struct of_device_id venus_dt_match[] = {
 	{ .compatible = "qcom,msm8916-venus", .data = &msm8916_res, },
 	{ .compatible = "qcom,msm8996-venus", .data = &msm8996_res, },
 	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
 	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
+	{ .compatible = "qcom,sc7180-venus", .data = &sc7180_res, },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, venus_dt_match);
-- 
1.9.1

Re: [PATCH 3/3] venus: core: add sc7180 DT compatible and resource struct

[Stanimir Varbanov]

Hi Dikshita,

Thanks for the patch!

On 12/20/19 9:59 AM, Dikshita Agarwal wrote:
> This add DT compatible string and resource structure for sc7180.
> 
> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
> ---
>  drivers/media/platform/qcom/venus/core.c | 58 +++++++++++++++++++++++++++++++-
>  1 file changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index c7525d9..e8c8b28 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -469,7 +469,7 @@ static __maybe_unused int venus_runtime_resume(struct device *dev)
>  	{ 2073600, 3929000, 0, 5551000, 0 },	/* 4096x2160@60 */
>  	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
>  	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
> -	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
> +	{  244800,  442000, 0,  659000, 0 },	/* 1920x1080@30 */

unrelated change, please drop it

>  };
>  
>  static const struct venus_resources sdm845_res = {
> @@ -521,11 +521,67 @@ static __maybe_unused int venus_runtime_resume(struct device *dev)
>  	.fwname = "qcom/venus-5.2/venus.mdt",
>  };
>  
> +static const struct freq_tbl sc7180_freq_table[] = {
> +	{  0, 380000000 },
> +	{  0, 340000000 },
> +	{  0, 270000000 },
> +	{  0, 150000000 },

why .load is zero?

> +};
> +
> +static struct codec_freq_data sc7180_codec_freq_data[] =  {
> +	{ V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_ENC, 675, 10 },
> +	{ V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_ENC, 675, 10 },
> +	{ V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_ENC, 675, 10 },
> +	{ V4L2_PIX_FMT_MPEG2, VIDC_SESSION_TYPE_DEC, 200, 10 },
> +	{ V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_DEC, 200, 10 },
> +	{ V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_DEC, 200, 10 },
> +	{ V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_DEC, 200, 10 },
> +	{ V4L2_PIX_FMT_VP9, VIDC_SESSION_TYPE_DEC, 200, 10 },
> +};

the table is exactly the same as sdm845 one, please reuse it.

> +
> +static const struct bw_tbl sc7180_bw_table_enc[] = {
> +	{  972000,  750000, 0, 0, 0 },	/* 3840x2160@30 */
> +	{  489600,  451000, 0, 0, 0 },	/* 1920x1080@60 */
> +	{  244800,  234000, 0, 0, 0 },	/* 1920x1080@30 */
> +};
> +
> +static const struct bw_tbl sc7180_bw_table_dec[] = {
> +	{ 1036800, 1386000, 0, 1875000, 0 },	/* 4096x2160@30 */
> +	{  489600,  865000, 0, 1146000, 0 },	/* 1920x1080@60 */
> +	{  244800,  530000, 0,  583000, 0 },	/* 1920x1080@30 */
> +};
> +
> +static const struct venus_resources sc7180_res = {
> +	.freq_tbl = sc7180_freq_table,
> +	.freq_tbl_size = ARRAY_SIZE(sc7180_freq_table),
> +	.bw_tbl_enc = sc7180_bw_table_enc,
> +	.bw_tbl_enc_size = ARRAY_SIZE(sc7180_bw_table_enc),
> +	.bw_tbl_dec = sc7180_bw_table_dec,
> +	.bw_tbl_dec_size = ARRAY_SIZE(sc7180_bw_table_dec),
> +	.codec_freq_data = sc7180_codec_freq_data,
> +	.codec_freq_data_size = ARRAY_SIZE(sc7180_codec_freq_data),
> +	.clks = {"core", "iface", "bus" },
> +	.clks_num = 3,
> +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
> +	.vcodec_clks_num = 2,
> +	.vcodec_pmdomains = { "venus", "vcodec0" },
> +	.vcodec_pmdomains_num = 2,
> +	.vcodec_num = 1,
> +	.max_load = 3110400,	/* 4096x2160@90 */

Looking into above bandwidth tables I can guess that the maximimum load
is reached at 4096x2160@30? If so you have to change it here.

<cut>

-- 
regards,
Stan

Re: [PATCH 1/3] arm64: dts: sc7180: Add Venus video codec DT node

[Stanimir Varbanov]

Hi Dikshita,

Thanks for the patch.

On 12/20/19 9:59 AM, Dikshita Agarwal wrote:
> This adds Venus video codec DT node for sc7180.
> 
> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 6876aae2..42c70f5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -10,6 +10,7 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/phy/phy-qcom-qusb2.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> +#include <dt-bindings/clock/qcom,videocc-sc7180.h>
>  
>  / {
>  	interrupt-parent = <&intc>;
> @@ -66,6 +67,11 @@
>  			compatible = "qcom,cmd-db";
>  			no-map;
>  		};
> +
> +		venus_mem: memory@8F600000 {
> +			reg = <0 0x8F600000 0 0x500000>;

Please use lower-case for hex numbers.

> +			no-map;
> +		};
>  	};
>  
>  	cpus {
> @@ -1042,6 +1048,36 @@
>  			};
>  		};
>  
> +		venus: video-codec@aa00000 {
> +			compatible = "qcom,sc7180-venus";
> +			reg = <0 0x0aa00000 0 0xff000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +			power-domains = <&videocc VENUS_GDSC>,
> +					<&videocc VCODEC0_GDSC>;
> +			power-domain-names = "venus", "vcodec0";
> +			clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>,
> +				<&videocc VIDEO_CC_VENUS_AHB_CLK>,
> +				<&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>,
> +				<&videocc VIDEO_CC_VCODEC0_CORE_CLK>,
> +				<&videocc VIDEO_CC_VCODEC0_AXI_CLK>;

could you align those entries to the first one (you can use tabs and
after that spaces to align)

> +			clock-names = "core", "iface", "bus",
> +					"vcodec0_core", "vcodec0_bus";
> +			iommus = <&apps_smmu 0x0C00 0x60>;

lower-case please

> +			memory-region = <&venus_mem>;
> +
> +			video-core0 {
> +					compatible = "venus-decoder";

something is wrong with the indentation?

Please run checkpatch with --strict

> +			};
> +
> +			video-core1 {
> +					compatible = "venus-encoder";
> +			};
> +
> +			video-firmware {
> +					iommus = <&apps_smmu 0x0C42 0x0>;

lower-case

> +			};

This subnode should be in sc7180-idp.dts, because we assume that by
default the qcom platforms have TZ.

> +		};
> +
>  		pdc: interrupt-controller@b220000 {
>  			compatible = "qcom,sc7180-pdc", "qcom,pdc";
>  			reg = <0 0x0b220000 0 0x30000>;
> 

-- 
regards,
Stan

Re: [PATCH 2/3] dt-bindings: media: venus: Add sc7180 DT schema

[Stanimir Varbanov]

Hi Dikshita,

Thanks for the patch!

Please see Rob's comments about DT schema for msm8916. You have to
address them too.

On 12/20/19 9:59 AM, Dikshita Agarwal wrote:
> Add new qcom,sc7180-venus DT binding schema.
> 
> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
> ---
>  .../bindings/media/qcom,venus-sc7180.yaml          | 131 +++++++++++++++++++++
>  1 file changed, 131 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml b/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml
> new file mode 100644
> index 0000000..a609739
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml
> @@ -0,0 +1,131 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/media/qcom,venus-sc7180.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm Venus video encode and decode accelerators
> +
> +maintainers:
> +  - Stanimir Varbanov <stanimir.varbanov@linaro.org>
> +
> +description: |
> +  The Venus IP is a video encode and decode accelerator present
> +  on Qualcomm platforms
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sc7180-venus
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 2
> +
> +  power-domain-names:
> +    maxItems: 2
> +    items:
> +      - const: venus
> +      - const: vcodec0
> +
> +  clocks:
> +    maxItems: 5
> +
> +  clock-names:
> +    maxItems: 5
> +    items:
> +      - const: core
> +      - const: iface
> +      - const: bus
> +      - const: vcodec0_core
> +      - const: vcodec0_bus
> +
> +  iommus:
> +    minItems: 1
> +    maxItems: 20
> +
> +  memory-region:
> +    maxItems: 1
> +
> +  video-core0:
> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: "venus-decoder"
> +
> +    required:
> +      - compatible
> +
> +  video-core1:
> +    type: object
> +
> +    properties:
> +      compatible:
> +        const: "venus-encoder"
> +
> +    required:
> +      - compatible
> +
> +  video-firmware:
> +    type: object
> +
> +    description: |
> +      Firmware subnode is needed when the platform does not
> +      have TrustZone.
> +
> +    properties:
> +      iommus:
> +        minItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - power-domains
> +  - power-domain-names
> +  - clocks
> +  - clock-names
> +  - iommus
> +  - memory-region
> +  - video-core0
> +  - video-core1
> +
> +examples:
> +  - |
> +        #include <dt-bindings/interrupt-controller/arm-gic.h>
> +        #include <dt-bindings/clock/qcom,videocc-sc7180.h>
> +
> +		venus: video-codec@aa00000 {
> +			compatible = "qcom,sc7180-venus";
> +			reg = <0 0x0aa00000 0 0xff000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +			power-domains = <&videocc VENUS_GDSC>,
> +					<&videocc VCODEC0_GDSC>;
> +
> +			power-domain-names = "venus", "vcodec0";
> +			clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>,
> +				<&videocc VIDEO_CC_VENUS_AHB_CLK>,
> +				<&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>,
> +				<&videocc VIDEO_CC_VCODEC0_CORE_CLK>,
> +				<&videocc VIDEO_CC_VCODEC0_AXI_CLK>;
> +			clock-names = "core", "iface", "bus",
> +					"vcodec0_core", "vcodec0_bus";
> +			iommus = <&apps_smmu 0x0C00 0x60>;
> +			memory-region = <&venus_mem>;
> +			video-core0 {
> +					compatible = "venus-decoder";
> +			};
> +			video-core1 {
> +					compatible = "venus-encoder";
> +			};
> +			video-firmware {
> +					iommus = <&apps_smmu 0x0C42 0x0>;
> +			};
> +		};
> 

-- 
regards,
Stan

Re: [PATCH 1/3] arm64: dts: sc7180: Add Venus video codec DT node

[dikshita]

Hi Stan,

Thanks for the review!
I will address all the comments in the next version.

On 2019-12-20 15:04, Stanimir Varbanov wrote:
> Hi Dikshita,
> 
> Thanks for the patch.
> 
> On 12/20/19 9:59 AM, Dikshita Agarwal wrote:
>> This adds Venus video codec DT node for sc7180.
>> 
>> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 
>> ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> index 6876aae2..42c70f5 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -10,6 +10,7 @@
>>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>>  #include <dt-bindings/phy/phy-qcom-qusb2.h>
>>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
>> +#include <dt-bindings/clock/qcom,videocc-sc7180.h>
>> 
>>  / {
>>  	interrupt-parent = <&intc>;
>> @@ -66,6 +67,11 @@
>>  			compatible = "qcom,cmd-db";
>>  			no-map;
>>  		};
>> +
>> +		venus_mem: memory@8F600000 {
>> +			reg = <0 0x8F600000 0 0x500000>;
> 
> Please use lower-case for hex numbers.
> 
>> +			no-map;
>> +		};
>>  	};
>> 
>>  	cpus {
>> @@ -1042,6 +1048,36 @@
>>  			};
>>  		};
>> 
>> +		venus: video-codec@aa00000 {
>> +			compatible = "qcom,sc7180-venus";
>> +			reg = <0 0x0aa00000 0 0xff000>;
>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +			power-domains = <&videocc VENUS_GDSC>,
>> +					<&videocc VCODEC0_GDSC>;
>> +			power-domain-names = "venus", "vcodec0";
>> +			clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>,
>> +				<&videocc VIDEO_CC_VENUS_AHB_CLK>,
>> +				<&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>,
>> +				<&videocc VIDEO_CC_VCODEC0_CORE_CLK>,
>> +				<&videocc VIDEO_CC_VCODEC0_AXI_CLK>;
> 
> could you align those entries to the first one (you can use tabs and
> after that spaces to align)
> 
>> +			clock-names = "core", "iface", "bus",
>> +					"vcodec0_core", "vcodec0_bus";
>> +			iommus = <&apps_smmu 0x0C00 0x60>;
> 
> lower-case please
> 
>> +			memory-region = <&venus_mem>;
>> +
>> +			video-core0 {
>> +					compatible = "venus-decoder";
> 
> something is wrong with the indentation?
> 
> Please run checkpatch with --strict
> 
>> +			};
>> +
>> +			video-core1 {
>> +					compatible = "venus-encoder";
>> +			};
>> +
>> +			video-firmware {
>> +					iommus = <&apps_smmu 0x0C42 0x0>;
> 
> lower-case
> 
>> +			};
> 
> This subnode should be in sc7180-idp.dts, because we assume that by
> default the qcom platforms have TZ.
> 
>> +		};
>> +
>>  		pdc: interrupt-controller@b220000 {
>>  			compatible = "qcom,sc7180-pdc", "qcom,pdc";
>>  			reg = <0 0x0b220000 0 0x30000>;
>> 

Re: [PATCH 3/3] venus: core: add sc7180 DT compatible and resource struct

[dikshita]

Hi Stan,

Thanks for the review!

On 2019-12-20 15:03, Stanimir Varbanov wrote:
> Hi Dikshita,
> 
> Thanks for the patch!
> 
> On 12/20/19 9:59 AM, Dikshita Agarwal wrote:
>> This add DT compatible string and resource structure for sc7180.
>> 
>> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
>> ---
>>  drivers/media/platform/qcom/venus/core.c | 58 
>> +++++++++++++++++++++++++++++++-
>>  1 file changed, 57 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/media/platform/qcom/venus/core.c 
>> b/drivers/media/platform/qcom/venus/core.c
>> index c7525d9..e8c8b28 100644
>> --- a/drivers/media/platform/qcom/venus/core.c
>> +++ b/drivers/media/platform/qcom/venus/core.c
>> @@ -469,7 +469,7 @@ static __maybe_unused int 
>> venus_runtime_resume(struct device *dev)
>>  	{ 2073600, 3929000, 0, 5551000, 0 },	/* 4096x2160@60 */
>>  	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
>>  	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
>> -	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
>> +	{  244800,  442000, 0,  659000, 0 },	/* 1920x1080@30 */
> 
> unrelated change, please drop it
Sure, I will address this in next version.
> 
>>  };
>> 
>>  static const struct venus_resources sdm845_res = {
>> @@ -521,11 +521,67 @@ static __maybe_unused int 
>> venus_runtime_resume(struct device *dev)
>>  	.fwname = "qcom/venus-5.2/venus.mdt",
>>  };
>> 
>> +static const struct freq_tbl sc7180_freq_table[] = {
>> +	{  0, 380000000 },
>> +	{  0, 340000000 },
>> +	{  0, 270000000 },
>> +	{  0, 150000000 },
> 
> why .load is zero?
.load is not being used any longer to calculate load and is a dummy 
value.
So keeping it 0.

> 
>> +};
>> +
>> +static struct codec_freq_data sc7180_codec_freq_data[] =  {
>> +	{ V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_ENC, 675, 10 },
>> +	{ V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_ENC, 675, 10 },
>> +	{ V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_ENC, 675, 10 },
>> +	{ V4L2_PIX_FMT_MPEG2, VIDC_SESSION_TYPE_DEC, 200, 10 },
>> +	{ V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_DEC, 200, 10 },
>> +	{ V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_DEC, 200, 10 },
>> +	{ V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_DEC, 200, 10 },
>> +	{ V4L2_PIX_FMT_VP9, VIDC_SESSION_TYPE_DEC, 200, 10 },
>> +};
> 
> the table is exactly the same as sdm845 one, please reuse it.
Sure, I will address this in next version.
> 
>> +
>> +static const struct bw_tbl sc7180_bw_table_enc[] = {
>> +	{  972000,  750000, 0, 0, 0 },	/* 3840x2160@30 */
>> +	{  489600,  451000, 0, 0, 0 },	/* 1920x1080@60 */
>> +	{  244800,  234000, 0, 0, 0 },	/* 1920x1080@30 */
>> +};
>> +
>> +static const struct bw_tbl sc7180_bw_table_dec[] = {
>> +	{ 1036800, 1386000, 0, 1875000, 0 },	/* 4096x2160@30 */
>> +	{  489600,  865000, 0, 1146000, 0 },	/* 1920x1080@60 */
>> +	{  244800,  530000, 0,  583000, 0 },	/* 1920x1080@30 */
>> +};
>> +
>> +static const struct venus_resources sc7180_res = {
>> +	.freq_tbl = sc7180_freq_table,
>> +	.freq_tbl_size = ARRAY_SIZE(sc7180_freq_table),
>> +	.bw_tbl_enc = sc7180_bw_table_enc,
>> +	.bw_tbl_enc_size = ARRAY_SIZE(sc7180_bw_table_enc),
>> +	.bw_tbl_dec = sc7180_bw_table_dec,
>> +	.bw_tbl_dec_size = ARRAY_SIZE(sc7180_bw_table_dec),
>> +	.codec_freq_data = sc7180_codec_freq_data,
>> +	.codec_freq_data_size = ARRAY_SIZE(sc7180_codec_freq_data),
>> +	.clks = {"core", "iface", "bus" },
>> +	.clks_num = 3,
>> +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
>> +	.vcodec_clks_num = 2,
>> +	.vcodec_pmdomains = { "venus", "vcodec0" },
>> +	.vcodec_pmdomains_num = 2,
>> +	.vcodec_num = 1,
>> +	.max_load = 3110400,	/* 4096x2160@90 */
> 
> Looking into above bandwidth tables I can guess that the maximimum load
> is reached at 4096x2160@30? If so you have to change it here.

After checking further on this I see that max_load can be removed since
it is not being used now to determine if H/W is overloaded or not.
What do you suggest?
> 
> <cut>

Re: [PATCH 3/3] venus: core: add sc7180 DT compatible and resource struct

[Stanimir Varbanov]

Hi Dikshita,

On 12/23/19 12:04 PM, dikshita@codeaurora.org wrote:
> Hi Stan,
> 
> Thanks for the review!
> 
> On 2019-12-20 15:03, Stanimir Varbanov wrote:
>> Hi Dikshita,
>>
>> Thanks for the patch!
>>
>> On 12/20/19 9:59 AM, Dikshita Agarwal wrote:
>>> This add DT compatible string and resource structure for sc7180.
>>>
>>> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
>>> ---
>>>  drivers/media/platform/qcom/venus/core.c | 58
>>> +++++++++++++++++++++++++++++++-
>>>  1 file changed, 57 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/core.c
>>> b/drivers/media/platform/qcom/venus/core.c
>>> index c7525d9..e8c8b28 100644
>>> --- a/drivers/media/platform/qcom/venus/core.c
>>> +++ b/drivers/media/platform/qcom/venus/core.c
>>> @@ -469,7 +469,7 @@ static __maybe_unused int
>>> venus_runtime_resume(struct device *dev)
>>>      { 2073600, 3929000, 0, 5551000, 0 },    /* 4096x2160@60 */
>>>      { 1036800, 1987000, 0, 2797000, 0 },    /* 4096x2160@30 */
>>>      {  489600, 1040000, 0, 1298000, 0 },    /* 1920x1080@60 */
>>> -    {  244800,  530000, 0,  659000, 0 },    /* 1920x1080@30 */
>>> +    {  244800,  442000, 0,  659000, 0 },    /* 1920x1080@30 */
>>
>> unrelated change, please drop it
> Sure, I will address this in next version.
>>
>>>  };
>>>
>>>  static const struct venus_resources sdm845_res = {
>>> @@ -521,11 +521,67 @@ static __maybe_unused int
>>> venus_runtime_resume(struct device *dev)
>>>      .fwname = "qcom/venus-5.2/venus.mdt",
>>>  };
>>>
>>> +static const struct freq_tbl sc7180_freq_table[] = {
>>> +    {  0, 380000000 },
>>> +    {  0, 340000000 },
>>> +    {  0, 270000000 },
>>> +    {  0, 150000000 },
>>
>> why .load is zero?
> .load is not being used any longer to calculate load and is a dummy value.
> So keeping it 0.

Hmm, ok I forgot about that fact. I suppose it is fine then.

I found some other issue. Looking into [1], for sc7180 we have two more
clock-controller frequencies, could you add them too in the table. And
last, in the same patch we have 19.2MHz do you think such frequency
makes any sense?

[1] https://lkml.org/lkml/2019/11/15/361
> 
>>
>>> +};
>>> +
>>> +static struct codec_freq_data sc7180_codec_freq_data[] =  {
>>> +    { V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_ENC, 675, 10 },
>>> +    { V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_ENC, 675, 10 },
>>> +    { V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_ENC, 675, 10 },
>>> +    { V4L2_PIX_FMT_MPEG2, VIDC_SESSION_TYPE_DEC, 200, 10 },
>>> +    { V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_DEC, 200, 10 },
>>> +    { V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_DEC, 200, 10 },
>>> +    { V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_DEC, 200, 10 },
>>> +    { V4L2_PIX_FMT_VP9, VIDC_SESSION_TYPE_DEC, 200, 10 },
>>> +};
>>
>> the table is exactly the same as sdm845 one, please reuse it.
> Sure, I will address this in next version.
>>
>>> +
>>> +static const struct bw_tbl sc7180_bw_table_enc[] = {
>>> +    {  972000,  750000, 0, 0, 0 },    /* 3840x2160@30 */
>>> +    {  489600,  451000, 0, 0, 0 },    /* 1920x1080@60 */
>>> +    {  244800,  234000, 0, 0, 0 },    /* 1920x1080@30 */
>>> +};
>>> +
>>> +static const struct bw_tbl sc7180_bw_table_dec[] = {
>>> +    { 1036800, 1386000, 0, 1875000, 0 },    /* 4096x2160@30 */
>>> +    {  489600,  865000, 0, 1146000, 0 },    /* 1920x1080@60 */
>>> +    {  244800,  530000, 0,  583000, 0 },    /* 1920x1080@30 */
>>> +};
>>> +
>>> +static const struct venus_resources sc7180_res = {
>>> +    .freq_tbl = sc7180_freq_table,
>>> +    .freq_tbl_size = ARRAY_SIZE(sc7180_freq_table),
>>> +    .bw_tbl_enc = sc7180_bw_table_enc,
>>> +    .bw_tbl_enc_size = ARRAY_SIZE(sc7180_bw_table_enc),
>>> +    .bw_tbl_dec = sc7180_bw_table_dec,
>>> +    .bw_tbl_dec_size = ARRAY_SIZE(sc7180_bw_table_dec),
>>> +    .codec_freq_data = sc7180_codec_freq_data,
>>> +    .codec_freq_data_size = ARRAY_SIZE(sc7180_codec_freq_data),
>>> +    .clks = {"core", "iface", "bus" },
>>> +    .clks_num = 3,
>>> +    .vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
>>> +    .vcodec_clks_num = 2,
>>> +    .vcodec_pmdomains = { "venus", "vcodec0" },
>>> +    .vcodec_pmdomains_num = 2,
>>> +    .vcodec_num = 1,
>>> +    .max_load = 3110400,    /* 4096x2160@90 */
>>
>> Looking into above bandwidth tables I can guess that the maximimum load
>> is reached at 4096x2160@30? If so you have to change it here.
> 
> After checking further on this I see that max_load can be removed since
> it is not being used now to determine if H/W is overloaded or not.
> What do you suggest?

Lets have it just for informational reasons.

-- 
regards,
Stan

Re: [PATCH 2/3] dt-bindings: media: venus: Add sc7180 DT schema

[dikshita]

Hi Stan,

Thanks for the review.
Sure, I will apply the same changes to this file as well once
you update DT schema for msm8916 in the next version.

On 2019-12-20 15:07, Stanimir Varbanov wrote:
> Hi Dikshita,
> 
> Thanks for the patch!
> 
> Please see Rob's comments about DT schema for msm8916. You have to
> address them too.
> 
> On 12/20/19 9:59 AM, Dikshita Agarwal wrote:
>> Add new qcom,sc7180-venus DT binding schema.
>> 
>> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
>> ---
>>  .../bindings/media/qcom,venus-sc7180.yaml          | 131 
>> +++++++++++++++++++++
>>  1 file changed, 131 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml 
>> b/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml
>> new file mode 100644
>> index 0000000..a609739
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,venus-sc7180.yaml
>> @@ -0,0 +1,131 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/media/qcom,venus-sc7180.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Qualcomm Venus video encode and decode accelerators
>> +
>> +maintainers:
>> +  - Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> +
>> +description: |
>> +  The Venus IP is a video encode and decode accelerator present
>> +  on Qualcomm platforms
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,sc7180-venus
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  power-domains:
>> +    maxItems: 2
>> +
>> +  power-domain-names:
>> +    maxItems: 2
>> +    items:
>> +      - const: venus
>> +      - const: vcodec0
>> +
>> +  clocks:
>> +    maxItems: 5
>> +
>> +  clock-names:
>> +    maxItems: 5
>> +    items:
>> +      - const: core
>> +      - const: iface
>> +      - const: bus
>> +      - const: vcodec0_core
>> +      - const: vcodec0_bus
>> +
>> +  iommus:
>> +    minItems: 1
>> +    maxItems: 20
>> +
>> +  memory-region:
>> +    maxItems: 1
>> +
>> +  video-core0:
>> +    type: object
>> +
>> +    properties:
>> +      compatible:
>> +        const: "venus-decoder"
>> +
>> +    required:
>> +      - compatible
>> +
>> +  video-core1:
>> +    type: object
>> +
>> +    properties:
>> +      compatible:
>> +        const: "venus-encoder"
>> +
>> +    required:
>> +      - compatible
>> +
>> +  video-firmware:
>> +    type: object
>> +
>> +    description: |
>> +      Firmware subnode is needed when the platform does not
>> +      have TrustZone.
>> +
>> +    properties:
>> +      iommus:
>> +        minItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - power-domains
>> +  - power-domain-names
>> +  - clocks
>> +  - clock-names
>> +  - iommus
>> +  - memory-region
>> +  - video-core0
>> +  - video-core1
>> +
>> +examples:
>> +  - |
>> +        #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +        #include <dt-bindings/clock/qcom,videocc-sc7180.h>
>> +
>> +		venus: video-codec@aa00000 {
>> +			compatible = "qcom,sc7180-venus";
>> +			reg = <0 0x0aa00000 0 0xff000>;
>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +			power-domains = <&videocc VENUS_GDSC>,
>> +					<&videocc VCODEC0_GDSC>;
>> +
>> +			power-domain-names = "venus", "vcodec0";
>> +			clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>,
>> +				<&videocc VIDEO_CC_VENUS_AHB_CLK>,
>> +				<&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>,
>> +				<&videocc VIDEO_CC_VCODEC0_CORE_CLK>,
>> +				<&videocc VIDEO_CC_VCODEC0_AXI_CLK>;
>> +			clock-names = "core", "iface", "bus",
>> +					"vcodec0_core", "vcodec0_bus";
>> +			iommus = <&apps_smmu 0x0C00 0x60>;
>> +			memory-region = <&venus_mem>;
>> +			video-core0 {
>> +					compatible = "venus-decoder";
>> +			};
>> +			video-core1 {
>> +					compatible = "venus-encoder";
>> +			};
>> +			video-firmware {
>> +					iommus = <&apps_smmu 0x0C42 0x0>;
>> +			};
>> +		};
>> 

Re: [PATCH 1/3] arm64: dts: sc7180: Add Venus video codec DT node

[Sai Prakash Ranjan]

Hi Stan,

On 2019-12-20 15:04, Stanimir Varbanov wrote:
> 
> This subnode should be in sc7180-idp.dts, because we assume that by
> default the qcom platforms have TZ.
> 

sc7180.dtsi will not be used on TZ based platforms which was the case 
for SDM845
where sdm845.dtsi was common for TZ (dragonboards) and non TZ(cheza 
boards) based platforms.
So in order to avoid duplicating this node in other board specific dts 
files, it
would be better to have it here itself.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/3] arm64: dts: sc7180: Add Venus video codec DT node

[Sai Prakash Ranjan]

Hi Stan,

On 2019-12-24 17:42, Sai Prakash Ranjan wrote:
> Hi Stan,
> 
> On 2019-12-20 15:04, Stanimir Varbanov wrote:
>> 
>> This subnode should be in sc7180-idp.dts, because we assume that by
>> default the qcom platforms have TZ.
>> 
> 
> sc7180.dtsi will not be used on TZ based platforms which was the case 
> for SDM845
> where sdm845.dtsi was common for TZ (dragonboards) and non TZ(cheza
> boards) based platforms.
> So in order to avoid duplicating this node in other board specific dts 
> files, it
> would be better to have it here itself.
> 

Sorry, please ignore my previous comment.
sc7180 will be used for compute platforms and some would be TZ based.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/3] arm64: dts: sc7180: Add Venus video codec DT node

[Bjorn Andersson]

On Thu 19 Dec 23:59 PST 2019, Dikshita Agarwal wrote:

> This adds Venus video codec DT node for sc7180.
> 
> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 6876aae2..42c70f5 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -10,6 +10,7 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/phy/phy-qcom-qusb2.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> +#include <dt-bindings/clock/qcom,videocc-sc7180.h>
>  
>  / {
>  	interrupt-parent = <&intc>;
> @@ -66,6 +67,11 @@
>  			compatible = "qcom,cmd-db";
>  			no-map;
>  		};
> +
> +		venus_mem: memory@8F600000 {
> +			reg = <0 0x8F600000 0 0x500000>;
> +			no-map;
> +		};
>  	};
>  
>  	cpus {
> @@ -1042,6 +1048,36 @@
>  			};
>  		};
>  
> +		venus: video-codec@aa00000 {
> +			compatible = "qcom,sc7180-venus";
> +			reg = <0 0x0aa00000 0 0xff000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +			power-domains = <&videocc VENUS_GDSC>,

Should this be aligned with the DT refactoring done for sdm845, where
the GDSC is moved into the *-core subnodes etc?

Regards,
Bjorn

> +					<&videocc VCODEC0_GDSC>;
> +			power-domain-names = "venus", "vcodec0";
> +			clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>,
> +				<&videocc VIDEO_CC_VENUS_AHB_CLK>,
> +				<&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>,
> +				<&videocc VIDEO_CC_VCODEC0_CORE_CLK>,
> +				<&videocc VIDEO_CC_VCODEC0_AXI_CLK>;
> +			clock-names = "core", "iface", "bus",
> +					"vcodec0_core", "vcodec0_bus";
> +			iommus = <&apps_smmu 0x0C00 0x60>;
> +			memory-region = <&venus_mem>;
> +
> +			video-core0 {
> +					compatible = "venus-decoder";
> +			};
> +
> +			video-core1 {
> +					compatible = "venus-encoder";
> +			};
> +
> +			video-firmware {
> +					iommus = <&apps_smmu 0x0C42 0x0>;
> +			};
> +		};
> +
>  		pdc: interrupt-controller@b220000 {
>  			compatible = "qcom,sc7180-pdc", "qcom,pdc";
>  			reg = <0 0x0b220000 0 0x30000>;
> -- 
> 1.9.1
> 

Re: [PATCH 1/3] arm64: dts: sc7180: Add Venus video codec DT node

[vgarodia]

Hi Bjorn,

Thanks for your review comments.

On 2019-12-29 08:48, Bjorn Andersson wrote:
> On Thu 19 Dec 23:59 PST 2019, Dikshita Agarwal wrote:
> 
>> This adds Venus video codec DT node for sc7180.
>> 
>> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sc7180.dtsi | 36 
>> ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> index 6876aae2..42c70f5 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -10,6 +10,7 @@
>>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>>  #include <dt-bindings/phy/phy-qcom-qusb2.h>
>>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
>> +#include <dt-bindings/clock/qcom,videocc-sc7180.h>
>> 
>>  / {
>>  	interrupt-parent = <&intc>;
>> @@ -66,6 +67,11 @@
>>  			compatible = "qcom,cmd-db";
>>  			no-map;
>>  		};
>> +
>> +		venus_mem: memory@8F600000 {
>> +			reg = <0 0x8F600000 0 0x500000>;
>> +			no-map;
>> +		};
>>  	};
>> 
>>  	cpus {
>> @@ -1042,6 +1048,36 @@
>>  			};
>>  		};
>> 
>> +		venus: video-codec@aa00000 {
>> +			compatible = "qcom,sc7180-venus";
>> +			reg = <0 0x0aa00000 0 0xff000>;
>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +			power-domains = <&videocc VENUS_GDSC>,
> 
> Should this be aligned with the DT refactoring done for sdm845, where
> the GDSC is moved into the *-core subnodes etc?
This is already aligned to new refactored design i.e clocks/GDSCs are no 
more
core specific.

> Regards,
> Bjorn
> 
>> +					<&videocc VCODEC0_GDSC>;
>> +			power-domain-names = "venus", "vcodec0";
>> +			clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>,
>> +				<&videocc VIDEO_CC_VENUS_AHB_CLK>,
>> +				<&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>,
>> +				<&videocc VIDEO_CC_VCODEC0_CORE_CLK>,
>> +				<&videocc VIDEO_CC_VCODEC0_AXI_CLK>;
>> +			clock-names = "core", "iface", "bus",
>> +					"vcodec0_core", "vcodec0_bus";
>> +			iommus = <&apps_smmu 0x0C00 0x60>;
>> +			memory-region = <&venus_mem>;
>> +
>> +			video-core0 {
>> +					compatible = "venus-decoder";
>> +			};
>> +
>> +			video-core1 {
>> +					compatible = "venus-encoder";
>> +			};
>> +
>> +			video-firmware {
>> +					iommus = <&apps_smmu 0x0C42 0x0>;
>> +			};
>> +		};
>> +
>>  		pdc: interrupt-controller@b220000 {
>>  			compatible = "qcom,sc7180-pdc", "qcom,pdc";
>>  			reg = <0 0x0b220000 0 0x30000>;
>> --
>> 1.9.1
>>