[PATCH 15/73] drm/amd/display: Don't use dc_link in link_encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/09/2017 03:05 PM, Harry Wentland wrote:
> From: Andrew Jiang <Andrew.Jiang at amd.com>
>
> dc_link is at a higher level than link_encoder, and we only want
> higher-level components to be able to access lower-level ones,
> not the other way around.
>
> Change-Id: I634b117b386938fb7ddba50c50484fadd54ad485
> Signed-off-by: Andrew Jiang <Andrew.Jiang at amd.com>
> Reviewed-by: Tony Cheng <Tony.Cheng at amd.com>
> Acked-by: Harry Wentland <harry.wentland at amd.com>
> ---
>   drivers/gpu/drm/amd/display/dc/core/dc_link.c      |  2 +-
>   drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c | 11 +++---
>   .../gpu/drm/amd/display/dc/dce/dce_link_encoder.c  | 34 +++++++---------
>   .../gpu/drm/amd/display/dc/dce/dce_link_encoder.h  |  5 +--
>   .../amd/display/dc/dce110/dce110_hw_sequencer.c    | 46 ++++++++++++----------
>   .../amd/display/dc/dce110/dce110_hw_sequencer.h    |  4 +-
>   .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c  |  3 ++
>   .../gpu/drm/amd/display/dc/inc/hw/link_encoder.h   |  2 +-
>   drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h  |  2 +-
>   .../amd/display/dc/virtual/virtual_link_encoder.c  |  3 +-
>   10 files changed, 57 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index a6a762a26fd2..3b394a5f1c66 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -1798,7 +1798,7 @@ static void disable_link(struct dc_link *link, enum signal_type signal)
>   		else
>   			dp_disable_link_phy_mst(link, signal);
>   	} else
> -		link->link_enc->funcs->disable_output(link->link_enc, signal, link);
> +		link->link_enc->funcs->disable_output(link->link_enc, signal);
>   }
>   
>   bool dp_active_dongle_validate_timing(
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
> index 9a33b471270a..f2902569be2e 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
> @@ -89,7 +89,7 @@ void dp_enable_link_phy(
>   
>   	if (dc_is_dp_sst_signal(signal)) {
>   		if (signal == SIGNAL_TYPE_EDP) {
> -			link->dc->hwss.edp_power_control(link->link_enc, true);
> +			link->dc->hwss.edp_power_control(link, true);
>   			link_enc->funcs->enable_dp_output(
>   						link_enc,
>   						link_settings,
> @@ -140,10 +140,10 @@ void dp_disable_link_phy(struct dc_link *link, enum signal_type signal)
>   	if (signal == SIGNAL_TYPE_EDP) {
>   		link->dc->hwss.edp_backlight_control(link, false);
>   		edp_receiver_ready_T9(link);
> -		link->link_enc->funcs->disable_output(link->link_enc, signal, link);
> -		link->dc->hwss.edp_power_control(link->link_enc, false);
> +		link->link_enc->funcs->disable_output(link->link_enc, signal);
> +		link->dc->hwss.edp_power_control(link, false);
>   	} else
> -		link->link_enc->funcs->disable_output(link->link_enc, signal, link);
> +		link->link_enc->funcs->disable_output(link->link_enc, signal);
>   
>   	/* Clear current link setting.*/
>   	memset(&link->cur_link_settings, 0,
> @@ -286,8 +286,7 @@ void dp_retrain_link_dp_test(struct dc_link *link,
>   
>   			link->link_enc->funcs->disable_output(
>   					link->link_enc,
> -					SIGNAL_TYPE_DISPLAY_PORT,
> -					link);
> +					SIGNAL_TYPE_DISPLAY_PORT);
>   
>   			/* Clear current link setting. */
>   			memset(&link->cur_link_settings, 0,
> 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 fe88852b4774..bad70c6b3aad 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
> @@ -845,8 +845,6 @@ void dce110_link_encoder_hw_init(
>   
>   		ASSERT(result == BP_RESULT_OK);
>   
> -	} else if (enc110->base.connector.id == CONNECTOR_ID_EDP) {
> -		ctx->dc->hwss.edp_power_control(enc, true);
>   	}
>   	aux_initialize(enc110);
>   
> @@ -1033,8 +1031,7 @@ void dce110_link_encoder_enable_dp_mst_output(
>    */
>   void dce110_link_encoder_disable_output(
>   	struct link_encoder *enc,
> -	enum signal_type signal,
> -	struct dc_link *link)
> +	enum signal_type signal)
>   {
>   	struct dce110_link_encoder *enc110 = TO_DCE110_LINK_ENC(enc);
>   	struct dc_context *ctx = enc110->base.ctx;
> @@ -1045,8 +1042,6 @@ void dce110_link_encoder_disable_output(
>   		/* OF_SKIP_POWER_DOWN_INACTIVE_ENCODER */
>   		return;
>   	}
> -	if (enc110->base.connector.id == CONNECTOR_ID_EDP)
> -		ctx->dc->hwss.edp_backlight_control(link, false);
>   	/* Power-down RX and disable GPU PHY should be paired.
>   	 * Disabling PHY without powering down RX may cause
>   	 * symbol lock loss, on which we will get DP Sink interrupt. */
> @@ -1078,19 +1073,20 @@ void dce110_link_encoder_disable_output(
>   	if (dc_is_dp_signal(signal))
>   		link_encoder_disable(enc110);
>   
> -	if (enc110->base.connector.id == CONNECTOR_ID_EDP) {
> -		/* power down eDP panel */
> -		/* TODO: Power control cause regression, we should implement
> -		 * it properly, for now just comment it.
> -		 *
> -		 * link_encoder_edp_wait_for_hpd_ready(
> -			link_enc,
> -			link_enc->connector,
> -			false);
> -
> -		 * link_encoder_edp_power_control(
> -				link_enc, false); */
> -	}
> +	/*
> +	 * TODO: Power control cause regression, we should implement
> +	 * it properly, for now just comment it.
> +	 */
> +//	if (enc110->base.connector.id == CONNECTOR_ID_EDP) {
> +//		/* power down eDP panel */
> +//		link_encoder_edp_wait_for_hpd_ready(
> +//				enc,
> +//				enc->connector,
> +//				false);
> +//
> +//		link_encoder_edp_power_control(
> +//				enc, false);
> +//	}
>   }

This is wrong comment style, please check 
https://www.kernel.org/doc/html/v4.10/process/coding-style.html , section 8.

Thanks,
Andrey

>   
>   void dce110_link_encoder_dp_set_lane_settings(
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.h b/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.h
> index 494067dedd03..8ca9afe47a2b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.h
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.h
> @@ -228,9 +228,8 @@ void dce110_link_encoder_enable_dp_mst_output(
>   
>   /* disable PHY output */
>   void dce110_link_encoder_disable_output(
> -	struct link_encoder *link_enc,
> -	enum signal_type signal,
> -	struct dc_link *link);
> +	struct link_encoder *enc,
> +	enum signal_type signal);
>   
>   /* set DP lane settings */
>   void dce110_link_encoder_dp_set_lane_settings(
> diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
> index b4504f1f49c0..4135de2d7203 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c
> @@ -814,11 +814,11 @@ static enum bp_result link_transmitter_control(
>    * eDP only.
>    */
>   void hwss_edp_wait_for_hpd_ready(
> -	struct link_encoder *enc,
> -	bool power_up)
> +		struct dc_link *link,
> +		bool power_up)
>   {
> -	struct dc_context *ctx = enc->ctx;
> -	struct graphics_object_id connector = enc->connector;
> +	struct dc_context *ctx = link->ctx;
> +	struct graphics_object_id connector = link->link_enc->connector;
>   	struct gpio *hpd;
>   	bool edp_hpd_high = false;
>   	uint32_t time_elapsed = 0;
> @@ -882,16 +882,16 @@ void hwss_edp_wait_for_hpd_ready(
>   }
>   
>   void hwss_edp_power_control(
> -	struct link_encoder *enc,
> -	bool power_up)
> +		struct dc_link *link,
> +		bool power_up)
>   {
> -	struct dc_context *ctx = enc->ctx;
> +	struct dc_context *ctx = link->ctx;
>   	struct dce_hwseq *hwseq = ctx->dc->hwseq;
>   	struct bp_transmitter_control cntl = { 0 };
>   	enum bp_result bp_result;
>   
>   
> -	if (dal_graphics_object_id_get_connector_id(enc->connector)
> +	if (dal_graphics_object_id_get_connector_id(link->link_enc->connector)
>   			!= CONNECTOR_ID_EDP) {
>   		BREAK_TO_DEBUGGER();
>   		return;
> @@ -907,11 +907,11 @@ void hwss_edp_power_control(
>   		cntl.action = power_up ?
>   			TRANSMITTER_CONTROL_POWER_ON :
>   			TRANSMITTER_CONTROL_POWER_OFF;
> -		cntl.transmitter = enc->transmitter;
> -		cntl.connector_obj_id = enc->connector;
> +		cntl.transmitter = link->link_enc->transmitter;
> +		cntl.connector_obj_id = link->link_enc->connector;
>   		cntl.coherent = false;
>   		cntl.lanes_number = LANE_COUNT_FOUR;
> -		cntl.hpd_sel = enc->hpd_source;
> +		cntl.hpd_sel = link->link_enc->hpd_source;
>   
>   		bp_result = link_transmitter_control(ctx->dc_bios, &cntl);
>   
> @@ -925,7 +925,7 @@ void hwss_edp_power_control(
>   				__func__, (power_up ? "On":"Off"));
>   	}
>   
> -	hwss_edp_wait_for_hpd_ready(enc, true);
> +	hwss_edp_wait_for_hpd_ready(link, true);
>   }
>   
>   /*todo: cloned in stream enc, fix*/
> @@ -934,14 +934,14 @@ void hwss_edp_power_control(
>    * eDP only. Control the backlight of the eDP panel
>    */
>   void hwss_edp_backlight_control(
> -	struct dc_link *link,
> -	bool enable)
> +		struct dc_link *link,
> +		bool enable)
>   {
> -	struct dce_hwseq *hws = link->dc->hwseq;
> -	struct dc_context *ctx = link->dc->ctx;
> +	struct dc_context *ctx = link->ctx;
> +	struct dce_hwseq *hws = ctx->dc->hwseq;
>   	struct bp_transmitter_control cntl = { 0 };
>   
> -	if (dal_graphics_object_id_get_connector_id(link->link_id)
> +	if (dal_graphics_object_id_get_connector_id(link->link_enc->connector)
>   		!= CONNECTOR_ID_EDP) {
>   		BREAK_TO_DEBUGGER();
>   		return;
> @@ -982,7 +982,7 @@ void hwss_edp_backlight_control(
>   	 * Enable it in the future if necessary.
>   	 */
>   	/* dc_service_sleep_in_milliseconds(50); */
> -	link_transmitter_control(link->dc->ctx->dc_bios, &cntl);
> +	link_transmitter_control(ctx->dc_bios, &cntl);
>   }
>   
>   void dce110_disable_stream(struct pipe_ctx *pipe_ctx, int option)
> @@ -1398,12 +1398,14 @@ static void power_down_encoders(struct dc *dc)
>   
>   			if (!dc->links[i]->wa_flags.dp_keep_receiver_powered)
>   				dp_receiver_power_ctrl(dc->links[i], false);
> -			if (connector_id == CONNECTOR_ID_EDP)
> +			if (connector_id == CONNECTOR_ID_EDP) {
>   				signal = SIGNAL_TYPE_EDP;
> +				hwss_edp_backlight_control(dc->links[i], false);
> +			}
>   		}
>   
>   		dc->links[i]->link_enc->funcs->disable_output(
> -				dc->links[i]->link_enc, signal, dc->links[i]);
> +				dc->links[i]->link_enc, signal);
>   	}
>   }
>   
> @@ -2539,6 +2541,10 @@ static void init_hw(struct dc *dc)
>   		 * required signal (which may be different from the
>   		 * default signal on connector). */
>   		struct dc_link *link = dc->links[i];
> +
> +		if (link->link_enc->connector.id == CONNECTOR_ID_EDP)
> +			dc->hwss.edp_power_control(link, true);
> +
>   		link->link_enc->funcs->hw_init(link->link_enc);
>   	}
>   
> diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.h
> index 4d72bb99be93..2dd6ac637572 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.h
> +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.h
> @@ -70,8 +70,8 @@ uint32_t dce110_get_min_vblank_time_us(const struct dc_state *context);
>   void dp_receiver_power_ctrl(struct dc_link *link, bool on);
>   
>   void hwss_edp_power_control(
> -	struct link_encoder *enc,
> -	bool power_up);
> +		struct dc_link *link,
> +		bool power_up);
>   
>   void hwss_edp_backlight_control(
>   	struct dc_link *link,
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index dc37551399ba..51b7cfe9581f 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -723,6 +723,9 @@ static void dcn10_init_hw(struct dc *dc)
>   		 */
>   		struct dc_link *link = dc->links[i];
>   
> +		if (link->link_enc->connector.id == CONNECTOR_ID_EDP)
> +			dc->hwss.edp_power_control(link, true);
> +
>   		link->link_enc->funcs->hw_init(link->link_enc);
>   	}
>   
> 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 3d33bcda7059..8a08f0a97f94 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
> @@ -111,7 +111,7 @@ struct link_encoder_funcs {
>   		const struct dc_link_settings *link_settings,
>   		enum clock_source_id clock_source);
>   	void (*disable_output)(struct link_encoder *link_enc,
> -		enum signal_type signal, struct dc_link *link);
> +		enum signal_type signal);
>   	void (*dp_set_lane_settings)(struct link_encoder *enc,
>   		const struct link_training_settings *link_settings);
>   	void (*dp_set_phy_pattern)(struct link_encoder *enc,
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> index cebbba345889..f3c5468854bd 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> @@ -184,7 +184,7 @@ struct hw_sequencer_funcs {
>   	void (*ready_shared_resources)(struct dc *dc, struct dc_state *context);
>   	void (*optimize_shared_resources)(struct dc *dc);
>   	void (*edp_power_control)(
> -			struct link_encoder *enc,
> +			struct dc_link *link,
>   			bool enable);
>   	void (*edp_backlight_control)(
>   			struct dc_link *link,
> diff --git a/drivers/gpu/drm/amd/display/dc/virtual/virtual_link_encoder.c b/drivers/gpu/drm/amd/display/dc/virtual/virtual_link_encoder.c
> index 88c2bde3f039..57a54a7b89e5 100644
> --- a/drivers/gpu/drm/amd/display/dc/virtual/virtual_link_encoder.c
> +++ b/drivers/gpu/drm/amd/display/dc/virtual/virtual_link_encoder.c
> @@ -58,8 +58,7 @@ static void virtual_link_encoder_enable_dp_mst_output(
>   
>   static void virtual_link_encoder_disable_output(
>   	struct link_encoder *link_enc,
> -	enum signal_type signal,
> -	struct dc_link *link) {}
> +	enum signal_type signal) {}
>   
>   static void virtual_link_encoder_dp_set_lane_settings(
>   	struct link_encoder *enc,



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux