[PATCH v3 7/7] media: iris: add platform data for kaanapali

Vikash Garodia posted 7 patches 2 weeks, 6 days ago
[PATCH v3 7/7] media: iris: add platform data for kaanapali
Posted by Vikash Garodia 2 weeks, 6 days ago
Add support for the kaanapali platform by re-using the SM8550
definitions and using the vpu4 ops.
Move the configurations that differs in a per-SoC platform header, that
will contain SoC specific data.

Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
 .../platform/qcom/iris/iris_platform_common.h      |  1 +
 .../media/platform/qcom/iris/iris_platform_gen2.c  | 90 ++++++++++++++++++++++
 .../platform/qcom/iris/iris_platform_kaanapali.h   | 83 ++++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_probe.c      |  4 +
 4 files changed, 178 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index df63a06b8401cd367c69ab8909af227f04bf69bf..d97e3fb18c3636abbbca3c2086c5263efeca3db5 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -41,6 +41,7 @@ enum pipe_type {
 	PIPE_4 = 4,
 };
 
+extern const struct iris_platform_data kaanapali_data;
 extern const struct iris_platform_data qcs8300_data;
 extern const struct iris_platform_data sc7280_data;
 extern const struct iris_platform_data sm8250_data;
diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
index 5da90d47f9c6eab4a7e6b17841fdc0e599397bf7..df906f6b9fcd80100872a12815036a3aad9e925b 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
@@ -12,6 +12,7 @@
 #include "iris_vpu_buffer.h"
 #include "iris_vpu_common.h"
 
+#include "iris_platform_kaanapali.h"
 #include "iris_platform_qcs8300.h"
 #include "iris_platform_sm8650.h"
 #include "iris_platform_sm8750.h"
@@ -921,6 +922,95 @@ static const u32 sm8550_enc_op_int_buf_tbl[] = {
 	BUF_SCRATCH_2,
 };
 
+const struct iris_platform_data kaanapali_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,
+	.get_vpu_buffer_size = iris_vpu4x_buf_size,
+	.vpu_ops = &iris_vpu4x_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 = kaanapali_clk_reset_table,
+	.clk_rst_tbl_size = ARRAY_SIZE(kaanapali_clk_reset_table),
+	.bw_tbl_dec = sm8550_bw_table_dec,
+	.bw_tbl_dec_size = ARRAY_SIZE(sm8550_bw_table_dec),
+	.pmdomain_tbl = kaanapali_pmdomain_table,
+	.pmdomain_tbl_size = ARRAY_SIZE(kaanapali_pmdomain_table),
+	.opp_pd_tbl = sm8550_opp_pd_table,
+	.opp_pd_tbl_size = ARRAY_SIZE(sm8550_opp_pd_table),
+	.clk_tbl = kaanapali_clk_table,
+	.clk_tbl_size = ARRAY_SIZE(kaanapali_clk_table),
+	.opp_clk_tbl = kaanapali_opp_clk_table,
+	/* Upper bound of DMA address range */
+	.dma_mask = 0xffc00000 - 1,
+	.fwname = "qcom/vpu/vpu40_p2_s7.mbn",
+	.pas_id = IRIS_PAS_ID,
+	.inst_iris_fmts = platform_fmts_sm8550_dec,
+	.inst_iris_fmts_size = ARRAY_SIZE(platform_fmts_sm8550_dec),
+	.inst_caps = &platform_inst_cap_sm8550,
+	.inst_fw_caps_dec = inst_fw_cap_sm8550_dec,
+	.inst_fw_caps_dec_size = ARRAY_SIZE(inst_fw_cap_sm8550_dec),
+	.inst_fw_caps_enc = inst_fw_cap_sm8550_enc,
+	.inst_fw_caps_enc_size = ARRAY_SIZE(inst_fw_cap_sm8550_enc),
+	.tz_cp_config_data = tz_cp_config_kaanapali,
+	.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_kaanapali),
+	.cb_data = kaanapali_cb_data,
+	.cb_data_size = ARRAY_SIZE(kaanapali_cb_data),
+	.core_arch = VIDEO_ARCH_LX,
+	.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
+	.ubwc_config = &ubwc_config_sm8550,
+	.num_vpp_pipe = 2,
+	.max_session_count = 16,
+	.max_core_mbpf = NUM_MBS_8K * 2,
+	.max_core_mbps = ((8192 * 4320) / 256) * 60,
+	.dec_input_config_params_default =
+		sm8550_vdec_input_config_params_default,
+	.dec_input_config_params_default_size =
+		ARRAY_SIZE(sm8550_vdec_input_config_params_default),
+	.dec_input_config_params_hevc =
+		sm8550_vdec_input_config_param_hevc,
+	.dec_input_config_params_hevc_size =
+		ARRAY_SIZE(sm8550_vdec_input_config_param_hevc),
+	.dec_input_config_params_vp9 =
+		sm8550_vdec_input_config_param_vp9,
+	.dec_input_config_params_vp9_size =
+		ARRAY_SIZE(sm8550_vdec_input_config_param_vp9),
+	.dec_output_config_params =
+		sm8550_vdec_output_config_params,
+	.dec_output_config_params_size =
+		ARRAY_SIZE(sm8550_vdec_output_config_params),
+
+	.enc_input_config_params =
+		sm8550_venc_input_config_params,
+	.enc_input_config_params_size =
+		ARRAY_SIZE(sm8550_venc_input_config_params),
+	.enc_output_config_params =
+		sm8550_venc_output_config_params,
+	.enc_output_config_params_size =
+		ARRAY_SIZE(sm8550_venc_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_avc = sm8550_vdec_subscribe_output_properties_avc,
+	.dec_output_prop_avc_size =
+		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_avc),
+	.dec_output_prop_hevc = sm8550_vdec_subscribe_output_properties_hevc,
+	.dec_output_prop_hevc_size =
+		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_hevc),
+	.dec_output_prop_vp9 = sm8550_vdec_subscribe_output_properties_vp9,
+	.dec_output_prop_vp9_size =
+		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_vp9),
+
+	.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),
+
+	.enc_op_int_buf_tbl = sm8550_enc_op_int_buf_tbl,
+	.enc_op_int_buf_tbl_size = ARRAY_SIZE(sm8550_enc_op_int_buf_tbl),
+};
+
 const struct iris_platform_data sm8550_data = {
 	.get_instance = iris_hfi_gen2_get_instance,
 	.init_hfi_command_ops = iris_hfi_gen2_command_ops_init,
diff --git a/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
new file mode 100644
index 0000000000000000000000000000000000000000..bdca1e5bf673353862c1554fb0420f73b3f519cb
--- /dev/null
+++ b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef __IRIS_PLATFORM_KAANAPALI_H__
+#define __IRIS_PLATFORM_KAANAPALI_H__
+
+#include <dt-bindings/media/qcom,kaanapali-iris.h>
+
+#define VIDEO_REGION_VM0_SECURE_NP_ID		1
+#define VIDEO_REGION_VM0_NONSECURE_NP_ID	5
+
+static const char *const kaanapali_clk_reset_table[] = {
+	"bus0",
+	"bus1",
+	"core",
+	"vcodec0_core",
+};
+
+static const char *const kaanapali_pmdomain_table[] = {
+	"venus",
+	"vcodec0",
+	"vpp0",
+	"vpp1",
+	"apv",
+};
+
+static const struct platform_clk_data kaanapali_clk_table[] = {
+	{ IRIS_AXI_CLK, "iface" },
+	{ IRIS_CTRL_CLK, "core" },
+	{ IRIS_HW_CLK, "vcodec0_core" },
+	{ IRIS_AXI1_CLK, "iface1" },
+	{ IRIS_CTRL_FREERUN_CLK, "core_freerun" },
+	{ IRIS_HW_FREERUN_CLK, "vcodec0_core_freerun" },
+	{ IRIS_BSE_HW_CLK, "vcodec_bse" },
+	{ IRIS_VPP0_HW_CLK, "vcodec_vpp0" },
+	{ IRIS_VPP1_HW_CLK, "vcodec_vpp1" },
+	{ IRIS_APV_HW_CLK, "vcodec_apv" },
+};
+
+static const char *const kaanapali_opp_clk_table[] = {
+	"vcodec0_core",
+	"vcodec_apv",
+	"vcodec_bse",
+	"core",
+	NULL,
+};
+
+static struct tz_cp_config tz_cp_config_kaanapali[] = {
+	{
+		.cp_start = VIDEO_REGION_VM0_SECURE_NP_ID,
+		.cp_size = 0,
+		.cp_nonpixel_start = 0x01000000,
+		.cp_nonpixel_size = 0x24800000,
+	},
+	{
+		.cp_start = VIDEO_REGION_VM0_NONSECURE_NP_ID,
+		.cp_size = 0,
+		.cp_nonpixel_start = 0x25800000,
+		.cp_nonpixel_size = 0xda400000,
+	},
+};
+
+static struct iris_context_bank kaanapali_cb_data[] = {
+	{
+		.name = "iris_bitstream",
+		.f_id = IRIS_BITSTREAM,
+		.region_mask = BIT(IRIS_BITSTREAM_REGION),
+	},
+	{
+		.name = "iris_non_pixel",
+		.f_id = IRIS_NON_PIXEL,
+		.region_mask = BIT(IRIS_NON_PIXEL_REGION),
+	},
+	{
+		.name = "iris_pixel",
+		.f_id = IRIS_PIXEL,
+		.region_mask = BIT(IRIS_PIXEL_REGION),
+	},
+};
+
+#endif /* __IRIS_PLATFORM_KAANAPALI_H__ */
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index 439e6e0fe8adf8287f81d26257ef2a7e9f21e53d..f6d8761daf0471d3aabec21c708445ee7698487b 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -406,6 +406,10 @@ static const struct dev_pm_ops iris_pm_ops = {
 };
 
 static const struct of_device_id iris_dt_match[] = {
+	{
+		.compatible = "qcom,kaanapali-iris",
+		.data = &kaanapali_data,
+	},
 	{
 		.compatible = "qcom,qcs8300-iris",
 		.data = &qcs8300_data,

-- 
2.34.1
Re: [PATCH v3 7/7] media: iris: add platform data for kaanapali
Posted by Dmitry Baryshkov 2 weeks, 6 days ago
On Fri, Mar 13, 2026 at 06:49:41PM +0530, Vikash Garodia wrote:
> Add support for the kaanapali platform by re-using the SM8550
> definitions and using the vpu4 ops.
> Move the configurations that differs in a per-SoC platform header, that
> will contain SoC specific data.
> 
> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> ---
>  .../platform/qcom/iris/iris_platform_common.h      |  1 +
>  .../media/platform/qcom/iris/iris_platform_gen2.c  | 90 ++++++++++++++++++++++
>  .../platform/qcom/iris/iris_platform_kaanapali.h   | 83 ++++++++++++++++++++
>  drivers/media/platform/qcom/iris/iris_probe.c      |  4 +
>  4 files changed, 178 insertions(+)
> 

> diff --git a/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..bdca1e5bf673353862c1554fb0420f73b3f519cb
> --- /dev/null
> +++ b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
> @@ -0,0 +1,83 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef __IRIS_PLATFORM_KAANAPALI_H__
> +#define __IRIS_PLATFORM_KAANAPALI_H__
> +
> +#include <dt-bindings/media/qcom,kaanapali-iris.h>

So, you are including the bindings here, from the header, which gets
included from the C source file including headers for all the platforms.
What if Kaanapali+1 (or +3) defines different sets of regions?

> +
> +#define VIDEO_REGION_VM0_SECURE_NP_ID		1
> +#define VIDEO_REGION_VM0_NONSECURE_NP_ID	5
> +
> +static const char *const kaanapali_clk_reset_table[] = {
> +	"bus0",
> +	"bus1",
> +	"core",
> +	"vcodec0_core",
> +};
> +
> +static const char *const kaanapali_pmdomain_table[] = {
> +	"venus",
> +	"vcodec0",
> +	"vpp0",
> +	"vpp1",
> +	"apv",
> +};
> +
> +static const struct platform_clk_data kaanapali_clk_table[] = {
> +	{ IRIS_AXI_CLK, "iface" },
> +	{ IRIS_CTRL_CLK, "core" },
> +	{ IRIS_HW_CLK, "vcodec0_core" },
> +	{ IRIS_AXI1_CLK, "iface1" },
> +	{ IRIS_CTRL_FREERUN_CLK, "core_freerun" },
> +	{ IRIS_HW_FREERUN_CLK, "vcodec0_core_freerun" },
> +	{ IRIS_BSE_HW_CLK, "vcodec_bse" },
> +	{ IRIS_VPP0_HW_CLK, "vcodec_vpp0" },
> +	{ IRIS_VPP1_HW_CLK, "vcodec_vpp1" },
> +	{ IRIS_APV_HW_CLK, "vcodec_apv" },
> +};
> +
> +static const char *const kaanapali_opp_clk_table[] = {
> +	"vcodec0_core",
> +	"vcodec_apv",
> +	"vcodec_bse",
> +	"core",
> +	NULL,
> +};
> +
> +static struct tz_cp_config tz_cp_config_kaanapali[] = {
> +	{
> +		.cp_start = VIDEO_REGION_VM0_SECURE_NP_ID,
> +		.cp_size = 0,
> +		.cp_nonpixel_start = 0x01000000,
> +		.cp_nonpixel_size = 0x24800000,
> +	},
> +	{
> +		.cp_start = VIDEO_REGION_VM0_NONSECURE_NP_ID,
> +		.cp_size = 0,
> +		.cp_nonpixel_start = 0x25800000,
> +		.cp_nonpixel_size = 0xda400000,
> +	},
> +};
> +
> +static struct iris_context_bank kaanapali_cb_data[] = {
> +	{
> +		.name = "iris_bitstream",
> +		.f_id = IRIS_BITSTREAM,
> +		.region_mask = BIT(IRIS_BITSTREAM_REGION),

I'd say, it's really easy to mix IRIS_BITSTREAM and
IRIS_BITSTREAM_REGION when looking at the code, which might be bad
because they are not equal.

> +	},
> +	{
> +		.name = "iris_non_pixel",
> +		.f_id = IRIS_NON_PIXEL,
> +		.region_mask = BIT(IRIS_NON_PIXEL_REGION),
> +	},
> +	{
> +		.name = "iris_pixel",
> +		.f_id = IRIS_PIXEL,
> +		.region_mask = BIT(IRIS_PIXEL_REGION),
> +	},
> +};
> +
> +#endif /* __IRIS_PLATFORM_KAANAPALI_H__ */
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index 439e6e0fe8adf8287f81d26257ef2a7e9f21e53d..f6d8761daf0471d3aabec21c708445ee7698487b 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -406,6 +406,10 @@ static const struct dev_pm_ops iris_pm_ops = {
>  };
>  
>  static const struct of_device_id iris_dt_match[] = {
> +	{
> +		.compatible = "qcom,kaanapali-iris",
> +		.data = &kaanapali_data,
> +	},
>  	{
>  		.compatible = "qcom,qcs8300-iris",
>  		.data = &qcs8300_data,
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry
Re: [PATCH v3 7/7] media: iris: add platform data for kaanapali
Posted by Vikash Garodia 5 days, 17 hours ago
On 3/13/2026 9:16 PM, Dmitry Baryshkov wrote:
> On Fri, Mar 13, 2026 at 06:49:41PM +0530, Vikash Garodia wrote:
>> Add support for the kaanapali platform by re-using the SM8550
>> definitions and using the vpu4 ops.
>> Move the configurations that differs in a per-SoC platform header, that
>> will contain SoC specific data.
>>
>> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>> ---
>>   .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>   .../media/platform/qcom/iris/iris_platform_gen2.c  | 90 ++++++++++++++++++++++
>>   .../platform/qcom/iris/iris_platform_kaanapali.h   | 83 ++++++++++++++++++++
>>   drivers/media/platform/qcom/iris/iris_probe.c      |  4 +
>>   4 files changed, 178 insertions(+)
>>
> 
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..bdca1e5bf673353862c1554fb0420f73b3f519cb
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
>> @@ -0,0 +1,83 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#ifndef __IRIS_PLATFORM_KAANAPALI_H__
>> +#define __IRIS_PLATFORM_KAANAPALI_H__
>> +
>> +#include <dt-bindings/media/qcom,kaanapali-iris.h>
> 
> So, you are including the bindings here, from the header, which gets
> included from the C source file including headers for all the platforms.
> What if Kaanapali+1 (or +3) defines different sets of regions?
> 

thats correct, to handle this, the soc platform data can be defined in 
iris_platform_kaanapali.c, and iris_platform_kaanapali.h can extern the 
platform data struct which gets included in gen2.c.
Also would have preferred naming the binding abi header 
"qcom,kaanapali-iris.h" to something generic so that it can be included 
in other platforms, but it seems like the maintainer does not like that 
idea.

>> +
>> +#define VIDEO_REGION_VM0_SECURE_NP_ID		1
>> +#define VIDEO_REGION_VM0_NONSECURE_NP_ID	5
>> +
>> +static const char *const kaanapali_clk_reset_table[] = {
>> +	"bus0",
>> +	"bus1",
>> +	"core",
>> +	"vcodec0_core",
>> +};
>> +
>> +static const char *const kaanapali_pmdomain_table[] = {
>> +	"venus",
>> +	"vcodec0",
>> +	"vpp0",
>> +	"vpp1",
>> +	"apv",
>> +};
>> +
>> +static const struct platform_clk_data kaanapali_clk_table[] = {
>> +	{ IRIS_AXI_CLK, "iface" },
>> +	{ IRIS_CTRL_CLK, "core" },
>> +	{ IRIS_HW_CLK, "vcodec0_core" },
>> +	{ IRIS_AXI1_CLK, "iface1" },
>> +	{ IRIS_CTRL_FREERUN_CLK, "core_freerun" },
>> +	{ IRIS_HW_FREERUN_CLK, "vcodec0_core_freerun" },
>> +	{ IRIS_BSE_HW_CLK, "vcodec_bse" },
>> +	{ IRIS_VPP0_HW_CLK, "vcodec_vpp0" },
>> +	{ IRIS_VPP1_HW_CLK, "vcodec_vpp1" },
>> +	{ IRIS_APV_HW_CLK, "vcodec_apv" },
>> +};
>> +
>> +static const char *const kaanapali_opp_clk_table[] = {
>> +	"vcodec0_core",
>> +	"vcodec_apv",
>> +	"vcodec_bse",
>> +	"core",
>> +	NULL,
>> +};
>> +
>> +static struct tz_cp_config tz_cp_config_kaanapali[] = {
>> +	{
>> +		.cp_start = VIDEO_REGION_VM0_SECURE_NP_ID,
>> +		.cp_size = 0,
>> +		.cp_nonpixel_start = 0x01000000,
>> +		.cp_nonpixel_size = 0x24800000,
>> +	},
>> +	{
>> +		.cp_start = VIDEO_REGION_VM0_NONSECURE_NP_ID,
>> +		.cp_size = 0,
>> +		.cp_nonpixel_start = 0x25800000,
>> +		.cp_nonpixel_size = 0xda400000,
>> +	},
>> +};
>> +
>> +static struct iris_context_bank kaanapali_cb_data[] = {
>> +	{
>> +		.name = "iris_bitstream",
>> +		.f_id = IRIS_BITSTREAM,
>> +		.region_mask = BIT(IRIS_BITSTREAM_REGION),
> 
> I'd say, it's really easy to mix IRIS_BITSTREAM and
> IRIS_BITSTREAM_REGION when looking at the code, which might be bad
> because they are not equal.

this logic have changed now given that the handling is now 1:1, rather 
than multi map case. Maybe..we can revisit this in v4.

Regards,
Vikash

> 
>> +	},
>> +	{
>> +		.name = "iris_non_pixel",
>> +		.f_id = IRIS_NON_PIXEL,
>> +		.region_mask = BIT(IRIS_NON_PIXEL_REGION),
>> +	},
>> +	{
>> +		.name = "iris_pixel",
>> +		.f_id = IRIS_PIXEL,
>> +		.region_mask = BIT(IRIS_PIXEL_REGION),
>> +	},
>> +};
>> +
>> +#endif /* __IRIS_PLATFORM_KAANAPALI_H__ */
>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
>> index 439e6e0fe8adf8287f81d26257ef2a7e9f21e53d..f6d8761daf0471d3aabec21c708445ee7698487b 100644
>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>> @@ -406,6 +406,10 @@ static const struct dev_pm_ops iris_pm_ops = {
>>   };
>>   
>>   static const struct of_device_id iris_dt_match[] = {
>> +	{
>> +		.compatible = "qcom,kaanapali-iris",
>> +		.data = &kaanapali_data,
>> +	},
>>   	{
>>   		.compatible = "qcom,qcs8300-iris",
>>   		.data = &qcs8300_data,
>>
>> -- 
>> 2.34.1
>>
>
Re: [PATCH v3 7/7] media: iris: add platform data for kaanapali
Posted by Krzysztof Kozlowski 2 weeks, 6 days ago
On 13/03/2026 16:46, Dmitry Baryshkov wrote:
> On Fri, Mar 13, 2026 at 06:49:41PM +0530, Vikash Garodia wrote:
>> Add support for the kaanapali platform by re-using the SM8550
>> definitions and using the vpu4 ops.
>> Move the configurations that differs in a per-SoC platform header, that
>> will contain SoC specific data.
>>
>> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>> ---
>>  .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>  .../media/platform/qcom/iris/iris_platform_gen2.c  | 90 ++++++++++++++++++++++
>>  .../platform/qcom/iris/iris_platform_kaanapali.h   | 83 ++++++++++++++++++++
>>  drivers/media/platform/qcom/iris/iris_probe.c      |  4 +
>>  4 files changed, 178 insertions(+)
>>
> 
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..bdca1e5bf673353862c1554fb0420f73b3f519cb
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
>> @@ -0,0 +1,83 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#ifndef __IRIS_PLATFORM_KAANAPALI_H__
>> +#define __IRIS_PLATFORM_KAANAPALI_H__
>> +
>> +#include <dt-bindings/media/qcom,kaanapali-iris.h>
> 
> So, you are including the bindings here, from the header, which gets
> included from the C source file including headers for all the platforms.
> What if Kaanapali+1 (or +3) defines different sets of regions?

Different problem - header file MUST NOT have data definitions.

That's basic of C, we don't write such code. First because it leads to
multiplied, redundant data. Second, that's not C coding style.

This pattern in Qualcomm Iris is terrible and I could accept variations
of existing data like below:

>
>> +
>> +#define VIDEO_REGION_VM0_SECURE_NP_ID		1
>> +#define VIDEO_REGION_VM0_NONSECURE_NP_ID	5
>> +
>> +static const char *const kaanapali_clk_reset_table[] = {
>> +	"bus0",
>> +	"bus1",
>> +	"core",
>> +	"vcodec0_core",
>> +};
>> +
>> +static const char *const kaanapali_pmdomain_table[] = {
>> +	"venus",
>> +	"vcodec0",
>> +	"vpp0",
>> +	"vpp1",
>> +	"apv",
>> +};
>> +
>> +static const struct platform_clk_data kaanapali_clk_table[] = {
>> +	{ IRIS_AXI_CLK, "iface" },
>> +	{ IRIS_CTRL_CLK, "core" },
>> +	{ IRIS_HW_CLK, "vcodec0_core" },
>> +	{ IRIS_AXI1_CLK, "iface1" },
>> +	{ IRIS_CTRL_FREERUN_CLK, "core_freerun" },
>> +	{ IRIS_HW_FREERUN_CLK, "vcodec0_core_freerun" },
>> +	{ IRIS_BSE_HW_CLK, "vcodec_bse" },
>> +	{ IRIS_VPP0_HW_CLK, "vcodec_vpp0" },
>> +	{ IRIS_VPP1_HW_CLK, "vcodec_vpp1" },
>> +	{ IRIS_APV_HW_CLK, "vcodec_apv" },
>> +};
>> +
>> +static const char *const kaanapali_opp_clk_table[] = {
>> +	"vcodec0_core",
>> +	"vcodec_apv",
>> +	"vcodec_bse",
>> +	"core",
>> +	NULL,
>> +};
>> +
>> +static struct tz_cp_config tz_cp_config_kaanapali[] = {

But this is new thus NAK.

Don't grow this broken pattern. There is no single reason data
definition should be placed in a header. No single one.

Best regards,
Krzysztof
Re: [PATCH v3 7/7] media: iris: add platform data for kaanapali
Posted by Vikash Garodia 5 days, 17 hours ago
On 3/13/2026 9:25 PM, Krzysztof Kozlowski wrote:
> On 13/03/2026 16:46, Dmitry Baryshkov wrote:
>> On Fri, Mar 13, 2026 at 06:49:41PM +0530, Vikash Garodia wrote:
>>> Add support for the kaanapali platform by re-using the SM8550
>>> definitions and using the vpu4 ops.
>>> Move the configurations that differs in a per-SoC platform header, that
>>> will contain SoC specific data.
>>>
>>> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>>> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>>> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>>> ---
>>>   .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>>   .../media/platform/qcom/iris/iris_platform_gen2.c  | 90 ++++++++++++++++++++++
>>>   .../platform/qcom/iris/iris_platform_kaanapali.h   | 83 ++++++++++++++++++++
>>>   drivers/media/platform/qcom/iris/iris_probe.c      |  4 +
>>>   4 files changed, 178 insertions(+)
>>>
>>
>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..bdca1e5bf673353862c1554fb0420f73b3f519cb
>>> --- /dev/null
>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_kaanapali.h
>>> @@ -0,0 +1,83 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright (c) Qualcomm Innovation Center, Inc. All rights reserved.
>>> + */
>>> +
>>> +#ifndef __IRIS_PLATFORM_KAANAPALI_H__
>>> +#define __IRIS_PLATFORM_KAANAPALI_H__
>>> +
>>> +#include <dt-bindings/media/qcom,kaanapali-iris.h>
>>
>> So, you are including the bindings here, from the header, which gets
>> included from the C source file including headers for all the platforms.
>> What if Kaanapali+1 (or +3) defines different sets of regions?
> 
> Different problem - header file MUST NOT have data definitions.
> 
> That's basic of C, we don't write such code. First because it leads to
> multiplied, redundant data. Second, that's not C coding style.

Ack, will define in c and extern in header approach

> 
> This pattern in Qualcomm Iris is terrible and I could accept variations
> of existing data like below:
> 
>>
>>> +
>>> +#define VIDEO_REGION_VM0_SECURE_NP_ID		1
>>> +#define VIDEO_REGION_VM0_NONSECURE_NP_ID	5
>>> +
>>> +static const char *const kaanapali_clk_reset_table[] = {
>>> +	"bus0",
>>> +	"bus1",
>>> +	"core",
>>> +	"vcodec0_core",
>>> +};
>>> +
>>> +static const char *const kaanapali_pmdomain_table[] = {
>>> +	"venus",
>>> +	"vcodec0",
>>> +	"vpp0",
>>> +	"vpp1",
>>> +	"apv",
>>> +};
>>> +
>>> +static const struct platform_clk_data kaanapali_clk_table[] = {
>>> +	{ IRIS_AXI_CLK, "iface" },
>>> +	{ IRIS_CTRL_CLK, "core" },
>>> +	{ IRIS_HW_CLK, "vcodec0_core" },
>>> +	{ IRIS_AXI1_CLK, "iface1" },
>>> +	{ IRIS_CTRL_FREERUN_CLK, "core_freerun" },
>>> +	{ IRIS_HW_FREERUN_CLK, "vcodec0_core_freerun" },
>>> +	{ IRIS_BSE_HW_CLK, "vcodec_bse" },
>>> +	{ IRIS_VPP0_HW_CLK, "vcodec_vpp0" },
>>> +	{ IRIS_VPP1_HW_CLK, "vcodec_vpp1" },
>>> +	{ IRIS_APV_HW_CLK, "vcodec_apv" },
>>> +};
>>> +
>>> +static const char *const kaanapali_opp_clk_table[] = {
>>> +	"vcodec0_core",
>>> +	"vcodec_apv",
>>> +	"vcodec_bse",
>>> +	"core",
>>> +	NULL,
>>> +};
>>> +
>>> +static struct tz_cp_config tz_cp_config_kaanapali[] = {
> 
> But this is new thus NAK.
> 
> Don't grow this broken pattern. There is no single reason data
> definition should be placed in a header. No single one.
> 
> Best regards,
> Krzysztof