On 2025-08-07 15:12, Harry Wentland wrote:
> 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.
>
While this shows on unreleased HW I wouldn't be surprised if it
repros on other (recent-ish) APUs (integrated GPUs). It's just
that this week's test was on currently unreleased HW.
Harry
> 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:
>
