[PATCH v3 0/4] media: venus: enable venus on qcs615

[PATCH v3 0/4] media: venus: enable venus on qcs615

[Renjiang Han]

This series enables 4xx venus encode/decode on qcs615 for BU.
Configure venus node in the devicetree, add resource data in the venus core.
And binding node to the venus core with compatible.

This DT change is dependent on [1]dts and [2] videocc dts,
[3]videocc driver and [4]smmu.

depends on:
[1]
https://lore.kernel.org/all/20241104-add_initial_support_for_qcs615-v5-0-9dde8d7b80b0@quicinc.com/
[2]
https://lore.kernel.org/lkml/20241108-qcs615-mm-dt-nodes-v1-0-b2669cac0624@quicinc.com/
[3]
https://lore.kernel.org/all/20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com/
[4]
https://lore.kernel.org/all/20241105032107.9552-1-quic_qqzhou@quicinc.com/

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
Changes in v3:
- 1. Remove the ‘|’ after 'description' in the qcom,qcs615-venus.yaml.
- 2. Add a blank line before 'opp-table' in the qcom,qcs615-venus.yaml.
- 3. Put ‘additionalProperties’ before ‘properties’ in the
qcom,qcs615-venus.yaml.
- 4. Update the subject of qcom,qcs615-venus.yaml patch.
- Link to v2: https://lore.kernel.org/r/20241112-add-venus-for-qcs615-v2-0-e67947f957af@quicinc.com

Changes in v2:
- 1. The change-id of DT and driver are removed.
- 2. Add qcom,qcs615-venus.yaml files to explain DT.
- 3. The order of driver's commit and DT's commit is adjusted. Place the
driver's commit before the DT's commit.
- 4. Extends driver's commit message.
- 5. Split DT's commit into two commits. Add the venus node to the
qcs615.dtsi file. Then in the qcs615-ride.dts file enable the venus node.
- 6. Modify alignment, sort, upper and lower case letters issue.
- 7. Update cover letter message description.

- Link to v1: https://lore.kernel.org/r/20241008-add_qcs615_video-v1-0-436ce07bfc63@quicinc.com

---
Renjiang Han (4):
      dt-bindings: qcom,qcs615-venus: add support for video hardware
      media: venus: core: add qcs615 platform data
      arm64: dts: qcom: add venus node for the qcs615
      arm64: dts: qcom: qcs615-ride: enable venus node

 .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/qcs615-ride.dts           |   4 +
 arch/arm64/boot/dts/qcom/qcs615.dtsi               |  86 ++++++++++
 drivers/media/platform/qcom/venus/core.c           |  50 ++++++
 4 files changed, 322 insertions(+)
---
base-commit: a39230ecf6b3057f5897bc4744a790070cfbe7a8
change-id: 20241112-add-venus-for-qcs615-686235e64fdd
prerequisite-message-id: <20241104-add_initial_support_for_qcs615-v5-0-9dde8d7b80b0@quicinc.com>
prerequisite-patch-id: 09782474af7eecf1013425fd34f9d2f082fb3616
prerequisite-patch-id: 04ca722967256efddc402b7bab94136a5174b0b9
prerequisite-patch-id: 82481c82a20345548e2cb292d3098ed51843b809
prerequisite-patch-id: 3bd8edd83297815fcb1b81fcd891d3c14908442f
prerequisite-patch-id: fc1cfec4ecd56e669c161c4d2c3797fc0abff0ae
prerequisite-message-id: <20241108-qcs615-mm-dt-nodes-v1-0-b2669cac0624@quicinc.com>
prerequisite-patch-id: bcb1328b70868bb9c87c0e4c48e5c9d38853bc60
prerequisite-patch-id: 8844a4661902eb44406639a3b7344416a0c88ed9
prerequisite-message-id: <20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com>
prerequisite-patch-id: 748a4e51bbedae9c6ebdbd642b2fd1badf958788
prerequisite-patch-id: 72a894a3b19fdbd431e1cec9397365bc5b27abfe
prerequisite-patch-id: da2b7a74f1afd58833c6a9a4544a0e271720641f
prerequisite-patch-id: 40b79fe0b9101f5db3bddad23551c1123572aee5
prerequisite-patch-id: cb93e5798f6bfe8cc3044c4ce973e3ae5f20dc6b
prerequisite-patch-id: 13b0dbf97ac1865d241791afb4b46a28ca499523
prerequisite-patch-id: 807019bedabd47c04f7ac78e9461d0b5a6e9131b
prerequisite-patch-id: 8e2e841401fefbd96d78dd4a7c47514058c83bf2
prerequisite-patch-id: 125bb8cb367109ba22cededf6e78754579e1ed03
prerequisite-patch-id: b3cc42570d5826a4704f7702e7b26af9a0fe57b0
prerequisite-patch-id: df8e2fdd997cbf6c0a107f1871ed9e2caaa97582
prerequisite-message-id: <20241105032107.9552-1-quic_qqzhou@quicinc.com>
prerequisite-patch-id: aaa7214fe86fade46ae5c245e0a44625fae1bad3
prerequisite-patch-id: 4db9f55207af45c6b64fff4f8929648a7fb44669
prerequisite-patch-id: 89ce719a863bf5e909989877f15f82b51552e449

Best regards,
-- 
Renjiang Han <quic_renjiang@quicinc.com>

[PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Renjiang Han]

Add support for Qualcomm video acceleration hardware used for video
stream decoding and encoding on QCOM QCS615.

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
 .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
 1 file changed, 182 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
@@ -0,0 +1,182 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm QCS615 Venus video encode and decode accelerators
+
+maintainers:
+  - Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
+  - Vikash Garodia <quic_vgarodia@quicinc.com>
+
+description:
+  The Venus IP is a video encode and decode accelerator present
+  on Qualcomm platforms
+
+allOf:
+  - $ref: qcom,venus-common.yaml#
+
+properties:
+  compatible:
+    const: qcom,qcs615-venus
+
+  power-domains:
+    minItems: 2
+    maxItems: 3
+
+  power-domain-names:
+    minItems: 2
+    items:
+      - const: venus
+      - const: vcodec0
+      - const: cx
+
+  clocks:
+    maxItems: 5
+
+  clock-names:
+    items:
+      - const: core
+      - const: iface
+      - const: bus
+      - const: vcodec0_core
+      - const: vcodec0_bus
+
+  iommus:
+    maxItems: 1
+
+  memory-region:
+    maxItems: 1
+
+  interconnects:
+    maxItems: 2
+
+  interconnect-names:
+    items:
+      - const: video-mem
+      - const: cpu-cfg
+
+  operating-points-v2: true
+
+  opp-table:
+    type: object
+
+  video-decoder:
+    type: object
+
+    additionalProperties: false
+
+    properties:
+      compatible:
+        const: venus-decoder
+
+    required:
+      - compatible
+
+  video-encoder:
+    type: object
+
+    additionalProperties: false
+
+    properties:
+      compatible:
+        const: venus-encoder
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - power-domain-names
+  - iommus
+  - video-decoder
+  - video-encoder
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,qcs615-videocc.h>
+    #include <dt-bindings/interconnect/qcom,qcs615-rpmh.h>
+    #include <dt-bindings/power/qcom,rpmhpd.h>
+
+    venus: video-codec@aa00000 {
+        compatible = "qcom,qcs615-venus";
+        reg = <0xaa00000 0x100000>;
+        interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+        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";
+
+        power-domains = <&videocc VENUS_GDSC>,
+                        <&videocc VCODEC0_GDSC>,
+                        <&rpmhpd RPMHPD_CX>;
+        power-domain-names = "venus",
+                             "vcodec0",
+                             "cx";
+
+        operating-points-v2 = <&venus_opp_table>;
+
+        interconnects = <&mmss_noc MASTER_VIDEO_P0 0
+                         &mc_virt SLAVE_EBI1 0>,
+                        <&gem_noc MASTER_APPSS_PROC 0
+                         &config_noc SLAVE_VENUS_CFG 0>;
+        interconnect-names = "video-mem",
+                             "cpu-cfg";
+
+        iommus = <&apps_smmu 0xe40 0x20>;
+
+        memory-region = <&pil_video_mem>;
+
+        video-decoder {
+            compatible = "venus-decoder";
+        };
+
+        video-encoder {
+            compatible = "venus-encoder";
+        };
+
+        venus_opp_table: opp-table {
+            compatible = "operating-points-v2";
+
+            opp-133330000 {
+                opp-hz = /bits/ 64 <133330000>;
+                required-opps = <&rpmhpd_opp_low_svs>;
+            };
+
+            opp-240000000 {
+                opp-hz = /bits/ 64 <240000000>;
+                required-opps = <&rpmhpd_opp_svs>;
+            };
+
+            opp-300000000 {
+                opp-hz = /bits/ 64 <300000000>;
+                required-opps = <&rpmhpd_opp_svs_l1>;
+            };
+
+            opp-380000000 {
+                opp-hz = /bits/ 64 <380000000>;
+                required-opps = <&rpmhpd_opp_nom>;
+            };
+
+            opp-410000000 {
+                opp-hz = /bits/ 64 <410000000>;
+                required-opps = <&rpmhpd_opp_turbo>;
+            };
+
+            opp-460000000 {
+                opp-hz = /bits/ 64 <460000000>;
+                required-opps = <&rpmhpd_opp_turbo_l1>;
+            };
+        };
+    };

-- 
2.34.1

[PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Renjiang Han]

Initialize the platform data and enable venus driver probe of QCS615
SoC.

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
 drivers/media/platform/qcom/venus/core.c | 50 ++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index 423deb5e94dcb193974da23f9bd2d905bfeab2d9..39d8bcf62fe4f72674746b75994cce6cbaee94eb 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -630,6 +630,55 @@ static const struct venus_resources msm8998_res = {
 	.fwname = "qcom/venus-4.4/venus.mbn",
 };
 
+static const struct freq_tbl qcs615_freq_table[] = {
+	{ 0, 460000000 },
+	{ 0, 410000000 },
+	{ 0, 380000000 },
+	{ 0, 300000000 },
+	{ 0, 240000000 },
+	{ 0, 133333333 },
+};
+
+static const struct bw_tbl qcs615_bw_table_enc[] = {
+	{  972000,  951000, 0, 1434000, 0 },	/* 3840x2160@30 */
+	{  489600,  723000, 0,  973000, 0 },	/* 1920x1080@60 */
+	{  244800,  370000, 0,	495000, 0 },	/* 1920x1080@30 */
+};
+
+static const struct bw_tbl qcs615_bw_table_dec[] = {
+	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
+	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
+	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
+};
+
+static const struct venus_resources qcs615_res = {
+	.freq_tbl = qcs615_freq_table,
+	.freq_tbl_size = ARRAY_SIZE(qcs615_freq_table),
+	.bw_tbl_enc = qcs615_bw_table_enc,
+	.bw_tbl_enc_size = ARRAY_SIZE(qcs615_bw_table_enc),
+	.bw_tbl_dec = qcs615_bw_table_dec,
+	.bw_tbl_dec_size = ARRAY_SIZE(qcs615_bw_table_dec),
+	.clks = {"core", "iface", "bus" },
+	.clks_num = 3,
+	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
+	.vcodec_clks_num = 2,
+	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
+	.vcodec_pmdomains_num = 2,
+	.opp_pmdomain = (const char *[]) { "cx" },
+	.vcodec_num = 1,
+	.hfi_version = HFI_VERSION_4XX,
+	.vpu_version = VPU_VERSION_AR50,
+	.vmem_id = VIDC_RESOURCE_NONE,
+	.vmem_size = 0,
+	.vmem_addr = 0,
+	.dma_mask = 0xe0000000 - 1,
+	.cp_start = 0,
+	.cp_size = 0x70800000,
+	.cp_nonpixel_start = 0x1000000,
+	.cp_nonpixel_size = 0x24800000,
+	.fwname = "qcom/venus-5.4/venus_s6.mbn",
+};
+
 static const struct freq_tbl sdm660_freq_table[] = {
 	{ 979200, 518400000 },
 	{ 489600, 441600000 },
@@ -937,6 +986,7 @@ 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,msm8998-venus", .data = &msm8998_res, },
+	{ .compatible = "qcom,qcs615-venus", .data = &qcs615_res, },
 	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
 	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
 	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },

-- 
2.34.1

[PATCH v3 3/4] arm64: dts: qcom: add venus node for the qcs615

[Renjiang Han]

Add venus node into devicetree for the qcs615 video.

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcs615.dtsi | 86 ++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
index 06deb5c499fe83f0eb20d7957ca14948de7aab34..18ad4da5ed194458aded424560f45a3a9f3163dc 100644
--- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
@@ -394,6 +394,11 @@ smem_region: smem@86000000 {
 			no-map;
 			hwlocks = <&tcsr_mutex 3>;
 		};
+
+		pil_video_mem: pil-video@93400000 {
+			reg = <0x0 0x93400000 0x0 0x500000>;
+			no-map;
+		};
 	};
 
 	soc: soc@0 {
@@ -530,6 +535,87 @@ gem_noc: interconnect@9680000 {
 			qcom,bcm-voters = <&apps_bcm_voter>;
 		};
 
+		venus: video-codec@aa00000 {
+			compatible = "qcom,qcs615-venus";
+			reg = <0x0 0xaa00000 0x0 0x100000>;
+			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+			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";
+
+			power-domains = <&videocc VENUS_GDSC>,
+					<&videocc VCODEC0_GDSC>,
+					<&rpmhpd RPMHPD_CX>;
+			power-domain-names = "venus",
+					     "vcodec0",
+					     "cx";
+
+			operating-points-v2 = <&venus_opp_table>;
+
+			interconnects = <&mmss_noc MASTER_VIDEO_P0 0
+					 &mc_virt SLAVE_EBI1 0>,
+					<&gem_noc MASTER_APPSS_PROC 0
+					 &config_noc SLAVE_VENUS_CFG 0>;
+			interconnect-names = "video-mem",
+					     "cpu-cfg";
+
+			iommus = <&apps_smmu 0xe40 0x20>;
+
+			memory-region = <&pil_video_mem>;
+
+			status = "disabled";
+
+			video-decoder {
+				compatible = "venus-decoder";
+			};
+
+			video-encoder {
+				compatible = "venus-encoder";
+			};
+
+			venus_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				opp-133330000 {
+					opp-hz = /bits/ 64 <133330000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+				};
+
+				opp-240000000 {
+					opp-hz = /bits/ 64 <240000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+				};
+
+				opp-300000000 {
+					opp-hz = /bits/ 64 <300000000>;
+					required-opps = <&rpmhpd_opp_svs_l1>;
+				};
+
+				opp-380000000 {
+					opp-hz = /bits/ 64 <380000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+				};
+
+				opp-410000000 {
+					opp-hz = /bits/ 64 <410000000>;
+					required-opps = <&rpmhpd_opp_turbo>;
+				};
+
+				opp-460000000 {
+					opp-hz = /bits/ 64 <460000000>;
+					required-opps = <&rpmhpd_opp_turbo_l1>;
+				};
+			};
+		};
+
 		videocc: clock-controller@ab00000 {
 			compatible = "qcom,qcs615-videocc";
 			reg = <0 0xab00000 0 0x10000>;

-- 
2.34.1

[PATCH v3 4/4] arm64: dts: qcom: qcs615-ride: enable venus node

[Renjiang Han]

Enable the venus node so that the video codec will start working.

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcs615-ride.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
index 4ef969a6af150933c72a7a83374a5a2657eebc1b..4a7bfffb7b86b21c412b16426019c9063368ca19 100644
--- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
@@ -217,6 +217,10 @@ &uart0 {
 	status = "okay";
 };
 
+&venus {
+	status = "okay";
+};
+
 &watchdog {
 	clocks = <&sleep_clk>;
 };

-- 
2.34.1

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Rob Herring (Arm)]

On Mon, 25 Nov 2024 11:04:49 +0530, Renjiang Han wrote:
> Add support for Qualcomm video acceleration hardware used for video
> stream decoding and encoding on QCOM QCS615.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
>  1 file changed, 182 insertions(+)
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dts:25:18: fatal error: dt-bindings/clock/qcom,qcs615-videocc.h: No such file or directory
   25 |         #include <dt-bindings/clock/qcom,qcs615-videocc.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.dtbs:129: Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1442: dt_binding_check] Error 2
make: *** [Makefile:224: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241125-add-venus-for-qcs615-v3-1-5a376b97a68e@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 v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Krzysztof Kozlowski]

On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote:
> Add support for Qualcomm video acceleration hardware used for video
> stream decoding and encoding on QCOM QCS615.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
>  1 file changed, 182 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml

Dependency for this patch must be mentioned here.

Amount of dependencies make it unmergeable and untesteable. I suggest
decoupling dependencies by removing clock constants.


> new file mode 100644
> index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
> @@ -0,0 +1,182 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm QCS615 Venus video encode and decode accelerators
> +
> +maintainers:
> +  - Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
> +  - Vikash Garodia <quic_vgarodia@quicinc.com>
> +
> +description:
> +  The Venus IP is a video encode and decode accelerator present
> +  on Qualcomm platforms
> +
> +allOf:
> +  - $ref: qcom,venus-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,qcs615-venus
> +
> +  power-domains:
> +    minItems: 2
> +    maxItems: 3
> +
> +  power-domain-names:
> +    minItems: 2
> +    items:
> +      - const: venus
> +      - const: vcodec0
> +      - const: cx
> +
> +  clocks:
> +    maxItems: 5
> +
> +  clock-names:
> +    items:
> +      - const: core
> +      - const: iface
> +      - const: bus
> +      - const: vcodec0_core
> +      - const: vcodec0_bus
> +
> +  iommus:
> +    maxItems: 1
> +
> +  memory-region:
> +    maxItems: 1
> +
> +  interconnects:
> +    maxItems: 2
> +
> +  interconnect-names:
> +    items:
> +      - const: video-mem
> +      - const: cpu-cfg
> +
> +  operating-points-v2: true
> +
> +  opp-table:
> +    type: object
> +
> +  video-decoder:
> +    type: object
> +
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        const: venus-decoder
> +
> +    required:
> +      - compatible
> +
> +  video-encoder:
> +    type: object

Both nodes are useless - no resources here, nothing to control. Do not
add nodes just to instantiate Linux drivers. Drop them.

Best regards,
Krzysztof

RE: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Renjiang Han (QUIC)]

On Monday, November 25, 2024 2:38 PM, Rob Herring (Arm) wrote:
> On Mon, 25 Nov 2024 11:04:49 +0530, Renjiang Han wrote:
> > Add support for Qualcomm video acceleration hardware used for video 
> > stream decoding and encoding on QCOM QCS615.
> >
> > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> > ---
> >  .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
> >  1 file changed, 182 insertions(+)
> >

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

> yamllint warnings/errors:

> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dts:25:18: fatal error: dt-bindings/clock/qcom,qcs615-videocc.h: No such file or directory
>    25 |         #include <dt-bindings/clock/qcom,qcs615-videocc.h>
>       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[2]: *** [scripts/Makefile.dtbs:129: Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dtb] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1442: dt_binding_check] Error 2
> make: *** [Makefile:224: __sub-make] Error 2

Thanks for your reply. "dt-bindings/clock/qcom,qcs615-videocc.h" has been added in https://lore.kernel.org/all/20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com/.

> doc reference errors (make refcheckdocs):

> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241125-add-venus-for-qcs615-v3-1-5a376b97a68e@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 v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Krzysztof Kozlowski]

On 25/11/2024 09:06, Renjiang Han (QUIC) wrote:
> On Monday, November 25, 2024 2:38 PM, Rob Herring (Arm) wrote:
>> On Mon, 25 Nov 2024 11:04:49 +0530, Renjiang Han wrote:
>>> Add support for Qualcomm video acceleration hardware used for video 
>>> stream decoding and encoding on QCOM QCS615.
>>>
>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>>> ---
>>>  .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
>>>  1 file changed, 182 insertions(+)
>>>
> 
>> My bot found errors running 'make dt_binding_check' on your patch:
> 
>> yamllint warnings/errors:
> 
>> dtschema/dtc warnings/errors:
>> Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dts:25:18: fatal error: dt-bindings/clock/qcom,qcs615-videocc.h: No such file or directory
>>    25 |         #include <dt-bindings/clock/qcom,qcs615-videocc.h>
>>       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> compilation terminated.
>> make[2]: *** [scripts/Makefile.dtbs:129: Documentation/devicetree/bindings/media/qcom,qcs615-venus.example.dtb] Error 1
>> make[2]: *** Waiting for unfinished jobs....
>> make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1442: dt_binding_check] Error 2
>> make: *** [Makefile:224: __sub-make] Error 2
> 
> Thanks for your reply. "dt-bindings/clock/qcom,qcs615-videocc.h" has been added in https://lore.kernel.org/all/20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com/.

Why do you ignore rest of the email?

> 
>> doc reference errors (make refcheckdocs):
> 
>> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241125-add-venus-for-qcs615-v3-1-5a376b97a68e@quicinc.com
> 
>> The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

Read, here.

Best regards,
Krzysztof

RE: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Renjiang Han (QUIC)]

On Monday, November 25, 2024 1:35 PM, Renjiang Han wrote:
> Initialize the platform data and enable venus driver probe of QCS615 SoC.

Forgot to add Reviewed-by, next version will add Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  drivers/media/platform/qcom/venus/core.c | 50 ++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)

> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index 423deb5e94dcb193974da23f9bd2d905bfeab2d9..39d8bcf62fe4f72674746b75994cce6cbaee94eb 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -630,6 +630,55 @@ static const struct venus_resources msm8998_res = {
>  	.fwname = "qcom/venus-4.4/venus.mbn",
>  };
 
> +static const struct freq_tbl qcs615_freq_table[] = {
> +	{ 0, 460000000 },
> +	{ 0, 410000000 },
> +	{ 0, 380000000 },
> +	{ 0, 300000000 },
> +	{ 0, 240000000 },
> +	{ 0, 133333333 },
> +};
> +
> +static const struct bw_tbl qcs615_bw_table_enc[] = {
> +	{  972000,  951000, 0, 1434000, 0 },	/* 3840x2160@30 */
> +	{  489600,  723000, 0,  973000, 0 },	/* 1920x1080@60 */
> +	{  244800,  370000, 0,	495000, 0 },	/* 1920x1080@30 */
> +};
> +
> +static const struct bw_tbl qcs615_bw_table_dec[] = {
> +	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
> +	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
> +	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
> +};
> +
> +static const struct venus_resources qcs615_res = {
> +	.freq_tbl = qcs615_freq_table,
> +	.freq_tbl_size = ARRAY_SIZE(qcs615_freq_table),
> +	.bw_tbl_enc = qcs615_bw_table_enc,
> +	.bw_tbl_enc_size = ARRAY_SIZE(qcs615_bw_table_enc),
> +	.bw_tbl_dec = qcs615_bw_table_dec,
> +	.bw_tbl_dec_size = ARRAY_SIZE(qcs615_bw_table_dec),
> +	.clks = {"core", "iface", "bus" },
> +	.clks_num = 3,
> +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
> +	.vcodec_clks_num = 2,
> +	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
> +	.vcodec_pmdomains_num = 2,
> +	.opp_pmdomain = (const char *[]) { "cx" },
> +	.vcodec_num = 1,
> +	.hfi_version = HFI_VERSION_4XX,
> +	.vpu_version = VPU_VERSION_AR50,
> +	.vmem_id = VIDC_RESOURCE_NONE,
> +	.vmem_size = 0,
> +	.vmem_addr = 0,
> +	.dma_mask = 0xe0000000 - 1,
> +	.cp_start = 0,
> +	.cp_size = 0x70800000,
> +	.cp_nonpixel_start = 0x1000000,
> +	.cp_nonpixel_size = 0x24800000,
> +	.fwname = "qcom/venus-5.4/venus_s6.mbn", };
> +
>  static const struct freq_tbl sdm660_freq_table[] = {
>  	{ 979200, 518400000 },
>  	{ 489600, 441600000 },
> @@ -937,6 +986,7 @@ 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,msm8998-venus", .data = &msm8998_res, },
> +	{ .compatible = "qcom,qcs615-venus", .data = &qcs615_res, },
>  	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
>  	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
>  	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },

> --
> 2.34.1

RE: [PATCH v3 3/4] arm64: dts: qcom: add venus node for the qcs615

[Renjiang Han (QUIC)]

On Mon 11/25/2024 1:35 PM, Renjiang Han wrote:
> Add venus node into devicetree for the qcs615 video.

Forgot to add Reviewed-by, next version will add Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs615.dtsi | 86 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)

> diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> index 06deb5c499fe83f0eb20d7957ca14948de7aab34..18ad4da5ed194458aded424560f45a3a9f3163dc 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> @@ -394,6 +394,11 @@ smem_region: smem@86000000 {
>  			no-map;
>  			hwlocks = <&tcsr_mutex 3>;
>  		};
> +
> +		pil_video_mem: pil-video@93400000 {
> +			reg = <0x0 0x93400000 0x0 0x500000>;
> +			no-map;
> +		};
>  	};
>  
>  	soc: soc@0 {
> @@ -530,6 +535,87 @@ gem_noc: interconnect@9680000 {
>  			qcom,bcm-voters = <&apps_bcm_voter>;
>  		};
>  
> +		venus: video-codec@aa00000 {
> +			compatible = "qcom,qcs615-venus";
> +			reg = <0x0 0xaa00000 0x0 0x100000>;
> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			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";
> +
> +			power-domains = <&videocc VENUS_GDSC>,
> +					<&videocc VCODEC0_GDSC>,
> +					<&rpmhpd RPMHPD_CX>;
> +			power-domain-names = "venus",
> +					     "vcodec0",
> +					     "cx";
> +
> +			operating-points-v2 = <&venus_opp_table>;
> +
> +			interconnects = <&mmss_noc MASTER_VIDEO_P0 0
> +					 &mc_virt SLAVE_EBI1 0>,
> +					<&gem_noc MASTER_APPSS_PROC 0
> +					 &config_noc SLAVE_VENUS_CFG 0>;
> +			interconnect-names = "video-mem",
> +					     "cpu-cfg";
> +
> +			iommus = <&apps_smmu 0xe40 0x20>;
> +
> +			memory-region = <&pil_video_mem>;
> +
> +			status = "disabled";
> +
> +			video-decoder {
> +				compatible = "venus-decoder";
> +			};
> +
> +			video-encoder {
> +				compatible = "venus-encoder";
> +			};
> +
> +			venus_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				opp-133330000 {
> +					opp-hz = /bits/ 64 <133330000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +				};
> +
> +				opp-240000000 {
> +					opp-hz = /bits/ 64 <240000000>;
> +					required-opps = <&rpmhpd_opp_svs>;
> +				};
> +
> +				opp-300000000 {
> +					opp-hz = /bits/ 64 <300000000>;
> +					required-opps = <&rpmhpd_opp_svs_l1>;
> +				};
> +
> +				opp-380000000 {
> +					opp-hz = /bits/ 64 <380000000>;
> +					required-opps = <&rpmhpd_opp_nom>;
> +				};
> +
> +				opp-410000000 {
> +					opp-hz = /bits/ 64 <410000000>;
> +					required-opps = <&rpmhpd_opp_turbo>;
> +				};
> +
> +				opp-460000000 {
> +					opp-hz = /bits/ 64 <460000000>;
> +					required-opps = <&rpmhpd_opp_turbo_l1>;
> +				};
> +			};
> +		};
> +
>  		videocc: clock-controller@ab00000 {
>  			compatible = "qcom,qcs615-videocc";
>  			reg = <0 0xab00000 0 0x10000>;
> 
> -- 
> 2.34.1

RE: [PATCH v3 4/4] arm64: dts: qcom: qcs615-ride: enable venus node

[Renjiang Han (QUIC)]

On Monday, November 25, 2024 1:35 PM, Renjiang Han wrote:
> Enable the venus node so that the video codec will start working.

Forgot to add Reviewed-by, next version will add Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs615-ride.dts | 4 ++++
>  1 file changed, 4 insertions(+)

> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> index 4ef969a6af150933c72a7a83374a5a2657eebc1b..4a7bfffb7b86b21c412b16426019c9063368ca19 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> @@ -217,6 +217,10 @@ &uart0 {
>  	status = "okay";
>  };
 
> +&venus {
> +	status = "okay";
> +};
> +
>  &watchdog {
>  	clocks = <&sleep_clk>;
>  };

> -- 
> 2.34.1

Re: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Krzysztof Kozlowski]

On 25/11/2024 09:44, Renjiang Han (QUIC) wrote:
> On Monday, November 25, 2024 1:35 PM, Renjiang Han wrote:
>> Initialize the platform data and enable venus driver probe of QCS615 SoC.
> 
> Forgot to add Reviewed-by, next version will add Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Please start using b4. You cannot add other people's tag this way - via
reply to email.

Best regards,
Krzysztof

RE: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Renjiang Han (QUIC)]

On Monday, November 25, 2024 4:57 PM, Krzysztof Kozlowski wrote:
> On 25/11/2024 09:44, Renjiang Han (QUIC) wrote:
> > On Monday, November 25, 2024 1:35 PM, Renjiang Han wrote:
> > > Initialize the platform data and enable venus driver probe of QCS615 SoC.
> > 
> > Forgot to add Reviewed-by, next version will add Reviewed-by: Bryan 
> > O'Donoghue <bryan.odonoghue@linaro.org>

> Please start using b4. You cannot add other people's tag this way - via reply to email.
Thanks for your comment. Next version will be updated with b4.

> Best regards,
> Krzysztof

Re: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Dmitry Baryshkov]

On Mon, Nov 25, 2024 at 11:04:50AM +0530, Renjiang Han wrote:
> Initialize the platform data and enable venus driver probe of QCS615
> SoC.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  drivers/media/platform/qcom/venus/core.c | 50 ++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index 423deb5e94dcb193974da23f9bd2d905bfeab2d9..39d8bcf62fe4f72674746b75994cce6cbaee94eb 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -630,6 +630,55 @@ static const struct venus_resources msm8998_res = {
>  	.fwname = "qcom/venus-4.4/venus.mbn",
>  };
>  
> +static const struct freq_tbl qcs615_freq_table[] = {
> +	{ 0, 460000000 },
> +	{ 0, 410000000 },
> +	{ 0, 380000000 },
> +	{ 0, 300000000 },
> +	{ 0, 240000000 },
> +	{ 0, 133333333 },
> +};
> +
> +static const struct bw_tbl qcs615_bw_table_enc[] = {
> +	{  972000,  951000, 0, 1434000, 0 },	/* 3840x2160@30 */
> +	{  489600,  723000, 0,  973000, 0 },	/* 1920x1080@60 */
> +	{  244800,  370000, 0,	495000, 0 },	/* 1920x1080@30 */
> +};
> +
> +static const struct bw_tbl qcs615_bw_table_dec[] = {
> +	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
> +	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
> +	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
> +};
> +
> +static const struct venus_resources qcs615_res = {
> +	.freq_tbl = qcs615_freq_table,
> +	.freq_tbl_size = ARRAY_SIZE(qcs615_freq_table),
> +	.bw_tbl_enc = qcs615_bw_table_enc,
> +	.bw_tbl_enc_size = ARRAY_SIZE(qcs615_bw_table_enc),
> +	.bw_tbl_dec = qcs615_bw_table_dec,
> +	.bw_tbl_dec_size = ARRAY_SIZE(qcs615_bw_table_dec),
> +	.clks = {"core", "iface", "bus" },
> +	.clks_num = 3,
> +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
> +	.vcodec_clks_num = 2,
> +	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
> +	.vcodec_pmdomains_num = 2,
> +	.opp_pmdomain = (const char *[]) { "cx" },
> +	.vcodec_num = 1,
> +	.hfi_version = HFI_VERSION_4XX,
> +	.vpu_version = VPU_VERSION_AR50,
> +	.vmem_id = VIDC_RESOURCE_NONE,
> +	.vmem_size = 0,
> +	.vmem_addr = 0,
> +	.dma_mask = 0xe0000000 - 1,
> +	.cp_start = 0,
> +	.cp_size = 0x70800000,
> +	.cp_nonpixel_start = 0x1000000,
> +	.cp_nonpixel_size = 0x24800000,
> +	.fwname = "qcom/venus-5.4/venus_s6.mbn",

I really want the firmware discussion of linux-firmware to be solved
first, before we land this patch.

SHort summary: can we use a single image for all 5.4 platforms (by using
v5 signatures, by using v6 signatures, v3 or any other kind of quirk).

> +};
> +
>  static const struct freq_tbl sdm660_freq_table[] = {
>  	{ 979200, 518400000 },
>  	{ 489600, 441600000 },
> @@ -937,6 +986,7 @@ 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,msm8998-venus", .data = &msm8998_res, },
> +	{ .compatible = "qcom,qcs615-venus", .data = &qcs615_res, },

The hardware seems to be the same as sc7180, only the frequencies
differ. Can we change the driver in a way that we don't have to add
another compat entry just for the sake of changing freqs / bandwidths?

>  	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
>  	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
>  	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

RE: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Renjiang Han (QUIC)]

On Monday, November 25, 2024 9:36 PM, Dmitry Baryshkov wrote:
> On Mon, Nov 25, 2024 at 11:04:50AM +0530, Renjiang Han wrote:
> > Initialize the platform data and enable venus driver probe of QCS615 
> > SoC.
> > 
> > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >
> > ---
> >  drivers/media/platform/qcom/venus/core.c | 50 
> > ++++++++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> > 
> > diff --git a/drivers/media/platform/qcom/venus/core.c 
> > b/drivers/media/platform/qcom/venus/core.c
> > index 
> > 423deb5e94dcb193974da23f9bd2d905bfeab2d9..39d8bcf62fe4f72674746b75994c
> > ce6cbaee94eb 100644
> > --- a/drivers/media/platform/qcom/venus/core.c
> > +++ b/drivers/media/platform/qcom/venus/core.c
> > @@ -630,6 +630,55 @@ static const struct venus_resources msm8998_res = {
> >  	.fwname = "qcom/venus-4.4/venus.mbn",  };
> >  
> > +static const struct freq_tbl qcs615_freq_table[] = {
> > +	{ 0, 460000000 },
> > +	{ 0, 410000000 },
> > +	{ 0, 380000000 },
> > +	{ 0, 300000000 },
> > +	{ 0, 240000000 },
> > +	{ 0, 133333333 },
> > +};
> > +
> > +static const struct bw_tbl qcs615_bw_table_enc[] = {
> > +	{  972000,  951000, 0, 1434000, 0 },	/* 3840x2160@30 */
> > +	{  489600,  723000, 0,  973000, 0 },	/* 1920x1080@60 */
> > +	{  244800,  370000, 0,	495000, 0 },	/* 1920x1080@30 */
> > +};
> > +
> > +static const struct bw_tbl qcs615_bw_table_dec[] = {
> > +	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
> > +	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
> > +	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
> > +};
> > +
> > +static const struct venus_resources qcs615_res = {
> > +	.freq_tbl = qcs615_freq_table,
> > +	.freq_tbl_size = ARRAY_SIZE(qcs615_freq_table),
> > +	.bw_tbl_enc = qcs615_bw_table_enc,
> > +	.bw_tbl_enc_size = ARRAY_SIZE(qcs615_bw_table_enc),
> > +	.bw_tbl_dec = qcs615_bw_table_dec,
> > +	.bw_tbl_dec_size = ARRAY_SIZE(qcs615_bw_table_dec),
> > +	.clks = {"core", "iface", "bus" },
> > +	.clks_num = 3,
> > +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
> > +	.vcodec_clks_num = 2,
> > +	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
> > +	.vcodec_pmdomains_num = 2,
> > +	.opp_pmdomain = (const char *[]) { "cx" },
> > +	.vcodec_num = 1,
> > +	.hfi_version = HFI_VERSION_4XX,
> > +	.vpu_version = VPU_VERSION_AR50,
> > +	.vmem_id = VIDC_RESOURCE_NONE,
> > +	.vmem_size = 0,
> > +	.vmem_addr = 0,
> > +	.dma_mask = 0xe0000000 - 1,
> > +	.cp_start = 0,
> > +	.cp_size = 0x70800000,
> > +	.cp_nonpixel_start = 0x1000000,
> > +	.cp_nonpixel_size = 0x24800000,
> > +	.fwname = "qcom/venus-5.4/venus_s6.mbn",

> I really want the firmware discussion of linux-firmware to be solved first,
> before we land this patch.

> SHort summary: can we use a single image for all 5.4 platforms (by using
> v5 signatures, by using v6 signatures, v3 or any other kind of quirk).
Thanks for your comment. We have discussed with the firmware team and
other teams if we can use the same firmware binary. The result is we'd better
use different firmware files. They should respond in the firmware binary
thread. I will push them and hope them respond as quickly as possible and
give reasons.
> > +};
> > +
> >  static const struct freq_tbl sdm660_freq_table[] = {
> >  	{ 979200, 518400000 },
> >  	{ 489600, 441600000 },
> > @@ -937,6 +986,7 @@ 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,msm8998-venus", .data = &msm8998_res, },
> > +	{ .compatible = "qcom,qcs615-venus", .data = &qcs615_res, },

> The hardware seems to be the same as sc7180, only the frequencies differ.
> Can we change the driver in a way that we don't have to add another
> compat entry just for the sake of changing freqs / bandwidths?

Thank you for your comment. I agree with you. But based on the Venus code
architecture and the distinction between different platforms, I think the
current changes are the simplest.

> >  	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
> >  	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
> >  	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
> > 
> > --
> > 2.34.1
> > 

> -- 
> With best wishes
> Dmitry

RE: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Renjiang Han (QUIC)]

On Mon 11/25/2024 3:50 PM, Krzysztof Kozlowski wrote:
> On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote:
> > Add support for Qualcomm video acceleration hardware used for video 
> > stream decoding and encoding on QCOM QCS615.
> > 
> > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >
> > ---
> >  .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
> >  1 file changed, 182 insertions(+)
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml 
> > b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml

> Dependency for this patch must be mentioned here.

> Amount of dependencies make it unmergeable and untesteable.
> I suggest decoupling dependencies by removing clock constants.

Thanks for your comment, I will try to remove the clock constants
and use clock id instead.

> > new file mode 100644
> > index 
> > 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a2485
> > 7c86c6865f89
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
> > @@ -0,0 +1,182 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm QCS615 Venus video encode and decode accelerators
> > +
> > +maintainers:
> > +  - Stanimir Varbanov <stanimir.k.varbanov@gmail.com> >
> > +  - Vikash Garodia <quic_vgarodia@quicinc.com> >
> > +
> > +description:
> > +  The Venus IP is a video encode and decode accelerator present
> > +  on Qualcomm platforms
> > +
> > +allOf:
> > +  - $ref: qcom,venus-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,qcs615-venus
> > +
> > +  power-domains:
> > +    minItems: 2
> > +    maxItems: 3
> > +
> > +  power-domain-names:
> > +    minItems: 2
> > +    items:
> > +      - const: venus
> > +      - const: vcodec0
> > +      - const: cx
> > +
> > +  clocks:
> > +    maxItems: 5
> > +
> > +  clock-names:
> > +    items:
> > +      - const: core
> > +      - const: iface
> > +      - const: bus
> > +      - const: vcodec0_core
> > +      - const: vcodec0_bus
> > +
> > +  iommus:
> > +    maxItems: 1
> > +
> > +  memory-region:
> > +    maxItems: 1
> > +
> > +  interconnects:
> > +    maxItems: 2
> > +
> > +  interconnect-names:
> > +    items:
> > +      - const: video-mem
> > +      - const: cpu-cfg
> > +
> > +  operating-points-v2: true
> > +
> > +  opp-table:
> > +    type: object
> > +
> > +  video-decoder:
> > +    type: object
> > +
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      compatible:
> > +        const: venus-decoder
> > +
> > +    required:
> > +      - compatible
> > +
> > +  video-encoder:
> > +    type: object

> Both nodes are useless - no resources here, nothing to control.
> Do not add nodes just to instantiate Linux drivers. Drop them.
Do you mean I should remove video-decoder and video-encoder from here?
If so, do I also need to remove these two nodes from the dtsi file and add
them in the qcs615-ride.dts file?

> Best regards,
> Krzysztof

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Krzysztof Kozlowski]

On 25/11/2024 16:49, Renjiang Han (QUIC) wrote:
>>> +  video-decoder:
>>> +    type: object
>>> +
>>> +    additionalProperties: false
>>> +
>>> +    properties:
>>> +      compatible:
>>> +        const: venus-decoder
>>> +
>>> +    required:
>>> +      - compatible
>>> +
>>> +  video-encoder:
>>> +    type: object
> 
>> Both nodes are useless - no resources here, nothing to control.
>> Do not add nodes just to instantiate Linux drivers. Drop them.
> Do you mean I should remove video-decoder and video-encoder from here?

Yes, that's my suggestion.

> If so, do I also need to remove these two nodes from the dtsi file and add

Yes

> them in the qcs615-ride.dts file?

Well, no, how would it pass dtbs_check?

Don't add nodes purely for Linux driver instantiation.

Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Dmitry Baryshkov]

On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote:
> Add support for Qualcomm video acceleration hardware used for video
> stream decoding and encoding on QCOM QCS615.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
>  1 file changed, 182 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
> @@ -0,0 +1,182 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm QCS615 Venus video encode and decode accelerators
> +
> +maintainers:
> +  - Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
> +  - Vikash Garodia <quic_vgarodia@quicinc.com>
> +
> +description:
> +  The Venus IP is a video encode and decode accelerator present
> +  on Qualcomm platforms
> +
> +allOf:
> +  - $ref: qcom,venus-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,qcs615-venus

Please extend sc7180-venus.yaml instead. No need to duplicate
unnecessary things.

> +
> +  power-domains:
> +    minItems: 2
> +    maxItems: 3

So, is it 2 or 3? You don't have legacy here, so you should know an
exact number.

> +
> +  power-domain-names:
> +    minItems: 2

And this one also can go away.

> +    items:
> +      - const: venus
> +      - const: vcodec0
> +      - const: cx
> +
> +  clocks:
> +    maxItems: 5
> +
> +  clock-names:
> +    items:
> +      - const: core
> +      - const: iface
> +      - const: bus
> +      - const: vcodec0_core
> +      - const: vcodec0_bus
> +
> +  iommus:
> +    maxItems: 1
> +
> +  memory-region:
> +    maxItems: 1
> +
> +  interconnects:
> +    maxItems: 2
> +
> +  interconnect-names:
> +    items:
> +      - const: video-mem
> +      - const: cpu-cfg
> +
> +  operating-points-v2: true
> +
> +  opp-table:
> +    type: object
> +
> +  video-decoder:
> +    type: object
> +
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        const: venus-decoder
> +
> +    required:
> +      - compatible
> +
> +  video-encoder:
> +    type: object
> +
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        const: venus-encoder
> +
> +    required:
> +      - compatible
> +
> +required:
> +  - compatible
> +  - power-domain-names
> +  - iommus
> +  - video-decoder
> +  - video-encoder
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,qcs615-videocc.h>
> +    #include <dt-bindings/interconnect/qcom,qcs615-rpmh.h>
> +    #include <dt-bindings/power/qcom,rpmhpd.h>
> +
> +    venus: video-codec@aa00000 {
> +        compatible = "qcom,qcs615-venus";
> +        reg = <0xaa00000 0x100000>;
> +        interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        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";
> +
> +        power-domains = <&videocc VENUS_GDSC>,
> +                        <&videocc VCODEC0_GDSC>,
> +                        <&rpmhpd RPMHPD_CX>;
> +        power-domain-names = "venus",
> +                             "vcodec0",
> +                             "cx";
> +
> +        operating-points-v2 = <&venus_opp_table>;
> +
> +        interconnects = <&mmss_noc MASTER_VIDEO_P0 0
> +                         &mc_virt SLAVE_EBI1 0>,
> +                        <&gem_noc MASTER_APPSS_PROC 0
> +                         &config_noc SLAVE_VENUS_CFG 0>;
> +        interconnect-names = "video-mem",
> +                             "cpu-cfg";
> +
> +        iommus = <&apps_smmu 0xe40 0x20>;
> +
> +        memory-region = <&pil_video_mem>;
> +
> +        video-decoder {
> +            compatible = "venus-decoder";
> +        };
> +
> +        video-encoder {
> +            compatible = "venus-encoder";
> +        };
> +
> +        venus_opp_table: opp-table {
> +            compatible = "operating-points-v2";
> +
> +            opp-133330000 {
> +                opp-hz = /bits/ 64 <133330000>;
> +                required-opps = <&rpmhpd_opp_low_svs>;
> +            };
> +
> +            opp-240000000 {
> +                opp-hz = /bits/ 64 <240000000>;
> +                required-opps = <&rpmhpd_opp_svs>;
> +            };
> +
> +            opp-300000000 {
> +                opp-hz = /bits/ 64 <300000000>;
> +                required-opps = <&rpmhpd_opp_svs_l1>;
> +            };
> +
> +            opp-380000000 {
> +                opp-hz = /bits/ 64 <380000000>;
> +                required-opps = <&rpmhpd_opp_nom>;
> +            };
> +
> +            opp-410000000 {
> +                opp-hz = /bits/ 64 <410000000>;
> +                required-opps = <&rpmhpd_opp_turbo>;
> +            };
> +
> +            opp-460000000 {
> +                opp-hz = /bits/ 64 <460000000>;
> +                required-opps = <&rpmhpd_opp_turbo_l1>;
> +            };
> +        };
> +    };
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Dmitry Baryshkov]

On Mon, Nov 25, 2024 at 03:34:19PM +0000, Renjiang Han (QUIC) wrote:
> On Monday, November 25, 2024 9:36 PM, Dmitry Baryshkov wrote:
> > On Mon, Nov 25, 2024 at 11:04:50AM +0530, Renjiang Han wrote:
> > > Initialize the platform data and enable venus driver probe of QCS615 
> > > SoC.
> > > 
> > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >
> > > ---
> > >  drivers/media/platform/qcom/venus/core.c | 50 
> > > ++++++++++++++++++++++++++++++++
> > >  1 file changed, 50 insertions(+)
> > > 
> > > diff --git a/drivers/media/platform/qcom/venus/core.c 
> > > b/drivers/media/platform/qcom/venus/core.c
> > > index 
> > > 423deb5e94dcb193974da23f9bd2d905bfeab2d9..39d8bcf62fe4f72674746b75994c
> > > ce6cbaee94eb 100644
> > > --- a/drivers/media/platform/qcom/venus/core.c
> > > +++ b/drivers/media/platform/qcom/venus/core.c
> > > @@ -630,6 +630,55 @@ static const struct venus_resources msm8998_res = {
> > >  	.fwname = "qcom/venus-4.4/venus.mbn",  };
> > >  
> > > +static const struct freq_tbl qcs615_freq_table[] = {
> > > +	{ 0, 460000000 },
> > > +	{ 0, 410000000 },
> > > +	{ 0, 380000000 },
> > > +	{ 0, 300000000 },
> > > +	{ 0, 240000000 },
> > > +	{ 0, 133333333 },
> > > +};
> > > +
> > > +static const struct bw_tbl qcs615_bw_table_enc[] = {
> > > +	{  972000,  951000, 0, 1434000, 0 },	/* 3840x2160@30 */
> > > +	{  489600,  723000, 0,  973000, 0 },	/* 1920x1080@60 */
> > > +	{  244800,  370000, 0,	495000, 0 },	/* 1920x1080@30 */
> > > +};
> > > +
> > > +static const struct bw_tbl qcs615_bw_table_dec[] = {
> > > +	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
> > > +	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
> > > +	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
> > > +};
> > > +
> > > +static const struct venus_resources qcs615_res = {
> > > +	.freq_tbl = qcs615_freq_table,
> > > +	.freq_tbl_size = ARRAY_SIZE(qcs615_freq_table),
> > > +	.bw_tbl_enc = qcs615_bw_table_enc,
> > > +	.bw_tbl_enc_size = ARRAY_SIZE(qcs615_bw_table_enc),
> > > +	.bw_tbl_dec = qcs615_bw_table_dec,
> > > +	.bw_tbl_dec_size = ARRAY_SIZE(qcs615_bw_table_dec),
> > > +	.clks = {"core", "iface", "bus" },
> > > +	.clks_num = 3,
> > > +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
> > > +	.vcodec_clks_num = 2,
> > > +	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
> > > +	.vcodec_pmdomains_num = 2,
> > > +	.opp_pmdomain = (const char *[]) { "cx" },
> > > +	.vcodec_num = 1,
> > > +	.hfi_version = HFI_VERSION_4XX,
> > > +	.vpu_version = VPU_VERSION_AR50,
> > > +	.vmem_id = VIDC_RESOURCE_NONE,
> > > +	.vmem_size = 0,
> > > +	.vmem_addr = 0,
> > > +	.dma_mask = 0xe0000000 - 1,
> > > +	.cp_start = 0,
> > > +	.cp_size = 0x70800000,
> > > +	.cp_nonpixel_start = 0x1000000,
> > > +	.cp_nonpixel_size = 0x24800000,
> > > +	.fwname = "qcom/venus-5.4/venus_s6.mbn",
> 
> > I really want the firmware discussion of linux-firmware to be solved first,
> > before we land this patch.
> 
> > SHort summary: can we use a single image for all 5.4 platforms (by using
> > v5 signatures, by using v6 signatures, v3 or any other kind of quirk).
> Thanks for your comment. We have discussed with the firmware team and
> other teams if we can use the same firmware binary. The result is we'd better
> use different firmware files. They should respond in the firmware binary
> thread. I will push them and hope them respond as quickly as possible and
> give reasons.
> > > +};
> > > +
> > >  static const struct freq_tbl sdm660_freq_table[] = {
> > >  	{ 979200, 518400000 },
> > >  	{ 489600, 441600000 },
> > > @@ -937,6 +986,7 @@ 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,msm8998-venus", .data = &msm8998_res, },
> > > +	{ .compatible = "qcom,qcs615-venus", .data = &qcs615_res, },
> 
> > The hardware seems to be the same as sc7180, only the frequencies differ.
> > Can we change the driver in a way that we don't have to add another
> > compat entry just for the sake of changing freqs / bandwidths?
> 
> Thank you for your comment. I agree with you. But based on the Venus code
> architecturE ANd the distinction between different platforms, I think the
> current changes are the simplest.

Well, it is simplest, correct. But not the best one. There is no plan no
migrate these platforms to the iris driver. So instead, please improve
the venus driver instead of just pushing the simplest change. I should
have been more explicit about it earlier.

> 
> > >  	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
> > >  	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
> > >  	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
> > > 
> > > --
> > > 2.34.1
> > > 
> 
> > -- 
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Renjiang Han]

On 11/25/2024 11:55 PM, Krzysztof Kozlowski wrote:
> On 25/11/2024 16:49, Renjiang Han (QUIC) wrote:
>>>> +  video-decoder:
>>>> +    type: object
>>>> +
>>>> +    additionalProperties: false
>>>> +
>>>> +    properties:
>>>> +      compatible:
>>>> +        const: venus-decoder
>>>> +
>>>> +    required:
>>>> +      - compatible
>>>> +
>>>> +  video-encoder:
>>>> +    type: object
>>> Both nodes are useless - no resources here, nothing to control.
>>> Do not add nodes just to instantiate Linux drivers. Drop them.
>> Do you mean I should remove video-decoder and video-encoder from here?
> Yes, that's my suggestion.
>
>> If so, do I also need to remove these two nodes from the dtsi file and add
> Yes
>
>> them in the qcs615-ride.dts file?
> Well, no, how would it pass dtbs_check?
>
> Don't add nodes purely for Linux driver instantiation.
OK, I got it. I'll update like this. If video-decoder and video-encoder are

removed from dtsi file and not added to qcs615-ride.dts file, then the

video decoder and encoder functions will not be available on the qcs615

platform. So I think these two nodes should be added to the

qcs615-ride.dts file to ensure that the qcs615 platform can enable the

video decoder and encoder functions.
> Best regards,
> Krzysztof

-- 
Best Regards,
Renjiang

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Krzysztof Kozlowski]

On 26/11/2024 07:07, Renjiang Han wrote:
> 
> On 11/25/2024 11:55 PM, Krzysztof Kozlowski wrote:
>> On 25/11/2024 16:49, Renjiang Han (QUIC) wrote:
>>>>> +  video-decoder:
>>>>> +    type: object
>>>>> +
>>>>> +    additionalProperties: false
>>>>> +
>>>>> +    properties:
>>>>> +      compatible:
>>>>> +        const: venus-decoder
>>>>> +
>>>>> +    required:
>>>>> +      - compatible
>>>>> +
>>>>> +  video-encoder:
>>>>> +    type: object
>>>> Both nodes are useless - no resources here, nothing to control.
>>>> Do not add nodes just to instantiate Linux drivers. Drop them.
>>> Do you mean I should remove video-decoder and video-encoder from here?
>> Yes, that's my suggestion.
>>
>>> If so, do I also need to remove these two nodes from the dtsi file and add
>> Yes
>>
>>> them in the qcs615-ride.dts file?
>> Well, no, how would it pass dtbs_check?
>>
>> Don't add nodes purely for Linux driver instantiation.
> OK, I got it. I'll update like this. If video-decoder and video-encoder are
> 
> removed from dtsi file and not added to qcs615-ride.dts file, then the
> 
> video decoder and encoder functions will not be available on the qcs615
> 
> platform. So I think these two nodes should be added to the
> 
> qcs615-ride.dts file to ensure that the qcs615 platform can enable the
> 
> video decoder and encoder functions.
You just repeated the same sentences. Address my comment instead - empty
device nodes should not be used just to instantiate Linux device drivers.

Best regards,
Krzysztof

Re: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Renjiang Han]

On 11/26/2024 12:20 AM, Dmitry Baryshkov wrote:
> On Mon, Nov 25, 2024 at 03:34:19PM +0000, Renjiang Han (QUIC) wrote:
>> On Monday, November 25, 2024 9:36 PM, Dmitry Baryshkov wrote:
>>> On Mon, Nov 25, 2024 at 11:04:50AM +0530, Renjiang Han wrote:
>>>> Initialize the platform data and enable venus driver probe of QCS615
>>>> SoC.
>>>>
>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >
>>>> ---
>>>>   drivers/media/platform/qcom/venus/core.c | 50
>>>> ++++++++++++++++++++++++++++++++
>>>>   1 file changed, 50 insertions(+)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/venus/core.c
>>>> b/drivers/media/platform/qcom/venus/core.c
>>>> index
>>>> 423deb5e94dcb193974da23f9bd2d905bfeab2d9..39d8bcf62fe4f72674746b75994c
>>>> ce6cbaee94eb 100644
>>>> --- a/drivers/media/platform/qcom/venus/core.c
>>>> +++ b/drivers/media/platform/qcom/venus/core.c
>>>> @@ -630,6 +630,55 @@ static const struct venus_resources msm8998_res = {
>>>>   	.fwname = "qcom/venus-4.4/venus.mbn",  };
>>>>   
>>>> +static const struct freq_tbl qcs615_freq_table[] = {
>>>> +	{ 0, 460000000 },
>>>> +	{ 0, 410000000 },
>>>> +	{ 0, 380000000 },
>>>> +	{ 0, 300000000 },
>>>> +	{ 0, 240000000 },
>>>> +	{ 0, 133333333 },
>>>> +};
>>>> +
>>>> +static const struct bw_tbl qcs615_bw_table_enc[] = {
>>>> +	{  972000,  951000, 0, 1434000, 0 },	/* 3840x2160@30 */
>>>> +	{  489600,  723000, 0,  973000, 0 },	/* 1920x1080@60 */
>>>> +	{  244800,  370000, 0,	495000, 0 },	/* 1920x1080@30 */
>>>> +};
>>>> +
>>>> +static const struct bw_tbl qcs615_bw_table_dec[] = {
>>>> +	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
>>>> +	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
>>>> +	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
>>>> +};
>>>> +
>>>> +static const struct venus_resources qcs615_res = {
>>>> +	.freq_tbl = qcs615_freq_table,
>>>> +	.freq_tbl_size = ARRAY_SIZE(qcs615_freq_table),
>>>> +	.bw_tbl_enc = qcs615_bw_table_enc,
>>>> +	.bw_tbl_enc_size = ARRAY_SIZE(qcs615_bw_table_enc),
>>>> +	.bw_tbl_dec = qcs615_bw_table_dec,
>>>> +	.bw_tbl_dec_size = ARRAY_SIZE(qcs615_bw_table_dec),
>>>> +	.clks = {"core", "iface", "bus" },
>>>> +	.clks_num = 3,
>>>> +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
>>>> +	.vcodec_clks_num = 2,
>>>> +	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
>>>> +	.vcodec_pmdomains_num = 2,
>>>> +	.opp_pmdomain = (const char *[]) { "cx" },
>>>> +	.vcodec_num = 1,
>>>> +	.hfi_version = HFI_VERSION_4XX,
>>>> +	.vpu_version = VPU_VERSION_AR50,
>>>> +	.vmem_id = VIDC_RESOURCE_NONE,
>>>> +	.vmem_size = 0,
>>>> +	.vmem_addr = 0,
>>>> +	.dma_mask = 0xe0000000 - 1,
>>>> +	.cp_start = 0,
>>>> +	.cp_size = 0x70800000,
>>>> +	.cp_nonpixel_start = 0x1000000,
>>>> +	.cp_nonpixel_size = 0x24800000,
>>>> +	.fwname = "qcom/venus-5.4/venus_s6.mbn",
>>> I really want the firmware discussion of linux-firmware to be solved first,
>>> before we land this patch.
>>> SHort summary: can we use a single image for all 5.4 platforms (by using
>>> v5 signatures, by using v6 signatures, v3 or any other kind of quirk).
>> Thanks for your comment. We have discussed with the firmware team and
>> other teams if we can use the same firmware binary. The result is we'd better
>> use different firmware files. They should respond in the firmware binary
>> thread. I will push them and hope them respond as quickly as possible and
>> give reasons.
>>>> +};
>>>> +
>>>>   static const struct freq_tbl sdm660_freq_table[] = {
>>>>   	{ 979200, 518400000 },
>>>>   	{ 489600, 441600000 },
>>>> @@ -937,6 +986,7 @@ 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,msm8998-venus", .data = &msm8998_res, },
>>>> +	{ .compatible = "qcom,qcs615-venus", .data = &qcs615_res, },
>>> The hardware seems to be the same as sc7180, only the frequencies differ.
>>> Can we change the driver in a way that we don't have to add another
>>> compat entry just for the sake of changing freqs / bandwidths?
>> Thank you for your comment. I agree with you. But based on the Venus code
>> architecturE ANd the distinction between different platforms, I think the
>> current changes are the simplest.
> Well, it is simplest, correct. But not the best one. There is no plan no
> migrate these platforms to the iris driver. So instead, please improve
> the venus driver instead of just pushing the simplest change. I should
> have been more explicit about it earlier.

Based on the current code architecture, I don't know if there is a 
better way. If we

refactor the code, it will take a lot of effort.

Therefore, I submit this change. Do you have a better approach?

Also, the driver architecture of iris is implemented as you said.

>>>>   	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
>>>>   	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
>>>>   	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
>>>>
>>>> --
>>>> 2.34.1
>>>>
>>> -- 
>>> With best wishes
>>> Dmitry

-- 
Best Regards,
Renjiang

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Renjiang Han]

On 11/26/2024 12:12 AM, Dmitry Baryshkov wrote:
> On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote:
>> Add support for Qualcomm video acceleration hardware used for video
>> stream decoding and encoding on QCOM QCS615.
>>
>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>> ---
>>   .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
>>   1 file changed, 182 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
>> @@ -0,0 +1,182 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm QCS615 Venus video encode and decode accelerators
>> +
>> +maintainers:
>> +  - Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
>> +  - Vikash Garodia <quic_vgarodia@quicinc.com>
>> +
>> +description:
>> +  The Venus IP is a video encode and decode accelerator present
>> +  on Qualcomm platforms
>> +
>> +allOf:
>> +  - $ref: qcom,venus-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,qcs615-venus
> Please extend sc7180-venus.yaml instead. No need to duplicate
> unnecessary things.

Thanks for your review. But I'm sorry I can't get it. The devicetree for

qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml

instead.

>> +
>> +  power-domains:
>> +    minItems: 2
>> +    maxItems: 3
> So, is it 2 or 3? You don't have legacy here, so you should know an
> exact number.
Got it. Both minItems and maxItems should be 3. I'll update with next 
version.
>
>> +
>> +  power-domain-names:
>> +    minItems: 2
> And this one also can go away.
OK. Thanks for pointing it out. I'll remove this minItems with next 
version.
>> +    items:
>> +      - const: venus
>> +      - const: vcodec0
>> +      - const: cx
>> +
>> +  clocks:
>> +    maxItems: 5
>> +
>> +  clock-names:
>> +    items:
>> +      - const: core
>> +      - const: iface
>> +      - const: bus
>> +      - const: vcodec0_core
>> +      - const: vcodec0_bus
>> +
>> +  iommus:
>> +    maxItems: 1
>> +
>> +  memory-region:
>> +    maxItems: 1
>> +
>> +  interconnects:
>> +    maxItems: 2
>> +
>> +  interconnect-names:
>> +    items:
>> +      - const: video-mem
>> +      - const: cpu-cfg
>> +
>> +  operating-points-v2: true
>> +
>> +  opp-table:
>> +    type: object
>> +
>> +  video-decoder:
>> +    type: object
>> +
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        const: venus-decoder
>> +
>> +    required:
>> +      - compatible
>> +
>> +  video-encoder:
>> +    type: object
>> +
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        const: venus-encoder
>> +
>> +    required:
>> +      - compatible
>> +
>> +required:
>> +  - compatible
>> +  - power-domain-names
>> +  - iommus
>> +  - video-decoder
>> +  - video-encoder
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/clock/qcom,qcs615-videocc.h>
>> +    #include <dt-bindings/interconnect/qcom,qcs615-rpmh.h>
>> +    #include <dt-bindings/power/qcom,rpmhpd.h>
>> +
>> +    venus: video-codec@aa00000 {
>> +        compatible = "qcom,qcs615-venus";
>> +        reg = <0xaa00000 0x100000>;
>> +        interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +        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";
>> +
>> +        power-domains = <&videocc VENUS_GDSC>,
>> +                        <&videocc VCODEC0_GDSC>,
>> +                        <&rpmhpd RPMHPD_CX>;
>> +        power-domain-names = "venus",
>> +                             "vcodec0",
>> +                             "cx";
>> +
>> +        operating-points-v2 = <&venus_opp_table>;
>> +
>> +        interconnects = <&mmss_noc MASTER_VIDEO_P0 0
>> +                         &mc_virt SLAVE_EBI1 0>,
>> +                        <&gem_noc MASTER_APPSS_PROC 0
>> +                         &config_noc SLAVE_VENUS_CFG 0>;
>> +        interconnect-names = "video-mem",
>> +                             "cpu-cfg";
>> +
>> +        iommus = <&apps_smmu 0xe40 0x20>;
>> +
>> +        memory-region = <&pil_video_mem>;
>> +
>> +        video-decoder {
>> +            compatible = "venus-decoder";
>> +        };
>> +
>> +        video-encoder {
>> +            compatible = "venus-encoder";
>> +        };
>> +
>> +        venus_opp_table: opp-table {
>> +            compatible = "operating-points-v2";
>> +
>> +            opp-133330000 {
>> +                opp-hz = /bits/ 64 <133330000>;
>> +                required-opps = <&rpmhpd_opp_low_svs>;
>> +            };
>> +
>> +            opp-240000000 {
>> +                opp-hz = /bits/ 64 <240000000>;
>> +                required-opps = <&rpmhpd_opp_svs>;
>> +            };
>> +
>> +            opp-300000000 {
>> +                opp-hz = /bits/ 64 <300000000>;
>> +                required-opps = <&rpmhpd_opp_svs_l1>;
>> +            };
>> +
>> +            opp-380000000 {
>> +                opp-hz = /bits/ 64 <380000000>;
>> +                required-opps = <&rpmhpd_opp_nom>;
>> +            };
>> +
>> +            opp-410000000 {
>> +                opp-hz = /bits/ 64 <410000000>;
>> +                required-opps = <&rpmhpd_opp_turbo>;
>> +            };
>> +
>> +            opp-460000000 {
>> +                opp-hz = /bits/ 64 <460000000>;
>> +                required-opps = <&rpmhpd_opp_turbo_l1>;
>> +            };
>> +        };
>> +    };
>>
>> -- 
>> 2.34.1
>>
-- 
Best Regards,
Renjiang

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Krzysztof Kozlowski]

On 26/11/2024 09:57, Renjiang Han wrote:
>>> +description:
>>> +  The Venus IP is a video encode and decode accelerator present
>>> +  on Qualcomm platforms
>>> +
>>> +allOf:
>>> +  - $ref: qcom,venus-common.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: qcom,qcs615-venus
>> Please extend sc7180-venus.yaml instead. No need to duplicate
>> unnecessary things.
> 
> Thanks for your review. But I'm sorry I can't get it. The devicetree for
> 
> qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml
> 
> instead.

DTSI is not relevant here to the bindings. I don't understand the
problem, so not sure what you are asking here about.
Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Renjiang Han]

On 11/26/2024 2:42 PM, Krzysztof Kozlowski wrote:
> On 26/11/2024 07:07, Renjiang Han wrote:
>> On 11/25/2024 11:55 PM, Krzysztof Kozlowski wrote:
>>> On 25/11/2024 16:49, Renjiang Han (QUIC) wrote:
>>>>>> +  video-decoder:
>>>>>> +    type: object
>>>>>> +
>>>>>> +    additionalProperties: false
>>>>>> +
>>>>>> +    properties:
>>>>>> +      compatible:
>>>>>> +        const: venus-decoder
>>>>>> +
>>>>>> +    required:
>>>>>> +      - compatible
>>>>>> +
>>>>>> +  video-encoder:
>>>>>> +    type: object
>>>>> Both nodes are useless - no resources here, nothing to control.
>>>>> Do not add nodes just to instantiate Linux drivers. Drop them.
>>>> Do you mean I should remove video-decoder and video-encoder from here?
>>> Yes, that's my suggestion.
>>>
>>>> If so, do I also need to remove these two nodes from the dtsi file and add
>>> Yes
>>>
>>>> them in the qcs615-ride.dts file?
>>> Well, no, how would it pass dtbs_check?
>>>
>>> Don't add nodes purely for Linux driver instantiation.
>> OK, I got it. I'll update like this. If video-decoder and video-encoder are
>>
>> removed from dtsi file and not added to qcs615-ride.dts file, then the
>>
>> video decoder and encoder functions will not be available on the qcs615
>>
>> platform. So I think these two nodes should be added to the
>>
>> qcs615-ride.dts file to ensure that the qcs615 platform can enable the
>>
>> video decoder and encoder functions.
> You just repeated the same sentences. Address my comment instead - empty
> device nodes should not be used just to instantiate Linux device drivers.

Thanks for your reply. I agree with your comment. The two nodes 
video-decoder and

video-encoder should not be placed in the devicetree. But this is 
affected by the venus

driver. On the old platform, some only need to enable the video-decoder 
function or

only enable the video-encoder function. So these two nodes were added to the

devicetree at that time. For new platforms, the iris driver will be used 
in the future,

and this situation will not occur.

> Best regards,
> Krzysztof

-- 
Best Regards,
Renjiang

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Krzysztof Kozlowski]

On 26/11/2024 10:39, Renjiang Han wrote:
>>>>> If so, do I also need to remove these two nodes from the dtsi file and add
>>>> Yes
>>>>
>>>>> them in the qcs615-ride.dts file?
>>>> Well, no, how would it pass dtbs_check?
>>>>
>>>> Don't add nodes purely for Linux driver instantiation.
>>> OK, I got it. I'll update like this. If video-decoder and video-encoder are
>>>
>>> removed from dtsi file and not added to qcs615-ride.dts file, then the
>>>
>>> video decoder and encoder functions will not be available on the qcs615
>>>
>>> platform. So I think these two nodes should be added to the
>>>
>>> qcs615-ride.dts file to ensure that the qcs615 platform can enable the
>>>
>>> video decoder and encoder functions.
>> You just repeated the same sentences. Address my comment instead - empty
>> device nodes should not be used just to instantiate Linux device drivers.
> 
> Thanks for your reply. I agree with your comment. The two nodes 
> video-decoder and
> 
> video-encoder should not be placed in the devicetree. But this is 
> affected by the venus
> 
> driver. On the old platform, some only need to enable the video-decoder 
> function or
> 
> only enable the video-encoder function. So these two nodes were added to the
> 
> devicetree at that time. For new platforms, the iris driver will be used 
> in the future,
> 
> and this situation will not occur.
These are new bindings, for new device, so please fix your driver. We
had similar talk long time ago and answer was that it's a legacy driver
which won't be developed. This means also no new devices. If you bring
new devices to old driver, instead of to new iris, then it means you
still develop old driver. Fix the old driver.


Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Renjiang Han]

On 11/26/2024 5:34 PM, Krzysztof Kozlowski wrote:
> On 26/11/2024 09:57, Renjiang Han wrote:
>>>> +description:
>>>> +  The Venus IP is a video encode and decode accelerator present
>>>> +  on Qualcomm platforms
>>>> +
>>>> +allOf:
>>>> +  - $ref: qcom,venus-common.yaml#
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: qcom,qcs615-venus
>>> Please extend sc7180-venus.yaml instead. No need to duplicate
>>> unnecessary things.
>> Thanks for your review. But I'm sorry I can't get it. The devicetree for
>>
>> qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml
>>
>> instead.
> DTSI is not relevant here to the bindings. I don't understand the
> problem, so not sure what you are asking here about.
The opp-table parameters are different in devicetree. Can we also use 
the same yaml file?
> Best regards,
> Krzysztof

-- 
Best Regards,
Renjiang

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Krzysztof Kozlowski]

On 26/11/2024 10:58, Renjiang Han wrote:
> 
> On 11/26/2024 5:34 PM, Krzysztof Kozlowski wrote:
>> On 26/11/2024 09:57, Renjiang Han wrote:
>>>>> +description:
>>>>> +  The Venus IP is a video encode and decode accelerator present
>>>>> +  on Qualcomm platforms
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: qcom,venus-common.yaml#
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: qcom,qcs615-venus
>>>> Please extend sc7180-venus.yaml instead. No need to duplicate
>>>> unnecessary things.
>>> Thanks for your review. But I'm sorry I can't get it. The devicetree for
>>>
>>> qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml
>>>
>>> instead.
>> DTSI is not relevant here to the bindings. I don't understand the
>> problem, so not sure what you are asking here about.
> The opp-table parameters are different in devicetree. Can we also use 
> the same yaml file?

Please look at existing bindings for other devices.

Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Dmitry Baryshkov]

On Tue, Nov 26, 2024 at 05:58:50PM +0800, Renjiang Han wrote:
> 
> On 11/26/2024 5:34 PM, Krzysztof Kozlowski wrote:
> > On 26/11/2024 09:57, Renjiang Han wrote:
> > > > > +description:
> > > > > +  The Venus IP is a video encode and decode accelerator present
> > > > > +  on Qualcomm platforms
> > > > > +
> > > > > +allOf:
> > > > > +  - $ref: qcom,venus-common.yaml#
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: qcom,qcs615-venus
> > > > Please extend sc7180-venus.yaml instead. No need to duplicate
> > > > unnecessary things.
> > > Thanks for your review. But I'm sorry I can't get it. The devicetree for
> > > 
> > > qcs615-venus is in qcs615.dtsi. I'm not sure how to use sc7180-venus.yaml
> > > 
> > > instead.
> > DTSI is not relevant here to the bindings. I don't understand the
> > problem, so not sure what you are asking here about.
> The opp-table parameters are different in devicetree. Can we also use the
> same yaml file?

Is the contents of the OPP table a part of the bindings?

> > Best regards,
> > Krzysztof
> 
> -- 
> Best Regards,
> Renjiang
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Dmitry Baryshkov]

On Tue, Nov 26, 2024 at 03:40:47PM +0800, Renjiang Han wrote:
> 
> On 11/26/2024 12:20 AM, Dmitry Baryshkov wrote:
> > On Mon, Nov 25, 2024 at 03:34:19PM +0000, Renjiang Han (QUIC) wrote:
> > > On Monday, November 25, 2024 9:36 PM, Dmitry Baryshkov wrote:
> > > > On Mon, Nov 25, 2024 at 11:04:50AM +0530, Renjiang Han wrote:
> > > > > Initialize the platform data and enable venus driver probe of QCS615
> > > > > SoC.
> > > > > 
> > > > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >
> > > > > ---
> > > > >   drivers/media/platform/qcom/venus/core.c | 50
> > > > > ++++++++++++++++++++++++++++++++
> > > > >   1 file changed, 50 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/media/platform/qcom/venus/core.c
> > > > > b/drivers/media/platform/qcom/venus/core.c
> > > > > index
> > > > > 423deb5e94dcb193974da23f9bd2d905bfeab2d9..39d8bcf62fe4f72674746b75994c
> > > > > ce6cbaee94eb 100644
> > > > > --- a/drivers/media/platform/qcom/venus/core.c
> > > > > +++ b/drivers/media/platform/qcom/venus/core.c
> > > > > @@ -630,6 +630,55 @@ static const struct venus_resources msm8998_res = {
> > > > >   	.fwname = "qcom/venus-4.4/venus.mbn",  };
> > > > > +static const struct freq_tbl qcs615_freq_table[] = {
> > > > > +	{ 0, 460000000 },
> > > > > +	{ 0, 410000000 },
> > > > > +	{ 0, 380000000 },
> > > > > +	{ 0, 300000000 },
> > > > > +	{ 0, 240000000 },
> > > > > +	{ 0, 133333333 },
> > > > > +};
> > > > > +
> > > > > +static const struct bw_tbl qcs615_bw_table_enc[] = {
> > > > > +	{  972000,  951000, 0, 1434000, 0 },	/* 3840x2160@30 */
> > > > > +	{  489600,  723000, 0,  973000, 0 },	/* 1920x1080@60 */
> > > > > +	{  244800,  370000, 0,	495000, 0 },	/* 1920x1080@30 */
> > > > > +};
> > > > > +
> > > > > +static const struct bw_tbl qcs615_bw_table_dec[] = {
> > > > > +	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
> > > > > +	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
> > > > > +	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
> > > > > +};
> > > > > +
> > > > > +static const struct venus_resources qcs615_res = {
> > > > > +	.freq_tbl = qcs615_freq_table,
> > > > > +	.freq_tbl_size = ARRAY_SIZE(qcs615_freq_table),
> > > > > +	.bw_tbl_enc = qcs615_bw_table_enc,
> > > > > +	.bw_tbl_enc_size = ARRAY_SIZE(qcs615_bw_table_enc),
> > > > > +	.bw_tbl_dec = qcs615_bw_table_dec,
> > > > > +	.bw_tbl_dec_size = ARRAY_SIZE(qcs615_bw_table_dec),
> > > > > +	.clks = {"core", "iface", "bus" },
> > > > > +	.clks_num = 3,
> > > > > +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
> > > > > +	.vcodec_clks_num = 2,
> > > > > +	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
> > > > > +	.vcodec_pmdomains_num = 2,
> > > > > +	.opp_pmdomain = (const char *[]) { "cx" },
> > > > > +	.vcodec_num = 1,
> > > > > +	.hfi_version = HFI_VERSION_4XX,
> > > > > +	.vpu_version = VPU_VERSION_AR50,
> > > > > +	.vmem_id = VIDC_RESOURCE_NONE,
> > > > > +	.vmem_size = 0,
> > > > > +	.vmem_addr = 0,
> > > > > +	.dma_mask = 0xe0000000 - 1,
> > > > > +	.cp_start = 0,
> > > > > +	.cp_size = 0x70800000,
> > > > > +	.cp_nonpixel_start = 0x1000000,
> > > > > +	.cp_nonpixel_size = 0x24800000,
> > > > > +	.fwname = "qcom/venus-5.4/venus_s6.mbn",
> > > > I really want the firmware discussion of linux-firmware to be solved first,
> > > > before we land this patch.
> > > > SHort summary: can we use a single image for all 5.4 platforms (by using
> > > > v5 signatures, by using v6 signatures, v3 or any other kind of quirk).
> > > Thanks for your comment. We have discussed with the firmware team and
> > > other teams if we can use the same firmware binary. The result is we'd better
> > > use different firmware files. They should respond in the firmware binary
> > > thread. I will push them and hope them respond as quickly as possible and
> > > give reasons.
> > > > > +};
> > > > > +
> > > > >   static const struct freq_tbl sdm660_freq_table[] = {
> > > > >   	{ 979200, 518400000 },
> > > > >   	{ 489600, 441600000 },
> > > > > @@ -937,6 +986,7 @@ 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,msm8998-venus", .data = &msm8998_res, },
> > > > > +	{ .compatible = "qcom,qcs615-venus", .data = &qcs615_res, },
> > > > The hardware seems to be the same as sc7180, only the frequencies differ.
> > > > Can we change the driver in a way that we don't have to add another
> > > > compat entry just for the sake of changing freqs / bandwidths?
> > > Thank you for your comment. I agree with you. But based on the Venus code
> > > architecturE ANd the distinction between different platforms, I think the
> > > current changes are the simplest.
> > Well, it is simplest, correct. But not the best one. There is no plan no
> > migrate these platforms to the iris driver. So instead, please improve
> > the venus driver instead of just pushing the simplest change. I should
> > have been more explicit about it earlier.
> 
> Based on the current code architecture, I don't know if there is a better
> way. If we
> 
> refactor the code, it will take a lot of effort.

Yes, please. The freq_tbl contents is a duplicate of the OPP table in
DT. Drop it from the driver.

> Therefore, I submit this change. Do you have a better approach?

NAK for this submission. Please spend some time and improve the driver
instead.

> 
> Also, the driver architecture of iris is implemented as you said.

Irrelevant. You are patching venus, not iris.

> > > > >   	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
> > > > >   	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
> > > > >   	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
> > > > > 
> > > > > --
> > > > > 2.34.1
> > > > > 
> > > > -- 
> > > > With best wishes
> > > > Dmitry
> 
> -- 
> Best Regards,
> Renjiang
> 

-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] media: venus: core: add qcs615 platform data

[Vikash Garodia]

On 11/26/2024 5:33 PM, Dmitry Baryshkov wrote:
> On Tue, Nov 26, 2024 at 03:40:47PM +0800, Renjiang Han wrote:
>>
>> On 11/26/2024 12:20 AM, Dmitry Baryshkov wrote:
>>> On Mon, Nov 25, 2024 at 03:34:19PM +0000, Renjiang Han (QUIC) wrote:
>>>> On Monday, November 25, 2024 9:36 PM, Dmitry Baryshkov wrote:
>>>>> On Mon, Nov 25, 2024 at 11:04:50AM +0530, Renjiang Han wrote:
>>>>>> Initialize the platform data and enable venus driver probe of QCS615
>>>>>> SoC.
>>>>>>
>>>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> >
>>>>>> ---
>>>>>>   drivers/media/platform/qcom/venus/core.c | 50
>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>   1 file changed, 50 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/media/platform/qcom/venus/core.c
>>>>>> b/drivers/media/platform/qcom/venus/core.c
>>>>>> index
>>>>>> 423deb5e94dcb193974da23f9bd2d905bfeab2d9..39d8bcf62fe4f72674746b75994c
>>>>>> ce6cbaee94eb 100644
>>>>>> --- a/drivers/media/platform/qcom/venus/core.c
>>>>>> +++ b/drivers/media/platform/qcom/venus/core.c
>>>>>> @@ -630,6 +630,55 @@ static const struct venus_resources msm8998_res = {
>>>>>>   	.fwname = "qcom/venus-4.4/venus.mbn",  };
>>>>>> +static const struct freq_tbl qcs615_freq_table[] = {
>>>>>> +	{ 0, 460000000 },
>>>>>> +	{ 0, 410000000 },
>>>>>> +	{ 0, 380000000 },
>>>>>> +	{ 0, 300000000 },
>>>>>> +	{ 0, 240000000 },
>>>>>> +	{ 0, 133333333 },
>>>>>> +};
>>>>>> +
>>>>>> +static const struct bw_tbl qcs615_bw_table_enc[] = {
>>>>>> +	{  972000,  951000, 0, 1434000, 0 },	/* 3840x2160@30 */
>>>>>> +	{  489600,  723000, 0,  973000, 0 },	/* 1920x1080@60 */
>>>>>> +	{  244800,  370000, 0,	495000, 0 },	/* 1920x1080@30 */
>>>>>> +};
>>>>>> +
>>>>>> +static const struct bw_tbl qcs615_bw_table_dec[] = {
>>>>>> +	{ 1036800, 1987000, 0, 2797000, 0 },	/* 4096x2160@30 */
>>>>>> +	{  489600, 1040000, 0, 1298000, 0 },	/* 1920x1080@60 */
>>>>>> +	{  244800,  530000, 0,  659000, 0 },	/* 1920x1080@30 */
>>>>>> +};
>>>>>> +
>>>>>> +static const struct venus_resources qcs615_res = {
>>>>>> +	.freq_tbl = qcs615_freq_table,
>>>>>> +	.freq_tbl_size = ARRAY_SIZE(qcs615_freq_table),
>>>>>> +	.bw_tbl_enc = qcs615_bw_table_enc,
>>>>>> +	.bw_tbl_enc_size = ARRAY_SIZE(qcs615_bw_table_enc),
>>>>>> +	.bw_tbl_dec = qcs615_bw_table_dec,
>>>>>> +	.bw_tbl_dec_size = ARRAY_SIZE(qcs615_bw_table_dec),
>>>>>> +	.clks = {"core", "iface", "bus" },
>>>>>> +	.clks_num = 3,
>>>>>> +	.vcodec0_clks = { "vcodec0_core", "vcodec0_bus" },
>>>>>> +	.vcodec_clks_num = 2,
>>>>>> +	.vcodec_pmdomains = (const char *[]) { "venus", "vcodec0" },
>>>>>> +	.vcodec_pmdomains_num = 2,
>>>>>> +	.opp_pmdomain = (const char *[]) { "cx" },
>>>>>> +	.vcodec_num = 1,
>>>>>> +	.hfi_version = HFI_VERSION_4XX,
>>>>>> +	.vpu_version = VPU_VERSION_AR50,
>>>>>> +	.vmem_id = VIDC_RESOURCE_NONE,
>>>>>> +	.vmem_size = 0,
>>>>>> +	.vmem_addr = 0,
>>>>>> +	.dma_mask = 0xe0000000 - 1,
>>>>>> +	.cp_start = 0,
>>>>>> +	.cp_size = 0x70800000,
>>>>>> +	.cp_nonpixel_start = 0x1000000,
>>>>>> +	.cp_nonpixel_size = 0x24800000,
>>>>>> +	.fwname = "qcom/venus-5.4/venus_s6.mbn",
>>>>> I really want the firmware discussion of linux-firmware to be solved first,
>>>>> before we land this patch.
>>>>> SHort summary: can we use a single image for all 5.4 platforms (by using
>>>>> v5 signatures, by using v6 signatures, v3 or any other kind of quirk).
>>>> Thanks for your comment. We have discussed with the firmware team and
>>>> other teams if we can use the same firmware binary. The result is we'd better
>>>> use different firmware files. They should respond in the firmware binary
>>>> thread. I will push them and hope them respond as quickly as possible and
>>>> give reasons.
>>>>>> +};
>>>>>> +
>>>>>>   static const struct freq_tbl sdm660_freq_table[] = {
>>>>>>   	{ 979200, 518400000 },
>>>>>>   	{ 489600, 441600000 },
>>>>>> @@ -937,6 +986,7 @@ 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,msm8998-venus", .data = &msm8998_res, },
>>>>>> +	{ .compatible = "qcom,qcs615-venus", .data = &qcs615_res, },
>>>>> The hardware seems to be the same as sc7180, only the frequencies differ.
>>>>> Can we change the driver in a way that we don't have to add another
>>>>> compat entry just for the sake of changing freqs / bandwidths?
>>>> Thank you for your comment. I agree with you. But based on the Venus code
>>>> architecturE ANd the distinction between different platforms, I think the
>>>> current changes are the simplest.
>>> Well, it is simplest, correct. But not the best one. There is no plan no
>>> migrate these platforms to the iris driver. So instead, please improve
>>> the venus driver instead of just pushing the simplest change. I should
>>> have been more explicit about it earlier.
>>
>> Based on the current code architecture, I don't know if there is a better
>> way. If we
>>
>> refactor the code, it will take a lot of effort.
> 
> Yes, please. The freq_tbl contents is a duplicate of the OPP table in
> DT. Drop it from the driver.
Agree, we should reuse OPP table in this case to set the required rate.
Renjiang, let us know your findings with using opp table.

Regards,
Vikash
>> Therefore, I submit this change. Do you have a better approach?
> 
> NAK for this submission. Please spend some time and improve the driver
> instead.
> 
>>
>> Also, the driver architecture of iris is implemented as you said.
> 
> Irrelevant. You are patching venus, not iris.
> 
>>>>>>   	{ .compatible = "qcom,sdm660-venus", .data = &sdm660_res, },
>>>>>>   	{ .compatible = "qcom,sdm845-venus", .data = &sdm845_res, },
>>>>>>   	{ .compatible = "qcom,sdm845-venus-v2", .data = &sdm845_res_v2, },
>>>>>>
>>>>>> --
>>>>>> 2.34.1
>>>>>>
>>>>> -- 
>>>>> With best wishes
>>>>> Dmitry
>>
>> -- 
>> Best Regards,
>> Renjiang
>>
> 

Re: [PATCH v3 1/4] dt-bindings: qcom,qcs615-venus: add support for video hardware

[Vikash Garodia]

On 11/25/2024 9:42 PM, Dmitry Baryshkov wrote:
> On Mon, Nov 25, 2024 at 11:04:49AM +0530, Renjiang Han wrote:
>> Add support for Qualcomm video acceleration hardware used for video
>> stream decoding and encoding on QCOM QCS615.
>>
>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>> ---
>>  .../bindings/media/qcom,qcs615-venus.yaml          | 182 +++++++++++++++++++++
>>  1 file changed, 182 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..7a3a01ff06d8b62bc2424a0a24857c86c6865f89
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,qcs615-venus.yaml
>> @@ -0,0 +1,182 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/qcom,qcs615-venus.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm QCS615 Venus video encode and decode accelerators
>> +
>> +maintainers:
>> +  - Stanimir Varbanov <stanimir.k.varbanov@gmail.com>
>> +  - Vikash Garodia <quic_vgarodia@quicinc.com>
>> +
>> +description:
>> +  The Venus IP is a video encode and decode accelerator present
>> +  on Qualcomm platforms
>> +
>> +allOf:
>> +  - $ref: qcom,venus-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,qcs615-venus
> 
> Please extend sc7180-venus.yaml instead. No need to duplicate
> unnecessary things.
Exactly, we should reuse the sc7180 bindings for this.

Regards,
Vikash
> 
>> +
>> +  power-domains:
>> +    minItems: 2
>> +    maxItems: 3
> 
> So, is it 2 or 3? You don't have legacy here, so you should know an
> exact number.
> 
>> +
>> +  power-domain-names:
>> +    minItems: 2
> 
> And this one also can go away.
> 
>> +    items:
>> +      - const: venus
>> +      - const: vcodec0
>> +      - const: cx
>> +
>> +  clocks:
>> +    maxItems: 5
>> +
>> +  clock-names:
>> +    items:
>> +      - const: core
>> +      - const: iface
>> +      - const: bus
>> +      - const: vcodec0_core
>> +      - const: vcodec0_bus
>> +
>> +  iommus:
>> +    maxItems: 1
>> +
>> +  memory-region:
>> +    maxItems: 1
>> +
>> +  interconnects:
>> +    maxItems: 2
>> +
>> +  interconnect-names:
>> +    items:
>> +      - const: video-mem
>> +      - const: cpu-cfg
>> +
>> +  operating-points-v2: true
>> +
>> +  opp-table:
>> +    type: object
>> +
>> +  video-decoder:
>> +    type: object
>> +
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        const: venus-decoder
>> +
>> +    required:
>> +      - compatible
>> +
>> +  video-encoder:
>> +    type: object
>> +
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        const: venus-encoder
>> +
>> +    required:
>> +      - compatible
>> +
>> +required:
>> +  - compatible
>> +  - power-domain-names
>> +  - iommus
>> +  - video-decoder
>> +  - video-encoder
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/clock/qcom,qcs615-videocc.h>
>> +    #include <dt-bindings/interconnect/qcom,qcs615-rpmh.h>
>> +    #include <dt-bindings/power/qcom,rpmhpd.h>
>> +
>> +    venus: video-codec@aa00000 {
>> +        compatible = "qcom,qcs615-venus";
>> +        reg = <0xaa00000 0x100000>;
>> +        interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +        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";
>> +
>> +        power-domains = <&videocc VENUS_GDSC>,
>> +                        <&videocc VCODEC0_GDSC>,
>> +                        <&rpmhpd RPMHPD_CX>;
>> +        power-domain-names = "venus",
>> +                             "vcodec0",
>> +                             "cx";
>> +
>> +        operating-points-v2 = <&venus_opp_table>;
>> +
>> +        interconnects = <&mmss_noc MASTER_VIDEO_P0 0
>> +                         &mc_virt SLAVE_EBI1 0>,
>> +                        <&gem_noc MASTER_APPSS_PROC 0
>> +                         &config_noc SLAVE_VENUS_CFG 0>;
>> +        interconnect-names = "video-mem",
>> +                             "cpu-cfg";
>> +
>> +        iommus = <&apps_smmu 0xe40 0x20>;
>> +
>> +        memory-region = <&pil_video_mem>;
>> +
>> +        video-decoder {
>> +            compatible = "venus-decoder";
>> +        };
>> +
>> +        video-encoder {
>> +            compatible = "venus-encoder";
>> +        };
>> +
>> +        venus_opp_table: opp-table {
>> +            compatible = "operating-points-v2";
>> +
>> +            opp-133330000 {
>> +                opp-hz = /bits/ 64 <133330000>;
>> +                required-opps = <&rpmhpd_opp_low_svs>;
>> +            };
>> +
>> +            opp-240000000 {
>> +                opp-hz = /bits/ 64 <240000000>;
>> +                required-opps = <&rpmhpd_opp_svs>;
>> +            };
>> +
>> +            opp-300000000 {
>> +                opp-hz = /bits/ 64 <300000000>;
>> +                required-opps = <&rpmhpd_opp_svs_l1>;
>> +            };
>> +
>> +            opp-380000000 {
>> +                opp-hz = /bits/ 64 <380000000>;
>> +                required-opps = <&rpmhpd_opp_nom>;
>> +            };
>> +
>> +            opp-410000000 {
>> +                opp-hz = /bits/ 64 <410000000>;
>> +                required-opps = <&rpmhpd_opp_turbo>;
>> +            };
>> +
>> +            opp-460000000 {
>> +                opp-hz = /bits/ 64 <460000000>;
>> +                required-opps = <&rpmhpd_opp_turbo_l1>;
>> +            };
>> +        };
>> +    };
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v3 3/4] arm64: dts: qcom: add venus node for the qcs615

[Konrad Dybcio]

On 25.11.2024 6:34 AM, Renjiang Han wrote:
> Add venus node into devicetree for the qcs615 video.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs615.dtsi | 86 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> index 06deb5c499fe83f0eb20d7957ca14948de7aab34..18ad4da5ed194458aded424560f45a3a9f3163dc 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
> @@ -394,6 +394,11 @@ smem_region: smem@86000000 {
>  			no-map;
>  			hwlocks = <&tcsr_mutex 3>;
>  		};
> +
> +		pil_video_mem: pil-video@93400000 {
> +			reg = <0x0 0x93400000 0x0 0x500000>;
> +			no-map;
> +		};
>  	};
>  
>  	soc: soc@0 {
> @@ -530,6 +535,87 @@ gem_noc: interconnect@9680000 {
>  			qcom,bcm-voters = <&apps_bcm_voter>;
>  		};
>  
> +		venus: video-codec@aa00000 {
> +			compatible = "qcom,qcs615-venus";
> +			reg = <0x0 0xaa00000 0x0 0x100000>;

Please pad the address part to 8 hex digits with leading zeroes

> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			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";
> +
> +			power-domains = <&videocc VENUS_GDSC>,
> +					<&videocc VCODEC0_GDSC>,
> +					<&rpmhpd RPMHPD_CX>;
> +			power-domain-names = "venus",
> +					     "vcodec0",
> +					     "cx";
> +
> +			operating-points-v2 = <&venus_opp_table>;
> +
> +			interconnects = <&mmss_noc MASTER_VIDEO_P0 0

QCOM_ICC_TAG_ALWAYS

> +					 &mc_virt SLAVE_EBI1 0>,
> +					<&gem_noc MASTER_APPSS_PROC 0
> +					 &config_noc SLAVE_VENUS_CFG 0>;

QCOM_ICC_TAG_ACTIVE_ONLY

Konrad

Re: [PATCH v3 3/4] arm64: dts: qcom: add venus node for the qcs615

[Bryan O'Donoghue]

On 25/11/2024 08:50, Renjiang Han (QUIC) wrote:
> On Mon 11/25/2024 1:35 PM, Renjiang Han wrote:
>> Add venus node into devicetree for the qcs615 video.
> Forgot to add Reviewed-by, next version will add Reviewed-by: Bryan O'Donoghue<bryan.odonoghue@linaro.org>

You'll need to drop the video-encoder and video-decoder nodes per 
Krzysztof's feeback.

https://lore.kernel.org/linux-arm-msm/436145fd-d65f-44ec-b950-c434775187ca@kernel.org

You could do that with this series:

https://lore.kernel.org/linux-media/20241128-media-staging-24-11-25-rb3-hw-compat-string-v4-0-fd062b399374@linaro.org/T/#m55b26747af4692da928ec9b531c4288c4e45c4d2

And the following change to your resource structure.

static const struct venus_resources qcs615_res = {
+	.dec_nodename = "video-decoder",
+	.enc_nodename = "video-encoder",
};

---
bod