[PATCH v2] drm/amd/display: Refactor construct_phy function in dc/link/link_factory.c
Srinivasan Shanmugam
srinivasan.shanmugam at amd.com
Mon Apr 29 08:30:38 UTC 2024
This commit refactors the construct_phy function. The original function
was large and complex.
The following functions were created:
- initialize_link: Handles the initial setup of the link object.
- handle_connector_type: Sets the connector_signal and irq_source_hpd_rx
based on the link_id.id.
- initialize_ddc_service: Initializes the ddc_service for the link.
- initialize_link_encoder: Initializes the link_encoder for the link.
Additionally, the error handling code that was originally in
construct_phy has been moved to the new functions. Each function now
returns a boolean value indicating whether the operation was successful.
If an error occurs, the construct_phy function jumps to the appropriate
label for error handling.
This refactoring reduces the size of the stack frame for each individual
function, fixes about the frame size being larger than 1024 bytes.
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: 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>
---
v2:
- "The handle_connector_type function is called within the
construct_phy function. If it encounters an unsupported connector
type, it returns false, maintained same as original logic before
refactoring"
.../drm/amd/display/dc/link/link_factory.c | 223 ++++++++++--------
1 file changed, 124 insertions(+), 99 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 cf22b8f28ba6..4ed8e170ebc5 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_factory.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_factory.c
@@ -448,73 +448,74 @@ static enum channel_id get_ddc_line(struct dc_link *link)
return channel;
}
-static bool construct_phy(struct dc_link *link,
- const struct link_init_data *init_params)
+static bool initialize_link_encoder(struct dc_link *link,
+ struct dc_context *dc_ctx,
+ const struct dc_vbios_funcs *bp_funcs)
{
- uint8_t i;
- struct ddc_service_init_data ddc_service_init_data = { 0 };
- 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 };
- DC_LOGGER_INIT(dc_ctx->logger);
+ enc_init_data.ctx = dc_ctx;
+ bp_funcs->get_src_obj(dc_ctx->dc_bios, link->link_id, 0, &enc_init_data.encoder);
+ enc_init_data.connector = link->link_id;
+ enc_init_data.channel = get_ddc_line(link);
+ enc_init_data.hpd_source = get_hpd_line(link);
- link->irq_source_hpd = DC_IRQ_SOURCE_INVALID;
- link->irq_source_hpd_rx = DC_IRQ_SOURCE_INVALID;
- link->link_status.dpcd_caps = &link->dpcd_caps;
+ link->hpd_src = enc_init_data.hpd_source;
- link->dc = init_params->dc;
- link->ctx = dc_ctx;
- link->link_index = init_params->link_index;
+ enc_init_data.transmitter = translate_encoder_to_transmitter(enc_init_data.encoder);
+ link->link_enc = link->dc->res_pool->funcs->link_enc_create(dc_ctx, &enc_init_data);
- memset(&link->preferred_training_settings, 0,
- sizeof(struct dc_link_training_overrides));
- memset(&link->preferred_link_setting, 0,
- sizeof(struct dc_link_settings));
-
- link->link_id =
- bios->funcs->get_connector_id(bios, init_params->connector_index);
+ DC_LOG_DC("BIOS object table - DP_IS_USB_C: %d",
+ link->link_enc->features.flags.bits.DP_IS_USB_C);
+ DC_LOG_DC("BIOS object table - IS_DP2_CAPABLE: %d",
+ link->link_enc->features.flags.bits.IS_DP2_CAPABLE);
- link->ep_type = DISPLAY_ENDPOINT_PHY;
+ if (!link->link_enc) {
+ DC_ERROR("Failed to create link encoder!\n");
+ return false;
+ }
- DC_LOG_DC("BIOS object table - link_id: %d", link->link_id.id);
+ /* Update link encoder tracking variables. These are used for the dynamic
+ * assignment of link encoders to streams.
+ */
+ link->eng_id = link->link_enc->preferred_engine;
+ link->dc->res_pool->link_encoders[link->eng_id - ENGINE_ID_DIGA] = link->link_enc;
+ link->dc->res_pool->dig_link_enc_count++;
- if (bios->funcs->get_disp_connector_caps_info) {
- bios->funcs->get_disp_connector_caps_info(bios, link->link_id, &disp_connect_caps_info);
- link->is_internal_display = disp_connect_caps_info.INTERNAL_DISPLAY;
- DC_LOG_DC("BIOS object table - is_internal_display: %d", link->is_internal_display);
- }
+ link->link_enc_hw_inst = link->link_enc->transmitter;
- if (link->link_id.type != OBJECT_TYPE_CONNECTOR) {
- dm_output_to_console("%s: Invalid Connector ObjectID from Adapter Service for connector index:%d! type %d expected %d\n",
- __func__, init_params->connector_index,
- link->link_id.type, OBJECT_TYPE_CONNECTOR);
- goto create_fail;
- }
+ return true;
+}
- if (link->dc->res_pool->funcs->link_init)
- link->dc->res_pool->funcs->link_init(link);
+static bool initialize_ddc_service(struct dc_link *link, struct dc_context *dc_ctx)
+{
+ struct ddc_service_init_data ddc_service_init_data = { 0 };
- link->hpd_gpio = link_get_hpd_gpio(link->ctx->dc_bios, link->link_id,
- link->ctx->gpio_service);
+ ddc_service_init_data.ctx = link->ctx;
+ ddc_service_init_data.id = link->link_id;
+ ddc_service_init_data.link = link;
+ link->ddc = link_create_ddc_service(&ddc_service_init_data);
- if (link->hpd_gpio) {
- dal_gpio_open(link->hpd_gpio, GPIO_MODE_INTERRUPT);
- dal_gpio_unlock_pin(link->hpd_gpio);
- link->irq_source_hpd = dal_irq_get_source(link->hpd_gpio);
+ if (!link->ddc) {
+ DC_ERROR("Failed to create ddc_service!\n");
+ return false;
+ }
- DC_LOG_DC("BIOS object table - hpd_gpio id: %d", link->hpd_gpio->id);
- DC_LOG_DC("BIOS object table - hpd_gpio en: %d", link->hpd_gpio->en);
+ if (!link->ddc->ddc_pin) {
+ DC_ERROR("Failed to get I2C info for connector!\n");
+ return false;
}
+ link->ddc_hw_inst = dal_ddc_get_line(get_ddc_pin(link->ddc));
+
+ return true;
+}
+
+static bool handle_connector_type(struct dc_link *link, struct dc_context *dc_ctx)
+{
switch (link->link_id.id) {
case CONNECTOR_ID_HDMI_TYPE_A:
link->connector_signal = SIGNAL_TYPE_HDMI_TYPE_A;
-
break;
case CONNECTOR_ID_SINGLE_LINK_DVID:
case CONNECTOR_ID_SINGLE_LINK_DVII:
@@ -527,23 +528,18 @@ static bool construct_phy(struct dc_link *link,
case CONNECTOR_ID_DISPLAY_PORT:
case CONNECTOR_ID_USBC:
link->connector_signal = SIGNAL_TYPE_DISPLAY_PORT;
-
if (link->hpd_gpio)
- link->irq_source_hpd_rx =
- dal_irq_get_rx_source(link->hpd_gpio);
-
+ link->irq_source_hpd_rx = dal_irq_get_rx_source(link->hpd_gpio);
break;
case CONNECTOR_ID_EDP:
link->connector_signal = SIGNAL_TYPE_EDP;
-
if (link->hpd_gpio) {
if (!link->dc->config.allow_edp_hotplug_detection)
link->irq_source_hpd = DC_IRQ_SOURCE_INVALID;
-
switch (link->dc->config.allow_edp_hotplug_detection) {
case HPD_EN_FOR_ALL_EDP:
link->irq_source_hpd_rx =
- dal_irq_get_rx_source(link->hpd_gpio);
+ dal_irq_get_rx_source(link->hpd_gpio);
break;
case HPD_EN_FOR_PRIMARY_EDP_ONLY:
if (link->link_index == 0)
@@ -564,69 +560,99 @@ static bool construct_phy(struct dc_link *link,
break;
}
}
-
break;
case CONNECTOR_ID_LVDS:
link->connector_signal = SIGNAL_TYPE_LVDS;
break;
default:
- DC_LOG_WARNING("Unsupported Connector type:%d!\n",
- link->link_id.id);
- goto create_fail;
+ DC_LOG_WARNING("Unsupported Connector type:%d!\n", link->link_id.id);
+ return false;
}
+ return true;
+}
- LINK_INFO("Connector[%d] description: signal: %s\n",
- init_params->connector_index,
- signal_type_to_string(link->connector_signal));
+static void initialize_link(struct dc_link *link, const struct link_init_data *init_params)
+{
+ struct dc_context *dc_ctx = init_params->ctx;
+ struct dc_bios *bios = init_params->dc->ctx->dc_bios;
- ddc_service_init_data.ctx = link->ctx;
- ddc_service_init_data.id = link->link_id;
- ddc_service_init_data.link = link;
- link->ddc = link_create_ddc_service(&ddc_service_init_data);
+ DC_LOGGER_INIT(dc_ctx->logger);
- if (!link->ddc) {
- DC_ERROR("Failed to create ddc_service!\n");
- goto ddc_create_fail;
+ link->irq_source_hpd = DC_IRQ_SOURCE_INVALID;
+ link->irq_source_hpd_rx = DC_IRQ_SOURCE_INVALID;
+ link->link_status.dpcd_caps = &link->dpcd_caps;
+
+ link->dc = init_params->dc;
+ link->ctx = dc_ctx;
+ link->link_index = init_params->link_index;
+
+ memset(&link->preferred_training_settings, 0,
+ sizeof(struct dc_link_training_overrides));
+ memset(&link->preferred_link_setting, 0,
+ sizeof(struct dc_link_settings));
+
+ link->link_id =
+ bios->funcs->get_connector_id(bios, init_params->connector_index);
+
+ link->ep_type = DISPLAY_ENDPOINT_PHY;
+
+ DC_LOG_DC("BIOS object table - link_id: %d", link->link_id.id);
+}
+
+static bool construct_phy(struct dc_link *link,
+ const struct link_init_data *init_params)
+{
+ u8 i;
+ struct dc_context *dc_ctx = init_params->ctx;
+ 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 };
+
+ initialize_link(link, init_params);
+
+ if (bios->funcs->get_disp_connector_caps_info) {
+ bios->funcs->get_disp_connector_caps_info(bios,
+ link->link_id, &disp_connect_caps_info);
+ link->is_internal_display = disp_connect_caps_info.INTERNAL_DISPLAY;
+ DC_LOG_DC("BIOS object table - is_internal_display: %d", link->is_internal_display);
}
- if (!link->ddc->ddc_pin) {
- DC_ERROR("Failed to get I2C info for connector!\n");
- goto ddc_create_fail;
+ if (link->link_id.type != OBJECT_TYPE_CONNECTOR) {
+ dm_output_to_console("%s: Invalid Connector ObjectID from Adapter Service for connector index:%d! type %d expected %d\n",
+ __func__, init_params->connector_index,
+ link->link_id.type, OBJECT_TYPE_CONNECTOR);
+ goto create_fail;
}
- link->ddc_hw_inst =
- dal_ddc_get_line(get_ddc_pin(link->ddc));
+ if (link->dc->res_pool->funcs->link_init)
+ link->dc->res_pool->funcs->link_init(link);
- enc_init_data.ctx = dc_ctx;
- bp_funcs->get_src_obj(dc_ctx->dc_bios, link->link_id, 0,
- &enc_init_data.encoder);
- enc_init_data.connector = link->link_id;
- enc_init_data.channel = get_ddc_line(link);
- enc_init_data.hpd_source = get_hpd_line(link);
+ link->hpd_gpio = link_get_hpd_gpio(link->ctx->dc_bios, link->link_id,
+ link->ctx->gpio_service);
- link->hpd_src = enc_init_data.hpd_source;
+ if (link->hpd_gpio) {
+ dal_gpio_open(link->hpd_gpio, GPIO_MODE_INTERRUPT);
+ dal_gpio_unlock_pin(link->hpd_gpio);
+ link->irq_source_hpd = dal_irq_get_source(link->hpd_gpio);
- enc_init_data.transmitter =
- translate_encoder_to_transmitter(enc_init_data.encoder);
- link->link_enc =
- link->dc->res_pool->funcs->link_enc_create(dc_ctx, &enc_init_data);
+ DC_LOG_DC("BIOS object table - hpd_gpio id: %d", link->hpd_gpio->id);
+ DC_LOG_DC("BIOS object table - hpd_gpio en: %d", link->hpd_gpio->en);
+ }
- DC_LOG_DC("BIOS object table - DP_IS_USB_C: %d", link->link_enc->features.flags.bits.DP_IS_USB_C);
- DC_LOG_DC("BIOS object table - IS_DP2_CAPABLE: %d", link->link_enc->features.flags.bits.IS_DP2_CAPABLE);
+ if (!handle_connector_type(link, dc_ctx))
+ goto create_fail;
- if (!link->link_enc) {
- DC_ERROR("Failed to create link encoder!\n");
- goto link_enc_create_fail;
- }
+ LINK_INFO("Connector[%d] description: signal: %s\n",
+ init_params->connector_index,
+ signal_type_to_string(link->connector_signal));
- /* Update link encoder tracking variables. These are used for the dynamic
- * assignment of link encoders to streams.
- */
- link->eng_id = link->link_enc->preferred_engine;
- link->dc->res_pool->link_encoders[link->eng_id - ENGINE_ID_DIGA] = link->link_enc;
- link->dc->res_pool->dig_link_enc_count++;
+ if (!initialize_ddc_service(link, dc_ctx))
+ goto create_fail;
- link->link_enc_hw_inst = link->link_enc->transmitter;
+ if (!initialize_link_encoder(link, dc_ctx, bp_funcs))
+ goto link_enc_create_fail;
if (link->dc->res_pool->funcs->panel_cntl_create &&
(link->link_id.id == CONNECTOR_ID_EDP ||
@@ -730,7 +756,6 @@ static bool construct_phy(struct dc_link *link,
link->panel_cntl->funcs->destroy(&link->panel_cntl);
panel_cntl_create_fail:
link_destroy_ddc_service(&link->ddc);
-ddc_create_fail:
create_fail:
if (link->hpd_gpio) {
--
2.34.1
More information about the amd-gfx
mailing list