All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 0/1] Display glitches fixes
@ 2020-12-04  8:18 Anshuman Gupta
  2020-12-04  8:18 ` [Intel-gfx] [PATCH 1/1] drm/i915/dp: optimize pps_lock wherever required Anshuman Gupta
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Anshuman Gupta @ 2020-12-04  8:18 UTC (permalink / raw)
  To: intel-gfx

TGL chrome-OS platform has observed display glitches while
display brightness is being changed rapidly.
It creates a scenario where brightness is being changed simultaneously
with display flips updates in different threads.

Brightness update requires pps_lock, current pps_lock implementation
requires to get AUX power domain, which triggers DC3CO disallow/allow
sequence being async with display flips.

This triggers a race between dc3co exit delay and display flip,
and potentially causing a display glitch due to some display register
being programmed before completing dc3co exit delay.
Exactly which register programming causing this glitch is still
being work in progressed.

This patch adds a optimization to avoid unnecessary "DC off"
power well enable/disable(D3CO disallow/allow) noise around
brightness update.

This patch helps to remove the display glitch in case of simultaneous
brightness update and display flips.

Anshuman Gupta (1):
  drm/i915/dp: optimize pps_lock wherever required

 drivers/gpu/drm/i915/display/intel_dp.c | 47 +++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

-- 
2.26.2

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Intel-gfx] [PATCH 1/1] drm/i915/dp: optimize pps_lock wherever required
  2020-12-04  8:18 [Intel-gfx] [PATCH 0/1] Display glitches fixes Anshuman Gupta
@ 2020-12-04  8:18 ` Anshuman Gupta
  2020-12-11 14:13   ` Jani Nikula
  2020-12-04  9:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success for Display glitches fixes (rev2) Patchwork
  2020-12-04 10:28 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 1 reply; 6+ messages in thread
From: Anshuman Gupta @ 2020-12-04  8:18 UTC (permalink / raw)
  To: intel-gfx

Reading backlight status from PPS register doesn't require
AUX power on the platform which has South Display Engine on PCH.
It invokes a unnecessary power well enable/disable noise.
optimize it wherever is possible.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 47 +++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 2d4d5e95af84..7e18e4ff50f4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -892,6 +892,47 @@ pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref)
 	return 0;
 }
 
+/*
+ * Platform with PCH based SDE doesn't require to enable AUX power
+ * for simple PPS register access like whether backlight is enabled.
+ * use pch_pps_lock()/pch_pps_unlock() wherever we don't require
+ * aux power to avoid unnecessary power well enable/disable back
+ * and forth.
+ */
+static intel_wakeref_t
+pch_pps_lock(struct intel_dp *intel_dp)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	intel_wakeref_t wakeref;
+
+	if (!HAS_PCH_SPLIT(dev_priv))
+		wakeref = intel_display_power_get(dev_priv,
+						  intel_aux_power_domain(dp_to_dig_port(intel_dp)));
+	else
+		wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
+
+	mutex_lock(&dev_priv->pps_mutex);
+
+	return wakeref;
+}
+
+static intel_wakeref_t
+pch_pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+	mutex_unlock(&dev_priv->pps_mutex);
+
+	if (!HAS_PCH_SPLIT(dev_priv))
+		intel_display_power_put(dev_priv,
+					intel_aux_power_domain(dp_to_dig_port(intel_dp)),
+					wakeref);
+	else
+		intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
+
+	return 0;
+}
+
 #define with_pps_lock(dp, wf) \
 	for ((wf) = pps_lock(dp); (wf); (wf) = pps_unlock((dp), (wf)))
 
@@ -3453,8 +3494,10 @@ static void intel_edp_backlight_power(struct intel_connector *connector,
 	bool is_enabled;
 
 	is_enabled = false;
-	with_pps_lock(intel_dp, wakeref)
-		is_enabled = ilk_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
+	wakeref = pch_pps_lock(intel_dp);
+	is_enabled = ilk_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
+	pch_pps_unlock(intel_dp, wakeref);
+
 	if (is_enabled == enable)
 		return;
 
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for Display glitches fixes (rev2)
  2020-12-04  8:18 [Intel-gfx] [PATCH 0/1] Display glitches fixes Anshuman Gupta
  2020-12-04  8:18 ` [Intel-gfx] [PATCH 1/1] drm/i915/dp: optimize pps_lock wherever required Anshuman Gupta
@ 2020-12-04  9:12 ` Patchwork
  2020-12-04 10:28 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-12-04  9:12 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 1383 bytes --]

== Series Details ==

Series: Display glitches fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/84394/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9441 -> Patchwork_19056
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

New tests
---------

  New tests have been introduced between CI_DRM_9441 and Patchwork_19056:

### New CI tests (1) ###

  * boot:
    - Statuses : 1 fail(s) 37 pass(s)
    - Exec time: [0.0] s

  


Changes
-------

  No changes found


Participating hosts (43 -> 38)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-tgl-y fi-bdw-samus 


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

  * Linux: CI_DRM_9441 -> Patchwork_19056

  CI-20190529: 20190529
  CI_DRM_9441: 6e992bb7585d1bee238776d8fe0512a70a22a1a0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5880: d67bad61dc9a7515f94a7eecadd3bcd6b4f9d49e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19056: 8da72c4348fd85d565c8e83b497b6c8f5233a3fa @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

8da72c4348fd drm/i915/dp: optimize pps_lock wherever required

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/index.html

[-- Attachment #1.2: Type: text/html, Size: 1984 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for Display glitches fixes (rev2)
  2020-12-04  8:18 [Intel-gfx] [PATCH 0/1] Display glitches fixes Anshuman Gupta
  2020-12-04  8:18 ` [Intel-gfx] [PATCH 1/1] drm/i915/dp: optimize pps_lock wherever required Anshuman Gupta
  2020-12-04  9:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success for Display glitches fixes (rev2) Patchwork
@ 2020-12-04 10:28 ` Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-12-04 10:28 UTC (permalink / raw)
  To: Anshuman Gupta; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 12317 bytes --]

== Series Details ==

Series: Display glitches fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/84394/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9441_full -> Patchwork_19056_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

New tests
---------

  New tests have been introduced between CI_DRM_9441_full and Patchwork_19056_full:

### New CI tests (1) ###

  * boot:
    - Statuses : 198 pass(s)
    - Exec time: [0.0] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_reloc@basic-many-active@rcs0:
    - shard-apl:          [PASS][1] -> [FAIL][2] ([i915#2389])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-apl2/igt@gem_exec_reloc@basic-many-active@rcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-apl7/igt@gem_exec_reloc@basic-many-active@rcs0.html

  * igt@gem_exec_whisper@basic-contexts-forked:
    - shard-glk:          [PASS][3] -> [DMESG-WARN][4] ([i915#118] / [i915#95])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-glk5/igt@gem_exec_whisper@basic-contexts-forked.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-glk4/igt@gem_exec_whisper@basic-contexts-forked.html

  * igt@gem_workarounds@suspend-resume:
    - shard-snb:          [PASS][5] -> [DMESG-WARN][6] ([i915#42])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-snb4/igt@gem_workarounds@suspend-resume.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-snb6/igt@gem_workarounds@suspend-resume.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding:
    - shard-skl:          [PASS][7] -> [FAIL][8] ([i915#54]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-skl1/igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-skl9/igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-hsw:          [PASS][9] -> [FAIL][10] ([i915#96])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([i915#52] / [i915#54])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-skl10/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-skl5/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1:
    - shard-apl:          [PASS][13] -> [FAIL][14] ([i915#79])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-apl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-apl2/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          [PASS][15] -> [INCOMPLETE][16] ([i915#155] / [i915#180])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109441]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-iclb3/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@kms_vblank@pipe-b-wait-idle-hang:
    - shard-snb:          [PASS][19] -> [SKIP][20] ([fdo#109271]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-snb5/igt@kms_vblank@pipe-b-wait-idle-hang.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-snb2/igt@kms_vblank@pipe-b-wait-idle-hang.html

  
#### Possible fixes ####

  * igt@feature_discovery@psr2:
    - shard-iclb:         [SKIP][21] ([i915#658]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-iclb6/igt@feature_discovery@psr2.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-iclb2/igt@feature_discovery@psr2.html

  * igt@gem_exec_gttfill@engines@rcs0:
    - shard-glk:          [DMESG-WARN][23] ([i915#118] / [i915#95]) -> [PASS][24] +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-glk7/igt@gem_exec_gttfill@engines@rcs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-glk1/igt@gem_exec_gttfill@engines@rcs0.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x64-offscreen:
    - shard-skl:          [FAIL][25] ([i915#54]) -> [PASS][26] +4 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-skl4/igt@kms_cursor_crc@pipe-b-cursor-64x64-offscreen.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-skl7/igt@kms_cursor_crc@pipe-b-cursor-64x64-offscreen.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][27] ([i915#72]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-glk1/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-glk8/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_cursor_legacy@cursor-vs-flip-toggle:
    - shard-hsw:          [FAIL][29] ([i915#2370]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-hsw1/igt@kms_cursor_legacy@cursor-vs-flip-toggle.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-toggle.html

  * igt@kms_cursor_legacy@flip-vs-cursor-legacy:
    - shard-tglb:         [FAIL][31] ([i915#2346]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-tglb1/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-tglb8/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-kbl:          [INCOMPLETE][33] ([i915#155] / [i915#180]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-kbl2/igt@kms_fbcon_fbt@fbc-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-kbl2/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1:
    - shard-skl:          [FAIL][35] ([i915#2122]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-skl3/igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-skl5/igt@kms_flip@plain-flip-fb-recreate-interruptible@c-edp1.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][37] ([i915#1188]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-skl2/igt@kms_hdr@bpc-switch-dpms.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-skl8/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane_lowres@pipe-c-tiling-yf:
    - shard-skl:          [DMESG-WARN][39] ([i915#1982]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-skl4/igt@kms_plane_lowres@pipe-c-tiling-yf.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-skl10/igt@kms_plane_lowres@pipe-c-tiling-yf.html

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

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-skl:          [INCOMPLETE][43] ([i915#198] / [i915#2295]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-skl5/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-skl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Warnings ####

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         [WARN][45] ([i915#2684]) -> [FAIL][46] ([i915#2680])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-iclb2/igt@i915_pm_rc6_residency@rc6-idle.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-iclb2/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@runner@aborted:
    - shard-kbl:          ([FAIL][47], [FAIL][48]) ([i915#2295] / [i915#2722] / [i915#483] / [i915#92]) -> ([FAIL][49], [FAIL][50]) ([i915#1814] / [i915#2295] / [i915#2722] / [i915#483])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-kbl6/igt@runner@aborted.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9441/shard-kbl2/igt@runner@aborted.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-kbl2/igt@runner@aborted.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19056/shard-kbl6/igt@runner@aborted.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1814]: https://gitlab.freedesktop.org/drm/intel/issues/1814
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2370]: https://gitlab.freedesktop.org/drm/intel/issues/2370
  [i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389
  [i915#2680]: https://gitlab.freedesktop.org/drm/intel/issues/2680
  [i915#2684]: https://gitlab.freedesktop.org/drm/intel/issues/2684
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#42]: https://gitlab.freedesktop.org/drm/intel/issues/42
  [i915#483]: https://gitlab.freedesktop.org/drm/intel/issues/483
  [i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
  [i915#96]: https://gitlab.freedesktop.org/drm/intel/issues/96


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

  No changes in participating hosts


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

  * Linux: CI_DRM_9441 -> Patchwork_19056

  CI-20190529: 20190529
  CI_DRM_9441: 6e992bb7585d1bee238776d8fe0512a70a22a1a0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5880: d67bad61dc9a7515f94a7eecadd3bcd6b4f9d49e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19056: 8da72c4348fd85d565c8e83b497b6c8f5233a3fa @ 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_19056/index.html

[-- Attachment #1.2: Type: text/html, Size: 14457 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH 1/1] drm/i915/dp: optimize pps_lock wherever required
  2020-12-04  8:18 ` [Intel-gfx] [PATCH 1/1] drm/i915/dp: optimize pps_lock wherever required Anshuman Gupta
@ 2020-12-11 14:13   ` Jani Nikula
  2020-12-15  6:40     ` Anshuman Gupta
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2020-12-11 14:13 UTC (permalink / raw)
  To: Anshuman Gupta, intel-gfx

On Fri, 04 Dec 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> Reading backlight status from PPS register doesn't require
> AUX power on the platform which has South Display Engine on PCH.
> It invokes a unnecessary power well enable/disable noise.
> optimize it wherever is possible.

Three aspects here:

1. What's the root cause for the glitches, really? AFAICT this is still
an open question, judging from the discussion in previous versions.

2. See why we end up here in the first place for brightness
updates. It's a long story (*), but maybe the fix isn't to optimize this
path, but to avoid calling this function for regular brightness updates
to begin with?

3. The implementation here seems like a hack, to be honest. Considering
the points above, it really has a bad vibe of papering over something
else.

BR,
Jani.



(*)
It was a Chrome OS requirement originally to be able to quickly switch
off backlight through the backlight sysfs interface, without switching
off the display through the KMS API. For whatever reason. We can't just
set the PWM to 0, because that may an invalid thing to do on some boards
out there. (On some device it ended up pulling other lanes on the eDP
connector to 0 V, but I digress.)

So the hack is we have a way to switch the eDP power sequencer backlight
bit off/on, as a substate of enabled backlight, through using the
backlight sysfs to set the brightness to 0 or using bl_power.

>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 47 +++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 2d4d5e95af84..7e18e4ff50f4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -892,6 +892,47 @@ pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref)
>  	return 0;
>  }
>  
> +/*
> + * Platform with PCH based SDE doesn't require to enable AUX power
> + * for simple PPS register access like whether backlight is enabled.
> + * use pch_pps_lock()/pch_pps_unlock() wherever we don't require
> + * aux power to avoid unnecessary power well enable/disable back
> + * and forth.
> + */
> +static intel_wakeref_t
> +pch_pps_lock(struct intel_dp *intel_dp)
> +{
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	intel_wakeref_t wakeref;
> +
> +	if (!HAS_PCH_SPLIT(dev_priv))
> +		wakeref = intel_display_power_get(dev_priv,
> +						  intel_aux_power_domain(dp_to_dig_port(intel_dp)));
> +	else
> +		wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> +
> +	mutex_lock(&dev_priv->pps_mutex);
> +
> +	return wakeref;
> +}
> +
> +static intel_wakeref_t
> +pch_pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref)
> +{
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> +	mutex_unlock(&dev_priv->pps_mutex);
> +
> +	if (!HAS_PCH_SPLIT(dev_priv))
> +		intel_display_power_put(dev_priv,
> +					intel_aux_power_domain(dp_to_dig_port(intel_dp)),
> +					wakeref);
> +	else
> +		intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> +
> +	return 0;
> +}
> +
>  #define with_pps_lock(dp, wf) \
>  	for ((wf) = pps_lock(dp); (wf); (wf) = pps_unlock((dp), (wf)))
>  
> @@ -3453,8 +3494,10 @@ static void intel_edp_backlight_power(struct intel_connector *connector,
>  	bool is_enabled;
>  
>  	is_enabled = false;
> -	with_pps_lock(intel_dp, wakeref)
> -		is_enabled = ilk_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
> +	wakeref = pch_pps_lock(intel_dp);
> +	is_enabled = ilk_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
> +	pch_pps_unlock(intel_dp, wakeref);
> +
>  	if (is_enabled == enable)
>  		return;

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH 1/1] drm/i915/dp: optimize pps_lock wherever required
  2020-12-11 14:13   ` Jani Nikula
@ 2020-12-15  6:40     ` Anshuman Gupta
  0 siblings, 0 replies; 6+ messages in thread
From: Anshuman Gupta @ 2020-12-15  6:40 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On 2020-12-11 at 16:13:56 +0200, Jani Nikula wrote:
> On Fri, 04 Dec 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> > Reading backlight status from PPS register doesn't require
> > AUX power on the platform which has South Display Engine on PCH.
> > It invokes a unnecessary power well enable/disable noise.
> > optimize it wherever is possible.
> 
> Three aspects here:
Thanks Jani for comments.
> 
> 1. What's the root cause for the glitches, really? AFAICT this is still
> an open question, judging from the discussion in previous versions.
Yes it is still open, but it can be concluded from the experiments (*)
that this issue is not due to race in driver between flips update and
brightness update.
> 
> 2. See why we end up here in the first place for brightness
> updates. It's a long story (*), but maybe the fix isn't to optimize this
> path, but to avoid calling this function for regular brightness updates
> to begin with?
Agree with you, may be this is the correct time to pursue for a correct fix.
> 
> 3. The implementation here seems like a hack, to be honest. Considering
> the points above, it really has a bad vibe of papering over something
> else.
Could you please provide your inputs to improve this patch so chrome-os
can use this patch for their consumption. 
Meanwhile in parallel we can work to fix this brightness interface.

(*) Experiments:
1. Get/Put POWER_DOMAIN_MODESET power domain always in atomic_commit_tail (suggested by Ville).
   Not helping to fix the glitch.
2. 200us delay in starts of atomic_commit_tail to serialze the flips against DC3CO exit delay(200us).
   Not helping to fix the glitch.
> 
> BR,
> Jani.
> 
> 
> 
> (*)
> It was a Chrome OS requirement originally to be able to quickly switch
> off backlight through the backlight sysfs interface, without switching
> off the display through the KMS API. For whatever reason. We can't just
> set the PWM to 0, because that may an invalid thing to do on some boards
> out there. (On some device it ended up pulling other lanes on the eDP
> connector to 0 V, but I digress.)
For my curiosity i am interested to know, how did other linux distribution 
like ubuntu handled the brightness update by dedicated brightness key before
this original requirement from chrome-os?
Thanks,
Anshuman Gupta.
> 
> So the hack is we have a way to switch the eDP power sequencer backlight
> bit off/on, as a substate of enabled backlight, through using the
> backlight sysfs to set the brightness to 0 or using bl_power.
> 
> >
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 47 +++++++++++++++++++++++--
> >  1 file changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 2d4d5e95af84..7e18e4ff50f4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -892,6 +892,47 @@ pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * Platform with PCH based SDE doesn't require to enable AUX power
> > + * for simple PPS register access like whether backlight is enabled.
> > + * use pch_pps_lock()/pch_pps_unlock() wherever we don't require
> > + * aux power to avoid unnecessary power well enable/disable back
> > + * and forth.
> > + */
> > +static intel_wakeref_t
> > +pch_pps_lock(struct intel_dp *intel_dp)
> > +{
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +	intel_wakeref_t wakeref;
> > +
> > +	if (!HAS_PCH_SPLIT(dev_priv))
> > +		wakeref = intel_display_power_get(dev_priv,
> > +						  intel_aux_power_domain(dp_to_dig_port(intel_dp)));
> > +	else
> > +		wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> > +
> > +	mutex_lock(&dev_priv->pps_mutex);
> > +
> > +	return wakeref;
> > +}
> > +
> > +static intel_wakeref_t
> > +pch_pps_unlock(struct intel_dp *intel_dp, intel_wakeref_t wakeref)
> > +{
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +
> > +	mutex_unlock(&dev_priv->pps_mutex);
> > +
> > +	if (!HAS_PCH_SPLIT(dev_priv))
> > +		intel_display_power_put(dev_priv,
> > +					intel_aux_power_domain(dp_to_dig_port(intel_dp)),
> > +					wakeref);
> > +	else
> > +		intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> > +
> > +	return 0;
> > +}
> > +
> >  #define with_pps_lock(dp, wf) \
> >  	for ((wf) = pps_lock(dp); (wf); (wf) = pps_unlock((dp), (wf)))
> >  
> > @@ -3453,8 +3494,10 @@ static void intel_edp_backlight_power(struct intel_connector *connector,
> >  	bool is_enabled;
> >  
> >  	is_enabled = false;
> > -	with_pps_lock(intel_dp, wakeref)
> > -		is_enabled = ilk_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
> > +	wakeref = pch_pps_lock(intel_dp);
> > +	is_enabled = ilk_get_pp_control(intel_dp) & EDP_BLC_ENABLE;
> > +	pch_pps_unlock(intel_dp, wakeref);
> > +
> >  	if (is_enabled == enable)
> >  		return;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-15  6:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-04  8:18 [Intel-gfx] [PATCH 0/1] Display glitches fixes Anshuman Gupta
2020-12-04  8:18 ` [Intel-gfx] [PATCH 1/1] drm/i915/dp: optimize pps_lock wherever required Anshuman Gupta
2020-12-11 14:13   ` Jani Nikula
2020-12-15  6:40     ` Anshuman Gupta
2020-12-04  9:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success for Display glitches fixes (rev2) Patchwork
2020-12-04 10:28 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.