[PATCH 15/73] drm/amd/display: Don't use dc_link in link_encoder
Andrey Grodzovsky
Andrey.Grodzovsky at amd.com
Fri Nov 10 16:44:23 UTC 2017
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,
More information about the amd-gfx
mailing list