Re: [PATCH v4 6/6] media: platform: qcom/iris: add sm8650 support
From: neil . armstrong
Date: Thu Apr 10 2025 - 11:40:39 EST
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@xxxxxxxxxx> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
---
.../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);