[PATCH] drm/i915: Provide more clues as to why MST is/is not used

[PATCH] drm/i915: Provide more clues as to why MST is/is not used

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Always print out the information whether the port and sink can each
do MST. And let's include the modparam in the debug output as well.
Makes life a little less confusing when you don't have to wonder
why MST isn't kicking in.

This does cause a slight change in our behaviour towards the sink.
Previously we only read the MSTM_CAP register after passing all
the other checks. Now we will read that register regardless. Hopefully
some crazy sink doesn't get confused by a simple register read.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 15a981ef5966..1a510cf58fb5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4050,16 +4050,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 }
 
 static bool
-intel_dp_can_mst(struct intel_dp *intel_dp)
+intel_dp_sink_can_mst(struct intel_dp *intel_dp)
 {
 	u8 mstm_cap;
 
-	if (!i915_modparams.enable_dp_mst)
-		return false;
-
-	if (!intel_dp->can_mst)
-		return false;
-
 	if (intel_dp->dpcd[DP_DPCD_REV] < 0x12)
 		return false;
 
@@ -4069,21 +4063,30 @@ intel_dp_can_mst(struct intel_dp *intel_dp)
 	return mstm_cap & DP_MST_CAP;
 }
 
+static bool
+intel_dp_can_mst(struct intel_dp *intel_dp)
+{
+	return i915_modparams.enable_dp_mst &&
+		intel_dp->can_mst &&
+		intel_dp_sink_can_mst(intel_dp);
+}
+
 static void
 intel_dp_configure_mst(struct intel_dp *intel_dp)
 {
-	if (!i915_modparams.enable_dp_mst)
-		return;
+	struct intel_encoder *encoder =
+		&dp_to_dig_port(intel_dp)->base;
+	bool sink_can_mst = intel_dp_sink_can_mst(intel_dp);
+
+	DRM_DEBUG_KMS("MST support? port %c: %s, sink: %s, modparam: %s\n",
+		      port_name(encoder->port), yesno(intel_dp->can_mst),
+		      yesno(sink_can_mst), yesno(i915_modparams.enable_dp_mst));
 
 	if (!intel_dp->can_mst)
 		return;
 
-	intel_dp->is_mst = intel_dp_can_mst(intel_dp);
-
-	if (intel_dp->is_mst)
-		DRM_DEBUG_KMS("Sink is MST capable\n");
-	else
-		DRM_DEBUG_KMS("Sink is not MST capable\n");
+	intel_dp->is_mst = sink_can_mst &&
+		i915_modparams.enable_dp_mst;
 
 	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
 					intel_dp->is_mst);
-- 
2.16.4

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

Re: [PATCH] drm/i915: Provide more clues as to why MST is/is not used

[Dhinakaran Pandiyan]

On Wednesday, October 3, 2018 11:42:10 AM PDT Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Always print out the information whether the port and sink can each
> do MST. And let's include the modparam in the debug output as well.
> Makes life a little less confusing when you don't have to wonder
> why MST isn't kicking in.
> 
> This does cause a slight change in our behaviour towards the sink.
> Previously we only read the MSTM_CAP register after passing all
> the other checks. Now we will read that register regardless. Hopefully
> some crazy sink doesn't get confused by a simple register read.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c index 15a981ef5966..1a510cf58fb5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4050,16 +4050,10 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  }
> 
>  static bool
> -intel_dp_can_mst(struct intel_dp *intel_dp)
> +intel_dp_sink_can_mst(struct intel_dp *intel_dp)
>  {
>  	u8 mstm_cap;
> 
> -	if (!i915_modparams.enable_dp_mst)
> -		return false;
> -
> -	if (!intel_dp->can_mst)
> -		return false;
> -
>  	if (intel_dp->dpcd[DP_DPCD_REV] < 0x12)
>  		return false;
> 
> @@ -4069,21 +4063,30 @@ intel_dp_can_mst(struct intel_dp *intel_dp)
>  	return mstm_cap & DP_MST_CAP;
>  }
> 
> +static bool
> +intel_dp_can_mst(struct intel_dp *intel_dp)
> +{
> +	return i915_modparams.enable_dp_mst &&
> +		intel_dp->can_mst &&
> +		intel_dp_sink_can_mst(intel_dp);
> +}
> +
>  static void
>  intel_dp_configure_mst(struct intel_dp *intel_dp)
>  {
> -	if (!i915_modparams.enable_dp_mst)
> -		return;
> +	struct intel_encoder *encoder =
> +		&dp_to_dig_port(intel_dp)->base;
> +	bool sink_can_mst = intel_dp_sink_can_mst(intel_dp);
> +
> +	DRM_DEBUG_KMS("MST support? port %c: %s, sink: %s, modparam: %s\n",
> +		      port_name(encoder->port), yesno(intel_dp->can_mst),
> +		      yesno(sink_can_mst), yesno(i915_modparams.enable_dp_mst));
> 
>  	if (!intel_dp->can_mst)
>  		return;
> 
> -	intel_dp->is_mst = intel_dp_can_mst(intel_dp);
> -
> -	if (intel_dp->is_mst)
> -		DRM_DEBUG_KMS("Sink is MST capable\n");
> -	else
> -		DRM_DEBUG_KMS("Sink is not MST capable\n");
> +	intel_dp->is_mst = sink_can_mst &&
> +		i915_modparams.enable_dp_mst;
Should we still keep this modparam around? I don't know if anyone actually 
uses it to work around MST issues. Even if there are MST issues, we should be 
fixing them.

I see the value in the debug message, 
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> 
>  	drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
>  					intel_dp->is_mst);




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

✓ Fi.CI.BAT: success for drm/i915: Provide more clues as to why MST is/is not used

[Patchwork]

== Series Details ==

Series: drm/i915: Provide more clues as to why MST is/is not used
URL   : https://patchwork.freedesktop.org/series/50518/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4923 -> Patchwork_10348 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10348 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10348, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10348:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rpm@module-reload:
      fi-hsw-4770r:       SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-cfl-8109u:       PASS -> INCOMPLETE (fdo#108126, fdo#107187)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000) +1

    
    ==== Possible fixes ====

    igt@drv_getparams_basic@basic-eu-total:
      fi-kbl-7560u:       INCOMPLETE (fdo#103665) -> PASS

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@drv_module_reload@basic-reload-inject:
      fi-hsw-4770r:       DMESG-WARN (fdo#107425, fdo#107924) -> PASS

    igt@gem_basic@bad-close:
      fi-glk-j4005:       DMESG-WARN (fdo#105719) -> PASS +1

    
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#107187 https://bugs.freedesktop.org/show_bug.cgi?id=107187
  fdo#107425 https://bugs.freedesktop.org/show_bug.cgi?id=107425
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107924 https://bugs.freedesktop.org/show_bug.cgi?id=107924
  fdo#108126 https://bugs.freedesktop.org/show_bug.cgi?id=108126


== Participating hosts (49 -> 42) ==

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 fi-bdw-samus 


== Build changes ==

    * Linux: CI_DRM_4923 -> Patchwork_10348

  CI_DRM_4923: ebe8b202efb15420473f26d2386d1302bfc91429 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4665: 267870165d9ef66b4ab423e4efe7bacba023d75e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10348: 94e496dcb58633193bf44ec38387f250db43b48d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

94e496dcb586 drm/i915: Provide more clues as to why MST is/is not used

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915: Provide more clues as to why MST is/is not used

[Patchwork]

== Series Details ==

Series: drm/i915: Provide more clues as to why MST is/is not used
URL   : https://patchwork.freedesktop.org/series/50518/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4923_full -> Patchwork_10348_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-blt:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039, fdo#108130) +1

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-skl:          NOTRUN -> FAIL (fdo#106641)

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-modeset-hang-newfb-render-b:
      shard-snb:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_chv_cursor_fail@pipe-a-64x64-left-edge:
      shard-skl:          NOTRUN -> FAIL (fdo#104671) +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
      shard-apl:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-render:
      shard-skl:          NOTRUN -> FAIL (fdo#103167)

    {igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min}:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +3

    igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
      shard-apl:          PASS -> DMESG-WARN (fdo#105602, fdo#103558) +22

    igt@kms_setmode@basic:
      shard-snb:          NOTRUN -> FAIL (fdo#99912)

    igt@pm_rpm@gem-evict-pwrite:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807)

    igt@pm_rpm@system-suspend-execbuf:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807, fdo#104108)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-skl:          INCOMPLETE (fdo#106886, fdo#107773) -> PASS
      shard-apl:          INCOMPLETE (fdo#103927, fdo#106886) -> PASS

    igt@gem_exec_async@concurrent-writes-bsd2:
      shard-skl:          INCOMPLETE -> SKIP

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
      shard-apl:          FAIL (fdo#103166) -> PASS

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    
    ==== Warnings ====

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-apl:          FAIL (fdo#106510, fdo#105458) -> DMESG-WARN (fdo#105602, fdo#103558)

    igt@kms_universal_plane@universal-plane-pipe-a-functional:
      shard-apl:          FAIL (fdo#103166) -> DMESG-WARN (fdo#105602, fdo#103558)

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

  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105458 https://bugs.freedesktop.org/show_bug.cgi?id=105458
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#106510 https://bugs.freedesktop.org/show_bug.cgi?id=106510
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108130 https://bugs.freedesktop.org/show_bug.cgi?id=108130
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 4) ==

  Missing    (2): shard-glk shard-hsw 


== Build changes ==

    * Linux: CI_DRM_4923 -> Patchwork_10348

  CI_DRM_4923: ebe8b202efb15420473f26d2386d1302bfc91429 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4665: 267870165d9ef66b4ab423e4efe7bacba023d75e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10348: 94e496dcb58633193bf44ec38387f250db43b48d @ 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_10348/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx