| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <71f3e23d-4f47-b047-9d41-9e3818f08849@quicinc.com>
Date: Mon, 21 Apr 2025 18:03:41 +0530
From: Vikash Garodia <quic_vgarodia@...cinc.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Dikshita Agarwal
<quic_dikshita@...cinc.com>,
Abhinav Kumar <quic_abhinavk@...cinc.com>,
"Mauro Carvalho Chehab" <mchehab@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio
<konradybcio@...nel.org>
CC: <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/4] media: iris: add qcs8300 platform data
On 4/18/2025 4:05 PM, Bryan O'Donoghue wrote:
> On 18/04/2025 07:28, Vikash Garodia wrote:
>> QCS8300 has a downscaled video core compared to SM8550, while it has
>> same bindings as that of SM8550. QCS8300.h captures the capabilities for
>> QCS8300 which is delta from SM8550.
>
> QCS8300 as a down-scaled .... compared to the SM8550.
> QSC8300 has the same bindings as SM8550 ?>
> Actually that makes not a world of sense as I read it.
>
> I'd suggest rewording this commit to just state what the QSC8300 itself can do
> without assuming the reader has any prior knowledge of the SM8550.
>
> Same comment for the other commits.
>
> Tell us what the QCS8300 is and what it does.
Given the patch adds the structures which are delta over 8550, it is more
relevant to compare with 8550 and describe the delta aspects.
>
>> Signed-off-by: Vikash Garodia <quic_vgarodia@...cinc.com>
>> ---
>> .../platform/qcom/iris/iris_platform_common.h | 1 +
>> .../media/platform/qcom/iris/iris_platform_gen2.c | 57 ++++++++++
>> .../platform/qcom/iris/iris_platform_qcs8300.h | 124 +++++++++++++++++++++
>> drivers/media/platform/qcom/iris/iris_probe.c | 4 +
>> 4 files changed, 186 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h
>> b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> index
>> 6bc3a7975b04d612f6c89206eae95dac678695fc..3191a910653ce4bd71de9a0b4465fd583602adf6 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> @@ -36,6 +36,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;
>> +extern struct iris_platform_data qcs8300_data;
>> enum platform_clk_type {
>> IRIS_AXI_CLK,
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> index
>> 5ff82296ee8ea5ad3954bd2254594048adcb8404..723e9f4cef42408168aca22b34ccd0a674a4fd25 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>> @@ -11,6 +11,7 @@
>> #include "iris_vpu_common.h"
>> #include "iris_platform_sm8650.h"
>> +#include "iris_platform_qcs8300.h"
>> #define VIDEO_ARCH_LX 1
>> @@ -326,3 +327,59 @@ struct iris_platform_data sm8650_data = {
>> .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),
>> };
>> +
>> +/*
>> + * Shares most of SM8550 data except:
>> + * - inst_caps to platform_inst_cap_qcs8300
>> + * - inst_fw_caps to inst_fw_cap_qcs8300
>> + */
>> +struct iris_platform_data qcs8300_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_s6.mbn",
>> + .pas_id = IRIS_PAS_ID,
>> + .inst_caps = &platform_inst_cap_qcs8300,
>> + .inst_fw_caps = inst_fw_cap_qcs8300,
>> + .inst_fw_caps_size = ARRAY_SIZE(inst_fw_cap_qcs8300),
>> + .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 = 2,
>> + .max_session_count = 16,
>> + .max_core_mbpf = ((4096 * 2176) / 256) * 4,
>> + .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),
>> +};
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_qcs8300.h
>> b/drivers/media/platform/qcom/iris/iris_platform_qcs8300.h
>> new file mode 100644
>> index
>> 0000000000000000000000000000000000000000..f82355d72fcffe7e361bd30877cccb83fe9b549f
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_qcs8300.h
>> @@ -0,0 +1,124 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2022-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +static struct platform_inst_fw_cap inst_fw_cap_qcs8300[] = {
>> + {
>> + .cap_id = PROFILE,
>> + .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE,
>> + .max = V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_HIGH,
>> + .step_or_mask = BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
>> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_HIGH) |
>> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) |
>> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
>> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_HIGH),
>> + .value = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
>> + .hfi_id = HFI_PROP_PROFILE,
>> + .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
>> + .set = iris_set_u32_enum,
>> + },
>> + {
>> + .cap_id = LEVEL,
>> + .min = V4L2_MPEG_VIDEO_H264_LEVEL_1_0,
>> + .max = V4L2_MPEG_VIDEO_H264_LEVEL_6_2,
>> + .step_or_mask = BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_0) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1B) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_1) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_2) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_3) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_2_0) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_2_1) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_2_2) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_3_0) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_3_1) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_3_2) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_4_0) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_4_1) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_4_2) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_5_0) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_5_1) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_5_2) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_6_0) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_6_1) |
>> + BIT(V4L2_MPEG_VIDEO_H264_LEVEL_6_2),
>> + .value = V4L2_MPEG_VIDEO_H264_LEVEL_6_1,
>> + .hfi_id = HFI_PROP_LEVEL,
>> + .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
>> + .set = iris_set_u32_enum,
>> + },
>> + {
>> + .cap_id = INPUT_BUF_HOST_MAX_COUNT,
>> + .min = DEFAULT_MAX_HOST_BUF_COUNT,
>> + .max = DEFAULT_MAX_HOST_BURST_BUF_COUNT,
>> + .step_or_mask = 1,
>> + .value = DEFAULT_MAX_HOST_BUF_COUNT,
>> + .hfi_id = HFI_PROP_BUFFER_HOST_MAX_COUNT,
>> + .flags = CAP_FLAG_INPUT_PORT,
>> + .set = iris_set_u32,
>> + },
>> + {
>> + .cap_id = STAGE,
>> + .min = STAGE_1,
>> + .max = STAGE_2,
>> + .step_or_mask = 1,
>> + .value = STAGE_2,
>> + .hfi_id = HFI_PROP_STAGE,
>> + .set = iris_set_stage,
>> + },
>> + {
>> + .cap_id = PIPE,
>> + .min = PIPE_1,
>> + .max = PIPE_2,
>> + .step_or_mask = 1,
>> + .value = PIPE_2,
>> + .hfi_id = HFI_PROP_PIPE,
>> + .set = iris_set_pipe,
>> + },
>> + {
>> + .cap_id = POC,
>> + .min = 0,
>> + .max = 2,
>> + .step_or_mask = 1,
>> + .value = 1,
>> + .hfi_id = HFI_PROP_PIC_ORDER_CNT_TYPE,
>> + },
>> + {
>> + .cap_id = CODED_FRAMES,
>> + .min = CODED_FRAMES_PROGRESSIVE,
>> + .max = CODED_FRAMES_PROGRESSIVE,
>> + .step_or_mask = 0,
>> + .value = CODED_FRAMES_PROGRESSIVE,
>> + .hfi_id = HFI_PROP_CODED_FRAMES,
>> + },
>> + {
>> + .cap_id = BIT_DEPTH,
>> + .min = BIT_DEPTH_8,
>> + .max = BIT_DEPTH_8,
>> + .step_or_mask = 1,
>> + .value = BIT_DEPTH_8,
>> + .hfi_id = HFI_PROP_LUMA_CHROMA_BIT_DEPTH,
>> + },
>> + {
>> + .cap_id = RAP_FRAME,
>> + .min = 0,
>> + .max = 1,
>> + .step_or_mask = 1,
>> + .value = 1,
>> + .hfi_id = HFI_PROP_DEC_START_FROM_RAP_FRAME,
>> + .flags = CAP_FLAG_INPUT_PORT,
>> + .set = iris_set_u32,
>> + },
>> +};
>> +
>> +static struct platform_inst_caps platform_inst_cap_qcs8300 = {
>> + .min_frame_width = 96,
>> + .max_frame_width = 4096,
>> + .min_frame_height = 96,
>> + .max_frame_height = 4096,
>> + .max_mbpf = (4096 * 2176) / 256,
>> + .mb_cycles_vpp = 200,
>> + .mb_cycles_fw = 326389,
>> + .mb_cycles_fw_vpp = 44156,
>> + .num_comv = 0,
>> +};
>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c
>> b/drivers/media/platform/qcom/iris/iris_probe.c
>> index
>> 7cd8650fbe9c09598670530103e3d5edf32953e7..e5f1896e55c390e920d206e7fc2c2be283bb39d8 100644
>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>> @@ -349,6 +349,10 @@ static const struct of_device_id iris_dt_match[] = {
>> .compatible = "qcom,sm8650-iris",
>> .data = &sm8650_data,
>> },
>> + {
>> + .compatible = "qcom,qcs8300-iris",
>> + .data = &qcs8300_data,
>> + },
> This is out-of-order, alphanumeric sorting puts qcs8300 before smX.
ok.
>
>> { },
>> };
>> MODULE_DEVICE_TABLE(of, iris_dt_match);
>>
>
> Also the ordering of this patch in the series is a bit odd.
>
> - Compat string
> - Driver changes
> - DT updates
>
> Please fix.
above sugested order looks better.
Regards,
Vikash
>
> ---
> bod
Powered by blists - more mailing lists