| Message ID | 20250410-topic-sm8x50-upstream-iris-catalog-v5-0-44a431574c25@linaro.org |
|---|---|
| Headers | show |
| Series | media: qcom: iris: re-organize catalog & add support for SM8650 | expand |
On 10/04/2025 21:20, Bryan O'Donoghue wrote: > On 10/04/2025 17:30, 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> >> --- >> .../media/platform/qcom/iris/iris_catalog_gen2.c | 1 + >> .../media/platform/qcom/iris/iris_catalog_sm8650.h | 68 ++++++++++++++++++++++ >> .../platform/qcom/iris/iris_platform_common.h | 1 + >> drivers/media/platform/qcom/iris/iris_probe.c | 4 ++ >> 4 files changed, 74 insertions(+) >> >> diff --git a/drivers/media/platform/qcom/iris/iris_catalog_gen2.c b/drivers/media/platform/qcom/iris/iris_catalog_gen2.c >> index c3f8ad004cb7f9317859b2594640c7138dbb6534..ad559351f1125d266dedac7eb6e91cda90bbae72 100644 >> --- a/drivers/media/platform/qcom/iris/iris_catalog_gen2.c >> +++ b/drivers/media/platform/qcom/iris/iris_catalog_gen2.c >> @@ -186,3 +186,4 @@ static const u32 sm8550_dec_op_int_buf_tbl[] = { >> /* platforms catalogs */ >> #include "iris_catalog_sm8550.h" >> +#include "iris_catalog_sm8650.h" >> diff --git a/drivers/media/platform/qcom/iris/iris_catalog_sm8650.h b/drivers/media/platform/qcom/iris/iris_catalog_sm8650.h >> new file mode 100644 >> index 0000000000000000000000000000000000000000..be8737dd4f3d9ec20a457d50076be1b4d841787c >> --- /dev/null >> +++ b/drivers/media/platform/qcom/iris/iris_catalog_sm8650.h >> @@ -0,0 +1,68 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#ifndef _IRIS_CATALOG_SM8650_H >> +#define _IRIS_CATALOG_SM8650_H >> + >> +#define VIDEO_ARCH_LX 1 >> + >> +static const char * const sm8650_clk_reset_table[] = { "bus", "core" }; >> + >> +static const char * const sm8650_controller_reset_table[] = { "xo" }; >> + >> +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), >> +}; >> + >> +#endif >> 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_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); >> > > This LGTM one thing is I think you should convert the sm8250 stuff into a corresponding iris_catalog_gen1.c This is done in patch 1 Neil > > Would be grateful if you could add that patch to a V6. > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 10/04/2025 21:44, Dmitry Baryshkov wrote: > On Thu, Apr 10, 2025 at 06:30:00PM +0200, Neil Armstrong wrote: >> Re-organize the platform support core into a gen1 catalog C file >> declaring common platform structure and include platform headers >> containing platform specific entries and iris_platform_data >> structure. >> >> The goal is to share most of the structure while having >> clear and separate per-SoC catalog files. >> >> The organization is based on the current drm/msm dpu1 catalog >> entries. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> drivers/media/platform/qcom/iris/Makefile | 2 +- >> .../media/platform/qcom/iris/iris_catalog_gen1.c | 83 ++++++++++++++++++++++ >> ...ris_platform_sm8250.c => iris_catalog_sm8250.h} | 80 ++------------------- > > I'd suggest _not_ to follow DPU here. I like the per-generation files, > but please consider keeping platform files as separate C files too. This would duplicate all tables, do we really want this ? I want just to add SM8650 support, not to entirely rework the whole iris driver. Neil > >> 3 files changed, 89 insertions(+), 76 deletions(-) >> >
On 11/04/2025 09:11, Neil Armstrong wrote: >> This LGTM one thing is I think you should convert the sm8250 stuff >> into a corresponding iris_catalog_gen1.c > > This is done in patch 1 > > Neil True, patches 1 & 2 didn't hit my inbox. Never mind. --- bod
On 10/04/2025 17:30, Neil Armstrong wrote: > Re-organize the platform support core into a gen1 catalog C file > declaring common platform structure and include platform headers > containing platform specific entries and iris_platform_data > structure. > > The goal is to share most of the structure while having > clear and separate per-SoC catalog files. > > The organization is based on the current drm/msm dpu1 catalog > entries. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/media/platform/qcom/iris/Makefile | 2 +- > .../media/platform/qcom/iris/iris_catalog_gen1.c | 83 ++++++++++++++++++++++ > ...ris_platform_sm8250.c => iris_catalog_sm8250.h} | 80 ++------------------- > 3 files changed, 89 insertions(+), 76 deletions(-) > > diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile > index 35390534534e93f4617c1036a05ca0921567ba1d..7e7bc5ca81e0f0119846ccaff7f79fd17b8298ca 100644 > --- a/drivers/media/platform/qcom/iris/Makefile > +++ b/drivers/media/platform/qcom/iris/Makefile > @@ -25,7 +25,7 @@ qcom-iris-objs += \ > iris_vpu_common.o \ > > ifeq ($(CONFIG_VIDEO_QCOM_VENUS),) > -qcom-iris-objs += iris_platform_sm8250.o > +qcom-iris-objs += iris_catalog_gen1.o > endif > > obj-$(CONFIG_VIDEO_QCOM_IRIS) += qcom-iris.o > diff --git a/drivers/media/platform/qcom/iris/iris_catalog_gen1.c b/drivers/media/platform/qcom/iris/iris_catalog_gen1.c > new file mode 100644 > index 0000000000000000000000000000000000000000..c4590f8996431eb5103d45f01c6bee2b38b848c2 > --- /dev/null > +++ b/drivers/media/platform/qcom/iris/iris_catalog_gen1.c > @@ -0,0 +1,83 @@ > +// 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" Any reason why these aren't alphabetised ? Please do so unless there's some technical reason to have in this order. > + > +/* Common SM8250 & variants */ > +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 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_default[] = { > + 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_FRAME_SIZE, > + 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, > +}; > + > +/* platforms catalogs */ > +#include "iris_catalog_sm8250.h" > diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c b/drivers/media/platform/qcom/iris/iris_catalog_sm8250.h > similarity index 59% > rename from drivers/media/platform/qcom/iris/iris_platform_sm8250.c > rename to drivers/media/platform/qcom/iris/iris_catalog_sm8250.h > index 5c86fd7b7b6fd36dc2d57a1705d915308b4c0f92..4d2df669b3e1df2ef2b0d2f88fc5f309b27546db 100644 > --- a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c > +++ b/drivers/media/platform/qcom/iris/iris_catalog_sm8250.h > @@ -1,55 +1,10 @@ > -// SPDX-License-Identifier: GPL-2.0-only > +/* 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, > -}; > +#ifndef _IRIS_CATALOG_SM8250_H > +#define _IRIS_CATALOG_SM8250_H __IRIS_CATALOG_SM8250_H__ as with other header guards. > > static void iris_set_sm8250_preset_registers(struct iris_core *core) > { > @@ -80,33 +35,6 @@ static const struct platform_clk_data sm8250_clk_table[] = { > {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_default[] = { > - 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_FRAME_SIZE, > - 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, > @@ -147,3 +75,5 @@ struct iris_platform_data sm8250_data = { > .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), > }; > + > +#endif > > -- > 2.34.1 > > Once done. Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 10/04/2025 17:30, Neil Armstrong wrote: > Re-organize the platform support core into a gen2 catalog C file > declaring common platform structure and include platform headers > containing platform specific entries and iris_platform_data > structure. > > The goal is to share most of the structure while having > clear and separate per-SoC catalog files. > > The organization is based on the current drm/msm dpu1 catalog > entries. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> Same comment on alphanumeric sort and header __PREFIXES__ fixup then Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 10/04/2025 17:29, Neil Armstrong wrote: > Re-organize the platform support core into a gen1 catalog C file > declaring common platform structure and include platform headers > containing platform specific entries and iris_platform_data > structure. > > The goal is to share most of the structure while having > clear and separate per-SoC catalog files. > > The organization is based on the curent drm/msm dpu1 catalog > entries. > > 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. > > Follow-up of [1]: > https://lore.kernel.org/all/20250409-topic-sm8x50-iris-v10-v4-0-40e411594285@linaro.org/ > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > Changes in v4: > - Reorganized into catalog, rebased sm8650 support on top > - Link to v4: https://lore.kernel.org/all/20250409-topic-sm8x50-iris-v10-v4-0-40e411594285@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 (8): > media: qcom: iris: move sm8250 to gen1 catalog > media: qcom: iris: move sm8550 to gen2 catalog > 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 | 6 +- > .../media/platform/qcom/iris/iris_catalog_gen1.c | 83 +++++++ > ...{iris_platform_sm8550.c => iris_catalog_gen2.c} | 85 +------ > ...ris_platform_sm8250.c => iris_catalog_sm8250.h} | 80 +----- > .../media/platform/qcom/iris/iris_catalog_sm8550.h | 91 +++++++ > .../media/platform/qcom/iris/iris_catalog_sm8650.h | 68 +++++ > drivers/media/platform/qcom/iris/iris_core.h | 1 + > .../platform/qcom/iris/iris_platform_common.h | 3 + > 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 + > 15 files changed, 598 insertions(+), 300 deletions(-) > --- > base-commit: 2bdde620f7f2bff2ff1cb7dc166859eaa0c78a7c > change-id: 20250410-topic-sm8x50-upstream-iris-catalog-3e2e4a033d6f > > Best regards, > -- > Neil Armstrong <neil.armstrong@linaro.org> > > Please fixup this 0007-media-platform-qcom-iris-add-support-for-vpu33.patch has no obvious style problems and is ready for submission. 0007-media-platform-qcom-iris-add-support-for-vpu33.patch:7: slighly ==> slightly also accounting for my comments in patches #1 and #2 you can add for the series Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 11/04/2025 13:50, Bryan O'Donoghue wrote: > On 10/04/2025 17:30, Neil Armstrong wrote: >> Re-organize the platform support core into a gen1 catalog C file >> declaring common platform structure and include platform headers >> containing platform specific entries and iris_platform_data >> structure. >> >> The goal is to share most of the structure while having >> clear and separate per-SoC catalog files. >> >> The organization is based on the current drm/msm dpu1 catalog >> entries. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> drivers/media/platform/qcom/iris/Makefile | 2 +- >> .../media/platform/qcom/iris/iris_catalog_gen1.c | 83 ++++++++++++++++++++++ >> ...ris_platform_sm8250.c => iris_catalog_sm8250.h} | 80 ++------------------- >> 3 files changed, 89 insertions(+), 76 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile >> index 35390534534e93f4617c1036a05ca0921567ba1d..7e7bc5ca81e0f0119846ccaff7f79fd17b8298ca 100644 >> --- a/drivers/media/platform/qcom/iris/Makefile >> +++ b/drivers/media/platform/qcom/iris/Makefile >> @@ -25,7 +25,7 @@ qcom-iris-objs += \ >> iris_vpu_common.o \ >> >> ifeq ($(CONFIG_VIDEO_QCOM_VENUS),) >> -qcom-iris-objs += iris_platform_sm8250.o >> +qcom-iris-objs += iris_catalog_gen1.o >> endif >> >> obj-$(CONFIG_VIDEO_QCOM_IRIS) += qcom-iris.o >> diff --git a/drivers/media/platform/qcom/iris/iris_catalog_gen1.c b/drivers/media/platform/qcom/iris/iris_catalog_gen1.c >> new file mode 100644 >> index 0000000000000000000000000000000000000000..c4590f8996431eb5103d45f01c6bee2b38b848c2 >> --- /dev/null >> +++ b/drivers/media/platform/qcom/iris/iris_catalog_gen1.c >> @@ -0,0 +1,83 @@ >> +// 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" > > Any reason why these aren't alphabetised ? Copied as-is, I guess the order can be important, especially the iris_core must be first. Neil > > Please do so unless there's some technical reason to have in this order. > >> + >> +/* Common SM8250 & variants */ >> +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 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_default[] = { >> + 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_FRAME_SIZE, >> + 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, >> +}; >> + >> +/* platforms catalogs */ >> +#include "iris_catalog_sm8250.h" >> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c b/drivers/media/platform/qcom/iris/iris_catalog_sm8250.h >> similarity index 59% >> rename from drivers/media/platform/qcom/iris/iris_platform_sm8250.c >> rename to drivers/media/platform/qcom/iris/iris_catalog_sm8250.h >> index 5c86fd7b7b6fd36dc2d57a1705d915308b4c0f92..4d2df669b3e1df2ef2b0d2f88fc5f309b27546db 100644 >> --- a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c >> +++ b/drivers/media/platform/qcom/iris/iris_catalog_sm8250.h >> @@ -1,55 +1,10 @@ >> -// SPDX-License-Identifier: GPL-2.0-only >> +/* 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, >> -}; >> +#ifndef _IRIS_CATALOG_SM8250_H >> +#define _IRIS_CATALOG_SM8250_H > > __IRIS_CATALOG_SM8250_H__ as with other header guards. > >> >> static void iris_set_sm8250_preset_registers(struct iris_core *core) >> { >> @@ -80,33 +35,6 @@ static const struct platform_clk_data sm8250_clk_table[] = { >> {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_default[] = { >> - 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_FRAME_SIZE, >> - 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, >> @@ -147,3 +75,5 @@ struct iris_platform_data sm8250_data = { >> .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), >> }; >> + >> +#endif >> >> -- >> 2.34.1 >> >> > Once done. > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 11/04/2025 12:55, Bryan O'Donoghue wrote: > On 10/04/2025 17:29, Neil Armstrong wrote: >> Re-organize the platform support core into a gen1 catalog C file >> declaring common platform structure and include platform headers >> containing platform specific entries and iris_platform_data >> structure. >> >> The goal is to share most of the structure while having >> clear and separate per-SoC catalog files. >> >> The organization is based on the curent drm/msm dpu1 catalog >> entries. >> >> 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. >> >> Follow-up of [1]: >> https://lore.kernel.org/all/20250409-topic-sm8x50-iris-v10-v4-0-40e411594285@linaro.org/ >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> Changes in v4: >> - Reorganized into catalog, rebased sm8650 support on top >> - Link to v4: https://lore.kernel.org/all/20250409-topic-sm8x50-iris-v10-v4-0-40e411594285@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 (8): >> media: qcom: iris: move sm8250 to gen1 catalog >> media: qcom: iris: move sm8550 to gen2 catalog >> 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 | 6 +- >> .../media/platform/qcom/iris/iris_catalog_gen1.c | 83 +++++++ >> ...{iris_platform_sm8550.c => iris_catalog_gen2.c} | 85 +------ >> ...ris_platform_sm8250.c => iris_catalog_sm8250.h} | 80 +----- >> .../media/platform/qcom/iris/iris_catalog_sm8550.h | 91 +++++++ >> .../media/platform/qcom/iris/iris_catalog_sm8650.h | 68 +++++ >> drivers/media/platform/qcom/iris/iris_core.h | 1 + >> .../platform/qcom/iris/iris_platform_common.h | 3 + >> 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 + >> 15 files changed, 598 insertions(+), 300 deletions(-) >> --- >> base-commit: 2bdde620f7f2bff2ff1cb7dc166859eaa0c78a7c >> change-id: 20250410-topic-sm8x50-upstream-iris-catalog-3e2e4a033d6f >> >> Best regards, >> -- >> Neil Armstrong <neil.armstrong@linaro.org> >> >> > > Please fixup this > > 0007-media-platform-qcom-iris-add-support-for-vpu33.patch has no obvious > style problems and is ready for submission. > 0007-media-platform-qcom-iris-add-support-for-vpu33.patch:7: slighly ==> > slightly > > also accounting for my comments in patches #1 and #2 you can add for the > series > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > There's an update to the yaml you need to account for now. https://gitlab.freedesktop.org/linux-media/users/bodonoghue/-/blob/next/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml?ref_type=heads#L25 --- bod
Re-organize the platform support core into a gen1 catalog C file declaring common platform structure and include platform headers containing platform specific entries and iris_platform_data structure. The goal is to share most of the structure while having clear and separate per-SoC catalog files. The organization is based on the curent drm/msm dpu1 catalog entries. 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. Follow-up of [1]: https://lore.kernel.org/all/20250409-topic-sm8x50-iris-v10-v4-0-40e411594285@linaro.org/ Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- Changes in v4: - Reorganized into catalog, rebased sm8650 support on top - Link to v4: https://lore.kernel.org/all/20250409-topic-sm8x50-iris-v10-v4-0-40e411594285@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 (8): media: qcom: iris: move sm8250 to gen1 catalog media: qcom: iris: move sm8550 to gen2 catalog 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 | 6 +- .../media/platform/qcom/iris/iris_catalog_gen1.c | 83 +++++++ ...{iris_platform_sm8550.c => iris_catalog_gen2.c} | 85 +------ ...ris_platform_sm8250.c => iris_catalog_sm8250.h} | 80 +----- .../media/platform/qcom/iris/iris_catalog_sm8550.h | 91 +++++++ .../media/platform/qcom/iris/iris_catalog_sm8650.h | 68 +++++ drivers/media/platform/qcom/iris/iris_core.h | 1 + .../platform/qcom/iris/iris_platform_common.h | 3 + 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 + 15 files changed, 598 insertions(+), 300 deletions(-) --- base-commit: 2bdde620f7f2bff2ff1cb7dc166859eaa0c78a7c change-id: 20250410-topic-sm8x50-upstream-iris-catalog-3e2e4a033d6f Best regards,