diff mbox

[1/5] drm/vblank: Do not update vblank counts if vblanks are already disabled.

Message ID 20171206224741.8600-1-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dhinakaran Pandiyan Dec. 6, 2017, 10:47 p.m. UTC 
Updating the vblank counts requires register reads and these reads may not
return meaningful values after the vblank interrupts are disabled as the
device may go to low power state. An additional change would be to allow
the driver to save the vblank counts before entering a low power state, but
that's for the future.

Also, disable vblanks after reading the HW counter in the case where
_crtc_vblank_off() is disabling vblanks.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/drm_vblank.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

Comments

Dhinakaran Pandiyan Dec. 7, 2017, 6:46 p.m. UTC | #1 
On Wed, 2017-12-06 at 23:16 +0000, Patchwork wrote:
> == Series Details ==

> 

> Series: series starting with [1/5] drm/vblank: Do not update vblank counts if vblanks are already disabled.

> URL   : https://patchwork.freedesktop.org/series/34996/

> State : failure

> 

> == Summary ==

> 

> Series 34996v1 series starting with [1/5] drm/vblank: Do not update vblank counts if vblanks are already disabled.

> https://patchwork.freedesktop.org/api/1.0/series/34996/revisions/1/mbox/

> 

> Test debugfs_test:

>         Subgroup read_all_entries:

>                 dmesg-fail -> PASS       (fi-elk-e7500) fdo#103989 +1

> Test gem_exec_reloc:

>         Subgroup basic-cpu-read-active:

>                 pass       -> FAIL       (fi-gdg-551) fdo#102582 +3

> Test kms_pipe_crc_basic:

>         Subgroup read-crc-pipe-a:

>                 pass       -> FAIL       (fi-skl-6700k)

>         Subgroup suspend-read-crc-pipe-b:

>                 pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

> Test kms_psr_sink_crc:

>         Subgroup psr_basic:

>                 pass       -> DMESG-WARN (fi-skl-6600u)

>                 pass       -> DMESG-WARN (fi-skl-6700hq)

>                 pass       -> DMESG-WARN (fi-kbl-7560u)

>                 pass       -> DMESG-WARN (fi-kbl-r)


Lockdep failures, I think switching to irqsave spin locks will fix
these.
> 

> fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989

> fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582

> fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

> 

> fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:438s

> fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:385s

> fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:522s

> fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:280s

> fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:505s

> fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:511s

> fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:491s

> fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:472s

> fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 

> fi-gdg-551       total:288  pass:174  dwarn:1   dfail:0   fail:5   skip:108 time:270s

> fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:542s

> fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:385s

> fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:260s

> fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s

> fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:444s

> fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:526s

> fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:478s

> fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:531s

> fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:595s

> fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s

> fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:547s

> fi-skl-6700hq    total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:26  time:561s

> fi-skl-6700k     total:288  pass:263  dwarn:0   dfail:0   fail:1   skip:24  time:519s

> fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:496s

> fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

> fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:423s

> Blacklisted hosts:

> fi-cnl-y         total:249  pass:224  dwarn:0   dfail:0   fail:0   skip:24 

> fi-glk-dsi       total:288  pass:186  dwarn:1   dfail:4   fail:0   skip:97  time:397s

> 

> 95f37eb3ebfd65f500e0e80c6788df200f4c2f99 drm-tip: 2017y-12m-06d-21h-01m-04s UTC integration manifest

> 2864dbfcc343 drm/i915: Use the vblank power domain disallow or disable DC states.

> b0c13977ae82 drm/i915: Introduce a non-blocking power domain for vblank interrupts

> c32242c2a8b1 drm/i915: Use an atomic_t array to track power domain use count.

> 6de634ba83cf drm/vblank: Restoring vblank counts after device runtime PM events.

> 43170fd34edb drm/vblank: Do not update vblank counts if vblanks are already disabled.

> 

> == Logs ==

> 

> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7432/

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan Dec. 7, 2017, 6:46 p.m. UTC | #2 
On Wed, 2017-12-06 at 23:16 +0000, Patchwork wrote:
> == Series Details ==

> 

> Series: series starting with [1/5] drm/vblank: Do not update vblank counts if vblanks are already disabled.

> URL   : https://patchwork.freedesktop.org/series/34996/

> State : failure

> 

> == Summary ==

> 

> Series 34996v1 series starting with [1/5] drm/vblank: Do not update vblank counts if vblanks are already disabled.

> https://patchwork.freedesktop.org/api/1.0/series/34996/revisions/1/mbox/

> 

> Test debugfs_test:

>         Subgroup read_all_entries:

>                 dmesg-fail -> PASS       (fi-elk-e7500) fdo#103989 +1

> Test gem_exec_reloc:

>         Subgroup basic-cpu-read-active:

>                 pass       -> FAIL       (fi-gdg-551) fdo#102582 +3

> Test kms_pipe_crc_basic:

>         Subgroup read-crc-pipe-a:

>                 pass       -> FAIL       (fi-skl-6700k)

>         Subgroup suspend-read-crc-pipe-b:

>                 pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

> Test kms_psr_sink_crc:

>         Subgroup psr_basic:

>                 pass       -> DMESG-WARN (fi-skl-6600u)

>                 pass       -> DMESG-WARN (fi-skl-6700hq)

>                 pass       -> DMESG-WARN (fi-kbl-7560u)

>                 pass       -> DMESG-WARN (fi-kbl-r)


Lockdep failures, I think switching to irqsave spin locks will fix
these.
> 

> fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989

> fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582

> fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

> 

> fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:438s

> fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:385s

> fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:522s

> fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:280s

> fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:505s

> fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:511s

> fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:491s

> fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:472s

> fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 

> fi-gdg-551       total:288  pass:174  dwarn:1   dfail:0   fail:5   skip:108 time:270s

> fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:542s

> fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:385s

> fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:260s

> fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s

> fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:444s

> fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:526s

> fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:478s

> fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:531s

> fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:595s

> fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s

> fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:547s

> fi-skl-6700hq    total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:26  time:561s

> fi-skl-6700k     total:288  pass:263  dwarn:0   dfail:0   fail:1   skip:24  time:519s

> fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:496s

> fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

> fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:423s

> Blacklisted hosts:

> fi-cnl-y         total:249  pass:224  dwarn:0   dfail:0   fail:0   skip:24 

> fi-glk-dsi       total:288  pass:186  dwarn:1   dfail:4   fail:0   skip:97  time:397s

> 

> 95f37eb3ebfd65f500e0e80c6788df200f4c2f99 drm-tip: 2017y-12m-06d-21h-01m-04s UTC integration manifest

> 2864dbfcc343 drm/i915: Use the vblank power domain disallow or disable DC states.

> b0c13977ae82 drm/i915: Introduce a non-blocking power domain for vblank interrupts

> c32242c2a8b1 drm/i915: Use an atomic_t array to track power domain use count.

> 6de634ba83cf drm/vblank: Restoring vblank counts after device runtime PM events.

> 43170fd34edb drm/vblank: Do not update vblank counts if vblanks are already disabled.

> 

> == Logs ==

> 

> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7432/

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Dhinakaran Pandiyan Dec. 7, 2017, 6:47 p.m. UTC | #3 
On Wed, 2017-12-06 at 23:16 +0000, Patchwork wrote:
> == Series Details ==

> 

> Series: series starting with [1/5] drm/vblank: Do not update vblank counts if vblanks are already disabled.

> URL   : https://patchwork.freedesktop.org/series/34996/

> State : failure

> 

> == Summary ==

> 

> Series 34996v1 series starting with [1/5] drm/vblank: Do not update vblank counts if vblanks are already disabled.

> https://patchwork.freedesktop.org/api/1.0/series/34996/revisions/1/mbox/

> 

> Test debugfs_test:

>         Subgroup read_all_entries:

>                 dmesg-fail -> PASS       (fi-elk-e7500) fdo#103989 +1

> Test gem_exec_reloc:

>         Subgroup basic-cpu-read-active:

>                 pass       -> FAIL       (fi-gdg-551) fdo#102582 +3

> Test kms_pipe_crc_basic:

>         Subgroup read-crc-pipe-a:

>                 pass       -> FAIL       (fi-skl-6700k)

>         Subgroup suspend-read-crc-pipe-b:

>                 pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

> Test kms_psr_sink_crc:

>         Subgroup psr_basic:

>                 pass       -> DMESG-WARN (fi-skl-6600u)

>                 pass       -> DMESG-WARN (fi-skl-6700hq)

>                 pass       -> DMESG-WARN (fi-kbl-7560u)

>                 pass       -> DMESG-WARN (fi-kbl-r)


Lockdep failures, I think switching to irqsave spin locks will fix
these.
> 

> fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989

> fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582

> fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

> 

> fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:438s

> fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:385s

> fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:522s

> fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:280s

> fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:505s

> fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:511s

> fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:491s

> fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:472s

> fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 

> fi-gdg-551       total:288  pass:174  dwarn:1   dfail:0   fail:5   skip:108 time:270s

> fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:542s

> fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:385s

> fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:260s

> fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s

> fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:444s

> fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:526s

> fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:478s

> fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:531s

> fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:595s

> fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s

> fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:547s

> fi-skl-6700hq    total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:26  time:561s

> fi-skl-6700k     total:288  pass:263  dwarn:0   dfail:0   fail:1   skip:24  time:519s

> fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:496s

> fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 

> fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:423s

> Blacklisted hosts:

> fi-cnl-y         total:249  pass:224  dwarn:0   dfail:0   fail:0   skip:24 

> fi-glk-dsi       total:288  pass:186  dwarn:1   dfail:4   fail:0   skip:97  time:397s

> 

> 95f37eb3ebfd65f500e0e80c6788df200f4c2f99 drm-tip: 2017y-12m-06d-21h-01m-04s UTC integration manifest

> 2864dbfcc343 drm/i915: Use the vblank power domain disallow or disable DC states.

> b0c13977ae82 drm/i915: Introduce a non-blocking power domain for vblank interrupts

> c32242c2a8b1 drm/i915: Use an atomic_t array to track power domain use count.

> 6de634ba83cf drm/vblank: Restoring vblank counts after device runtime PM events.

> 43170fd34edb drm/vblank: Do not update vblank counts if vblanks are already disabled.

> 

> == Logs ==

> 

> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7432/

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 32d9bcf5be7f..7eee82c06ed8 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -347,23 +347,14 @@  void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
 	spin_lock_irqsave(&dev->vblank_time_lock, irqflags);
 
 	/*
-	 * Only disable vblank interrupts if they're enabled. This avoids
-	 * calling the ->disable_vblank() operation in atomic context with the
-	 * hardware potentially runtime suspended.
-	 */
-	if (vblank->enabled) {
-		__disable_vblank(dev, pipe);
-		vblank->enabled = false;
-	}
-
-	/*
-	 * Always update the count and timestamp to maintain the
+	 * Update the count and timestamp to maintain the
 	 * appearance that the counter has been ticking all along until
 	 * this time. This makes the count account for the entire time
 	 * between drm_crtc_vblank_on() and drm_crtc_vblank_off().
 	 */
 	drm_update_vblank_count(dev, pipe, false);
-
+	__disable_vblank(dev, pipe);
+	vblank->enabled = false;
 	spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
 }
 
@@ -1122,8 +1113,12 @@  void drm_crtc_vblank_off(struct drm_crtc *crtc)
 		      pipe, vblank->enabled, vblank->inmodeset);
 
 	/* Avoid redundant vblank disables without previous
-	 * drm_crtc_vblank_on(). */
-	if (drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset)
+	 * drm_crtc_vblank_on() and only disable them if they're enabled. This
+	 * avoids calling the ->disable_vblank() operation in atomic context
+	 * with the hardware potentially runtime suspended.
+	 */
+	if ((drm_core_check_feature(dev, DRIVER_ATOMIC) || !vblank->inmodeset) &&
+	    vblank->enabled)
 		drm_vblank_disable_and_save(dev, pipe);
 
 	wake_up(&vblank->queue);