[PATCH 0/5] drm/amd/display: DC analog connector fixes

[PATCH 0/5] drm/amd/display: DC analog connector fixes

[Timur Kristóf]

Fix a few issues with the initial DC analog connector
implementation that I've noticed since. I highly recommend
to backport this series to kernel versions that include
the DC analog series.

Timur Kristóf (5):
  drm/amd/display: Pass proper DAC encoder ID to VBIOS
  drm/amd/display: Correct color depth for SelectCRTC_Source
  drm/amd/display: Add missing encoder setup to DACnEncoderControl
  drm/amd/display: Setup DAC encoder before using it
  drm/amd/display: Don't repeat DAC load detection

 .../gpu/drm/amd/display/dc/bios/bios_parser.c |  4 +-
 .../drm/amd/display/dc/bios/command_table.c   | 44 +++++++++++++++----
 .../drm/amd/display/dc/bios/command_table.h   |  4 +-
 .../drm/amd/display/dc/dce/dce_link_encoder.c |  2 +
 .../amd/display/dc/dce/dce_stream_encoder.c   | 25 ++++++++++-
 .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 28 +-----------
 .../drm/amd/display/dc/inc/hw/link_encoder.h  |  2 +
 .../amd/display/dc/inc/hw/stream_encoder.h    |  4 ++
 .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 ++
 .../drm/amd/display/dc/link/link_detection.c  |  7 ++-
 .../drm/amd/display/dc/link/link_factory.c    | 10 ++---
 .../dc/resource/dce110/dce110_resource.c      |  2 +
 .../amd/display/include/bios_parser_types.h   |  2 +-
 13 files changed, 87 insertions(+), 51 deletions(-)

-- 
2.52.0

[PATCH 1/5] drm/amd/display: Pass proper DAC encoder ID to VBIOS

[Timur Kristóf]

Similarly to the analog_engine field, add a new	analog_id field
which contains the encoder ID of the analog encoder that
corresponds to the link encoder.

Previously, the default encoder ID of the link encoder was used,
which meant that we passed the wrong ID in case of DVI-I.

Fixes: 7d63519cd9ca ("drm/amd/display: Add concept of analog encoders (v2)")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c  |  2 ++
 drivers/gpu/drm/amd/display/dc/inc/hw/link_encoder.h   |  2 ++
 drivers/gpu/drm/amd/display/dc/link/link_factory.c     | 10 +++++-----
 .../amd/display/dc/resource/dce110/dce110_resource.c   |  2 ++
 4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c
index 87dbb8d7ed27..5c1a10f77733 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c
@@ -850,6 +850,7 @@ void dce110_link_encoder_construct(
 	enc110->base.funcs = &dce110_lnk_enc_funcs;
 	enc110->base.ctx = init_data->ctx;
 	enc110->base.id = init_data->encoder;
+	enc110->base.analog_id = init_data->analog_encoder;
 
 	enc110->base.hpd_source = init_data->hpd_source;
 	enc110->base.connector = init_data->connector;
@@ -1793,6 +1794,7 @@ void dce60_link_encoder_construct(
 	enc110->base.funcs = &dce60_lnk_enc_funcs;
 	enc110->base.ctx = init_data->ctx;
 	enc110->base.id = init_data->encoder;
+	enc110->base.analog_id = init_data->analog_encoder;
 
 	enc110->base.hpd_source = init_data->hpd_source;
 	enc110->base.connector = init_data->connector;
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/link_encoder.h b/drivers/gpu/drm/amd/display/dc/inc/hw/link_encoder.h
index df512920a9fa..e638325e35ec 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/link_encoder.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/link_encoder.h
@@ -47,6 +47,7 @@ struct encoder_init_data {
 	enum hpd_source_id hpd_source;
 	/* TODO: in DAL2, here was pointer to EventManagerInterface */
 	struct graphics_object_id encoder;
+	struct graphics_object_id analog_encoder;
 	enum engine_id analog_engine;
 	struct dc_context *ctx;
 	enum transmitter transmitter;
@@ -81,6 +82,7 @@ struct link_encoder {
 	int32_t aux_channel_offset;
 	struct dc_context *ctx;
 	struct graphics_object_id id;
+	struct graphics_object_id analog_id;
 	struct graphics_object_id connector;
 	uint32_t output_signals;
 	enum engine_id preferred_engine;
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 a6e2b0821969..23a302d7fbc8 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_factory.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_factory.c
@@ -451,20 +451,19 @@ static enum channel_id get_ddc_line(struct dc_link *link)
 	return channel;
 }
 
-static enum engine_id find_analog_engine(struct dc_link *link)
+static enum engine_id find_analog_engine(struct dc_link *link, struct graphics_object_id *enc)
 {
 	struct dc_bios *bp = link->ctx->dc_bios;
-	struct graphics_object_id encoder = {0};
 	enum bp_result bp_result = BP_RESULT_OK;
 	int i;
 
 	for (i = 0; i < 3; i++) {
-		bp_result = bp->funcs->get_src_obj(bp, link->link_id, i, &encoder);
+		bp_result = bp->funcs->get_src_obj(bp, link->link_id, i, enc);
 
 		if (bp_result != BP_RESULT_OK)
 			return ENGINE_ID_UNKNOWN;
 
-		switch (encoder.id) {
+		switch (enc->id) {
 		case ENCODER_ID_INTERNAL_DAC1:
 		case ENCODER_ID_INTERNAL_KLDSCP_DAC1:
 			return ENGINE_ID_DACA;
@@ -474,6 +473,7 @@ static enum engine_id find_analog_engine(struct dc_link *link)
 		}
 	}
 
+	memset(enc, 0, sizeof(*enc));
 	return ENGINE_ID_UNKNOWN;
 }
 
@@ -527,7 +527,7 @@ static bool construct_phy(struct dc_link *link,
 	 */
 	bp_funcs->get_src_obj(bios, link->link_id, 0, &enc_init_data.encoder);
 	enc_init_data.transmitter = translate_encoder_to_transmitter(enc_init_data.encoder);
-	enc_init_data.analog_engine = find_analog_engine(link);
+	enc_init_data.analog_engine = find_analog_engine(link, &enc_init_data.analog_encoder);
 
 	link->ep_type = DISPLAY_ENDPOINT_PHY;
 
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/resource/dce110/dce110_resource.c
index cd54382c0af3..7c09825cd9bd 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dce110/dce110_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dce110/dce110_resource.c
@@ -895,6 +895,8 @@ static void get_pixel_clock_parameters(
 	 */
 	pixel_clk_params->requested_pix_clk_100hz = stream->timing.pix_clk_100hz;
 	pixel_clk_params->encoder_object_id = stream->link->link_enc->id;
+	if (dc_is_rgb_signal(pipe_ctx->stream->signal))
+		pixel_clk_params->encoder_object_id = stream->link->link_enc->analog_id;
 	pixel_clk_params->signal_type = pipe_ctx->stream->signal;
 	pixel_clk_params->controller_id = pipe_ctx->stream_res.tg->inst + 1;
 	/* TODO: un-hardcode*/
-- 
2.52.0

[PATCH 2/5] drm/amd/display: Correct color depth for SelectCRTC_Source

[Timur Kristóf]

Pass the correct enum values as expected by the VBIOS.
Previously the actual bit depth integer value was passed,
which was a mistake.

Fixes: 523aabc65da0 ("drm/amd/display: Add SelectCRTC_Source to BIOS parser")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 .../drm/amd/display/dc/bios/command_table.c   | 25 ++++++++++++++++-
 .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 28 +------------------
 .../amd/display/include/bios_parser_types.h   |  2 +-
 3 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
index 22457f417e65..d56c0d3763dd 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
@@ -1797,7 +1797,30 @@ static enum bp_result select_crtc_source_v3(
 		&params.ucEncodeMode))
 		return BP_RESULT_BADINPUT;
 
-	params.ucDstBpc = bp_params->bit_depth;
+	switch (bp_params->color_depth) {
+	case COLOR_DEPTH_UNDEFINED:
+		params.ucDstBpc = PANEL_BPC_UNDEFINE;
+		break;
+	case COLOR_DEPTH_666:
+		params.ucDstBpc = PANEL_6BIT_PER_COLOR;
+		break;
+	default:
+	case COLOR_DEPTH_888:
+		params.ucDstBpc = PANEL_8BIT_PER_COLOR;
+		break;
+	case COLOR_DEPTH_101010:
+		params.ucDstBpc = PANEL_10BIT_PER_COLOR;
+		break;
+	case COLOR_DEPTH_121212:
+		params.ucDstBpc = PANEL_12BIT_PER_COLOR;
+		break;
+	case COLOR_DEPTH_141414:
+		dm_error("14-bit color not supported by SelectCRTC_Source v3\n");
+		break;
+	case COLOR_DEPTH_161616:
+		params.ucDstBpc = PANEL_16BIT_PER_COLOR;
+		break;
+	}
 
 	if (EXEC_BIOS_CMD_TABLE(SelectCRTC_Source, params))
 		result = BP_RESULT_OK;
diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
index 94e66d96c403..6f6eacf62b5f 100644
--- a/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/hwss/dce110/dce110_hwseq.c
@@ -1611,38 +1611,12 @@ dce110_select_crtc_source(struct pipe_ctx *pipe_ctx)
 	struct dc_bios *bios = link->ctx->dc_bios;
 	struct bp_crtc_source_select crtc_source_select = {0};
 	enum engine_id engine_id = link->link_enc->preferred_engine;
-	uint8_t bit_depth;
 
 	if (dc_is_rgb_signal(pipe_ctx->stream->signal))
 		engine_id = link->link_enc->analog_engine;
 
-	switch (pipe_ctx->stream->timing.display_color_depth) {
-	case COLOR_DEPTH_UNDEFINED:
-		bit_depth = 0;
-		break;
-	case COLOR_DEPTH_666:
-		bit_depth = 6;
-		break;
-	default:
-	case COLOR_DEPTH_888:
-		bit_depth = 8;
-		break;
-	case COLOR_DEPTH_101010:
-		bit_depth = 10;
-		break;
-	case COLOR_DEPTH_121212:
-		bit_depth = 12;
-		break;
-	case COLOR_DEPTH_141414:
-		bit_depth = 14;
-		break;
-	case COLOR_DEPTH_161616:
-		bit_depth = 16;
-		break;
-	}
-
 	crtc_source_select.controller_id = CONTROLLER_ID_D0 + pipe_ctx->stream_res.tg->inst;
-	crtc_source_select.bit_depth = bit_depth;
+	crtc_source_select.color_depth = pipe_ctx->stream->timing.display_color_depth;
 	crtc_source_select.engine_id = engine_id;
 	crtc_source_select.sink_signal = pipe_ctx->stream->signal;
 
diff --git a/drivers/gpu/drm/amd/display/include/bios_parser_types.h b/drivers/gpu/drm/amd/display/include/bios_parser_types.h
index 973b6bdbac63..f40dc612ec73 100644
--- a/drivers/gpu/drm/amd/display/include/bios_parser_types.h
+++ b/drivers/gpu/drm/amd/display/include/bios_parser_types.h
@@ -136,7 +136,7 @@ struct bp_crtc_source_select {
 	enum engine_id engine_id;
 	enum controller_id controller_id;
 	enum signal_type sink_signal;
-	uint8_t bit_depth;
+	enum dc_color_depth color_depth;
 };
 
 struct bp_transmitter_control {
-- 
2.52.0

[PATCH 3/5] drm/amd/display: Add missing encoder setup to DACnEncoderControl

[Timur Kristóf]

Apparently the DAC encoder needs to be set up before use.
The BIOS parser in DC did not support this so I assumed it was
not necessary, but the DAC doesn't work without it on some GPUs.

Fixes: 6c51114c17d6 ("drm/amd/display: Hook up DAC to bios_parser_encoder_control")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 .../gpu/drm/amd/display/dc/bios/bios_parser.c |  4 ++--
 .../drm/amd/display/dc/bios/command_table.c   | 19 +++++++++++--------
 .../drm/amd/display/dc/bios/command_table.h   |  4 ++--
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
index 4120d6c4c5e4..7c49a4d59cd9 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
@@ -763,14 +763,14 @@ static enum bp_result bios_parser_encoder_control(
 			return BP_RESULT_FAILURE;
 
 		return bp->cmd_tbl.dac1_encoder_control(
-			bp, cntl->action == ENCODER_CONTROL_ENABLE,
+			bp, cntl->action,
 			cntl->pixel_clock, ATOM_DAC1_PS2);
 	} else if (cntl->engine_id == ENGINE_ID_DACB) {
 		if (!bp->cmd_tbl.dac2_encoder_control)
 			return BP_RESULT_FAILURE;
 
 		return bp->cmd_tbl.dac2_encoder_control(
-			bp, cntl->action == ENCODER_CONTROL_ENABLE,
+			bp, cntl->action,
 			cntl->pixel_clock, ATOM_DAC1_PS2);
 	}
 
diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
index d56c0d3763dd..76a3559f0ddc 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
@@ -1838,12 +1838,12 @@ static enum bp_result select_crtc_source_v3(
 
 static enum bp_result dac1_encoder_control_v1(
 	struct bios_parser *bp,
-	bool enable,
+	enum bp_encoder_control_action action,
 	uint32_t pixel_clock,
 	uint8_t dac_standard);
 static enum bp_result dac2_encoder_control_v1(
 	struct bios_parser *bp,
-	bool enable,
+	enum bp_encoder_control_action action,
 	uint32_t pixel_clock,
 	uint8_t dac_standard);
 
@@ -1869,12 +1869,15 @@ static void init_dac_encoder_control(struct bios_parser *bp)
 
 static void dac_encoder_control_prepare_params(
 	DAC_ENCODER_CONTROL_PS_ALLOCATION *params,
-	bool enable,
+	enum bp_encoder_control_action action,
 	uint32_t pixel_clock,
 	uint8_t dac_standard)
 {
 	params->ucDacStandard = dac_standard;
-	if (enable)
+	if (action == ENCODER_CONTROL_SETUP ||
+	    action == ENCODER_CONTROL_INIT)
+		params->ucAction = ATOM_ENCODER_INIT;
+	else if (action == ENCODER_CONTROL_ENABLE)
 		params->ucAction = ATOM_ENABLE;
 	else
 		params->ucAction = ATOM_DISABLE;
@@ -1887,7 +1890,7 @@ static void dac_encoder_control_prepare_params(
 
 static enum bp_result dac1_encoder_control_v1(
 	struct bios_parser *bp,
-	bool enable,
+	enum bp_encoder_control_action action,
 	uint32_t pixel_clock,
 	uint8_t dac_standard)
 {
@@ -1896,7 +1899,7 @@ static enum bp_result dac1_encoder_control_v1(
 
 	dac_encoder_control_prepare_params(
 		&params,
-		enable,
+		action,
 		pixel_clock,
 		dac_standard);
 
@@ -1908,7 +1911,7 @@ static enum bp_result dac1_encoder_control_v1(
 
 static enum bp_result dac2_encoder_control_v1(
 	struct bios_parser *bp,
-	bool enable,
+	enum bp_encoder_control_action action,
 	uint32_t pixel_clock,
 	uint8_t dac_standard)
 {
@@ -1917,7 +1920,7 @@ static enum bp_result dac2_encoder_control_v1(
 
 	dac_encoder_control_prepare_params(
 		&params,
-		enable,
+		action,
 		pixel_clock,
 		dac_standard);
 
diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.h b/drivers/gpu/drm/amd/display/dc/bios/command_table.h
index e89b1ba0048b..78bdbcaa61c8 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table.h
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.h
@@ -57,12 +57,12 @@ struct cmd_tbl {
 		struct bp_crtc_source_select *bp_params);
 	enum bp_result (*dac1_encoder_control)(
 		struct bios_parser *bp,
-		bool enable,
+		enum bp_encoder_control_action action,
 		uint32_t pixel_clock,
 		uint8_t dac_standard);
 	enum bp_result (*dac2_encoder_control)(
 		struct bios_parser *bp,
-		bool enable,
+		enum bp_encoder_control_action action,
 		uint32_t pixel_clock,
 		uint8_t dac_standard);
 	enum bp_result (*dac1_output_control)(
-- 
2.52.0

[PATCH 4/5] drm/amd/display: Setup DAC encoder before using it

[Timur Kristóf]

Just like for other signal types, call the encoder control
command in the VBIOS to setup the DAC encoder.
The DAC doesn't work well without this on some GPUs,
for example the Radeon HD 7790 needs this.

Fixes: 7d63519cd9ca ("drm/amd/display: Add concept of analog encoders (v2)")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 .../amd/display/dc/dce/dce_stream_encoder.c   | 25 ++++++++++++++++++-
 .../amd/display/dc/inc/hw/stream_encoder.h    |  4 +++
 .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 +++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
index 574618d5d4a4..1fbf6263c235 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
@@ -693,6 +693,24 @@ static void dce110_stream_encoder_lvds_set_stream_attribute(
 	ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
 }
 
+static void dce110_stream_encoder_analog_set_stream_attribute(
+	struct stream_encoder *enc,
+	struct dc_crtc_timing *crtc_timing)
+{
+	struct dce110_stream_encoder *enc110 = DCE110STRENC_FROM_STRENC(enc);
+	struct bp_encoder_control cntl = {0};
+
+	cntl.action = ENCODER_CONTROL_SETUP;
+	cntl.engine_id = enc110->base.id;
+	cntl.pixel_clock = crtc_timing->pix_clk_100hz / 10;
+
+	if (enc110->base.bp->funcs->encoder_control(
+			enc110->base.bp, &cntl) != BP_RESULT_OK)
+		return;
+
+	ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
+}
+
 static void dce110_stream_encoder_set_throttled_vcp_size(
 	struct stream_encoder *enc,
 	struct fixed31_32 avg_time_slots_per_mtp)
@@ -1521,6 +1539,8 @@ static const struct stream_encoder_funcs dce110_str_enc_funcs = {
 		dce110_stream_encoder_dvi_set_stream_attribute,
 	.lvds_set_stream_attribute =
 		dce110_stream_encoder_lvds_set_stream_attribute,
+	.analog_set_stream_attribute =
+		dce110_stream_encoder_analog_set_stream_attribute,
 	.set_throttled_vcp_size =
 		dce110_stream_encoder_set_throttled_vcp_size,
 	.update_hdmi_info_packets =
@@ -1568,7 +1588,10 @@ void dce110_stream_encoder_construct(
 	enc110->se_mask = se_mask;
 }
 
-static const struct stream_encoder_funcs dce110_an_str_enc_funcs = {};
+static const struct stream_encoder_funcs dce110_an_str_enc_funcs = {
+	.analog_set_stream_attribute =
+		dce110_stream_encoder_analog_set_stream_attribute,
+};
 
 void dce110_analog_stream_encoder_construct(
 	struct dce110_stream_encoder *enc110,
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
index 27f950ae45ee..9658b06f32f7 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
@@ -153,6 +153,10 @@ struct stream_encoder_funcs {
 		struct stream_encoder *enc,
 		struct dc_crtc_timing *crtc_timing);
 
+	void (*analog_set_stream_attribute)(
+		struct stream_encoder *enc,
+		struct dc_crtc_timing *crtc_timing);
+
 	void (*set_throttled_vcp_size)(
 		struct stream_encoder *enc,
 		struct fixed31_32 avg_time_slots_per_mtp);
diff --git a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
index befa67b2b2ae..90ff4fdd79b7 100644
--- a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
+++ b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
@@ -148,6 +148,10 @@ void setup_dio_stream_attribute(struct pipe_ctx *pipe_ctx)
 		stream_encoder->funcs->lvds_set_stream_attribute(
 				stream_encoder,
 				&stream->timing);
+	else if (dc_is_rgb_signal(stream->signal))
+		stream_encoder->funcs->analog_set_stream_attribute(
+				stream_encoder,
+				&stream->timing);
 
 	if (dc_is_dp_signal(stream->signal))
 		link->dc->link_srv->dp_trace_source_sequence(link,
-- 
2.52.0

Re: [PATCH 4/5] drm/amd/display: Setup DAC encoder before using it

[timur.kristof]

On Sat, 2025-12-06 at 03:31 +0100, Timur Kristóf wrote:
> Just like for other signal types, call the encoder control
> command in the VBIOS to setup the DAC encoder.
> The DAC doesn't work well without this on some GPUs,
> for example the Radeon HD 7790 needs this.
> 
> Fixes: 7d63519cd9ca ("drm/amd/display: Add concept of analog encoders
> (v2)")
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>  .../amd/display/dc/dce/dce_stream_encoder.c   | 25
> ++++++++++++++++++-
>  .../amd/display/dc/inc/hw/stream_encoder.h    |  4 +++
>  .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 +++
>  3 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> index 574618d5d4a4..1fbf6263c235 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> @@ -693,6 +693,24 @@ static void
> dce110_stream_encoder_lvds_set_stream_attribute(
>  	ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
>  }
>  
> +static void dce110_stream_encoder_analog_set_stream_attribute(
> +	struct stream_encoder *enc,
> +	struct dc_crtc_timing *crtc_timing)
> +{
> +	struct dce110_stream_encoder *enc110 =
> DCE110STRENC_FROM_STRENC(enc);
> +	struct bp_encoder_control cntl = {0};
> +
> +	cntl.action = ENCODER_CONTROL_SETUP;
> +	cntl.engine_id = enc110->base.id;

Found a mistake here.
This should use the analog_id added in the previous patch.

> +	cntl.pixel_clock = crtc_timing->pix_clk_100hz / 10;
> +
> +	if (enc110->base.bp->funcs->encoder_control(
> +			enc110->base.bp, &cntl) != BP_RESULT_OK)
> +		return;
> +
> +	ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
> +}
> +
>  static void dce110_stream_encoder_set_throttled_vcp_size(
>  	struct stream_encoder *enc,
>  	struct fixed31_32 avg_time_slots_per_mtp)
> @@ -1521,6 +1539,8 @@ static const struct stream_encoder_funcs
> dce110_str_enc_funcs = {
>  		dce110_stream_encoder_dvi_set_stream_attribute,
>  	.lvds_set_stream_attribute =
>  		dce110_stream_encoder_lvds_set_stream_attribute,
> +	.analog_set_stream_attribute =
> +		dce110_stream_encoder_analog_set_stream_attribute,
>  	.set_throttled_vcp_size =
>  		dce110_stream_encoder_set_throttled_vcp_size,
>  	.update_hdmi_info_packets =
> @@ -1568,7 +1588,10 @@ void dce110_stream_encoder_construct(
>  	enc110->se_mask = se_mask;
>  }
>  
> -static const struct stream_encoder_funcs dce110_an_str_enc_funcs =
> {};
> +static const struct stream_encoder_funcs dce110_an_str_enc_funcs = {
> +	.analog_set_stream_attribute =
> +		dce110_stream_encoder_analog_set_stream_attribute,
> +};
>  
>  void dce110_analog_stream_encoder_construct(
>  	struct dce110_stream_encoder *enc110,
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> index 27f950ae45ee..9658b06f32f7 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> @@ -153,6 +153,10 @@ struct stream_encoder_funcs {
>  		struct stream_encoder *enc,
>  		struct dc_crtc_timing *crtc_timing);
>  
> +	void (*analog_set_stream_attribute)(
> +		struct stream_encoder *enc,
> +		struct dc_crtc_timing *crtc_timing);
> +
>  	void (*set_throttled_vcp_size)(
>  		struct stream_encoder *enc,
>  		struct fixed31_32 avg_time_slots_per_mtp);
> diff --git a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> index befa67b2b2ae..90ff4fdd79b7 100644
> --- a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> +++ b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> @@ -148,6 +148,10 @@ void setup_dio_stream_attribute(struct pipe_ctx
> *pipe_ctx)
>  		stream_encoder->funcs->lvds_set_stream_attribute(
>  				stream_encoder,
>  				&stream->timing);
> +	else if (dc_is_rgb_signal(stream->signal))
> +		stream_encoder->funcs->analog_set_stream_attribute(
> +				stream_encoder,
> +				&stream->timing);
>  
>  	if (dc_is_dp_signal(stream->signal))
>  		link->dc->link_srv->dp_trace_source_sequence(link,

Re: [PATCH 4/5] drm/amd/display: Setup DAC encoder before using it

[Alex Deucher]

On Mon, Dec 8, 2025 at 10:21 AM <timur.kristof@gmail.com> wrote:
>
> On Sat, 2025-12-06 at 03:31 +0100, Timur Kristóf wrote:
> > Just like for other signal types, call the encoder control
> > command in the VBIOS to setup the DAC encoder.
> > The DAC doesn't work well without this on some GPUs,
> > for example the Radeon HD 7790 needs this.
> >
> > Fixes: 7d63519cd9ca ("drm/amd/display: Add concept of analog encoders
> > (v2)")
> > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > ---
> >  .../amd/display/dc/dce/dce_stream_encoder.c   | 25
> > ++++++++++++++++++-
> >  .../amd/display/dc/inc/hw/stream_encoder.h    |  4 +++
> >  .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 +++
> >  3 files changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > index 574618d5d4a4..1fbf6263c235 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > @@ -693,6 +693,24 @@ static void
> > dce110_stream_encoder_lvds_set_stream_attribute(
> >       ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
> >  }
> >
> > +static void dce110_stream_encoder_analog_set_stream_attribute(
> > +     struct stream_encoder *enc,
> > +     struct dc_crtc_timing *crtc_timing)
> > +{
> > +     struct dce110_stream_encoder *enc110 =
> > DCE110STRENC_FROM_STRENC(enc);
> > +     struct bp_encoder_control cntl = {0};
> > +
> > +     cntl.action = ENCODER_CONTROL_SETUP;
> > +     cntl.engine_id = enc110->base.id;
>
> Found a mistake here.
> This should use the analog_id added in the previous patch.

The series looks good to me.  Do you want to resend with this fixed?
I'll apply it.

Thanks,

Alex

>
> > +     cntl.pixel_clock = crtc_timing->pix_clk_100hz / 10;
> > +
> > +     if (enc110->base.bp->funcs->encoder_control(
> > +                     enc110->base.bp, &cntl) != BP_RESULT_OK)
> > +             return;
> > +
> > +     ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
> > +}
> > +
> >  static void dce110_stream_encoder_set_throttled_vcp_size(
> >       struct stream_encoder *enc,
> >       struct fixed31_32 avg_time_slots_per_mtp)
> > @@ -1521,6 +1539,8 @@ static const struct stream_encoder_funcs
> > dce110_str_enc_funcs = {
> >               dce110_stream_encoder_dvi_set_stream_attribute,
> >       .lvds_set_stream_attribute =
> >               dce110_stream_encoder_lvds_set_stream_attribute,
> > +     .analog_set_stream_attribute =
> > +             dce110_stream_encoder_analog_set_stream_attribute,
> >       .set_throttled_vcp_size =
> >               dce110_stream_encoder_set_throttled_vcp_size,
> >       .update_hdmi_info_packets =
> > @@ -1568,7 +1588,10 @@ void dce110_stream_encoder_construct(
> >       enc110->se_mask = se_mask;
> >  }
> >
> > -static const struct stream_encoder_funcs dce110_an_str_enc_funcs =
> > {};
> > +static const struct stream_encoder_funcs dce110_an_str_enc_funcs = {
> > +     .analog_set_stream_attribute =
> > +             dce110_stream_encoder_analog_set_stream_attribute,
> > +};
> >
> >  void dce110_analog_stream_encoder_construct(
> >       struct dce110_stream_encoder *enc110,
> > diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > index 27f950ae45ee..9658b06f32f7 100644
> > --- a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > @@ -153,6 +153,10 @@ struct stream_encoder_funcs {
> >               struct stream_encoder *enc,
> >               struct dc_crtc_timing *crtc_timing);
> >
> > +     void (*analog_set_stream_attribute)(
> > +             struct stream_encoder *enc,
> > +             struct dc_crtc_timing *crtc_timing);
> > +
> >       void (*set_throttled_vcp_size)(
> >               struct stream_encoder *enc,
> >               struct fixed31_32 avg_time_slots_per_mtp);
> > diff --git a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > index befa67b2b2ae..90ff4fdd79b7 100644
> > --- a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > +++ b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > @@ -148,6 +148,10 @@ void setup_dio_stream_attribute(struct pipe_ctx
> > *pipe_ctx)
> >               stream_encoder->funcs->lvds_set_stream_attribute(
> >                               stream_encoder,
> >                               &stream->timing);
> > +     else if (dc_is_rgb_signal(stream->signal))
> > +             stream_encoder->funcs->analog_set_stream_attribute(
> > +                             stream_encoder,
> > +                             &stream->timing);
> >
> >       if (dc_is_dp_signal(stream->signal))
> >               link->dc->link_srv->dp_trace_source_sequence(link,

Re: [PATCH 4/5] drm/amd/display: Setup DAC encoder before using it

[timur.kristof]

On Mon, 2025-12-08 at 10:27 -0500, Alex Deucher wrote:
> On Mon, Dec 8, 2025 at 10:21 AM <timur.kristof@gmail.com> wrote:
> > 
> > On Sat, 2025-12-06 at 03:31 +0100, Timur Kristóf wrote:
> > > Just like for other signal types, call the encoder control
> > > command in the VBIOS to setup the DAC encoder.
> > > The DAC doesn't work well without this on some GPUs,
> > > for example the Radeon HD 7790 needs this.
> > > 
> > > Fixes: 7d63519cd9ca ("drm/amd/display: Add concept of analog
> > > encoders
> > > (v2)")
> > > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > > ---
> > >  .../amd/display/dc/dce/dce_stream_encoder.c   | 25
> > > ++++++++++++++++++-
> > >  .../amd/display/dc/inc/hw/stream_encoder.h    |  4 +++
> > >  .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 +++
> > >  3 files changed, 32 insertions(+), 1 deletion(-)
> > > 
> > > diff --git
> > > a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > > b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > > index 574618d5d4a4..1fbf6263c235 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > > @@ -693,6 +693,24 @@ static void
> > > dce110_stream_encoder_lvds_set_stream_attribute(
> > >       ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
> > >  }
> > > 
> > > +static void dce110_stream_encoder_analog_set_stream_attribute(
> > > +     struct stream_encoder *enc,
> > > +     struct dc_crtc_timing *crtc_timing)
> > > +{
> > > +     struct dce110_stream_encoder *enc110 =
> > > DCE110STRENC_FROM_STRENC(enc);
> > > +     struct bp_encoder_control cntl = {0};
> > > +
> > > +     cntl.action = ENCODER_CONTROL_SETUP;
> > > +     cntl.engine_id = enc110->base.id;
> > 
> > Found a mistake here.
> > This should use the analog_id added in the previous patch.
> 
> The series looks good to me.  Do you want to resend with this fixed?
> I'll apply it.
> 
> Thanks,
> 
> Alex

Thanks Alex.
I will send a second version with this fixed.

Also I would like to wait for Mauro to test it. The series fixes the
"black screen" issue on my HD 7790, but Mauro told me that it doesn't
fix it for him.

> 
> > 
> > > +     cntl.pixel_clock = crtc_timing->pix_clk_100hz / 10;
> > > +
> > > +     if (enc110->base.bp->funcs->encoder_control(
> > > +                     enc110->base.bp, &cntl) != BP_RESULT_OK)
> > > +             return;
> > > +
> > > +     ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
> > > +}
> > > +
> > >  static void dce110_stream_encoder_set_throttled_vcp_size(
> > >       struct stream_encoder *enc,
> > >       struct fixed31_32 avg_time_slots_per_mtp)
> > > @@ -1521,6 +1539,8 @@ static const struct stream_encoder_funcs
> > > dce110_str_enc_funcs = {
> > >               dce110_stream_encoder_dvi_set_stream_attribute,
> > >       .lvds_set_stream_attribute =
> > >               dce110_stream_encoder_lvds_set_stream_attribute,
> > > +     .analog_set_stream_attribute =
> > > +             dce110_stream_encoder_analog_set_stream_attribute,
> > >       .set_throttled_vcp_size =
> > >               dce110_stream_encoder_set_throttled_vcp_size,
> > >       .update_hdmi_info_packets =
> > > @@ -1568,7 +1588,10 @@ void dce110_stream_encoder_construct(
> > >       enc110->se_mask = se_mask;
> > >  }
> > > 
> > > -static const struct stream_encoder_funcs dce110_an_str_enc_funcs
> > > =
> > > {};
> > > +static const struct stream_encoder_funcs dce110_an_str_enc_funcs
> > > = {
> > > +     .analog_set_stream_attribute =
> > > +             dce110_stream_encoder_analog_set_stream_attribute,
> > > +};
> > > 
> > >  void dce110_analog_stream_encoder_construct(
> > >       struct dce110_stream_encoder *enc110,
> > > diff --git
> > > a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > > b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > > index 27f950ae45ee..9658b06f32f7 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > > +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > > @@ -153,6 +153,10 @@ struct stream_encoder_funcs {
> > >               struct stream_encoder *enc,
> > >               struct dc_crtc_timing *crtc_timing);
> > > 
> > > +     void (*analog_set_stream_attribute)(
> > > +             struct stream_encoder *enc,
> > > +             struct dc_crtc_timing *crtc_timing);
> > > +
> > >       void (*set_throttled_vcp_size)(
> > >               struct stream_encoder *enc,
> > >               struct fixed31_32 avg_time_slots_per_mtp);
> > > diff --git
> > > a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > > b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > > index befa67b2b2ae..90ff4fdd79b7 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > > +++ b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > > @@ -148,6 +148,10 @@ void setup_dio_stream_attribute(struct
> > > pipe_ctx
> > > *pipe_ctx)
> > >               stream_encoder->funcs->lvds_set_stream_attribute(
> > >                               stream_encoder,
> > >                               &stream->timing);
> > > +     else if (dc_is_rgb_signal(stream->signal))
> > > +             stream_encoder->funcs->analog_set_stream_attribute(
> > > +                             stream_encoder,
> > > +                             &stream->timing);
> > > 
> > >       if (dc_is_dp_signal(stream->signal))
> > >               link->dc->link_srv->dp_trace_source_sequence(link,

Re: [PATCH 4/5] drm/amd/display: Setup DAC encoder before using it

[Alex Deucher]

On Mon, Dec 8, 2025 at 10:32 AM <timur.kristof@gmail.com> wrote:
>
> On Mon, 2025-12-08 at 10:27 -0500, Alex Deucher wrote:
> > On Mon, Dec 8, 2025 at 10:21 AM <timur.kristof@gmail.com> wrote:
> > >
> > > On Sat, 2025-12-06 at 03:31 +0100, Timur Kristóf wrote:
> > > > Just like for other signal types, call the encoder control
> > > > command in the VBIOS to setup the DAC encoder.
> > > > The DAC doesn't work well without this on some GPUs,
> > > > for example the Radeon HD 7790 needs this.
> > > >
> > > > Fixes: 7d63519cd9ca ("drm/amd/display: Add concept of analog
> > > > encoders
> > > > (v2)")
> > > > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > > > ---
> > > >  .../amd/display/dc/dce/dce_stream_encoder.c   | 25
> > > > ++++++++++++++++++-
> > > >  .../amd/display/dc/inc/hw/stream_encoder.h    |  4 +++
> > > >  .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 +++
> > > >  3 files changed, 32 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git
> > > > a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > > > b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > > > index 574618d5d4a4..1fbf6263c235 100644
> > > > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > > > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> > > > @@ -693,6 +693,24 @@ static void
> > > > dce110_stream_encoder_lvds_set_stream_attribute(
> > > >       ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
> > > >  }
> > > >
> > > > +static void dce110_stream_encoder_analog_set_stream_attribute(
> > > > +     struct stream_encoder *enc,
> > > > +     struct dc_crtc_timing *crtc_timing)
> > > > +{
> > > > +     struct dce110_stream_encoder *enc110 =
> > > > DCE110STRENC_FROM_STRENC(enc);
> > > > +     struct bp_encoder_control cntl = {0};
> > > > +
> > > > +     cntl.action = ENCODER_CONTROL_SETUP;
> > > > +     cntl.engine_id = enc110->base.id;
> > >
> > > Found a mistake here.
> > > This should use the analog_id added in the previous patch.
> >
> > The series looks good to me.  Do you want to resend with this fixed?
> > I'll apply it.
> >
> > Thanks,
> >
> > Alex
>
> Thanks Alex.
> I will send a second version with this fixed.

Feel free to add my RB for the series.

>
> Also I would like to wait for Mauro to test it. The series fixes the
> "black screen" issue on my HD 7790, but Mauro told me that it doesn't
> fix it for him.

Thanks for the update.  Sounds like it's still a valid bug fix even if
additional fixes may be needed for Mauro's system.

Alex


>
> >
> > >
> > > > +     cntl.pixel_clock = crtc_timing->pix_clk_100hz / 10;
> > > > +
> > > > +     if (enc110->base.bp->funcs->encoder_control(
> > > > +                     enc110->base.bp, &cntl) != BP_RESULT_OK)
> > > > +             return;
> > > > +
> > > > +     ASSERT(crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB);
> > > > +}
> > > > +
> > > >  static void dce110_stream_encoder_set_throttled_vcp_size(
> > > >       struct stream_encoder *enc,
> > > >       struct fixed31_32 avg_time_slots_per_mtp)
> > > > @@ -1521,6 +1539,8 @@ static const struct stream_encoder_funcs
> > > > dce110_str_enc_funcs = {
> > > >               dce110_stream_encoder_dvi_set_stream_attribute,
> > > >       .lvds_set_stream_attribute =
> > > >               dce110_stream_encoder_lvds_set_stream_attribute,
> > > > +     .analog_set_stream_attribute =
> > > > +             dce110_stream_encoder_analog_set_stream_attribute,
> > > >       .set_throttled_vcp_size =
> > > >               dce110_stream_encoder_set_throttled_vcp_size,
> > > >       .update_hdmi_info_packets =
> > > > @@ -1568,7 +1588,10 @@ void dce110_stream_encoder_construct(
> > > >       enc110->se_mask = se_mask;
> > > >  }
> > > >
> > > > -static const struct stream_encoder_funcs dce110_an_str_enc_funcs
> > > > =
> > > > {};
> > > > +static const struct stream_encoder_funcs dce110_an_str_enc_funcs
> > > > = {
> > > > +     .analog_set_stream_attribute =
> > > > +             dce110_stream_encoder_analog_set_stream_attribute,
> > > > +};
> > > >
> > > >  void dce110_analog_stream_encoder_construct(
> > > >       struct dce110_stream_encoder *enc110,
> > > > diff --git
> > > > a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > > > b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > > > index 27f950ae45ee..9658b06f32f7 100644
> > > > --- a/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > > > +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/stream_encoder.h
> > > > @@ -153,6 +153,10 @@ struct stream_encoder_funcs {
> > > >               struct stream_encoder *enc,
> > > >               struct dc_crtc_timing *crtc_timing);
> > > >
> > > > +     void (*analog_set_stream_attribute)(
> > > > +             struct stream_encoder *enc,
> > > > +             struct dc_crtc_timing *crtc_timing);
> > > > +
> > > >       void (*set_throttled_vcp_size)(
> > > >               struct stream_encoder *enc,
> > > >               struct fixed31_32 avg_time_slots_per_mtp);
> > > > diff --git
> > > > a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > > > b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > > > index befa67b2b2ae..90ff4fdd79b7 100644
> > > > --- a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > > > +++ b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > > > @@ -148,6 +148,10 @@ void setup_dio_stream_attribute(struct
> > > > pipe_ctx
> > > > *pipe_ctx)
> > > >               stream_encoder->funcs->lvds_set_stream_attribute(
> > > >                               stream_encoder,
> > > >                               &stream->timing);
> > > > +     else if (dc_is_rgb_signal(stream->signal))
> > > > +             stream_encoder->funcs->analog_set_stream_attribute(
> > > > +                             stream_encoder,
> > > > +                             &stream->timing);
> > > >
> > > >       if (dc_is_dp_signal(stream->signal))
> > > >               link->dc->link_srv->dp_trace_source_sequence(link,

[PATCH 5/5] drm/amd/display: Don't repeat DAC load detection

[Timur Kristóf]

The analog link detection code path had already performed the
DAC load detection by the time the EDID read is attempted.
So there is no need to repeat the DAC load detection,
we can know that no display is connected if no EDID is read.

Fixes: ee9fe14a64a2 ("drm/amd/display: Use DAC load detection on analog connectors (v2)")
Suggested-by: Alex Hung <alex.hung@amd.com>
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/display/dc/link/link_detection.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
index 6d31f4967f1a..5c769f51f1ea 100644
--- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
+++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
@@ -1169,11 +1169,10 @@ static bool detect_link_and_local_sink(struct dc_link *link,
 			 * - cheap DVI-A cable or adapter that doesn't connect DDC
 			 */
 			if (dc_connector_supports_analog(link->link_id.id)) {
-				/* If we didn't do DAC load detection yet, do it now
-				 * to verify there really is a display connected.
+				/* If we didn't already detect a display using
+				 * DAC load detection, we know it isn't connected.
 				 */
-				if (link->type != dc_connection_dac_load &&
-					!link_detect_dac_load_detect(link)) {
+				if (link->type != dc_connection_dac_load) {
 					if (prev_sink)
 						dc_sink_release(prev_sink);
 					link_disconnect_sink(link);
-- 
2.52.0

Re: [PATCH 0/5] drm/amd/display: DC analog connector fixes

[Alex Deucher]

On Fri, Dec 5, 2025 at 9:49 PM Timur Kristóf <timur.kristof@gmail.com> wrote:
>
> Fix a few issues with the initial DC analog connector
> implementation that I've noticed since. I highly recommend
> to backport this series to kernel versions that include
> the DC analog series.

Did you have an update for this series, or are they ready to be applied?

Alex

>
> Timur Kristóf (5):
>   drm/amd/display: Pass proper DAC encoder ID to VBIOS
>   drm/amd/display: Correct color depth for SelectCRTC_Source
>   drm/amd/display: Add missing encoder setup to DACnEncoderControl
>   drm/amd/display: Setup DAC encoder before using it
>   drm/amd/display: Don't repeat DAC load detection
>
>  .../gpu/drm/amd/display/dc/bios/bios_parser.c |  4 +-
>  .../drm/amd/display/dc/bios/command_table.c   | 44 +++++++++++++++----
>  .../drm/amd/display/dc/bios/command_table.h   |  4 +-
>  .../drm/amd/display/dc/dce/dce_link_encoder.c |  2 +
>  .../amd/display/dc/dce/dce_stream_encoder.c   | 25 ++++++++++-
>  .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 28 +-----------
>  .../drm/amd/display/dc/inc/hw/link_encoder.h  |  2 +
>  .../amd/display/dc/inc/hw/stream_encoder.h    |  4 ++
>  .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 ++
>  .../drm/amd/display/dc/link/link_detection.c  |  7 ++-
>  .../drm/amd/display/dc/link/link_factory.c    | 10 ++---
>  .../dc/resource/dce110/dce110_resource.c      |  2 +
>  .../amd/display/include/bios_parser_types.h   |  2 +-
>  13 files changed, 87 insertions(+), 51 deletions(-)
>
> --
> 2.52.0
>

Re: [PATCH 0/5] drm/amd/display: DC analog connector fixes

[Alex Hung]

Hi Alex,

Timur told me that he has not received feedback he needed to work on V2 
last week so I rebased this series and sent it to our promotion test 
this week while he waits.

If this series passes promotion before V2 is possible, we can apply this 
series first.

Cheers,

On 12/16/25 08:27, Alex Deucher wrote:
> On Fri, Dec 5, 2025 at 9:49 PM Timur Kristóf <timur.kristof@gmail.com> wrote:
>>
>> Fix a few issues with the initial DC analog connector
>> implementation that I've noticed since. I highly recommend
>> to backport this series to kernel versions that include
>> the DC analog series.
> 
> Did you have an update for this series, or are they ready to be applied?
> 
> Alex
> 
>>
>> Timur Kristóf (5):
>>    drm/amd/display: Pass proper DAC encoder ID to VBIOS
>>    drm/amd/display: Correct color depth for SelectCRTC_Source
>>    drm/amd/display: Add missing encoder setup to DACnEncoderControl
>>    drm/amd/display: Setup DAC encoder before using it
>>    drm/amd/display: Don't repeat DAC load detection
>>
>>   .../gpu/drm/amd/display/dc/bios/bios_parser.c |  4 +-
>>   .../drm/amd/display/dc/bios/command_table.c   | 44 +++++++++++++++----
>>   .../drm/amd/display/dc/bios/command_table.h   |  4 +-
>>   .../drm/amd/display/dc/dce/dce_link_encoder.c |  2 +
>>   .../amd/display/dc/dce/dce_stream_encoder.c   | 25 ++++++++++-
>>   .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 28 +-----------
>>   .../drm/amd/display/dc/inc/hw/link_encoder.h  |  2 +
>>   .../amd/display/dc/inc/hw/stream_encoder.h    |  4 ++
>>   .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 ++
>>   .../drm/amd/display/dc/link/link_detection.c  |  7 ++-
>>   .../drm/amd/display/dc/link/link_factory.c    | 10 ++---
>>   .../dc/resource/dce110/dce110_resource.c      |  2 +
>>   .../amd/display/include/bios_parser_types.h   |  2 +-
>>   13 files changed, 87 insertions(+), 51 deletions(-)
>>
>> --
>> 2.52.0
>>

Re: [PATCH 0/5] drm/amd/display: DC analog connector fixes

[Timur Kristóf]

On 2025. december 16., kedd 9:27:34 középső államokbeli zónaidő Alex Deucher 
wrote:
> On Fri, Dec 5, 2025 at 9:49 PM Timur Kristóf <timur.kristof@gmail.com> 
wrote:
> > Fix a few issues with the initial DC analog connector
> > implementation that I've noticed since. I highly recommend
> > to backport this series to kernel versions that include
> > the DC analog series.
> 
> Did you have an update for this series, or are they ready to be applied?
> 
> Alex

Hi,

I re-reviewed my patches in this series again and I realized that my comment 
was mistaken (I confused link encoder with stream encoder). So the patches 
should be good as-is.

However, in the meantime Mauro got back to me this weekend, and told me that 
he determined that the patch "Setup DAC encoder before using it" not only 
doesn't fix it, but regresses the analog connector on his HD 7790 further. 
Before this patch, he got a black screen on boot, but the DAC started working 
after suspend/resume. After this patch, it no longer works for him even after 
suspend/resume.

What do you think is the way to proceed?

A) Merge all patches but that one. I'll submit a corrected patch once we figure 
out the HD 7790.
B) Merge the whole series as-is and I can submit a fix once we figured out what 
is going on with the HD 7790.

Thanks & best regards,
Timur

> 
> > Timur Kristóf (5):
> >   drm/amd/display: Pass proper DAC encoder ID to VBIOS
> >   drm/amd/display: Correct color depth for SelectCRTC_Source
> >   drm/amd/display: Add missing encoder setup to DACnEncoderControl
> >   drm/amd/display: Setup DAC encoder before using it
> >   drm/amd/display: Don't repeat DAC load detection
> >  
> >  .../gpu/drm/amd/display/dc/bios/bios_parser.c |  4 +-
> >  .../drm/amd/display/dc/bios/command_table.c   | 44 +++++++++++++++----
> >  .../drm/amd/display/dc/bios/command_table.h   |  4 +-
> >  .../drm/amd/display/dc/dce/dce_link_encoder.c |  2 +
> >  .../amd/display/dc/dce/dce_stream_encoder.c   | 25 ++++++++++-
> >  .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 28 +-----------
> >  .../drm/amd/display/dc/inc/hw/link_encoder.h  |  2 +
> >  .../amd/display/dc/inc/hw/stream_encoder.h    |  4 ++
> >  .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 ++
> >  .../drm/amd/display/dc/link/link_detection.c  |  7 ++-
> >  .../drm/amd/display/dc/link/link_factory.c    | 10 ++---
> >  .../dc/resource/dce110/dce110_resource.c      |  2 +
> >  .../amd/display/include/bios_parser_types.h   |  2 +-
> >  13 files changed, 87 insertions(+), 51 deletions(-)
> > 
> > --
> > 2.52.0

Re: [PATCH 0/5] drm/amd/display: DC analog connector fixes

[Alex Hung]

On 12/16/25 11:21, Timur Kristóf wrote:
> On 2025. december 16., kedd 9:27:34 középső államokbeli zónaidő Alex Deucher
> wrote:
>> On Fri, Dec 5, 2025 at 9:49 PM Timur Kristóf <timur.kristof@gmail.com>
> wrote:
>>> Fix a few issues with the initial DC analog connector
>>> implementation that I've noticed since. I highly recommend
>>> to backport this series to kernel versions that include
>>> the DC analog series.
>>
>> Did you have an update for this series, or are they ready to be applied?
>>
>> Alex
> 
> Hi,
> 
> I re-reviewed my patches in this series again and I realized that my comment
> was mistaken (I confused link encoder with stream encoder). So the patches
> should be good as-is.
> 
> However, in the meantime Mauro got back to me this weekend, and told me that
> he determined that the patch "Setup DAC encoder before using it" not only
> doesn't fix it, but regresses the analog connector on his HD 7790 further.
> Before this patch, he got a black screen on boot, but the DAC started working
> after suspend/resume. After this patch, it no longer works for him even after
> suspend/resume.
> 
> What do you think is the way to proceed?
> 
> A) Merge all patches but that one. I'll submit a corrected patch once we figure
> out the HD 7790.
> B) Merge the whole series as-is and I can submit a fix once we figured out what
> is going on with the HD 7790.

Plan A sounds better. Let's not merge a patch with a known issue. I will 
also share test results by the end of the week.

> 
> Thanks & best regards,
> Timur
> 
>>
>>> Timur Kristóf (5):
>>>    drm/amd/display: Pass proper DAC encoder ID to VBIOS
>>>    drm/amd/display: Correct color depth for SelectCRTC_Source
>>>    drm/amd/display: Add missing encoder setup to DACnEncoderControl
>>>    drm/amd/display: Setup DAC encoder before using it
>>>    drm/amd/display: Don't repeat DAC load detection
>>>   
>>>   .../gpu/drm/amd/display/dc/bios/bios_parser.c |  4 +-
>>>   .../drm/amd/display/dc/bios/command_table.c   | 44 +++++++++++++++----
>>>   .../drm/amd/display/dc/bios/command_table.h   |  4 +-
>>>   .../drm/amd/display/dc/dce/dce_link_encoder.c |  2 +
>>>   .../amd/display/dc/dce/dce_stream_encoder.c   | 25 ++++++++++-
>>>   .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 28 +-----------
>>>   .../drm/amd/display/dc/inc/hw/link_encoder.h  |  2 +
>>>   .../amd/display/dc/inc/hw/stream_encoder.h    |  4 ++
>>>   .../amd/display/dc/link/hwss/link_hwss_dio.c  |  4 ++
>>>   .../drm/amd/display/dc/link/link_detection.c  |  7 ++-
>>>   .../drm/amd/display/dc/link/link_factory.c    | 10 ++---
>>>   .../dc/resource/dce110/dce110_resource.c      |  2 +
>>>   .../amd/display/include/bios_parser_types.h   |  2 +-
>>>   13 files changed, 87 insertions(+), 51 deletions(-)
>>>
>>> --
>>> 2.52.0
> 
> 
> 
> 

Re: [PATCH 0/5] drm/amd/display: DC analog connector fixes

[Timur Kristóf]

> >> 
> >> Did you have an update for this series, or are they ready to be applied?
> >> 
> >> Alex
> > 
> > Hi,
> > 
> > I re-reviewed my patches in this series again and I realized that my
> > comment was mistaken (I confused link encoder with stream encoder). So
> > the patches should be good as-is.
> > 
> > However, in the meantime Mauro got back to me this weekend, and told me
> > that he determined that the patch "Setup DAC encoder before using it" not
> > only doesn't fix it, but regresses the analog connector on his HD 7790
> > further. Before this patch, he got a black screen on boot, but the DAC
> > started working after suspend/resume. After this patch, it no longer
> > works for him even after suspend/resume.
> > 
> > What do you think is the way to proceed?
> > 
> > A) Merge all patches but that one. I'll submit a corrected patch once we
> > figure out the HD 7790.
> > B) Merge the whole series as-is and I can submit a fix once we figured out
> > what is going on with the HD 7790.
> 
> Plan A sounds better. Let's not merge a patch with a known issue. I will
> also share test results by the end of the week.
> 

Hi Alex,

Thank you, that sounds like a good plan to me.

Timur

Re: [PATCH 0/5] drm/amd/display: DC analog connector fixes

[Alex Hung]

On 12/16/25 17:26, Timur Kristóf wrote:
>>>>
>>>> Did you have an update for this series, or are they ready to be applied?
>>>>
>>>> Alex
>>>
>>> Hi,
>>>
>>> I re-reviewed my patches in this series again and I realized that my
>>> comment was mistaken (I confused link encoder with stream encoder). So
>>> the patches should be good as-is.
>>>
>>> However, in the meantime Mauro got back to me this weekend, and told me
>>> that he determined that the patch "Setup DAC encoder before using it" not
>>> only doesn't fix it, but regresses the analog connector on his HD 7790
>>> further. Before this patch, he got a black screen on boot, but the DAC
>>> started working after suspend/resume. After this patch, it no longer
>>> works for him even after suspend/resume.
>>>
>>> What do you think is the way to proceed?
>>>
>>> A) Merge all patches but that one. I'll submit a corrected patch once we
>>> figure out the HD 7790.
>>> B) Merge the whole series as-is and I can submit a fix once we figured out
>>> what is going on with the HD 7790.
>>
>> Plan A sounds better. Let's not merge a patch with a known issue. I will
>> also share test results by the end of the week.
>>
> 
> Hi Alex,
> 
> Thank you, that sounds like a good plan to me.
> 
> Timur
> 
> 
> 

Hi Timur,

No regression is found by promotion test.