[Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/mst: filter out the display mode exceed sink's capability

[Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/mst: filter out the display mode exceed sink's capability

[Patchwork]

== Series Details ==

Series: drm/i915/mst: filter out the display mode exceed sink's capability
URL   : https://patchwork.freedesktop.org/series/76095/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
/home/cidrm/kernel/Documentation/gpu/i915.rst:610: WARNING: duplicate label gpu/i915:layout, other instance in /home/cidrm/kernel/Documentation/gpu/i915.rst

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/mst: filter out the display mode exceed sink's capability

[Patchwork]

== Series Details ==

Series: drm/i915/mst: filter out the display mode exceed sink's capability
URL   : https://patchwork.freedesktop.org/series/76095/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8318 -> Patchwork_17348
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/index.html

Known issues
------------

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

### IGT changes ###

#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [FAIL][1] ([i915#62]) -> [SKIP][2] ([fdo#109271])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62


Participating hosts (51 -> 45)
------------------------------

  Missing    (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8318 -> Patchwork_17348

  CI-20190529: 20190529
  CI_DRM_8318: 9bca43263f42b286d5b761437854909817c6ff3b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5599: cdb07101dda33e2fcb0f4c2aa199c47159d88f35 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17348: 769dba11f1d742f15e134059634023f1a747e7fa @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

769dba11f1d7 drm/i915/mst: filter out the display mode exceed sink's capability

== Logs ==

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

[Intel-gfx] [PATCH] drm/i915/mst: filter out the display mode exceed sink's capability

[Lee Shawn C]

So far, max dot clock rate for MST mode rely on physcial
bandwidth limitation. It would caused compatibility issue
if source display resolution exceed MST hub output ability.

For example, source DUT had DP 1.2 output capability.
And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
monitor connected. Source would retrieve EDID from external
and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps
because it did not surpass DP physical bandwidth limitation.
Do modeset to 4k@60fps, source output display data but MST
docking can't output HDMI properly due to this resolution
already over HDMI 1.4 spec.

Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn
instead of available_pbn for bandwidth checks").
Source driver should refer to full_pbn to evaluate sink
output capability. And filter out the resolution surpass
sink output limitation.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 ++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 61605eb8c2af..68697f463dab 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -609,6 +609,26 @@ static int intel_dp_mst_get_modes(struct drm_connector *connector)
 	return intel_dp_mst_get_ddc_modes(connector);
 }
 
+static bool
+intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector *connector, int clock, int bpp)
+{
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_dp *intel_dp = intel_connector->mst_port;
+	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
+	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
+	bool ret = false;
+
+	if (!mgr)
+		return ret;
+
+	mutex_lock(&mgr->lock);
+	if (port->full_pbn)
+		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port->full_pbn);
+	mutex_unlock(&mgr->lock);
+
+	return ret;
+}
+
 static enum drm_mode_status
 intel_dp_mst_mode_valid(struct drm_connector *connector,
 			struct drm_display_mode *mode)
@@ -631,7 +651,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
 	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
 	mode_rate = intel_dp_link_required(mode->clock, 18);
 
-	/* TODO - validate mode against available PBN for link */
+	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode->clock, 24))
+		return MODE_CLOCK_HIGH;
+
 	if (mode->clock < 10000)
 		return MODE_CLOCK_LOW;
 
-- 
2.17.1

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

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/mst: filter out the display mode exceed sink's capability

[Patchwork]

== Series Details ==

Series: drm/i915/mst: filter out the display mode exceed sink's capability
URL   : https://patchwork.freedesktop.org/series/76095/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8318_full -> Patchwork_17348_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([i915#180]) +2 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-apl8/igt@gem_softpin@noreloc-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-apl4/igt@gem_softpin@noreloc-s3.html

  * igt@i915_pm_rps@waitboost:
    - shard-apl:          [PASS][3] -> [FAIL][4] ([i915#39])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-apl3/igt@i915_pm_rps@waitboost.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-apl3/igt@i915_pm_rps@waitboost.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen:
    - shard-apl:          [PASS][5] -> [FAIL][6] ([i915#54] / [i915#95])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-apl3/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-apl3/igt@kms_cursor_crc@pipe-a-cursor-128x42-offscreen.html

  * igt@kms_cursor_crc@pipe-a-cursor-128x42-random:
    - shard-kbl:          [PASS][7] -> [FAIL][8] ([i915#54] / [i915#93] / [i915#95])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-128x42-random.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-128x42-random.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-tglb:         [PASS][9] -> [SKIP][10] ([i915#668])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-skl:          [PASS][11] -> [INCOMPLETE][12] ([i915#123] / [i915#69])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-skl2/igt@kms_frontbuffer_tracking@psr-suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-skl1/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - shard-skl:          [PASS][13] -> [INCOMPLETE][14] ([i915#69])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-skl8/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-skl4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [PASS][15] -> [DMESG-WARN][16] ([i915#180])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane@plane-position-hole-pipe-a-planes:
    - shard-snb:          [PASS][17] -> [SKIP][18] ([fdo#109271]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-snb2/igt@kms_plane@plane-position-hole-pipe-a-planes.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-snb1/igt@kms_plane@plane-position-hole-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108145] / [i915#265])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-iclb8/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][23] -> [FAIL][24] ([i915#31])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-apl7/igt@kms_setmode@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-apl8/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [DMESG-WARN][25] ([i915#716]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-skl7/igt@gen9_exec_parse@allowed-single.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-skl8/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-snb:          [FAIL][27] ([i915#1066]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-snb4/igt@i915_pm_rc6_residency@rc6-idle.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-snb6/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@i915_selftest@live@requests:
    - shard-tglb:         [INCOMPLETE][29] ([i915#1531] / [i915#1658]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-tglb1/igt@i915_selftest@live@requests.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-tglb6/igt@i915_selftest@live@requests.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-kbl:          [FAIL][31] ([i915#93] / [i915#95]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-kbl7/igt@kms_fbcon_fbt@fbc-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-kbl4/igt@kms_fbcon_fbt@fbc-suspend.html

  * {igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1}:
    - shard-skl:          [FAIL][33] ([i915#34]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-skl3/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-skl9/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [FAIL][35] ([i915#1188]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-skl4/igt@kms_hdr@bpc-switch.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-skl3/igt@kms_hdr@bpc-switch.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][37] ([fdo#108145] / [i915#265]) -> [PASS][38] +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-apl:          [FAIL][39] ([i915#95]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-apl1/igt@kms_plane_multiple@atomic-pipe-a-tiling-y.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-apl8/igt@kms_plane_multiple@atomic-pipe-a-tiling-y.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-iclb8/igt@kms_psr@psr2_primary_mmap_cpu.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][43] ([i915#180]) -> [PASS][44] +4 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-kbl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * {igt@sysfs_timeslice_duration@timeout@vecs0}:
    - shard-iclb:         [FAIL][45] -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-iclb8/igt@sysfs_timeslice_duration@timeout@vecs0.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-iclb3/igt@sysfs_timeslice_duration@timeout@vecs0.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][47] ([i915#658]) -> [SKIP][48] ([i915#588])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-iclb1/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-kbl:          [FAIL][49] ([fdo#108145] / [i915#265]) -> [FAIL][50] ([fdo#108145] / [i915#265] / [i915#62])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-kbl7/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-kbl4/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-apl:          [FAIL][51] ([fdo#108145] / [i915#265]) -> [FAIL][52] ([fdo#108145] / [i915#265] / [i915#95])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8318/shard-apl3/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17348/shard-apl7/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#1066]: https://gitlab.freedesktop.org/drm/intel/issues/1066
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
  [i915#1531]: https://gitlab.freedesktop.org/drm/intel/issues/1531
  [i915#1658]: https://gitlab.freedesktop.org/drm/intel/issues/1658
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#39]: https://gitlab.freedesktop.org/drm/intel/issues/39
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#668]: https://gitlab.freedesktop.org/drm/intel/issues/668
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8318 -> Patchwork_17348

  CI-20190529: 20190529
  CI_DRM_8318: 9bca43263f42b286d5b761437854909817c6ff3b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5599: cdb07101dda33e2fcb0f4c2aa199c47159d88f35 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17348: 769dba11f1d742f15e134059634023f1a747e7fa @ 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_17348/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/mst: filter out the display mode exceed sink's capability

[Lyude Paul]

Hi! Sorry this took me a little while to get back to, I had a couple of MST
regressions that I had to look into

On Sat, 2020-04-18 at 05:24 +0800, Lee Shawn C wrote:
> So far, max dot clock rate for MST mode rely on physcial
> bandwidth limitation. It would caused compatibility issue
> if source display resolution exceed MST hub output ability.
> 
> For example, source DUT had DP 1.2 output capability.
> And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
> monitor connected. Source would retrieve EDID from external
> and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps
> because it did not surpass DP physical bandwidth limitation.
> Do modeset to 4k@60fps, source output display data but MST
> docking can't output HDMI properly due to this resolution
> already over HDMI 1.4 spec.
> 
> Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn
> instead of available_pbn for bandwidth checks").
> Source driver should refer to full_pbn to evaluate sink
> output capability. And filter out the resolution surpass
> sink output limitation.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 ++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 61605eb8c2af..68697f463dab 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -609,6 +609,26 @@ static int intel_dp_mst_get_modes(struct drm_connector
> *connector)
>  	return intel_dp_mst_get_ddc_modes(connector);
>  }
>  
> +static bool
> +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
> *connector, int clock, int bpp)
> +{
> +	struct intel_connector *intel_connector =
> to_intel_connector(connector);
> +	struct intel_dp *intel_dp = intel_connector->mst_port;
> +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
> +	bool ret = false;
> +
> +	if (!mgr)
> +		return ret;
> +
> +	mutex_lock(&mgr->lock);

This isn't the right lock for this - mgr->lock protects the topology layout
(e.g. drm_dp_mst_port.mstb, drm_dp_mst_port.next, and
drm_dp_mst_branch.ports). port->full_pbn is protected under
&drm_dp_mst_topology_mgr.base.lock (not drm_dp_mst_topology_mgr.lock), so you
need to first add a version of .mode_valid() to struct
drm_connector_helper_funcs that accepts a drm_modeset_acquire_ctx so you can
use that to safely grab &drm_dp_mst_topology_mgr.base.lock.

> +	if (port->full_pbn)

Also - there's no reason to check if (port->full_pbn) here, so long as you're
holding the right locks this should always be populated by the time we create
the drm_connector for the MST port (if it's not, that's a bug that needs to be
fixed).

> +		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port-
> >full_pbn);

Finally - I'd say we should probably have a separate patch to add a helper for
calculating the PBN and checking it against port->full_pbn. Maybe something
that looks like this:

int drm_dp_mst_mode_valid(struct drm_dp_mst_port *port, struct
drm_modeset_acquire_ctx *ctx, int clock, int bpp) {
	int ret = MODE_VALID;
	/* TODO: DSC support */

	/* …try grabbing locks here…*/
	if (drm_dp_calc_pbn_mode(clock, bpp, false) > port->full_pbn)
		ret = MODE_CLOCK_HIGH;

	return ret;
}

That way we might be able to add some checks for DSC capable connectors later
once I've migrated most of the DSC code from amdgpu into the MST helpers
> +	mutex_unlock(&mgr->lock);
> +
> +	return ret;
> +}
> +
>  static enum drm_mode_status
>  intel_dp_mst_mode_valid(struct drm_connector *connector,
>  			struct drm_display_mode *mode)
> @@ -631,7 +651,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
>  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>  	mode_rate = intel_dp_link_required(mode->clock, 18);
>  
> -	/* TODO - validate mode against available PBN for link */
> +	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode-
> >clock, 24))
> +		return MODE_CLOCK_HIGH;
> +
>  	if (mode->clock < 10000)
>  		return MODE_CLOCK_LOW;
>  
-- 
Cheers,
	Lyude Paul (she/her)
	Associate Software Engineer at Red Hat

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

Re: [Intel-gfx] [PATCH] drm/i915/mst: filter out the display mode exceed sink's capability

[Lee, Shawn C]

On Sat, 2020-04-25, Lyude Paul wrote:
>
>Hi! Sorry this took me a little while to get back to, I had a couple of MST regressions that I had to look into
>
>On Sat, 2020-04-18 at 05:24 +0800, Lee Shawn C wrote:
>> So far, max dot clock rate for MST mode rely on physcial bandwidth 
>> limitation. It would caused compatibility issue if source display 
>> resolution exceed MST hub output ability.
>> 
>> For example, source DUT had DP 1.2 output capability.
>> And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 monitor 
>> connected. Source would retrieve EDID from external and get max 
>> resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did not 
>> surpass DP physical bandwidth limitation.
>> Do modeset to 4k@60fps, source output display data but MST docking 
>> can't output HDMI properly due to this resolution already over HDMI 
>> 1.4 spec.
>> 
>> Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn instead of 
>> available_pbn for bandwidth checks").
>> Source driver should refer to full_pbn to evaluate sink output 
>> capability. And filter out the resolution surpass sink output 
>> limitation.
>> 
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Cooper Chiou <cooper.chiou@intel.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 
>> ++++++++++++++++++++-
>>  1 file changed, 23 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index 61605eb8c2af..68697f463dab 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -609,6 +609,26 @@ static int intel_dp_mst_get_modes(struct 
>> drm_connector
>> *connector)
>>  	return intel_dp_mst_get_ddc_modes(connector);
>>  }
>>  
>> +static bool
>> +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
>> *connector, int clock, int bpp)
>> +{
>> +	struct intel_connector *intel_connector =
>> to_intel_connector(connector);
>> +	struct intel_dp *intel_dp = intel_connector->mst_port;
>> +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
>> +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
>> +	bool ret = false;
>> +
>> +	if (!mgr)
>> +		return ret;
>> +
>> +	mutex_lock(&mgr->lock);
>
>This isn't the right lock for this - mgr->lock protects the topology layout (e.g. drm_dp_mst_port.mstb, drm_dp_mst_port.next, and drm_dp_mst_branch.ports). port->full_pbn is protected under &drm_dp_mst_topology_mgr.base.lock (not drm_dp_mst_topology_mgr.lock), so you need to first add a version of .mode_valid() to struct drm_connector_helper_funcs that accepts a drm_modeset_acquire_ctx so you can use that to safely grab &drm_dp_mst_topology_mgr.base.lock.
>

Thanks for comments! I will correct the lock to protect port->full_pbn.

>> +	if (port->full_pbn)
>
>Also - there's no reason to check if (port->full_pbn) here, so long as you're holding the right locks this should always be populated by the time we create the drm_connector for the MST port (if it's not, that's a bug that needs to be fixed).
>

Just want to make sure full_pbn value is greater than zero. As you mention in another patch, it's hard to predict sink report full or available PBN properly.

>> +		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port-
>> >full_pbn);
>
>Finally - I'd say we should probably have a separate patch to add a helper for calculating the PBN and checking it against port->full_pbn. Maybe something that looks like this:
>
>int drm_dp_mst_mode_valid(struct drm_dp_mst_port *port, struct drm_modeset_acquire_ctx *ctx, int clock, int bpp) {
>	int ret = MODE_VALID;
>	/* TODO: DSC support */
>
>	/* ...try grabbing locks here...*/
>	if (drm_dp_calc_pbn_mode(clock, bpp, false) > port->full_pbn)
>		ret = MODE_CLOCK_HIGH;
>
>	return ret;
>}
>
>That way we might be able to add some checks for DSC capable connectors later once I've migrated most of the DSC code from amdgpu into the MST helpers

This sounds good. DRM driver provide a public function call for different vendor to check the current mode exceed PBN's limitation or not.

>> +	mutex_unlock(&mgr->lock);
>> +
>> +	return ret;
>> +}
>> +
>>  static enum drm_mode_status
>>  intel_dp_mst_mode_valid(struct drm_connector *connector,
>>  			struct drm_display_mode *mode)
>> @@ -631,7 +651,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
>>  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>>  	mode_rate = intel_dp_link_required(mode->clock, 18);
>>  
>> -	/* TODO - validate mode against available PBN for link */
>> +	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode-
>> >clock, 24))
>> +		return MODE_CLOCK_HIGH;
>> +
>>  	if (mode->clock < 10000)
>>  		return MODE_CLOCK_LOW;
>>  
>--
>Cheers,
>	Lyude Paul (she/her)
>	Associate Software Engineer at Red Hat
>

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

Re: [Intel-gfx] [PATCH] drm/i915/mst: filter out the display mode exceed sink's capability

[Lyude Paul]

On Thu, 2020-04-30 at 02:37 +0000, Lee, Shawn C wrote:
> On Sat, 2020-04-25, Lyude Paul wrote:
> > Hi! Sorry this took me a little while to get back to, I had a couple of
> > MST regressions that I had to look into
> > 
> > On Sat, 2020-04-18 at 05:24 +0800, Lee Shawn C wrote:
> > > So far, max dot clock rate for MST mode rely on physcial bandwidth 
> > > limitation. It would caused compatibility issue if source display 
> > > resolution exceed MST hub output ability.
> > > 
> > > For example, source DUT had DP 1.2 output capability.
> > > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 monitor 
> > > connected. Source would retrieve EDID from external and get max 
> > > resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did not 
> > > surpass DP physical bandwidth limitation.
> > > Do modeset to 4k@60fps, source output display data but MST docking 
> > > can't output HDMI properly due to this resolution already over HDMI 
> > > 1.4 spec.
> > > 
> > > Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn instead of 
> > > available_pbn for bandwidth checks").
> > > Source driver should refer to full_pbn to evaluate sink output 
> > > capability. And filter out the resolution surpass sink output 
> > > limitation.
> > > 
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 
> > > ++++++++++++++++++++-
> > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > index 61605eb8c2af..68697f463dab 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > @@ -609,6 +609,26 @@ static int intel_dp_mst_get_modes(struct 
> > > drm_connector
> > > *connector)
> > >  	return intel_dp_mst_get_ddc_modes(connector);
> > >  }
> > >  
> > > +static bool
> > > +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
> > > *connector, int clock, int bpp)
> > > +{
> > > +	struct intel_connector *intel_connector =
> > > to_intel_connector(connector);
> > > +	struct intel_dp *intel_dp = intel_connector->mst_port;
> > > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> > > +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
> > > +	bool ret = false;
> > > +
> > > +	if (!mgr)
> > > +		return ret;
> > > +
> > > +	mutex_lock(&mgr->lock);
> > 
> > This isn't the right lock for this - mgr->lock protects the topology
> > layout (e.g. drm_dp_mst_port.mstb, drm_dp_mst_port.next, and
> > drm_dp_mst_branch.ports). port->full_pbn is protected under
> > &drm_dp_mst_topology_mgr.base.lock (not drm_dp_mst_topology_mgr.lock), so
> > you need to first add a version of .mode_valid() to struct
> > drm_connector_helper_funcs that accepts a drm_modeset_acquire_ctx so you
> > can use that to safely grab &drm_dp_mst_topology_mgr.base.lock.
> > 
> 
> Thanks for comments! I will correct the lock to protect port->full_pbn.
> 
> > > +	if (port->full_pbn)
> > 
> > Also - there's no reason to check if (port->full_pbn) here, so long as
> > you're holding the right locks this should always be populated by the time
> > we create the drm_connector for the MST port (if it's not, that's a bug
> > that needs to be fixed).
> > 
> 
> Just want to make sure full_pbn value is greater than zero. As you mention
> in another patch, it's hard to predict sink report full or available PBN
> properly.

Sorry this took me a little while to respond to, crunch time came up at work…
Anyway-have you seen issues with full_pbn reporting on hubs? I've seen plenty
of problems with available_pbn reporting, but the reason we switched over to
full_pbn in the first place is because that seemed to be the one hubs reported
reliably. We actually make the assumption full_pbn is always > 0 if
something's connected in some places in the MST helpers, so if we've got cases
of full_pbn lying as well on some hubs we might want to fix that.

> 
> > > +		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port-
> > > > full_pbn);
> > 
> > Finally - I'd say we should probably have a separate patch to add a helper
> > for calculating the PBN and checking it against port->full_pbn. Maybe
> > something that looks like this:
> > 
> > int drm_dp_mst_mode_valid(struct drm_dp_mst_port *port, struct
> > drm_modeset_acquire_ctx *ctx, int clock, int bpp) {
> > 	int ret = MODE_VALID;
> > 	/* TODO: DSC support */
> > 
> > 	/* ...try grabbing locks here...*/
> > 	if (drm_dp_calc_pbn_mode(clock, bpp, false) > port->full_pbn)
> > 		ret = MODE_CLOCK_HIGH;
> > 
> > 	return ret;
> > }
> > 
> > That way we might be able to add some checks for DSC capable connectors
> > later once I've migrated most of the DSC code from amdgpu into the MST
> > helpers
> 
> This sounds good. DRM driver provide a public function call for different
> vendor to check the current mode exceed PBN's limitation or not.
> 
> > > +	mutex_unlock(&mgr->lock);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  static enum drm_mode_status
> > >  intel_dp_mst_mode_valid(struct drm_connector *connector,
> > >  			struct drm_display_mode *mode)
> > > @@ -631,7 +651,9 @@ intel_dp_mst_mode_valid(struct drm_connector
> > > *connector,
> > >  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
> > >  	mode_rate = intel_dp_link_required(mode->clock, 18);
> > >  
> > > -	/* TODO - validate mode against available PBN for link */
> > > +	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode-
> > > > clock, 24))
> > > +		return MODE_CLOCK_HIGH;
> > > +
> > >  	if (mode->clock < 10000)
> > >  		return MODE_CLOCK_LOW;
> > >  
> > --
> > Cheers,
> > 	Lyude Paul (she/her)
> > 	Associate Software Engineer at Red Hat
> > 
> 
> Best regards,
> Shawn
> 
-- 
Cheers,
	Lyude Paul (she/her)
	Associate Software Engineer at Red Hat

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

Re: [Intel-gfx] [PATCH] drm/i915/mst: filter out the display mode exceed sink's capability

[Lee, Shawn C]

On Thu, 2020-05-07, Lyude Paul wrote:
>On Thu, 2020-04-30 at 02:37 +0000, Lee, Shawn C wrote:
>> On Sat, 2020-04-25, Lyude Paul wrote:
>> > Hi! Sorry this took me a little while to get back to, I had a couple of
>> > MST regressions that I had to look into
>> > 
>> > On Sat, 2020-04-18 at 05:24 +0800, Lee Shawn C wrote:
>> > > So far, max dot clock rate for MST mode rely on physcial bandwidth 
>> > > limitation. It would caused compatibility issue if source display 
>> > > resolution exceed MST hub output ability.
>> > > 
>> > > For example, source DUT had DP 1.2 output capability.
>> > > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 monitor 
>> > > connected. Source would retrieve EDID from external and get max 
>> > > resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did not 
>> > > surpass DP physical bandwidth limitation.
>> > > Do modeset to 4k@60fps, source output display data but MST docking 
>> > > can't output HDMI properly due to this resolution already over HDMI 
>> > > 1.4 spec.
>> > > 
>> > > Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn instead of 
>> > > available_pbn for bandwidth checks").
>> > > Source driver should refer to full_pbn to evaluate sink output 
>> > > capability. And filter out the resolution surpass sink output 
>> > > limitation.
>> > > 
>> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
>> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> > > Cc: Cooper Chiou <cooper.chiou@intel.com>
>> > > Cc: Lyude Paul <lyude@redhat.com>
>> > > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 
>> > > ++++++++++++++++++++-
>> > >  1 file changed, 23 insertions(+), 1 deletion(-)
>> > > 
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > > index 61605eb8c2af..68697f463dab 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > > @@ -609,6 +609,26 @@ static int intel_dp_mst_get_modes(struct 
>> > > drm_connector
>> > > *connector)
>> > >  	return intel_dp_mst_get_ddc_modes(connector);
>> > >  }
>> > >  
>> > > +static bool
>> > > +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
>> > > *connector, int clock, int bpp)
>> > > +{
>> > > +	struct intel_connector *intel_connector =
>> > > to_intel_connector(connector);
>> > > +	struct intel_dp *intel_dp = intel_connector->mst_port;
>> > > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
>> > > +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
>> > > +	bool ret = false;
>> > > +
>> > > +	if (!mgr)
>> > > +		return ret;
>> > > +
>> > > +	mutex_lock(&mgr->lock);
>> > 
>> > This isn't the right lock for this - mgr->lock protects the topology
>> > layout (e.g. drm_dp_mst_port.mstb, drm_dp_mst_port.next, and
>> > drm_dp_mst_branch.ports). port->full_pbn is protected under
>> > &drm_dp_mst_topology_mgr.base.lock (not drm_dp_mst_topology_mgr.lock), so
>> > you need to first add a version of .mode_valid() to struct
>> > drm_connector_helper_funcs that accepts a drm_modeset_acquire_ctx so you
>> > can use that to safely grab &drm_dp_mst_topology_mgr.base.lock.
>> > 
>> 
>> Thanks for comments! I will correct the lock to protect port->full_pbn.
>> 
>> > > +	if (port->full_pbn)
>> > 
>> > Also - there's no reason to check if (port->full_pbn) here, so long as
>> > you're holding the right locks this should always be populated by the time
>> > we create the drm_connector for the MST port (if it's not, that's a bug
>> > that needs to be fixed).
>> > 
>> 
>> Just want to make sure full_pbn value is greater than zero. As you mention
>> in another patch, it's hard to predict sink report full or available PBN
>> properly.
>
>Sorry this took me a little while to respond to, crunch time came up at work...
>Anyway-have you seen issues with full_pbn reporting on hubs? I've seen plenty
>of problems with available_pbn reporting, but the reason we switched over to
>full_pbn in the first place is because that seemed to be the one hubs reported
>reliably. We actually make the assumption full_pbn is always > 0 if
>something's connected in some places in the MST helpers, so if we've got cases
>of full_pbn lying as well on some hubs we might want to fix that.
>

No, we don't see MST hub report full_pbn = 0 so far. Just like what you said.
This is used to guarantee driver always have valid full_pbn for bandwidth calcuation.
If not, driver just bypass the PBN comparison.

>> 
>> > > +		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port-
>> > > > full_pbn);
>> > 
>> > Finally - I'd say we should probably have a separate patch to add a helper
>> > for calculating the PBN and checking it against port->full_pbn. Maybe
>> > something that looks like this:
>> > 
>> > int drm_dp_mst_mode_valid(struct drm_dp_mst_port *port, struct
>> > drm_modeset_acquire_ctx *ctx, int clock, int bpp) {
>> > 	int ret = MODE_VALID;
>> > 	/* TODO: DSC support */
>> > 
>> > 	/* ...try grabbing locks here...*/
>> > 	if (drm_dp_calc_pbn_mode(clock, bpp, false) > port->full_pbn)
>> > 		ret = MODE_CLOCK_HIGH;
>> > 
>> > 	return ret;
>> > }
>> > 
>> > That way we might be able to add some checks for DSC capable connectors
>> > later once I've migrated most of the DSC code from amdgpu into the MST
>> > helpers
>> 
>> This sounds good. DRM driver provide a public function call for different
>> vendor to check the current mode exceed PBN's limitation or not.
>> 
>> > > +	mutex_unlock(&mgr->lock);
>> > > +
>> > > +	return ret;
>> > > +}
>> > > +
>> > >  static enum drm_mode_status
>> > >  intel_dp_mst_mode_valid(struct drm_connector *connector,
>> > >  			struct drm_display_mode *mode)
>> > > @@ -631,7 +651,9 @@ intel_dp_mst_mode_valid(struct drm_connector
>> > > *connector,
>> > >  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>> > >  	mode_rate = intel_dp_link_required(mode->clock, 18);
>> > >  
>> > > -	/* TODO - validate mode against available PBN for link */
>> > > +	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode-
>> > > > clock, 24))
>> > > +		return MODE_CLOCK_HIGH;
>> > > +
>> > >  	if (mode->clock < 10000)
>> > >  		return MODE_CLOCK_LOW;
>> > >  
>> > --
>> > Cheers,
>> > 	Lyude Paul (she/her)
>> > 	Associate Software Engineer at Red Hat
>> > 
>>

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

Re: [Intel-gfx] [PATCH] drm/i915/mst: filter out the display mode exceed sink's capability

[Ville Syrjälä]

On Thu, May 07, 2020 at 06:46:17PM -0400, Lyude Paul wrote:
> On Thu, 2020-04-30 at 02:37 +0000, Lee, Shawn C wrote:
> > On Sat, 2020-04-25, Lyude Paul wrote:
> > > Hi! Sorry this took me a little while to get back to, I had a couple of
> > > MST regressions that I had to look into
> > > 
> > > On Sat, 2020-04-18 at 05:24 +0800, Lee Shawn C wrote:
> > > > So far, max dot clock rate for MST mode rely on physcial bandwidth 
> > > > limitation. It would caused compatibility issue if source display 
> > > > resolution exceed MST hub output ability.
> > > > 
> > > > For example, source DUT had DP 1.2 output capability.
> > > > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 monitor 
> > > > connected. Source would retrieve EDID from external and get max 
> > > > resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did not 
> > > > surpass DP physical bandwidth limitation.
> > > > Do modeset to 4k@60fps, source output display data but MST docking 
> > > > can't output HDMI properly due to this resolution already over HDMI 
> > > > 1.4 spec.
> > > > 
> > > > Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn instead of 
> > > > available_pbn for bandwidth checks").
> > > > Source driver should refer to full_pbn to evaluate sink output 
> > > > capability. And filter out the resolution surpass sink output 
> > > > limitation.
> > > > 
> > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > > > Cc: Lyude Paul <lyude@redhat.com>
> > > > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 
> > > > ++++++++++++++++++++-
> > > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > index 61605eb8c2af..68697f463dab 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > @@ -609,6 +609,26 @@ static int intel_dp_mst_get_modes(struct 
> > > > drm_connector
> > > > *connector)
> > > >  	return intel_dp_mst_get_ddc_modes(connector);
> > > >  }
> > > >  
> > > > +static bool
> > > > +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
> > > > *connector, int clock, int bpp)
> > > > +{
> > > > +	struct intel_connector *intel_connector =
> > > > to_intel_connector(connector);
> > > > +	struct intel_dp *intel_dp = intel_connector->mst_port;
> > > > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> > > > +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
> > > > +	bool ret = false;
> > > > +
> > > > +	if (!mgr)
> > > > +		return ret;
> > > > +
> > > > +	mutex_lock(&mgr->lock);
> > > 
> > > This isn't the right lock for this - mgr->lock protects the topology
> > > layout (e.g. drm_dp_mst_port.mstb, drm_dp_mst_port.next, and
> > > drm_dp_mst_branch.ports). port->full_pbn is protected under
> > > &drm_dp_mst_topology_mgr.base.lock (not drm_dp_mst_topology_mgr.lock), so
> > > you need to first add a version of .mode_valid() to struct
> > > drm_connector_helper_funcs that accepts a drm_modeset_acquire_ctx so you
> > > can use that to safely grab &drm_dp_mst_topology_mgr.base.lock.
> > > 
> > 
> > Thanks for comments! I will correct the lock to protect port->full_pbn.
> > 
> > > > +	if (port->full_pbn)
> > > 
> > > Also - there's no reason to check if (port->full_pbn) here, so long as
> > > you're holding the right locks this should always be populated by the time
> > > we create the drm_connector for the MST port (if it's not, that's a bug
> > > that needs to be fixed).
> > > 
> > 
> > Just want to make sure full_pbn value is greater than zero. As you mention
> > in another patch, it's hard to predict sink report full or available PBN
> > properly.
> 
> Sorry this took me a little while to respond to, crunch time came up at work…
> Anyway-have you seen issues with full_pbn reporting on hubs? I've seen plenty
> of problems with available_pbn reporting, but the reason we switched over to
> full_pbn in the first place is because that seemed to be the one hubs reported
> reliably. We actually make the assumption full_pbn is always > 0 if
> something's connected in some places in the MST helpers, so if we've got cases
> of full_pbn lying as well on some hubs we might want to fix that.

We have at least one full_pbn==0 issue in ci:
<4>[    5.061345] WARNING: CPU: 0 PID: 376 at drivers/gpu/drm/drm_dp_mst_topology.c:4889 drm_dp_mst_atomic_check_mstb_bw_limit+0x193/0x1c0

https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8460/fi-kbl-7560u/boot0.txt

Dunno if that's the MST device reporting zero or due to some other
bug, but at least CI is currently borked on that machine.

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/mst: filter out the display mode exceed sink's capability (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/mst: filter out the display mode exceed sink's capability (rev2)
URL   : https://patchwork.freedesktop.org/series/76095/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8466 -> Patchwork_17629
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/index.html


Changes
-------

  No changes found


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

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-kbl-7560u fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8466 -> Patchwork_17629

  CI-20190529: 20190529
  CI_DRM_8466: eea130b942bec4cb8c19514b3a63aed25e4dec27 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5646: 5a5a3162a7638b3ae38b6dc2545622c204d1b97c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17629: 269c1ff2c6f55bae701dbff3ebfcd1a0979b09ca @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

269c1ff2c6f5 drm/i915/mst: filter out the display mode exceed sink's capability

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/mst: filter out the display mode exceed sink's capability (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/mst: filter out the display mode exceed sink's capability (rev2)
URL   : https://patchwork.freedesktop.org/series/76095/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8466_full -> Patchwork_17629_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_crc@pipe-c-cursor-128x42-random:
    - shard-skl:          [PASS][1] -> [FAIL][2] ([i915#54])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-skl1/igt@kms_cursor_crc@pipe-c-cursor-128x42-random.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-skl9/igt@kms_cursor_crc@pipe-c-cursor-128x42-random.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([i915#180])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-apl8/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-apl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [PASS][5] -> [FAIL][6] ([i915#1188])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-skl6/igt@kms_hdr@bpc-switch.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-skl7/igt@kms_hdr@bpc-switch.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-kbl:          [PASS][7] -> [DMESG-WARN][8] ([i915#180]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#108145] / [i915#265]) +2 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_cursor@pipe-a-viewport-size-64:
    - shard-apl:          [PASS][11] -> [FAIL][12] ([i915#1559] / [i915#95])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-apl1/igt@kms_plane_cursor@pipe-a-viewport-size-64.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-apl3/igt@kms_plane_cursor@pipe-a-viewport-size-64.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109441]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-iclb2/igt@kms_psr@psr2_no_drrs.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-iclb8/igt@kms_psr@psr2_no_drrs.html

  
#### Possible fixes ####

  * igt@kms_cursor_crc@pipe-c-cursor-64x21-onscreen:
    - shard-skl:          [FAIL][15] ([i915#54]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-skl2/igt@kms_cursor_crc@pipe-c-cursor-64x21-onscreen.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-skl5/igt@kms_cursor_crc@pipe-c-cursor-64x21-onscreen.html

  * igt@kms_cursor_legacy@pipe-d-single-move:
    - shard-tglb:         [SKIP][17] -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-tglb3/igt@kms_cursor_legacy@pipe-d-single-move.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-tglb6/igt@kms_cursor_legacy@pipe-d-single-move.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [SKIP][19] ([fdo#109349]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-iclb4/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * {igt@kms_flip@flip-vs-suspend-interruptible@a-dp1}:
    - shard-kbl:          [DMESG-WARN][21] ([i915#180]) -> [PASS][22] +11 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-kbl4/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * {igt@kms_flip@flip-vs-suspend-interruptible@c-dp1}:
    - shard-apl:          [DMESG-WARN][23] ([i915#180]) -> [PASS][24] +4 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][25] ([i915#1188]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-skl5/igt@kms_hdr@bpc-switch-dpms.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][27] ([fdo#108145] / [i915#265]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][29] ([fdo#109642] / [fdo#111068]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-iclb4/igt@kms_psr2_su@frontbuffer.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][31] ([fdo#109441]) -> [PASS][32] +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-iclb4/igt@kms_psr@psr2_basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-iclb2/igt@kms_psr@psr2_basic.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][33] ([i915#31]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-apl8/igt@kms_setmode@basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-apl4/igt@kms_setmode@basic.html

  * {igt@perf@blocking-parameterized}:
    - shard-iclb:         [FAIL][35] ([i915#1542]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-iclb7/igt@perf@blocking-parameterized.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-iclb5/igt@perf@blocking-parameterized.html

  
#### Warnings ####

  * igt@kms_content_protection@atomic:
    - shard-apl:          [TIMEOUT][37] ([i915#1319]) -> [FAIL][38] ([fdo#110321] / [fdo#110336])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-apl4/igt@kms_content_protection@atomic.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-apl1/igt@kms_content_protection@atomic.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-apl:          [FAIL][39] ([fdo#108145] / [i915#265] / [i915#95]) -> [FAIL][40] ([fdo#108145] / [i915#265])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8466/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17629/shard-apl7/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1559]: https://gitlab.freedesktop.org/drm/intel/issues/1559
  [i915#1732]: https://gitlab.freedesktop.org/drm/intel/issues/1732
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8466 -> Patchwork_17629

  CI-20190529: 20190529
  CI_DRM_8466: eea130b942bec4cb8c19514b3a63aed25e4dec27 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5646: 5a5a3162a7638b3ae38b6dc2545622c204d1b97c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17629: 269c1ff2c6f55bae701dbff3ebfcd1a0979b09ca @ 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_17629/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] drm/i915/mst: filter out the display mode exceed sink's capability

[Lee Shawn C]

So far, max dot clock rate for MST mode rely on physcial
bandwidth limitation. It would caused compatibility issue
if source display resolution exceed MST hub output ability.

For example, source DUT had DP 1.2 output capability.
And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
monitor connected. Source would retrieve EDID from external
and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps
because it did not surpass DP physical bandwidth limitation.
Do modeset to 4k@60fps, source output display data but MST
docking can't output HDMI properly due to this resolution
already over HDMI 1.4 spec.

Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn
instead of available_pbn for bandwidth checks").
Source driver should refer to full_pbn to evaluate sink
output capability. And filter out the resolution surpass
sink output limitation.

v2: Using mgr->base.lock to protect full_pbn.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 ++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 74559379384a..6b1864ce3771 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -611,6 +611,26 @@ static int intel_dp_mst_get_modes(struct drm_connector *connector)
 	return intel_dp_mst_get_ddc_modes(connector);
 }
 
+static bool
+intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector *connector, int clock, int bpp)
+{
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_dp *intel_dp = intel_connector->mst_port;
+	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
+	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
+	bool ret = false;
+
+	if (!mgr)
+		return ret;
+
+	drm_modeset_lock(&mgr->base.lock, NULL);
+	if (port->full_pbn)
+		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port->full_pbn);
+	drm_modeset_unlock(&mgr->base.lock);
+
+	return ret;
+}
+
 static enum drm_mode_status
 intel_dp_mst_mode_valid(struct drm_connector *connector,
 			struct drm_display_mode *mode)
@@ -633,7 +653,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
 	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
 	mode_rate = intel_dp_link_required(mode->clock, 18);
 
-	/* TODO - validate mode against available PBN for link */
+	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode->clock, 24))
+		return MODE_CLOCK_HIGH;
+
 	if (mode->clock < 10000)
 		return MODE_CLOCK_LOW;
 
-- 
2.17.1

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

Re: [Intel-gfx] [PATCH] drm/i915/mst: filter out the display mode exceed sink's capability

[Lee, Shawn C]

On Thu, May 11, 2020, Ville Syrjälä wrote:
>On Thu, May 07, 2020 at 06:46:17PM -0400, Lyude Paul wrote:
>> On Thu, 2020-04-30 at 02:37 +0000, Lee, Shawn C wrote:
>> > On Sat, 2020-04-25, Lyude Paul wrote:
>> > > Hi! Sorry this took me a little while to get back to, I had a couple of
>> > > MST regressions that I had to look into
>> > > 
>> > > On Sat, 2020-04-18 at 05:24 +0800, Lee Shawn C wrote:
>> > > > So far, max dot clock rate for MST mode rely on physcial bandwidth 
>> > > > limitation. It would caused compatibility issue if source display 
>> > > > resolution exceed MST hub output ability.
>> > > > 
>> > > > For example, source DUT had DP 1.2 output capability.
>> > > > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 monitor 
>> > > > connected. Source would retrieve EDID from external and get max 
>> > > > resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did not 
>> > > > surpass DP physical bandwidth limitation.
>> > > > Do modeset to 4k@60fps, source output display data but MST docking 
>> > > > can't output HDMI properly due to this resolution already over HDMI 
>> > > > 1.4 spec.
>> > > > 
>> > > > Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn instead of 
>> > > > available_pbn for bandwidth checks").
>> > > > Source driver should refer to full_pbn to evaluate sink output 
>> > > > capability. And filter out the resolution surpass sink output 
>> > > > limitation.
>> > > > 
>> > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
>> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> > > > Cc: Cooper Chiou <cooper.chiou@intel.com>
>> > > > Cc: Lyude Paul <lyude@redhat.com>
>> > > > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> > > > ---
>> > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 
>> > > > ++++++++++++++++++++-
>> > > >  1 file changed, 23 insertions(+), 1 deletion(-)
>> > > > 
>> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > > > index 61605eb8c2af..68697f463dab 100644
>> > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > > > @@ -609,6 +609,26 @@ static int intel_dp_mst_get_modes(struct 
>> > > > drm_connector
>> > > > *connector)
>> > > >  	return intel_dp_mst_get_ddc_modes(connector);
>> > > >  }
>> > > >  
>> > > > +static bool
>> > > > +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
>> > > > *connector, int clock, int bpp)
>> > > > +{
>> > > > +	struct intel_connector *intel_connector =
>> > > > to_intel_connector(connector);
>> > > > +	struct intel_dp *intel_dp = intel_connector->mst_port;
>> > > > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
>> > > > +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
>> > > > +	bool ret = false;
>> > > > +
>> > > > +	if (!mgr)
>> > > > +		return ret;
>> > > > +
>> > > > +	mutex_lock(&mgr->lock);
>> > > 
>> > > This isn't the right lock for this - mgr->lock protects the topology
>> > > layout (e.g. drm_dp_mst_port.mstb, drm_dp_mst_port.next, and
>> > > drm_dp_mst_branch.ports). port->full_pbn is protected under
>> > > &drm_dp_mst_topology_mgr.base.lock (not drm_dp_mst_topology_mgr.lock), so
>> > > you need to first add a version of .mode_valid() to struct
>> > > drm_connector_helper_funcs that accepts a drm_modeset_acquire_ctx so you
>> > > can use that to safely grab &drm_dp_mst_topology_mgr.base.lock.
>> > > 
>> > 
>> > Thanks for comments! I will correct the lock to protect port->full_pbn.
>> > 
>> > > > +	if (port->full_pbn)
>> > > 
>> > > Also - there's no reason to check if (port->full_pbn) here, so long as
>> > > you're holding the right locks this should always be populated by the time
>> > > we create the drm_connector for the MST port (if it's not, that's a bug
>> > > that needs to be fixed).
>> > > 
>> > 
>> > Just want to make sure full_pbn value is greater than zero. As you mention
>> > in another patch, it's hard to predict sink report full or available PBN
>> > properly.
>> 
>> Sorry this took me a little while to respond to, crunch time came up at work.
>> Anyway-have you seen issues with full_pbn reporting on hubs? I've seen plenty
>> of problems with available_pbn reporting, but the reason we switched over to
>> full_pbn in the first place is because that seemed to be the one hubs reported
>> reliably. We actually make the assumption full_pbn is always > 0 if
>> something's connected in some places in the MST helpers, so if we've got cases
>> of full_pbn lying as well on some hubs we might want to fix that.
>
>We have at least one full_pbn==0 issue in ci:
><4>[    5.061345] WARNING: CPU: 0 PID: 376 at drivers/gpu/drm/drm_dp_mst_topology.c:4889 drm_dp_mst_atomic_check_mstb_bw_limit+0x193/0x1c0
>
>https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8460/fi-kbl-7560u/boot0.txt
>
>Dunno if that's the MST device reporting zero or due to some other
>bug, but at least CI is currently borked on that machine.

Thanks for sharing! It will be good to keep checking full_pbn to avoid unexpected issue.
BTW, I just upload patch v2. Could you please share your comments about that?

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

Re: [Intel-gfx] [PATCH] drm/i915/mst: filter out the display mode exceed sink's capability

[Lyude Paul]

On Tue, 2020-05-12 at 06:17 +0000, Lee, Shawn C wrote:
> On Thu, May 11, 2020, Ville Syrjälä wrote:
> > On Thu, May 07, 2020 at 06:46:17PM -0400, Lyude Paul wrote:
> > > On Thu, 2020-04-30 at 02:37 +0000, Lee, Shawn C wrote:
> > > > On Sat, 2020-04-25, Lyude Paul wrote:
> > > > > Hi! Sorry this took me a little while to get back to, I had a couple
> > > > > of
> > > > > MST regressions that I had to look into
> > > > > 
> > > > > On Sat, 2020-04-18 at 05:24 +0800, Lee Shawn C wrote:
> > > > > > So far, max dot clock rate for MST mode rely on physcial
> > > > > > bandwidth 
> > > > > > limitation. It would caused compatibility issue if source display 
> > > > > > resolution exceed MST hub output ability.
> > > > > > 
> > > > > > For example, source DUT had DP 1.2 output capability.
> > > > > > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
> > > > > > monitor 
> > > > > > connected. Source would retrieve EDID from external and get max 
> > > > > > resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did
> > > > > > not 
> > > > > > surpass DP physical bandwidth limitation.
> > > > > > Do modeset to 4k@60fps, source output display data but MST
> > > > > > docking 
> > > > > > can't output HDMI properly due to this resolution already over
> > > > > > HDMI 
> > > > > > 1.4 spec.
> > > > > > 
> > > > > > Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn instead
> > > > > > of 
> > > > > > available_pbn for bandwidth checks").
> > > > > > Source driver should refer to full_pbn to evaluate sink output 
> > > > > > capability. And filter out the resolution surpass sink output 
> > > > > > limitation.
> > > > > > 
> > > > > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > > > > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > > > > > Cc: Lyude Paul <lyude@redhat.com>
> > > > > > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 
> > > > > > ++++++++++++++++++++-
> > > > > >  1 file changed, 23 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > > index 61605eb8c2af..68697f463dab 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > > > > > @@ -609,6 +609,26 @@ static int intel_dp_mst_get_modes(struct 
> > > > > > drm_connector
> > > > > > *connector)
> > > > > >  	return intel_dp_mst_get_ddc_modes(connector);
> > > > > >  }
> > > > > >  
> > > > > > +static bool
> > > > > > +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
> > > > > > *connector, int clock, int bpp)
> > > > > > +{
> > > > > > +	struct intel_connector *intel_connector =
> > > > > > to_intel_connector(connector);
> > > > > > +	struct intel_dp *intel_dp = intel_connector->mst_port;
> > > > > > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> > > > > > +	struct drm_dp_mst_port *port =
> > > > > > (to_intel_connector(connector))->port;
> > > > > > +	bool ret = false;
> > > > > > +
> > > > > > +	if (!mgr)
> > > > > > +		return ret;
> > > > > > +
> > > > > > +	mutex_lock(&mgr->lock);
> > > > > 
> > > > > This isn't the right lock for this - mgr->lock protects the topology
> > > > > layout (e.g. drm_dp_mst_port.mstb, drm_dp_mst_port.next, and
> > > > > drm_dp_mst_branch.ports). port->full_pbn is protected under
> > > > > &drm_dp_mst_topology_mgr.base.lock (not
> > > > > drm_dp_mst_topology_mgr.lock), so
> > > > > you need to first add a version of .mode_valid() to struct
> > > > > drm_connector_helper_funcs that accepts a drm_modeset_acquire_ctx so
> > > > > you
> > > > > can use that to safely grab &drm_dp_mst_topology_mgr.base.lock.
> > > > > 
> > > > 
> > > > Thanks for comments! I will correct the lock to protect port-
> > > > >full_pbn.
> > > > 
> > > > > > +	if (port->full_pbn)
> > > > > 
> > > > > Also - there's no reason to check if (port->full_pbn) here, so long
> > > > > as
> > > > > you're holding the right locks this should always be populated by
> > > > > the time
> > > > > we create the drm_connector for the MST port (if it's not, that's a
> > > > > bug
> > > > > that needs to be fixed).
> > > > > 
> > > > 
> > > > Just want to make sure full_pbn value is greater than zero. As you
> > > > mention
> > > > in another patch, it's hard to predict sink report full or available
> > > > PBN
> > > > properly.
> > > 
> > > Sorry this took me a little while to respond to, crunch time came up at
> > > work.
> > > Anyway-have you seen issues with full_pbn reporting on hubs? I've seen
> > > plenty
> > > of problems with available_pbn reporting, but the reason we switched
> > > over to
> > > full_pbn in the first place is because that seemed to be the one hubs
> > > reported
> > > reliably. We actually make the assumption full_pbn is always > 0 if
> > > something's connected in some places in the MST helpers, so if we've got
> > > cases
> > > of full_pbn lying as well on some hubs we might want to fix that.
> > 
> > We have at least one full_pbn==0 issue in ci:
> > <4>[    5.061345] WARNING: CPU: 0 PID: 376 at
> > drivers/gpu/drm/drm_dp_mst_topology.c:4889
> > drm_dp_mst_atomic_check_mstb_bw_limit+0x193/0x1c0
> > 
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8460/fi-kbl-7560u/boot0.txt
> > 

Oh yikes-yeah, it's definitely reporting 0 PBN, that's a new one. Anyway-I'll
take a look at this patch sometime today. I'd like to look more into that hub
though, I'm almost certain that full_pbn = 0 is against the MST spec, but the
hub does seem to be reporting that:

<7>[    5.002279] [drm:drm_dp_send_enum_path_resources] enum path resources 3:
0 0

Since hubs will set full_pbn based on the current downstream link training
settings for the mst port though, I wonder if maybe this hub is just failing
to do link training downstream or failing to detect a display downstream?
Probably would be useful to know what kind of a hub this is, and what kind of
displays are hooked up to it. Also, having drm DP enabled in those logs would
be quite helpful as well

> > Dunno if that's the MST device reporting zero or due to some other
> > bug, but at least CI is currently borked on that machine.
> 
> Thanks for sharing! It will be good to keep checking full_pbn to avoid
> unexpected issue.
> BTW, I just upload patch v2. Could you please share your comments about
> that?
> 
> Best regards,
> Shawn
> 
-- 
Cheers,
	Lyude Paul (she/her)
	Associate Software Engineer at Red Hat

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

Re: [Intel-gfx] [PATCH v2] drm/i915/mst: filter out the display mode exceed sink's capability

[Lyude Paul]

sorry it's been taking me a while to get back to you with these, crunch time at
my work started so i've been really busy over the last week.

On Tue, 2020-05-12 at 07:19 +0800, Lee Shawn C wrote:
> So far, max dot clock rate for MST mode rely on physcial
> bandwidth limitation. It would caused compatibility issue
> if source display resolution exceed MST hub output ability.
> 
> For example, source DUT had DP 1.2 output capability.
> And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
> monitor connected. Source would retrieve EDID from external
> and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps
> because it did not surpass DP physical bandwidth limitation.
> Do modeset to 4k@60fps, source output display data but MST
> docking can't output HDMI properly due to this resolution
> already over HDMI 1.4 spec.
> 
> Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn
> instead of available_pbn for bandwidth checks").
> Source driver should refer to full_pbn to evaluate sink
> output capability. And filter out the resolution surpass
> sink output limitation.
> 
> v2: Using mgr->base.lock to protect full_pbn.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 24 ++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 74559379384a..6b1864ce3771 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -611,6 +611,26 @@ static int intel_dp_mst_get_modes(struct drm_connector
> *connector)
>  	return intel_dp_mst_get_ddc_modes(connector);
>  }
>  
> +static bool
> +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector *connector,
> int clock, int bpp)
> +{
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	struct intel_dp *intel_dp = intel_connector->mst_port;
> +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
> +	bool ret = false;
> +
> +	if (!mgr)
> +		return ret;
> +
> +	drm_modeset_lock(&mgr->base.lock, NULL);
> +	if (port->full_pbn)
> +		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port-
> >full_pbn);
> +	drm_modeset_unlock(&mgr->base.lock);

You're so close, but not quite! I'm fairly sure ->mode_valid() gets called with
drm_device.mode_config.connection_mutex already locked, which means that you
need to use the same drm_modeset_acquire context (if there isn't one, you need
to make it) so that you can safely roll back and retry in case there's a
deadlock between drm_dp_mst_topology_mgr.base.lock and
drm_device.mode_config.connection_mutex .

fwiw - I'm sensing some confusion here about ww_mutexes (wouldn't be the first
time, they're unusual). If you haven't already, it's probably worth reading up
on:

https://lwn.net/Articles/548909/

Keep in mind when you pipe down the context by adding a new version of the
->mode_valid() hook you won't want to drop any locks once you're done in this
function. They'll get dropped automatically when the caller of the new
->mode_valid() hook (so, the one where you get the drm_modeset_acquire_ctx from)
calls drm_modeset_drop_locks().

> +
> +	return ret;
> +}
> +
>  static enum drm_mode_status
>  intel_dp_mst_mode_valid(struct drm_connector *connector,
>  			struct drm_display_mode *mode)
> @@ -633,7 +653,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
>  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>  	mode_rate = intel_dp_link_required(mode->clock, 18);
>  
> -	/* TODO - validate mode against available PBN for link */
> +	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode->clock, 
> 24))
> +		return MODE_CLOCK_HIGH;
> +
>  	if (mode->clock < 10000)
>  		return MODE_CLOCK_LOW;
>  

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

[Intel-gfx] [PATCH v3] drm/i915/mst: filter out the display mode exceed sink's capability

[Lee Shawn C]

So far, max dot clock rate for MST mode rely on physcial
bandwidth limitation. It would caused compatibility issue
if source display resolution exceed MST hub output ability.

For example, source DUT had DP 1.2 output capability.
And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
monitor connected. Source would retrieve EDID from external
and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps
because it did not surpass DP physical bandwidth limitation.
Do modeset to 4k@60fps, source output display data but MST
docking can't output HDMI properly due to this resolution
already over HDMI 1.4 spec.

Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn
instead of available_pbn for bandwidth checks").
Source driver should refer to full_pbn to evaluate sink
output capability. And filter out the resolution surpass
sink output limitation.

v2: Using mgr->base.lock to protect full_pbn.
v3: Add ctx lock.

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 30 ++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 74559379384a..6b6f7eef5b68 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -611,6 +611,32 @@ static int intel_dp_mst_get_modes(struct drm_connector *connector)
 	return intel_dp_mst_get_ddc_modes(connector);
 }
 
+static bool
+intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector *connector, int clock, int bpp)
+{
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	struct intel_dp *intel_dp = intel_connector->mst_port;
+	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
+	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
+	struct drm_modeset_acquire_ctx ctx;
+	bool ret = false;
+
+	if (!mgr)
+		return ret;
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	drm_modeset_lock(&mgr->base.lock, &ctx);
+	if (port->full_pbn)
+		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port->full_pbn);
+	drm_modeset_unlock(&mgr->base.lock);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	return ret;
+}
+
 static enum drm_mode_status
 intel_dp_mst_mode_valid(struct drm_connector *connector,
 			struct drm_display_mode *mode)
@@ -633,7 +659,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
 	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
 	mode_rate = intel_dp_link_required(mode->clock, 18);
 
-	/* TODO - validate mode against available PBN for link */
+	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode->clock, 24))
+		return MODE_CLOCK_HIGH;
+
 	if (mode->clock < 10000)
 		return MODE_CLOCK_LOW;
 
-- 
2.17.1

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/mst: filter out the display mode exceed sink's capability (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/mst: filter out the display mode exceed sink's capability (rev3)
URL   : https://patchwork.freedesktop.org/series/76095/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8498 -> Patchwork_17706
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       [PASS][1] -> [FAIL][2] ([i915#262])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [SKIP][3] ([fdo#109271]) -> [FAIL][4] ([i915#62])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62


Participating hosts (52 -> 44)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_8498 -> Patchwork_17706

  CI-20190529: 20190529
  CI_DRM_8498: 1493c649ae92207a758afa50a639275bd6c80e2e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5659: 66ab5e42811fee3dea8c21ab29e70e323a0650de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17706: 8c7c02df8f7602265c2cc0fa5dc96d7c84b3cf5e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

8c7c02df8f76 drm/i915/mst: filter out the display mode exceed sink's capability

== Logs ==

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

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/mst: filter out the display mode exceed sink's capability (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/mst: filter out the display mode exceed sink's capability (rev3)
URL   : https://patchwork.freedesktop.org/series/76095/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8498_full -> Patchwork_17706_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Possible new issues
-------------------

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@sysfs_preempt_timeout@timeout@vecs0}:
    - shard-glk:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-glk1/igt@sysfs_preempt_timeout@timeout@vecs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-glk9/igt@sysfs_preempt_timeout@timeout@vecs0.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-skl:          [PASS][3] -> [INCOMPLETE][4] ([i915#300])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-skl5/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-skl10/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [PASS][5] -> [DMESG-WARN][6] ([i915#180]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-kbl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip_tiling@flip-changes-tiling:
    - shard-apl:          [PASS][7] -> [FAIL][8] ([i915#95])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-apl8/igt@kms_flip_tiling@flip-changes-tiling.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-apl2/igt@kms_flip_tiling@flip-changes-tiling.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([i915#1188])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-skl5/igt@kms_hdr@bpc-switch-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-skl10/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][11] -> [DMESG-WARN][12] ([i915#180]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#108145] / [i915#265]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#109441]) +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-iclb2/igt@kms_psr@psr2_sprite_render.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-iclb3/igt@kms_psr@psr2_sprite_render.html

  
#### Possible fixes ####

  * igt@gen9_exec_parse@allowed-all:
    - shard-apl:          [DMESG-WARN][17] ([i915#1436] / [i915#716]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-apl1/igt@gen9_exec_parse@allowed-all.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-apl7/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_selftest@live@execlists:
    - shard-skl:          [INCOMPLETE][19] ([i915#1795] / [i915#1874]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-skl2/igt@i915_selftest@live@execlists.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-skl8/igt@i915_selftest@live@execlists.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][21] ([i915#180]) -> [PASS][22] +3 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * {igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1}:
    - shard-glk:          [FAIL][23] ([i915#79]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-glk9/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-glk5/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-hdmi-a1.html

  * {igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1}:
    - shard-skl:          [FAIL][25] ([i915#79]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-skl1/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-skl4/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html

  * {igt@kms_flip@flip-vs-suspend-interruptible@c-dp1}:
    - shard-apl:          [DMESG-WARN][27] ([i915#180]) -> [PASS][28] +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [FAIL][29] ([i915#1188]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-skl10/igt@kms_hdr@bpc-switch.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-skl9/igt@kms_hdr@bpc-switch.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][31] ([i915#173]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-iclb1/igt@kms_psr@no_drrs.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-iclb6/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [SKIP][33] ([fdo#109441]) -> [PASS][34] +2 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-iclb1/igt@kms_psr@psr2_primary_page_flip.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html

  * {igt@perf@polling-parameterized}:
    - shard-tglb:         [FAIL][35] ([i915#1542]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-tglb1/igt@perf@polling-parameterized.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-tglb3/igt@perf@polling-parameterized.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         [FAIL][37] ([i915#454]) -> [SKIP][38] ([i915#468])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-tglb3/igt@i915_pm_dc@dc6-dpms.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-tglb:         [SKIP][39] ([i915#468]) -> [FAIL][40] ([i915#454])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-tglb2/igt@i915_pm_dc@dc6-psr.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-tglb1/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_content_protection@atomic:
    - shard-apl:          [DMESG-FAIL][41] ([fdo#110321]) -> [TIMEOUT][42] ([i915#1319])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-apl7/igt@kms_content_protection@atomic.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-apl7/igt@kms_content_protection@atomic.html

  * igt@kms_content_protection@legacy:
    - shard-apl:          [FAIL][43] ([fdo#110321] / [fdo#110336]) -> [TIMEOUT][44] ([i915#1319])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-apl6/igt@kms_content_protection@legacy.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-apl3/igt@kms_content_protection@legacy.html

  * igt@kms_content_protection@lic:
    - shard-kbl:          [FAIL][45] ([fdo#110321]) -> [TIMEOUT][46] ([i915#1319])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8498/shard-kbl3/igt@kms_content_protection@lic.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17706/shard-kbl3/igt@kms_content_protection@lic.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#173]: https://gitlab.freedesktop.org/drm/intel/issues/173
  [i915#1795]: https://gitlab.freedesktop.org/drm/intel/issues/1795
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1874]: https://gitlab.freedesktop.org/drm/intel/issues/1874
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#300]: https://gitlab.freedesktop.org/drm/intel/issues/300
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_8498 -> Patchwork_17706

  CI-20190529: 20190529
  CI_DRM_8498: 1493c649ae92207a758afa50a639275bd6c80e2e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5659: 66ab5e42811fee3dea8c21ab29e70e323a0650de @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17706: 8c7c02df8f7602265c2cc0fa5dc96d7c84b3cf5e @ 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_17706/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915/mst: filter out the display mode exceed sink's capability

[Lyude Paul]

On Tue, 2020-05-19 at 11:56 +0800, Lee Shawn C wrote:
> So far, max dot clock rate for MST mode rely on physcial
> bandwidth limitation. It would caused compatibility issue
> if source display resolution exceed MST hub output ability.
> 
> For example, source DUT had DP 1.2 output capability.
> And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
> monitor connected. Source would retrieve EDID from external
> and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps
> because it did not surpass DP physical bandwidth limitation.
> Do modeset to 4k@60fps, source output display data but MST
> docking can't output HDMI properly due to this resolution
> already over HDMI 1.4 spec.
> 
> Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn
> instead of available_pbn for bandwidth checks").
> Source driver should refer to full_pbn to evaluate sink
> output capability. And filter out the resolution surpass
> sink output limitation.
> 
> v2: Using mgr->base.lock to protect full_pbn.
> v3: Add ctx lock.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 30 ++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 74559379384a..6b6f7eef5b68 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -611,6 +611,32 @@ static int intel_dp_mst_get_modes(struct drm_connector
> *connector)
>  	return intel_dp_mst_get_ddc_modes(connector);
>  }
>  
> +static bool
> +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector *connector,
> int clock, int bpp)
> +{
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	struct intel_dp *intel_dp = intel_connector->mst_port;
> +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
> +	struct drm_modeset_acquire_ctx ctx;
> +	bool ret = false;
> +
> +	if (!mgr)
> +		return ret;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +	drm_modeset_lock(&mgr->base.lock, &ctx);
> +	if (port->full_pbn)
> +		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port-
> >full_pbn);
> +	drm_modeset_unlock(&mgr->base.lock);
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +
> +	return ret;
> +}

Sorry to do this, but this still isn't right :s, and will likely end up
deadlocking. I'm going to write up a fixed version of this patch and add you in
the Co-developed-by: tag, shouldn't take too long for me to do so I can send it
out today
> +
>  static enum drm_mode_status
>  intel_dp_mst_mode_valid(struct drm_connector *connector,
>  			struct drm_display_mode *mode)
> @@ -633,7 +659,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
>  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>  	mode_rate = intel_dp_link_required(mode->clock, 18);
>  
> -	/* TODO - validate mode against available PBN for link */
> +	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode->clock, 
> 24))
> +		return MODE_CLOCK_HIGH;
> +
>  	if (mode->clock < 10000)
>  		return MODE_CLOCK_LOW;
>  

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

Re: [Intel-gfx] [PATCH v3] drm/i915/mst: filter out the display mode exceed sink's capability

[Lyude Paul]

On Fri, 2020-05-22 at 14:35 -0400, Lyude Paul wrote:
> 
> On Tue, 2020-05-19 at 11:56 +0800, Lee Shawn C wrote:
> > So far, max dot clock rate for MST mode rely on physcial
> > bandwidth limitation. It would caused compatibility issue
> > if source display resolution exceed MST hub output ability.
> > 
> > For example, source DUT had DP 1.2 output capability.
> > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0
> > monitor connected. Source would retrieve EDID from external
> > and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps
> > because it did not surpass DP physical bandwidth limitation.
> > Do modeset to 4k@60fps, source output display data but MST
> > docking can't output HDMI properly due to this resolution
> > already over HDMI 1.4 spec.
> > 
> > Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn
> > instead of available_pbn for bandwidth checks").
> > Source driver should refer to full_pbn to evaluate sink
> > output capability. And filter out the resolution surpass
> > sink output limitation.
> > 
> > v2: Using mgr->base.lock to protect full_pbn.
> > v3: Add ctx lock.
> > 
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > Cc: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 30 ++++++++++++++++++++-
> >  1 file changed, 29 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 74559379384a..6b6f7eef5b68 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -611,6 +611,32 @@ static int intel_dp_mst_get_modes(struct drm_connector
> > *connector)
> >  	return intel_dp_mst_get_ddc_modes(connector);
> >  }
> >  
> > +static bool
> > +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
> > *connector,
> > int clock, int bpp)
> > +{
> > +	struct intel_connector *intel_connector = to_intel_connector(connector);
> > +	struct intel_dp *intel_dp = intel_connector->mst_port;
> > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
> > +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
> > +	struct drm_modeset_acquire_ctx ctx;
> > +	bool ret = false;
> > +
> > +	if (!mgr)
> > +		return ret;
> > +
> > +	drm_modeset_acquire_init(&ctx, 0);
> > +
> > +	drm_modeset_lock(&mgr->base.lock, &ctx);
> > +	if (port->full_pbn)
> > +		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port-
> > > full_pbn);
> > +	drm_modeset_unlock(&mgr->base.lock);
> > +
> > +	drm_modeset_drop_locks(&ctx);
> > +	drm_modeset_acquire_fini(&ctx);
> > +
> > +	return ret;
> > +}
> 
> Sorry to do this, but this still isn't right :s, and will likely end up
> deadlocking. I'm going to write up a fixed version of this patch and add you
> in
> the Co-developed-by: tag, shouldn't take too long for me to do so I can send
> it
> out today

Completely forgot to actually explain what was wrong here - this creates an
acquisition context that would be separate from the caller's lock acquisition
context. So, drm_helper_probe_single_connector_modes() would grab
connection_mutex before calling the hook, and since it's not using an
acquisition context we could potentially deadlock when locking &mgr->base.lock
if another thread locked &mgr->base.lock before connection_mutex. If we add a
lock acquisition context to drm_helper_probe_single_connector_modes and use that
to grab &mgr->base.lock, the ww-mutex code can detect the deadlock and give us a
chance to back off and try again.

anyway-i'll send out the new version of the patch asap

> > +
> >  static enum drm_mode_status
> >  intel_dp_mst_mode_valid(struct drm_connector *connector,
> >  			struct drm_display_mode *mode)
> > @@ -633,7 +659,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
> >  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
> >  	mode_rate = intel_dp_link_required(mode->clock, 18);
> >  
> > -	/* TODO - validate mode against available PBN for link */
> > +	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode-
> > >clock, 
> > 24))
> > +		return MODE_CLOCK_HIGH;
> > +
> >  	if (mode->clock < 10000)
> >  		return MODE_CLOCK_LOW;
> >  

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

Re: [Intel-gfx] [PATCH v3] drm/i915/mst: filter out the display mode exceed sink's capability

[Lee, Shawn C]

On Fri, 2020-05-22, 06:35 p.m, Lyude Paul wrote:
>On Fri, 2020-05-22 at 14:35 -0400, Lyude Paul wrote:
>> 
>> On Tue, 2020-05-19 at 11:56 +0800, Lee Shawn C wrote:
>> > So far, max dot clock rate for MST mode rely on physcial bandwidth 
>> > limitation. It would caused compatibility issue if source display 
>> > resolution exceed MST hub output ability.
>> > 
>> > For example, source DUT had DP 1.2 output capability.
>> > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 monitor 
>> > connected. Source would retrieve EDID from external and get max 
>> > resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did not 
>> > surpass DP physical bandwidth limitation.
>> > Do modeset to 4k@60fps, source output display data but MST docking 
>> > can't output HDMI properly due to this resolution already over HDMI 
>> > 1.4 spec.
>> > 
>> > Refer to commit <fcf463807596> ("drm/dp_mst: Use full_pbn instead of 
>> > available_pbn for bandwidth checks").
>> > Source driver should refer to full_pbn to evaluate sink output 
>> > capability. And filter out the resolution surpass sink output 
>> > limitation.
>> > 
>> > v2: Using mgr->base.lock to protect full_pbn.
>> > v3: Add ctx lock.
>> > 
>> > Cc: Manasi Navare <manasi.d.navare@intel.com>
>> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> > Cc: Cooper Chiou <cooper.chiou@intel.com>
>> > Cc: Lyude Paul <lyude@redhat.com>
>> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_dp_mst.c | 30 
>> > ++++++++++++++++++++-
>> >  1 file changed, 29 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > index 74559379384a..6b6f7eef5b68 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > @@ -611,6 +611,32 @@ static int intel_dp_mst_get_modes(struct 
>> > drm_connector
>> > *connector)
>> >  	return intel_dp_mst_get_ddc_modes(connector);
>> >  }
>> >  
>> > +static bool
>> > +intel_dp_mst_mode_clock_exceed_pbn_bandwidth(struct drm_connector
>> > *connector,
>> > int clock, int bpp)
>> > +{
>> > +	struct intel_connector *intel_connector = to_intel_connector(connector);
>> > +	struct intel_dp *intel_dp = intel_connector->mst_port;
>> > +	struct drm_dp_mst_topology_mgr *mgr = &intel_dp->mst_mgr;
>> > +	struct drm_dp_mst_port *port = (to_intel_connector(connector))->port;
>> > +	struct drm_modeset_acquire_ctx ctx;
>> > +	bool ret = false;
>> > +
>> > +	if (!mgr)
>> > +		return ret;
>> > +
>> > +	drm_modeset_acquire_init(&ctx, 0);
>> > +
>> > +	drm_modeset_lock(&mgr->base.lock, &ctx);
>> > +	if (port->full_pbn)
>> > +		ret = (drm_dp_calc_pbn_mode(clock, bpp, false) > port-
>> > > full_pbn);
>> > +	drm_modeset_unlock(&mgr->base.lock);
>> > +
>> > +	drm_modeset_drop_locks(&ctx);
>> > +	drm_modeset_acquire_fini(&ctx);
>> > +
>> > +	return ret;
>> > +}
>> 
>> Sorry to do this, but this still isn't right :s, and will likely end 
>> up deadlocking. I'm going to write up a fixed version of this patch 
>> and add you in the Co-developed-by: tag, shouldn't take too long for 
>> me to do so I can send it out today
>
>Completely forgot to actually explain what was wrong here - this creates an acquisition context that would be separate from the caller's lock acquisition context. So, drm_helper_probe_single_connector_modes() would grab connection_mutex before calling the hook, and since it's not using an acquisition context we could potentially deadlock when locking &mgr->base.lock if another thread locked &mgr->base.lock before connection_mutex. If we add a lock acquisition context to drm_helper_probe_single_connector_modes and use that to grab &mgr->base.lock, the ww-mutex code can detect the deadlock and give us a chance to back off and try again.
>
>anyway-i'll send out the new version of the patch asap
>

I already test these 2 patches from https://patchwork.freedesktop.org/series/77573/.

Here is my test env.
1. DUT with DP 1.2 support max 4k@60fps output.                 
2. MST hub with HDMI 1.4 support max 4k@30fps output.           
3. BenQ monitor support HDMI 2.0 max 4k@60fps input.            
                                                                
WIth these 2 changes, DUT works meet our expection.             
The resolution from EDID that over 4K@30fps will not be support.

>> > +
>> >  static enum drm_mode_status
>> >  intel_dp_mst_mode_valid(struct drm_connector *connector,
>> >  			struct drm_display_mode *mode)
>> > @@ -633,7 +659,9 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
>> >  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>> >  	mode_rate = intel_dp_link_required(mode->clock, 18);
>> >  
>> > -	/* TODO - validate mode against available PBN for link */
>> > +	if (intel_dp_mst_mode_clock_exceed_pbn_bandwidth(connector, mode-
>> > >clock,
>> > 24))
>> > +		return MODE_CLOCK_HIGH;
>> > +
>> >  	if (mode->clock < 10000)
>> >  		return MODE_CLOCK_LOW;
>> >  
>

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