On 12/18/23 12:32, Dikshita Agarwal wrote:
Capabilities are set of video specifications and features supported
by a specific platform(SOC). Each capability is defined with
min, max, range, default value and corresponding HFI.
Also, platform data defines different resource details for
a specific platform(SOC). This change defines resource tables
for sm8550 platform data and use for initializing these resources.
Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx>
---
[...]
[...]
- ret = protect_secure_region(CP_START, CP_SIZE, CP_NONPIXEL_START,
- CP_NONPIXEL_SIZE, IRIS_PAS_ID);
+ cp_start = core->cap[CP_START].value;
+ cp_size = core->cap[CP_SIZE].value;
+ cp_nonpixel_start = core->cap[CP_NONPIXEL_START].value;
+ cp_nonpixel_size = core->cap[CP_NONPIXEL_SIZE].value;
but you just hardcoded these a couple patches ago.. are they
variable after all?
[...]
+ {DEC_CODECS, H264 | HEVC | VP9},
+ {MAX_SESSION_COUNT, 16},
+ {MAX_MBPF, 278528}, /* ((8192x4352)/256) * 2 */
+ {MAX_MBPS, 7833600}, /* max_load 7680x4320@60fps */
+ {NUM_VPP_PIPE, 4},
+ {HW_RESPONSE_TIMEOUT, HW_RESPONSE_TIMEOUT_VALUE},
+ {DMA_MASK, GENMASK(31, 29) - 1},
+ {CP_START, 0},
+ {CP_SIZE, 0x25800000},
+ {CP_NONPIXEL_START, 0x01000000},
+ {CP_NONPIXEL_SIZE, 0x24800000},
Why this and not an array of enum-indexed u32s?
+};
+
+static struct plat_inst_cap instance_cap_data_sm8550[] = {
you know all of the possible members here as well, you can just
create a struct of actual configurations instead of turning them
into generic capabilities that you have to parse either way at
some point
[...]
+
+static const struct bus_info sm8550_bus_table[] = {
+ { NULL, "iris-cnoc", 1000, 1000 },
+ { NULL, "iris-ddr", 1000, 15000000 },
This is a copy of the common data from the previous patches that you're
now dropping (why was it there in the first place then?). Is it specific
to this platform, or can it be reused?
+};
+
+static const struct clock_info sm8550_clk_table[] = {
+ { NULL, "gcc_video_axi0", GCC_VIDEO_AXI0_CLK, 0 },
+ { NULL, "core_clk", VIDEO_CC_MVS0C_CLK, 0 },
+ { NULL, "vcodec_core", VIDEO_CC_MVS0_CLK, 1 },
+};
Are these the input pad names?
+
+static const char * const sm8550_clk_reset_table[] = { "video_axi_reset", NULL };
Ditto
+
+static const char * const sm8550_pd_table[] = { "iris-ctl", "vcodec", NULL };
Ditto
+
+static const char * const sm8550_opp_pd_table[] = { "mxc", "mmcx", NULL };
Ditto
+
+static const struct bw_info sm8550_bw_table_dec[] = {
+ { 2073600, 1608000, 2742000 }, /* 4096x2160@60 */
+ { 1036800, 826000, 1393000 }, /* 4096x2160@30 */
+ { 489600, 567000, 723000 }, /* 1920x1080@60 */
+ { 244800, 294000, 372000 }, /* 1920x1080@30 */
+};
+
+static const struct reg_preset_info sm8550_reg_preset_table[] = {
+ { 0xB0088, 0x0, 0x11 },
lowercase hex for non-defines, please
+};
+
+static struct ubwc_config_data ubwc_config_sm8550[] = {
+ UBWC_CONFIG(8, 32, 16, 0, 1, 1, 1),
I think it would be far more telling to drop this #define and fill
in the values inline
+};
+
+struct platform_data sm8550_data = {
+ .bus_tbl = sm8550_bus_table,
+ .bus_tbl_size = ARRAY_SIZE(sm8550_bus_table),
+ .clk_tbl = sm8550_clk_table,
+ .clk_tbl_size = ARRAY_SIZE(sm8550_clk_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),
+
+ .pd_tbl = sm8550_pd_table,
+ .pd_tbl_size = ARRAY_SIZE(sm8550_pd_table),
+ .opp_pd_tbl = sm8550_opp_pd_table,
+ .opp_pd_tbl_size = ARRAY_SIZE(sm8550_opp_pd_table),
+
+ .reg_prst_tbl = sm8550_reg_preset_table,
+ .reg_prst_tbl_size = ARRAY_SIZE(sm8550_reg_preset_table),
+ .fwname = "vpu30_4v.mbn",
+ .pas_id = 9,
+
+ .core_data = core_data_sm8550,
+ .core_data_size = ARRAY_SIZE(core_data_sm8550),
+ .inst_cap_data = instance_cap_data_sm8550,
+ .inst_cap_data_size = ARRAY_SIZE(instance_cap_data_sm8550),
+ .ubwc_config = ubwc_config_sm8550,
Unless any of these are going to be reused, please inline all of the
values here..
Konrad
