• Imre Deak's avatar
    drm/i915: Add support for asynchronous display power disabling · e0da2d63
    Imre Deak authored
    By disabling a power domain asynchronously we can restrict holding a
    reference on that power domain to the actual code sequence that
    requires the power to be on for the HW access it's doing, by also
    avoiding unneeded on-off-on togglings of the power domain (since the
    disabling happens with a delay).
    
    One benefit is potential power saving due to the following two reasons:
    1. The fact that we will now be holding the reference only for the
       necessary duration by the end of the patchset. While simply not
       delaying the disabling has the same benefit, it has the problem that
       frequent on-off-on power switching has its own power cost (see the 2.
       point below) and the debug trace for power well on/off events will
       cause a lot of dmesg spam (see details about this further below).
    2. Avoiding the power cost of freuqent on-off-on power switching. This
       requires us to find the optimal disabling delay based on the measured
       power cost of on->off and off->on switching of each power well vs.
       the power of keeping the given power well on.
    
       In this patchset I'm not providing this optimal delay for two
       reasons:
       a) I don't have the means yet to perform the measurement (with high
          enough signal-to-noise ratio, or with the help of an energy
          counter that takes switching into account). I'm currently looking
          for a way to measure this.
    
       b) Before reducing the disabling delay we need an alternative way for
          debug tracing powerwell on/off events. Simply avoiding/throttling
          the debug messages is not a solution, see further below.
    
       Note that even in the case where we can't measure any considerable
       power cost of frequent on-off switching of powerwells, it still would
       make sense to do the disabling asynchronously (with 0 delay) to avoid
       blocking on the disabling. On VLV I measured this disabling time
       overhead to be 1ms on average with a worst case of 4ms.
    
    In the case of the AUX power domains on ICL we would also need to keep
    the sequence where we hold the power reference short, the way it would
    be by the end of this patchset where we hold it only for the actual AUX
    transfer. Anything else would make the locking we need for ICL TypeC
    ports (whenever we hold a reference on any AUX power domain) rather
    problematic, adding for instance unnecessary lockdep dependencies to
    the required TypeC port lock.
    
    I chose the disabling delay to be 100msec for now to avoid the unneeded
    toggling (and so not to introduce dmesg spamming) in the DP MST sideband
    signaling code. We could optimize this delay later, once we have the
    means to measure the switching power cost (see above).
    
    Note that simply removing/throttling the debug tracing for power well
    on/off events is not a solution. We need to know the exact spots of
    these events and cannot rely only on incorrect register accesses caught
    (due to not holding a wakeref at the time of access). Incorrect
    powerwell enabling/disabling could lead to other problems, for instance
    we need to keep certain powerwells enabled for the duration of modesets
    and AUX transfers.
    
    v2:
    - Clarify the commit log parts about power cost measurement and the
      problem of simply removing/throttling debug tracing. (Chris)
    - Optimize out local wakeref vars at intel_runtime_pm_put_raw() and
      intel_display_power_put_async() call sites if
      CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n. (Chris)
    - Rebased on v2 of the wakeref w/o power-on guarantee patch.
    - Add missing docbook headers.
    v3:
    - Checkpatch spelling/missing-empty-line fix.
    v4:
    - Fix unintended local wakeref var optimization when using
      call-arguments with side-effects, by using inline funcs instead of
      macros. In this patch in particular this will fix the
      intel_display_power_grab_async_put_ref()->intel_runtime_pm_put_raw()
      call).
    
      No size change in practice (would be the same disregarding the
      corresponding change in intel_display_power_grab_async_put_ref()):
      $ size i915-macro.ko
         text	   data	    bss	    dec	    hex	filename
      2455190	 105890	  10272	2571352	 273c58	i915-macro.ko
      $ size i915-inline.ko
         text	   data	    bss	    dec	    hex	filename
      2455195	 105890	  10272	2571357	 273c5d	i915-inline.ko
    
      Kudos to Stan for reporting the raw-wakeref WARNs this issue caused. His
      config has CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n, which I didn't retest
      after v1, and we are also not testing this config in CI.
    
      Now tested both with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y/n on ICL,
      connecting both Chamelium and regular DP, HDMI sinks.
    
    Cc: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
    Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
    Signed-off-by: default avatarImre Deak <imre.deak@intel.com>
    Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
    Link: https://patchwork.freedesktop.org/patch/msgid/20190513192533.12586-1-imre.deak@intel.com
    e0da2d63
intel_runtime_pm.h 5.09 KB