Re: [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog

[Neil Armstrong <neil.armstrong@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>