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

[PATCH v6 0/4] 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/20241108-qcs615-mm-dt-nodes-v1-0-b2669cac0624@quicinc.com
[2] https://lore.kernel.org/all/20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com
[3] https://lore.kernel.org/all/20241209-media-staging-24-11-25-rb3-hw-compat-string-v5-0-ef7e5f85f302@linaro.org

Signed-off-by: Renjiang Han <quic_renjiang@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 (4):
      dt-bindings: media: add support for video hardware on QCS615 platform
      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

 .../bindings/media/qcom,sc7180-venus.yaml          |  7 +-
 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 +++++++++++----
 4 files changed, 127 insertions(+), 15 deletions(-)
---
base-commit: 3e42dc9229c5950e84b1ed705f94ed75ed208228
change-id: 20241219-add-venus-for-qcs615-238af570d03d
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: <20241209-media-staging-24-11-25-rb3-hw-compat-string-v5-0-ef7e5f85f302@linaro.org>
prerequisite-patch-id: 3cee337bfc515f0fda75c8406e0e472313a4c895
prerequisite-patch-id: fb56cdcbac866038943e033ceaa898e0582e5365
prerequisite-patch-id: f7297a975e4ffcdb40e94cc7d050a1aa089b9e43

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

[PATCH v6 1/4] dt-bindings: media: add support for video hardware on QCS615 platform

[Renjiang Han]

QCS615 uses the same video core as SC7180.

Therefore, add qcom,qcs615-venus compatible to qcom,sc7180-venus.yaml to
enable video hardware support on QCS615 platform. Make qcom,qcs615-venus
fallback to qcom,sc7180-venus to ensure compatibility with existing
configurations.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
 Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
index 83c4a5d95f020437bd160d6456850bc84a2cf5ff..bfd8b1ad473128c974bce84639cb0aff59d8c2cc 100644
--- a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
@@ -18,7 +18,12 @@ allOf:
 
 properties:
   compatible:
-    const: qcom,sc7180-venus
+    oneOf:
+      - items:
+          - enum:
+              - qcom,qcs615-venus
+          - const: qcom,sc7180-venus
+      - const: qcom,sc7180-venus
 
   power-domains:
     minItems: 2

-- 
2.34.1

[PATCH v6 2/4] 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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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)) {
@@ -627,12 +633,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) {
@@ -655,7 +664,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);
@@ -1078,7 +1091,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++)
@@ -1104,19 +1119,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 v6 3/4] 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 872de9ab8f38d64e8f979bf3af8f122f8ff075d8..174d0a459a3c15defaebef16c7586dcda8618590 100644
--- a/arch/arm64/boot/dts/qcom/qcs615.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi
@@ -427,6 +427,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 {
@@ -2807,6 +2812,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 0xab00000 0 0x10000>;

-- 
2.34.1

[PATCH v6 4/4] 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 a25928933e2b66241258e418c6e5bc36c306101e..de954ede27f0942397d982f9aa725e59a8de9657 100644
--- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
@@ -237,6 +237,10 @@ &usb_1_dwc3 {
 	dr_mode = "peripheral";
 };
 
+&venus {
+	status = "okay";
+};
+
 &watchdog {
 	clocks = <&sleep_clk>;
 };

-- 
2.34.1

Re: [PATCH v6 1/4] dt-bindings: media: add support for video hardware on QCS615 platform

[Krzysztof Kozlowski]

On 19/12/2024 06:41, Renjiang Han wrote:
> QCS615 uses the same video core as SC7180.
> 
> Therefore, add qcom,qcs615-venus compatible to qcom,sc7180-venus.yaml to
> enable video hardware support on QCS615 platform. Make qcom,qcs615-venus
> fallback to qcom,sc7180-venus to ensure compatibility with existing
> configurations.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
> ---

I received this multiple times. Don't send so many versions the same day
(or don't send to people internal testing things, not sure what was the
point of other posting).

Best regards,
Krzysztof

Re: [PATCH v6 1/4] dt-bindings: media: add support for video hardware on QCS615 platform

[Renjiang Han]

On 12/19/2024 4:19 PM, Krzysztof Kozlowski wrote:
> On 19/12/2024 06:41, Renjiang Han wrote:
>> QCS615 uses the same video core as SC7180.
>>
>> Therefore, add qcom,qcs615-venus compatible to qcom,sc7180-venus.yaml to
>> enable video hardware support on QCS615 platform. Make qcom,qcs615-venus
>> fallback to qcom,sc7180-venus to ensure compatibility with existing
>> configurations.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
>> ---
> I received this multiple times. Don't send so many versions the same day
> (or don't send to people internal testing things, not sure what was the
> point of other posting).
>
> Best regards,
> Krzysztof
OK, got it. I asked internal colleagues to review it first, and then 
post it.

-- 
Best Regards,
Renjiang

Re: [PATCH v6 2/4] media: venus: pm_helpers: use opp-table for the frequency

[Konrad Dybcio]

On 19.12.2024 6:41 AM, Renjiang Han wrote:
> 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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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);
> +	}

I'm not super convinced how this could have ever worked without
scaling voltage levels, by the way. Perhaps this will squash some
random bugs :|

Konrad

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

[Bjorn Andersson]

On Thu, Dec 19, 2024 at 11:11:52AM +0530, Renjiang Han wrote:
> 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/20241108-qcs615-mm-dt-nodes-v1-0-b2669cac0624@quicinc.com

This series has requests for changes and will as such never be merged.

Rather than relying on the maintainer to coordinate your cross-team
dependencies, it would be preferable if you grouped your patches per
functional area (i.e. you include Taniya's patch to add videocc) in your
series.

> [2] https://lore.kernel.org/all/20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com

Same goes for this.

I'll drop the two two dts patches from my review queue as they have
unmet dependencies. Please resubmit them together with dependencies or
once the dependencies are available in linux-next.

Regards,
Bjorn

> [3] https://lore.kernel.org/all/20241209-media-staging-24-11-25-rb3-hw-compat-string-v5-0-ef7e5f85f302@linaro.org
> 
> Signed-off-by: Renjiang Han <quic_renjiang@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 (4):
>       dt-bindings: media: add support for video hardware on QCS615 platform
>       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
> 
>  .../bindings/media/qcom,sc7180-venus.yaml          |  7 +-
>  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 +++++++++++----
>  4 files changed, 127 insertions(+), 15 deletions(-)
> ---
> base-commit: 3e42dc9229c5950e84b1ed705f94ed75ed208228
> change-id: 20241219-add-venus-for-qcs615-238af570d03d
> 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: <20241209-media-staging-24-11-25-rb3-hw-compat-string-v5-0-ef7e5f85f302@linaro.org>
> prerequisite-patch-id: 3cee337bfc515f0fda75c8406e0e472313a4c895
> prerequisite-patch-id: fb56cdcbac866038943e033ceaa898e0582e5365
> prerequisite-patch-id: f7297a975e4ffcdb40e94cc7d050a1aa089b9e43
> 
> Best regards,
> -- 
> Renjiang Han <quic_renjiang@quicinc.com>
> 

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

[Renjiang Han]

On 12/27/2024 6:19 AM, Bjorn Andersson wrote:
>> 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/20241108-qcs615-mm-dt-nodes-v1-0-b2669cac0624@quicinc.com
> This series has requests for changes and will as such never be merged.
>
> Rather than relying on the maintainer to coordinate your cross-team
> dependencies, it would be preferable if you grouped your patches per
> functional area (i.e. you include Taniya's patch to add videocc) in your
> series.
>
>> [2]https://lore.kernel.org/all/20241108-qcs615-mm-clockcontroller-v3-0-7d3b2d235fdf@quicinc.com
> Same goes for this.
>
> I'll drop the two two dts patches from my review queue as they have
> unmet dependencies. Please resubmit them together with dependencies or
> once the dependencies are available in linux-next.
>
> Regards,
> Bjorn
  Thanks for your comment, I will sync with Taniya and ask her to update
  these two patch series. Then I will update dependency versions.

-- 
Best Regards,
Renjiang

Re: [PATCH v6 2/4] media: venus: pm_helpers: use opp-table for the frequency

[Renjiang Han]

On 12/23/2024 10:17 PM, Konrad Dybcio wrote:
> On 19.12.2024 6:41 AM, Renjiang Han wrote:
>> 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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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);
>> +	}
> I'm not super convinced how this could have ever worked without
> scaling voltage levels, by the way. Perhaps this will squash some
> random bugs :|
>
> Konrad
  Thanks for your comment.
  The default value of freq is 0, and then dev_pm_opp_find_freq_ceil is
  used to match freq to the maximum value in opp-table that is close to
  0. The frequency values ​​in opp-table and freq_tbl are the same, and
  dev_pm_opp_find_freq_ceil is used to assign the minimum value in
  opp-table to freq. So the logic is the same as before. I'm not sure if
  I've answered your concern.

-- 
Best Regards,
Renjiang

Re: [PATCH v6 2/4] media: venus: pm_helpers: use opp-table for the frequency

[Konrad Dybcio]

On 2.01.2025 6:38 AM, Renjiang Han wrote:
> 
> On 12/23/2024 10:17 PM, Konrad Dybcio wrote:
>> On 19.12.2024 6:41 AM, Renjiang Han wrote:
>>> 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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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);
>>> +    }
>> I'm not super convinced how this could have ever worked without
>> scaling voltage levels, by the way. Perhaps this will squash some
>> random bugs :|
>>
>> Konrad
>  Thanks for your comment.
>  The default value of freq is 0, and then dev_pm_opp_find_freq_ceil is
>  used to match freq to the maximum value in opp-table that is close to
>  0. The frequency values ​​in opp-table and freq_tbl are the same, and
>  dev_pm_opp_find_freq_ceil is used to assign the minimum value in
>  opp-table to freq. So the logic is the same as before. I'm not sure if
>  I've answered your concern.

We talked offline, but for the record: my concern here was about
clk_set_rate() not scaling RPM/h voltage corners, which this patch
fixes

Konrad

Re: [PATCH v6 2/4] media: venus: pm_helpers: use opp-table for the frequency

[Bryan O'Donoghue]

On 09/01/2025 13:05, Konrad Dybcio wrote:
> On 2.01.2025 6:38 AM, Renjiang Han wrote:
>>
>> On 12/23/2024 10:17 PM, Konrad Dybcio wrote:
>>> On 19.12.2024 6:41 AM, Renjiang Han wrote:
>>>> 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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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);
>>>> +    }
>>> I'm not super convinced how this could have ever worked without
>>> scaling voltage levels, by the way. Perhaps this will squash some
>>> random bugs :|
>>>
>>> Konrad
>>   Thanks for your comment.
>>   The default value of freq is 0, and then dev_pm_opp_find_freq_ceil is
>>   used to match freq to the maximum value in opp-table that is close to
>>   0. The frequency values ​​in opp-table and freq_tbl are the same, and
>>   dev_pm_opp_find_freq_ceil is used to assign the minimum value in
>>   opp-table to freq. So the logic is the same as before. I'm not sure if
>>   I've answered your concern.
> 
> We talked offline, but for the record: my concern here was about
> clk_set_rate() not scaling RPM/h voltage corners, which this patch
> fixes
> 
> Konrad

Konrad is this an RB from you, do you have any other concerns with this 
code ?

Dikshita, Vikash ?

I'll give it a test myself ASAP but any other comments or R/B would be 
helpful.

---
bod

Re: [PATCH v6 2/4] media: venus: pm_helpers: use opp-table for the frequency

[Vikash Garodia]

On 4/7/2025 9:09 PM, Bryan O'Donoghue wrote:
> On 09/01/2025 13:05, Konrad Dybcio wrote:
>> On 2.01.2025 6:38 AM, Renjiang Han wrote:
>>>
>>> On 12/23/2024 10:17 PM, Konrad Dybcio wrote:
>>>> On 19.12.2024 6:41 AM, Renjiang Han wrote:
>>>>> 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
>>>>> 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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);
>>>>> +    }
>>>> I'm not super convinced how this could have ever worked without
>>>> scaling voltage levels, by the way. Perhaps this will squash some
>>>> random bugs :|
>>>>
>>>> Konrad
>>>   Thanks for your comment.
>>>   The default value of freq is 0, and then dev_pm_opp_find_freq_ceil is
>>>   used to match freq to the maximum value in opp-table that is close to
>>>   0. The frequency values ​​in opp-table and freq_tbl are the same, and
>>>   dev_pm_opp_find_freq_ceil is used to assign the minimum value in
>>>   opp-table to freq. So the logic is the same as before. I'm not sure if
>>>   I've answered your concern.
>>
>> We talked offline, but for the record: my concern here was about
>> clk_set_rate() not scaling RPM/h voltage corners, which this patch
>> fixes
>>
>> Konrad
> 
> Konrad is this an RB from you, do you have any other concerns with this code ?
> 
> Dikshita, Vikash ?
I have reviewed this change and it looks good to me. It was mainly the
dependencies to videocc for qcs615 which was keeping the change on hold.

Regards,
Vikash
> 
> I'll give it a test myself ASAP but any other comments or R/B would be helpful.
> 
> ---
> bod

Re: [PATCH v6 2/4] media: venus: pm_helpers: use opp-table for the frequency

[Vikash Garodia]

On 12/19/2024 11:11 AM, Renjiang Han wrote:
> 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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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)) {
> @@ -627,12 +633,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) {
> @@ -655,7 +664,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);
> @@ -1078,7 +1091,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++)
> @@ -1104,19 +1119,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:
> 
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>

Re: [PATCH v6 2/4] media: venus: pm_helpers: use opp-table for the frequency

[Konrad Dybcio]

On 4/7/25 5:39 PM, Bryan O'Donoghue wrote:
> On 09/01/2025 13:05, Konrad Dybcio wrote:
>> On 2.01.2025 6:38 AM, Renjiang Han wrote:
>>>
>>> On 12/23/2024 10:17 PM, Konrad Dybcio wrote:
>>>> On 19.12.2024 6:41 AM, Renjiang Han wrote:
>>>>> 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 33a5a659c0ada0ca97eebb5522c5f349f95c49c7..b61c0ad152878870b7223efa274e137d3636433b 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);
>>>>> +    }
>>>> I'm not super convinced how this could have ever worked without
>>>> scaling voltage levels, by the way. Perhaps this will squash some
>>>> random bugs :|
>>>>
>>>> Konrad
>>>   Thanks for your comment.
>>>   The default value of freq is 0, and then dev_pm_opp_find_freq_ceil is
>>>   used to match freq to the maximum value in opp-table that is close to
>>>   0. The frequency values ​​in opp-table and freq_tbl are the same, and
>>>   dev_pm_opp_find_freq_ceil is used to assign the minimum value in
>>>   opp-table to freq. So the logic is the same as before. I'm not sure if
>>>   I've answered your concern.
>>
>> We talked offline, but for the record: my concern here was about
>> clk_set_rate() not scaling RPM/h voltage corners, which this patch
>> fixes
>>
>> Konrad
> 
> Konrad is this an RB from you, do you have any other concerns with this code ?

OK the comment above was misleading - back then I must have thought this patch
changed clk_set_rate to dev_pm_opp_set_rate - which is not what it does

I won't be blocking this, but I'm not super keen on thoroughly reviewing it
either - there are a lot of raw clk_ calls that are hard to keep track of.. 

Konrad

Re: [PATCH v6 4/4] arm64: dts: qcom: qcs615-ride: enable venus node to initialize video codec

[Nicolas Dufresne]

Hi,

looks like your patch has been forgotten, hopefully this can be resolved. If
it does not get pick in a week or so, feel free to resend.

Le jeudi 19 décembre 2024 à 11:11 +0530, Renjiang Han a écrit :
> 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>

Acked-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

regards,
Nicolas

> ---
>  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 a25928933e2b66241258e418c6e5bc36c306101e..de954ede27f0942397d982f9aa725e59a8de9657 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> @@ -237,6 +237,10 @@ &usb_1_dwc3 {
>  	dr_mode = "peripheral";
>  };
>  
> +&venus {
> +	status = "okay";
> +};
> +
>  &watchdog {
>  	clocks = <&sleep_clk>;
>  };