It is a fact that scheduled work is now improved. But it is also a fact that on core platforms that shouldn't be needed. We only need to actually wait on VLV/CHV. The immediate enabling is actually not an issue for the HW perspective for core platforms that have HW tracking. HW will wait few identical idle frames before transitioning to actual psr active anyways. Note that this patch also remove the delayed activation on HSW and BDW introduced by commit 'd0ac896a477d ("drm/i915: Delay first PSR activation.")'. This was introduced to fix a blank screen on VLV/CHV and also masked some frozen screens on other core platforms. Probably the same that we are now properly hunting and fixing. Furthermore, if we stop using delayed activation on core platforms we will be able, on following up patches, to use available workarounds to make HW tracking properly exit PSR instead of the big nuke of disabling psr and re-enable on exit and activate respectively. At least on few reliable cases. v2:(DK): Remove unnecessary WARN_ONs and make some other VLV | CHV more readable. Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> --- drivers/gpu/drm/i915/i915_debugfs.c | 22 +++++++++++----------- drivers/gpu/drm/i915/intel_psr.c | 28 ++++++++++++++++++---------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 1ac942d1742e..fb106b4a3eaf 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2573,19 +2573,14 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) seq_printf(m, "Busy frontbuffer bits: 0x%03x\n", dev_priv->psr.busy_frontbuffer_bits); - if (timer_pending(&dev_priv->psr.activate_timer)) - seq_printf(m, "Activate scheduled: yes, in %ldms\n", - (long)(dev_priv->psr.earliest_activate - jiffies) * + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { + if (timer_pending(&dev_priv->psr.activate_timer)) + seq_printf(m, "Activate scheduled: yes, in %ldms\n", + (long)(dev_priv->psr.earliest_activate - jiffies) * 1000 / HZ); - else - seq_printf(m, "Re-enable scheduled: no\n"); - - if (HAS_DDI(dev_priv)) { - if (dev_priv->psr.psr2_support) - enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE; else - enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE; - } else { + seq_printf(m, "Re-enable scheduled: no\n"); + for_each_pipe(dev_priv, pipe) { enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv, pipe); @@ -2604,6 +2599,11 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) intel_display_power_put(dev_priv, power_domain); } + } else { + if (dev_priv->psr.psr2_support) + enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE; + else + enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE; } seq_printf(m, "Main link in standby mode: %s\n", diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index c10d5225dc7c..6d413a7bb39f 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -575,6 +575,9 @@ static void intel_psr_schedule(struct drm_i915_private *dev_priv, { unsigned long next; + if (WARN_ON(!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))) + return; + lockdep_assert_held(&dev_priv->psr.lock); /* @@ -667,9 +670,7 @@ void intel_psr_enable(struct intel_dp *intel_dp, dev_priv->psr.enable_source(intel_dp, crtc_state); dev_priv->psr.enabled = intel_dp; - if (INTEL_GEN(dev_priv) >= 9) { - intel_psr_activate(intel_dp); - } else { + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { /* * FIXME: Activation should happen immediately since this * function is just called after pipe is fully trained and @@ -677,10 +678,10 @@ void intel_psr_enable(struct intel_dp *intel_dp, * However on some platforms we face issues when first * activation follows a modeset so quickly. * - On VLV/CHV we get bank screen on first activation - * - On HSW/BDW we get a recoverable frozen screen until - * next exit-activate sequence. */ intel_psr_schedule(dev_priv, intel_dp->panel_power_cycle_delay * 5); + } else { + intel_psr_activate(intel_dp); } unlock: @@ -1072,9 +1073,12 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, if (frontbuffer_bits) intel_psr_exit(dev_priv); - if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) - intel_psr_schedule(dev_priv, 100); - + if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) { + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) + intel_psr_schedule(dev_priv, 100); + else + intel_psr_activate(dev_priv->psr.enabled); + } mutex_unlock(&dev_priv->psr.lock); } @@ -1121,8 +1125,12 @@ void intel_psr_init(struct drm_i915_private *dev_priv) dev_priv->psr.link_standby = false; } - timer_setup(&dev_priv->psr.activate_timer, intel_psr_timer_fn, 0); - INIT_WORK(&dev_priv->psr.activate_work, intel_psr_work); + if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { + timer_setup(&dev_priv->psr.activate_timer, + intel_psr_timer_fn, 0); + INIT_WORK(&dev_priv->psr.activate_work, intel_psr_work); + } + mutex_init(&dev_priv->psr.lock); if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { -- 2.13.6 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx