[PATCH 0/3][SRU][Bionic] amdgpu with mst WARNING on blanking

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/3][SRU][Bionic] amdgpu with mst WARNING on blanking

Dan Streetman
From: Dan Streetman <[hidden email]>

This series fixes amdgpu screen blanking when using displayport monitors
in MST configuration (i.e. displayport "daisy chaining").

In Cosmic, only the 3rd patch is needed (first 2 are already included),
and I sent that separately.

Charlene Liu (1):
  drm/amd/display: eDP sequence BL off first then DP blank.

Jerry (Fangzhi) Zuo (1):
  drm/amd/display: Fix MST dp_blank REG_WAIT timeout

Yongqiang Sun (1):
  drm/amd/display: Move wait for hpd ready out from edp power control.

 drivers/gpu/drm/amd/display/dc/core/dc_link.c | 24 ++++++++++++--
 .../drm/amd/display/dc/core/dc_link_hwss.c    | 11 +------
 .../drm/amd/display/dc/dce/dce_link_encoder.c | 17 ++--------
 .../display/dc/dce110/dce110_hw_sequencer.c   | 33 +++++++++++++------
 .../display/dc/dce110/dce110_hw_sequencer.h   |  5 +++
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  4 ++-
 .../gpu/drm/amd/display/dc/inc/hw_sequencer.h |  3 ++
 7 files changed, 60 insertions(+), 37 deletions(-)

--
2.19.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/3][SRU][Bionic] drm/amd/display: Move wait for hpd ready out from edp power control.

Dan Streetman
From: Yongqiang Sun <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1814308

It may take over 200ms for wait hpd ready. To optimize the resume time,
we can power on eDP in init_hw, wait for hpd ready when doing link
training.

also create separate eDP enable function to make sure eDP is powered up
before doing and DPCD access, as HPD low will result in DPDC transaction
failure.

After optimization,
setpowerstate 145ms -> 9.8ms,
DPMS 387ms -> 18.9ms

Signed-off-by: Yongqiang Sun <[hidden email]>
Signed-off-by: Tony Cheng <[hidden email]>
Reviewed-by: Tony Cheng <[hidden email]>
Acked-by: Harry Wentland <[hidden email]>
Signed-off-by: Alex Deucher <[hidden email]>
(backported from commit 904623ee5936e2226009b2f238f28781aecd2565)
Signed-off-by: Dan Streetman <[hidden email]>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c | 25 ++++++++++++++++++-
 .../drm/amd/display/dc/core/dc_link_hwss.c    | 11 +-------
 .../drm/amd/display/dc/dce/dce_link_encoder.c | 14 -----------
 .../display/dc/dce110/dce110_hw_sequencer.c   | 12 +++------
 .../display/dc/dce110/dce110_hw_sequencer.h   |  4 +++
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  3 ++-
 .../gpu/drm/amd/display/dc/inc/hw_sequencer.h |  2 ++
 7 files changed, 36 insertions(+), 35 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 42a111b9505d..ca0adf46d1e8 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1271,6 +1271,24 @@ static enum dc_status enable_link_dp(
  return status;
 }
 
+static enum dc_status enable_link_edp(
+ struct dc_state *state,
+ struct pipe_ctx *pipe_ctx)
+{
+ enum dc_status status;
+ struct dc_stream_state *stream = pipe_ctx->stream;
+ struct dc_link *link = stream->sink->link;
+
+ link->dc->hwss.edp_power_control(link->link_enc, true);
+ link->dc->hwss.edp_wait_for_hpd_ready(link->link_enc, true);
+
+ status = enable_link_dp(state, pipe_ctx);
+
+ link->dc->hwss.edp_backlight_control(link, true);
+
+ return status;
+}
+
 static enum dc_status enable_link_dp_mst(
  struct dc_state *state,
  struct pipe_ctx *pipe_ctx)
@@ -1746,9 +1764,11 @@ static enum dc_status enable_link(
  enum dc_status status = DC_ERROR_UNEXPECTED;
  switch (pipe_ctx->stream->signal) {
  case SIGNAL_TYPE_DISPLAY_PORT:
- case SIGNAL_TYPE_EDP:
  status = enable_link_dp(state, pipe_ctx);
  break;
+ case SIGNAL_TYPE_EDP:
+ status = enable_link_edp(state, pipe_ctx);
+ break;
  case SIGNAL_TYPE_DISPLAY_PORT_MST:
  status = enable_link_dp_mst(state, pipe_ctx);
  msleep(200);
@@ -2420,6 +2440,9 @@ void core_link_disable_stream(struct pipe_ctx *pipe_ctx, int option)
  if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
  deallocate_mst_payload(pipe_ctx);
 
+ if (pipe_ctx->stream->signal == SIGNAL_TYPE_EDP)
+ core_dc->hwss.edp_backlight_control(pipe_ctx->stream->sink->link, false);
+
  core_dc->hwss.disable_stream(pipe_ctx, option);
 
  disable_link(pipe_ctx->stream->sink->link, pipe_ctx->stream->signal);
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..9852e766d7fb 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
@@ -88,15 +88,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_enc->funcs->enable_dp_output(
- link_enc,
- link_settings,
- clock_source);
- link->dc->hwss.edp_backlight_control(link, true);
- } else
- link_enc->funcs->enable_dp_output(
+ link_enc->funcs->enable_dp_output(
  link_enc,
  link_settings,
  clock_source);
@@ -138,7 +130,6 @@ void dp_disable_link_phy(struct dc_link *link, enum signal_type signal)
  dp_receiver_power_ctrl(link, false);
 
  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);
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 00c728260616..b24f8c70ced8 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
@@ -1087,20 +1087,6 @@ void dce110_link_encoder_disable_output(
  /* disable encoder */
  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); */
- }
 }
 
 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 d844fadcd56f..6847fbe8feff 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
@@ -924,8 +924,6 @@ void hwss_edp_power_control(
  "%s: Skipping Panel Power action: %s\n",
  __func__, (power_up ? "On":"Off"));
  }
-
- hwss_edp_wait_for_hpd_ready(enc, true);
 }
 
 /*todo: cloned in stream enc, fix*/
@@ -1026,11 +1024,9 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx, int option)
  }
 
  /* blank at encoder level */
- if (dc_is_dp_signal(pipe_ctx->stream->signal)) {
- if (pipe_ctx->stream->sink->link->connector_signal == SIGNAL_TYPE_EDP)
- hwss_edp_backlight_control(link, false);
+ if (dc_is_dp_signal(pipe_ctx->stream->signal))
  pipe_ctx->stream_res.stream_enc->funcs->dp_blank(pipe_ctx->stream_res.stream_enc);
- }
+
  link->link_enc->funcs->connect_dig_be_to_fe(
  link->link_enc,
  pipe_ctx->stream_res.stream_enc->id,
@@ -1042,15 +1038,12 @@ void dce110_unblank_stream(struct pipe_ctx *pipe_ctx,
  struct dc_link_settings *link_settings)
 {
  struct encoder_unblank_param params = { { 0 } };
- struct dc_link *link = pipe_ctx->stream->sink->link;
 
  /* only 3 items below are used by unblank */
  params.pixel_clk_khz =
  pipe_ctx->stream->timing.pix_clk_khz;
  params.link_settings.link_rate = link_settings->link_rate;
  pipe_ctx->stream_res.stream_enc->funcs->dp_unblank(pipe_ctx->stream_res.stream_enc, &params);
- if (link->connector_signal == SIGNAL_TYPE_EDP)
- hwss_edp_backlight_control(link, true);
 }
 
 
@@ -2998,6 +2991,7 @@ static const struct hw_sequencer_funcs dce110_funcs = {
  .optimize_shared_resources = optimize_shared_resources,
  .edp_backlight_control = hwss_edp_backlight_control,
  .edp_power_control = hwss_edp_power_control,
+ .edp_wait_for_hpd_ready = hwss_edp_wait_for_hpd_ready,
 };
 
 void dce110_hw_sequencer_construct(struct dc *dc)
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..9e057148f3e1 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
@@ -77,5 +77,9 @@ void hwss_edp_backlight_control(
  struct dc_link *link,
  bool enable);
 
+void hwss_edp_wait_for_hpd_ready(
+ struct link_encoder *enc,
+ bool power_up);
+
 #endif /* __DC_HWSS_DCE110_H__ */
 
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 cc44ce591900..50d9492b57a3 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
@@ -2973,7 +2973,8 @@ static const struct hw_sequencer_funcs dcn10_funcs = {
  .ready_shared_resources = ready_shared_resources,
  .optimize_shared_resources = optimize_shared_resources,
  .edp_backlight_control = hwss_edp_backlight_control,
- .edp_power_control = hwss_edp_power_control
+ .edp_power_control = hwss_edp_power_control,
+ .edp_wait_for_hpd_ready = hwss_edp_wait_for_hpd_ready,
 };
 
 
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 8734689a9245..62837e176383 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
@@ -184,6 +184,8 @@ struct hw_sequencer_funcs {
  void (*edp_backlight_control)(
  struct dc_link *link,
  bool enable);
+ void (*edp_wait_for_hpd_ready)(struct link_encoder *enc, bool power_up);
+
 };
 
 void color_space_to_black_color(
--
2.19.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/3][SRU][Bionic] drm/amd/display: eDP sequence BL off first then DP blank.

Dan Streetman
In reply to this post by Dan Streetman
From: Charlene Liu <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1814308

Signed-off-by: Charlene Liu <[hidden email]>
Reviewed-by: Anthony Koo <[hidden email]>
Acked-by: Harry Wentland <[hidden email]>
Signed-off-by: Alex Deucher <[hidden email]>
(backported from commit 41b497421a1f07ab99814da740984f907747120b)
Signed-off-by: Dan Streetman <[hidden email]>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c |  7 ++-----
 .../drm/amd/display/dc/dce/dce_link_encoder.c |  3 +++
 .../display/dc/dce110/dce110_hw_sequencer.c   | 21 ++++++++++++++++++-
 .../display/dc/dce110/dce110_hw_sequencer.h   |  1 +
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  1 +
 .../gpu/drm/amd/display/dc/inc/hw_sequencer.h |  1 +
 6 files changed, 28 insertions(+), 6 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 ca0adf46d1e8..d3cfe467d43d 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1279,13 +1279,12 @@ static enum dc_status enable_link_edp(
  struct dc_stream_state *stream = pipe_ctx->stream;
  struct dc_link *link = stream->sink->link;
 
+ /*in case it is not on*/
  link->dc->hwss.edp_power_control(link->link_enc, true);
  link->dc->hwss.edp_wait_for_hpd_ready(link->link_enc, true);
 
  status = enable_link_dp(state, pipe_ctx);
 
- link->dc->hwss.edp_backlight_control(link, true);
-
  return status;
 }
 
@@ -2428,7 +2427,6 @@ void core_link_enable_stream(
  if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
  allocate_mst_payload(pipe_ctx);
 
- if (dc_is_dp_signal(pipe_ctx->stream->signal))
  core_dc->hwss.unblank_stream(pipe_ctx,
  &pipe_ctx->stream->sink->link->cur_link_settings);
 }
@@ -2440,8 +2438,7 @@ void core_link_disable_stream(struct pipe_ctx *pipe_ctx, int option)
  if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
  deallocate_mst_payload(pipe_ctx);
 
- if (pipe_ctx->stream->signal == SIGNAL_TYPE_EDP)
- core_dc->hwss.edp_backlight_control(pipe_ctx->stream->sink->link, false);
+ core_dc->hwss.blank_stream(pipe_ctx);
 
  core_dc->hwss.disable_stream(pipe_ctx, option);
 
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 b24f8c70ced8..5caff3aac9d7 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
@@ -838,6 +838,9 @@ void dce110_link_encoder_hw_init(
  cntl.coherent = false;
  cntl.hpd_sel = enc110->base.hpd_source;
 
+ if (enc110->base.connector.id == CONNECTOR_ID_EDP)
+ cntl.signal = SIGNAL_TYPE_EDP;
+
  result = link_transmitter_control(enc110, &cntl);
 
  if (result != BP_RESULT_OK) {
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 6847fbe8feff..a54765873b15 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
@@ -1038,12 +1038,30 @@ void dce110_unblank_stream(struct pipe_ctx *pipe_ctx,
  struct dc_link_settings *link_settings)
 {
  struct encoder_unblank_param params = { { 0 } };
+ struct dc_stream_state *stream = pipe_ctx->stream;
+ struct dc_link *link = stream->sink->link;
 
  /* only 3 items below are used by unblank */
  params.pixel_clk_khz =
  pipe_ctx->stream->timing.pix_clk_khz;
  params.link_settings.link_rate = link_settings->link_rate;
- pipe_ctx->stream_res.stream_enc->funcs->dp_unblank(pipe_ctx->stream_res.stream_enc, &params);
+
+ if (dc_is_dp_signal(pipe_ctx->stream->signal))
+ pipe_ctx->stream_res.stream_enc->funcs->dp_unblank(pipe_ctx->stream_res.stream_enc, &params);
+
+ if (link->local_sink && link->local_sink->sink_signal == SIGNAL_TYPE_EDP)
+ link->dc->hwss.edp_backlight_control(link, true);
+}
+void dce110_blank_stream(struct pipe_ctx *pipe_ctx)
+{
+ struct dc_stream_state *stream = pipe_ctx->stream;
+ struct dc_link *link = stream->sink->link;
+
+ if (link->local_sink && link->local_sink->sink_signal == SIGNAL_TYPE_EDP)
+ link->dc->hwss.edp_backlight_control(link, false);
+
+ if (dc_is_dp_signal(pipe_ctx->stream->signal))
+ pipe_ctx->stream_res.stream_enc->funcs->dp_blank(pipe_ctx->stream_res.stream_enc);
 }
 
 
@@ -2974,6 +2992,7 @@ static const struct hw_sequencer_funcs dce110_funcs = {
  .enable_stream = dce110_enable_stream,
  .disable_stream = dce110_disable_stream,
  .unblank_stream = dce110_unblank_stream,
+ .blank_stream = dce110_blank_stream,
  .enable_display_pipe_clock_gating = enable_display_pipe_clock_gating,
  .enable_display_power_gating = dce110_enable_display_power_gating,
  .power_down_front_end = dce110_power_down_fe,
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 9e057148f3e1..5d5912f15205 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
@@ -52,6 +52,7 @@ void dce110_disable_stream(struct pipe_ctx *pipe_ctx, int option);
 void dce110_unblank_stream(struct pipe_ctx *pipe_ctx,
  struct dc_link_settings *link_settings);
 
+void dce110_blank_stream(struct pipe_ctx *pipe_ctx);
 void dce110_update_info_frame(struct pipe_ctx *pipe_ctx);
 
 void dce110_set_avmute(struct pipe_ctx *pipe_ctx, bool enable);
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 50d9492b57a3..5e5f202a1ad1 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
@@ -2956,6 +2956,7 @@ static const struct hw_sequencer_funcs dcn10_funcs = {
  .enable_stream = dce110_enable_stream,
  .disable_stream = dce110_disable_stream,
  .unblank_stream = dce110_unblank_stream,
+ .blank_stream = dce110_blank_stream,
  .enable_display_power_gating = dcn10_dummy_display_power_gating,
  .power_down_front_end = dcn10_power_down_fe,
  .power_on_front_end = dcn10_power_on_fe,
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 62837e176383..e8fd420120da 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
@@ -140,6 +140,7 @@ struct hw_sequencer_funcs {
  void (*unblank_stream)(struct pipe_ctx *pipe_ctx,
  struct dc_link_settings *link_settings);
 
+ void (*blank_stream)(struct pipe_ctx *pipe_ctx);
  void (*pipe_control_lock)(
  struct dc *dc,
  struct pipe_ctx *pipe,
--
2.19.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/3][SRU][Bionic] drm/amd/display: Fix MST dp_blank REG_WAIT timeout

Dan Streetman
In reply to this post by Dan Streetman
From: "Jerry (Fangzhi) Zuo" <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1814308

Need to blank stream before deallocate MST payload.

[drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 10us * 3000 tries - dce110_stream_encoder_dp_blank line:944
------------[ cut here ]------------
WARNING: CPU: 0 PID: 2201 at /var/lib/dkms/amdgpu/18.50-690240/build/amd/amdgpu/../display/dc/dc_helper.c:249 generic_reg_wait+0xe7/0x160 [amdgpu]
Call Trace:
 dce110_stream_encoder_dp_blank+0x11c/0x180 [amdgpu]
 core_link_disable_stream+0x40/0x230 [amdgpu]
 ? generic_reg_update_ex+0xdb/0x130 [amdgpu]
 dce110_reset_hw_ctx_wrap+0xb7/0x1f0 [amdgpu]
 dce110_apply_ctx_to_hw+0x30/0x430 [amdgpu]
 ? dce110_apply_ctx_for_surface+0x206/0x260 [amdgpu]
 dc_commit_state+0x2ba/0x4d0 [amdgpu]
 amdgpu_dm_atomic_commit_tail+0x297/0xd70 [amdgpu]
 ? amdgpu_bo_pin_restricted+0x58/0x260 [amdgpu]
 ? wait_for_completion_timeout+0x1f/0x120
 ? wait_for_completion_interruptible+0x1c/0x160
 commit_tail+0x3d/0x60 [drm_kms_helper]
 drm_atomic_helper_commit+0xf6/0x100 [drm_kms_helper]
 drm_atomic_connector_commit_dpms+0xe5/0xf0 [drm]
 drm_mode_obj_set_property_ioctl+0x14f/0x250 [drm]
 drm_mode_connector_property_set_ioctl+0x2e/0x40 [drm]
 drm_ioctl+0x1e0/0x430 [drm]
 ? drm_mode_connector_set_obj_prop+0x70/0x70 [drm]
 ? ep_read_events_proc+0xb0/0xb0
 ? ep_scan_ready_list.constprop.18+0x1e6/0x1f0
 ? timerqueue_add+0x52/0x80
 amdgpu_drm_ioctl+0x49/0x80 [amdgpu]
 do_vfs_ioctl+0x90/0x5f0
 SyS_ioctl+0x74/0x80
 do_syscall_64+0x74/0x140
 entry_SYSCALL_64_after_hwframe+0x3d/0xa2
---[ end trace 3ed7b77a97d60f72 ]---

Signed-off-by: Jerry (Fangzhi) Zuo <[hidden email]>
Reviewed-by: Hersen Wu <[hidden email]>
Acked-by: Harry Wentland <[hidden email]>
Acked-by: Alex Deucher <[hidden email]>
Tested-by: Lyude Paul <[hidden email]>
Signed-off-by: Alex Deucher <[hidden email]>
Cc: [hidden email]
(cherry-picked from commit 8c9d90eebd23b6d40ddf4ce5df5ca2b932336a06)
Signed-off-by: Dan Streetman <[hidden email]>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c | 4 ++--
 1 file changed, 2 insertions(+), 2 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 d3cfe467d43d..b0555abec7cf 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -2435,11 +2435,11 @@ void core_link_disable_stream(struct pipe_ctx *pipe_ctx, int option)
 {
  struct dc  *core_dc = pipe_ctx->stream->ctx->dc;
 
+ core_dc->hwss.blank_stream(pipe_ctx);
+
  if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
  deallocate_mst_payload(pipe_ctx);
 
- core_dc->hwss.blank_stream(pipe_ctx);
-
  core_dc->hwss.disable_stream(pipe_ctx, option);
 
  disable_link(pipe_ctx->stream->sink->link, pipe_ctx->stream->signal);
--
2.19.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/3][SRU][Bionic] amdgpu with mst WARNING on blanking

Stefan Bader-2
In reply to this post by Dan Streetman
On 06.02.19 20:24, Dan Streetman wrote:
> From: Dan Streetman <[hidden email]>
>
> This series fixes amdgpu screen blanking when using displayport monitors
> in MST configuration (i.e. displayport "daisy chaining").
>
> In Cosmic, only the 3rd patch is needed (first 2 are already included),
> and I sent that separately.

For the backports (especially since they consist of some delta) you should add
some hinting:

(backported from ...
[Context adjustments [and...]]

Generally I am a bit concerned about those pre-reqs since they modify some
"core" parts and I cannot clearly say which other drivers base on this. Do those
changes require modifications to those other drivers?
At least there were no follow-up commits as far as I can tell.
>
> Charlene Liu (1):
>   drm/amd/display: eDP sequence BL off first then DP blank.

Hunks like below (from above patch) make me doubt the upstream quality:

@@ -2309,7 +2308,6 @@ void core_link_enable_stream(
        if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
                allocate_mst_payload(pipe_ctx);

-       if (dc_is_dp_signal(pipe_ctx->stream->signal))
                core_dc->hwss.unblank_stream(pipe_ctx,
                        &pipe_ctx->stream->sink->link->cur_link_settings);
 }

Not changing the indentation level of the unblank call make it totally unclear
whether the if case before needs {} or not.

>
> Jerry (Fangzhi) Zuo (1):
>   drm/amd/display: Fix MST dp_blank REG_WAIT timeout

Here "cherry picked" again instead of cherry-picked.

Could you follow-up with explanations about the backports (how much needed
changing), so we could fix up the commits.

-Stefan

>
> Yongqiang Sun (1):
>   drm/amd/display: Move wait for hpd ready out from edp power control.
>
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 24 ++++++++++++--
>  .../drm/amd/display/dc/core/dc_link_hwss.c    | 11 +------
>  .../drm/amd/display/dc/dce/dce_link_encoder.c | 17 ++--------
>  .../display/dc/dce110/dce110_hw_sequencer.c   | 33 +++++++++++++------
>  .../display/dc/dce110/dce110_hw_sequencer.h   |  5 +++
>  .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  4 ++-
>  .../gpu/drm/amd/display/dc/inc/hw_sequencer.h |  3 ++
>  7 files changed, 60 insertions(+), 37 deletions(-)
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/3][SRU][Bionic] amdgpu with mst WARNING on blanking

Dan Streetman
On Thu, Feb 7, 2019 at 4:50 AM Stefan Bader <[hidden email]> wrote:

>
> On 06.02.19 20:24, Dan Streetman wrote:
> > From: Dan Streetman <[hidden email]>
> >
> > This series fixes amdgpu screen blanking when using displayport monitors
> > in MST configuration (i.e. displayport "daisy chaining").
> >
> > In Cosmic, only the 3rd patch is needed (first 2 are already included),
> > and I sent that separately.
>
> For the backports (especially since they consist of some delta) you should add
> some hinting:
>
> (backported from ...
> [Context adjustments [and...]]

ack, will resend with hinting

>
> Generally I am a bit concerned about those pre-reqs since they modify some
> "core" parts and I cannot clearly say which other drivers base on this. Do those
> changes require modifications to those other drivers?
> At least there were no follow-up commits as far as I can tell.
> >
> > Charlene Liu (1):
> >   drm/amd/display: eDP sequence BL off first then DP blank.
>
> Hunks like below (from above patch) make me doubt the upstream quality:
>
> @@ -2309,7 +2308,6 @@ void core_link_enable_stream(
>         if (pipe_ctx->stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST)
>                 allocate_mst_payload(pipe_ctx);
>
> -       if (dc_is_dp_signal(pipe_ctx->stream->signal))
>                 core_dc->hwss.unblank_stream(pipe_ctx,
>                         &pipe_ctx->stream->sink->link->cur_link_settings);
>  }
>
> Not changing the indentation level of the unblank call make it totally unclear
> whether the if case before needs {} or not.
>

indeed, i noticed that as well (and the build does emit a warning
basically saying 'do you really mean for this to not be inside the if
block?')

however i checked later commits and the alignment is fixed in commit
f3b72c7b00bd36773005e1bfea6b2bb558eb254f.

But I agree very much, it's sloppy and makes me question how closely
the upstream patches for the amdgpu driver were really reviewed.

> >
> > Jerry (Fangzhi) Zuo (1):
> >   drm/amd/display: Fix MST dp_blank REG_WAIT timeout
>
> Here "cherry picked" again instead of cherry-picked.

sorry, will fix in resend.

>
> Could you follow-up with explanations about the backports (how much needed
> changing), so we could fix up the commits.
>
> -Stefan
> >
> > Yongqiang Sun (1):
> >   drm/amd/display: Move wait for hpd ready out from edp power control.
> >
> >  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 24 ++++++++++++--
> >  .../drm/amd/display/dc/core/dc_link_hwss.c    | 11 +------
> >  .../drm/amd/display/dc/dce/dce_link_encoder.c | 17 ++--------
> >  .../display/dc/dce110/dce110_hw_sequencer.c   | 33 +++++++++++++------
> >  .../display/dc/dce110/dce110_hw_sequencer.h   |  5 +++
> >  .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  4 ++-
> >  .../gpu/drm/amd/display/dc/inc/hw_sequencer.h |  3 ++
> >  7 files changed, 60 insertions(+), 37 deletions(-)
> >
>
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team