[PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Imre Deak]

commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
inadvertently stopped enabling the pipe clock for any DP-MST stream
after the first one. It also rearranged the pipe clock enabling wrt.
initial MST payload allocation step (which may or may not be a
problem, but it's contrary to the spec.).

Fix things by making the above commit truly a non-functional change.

Fixes: commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107365
Reported-by: Lyude Paul <lyude@redhat.com>
Reported-by: dmummenschanz@web.de
Tested-by: dmummenschanz@web.de
Cc: Lyude Paul <lyude@redhat.com>
Cc: dmummenschanz@web.de
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    | 17 +++++++++--------
 drivers/gpu/drm/i915/intel_dp_mst.c |  4 ++++
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f3b115ce4029..dcb1a98d624d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2912,7 +2912,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 
 	icl_enable_phy_clock_gating(dig_port);
 
-	intel_ddi_enable_pipe_clock(crtc_state);
+	if (!is_mst)
+		intel_ddi_enable_pipe_clock(crtc_state);
 }
 
 static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
@@ -3015,14 +3016,14 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
 	bool is_mst = intel_crtc_has_type(old_crtc_state,
 					  INTEL_OUTPUT_DP_MST);
 
-	intel_ddi_disable_pipe_clock(old_crtc_state);
-
-	/*
-	 * Power down sink before disabling the port, otherwise we end
-	 * up getting interrupts from the sink on detecting link loss.
-	 */
-	if (!is_mst)
+	if (!is_mst) {
+		intel_ddi_disable_pipe_clock(old_crtc_state);
+		/*
+		 * Power down sink before disabling the port, otherwise we end
+		 * up getting interrupts from the sink on detecting link loss.
+		 */
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
+	}
 
 	intel_disable_ddi_buf(encoder);
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 352e5216cc65..77920f1a3da1 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -166,6 +166,8 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
 	struct intel_connector *connector =
 		to_intel_connector(old_conn_state->connector);
 
+	intel_ddi_disable_pipe_clock(old_crtc_state);
+
 	/* this can fail */
 	drm_dp_check_act_status(&intel_dp->mst_mgr);
 	/* and this can also fail */
@@ -249,6 +251,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
 	I915_WRITE(DP_TP_STATUS(port), temp);
 
 	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
+
+	intel_ddi_enable_pipe_clock(pipe_config);
 }
 
 static void intel_mst_enable_dp(struct intel_encoder *encoder,
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Patchwork]

== Series Details ==

Series: drm/i915/dp_mst: Fix enabling pipe clock for all streams
URL   : https://patchwork.freedesktop.org/series/49025/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4750 -> Patchwork_10063 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49025/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10063 that come from known issues:

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
      {fi-byt-clapper}:   FAIL (fdo#103191, fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362


== Participating hosts (52 -> 48) ==

  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4750 -> Patchwork_10063

  CI_DRM_4750: ef9613f5ddd35f2bd2834489b6d96e54c0cae8c6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4618: 9d83154c898b5acc8b462d17104df50cfd71e9a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10063: bb1f39c1120a86a6d33ca3c3fe8679e33a73f133 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bb1f39c1120a drm/i915/dp_mst: Fix enabling pipe clock for all streams

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10063/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Ville Syrjälä]

On Fri, Aug 31, 2018 at 08:47:39PM +0300, Imre Deak wrote:
> commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> inadvertently stopped enabling the pipe clock for any DP-MST stream
> after the first one. It also rearranged the pipe clock enabling wrt.
> initial MST payload allocation step (which may or may not be a
> problem, but it's contrary to the spec.).
> 
> Fix things by making the above commit truly a non-functional change.
> 
> Fixes: commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107365
> Reported-by: Lyude Paul <lyude@redhat.com>
> Reported-by: dmummenschanz@web.de
> Tested-by: dmummenschanz@web.de
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: dmummenschanz@web.de
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c    | 17 +++++++++--------
>  drivers/gpu/drm/i915/intel_dp_mst.c |  4 ++++
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f3b115ce4029..dcb1a98d624d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2912,7 +2912,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  
>  	icl_enable_phy_clock_gating(dig_port);
>  
> -	intel_ddi_enable_pipe_clock(crtc_state);
> +	if (!is_mst)
> +		intel_ddi_enable_pipe_clock(crtc_state);
>  }
>  
>  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> @@ -3015,14 +3016,14 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
>  	bool is_mst = intel_crtc_has_type(old_crtc_state,
>  					  INTEL_OUTPUT_DP_MST);
>  
> -	intel_ddi_disable_pipe_clock(old_crtc_state);
> -
> -	/*
> -	 * Power down sink before disabling the port, otherwise we end
> -	 * up getting interrupts from the sink on detecting link loss.
> -	 */
> -	if (!is_mst)
> +	if (!is_mst) {
> +		intel_ddi_disable_pipe_clock(old_crtc_state);
> +		/*
> +		 * Power down sink before disabling the port, otherwise we end
> +		 * up getting interrupts from the sink on detecting link loss.
> +		 */
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> +	}
>  
>  	intel_disable_ddi_buf(encoder);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 352e5216cc65..77920f1a3da1 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -166,6 +166,8 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	struct intel_connector *connector =
>  		to_intel_connector(old_conn_state->connector);
>  
> +	intel_ddi_disable_pipe_clock(old_crtc_state);
> +
>  	/* this can fail */
>  	drm_dp_check_act_status(&intel_dp->mst_mgr);
>  	/* and this can also fail */
> @@ -249,6 +251,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
>  	I915_WRITE(DP_TP_STATUS(port), temp);
>  
>  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> +
> +	intel_ddi_enable_pipe_clock(pipe_config);
>  }
>  
>  static void intel_mst_enable_dp(struct intel_encoder *encoder,
> -- 
> 2.13.2

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Lyude Paul]

Tested on my T450s, fixes the regression with MST!

Consider this:
Tested-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>

On Fri, 2018-08-31 at 20:47 +0300, Imre Deak wrote:
> commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> inadvertently stopped enabling the pipe clock for any DP-MST stream
> after the first one. It also rearranged the pipe clock enabling wrt.
> initial MST payload allocation step (which may or may not be a
> problem, but it's contrary to the spec.).
> 
> Fix things by making the above commit truly a non-functional change.
> 
> Fixes: commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to
> encoders")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107365
> Reported-by: Lyude Paul <lyude@redhat.com>
> Reported-by: dmummenschanz@web.de
> Tested-by: dmummenschanz@web.de
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: dmummenschanz@web.de
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c    | 17 +++++++++--------
>  drivers/gpu/drm/i915/intel_dp_mst.c |  4 ++++
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index f3b115ce4029..dcb1a98d624d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2912,7 +2912,8 @@ static void intel_ddi_pre_enable_dp(struct
> intel_encoder *encoder,
>  
>  	icl_enable_phy_clock_gating(dig_port);
>  
> -	intel_ddi_enable_pipe_clock(crtc_state);
> +	if (!is_mst)
> +		intel_ddi_enable_pipe_clock(crtc_state);
>  }
>  
>  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> @@ -3015,14 +3016,14 @@ static void intel_ddi_post_disable_dp(struct
> intel_encoder *encoder,
>  	bool is_mst = intel_crtc_has_type(old_crtc_state,
>  					  INTEL_OUTPUT_DP_MST);
>  
> -	intel_ddi_disable_pipe_clock(old_crtc_state);
> -
> -	/*
> -	 * Power down sink before disabling the port, otherwise we end
> -	 * up getting interrupts from the sink on detecting link loss.
> -	 */
> -	if (!is_mst)
> +	if (!is_mst) {
> +		intel_ddi_disable_pipe_clock(old_crtc_state);
> +		/*
> +		 * Power down sink before disabling the port, otherwise we end
> +		 * up getting interrupts from the sink on detecting link loss.
> +		 */
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> +	}
>  
>  	intel_disable_ddi_buf(encoder);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 352e5216cc65..77920f1a3da1 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -166,6 +166,8 @@ static void intel_mst_post_disable_dp(struct
> intel_encoder *encoder,
>  	struct intel_connector *connector =
>  		to_intel_connector(old_conn_state->connector);
>  
> +	intel_ddi_disable_pipe_clock(old_crtc_state);
> +
>  	/* this can fail */
>  	drm_dp_check_act_status(&intel_dp->mst_mgr);
>  	/* and this can also fail */
> @@ -249,6 +251,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder
> *encoder,
>  	I915_WRITE(DP_TP_STATUS(port), temp);
>  
>  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> +
> +	intel_ddi_enable_pipe_clock(pipe_config);
>  }
>  
>  static void intel_mst_enable_dp(struct intel_encoder *encoder,
-- 
Cheers,
	Lyude Paul

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Rodrigo Vivi]

On Fri, Aug 31, 2018 at 08:47:39PM +0300, Imre Deak wrote:
> commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> inadvertently stopped enabling the pipe clock for any DP-MST stream
> after the first one. It also rearranged the pipe clock enabling wrt.
> initial MST payload allocation step (which may or may not be a
> problem, but it's contrary to the spec.).
> 
> Fix things by making the above commit truly a non-functional change.
> 
> Fixes: commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107365
> Reported-by: Lyude Paul <lyude@redhat.com>
> Reported-by: dmummenschanz@web.de
> Tested-by: dmummenschanz@web.de
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: dmummenschanz@web.de
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c    | 17 +++++++++--------
>  drivers/gpu/drm/i915/intel_dp_mst.c |  4 ++++
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f3b115ce4029..dcb1a98d624d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2912,7 +2912,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  
>  	icl_enable_phy_clock_gating(dig_port);
>  
> -	intel_ddi_enable_pipe_clock(crtc_state);
> +	if (!is_mst)
> +		intel_ddi_enable_pipe_clock(crtc_state);
>  }
>  
>  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> @@ -3015,14 +3016,14 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
>  	bool is_mst = intel_crtc_has_type(old_crtc_state,
>  					  INTEL_OUTPUT_DP_MST);
>  
> -	intel_ddi_disable_pipe_clock(old_crtc_state);
> -
> -	/*
> -	 * Power down sink before disabling the port, otherwise we end
> -	 * up getting interrupts from the sink on detecting link loss.
> -	 */
> -	if (!is_mst)
> +	if (!is_mst) {
> +		intel_ddi_disable_pipe_clock(old_crtc_state);
> +		/*
> +		 * Power down sink before disabling the port, otherwise we end
> +		 * up getting interrupts from the sink on detecting link loss.
> +		 */
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> +	}
>  
>  	intel_disable_ddi_buf(encoder);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 352e5216cc65..77920f1a3da1 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -166,6 +166,8 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	struct intel_connector *connector =
>  		to_intel_connector(old_conn_state->connector);
>  
> +	intel_ddi_disable_pipe_clock(old_crtc_state);
> +
>  	/* this can fail */
>  	drm_dp_check_act_status(&intel_dp->mst_mgr);
>  	/* and this can also fail */
> @@ -249,6 +251,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
>  	I915_WRITE(DP_TP_STATUS(port), temp);
>  
>  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> +
> +	intel_ddi_enable_pipe_clock(pipe_config);
>  }
>  
>  static void intel_mst_enable_dp(struct intel_encoder *encoder,
> -- 
> 2.13.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Patchwork]

== Series Details ==

Series: drm/i915/dp_mst: Fix enabling pipe clock for all streams
URL   : https://patchwork.freedesktop.org/series/49025/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4750_full -> Patchwork_10063_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

  Here are the changes found in Patchwork_10063_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540, fdo#106886)

    
    ==== Possible fixes ====

    igt@gem_exec_await@wide-contexts:
      shard-kbl:          FAIL (fdo#105900) -> PASS

    igt@gem_exec_suspend@basic-s3-devices:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@kms_frontbuffer_tracking@fbc-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#105959) -> PASS

    igt@perf@blocking:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4750 -> Patchwork_10063

  CI_DRM_4750: ef9613f5ddd35f2bd2834489b6d96e54c0cae8c6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4618: 9d83154c898b5acc8b462d17104df50cfd71e9a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10063: bb1f39c1120a86a6d33ca3c3fe8679e33a73f133 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10063/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Imre Deak]

On Sat, Sep 01, 2018 at 04:38:23AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/dp_mst: Fix enabling pipe clock for all streams
> URL   : https://patchwork.freedesktop.org/series/49025/
> State : success

Pushed to -dinq, thanks for the report, review and testing.

> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_4750_full -> Patchwork_10063_full =
> 
> == Summary - SUCCESS ==
> 
>   No regressions found.
> 
>   
> 
> == Known issues ==
> 
>   Here are the changes found in Patchwork_10063_full that come from known issues:
> 
>   === IGT changes ===
> 
>     ==== Issues hit ====
> 
>     igt@drv_suspend@shrink:
>       shard-hsw:          PASS -> INCOMPLETE (fdo#103540, fdo#106886)
> 
>     
>     ==== Possible fixes ====
> 
>     igt@gem_exec_await@wide-contexts:
>       shard-kbl:          FAIL (fdo#105900) -> PASS
> 
>     igt@gem_exec_suspend@basic-s3-devices:
>       shard-snb:          INCOMPLETE (fdo#105411) -> PASS
> 
>     igt@kms_frontbuffer_tracking@fbc-suspend:
>       shard-kbl:          INCOMPLETE (fdo#103665, fdo#105959) -> PASS
> 
>     igt@perf@blocking:
>       shard-hsw:          FAIL (fdo#102252) -> PASS
> 
>     
>   fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
>   fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
>   fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
>   fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
>   fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
>   fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
>   fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
> 
> 
> == Participating hosts (5 -> 5) ==
> 
>   No changes in participating hosts
> 
> 
> == Build changes ==
> 
>     * Linux: CI_DRM_4750 -> Patchwork_10063
> 
>   CI_DRM_4750: ef9613f5ddd35f2bd2834489b6d96e54c0cae8c6 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_4618: 9d83154c898b5acc8b462d17104df50cfd71e9a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_10063: bb1f39c1120a86a6d33ca3c3fe8679e33a73f133 @ git://anongit.freedesktop.org/gfx-ci/linux
>   piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10063/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Jani Nikula]

On Fri, 31 Aug 2018, Imre Deak <imre.deak@intel.com> wrote:
> commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> inadvertently stopped enabling the pipe clock for any DP-MST stream
> after the first one. It also rearranged the pipe clock enabling wrt.
> initial MST payload allocation step (which may or may not be a
> problem, but it's contrary to the spec.).
>
> Fix things by making the above commit truly a non-functional change.

What kind of MST setups do we have in CI? Why didn't they catch this?

BR,
Jani.



>
> Fixes: commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107365
> Reported-by: Lyude Paul <lyude@redhat.com>
> Reported-by: dmummenschanz@web.de
> Tested-by: dmummenschanz@web.de
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: dmummenschanz@web.de
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c    | 17 +++++++++--------
>  drivers/gpu/drm/i915/intel_dp_mst.c |  4 ++++
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f3b115ce4029..dcb1a98d624d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2912,7 +2912,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  
>  	icl_enable_phy_clock_gating(dig_port);
>  
> -	intel_ddi_enable_pipe_clock(crtc_state);
> +	if (!is_mst)
> +		intel_ddi_enable_pipe_clock(crtc_state);
>  }
>  
>  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> @@ -3015,14 +3016,14 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
>  	bool is_mst = intel_crtc_has_type(old_crtc_state,
>  					  INTEL_OUTPUT_DP_MST);
>  
> -	intel_ddi_disable_pipe_clock(old_crtc_state);
> -
> -	/*
> -	 * Power down sink before disabling the port, otherwise we end
> -	 * up getting interrupts from the sink on detecting link loss.
> -	 */
> -	if (!is_mst)
> +	if (!is_mst) {
> +		intel_ddi_disable_pipe_clock(old_crtc_state);
> +		/*
> +		 * Power down sink before disabling the port, otherwise we end
> +		 * up getting interrupts from the sink on detecting link loss.
> +		 */
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> +	}
>  
>  	intel_disable_ddi_buf(encoder);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 352e5216cc65..77920f1a3da1 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -166,6 +166,8 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
>  	struct intel_connector *connector =
>  		to_intel_connector(old_conn_state->connector);
>  
> +	intel_ddi_disable_pipe_clock(old_crtc_state);
> +
>  	/* this can fail */
>  	drm_dp_check_act_status(&intel_dp->mst_mgr);
>  	/* and this can also fail */
> @@ -249,6 +251,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
>  	I915_WRITE(DP_TP_STATUS(port), temp);
>  
>  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> +
> +	intel_ddi_enable_pipe_clock(pipe_config);
>  }
>  
>  static void intel_mst_enable_dp(struct intel_encoder *encoder,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Imre Deak]

On Tue, Sep 04, 2018 at 03:08:16PM +0300, Jani Nikula wrote:
> On Fri, 31 Aug 2018, Imre Deak <imre.deak@intel.com> wrote:
> > commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> > inadvertently stopped enabling the pipe clock for any DP-MST stream
> > after the first one. It also rearranged the pipe clock enabling wrt.
> > initial MST payload allocation step (which may or may not be a
> > problem, but it's contrary to the spec.).
> >
> > Fix things by making the above commit truly a non-functional change.
> 
> What kind of MST setups do we have in CI? Why didn't they catch this?

What we'd need is a dock/other branch device with an DP-MST input and
two outputs. That would exercise the case that broke here.

> 
> BR,
> Jani.
> 
> 
> 
> >
> > Fixes: commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to encoders")
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107365
> > Reported-by: Lyude Paul <lyude@redhat.com>
> > Reported-by: dmummenschanz@web.de
> > Tested-by: dmummenschanz@web.de
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: dmummenschanz@web.de
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c    | 17 +++++++++--------
> >  drivers/gpu/drm/i915/intel_dp_mst.c |  4 ++++
> >  2 files changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index f3b115ce4029..dcb1a98d624d 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2912,7 +2912,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> >  
> >  	icl_enable_phy_clock_gating(dig_port);
> >  
> > -	intel_ddi_enable_pipe_clock(crtc_state);
> > +	if (!is_mst)
> > +		intel_ddi_enable_pipe_clock(crtc_state);
> >  }
> >  
> >  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> > @@ -3015,14 +3016,14 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
> >  	bool is_mst = intel_crtc_has_type(old_crtc_state,
> >  					  INTEL_OUTPUT_DP_MST);
> >  
> > -	intel_ddi_disable_pipe_clock(old_crtc_state);
> > -
> > -	/*
> > -	 * Power down sink before disabling the port, otherwise we end
> > -	 * up getting interrupts from the sink on detecting link loss.
> > -	 */
> > -	if (!is_mst)
> > +	if (!is_mst) {
> > +		intel_ddi_disable_pipe_clock(old_crtc_state);
> > +		/*
> > +		 * Power down sink before disabling the port, otherwise we end
> > +		 * up getting interrupts from the sink on detecting link loss.
> > +		 */
> >  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > +	}
> >  
> >  	intel_disable_ddi_buf(encoder);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 352e5216cc65..77920f1a3da1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -166,6 +166,8 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
> >  	struct intel_connector *connector =
> >  		to_intel_connector(old_conn_state->connector);
> >  
> > +	intel_ddi_disable_pipe_clock(old_crtc_state);
> > +
> >  	/* this can fail */
> >  	drm_dp_check_act_status(&intel_dp->mst_mgr);
> >  	/* and this can also fail */
> > @@ -249,6 +251,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder,
> >  	I915_WRITE(DP_TP_STATUS(port), temp);
> >  
> >  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> > +
> > +	intel_ddi_enable_pipe_clock(pipe_config);
> >  }
> >  
> >  static void intel_mst_enable_dp(struct intel_encoder *encoder,
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Dhinakaran Pandiyan]

On Tuesday, September 4, 2018 5:54:16 AM PDT Imre Deak wrote:
> On Tue, Sep 04, 2018 at 03:08:16PM +0300, Jani Nikula wrote:
> > On Fri, 31 Aug 2018, Imre Deak <imre.deak@intel.com> wrote:
> > > commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to
> > > encoders")
> > > inadvertently stopped enabling the pipe clock for any DP-MST stream
> > > after the first one. It also rearranged the pipe clock enabling wrt.
> > > initial MST payload allocation step (which may or may not be a
> > > problem, but it's contrary to the spec.).
> > > 
> > > Fix things by making the above commit truly a non-functional change.
> > 
> > What kind of MST setups do we have in CI? Why didn't they catch this?
> 
> What we'd need is a dock/other branch device with an DP-MST input and
> two outputs. That would exercise the case that broke here.
> 
I had the same question. We have  these two in CI, but both have only one 
external display attached.
fi-kbl-7560u	Dell XPS 13	Kaby Lake / i7-7560u / Iris Plus Graphics 640 GT3e	
eDP, DELL TB16->TB->DP-MST
fi-cfl-s3	Intel Coffee Lake-S RVP	Coffee Lake	eDP-PSR, DP-MST (HDMI)

Tomi,
Is it possible to have another external display connected to one of these?


> > BR,
> > Jani.
> > 
> > > Fixes: commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to
> > > encoders") Bugzilla:
> > > https://bugs.freedesktop.org/show_bug.cgi?id=107365
> > > Reported-by: Lyude Paul <lyude@redhat.com>
> > > Reported-by: dmummenschanz@web.de
> > > Tested-by: dmummenschanz@web.de
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Cc: dmummenschanz@web.de
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > > 
> > >  drivers/gpu/drm/i915/intel_ddi.c    | 17 +++++++++--------
> > >  drivers/gpu/drm/i915/intel_dp_mst.c |  4 ++++
> > >  2 files changed, 13 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c index f3b115ce4029..dcb1a98d624d
> > > 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -2912,7 +2912,8 @@ static void intel_ddi_pre_enable_dp(struct
> > > intel_encoder *encoder,> > 
> > >  	icl_enable_phy_clock_gating(dig_port);
> > > 
> > > -	intel_ddi_enable_pipe_clock(crtc_state);
> > > +	if (!is_mst)
> > > +		intel_ddi_enable_pipe_clock(crtc_state);
> > > 
> > >  }
> > >  
> > >  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> > > 
> > > @@ -3015,14 +3016,14 @@ static void intel_ddi_post_disable_dp(struct
> > > intel_encoder *encoder,> > 
> > >  	bool is_mst = intel_crtc_has_type(old_crtc_state,
> > >  	
> > >  					  INTEL_OUTPUT_DP_MST);
> > > 
> > > -	intel_ddi_disable_pipe_clock(old_crtc_state);
> > > -
> > > -	/*
> > > -	 * Power down sink before disabling the port, otherwise we end
> > > -	 * up getting interrupts from the sink on detecting link loss.
> > > -	 */
> > > -	if (!is_mst)
> > > +	if (!is_mst) {
> > > +		intel_ddi_disable_pipe_clock(old_crtc_state);
> > > +		/*
> > > +		 * Power down sink before disabling the port, otherwise we end
> > > +		 * up getting interrupts from the sink on detecting link loss.
> > > +		 */
> > > 
> > >  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > > 
> > > +	}
> > > 
> > >  	intel_disable_ddi_buf(encoder);
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > b/drivers/gpu/drm/i915/intel_dp_mst.c index 352e5216cc65..77920f1a3da1
> > > 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > @@ -166,6 +166,8 @@ static void intel_mst_post_disable_dp(struct
> > > intel_encoder *encoder,> > 
> > >  	struct intel_connector *connector =
> > >  	
> > >  		to_intel_connector(old_conn_state->connector);
> > > 
> > > +	intel_ddi_disable_pipe_clock(old_crtc_state);
> > > +
> > > 
> > >  	/* this can fail */
> > >  	drm_dp_check_act_status(&intel_dp->mst_mgr);
> > >  	/* and this can also fail */
> > > 
> > > @@ -249,6 +251,8 @@ static void intel_mst_pre_enable_dp(struct
> > > intel_encoder *encoder,> > 
> > >  	I915_WRITE(DP_TP_STATUS(port), temp);
> > >  	
> > >  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> > > 
> > > +
> > > +	intel_ddi_enable_pipe_clock(pipe_config);
> > > 
> > >  }
> > >  
> > >  static void intel_mst_enable_dp(struct intel_encoder *encoder,
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx




_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Rodrigo Vivi]

On Tue, Sep 04, 2018 at 03:53:51PM -0700, Dhinakaran Pandiyan wrote:
> On Tuesday, September 4, 2018 5:54:16 AM PDT Imre Deak wrote:
> > On Tue, Sep 04, 2018 at 03:08:16PM +0300, Jani Nikula wrote:
> > > On Fri, 31 Aug 2018, Imre Deak <imre.deak@intel.com> wrote:
> > > > commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to
> > > > encoders")
> > > > inadvertently stopped enabling the pipe clock for any DP-MST stream
> > > > after the first one. It also rearranged the pipe clock enabling wrt.
> > > > initial MST payload allocation step (which may or may not be a
> > > > problem, but it's contrary to the spec.).
> > > > 
> > > > Fix things by making the above commit truly a non-functional change.
> > > 
> > > What kind of MST setups do we have in CI? Why didn't they catch this?
> > 
> > What we'd need is a dock/other branch device with an DP-MST input and
> > two outputs. That would exercise the case that broke here.
> > 
> I had the same question. We have  these two in CI, but both have only one 
> external display attached.
> fi-kbl-7560u	Dell XPS 13	Kaby Lake / i7-7560u / Iris Plus Graphics 640 GT3e	
> eDP, DELL TB16->TB->DP-MST
> fi-cfl-s3	Intel Coffee Lake-S RVP	Coffee Lake	eDP-PSR, DP-MST (HDMI)
> 
> Tomi,
> Is it possible to have another external display connected to one of these?

or to both of them?!

I'm seeing MST related regressions since 4.16 so I'd like to have few
different configuration if possible with more than one monitor.

at least one kind of dock and one kind of chain both with 2 monitors
plugged in would be good.

> 
> 
> > > BR,
> > > Jani.
> > > 
> > > > Fixes: commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling to
> > > > encoders") Bugzilla:
> > > > https://bugs.freedesktop.org/show_bug.cgi?id=107365
> > > > Reported-by: Lyude Paul <lyude@redhat.com>
> > > > Reported-by: dmummenschanz@web.de
> > > > Tested-by: dmummenschanz@web.de
> > > > Cc: Lyude Paul <lyude@redhat.com>
> > > > Cc: dmummenschanz@web.de
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > > 
> > > >  drivers/gpu/drm/i915/intel_ddi.c    | 17 +++++++++--------
> > > >  drivers/gpu/drm/i915/intel_dp_mst.c |  4 ++++
> > > >  2 files changed, 13 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/intel_ddi.c index f3b115ce4029..dcb1a98d624d
> > > > 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > @@ -2912,7 +2912,8 @@ static void intel_ddi_pre_enable_dp(struct
> > > > intel_encoder *encoder,> > 
> > > >  	icl_enable_phy_clock_gating(dig_port);
> > > > 
> > > > -	intel_ddi_enable_pipe_clock(crtc_state);
> > > > +	if (!is_mst)
> > > > +		intel_ddi_enable_pipe_clock(crtc_state);
> > > > 
> > > >  }
> > > >  
> > > >  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> > > > 
> > > > @@ -3015,14 +3016,14 @@ static void intel_ddi_post_disable_dp(struct
> > > > intel_encoder *encoder,> > 
> > > >  	bool is_mst = intel_crtc_has_type(old_crtc_state,
> > > >  	
> > > >  					  INTEL_OUTPUT_DP_MST);
> > > > 
> > > > -	intel_ddi_disable_pipe_clock(old_crtc_state);
> > > > -
> > > > -	/*
> > > > -	 * Power down sink before disabling the port, otherwise we end
> > > > -	 * up getting interrupts from the sink on detecting link loss.
> > > > -	 */
> > > > -	if (!is_mst)
> > > > +	if (!is_mst) {
> > > > +		intel_ddi_disable_pipe_clock(old_crtc_state);
> > > > +		/*
> > > > +		 * Power down sink before disabling the port, otherwise we end
> > > > +		 * up getting interrupts from the sink on detecting link loss.
> > > > +		 */
> > > > 
> > > >  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > > > 
> > > > +	}
> > > > 
> > > >  	intel_disable_ddi_buf(encoder);
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > > b/drivers/gpu/drm/i915/intel_dp_mst.c index 352e5216cc65..77920f1a3da1
> > > > 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > > > @@ -166,6 +166,8 @@ static void intel_mst_post_disable_dp(struct
> > > > intel_encoder *encoder,> > 
> > > >  	struct intel_connector *connector =
> > > >  	
> > > >  		to_intel_connector(old_conn_state->connector);
> > > > 
> > > > +	intel_ddi_disable_pipe_clock(old_crtc_state);
> > > > +
> > > > 
> > > >  	/* this can fail */
> > > >  	drm_dp_check_act_status(&intel_dp->mst_mgr);
> > > >  	/* and this can also fail */
> > > > 
> > > > @@ -249,6 +251,8 @@ static void intel_mst_pre_enable_dp(struct
> > > > intel_encoder *encoder,> > 
> > > >  	I915_WRITE(DP_TP_STATUS(port), temp);
> > > >  	
> > > >  	ret = drm_dp_update_payload_part1(&intel_dp->mst_mgr);
> > > > 
> > > > +
> > > > +	intel_ddi_enable_pipe_clock(pipe_config);
> > > > 
> > > >  }
> > > >  
> > > >  static void intel_mst_enable_dp(struct intel_encoder *encoder,
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Dhinakaran Pandiyan]

On Tue, 2018-09-04 at 16:19 -0700, Rodrigo Vivi wrote:
> On Tue, Sep 04, 2018 at 03:53:51PM -0700, Dhinakaran Pandiyan wrote:
> > On Tuesday, September 4, 2018 5:54:16 AM PDT Imre Deak wrote:
> > > On Tue, Sep 04, 2018 at 03:08:16PM +0300, Jani Nikula wrote:
> > > > On Fri, 31 Aug 2018, Imre Deak <imre.deak@intel.com> wrote:
> > > > > commit afb2c4437dae ("drm/i915/ddi: Push pipe clock enabling
> > > > > to
> > > > > encoders")
> > > > > inadvertently stopped enabling the pipe clock for any DP-MST
> > > > > stream
> > > > > after the first one. It also rearranged the pipe clock
> > > > > enabling wrt.
> > > > > initial MST payload allocation step (which may or may not be
> > > > > a
> > > > > problem, but it's contrary to the spec.).
> > > > > 
> > > > > Fix things by making the above commit truly a non-functional
> > > > > change.
> > > > 
> > > > What kind of MST setups do we have in CI? Why didn't they catch
> > > > this?
> > > 
> > > What we'd need is a dock/other branch device with an DP-MST input
> > > and
> > > two outputs. That would exercise the case that broke here.
> > > 
> > 
> > I had the same question. We have  these two in CI, but both have
> > only one 
> > external display attached.
> > fi-kbl-7560u  Dell XPS 13     Kaby Lake / i7-7560u / Iris Plus
> > Graphics 640 GT3e      
> > eDP, DELL TB16->TB->DP-MST
> > fi-cfl-s3     Intel Coffee Lake-S RVP Coffee Lake     eDP-PSR, DP-
> > MST (HDMI)
> > 
> > Tomi,
> > Is it possible to have another external display connected to one of
> > these?
> 
> or to both of them?!
> 
:) Not sure why I wanted only one of them.


> I'm seeing MST related regressions since 4.16 so I'd like to have few
> different configuration if possible with more than one monitor.

A 4K@60Hz monitor along with a full-HD (or higher) monitor will be
useful to test the link BW code too. Together, they should be exceeding
the 63 vcpi slot limit. We can then write new IGT's to verify that.


> 
> at least one kind of dock and one kind of chain both with 2 monitors
> plugged in would be good.


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Jani Nikula]

On Tue, 04 Sep 2018, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> On Tue, 2018-09-04 at 16:19 -0700, Rodrigo Vivi wrote:
>> On Tue, Sep 04, 2018 at 03:53:51PM -0700, Dhinakaran Pandiyan wrote:
>> > Is it possible to have another external display connected to one of
>> > these?
>> 
>> or to both of them?!
>> 
> :) Not sure why I wanted only one of them.

Heh, well, we did have a bug in the past where we failed if the device
was booted with no displays. Diversify, don't maximize. ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/dp_mst: Fix enabling pipe clock for all streams

[Rodrigo Vivi]

On Wed, Sep 05, 2018 at 01:01:58PM +0300, Jani Nikula wrote:
1;5202;0c> On Tue, 04 Sep 2018, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> > On Tue, 2018-09-04 at 16:19 -0700, Rodrigo Vivi wrote:
> >> On Tue, Sep 04, 2018 at 03:53:51PM -0700, Dhinakaran Pandiyan wrote:
> >> > Is it possible to have another external display connected to one of
> >> > these?
> >> 
> >> or to both of them?!
> >> 
> > :) Not sure why I wanted only one of them.
> 
> Heh, well, we did have a bug in the past where we failed if the device
> was booted with no displays. Diversify, don't maximize. ;)

hehe :)

yeap, makes sense!

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx