[PATCH 0/3] media: iris: Add support for SM8750 (VPU v3.5)

[PATCH 0/3] media: iris: Add support for SM8750 (VPU v3.5)

[Krzysztof Kozlowski]

Add support for SM8750 Iris codec with major differences against
previous generation SM8650.

DTS will follow up separately (depends on other DTS patches so cannot be
merged as is).

v4l2-compliance report:

v4l2-compliance 1.26.1-5142, 64 bits, 64-bit time_t
v4l2-compliance SHA: 4aee01a02792 2023-12-12 21:40:38

Compliance test for iris_driver device /dev/video0:

Driver Info:
        Driver name      : iris_driver
        Card type        : Iris Decoder
        Bus info         : platform:aa00000.video-codec
        Driver version   : 6.16.0
        Capabilities     : 0x84204000
                Video Memory-to-Memory Multiplanar
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x04204000
                Video Memory-to-Memory Multiplanar
                Streaming
                Extended Pix Format
        Detected Stateful Decoder

Required ioctls:
        test VIDIOC_QUERYCAP: OK
        test invalid ioctls: OK

Allow for multiple opens:
        test second /dev/video0 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 0 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
        test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
        test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls:
        test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
        test VIDIOC_QUERYCTRL: OK
        test VIDIOC_G/S_CTRL: OK
        test VIDIOC_G/S/TRY_EXT_CTRLS: OK
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
        test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
        Standard Controls: 10 Private Controls: 0

Format ioctls:
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
        test VIDIOC_G/S_PARM: OK (Not Supported)
        test VIDIOC_G_FBUF: OK (Not Supported)
        test VIDIOC_G_FMT: OK
        test VIDIOC_TRY_FMT: OK
        test VIDIOC_S_FMT: OK
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK
        test Composing: OK
        test Scaling: OK (Not Supported)

Codec ioctls:
        test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
        test VIDIOC_G_ENC_INDEX: OK (Not Supported)
        test VIDIOC_(TRY_)DECODER_CMD: OK

Buffer ioctls:
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test CREATE_BUFS maximum buffers: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Total for iris_driver device /dev/video0: 46, Succeeded: 46, Failed: 0, Warnings: 0

Best regards,
Krzysztof

---
Krzysztof Kozlowski (3):
      media: dt-bindings: qcom,sm8550-iris: Add SM8750 video codec
      media: iris: Split power on per variants
      media: iris: Add support for SM8750 (VPU v3.5)

 .../bindings/media/qcom,sm8750-iris.yaml           | 186 ++++++++++++++++++
 .../platform/qcom/iris/iris_platform_common.h      |   6 +-
 .../media/platform/qcom/iris/iris_platform_gen2.c  |  68 +++++++
 .../platform/qcom/iris/iris_platform_sm8750.h      |  22 +++
 drivers/media/platform/qcom/iris/iris_probe.c      |   4 +
 drivers/media/platform/qcom/iris/iris_vpu2.c       |   2 +
 drivers/media/platform/qcom/iris/iris_vpu3x.c      | 207 +++++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_vpu_common.c |  12 +-
 drivers/media/platform/qcom/iris/iris_vpu_common.h |   6 +
 9 files changed, 508 insertions(+), 5 deletions(-)
---
base-commit: 709a73d51f11d75ee2aee4f690e4ecd8bc8e9bf3
change-id: 20250714-sm8750-iris-444d7214c903
prerequisite-message-id: <20250702134158.210966-2-krzysztof.kozlowski@linaro.org>
prerequisite-patch-id: 1658ac2fc03eb4b33a236c2dfc2a053249068354

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

[PATCH 1/3] media: dt-bindings: qcom,sm8550-iris: Add SM8750 video codec

[Krzysztof Kozlowski]

Add binding for Qualcom SM8750 Iris video codec, which comes with
significantly different powering up sequence than previous SM8650, thus
different clocks and resets.  For consistency keep existing clock and
clock-names naming, so the list shares common part.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/media/qcom,sm8750-iris.yaml           | 186 +++++++++++++++++++++
 1 file changed, 186 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..e767ebae7c4022d406d61a7bf606b8d878d8632e
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml
@@ -0,0 +1,186 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/qcom,sm8750-iris.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SM8750 SpC Iris video encoder and decoder
+
+maintainers:
+  - Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
+
+description:
+  The Iris video processing unit on Qualcomm SM8750 SoC is a video encode and
+  decode accelerator.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sm8750-iris
+
+  clocks:
+    maxItems: 6
+
+  clock-names:
+    items:
+      - const: iface          # AXI0
+      - const: core
+      - const: vcodec0_core
+      - const: iface1         # AXI1
+      - const: core_freerun
+      - const: vcodec0_core_freerun
+
+  dma-coherent: true
+
+  interconnects:
+    maxItems: 2
+
+  interconnect-names:
+    items:
+      - const: cpu-cfg
+      - const: video-mem
+
+  iommus:
+    maxItems: 2
+
+  operating-points-v2: true
+  opp-table:
+    type: object
+
+  power-domains:
+    maxItems: 4
+
+  power-domain-names:
+    items:
+      - const: venus
+      - const: vcodec0
+      - const: mxc
+      - const: mmcx
+
+  resets:
+    maxItems: 4
+
+  reset-names:
+    items:
+      - const: bus0
+      - const: bus1
+      - const: core
+      - const: vcodec0_core
+
+required:
+  - compatible
+  - dma-coherent
+  - interconnects
+  - interconnect-names
+  - iommus
+  - power-domain-names
+  - resets
+  - reset-names
+
+allOf:
+  - $ref: qcom,venus-common.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/clock/qcom,sm8750-gcc.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interconnect/qcom,icc.h>
+    #include <dt-bindings/interconnect/qcom,sm8750-rpmh.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+    #include <dt-bindings/power/qcom,rpmhpd.h>
+
+    video-codec@aa00000 {
+        compatible = "qcom,sm8750-iris";
+        reg = <0x0aa00000 0xf0000>;
+
+        clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
+                 <&videocc_mvs0c_clk>,
+                 <&videocc_mvs0_clk>,
+                 <&gcc GCC_VIDEO_AXI1_CLK>,
+                 <&videocc_mvs0c_freerun_clk>,
+                 <&videocc_mvs0_freerun_clk>;
+        clock-names = "iface",
+                      "core",
+                      "vcodec0_core",
+                      "iface1",
+                      "core_freerun",
+                      "vcodec0_core_freerun";
+
+        dma-coherent;
+        iommus = <&apps_smmu 0x1940 0>,
+                 <&apps_smmu 0x1947 0>;
+
+        interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+                         &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
+                        <&mmss_noc MASTER_VIDEO_MVP QCOM_ICC_TAG_ALWAYS
+                         &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+        interconnect-names = "cpu-cfg",
+                             "video-mem";
+
+        interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+        operating-points-v2 = <&iris_opp_table>;
+
+        memory-region = <&video_mem>;
+
+        power-domains = <&videocc_mvs0c_gdsc>,
+                        <&videocc_mvs0_gdsc>,
+                        <&rpmhpd RPMHPD_MXC>,
+                        <&rpmhpd RPMHPD_MMCX>;
+        power-domain-names = "venus",
+                             "vcodec0",
+                             "mxc",
+                             "mmcx";
+
+        resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>,
+                 <&gcc GCC_VIDEO_AXI1_CLK_ARES>,
+                 <&videocc_mvs0c_freerun_clk_ares>,
+                 <&videocc_mvs0_freerun_clk_ares>;
+        reset-names = "bus0",
+                      "bus1",
+                      "core",
+                      "vcodec0_core";
+
+        iris_opp_table: opp-table {
+            compatible = "operating-points-v2";
+
+            opp-240000000 {
+                opp-hz = /bits/ 64 <240000000>;
+                required-opps = <&rpmhpd_opp_low_svs_d1>,
+                                <&rpmhpd_opp_low_svs_d1>;
+            };
+
+            opp-338000000 {
+                opp-hz = /bits/ 64 <338000000>;
+                required-opps = <&rpmhpd_opp_low_svs>,
+                                <&rpmhpd_opp_low_svs>;
+            };
+
+            opp-420000000 {
+                opp-hz = /bits/ 64 <420000000>;
+                required-opps = <&rpmhpd_opp_svs>,
+                                <&rpmhpd_opp_svs>;
+            };
+
+            opp-444000000 {
+                opp-hz = /bits/ 64 <444000000>;
+                required-opps = <&rpmhpd_opp_svs_l1>,
+                                <&rpmhpd_opp_svs_l1>;
+            };
+
+            opp-533333334 {
+                opp-hz = /bits/ 64 <533333334>;
+                required-opps = <&rpmhpd_opp_nom>,
+                                <&rpmhpd_opp_nom>;
+            };
+
+            opp-630000000 {
+                opp-hz = /bits/ 64 <630000000>;
+                required-opps = <&rpmhpd_opp_turbo>,
+                                <&rpmhpd_opp_turbo>;
+            };
+        };
+    };

-- 
2.43.0

[PATCH 2/3] media: iris: Split power on per variants

[Krzysztof Kozlowski]

Current devices use same power up sequence, but starting with Qualcomm
SM8750 (VPU v3.5) the sequence will grow quite a bit, so allow
customizing it.  No functional change so far for existing devices.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/media/platform/qcom/iris/iris_vpu2.c       | 2 ++
 drivers/media/platform/qcom/iris/iris_vpu3x.c      | 4 ++++
 drivers/media/platform/qcom/iris/iris_vpu_common.c | 8 ++++----
 drivers/media/platform/qcom/iris/iris_vpu_common.h | 4 ++++
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vpu2.c b/drivers/media/platform/qcom/iris/iris_vpu2.c
index 7cf1bfc352d34b897451061b5c14fbe90276433d..de7d142316d2dc9ab0c4ad9cc8161c87ac949b4c 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu2.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu2.c
@@ -34,6 +34,8 @@ static u64 iris_vpu2_calc_freq(struct iris_inst *inst, size_t data_size)
 
 const struct vpu_ops iris_vpu2_ops = {
 	.power_off_hw = iris_vpu_power_off_hw,
+	.power_on_hw = iris_vpu_power_on_hw,
 	.power_off_controller = iris_vpu_power_off_controller,
+	.power_on_controller = iris_vpu_power_on_controller,
 	.calc_freq = iris_vpu2_calc_freq,
 };
diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
index 9b7c9a1495ee2f51c60b1142b2ed4680ff798f0a..c235112057aa7b7eab1995737541b7a8276ff18b 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
@@ -264,12 +264,16 @@ static u64 iris_vpu3x_calculate_frequency(struct iris_inst *inst, size_t data_si
 
 const struct vpu_ops iris_vpu3_ops = {
 	.power_off_hw = iris_vpu3_power_off_hardware,
+	.power_on_hw = iris_vpu_power_on_hw,
 	.power_off_controller = iris_vpu_power_off_controller,
+	.power_on_controller = iris_vpu_power_on_controller,
 	.calc_freq = iris_vpu3x_calculate_frequency,
 };
 
 const struct vpu_ops iris_vpu33_ops = {
 	.power_off_hw = iris_vpu33_power_off_hardware,
+	.power_on_hw = iris_vpu_power_on_hw,
 	.power_off_controller = iris_vpu33_power_off_controller,
+	.power_on_controller = iris_vpu_power_on_controller,
 	.calc_freq = iris_vpu3x_calculate_frequency,
 };
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index 42a7c53ce48eb56a4210c7e25c707a1b0881a8ce..6c51002f72ab3d9e16d5a2a50ac712fac91ae25c 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -271,7 +271,7 @@ void iris_vpu_power_off(struct iris_core *core)
 		disable_irq_nosync(core->irq);
 }
 
-static int iris_vpu_power_on_controller(struct iris_core *core)
+int iris_vpu_power_on_controller(struct iris_core *core)
 {
 	u32 rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
 	int ret;
@@ -302,7 +302,7 @@ static int iris_vpu_power_on_controller(struct iris_core *core)
 	return ret;
 }
 
-static int iris_vpu_power_on_hw(struct iris_core *core)
+int iris_vpu_power_on_hw(struct iris_core *core)
 {
 	int ret;
 
@@ -337,11 +337,11 @@ int iris_vpu_power_on(struct iris_core *core)
 	if (ret)
 		goto err;
 
-	ret = iris_vpu_power_on_controller(core);
+	ret = core->iris_platform_data->vpu_ops->power_on_controller(core);
 	if (ret)
 		goto err_unvote_icc;
 
-	ret = iris_vpu_power_on_hw(core);
+	ret = core->iris_platform_data->vpu_ops->power_on_hw(core);
 	if (ret)
 		goto err_power_off_ctrl;
 
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
index 93b7fa27be3bfa1cf6a3e83cc192cdb89d63575f..d95b305ca5a89ba8f08aefb6e6acd9ea4a721a8b 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
@@ -14,7 +14,9 @@ extern const struct vpu_ops iris_vpu33_ops;
 
 struct vpu_ops {
 	void (*power_off_hw)(struct iris_core *core);
+	int (*power_on_hw)(struct iris_core *core);
 	int (*power_off_controller)(struct iris_core *core);
+	int (*power_on_controller)(struct iris_core *core);
 	u64 (*calc_freq)(struct iris_inst *inst, size_t data_size);
 };
 
@@ -23,6 +25,8 @@ void iris_vpu_raise_interrupt(struct iris_core *core);
 void iris_vpu_clear_interrupt(struct iris_core *core);
 int iris_vpu_watchdog(struct iris_core *core, u32 intr_status);
 int iris_vpu_prepare_pc(struct iris_core *core);
+int iris_vpu_power_on_controller(struct iris_core *core);
+int iris_vpu_power_on_hw(struct iris_core *core);
 int iris_vpu_power_on(struct iris_core *core);
 int iris_vpu_power_off_controller(struct iris_core *core);
 void iris_vpu_power_off_hw(struct iris_core *core);

-- 
2.43.0

[PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Krzysztof Kozlowski]

Add support for SM8750 Iris codec with major differences against
previous generation SM8650:

1. New clocks and new resets, thus new power up and power down
   sequences,

2. New WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0 register programmed
   during boot-up

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../platform/qcom/iris/iris_platform_common.h      |   6 +-
 .../media/platform/qcom/iris/iris_platform_gen2.c  |  68 +++++++
 .../platform/qcom/iris/iris_platform_sm8750.h      |  22 +++
 drivers/media/platform/qcom/iris/iris_probe.c      |   4 +
 drivers/media/platform/qcom/iris/iris_vpu3x.c      | 203 +++++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_vpu_common.c |   4 +
 drivers/media/platform/qcom/iris/iris_vpu_common.h |   2 +
 7 files changed, 308 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index adafdce8a856f9c661aabc5ca28f0faceaa93551..fd5a6e69e01cfd00253f4ffb282d40112b93073b 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -38,11 +38,15 @@ extern struct iris_platform_data qcs8300_data;
 extern struct iris_platform_data sm8250_data;
 extern struct iris_platform_data sm8550_data;
 extern struct iris_platform_data sm8650_data;
+extern struct iris_platform_data sm8750_data;
 
 enum platform_clk_type {
-	IRIS_AXI_CLK,
+	IRIS_AXI_CLK, /* AXI0 in case of platforms with multiple AXI clocks */
 	IRIS_CTRL_CLK,
 	IRIS_HW_CLK,
+	IRIS_AXI1_CLK,
+	IRIS_CTRL_FREERUN_CLK,
+	IRIS_HW_FREERUN_CLK,
 };
 
 struct platform_clk_data {
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index d3026b2bcb708c7ec31f134f628df7e57b54af4f..795efe2226228c4d7155ce18ff42ba9cb74b4af2 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2025 Linaro Ltd
  */
 
 #include "iris_core.h"
@@ -12,6 +13,7 @@
 
 #include "iris_platform_qcs8300.h"
 #include "iris_platform_sm8650.h"
+#include "iris_platform_sm8750.h"
 
 #define VIDEO_ARCH_LX 1
 
@@ -463,6 +465,72 @@ struct iris_platform_data sm8650_data = {
 	.dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_op_int_buf_tbl),
 };
 
+struct iris_platform_data sm8750_data = {
+	.get_instance = iris_hfi_gen2_get_instance,
+	.init_hfi_command_ops = iris_hfi_gen2_command_ops_init,
+	.init_hfi_response_ops = iris_hfi_gen2_response_ops_init,
+	.vpu_ops = &iris_vpu35_ops,
+	.set_preset_registers = iris_set_sm8550_preset_registers,
+	.icc_tbl = sm8550_icc_table,
+	.icc_tbl_size = ARRAY_SIZE(sm8550_icc_table),
+	.clk_rst_tbl = sm8750_clk_reset_table,
+	.clk_rst_tbl_size = ARRAY_SIZE(sm8750_clk_reset_table),
+	.bw_tbl_dec = sm8550_bw_table_dec,
+	.bw_tbl_dec_size = ARRAY_SIZE(sm8550_bw_table_dec),
+	.pmdomain_tbl = sm8550_pmdomain_table,
+	.pmdomain_tbl_size = ARRAY_SIZE(sm8550_pmdomain_table),
+	.opp_pd_tbl = sm8550_opp_pd_table,
+	.opp_pd_tbl_size = ARRAY_SIZE(sm8550_opp_pd_table),
+	.clk_tbl = sm8750_clk_table,
+	.clk_tbl_size = ARRAY_SIZE(sm8750_clk_table),
+	/* Upper bound of DMA address range */
+	.dma_mask = 0xe0000000 - 1,
+	.fwname = "qcom/vpu/vpu35_4v.mbn",
+	.pas_id = IRIS_PAS_ID,
+	.inst_caps = &platform_inst_cap_sm8550,
+	.inst_fw_caps = inst_fw_cap_sm8550,
+	.inst_fw_caps_size = ARRAY_SIZE(inst_fw_cap_sm8550),
+	.tz_cp_config_data = &tz_cp_config_sm8550,
+	.core_arch = VIDEO_ARCH_LX,
+	.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
+	.ubwc_config = &ubwc_config_sm8550,
+	.num_vpp_pipe = 4,
+	.max_session_count = 16,
+	.max_core_mbpf = NUM_MBS_8K * 2,
+	.input_config_params_default =
+		sm8550_vdec_input_config_params_default,
+	.input_config_params_default_size =
+		ARRAY_SIZE(sm8550_vdec_input_config_params_default),
+	.input_config_params_hevc =
+		sm8550_vdec_input_config_param_hevc,
+	.input_config_params_hevc_size =
+		ARRAY_SIZE(sm8550_vdec_input_config_param_hevc),
+	.input_config_params_vp9 =
+		sm8550_vdec_input_config_param_vp9,
+	.input_config_params_vp9_size =
+		ARRAY_SIZE(sm8550_vdec_input_config_param_vp9),
+	.output_config_params =
+		sm8550_vdec_output_config_params,
+	.output_config_params_size =
+		ARRAY_SIZE(sm8550_vdec_output_config_params),
+	.dec_input_prop = sm8550_vdec_subscribe_input_properties,
+	.dec_input_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_input_properties),
+	.dec_output_prop_avc = sm8550_vdec_subscribe_output_properties_avc,
+	.dec_output_prop_avc_size =
+		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_avc),
+	.dec_output_prop_hevc = sm8550_vdec_subscribe_output_properties_hevc,
+	.dec_output_prop_hevc_size =
+		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_hevc),
+	.dec_output_prop_vp9 = sm8550_vdec_subscribe_output_properties_vp9,
+	.dec_output_prop_vp9_size =
+		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_vp9),
+
+	.dec_ip_int_buf_tbl = sm8550_dec_ip_int_buf_tbl,
+	.dec_ip_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_ip_int_buf_tbl),
+	.dec_op_int_buf_tbl = sm8550_dec_op_int_buf_tbl,
+	.dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_op_int_buf_tbl),
+};
+
 /*
  * Shares most of SM8550 data except:
  * - inst_caps to platform_inst_cap_qcs8300
diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8750.h b/drivers/media/platform/qcom/iris/iris_platform_sm8750.h
new file mode 100644
index 0000000000000000000000000000000000000000..719056656a5baf48a7bced634d2582629333cf5c
--- /dev/null
+++ b/drivers/media/platform/qcom/iris/iris_platform_sm8750.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2025 Linaro Ltd
+ */
+
+#ifndef __MEDIA_IRIS_PLATFORM_SM8750_H__
+#define __MEDIA_IRIS_PLATFORM_SM8750_H__
+
+static const char * const sm8750_clk_reset_table[] = {
+	"bus0", "bus1", "core", "vcodec0_core"
+};
+
+static const struct platform_clk_data sm8750_clk_table[] = {
+	{IRIS_AXI_CLK,		"iface"			},
+	{IRIS_CTRL_CLK,		"core"			},
+	{IRIS_HW_CLK,		"vcodec0_core"		},
+	{IRIS_AXI1_CLK,		"iface1"		},
+	{IRIS_CTRL_FREERUN_CLK,	"core_freerun"		},
+	{IRIS_HW_FREERUN_CLK,	"vcodec0_core_freerun"	},
+};
+
+#endif
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index 4e6e92357968d7419f114cc0ffa9b571bad19e46..5fb936a04155e72f4298cd6760eff6e9d1da6310 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -353,6 +353,10 @@ static const struct of_device_id iris_dt_match[] = {
 		.compatible = "qcom,sm8650-iris",
 		.data = &sm8650_data,
 	},
+	{
+		.compatible = "qcom,sm8750-iris",
+		.data = &sm8750_data,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, iris_dt_match);
diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
index c235112057aa7b7eab1995737541b7a8276ff18b..b00702a4d6c23258550a77373eb34740e785ef22 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2025 Linaro Ltd
  */
 
 #include <linux/iopoll.h>
@@ -19,8 +20,11 @@
 #define WRAPPER_IRIS_CPU_NOC_LPI_CONTROL	(WRAPPER_BASE_OFFS + 0x5C)
 #define REQ_POWER_DOWN_PREP			BIT(0)
 #define WRAPPER_IRIS_CPU_NOC_LPI_STATUS		(WRAPPER_BASE_OFFS + 0x60)
+#define WRAPPER_CORE_POWER_CONTROL		(WRAPPER_BASE_OFFS + 0x84)
 #define WRAPPER_CORE_CLOCK_CONFIG		(WRAPPER_BASE_OFFS + 0x88)
 #define CORE_CLK_RUN				0x0
+/* VPU v3.5 */
+#define WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0	(WRAPPER_BASE_OFFS + 0x78)
 
 #define WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG		(WRAPPER_TZ_BASE_OFFS + 0x14)
 #define CTL_AXI_CLK_HALT			BIT(0)
@@ -52,6 +56,8 @@
 #define AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL	(AON_BASE_OFFS + 0x20)
 #define NOC_HALT				BIT(0)
 #define AON_WRAPPER_SPARE			(AON_BASE_OFFS + 0x28)
+#define AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_CONTROL	(AON_BASE_OFFS + 0x2C)
+#define AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_STATUS	(AON_BASE_OFFS + 0x30)
 
 static bool iris_vpu3x_hw_power_collapsed(struct iris_core *core)
 {
@@ -225,6 +231,194 @@ static int iris_vpu33_power_off_controller(struct iris_core *core)
 	return 0;
 }
 
+static int iris_vpu35_power_on_hw(struct iris_core *core)
+{
+	int ret;
+	u32 val;
+
+	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
+	if (ret)
+		return ret;
+
+	/* Switch GDSC to SW control */
+	writel(0x0, core->reg_base + WRAPPER_CORE_POWER_CONTROL);
+	ret = readl_poll_timeout(core->reg_base + WRAPPER_CORE_POWER_STATUS,
+				 val, val & BIT(1), 200, 2000);
+	if (ret)
+		goto err_disable_power;
+
+	ret = iris_prepare_enable_clock(core, IRIS_AXI_CLK);
+	if (ret)
+		goto err_gdsc;
+
+	ret = iris_prepare_enable_clock(core, IRIS_HW_FREERUN_CLK);
+	if (ret)
+		goto err_disable_axi_clk;
+
+	ret = iris_prepare_enable_clock(core, IRIS_HW_CLK);
+	if (ret)
+		goto err_disable_hw_free_clk;
+
+	ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
+	if (ret)
+		goto err_disable_hw_clk;
+
+	return 0;
+
+err_disable_hw_clk:
+	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
+err_disable_hw_free_clk:
+	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
+err_disable_axi_clk:
+	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
+err_gdsc:
+	writel(BIT(0), core->reg_base + WRAPPER_CORE_POWER_CONTROL);
+err_disable_power:
+	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
+
+	return ret;
+}
+
+static void iris_vpu35_power_off_hw(struct iris_core *core)
+{
+	u32 val = 0, value, i;
+	int ret;
+
+	if (iris_vpu3x_hw_power_collapsed(core))
+		goto disable_power;
+
+	value = readl(core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
+	if (value)
+		writel(CORE_CLK_RUN, core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
+
+	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
+		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
+					 val, val & 0x400000, 2000, 20000);
+		if (ret)
+			goto disable_power;
+	}
+
+	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
+				 val, val & BIT(0), 200, 2000);
+	if (ret)
+		goto disable_power;
+
+	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
+	writel(BIT(0), core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
+
+	writel(CORE_BRIDGE_SW_RESET | CORE_BRIDGE_HW_RESET_DISABLE,
+	       core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
+	writel(CORE_BRIDGE_HW_RESET_DISABLE, core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
+	writel(0x0, core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
+
+disable_power:
+	dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], false);
+	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
+	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
+	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
+
+	writel(BIT(0), core->reg_base + WRAPPER_CORE_POWER_CONTROL);
+	/*
+	 * Do not wait for power-down, because hardware might delay it (it
+	 * always timeouts).
+	 */
+
+	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
+}
+
+static int iris_vpu35_power_off_controller(struct iris_core *core)
+{
+	u32 xo_rst_tbl_size = core->iris_platform_data->controller_rst_tbl_size;
+	u32 clk_rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
+	u32 val = 0;
+	int ret;
+
+	writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, core->reg_base + CPU_CS_X2RPMH);
+
+	writel(REQ_POWER_DOWN_PREP, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL);
+
+	ret = readl_poll_timeout(core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_STATUS,
+				 val, val & BIT(0), 200, 2000);
+	if (ret)
+		goto disable_power;
+
+	writel(0x0, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL);
+
+	writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_CONTROL);
+	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_STATUS,
+				 val, val & (BIT(0) | BIT(1) | BIT(2)), 15, 1000);
+	if (ret)
+		goto disable_power;
+
+	writel(0x0, core->reg_base + AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_CONTROL);
+
+	writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL);
+
+	ret = readl_poll_timeout(core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_STATUS,
+				 val, val == 0, 200, 2000);
+	if (ret)
+		goto disable_power;
+
+disable_power:
+	iris_disable_unprepare_clock(core, IRIS_CTRL_CLK);
+	iris_disable_unprepare_clock(core, IRIS_CTRL_FREERUN_CLK);
+	iris_disable_unprepare_clock(core, IRIS_AXI1_CLK);
+
+	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
+
+	reset_control_bulk_reset(clk_rst_tbl_size, core->resets);
+
+	reset_control_bulk_assert(xo_rst_tbl_size, core->controller_resets);
+
+	usleep_range(400, 500);
+
+	reset_control_bulk_deassert(xo_rst_tbl_size, core->controller_resets);
+
+	return 0;
+}
+
+static int iris_vpu35_power_on_controller(struct iris_core *core)
+{
+	u32 rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
+	int ret;
+
+	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
+	if (ret)
+		return ret;
+
+	ret = reset_control_bulk_reset(rst_tbl_size, core->resets);
+	if (ret)
+		goto err_disable_power;
+
+	ret = iris_prepare_enable_clock(core, IRIS_AXI1_CLK);
+	if (ret)
+		goto err_disable_power;
+
+	ret = iris_prepare_enable_clock(core, IRIS_CTRL_FREERUN_CLK);
+	if (ret)
+		goto err_disable_axi1_clk;
+
+	ret = iris_prepare_enable_clock(core, IRIS_CTRL_CLK);
+	if (ret)
+		goto err_disable_ctrl_free_clk;
+
+	return 0;
+
+err_disable_ctrl_free_clk:
+	iris_disable_unprepare_clock(core, IRIS_CTRL_FREERUN_CLK);
+err_disable_axi1_clk:
+	iris_disable_unprepare_clock(core, IRIS_AXI1_CLK);
+err_disable_power:
+	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
+
+	return ret;
+}
+
+static void iris_vpu35_program_bootup_registers(struct iris_core *core)
+{
+	writel(0x1, core->reg_base + WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0);
+}
+
 static u64 iris_vpu3x_calculate_frequency(struct iris_inst *inst, size_t data_size)
 {
 	struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
@@ -277,3 +471,12 @@ const struct vpu_ops iris_vpu33_ops = {
 	.power_on_controller = iris_vpu_power_on_controller,
 	.calc_freq = iris_vpu3x_calculate_frequency,
 };
+
+const struct vpu_ops iris_vpu35_ops = {
+	.power_off_hw = iris_vpu35_power_off_hw,
+	.power_on_hw = iris_vpu35_power_on_hw,
+	.power_off_controller = iris_vpu35_power_off_controller,
+	.power_on_controller = iris_vpu35_power_on_controller,
+	.program_bootup_registers = iris_vpu35_program_bootup_registers,
+	.calc_freq = iris_vpu3x_calculate_frequency,
+};
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index 6c51002f72ab3d9e16d5a2a50ac712fac91ae25c..bb98950e018fadf69ac4f41b3037f7fd6ac33c5b 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -84,6 +84,7 @@ static void iris_vpu_interrupt_init(struct iris_core *core)
 static void iris_vpu_setup_ucregion_memory_map(struct iris_core *core)
 {
 	u32 queue_size, value;
+	const struct vpu_ops *vpu_ops = core->iris_platform_data->vpu_ops;
 
 	/* Iris hardware requires 4K queue alignment */
 	queue_size = ALIGN(sizeof(struct iris_hfi_queue_table_header) +
@@ -105,6 +106,9 @@ static void iris_vpu_setup_ucregion_memory_map(struct iris_core *core)
 		value = (u32)core->sfr_daddr + core->iris_platform_data->core_arch;
 		writel(value, core->reg_base + SFR_ADDR);
 	}
+
+	if (vpu_ops->program_bootup_registers)
+		vpu_ops->program_bootup_registers(core);
 }
 
 int iris_vpu_boot_firmware(struct iris_core *core)
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
index d95b305ca5a89ba8f08aefb6e6acd9ea4a721a8b..d636e287457adf0c44540af5c85cfa69decbca8b 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
@@ -11,12 +11,14 @@ struct iris_core;
 extern const struct vpu_ops iris_vpu2_ops;
 extern const struct vpu_ops iris_vpu3_ops;
 extern const struct vpu_ops iris_vpu33_ops;
+extern const struct vpu_ops iris_vpu35_ops;
 
 struct vpu_ops {
 	void (*power_off_hw)(struct iris_core *core);
 	int (*power_on_hw)(struct iris_core *core);
 	int (*power_off_controller)(struct iris_core *core);
 	int (*power_on_controller)(struct iris_core *core);
+	void (*program_bootup_registers)(struct iris_core *core);
 	u64 (*calc_freq)(struct iris_inst *inst, size_t data_size);
 };
 

-- 
2.43.0

Re: [PATCH 1/3] media: dt-bindings: qcom,sm8550-iris: Add SM8750 video codec

[Rob Herring]

On Mon, Jul 14, 2025 at 03:41:16PM +0200, Krzysztof Kozlowski wrote:
> Add binding for Qualcom SM8750 Iris video codec, which comes with
> significantly different powering up sequence than previous SM8650, thus
> different clocks and resets.  For consistency keep existing clock and
> clock-names naming, so the list shares common part.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/media/qcom,sm8750-iris.yaml           | 186 +++++++++++++++++++++
>  1 file changed, 186 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..e767ebae7c4022d406d61a7bf606b8d878d8632e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/qcom,sm8750-iris.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SM8750 SpC Iris video encoder and decoder

SpC? Or SoC...

With that defined or fixed,

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Dikshita Agarwal]

On 7/14/2025 7:11 PM, Krzysztof Kozlowski wrote:
> Add support for SM8750 Iris codec with major differences against
> previous generation SM8650:
> 
> 1. New clocks and new resets, thus new power up and power down
>    sequences,
> 
> 2. New WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0 register programmed
>    during boot-up
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../platform/qcom/iris/iris_platform_common.h      |   6 +-
>  .../media/platform/qcom/iris/iris_platform_gen2.c  |  68 +++++++
>  .../platform/qcom/iris/iris_platform_sm8750.h      |  22 +++
>  drivers/media/platform/qcom/iris/iris_probe.c      |   4 +
>  drivers/media/platform/qcom/iris/iris_vpu3x.c      | 203 +++++++++++++++++++++
>  drivers/media/platform/qcom/iris/iris_vpu_common.c |   4 +
>  drivers/media/platform/qcom/iris/iris_vpu_common.h |   2 +
>  7 files changed, 308 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index adafdce8a856f9c661aabc5ca28f0faceaa93551..fd5a6e69e01cfd00253f4ffb282d40112b93073b 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -38,11 +38,15 @@ extern struct iris_platform_data qcs8300_data;
>  extern struct iris_platform_data sm8250_data;
>  extern struct iris_platform_data sm8550_data;
>  extern struct iris_platform_data sm8650_data;
> +extern struct iris_platform_data sm8750_data;
>  
>  enum platform_clk_type {
> -	IRIS_AXI_CLK,
> +	IRIS_AXI_CLK, /* AXI0 in case of platforms with multiple AXI clocks */
>  	IRIS_CTRL_CLK,
>  	IRIS_HW_CLK,
> +	IRIS_AXI1_CLK,
> +	IRIS_CTRL_FREERUN_CLK,
> +	IRIS_HW_FREERUN_CLK,
>  };
>  
>  struct platform_clk_data {
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> index d3026b2bcb708c7ec31f134f628df7e57b54af4f..795efe2226228c4d7155ce18ff42ba9cb74b4af2 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2025 Linaro Ltd
>   */
>  
>  #include "iris_core.h"
> @@ -12,6 +13,7 @@
>  
>  #include "iris_platform_qcs8300.h"
>  #include "iris_platform_sm8650.h"
> +#include "iris_platform_sm8750.h"
>  
>  #define VIDEO_ARCH_LX 1
>  
> @@ -463,6 +465,72 @@ struct iris_platform_data sm8650_data = {
>  	.dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_op_int_buf_tbl),
>  };
>  
> +struct iris_platform_data sm8750_data = {
> +	.get_instance = iris_hfi_gen2_get_instance,
> +	.init_hfi_command_ops = iris_hfi_gen2_command_ops_init,
> +	.init_hfi_response_ops = iris_hfi_gen2_response_ops_init,
> +	.vpu_ops = &iris_vpu35_ops,
> +	.set_preset_registers = iris_set_sm8550_preset_registers,
> +	.icc_tbl = sm8550_icc_table,
> +	.icc_tbl_size = ARRAY_SIZE(sm8550_icc_table),
> +	.clk_rst_tbl = sm8750_clk_reset_table,
> +	.clk_rst_tbl_size = ARRAY_SIZE(sm8750_clk_reset_table),
> +	.bw_tbl_dec = sm8550_bw_table_dec,
> +	.bw_tbl_dec_size = ARRAY_SIZE(sm8550_bw_table_dec),
> +	.pmdomain_tbl = sm8550_pmdomain_table,
> +	.pmdomain_tbl_size = ARRAY_SIZE(sm8550_pmdomain_table),
> +	.opp_pd_tbl = sm8550_opp_pd_table,
> +	.opp_pd_tbl_size = ARRAY_SIZE(sm8550_opp_pd_table),
> +	.clk_tbl = sm8750_clk_table,
> +	.clk_tbl_size = ARRAY_SIZE(sm8750_clk_table),
> +	/* Upper bound of DMA address range */
> +	.dma_mask = 0xe0000000 - 1,
> +	.fwname = "qcom/vpu/vpu35_4v.mbn",
Could you clarify where this firmware has been merged? Also, it appears
that the naming convention hasn't been followed.
> +	.pas_id = IRIS_PAS_ID,
> +	.inst_caps = &platform_inst_cap_sm8550,
> +	.inst_fw_caps = inst_fw_cap_sm8550,
> +	.inst_fw_caps_size = ARRAY_SIZE(inst_fw_cap_sm8550),
> +	.tz_cp_config_data = &tz_cp_config_sm8550,
> +	.core_arch = VIDEO_ARCH_LX,
> +	.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
> +	.ubwc_config = &ubwc_config_sm8550,
> +	.num_vpp_pipe = 4,
> +	.max_session_count = 16,
> +	.max_core_mbpf = NUM_MBS_8K * 2,
> +	.input_config_params_default =
> +		sm8550_vdec_input_config_params_default,
> +	.input_config_params_default_size =
> +		ARRAY_SIZE(sm8550_vdec_input_config_params_default),
> +	.input_config_params_hevc =
> +		sm8550_vdec_input_config_param_hevc,
> +	.input_config_params_hevc_size =
> +		ARRAY_SIZE(sm8550_vdec_input_config_param_hevc),
> +	.input_config_params_vp9 =
> +		sm8550_vdec_input_config_param_vp9,
> +	.input_config_params_vp9_size =
> +		ARRAY_SIZE(sm8550_vdec_input_config_param_vp9),
> +	.output_config_params =
> +		sm8550_vdec_output_config_params,
> +	.output_config_params_size =
> +		ARRAY_SIZE(sm8550_vdec_output_config_params),
> +	.dec_input_prop = sm8550_vdec_subscribe_input_properties,
> +	.dec_input_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_input_properties),
> +	.dec_output_prop_avc = sm8550_vdec_subscribe_output_properties_avc,
> +	.dec_output_prop_avc_size =
> +		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_avc),
> +	.dec_output_prop_hevc = sm8550_vdec_subscribe_output_properties_hevc,
> +	.dec_output_prop_hevc_size =
> +		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_hevc),
> +	.dec_output_prop_vp9 = sm8550_vdec_subscribe_output_properties_vp9,
> +	.dec_output_prop_vp9_size =
> +		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_vp9),
> +
> +	.dec_ip_int_buf_tbl = sm8550_dec_ip_int_buf_tbl,
> +	.dec_ip_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_ip_int_buf_tbl),
> +	.dec_op_int_buf_tbl = sm8550_dec_op_int_buf_tbl,
> +	.dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_op_int_buf_tbl),
> +};
> +
>  /*
>   * Shares most of SM8550 data except:
>   * - inst_caps to platform_inst_cap_qcs8300
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8750.h b/drivers/media/platform/qcom/iris/iris_platform_sm8750.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..719056656a5baf48a7bced634d2582629333cf5c
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8750.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2025 Linaro Ltd
> + */
> +
> +#ifndef __MEDIA_IRIS_PLATFORM_SM8750_H__
> +#define __MEDIA_IRIS_PLATFORM_SM8750_H__
> +
> +static const char * const sm8750_clk_reset_table[] = {
> +	"bus0", "bus1", "core", "vcodec0_core"
> +};
> +
> +static const struct platform_clk_data sm8750_clk_table[] = {
> +	{IRIS_AXI_CLK,		"iface"			},
> +	{IRIS_CTRL_CLK,		"core"			},
> +	{IRIS_HW_CLK,		"vcodec0_core"		},
> +	{IRIS_AXI1_CLK,		"iface1"		},
> +	{IRIS_CTRL_FREERUN_CLK,	"core_freerun"		},
> +	{IRIS_HW_FREERUN_CLK,	"vcodec0_core_freerun"	},
> +};
> +
> +#endif
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index 4e6e92357968d7419f114cc0ffa9b571bad19e46..5fb936a04155e72f4298cd6760eff6e9d1da6310 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -353,6 +353,10 @@ static const struct of_device_id iris_dt_match[] = {
>  		.compatible = "qcom,sm8650-iris",
>  		.data = &sm8650_data,
>  	},
> +	{
> +		.compatible = "qcom,sm8750-iris",
> +		.data = &sm8750_data,
> +	},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, iris_dt_match);
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> index c235112057aa7b7eab1995737541b7a8276ff18b..b00702a4d6c23258550a77373eb34740e785ef22 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2025 Linaro Ltd
>   */
>  
>  #include <linux/iopoll.h>
> @@ -19,8 +20,11 @@
>  #define WRAPPER_IRIS_CPU_NOC_LPI_CONTROL	(WRAPPER_BASE_OFFS + 0x5C)
>  #define REQ_POWER_DOWN_PREP			BIT(0)
>  #define WRAPPER_IRIS_CPU_NOC_LPI_STATUS		(WRAPPER_BASE_OFFS + 0x60)
> +#define WRAPPER_CORE_POWER_CONTROL		(WRAPPER_BASE_OFFS + 0x84)
>  #define WRAPPER_CORE_CLOCK_CONFIG		(WRAPPER_BASE_OFFS + 0x88)
>  #define CORE_CLK_RUN				0x0
> +/* VPU v3.5 */
> +#define WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0	(WRAPPER_BASE_OFFS + 0x78)
>  
>  #define WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG		(WRAPPER_TZ_BASE_OFFS + 0x14)
>  #define CTL_AXI_CLK_HALT			BIT(0)
> @@ -52,6 +56,8 @@
>  #define AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL	(AON_BASE_OFFS + 0x20)
>  #define NOC_HALT				BIT(0)
>  #define AON_WRAPPER_SPARE			(AON_BASE_OFFS + 0x28)
> +#define AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_CONTROL	(AON_BASE_OFFS + 0x2C)
> +#define AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_STATUS	(AON_BASE_OFFS + 0x30)
>  
>  static bool iris_vpu3x_hw_power_collapsed(struct iris_core *core)
>  {
> @@ -225,6 +231,194 @@ static int iris_vpu33_power_off_controller(struct iris_core *core)
>  	return 0;
>  }
>  
> +static int iris_vpu35_power_on_hw(struct iris_core *core)
> +{
> +	int ret;
> +	u32 val;
> +
> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
> +	if (ret)
> +		return ret;
> +
> +	/* Switch GDSC to SW control */
> +	writel(0x0, core->reg_base + WRAPPER_CORE_POWER_CONTROL);
GDSCs have been transitioned from HW_CTRL to HW_CTRL_TRIGGER, placing them
under software control by default, what is the need of doing this?
> +	ret = readl_poll_timeout(core->reg_base + WRAPPER_CORE_POWER_STATUS,
> +				 val, val & BIT(1), 200, 2000);
> +	if (ret)
> +		goto err_disable_power;
> +
> +	ret = iris_prepare_enable_clock(core, IRIS_AXI_CLK);
> +	if (ret)
> +		goto err_gdsc;
> +
> +	ret = iris_prepare_enable_clock(core, IRIS_HW_FREERUN_CLK);
> +	if (ret)
> +		goto err_disable_axi_clk;
> +
> +	ret = iris_prepare_enable_clock(core, IRIS_HW_CLK);
> +	if (ret)
> +		goto err_disable_hw_free_clk;
> +
> +	ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
> +	if (ret)
> +		goto err_disable_hw_clk;
> +
> +	return 0;
> +
> +err_disable_hw_clk:
> +	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
> +err_disable_hw_free_clk:
> +	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
> +err_disable_axi_clk:
> +	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
> +err_gdsc:
> +	writel(BIT(0), core->reg_base + WRAPPER_CORE_POWER_CONTROL);
> +err_disable_power:
> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
> +
> +	return ret;
> +}
> +
> +static void iris_vpu35_power_off_hw(struct iris_core *core)
> +{
> +	u32 val = 0, value, i;
> +	int ret;
> +
> +	if (iris_vpu3x_hw_power_collapsed(core))
> +		goto disable_power;
> +
> +	value = readl(core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
> +	if (value)
> +		writel(CORE_CLK_RUN, core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
> +
> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
> +					 val, val & 0x400000, 2000, 20000);
> +		if (ret)
> +			goto disable_power;
> +	}
> +
> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
> +				 val, val & BIT(0), 200, 2000);
what are you polling here for?
> +	if (ret)
> +		goto disable_power;
> +
> +	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
Could you share the reference for this sqeunece, this looks half-cooked.
Would recommend following Hardware programmin guide(HPG) for this.
> +	writel(BIT(0), core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
> +
> +	writel(CORE_BRIDGE_SW_RESET | CORE_BRIDGE_HW_RESET_DISABLE,
> +	       core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
> +	writel(CORE_BRIDGE_HW_RESET_DISABLE, core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
> +	writel(0x0, core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
> +
> +disable_power:
> +	dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], false);
> +	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
> +	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
> +	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
> +
> +	writel(BIT(0), core->reg_base + WRAPPER_CORE_POWER_CONTROL);
> +	/*
> +	 * Do not wait for power-down, because hardware might delay it (it
> +	 * always timeouts).
> +	 */
> +
> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
> +}
> +
> +static int iris_vpu35_power_off_controller(struct iris_core *core)
> +{
> +	u32 xo_rst_tbl_size = core->iris_platform_data->controller_rst_tbl_size;
where is controller_rst_tbl_size defined for this SOC?
> +	u32 clk_rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
> +	u32 val = 0;
> +	int ret;
> +
> +	writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, core->reg_base + CPU_CS_X2RPMH);
> +
> +	writel(REQ_POWER_DOWN_PREP, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL);
> +
> +	ret = readl_poll_timeout(core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_STATUS,
> +				 val, val & BIT(0), 200, 2000);
> +	if (ret)
> +		goto disable_power;
> +
> +	writel(0x0, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL);
> +
> +	writel(REQ_POWER_DOWN_PREP, core->reg_base + AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_CONTROL);
> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_STATUS,
> +				 val, val & (BIT(0) | BIT(1) | BIT(2)), 15, 1000);
doesn't seems right and missing retry logic. would recommend referring HPG.
> +	if (ret)
> +		goto disable_power;
> +
> +	writel(0x0, core->reg_base + AON_WRAPPER_MVP_VIDEO_CTL_NOC_LPI_CONTROL);
> +
> +	writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL);
> +
> +	ret = readl_poll_timeout(core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_STATUS,
> +				 val, val == 0, 200, 2000);
> +	if (ret)
> +		goto disable_power;
> +
> +disable_power:
> +	iris_disable_unprepare_clock(core, IRIS_CTRL_CLK);
> +	iris_disable_unprepare_clock(core, IRIS_CTRL_FREERUN_CLK);
> +	iris_disable_unprepare_clock(core, IRIS_AXI1_CLK);
> +
> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
> +
> +	reset_control_bulk_reset(clk_rst_tbl_size, core->resets);
> +
> +	reset_control_bulk_assert(xo_rst_tbl_size, core->controller_resets);
??
> +
> +	usleep_range(400, 500);
> +
> +	reset_control_bulk_deassert(xo_rst_tbl_size, core->controller_resets);
> +
> +	return 0;
> +}
> +
> +static int iris_vpu35_power_on_controller(struct iris_core *core)
> +{
> +	u32 rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
> +	int ret;
> +
> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
> +	if (ret)
> +		return ret;
> +
> +	ret = reset_control_bulk_reset(rst_tbl_size, core->resets);
what is the reference for this?

Thanks,
Dikshita
> +	if (ret)
> +		goto err_disable_power;
> +
> +	ret = iris_prepare_enable_clock(core, IRIS_AXI1_CLK);
> +	if (ret)
> +		goto err_disable_power;
> +
> +	ret = iris_prepare_enable_clock(core, IRIS_CTRL_FREERUN_CLK);
> +	if (ret)
> +		goto err_disable_axi1_clk;
> +
> +	ret = iris_prepare_enable_clock(core, IRIS_CTRL_CLK);
> +	if (ret)
> +		goto err_disable_ctrl_free_clk;
> +
> +	return 0;
> +
> +err_disable_ctrl_free_clk:
> +	iris_disable_unprepare_clock(core, IRIS_CTRL_FREERUN_CLK);
> +err_disable_axi1_clk:
> +	iris_disable_unprepare_clock(core, IRIS_AXI1_CLK);
> +err_disable_power:
> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
> +
> +	return ret;
> +}
> +
> +static void iris_vpu35_program_bootup_registers(struct iris_core *core)
> +{
> +	writel(0x1, core->reg_base + WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0);
> +}
> +
>  static u64 iris_vpu3x_calculate_frequency(struct iris_inst *inst, size_t data_size)
>  {
>  	struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
> @@ -277,3 +471,12 @@ const struct vpu_ops iris_vpu33_ops = {
>  	.power_on_controller = iris_vpu_power_on_controller,
>  	.calc_freq = iris_vpu3x_calculate_frequency,
>  };
> +
> +const struct vpu_ops iris_vpu35_ops = {
> +	.power_off_hw = iris_vpu35_power_off_hw,
> +	.power_on_hw = iris_vpu35_power_on_hw,
> +	.power_off_controller = iris_vpu35_power_off_controller,
> +	.power_on_controller = iris_vpu35_power_on_controller,
> +	.program_bootup_registers = iris_vpu35_program_bootup_registers,
> +	.calc_freq = iris_vpu3x_calculate_frequency,
> +};
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> index 6c51002f72ab3d9e16d5a2a50ac712fac91ae25c..bb98950e018fadf69ac4f41b3037f7fd6ac33c5b 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> @@ -84,6 +84,7 @@ static void iris_vpu_interrupt_init(struct iris_core *core)
>  static void iris_vpu_setup_ucregion_memory_map(struct iris_core *core)
>  {
>  	u32 queue_size, value;
> +	const struct vpu_ops *vpu_ops = core->iris_platform_data->vpu_ops;
>  
>  	/* Iris hardware requires 4K queue alignment */
>  	queue_size = ALIGN(sizeof(struct iris_hfi_queue_table_header) +
> @@ -105,6 +106,9 @@ static void iris_vpu_setup_ucregion_memory_map(struct iris_core *core)
>  		value = (u32)core->sfr_daddr + core->iris_platform_data->core_arch;
>  		writel(value, core->reg_base + SFR_ADDR);
>  	}
> +
> +	if (vpu_ops->program_bootup_registers)
> +		vpu_ops->program_bootup_registers(core);
>  }
>  
>  int iris_vpu_boot_firmware(struct iris_core *core)
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
> index d95b305ca5a89ba8f08aefb6e6acd9ea4a721a8b..d636e287457adf0c44540af5c85cfa69decbca8b 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
> @@ -11,12 +11,14 @@ struct iris_core;
>  extern const struct vpu_ops iris_vpu2_ops;
>  extern const struct vpu_ops iris_vpu3_ops;
>  extern const struct vpu_ops iris_vpu33_ops;
> +extern const struct vpu_ops iris_vpu35_ops;
>  
>  struct vpu_ops {
>  	void (*power_off_hw)(struct iris_core *core);
>  	int (*power_on_hw)(struct iris_core *core);
>  	int (*power_off_controller)(struct iris_core *core);
>  	int (*power_on_controller)(struct iris_core *core);
> +	void (*program_bootup_registers)(struct iris_core *core);
>  	u64 (*calc_freq)(struct iris_inst *inst, size_t data_size);
>  };
>  
> 

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Krzysztof Kozlowski]

On 16/07/2025 11:10, Dikshita Agarwal wrote:
> 
> 
> On 7/14/2025 7:11 PM, Krzysztof Kozlowski wrote:
>> Add support for SM8750 Iris codec with major differences against
>> previous generation SM8650:
>>
>> 1. New clocks and new resets, thus new power up and power down
>>    sequences,
>>
>> 2. New WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0 register programmed
>>    during boot-up
>>



Please kindly trim the replies from unnecessary context. It makes it
much easier to find new content.


>> +struct iris_platform_data sm8750_data = {
>> +	.get_instance = iris_hfi_gen2_get_instance,
>> +	.init_hfi_command_ops = iris_hfi_gen2_command_ops_init,
>> +	.init_hfi_response_ops = iris_hfi_gen2_response_ops_init,
>> +	.vpu_ops = &iris_vpu35_ops,
>> +	.set_preset_registers = iris_set_sm8550_preset_registers,
>> +	.icc_tbl = sm8550_icc_table,
>> +	.icc_tbl_size = ARRAY_SIZE(sm8550_icc_table),
>> +	.clk_rst_tbl = sm8750_clk_reset_table,
>> +	.clk_rst_tbl_size = ARRAY_SIZE(sm8750_clk_reset_table),
>> +	.bw_tbl_dec = sm8550_bw_table_dec,
>> +	.bw_tbl_dec_size = ARRAY_SIZE(sm8550_bw_table_dec),
>> +	.pmdomain_tbl = sm8550_pmdomain_table,
>> +	.pmdomain_tbl_size = ARRAY_SIZE(sm8550_pmdomain_table),
>> +	.opp_pd_tbl = sm8550_opp_pd_table,
>> +	.opp_pd_tbl_size = ARRAY_SIZE(sm8550_opp_pd_table),
>> +	.clk_tbl = sm8750_clk_table,
>> +	.clk_tbl_size = ARRAY_SIZE(sm8750_clk_table),
>> +	/* Upper bound of DMA address range */
>> +	.dma_mask = 0xe0000000 - 1,
>> +	.fwname = "qcom/vpu/vpu35_4v.mbn",
> Could you clarify where this firmware has been merged? Also, it appears
> that the naming convention hasn't been followed.


I mentioned in the DTS patchset but not here, so I will add it in the
cover letter - firmware is not released. About the name I cannot
comment, that's the name I got from qcom. Happy to use whatever name you
prefer.



>> +static int iris_vpu35_power_on_hw(struct iris_core *core)
>> +{
>> +	int ret;
>> +	u32 val;
>> +
>> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Switch GDSC to SW control */
>> +	writel(0x0, core->reg_base + WRAPPER_CORE_POWER_CONTROL);
> GDSCs have been transitioned from HW_CTRL to HW_CTRL_TRIGGER, placing them
> under software control by default, what is the need of doing this?
>> +	ret = readl_poll_timeout(core->reg_base + WRAPPER_CORE_POWER_STATUS,
>> +				 val, val & BIT(1), 200, 2000);


The need comes from differences between this and previous generation,
mostly based on downstream sources. I think the hardware just did not
boot up without it.

You need to fix your email client to add line breaks around your
replies, because it is very difficult to spot them. It's close to
impossible...


>> +	if (ret)
>> +		goto err_disable_power;
>> +
>> +	ret = iris_prepare_enable_clock(core, IRIS_AXI_CLK);
>> +	if (ret)
>> +		goto err_gdsc;
>> +
>> +	ret = iris_prepare_enable_clock(core, IRIS_HW_FREERUN_CLK);
>> +	if (ret)
>> +		goto err_disable_axi_clk;
>> +
>> +	ret = iris_prepare_enable_clock(core, IRIS_HW_CLK);
>> +	if (ret)
>> +		goto err_disable_hw_free_clk;
>> +
>> +	ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
>> +	if (ret)
>> +		goto err_disable_hw_clk;
>> +
>> +	return 0;
>> +
>> +err_disable_hw_clk:
>> +	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
>> +err_disable_hw_free_clk:
>> +	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
>> +err_disable_axi_clk:
>> +	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
>> +err_gdsc:
>> +	writel(BIT(0), core->reg_base + WRAPPER_CORE_POWER_CONTROL);
>> +err_disable_power:
>> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>> +
>> +	return ret;
>> +}
>> +
>> +static void iris_vpu35_power_off_hw(struct iris_core *core)
>> +{
>> +	u32 val = 0, value, i;
>> +	int ret;
>> +
>> +	if (iris_vpu3x_hw_power_collapsed(core))
>> +		goto disable_power;
>> +
>> +	value = readl(core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
>> +	if (value)
>> +		writel(CORE_CLK_RUN, core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
>> +
>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>> +					 val, val & 0x400000, 2000, 20000);
>> +		if (ret)
>> +			goto disable_power;
>> +	}
>> +
>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>> +				 val, val & BIT(0), 200, 2000);
> what are you polling here for?


This is not different than existing code. I don't understand why you are
commenting on something which is already there.

>> +	if (ret)
>> +		goto disable_power;
>> +
>> +	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
> Could you share the reference for this sqeunece, this looks half-cooked.
> Would recommend following Hardware programmin guide(HPG) for this.


Why? Look at existing code. It's the same.

I think I responded to all your comments - it barely possible to spot
them in the quote.

Best regards,
Krzysztof

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Dikshita Agarwal]

On 7/16/2025 2:58 PM, Krzysztof Kozlowski wrote:
> On 16/07/2025 11:10, Dikshita Agarwal wrote:
>>
>>
>> On 7/14/2025 7:11 PM, Krzysztof Kozlowski wrote:
>>> Add support for SM8750 Iris codec with major differences against
>>> previous generation SM8650:
>>>
>>> 1. New clocks and new resets, thus new power up and power down
>>>    sequences,
>>>
>>> 2. New WRAPPER_IRIS_VCODEC_VPU_WRAPPER_SPARE_0 register programmed
>>>    during boot-up
>>>
> 
> 
> 
> Please kindly trim the replies from unnecessary context. It makes it
> much easier to find new content.
> 
> 
>>> +struct iris_platform_data sm8750_data = {
>>> +	.get_instance = iris_hfi_gen2_get_instance,
>>> +	.init_hfi_command_ops = iris_hfi_gen2_command_ops_init,
>>> +	.init_hfi_response_ops = iris_hfi_gen2_response_ops_init,
>>> +	.vpu_ops = &iris_vpu35_ops,
>>> +	.set_preset_registers = iris_set_sm8550_preset_registers,
>>> +	.icc_tbl = sm8550_icc_table,
>>> +	.icc_tbl_size = ARRAY_SIZE(sm8550_icc_table),
>>> +	.clk_rst_tbl = sm8750_clk_reset_table,
>>> +	.clk_rst_tbl_size = ARRAY_SIZE(sm8750_clk_reset_table),
>>> +	.bw_tbl_dec = sm8550_bw_table_dec,
>>> +	.bw_tbl_dec_size = ARRAY_SIZE(sm8550_bw_table_dec),
>>> +	.pmdomain_tbl = sm8550_pmdomain_table,
>>> +	.pmdomain_tbl_size = ARRAY_SIZE(sm8550_pmdomain_table),
>>> +	.opp_pd_tbl = sm8550_opp_pd_table,
>>> +	.opp_pd_tbl_size = ARRAY_SIZE(sm8550_opp_pd_table),
>>> +	.clk_tbl = sm8750_clk_table,
>>> +	.clk_tbl_size = ARRAY_SIZE(sm8750_clk_table),
>>> +	/* Upper bound of DMA address range */
>>> +	.dma_mask = 0xe0000000 - 1,
>>> +	.fwname = "qcom/vpu/vpu35_4v.mbn",
>> Could you clarify where this firmware has been merged? Also, it appears
>> that the naming convention hasn't been followed.
> 
> 
> I mentioned in the DTS patchset but not here, so I will add it in the
> cover letter - firmware is not released. About the name I cannot
> comment, that's the name I got from qcom. Happy to use whatever name you
> prefer.
> 


You can name it vpu35_p4.mbn to maintain consistency with the current
naming convention.


> 
> 
>>> +static int iris_vpu35_power_on_hw(struct iris_core *core)
>>> +{
>>> +	int ret;
>>> +	u32 val;
>>> +
>>> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	/* Switch GDSC to SW control */
>>> +	writel(0x0, core->reg_base + WRAPPER_CORE_POWER_CONTROL);
>> GDSCs have been transitioned from HW_CTRL to HW_CTRL_TRIGGER, placing them
>> under software control by default, what is the need of doing this?
>>> +	ret = readl_poll_timeout(core->reg_base + WRAPPER_CORE_POWER_STATUS,
>>> +				 val, val & BIT(1), 200, 2000);
> 
> 
> The need comes from differences between this and previous generation,


which previous generation you’re referring to?
HW_CTRL_TRIGGER is supported on SM8550 and all later SOCs, and if you look
at videocc changes, same applies to SM8750 as well.



> mostly based on downstream sources. I think the hardware just did not
> boot up without it.


That shouldn’t be the case. The downstream design is different, which is
why the driver requires the above code to move the GDSC to software control
before enabling the clock. With HW_CTRL_TRIGGER, this step isn’t needed, so
the above code is unnecessary.


> 
> You need to fix your email client to add line breaks around your
> replies, because it is very difficult to spot them. It's close to
> impossible...
> 
> 
>>> +	if (ret)
>>> +		goto err_disable_power;
>>> +
>>> +	ret = iris_prepare_enable_clock(core, IRIS_AXI_CLK);
>>> +	if (ret)
>>> +		goto err_gdsc;
>>> +
>>> +	ret = iris_prepare_enable_clock(core, IRIS_HW_FREERUN_CLK);
>>> +	if (ret)
>>> +		goto err_disable_axi_clk;
>>> +
>>> +	ret = iris_prepare_enable_clock(core, IRIS_HW_CLK);
>>> +	if (ret)
>>> +		goto err_disable_hw_free_clk;
>>> +
>>> +	ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
>>> +	if (ret)
>>> +		goto err_disable_hw_clk;
>>> +
>>> +	return 0;
>>> +
>>> +err_disable_hw_clk:
>>> +	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
>>> +err_disable_hw_free_clk:
>>> +	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
>>> +err_disable_axi_clk:
>>> +	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
>>> +err_gdsc:
>>> +	writel(BIT(0), core->reg_base + WRAPPER_CORE_POWER_CONTROL);
>>> +err_disable_power:
>>> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static void iris_vpu35_power_off_hw(struct iris_core *core)
>>> +{
>>> +	u32 val = 0, value, i;
>>> +	int ret;
>>> +
>>> +	if (iris_vpu3x_hw_power_collapsed(core))
>>> +		goto disable_power;
>>> +
>>> +	value = readl(core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
>>> +	if (value)
>>> +		writel(CORE_CLK_RUN, core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
>>> +
>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>> +					 val, val & 0x400000, 2000, 20000);
>>> +		if (ret)
>>> +			goto disable_power;
>>> +	}
>>> +
>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>> +				 val, val & BIT(0), 200, 2000);
>> what are you polling here for?
> 
> 
> This is not different than existing code. I don't understand why you are
> commenting on something which is already there.

Which code are you referring to?

You are not setting AON_WRAPPER_MVP_NOC_LPI_CONTROL and polling for its status.

The current code is incomplete and missing several steps.
Please review and provide a corrected version.


> 
>>> +	if (ret)
>>> +		goto disable_power;
>>> +
>>> +	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
>> Could you share the reference for this sqeunece, this looks half-cooked.
>> Would recommend following Hardware programmin guide(HPG) for this.
> 
> 
> Why? Look at existing code. It's the same.


Which existing code? Please be specific.
I don't think you referred to downstream code for this, because I see a lot
of missing pieces here.


> 
> I think I responded to all your comments - it barely possible to spot
> them in the quote.
> 


No, you have missed some of the later comments. Since the code is snipped,
I can’t point out those comments here.


Thanks,
Dikshita

> Best regards,
> Krzysztof

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Krzysztof Kozlowski]

On 17/07/2025 09:37, Dikshita Agarwal wrote:
>>>> +	.clk_tbl_size = ARRAY_SIZE(sm8750_clk_table),
>>>> +	/* Upper bound of DMA address range */
>>>> +	.dma_mask = 0xe0000000 - 1,
>>>> +	.fwname = "qcom/vpu/vpu35_4v.mbn",
>>> Could you clarify where this firmware has been merged? Also, it appears
>>> that the naming convention hasn't been followed.
>>
>>
>> I mentioned in the DTS patchset but not here, so I will add it in the
>> cover letter - firmware is not released. About the name I cannot
>> comment, that's the name I got from qcom. Happy to use whatever name you
>> prefer.
>>
> 
> 
> You can name it vpu35_p4.mbn to maintain consistency with the current
> naming convention.


Sure.

> 
> 
>>
>>
>>>> +static int iris_vpu35_power_on_hw(struct iris_core *core)
>>>> +{
>>>> +	int ret;
>>>> +	u32 val;
>>>> +
>>>> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	/* Switch GDSC to SW control */
>>>> +	writel(0x0, core->reg_base + WRAPPER_CORE_POWER_CONTROL);
>>> GDSCs have been transitioned from HW_CTRL to HW_CTRL_TRIGGER, placing them
>>> under software control by default, what is the need of doing this?
>>>> +	ret = readl_poll_timeout(core->reg_base + WRAPPER_CORE_POWER_STATUS,
>>>> +				 val, val & BIT(1), 200, 2000);
>>
>>
>> The need comes from differences between this and previous generation,
> 
> 
> which previous generation you’re referring to?


The one I mentioned in the commit msg - SM8650.

> HW_CTRL_TRIGGER is supported on SM8550 and all later SOCs, and if you look
> at videocc changes, same applies to SM8750 as well.
> 
> 
> 
>> mostly based on downstream sources. I think the hardware just did not
>> boot up without it.
> 
> 
> That shouldn’t be the case. The downstream design is different, which is
> why the driver requires the above code to move the GDSC to software control
> before enabling the clock. With HW_CTRL_TRIGGER, this step isn’t needed, so
> the above code is unnecessary.
> 
> 
>>
>> You need to fix your email client to add line breaks around your
>> replies, because it is very difficult to spot them. It's close to
>> impossible...
>>
>>
>>>> +	if (ret)
>>>> +		goto err_disable_power;
>>>> +
>>>> +	ret = iris_prepare_enable_clock(core, IRIS_AXI_CLK);
>>>> +	if (ret)
>>>> +		goto err_gdsc;
>>>> +
>>>> +	ret = iris_prepare_enable_clock(core, IRIS_HW_FREERUN_CLK);
>>>> +	if (ret)
>>>> +		goto err_disable_axi_clk;
>>>> +
>>>> +	ret = iris_prepare_enable_clock(core, IRIS_HW_CLK);
>>>> +	if (ret)
>>>> +		goto err_disable_hw_free_clk;
>>>> +
>>>> +	ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
>>>> +	if (ret)
>>>> +		goto err_disable_hw_clk;
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err_disable_hw_clk:
>>>> +	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
>>>> +err_disable_hw_free_clk:
>>>> +	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
>>>> +err_disable_axi_clk:
>>>> +	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
>>>> +err_gdsc:
>>>> +	writel(BIT(0), core->reg_base + WRAPPER_CORE_POWER_CONTROL);
>>>> +err_disable_power:
>>>> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static void iris_vpu35_power_off_hw(struct iris_core *core)
>>>> +{
>>>> +	u32 val = 0, value, i;
>>>> +	int ret;
>>>> +
>>>> +	if (iris_vpu3x_hw_power_collapsed(core))
>>>> +		goto disable_power;
>>>> +
>>>> +	value = readl(core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
>>>> +	if (value)
>>>> +		writel(CORE_CLK_RUN, core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
>>>> +
>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>> +					 val, val & 0x400000, 2000, 20000);
>>>> +		if (ret)
>>>> +			goto disable_power;
>>>> +	}
>>>> +
>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>> +				 val, val & BIT(0), 200, 2000);
>>> what are you polling here for?
>>
>>
>> This is not different than existing code. I don't understand why you are
>> commenting on something which is already there.
> 
> Which code are you referring to?

To the existing vpu33 which had Reviewed-by: Vikash Garodia
<quic_vgarodia@quicinc.com>

You understand that everything here is the same, everything is a copy
while adding just few more things?

My patch is not doing in this respect anything different that what you
reviewed.


> 
> You are not setting AON_WRAPPER_MVP_NOC_LPI_CONTROL and polling for its status.

True, neither old reviewed code has done. I am not changing or fixing
any existing logic, I am only adding new clocks and resets.

> 
> The current code is incomplete and missing several steps.

Current you mean what was:
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
?

> Please review and provide a corrected version.
> 
> 
>>
>>>> +	if (ret)
>>>> +		goto disable_power;
>>>> +
>>>> +	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
>>> Could you share the reference for this sqeunece, this looks half-cooked.
>>> Would recommend following Hardware programmin guide(HPG) for this.
>>
>>
>> Why? Look at existing code. It's the same.
> 
> 
> Which existing code? Please be specific.


Existing upstream VPU33 which this builts on top of. And that existing
upstream VPU33 was Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>

> I don't think you referred to downstream code for this, because I see a lot
> of missing pieces here.
> 
> 
>>
>> I think I responded to all your comments - it barely possible to spot
>> them in the quote.
>>
> 
> 
> No, you have missed some of the later comments. Since the code is snipped,
> I can’t point out those comments here.


It's impossible to find them in the original response.


Best regards,
Krzysztof

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Krzysztof Kozlowski]

On 17/07/2025 09:37, Dikshita Agarwal wrote:
> 
> 
>> mostly based on downstream sources. I think the hardware just did not
>> boot up without it.
> 
> 
> That shouldn’t be the case. The downstream design is different, which is
> why the driver requires the above code to move the GDSC to software control
> before enabling the clock. With HW_CTRL_TRIGGER, this step isn’t needed, so
> the above code is unnecessary.

Ack.

Best regards,
Krzysztof

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Dikshita Agarwal]

On 7/17/2025 3:04 PM, Krzysztof Kozlowski wrote:
> On 17/07/2025 09:37, Dikshita Agarwal wrote:
>>>>> +	.clk_tbl_size = ARRAY_SIZE(sm8750_clk_table),
>>>>> +	/* Upper bound of DMA address range */
>>>>> +	.dma_mask = 0xe0000000 - 1,
>>>>> +	.fwname = "qcom/vpu/vpu35_4v.mbn",
>>>> Could you clarify where this firmware has been merged? Also, it appears
>>>> that the naming convention hasn't been followed.
>>>
>>>
>>> I mentioned in the DTS patchset but not here, so I will add it in the
>>> cover letter - firmware is not released. About the name I cannot
>>> comment, that's the name I got from qcom. Happy to use whatever name you
>>> prefer.
>>>
>>
>>
>> You can name it vpu35_p4.mbn to maintain consistency with the current
>> naming convention.
> 
> 
> Sure.
> 
>>
>>
>>>
>>>
>>>>> +static int iris_vpu35_power_on_hw(struct iris_core *core)
>>>>> +{
>>>>> +	int ret;
>>>>> +	u32 val;
>>>>> +
>>>>> +	ret = iris_enable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>>>>> +	if (ret)
>>>>> +		return ret;
>>>>> +
>>>>> +	/* Switch GDSC to SW control */
>>>>> +	writel(0x0, core->reg_base + WRAPPER_CORE_POWER_CONTROL);
>>>> GDSCs have been transitioned from HW_CTRL to HW_CTRL_TRIGGER, placing them
>>>> under software control by default, what is the need of doing this?
>>>>> +	ret = readl_poll_timeout(core->reg_base + WRAPPER_CORE_POWER_STATUS,
>>>>> +				 val, val & BIT(1), 200, 2000);
>>>
>>>
>>> The need comes from differences between this and previous generation,
>>
>>
>> which previous generation you’re referring to?
> 
> 
> The one I mentioned in the commit msg - SM8650.
> 
>> HW_CTRL_TRIGGER is supported on SM8550 and all later SOCs, and if you look
>> at videocc changes, same applies to SM8750 as well.
>>
>>
>>
>>> mostly based on downstream sources. I think the hardware just did not
>>> boot up without it.
>>
>>
>> That shouldn’t be the case. The downstream design is different, which is
>> why the driver requires the above code to move the GDSC to software control
>> before enabling the clock. With HW_CTRL_TRIGGER, this step isn’t needed, so
>> the above code is unnecessary.
>>
>>
>>>
>>> You need to fix your email client to add line breaks around your
>>> replies, because it is very difficult to spot them. It's close to
>>> impossible...
>>>
>>>
>>>>> +	if (ret)
>>>>> +		goto err_disable_power;
>>>>> +
>>>>> +	ret = iris_prepare_enable_clock(core, IRIS_AXI_CLK);
>>>>> +	if (ret)
>>>>> +		goto err_gdsc;
>>>>> +
>>>>> +	ret = iris_prepare_enable_clock(core, IRIS_HW_FREERUN_CLK);
>>>>> +	if (ret)
>>>>> +		goto err_disable_axi_clk;
>>>>> +
>>>>> +	ret = iris_prepare_enable_clock(core, IRIS_HW_CLK);
>>>>> +	if (ret)
>>>>> +		goto err_disable_hw_free_clk;
>>>>> +
>>>>> +	ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
>>>>> +	if (ret)
>>>>> +		goto err_disable_hw_clk;
>>>>> +
>>>>> +	return 0;
>>>>> +
>>>>> +err_disable_hw_clk:
>>>>> +	iris_disable_unprepare_clock(core, IRIS_HW_CLK);
>>>>> +err_disable_hw_free_clk:
>>>>> +	iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
>>>>> +err_disable_axi_clk:
>>>>> +	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
>>>>> +err_gdsc:
>>>>> +	writel(BIT(0), core->reg_base + WRAPPER_CORE_POWER_CONTROL);
>>>>> +err_disable_power:
>>>>> +	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
>>>>> +
>>>>> +	return ret;
>>>>> +}
>>>>> +
>>>>> +static void iris_vpu35_power_off_hw(struct iris_core *core)
>>>>> +{
>>>>> +	u32 val = 0, value, i;
>>>>> +	int ret;
>>>>> +
>>>>> +	if (iris_vpu3x_hw_power_collapsed(core))
>>>>> +		goto disable_power;
>>>>> +
>>>>> +	value = readl(core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
>>>>> +	if (value)
>>>>> +		writel(CORE_CLK_RUN, core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
>>>>> +
>>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>>> +					 val, val & 0x400000, 2000, 20000);
>>>>> +		if (ret)
>>>>> +			goto disable_power;
>>>>> +	}
>>>>> +
>>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>>> +				 val, val & BIT(0), 200, 2000);
>>>> what are you polling here for?
>>>
>>>
>>> This is not different than existing code. I don't understand why you are
>>> commenting on something which is already there.
>>
>> Which code are you referring to?
> 
> To the existing vpu33 which had Reviewed-by: Vikash Garodia
> <quic_vgarodia@quicinc.com>
> 
> You understand that everything here is the same, everything is a copy
> while adding just few more things?
> 
> My patch is not doing in this respect anything different that what you
> reviewed.
> 

It seems to have been missed in vpu33 power off sequence as well and should
be fixed.

Still, as mentioned earlier as well, your reference should be
HPG/downstream driver of SM8750 not the previous generation (SM8650).

Thanks,
Dikshita

> 
>>
>> You are not setting AON_WRAPPER_MVP_NOC_LPI_CONTROL and polling for its status.
> 
> True, neither old reviewed code has done. I am not changing or fixing
> any existing logic, I am only adding new clocks and resets.
> 
>>
>> The current code is incomplete and missing several steps.
> 
> Current you mean what was:
> Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> ?
> 
>> Please review and provide a corrected version.
>>
>>
>>>
>>>>> +	if (ret)
>>>>> +		goto disable_power;
>>>>> +
>>>>> +	/* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
>>>> Could you share the reference for this sqeunece, this looks half-cooked.
>>>> Would recommend following Hardware programmin guide(HPG) for this.
>>>
>>>
>>> Why? Look at existing code. It's the same.
>>
>>
>> Which existing code? Please be specific.
> 
> 
> Existing upstream VPU33 which this builts on top of. And that existing
> upstream VPU33 was Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> 
>> I don't think you referred to downstream code for this, because I see a lot
>> of missing pieces here.
>>
>>
>>>
>>> I think I responded to all your comments - it barely possible to spot
>>> them in the quote.
>>>
>>
>>
>> No, you have missed some of the later comments. Since the code is snipped,
>> I can’t point out those comments here.
> 
> 
> It's impossible to find them in the original response.
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Krzysztof Kozlowski]

On 17/07/2025 12:50, Dikshita Agarwal wrote:
>>>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>>>> +					 val, val & 0x400000, 2000, 20000);
>>>>>> +		if (ret)
>>>>>> +			goto disable_power;
>>>>>> +	}
>>>>>> +
>>>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>>>> +				 val, val & BIT(0), 200, 2000);
>>>>> what are you polling here for?
>>>>
>>>>
>>>> This is not different than existing code. I don't understand why you are
>>>> commenting on something which is already there.
>>>
>>> Which code are you referring to?
>>
>> To the existing vpu33 which had Reviewed-by: Vikash Garodia
>> <quic_vgarodia@quicinc.com>
>>
>> You understand that everything here is the same, everything is a copy
>> while adding just few more things?
>>
>> My patch is not doing in this respect anything different that what you
>> reviewed.
>>
> 
> It seems to have been missed in vpu33 power off sequence as well and should
> be fixed.
> 
> Still, as mentioned earlier as well, your reference should be
> HPG/downstream driver of SM8750 not the previous generation (SM8650).

Yes and partially no, because we write upstream code matching or
extending existing upstream driver. As you said earlier, downstream is
not the truth always:

"That shouldn’t be the case. The downstream design is different, which
is why the driver requires the above code to move the GDSC"

so here I built on top of SM8650 and re-iterate whatever mistakes are
there. The best if someone fixes VPU33 and then I rebase on top,
re-using fixed code as my base.

Best regards,
Krzysztof

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Vikash Garodia]

On 7/17/2025 4:24 PM, Krzysztof Kozlowski wrote:
> On 17/07/2025 12:50, Dikshita Agarwal wrote:
>>>>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>>>>> +					 val, val & 0x400000, 2000, 20000);
>>>>>>> +		if (ret)
>>>>>>> +			goto disable_power;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>>>>> +				 val, val & BIT(0), 200, 2000);
>>>>>> what are you polling here for?
>>>>>
>>>>>
>>>>> This is not different than existing code. I don't understand why you are
>>>>> commenting on something which is already there.
>>>>
>>>> Which code are you referring to?
>>>
>>> To the existing vpu33 which had Reviewed-by: Vikash Garodia
>>> <quic_vgarodia@quicinc.com>
>>>
>>> You understand that everything here is the same, everything is a copy
>>> while adding just few more things?
>>>
>>> My patch is not doing in this respect anything different that what you
>>> reviewed.
>>>
>>
>> It seems to have been missed in vpu33 power off sequence as well and should
>> be fixed.
>>
>> Still, as mentioned earlier as well, your reference should be
>> HPG/downstream driver of SM8750 not the previous generation (SM8650).
> 
> Yes and partially no, because we write upstream code matching or
> extending existing upstream driver. As you said earlier, downstream is
> not the truth always:
> 
> "That shouldn’t be the case. The downstream design is different, which
> is why the driver requires the above code to move the GDSC"
> 
> so here I built on top of SM8650 and re-iterate whatever mistakes are
> there. The best if someone fixes VPU33 and then I rebase on top,
> re-using fixed code as my base.

You have mixed different comments made earlier.

1. Downstream GDSCs are still in HW_CTRL mode, while upstream GDSCs are migrated
to HW_CTRL_TRIGGER. This does not need a fix in SM8650, but in the
"iris_vpu35_power_on_hw" which you have added in this patch for SM8750.

2. Register write "AON_WRAPPER_MVP_NOC_LPI_CONTROL" with 0x1 is needed on both
SM8650 and SM8750, before polling AON_WRAPPER_MVP_NOC_LPI_STATUS in
"iris_vpu35_power_off_hw" function.

I can soon submit a patch to fix SM8650 with the missing register write, but i
do not see a need to wait for it to continue your development on SM8750.

Regards,
Vikash

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Dikshita Agarwal]

On 7/17/2025 4:24 PM, Krzysztof Kozlowski wrote:
> On 17/07/2025 12:50, Dikshita Agarwal wrote:
>>>>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>>>>> +					 val, val & 0x400000, 2000, 20000);
>>>>>>> +		if (ret)
>>>>>>> +			goto disable_power;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>>>>> +				 val, val & BIT(0), 200, 2000);
>>>>>> what are you polling here for?
>>>>>
>>>>>
>>>>> This is not different than existing code. I don't understand why you are
>>>>> commenting on something which is already there.
>>>>
>>>> Which code are you referring to?
>>>
>>> To the existing vpu33 which had Reviewed-by: Vikash Garodia
>>> <quic_vgarodia@quicinc.com>
>>>
>>> You understand that everything here is the same, everything is a copy
>>> while adding just few more things?
>>>
>>> My patch is not doing in this respect anything different that what you
>>> reviewed.
>>>
>>
>> It seems to have been missed in vpu33 power off sequence as well and should
>> be fixed.
>>
>> Still, as mentioned earlier as well, your reference should be
>> HPG/downstream driver of SM8750 not the previous generation (SM8650).
> 
> Yes and partially no, because we write upstream code matching or
> extending existing upstream driver. As you said earlier, downstream is
> not the truth always:

You're writing the power sequence for a new generation, so referencing the
previous generation is totally wrong. Power sequences can vary between
generations — that's precisely why HPG exists.

I've already pointed this out multiple times, but let me reiterate one last
time:
The current power sequence code is incomplete.
Copying the SM8650 code to SM8750 is not appropriate — it's the wrong
reference.

Regards,
Dikshita

> 
> "That shouldn’t be the case. The downstream design is different, which
> is why the driver requires the above code to move the GDSC"
> 
> so here I built on top of SM8650 and re-iterate whatever mistakes are
> there. The best if someone fixes VPU33 and then I rebase on top,
> re-using fixed code as my base.
> > Best regards,
> Krzysztof

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Krzysztof Kozlowski]

On 17/07/2025 14:22, Dikshita Agarwal wrote:
> 
> 
> On 7/17/2025 4:24 PM, Krzysztof Kozlowski wrote:
>> On 17/07/2025 12:50, Dikshita Agarwal wrote:
>>>>>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>>>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>>>>>> +					 val, val & 0x400000, 2000, 20000);
>>>>>>>> +		if (ret)
>>>>>>>> +			goto disable_power;
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>>>>>> +				 val, val & BIT(0), 200, 2000);
>>>>>>> what are you polling here for?
>>>>>>
>>>>>>
>>>>>> This is not different than existing code. I don't understand why you are
>>>>>> commenting on something which is already there.
>>>>>
>>>>> Which code are you referring to?
>>>>
>>>> To the existing vpu33 which had Reviewed-by: Vikash Garodia
>>>> <quic_vgarodia@quicinc.com>
>>>>
>>>> You understand that everything here is the same, everything is a copy
>>>> while adding just few more things?
>>>>
>>>> My patch is not doing in this respect anything different that what you
>>>> reviewed.
>>>>
>>>
>>> It seems to have been missed in vpu33 power off sequence as well and should
>>> be fixed.
>>>
>>> Still, as mentioned earlier as well, your reference should be
>>> HPG/downstream driver of SM8750 not the previous generation (SM8650).
>>
>> Yes and partially no, because we write upstream code matching or
>> extending existing upstream driver. As you said earlier, downstream is
>> not the truth always:
> 
> You're writing the power sequence for a new generation, so referencing the
> previous generation is totally wrong. Power sequences can vary between


No. I am extending existing source code in hopefully compatible or
matching style.

We do not follow here downstream approaches of reimplementing everything
from scratch on every new generation.

> generations — that's precisely why HPG exists.
> 
> I've already pointed this out multiple times, but let me reiterate one last
> time:
> The current power sequence code is incomplete.
> Copying the SM8650 code to SM8750 is not appropriate — it's the wrong
> reference.
You only pointed out missing AON_WRAPPER_MVP_NOC_LPI_CONTROL, so
"multiple times" is not accurate.


Best regards,
Krzysztof

Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

[Krzysztof Kozlowski]

On 17/07/2025 14:02, Vikash Garodia wrote:
> 
> On 7/17/2025 4:24 PM, Krzysztof Kozlowski wrote:
>> On 17/07/2025 12:50, Dikshita Agarwal wrote:
>>>>>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>>>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>>>>>> +					 val, val & 0x400000, 2000, 20000);
>>>>>>>> +		if (ret)
>>>>>>>> +			goto disable_power;
>>>>>>>> +	}
>>>>>>>> +
>>>>>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>>>>>> +				 val, val & BIT(0), 200, 2000);
>>>>>>> what are you polling here for?
>>>>>>
>>>>>>
>>>>>> This is not different than existing code. I don't understand why you are
>>>>>> commenting on something which is already there.
>>>>>
>>>>> Which code are you referring to?
>>>>
>>>> To the existing vpu33 which had Reviewed-by: Vikash Garodia
>>>> <quic_vgarodia@quicinc.com>
>>>>
>>>> You understand that everything here is the same, everything is a copy
>>>> while adding just few more things?
>>>>
>>>> My patch is not doing in this respect anything different that what you
>>>> reviewed.
>>>>
>>>
>>> It seems to have been missed in vpu33 power off sequence as well and should
>>> be fixed.
>>>
>>> Still, as mentioned earlier as well, your reference should be
>>> HPG/downstream driver of SM8750 not the previous generation (SM8650).
>>
>> Yes and partially no, because we write upstream code matching or
>> extending existing upstream driver. As you said earlier, downstream is
>> not the truth always:
>>
>> "That shouldn’t be the case. The downstream design is different, which
>> is why the driver requires the above code to move the GDSC"
>>
>> so here I built on top of SM8650 and re-iterate whatever mistakes are
>> there. The best if someone fixes VPU33 and then I rebase on top,
>> re-using fixed code as my base.
> 
> You have mixed different comments made earlier.


I did not mix. I used them here to show how pointless arguments you keep
making instead of focusing on technical aspects.

Once you say that, other place you say something else.

> 
> 1. Downstream GDSCs are still in HW_CTRL mode, while upstream GDSCs are migrated
> to HW_CTRL_TRIGGER. This does not need a fix in SM8650, but in the
> "iris_vpu35_power_on_hw" which you have added in this patch for SM8750.

No one discusses this.

> 
> 2. Register write "AON_WRAPPER_MVP_NOC_LPI_CONTROL" with 0x1 is needed on both
> SM8650 and SM8750, before polling AON_WRAPPER_MVP_NOC_LPI_STATUS in
> "iris_vpu35_power_off_hw" function.
> 
> I can soon submit a patch to fix SM8650 with the missing register write, but i

Great!

> do not see a need to wait for it to continue your development on SM8750.


I am not sure if you both understand how upstream development works. We
reduce to a reasonable minimum duplicated codes and different solutions,
so that's why my code is a copy of existing code plus new things for SM8750.

The goal of upstream is not to implement SM8750 completely different.
Please switch downstream approach to above re-usage approach.

And that's why your fix is important because I am going to copy exactly
that part of code and I should not come with different code.


Best regards,
Krzysztof