AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/display: Restore analog connector support
@ 2026-04-15 22:41 Roman.Li
  2026-04-16 15:46 ` Alex Hung
  2026-04-17 18:59 ` Alex Deucher
  0 siblings, 2 replies; 6+ messages in thread
From: Roman.Li @ 2026-04-15 22:41 UTC (permalink / raw)
  To: amd-gfx
  Cc: alexander.deucher, Harry.Wentland, Sunpeng.Li, Aurabindo.Pillai,
	alex.hung, timur.kristof, Roman Li

From: Roman Li <Roman.Li@amd.com>

[Why]
The analog connector support was accidentally removed,
causing a crash when connecting an analog monitor.

[How]
This patch restores the functions and pointers required for proper analog
and DP bridge encoder support on legacy GPUs.

Fixes: 66715fc0ecfd ("drm/amd/display: Sync dcn42 with DC 3.2.373")
Cc: Timur Kristóf <timur.kristof@gmail.com>
Signed-off-by: Roman Li <Roman.Li@amd.com>
---
 .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 53 +++++++++++++++++++
 1 file changed, 53 insertions(+)

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 5273ca09fe12..e76abc877f4a 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
@@ -677,6 +677,48 @@ dce110_dac_encoder_control(struct pipe_ctx *pipe_ctx, bool enable)
 	bios->funcs->encoder_control(bios, &encoder_control);
 }
 
+static void
+dce110_external_encoder_control(enum bp_external_encoder_control_action action,
+				struct dc_link *link,
+				struct dc_crtc_timing *timing)
+{
+	struct dc *dc = link->ctx->dc;
+	struct dc_bios *bios = link->ctx->dc_bios;
+	const struct dc_link_settings *link_settings = &link->cur_link_settings;
+	enum bp_result bp_result = BP_RESULT_OK;
+	struct bp_external_encoder_control ext_cntl = {
+		.action = action,
+		.connector_obj_id = link->link_enc->connector,
+		.encoder_id = link->ext_enc_id,
+		.lanes_number = link_settings->lane_count,
+		.link_rate = link_settings->link_rate,
+
+		/* Use signal type of the real link encoder, ie. DP */
+		.signal = link->connector_signal,
+
+		/* We don't know the timing yet when executing the SETUP action,
+		 * so use a reasonably high default value. It seems that ENABLE
+		 * can change the actual pixel clock but doesn't work with higher
+		 * pixel clocks than what SETUP was called with.
+		 */
+		.pixel_clock = timing ? timing->pix_clk_100hz / 10 : 300000,
+		.color_depth = timing ? timing->display_color_depth : COLOR_DEPTH_888,
+	};
+	DC_LOGGER_INIT(dc->ctx);
+
+	bp_result = bios->funcs->external_encoder_control(bios, &ext_cntl);
+
+	if (bp_result != BP_RESULT_OK)
+		DC_LOG_ERROR("Failed to execute external encoder action: 0x%x\n", action);
+}
+
+static void
+dce110_prepare_ddc(struct dc_link *link)
+{
+	if (link->ext_enc_id.id)
+		dce110_external_encoder_control(EXTERNAL_ENCODER_CONTROL_DDC_SETUP, link, NULL);
+}
+
 static bool
 dce110_dac_load_detect(struct dc_link *link)
 {
@@ -3376,6 +3418,15 @@ void dce110_enable_tmds_link_output(struct dc_link *link,
 	link->phy_state.symclk_state = SYMCLK_ON_TX_ON;
 }
 
+static void dce110_enable_analog_link_output(
+		struct dc_link *link,
+		uint32_t pix_clk_100hz)
+{
+	link->link_enc->funcs->enable_analog_output(
+			link->link_enc,
+			pix_clk_100hz);
+}
+
 void dce110_enable_dp_link_output(
 		struct dc_link *link,
 		const struct link_resource *link_res,
@@ -3513,8 +3564,10 @@ static const struct hw_sequencer_funcs dce110_funcs = {
 	.enable_lvds_link_output = dce110_enable_lvds_link_output,
 	.enable_tmds_link_output = dce110_enable_tmds_link_output,
 	.enable_dp_link_output = dce110_enable_dp_link_output,
+	.enable_analog_link_output = dce110_enable_analog_link_output,
 	.disable_link_output = dce110_disable_link_output,
 	.dac_load_detect = dce110_dac_load_detect,
+	.prepare_ddc = dce110_prepare_ddc,
 };
 
 static const struct hwseq_private_funcs dce110_private_funcs = {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: Restore analog connector support
  2026-04-15 22:41 [PATCH] drm/amd/display: Restore analog connector support Roman.Li
@ 2026-04-16 15:46 ` Alex Hung
  2026-04-16 17:38   ` Timur Kristóf
  2026-04-17 18:59 ` Alex Deucher
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Hung @ 2026-04-16 15:46 UTC (permalink / raw)
  To: Roman.Li, amd-gfx
  Cc: alexander.deucher, Harry.Wentland, Sunpeng.Li, Aurabindo.Pillai,
	timur.kristof

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 4/15/26 16:41, Roman.Li@amd.com wrote:
> From: Roman Li <Roman.Li@amd.com>
> 
> [Why]
> The analog connector support was accidentally removed,
> causing a crash when connecting an analog monitor.
> 
> [How]
> This patch restores the functions and pointers required for proper analog
> and DP bridge encoder support on legacy GPUs.
> 
> Fixes: 66715fc0ecfd ("drm/amd/display: Sync dcn42 with DC 3.2.373")
> Cc: Timur Kristóf <timur.kristof@gmail.com>
> Signed-off-by: Roman Li <Roman.Li@amd.com>
> ---
>   .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 53 +++++++++++++++++++
>   1 file changed, 53 insertions(+)
> 
> 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 5273ca09fe12..e76abc877f4a 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
> @@ -677,6 +677,48 @@ dce110_dac_encoder_control(struct pipe_ctx *pipe_ctx, bool enable)
>   	bios->funcs->encoder_control(bios, &encoder_control);
>   }
>   
> +static void
> +dce110_external_encoder_control(enum bp_external_encoder_control_action action,
> +				struct dc_link *link,
> +				struct dc_crtc_timing *timing)
> +{
> +	struct dc *dc = link->ctx->dc;
> +	struct dc_bios *bios = link->ctx->dc_bios;
> +	const struct dc_link_settings *link_settings = &link->cur_link_settings;
> +	enum bp_result bp_result = BP_RESULT_OK;
> +	struct bp_external_encoder_control ext_cntl = {
> +		.action = action,
> +		.connector_obj_id = link->link_enc->connector,
> +		.encoder_id = link->ext_enc_id,
> +		.lanes_number = link_settings->lane_count,
> +		.link_rate = link_settings->link_rate,
> +
> +		/* Use signal type of the real link encoder, ie. DP */
> +		.signal = link->connector_signal,
> +
> +		/* We don't know the timing yet when executing the SETUP action,
> +		 * so use a reasonably high default value. It seems that ENABLE
> +		 * can change the actual pixel clock but doesn't work with higher
> +		 * pixel clocks than what SETUP was called with.
> +		 */
> +		.pixel_clock = timing ? timing->pix_clk_100hz / 10 : 300000,
> +		.color_depth = timing ? timing->display_color_depth : COLOR_DEPTH_888,
> +	};
> +	DC_LOGGER_INIT(dc->ctx);
> +
> +	bp_result = bios->funcs->external_encoder_control(bios, &ext_cntl);
> +
> +	if (bp_result != BP_RESULT_OK)
> +		DC_LOG_ERROR("Failed to execute external encoder action: 0x%x\n", action);
> +}
> +
> +static void
> +dce110_prepare_ddc(struct dc_link *link)
> +{
> +	if (link->ext_enc_id.id)
> +		dce110_external_encoder_control(EXTERNAL_ENCODER_CONTROL_DDC_SETUP, link, NULL);
> +}
> +
>   static bool
>   dce110_dac_load_detect(struct dc_link *link)
>   {
> @@ -3376,6 +3418,15 @@ void dce110_enable_tmds_link_output(struct dc_link *link,
>   	link->phy_state.symclk_state = SYMCLK_ON_TX_ON;
>   }
>   
> +static void dce110_enable_analog_link_output(
> +		struct dc_link *link,
> +		uint32_t pix_clk_100hz)
> +{
> +	link->link_enc->funcs->enable_analog_output(
> +			link->link_enc,
> +			pix_clk_100hz);
> +}
> +
>   void dce110_enable_dp_link_output(
>   		struct dc_link *link,
>   		const struct link_resource *link_res,
> @@ -3513,8 +3564,10 @@ static const struct hw_sequencer_funcs dce110_funcs = {
>   	.enable_lvds_link_output = dce110_enable_lvds_link_output,
>   	.enable_tmds_link_output = dce110_enable_tmds_link_output,
>   	.enable_dp_link_output = dce110_enable_dp_link_output,
> +	.enable_analog_link_output = dce110_enable_analog_link_output,
>   	.disable_link_output = dce110_disable_link_output,
>   	.dac_load_detect = dce110_dac_load_detect,
> +	.prepare_ddc = dce110_prepare_ddc,
>   };
>   
>   static const struct hwseq_private_funcs dce110_private_funcs = {


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: Restore analog connector support
  2026-04-16 15:46 ` Alex Hung
@ 2026-04-16 17:38   ` Timur Kristóf
  2026-04-17 19:35     ` Timur Kristóf
  0 siblings, 1 reply; 6+ messages in thread
From: Timur Kristóf @ 2026-04-16 17:38 UTC (permalink / raw)
  To: Alex Hung
  Cc: Roman Li, amd-gfx list, Deucher, Alexander, Harry Wentland,
	Leo Li, Aurabindo Pillai

[-- Attachment #1: Type: text/plain, Size: 4688 bytes --]

Hi,

I'll test this on a few different old GPUs and get back to you.

Thanks,
Timur

Alex Hung <alex.hung@amd.com> ezt írta (időpont: 2026. ápr. 16., Csü 17:46):

> Reviewed-by: Alex Hung <alex.hung@amd.com>
>
> On 4/15/26 16:41, Roman.Li@amd.com wrote:
> > From: Roman Li <Roman.Li@amd.com>
> >
> > [Why]
> > The analog connector support was accidentally removed,
> > causing a crash when connecting an analog monitor.
> >
> > [How]
> > This patch restores the functions and pointers required for proper analog
> > and DP bridge encoder support on legacy GPUs.
> >
> > Fixes: 66715fc0ecfd ("drm/amd/display: Sync dcn42 with DC 3.2.373")
> > Cc: Timur Kristóf <timur.kristof@gmail.com>
> > Signed-off-by: Roman Li <Roman.Li@amd.com>
> > ---
> >   .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 53 +++++++++++++++++++
> >   1 file changed, 53 insertions(+)
> >
> > 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 5273ca09fe12..e76abc877f4a 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
> > @@ -677,6 +677,48 @@ dce110_dac_encoder_control(struct pipe_ctx
> *pipe_ctx, bool enable)
> >       bios->funcs->encoder_control(bios, &encoder_control);
> >   }
> >
> > +static void
> > +dce110_external_encoder_control(enum bp_external_encoder_control_action
> action,
> > +                             struct dc_link *link,
> > +                             struct dc_crtc_timing *timing)
> > +{
> > +     struct dc *dc = link->ctx->dc;
> > +     struct dc_bios *bios = link->ctx->dc_bios;
> > +     const struct dc_link_settings *link_settings =
> &link->cur_link_settings;
> > +     enum bp_result bp_result = BP_RESULT_OK;
> > +     struct bp_external_encoder_control ext_cntl = {
> > +             .action = action,
> > +             .connector_obj_id = link->link_enc->connector,
> > +             .encoder_id = link->ext_enc_id,
> > +             .lanes_number = link_settings->lane_count,
> > +             .link_rate = link_settings->link_rate,
> > +
> > +             /* Use signal type of the real link encoder, ie. DP */
> > +             .signal = link->connector_signal,
> > +
> > +             /* We don't know the timing yet when executing the SETUP
> action,
> > +              * so use a reasonably high default value. It seems that
> ENABLE
> > +              * can change the actual pixel clock but doesn't work with
> higher
> > +              * pixel clocks than what SETUP was called with.
> > +              */
> > +             .pixel_clock = timing ? timing->pix_clk_100hz / 10 :
> 300000,
> > +             .color_depth = timing ? timing->display_color_depth :
> COLOR_DEPTH_888,
> > +     };
> > +     DC_LOGGER_INIT(dc->ctx);
> > +
> > +     bp_result = bios->funcs->external_encoder_control(bios, &ext_cntl);
> > +
> > +     if (bp_result != BP_RESULT_OK)
> > +             DC_LOG_ERROR("Failed to execute external encoder action:
> 0x%x\n", action);
> > +}
> > +
> > +static void
> > +dce110_prepare_ddc(struct dc_link *link)
> > +{
> > +     if (link->ext_enc_id.id)
> > +
>  dce110_external_encoder_control(EXTERNAL_ENCODER_CONTROL_DDC_SETUP, link,
> NULL);
> > +}
> > +
> >   static bool
> >   dce110_dac_load_detect(struct dc_link *link)
> >   {
> > @@ -3376,6 +3418,15 @@ void dce110_enable_tmds_link_output(struct
> dc_link *link,
> >       link->phy_state.symclk_state = SYMCLK_ON_TX_ON;
> >   }
> >
> > +static void dce110_enable_analog_link_output(
> > +             struct dc_link *link,
> > +             uint32_t pix_clk_100hz)
> > +{
> > +     link->link_enc->funcs->enable_analog_output(
> > +                     link->link_enc,
> > +                     pix_clk_100hz);
> > +}
> > +
> >   void dce110_enable_dp_link_output(
> >               struct dc_link *link,
> >               const struct link_resource *link_res,
> > @@ -3513,8 +3564,10 @@ static const struct hw_sequencer_funcs
> dce110_funcs = {
> >       .enable_lvds_link_output = dce110_enable_lvds_link_output,
> >       .enable_tmds_link_output = dce110_enable_tmds_link_output,
> >       .enable_dp_link_output = dce110_enable_dp_link_output,
> > +     .enable_analog_link_output = dce110_enable_analog_link_output,
> >       .disable_link_output = dce110_disable_link_output,
> >       .dac_load_detect = dce110_dac_load_detect,
> > +     .prepare_ddc = dce110_prepare_ddc,
> >   };
> >
> >   static const struct hwseq_private_funcs dce110_private_funcs = {
>
>

[-- Attachment #2: Type: text/html, Size: 6416 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: Restore analog connector support
  2026-04-15 22:41 [PATCH] drm/amd/display: Restore analog connector support Roman.Li
  2026-04-16 15:46 ` Alex Hung
@ 2026-04-17 18:59 ` Alex Deucher
  1 sibling, 0 replies; 6+ messages in thread
From: Alex Deucher @ 2026-04-17 18:59 UTC (permalink / raw)
  To: Roman.Li
  Cc: amd-gfx, alexander.deucher, Harry.Wentland, Sunpeng.Li,
	Aurabindo.Pillai, alex.hung, timur.kristof

On Wed, Apr 15, 2026 at 6:42 PM <Roman.Li@amd.com> wrote:
>
> From: Roman Li <Roman.Li@amd.com>
>
> [Why]
> The analog connector support was accidentally removed,
> causing a crash when connecting an analog monitor.
>
> [How]
> This patch restores the functions and pointers required for proper analog
> and DP bridge encoder support on legacy GPUs.
>
> Fixes: 66715fc0ecfd ("drm/amd/display: Sync dcn42 with DC 3.2.373")

66715fc0ecfd looks like it also accidently dropped some of the new
external encoder handling from bios_parser.c, dc_bios_types.h, and
dce110_hwseq.c

Alex

> Cc: Timur Kristóf <timur.kristof@gmail.com>
> Signed-off-by: Roman Li <Roman.Li@amd.com>
> ---
>  .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> 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 5273ca09fe12..e76abc877f4a 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
> @@ -677,6 +677,48 @@ dce110_dac_encoder_control(struct pipe_ctx *pipe_ctx, bool enable)
>         bios->funcs->encoder_control(bios, &encoder_control);
>  }
>
> +static void
> +dce110_external_encoder_control(enum bp_external_encoder_control_action action,
> +                               struct dc_link *link,
> +                               struct dc_crtc_timing *timing)
> +{
> +       struct dc *dc = link->ctx->dc;
> +       struct dc_bios *bios = link->ctx->dc_bios;
> +       const struct dc_link_settings *link_settings = &link->cur_link_settings;
> +       enum bp_result bp_result = BP_RESULT_OK;
> +       struct bp_external_encoder_control ext_cntl = {
> +               .action = action,
> +               .connector_obj_id = link->link_enc->connector,
> +               .encoder_id = link->ext_enc_id,
> +               .lanes_number = link_settings->lane_count,
> +               .link_rate = link_settings->link_rate,
> +
> +               /* Use signal type of the real link encoder, ie. DP */
> +               .signal = link->connector_signal,
> +
> +               /* We don't know the timing yet when executing the SETUP action,
> +                * so use a reasonably high default value. It seems that ENABLE
> +                * can change the actual pixel clock but doesn't work with higher
> +                * pixel clocks than what SETUP was called with.
> +                */
> +               .pixel_clock = timing ? timing->pix_clk_100hz / 10 : 300000,
> +               .color_depth = timing ? timing->display_color_depth : COLOR_DEPTH_888,
> +       };
> +       DC_LOGGER_INIT(dc->ctx);
> +
> +       bp_result = bios->funcs->external_encoder_control(bios, &ext_cntl);
> +
> +       if (bp_result != BP_RESULT_OK)
> +               DC_LOG_ERROR("Failed to execute external encoder action: 0x%x\n", action);
> +}
> +
> +static void
> +dce110_prepare_ddc(struct dc_link *link)
> +{
> +       if (link->ext_enc_id.id)
> +               dce110_external_encoder_control(EXTERNAL_ENCODER_CONTROL_DDC_SETUP, link, NULL);
> +}
> +
>  static bool
>  dce110_dac_load_detect(struct dc_link *link)
>  {
> @@ -3376,6 +3418,15 @@ void dce110_enable_tmds_link_output(struct dc_link *link,
>         link->phy_state.symclk_state = SYMCLK_ON_TX_ON;
>  }
>
> +static void dce110_enable_analog_link_output(
> +               struct dc_link *link,
> +               uint32_t pix_clk_100hz)
> +{
> +       link->link_enc->funcs->enable_analog_output(
> +                       link->link_enc,
> +                       pix_clk_100hz);
> +}
> +
>  void dce110_enable_dp_link_output(
>                 struct dc_link *link,
>                 const struct link_resource *link_res,
> @@ -3513,8 +3564,10 @@ static const struct hw_sequencer_funcs dce110_funcs = {
>         .enable_lvds_link_output = dce110_enable_lvds_link_output,
>         .enable_tmds_link_output = dce110_enable_tmds_link_output,
>         .enable_dp_link_output = dce110_enable_dp_link_output,
> +       .enable_analog_link_output = dce110_enable_analog_link_output,
>         .disable_link_output = dce110_disable_link_output,
>         .dac_load_detect = dce110_dac_load_detect,
> +       .prepare_ddc = dce110_prepare_ddc,
>  };
>
>  static const struct hwseq_private_funcs dce110_private_funcs = {
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amd/display: Restore analog connector support
  2026-04-16 17:38   ` Timur Kristóf
@ 2026-04-17 19:35     ` Timur Kristóf
  2026-04-17 20:38       ` Li, Roman
  0 siblings, 1 reply; 6+ messages in thread
From: Timur Kristóf @ 2026-04-17 19:35 UTC (permalink / raw)
  To: Alex Hung
  Cc: Roman Li, amd-gfx list, Deucher, Alexander, Harry Wentland,
	Leo Li, Aurabindo Pillai

Hi,

Tested this on the following:
HD 7870 XT - works
R9 280X (Tahiti) - works
R9 380X (Tonga) - works
A10-7850K (Kaveri) - broken

It seems that some other chunks of code were removed which were necessary for 
external DP bridge encoders to work properly. Those will need to be added back 
too, otherwise we regress Kaveri APUs.

Best regards,
Timur


On Thursday, April 16, 2026 7:38:39 PM Central European Summer Time Timur 
Kristóf wrote:
> Hi,
> 
> I'll test this on a few different old GPUs and get back to you.
> 
> Thanks,
> Timur
> 
> Alex Hung <alex.hung@amd.com> ezt írta (időpont: 2026. ápr. 16., Csü 17:46):
> > Reviewed-by: Alex Hung <alex.hung@amd.com>
> > 
> > On 4/15/26 16:41, Roman.Li@amd.com wrote:
> > > From: Roman Li <Roman.Li@amd.com>
> > > 
> > > [Why]
> > > The analog connector support was accidentally removed,
> > > causing a crash when connecting an analog monitor.
> > > 
> > > [How]
> > > This patch restores the functions and pointers required for proper
> > > analog
> > > and DP bridge encoder support on legacy GPUs.
> > > 
> > > Fixes: 66715fc0ecfd ("drm/amd/display: Sync dcn42 with DC 3.2.373")
> > > Cc: Timur Kristóf <timur.kristof@gmail.com>
> > > Signed-off-by: Roman Li <Roman.Li@amd.com>
> > > ---
> > > 
> > >   .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 53 +++++++++++++++++++
> > >   1 file changed, 53 insertions(+)
> > > 
> > > 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 5273ca09fe12..e76abc877f4a 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
> > > @@ -677,6 +677,48 @@ dce110_dac_encoder_control(struct pipe_ctx
> > 
> > *pipe_ctx, bool enable)
> > 
> > >       bios->funcs->encoder_control(bios, &encoder_control);
> > >   
> > >   }
> > > 
> > > +static void
> > > +dce110_external_encoder_control(enum bp_external_encoder_control_action
> > 
> > action,
> > 
> > > +                             struct dc_link *link,
> > > +                             struct dc_crtc_timing *timing)
> > > +{
> > > +     struct dc *dc = link->ctx->dc;
> > > +     struct dc_bios *bios = link->ctx->dc_bios;
> > > +     const struct dc_link_settings *link_settings =
> > 
> > &link->cur_link_settings;
> > 
> > > +     enum bp_result bp_result = BP_RESULT_OK;
> > > +     struct bp_external_encoder_control ext_cntl = {
> > > +             .action = action,
> > > +             .connector_obj_id = link->link_enc->connector,
> > > +             .encoder_id = link->ext_enc_id,
> > > +             .lanes_number = link_settings->lane_count,
> > > +             .link_rate = link_settings->link_rate,
> > > +
> > > +             /* Use signal type of the real link encoder, ie. DP */
> > > +             .signal = link->connector_signal,
> > > +
> > > +             /* We don't know the timing yet when executing the SETUP
> > 
> > action,
> > 
> > > +              * so use a reasonably high default value. It seems that
> > 
> > ENABLE
> > 
> > > +              * can change the actual pixel clock but doesn't work with
> > 
> > higher
> > 
> > > +              * pixel clocks than what SETUP was called with.
> > > +              */
> > 
> > > +             .pixel_clock = timing ? timing->pix_clk_100hz / 10 :
> > 300000,
> > 
> > > +             .color_depth = timing ? timing->display_color_depth :
> > COLOR_DEPTH_888,
> > 
> > > +     };
> > > +     DC_LOGGER_INIT(dc->ctx);
> > > +
> > > +     bp_result = bios->funcs->external_encoder_control(bios,
> > > &ext_cntl);
> > > +
> > > +     if (bp_result != BP_RESULT_OK)
> > 
> > > +             DC_LOG_ERROR("Failed to execute external encoder action:
> > 0x%x\n", action);
> > 
> > > +}
> > > +
> > > +static void
> > > +dce110_prepare_ddc(struct dc_link *link)
> > > +{
> > > +     if (link->ext_enc_id.id)
> > > +
> >  
> >  dce110_external_encoder_control(EXTERNAL_ENCODER_CONTROL_DDC_SETUP, link,
> > 
> > NULL);
> > 
> > > +}
> > > +
> > > 
> > >   static bool
> > >   dce110_dac_load_detect(struct dc_link *link)
> > >   {
> > > 
> > > @@ -3376,6 +3418,15 @@ void dce110_enable_tmds_link_output(struct
> > 
> > dc_link *link,
> > 
> > >       link->phy_state.symclk_state = SYMCLK_ON_TX_ON;
> > >   
> > >   }
> > > 
> > > +static void dce110_enable_analog_link_output(
> > > +             struct dc_link *link,
> > > +             uint32_t pix_clk_100hz)
> > > +{
> > > +     link->link_enc->funcs->enable_analog_output(
> > > +                     link->link_enc,
> > > +                     pix_clk_100hz);
> > > +}
> > > +
> > > 
> > >   void dce110_enable_dp_link_output(
> > >   
> > >               struct dc_link *link,
> > >               const struct link_resource *link_res,
> > > 
> > > @@ -3513,8 +3564,10 @@ static const struct hw_sequencer_funcs
> > 
> > dce110_funcs = {
> > 
> > >       .enable_lvds_link_output = dce110_enable_lvds_link_output,
> > >       .enable_tmds_link_output = dce110_enable_tmds_link_output,
> > >       .enable_dp_link_output = dce110_enable_dp_link_output,
> > > 
> > > +     .enable_analog_link_output = dce110_enable_analog_link_output,
> > > 
> > >       .disable_link_output = dce110_disable_link_output,
> > >       .dac_load_detect = dce110_dac_load_detect,
> > > 
> > > +     .prepare_ddc = dce110_prepare_ddc,
> > > 
> > >   };
> > >   
> > >   static const struct hwseq_private_funcs dce110_private_funcs = {





^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] drm/amd/display: Restore analog connector support
  2026-04-17 19:35     ` Timur Kristóf
@ 2026-04-17 20:38       ` Li, Roman
  0 siblings, 0 replies; 6+ messages in thread
From: Li, Roman @ 2026-04-17 20:38 UTC (permalink / raw)
  To: Timur Kristóf, Hung, Alex
  Cc: amd-gfx list, Deucher, Alexander, Wentland, Harry,
	Li,  Sun peng (Leo), Pillai, Aurabindo

[Public]

Hi Timur,

Thanks for sharing test results.
I will send V2 shortly.

Thanks,
Roman

> -----Original Message-----
> From: Timur Kristóf <timur.kristof@gmail.com>
> Sent: Friday, April 17, 2026 3:36 PM
> To: Hung, Alex <Alex.Hung@amd.com>
> Cc: Li, Roman <Roman.Li@amd.com>; amd-gfx list <amd-
> gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>;
> Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo)
> <Sunpeng.Li@amd.com>; Pillai, Aurabindo <Aurabindo.Pillai@amd.com>
> Subject: Re: [PATCH] drm/amd/display: Restore analog connector support
>
> Hi,
>
> Tested this on the following:
> HD 7870 XT - works
> R9 280X (Tahiti) - works
> R9 380X (Tonga) - works
> A10-7850K (Kaveri) - broken
>
> It seems that some other chunks of code were removed which were necessary for
> external DP bridge encoders to work properly. Those will need to be added back
> too, otherwise we regress Kaveri APUs.
>
> Best regards,
> Timur
>
>
> On Thursday, April 16, 2026 7:38:39 PM Central European Summer Time Timur
> Kristóf wrote:
> > Hi,
> >
> > I'll test this on a few different old GPUs and get back to you.
> >
> > Thanks,
> > Timur
> >
> > Alex Hung <alex.hung@amd.com> ezt írta (időpont: 2026. ápr. 16., Csü 17:46):
> > > Reviewed-by: Alex Hung <alex.hung@amd.com>
> > >
> > > On 4/15/26 16:41, Roman.Li@amd.com wrote:
> > > > From: Roman Li <Roman.Li@amd.com>
> > > >
> > > > [Why]
> > > > The analog connector support was accidentally removed, causing a
> > > > crash when connecting an analog monitor.
> > > >
> > > > [How]
> > > > This patch restores the functions and pointers required for proper
> > > > analog and DP bridge encoder support on legacy GPUs.
> > > >
> > > > Fixes: 66715fc0ecfd ("drm/amd/display: Sync dcn42 with DC
> > > > 3.2.373")
> > > > Cc: Timur Kristóf <timur.kristof@gmail.com>
> > > > Signed-off-by: Roman Li <Roman.Li@amd.com>
> > > > ---
> > > >
> > > >   .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 53
> +++++++++++++++++++
> > > >   1 file changed, 53 insertions(+)
> > > >
> > > > 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 5273ca09fe12..e76abc877f4a 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
> > > > @@ -677,6 +677,48 @@ dce110_dac_encoder_control(struct pipe_ctx
> > >
> > > *pipe_ctx, bool enable)
> > >
> > > >       bios->funcs->encoder_control(bios, &encoder_control);
> > > >
> > > >   }
> > > >
> > > > +static void
> > > > +dce110_external_encoder_control(enum
> > > > +bp_external_encoder_control_action
> > >
> > > action,
> > >
> > > > +                             struct dc_link *link,
> > > > +                             struct dc_crtc_timing *timing) {
> > > > +     struct dc *dc = link->ctx->dc;
> > > > +     struct dc_bios *bios = link->ctx->dc_bios;
> > > > +     const struct dc_link_settings *link_settings =
> > >
> > > &link->cur_link_settings;
> > >
> > > > +     enum bp_result bp_result = BP_RESULT_OK;
> > > > +     struct bp_external_encoder_control ext_cntl = {
> > > > +             .action = action,
> > > > +             .connector_obj_id = link->link_enc->connector,
> > > > +             .encoder_id = link->ext_enc_id,
> > > > +             .lanes_number = link_settings->lane_count,
> > > > +             .link_rate = link_settings->link_rate,
> > > > +
> > > > +             /* Use signal type of the real link encoder, ie. DP */
> > > > +             .signal = link->connector_signal,
> > > > +
> > > > +             /* We don't know the timing yet when executing the
> > > > + SETUP
> > >
> > > action,
> > >
> > > > +              * so use a reasonably high default value. It seems
> > > > + that
> > >
> > > ENABLE
> > >
> > > > +              * can change the actual pixel clock but doesn't
> > > > + work with
> > >
> > > higher
> > >
> > > > +              * pixel clocks than what SETUP was called with.
> > > > +              */
> > >
> > > > +             .pixel_clock = timing ? timing->pix_clk_100hz / 10 :
> > > 300000,
> > >
> > > > +             .color_depth = timing ? timing->display_color_depth :
> > > COLOR_DEPTH_888,
> > >
> > > > +     };
> > > > +     DC_LOGGER_INIT(dc->ctx);
> > > > +
> > > > +     bp_result = bios->funcs->external_encoder_control(bios,
> > > > &ext_cntl);
> > > > +
> > > > +     if (bp_result != BP_RESULT_OK)
> > >
> > > > +             DC_LOG_ERROR("Failed to execute external encoder action:
> > > 0x%x\n", action);
> > >
> > > > +}
> > > > +
> > > > +static void
> > > > +dce110_prepare_ddc(struct dc_link *link) {
> > > > +     if (link->ext_enc_id.id)
> > > > +
> > >
> > >
> dce110_external_encoder_control(EXTERNAL_ENCODER_CONTROL_DDC_SE
> TUP,
> > > link,
> > >
> > > NULL);
> > >
> > > > +}
> > > > +
> > > >
> > > >   static bool
> > > >   dce110_dac_load_detect(struct dc_link *link)
> > > >   {
> > > >
> > > > @@ -3376,6 +3418,15 @@ void dce110_enable_tmds_link_output(struct
> > >
> > > dc_link *link,
> > >
> > > >       link->phy_state.symclk_state = SYMCLK_ON_TX_ON;
> > > >
> > > >   }
> > > >
> > > > +static void dce110_enable_analog_link_output(
> > > > +             struct dc_link *link,
> > > > +             uint32_t pix_clk_100hz) {
> > > > +     link->link_enc->funcs->enable_analog_output(
> > > > +                     link->link_enc,
> > > > +                     pix_clk_100hz); }
> > > > +
> > > >
> > > >   void dce110_enable_dp_link_output(
> > > >
> > > >               struct dc_link *link,
> > > >               const struct link_resource *link_res,
> > > >
> > > > @@ -3513,8 +3564,10 @@ static const struct hw_sequencer_funcs
> > >
> > > dce110_funcs = {
> > >
> > > >       .enable_lvds_link_output = dce110_enable_lvds_link_output,
> > > >       .enable_tmds_link_output = dce110_enable_tmds_link_output,
> > > >       .enable_dp_link_output = dce110_enable_dp_link_output,
> > > >
> > > > +     .enable_analog_link_output =
> > > > + dce110_enable_analog_link_output,
> > > >
> > > >       .disable_link_output = dce110_disable_link_output,
> > > >       .dac_load_detect = dce110_dac_load_detect,
> > > >
> > > > +     .prepare_ddc = dce110_prepare_ddc,
> > > >
> > > >   };
> > > >
> > > >   static const struct hwseq_private_funcs dce110_private_funcs = {
>
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-04-17 20:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-15 22:41 [PATCH] drm/amd/display: Restore analog connector support Roman.Li
2026-04-16 15:46 ` Alex Hung
2026-04-16 17:38   ` Timur Kristóf
2026-04-17 19:35     ` Timur Kristóf
2026-04-17 20:38       ` Li, Roman
2026-04-17 18:59 ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox