| Message ID | 20250410-topic-sm8x50-upstream-iris-catalog-v5-1-44a431574c25@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: qcom: iris: re-organize catalog & add support for SM8650 | expand |
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. > 3 files changed, 89 insertions(+), 76 deletions(-) >
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 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 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 Fri, Apr 11, 2025 at 10:14:02AM +0200, Neil Armstrong wrote: > 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 ? No. Keep the tables that are shared in iris_catalog_gen1.c, keep platform data in iris_catalog_sm8250.c and iris_catalog_sm8550.c (and later iris_catalog_sm8650.c) > > I want just to add SM8650 support, not to entirely rework the > whole iris driver. > > Neil > > > > > > 3 files changed, 89 insertions(+), 76 deletions(-) > > > > > >
On 14/04/2025 09:39, Dmitry Baryshkov wrote: > On Fri, Apr 11, 2025 at 10:14:02AM +0200, Neil Armstrong wrote: >> 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 ? > > No. Keep the tables that are shared in iris_catalog_gen1.c, keep > platform data in iris_catalog_sm8250.c and iris_catalog_sm8550.c (and > later iris_catalog_sm8650.c) This won't work, we need ARRAY_SIZE() for most of the tables Neil > >> >> I want just to add SM8650 support, not to entirely rework the >> whole iris driver. >> >> Neil >> >>> >>>> 3 files changed, 89 insertions(+), 76 deletions(-) >>>> >>> >> >
On 14/04/2025 12:07, Neil Armstrong wrote: > On 14/04/2025 09:39, Dmitry Baryshkov wrote: >> On Fri, Apr 11, 2025 at 10:14:02AM +0200, Neil Armstrong wrote: >>> 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 ? >> >> No. Keep the tables that are shared in iris_catalog_gen1.c, keep >> platform data in iris_catalog_sm8250.c and iris_catalog_sm8550.c (and >> later iris_catalog_sm8650.c) > > This won't work, we need ARRAY_SIZE() for most of the tables I see. Can you do it other way around: export platform-specific data from the iris_catalog_sm8250.c and use it inside iris_catalog_gen1.c? > > Neil > >> >>> >>> I want just to add SM8650 support, not to entirely rework the >>> whole iris driver. >>> >>> Neil >>> >>>> >>>>> 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" + +/* 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 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
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(-)