[PATCH v7 0/3] media: venus: enable venus on qcs615

[PATCH v7 0/3] media: venus: enable venus on qcs615

[Renjiang Han]

QCS615 uses the same video core as SC7180, so reuse the same resource
data of SC7180 for QCS615 to enable video functionality.

There are no resources for the video-decoder and video-encoder nodes
in the device tree, so remove these two nodes from the device tree. In
addition, to ensure that the video codec functions properly, use [3]
to add encoder and decoder node entries in the venus driver.

Validated this series on QCS615 and SC7180.

depends on:
[1] https://lore.kernel.org/all/20250424-qcs615-mm-v7-clock-controllers-v8-0-bacad5b3659a@quicinc.com
[2] https://lore.kernel.org/all/20250119-qcs615-mm-v2-dt-nodes-v2-0-c46ab4080989@quicinc.com

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
Changes in v7:
- 1. Update devicetree patch to fix the cherry-pick patch conflict issue.
- 2. Remove dt-bindings patch from this patch series due to it has been
picked.
- Link to v6: https://lore.kernel.org/r/20241219-add-venus-for-qcs615-v6-0-e9a74d3b003d@quicinc.com

Changes in v6:
- 1. Remove video-decoder and video-encoder nodes from the device tree
- 2. Add a new dependency.
- 3. Fix missing tag.
- 4. Update commit message.
- Link to v5: https://lore.kernel.org/r/20241217-add-venus-for-qcs615-v5-0-747395d9e630@quicinc.com

Changes in v5:
- 1. Remove extra blank lines in yaml files.
- 2. Add new variables in the driver while keeping the order of the
original variables. And remove unnecessary variable initialization.
- 3. Update commit message.
- 4. Update the order of nodes in the device tree.
- Link to v4: https://lore.kernel.org/r/20241213-add-venus-for-qcs615-v4-0-7e2c9a72d309@quicinc.com

Changes in v4:
- 1. Remove qcom,qcs615-venus.yaml and use qcom,sc7180-venus.yaml for
qcs615 dt-bindings.
- 2. Add "qcom,qcs615-venus" compatible into qcom,sc7180-venus.yaml.
- 3. Remove qcs615 resource from the driver and use sc7180 resource for
the qcs615.
- 4. Use the frequency in the opp-table in devicetree for the driver.
For compatibility, if getting data from the opp table fails, the data
in the frequency table will be used.
- 5. Keep the reverse Christmas tree order coding style.
- 6. Add "qcom,sc7180-venus" compatible in devicetree.
- 7. Update cover letter message.
- Link to v3: https://lore.kernel.org/r/20241125-add-venus-for-qcs615-v3-0-5a376b97a68e@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 (3):
      media: venus: pm_helpers: use opp-table for the frequency
      arm64: dts: qcom: qcs615: add venus node to devicetree
      arm64: dts: qcom: qcs615-ride: enable venus node to initialize video codec

 arch/arm64/boot/dts/qcom/qcs615-ride.dts       |  4 ++
 arch/arm64/boot/dts/qcom/qcs615.dtsi           | 78 ++++++++++++++++++++++++++
 drivers/media/platform/qcom/venus/pm_helpers.c | 53 ++++++++++++-----
 3 files changed, 121 insertions(+), 14 deletions(-)
---
base-commit: 176e917e010cb7dcc605f11d2bc33f304292482b
change-id: 20250526-add-venus-for-qcs615-a547540656d1
prerequisite-message-id: <20250424-qcs615-mm-v7-clock-controllers-v8-0-bacad5b3659a@quicinc.com>
prerequisite-patch-id: 90315d53ce2ec33b6503433068dd9a3ecdba8e8c
prerequisite-patch-id: 62ba590f968bf83ef3bf623e37be71aa861649c3
prerequisite-patch-id: b9e3a2663e27dc60be0eff97baf3739db8516eeb
prerequisite-patch-id: 23062409b23977940c958bf22a215ae5dc45e93a
prerequisite-patch-id: faf0d569634dad432f67acd073343e47add0ee68
prerequisite-patch-id: 9a0caaaa8d25634dd0db5edffbc939eb7e734c6c
prerequisite-patch-id: 1a1dbf7144745dfbc60c0f2efcad188d1fc26779
prerequisite-patch-id: 2327271def3656283d53dadb2ce9f8cd561249d1
prerequisite-patch-id: b12e39a6a0763b8ec23c99c82f3ac6acdca26f85
prerequisite-patch-id: 71f0eb0fb98c3177dcbe6736c120cba4efef0c33
prerequisite-message-id: <20250119-qcs615-mm-v2-dt-nodes-v2-0-c46ab4080989@quicinc.com>
prerequisite-patch-id: afd2dce9e6066b1f6ce0b41ceafe0dd47ad97c40
prerequisite-patch-id: f8d64c8cf6cd883dc7bbb2a4ed6d5a4db85c536d

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

[PATCH v7 1/3] media: venus: pm_helpers: use opp-table for the frequency

[Renjiang Han]

The frequency value in the opp-table in the device tree and the freq_tbl
in the driver are the same.

Therefore, update pm_helpers.c to use the opp-table for frequency values
for the v4 core.
If getting data from the opp table fails, fall back to using the frequency
table.

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

diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index 409aa9bd0b5d099c993eedb03177ec5ed918b4a0..434dd66076e8faf7f3feac6c29152789f8d2f81b 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -43,14 +43,20 @@ static int core_clks_enable(struct venus_core *core)
 	const struct venus_resources *res = core->res;
 	const struct freq_tbl *freq_tbl = core->res->freq_tbl;
 	unsigned int freq_tbl_size = core->res->freq_tbl_size;
+	struct device *dev = core->dev;
+	struct dev_pm_opp *opp;
 	unsigned long freq;
 	unsigned int i;
 	int ret;
 
-	if (!freq_tbl)
-		return -EINVAL;
-
-	freq = freq_tbl[freq_tbl_size - 1].freq;
+	opp = dev_pm_opp_find_freq_ceil(dev, &freq);
+	if (IS_ERR(opp)) {
+		if (!freq_tbl)
+			return -EINVAL;
+		freq = freq_tbl[freq_tbl_size - 1].freq;
+	} else {
+		dev_pm_opp_put(opp);
+	}
 
 	for (i = 0; i < res->clks_num; i++) {
 		if (IS_V6(core)) {
@@ -631,12 +637,15 @@ min_loaded_core(struct venus_inst *inst, u32 *min_coreid, u32 *min_load, bool lo
 
 static int decide_core(struct venus_inst *inst)
 {
+	const struct freq_tbl *freq_tbl = inst->core->res->freq_tbl;
 	const u32 ptype = HFI_PROPERTY_CONFIG_VIDEOCORES_USAGE;
 	struct venus_core *core = inst->core;
 	u32 min_coreid, min_load, cur_inst_load;
 	u32 min_lp_coreid, min_lp_load, cur_inst_lp_load;
 	struct hfi_videocores_usage_type cu;
-	unsigned long max_freq;
+	unsigned long max_freq = ULONG_MAX;
+	struct device *dev = core->dev;
+	struct dev_pm_opp *opp;
 	int ret = 0;
 
 	if (legacy_binding) {
@@ -659,7 +668,11 @@ static int decide_core(struct venus_inst *inst)
 	cur_inst_lp_load *= inst->clk_data.low_power_freq;
 	/*TODO : divide this inst->load by work_route */
 
-	max_freq = core->res->freq_tbl[0].freq;
+	opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
+	if (IS_ERR(opp))
+		max_freq = freq_tbl[0].freq;
+	else
+		dev_pm_opp_put(opp);
 
 	min_loaded_core(inst, &min_coreid, &min_load, false);
 	min_loaded_core(inst, &min_lp_coreid, &min_lp_load, true);
@@ -1082,7 +1095,9 @@ static int load_scale_v4(struct venus_inst *inst)
 	unsigned int num_rows = core->res->freq_tbl_size;
 	struct device *dev = core->dev;
 	unsigned long freq = 0, freq_core1 = 0, freq_core2 = 0;
+	unsigned long max_freq = ULONG_MAX;
 	unsigned long filled_len = 0;
+	struct dev_pm_opp *opp;
 	int i, ret = 0;
 
 	for (i = 0; i < inst->num_input_bufs; i++)
@@ -1108,19 +1123,29 @@ static int load_scale_v4(struct venus_inst *inst)
 
 	freq = max(freq_core1, freq_core2);
 
-	if (freq > table[0].freq) {
-		dev_dbg(dev, VDBGL "requested clock rate: %lu scaling clock rate : %lu\n",
-			freq, table[0].freq);
+	opp = dev_pm_opp_find_freq_floor(dev, &max_freq);
+	if (IS_ERR(opp))
+		max_freq = table[0].freq;
+	else
+		dev_pm_opp_put(opp);
 
-		freq = table[0].freq;
+	if (freq > max_freq) {
+		dev_dbg(dev, VDBGL "requested clock rate: %lu scaling clock rate : %lu\n",
+			freq, max_freq);
+		freq = max_freq;
 		goto set_freq;
 	}
 
-	for (i = num_rows - 1 ; i >= 0; i--) {
-		if (freq <= table[i].freq) {
-			freq = table[i].freq;
-			break;
+	opp = dev_pm_opp_find_freq_ceil(dev, &freq);
+	if (IS_ERR(opp)) {
+		for (i = num_rows - 1 ; i >= 0; i--) {
+			if (freq <= table[i].freq) {
+				freq = table[i].freq;
+				break;
+			}
 		}
+	} else {
+		dev_pm_opp_put(opp);
 	}
 
 set_freq:

-- 
2.34.1

[PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Renjiang Han]

Add the venus node to the devicetree for the qcs615 platform to enable
video functionality. The qcs615 platform currently lacks video
functionality due to the absence of the venus node. Fallback to sc7180 due
to the same video core.

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

diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi
index 9befd9a40113245137779d96ac4e822cb9a142c5..a6cb7c5e4e76899a3c12f8c7656d0cf66db43895 100644
--- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
@@ -449,6 +449,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 {
@@ -3253,6 +3258,79 @@ gem_noc: interconnect@9680000 {
 			qcom,bcm-voters = <&apps_bcm_voter>;
 		};
 
+		venus: video-codec@aa00000 {
+			compatible = "qcom,qcs615-venus", "qcom,sc7180-venus";
+			reg = <0x0 0x0aa00000 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 QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
+					 &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>;
+			interconnect-names = "video-mem",
+					     "cpu-cfg";
+
+			iommus = <&apps_smmu 0xe40 0x20>;
+
+			memory-region = <&pil_video_mem>;
+
+			status = "disabled";
+
+			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 0x0ab00000 0 0x10000>;

-- 
2.34.1

[PATCH v7 3/3] arm64: dts: qcom: qcs615-ride: enable venus node to initialize video codec

[Renjiang Han]

Enable the venus node to allow the video codec to start working properly
by setting its status to "okay".

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 2b5aa3c66867676bda59ff82b902b6e4974126f8..0686f5c10bdaf7ba3f522e16acd2107d25742dd9 100644
--- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
@@ -338,6 +338,10 @@ &ufs_mem_phy {
 	status = "okay";
 };
 
+&venus {
+	status = "okay";
+};
+
 &watchdog {
 	clocks = <&sleep_clk>;
 };

-- 
2.34.1

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Konrad Dybcio]

On 5/27/25 5:32 AM, Renjiang Han wrote:
> Add the venus node to the devicetree for the qcs615 platform to enable
> video functionality. The qcs615 platform currently lacks video
> functionality due to the absence of the venus node. Fallback to sc7180 due
> to the same video core.
> 
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---

[...]

> +			interconnects = <&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
> +					 &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>;

QCOM_ICC_TAG_ACTIVE_ONLY on the second path

> +			interconnect-names = "video-mem",
> +					     "cpu-cfg";
> +
> +			iommus = <&apps_smmu 0xe40 0x20>;

fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)

> +
> +			memory-region = <&pil_video_mem>;
> +
> +			status = "disabled";
> +
> +			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>;

nom_l1

> +				};
> +
> +				opp-460000000 {
> +					opp-hz = /bits/ 64 <460000000>;
> +					required-opps = <&rpmhpd_opp_turbo_l1>;

turbo

Konrad

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Renjiang Han]

On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
> On 5/27/25 5:32 AM, Renjiang Han wrote:
>> Add the venus node to the devicetree for the qcs615 platform to enable
>> video functionality. The qcs615 platform currently lacks video
>> functionality due to the absence of the venus node. Fallback to sc7180 due
>> to the same video core.
>>
>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>> ---
> [...]
>
>> +			interconnects = <&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
>> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>> +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>> +					 &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>;
> QCOM_ICC_TAG_ACTIVE_ONLY on the second path

Thanks for your comment. I'll update it in next version.

             interconnects = <&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS
                      &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
                     <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
                      &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;

>> +			interconnect-names = "video-mem",
>> +					     "cpu-cfg";
>> +
>> +			iommus = <&apps_smmu 0xe40 0x20>;
> fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
OK. Will update it with next version.
>> +
>> +			memory-region = <&pil_video_mem>;
>> +
>> +			status = "disabled";
>> +
>> +			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>;
> nom_l1
>
>> +				};
>> +
>> +				opp-460000000 {
>> +					opp-hz = /bits/ 64 <460000000>;
>> +					required-opps = <&rpmhpd_opp_turbo_l1>;
> turbo

Thanks for your comment, will update like this in next version.

                 opp-410000000 {

                     opp-hz = /bits/ 64 <410000000>;
                     required-opps = <&rpmhpd_opp_nom_l1>;
                 };

                 opp-460000000 {
                     opp-hz = /bits/ 64 <460000000>;
                     required-opps = <&rpmhpd_opp_turbo>;
                 };
>
> Konrad

-- 
Best Regards,
Renjiang

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Dmitry Baryshkov]

On Wed, May 28, 2025 at 05:13:06PM +0800, Renjiang Han wrote:
> 
> On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
> > On 5/27/25 5:32 AM, Renjiang Han wrote:
> > > Add the venus node to the devicetree for the qcs615 platform to enable
> > > video functionality. The qcs615 platform currently lacks video
> > > functionality due to the absence of the venus node. Fallback to sc7180 due
> > > to the same video core.
> > > 
> > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> > > ---
> > [...]
> > 
> > > +			interconnect-names = "video-mem",
> > > +					     "cpu-cfg";
> > > +
> > > +			iommus = <&apps_smmu 0xe40 0x20>;
> > fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
> OK. Will update it with next version.

How would you update this?

-- 
With best wishes
Dmitry

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Renjiang Han]

On 5/28/2025 7:04 PM, Dmitry Baryshkov wrote:
> On Wed, May 28, 2025 at 05:13:06PM +0800, Renjiang Han wrote:
>> On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
>>> On 5/27/25 5:32 AM, Renjiang Han wrote:
>>>> Add the venus node to the devicetree for the qcs615 platform to enable
>>>> video functionality. The qcs615 platform currently lacks video
>>>> functionality due to the absence of the venus node. Fallback to sc7180 due
>>>> to the same video core.
>>>>
>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>>>> ---
>>> [...]
>>>
>>>> +			interconnect-names = "video-mem",
>>>> +					     "cpu-cfg";
>>>> +
>>>> +			iommus = <&apps_smmu 0xe40 0x20>;
>>> fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
>> OK. Will update it with next version.
> How would you update this?
Thanks for your comments. I'll update it like this.
iommus = <&apps_smmu 0xe60 0x20>;

This 0xe40 SID was based on a previous project. However, after rechecking
the documentation yesterday and confirming with colleagues, the correct
SID value should be 0xe60. I’ve also validated it on local device, it
works as expected. The reason 0xe40 seemed to work earlier is due to the
mask value being 0x20, which causes the effective SID derived from 0xe40
to be the same as 0xe60.
>
-- 
Best Regards,
Renjiang

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Dmitry Baryshkov]

On Thu, May 29, 2025 at 10:29:46AM +0800, Renjiang Han wrote:
> 
> On 5/28/2025 7:04 PM, Dmitry Baryshkov wrote:
> > On Wed, May 28, 2025 at 05:13:06PM +0800, Renjiang Han wrote:
> > > On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
> > > > On 5/27/25 5:32 AM, Renjiang Han wrote:
> > > > > Add the venus node to the devicetree for the qcs615 platform to enable
> > > > > video functionality. The qcs615 platform currently lacks video
> > > > > functionality due to the absence of the venus node. Fallback to sc7180 due
> > > > > to the same video core.
> > > > > 
> > > > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> > > > > ---
> > > > [...]
> > > > 
> > > > > +			interconnect-names = "video-mem",
> > > > > +					     "cpu-cfg";
> > > > > +
> > > > > +			iommus = <&apps_smmu 0xe40 0x20>;
> > > > fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
> > > OK. Will update it with next version.
> > How would you update this?
> Thanks for your comments. I'll update it like this.
> iommus = <&apps_smmu 0xe60 0x20>;
> 
> This 0xe40 SID was based on a previous project. However, after rechecking
> the documentation yesterday and confirming with colleagues, the correct
> SID value should be 0xe60. I’ve also validated it on local device, it
> works as expected. The reason 0xe40 seemed to work earlier is due to the
> mask value being 0x20, which causes the effective SID derived from 0xe40
> to be the same as 0xe60.

Using 0xe60 would be counterintuitive, as we have a non-zero masked bits
in the base value. It should be either <0xe60 0x0> or <0xe40 0x20>.

-- 
With best wishes
Dmitry

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Renjiang Han]

On 6/3/2025 9:21 PM, Dmitry Baryshkov wrote:
> On Thu, May 29, 2025 at 10:29:46AM +0800, Renjiang Han wrote:
>> On 5/28/2025 7:04 PM, Dmitry Baryshkov wrote:
>>> On Wed, May 28, 2025 at 05:13:06PM +0800, Renjiang Han wrote:
>>>> On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
>>>>> On 5/27/25 5:32 AM, Renjiang Han wrote:
>>>>>> Add the venus node to the devicetree for the qcs615 platform to enable
>>>>>> video functionality. The qcs615 platform currently lacks video
>>>>>> functionality due to the absence of the venus node. Fallback to sc7180 due
>>>>>> to the same video core.
>>>>>>
>>>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>>>>>> ---
>>>>> [...]
>>>>>
>>>>>> +			interconnect-names = "video-mem",
>>>>>> +					     "cpu-cfg";
>>>>>> +
>>>>>> +			iommus = <&apps_smmu 0xe40 0x20>;
>>>>> fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
>>>> OK. Will update it with next version.
>>> How would you update this?
>> Thanks for your comments. I'll update it like this.
>> iommus = <&apps_smmu 0xe60 0x20>;
>>
>> This 0xe40 SID was based on a previous project. However, after rechecking
>> the documentation yesterday and confirming with colleagues, the correct
>> SID value should be 0xe60. I’ve also validated it on local device, it
>> works as expected. The reason 0xe40 seemed to work earlier is due to the
>> mask value being 0x20, which causes the effective SID derived from 0xe40
>> to be the same as 0xe60.
> Using 0xe60 would be counterintuitive, as we have a non-zero masked bits
> in the base value. It should be either <0xe60 0x0> or <0xe40 0x20>.

Hi Dmitry

Thank you for your comment.

I’ve followed up on this sid with a colleague from the kernel team,
and based on our discussion, it seems that the sid in this case should
be the result sid. The actual sid is 0xe60, and with a mask of 0x20,
the resulting sid would be 0xe40. Therefore, it should be <0xe40 0x20>.

@Konrad, I’d appreciate any thoughts or suggestions you might have on it.

>
-- 
Best Regards,
Renjiang

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Konrad Dybcio]

On 6/4/25 2:05 PM, Renjiang Han wrote:
> 
> On 6/3/2025 9:21 PM, Dmitry Baryshkov wrote:
>> On Thu, May 29, 2025 at 10:29:46AM +0800, Renjiang Han wrote:
>>> On 5/28/2025 7:04 PM, Dmitry Baryshkov wrote:
>>>> On Wed, May 28, 2025 at 05:13:06PM +0800, Renjiang Han wrote:
>>>>> On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
>>>>>> On 5/27/25 5:32 AM, Renjiang Han wrote:
>>>>>>> Add the venus node to the devicetree for the qcs615 platform to enable
>>>>>>> video functionality. The qcs615 platform currently lacks video
>>>>>>> functionality due to the absence of the venus node. Fallback to sc7180 due
>>>>>>> to the same video core.
>>>>>>>
>>>>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>>>>>>> ---
>>>>>> [...]
>>>>>>
>>>>>>> +            interconnect-names = "video-mem",
>>>>>>> +                         "cpu-cfg";
>>>>>>> +
>>>>>>> +            iommus = <&apps_smmu 0xe40 0x20>;
>>>>>> fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
>>>>> OK. Will update it with next version.
>>>> How would you update this?
>>> Thanks for your comments. I'll update it like this.
>>> iommus = <&apps_smmu 0xe60 0x20>;
>>>
>>> This 0xe40 SID was based on a previous project. However, after rechecking
>>> the documentation yesterday and confirming with colleagues, the correct
>>> SID value should be 0xe60. I’ve also validated it on local device, it
>>> works as expected. The reason 0xe40 seemed to work earlier is due to the
>>> mask value being 0x20, which causes the effective SID derived from 0xe40
>>> to be the same as 0xe60.
>> Using 0xe60 would be counterintuitive, as we have a non-zero masked bits
>> in the base value. It should be either <0xe60 0x0> or <0xe40 0x20>.
> 
> Hi Dmitry
> 
> Thank you for your comment.
> 
> I’ve followed up on this sid with a colleague from the kernel team,
> and based on our discussion, it seems that the sid in this case should
> be the result sid. The actual sid is 0xe60, and with a mask of 0x20,
> the resulting sid would be 0xe40. Therefore, it should be <0xe40 0x20>.
> 
> @Konrad, I’d appreciate any thoughts or suggestions you might have on it.

What our docs describe as 'result sid' is literally 'base ~& mask', so if
we used that, setting the mask would be useless..

Now, some old NHLOS builds are known to cause issues if the values aren't
exactly what they expect (some whitelisting must be going on there).

I don't think this should be an issue on this platform, but let's just
use 0xe60 0x20 here to reflect the real values

Konrad

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Dmitry Baryshkov]

On Wed, Jun 04, 2025 at 03:24:25PM +0200, Konrad Dybcio wrote:
> On 6/4/25 2:05 PM, Renjiang Han wrote:
> > 
> > On 6/3/2025 9:21 PM, Dmitry Baryshkov wrote:
> >> On Thu, May 29, 2025 at 10:29:46AM +0800, Renjiang Han wrote:
> >>> On 5/28/2025 7:04 PM, Dmitry Baryshkov wrote:
> >>>> On Wed, May 28, 2025 at 05:13:06PM +0800, Renjiang Han wrote:
> >>>>> On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
> >>>>>> On 5/27/25 5:32 AM, Renjiang Han wrote:
> >>>>>>> Add the venus node to the devicetree for the qcs615 platform to enable
> >>>>>>> video functionality. The qcs615 platform currently lacks video
> >>>>>>> functionality due to the absence of the venus node. Fallback to sc7180 due
> >>>>>>> to the same video core.
> >>>>>>>
> >>>>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> >>>>>>> ---
> >>>>>> [...]
> >>>>>>
> >>>>>>> +            interconnect-names = "video-mem",
> >>>>>>> +                         "cpu-cfg";
> >>>>>>> +
> >>>>>>> +            iommus = <&apps_smmu 0xe40 0x20>;
> >>>>>> fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
> >>>>> OK. Will update it with next version.
> >>>> How would you update this?
> >>> Thanks for your comments. I'll update it like this.
> >>> iommus = <&apps_smmu 0xe60 0x20>;
> >>>
> >>> This 0xe40 SID was based on a previous project. However, after rechecking
> >>> the documentation yesterday and confirming with colleagues, the correct
> >>> SID value should be 0xe60. I’ve also validated it on local device, it
> >>> works as expected. The reason 0xe40 seemed to work earlier is due to the
> >>> mask value being 0x20, which causes the effective SID derived from 0xe40
> >>> to be the same as 0xe60.
> >> Using 0xe60 would be counterintuitive, as we have a non-zero masked bits
> >> in the base value. It should be either <0xe60 0x0> or <0xe40 0x20>.
> > 
> > Hi Dmitry
> > 
> > Thank you for your comment.
> > 
> > I’ve followed up on this sid with a colleague from the kernel team,
> > and based on our discussion, it seems that the sid in this case should
> > be the result sid. The actual sid is 0xe60, and with a mask of 0x20,
> > the resulting sid would be 0xe40. Therefore, it should be <0xe40 0x20>.
> > 
> > @Konrad, I’d appreciate any thoughts or suggestions you might have on it.
> 
> What our docs describe as 'result sid' is literally 'base ~& mask', so if
> we used that, setting the mask would be useless..
> 
> Now, some old NHLOS builds are known to cause issues if the values aren't
> exactly what they expect (some whitelisting must be going on there).
> 
> I don't think this should be an issue on this platform, but let's just
> use 0xe60 0x20 here to reflect the real values

Isn't 0xe40 also 'real'?

-- 
With best wishes
Dmitry

Re: [PATCH v7 2/3] arm64: dts: qcom: qcs615: add venus node to devicetree

[Konrad Dybcio]

On 6/4/25 3:41 PM, Dmitry Baryshkov wrote:
> On Wed, Jun 04, 2025 at 03:24:25PM +0200, Konrad Dybcio wrote:
>> On 6/4/25 2:05 PM, Renjiang Han wrote:
>>>
>>> On 6/3/2025 9:21 PM, Dmitry Baryshkov wrote:
>>>> On Thu, May 29, 2025 at 10:29:46AM +0800, Renjiang Han wrote:
>>>>> On 5/28/2025 7:04 PM, Dmitry Baryshkov wrote:
>>>>>> On Wed, May 28, 2025 at 05:13:06PM +0800, Renjiang Han wrote:
>>>>>>> On 5/27/2025 9:57 PM, Konrad Dybcio wrote:
>>>>>>>> On 5/27/25 5:32 AM, Renjiang Han wrote:
>>>>>>>>> Add the venus node to the devicetree for the qcs615 platform to enable
>>>>>>>>> video functionality. The qcs615 platform currently lacks video
>>>>>>>>> functionality due to the absence of the venus node. Fallback to sc7180 due
>>>>>>>>> to the same video core.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>>>>>>>>> ---
>>>>>>>> [...]
>>>>>>>>
>>>>>>>>> +            interconnect-names = "video-mem",
>>>>>>>>> +                         "cpu-cfg";
>>>>>>>>> +
>>>>>>>>> +            iommus = <&apps_smmu 0xe40 0x20>;
>>>>>>>> fwiw docs mention 0xe60 0x20 (which result in the exact same resulting sid)
>>>>>>> OK. Will update it with next version.
>>>>>> How would you update this?
>>>>> Thanks for your comments. I'll update it like this.
>>>>> iommus = <&apps_smmu 0xe60 0x20>;
>>>>>
>>>>> This 0xe40 SID was based on a previous project. However, after rechecking
>>>>> the documentation yesterday and confirming with colleagues, the correct
>>>>> SID value should be 0xe60. I’ve also validated it on local device, it
>>>>> works as expected. The reason 0xe40 seemed to work earlier is due to the
>>>>> mask value being 0x20, which causes the effective SID derived from 0xe40
>>>>> to be the same as 0xe60.
>>>> Using 0xe60 would be counterintuitive, as we have a non-zero masked bits
>>>> in the base value. It should be either <0xe60 0x0> or <0xe40 0x20>.
>>>
>>> Hi Dmitry
>>>
>>> Thank you for your comment.
>>>
>>> I’ve followed up on this sid with a colleague from the kernel team,
>>> and based on our discussion, it seems that the sid in this case should
>>> be the result sid. The actual sid is 0xe60, and with a mask of 0x20,
>>> the resulting sid would be 0xe40. Therefore, it should be <0xe40 0x20>.
>>>
>>> @Konrad, I’d appreciate any thoughts or suggestions you might have on it.
>>
>> What our docs describe as 'result sid' is literally 'base ~& mask', so if
>> we used that, setting the mask would be useless..
>>
>> Now, some old NHLOS builds are known to cause issues if the values aren't
>> exactly what they expect (some whitelisting must be going on there).
>>
>> I don't think this should be an issue on this platform, but let's just
>> use 0xe60 0x20 here to reflect the real values
> 
> Isn't 0xe40 also 'real'?

0xe60 and 0xe40 (unmasked) are two separate streams

Konrad>