On Thu, Jan 16, 2025 at 12:32:34PM +0530, Dikshita Agarwal wrote:
> Initialize the platform data and enable video driver probe of SM8250
> SoC. Add a kernel param to select between venus and iris drivers for
> platforms supported by both drivers, for ex: SM8250.
>
> This is for preview only, and I will post a proper v10,
> if everyone is OK with this RFC patch.
>
> Tested-by: Stefan Schmidt <stefan.schmidt@xxxxxxxxxx> # x1e80100 (Dell XPS 13 9345)
> Reviewed-by: Stefan Schmidt <stefan.schmidt@xxxxxxxxxx>
> Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> # on SM8550-QRD
> Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> # on SM8550-SDK
> Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx>
Hmm, it seems lore has only this patch from the whole series. Was it
sent properly?
> ---
> drivers/media/platform/qcom/iris/Makefile | 1 +
> drivers/media/platform/qcom/iris/iris_core.h | 1 +
> .../platform/qcom/iris/iris_platform_common.h | 1 +
> .../platform/qcom/iris/iris_platform_sm8250.c | 149 ++++++++++++++++++
> drivers/media/platform/qcom/iris/iris_probe.c | 48 ++++++
> drivers/media/platform/qcom/venus/core.c | 5 +
> drivers/media/platform/qcom/venus/core.h | 1 +
> 7 files changed, 206 insertions(+)
> create mode 100644 drivers/media/platform/qcom/iris/iris_platform_sm8250.c
>
> diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
> index ca31db847273..a746681e03cd 100644
> --- a/drivers/media/platform/qcom/iris/Makefile
> +++ b/drivers/media/platform/qcom/iris/Makefile
> @@ -9,6 +9,7 @@ iris-objs += iris_buffer.o \
> iris_hfi_gen2_packet.o \
> iris_hfi_gen2_response.o \
> iris_hfi_queue.o \
> + iris_platform_sm8250.o \
> iris_platform_sm8550.o \
> iris_power.o \
> iris_probe.o \
> diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
> index 37fb4919fecc..f2e478c25c02 100644
> --- a/drivers/media/platform/qcom/iris/iris_core.h
> +++ b/drivers/media/platform/qcom/iris/iris_core.h
> @@ -107,5 +107,6 @@ struct iris_core {
>
> int iris_core_init(struct iris_core *core);
> void iris_core_deinit(struct iris_core *core);
> +bool iris_should_not_bind(struct device *dev);
>
> #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index 189dd081ad0a..af24ce4fc417 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -34,6 +34,7 @@ enum pipe_type {
> };
>
> extern struct iris_platform_data sm8550_data;
> +extern struct iris_platform_data sm8250_data;
Keep it sorted, please.
>
> enum platform_clk_type {
> IRIS_AXI_CLK,
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c b/drivers/media/platform/qcom/iris/iris_platform_sm8250.c
> new file mode 100644
> index 000000000000..b14b1c5d632c
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8250.c
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include "iris_core.h"
> +#include "iris_ctrls.h"
> +#include "iris_platform_common.h"
> +#include "iris_resources.h"
> +#include "iris_hfi_gen1.h"
> +#include "iris_hfi_gen1_defines.h"
> +#include "iris_vpu_common.h"
> +
> +static struct platform_inst_fw_cap inst_fw_cap_sm8250[] = {
> + {
> + .cap_id = PIPE,
> + .min = PIPE_1,
> + .max = PIPE_4,
> + .step_or_mask = 1,
> + .value = PIPE_4,
> + .hfi_id = HFI_PROPERTY_PARAM_WORK_ROUTE,
> + .set = iris_set_pipe,
> + },
> + {
> + .cap_id = STAGE,
> + .min = STAGE_1,
> + .max = STAGE_2,
> + .step_or_mask = 1,
> + .value = STAGE_2,
> + .hfi_id = HFI_PROPERTY_PARAM_WORK_MODE,
> + .set = iris_set_stage,
> + },
> + {
> + .cap_id = DEBLOCK,
> + .min = 0,
> + .max = 1,
> + .step_or_mask = 1,
> + .value = 0,
> + .hfi_id = HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER,
> + .set = iris_set_u32,
> + },
> +};
> +
> +static struct platform_inst_caps platform_inst_cap_sm8250 = {
> + .min_frame_width = 128,
> + .max_frame_width = 8192,
> + .min_frame_height = 128,
> + .max_frame_height = 8192,
> + .max_mbpf = 138240,
> + .mb_cycles_vsp = 25,
> + .mb_cycles_vpp = 200,
> +};
> +
> +static void iris_set_sm8250_preset_registers(struct iris_core *core)
> +{
> + writel(0x0, core->reg_base + 0xB0088);
> +}
> +
> +static const struct icc_info sm8250_icc_table[] = {
> + { "cpu-cfg", 1000, 1000 },
> + { "video-mem", 1000, 15000000 },
> +};
> +
> +static const char * const sm8250_clk_reset_table[] = { "bus", "core" };
> +
> +static const struct bw_info sm8250_bw_table_dec[] = {
> + { ((4096 * 2160) / 256) * 60, 2403000 },
> + { ((4096 * 2160) / 256) * 30, 1224000 },
> + { ((1920 * 1080) / 256) * 60, 812000 },
> + { ((1920 * 1080) / 256) * 30, 416000 },
> +};
> +
> +static const char * const sm8250_pmdomain_table[] = { "venus", "vcodec0" };
> +
> +static const char * const sm8250_opp_pd_table[] = { "mx" };
> +
> +static const struct platform_clk_data sm8250_clk_table[] = {
> + {IRIS_AXI_CLK, "iface" },
> + {IRIS_CTRL_CLK, "core" },
> + {IRIS_HW_CLK, "vcodec0_core" },
> +};
> +
> +static struct tz_cp_config tz_cp_config_sm8250 = {
> + .cp_start = 0,
> + .cp_size = 0x25800000,
> + .cp_nonpixel_start = 0x01000000,
> + .cp_nonpixel_size = 0x24800000,
> +};
> +
> +static const u32 sm8250_vdec_input_config_param[] = {
> + HFI_PROPERTY_PARAM_FRAME_SIZE,
> + HFI_PROPERTY_CONFIG_VIDEOCORES_USAGE,
> + HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SELECT,
> + HFI_PROPERTY_PARAM_UNCOMPRESSED_PLANE_ACTUAL_CONSTRAINTS_INFO,
> + HFI_PROPERTY_PARAM_BUFFER_COUNT_ACTUAL,
> + HFI_PROPERTY_PARAM_VDEC_MULTI_STREAM,
> + HFI_PROPERTY_PARAM_BUFFER_SIZE_ACTUAL,
> + HFI_PROPERTY_PARAM_BUFFER_ALLOC_MODE,
> +};
> +
> +static const u32 sm8250_dec_ip_int_buf_tbl[] = {
> + BUF_BIN,
> + BUF_SCRATCH_1,
> +};
> +
> +static const u32 sm8250_dec_op_int_buf_tbl[] = {
> + BUF_DPB,
> +};
> +
> +struct iris_platform_data sm8250_data = {
> + .get_instance = iris_hfi_gen1_get_instance,
> + .init_hfi_command_ops = &iris_hfi_gen1_command_ops_init,
> + .init_hfi_response_ops = iris_hfi_gen1_response_ops_init,
> + .vpu_ops = &iris_vpu2_ops,
> + .set_preset_registers = iris_set_sm8250_preset_registers,
> + .icc_tbl = sm8250_icc_table,
> + .icc_tbl_size = ARRAY_SIZE(sm8250_icc_table),
> + .clk_rst_tbl = sm8250_clk_reset_table,
> + .clk_rst_tbl_size = ARRAY_SIZE(sm8250_clk_reset_table),
> + .bw_tbl_dec = sm8250_bw_table_dec,
> + .bw_tbl_dec_size = ARRAY_SIZE(sm8250_bw_table_dec),
> + .pmdomain_tbl = sm8250_pmdomain_table,
> + .pmdomain_tbl_size = ARRAY_SIZE(sm8250_pmdomain_table),
> + .opp_pd_tbl = sm8250_opp_pd_table,
> + .opp_pd_tbl_size = ARRAY_SIZE(sm8250_opp_pd_table),
> + .clk_tbl = sm8250_clk_table,
> + .clk_tbl_size = ARRAY_SIZE(sm8250_clk_table),
> + /* Upper bound of DMA address range */
> + .dma_mask = 0xe0000000 - 1,
> + .fwname = "qcom/vpu-1.0/venus.mbn",
> + .pas_id = IRIS_PAS_ID,
> + .inst_caps = &platform_inst_cap_sm8250,
> + .inst_fw_caps = inst_fw_cap_sm8250,
> + .inst_fw_caps_size = ARRAY_SIZE(inst_fw_cap_sm8250),
> + .tz_cp_config_data = &tz_cp_config_sm8250,
> + .hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
> + .num_vpp_pipe = 4,
> + .max_session_count = 16,
> + .max_core_mbpf = (8192 * 4352) / 256,
> + .input_config_params =
> + sm8250_vdec_input_config_param,
> + .input_config_params_size =
> + ARRAY_SIZE(sm8250_vdec_input_config_param),
> +
> + .dec_ip_int_buf_tbl = sm8250_dec_ip_int_buf_tbl,
> + .dec_ip_int_buf_tbl_size = ARRAY_SIZE(sm8250_dec_ip_int_buf_tbl),
> + .dec_op_int_buf_tbl = sm8250_dec_op_int_buf_tbl,
> + .dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8250_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 954cc7c0cc97..a9cec0d15dcb 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -189,6 +189,45 @@ static void iris_sys_error_handler(struct work_struct *work)
> iris_core_init(core);
> }
>
> +/* The venus driver supports only hfi gen1 to communicate with the firmware while
> + * the iris driver supports both hfi gen1 and hfi gen2.
> + * The support of hfi gen1 is added to the iris driver with the intention that
> + * it can support old gen1 interface based firmware, while enabling gen2 based future SOCs.
> + * With this, the plan is to migrate older SOCs from venus to iris.
> + * As of now, since the iris driver supports only entry level features and doesn't have
> + * feature parity with the venus driver, a runtime-selection is provided to user via
> + * module parameter 'prefer_venus' to select the driver.
> + * This selection is available only for the SoCs which are supported by both venus
> + * and iris eg: SM8250.
> + * When the feature parity is achieved, the plan is to switch the default to point to
> + * the iris driver, then gradually start removing platforms from venus.
> + * Hardware supported by only venus - 8916, 8996, SDM660, SDM845, SC7180, SC7280
Please mention that SC7280 should also be migrated to the Iris driver by
the interested parties.
> + * Hardware supported by only iris - SM8550
> + * Hardware supported by both venus and iris - SM8250
> + */
> +
> +#if IS_REACHABLE(CONFIG_VIDEO_QCOM_VENUS)
> +static bool prefer_venus = true;
> +MODULE_PARM_DESC(prefer_venus, "Select whether venus or iris driver should be preferred");
> +module_param(prefer_venus, bool, 0444);
> +
> +/* list all platforms supported by both venus and iris drivers */
> +static const char *const venus_to_iris_migration[] = {
> + "qcom,sm8250-venus",
> + NULL,
> +};
> +
> +bool iris_should_not_bind(struct device *dev)
> +{
> + /* If it is in the migration list, use venus */
> + if (of_device_compatible_match(dev->of_node, venus_to_iris_migration))
> + return prefer_venus;
> +
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(iris_should_not_bind);
> +#endif
> +
> static int iris_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -196,6 +235,11 @@ static int iris_probe(struct platform_device *pdev)
> u64 dma_mask;
> int ret;
>
> +#if IS_REACHABLE(CONFIG_VIDEO_QCOM_VENUS)
> + if (iris_should_not_bind(&pdev->dev))
> + return -ENODEV;
> +#endif
> +
> core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);
> if (!core)
> return -ENOMEM;
> @@ -324,6 +368,10 @@ static const struct of_device_id iris_dt_match[] = {
> .compatible = "qcom,sm8550-iris",
> .data = &sm8550_data,
> },
> + {
> + .compatible = "qcom,sm8250-venus",
> + .data = &sm8250_data,
> + },
> { },
> };
> MODULE_DEVICE_TABLE(of, iris_dt_match);
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index 77d48578ecd2..9116188bfe74 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -375,6 +375,11 @@ static int venus_probe(struct platform_device *pdev)
> struct venus_core *core;
> int ret;
>
> +#if IS_REACHABLE(CONFIG_VIDEO_QCOM_IRIS)
> + if (!iris_should_not_bind(&pdev->dev))
> + return -ENODEV;
> +#endif
> +
The practice of having preprocessor code in function body is frowned
upon. Please add a stub function instead.
Also, as I wrote earlier, this call will result in (unnecessary) runtime
dependency from venus to the iris module. Please add a small helper
module which basically contains this function (and a separate header
file for it). Then both iris and venus drivers can use that function.
> core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
> if (!core)
> return -ENOMEM;
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index abeeafa86697..e2e7d8ec9807 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -570,4 +570,5 @@ is_fw_rev_or_older(struct venus_core *core, u32 vmajor, u32 vminor, u32 vrev)
> }
>
> void venus_close_common(struct venus_inst *inst);
> +extern bool iris_should_not_bind(struct device *dev);
> #endif
> --
> 2.34.1
>
--
With best wishes
Dmitry
