[PATCH v4 0/6] media: qcom: iris: add support for SM8650

[PATCH v4 0/6] media: qcom: iris: add support for SM8650

[Neil Armstrong]

Add support for the IRIS accelerator for the SM8650
platform, which uses the iris33 hardware.

The vpu33 requires a different reset & poweroff sequence
in order to properly get out of runtime suspend.

Based on the downstream implementation at:
- https://git.codelinaro.org/clo/la/platform/vendor/opensource/video-driver/
  branch video-kernel.lnx.4.0.r4-rel

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v4:
- collected tags
- un-split power_off in vpu3x
- removed useless function defines
- added back vpu3x disappeared rename commit
- Link to v3: https://lore.kernel.org/r/20250407-topic-sm8x50-iris-v10-v3-0-63569f6d04aa@linaro.org

Changes in v3:
- Collected review tags
- Removed bulky reset_controller ops
- Removed iris_vpu_power_off_controller split
- Link to v2: https://lore.kernel.org/r/20250305-topic-sm8x50-iris-v10-v2-0-bd65a3fc099e@linaro.org

Changes in v2:
- Collected bindings review
- Reworked rest handling by adding a secondary optional table to be used by controller poweroff
- Reworked power_off_controller to be reused and extended by vpu33 support
- Removed useless and unneeded vpu33 init
- Moved vpu33 into vpu3x files to reuse code from vpu3
- Moved sm8650 data table into sm8550
- Link to v1: https://lore.kernel.org/r/20250225-topic-sm8x50-iris-v10-v1-0-128ef05d9665@linaro.org

---
Neil Armstrong (6):
      dt-bindings: media: qcom,sm8550-iris: document SM8650 IRIS accelerator
      media: platform: qcom/iris: add power_off_controller to vpu_ops
      media: platform: qcom/iris: introduce optional controller_rst_tbl
      media: platform: qcom/iris: rename iris_vpu3 to iris_vpu3x
      media: platform: qcom/iris: add support for vpu33
      media: platform: qcom/iris: add sm8650 support

 .../bindings/media/qcom,sm8550-iris.yaml           |  33 ++-
 drivers/media/platform/qcom/iris/Makefile          |   2 +-
 drivers/media/platform/qcom/iris/iris_core.h       |   1 +
 .../platform/qcom/iris/iris_platform_common.h      |   3 +
 .../platform/qcom/iris/iris_platform_sm8550.c      |  64 +++++
 drivers/media/platform/qcom/iris/iris_probe.c      |  43 +++-
 drivers/media/platform/qcom/iris/iris_vpu2.c       |   1 +
 drivers/media/platform/qcom/iris/iris_vpu3.c       | 122 ---------
 drivers/media/platform/qcom/iris/iris_vpu3x.c      | 275 +++++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_vpu_common.c |   4 +-
 drivers/media/platform/qcom/iris/iris_vpu_common.h |   3 +
 11 files changed, 409 insertions(+), 142 deletions(-)
---
base-commit: 0d6ed9e013fcc33da9676ed870454d2a014a5aee
change-id: 20250225-topic-sm8x50-iris-v10-a219b8a8b477

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>

[PATCH v4 1/6] dt-bindings: media: qcom,sm8550-iris: document SM8650 IRIS accelerator

[Neil Armstrong]

Document the IRIS video decoder and encoder accelerator found in the
SM8650 platform, it requires 2 more reset lines in addition to the
properties required for the SM8550 platform.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../bindings/media/qcom,sm8550-iris.yaml           | 33 ++++++++++++++++++----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index e424ea84c211f473a799481fd5463a16580187ed..536cf458dcb08141e5a1ec8c3df964196e599a57 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -14,12 +14,11 @@ description:
   The iris video processing unit is a video encode and decode accelerator
   present on Qualcomm platforms.
 
-allOf:
-  - $ref: qcom,venus-common.yaml#
-
 properties:
   compatible:
-    const: qcom,sm8550-iris
+    enum:
+      - qcom,sm8550-iris
+      - qcom,sm8650-iris
 
   power-domains:
     maxItems: 4
@@ -49,11 +48,15 @@ properties:
       - const: video-mem
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3
 
   reset-names:
+    minItems: 1
     items:
       - const: bus
+      - const: xo
+      - const: core
 
   iommus:
     maxItems: 2
@@ -75,6 +78,26 @@ required:
   - iommus
   - dma-coherent
 
+allOf:
+  - $ref: qcom,venus-common.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - qcom,sm8650-iris
+    then:
+      properties:
+        resets:
+          minItems: 3
+        reset-names:
+          minItems: 3
+    else:
+      properties:
+        resets:
+          maxItems: 1
+        reset-names:
+          maxItems: 1
+
 unevaluatedProperties: false
 
 examples:

-- 
2.34.1

[PATCH v4 2/6] media: platform: qcom/iris: add power_off_controller to vpu_ops

[Neil Armstrong]

In order to support the SM8650 iris33 hardware, we need to provide a
specific constoller power off sequences via the vpu_ops callbacks.

Add the callback, and use the current helper for currently supported
platforms.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/media/platform/qcom/iris/iris_vpu2.c       | 1 +
 drivers/media/platform/qcom/iris/iris_vpu3.c       | 1 +
 drivers/media/platform/qcom/iris/iris_vpu_common.c | 4 ++--
 drivers/media/platform/qcom/iris/iris_vpu_common.h | 2 ++
 4 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vpu2.c b/drivers/media/platform/qcom/iris/iris_vpu2.c
index 8f502aed43ce2fa6a272a2ce14ff1ca54d3e63a2..7cf1bfc352d34b897451061b5c14fbe90276433d 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu2.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu2.c
@@ -34,5 +34,6 @@ 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_off_controller = iris_vpu_power_off_controller,
 	.calc_freq = iris_vpu2_calc_freq,
 };
diff --git a/drivers/media/platform/qcom/iris/iris_vpu3.c b/drivers/media/platform/qcom/iris/iris_vpu3.c
index b484638e6105a69319232f667ee7ae95e3853698..13dab61427b8bd0491b69a9bc5f5144d27d17362 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3.c
@@ -118,5 +118,6 @@ static u64 iris_vpu3_calculate_frequency(struct iris_inst *inst, size_t data_siz
 
 const struct vpu_ops iris_vpu3_ops = {
 	.power_off_hw = iris_vpu3_power_off_hardware,
+	.power_off_controller = iris_vpu_power_off_controller,
 	.calc_freq = iris_vpu3_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 fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..268e45acaa7c0e3fe237123c62f0133d9dface14 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -211,7 +211,7 @@ int iris_vpu_prepare_pc(struct iris_core *core)
 	return -EAGAIN;
 }
 
-static int iris_vpu_power_off_controller(struct iris_core *core)
+int iris_vpu_power_off_controller(struct iris_core *core)
 {
 	u32 val = 0;
 	int ret;
@@ -264,7 +264,7 @@ void iris_vpu_power_off(struct iris_core *core)
 {
 	dev_pm_opp_set_rate(core->dev, 0);
 	core->iris_platform_data->vpu_ops->power_off_hw(core);
-	iris_vpu_power_off_controller(core);
+	core->iris_platform_data->vpu_ops->power_off_controller(core);
 	iris_unset_icc_bw(core);
 
 	if (!iris_vpu_watchdog(core, core->intr_status))
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
index 63fa1fa5a4989e48aebdb6c7619c140000c0b44c..f8965661c602f990d5a7057565f79df4112d097e 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
@@ -13,6 +13,7 @@ extern const struct vpu_ops iris_vpu3_ops;
 
 struct vpu_ops {
 	void (*power_off_hw)(struct iris_core *core);
+	int (*power_off_controller)(struct iris_core *core);
 	u64 (*calc_freq)(struct iris_inst *inst, size_t data_size);
 };
 
@@ -22,6 +23,7 @@ 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(struct iris_core *core);
+int iris_vpu_power_off_controller(struct iris_core *core);
 void iris_vpu_power_off_hw(struct iris_core *core);
 void iris_vpu_power_off(struct iris_core *core);
 

-- 
2.34.1

[PATCH v4 3/6] media: platform: qcom/iris: introduce optional controller_rst_tbl

[Neil Armstrong]

Introduce an optional controller_rst_tbl use to store reset lines
used to reset part of the controller.

This is necessary for the vpu3 support, when the xo reset line
must be asserted separately from the other reset line
on power off operation.

Factor the iris_init_resets() logic to allow requesting
multiple reset tables.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/media/platform/qcom/iris/iris_core.h       |  1 +
 .../platform/qcom/iris/iris_platform_common.h      |  2 ++
 drivers/media/platform/qcom/iris/iris_probe.c      | 39 +++++++++++++++-------
 3 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
index 37fb4919fecc62182784b4dca90fcab47dd38a80..78143855b277cd3ebdc7a1e7f35f6df284aa364c 100644
--- a/drivers/media/platform/qcom/iris/iris_core.h
+++ b/drivers/media/platform/qcom/iris/iris_core.h
@@ -82,6 +82,7 @@ struct iris_core {
 	struct clk_bulk_data			*clock_tbl;
 	u32					clk_count;
 	struct reset_control_bulk_data		*resets;
+	struct reset_control_bulk_data		*controller_resets;
 	const struct iris_platform_data		*iris_platform_data;
 	enum iris_core_state			state;
 	dma_addr_t				iface_q_table_daddr;
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index f6b15d2805fb2004699709bb12cd7ce9b052180c..fdd40fd80178c4c66b37e392d07a0a62f492f108 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -156,6 +156,8 @@ struct iris_platform_data {
 	unsigned int clk_tbl_size;
 	const char * const *clk_rst_tbl;
 	unsigned int clk_rst_tbl_size;
+	const char * const *controller_rst_tbl;
+	unsigned int controller_rst_tbl_size;
 	u64 dma_mask;
 	const char *fwname;
 	u32 pas_id;
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index aca442dcc153830e6252d1dca87afb38c0b9eb8f..4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -91,25 +91,40 @@ static int iris_init_clocks(struct iris_core *core)
 	return 0;
 }
 
-static int iris_init_resets(struct iris_core *core)
+static int iris_init_reset_table(struct iris_core *core,
+				 struct reset_control_bulk_data **resets,
+				 const char * const *rst_tbl, u32 rst_tbl_size)
 {
-	const char * const *rst_tbl;
-	u32 rst_tbl_size;
 	u32 i = 0;
 
-	rst_tbl = core->iris_platform_data->clk_rst_tbl;
-	rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
-
-	core->resets = devm_kzalloc(core->dev,
-				    sizeof(*core->resets) * rst_tbl_size,
-				    GFP_KERNEL);
-	if (!core->resets)
+	*resets = devm_kzalloc(core->dev,
+			       sizeof(struct reset_control_bulk_data) * rst_tbl_size,
+			       GFP_KERNEL);
+	if (!*resets)
 		return -ENOMEM;
 
 	for (i = 0; i < rst_tbl_size; i++)
-		core->resets[i].id = rst_tbl[i];
+		(*resets)[i].id = rst_tbl[i];
+
+	return devm_reset_control_bulk_get_exclusive(core->dev, rst_tbl_size, *resets);
+}
+
+static int iris_init_resets(struct iris_core *core)
+{
+	int ret;
+
+	ret = iris_init_reset_table(core, &core->resets,
+				    core->iris_platform_data->clk_rst_tbl,
+				    core->iris_platform_data->clk_rst_tbl_size);
+	if (ret)
+		return ret;
+
+	if (!core->iris_platform_data->controller_rst_tbl_size)
+		return 0;
 
-	return devm_reset_control_bulk_get_exclusive(core->dev, rst_tbl_size, core->resets);
+	return iris_init_reset_table(core, &core->controller_resets,
+				     core->iris_platform_data->controller_rst_tbl,
+				     core->iris_platform_data->controller_rst_tbl_size);
 }
 
 static int iris_init_resources(struct iris_core *core)

-- 
2.34.1

[PATCH v4 4/6] media: platform: qcom/iris: rename iris_vpu3 to iris_vpu3x

[Neil Armstrong]

The vpu33 HW is very close to vpu3, and shares most of the
operations, so rename file to vpu3x since we'll handle all vpu3
variants in it.

Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/media/platform/qcom/iris/Makefile                      | 2 +-
 drivers/media/platform/qcom/iris/{iris_vpu3.c => iris_vpu3x.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
index 35390534534e93f4617c1036a05ca0921567ba1d..473aaf655448180ade917e642289677fc1277f99 100644
--- a/drivers/media/platform/qcom/iris/Makefile
+++ b/drivers/media/platform/qcom/iris/Makefile
@@ -20,7 +20,7 @@ qcom-iris-objs += \
              iris_vb2.o \
              iris_vdec.o \
              iris_vpu2.o \
-             iris_vpu3.o \
+             iris_vpu3x.o \
              iris_vpu_buffer.o \
              iris_vpu_common.o \
 
diff --git a/drivers/media/platform/qcom/iris/iris_vpu3.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
similarity index 100%
rename from drivers/media/platform/qcom/iris/iris_vpu3.c
rename to drivers/media/platform/qcom/iris/iris_vpu3x.c

-- 
2.34.1

[PATCH v4 5/6] media: platform: qcom/iris: add support for vpu33

[Neil Armstrong]

The IRIS acceleration found in the SM8650 platforms uses the vpu33
hardware version, and requires a slighly different reset and power off
sequences in order to properly get out of runtime suspend.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/media/platform/qcom/iris/iris_vpu3x.c      | 160 ++++++++++++++++++++-
 drivers/media/platform/qcom/iris/iris_vpu_common.h |   1 +
 2 files changed, 157 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
index 13dab61427b8bd0491b69a9bc5f5144d27d17362..5d8b437ded2647a71c0c0e6b51a0942919ef6076 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
@@ -4,20 +4,39 @@
  */
 
 #include <linux/iopoll.h>
+#include <linux/reset.h>
 
 #include "iris_instance.h"
 #include "iris_vpu_common.h"
 #include "iris_vpu_register_defines.h"
 
+#define WRAPPER_TZ_BASE_OFFS			0x000C0000
+#define AON_BASE_OFFS				0x000E0000
 #define AON_MVP_NOC_RESET			0x0001F000
 
+#define WRAPPER_DEBUG_BRIDGE_LPI_CONTROL	(WRAPPER_BASE_OFFS + 0x54)
+#define WRAPPER_DEBUG_BRIDGE_LPI_STATUS		(WRAPPER_BASE_OFFS + 0x58)
+#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_CLOCK_CONFIG		(WRAPPER_BASE_OFFS + 0x88)
 #define CORE_CLK_RUN				0x0
 
+#define WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG		(WRAPPER_TZ_BASE_OFFS + 0x14)
+#define CTL_AXI_CLK_HALT			BIT(0)
+#define CTL_CLK_HALT				BIT(1)
+
+#define WRAPPER_TZ_QNS4PDXFIFO_RESET		(WRAPPER_TZ_BASE_OFFS + 0x18)
+#define RESET_HIGH				BIT(0)
+
 #define CPU_CS_AHB_BRIDGE_SYNC_RESET		(CPU_CS_BASE_OFFS + 0x160)
 #define CORE_BRIDGE_SW_RESET			BIT(0)
 #define CORE_BRIDGE_HW_RESET_DISABLE		BIT(1)
 
+#define CPU_CS_X2RPMH				(CPU_CS_BASE_OFFS + 0x168)
+#define MSK_SIGNAL_FROM_TENSILICA		BIT(0)
+#define MSK_CORE_POWER_ON			BIT(1)
+
 #define AON_WRAPPER_MVP_NOC_RESET_REQ		(AON_MVP_NOC_RESET + 0x000)
 #define VIDEO_NOC_RESET_REQ			(BIT(0) | BIT(1))
 
@@ -25,7 +44,16 @@
 
 #define VCODEC_SS_IDLE_STATUSN			(VCODEC_BASE_OFFS + 0x70)
 
-static bool iris_vpu3_hw_power_collapsed(struct iris_core *core)
+#define AON_WRAPPER_MVP_NOC_LPI_CONTROL		(AON_BASE_OFFS)
+#define AON_WRAPPER_MVP_NOC_LPI_STATUS		(AON_BASE_OFFS + 0x4)
+
+#define AON_WRAPPER_MVP_NOC_CORE_SW_RESET	(AON_BASE_OFFS + 0x18)
+#define SW_RESET				BIT(0)
+#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)
+
+static bool iris_vpu3x_hw_power_collapsed(struct iris_core *core)
 {
 	u32 value, pwr_status;
 
@@ -40,7 +68,7 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
 	u32 reg_val = 0, value, i;
 	int ret;
 
-	if (iris_vpu3_hw_power_collapsed(core))
+	if (iris_vpu3x_hw_power_collapsed(core))
 		goto disable_power;
 
 	dev_err(core->dev, "video hw is power on\n");
@@ -79,7 +107,125 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
 	iris_vpu_power_off_hw(core);
 }
 
-static u64 iris_vpu3_calculate_frequency(struct iris_inst *inst, size_t data_size)
+static void iris_vpu33_power_off_hardware(struct iris_core *core)
+{
+	u32 reg_val = 0, value, i;
+	int ret;
+
+	if (iris_vpu3x_hw_power_collapsed(core))
+		goto disable_power;
+
+	dev_err(core->dev, "video hw is power on\n");
+
+	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,
+					 reg_val, reg_val & 0x400000, 2000, 20000);
+		if (ret)
+			goto disable_power;
+	}
+
+	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
+			reg_val, reg_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:
+	iris_vpu_power_off_hw(core);
+}
+
+static int iris_vpu33_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_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;
+
+	writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT,
+	       core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG);
+	writel(RESET_HIGH, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET);
+	writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET);
+	writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG);
+
+	reset_control_bulk_reset(clk_rst_tbl_size, core->resets);
+
+	/* Disable MVP NoC clock */
+	val = readl(core->reg_base + AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL);
+	val |= NOC_HALT;
+	writel(val, core->reg_base + AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL);
+
+	/* enable MVP NoC reset */
+	val = readl(core->reg_base + AON_WRAPPER_MVP_NOC_CORE_SW_RESET);
+	val |= SW_RESET;
+	writel(val, core->reg_base + AON_WRAPPER_MVP_NOC_CORE_SW_RESET);
+
+	/* poll AON spare register bit0 to become zero with 50ms timeout */
+	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_SPARE,
+				 val, (val & BIT(0)) == 0, 1000, 50000);
+	if (ret)
+		goto disable_power;
+
+	/* enable bit(1) to avoid cvp noc xo reset */
+	val = readl(core->reg_base + AON_WRAPPER_SPARE);
+	val |= BIT(1);
+	writel(val, core->reg_base + AON_WRAPPER_SPARE);
+
+	reset_control_bulk_assert(xo_rst_tbl_size, core->controller_resets);
+
+	/* De-assert MVP NoC reset */
+	val = readl(core->reg_base + AON_WRAPPER_MVP_NOC_CORE_SW_RESET);
+	val &= ~SW_RESET;
+	writel(val, core->reg_base + AON_WRAPPER_MVP_NOC_CORE_SW_RESET);
+
+	usleep_range(80, 100);
+
+	reset_control_bulk_deassert(xo_rst_tbl_size, core->controller_resets);
+
+	/* reset AON spare register */
+	writel(0, core->reg_base + AON_WRAPPER_SPARE);
+
+	/* Enable MVP NoC clock */
+	val = readl(core->reg_base + AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL);
+	val &= ~NOC_HALT;
+	writel(val, core->reg_base + AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL);
+
+	iris_disable_unprepare_clock(core, IRIS_CTRL_CLK);
+
+disable_power:
+	iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
+	iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
+
+	return 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;
 	struct v4l2_format *inp_f = inst->fmt_src;
@@ -119,5 +265,11 @@ static u64 iris_vpu3_calculate_frequency(struct iris_inst *inst, size_t data_siz
 const struct vpu_ops iris_vpu3_ops = {
 	.power_off_hw = iris_vpu3_power_off_hardware,
 	.power_off_controller = iris_vpu_power_off_controller,
-	.calc_freq = iris_vpu3_calculate_frequency,
+	.calc_freq = iris_vpu3x_calculate_frequency,
+};
+
+const struct vpu_ops iris_vpu33_ops = {
+	.power_off_hw = iris_vpu33_power_off_hardware,
+	.power_off_controller = iris_vpu33_power_off_controller,
+	.calc_freq = iris_vpu3x_calculate_frequency,
 };
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
index f8965661c602f990d5a7057565f79df4112d097e..93b7fa27be3bfa1cf6a3e83cc192cdb89d63575f 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
@@ -10,6 +10,7 @@ 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;
 
 struct vpu_ops {
 	void (*power_off_hw)(struct iris_core *core);

-- 
2.34.1

[PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[Neil Armstrong]

Add support for the SM8650 platform by re-using the SM8550
definitions and using the vpu33 ops.

The SM8650/vpu33 requires more reset lines, but the H.264
decoder capabilities are identical.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../platform/qcom/iris/iris_platform_common.h      |  1 +
 .../platform/qcom/iris/iris_platform_sm8550.c      | 64 ++++++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
 3 files changed, 69 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -35,6 +35,7 @@ enum pipe_type {
 
 extern struct iris_platform_data sm8250_data;
 extern struct iris_platform_data sm8550_data;
+extern struct iris_platform_data sm8650_data;
 
 enum platform_clk_type {
 	IRIS_AXI_CLK,
diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
index 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
@@ -144,6 +144,10 @@ static const struct icc_info sm8550_icc_table[] = {
 
 static const char * const sm8550_clk_reset_table[] = { "bus" };
 
+static const char * const sm8650_clk_reset_table[] = { "bus", "core" };
+
+static const char * const sm8650_controller_reset_table[] = { "xo" };
+
 static const struct bw_info sm8550_bw_table_dec[] = {
 	{ ((4096 * 2160) / 256) * 60, 1608000 },
 	{ ((4096 * 2160) / 256) * 30,  826000 },
@@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
 	.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:
+ * - vpu_ops to iris_vpu33_ops
+ * - clk_rst_tbl to sm8650_clk_reset_table
+ * - controller_rst_tbl to sm8650_controller_reset_table
+ * - fwname to "qcom/vpu/vpu33_p4.mbn"
+ */
+struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
+	.clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
+	.controller_rst_tbl = sm8650_controller_reset_table,
+	.controller_rst_tbl_size = ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
+	.clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
+	/* Upper bound of DMA address range */
+	.dma_mask = 0xe0000000 - 1,
+	.fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
+	.input_config_params =
+		sm8550_vdec_input_config_params,
+	.input_config_params_size =
+		ARRAY_SIZE(sm8550_vdec_input_config_params),
+	.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 = sm8550_vdec_subscribe_output_properties,
+	.dec_output_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
+
+	.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),
+};
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -345,6 +345,10 @@ static const struct of_device_id iris_dt_match[] = {
 			.data = &sm8250_data,
 		},
 #endif
+	{
+		.compatible = "qcom,sm8650-iris",
+		.data = &sm8650_data,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, iris_dt_match);

-- 
2.34.1

Re: [PATCH v4 2/6] media: platform: qcom/iris: add power_off_controller to vpu_ops

[Vikash Garodia]

On 4/9/2025 8:08 PM, Neil Armstrong wrote:
> In order to support the SM8650 iris33 hardware, we need to provide a
> specific constoller power off sequences via the vpu_ops callbacks.
> 
> Add the callback, and use the current helper for currently supported
> platforms.
> 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  drivers/media/platform/qcom/iris/iris_vpu2.c       | 1 +
>  drivers/media/platform/qcom/iris/iris_vpu3.c       | 1 +
>  drivers/media/platform/qcom/iris/iris_vpu_common.c | 4 ++--
>  drivers/media/platform/qcom/iris/iris_vpu_common.h | 2 ++
>  4 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu2.c b/drivers/media/platform/qcom/iris/iris_vpu2.c
> index 8f502aed43ce2fa6a272a2ce14ff1ca54d3e63a2..7cf1bfc352d34b897451061b5c14fbe90276433d 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu2.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu2.c
> @@ -34,5 +34,6 @@ 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_off_controller = iris_vpu_power_off_controller,
>  	.calc_freq = iris_vpu2_calc_freq,
>  };
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3.c b/drivers/media/platform/qcom/iris/iris_vpu3.c
> index b484638e6105a69319232f667ee7ae95e3853698..13dab61427b8bd0491b69a9bc5f5144d27d17362 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu3.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu3.c
> @@ -118,5 +118,6 @@ static u64 iris_vpu3_calculate_frequency(struct iris_inst *inst, size_t data_siz
>  
>  const struct vpu_ops iris_vpu3_ops = {
>  	.power_off_hw = iris_vpu3_power_off_hardware,
> +	.power_off_controller = iris_vpu_power_off_controller,
>  	.calc_freq = iris_vpu3_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 fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..268e45acaa7c0e3fe237123c62f0133d9dface14 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
> @@ -211,7 +211,7 @@ int iris_vpu_prepare_pc(struct iris_core *core)
>  	return -EAGAIN;
>  }
>  
> -static int iris_vpu_power_off_controller(struct iris_core *core)
> +int iris_vpu_power_off_controller(struct iris_core *core)
>  {
>  	u32 val = 0;
>  	int ret;
> @@ -264,7 +264,7 @@ void iris_vpu_power_off(struct iris_core *core)
>  {
>  	dev_pm_opp_set_rate(core->dev, 0);
>  	core->iris_platform_data->vpu_ops->power_off_hw(core);
> -	iris_vpu_power_off_controller(core);
> +	core->iris_platform_data->vpu_ops->power_off_controller(core);
>  	iris_unset_icc_bw(core);
>  
>  	if (!iris_vpu_watchdog(core, core->intr_status))
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
> index 63fa1fa5a4989e48aebdb6c7619c140000c0b44c..f8965661c602f990d5a7057565f79df4112d097e 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
> @@ -13,6 +13,7 @@ extern const struct vpu_ops iris_vpu3_ops;
>  
>  struct vpu_ops {
>  	void (*power_off_hw)(struct iris_core *core);
> +	int (*power_off_controller)(struct iris_core *core);
>  	u64 (*calc_freq)(struct iris_inst *inst, size_t data_size);
>  };
>  
> @@ -22,6 +23,7 @@ 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(struct iris_core *core);
> +int iris_vpu_power_off_controller(struct iris_core *core);
>  void iris_vpu_power_off_hw(struct iris_core *core);
>  void iris_vpu_power_off(struct iris_core *core);

Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>

Re: [PATCH v4 4/6] media: platform: qcom/iris: rename iris_vpu3 to iris_vpu3x

[Vikash Garodia]

On 4/9/2025 8:08 PM, Neil Armstrong wrote:
> The vpu33 HW is very close to vpu3, and shares most of the
> operations, so rename file to vpu3x since we'll handle all vpu3
> variants in it.
> 
> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  drivers/media/platform/qcom/iris/Makefile                      | 2 +-
>  drivers/media/platform/qcom/iris/{iris_vpu3.c => iris_vpu3x.c} | 0
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
> index 35390534534e93f4617c1036a05ca0921567ba1d..473aaf655448180ade917e642289677fc1277f99 100644
> --- a/drivers/media/platform/qcom/iris/Makefile
> +++ b/drivers/media/platform/qcom/iris/Makefile
> @@ -20,7 +20,7 @@ qcom-iris-objs += \
>               iris_vb2.o \
>               iris_vdec.o \
>               iris_vpu2.o \
> -             iris_vpu3.o \
> +             iris_vpu3x.o \
>               iris_vpu_buffer.o \
>               iris_vpu_common.o \
>  
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> similarity index 100%
> rename from drivers/media/platform/qcom/iris/iris_vpu3.c
> rename to drivers/media/platform/qcom/iris/iris_vpu3x.c

Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>

Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[Vikash Garodia]

Hi Neil,

On 4/9/2025 8:08 PM, Neil Armstrong wrote:
> Add support for the SM8650 platform by re-using the SM8550
> definitions and using the vpu33 ops.
> 
> The SM8650/vpu33 requires more reset lines, but the H.264
> decoder capabilities are identical.
> 
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../platform/qcom/iris/iris_platform_common.h      |  1 +
>  .../platform/qcom/iris/iris_platform_sm8550.c      | 64 ++++++++++++++++++++++
>  drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
>  3 files changed, 69 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -35,6 +35,7 @@ enum pipe_type {
>  
>  extern struct iris_platform_data sm8250_data;
>  extern struct iris_platform_data sm8550_data;
> +extern struct iris_platform_data sm8650_data;
>  
>  enum platform_clk_type {
>  	IRIS_AXI_CLK,
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
> index 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
> @@ -144,6 +144,10 @@ static const struct icc_info sm8550_icc_table[] = {
>  
>  static const char * const sm8550_clk_reset_table[] = { "bus" };
>  
> +static const char * const sm8650_clk_reset_table[] = { "bus", "core" };
> +
> +static const char * const sm8650_controller_reset_table[] = { "xo" };
> +
>  static const struct bw_info sm8550_bw_table_dec[] = {
>  	{ ((4096 * 2160) / 256) * 60, 1608000 },
>  	{ ((4096 * 2160) / 256) * 30,  826000 },
> @@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
>  	.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:
> + * - vpu_ops to iris_vpu33_ops
> + * - clk_rst_tbl to sm8650_clk_reset_table
> + * - controller_rst_tbl to sm8650_controller_reset_table
> + * - fwname to "qcom/vpu/vpu33_p4.mbn"
> + */
> +struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
> +	.clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
> +	.controller_rst_tbl = sm8650_controller_reset_table,
> +	.controller_rst_tbl_size = ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
> +	.clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
> +	/* Upper bound of DMA address range */
> +	.dma_mask = 0xe0000000 - 1,
> +	.fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
> +	.input_config_params =
> +		sm8550_vdec_input_config_params,
> +	.input_config_params_size =
> +		ARRAY_SIZE(sm8550_vdec_input_config_params),
> +	.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 = sm8550_vdec_subscribe_output_properties,
> +	.dec_output_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
> +
> +	.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),
> +};
While i was extending the data for QCS8300 (one another iris-v3 variant), i
realize that this file iris_platform_sm8550.c is getting dumped with all SOC
platform data. It would be a good idea at this point to split it into something
like this
1. Introduce SOC specific c file and move the respective SOC platform data to
it, for ex, in this case sm8650_data
2. Move the common structs from iris_platform_sm8550.c to
iris_platform_common.h. This way more SOCs getting added in future, can include
the common header to reuse them, otherwise it would end up using 8550.c for all
future SOC.

Share your comments if you have any better approach to manage/re-use these
platform data considering more SOCs getting added.

Regards,
Vikash

> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -345,6 +345,10 @@ static const struct of_device_id iris_dt_match[] = {
>  			.data = &sm8250_data,
>  		},
>  #endif
> +	{
> +		.compatible = "qcom,sm8650-iris",
> +		.data = &sm8650_data,
> +	},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, iris_dt_match);
> 

Re: [PATCH v4 4/6] media: platform: qcom/iris: rename iris_vpu3 to iris_vpu3x

[Dikshita Agarwal]

On 4/9/2025 8:08 PM, Neil Armstrong wrote:
> The vpu33 HW is very close to vpu3, and shares most of the
> operations, so rename file to vpu3x since we'll handle all vpu3
> variants in it.
> 
> Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  drivers/media/platform/qcom/iris/Makefile                      | 2 +-
>  drivers/media/platform/qcom/iris/{iris_vpu3.c => iris_vpu3x.c} | 0
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
> index 35390534534e93f4617c1036a05ca0921567ba1d..473aaf655448180ade917e642289677fc1277f99 100644
> --- a/drivers/media/platform/qcom/iris/Makefile
> +++ b/drivers/media/platform/qcom/iris/Makefile
> @@ -20,7 +20,7 @@ qcom-iris-objs += \
>               iris_vb2.o \
>               iris_vdec.o \
>               iris_vpu2.o \
> -             iris_vpu3.o \
> +             iris_vpu3x.o \
>               iris_vpu_buffer.o \
>               iris_vpu_common.o \
>  
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
> similarity index 100%
> rename from drivers/media/platform/qcom/iris/iris_vpu3.c
> rename to drivers/media/platform/qcom/iris/iris_vpu3x.c
> 
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>

Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[Neil Armstrong]

On 09/04/2025 18:57, Vikash Garodia wrote:
> Hi Neil,
> 
> On 4/9/2025 8:08 PM, Neil Armstrong wrote:
>> Add support for the SM8650 platform by re-using the SM8550
>> definitions and using the vpu33 ops.
>>
>> The SM8650/vpu33 requires more reset lines, but the H.264
>> decoder capabilities are identical.
>>
>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>   .../platform/qcom/iris/iris_platform_sm8550.c      | 64 ++++++++++++++++++++++
>>   drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
>>   3 files changed, 69 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> index fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> @@ -35,6 +35,7 @@ enum pipe_type {
>>   
>>   extern struct iris_platform_data sm8250_data;
>>   extern struct iris_platform_data sm8550_data;
>> +extern struct iris_platform_data sm8650_data;
>>   
>>   enum platform_clk_type {
>>   	IRIS_AXI_CLK,
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>> index 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>> @@ -144,6 +144,10 @@ static const struct icc_info sm8550_icc_table[] = {
>>   
>>   static const char * const sm8550_clk_reset_table[] = { "bus" };
>>   
>> +static const char * const sm8650_clk_reset_table[] = { "bus", "core" };
>> +
>> +static const char * const sm8650_controller_reset_table[] = { "xo" };
>> +
>>   static const struct bw_info sm8550_bw_table_dec[] = {
>>   	{ ((4096 * 2160) / 256) * 60, 1608000 },
>>   	{ ((4096 * 2160) / 256) * 30,  826000 },
>> @@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
>>   	.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:
>> + * - vpu_ops to iris_vpu33_ops
>> + * - clk_rst_tbl to sm8650_clk_reset_table
>> + * - controller_rst_tbl to sm8650_controller_reset_table
>> + * - fwname to "qcom/vpu/vpu33_p4.mbn"
>> + */
>> +struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
>> +	.clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
>> +	.controller_rst_tbl = sm8650_controller_reset_table,
>> +	.controller_rst_tbl_size = ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
>> +	.clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
>> +	/* Upper bound of DMA address range */
>> +	.dma_mask = 0xe0000000 - 1,
>> +	.fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
>> +	.input_config_params =
>> +		sm8550_vdec_input_config_params,
>> +	.input_config_params_size =
>> +		ARRAY_SIZE(sm8550_vdec_input_config_params),
>> +	.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 = sm8550_vdec_subscribe_output_properties,
>> +	.dec_output_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
>> +
>> +	.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),
>> +};
> While i was extending the data for QCS8300 (one another iris-v3 variant), i
> realize that this file iris_platform_sm8550.c is getting dumped with all SOC
> platform data. It would be a good idea at this point to split it into something
> like this
> 1. Introduce SOC specific c file and move the respective SOC platform data to
> it, for ex, in this case sm8650_data
> 2. Move the common structs from iris_platform_sm8550.c to
> iris_platform_common.h. This way more SOCs getting added in future, can include
> the common header to reuse them, otherwise it would end up using 8550.c for all
> future SOC.
> 
> Share your comments if you have any better approach to manage/re-use these
> platform data considering more SOCs getting added.

Right, yes the architecture is fine, but I don't feel iris_platform_common is the right
place, perhaps we could introduce a platform_catalog.c where we could place all the common
platform data and reuse them from the platform_<soc>.c files ?

I can design prototype on top of this patchset as an RFC.

Neil

> 
> Regards,
> Vikash
> 
>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
>> index 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>> @@ -345,6 +345,10 @@ static const struct of_device_id iris_dt_match[] = {
>>   			.data = &sm8250_data,
>>   		},
>>   #endif
>> +	{
>> +		.compatible = "qcom,sm8650-iris",
>> +		.data = &sm8650_data,
>> +	},
>>   	{ },
>>   };
>>   MODULE_DEVICE_TABLE(of, iris_dt_match);
>>

Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[Vikash Garodia]

On 4/10/2025 2:31 PM, Neil Armstrong wrote:
> On 09/04/2025 18:57, Vikash Garodia wrote:
>> Hi Neil,
>>
>> On 4/9/2025 8:08 PM, Neil Armstrong wrote:
>>> Add support for the SM8650 platform by re-using the SM8550
>>> definitions and using the vpu33 ops.
>>>
>>> The SM8650/vpu33 requires more reset lines, but the H.264
>>> decoder capabilities are identical.
>>>
>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>> ---
>>>   .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>>   .../platform/qcom/iris/iris_platform_sm8550.c      | 64 ++++++++++++++++++++++
>>>   drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
>>>   3 files changed, 69 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>> b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>> index
>>> fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>> @@ -35,6 +35,7 @@ enum pipe_type {
>>>     extern struct iris_platform_data sm8250_data;
>>>   extern struct iris_platform_data sm8550_data;
>>> +extern struct iris_platform_data sm8650_data;
>>>     enum platform_clk_type {
>>>       IRIS_AXI_CLK,
>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>> b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>> index
>>> 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
>>> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>> @@ -144,6 +144,10 @@ static const struct icc_info sm8550_icc_table[] = {
>>>     static const char * const sm8550_clk_reset_table[] = { "bus" };
>>>   +static const char * const sm8650_clk_reset_table[] = { "bus", "core" };
>>> +
>>> +static const char * const sm8650_controller_reset_table[] = { "xo" };
>>> +
>>>   static const struct bw_info sm8550_bw_table_dec[] = {
>>>       { ((4096 * 2160) / 256) * 60, 1608000 },
>>>       { ((4096 * 2160) / 256) * 30,  826000 },
>>> @@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
>>>       .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:
>>> + * - vpu_ops to iris_vpu33_ops
>>> + * - clk_rst_tbl to sm8650_clk_reset_table
>>> + * - controller_rst_tbl to sm8650_controller_reset_table
>>> + * - fwname to "qcom/vpu/vpu33_p4.mbn"
>>> + */
>>> +struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
>>> +    .clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
>>> +    .controller_rst_tbl = sm8650_controller_reset_table,
>>> +    .controller_rst_tbl_size = ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
>>> +    .clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
>>> +    /* Upper bound of DMA address range */
>>> +    .dma_mask = 0xe0000000 - 1,
>>> +    .fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
>>> +    .input_config_params =
>>> +        sm8550_vdec_input_config_params,
>>> +    .input_config_params_size =
>>> +        ARRAY_SIZE(sm8550_vdec_input_config_params),
>>> +    .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 = sm8550_vdec_subscribe_output_properties,
>>> +    .dec_output_prop_size =
>>> ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
>>> +
>>> +    .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),
>>> +};
>> While i was extending the data for QCS8300 (one another iris-v3 variant), i
>> realize that this file iris_platform_sm8550.c is getting dumped with all SOC
>> platform data. It would be a good idea at this point to split it into something
>> like this
>> 1. Introduce SOC specific c file and move the respective SOC platform data to
>> it, for ex, in this case sm8650_data
>> 2. Move the common structs from iris_platform_sm8550.c to
>> iris_platform_common.h. This way more SOCs getting added in future, can include
>> the common header to reuse them, otherwise it would end up using 8550.c for all
>> future SOC.
>>
>> Share your comments if you have any better approach to manage/re-use these
>> platform data considering more SOCs getting added.
> 
> Right, yes the architecture is fine, but I don't feel iris_platform_common is
> the right
> place, perhaps we could introduce a platform_catalog.c where we could place all
> the common
> platform data and reuse them from the platform_<soc>.c files ?
Common structs would certainly need to be part of a header which can be
included. Where do you plan to keep common struct to be used across SOC specific
file in your approach ?
> 
> I can design prototype on top of this patchset as an RFC.
I was thinking that the changes are not that big, and can be done in existing
series though.

Thanks,
Vikash
> 
> Neil
> 
>>
>> Regards,
>> Vikash
>>
>>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c
>>> b/drivers/media/platform/qcom/iris/iris_probe.c
>>> index
>>> 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
>>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>>> @@ -345,6 +345,10 @@ static const struct of_device_id iris_dt_match[] = {
>>>               .data = &sm8250_data,
>>>           },
>>>   #endif
>>> +    {
>>> +        .compatible = "qcom,sm8650-iris",
>>> +        .data = &sm8650_data,
>>> +    },
>>>       { },
>>>   };
>>>   MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>
> 

Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[neil.armstrong]

On 10/04/2025 11:13, Vikash Garodia wrote:
> 
> On 4/10/2025 2:31 PM, Neil Armstrong wrote:
>> On 09/04/2025 18:57, Vikash Garodia wrote:
>>> Hi Neil,
>>>
>>> On 4/9/2025 8:08 PM, Neil Armstrong wrote:
>>>> Add support for the SM8650 platform by re-using the SM8550
>>>> definitions and using the vpu33 ops.
>>>>
>>>> The SM8650/vpu33 requires more reset lines, but the H.264
>>>> decoder capabilities are identical.
>>>>
>>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>>    .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>>>    .../platform/qcom/iris/iris_platform_sm8550.c      | 64 ++++++++++++++++++++++
>>>>    drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
>>>>    3 files changed, 69 insertions(+)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> index
>>>> fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> @@ -35,6 +35,7 @@ enum pipe_type {
>>>>      extern struct iris_platform_data sm8250_data;
>>>>    extern struct iris_platform_data sm8550_data;
>>>> +extern struct iris_platform_data sm8650_data;
>>>>      enum platform_clk_type {
>>>>        IRIS_AXI_CLK,
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>> b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>> index
>>>> 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>> @@ -144,6 +144,10 @@ static const struct icc_info sm8550_icc_table[] = {
>>>>      static const char * const sm8550_clk_reset_table[] = { "bus" };
>>>>    +static const char * const sm8650_clk_reset_table[] = { "bus", "core" };
>>>> +
>>>> +static const char * const sm8650_controller_reset_table[] = { "xo" };
>>>> +
>>>>    static const struct bw_info sm8550_bw_table_dec[] = {
>>>>        { ((4096 * 2160) / 256) * 60, 1608000 },
>>>>        { ((4096 * 2160) / 256) * 30,  826000 },
>>>> @@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
>>>>        .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:
>>>> + * - vpu_ops to iris_vpu33_ops
>>>> + * - clk_rst_tbl to sm8650_clk_reset_table
>>>> + * - controller_rst_tbl to sm8650_controller_reset_table
>>>> + * - fwname to "qcom/vpu/vpu33_p4.mbn"
>>>> + */
>>>> +struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
>>>> +    .clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
>>>> +    .controller_rst_tbl = sm8650_controller_reset_table,
>>>> +    .controller_rst_tbl_size = ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
>>>> +    .clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
>>>> +    /* Upper bound of DMA address range */
>>>> +    .dma_mask = 0xe0000000 - 1,
>>>> +    .fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
>>>> +    .input_config_params =
>>>> +        sm8550_vdec_input_config_params,
>>>> +    .input_config_params_size =
>>>> +        ARRAY_SIZE(sm8550_vdec_input_config_params),
>>>> +    .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 = sm8550_vdec_subscribe_output_properties,
>>>> +    .dec_output_prop_size =
>>>> ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
>>>> +
>>>> +    .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),
>>>> +};
>>> While i was extending the data for QCS8300 (one another iris-v3 variant), i
>>> realize that this file iris_platform_sm8550.c is getting dumped with all SOC
>>> platform data. It would be a good idea at this point to split it into something
>>> like this
>>> 1. Introduce SOC specific c file and move the respective SOC platform data to
>>> it, for ex, in this case sm8650_data
>>> 2. Move the common structs from iris_platform_sm8550.c to
>>> iris_platform_common.h. This way more SOCs getting added in future, can include
>>> the common header to reuse them, otherwise it would end up using 8550.c for all
>>> future SOC.
>>>
>>> Share your comments if you have any better approach to manage/re-use these
>>> platform data considering more SOCs getting added.
>>
>> Right, yes the architecture is fine, but I don't feel iris_platform_common is
>> the right
>> place, perhaps we could introduce a platform_catalog.c where we could place all
>> the common
>> platform data and reuse them from the platform_<soc>.c files ?
> Common structs would certainly need to be part of a header which can be
> included. Where do you plan to keep common struct to be used across SOC specific
> file in your approach ?

Common struct in header would mean they would be duplicated for each platform
when compiled, which would sub-optimal.

I tried to look at the drm/msm/dpu1 catalog, but:
1) in our case 99% is common, only a few stuff differs
2) we have a single struct which requires the array sizes

This makes using the drm/msm/dpu1 catalog structure impossible.

>>
>> I can design prototype on top of this patchset as an RFC.
> I was thinking that the changes are not that big, and can be done in existing
> series though.

I disagree, this is a much larger subject than sm8650 support itself.

Neil

> 
> Thanks,
> Vikash
>>
>> Neil
>>
>>>
>>> Regards,
>>> Vikash
>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c
>>>> b/drivers/media/platform/qcom/iris/iris_probe.c
>>>> index
>>>> 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>>>> @@ -345,6 +345,10 @@ static const struct of_device_id iris_dt_match[] = {
>>>>                .data = &sm8250_data,
>>>>            },
>>>>    #endif
>>>> +    {
>>>> +        .compatible = "qcom,sm8650-iris",
>>>> +        .data = &sm8650_data,
>>>> +    },
>>>>        { },
>>>>    };
>>>>    MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>>
>>

Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[Dikshita Agarwal]

On 4/10/2025 2:43 PM, Vikash Garodia wrote:
> 
> On 4/10/2025 2:31 PM, Neil Armstrong wrote:
>> On 09/04/2025 18:57, Vikash Garodia wrote:
>>> Hi Neil,
>>>
>>> On 4/9/2025 8:08 PM, Neil Armstrong wrote:
>>>> Add support for the SM8650 platform by re-using the SM8550
>>>> definitions and using the vpu33 ops.
>>>>
>>>> The SM8650/vpu33 requires more reset lines, but the H.264
>>>> decoder capabilities are identical.
>>>>
>>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>>   .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>>>   .../platform/qcom/iris/iris_platform_sm8550.c      | 64 ++++++++++++++++++++++
>>>>   drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
>>>>   3 files changed, 69 insertions(+)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> index
>>>> fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> @@ -35,6 +35,7 @@ enum pipe_type {
>>>>     extern struct iris_platform_data sm8250_data;
>>>>   extern struct iris_platform_data sm8550_data;
>>>> +extern struct iris_platform_data sm8650_data;
>>>>     enum platform_clk_type {
>>>>       IRIS_AXI_CLK,
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>> b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>> index
>>>> 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>> @@ -144,6 +144,10 @@ static const struct icc_info sm8550_icc_table[] = {
>>>>     static const char * const sm8550_clk_reset_table[] = { "bus" };
>>>>   +static const char * const sm8650_clk_reset_table[] = { "bus", "core" };
>>>> +
>>>> +static const char * const sm8650_controller_reset_table[] = { "xo" };
>>>> +
>>>>   static const struct bw_info sm8550_bw_table_dec[] = {
>>>>       { ((4096 * 2160) / 256) * 60, 1608000 },
>>>>       { ((4096 * 2160) / 256) * 30,  826000 },
>>>> @@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
>>>>       .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:
>>>> + * - vpu_ops to iris_vpu33_ops
>>>> + * - clk_rst_tbl to sm8650_clk_reset_table
>>>> + * - controller_rst_tbl to sm8650_controller_reset_table
>>>> + * - fwname to "qcom/vpu/vpu33_p4.mbn"
>>>> + */
>>>> +struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
>>>> +    .clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
>>>> +    .controller_rst_tbl = sm8650_controller_reset_table,
>>>> +    .controller_rst_tbl_size = ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
>>>> +    .clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
>>>> +    /* Upper bound of DMA address range */
>>>> +    .dma_mask = 0xe0000000 - 1,
>>>> +    .fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
>>>> +    .input_config_params =
>>>> +        sm8550_vdec_input_config_params,
>>>> +    .input_config_params_size =
>>>> +        ARRAY_SIZE(sm8550_vdec_input_config_params),
>>>> +    .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 = sm8550_vdec_subscribe_output_properties,
>>>> +    .dec_output_prop_size =
>>>> ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
>>>> +
>>>> +    .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),
>>>> +};
>>> While i was extending the data for QCS8300 (one another iris-v3 variant), i
>>> realize that this file iris_platform_sm8550.c is getting dumped with all SOC
>>> platform data. It would be a good idea at this point to split it into something
>>> like this
>>> 1. Introduce SOC specific c file and move the respective SOC platform data to
>>> it, for ex, in this case sm8650_data
>>> 2. Move the common structs from iris_platform_sm8550.c to
>>> iris_platform_common.h. This way more SOCs getting added in future, can include
>>> the common header to reuse them, otherwise it would end up using 8550.c for all
>>> future SOC.
>>>
>>> Share your comments if you have any better approach to manage/re-use these
>>> platform data considering more SOCs getting added.
>>
>> Right, yes the architecture is fine, but I don't feel iris_platform_common is
>> the right
>> place, perhaps we could introduce a platform_catalog.c where we could place all
>> the common
>> platform data and reuse them from the platform_<soc>.c files ?
> Common structs would certainly need to be part of a header which can be
> included. Where do you plan to keep common struct to be used across SOC specific
> file in your approach ?
>>
>> I can design prototype on top of this patchset as an RFC.
> I was thinking that the changes are not that big, and can be done in existing
> series though.
> 
For now, I think you can introduce a platform_sm8650.c as part of this
series and use the common structure from platform_sm8550.c.
Shouldn't be a big change.

Later you can post a separate patch series to add platform_catalog.c and
have common struct placed there which can be used across different SOC
platform files.

or other way is,
Post a patch series to introduce platform_catalog.c with common struct and
then rebase your 8650 series on top of it.

Thanks,
Dikshita
> Thanks,
> Vikash
>>
>> Neil
>>
>>>
>>> Regards,
>>> Vikash
>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c
>>>> b/drivers/media/platform/qcom/iris/iris_probe.c
>>>> index
>>>> 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>>>> @@ -345,6 +345,10 @@ static const struct of_device_id iris_dt_match[] = {
>>>>               .data = &sm8250_data,
>>>>           },
>>>>   #endif
>>>> +    {
>>>> +        .compatible = "qcom,sm8650-iris",
>>>> +        .data = &sm8650_data,
>>>> +    },
>>>>       { },
>>>>   };
>>>>   MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>>
>>

Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[neil.armstrong]

On 10/04/2025 15:07, Dikshita Agarwal wrote:
> 
> 
> On 4/10/2025 2:43 PM, Vikash Garodia wrote:
>>
>> On 4/10/2025 2:31 PM, Neil Armstrong wrote:
>>> On 09/04/2025 18:57, Vikash Garodia wrote:
>>>> Hi Neil,
>>>>
>>>> On 4/9/2025 8:08 PM, Neil Armstrong wrote:
>>>>> Add support for the SM8650 platform by re-using the SM8550
>>>>> definitions and using the vpu33 ops.
>>>>>
>>>>> The SM8650/vpu33 requires more reset lines, but the H.264
>>>>> decoder capabilities are identical.
>>>>>
>>>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>> ---
>>>>>    .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>>>>    .../platform/qcom/iris/iris_platform_sm8550.c      | 64 ++++++++++++++++++++++
>>>>>    drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
>>>>>    3 files changed, 69 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>> b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>> index
>>>>> fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
>>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>> @@ -35,6 +35,7 @@ enum pipe_type {
>>>>>      extern struct iris_platform_data sm8250_data;
>>>>>    extern struct iris_platform_data sm8550_data;
>>>>> +extern struct iris_platform_data sm8650_data;
>>>>>      enum platform_clk_type {
>>>>>        IRIS_AXI_CLK,
>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>> b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>> index
>>>>> 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
>>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>> @@ -144,6 +144,10 @@ static const struct icc_info sm8550_icc_table[] = {
>>>>>      static const char * const sm8550_clk_reset_table[] = { "bus" };
>>>>>    +static const char * const sm8650_clk_reset_table[] = { "bus", "core" };
>>>>> +
>>>>> +static const char * const sm8650_controller_reset_table[] = { "xo" };
>>>>> +
>>>>>    static const struct bw_info sm8550_bw_table_dec[] = {
>>>>>        { ((4096 * 2160) / 256) * 60, 1608000 },
>>>>>        { ((4096 * 2160) / 256) * 30,  826000 },
>>>>> @@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
>>>>>        .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:
>>>>> + * - vpu_ops to iris_vpu33_ops
>>>>> + * - clk_rst_tbl to sm8650_clk_reset_table
>>>>> + * - controller_rst_tbl to sm8650_controller_reset_table
>>>>> + * - fwname to "qcom/vpu/vpu33_p4.mbn"
>>>>> + */
>>>>> +struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
>>>>> +    .clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
>>>>> +    .controller_rst_tbl = sm8650_controller_reset_table,
>>>>> +    .controller_rst_tbl_size = ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
>>>>> +    .clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
>>>>> +    /* Upper bound of DMA address range */
>>>>> +    .dma_mask = 0xe0000000 - 1,
>>>>> +    .fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
>>>>> +    .input_config_params =
>>>>> +        sm8550_vdec_input_config_params,
>>>>> +    .input_config_params_size =
>>>>> +        ARRAY_SIZE(sm8550_vdec_input_config_params),
>>>>> +    .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 = sm8550_vdec_subscribe_output_properties,
>>>>> +    .dec_output_prop_size =
>>>>> ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
>>>>> +
>>>>> +    .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),
>>>>> +};
>>>> While i was extending the data for QCS8300 (one another iris-v3 variant), i
>>>> realize that this file iris_platform_sm8550.c is getting dumped with all SOC
>>>> platform data. It would be a good idea at this point to split it into something
>>>> like this
>>>> 1. Introduce SOC specific c file and move the respective SOC platform data to
>>>> it, for ex, in this case sm8650_data
>>>> 2. Move the common structs from iris_platform_sm8550.c to
>>>> iris_platform_common.h. This way more SOCs getting added in future, can include
>>>> the common header to reuse them, otherwise it would end up using 8550.c for all
>>>> future SOC.
>>>>
>>>> Share your comments if you have any better approach to manage/re-use these
>>>> platform data considering more SOCs getting added.
>>>
>>> Right, yes the architecture is fine, but I don't feel iris_platform_common is
>>> the right
>>> place, perhaps we could introduce a platform_catalog.c where we could place all
>>> the common
>>> platform data and reuse them from the platform_<soc>.c files ?
>> Common structs would certainly need to be part of a header which can be
>> included. Where do you plan to keep common struct to be used across SOC specific
>> file in your approach ?
>>>
>>> I can design prototype on top of this patchset as an RFC.
>> I was thinking that the changes are not that big, and can be done in existing
>> series though.
>>
> For now, I think you can introduce a platform_sm8650.c as part of this
> series and use the common structure from platform_sm8550.c.
> Shouldn't be a big change.

I tried but I don't how to solve this, you need a build-time ARRAY_SIZE for
the arrays, so they need to be in the same .c file to allow a build-time
evaluation of ARRAY_SIZE. If he common tables are moved into a header
they will be duplicated into both platform_sm8650 & platform_sm8550 objects
which is not what we want.

The solution would be to write all the platform tables & iris_platform_data
into headers, with common headers, then include those into a platform_catalog.c
like is done for the drm/msm/dpu1 catalog.

Neil

> 
> Later you can post a separate patch series to add platform_catalog.c and
> have common struct placed there which can be used across different SOC
> platform files.
> 
> or other way is,
> Post a patch series to introduce platform_catalog.c with common struct and
> then rebase your 8650 series on top of it.
> 
> Thanks,
> Dikshita
>> Thanks,
>> Vikash
>>>
>>> Neil
>>>
>>>>
>>>> Regards,
>>>> Vikash
>>>>
>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c
>>>>> b/drivers/media/platform/qcom/iris/iris_probe.c
>>>>> index
>>>>> 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
>>>>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>>>>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>>>>> @@ -345,6 +345,10 @@ static const struct of_device_id iris_dt_match[] = {
>>>>>                .data = &sm8250_data,
>>>>>            },
>>>>>    #endif
>>>>> +    {
>>>>> +        .compatible = "qcom,sm8650-iris",
>>>>> +        .data = &sm8650_data,
>>>>> +    },
>>>>>        { },
>>>>>    };
>>>>>    MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>>>
>>>

Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[Bryan O'Donoghue]

On 10/04/2025 16:40, neil.armstrong@linaro.org wrote:
> On 10/04/2025 15:07, Dikshita Agarwal wrote:
>>
>>
>> On 4/10/2025 2:43 PM, Vikash Garodia wrote:
>>>
>>> On 4/10/2025 2:31 PM, Neil Armstrong wrote:
>>>> On 09/04/2025 18:57, Vikash Garodia wrote:
>>>>> Hi Neil,
>>>>>
>>>>> On 4/9/2025 8:08 PM, Neil Armstrong wrote:
>>>>>> Add support for the SM8650 platform by re-using the SM8550
>>>>>> definitions and using the vpu33 ops.
>>>>>>
>>>>>> The SM8650/vpu33 requires more reset lines, but the H.264
>>>>>> decoder capabilities are identical.
>>>>>>
>>>>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>> ---
>>>>>>    .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>>>>>    .../platform/qcom/iris/iris_platform_sm8550.c      | 64 +++++++ 
>>>>>> +++++++++++++++
>>>>>>    drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
>>>>>>    3 files changed, 69 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>> b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>> index
>>>>>> fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
>>>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>> @@ -35,6 +35,7 @@ enum pipe_type {
>>>>>>      extern struct iris_platform_data sm8250_data;
>>>>>>    extern struct iris_platform_data sm8550_data;
>>>>>> +extern struct iris_platform_data sm8650_data;
>>>>>>      enum platform_clk_type {
>>>>>>        IRIS_AXI_CLK,
>>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>>> b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>>> index
>>>>>> 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
>>>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>>> @@ -144,6 +144,10 @@ static const struct icc_info 
>>>>>> sm8550_icc_table[] = {
>>>>>>      static const char * const sm8550_clk_reset_table[] = { "bus" };
>>>>>>    +static const char * const sm8650_clk_reset_table[] = { "bus", 
>>>>>> "core" };
>>>>>> +
>>>>>> +static const char * const sm8650_controller_reset_table[] = 
>>>>>> { "xo" };
>>>>>> +
>>>>>>    static const struct bw_info sm8550_bw_table_dec[] = {
>>>>>>        { ((4096 * 2160) / 256) * 60, 1608000 },
>>>>>>        { ((4096 * 2160) / 256) * 30,  826000 },
>>>>>> @@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
>>>>>>        .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:
>>>>>> + * - vpu_ops to iris_vpu33_ops
>>>>>> + * - clk_rst_tbl to sm8650_clk_reset_table
>>>>>> + * - controller_rst_tbl to sm8650_controller_reset_table
>>>>>> + * - fwname to "qcom/vpu/vpu33_p4.mbn"
>>>>>> + */
>>>>>> +struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
>>>>>> +    .clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
>>>>>> +    .controller_rst_tbl = sm8650_controller_reset_table,
>>>>>> +    .controller_rst_tbl_size = 
>>>>>> ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
>>>>>> +    .clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
>>>>>> +    /* Upper bound of DMA address range */
>>>>>> +    .dma_mask = 0xe0000000 - 1,
>>>>>> +    .fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
>>>>>> +    .input_config_params =
>>>>>> +        sm8550_vdec_input_config_params,
>>>>>> +    .input_config_params_size =
>>>>>> +        ARRAY_SIZE(sm8550_vdec_input_config_params),
>>>>>> +    .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 = sm8550_vdec_subscribe_output_properties,
>>>>>> +    .dec_output_prop_size =
>>>>>> ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
>>>>>> +
>>>>>> +    .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),
>>>>>> +};
>>>>> While i was extending the data for QCS8300 (one another iris-v3 
>>>>> variant), i
>>>>> realize that this file iris_platform_sm8550.c is getting dumped 
>>>>> with all SOC
>>>>> platform data. It would be a good idea at this point to split it 
>>>>> into something
>>>>> like this
>>>>> 1. Introduce SOC specific c file and move the respective SOC 
>>>>> platform data to
>>>>> it, for ex, in this case sm8650_data
>>>>> 2. Move the common structs from iris_platform_sm8550.c to
>>>>> iris_platform_common.h. This way more SOCs getting added in future, 
>>>>> can include
>>>>> the common header to reuse them, otherwise it would end up using 
>>>>> 8550.c for all
>>>>> future SOC.
>>>>>
>>>>> Share your comments if you have any better approach to manage/re- 
>>>>> use these
>>>>> platform data considering more SOCs getting added.
>>>>
>>>> Right, yes the architecture is fine, but I don't feel 
>>>> iris_platform_common is
>>>> the right
>>>> place, perhaps we could introduce a platform_catalog.c where we 
>>>> could place all
>>>> the common
>>>> platform data and reuse them from the platform_<soc>.c files ?
>>> Common structs would certainly need to be part of a header which can be
>>> included. Where do you plan to keep common struct to be used across 
>>> SOC specific
>>> file in your approach ?
>>>>
>>>> I can design prototype on top of this patchset as an RFC.
>>> I was thinking that the changes are not that big, and can be done in 
>>> existing
>>> series though.
>>>
>> For now, I think you can introduce a platform_sm8650.c as part of this
>> series and use the common structure from platform_sm8550.c.
>> Shouldn't be a big change.
> 
> I tried but I don't how to solve this, you need a build-time ARRAY_SIZE for
> the arrays, so they need to be in the same .c file to allow a build-time
> evaluation of ARRAY_SIZE. If he common tables are moved into a header
> they will be duplicated into both platform_sm8650 & platform_sm8550 objects
> which is not what we want.
> 
> The solution would be to write all the platform tables & iris_platform_data
> into headers, with common headers, then include those into a 
> platform_catalog.c
> like is done for the drm/msm/dpu1 catalog.
> 
> Neil
> 
>>
>> Later you can post a separate patch series to add platform_catalog.c and
>> have common struct placed there which can be used across different SOC
>> platform files.
>>
>> or other way is,
>> Post a patch series to introduce platform_catalog.c with common struct 
>> and
>> then rebase your 8650 series on top of it.
>>
>> Thanks,
>> Dikshita
>>> Thanks,
>>> Vikash
>>>>
>>>> Neil
>>>>
>>>>>
>>>>> Regards,
>>>>> Vikash
>>>>>
>>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c
>>>>>> b/drivers/media/platform/qcom/iris/iris_probe.c
>>>>>> index
>>>>>> 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
>>>>>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>>>>>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>>>>>> @@ -345,6 +345,10 @@ static const struct of_device_id 
>>>>>> iris_dt_match[] = {
>>>>>>                .data = &sm8250_data,
>>>>>>            },
>>>>>>    #endif
>>>>>> +    {
>>>>>> +        .compatible = "qcom,sm8650-iris",
>>>>>> +        .data = &sm8650_data,
>>>>>> +    },
>>>>>>        { },
>>>>>>    };
>>>>>>    MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>>>>
>>>>
> 

Eh as I read the suggestion about putting the structs - instantiating 
the structs in the header, I wondered if that would link but anyway.

One way to solve this without going the dpu1 route right now is to 
rename the platform files

iris_platform_sm8250.c -> iris_platform_common_hfi_gen1.c
iris_platform_sm8550.c -> iris_platform_common_hfi_gen2.c

The differentiation around HFI into "generations" instead of 
incrementing the already existing HFIXXX version is unfortunate.

At least this way we have a repository of common HFI code located in 
each file, in expectation of HFI GEN3 whenever.

---
bod

Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support

[neil.armstrong]

On 10/04/2025 18:06, Bryan O'Donoghue wrote:
> On 10/04/2025 16:40, neil.armstrong@linaro.org wrote:
>> On 10/04/2025 15:07, Dikshita Agarwal wrote:
>>>
>>>
>>> On 4/10/2025 2:43 PM, Vikash Garodia wrote:
>>>>
>>>> On 4/10/2025 2:31 PM, Neil Armstrong wrote:
>>>>> On 09/04/2025 18:57, Vikash Garodia wrote:
>>>>>> Hi Neil,
>>>>>>
>>>>>> On 4/9/2025 8:08 PM, Neil Armstrong wrote:
>>>>>>> Add support for the SM8650 platform by re-using the SM8550
>>>>>>> definitions and using the vpu33 ops.
>>>>>>>
>>>>>>> The SM8650/vpu33 requires more reset lines, but the H.264
>>>>>>> decoder capabilities are identical.
>>>>>>>
>>>>>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
>>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>>>>> ---
>>>>>>>    .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>>>>>>    .../platform/qcom/iris/iris_platform_sm8550.c      | 64 +++++++ +++++++++++++++
>>>>>>>    drivers/media/platform/qcom/iris/iris_probe.c      |  4 ++
>>>>>>>    3 files changed, 69 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>>> b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>>> index
>>>>>>> fdd40fd80178c4c66b37e392d07a0a62f492f108..6bc3a7975b04d612f6c89206eae95dac678695fc 100644
>>>>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>>> @@ -35,6 +35,7 @@ enum pipe_type {
>>>>>>>      extern struct iris_platform_data sm8250_data;
>>>>>>>    extern struct iris_platform_data sm8550_data;
>>>>>>> +extern struct iris_platform_data sm8650_data;
>>>>>>>      enum platform_clk_type {
>>>>>>>        IRIS_AXI_CLK,
>>>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>>>> b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>>>> index
>>>>>>> 35d278996c430f2856d0fe59586930061a271c3e..d0f8fa960d53367023e41bc5807ba3f8beae2efc 100644
>>>>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
>>>>>>> @@ -144,6 +144,10 @@ static const struct icc_info sm8550_icc_table[] = {
>>>>>>>      static const char * const sm8550_clk_reset_table[] = { "bus" };
>>>>>>>    +static const char * const sm8650_clk_reset_table[] = { "bus", "core" };
>>>>>>> +
>>>>>>> +static const char * const sm8650_controller_reset_table[] = { "xo" };
>>>>>>> +
>>>>>>>    static const struct bw_info sm8550_bw_table_dec[] = {
>>>>>>>        { ((4096 * 2160) / 256) * 60, 1608000 },
>>>>>>>        { ((4096 * 2160) / 256) * 30,  826000 },
>>>>>>> @@ -264,3 +268,63 @@ struct iris_platform_data sm8550_data = {
>>>>>>>        .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:
>>>>>>> + * - vpu_ops to iris_vpu33_ops
>>>>>>> + * - clk_rst_tbl to sm8650_clk_reset_table
>>>>>>> + * - controller_rst_tbl to sm8650_controller_reset_table
>>>>>>> + * - fwname to "qcom/vpu/vpu33_p4.mbn"
>>>>>>> + */
>>>>>>> +struct iris_platform_data sm8650_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_vpu33_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 = sm8650_clk_reset_table,
>>>>>>> +    .clk_rst_tbl_size = ARRAY_SIZE(sm8650_clk_reset_table),
>>>>>>> +    .controller_rst_tbl = sm8650_controller_reset_table,
>>>>>>> +    .controller_rst_tbl_size = ARRAY_SIZE(sm8650_controller_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 = sm8550_clk_table,
>>>>>>> +    .clk_tbl_size = ARRAY_SIZE(sm8550_clk_table),
>>>>>>> +    /* Upper bound of DMA address range */
>>>>>>> +    .dma_mask = 0xe0000000 - 1,
>>>>>>> +    .fwname = "qcom/vpu/vpu33_p4.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 = ((8192 * 4352) / 256) * 2,
>>>>>>> +    .input_config_params =
>>>>>>> +        sm8550_vdec_input_config_params,
>>>>>>> +    .input_config_params_size =
>>>>>>> +        ARRAY_SIZE(sm8550_vdec_input_config_params),
>>>>>>> +    .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 = sm8550_vdec_subscribe_output_properties,
>>>>>>> +    .dec_output_prop_size =
>>>>>>> ARRAY_SIZE(sm8550_vdec_subscribe_output_properties),
>>>>>>> +
>>>>>>> +    .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),
>>>>>>> +};
>>>>>> While i was extending the data for QCS8300 (one another iris-v3 variant), i
>>>>>> realize that this file iris_platform_sm8550.c is getting dumped with all SOC
>>>>>> platform data. It would be a good idea at this point to split it into something
>>>>>> like this
>>>>>> 1. Introduce SOC specific c file and move the respective SOC platform data to
>>>>>> it, for ex, in this case sm8650_data
>>>>>> 2. Move the common structs from iris_platform_sm8550.c to
>>>>>> iris_platform_common.h. This way more SOCs getting added in future, can include
>>>>>> the common header to reuse them, otherwise it would end up using 8550.c for all
>>>>>> future SOC.
>>>>>>
>>>>>> Share your comments if you have any better approach to manage/re- use these
>>>>>> platform data considering more SOCs getting added.
>>>>>
>>>>> Right, yes the architecture is fine, but I don't feel iris_platform_common is
>>>>> the right
>>>>> place, perhaps we could introduce a platform_catalog.c where we could place all
>>>>> the common
>>>>> platform data and reuse them from the platform_<soc>.c files ?
>>>> Common structs would certainly need to be part of a header which can be
>>>> included. Where do you plan to keep common struct to be used across SOC specific
>>>> file in your approach ?
>>>>>
>>>>> I can design prototype on top of this patchset as an RFC.
>>>> I was thinking that the changes are not that big, and can be done in existing
>>>> series though.
>>>>
>>> For now, I think you can introduce a platform_sm8650.c as part of this
>>> series and use the common structure from platform_sm8550.c.
>>> Shouldn't be a big change.
>>
>> I tried but I don't how to solve this, you need a build-time ARRAY_SIZE for
>> the arrays, so they need to be in the same .c file to allow a build-time
>> evaluation of ARRAY_SIZE. If he common tables are moved into a header
>> they will be duplicated into both platform_sm8650 & platform_sm8550 objects
>> which is not what we want.
>>
>> The solution would be to write all the platform tables & iris_platform_data
>> into headers, with common headers, then include those into a platform_catalog.c
>> like is done for the drm/msm/dpu1 catalog.
>>
>> Neil
>>
>>>
>>> Later you can post a separate patch series to add platform_catalog.c and
>>> have common struct placed there which can be used across different SOC
>>> platform files.
>>>
>>> or other way is,
>>> Post a patch series to introduce platform_catalog.c with common struct and
>>> then rebase your 8650 series on top of it.
>>>
>>> Thanks,
>>> Dikshita
>>>> Thanks,
>>>> Vikash
>>>>>
>>>>> Neil
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Vikash
>>>>>>
>>>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c
>>>>>>> b/drivers/media/platform/qcom/iris/iris_probe.c
>>>>>>> index
>>>>>>> 4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e..7cd8650fbe9c09598670530103e3d5edf32953e7 100644
>>>>>>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>>>>>>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>>>>>>> @@ -345,6 +345,10 @@ static const struct of_device_id iris_dt_match[] = {
>>>>>>>                .data = &sm8250_data,
>>>>>>>            },
>>>>>>>    #endif
>>>>>>> +    {
>>>>>>> +        .compatible = "qcom,sm8650-iris",
>>>>>>> +        .data = &sm8650_data,
>>>>>>> +    },
>>>>>>>        { },
>>>>>>>    };
>>>>>>>    MODULE_DEVICE_TABLE(of, iris_dt_match);
>>>>>>>
>>>>>
>>
> 
> Eh as I read the suggestion about putting the structs - instantiating the structs in the header, I wondered if that would link but anyway.
> 
> One way to solve this without going the dpu1 route right now is to rename the platform files
> 
> iris_platform_sm8250.c -> iris_platform_common_hfi_gen1.c
> iris_platform_sm8550.c -> iris_platform_common_hfi_gen2.c
> 
> The differentiation around HFI into "generations" instead of incrementing the already existing HFIXXX version is unfortunate.
> 
> At least this way we have a repository of common HFI code located in each file, in expectation of HFI GEN3 whenever.

Yeah I somehow did something like that, please review:
https://lore.kernel.org/all/20250410-topic-sm8x50-upstream-iris-catalog-v5-0-44a431574c25@linaro.org/

Neil

> 
> ---
> bod