• Lyude's avatar
    drm/i915/skl: Add support for the SAGV, fix underrun hangs · f4033726
    Lyude authored
    Since the watermark calculations for Skylake are still broken, we're apt
    to hitting underruns very easily under multi-monitor configurations.
    While it would be lovely if this was fixed, it's not. Another problem
    that's been coming from this however, is the mysterious issue of
    underruns causing full system hangs. An easy way to reproduce this with
    a skylake system:
    
    - Get a laptop with a skylake GPU, and hook up two external monitors to
      it
    - Move the cursor from the built-in LCD to one of the external displays
      as quickly as you can
    - You'll get a few pipe underruns, and eventually the entire system will
      just freeze.
    
    After doing a lot of investigation and reading through the bspec, I
    found the existence of the SAGV, which is responsible for adjusting the
    system agent voltage and clock frequencies depending on how much power
    we need. According to the bspec:
    
    "The display engine access to system memory is blocked during the
     adjustment time. SAGV defaults to enabled. Software must use the
     GT-driver pcode mailbox to disable SAGV when the display engine is not
     able to tolerate the blocking time."
    
    The rest of the bspec goes on to explain that software can simply leave
    the SAGV enabled, and disable it when we use interlaced pipes/have more
    then one pipe active.
    
    Sure enough, with this patchset the system hangs resulting from pipe
    underruns on Skylake have completely vanished on my T460s. Additionally,
    the bspec mentions turning off the SAGV	with more then one pipe enabled
    as a workaround for display underruns. While this patch doesn't entirely
    fix that, it looks like it does improve the situation a little bit so
    it's likely this is going to be required to make watermarks on Skylake
    fully functional.
    
    This will still need additional work in the future: we shouldn't be
    enabling the SAGV if any of the currently enabled planes can't enable WM
    levels that introduce latencies >= 30 µs.
    
    Changes since v11:
     - Add skl_can_enable_sagv()
     - Make sure we don't enable SAGV when not all planes can enable
       watermarks >= the SAGV engine block time. I was originally going to
       save this for later, but I recently managed to run into a machine
       that was having problems with a single pipe configuration + SAGV.
     - Make comparisons to I915_SKL_SAGV_NOT_CONTROLLED explicit
     - Change I915_SAGV_DYNAMIC_FREQ to I915_SAGV_ENABLE
     - Move printks outside of mutexes
     - Don't print error messages twice
    Changes since v10:
     - Apparently sandybridge_pcode_read actually writes values and reads
       them back, despite it's misleading function name. This means we've
       been doing this mostly wrong and have been writing garbage to the
       SAGV control. Because of this, we no longer attempt to read the SAGV
       status during initialization (since there are no helpers for this).
     - mlankhorst noticed that this patch was breaking on some very early
       pre-release Skylake machines, which apparently don't allow you to
       disable the SAGV. To prevent machines from failing tests due to SAGV
       errors, if the first time we try to control the SAGV results in the
       mailbox indicating an invalid command, we just disable future attempts
       to control the SAGV state by setting dev_priv->skl_sagv_status to
       I915_SKL_SAGV_NOT_CONTROLLED and make a note of it in dmesg.
     - Move mutex_unlock() a little higher in skl_enable_sagv(). This
       doesn't actually fix anything, but lets us release the lock a little
       sooner since we're finished with it.
    Changes since v9:
     - Only enable/disable sagv on Skylake
    Changes since v8:
     - Add intel_state->modeset guard to the conditional for
       skl_enable_sagv()
    Changes since v7:
     - Remove GEN9_SAGV_LOW_FREQ, replace with GEN9_SAGV_IS_ENABLED (that's
       all we use it for anyway)
     - Use GEN9_SAGV_IS_ENABLED instead of 0x1 for clarification
     - Fix a styling error that snuck past me
    Changes since v6:
     - Protect skl_enable_sagv() with intel_state->modeset conditional in
       intel_atomic_commit_tail()
    Changes since v5:
     - Don't use is_power_of_2. Makes things confusing
     - Don't use the old state to figure out whether or not to
       enable/disable the sagv, use the new one
     - Split the loop in skl_disable_sagv into it's own function
     - Move skl_sagv_enable/disable() calls into intel_atomic_commit_tail()
    Changes since v4:
     - Use is_power_of_2 against active_crtcs to check whether we have > 1
       pipe enabled
     - Fix skl_sagv_get_hw_state(): (temp & 0x1) indicates disabled, 0x0
       enabled
     - Call skl_sagv_enable/disable() from pre/post-plane updates
    Changes since v3:
     - Use time_before() to compare timeout to jiffies
    Changes since v2:
     - Really apply minor style nitpicks to patch this time
    Changes since v1:
     - Added comments about this probably being one of the requirements to
       fixing Skylake's watermark issues
     - Minor style nitpicks from Matt Roper
     - Disable these functions on Broxton, since it doesn't have an SAGV
    Signed-off-by: default avatarLyude <cpaul@redhat.com>
    Cc: Matt Roper <matthew.d.roper@intel.com>
    Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
    Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Cc: stable@vger.kernel.org
    Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Link: http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-3-git-send-email-cpaul@redhat.com
    [mlankhorst: ENOSYS -> ENXIO, whitespace fixes]
    
    (cherry picked from commit 656d1b89)
    Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
    f4033726
i915_drv.h 123 KB