[PATCH v3] drm/amd/display: Refactor construct_phy function in dc/link/link_factory.c
Liu, Wenjing
Wenjing.Liu at amd.com
Tue May 21 15:56:53 UTC 2024
[AMD Official Use Only - AMD Internal Distribution Only]
Reviewed-by: Wenjing Liu <wenjing.liu at amd.com>
-----Original Message-----
From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
Sent: Friday, May 10, 2024 6:12 AM
To: Liu, Wenjing <Wenjing.Liu at amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira at amd.com>; Pillai, Aurabindo <Aurabindo.Pillai at amd.com>
Cc: amd-gfx at lists.freedesktop.org; SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>; Zuo, Jerry <Jerry.Zuo at amd.com>; Zhuo, Lillian <Qingqing.Zhuo at amd.com>; Chung, ChiaHsuan (Tom) <ChiaHsuan.Chung at amd.com>; Lee, Alvin <Alvin.Lee2 at amd.com>; Li, Roman <Roman.Li at amd.com>; Wu, Hersen <hersenxs.wu at amd.com>; Hung, Alex <Alex.Hung at amd.com>; Wentland, Harry <Harry.Wentland at amd.com>
Subject: [PATCH v3] drm/amd/display: Refactor construct_phy function in dc/link/link_factory.c
This commit modifies the construct_phy function to handle the case where `bios->integrated_info` is NULL and to address a compiler warning about a large stack allocation.
Upon examination, it was found that the local `integrated_info` structure was just used to copy values which is large and was being declared directly on the stack which could potentially lead to performance issues. This commit changes the code to use `bios->integrated_info` directly, which avoids the need for a large stack allocation.
The function now checks if `bios->integrated_info` is NULL before entering a for loop that uses it. If `bios->integrated_info` is NULL, the function skips the for loop and continues executing the rest of the code. This ensures that the function behaves correctly when `bios->integrated_info` is NULL and improves compatibility with dGPUs.
Fixes the below with gcc W=1:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c: In function ‘construct_phy’:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c:743:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
Cc: Wenjing Liu <wenjing.liu at amd.com>
Cc: Jerry Zuo <jerry.zuo at amd.com>
Cc: Qingqing Zhuo <qingqing.zhuo at amd.com>
Cc: Tom Chung <chiahsuan.chung at amd.com>
Cc: Alvin Lee <alvin.lee2 at amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
Cc: Roman Li <roman.li at amd.com>
Cc: Hersen Wu <hersenxs.wu at amd.com>
Cc: Alex Hung <alex.hung at amd.com>
Cc: Aurabindo Pillai <aurabindo.pillai at amd.com>
Cc: Harry Wentland <harry.wentland at amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
Suggested-by: Wenjing Liu <wenjing.liu at amd.com>
---
v3:
- Directly used bios->integrated_info instead of integrated_info to
avoid large copying (Wenjing)
.../drm/amd/display/dc/link/link_factory.c | 67 ++++++++++---------
1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_factory.c b/drivers/gpu/drm/amd/display/dc/link/link_factory.c
index 2c3f5d662285..8073fdae9cb1 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_factory.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_factory.c
@@ -456,7 +456,6 @@ static bool construct_phy(struct dc_link *link,
struct dc_context *dc_ctx = init_params->ctx;
struct encoder_init_data enc_init_data = { 0 };
struct panel_cntl_init_data panel_cntl_init_data = { 0 };
- struct integrated_info info = { 0 };
struct dc_bios *bios = init_params->dc->ctx->dc_bios;
const struct dc_vbios_funcs *bp_funcs = bios->funcs;
struct bp_disp_connector_caps_info disp_connect_caps_info = { 0 }; @@ -671,42 +670,44 @@ static bool construct_phy(struct dc_link *link,
break;
}
- if (bios->integrated_info)
- info = *bios->integrated_info;
-
- /* Look for channel mapping corresponding to connector and device tag */
- for (i = 0; i < MAX_NUMBER_OF_EXT_DISPLAY_PATH; i++) {
- struct external_display_path *path =
- &info.ext_disp_conn_info.path[i];
-
- if (path->device_connector_id.enum_id == link->link_id.enum_id &&
- path->device_connector_id.id == link->link_id.id &&
- path->device_connector_id.type == link->link_id.type) {
- if (link->device_tag.acpi_device != 0 &&
- path->device_acpi_enum == link->device_tag.acpi_device) {
- link->ddi_channel_mapping = path->channel_mapping;
- link->chip_caps = path->caps;
- DC_LOG_DC("BIOS object table - ddi_channel_mapping: 0x%04X", link->ddi_channel_mapping.raw);
- DC_LOG_DC("BIOS object table - chip_caps: %d", link->chip_caps);
- } else if (path->device_tag ==
- link->device_tag.dev_id.raw_device_tag) {
- link->ddi_channel_mapping = path->channel_mapping;
- link->chip_caps = path->caps;
- DC_LOG_DC("BIOS object table - ddi_channel_mapping: 0x%04X", link->ddi_channel_mapping.raw);
- DC_LOG_DC("BIOS object table - chip_caps: %d", link->chip_caps);
- }
+ if (bios->integrated_info) {
+ /* Look for channel mapping corresponding to connector and device tag */
+ for (i = 0; i < MAX_NUMBER_OF_EXT_DISPLAY_PATH; i++) {
+ struct external_display_path *path =
+ &bios->integrated_info->ext_disp_conn_info.path[i];
+
+ if (path->device_connector_id.enum_id == link->link_id.enum_id &&
+ path->device_connector_id.id == link->link_id.id &&
+ path->device_connector_id.type == link->link_id.type) {
+ if (link->device_tag.acpi_device != 0 &&
+ path->device_acpi_enum == link->device_tag.acpi_device) {
+ link->ddi_channel_mapping = path->channel_mapping;
+ link->chip_caps = path->caps;
+ DC_LOG_DC("BIOS object table - ddi_channel_mapping: 0x%04X",
+ link->ddi_channel_mapping.raw);
+ DC_LOG_DC("BIOS object table - chip_caps: %d",
+ link->chip_caps);
+ } else if (path->device_tag ==
+ link->device_tag.dev_id.raw_device_tag) {
+ link->ddi_channel_mapping = path->channel_mapping;
+ link->chip_caps = path->caps;
+ DC_LOG_DC("BIOS object table - ddi_channel_mapping: 0x%04X",
+ link->ddi_channel_mapping.raw);
+ DC_LOG_DC("BIOS object table - chip_caps: %d",
+ link->chip_caps);
+ }
+
+ if (link->chip_caps & EXT_DISPLAY_PATH_CAPS__DP_FIXED_VS_EN) {
+ link->bios_forced_drive_settings.VOLTAGE_SWING =
+ (bios->integrated_info->ext_disp_conn_info.fixdpvoltageswing & 0x3);
+ link->bios_forced_drive_settings.PRE_EMPHASIS =
+ ((bios->integrated_info->ext_disp_conn_info.fixdpvoltageswing >> 2) & 0x3);
+ }
- if (link->chip_caps & EXT_DISPLAY_PATH_CAPS__DP_FIXED_VS_EN) {
- link->bios_forced_drive_settings.VOLTAGE_SWING =
- (info.ext_disp_conn_info.fixdpvoltageswing & 0x3);
- link->bios_forced_drive_settings.PRE_EMPHASIS =
- ((info.ext_disp_conn_info.fixdpvoltageswing >> 2) & 0x3);
+ break;
}
-
- break;
}
}
-
if (bios->funcs->get_atom_dc_golden_table)
bios->funcs->get_atom_dc_golden_table(bios);
--
2.34.1
More information about the amd-gfx
mailing list