[PATCH 24/70] drm/amd/display: Split DPMS ON into parts

Wayne Lin <[email protected]>
Newsgroups org.freedesktop.lists.amd-gfx
Message-ID <[email protected]>
From: Dominik Kaszewski <[email protected]>

[Why]
Ongoing optimization efforts require splitting DPMS ON around
enable_link, in order to enable running multiple sequences
in parallel.

[How]
* Split link_set_dpms_on across enable_link
* Add return value indicating whether the programming sequence
succeeded and/or run to the end.

Reviewed-by: Alvin Lee <[email protected]>
Signed-off-by: Dominik Kaszewski <[email protected]>
Signed-off-by: Wayne Lin <[email protected]>
---
 .../gpu/drm/amd/display/dc/inc/core_status.h  |  10 +
 .../gpu/drm/amd/display/dc/inc/link_service.h |   4 +-
 .../gpu/drm/amd/display/dc/link/link_dpms.c   | 184 ++++++++++++++----
 .../gpu/drm/amd/display/dc/link/link_dpms.h   |   4 +-
 4 files changed, 156 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_status.h b/drivers/gpu/drm/amd/display/dc/inc/core_status.h
index 388f801f4582..1a17e727ed04 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/core_status.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/core_status.h
@@ -62,6 +62,16 @@ enum dc_status {
 	DC_FAIL_DP_LINK_BANDWIDTH = 28,
 	DC_FAIL_HW_CURSOR_SUPPORT = 29,
 	DC_FAIL_DP_TUNNEL_BW_VALIDATE = 30,
+
+	/// Link protocol handshake and DPMS hardware programming successful.
+	DC_DPMS_SUCCESS = DC_OK,
+	/// Handshake skipped by optimized path, programming successfully completed.
+	DC_DPMS_SKIPPED_HANDSHAKE = 31,
+	/// Handshake failed, programming successful, DCN in consistent state.
+	DC_DPMS_FAILED_HANDSHAKE = 32,
+	/// Handshake failed, programming aborted, DCN may be in inconsistent state.
+	DC_DPMS_FAILED_INCOMPLETE = 33,
+
 	DC_ERROR_UNEXPECTED = -1
 };
 
diff --git a/drivers/gpu/drm/amd/display/dc/inc/link_service.h b/drivers/gpu/drm/amd/display/dc/inc/link_service.h
index addeb3e3b25a..026d28046eea 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/link_service.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/link_service.h
@@ -155,8 +155,8 @@ struct link_service {
 
 
 	/*************************** DPMS *************************************/
-	void (*set_dpms_on)(struct dc_state *state, struct pipe_ctx *pipe_ctx);
-	void (*set_dpms_off)(struct pipe_ctx *pipe_ctx);
+	enum dc_status (*set_dpms_on)(struct dc_state *state, struct pipe_ctx *pipe_ctx);
+	enum dc_status (*set_dpms_off)(struct pipe_ctx *pipe_ctx);
 	void (*resume)(struct dc_link *link);
 	void (*blank_all_dp_displays)(struct dc *dc);
 	void (*blank_all_edp_displays)(struct dc *dc);
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
index 335ae952ef60..949c669eb259 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
@@ -528,19 +528,27 @@ static bool write_i2c_redriver_setting(
 	return success;
 }
 
+static struct link_encoder *get_link_encoder(struct pipe_ctx *pipe_ctx)
+{
+	struct link_encoder *link_enc
+			= pipe_ctx->stream->ctx->dc->config.unify_link_enc_assignment
+			? pipe_ctx->link_res.dio_link_enc
+			: link_enc_cfg_get_link_enc(pipe_ctx->stream->link);
+
+	ASSERT(link_enc);
+	return link_enc;
+}
+
 static void update_psp_stream_config(struct pipe_ctx *pipe_ctx, bool dpms_off)
 {
 	struct cp_psp *cp_psp = &pipe_ctx->stream->ctx->cp_psp;
-	struct link_encoder *link_enc = pipe_ctx->link_res.dio_link_enc;
+	struct link_encoder *link_enc = get_link_encoder(pipe_ctx);
 	struct cp_psp_stream_config config = {0};
 	enum dp_panel_mode panel_mode =
 			dp_get_panel_mode(pipe_ctx->stream->link);
 
 	if (cp_psp == NULL || cp_psp->funcs.update_stream_config == NULL)
 		return;
-	if (!pipe_ctx->stream->ctx->dc->config.unify_link_enc_assignment)
-		link_enc = link_enc_cfg_get_link_enc(pipe_ctx->stream->link);
-	ASSERT(link_enc);
 	if (link_enc == NULL)
 		return;
 
@@ -2375,7 +2383,7 @@ static struct vpg *get_vpg(struct pipe_ctx *pipe_ctx)
 		return pipe_ctx->stream_res.stream_enc->vpg;
 }
 
-void link_set_dpms_off(struct pipe_ctx *pipe_ctx)
+enum dc_status link_set_dpms_off(struct pipe_ctx *pipe_ctx)
 {
 	DC_LOGGER_INIT(pipe_ctx->stream->ctx->logger);
 	struct dc_stream_state *stream = pipe_ctx->stream;
@@ -2387,7 +2395,8 @@ void link_set_dpms_off(struct pipe_ctx *pipe_ctx)
 	ASSERT(is_master_pipe_for_link(link, pipe_ctx));
 
 	if (dc_is_virtual_signal(stream->signal))
-		return;
+		/* No real hardware to program for virtual signals. */
+		return DC_DPMS_SKIPPED_HANDSHAKE;
 
 	if (stream->sink) {
 		if (stream->sink->sink_signal != SIGNAL_TYPE_VIRTUAL &&
@@ -2481,18 +2490,23 @@ void link_set_dpms_off(struct pipe_ctx *pipe_ctx)
 		/* since current psp not loaded, we need to reset it to default */
 		link->panel_mode = panel_mode;
 	}
+
+	return DC_DPMS_SUCCESS;
 }
 
-void link_set_dpms_on(
+static enum dc_status link_set_dpms_on_pre_enable_link(
 		struct dc_state *state,
-		struct pipe_ctx *pipe_ctx)
+		struct pipe_ctx *pipe_ctx
+)
 {
+	// Used conditionally in ifdef'ed diagnostic builds
+	(void) state;
+
 	DC_LOGGER_INIT(pipe_ctx->stream->ctx->logger);
 	struct dc_stream_state *stream = pipe_ctx->stream;
 	struct dc *dc = stream->ctx->dc;
 	struct dc_link *link = stream->link;
-	enum dc_status status;
-	struct link_encoder *link_enc = pipe_ctx->link_res.dio_link_enc;
+
 	enum otg_out_mux_dest otg_out_dest = OUT_MUX_DIO;
 	struct vpg *vpg = get_vpg(pipe_ctx);
 	const struct link_hwss *link_hwss = get_link_hwss(link, &pipe_ctx->link_res);
@@ -2502,7 +2516,8 @@ void link_set_dpms_on(
 	ASSERT(is_master_pipe_for_link(link, pipe_ctx));
 
 	if (dc_is_virtual_signal(stream->signal))
-		return;
+		/* No real hardware to program for virtual signals. */
+		return DC_DPMS_SKIPPED_HANDSHAKE;
 
 	if (stream->sink) {
 		if (stream->sink->sink_signal != SIGNAL_TYPE_VIRTUAL &&
@@ -2516,13 +2531,12 @@ void link_set_dpms_on(
 	}
 
 	link_wait_for_unlocked(link);
-	if (!dc->config.unify_link_enc_assignment)
-		link_enc = link_enc_cfg_get_link_enc(link);
-	ASSERT(link_enc);
 
 	if (!dc_is_virtual_signal(stream->signal)
 			&& !dc_is_hdmi_frl_signal(stream->signal)
 			&& !dp_is_128b_132b_signal(pipe_ctx)) {
+		struct link_encoder *link_enc = get_link_encoder(pipe_ctx);
+
 		if (link_enc)
 			link_enc->funcs->setup(
 				link_enc,
@@ -2569,7 +2583,9 @@ void link_set_dpms_on(
 		}
 
 		update_psp_stream_config(pipe_ctx, false);
-		return;
+
+		/* Seamless boot: hardware already enabled by BIOS; skip link training. */
+		return DC_DPMS_SKIPPED_HANDSHAKE;
 	}
 
 	/* eDP lit up by bios already, no need to enable again. */
@@ -2587,11 +2603,16 @@ void link_set_dpms_on(
 			msleep(post_oui_delay);
 		}
 
-		return;
+		/* eDP already lit by BIOS; skip standard enable steps. */
+		return DC_DPMS_SKIPPED_HANDSHAKE;
 	}
 
 	if (stream->dpms_off)
-		return;
+		/*
+		 * Stream is configured as DPMS-off; skip link enable.
+		 * Hardware will NOT be in a fully enabled state after this early exit.
+		 */
+		return DC_DPMS_FAILED_INCOMPLETE;
 
 	/* For Dp tunneling link, a pending HPD means that we have a race condition between processing
 	 * current link and processing the pending HPD. If we enable the link now, we may end up with a
@@ -2599,7 +2620,11 @@ void link_set_dpms_on(
 	 */
 	if (link->ep_type == DISPLAY_ENDPOINT_USB4_DPIA && link->is_hpd_pending) {
 		DC_LOG_DEBUG("%s, Link%d HPD is pending, not enable it.\n", __func__, link->link_index);
-		return;
+		/*
+		 * Pending HPD on USB4 DPIA link: skip enable to avoid race condition.
+		 * Hardware will NOT be in a fully enabled state after this early exit.
+		 */
+		return DC_DPMS_FAILED_INCOMPLETE;
 	}
 
 	/* Have to setup DSC before DIG FE and BE are connected (which happens before the
@@ -2617,30 +2642,61 @@ void link_set_dpms_on(
 	if (link->replay_settings.config.replay_supported && !dc_is_embedded_signal(link->connector_signal))
 		dp_setup_replay(link, stream);
 
-	// TODO: Split DPMS-on into 3 functions at this point
-	status = enable_link(state, pipe_ctx);
+	return DC_DPMS_SUCCESS;
+}
+
+static enum dc_status link_set_dpms_on_enable_link(
+		struct dc_state *state,
+		struct pipe_ctx *pipe_ctx
+)
+{
+	DC_LOGGER_INIT(pipe_ctx->stream->ctx->logger);
+	struct dc_stream_state *stream = pipe_ctx->stream;
+	struct dc_link *link = stream->link;
+	const enum dc_status status = enable_link(state, pipe_ctx);
+
+	if (status == DC_OK)
+		return DC_DPMS_SUCCESS;
 
-	if (status != DC_OK) {
-		DC_LOG_WARNING("enabling link %u failed: %d\n",
-		link->link_index,
-		status);
+	DC_LOG_WARNING("enabling link %u failed: %d\n", link->link_index, status);
 
-		/* Abort stream enable *unless* the failure was due to
-		 * DP link training - some DP monitors will recover and
-		 * show the stream anyway. But MST displays can't proceed
-		 * without link training.
-		 */
-		if ((status != DC_FAIL_DP_LINK_TRAINING &&
-				status != DC_FAIL_HDMI_FRL_LINK_TRAINING) ||
-				stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST) {
-			if (false == link->link_status.link_active)
-				disable_link(link, &pipe_ctx->link_res,
-						stream->signal);
-			BREAK_TO_DEBUGGER();
-			return;
-		}
+	/* Abort stream enable *unless* the failure was due to
+	 * DP link training - some DP monitors will recover and
+	 * show the stream anyway. But MST displays can't proceed
+	 * without link training.
+	 */
+	switch (status) {
+	case DC_FAIL_DP_LINK_TRAINING:
+	case DC_FAIL_HDMI_FRL_LINK_TRAINING:
+		if (stream->signal != SIGNAL_TYPE_DISPLAY_PORT_MST)
+			return DC_DPMS_SUCCESS;
+		break;
+
+	default:
+		break;
 	}
-	// TODO: Split DPMS-on into 3 functions at this point
+
+	if (!link->link_status.link_active)
+		disable_link(link, &pipe_ctx->link_res, stream->signal);
+
+	/*
+	 * Link enable failed; do NOT set skip_remaining so that post_enable_link
+	 * still runs and leaves hardware in a consistent state.
+	 */
+	return DC_DPMS_FAILED_HANDSHAKE;
+}
+
+static enum dc_status link_set_dpms_on_post_enable_link(
+		struct dc_state *state,
+		struct pipe_ctx *pipe_ctx
+)
+{
+	// Used conditionally in ifdef'ed diagnostic builds
+	(void) state;
+
+	struct dc_stream_state *stream = pipe_ctx->stream;
+	struct dc_link *link = stream->link;
+	struct hw_sequencer_funcs *hwss = &stream->ctx->dc->hwss;
 
 	if (stream->timing.flags.DSC && dc_is_hdmi_frl_signal(stream->signal))
 		//TODO: bring HDMI FRL in line with DP
@@ -2659,13 +2715,15 @@ void link_set_dpms_on(
 	if (!(dc_is_virtual_signal(stream->signal) ||
 			dc_is_hdmi_frl_signal(stream->signal) ||
 			dp_is_128b_132b_signal(pipe_ctx))) {
+		struct link_encoder *link_enc = get_link_encoder(pipe_ctx);
+
 		if (link_enc)
 			link_enc->funcs->setup(
 					link_enc,
 					stream->signal);
 	}
 
-	dc->hwss.enable_stream(pipe_ctx);
+	hwss->enable_stream(pipe_ctx);
 
 	/* Set DPS PPS SDP (AKA "info frames") */
 	if (stream->timing.flags.DSC) {
@@ -2697,7 +2755,7 @@ void link_set_dpms_on(
 			link->is_display_mux_present)
 		msleep(20);
 
-	dc->hwss.unblank_stream(pipe_ctx,
+	hwss->unblank_stream(pipe_ctx,
 		&link->cur_link_settings);
 
 	if (stream->sink_patches.delay_ignore_msa > 0)
@@ -2707,8 +2765,50 @@ void link_set_dpms_on(
 		enable_stream_features(pipe_ctx);
 	update_psp_stream_config(pipe_ctx, false);
 
-	dc->hwss.enable_audio_stream(pipe_ctx);
+	hwss->enable_audio_stream(pipe_ctx);
 
 	if (dc_is_hdmi_signal(stream->signal))
 		set_avmute(pipe_ctx, false);
+
+	return DC_DPMS_SUCCESS;
+}
+
+enum dc_status link_set_dpms_on(
+		struct dc_state *state,
+		struct pipe_ctx *pipe_ctx
+)
+{
+	enum dc_status result = DC_DPMS_SUCCESS;
+
+	typedef enum dc_status (*step)(struct dc_state *, struct pipe_ctx *);
+	const step steps[] = {
+		link_set_dpms_on_pre_enable_link,
+		link_set_dpms_on_enable_link,
+		link_set_dpms_on_post_enable_link,
+	};
+
+	for (size_t i = 0; i < ARRAY_SIZE(steps); i++) {
+		const enum dc_status step_result = steps[i](state, pipe_ctx);
+
+		switch (step_result) {
+		case DC_DPMS_SUCCESS:
+		case DC_DPMS_FAILED_HANDSHAKE:
+			// Enum is ordered from "best" to "worst" results
+			result = max(result, step_result);
+			break;
+
+		case DC_DPMS_SKIPPED_HANDSHAKE:
+			return step_result;
+
+		case DC_DPMS_FAILED_INCOMPLETE:
+			ASSERT(false);
+			return step_result;
+
+		default:
+			ASSERT(false);
+			return step_result;
+		}
+	}
+
+	return result;
 }
diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.h b/drivers/gpu/drm/amd/display/dc/link/link_dpms.h
index e8662147dd8e..6559bc04d90e 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.h
+++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.h
@@ -27,10 +27,10 @@
 #define __DC_LINK_DPMS_H__
 
 #include "link_service.h"
-void link_set_dpms_on(
+enum dc_status link_set_dpms_on(
 		struct dc_state *state,
 		struct pipe_ctx *pipe_ctx);
-void link_set_dpms_off(struct pipe_ctx *pipe_ctx);
+enum dc_status link_set_dpms_off(struct pipe_ctx *pipe_ctx);
 void link_resume(struct dc_link *link);
 void link_blank_all_dp_displays(struct dc *dc);
 void link_blank_all_edp_displays(struct dc *dc);
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.