From: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
To: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>,
Aurabindo Pillai <aurabindo.pillai@amd.com>
Cc: <amd-gfx@lists.freedesktop.org>,
Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>,
Wenjing Liu <wenjing.liu@amd.com>,
"Qingqing Zhuo" <qingqing.zhuo@amd.com>,
Tom Chung <chiahsuan.chung@amd.com>,
Alvin Lee <alvin.lee2@amd.com>, Roman Li <roman.li@amd.com>,
Hersen Wu <hersenxs.wu@amd.com>, Alex Hung <alex.hung@amd.com>,
Harry Wentland <harry.wentland@amd.com>
Subject: [PATCH] drm/amd/display: Refactor construct_phy function in dc/link/link_factory.c
Date: Mon, 29 Apr 2024 12:24:51 +0530 [thread overview]
Message-ID: <20240429065451.180745-1-srinivasan.shanmugam@amd.com> (raw)
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@amd.com>
Cc: Qingqing Zhuo <qingqing.zhuo@amd.com>
Cc: Tom Chung <chiahsuan.chung@amd.com>
Cc: Alvin Lee <alvin.lee2@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Roman Li <roman.li@amd.com>
Cc: Hersen Wu <hersenxs.wu@amd.com>
Cc: Alex Hung <alex.hung@amd.com>
Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
.../drm/amd/display/dc/link/link_factory.c | 221 ++++++++++--------
1 file changed, 122 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..af373824db8c 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 void 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,97 @@ 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);
+ break;
}
+}
- 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);
+ handle_connector_type(link, dc_ctx);
- 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 +754,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
next reply other threads:[~2024-04-29 6:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 6:54 Srinivasan Shanmugam [this message]
2024-04-29 8:30 ` [PATCH v2] drm/amd/display: Refactor construct_phy function in dc/link/link_factory.c Srinivasan Shanmugam
2024-05-10 10:12 ` [PATCH v3] " Srinivasan Shanmugam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240429065451.180745-1-srinivasan.shanmugam@amd.com \
--to=srinivasan.shanmugam@amd.com \
--cc=Rodrigo.Siqueira@amd.com \
--cc=alex.hung@amd.com \
--cc=alvin.lee2@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=aurabindo.pillai@amd.com \
--cc=chiahsuan.chung@amd.com \
--cc=harry.wentland@amd.com \
--cc=hersenxs.wu@amd.com \
--cc=qingqing.zhuo@amd.com \
--cc=roman.li@amd.com \
--cc=wenjing.liu@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.