Re: [PATCH 13/23] drm/amd/display: Support DAC in dce110_hwseq

[Harry Wentland <harry.wentland@amd.com>]

On 2025-09-26 14:01, Timur Kristóf wrote:
> The dce110_hwseq is used by all DCE hardware,
> so add the DAC support here.
> 
> When enabling/disabling a stream for a RGB signal,
> this will call the VBIOS to enable/disable the DAC.
> Additionally, when applying the controller context,
> call SelectCRTC_Source from VBIOS in order to
> direct the CRTC output to the DAC.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>  .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 75 ++++++++++++++++++-
>  1 file changed, 73 insertions(+), 2 deletions(-)
> 
> 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 24184b4eb352..975913375d05 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
> @@ -659,6 +659,20 @@ void dce110_update_info_frame(struct pipe_ctx *pipe_ctx)
>  	}
>  }
>  
> +static void
> +dce110_dac_encoder_control(struct pipe_ctx *pipe_ctx, bool enable)
> +{
> +	struct dc_link *link = pipe_ctx->stream->link;
> +	struct dc_bios *bios = link->ctx->dc_bios;
> +	struct bp_encoder_control encoder_control = {0};
> +
> +	encoder_control.action = enable ? ENCODER_CONTROL_ENABLE : ENCODER_CONTROL_DISABLE;
> +	encoder_control.engine_id = link->link_enc->analog_engine;
> +	encoder_control.pixel_clock = pipe_ctx->stream->timing.pix_clk_100hz / 10;
> +
> +	bios->funcs->encoder_control(bios, &encoder_control);
> +}
> +
>  void dce110_enable_stream(struct pipe_ctx *pipe_ctx)
>  {
>  	enum dc_lane_count lane_count =
> @@ -689,6 +703,9 @@ void dce110_enable_stream(struct pipe_ctx *pipe_ctx)
>  		early_control = lane_count;
>  
>  	tg->funcs->set_early_control(tg, early_control);
> +
> +	if (dc_is_rgb_signal(pipe_ctx->stream->signal))
> +		dce110_dac_encoder_control(pipe_ctx, true);
>  }
>  
>  static enum bp_result link_transmitter_control(
> @@ -1176,7 +1193,8 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx)
>  		pipe_ctx->stream_res.stream_enc->funcs->stop_dp_info_packets(
>  			pipe_ctx->stream_res.stream_enc);
>  
> -	dc->hwss.disable_audio_stream(pipe_ctx);
> +	if (!dc_is_rgb_signal(pipe_ctx->stream->signal))
> +		dc->hwss.disable_audio_stream(pipe_ctx);

Nit: this hunk would fit better in Patch 7.

Harry

>  
>  	link_hwss->reset_stream_encoder(pipe_ctx);
>  
> @@ -1196,6 +1214,9 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx)
>  		dccg->funcs->disable_symclk_se(dccg, stream_enc->stream_enc_inst,
>  					       link_enc->transmitter - TRANSMITTER_UNIPHY_A);
>  	}
> +
> +	if (dc_is_rgb_signal(pipe_ctx->stream->signal))
> +		dce110_dac_encoder_control(pipe_ctx, false);
>  }
>  
>  void dce110_unblank_stream(struct pipe_ctx *pipe_ctx,
> @@ -1581,6 +1602,51 @@ static enum dc_status dce110_enable_stream_timing(
>  	return DC_OK;
>  }
>  
> +static void
> +dce110_select_crtc_source(struct pipe_ctx *pipe_ctx)
> +{
> +	struct dc_link *link = pipe_ctx->stream->link;
> +	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.engine_id = engine_id;
> +	crtc_source_select.sink_signal = pipe_ctx->stream->signal;
> +
> +	bios->funcs->select_crtc_source(bios, &crtc_source_select);
> +}
> +
>  enum dc_status dce110_apply_single_controller_ctx_to_hw(
>  		struct pipe_ctx *pipe_ctx,
>  		struct dc_state *context,
> @@ -1600,6 +1666,10 @@ enum dc_status dce110_apply_single_controller_ctx_to_hw(
>  		hws->funcs.disable_stream_gating(dc, pipe_ctx);
>  	}
>  
> +	if (pipe_ctx->stream->signal == SIGNAL_TYPE_RGB) {
> +		dce110_select_crtc_source(pipe_ctx);
> +	}
> +
>  	if (pipe_ctx->stream_res.audio != NULL) {
>  		struct audio_output audio_output = {0};
>  
> @@ -1679,7 +1749,8 @@ enum dc_status dce110_apply_single_controller_ctx_to_hw(
>  		pipe_ctx->stream_res.tg->funcs->set_static_screen_control(
>  				pipe_ctx->stream_res.tg, event_triggers, 2);
>  
> -	if (!dc_is_virtual_signal(pipe_ctx->stream->signal))
> +	if (!dc_is_virtual_signal(pipe_ctx->stream->signal) &&
> +		!dc_is_rgb_signal(pipe_ctx->stream->signal))
>  		pipe_ctx->stream_res.stream_enc->funcs->dig_connect_to_otg(
>  			pipe_ctx->stream_res.stream_enc,
>  			pipe_ctx->stream_res.tg->inst);