On 2025-07-23 11:58, Timur Kristóf wrote: > Analog displays typically have a DDC connection which can be > used by the GPU to read EDID. This commit adds the capability > to probe analog displays using DDC, reading the EDID header and > deciding whether the analog link is connected based on the data > that was read. > > As a reference, I used the following functions: > amdgpu_connector_vga_detect > amdgpu_display_ddc_probe > > DAC load detection will be implemented in a separate commit.
Another regression in our internal testing with this patch, unfortunately only on not-yet released HW. I wonder if pipe-ctx->stream could be NULL in some cases. Harry > > Signed-off-by: Timur Kristóf <[email protected]> > --- > .../amd/display/dc/link/hwss/link_hwss_dio.c | 16 ++-- > .../drm/amd/display/dc/link/link_detection.c | 80 ++++++++++++++++++- > .../gpu/drm/amd/display/dc/link/link_dpms.c | 3 + > .../drm/amd/display/dc/link/link_factory.c | 3 + > 4 files changed, 95 insertions(+), 7 deletions(-) > > 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 f3470716734d..b9ebb992dc98 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 > @@ -58,8 +58,9 @@ void setup_dio_stream_encoder(struct pipe_ctx *pipe_ctx) > return; > } > > - link_enc->funcs->connect_dig_be_to_fe(link_enc, > - pipe_ctx->stream_res.stream_enc->id, true); > + if (!dc_is_rgb_signal(pipe_ctx->stream->signal)) > + link_enc->funcs->connect_dig_be_to_fe(link_enc, > + pipe_ctx->stream_res.stream_enc->id, true); > if (dc_is_dp_signal(pipe_ctx->stream->signal)) > > pipe_ctx->stream->ctx->dc->link_srv->dp_trace_source_sequence(pipe_ctx->stream->link, > DPCD_SOURCE_SEQ_AFTER_CONNECT_DIG_FE_BE); > @@ -98,10 +99,13 @@ void reset_dio_stream_encoder(struct pipe_ctx *pipe_ctx) > if (stream_enc->funcs->enable_stream) > stream_enc->funcs->enable_stream(stream_enc, > pipe_ctx->stream->signal, false); > - link_enc->funcs->connect_dig_be_to_fe( > - link_enc, > - pipe_ctx->stream_res.stream_enc->id, > - false); > + > + if (!dc_is_rgb_signal(pipe_ctx->stream->signal)) > + link_enc->funcs->connect_dig_be_to_fe( > + link_enc, > + pipe_ctx->stream_res.stream_enc->id, > + false); > + > if (dc_is_dp_signal(pipe_ctx->stream->signal)) > pipe_ctx->stream->ctx->dc->link_srv->dp_trace_source_sequence( > pipe_ctx->stream->link, > 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 827b630daf49..fcabc83464af 100644 > --- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c > +++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c > @@ -942,6 +942,12 @@ static bool detect_link_and_local_sink(struct dc_link > *link, > break; > } > > + case SIGNAL_TYPE_RGB: { > + sink_caps.transaction_type = DDC_TRANSACTION_TYPE_I2C; > + sink_caps.signal = SIGNAL_TYPE_RGB; > + break; > + } > + > case SIGNAL_TYPE_LVDS: { > sink_caps.transaction_type = DDC_TRANSACTION_TYPE_I2C; > sink_caps.signal = SIGNAL_TYPE_LVDS; > @@ -1133,9 +1139,17 @@ static bool detect_link_and_local_sink(struct dc_link > *link, > sink = prev_sink; > prev_sink = NULL; > } > - query_hdcp_capability(sink->sink_signal, link); > + > + if (!sink->edid_caps.analog) > + query_hdcp_capability(sink->sink_signal, link); > } > > + /* DVI-I connector connected to analog display. */ > + if ((link->link_enc->connector.id == > CONNECTOR_ID_DUAL_LINK_DVII || > + link->link_enc->connector.id == > CONNECTOR_ID_SINGLE_LINK_DVII) && > + sink->edid_caps.analog) > + sink->sink_signal = SIGNAL_TYPE_RGB; > + > /* HDMI-DVI Dongle */ > if (sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A && > !sink->edid_caps.edid_hdmi) > @@ -1228,6 +1242,64 @@ static bool detect_link_and_local_sink(struct dc_link > *link, > return true; > } > > +/** > + * Evaluates whether an EDID header is acceptable, > + * for the purpose of determining a connection with a display. > + */ > +static bool link_detect_evaluate_edid_header(uint8_t edid_header[8]) > +{ > + int edid_header_score = 0; > + int i; > + > + for (i = 0; i < 8; ++i) > + edid_header_score += edid_header[i] == ((i == 0 || i == 7) ? > 0x00 : 0xff); > + > + return edid_header_score >= 6; > +} > + > +/** > + * Tries to detect a connected display by probing the DDC > + * and reading the EDID header. > + * The probing is considered successful if we receive a > + * reply from the DDC over I2C and the EDID header matches. > + */ > +static bool link_detect_ddc_probe(struct dc_link *link) > +{ > + if (!link->ddc) > + return false; > + > + uint8_t edid_header[8] = {0}; > + bool ddc_probed = i2c_read(link->ddc, 0x50, edid_header, > sizeof(edid_header)); > + > + if (!ddc_probed) > + return false; > + > + if (!link_detect_evaluate_edid_header(edid_header)) > + return false; > + > + return true; > +} > + > +/** > + * Determines if there is an analog sink connected. > + */ > +static bool link_detect_analog(struct dc_link *link, enum dc_connection_type > *type) > +{ > + /* Don't care about connectors that don't support an analog signal. */ > + if (link->link_enc->connector.id != CONNECTOR_ID_VGA && > + link->link_enc->connector.id != CONNECTOR_ID_SINGLE_LINK_DVII && > + link->link_enc->connector.id != CONNECTOR_ID_DUAL_LINK_DVII) > + return false; > + > + if (link_detect_ddc_probe(link)) { > + *type = dc_connection_single; > + return true; > + } > + > + *type = dc_connection_none; > + return true; > +} > + > /* > * link_detect_connection_type() - Determine if there is a sink connected > * > @@ -1238,6 +1310,7 @@ static bool detect_link_and_local_sink(struct dc_link > *link, > bool link_detect_connection_type(struct dc_link *link, enum > dc_connection_type *type) > { > uint32_t is_hpd_high = 0; > + bool supports_hpd = link->irq_source_hpd != DC_IRQ_SOURCE_INVALID; > > if (link->connector_signal == SIGNAL_TYPE_LVDS) { > *type = dc_connection_single; > @@ -1261,6 +1334,8 @@ bool link_detect_connection_type(struct dc_link *link, > enum dc_connection_type * > return true; > } > > + if (!supports_hpd) > + return link_detect_analog(link, type); > > if (!query_hpd_status(link, &is_hpd_high)) > goto hpd_gpio_failure; > @@ -1269,6 +1344,9 @@ bool link_detect_connection_type(struct dc_link *link, > enum dc_connection_type * > *type = dc_connection_single; > /* TODO: need to do the actual detection */ > } else { > + if (link_detect_analog(link, type)) > + return true; > + > *type = dc_connection_none; > if (link->connector_signal == SIGNAL_TYPE_EDP) { > /* eDP is not connected, power down it */ > 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 d6b7347c6c11..ac25d89a4148 100644 > --- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c > +++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c > @@ -2256,6 +2256,9 @@ static enum dc_status enable_link( > enable_link_lvds(pipe_ctx); > status = DC_OK; > break; > + case SIGNAL_TYPE_RGB: > + status = DC_OK; > + break; > case SIGNAL_TYPE_VIRTUAL: > status = enable_link_virtual(pipe_ctx); > break; > 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 71c10a1261b9..c9725fd316f6 100644 > --- a/drivers/gpu/drm/amd/display/dc/link/link_factory.c > +++ b/drivers/gpu/drm/amd/display/dc/link/link_factory.c > @@ -555,6 +555,9 @@ static bool construct_phy(struct dc_link *link, > case CONNECTOR_ID_DUAL_LINK_DVII: > link->connector_signal = SIGNAL_TYPE_DVI_DUAL_LINK; > break; > + case CONNECTOR_ID_VGA: > + link->connector_signal = SIGNAL_TYPE_RGB; > + break; > case CONNECTOR_ID_DISPLAY_PORT: > case CONNECTOR_ID_MXM: > case CONNECTOR_ID_USBC:
