| Message ID | 20250305-topic-sm8x50-iris-v10-v2-7-bd65a3fc099e@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/7] dt-bindings: media: qcom,sm8550-iris: document SM8650 IRIS accelerator | expand |
On Thu, Mar 06, 2025 at 06:46:06PM +0530, Dikshita Agarwal wrote: > > > On 3/6/2025 12:35 AM, 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.284 > > decoder capabilities are identical. > > > > 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 = { [...] > > + > > + .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), > > +}; > This approach looks good to me, reusing the platform data like this keeps > the code cleaner and avoids duplication. I think this is a good way to > handle the differences while sharing the common parts. Just to share some thoughts. We had this kind of data-sharing in the DPU driver. It looked good in the beginning, but at some point we found ourselves stuck with the platform data being named semi-randomly (following the first-added SoC instead of the first-in-family). Modifying "catalog" data became troublesome as it was no longer clear, which chipsets are going to be affected by the change. So, after some thought we ended up duplicating data all over the catalog files for the sake of them being easy to modify. There are some ideas on how to simplify that, but for now we have (almost) full data set for each SoC. For example, imagine somebody adding sm8450 support and sm8450 reusing sm8550 data. It would be a bit troublesome to remember that changing sm8550 data would affect sm8450. And maybe some of the SAR, SA or QCM/QCS platforms. I think you see the point. If you are to explore the data sharing solution, I'd suggest exploring an idea similar to the DPU catalog: start naming each of the platform files with some kind of generation-like ID. In case of DPU it was easy as each DPU instance has unique version. For the Iris devices it might be as easy as vpu30_sm8550, vpu33_sm8650, etc. It might make it easier to make assumptions and derive common pieces of data.
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);
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.284 decoder capabilities are identical. 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(+)