* [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
@ 2025-04-10 16:30 ` Neil Armstrong
2025-04-10 19:44 ` Dmitry Baryshkov
2025-04-11 11:50 ` Bryan O'Donoghue
2025-04-10 16:30 ` [PATCH RFC v5 2/8] media: qcom: iris: move sm8550 to gen2 catalog Neil Armstrong
` (8 subsequent siblings)
9 siblings, 2 replies; 31+ messages in thread
From: Neil Armstrong @ 2025-04-10 16:30 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree, Neil Armstrong
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"
+
+/* 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
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog
2025-04-10 16:30 ` [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog Neil Armstrong
@ 2025-04-10 19:44 ` Dmitry Baryshkov
2025-04-11 8:14 ` Neil Armstrong
2025-04-11 11:50 ` Bryan O'Donoghue
1 sibling, 1 reply; 31+ messages in thread
From: Dmitry Baryshkov @ 2025-04-10 19:44 UTC (permalink / raw)
To: Neil Armstrong
Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, linux-kernel, linux-media,
linux-arm-msm, devicetree
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(-)
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog
2025-04-10 19:44 ` Dmitry Baryshkov
@ 2025-04-11 8:14 ` Neil Armstrong
2025-04-14 7:39 ` Dmitry Baryshkov
0 siblings, 1 reply; 31+ messages in thread
From: Neil Armstrong @ 2025-04-11 8:14 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, linux-kernel, linux-media,
linux-arm-msm, devicetree
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(-)
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog
2025-04-11 8:14 ` Neil Armstrong
@ 2025-04-14 7:39 ` Dmitry Baryshkov
2025-04-14 9:07 ` Neil Armstrong
0 siblings, 1 reply; 31+ messages in thread
From: Dmitry Baryshkov @ 2025-04-14 7:39 UTC (permalink / raw)
To: Neil Armstrong
Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, linux-kernel, linux-media,
linux-arm-msm, devicetree
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(-)
> > >
> >
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog
2025-04-14 7:39 ` Dmitry Baryshkov
@ 2025-04-14 9:07 ` Neil Armstrong
2025-04-14 9:28 ` Dmitry Baryshkov
0 siblings, 1 reply; 31+ messages in thread
From: Neil Armstrong @ 2025-04-14 9:07 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, linux-kernel, linux-media,
linux-arm-msm, devicetree
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(-)
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog
2025-04-14 9:07 ` Neil Armstrong
@ 2025-04-14 9:28 ` Dmitry Baryshkov
0 siblings, 0 replies; 31+ messages in thread
From: Dmitry Baryshkov @ 2025-04-14 9:28 UTC (permalink / raw)
To: neil.armstrong
Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, linux-kernel, linux-media,
linux-arm-msm, devicetree
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(-)
>>>>>
>>>>
>>>
>>
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog
2025-04-10 16:30 ` [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog Neil Armstrong
2025-04-10 19:44 ` Dmitry Baryshkov
@ 2025-04-11 11:50 ` Bryan O'Donoghue
2025-04-11 13:18 ` Neil Armstrong
1 sibling, 1 reply; 31+ messages in thread
From: Bryan O'Donoghue @ 2025-04-11 11:50 UTC (permalink / raw)
To: Neil Armstrong, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree
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>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog
2025-04-11 11:50 ` Bryan O'Donoghue
@ 2025-04-11 13:18 ` Neil Armstrong
0 siblings, 0 replies; 31+ messages in thread
From: Neil Armstrong @ 2025-04-11 13:18 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal,
Abhinav Kumar, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree
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>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH RFC v5 2/8] media: qcom: iris: move sm8550 to gen2 catalog
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
2025-04-10 16:30 ` [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog Neil Armstrong
@ 2025-04-10 16:30 ` Neil Armstrong
2025-04-11 11:52 ` Bryan O'Donoghue
2025-04-10 16:30 ` [PATCH RFC v5 3/8] dt-bindings: media: qcom,sm8550-iris: document SM8650 IRIS accelerator Neil Armstrong
` (7 subsequent siblings)
9 siblings, 1 reply; 31+ messages in thread
From: Neil Armstrong @ 2025-04-10 16:30 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree, Neil Armstrong
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>
---
drivers/media/platform/qcom/iris/Makefile | 2 +-
...{iris_platform_sm8550.c => iris_catalog_gen2.c} | 84 +-------------------
.../media/platform/qcom/iris/iris_catalog_sm8550.h | 91 ++++++++++++++++++++++
3 files changed, 95 insertions(+), 82 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
index 7e7bc5ca81e0f0119846ccaff7f79fd17b8298ca..379359c5c7e1e4ca39b1216335cd8cf2317b6308 100644
--- a/drivers/media/platform/qcom/iris/Makefile
+++ b/drivers/media/platform/qcom/iris/Makefile
@@ -10,7 +10,7 @@ qcom-iris-objs += \
iris_hfi_gen2_packet.o \
iris_hfi_gen2_response.o \
iris_hfi_queue.o \
- iris_platform_sm8550.o \
+ iris_catalog_gen2.o \
iris_power.o \
iris_probe.o \
iris_resources.o \
diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_catalog_gen2.c
similarity index 61%
rename from drivers/media/platform/qcom/iris/iris_platform_sm8550.c
rename to drivers/media/platform/qcom/iris/iris_catalog_gen2.c
index 35d278996c430f2856d0fe59586930061a271c3e..c3f8ad004cb7f9317859b2594640c7138dbb6534 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c
+++ b/drivers/media/platform/qcom/iris/iris_catalog_gen2.c
@@ -10,8 +10,7 @@
#include "iris_platform_common.h"
#include "iris_vpu_common.h"
-#define VIDEO_ARCH_LX 1
-
+/* Common SM8550 & variants */
static struct platform_inst_fw_cap inst_fw_cap_sm8550[] = {
{
.cap_id = PROFILE,
@@ -132,35 +131,6 @@ static struct platform_inst_caps platform_inst_cap_sm8550 = {
.num_comv = 0,
};
-static void iris_set_sm8550_preset_registers(struct iris_core *core)
-{
- writel(0x0, core->reg_base + 0xB0088);
-}
-
-static const struct icc_info sm8550_icc_table[] = {
- { "cpu-cfg", 1000, 1000 },
- { "video-mem", 1000, 15000000 },
-};
-
-static const char * const sm8550_clk_reset_table[] = { "bus" };
-
-static const struct bw_info sm8550_bw_table_dec[] = {
- { ((4096 * 2160) / 256) * 60, 1608000 },
- { ((4096 * 2160) / 256) * 30, 826000 },
- { ((1920 * 1080) / 256) * 60, 567000 },
- { ((1920 * 1080) / 256) * 30, 294000 },
-};
-
-static const char * const sm8550_pmdomain_table[] = { "venus", "vcodec0" };
-
-static const char * const sm8550_opp_pd_table[] = { "mxc", "mmcx" };
-
-static const struct platform_clk_data sm8550_clk_table[] = {
- {IRIS_AXI_CLK, "iface" },
- {IRIS_CTRL_CLK, "core" },
- {IRIS_HW_CLK, "vcodec0_core" },
-};
-
static struct ubwc_config_data ubwc_config_sm8550 = {
.max_channels = 8,
.mal_length = 32,
@@ -214,53 +184,5 @@ static const u32 sm8550_dec_op_int_buf_tbl[] = {
BUF_DPB,
};
-struct iris_platform_data sm8550_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_vpu3_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 = sm8550_clk_reset_table,
- .clk_rst_tbl_size = ARRAY_SIZE(sm8550_clk_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/vpu30_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),
-};
+/* platforms catalogs */
+#include "iris_catalog_sm8550.h"
diff --git a/drivers/media/platform/qcom/iris/iris_catalog_sm8550.h b/drivers/media/platform/qcom/iris/iris_catalog_sm8550.h
new file mode 100644
index 0000000000000000000000000000000000000000..e101eed6568bfc7c62651491daad0e9e5b0224e5
--- /dev/null
+++ b/drivers/media/platform/qcom/iris/iris_catalog_sm8550.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _IRIS_CATALOG_SM8550_H
+#define _IRIS_CATALOG_SM8550_H
+
+#define VIDEO_ARCH_LX 1
+
+static void iris_set_sm8550_preset_registers(struct iris_core *core)
+{
+ writel(0x0, core->reg_base + 0xB0088);
+}
+
+static const struct icc_info sm8550_icc_table[] = {
+ { "cpu-cfg", 1000, 1000 },
+ { "video-mem", 1000, 15000000 },
+};
+
+static const char * const sm8550_clk_reset_table[] = { "bus" };
+
+static const struct bw_info sm8550_bw_table_dec[] = {
+ { ((4096 * 2160) / 256) * 60, 1608000 },
+ { ((4096 * 2160) / 256) * 30, 826000 },
+ { ((1920 * 1080) / 256) * 60, 567000 },
+ { ((1920 * 1080) / 256) * 30, 294000 },
+};
+
+static const char * const sm8550_pmdomain_table[] = { "venus", "vcodec0" };
+
+static const char * const sm8550_opp_pd_table[] = { "mxc", "mmcx" };
+
+static const struct platform_clk_data sm8550_clk_table[] = {
+ {IRIS_AXI_CLK, "iface" },
+ {IRIS_CTRL_CLK, "core" },
+ {IRIS_HW_CLK, "vcodec0_core" },
+};
+
+struct iris_platform_data sm8550_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_vpu3_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 = sm8550_clk_reset_table,
+ .clk_rst_tbl_size = ARRAY_SIZE(sm8550_clk_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/vpu30_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
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 2/8] media: qcom: iris: move sm8550 to gen2 catalog
2025-04-10 16:30 ` [PATCH RFC v5 2/8] media: qcom: iris: move sm8550 to gen2 catalog Neil Armstrong
@ 2025-04-11 11:52 ` Bryan O'Donoghue
0 siblings, 0 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2025-04-11 11:52 UTC (permalink / raw)
To: Neil Armstrong, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree
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>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH RFC v5 3/8] dt-bindings: media: qcom,sm8550-iris: document SM8650 IRIS accelerator
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
2025-04-10 16:30 ` [PATCH RFC v5 1/8] media: qcom: iris: move sm8250 to gen1 catalog Neil Armstrong
2025-04-10 16:30 ` [PATCH RFC v5 2/8] media: qcom: iris: move sm8550 to gen2 catalog Neil Armstrong
@ 2025-04-10 16:30 ` Neil Armstrong
2025-04-10 16:30 ` [PATCH RFC v5 4/8] media: platform: qcom/iris: add power_off_controller to vpu_ops Neil Armstrong
` (6 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Neil Armstrong @ 2025-04-10 16:30 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Neil Armstrong, Bryan O'Donoghue
Document the IRIS video decoder and encoder accelerator found in the
SM8650 platform, it requires 2 more reset lines in addition to the
properties required for the SM8550 platform.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
.../bindings/media/qcom,sm8550-iris.yaml | 33 ++++++++++++++++++----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index e424ea84c211f473a799481fd5463a16580187ed..536cf458dcb08141e5a1ec8c3df964196e599a57 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -14,12 +14,11 @@ description:
The iris video processing unit is a video encode and decode accelerator
present on Qualcomm platforms.
-allOf:
- - $ref: qcom,venus-common.yaml#
-
properties:
compatible:
- const: qcom,sm8550-iris
+ enum:
+ - qcom,sm8550-iris
+ - qcom,sm8650-iris
power-domains:
maxItems: 4
@@ -49,11 +48,15 @@ properties:
- const: video-mem
resets:
- maxItems: 1
+ minItems: 1
+ maxItems: 3
reset-names:
+ minItems: 1
items:
- const: bus
+ - const: xo
+ - const: core
iommus:
maxItems: 2
@@ -75,6 +78,26 @@ required:
- iommus
- dma-coherent
+allOf:
+ - $ref: qcom,venus-common.yaml#
+ - if:
+ properties:
+ compatible:
+ enum:
+ - qcom,sm8650-iris
+ then:
+ properties:
+ resets:
+ minItems: 3
+ reset-names:
+ minItems: 3
+ else:
+ properties:
+ resets:
+ maxItems: 1
+ reset-names:
+ maxItems: 1
+
unevaluatedProperties: false
examples:
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH RFC v5 4/8] media: platform: qcom/iris: add power_off_controller to vpu_ops
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
` (2 preceding siblings ...)
2025-04-10 16:30 ` [PATCH RFC v5 3/8] dt-bindings: media: qcom,sm8550-iris: document SM8650 IRIS accelerator Neil Armstrong
@ 2025-04-10 16:30 ` Neil Armstrong
2025-04-10 16:30 ` [PATCH RFC v5 5/8] media: platform: qcom/iris: introduce optional controller_rst_tbl Neil Armstrong
` (5 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Neil Armstrong @ 2025-04-10 16:30 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Neil Armstrong, Bryan O'Donoghue
In order to support the SM8650 iris33 hardware, we need to provide a
specific constoller power off sequences via the vpu_ops callbacks.
Add the callback, and use the current helper for currently supported
platforms.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/media/platform/qcom/iris/iris_vpu2.c | 1 +
drivers/media/platform/qcom/iris/iris_vpu3.c | 1 +
drivers/media/platform/qcom/iris/iris_vpu_common.c | 4 ++--
drivers/media/platform/qcom/iris/iris_vpu_common.h | 2 ++
4 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_vpu2.c b/drivers/media/platform/qcom/iris/iris_vpu2.c
index 8f502aed43ce2fa6a272a2ce14ff1ca54d3e63a2..7cf1bfc352d34b897451061b5c14fbe90276433d 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu2.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu2.c
@@ -34,5 +34,6 @@ static u64 iris_vpu2_calc_freq(struct iris_inst *inst, size_t data_size)
const struct vpu_ops iris_vpu2_ops = {
.power_off_hw = iris_vpu_power_off_hw,
+ .power_off_controller = iris_vpu_power_off_controller,
.calc_freq = iris_vpu2_calc_freq,
};
diff --git a/drivers/media/platform/qcom/iris/iris_vpu3.c b/drivers/media/platform/qcom/iris/iris_vpu3.c
index b484638e6105a69319232f667ee7ae95e3853698..13dab61427b8bd0491b69a9bc5f5144d27d17362 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3.c
@@ -118,5 +118,6 @@ static u64 iris_vpu3_calculate_frequency(struct iris_inst *inst, size_t data_siz
const struct vpu_ops iris_vpu3_ops = {
.power_off_hw = iris_vpu3_power_off_hardware,
+ .power_off_controller = iris_vpu_power_off_controller,
.calc_freq = iris_vpu3_calculate_frequency,
};
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index fe9896d66848cdcd8c67bd45bbf3b6ce4a01ab10..268e45acaa7c0e3fe237123c62f0133d9dface14 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -211,7 +211,7 @@ int iris_vpu_prepare_pc(struct iris_core *core)
return -EAGAIN;
}
-static int iris_vpu_power_off_controller(struct iris_core *core)
+int iris_vpu_power_off_controller(struct iris_core *core)
{
u32 val = 0;
int ret;
@@ -264,7 +264,7 @@ void iris_vpu_power_off(struct iris_core *core)
{
dev_pm_opp_set_rate(core->dev, 0);
core->iris_platform_data->vpu_ops->power_off_hw(core);
- iris_vpu_power_off_controller(core);
+ core->iris_platform_data->vpu_ops->power_off_controller(core);
iris_unset_icc_bw(core);
if (!iris_vpu_watchdog(core, core->intr_status))
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
index 63fa1fa5a4989e48aebdb6c7619c140000c0b44c..f8965661c602f990d5a7057565f79df4112d097e 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
@@ -13,6 +13,7 @@ extern const struct vpu_ops iris_vpu3_ops;
struct vpu_ops {
void (*power_off_hw)(struct iris_core *core);
+ int (*power_off_controller)(struct iris_core *core);
u64 (*calc_freq)(struct iris_inst *inst, size_t data_size);
};
@@ -22,6 +23,7 @@ void iris_vpu_clear_interrupt(struct iris_core *core);
int iris_vpu_watchdog(struct iris_core *core, u32 intr_status);
int iris_vpu_prepare_pc(struct iris_core *core);
int iris_vpu_power_on(struct iris_core *core);
+int iris_vpu_power_off_controller(struct iris_core *core);
void iris_vpu_power_off_hw(struct iris_core *core);
void iris_vpu_power_off(struct iris_core *core);
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH RFC v5 5/8] media: platform: qcom/iris: introduce optional controller_rst_tbl
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
` (3 preceding siblings ...)
2025-04-10 16:30 ` [PATCH RFC v5 4/8] media: platform: qcom/iris: add power_off_controller to vpu_ops Neil Armstrong
@ 2025-04-10 16:30 ` Neil Armstrong
2025-04-10 16:30 ` [PATCH RFC v5 6/8] media: platform: qcom/iris: rename iris_vpu3 to iris_vpu3x Neil Armstrong
` (4 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Neil Armstrong @ 2025-04-10 16:30 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Neil Armstrong, Bryan O'Donoghue
Introduce an optional controller_rst_tbl use to store reset lines
used to reset part of the controller.
This is necessary for the vpu3 support, when the xo reset line
must be asserted separately from the other reset line
on power off operation.
Factor the iris_init_resets() logic to allow requesting
multiple reset tables.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/media/platform/qcom/iris/iris_core.h | 1 +
.../platform/qcom/iris/iris_platform_common.h | 2 ++
drivers/media/platform/qcom/iris/iris_probe.c | 39 +++++++++++++++-------
3 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
index 37fb4919fecc62182784b4dca90fcab47dd38a80..78143855b277cd3ebdc7a1e7f35f6df284aa364c 100644
--- a/drivers/media/platform/qcom/iris/iris_core.h
+++ b/drivers/media/platform/qcom/iris/iris_core.h
@@ -82,6 +82,7 @@ struct iris_core {
struct clk_bulk_data *clock_tbl;
u32 clk_count;
struct reset_control_bulk_data *resets;
+ struct reset_control_bulk_data *controller_resets;
const struct iris_platform_data *iris_platform_data;
enum iris_core_state state;
dma_addr_t iface_q_table_daddr;
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index f6b15d2805fb2004699709bb12cd7ce9b052180c..fdd40fd80178c4c66b37e392d07a0a62f492f108 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -156,6 +156,8 @@ struct iris_platform_data {
unsigned int clk_tbl_size;
const char * const *clk_rst_tbl;
unsigned int clk_rst_tbl_size;
+ const char * const *controller_rst_tbl;
+ unsigned int controller_rst_tbl_size;
u64 dma_mask;
const char *fwname;
u32 pas_id;
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index aca442dcc153830e6252d1dca87afb38c0b9eb8f..4f8bce6e2002bffee4c93dcaaf6e52bf4e40992e 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -91,25 +91,40 @@ static int iris_init_clocks(struct iris_core *core)
return 0;
}
-static int iris_init_resets(struct iris_core *core)
+static int iris_init_reset_table(struct iris_core *core,
+ struct reset_control_bulk_data **resets,
+ const char * const *rst_tbl, u32 rst_tbl_size)
{
- const char * const *rst_tbl;
- u32 rst_tbl_size;
u32 i = 0;
- rst_tbl = core->iris_platform_data->clk_rst_tbl;
- rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
-
- core->resets = devm_kzalloc(core->dev,
- sizeof(*core->resets) * rst_tbl_size,
- GFP_KERNEL);
- if (!core->resets)
+ *resets = devm_kzalloc(core->dev,
+ sizeof(struct reset_control_bulk_data) * rst_tbl_size,
+ GFP_KERNEL);
+ if (!*resets)
return -ENOMEM;
for (i = 0; i < rst_tbl_size; i++)
- core->resets[i].id = rst_tbl[i];
+ (*resets)[i].id = rst_tbl[i];
+
+ return devm_reset_control_bulk_get_exclusive(core->dev, rst_tbl_size, *resets);
+}
+
+static int iris_init_resets(struct iris_core *core)
+{
+ int ret;
+
+ ret = iris_init_reset_table(core, &core->resets,
+ core->iris_platform_data->clk_rst_tbl,
+ core->iris_platform_data->clk_rst_tbl_size);
+ if (ret)
+ return ret;
+
+ if (!core->iris_platform_data->controller_rst_tbl_size)
+ return 0;
- return devm_reset_control_bulk_get_exclusive(core->dev, rst_tbl_size, core->resets);
+ return iris_init_reset_table(core, &core->controller_resets,
+ core->iris_platform_data->controller_rst_tbl,
+ core->iris_platform_data->controller_rst_tbl_size);
}
static int iris_init_resources(struct iris_core *core)
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH RFC v5 6/8] media: platform: qcom/iris: rename iris_vpu3 to iris_vpu3x
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
` (4 preceding siblings ...)
2025-04-10 16:30 ` [PATCH RFC v5 5/8] media: platform: qcom/iris: introduce optional controller_rst_tbl Neil Armstrong
@ 2025-04-10 16:30 ` Neil Armstrong
2025-04-10 16:30 ` [PATCH RFC v5 7/8] media: platform: qcom/iris: add support for vpu33 Neil Armstrong
` (3 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Neil Armstrong @ 2025-04-10 16:30 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Neil Armstrong, Bryan O'Donoghue
The vpu33 HW is very close to vpu3, and shares most of the
operations, so rename file to vpu3x since we'll handle all vpu3
variants in it.
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/media/platform/qcom/iris/Makefile | 2 +-
drivers/media/platform/qcom/iris/{iris_vpu3.c => iris_vpu3x.c} | 0
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
index 379359c5c7e1e4ca39b1216335cd8cf2317b6308..15ca63084ddc5c5ca34a79ff37064c5f7c5bfa7e 100644
--- a/drivers/media/platform/qcom/iris/Makefile
+++ b/drivers/media/platform/qcom/iris/Makefile
@@ -20,7 +20,7 @@ qcom-iris-objs += \
iris_vb2.o \
iris_vdec.o \
iris_vpu2.o \
- iris_vpu3.o \
+ iris_vpu3x.o \
iris_vpu_buffer.o \
iris_vpu_common.o \
diff --git a/drivers/media/platform/qcom/iris/iris_vpu3.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
similarity index 100%
rename from drivers/media/platform/qcom/iris/iris_vpu3.c
rename to drivers/media/platform/qcom/iris/iris_vpu3x.c
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH RFC v5 7/8] media: platform: qcom/iris: add support for vpu33
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
` (5 preceding siblings ...)
2025-04-10 16:30 ` [PATCH RFC v5 6/8] media: platform: qcom/iris: rename iris_vpu3 to iris_vpu3x Neil Armstrong
@ 2025-04-10 16:30 ` Neil Armstrong
2025-04-10 16:30 ` [PATCH RFC v5 8/8] media: platform: qcom/iris: add sm8650 support Neil Armstrong
` (2 subsequent siblings)
9 siblings, 0 replies; 31+ messages in thread
From: Neil Armstrong @ 2025-04-10 16:30 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Neil Armstrong, Bryan O'Donoghue
The IRIS acceleration found in the SM8650 platforms uses the vpu33
hardware version, and requires a slighly different reset and power off
sequences in order to properly get out of runtime suspend.
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # x1e Dell
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/media/platform/qcom/iris/iris_vpu3x.c | 160 ++++++++++++++++++++-
drivers/media/platform/qcom/iris/iris_vpu_common.h | 1 +
2 files changed, 157 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
index 13dab61427b8bd0491b69a9bc5f5144d27d17362..9b7c9a1495ee2f51c60b1142b2ed4680ff798f0a 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
@@ -4,20 +4,39 @@
*/
#include <linux/iopoll.h>
+#include <linux/reset.h>
#include "iris_instance.h"
#include "iris_vpu_common.h"
#include "iris_vpu_register_defines.h"
+#define WRAPPER_TZ_BASE_OFFS 0x000C0000
+#define AON_BASE_OFFS 0x000E0000
#define AON_MVP_NOC_RESET 0x0001F000
+#define WRAPPER_DEBUG_BRIDGE_LPI_CONTROL (WRAPPER_BASE_OFFS + 0x54)
+#define WRAPPER_DEBUG_BRIDGE_LPI_STATUS (WRAPPER_BASE_OFFS + 0x58)
+#define WRAPPER_IRIS_CPU_NOC_LPI_CONTROL (WRAPPER_BASE_OFFS + 0x5C)
+#define REQ_POWER_DOWN_PREP BIT(0)
+#define WRAPPER_IRIS_CPU_NOC_LPI_STATUS (WRAPPER_BASE_OFFS + 0x60)
#define WRAPPER_CORE_CLOCK_CONFIG (WRAPPER_BASE_OFFS + 0x88)
#define CORE_CLK_RUN 0x0
+#define WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG (WRAPPER_TZ_BASE_OFFS + 0x14)
+#define CTL_AXI_CLK_HALT BIT(0)
+#define CTL_CLK_HALT BIT(1)
+
+#define WRAPPER_TZ_QNS4PDXFIFO_RESET (WRAPPER_TZ_BASE_OFFS + 0x18)
+#define RESET_HIGH BIT(0)
+
#define CPU_CS_AHB_BRIDGE_SYNC_RESET (CPU_CS_BASE_OFFS + 0x160)
#define CORE_BRIDGE_SW_RESET BIT(0)
#define CORE_BRIDGE_HW_RESET_DISABLE BIT(1)
+#define CPU_CS_X2RPMH (CPU_CS_BASE_OFFS + 0x168)
+#define MSK_SIGNAL_FROM_TENSILICA BIT(0)
+#define MSK_CORE_POWER_ON BIT(1)
+
#define AON_WRAPPER_MVP_NOC_RESET_REQ (AON_MVP_NOC_RESET + 0x000)
#define VIDEO_NOC_RESET_REQ (BIT(0) | BIT(1))
@@ -25,7 +44,16 @@
#define VCODEC_SS_IDLE_STATUSN (VCODEC_BASE_OFFS + 0x70)
-static bool iris_vpu3_hw_power_collapsed(struct iris_core *core)
+#define AON_WRAPPER_MVP_NOC_LPI_CONTROL (AON_BASE_OFFS)
+#define AON_WRAPPER_MVP_NOC_LPI_STATUS (AON_BASE_OFFS + 0x4)
+
+#define AON_WRAPPER_MVP_NOC_CORE_SW_RESET (AON_BASE_OFFS + 0x18)
+#define SW_RESET BIT(0)
+#define AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL (AON_BASE_OFFS + 0x20)
+#define NOC_HALT BIT(0)
+#define AON_WRAPPER_SPARE (AON_BASE_OFFS + 0x28)
+
+static bool iris_vpu3x_hw_power_collapsed(struct iris_core *core)
{
u32 value, pwr_status;
@@ -40,7 +68,7 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
u32 reg_val = 0, value, i;
int ret;
- if (iris_vpu3_hw_power_collapsed(core))
+ if (iris_vpu3x_hw_power_collapsed(core))
goto disable_power;
dev_err(core->dev, "video hw is power on\n");
@@ -79,7 +107,125 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
iris_vpu_power_off_hw(core);
}
-static u64 iris_vpu3_calculate_frequency(struct iris_inst *inst, size_t data_size)
+static void iris_vpu33_power_off_hardware(struct iris_core *core)
+{
+ u32 reg_val = 0, value, i;
+ int ret;
+
+ if (iris_vpu3x_hw_power_collapsed(core))
+ goto disable_power;
+
+ dev_err(core->dev, "video hw is power on\n");
+
+ value = readl(core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
+ if (value)
+ writel(CORE_CLK_RUN, core->reg_base + WRAPPER_CORE_CLOCK_CONFIG);
+
+ for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
+ ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
+ reg_val, reg_val & 0x400000, 2000, 20000);
+ if (ret)
+ goto disable_power;
+ }
+
+ ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
+ reg_val, reg_val & BIT(0), 200, 2000);
+ if (ret)
+ goto disable_power;
+
+ /* set MNoC to low power, set PD_NOC_QREQ (bit 0) */
+ writel(BIT(0), core->reg_base + AON_WRAPPER_MVP_NOC_LPI_CONTROL);
+
+ writel(CORE_BRIDGE_SW_RESET | CORE_BRIDGE_HW_RESET_DISABLE,
+ core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
+ writel(CORE_BRIDGE_HW_RESET_DISABLE, core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
+ writel(0x0, core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
+
+disable_power:
+ iris_vpu_power_off_hw(core);
+}
+
+static int iris_vpu33_power_off_controller(struct iris_core *core)
+{
+ u32 xo_rst_tbl_size = core->iris_platform_data->controller_rst_tbl_size;
+ u32 clk_rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
+ u32 val = 0;
+ int ret;
+
+ writel(MSK_SIGNAL_FROM_TENSILICA | MSK_CORE_POWER_ON, core->reg_base + CPU_CS_X2RPMH);
+
+ writel(REQ_POWER_DOWN_PREP, core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_CONTROL);
+
+ ret = readl_poll_timeout(core->reg_base + WRAPPER_IRIS_CPU_NOC_LPI_STATUS,
+ val, val & BIT(0), 200, 2000);
+ if (ret)
+ goto disable_power;
+
+ writel(0x0, core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_CONTROL);
+
+ ret = readl_poll_timeout(core->reg_base + WRAPPER_DEBUG_BRIDGE_LPI_STATUS,
+ val, val == 0, 200, 2000);
+ if (ret)
+ goto disable_power;
+
+ writel(CTL_AXI_CLK_HALT | CTL_CLK_HALT,
+ core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG);
+ writel(RESET_HIGH, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET);
+ writel(0x0, core->reg_base + WRAPPER_TZ_QNS4PDXFIFO_RESET);
+ writel(0x0, core->reg_base + WRAPPER_TZ_CTL_AXI_CLOCK_CONFIG);
+
+ reset_control_bulk_reset(clk_rst_tbl_size, core->resets);
+
+ /* Disable MVP NoC clock */
+ val = readl(core->reg_base + AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL);
+ val |= NOC_HALT;
+ writel(val, core->reg_base + AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL);
+
+ /* enable MVP NoC reset */
+ val = readl(core->reg_base + AON_WRAPPER_MVP_NOC_CORE_SW_RESET);
+ val |= SW_RESET;
+ writel(val, core->reg_base + AON_WRAPPER_MVP_NOC_CORE_SW_RESET);
+
+ /* poll AON spare register bit0 to become zero with 50ms timeout */
+ ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_SPARE,
+ val, (val & BIT(0)) == 0, 1000, 50000);
+ if (ret)
+ goto disable_power;
+
+ /* enable bit(1) to avoid cvp noc xo reset */
+ val = readl(core->reg_base + AON_WRAPPER_SPARE);
+ val |= BIT(1);
+ writel(val, core->reg_base + AON_WRAPPER_SPARE);
+
+ reset_control_bulk_assert(xo_rst_tbl_size, core->controller_resets);
+
+ /* De-assert MVP NoC reset */
+ val = readl(core->reg_base + AON_WRAPPER_MVP_NOC_CORE_SW_RESET);
+ val &= ~SW_RESET;
+ writel(val, core->reg_base + AON_WRAPPER_MVP_NOC_CORE_SW_RESET);
+
+ usleep_range(80, 100);
+
+ reset_control_bulk_deassert(xo_rst_tbl_size, core->controller_resets);
+
+ /* reset AON spare register */
+ writel(0, core->reg_base + AON_WRAPPER_SPARE);
+
+ /* Enable MVP NoC clock */
+ val = readl(core->reg_base + AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL);
+ val &= ~NOC_HALT;
+ writel(val, core->reg_base + AON_WRAPPER_MVP_NOC_CORE_CLK_CONTROL);
+
+ iris_disable_unprepare_clock(core, IRIS_CTRL_CLK);
+
+disable_power:
+ iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_CTRL_POWER_DOMAIN]);
+ iris_disable_unprepare_clock(core, IRIS_AXI_CLK);
+
+ return 0;
+}
+
+static u64 iris_vpu3x_calculate_frequency(struct iris_inst *inst, size_t data_size)
{
struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
struct v4l2_format *inp_f = inst->fmt_src;
@@ -119,5 +265,11 @@ static u64 iris_vpu3_calculate_frequency(struct iris_inst *inst, size_t data_siz
const struct vpu_ops iris_vpu3_ops = {
.power_off_hw = iris_vpu3_power_off_hardware,
.power_off_controller = iris_vpu_power_off_controller,
- .calc_freq = iris_vpu3_calculate_frequency,
+ .calc_freq = iris_vpu3x_calculate_frequency,
+};
+
+const struct vpu_ops iris_vpu33_ops = {
+ .power_off_hw = iris_vpu33_power_off_hardware,
+ .power_off_controller = iris_vpu33_power_off_controller,
+ .calc_freq = iris_vpu3x_calculate_frequency,
};
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
index f8965661c602f990d5a7057565f79df4112d097e..93b7fa27be3bfa1cf6a3e83cc192cdb89d63575f 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
@@ -10,6 +10,7 @@ struct iris_core;
extern const struct vpu_ops iris_vpu2_ops;
extern const struct vpu_ops iris_vpu3_ops;
+extern const struct vpu_ops iris_vpu33_ops;
struct vpu_ops {
void (*power_off_hw)(struct iris_core *core);
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH RFC v5 8/8] media: platform: qcom/iris: add sm8650 support
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
` (6 preceding siblings ...)
2025-04-10 16:30 ` [PATCH RFC v5 7/8] media: platform: qcom/iris: add support for vpu33 Neil Armstrong
@ 2025-04-10 16:30 ` Neil Armstrong
2025-04-10 19:20 ` Bryan O'Donoghue
2025-04-11 11:55 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Bryan O'Donoghue
2025-04-14 7:35 ` Neil Armstrong
9 siblings, 1 reply; 31+ messages in thread
From: Neil Armstrong @ 2025-04-10 16:30 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Neil Armstrong, Bryan O'Donoghue
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);
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 8/8] media: platform: qcom/iris: add sm8650 support
2025-04-10 16:30 ` [PATCH RFC v5 8/8] media: platform: qcom/iris: add sm8650 support Neil Armstrong
@ 2025-04-10 19:20 ` Bryan O'Donoghue
2025-04-11 8:11 ` Neil Armstrong
0 siblings, 1 reply; 31+ messages in thread
From: Bryan O'Donoghue @ 2025-04-10 19:20 UTC (permalink / raw)
To: Neil Armstrong, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree
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
Would be grateful if you could add that patch to a V6.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 8/8] media: platform: qcom/iris: add sm8650 support
2025-04-10 19:20 ` Bryan O'Donoghue
@ 2025-04-11 8:11 ` Neil Armstrong
2025-04-11 11:29 ` Bryan O'Donoghue
0 siblings, 1 reply; 31+ messages in thread
From: Neil Armstrong @ 2025-04-11 8:11 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal,
Abhinav Kumar, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree
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>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 8/8] media: platform: qcom/iris: add sm8650 support
2025-04-11 8:11 ` Neil Armstrong
@ 2025-04-11 11:29 ` Bryan O'Donoghue
0 siblings, 0 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2025-04-11 11:29 UTC (permalink / raw)
To: neil.armstrong, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
` (7 preceding siblings ...)
2025-04-10 16:30 ` [PATCH RFC v5 8/8] media: platform: qcom/iris: add sm8650 support Neil Armstrong
@ 2025-04-11 11:55 ` Bryan O'Donoghue
2025-04-11 15:35 ` Bryan O'Donoghue
2025-04-14 7:35 ` Neil Armstrong
9 siblings, 1 reply; 31+ messages in thread
From: Bryan O'Donoghue @ 2025-04-11 11:55 UTC (permalink / raw)
To: Neil Armstrong, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
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>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-11 11:55 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Bryan O'Donoghue
@ 2025-04-11 15:35 ` Bryan O'Donoghue
0 siblings, 0 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2025-04-11 15:35 UTC (permalink / raw)
To: Bryan O'Donoghue, Neil Armstrong, Vikash Garodia,
Dikshita Agarwal, Abhinav Kumar, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-10 16:29 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Neil Armstrong
` (8 preceding siblings ...)
2025-04-11 11:55 ` [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650 Bryan O'Donoghue
@ 2025-04-14 7:35 ` Neil Armstrong
2025-04-14 10:54 ` Vikash Garodia
9 siblings, 1 reply; 31+ messages in thread
From: Neil Armstrong @ 2025-04-14 7:35 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
Hi Vikash, Dikshita,
On 10/04/2025 18: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.
Any feedback on this patchset ?
Thanks,
Neil
>
> 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,
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-14 7:35 ` Neil Armstrong
@ 2025-04-14 10:54 ` Vikash Garodia
2025-04-14 12:09 ` neil.armstrong
2025-04-14 12:10 ` Dmitry Baryshkov
0 siblings, 2 replies; 31+ messages in thread
From: Vikash Garodia @ 2025-04-14 10:54 UTC (permalink / raw)
To: neil.armstrong, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
Hi Neil,
On 4/14/2025 1:05 PM, Neil Armstrong wrote:
> Hi Vikash, Dikshita,
>
> On 10/04/2025 18: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.
>
> Any feedback on this patchset ?
Myself and Dikshita went through the approach you are bringing here, let me
update some context here:
- sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the scaled
down variant i.e have 2 PIPE vs others having 4. Similarly there are other
irisv3 having 1 pipe as well.
- With above variations, firmware and instance caps would change for the variant
SOCs.
- Above these, few(less) bindings/connections specific delta would be there,
like there is reset delta in sm8550 and sm8650.
Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific tables and
SOC platform data, i.e sm8650_data (for sm8650). On top of this, individual SOC
specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
preset register table, etc and export these delta structs in xxx_gen1.c or
xxx_gen2.c.
Going with above approach, sm8650.c would have only one reset table for now.
Later if any delta is identified, the same can be added in it. All other common
structs, can reside in xxx_gen2.c for now.
Regards,
Vikash
>
> Thanks,
> Neil
>
>>
>> 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,
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-14 10:54 ` Vikash Garodia
@ 2025-04-14 12:09 ` neil.armstrong
2025-04-14 19:48 ` Vikash Garodia
2025-04-14 12:10 ` Dmitry Baryshkov
1 sibling, 1 reply; 31+ messages in thread
From: neil.armstrong @ 2025-04-14 12:09 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
Hi,
On 14/04/2025 12:54, Vikash Garodia wrote:
> Hi Neil,
>
> On 4/14/2025 1:05 PM, Neil Armstrong wrote:
>> Hi Vikash, Dikshita,
>>
>> On 10/04/2025 18: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.
>>
>> Any feedback on this patchset ?
> Myself and Dikshita went through the approach you are bringing here, let me
> update some context here:
> - sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the scaled
> down variant i.e have 2 PIPE vs others having 4. Similarly there are other
> irisv3 having 1 pipe as well.
> - With above variations, firmware and instance caps would change for the variant
> SOCs.
> - Above these, few(less) bindings/connections specific delta would be there,
> like there is reset delta in sm8550 and sm8650.
>
> Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific tables and
> SOC platform data, i.e sm8650_data (for sm8650). On top of this, individual SOC
> specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
> preset register table, etc and export these delta structs in xxx_gen1.c or
> xxx_gen2.c.
>
> Going with above approach, sm8650.c would have only one reset table for now.
> Later if any delta is identified, the same can be added in it. All other common
> structs, can reside in xxx_gen2.c for now.
Thanks for reviewing, but...
Sorry I don't understand what you and Dmitry are asking me...
If I try really hard, you would like to have:
iris_catalog_sm8550.c
- iris_set_sm8550_preset_registers
- sm8550_icc_table
- sm8550_clk_reset_table
- sm8550_bw_table_dec
- sm8550_pmdomain_table
- sm8550_opp_pd_table
- sm8550_clk_table
iris_catalog_sm8650.c
- sm8650_clk_reset_table
- sm8650_controller_reset_table
iris_catalog_gen2.c
- iris_hfi_gen2_command_ops_init
- iris_hfi_gen2_response_ops_init
...
- sm8550_dec_op_int_buf_tbl
and:
- struct iris_platform_data sm8550_data
- struct iris_platform_data sm8650_data
using data from iris_catalog_sm8550.c & iris_catalog_sm8550.c
So this is basically what I _already_ propose except
you move data in separate .c files for no reasons,
please explain why you absolutely want distinct .c
files per SoC. We are no more in the 1990's and we camn
defintely have big .c files.
And we still have a big issue, how to get the:
- ARRAY_SIZE(sm8550_clk_reset_table)
- ARRAY_SIZE(sm8550_bw_table_dec)
- ARRAY_SIZE(sm8550_pmdomain_table)
...
since they are declared in a separate .c file and you
need a compile-time const value to fill all the _size
attribute in iris_platform_data.
So I recall my goal, I just want to add sm8650 support,
and I'm not the owner of this driver, and I'm really happy
to help, but giving me random ideas to solve your problem
doesn't help us at all going forward.
Neil
>
> Regards,
> Vikash
>>
>> Thanks,
>> Neil
>>
>>>
>>> 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,
>>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-14 12:09 ` neil.armstrong
@ 2025-04-14 19:48 ` Vikash Garodia
2025-04-15 8:24 ` neil.armstrong
0 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2025-04-14 19:48 UTC (permalink / raw)
To: neil.armstrong, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
On 4/14/2025 5:39 PM, neil.armstrong@linaro.org wrote:
> Hi,
>
> On 14/04/2025 12:54, Vikash Garodia wrote:
>> Hi Neil,
>>
>> On 4/14/2025 1:05 PM, Neil Armstrong wrote:
>>> Hi Vikash, Dikshita,
>>>
>>> On 10/04/2025 18: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.
>>>
>>> Any feedback on this patchset ?
>> Myself and Dikshita went through the approach you are bringing here, let me
>> update some context here:
>> - sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the scaled
>> down variant i.e have 2 PIPE vs others having 4. Similarly there are other
>> irisv3 having 1 pipe as well.
>> - With above variations, firmware and instance caps would change for the variant
>> SOCs.
>> - Above these, few(less) bindings/connections specific delta would be there,
>> like there is reset delta in sm8550 and sm8650.
>>
>> Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific tables and
>> SOC platform data, i.e sm8650_data (for sm8650). On top of this, individual SOC
>> specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
>> preset register table, etc and export these delta structs in xxx_gen1.c or
>> xxx_gen2.c.
>>
>> Going with above approach, sm8650.c would have only one reset table for now.
>> Later if any delta is identified, the same can be added in it. All other common
>> structs, can reside in xxx_gen2.c for now.
>
> Thanks for reviewing, but...
> Sorry I don't understand what you and Dmitry are asking me...
>
> If I try really hard, you would like to have:
>
> iris_catalog_sm8550.c
> - iris_set_sm8550_preset_registers
> - sm8550_icc_table
> - sm8550_clk_reset_table
> - sm8550_bw_table_dec
> - sm8550_pmdomain_table
> - sm8550_opp_pd_table
> - sm8550_clk_table
Move or rename existing 8550.c as xxx_gen2.c. This is with the existing
assumption that everything under 8550.c is common for all gen2 to come in future.
>
> iris_catalog_sm8650.c
> - sm8650_clk_reset_table
> - sm8650_controller_reset_table
yes, since reset is the only delta.
>
> iris_catalog_gen2.c
> - iris_hfi_gen2_command_ops_init
> - iris_hfi_gen2_response_ops_init
> ...
> - sm8550_dec_op_int_buf_tbl
>
> and:
> - struct iris_platform_data sm8550_data
> - struct iris_platform_data sm8650_data
all this goes to xxx_gen2.c as well.
> using data from iris_catalog_sm8550.c & iris_catalog_sm8550.c
>
> So this is basically what I _already_ propose except
> you move data in separate .c files for no reasons,
> please explain why you absolutely want distinct .c
> files per SoC. We are no more in the 1990's and we camn
> defintely have big .c files.
Its not about the size of file alone, it is easy to understand later what would
be the delta in the SOCs and what would common. For ex, just navigating through
sm8650.c, anyone can comment that reset is the delta.
>
> And we still have a big issue, how to get the:
> - ARRAY_SIZE(sm8550_clk_reset_table)
> - ARRAY_SIZE(sm8550_bw_table_dec)
> - ARRAY_SIZE(sm8550_pmdomain_table)
> ...
>
> since they are declared in a separate .c file and you
> need a compile-time const value to fill all the _size
> attribute in iris_platform_data.
I have not tries this, but isn't extern-ing the soc structs (in your case reset
tables) into xxx_gen2.c enough here ? Also i think the tables you are pointing
here, lies in the xxx_gen2.c only, so i am sure above ones would not be an issue
at all. The only delta struct is reset table, lets see if extern helps.
Regards,
Vikash
>
> So I recall my goal, I just want to add sm8650 support,
> and I'm not the owner of this driver, and I'm really happy
> to help, but giving me random ideas to solve your problem
> doesn't help us at all going forward.
>
> Neil
>
>>
>> Regards,
>> Vikash
>>>
>>> Thanks,
>>> Neil
>>>
>>>>
>>>> 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,
>>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-14 19:48 ` Vikash Garodia
@ 2025-04-15 8:24 ` neil.armstrong
2025-04-15 11:26 ` Vikash Garodia
0 siblings, 1 reply; 31+ messages in thread
From: neil.armstrong @ 2025-04-15 8:24 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
Hi,
On 14/04/2025 21:48, Vikash Garodia wrote:
>
> On 4/14/2025 5:39 PM, neil.armstrong@linaro.org wrote:
>> Hi,
>>
>> On 14/04/2025 12:54, Vikash Garodia wrote:
>>> Hi Neil,
>>>
>>> On 4/14/2025 1:05 PM, Neil Armstrong wrote:
>>>> Hi Vikash, Dikshita,
>>>>
>>>> On 10/04/2025 18: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.
>>>>
>>>> Any feedback on this patchset ?
>>> Myself and Dikshita went through the approach you are bringing here, let me
>>> update some context here:
>>> - sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the scaled
>>> down variant i.e have 2 PIPE vs others having 4. Similarly there are other
>>> irisv3 having 1 pipe as well.
>>> - With above variations, firmware and instance caps would change for the variant
>>> SOCs.
>>> - Above these, few(less) bindings/connections specific delta would be there,
>>> like there is reset delta in sm8550 and sm8650.
>>>
>>> Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific tables and
>>> SOC platform data, i.e sm8650_data (for sm8650). On top of this, individual SOC
>>> specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
>>> preset register table, etc and export these delta structs in xxx_gen1.c or
>>> xxx_gen2.c.
>>>
>>> Going with above approach, sm8650.c would have only one reset table for now.
>>> Later if any delta is identified, the same can be added in it. All other common
>>> structs, can reside in xxx_gen2.c for now.
>>
>> Thanks for reviewing, but...
>> Sorry I don't understand what you and Dmitry are asking me...
>>
>> If I try really hard, you would like to have:
>>
>> iris_catalog_sm8550.c
>> - iris_set_sm8550_preset_registers
>> - sm8550_icc_table
>> - sm8550_clk_reset_table
>> - sm8550_bw_table_dec
>> - sm8550_pmdomain_table
>> - sm8550_opp_pd_table
>> - sm8550_clk_table
> Move or rename existing 8550.c as xxx_gen2.c. This is with the existing
> assumption that everything under 8550.c is common for all gen2 to come in future.
>>
>> iris_catalog_sm8650.c
>> - sm8650_clk_reset_table
>> - sm8650_controller_reset_table
> yes, since reset is the only delta.
>>
>> iris_catalog_gen2.c
>> - iris_hfi_gen2_command_ops_init
>> - iris_hfi_gen2_response_ops_init
>> ...
>> - sm8550_dec_op_int_buf_tbl
>>
>> and:
>> - struct iris_platform_data sm8550_data
>> - struct iris_platform_data sm8650_data
> all this goes to xxx_gen2.c as well.
Yeah so this is exactly my current approach, except it use .h files
for each SoC for simplicity.
>
>> using data from iris_catalog_sm8550.c & iris_catalog_sm8550.c
>>
>> So this is basically what I _already_ propose except
>> you move data in separate .c files for no reasons,
>> please explain why you absolutely want distinct .c
>> files per SoC. We are no more in the 1990's and we camn
>> defintely have big .c files.
> Its not about the size of file alone, it is easy to understand later what would
> be the delta in the SOCs and what would common. For ex, just navigating through
> sm8650.c, anyone can comment that reset is the delta.
What's the problem with the current approach with .h file for each SoC ?
>>
>> And we still have a big issue, how to get the:
>> - ARRAY_SIZE(sm8550_clk_reset_table)
>> - ARRAY_SIZE(sm8550_bw_table_dec)
>> - ARRAY_SIZE(sm8550_pmdomain_table)
>> ...
>>
>> since they are declared in a separate .c file and you
>> need a compile-time const value to fill all the _size
>> attribute in iris_platform_data.
> I have not tries this, but isn't extern-ing the soc structs (in your case reset
> tables) into xxx_gen2.c enough here ? Also i think the tables you are pointing
> here, lies in the xxx_gen2.c only, so i am sure above ones would not be an issue
> at all. The only delta struct is reset table, lets see if extern helps.
No it doesn't, because I wrote C for the last 25 years, and I tried it already,
I also tried to export a const int with the table size, and it also doesn't work.
The 3 only ways are:
1) add defines with sizes for each table
2) add a NULL entry at the end of each table, and update all code using those tables
3) declare in the same scope, which is my current proposal
Neil
>
> Regards,
> Vikash
>>
>> So I recall my goal, I just want to add sm8650 support,
>> and I'm not the owner of this driver, and I'm really happy
>> to help, but giving me random ideas to solve your problem
>> doesn't help us at all going forward.
>>
>> Neil
>>
>>>
>>> Regards,
>>> Vikash
>>>>
>>>> Thanks,
>>>> Neil
>>>>
>>>>>
>>>>> 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,
>>>>
>>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-15 8:24 ` neil.armstrong
@ 2025-04-15 11:26 ` Vikash Garodia
2025-04-15 11:55 ` neil.armstrong
0 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2025-04-15 11:26 UTC (permalink / raw)
To: neil.armstrong, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
On 4/15/2025 1:54 PM, neil.armstrong@linaro.org wrote:
> Hi,
>
> On 14/04/2025 21:48, Vikash Garodia wrote:
>>
>> On 4/14/2025 5:39 PM, neil.armstrong@linaro.org wrote:
>>> Hi,
>>>
>>> On 14/04/2025 12:54, Vikash Garodia wrote:
>>>> Hi Neil,
>>>>
>>>> On 4/14/2025 1:05 PM, Neil Armstrong wrote:
>>>>> Hi Vikash, Dikshita,
>>>>>
>>>>> On 10/04/2025 18: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.
>>>>>
>>>>> Any feedback on this patchset ?
>>>> Myself and Dikshita went through the approach you are bringing here, let me
>>>> update some context here:
>>>> - sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the scaled
>>>> down variant i.e have 2 PIPE vs others having 4. Similarly there are other
>>>> irisv3 having 1 pipe as well.
>>>> - With above variations, firmware and instance caps would change for the
>>>> variant
>>>> SOCs.
>>>> - Above these, few(less) bindings/connections specific delta would be there,
>>>> like there is reset delta in sm8550 and sm8650.
>>>>
>>>> Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific tables and
>>>> SOC platform data, i.e sm8650_data (for sm8650). On top of this, individual SOC
>>>> specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
>>>> preset register table, etc and export these delta structs in xxx_gen1.c or
>>>> xxx_gen2.c.
>>>>
>>>> Going with above approach, sm8650.c would have only one reset table for now.
>>>> Later if any delta is identified, the same can be added in it. All other common
>>>> structs, can reside in xxx_gen2.c for now.
>>>
>>> Thanks for reviewing, but...
>>> Sorry I don't understand what you and Dmitry are asking me...
>>>
>>> If I try really hard, you would like to have:
>>>
>>> iris_catalog_sm8550.c
>>> - iris_set_sm8550_preset_registers
>>> - sm8550_icc_table
>>> - sm8550_clk_reset_table
>>> - sm8550_bw_table_dec
>>> - sm8550_pmdomain_table
>>> - sm8550_opp_pd_table
>>> - sm8550_clk_table
>> Move or rename existing 8550.c as xxx_gen2.c. This is with the existing
>> assumption that everything under 8550.c is common for all gen2 to come in future.
>>>
>>> iris_catalog_sm8650.c
>>> - sm8650_clk_reset_table
>>> - sm8650_controller_reset_table
>> yes, since reset is the only delta.
>>>
>>> iris_catalog_gen2.c
>>> - iris_hfi_gen2_command_ops_init
>>> - iris_hfi_gen2_response_ops_init
>>> ...
>>> - sm8550_dec_op_int_buf_tbl
>>>
>>> and:
>>> - struct iris_platform_data sm8550_data
>>> - struct iris_platform_data sm8650_data
>> all this goes to xxx_gen2.c as well.
>
> Yeah so this is exactly my current approach, except it use .h files
> for each SoC for simplicity.
>
>>
>>> using data from iris_catalog_sm8550.c & iris_catalog_sm8550.c
>>>
>>> So this is basically what I _already_ propose except
>>> you move data in separate .c files for no reasons,
>>> please explain why you absolutely want distinct .c
>>> files per SoC. We are no more in the 1990's and we camn
>>> defintely have big .c files.
>> Its not about the size of file alone, it is easy to understand later what would
>> be the delta in the SOCs and what would common. For ex, just navigating through
>> sm8650.c, anyone can comment that reset is the delta.
>
> What's the problem with the current approach with .h file for each SoC ?
>
>>>
>>> And we still have a big issue, how to get the:
>>> - ARRAY_SIZE(sm8550_clk_reset_table)
>>> - ARRAY_SIZE(sm8550_bw_table_dec)
>>> - ARRAY_SIZE(sm8550_pmdomain_table)
>>> ...
>>>
>>> since they are declared in a separate .c file and you
>>> need a compile-time const value to fill all the _size
>>> attribute in iris_platform_data.
>> I have not tries this, but isn't extern-ing the soc structs (in your case reset
>> tables) into xxx_gen2.c enough here ? Also i think the tables you are pointing
>> here, lies in the xxx_gen2.c only, so i am sure above ones would not be an issue
>> at all. The only delta struct is reset table, lets see if extern helps.
>
> No it doesn't, because I wrote C for the last 25 years, and I tried it already,
> I also tried to export a const int with the table size, and it also doesn't work.
Got it, i tried too, it didn't work.
>
> The 3 only ways are:
> 1) add defines with sizes for each table
This leaves manual update everytime.
> 2) add a NULL entry at the end of each table, and update all code using those
> tables
Does not sound right to update the code, just to get the size.
> 3) declare in the same scope, which is my current proposalThe proposal in the RFC is about moving the common structs to 8550.h, rather, it
can be kept in xxx_gen2.c.
8550.h can have the delta part (i.e reset tables) and can be included in
xxx_gen2.c. sm8650_data can reside in xxx_gen2.c, soc headers just brings the
delta structs which can be overridden from common in xxx_gen2.c
I am good with the header approach which contains the delta over and above
xxx_gen2.c.
Regards,
Vikash
> Neil
>
>>
>> Regards,
>> Vikash
>>>
>>> So I recall my goal, I just want to add sm8650 support,
>>> and I'm not the owner of this driver, and I'm really happy
>>> to help, but giving me random ideas to solve your problem
>>> doesn't help us at all going forward.
>>>
>>> Neil
>>>
>>>>
>>>> Regards,
>>>> Vikash
>>>>>
>>>>> Thanks,
>>>>> Neil
>>>>>
>>>>>>
>>>>>> 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,
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-15 11:26 ` Vikash Garodia
@ 2025-04-15 11:55 ` neil.armstrong
2025-04-15 12:02 ` Vikash Garodia
0 siblings, 1 reply; 31+ messages in thread
From: neil.armstrong @ 2025-04-15 11:55 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
Hi,
On 15/04/2025 13:26, Vikash Garodia wrote:
>
> On 4/15/2025 1:54 PM, neil.armstrong@linaro.org wrote:
>> Hi,
>>
>> On 14/04/2025 21:48, Vikash Garodia wrote:
>>>
>>> On 4/14/2025 5:39 PM, neil.armstrong@linaro.org wrote:
>>>> Hi,
>>>>
>>>> On 14/04/2025 12:54, Vikash Garodia wrote:
>>>>> Hi Neil,
>>>>>
>>>>> On 4/14/2025 1:05 PM, Neil Armstrong wrote:
>>>>>> Hi Vikash, Dikshita,
>>>>>>
>>>>>> On 10/04/2025 18: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.
>>>>>>
>>>>>> Any feedback on this patchset ?
>>>>> Myself and Dikshita went through the approach you are bringing here, let me
>>>>> update some context here:
>>>>> - sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the scaled
>>>>> down variant i.e have 2 PIPE vs others having 4. Similarly there are other
>>>>> irisv3 having 1 pipe as well.
>>>>> - With above variations, firmware and instance caps would change for the
>>>>> variant
>>>>> SOCs.
>>>>> - Above these, few(less) bindings/connections specific delta would be there,
>>>>> like there is reset delta in sm8550 and sm8650.
>>>>>
>>>>> Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific tables and
>>>>> SOC platform data, i.e sm8650_data (for sm8650). On top of this, individual SOC
>>>>> specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
>>>>> preset register table, etc and export these delta structs in xxx_gen1.c or
>>>>> xxx_gen2.c.
>>>>>
>>>>> Going with above approach, sm8650.c would have only one reset table for now.
>>>>> Later if any delta is identified, the same can be added in it. All other common
>>>>> structs, can reside in xxx_gen2.c for now.
>>>>
>>>> Thanks for reviewing, but...
>>>> Sorry I don't understand what you and Dmitry are asking me...
>>>>
>>>> If I try really hard, you would like to have:
>>>>
>>>> iris_catalog_sm8550.c
>>>> - iris_set_sm8550_preset_registers
>>>> - sm8550_icc_table
>>>> - sm8550_clk_reset_table
>>>> - sm8550_bw_table_dec
>>>> - sm8550_pmdomain_table
>>>> - sm8550_opp_pd_table
>>>> - sm8550_clk_table
>>> Move or rename existing 8550.c as xxx_gen2.c. This is with the existing
>>> assumption that everything under 8550.c is common for all gen2 to come in future.
>>>>
>>>> iris_catalog_sm8650.c
>>>> - sm8650_clk_reset_table
>>>> - sm8650_controller_reset_table
>>> yes, since reset is the only delta.
>>>>
>>>> iris_catalog_gen2.c
>>>> - iris_hfi_gen2_command_ops_init
>>>> - iris_hfi_gen2_response_ops_init
>>>> ...
>>>> - sm8550_dec_op_int_buf_tbl
>>>>
>>>> and:
>>>> - struct iris_platform_data sm8550_data
>>>> - struct iris_platform_data sm8650_data
>>> all this goes to xxx_gen2.c as well.
>>
>> Yeah so this is exactly my current approach, except it use .h files
>> for each SoC for simplicity.
>>
>>>
>>>> using data from iris_catalog_sm8550.c & iris_catalog_sm8550.c
>>>>
>>>> So this is basically what I _already_ propose except
>>>> you move data in separate .c files for no reasons,
>>>> please explain why you absolutely want distinct .c
>>>> files per SoC. We are no more in the 1990's and we camn
>>>> defintely have big .c files.
>>> Its not about the size of file alone, it is easy to understand later what would
>>> be the delta in the SOCs and what would common. For ex, just navigating through
>>> sm8650.c, anyone can comment that reset is the delta.
>>
>> What's the problem with the current approach with .h file for each SoC ?
>>
>>>>
>>>> And we still have a big issue, how to get the:
>>>> - ARRAY_SIZE(sm8550_clk_reset_table)
>>>> - ARRAY_SIZE(sm8550_bw_table_dec)
>>>> - ARRAY_SIZE(sm8550_pmdomain_table)
>>>> ...
>>>>
>>>> since they are declared in a separate .c file and you
>>>> need a compile-time const value to fill all the _size
>>>> attribute in iris_platform_data.
>>> I have not tries this, but isn't extern-ing the soc structs (in your case reset
>>> tables) into xxx_gen2.c enough here ? Also i think the tables you are pointing
>>> here, lies in the xxx_gen2.c only, so i am sure above ones would not be an issue
>>> at all. The only delta struct is reset table, lets see if extern helps.
>>
>> No it doesn't, because I wrote C for the last 25 years, and I tried it already,
>> I also tried to export a const int with the table size, and it also doesn't work.
> Got it, i tried too, it didn't work.
>>
>> The 3 only ways are:
>> 1) add defines with sizes for each table
> This leaves manual update everytime.
>
>> 2) add a NULL entry at the end of each table, and update all code using those
>> tables
> Does not sound right to update the code, just to get the size.
>
>> 3) declare in the same scope, which is my current proposalThe proposal in the RFC is about moving the common structs to 8550.h, rather, it
> can be kept in xxx_gen2.c.
> 8550.h can have the delta part (i.e reset tables) and can be included in
> xxx_gen2.c. sm8650_data can reside in xxx_gen2.c, soc headers just brings the
> delta structs which can be overridden from common in xxx_gen2.c
> I am good with the header approach which contains the delta over and above
> xxx_gen2.c.
I'll try to do that, but now I don't see the point of the SoC header files if
they only contain the reset tables.
Neil
>
> Regards,
> Vikash
>> Neil
>>
>>>
>>> Regards,
>>> Vikash
>>>>
>>>> So I recall my goal, I just want to add sm8650 support,
>>>> and I'm not the owner of this driver, and I'm really happy
>>>> to help, but giving me random ideas to solve your problem
>>>> doesn't help us at all going forward.
>>>>
>>>> Neil
>>>>
>>>>>
>>>>> Regards,
>>>>> Vikash
>>>>>>
>>>>>> Thanks,
>>>>>> Neil
>>>>>>
>>>>>>>
>>>>>>> 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,
>>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-15 11:55 ` neil.armstrong
@ 2025-04-15 12:02 ` Vikash Garodia
0 siblings, 0 replies; 31+ messages in thread
From: Vikash Garodia @ 2025-04-15 12:02 UTC (permalink / raw)
To: neil.armstrong, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel
Cc: linux-kernel, linux-media, linux-arm-msm, devicetree,
Bryan O'Donoghue
On 4/15/2025 5:25 PM, neil.armstrong@linaro.org wrote:
> Hi,
>
> On 15/04/2025 13:26, Vikash Garodia wrote:
>>
>> On 4/15/2025 1:54 PM, neil.armstrong@linaro.org wrote:
>>> Hi,
>>>
>>> On 14/04/2025 21:48, Vikash Garodia wrote:
>>>>
>>>> On 4/14/2025 5:39 PM, neil.armstrong@linaro.org wrote:
>>>>> Hi,
>>>>>
>>>>> On 14/04/2025 12:54, Vikash Garodia wrote:
>>>>>> Hi Neil,
>>>>>>
>>>>>> On 4/14/2025 1:05 PM, Neil Armstrong wrote:
>>>>>>> Hi Vikash, Dikshita,
>>>>>>>
>>>>>>> On 10/04/2025 18: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.
>>>>>>>
>>>>>>> Any feedback on this patchset ?
>>>>>> Myself and Dikshita went through the approach you are bringing here, let me
>>>>>> update some context here:
>>>>>> - sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the
>>>>>> scaled
>>>>>> down variant i.e have 2 PIPE vs others having 4. Similarly there are other
>>>>>> irisv3 having 1 pipe as well.
>>>>>> - With above variations, firmware and instance caps would change for the
>>>>>> variant
>>>>>> SOCs.
>>>>>> - Above these, few(less) bindings/connections specific delta would be there,
>>>>>> like there is reset delta in sm8550 and sm8650.
>>>>>>
>>>>>> Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific
>>>>>> tables and
>>>>>> SOC platform data, i.e sm8650_data (for sm8650). On top of this,
>>>>>> individual SOC
>>>>>> specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
>>>>>> preset register table, etc and export these delta structs in xxx_gen1.c or
>>>>>> xxx_gen2.c.
>>>>>>
>>>>>> Going with above approach, sm8650.c would have only one reset table for now.
>>>>>> Later if any delta is identified, the same can be added in it. All other
>>>>>> common
>>>>>> structs, can reside in xxx_gen2.c for now.
>>>>>
>>>>> Thanks for reviewing, but...
>>>>> Sorry I don't understand what you and Dmitry are asking me...
>>>>>
>>>>> If I try really hard, you would like to have:
>>>>>
>>>>> iris_catalog_sm8550.c
>>>>> - iris_set_sm8550_preset_registers
>>>>> - sm8550_icc_table
>>>>> - sm8550_clk_reset_table
>>>>> - sm8550_bw_table_dec
>>>>> - sm8550_pmdomain_table
>>>>> - sm8550_opp_pd_table
>>>>> - sm8550_clk_table
>>>> Move or rename existing 8550.c as xxx_gen2.c. This is with the existing
>>>> assumption that everything under 8550.c is common for all gen2 to come in
>>>> future.
>>>>>
>>>>> iris_catalog_sm8650.c
>>>>> - sm8650_clk_reset_table
>>>>> - sm8650_controller_reset_table
>>>> yes, since reset is the only delta.
>>>>>
>>>>> iris_catalog_gen2.c
>>>>> - iris_hfi_gen2_command_ops_init
>>>>> - iris_hfi_gen2_response_ops_init
>>>>> ...
>>>>> - sm8550_dec_op_int_buf_tbl
>>>>>
>>>>> and:
>>>>> - struct iris_platform_data sm8550_data
>>>>> - struct iris_platform_data sm8650_data
>>>> all this goes to xxx_gen2.c as well.
>>>
>>> Yeah so this is exactly my current approach, except it use .h files
>>> for each SoC for simplicity.
>>>
>>>>
>>>>> using data from iris_catalog_sm8550.c & iris_catalog_sm8550.c
>>>>>
>>>>> So this is basically what I _already_ propose except
>>>>> you move data in separate .c files for no reasons,
>>>>> please explain why you absolutely want distinct .c
>>>>> files per SoC. We are no more in the 1990's and we camn
>>>>> defintely have big .c files.
>>>> Its not about the size of file alone, it is easy to understand later what would
>>>> be the delta in the SOCs and what would common. For ex, just navigating through
>>>> sm8650.c, anyone can comment that reset is the delta.
>>>
>>> What's the problem with the current approach with .h file for each SoC ?
>>>
>>>>>
>>>>> And we still have a big issue, how to get the:
>>>>> - ARRAY_SIZE(sm8550_clk_reset_table)
>>>>> - ARRAY_SIZE(sm8550_bw_table_dec)
>>>>> - ARRAY_SIZE(sm8550_pmdomain_table)
>>>>> ...
>>>>>
>>>>> since they are declared in a separate .c file and you
>>>>> need a compile-time const value to fill all the _size
>>>>> attribute in iris_platform_data.
>>>> I have not tries this, but isn't extern-ing the soc structs (in your case reset
>>>> tables) into xxx_gen2.c enough here ? Also i think the tables you are pointing
>>>> here, lies in the xxx_gen2.c only, so i am sure above ones would not be an
>>>> issue
>>>> at all. The only delta struct is reset table, lets see if extern helps.
>>>
>>> No it doesn't, because I wrote C for the last 25 years, and I tried it already,
>>> I also tried to export a const int with the table size, and it also doesn't
>>> work.
>> Got it, i tried too, it didn't work.
>>>
>>> The 3 only ways are:
>>> 1) add defines with sizes for each table
>> This leaves manual update everytime.
>>
>>> 2) add a NULL entry at the end of each table, and update all code using those
>>> tables
>> Does not sound right to update the code, just to get the size.
>>
>>> 3) declare in the same scope, which is my current proposalThe proposal in the
>>> RFC is about moving the common structs to 8550.h, rather, it
>> can be kept in xxx_gen2.c.
>> 8550.h can have the delta part (i.e reset tables) and can be included in
>> xxx_gen2.c. sm8650_data can reside in xxx_gen2.c, soc headers just brings the
>> delta structs which can be overridden from common in xxx_gen2.c
>> I am good with the header approach which contains the delta over and above
>> xxx_gen2.c.
>
> I'll try to do that, but now I don't see the point of the SoC header files if
> they only contain the reset tables.
We already know that preset registers are also coming in (it is still a mystery
though on why h264 dec works without it, i have not got chance to explore it
more), so it would be useful when you extend it later.
Regards,
Vikash
>
> Neil
>
>>
>> Regards,
>> Vikash
>>> Neil
>>>
>>>>
>>>> Regards,
>>>> Vikash
>>>>>
>>>>> So I recall my goal, I just want to add sm8650 support,
>>>>> and I'm not the owner of this driver, and I'm really happy
>>>>> to help, but giving me random ideas to solve your problem
>>>>> doesn't help us at all going forward.
>>>>>
>>>>> Neil
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Vikash
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Neil
>>>>>>>
>>>>>>>>
>>>>>>>> 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,
>>>>>>>
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH RFC v5 0/8] media: qcom: iris: re-organize catalog & add support for SM8650
2025-04-14 10:54 ` Vikash Garodia
2025-04-14 12:09 ` neil.armstrong
@ 2025-04-14 12:10 ` Dmitry Baryshkov
1 sibling, 0 replies; 31+ messages in thread
From: Dmitry Baryshkov @ 2025-04-14 12:10 UTC (permalink / raw)
To: Vikash Garodia
Cc: neil.armstrong, Dikshita Agarwal, Abhinav Kumar,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, linux-kernel, linux-media,
linux-arm-msm, devicetree, Bryan O'Donoghue
On Mon, Apr 14, 2025 at 04:24:40PM +0530, Vikash Garodia wrote:
> Hi Neil,
>
> On 4/14/2025 1:05 PM, Neil Armstrong wrote:
> > Hi Vikash, Dikshita,
> >
> > On 10/04/2025 18: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.
> >
> > Any feedback on this patchset ?
> Myself and Dikshita went through the approach you are bringing here, let me
> update some context here:
> - sm8550, sm8650, sm8775p, qcs8300 are all irisv3, while qcs8300 is the scaled
> down variant i.e have 2 PIPE vs others having 4. Similarly there are other
> irisv3 having 1 pipe as well.
> - With above variations, firmware and instance caps would change for the variant
> SOCs.
> - Above these, few(less) bindings/connections specific delta would be there,
> like there is reset delta in sm8550 and sm8650.
>
> Given above, xxx_gen1.c and xxx_gen2.c can have all binding specific tables and
> SOC platform data, i.e sm8650_data (for sm8650). On top of this, individual SOC
> specific .c file can have any delta, from xxx_gen1/2.c) like reset table or
> preset register table, etc and export these delta structs in xxx_gen1.c or
> xxx_gen2.c.
>
> Going with above approach, sm8650.c would have only one reset table for now.
> Later if any delta is identified, the same can be added in it. All other common
> structs, can reside in xxx_gen2.c for now.
SGTM.
>
> Regards,
> Vikash
> >
> > Thanks,
> > Neil
> >
> >>
> >> 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,
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 31+ messages in thread