Skip to content

Commit

Permalink
drm/i915: Track all held rpm wakerefs
Browse files Browse the repository at this point in the history
Everytime we take a wakeref, record the stack trace of where it was
taken; clearing the set if we ever drop back to no owners. For debugging
a rpm leak, we can look at all the current wakerefs and check if they
have a matching rpm_put.

v2: Use skip=0 for unwinding the stack as it appears our noinline
function doesn't appear on the stack (nor does save_stack_trace itself!)
v3: Allow rpm->debug_count to disappear between inspections and so
avoid calling krealloc(0) as that may return a ZERO_PTR not NULL! (Mika)
v4: Show who last acquire/released the runtime pm

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Tested-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190114142129.24398-1-chris@chris-wilson.co.uk
  • Loading branch information
Chris Wilson committed Jan 14, 2019
1 parent 74256b7 commit bd780f3
Show file tree
Hide file tree
Showing 7 changed files with 324 additions and 50 deletions.
3 changes: 2 additions & 1 deletion drivers/gpu/drm/i915/Kconfig.debug
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ config DRM_I915_DEBUG
select DEBUG_FS
select PREEMPT_COUNT
select I2C_CHARDEV
select STACKDEPOT
select DRM_DP_AUX_CHARDEV
select X86_MSR # used by igt/pm_rpm
select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks)
select DRM_DEBUG_MM if DRM=y
select STACKDEPOT if DRM=y # for DRM_DEBUG_MM
select DRM_DEBUG_SELFTEST
select SW_SYNC # signaling validation framework (igt/syncobj*)
select DRM_I915_SW_FENCE_DEBUG_OBJECTS
Expand Down Expand Up @@ -173,6 +173,7 @@ config DRM_I915_DEBUG_RUNTIME_PM
bool "Enable extra state checking for runtime PM"
depends on DRM_I915
default n
select STACKDEPOT
help
Choose this option to turn on extra state checking for the
runtime PM functionality. This may introduce overhead during
Expand Down
6 changes: 6 additions & 0 deletions drivers/gpu/drm/i915/i915_debugfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2702,6 +2702,12 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
pci_power_name(pdev->current_state),
pdev->current_state);

if (IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)) {
struct drm_printer p = drm_seq_file_printer(m);

print_intel_runtime_pm_wakeref(dev_priv, &p);
}

return 0;
}

Expand Down
8 changes: 5 additions & 3 deletions drivers/gpu/drm/i915/i915_drv.c
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
mutex_init(&dev_priv->pps_mutex);

i915_memcpy_init_early(dev_priv);
intel_runtime_pm_init_early(dev_priv);

ret = i915_workqueues_init(dev_priv);
if (ret < 0)
Expand Down Expand Up @@ -1807,8 +1808,7 @@ void i915_driver_unload(struct drm_device *dev)
i915_driver_cleanup_mmio(dev_priv);

enable_rpm_wakeref_asserts(dev_priv);

WARN_ON(atomic_read(&dev_priv->runtime_pm.wakeref_count));
intel_runtime_pm_cleanup(dev_priv);
}

static void i915_driver_release(struct drm_device *dev)
Expand Down Expand Up @@ -2010,6 +2010,8 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)

out:
enable_rpm_wakeref_asserts(dev_priv);
if (!dev_priv->uncore.user_forcewake.count)
intel_runtime_pm_cleanup(dev_priv);

return ret;
}
Expand Down Expand Up @@ -2965,7 +2967,7 @@ static int intel_runtime_suspend(struct device *kdev)
}

enable_rpm_wakeref_asserts(dev_priv);
WARN_ON_ONCE(atomic_read(&dev_priv->runtime_pm.wakeref_count));
intel_runtime_pm_cleanup(dev_priv);

if (intel_uncore_arm_unclaimed_mmio_detection(dev_priv))
DRM_ERROR("Unclaimed access detected prior to suspending\n");
Expand Down
20 changes: 20 additions & 0 deletions drivers/gpu/drm/i915/i915_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include <linux/pm_qos.h>
#include <linux/reservation.h>
#include <linux/shmem_fs.h>
#include <linux/stackdepot.h>

#include <drm/intel-gtt.h>
#include <drm/drm_legacy.h> /* for struct drm_dma_handle */
Expand Down Expand Up @@ -1156,6 +1157,25 @@ struct i915_runtime_pm {
atomic_t wakeref_count;
bool suspended;
bool irqs_enabled;

#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
/*
* To aide detection of wakeref leaks and general misuse, we
* track all wakeref holders. With manual markup (i.e. returning
* a cookie to each rpm_get caller which they then supply to their
* paired rpm_put) we can remove corresponding pairs of and keep
* the array trimmed to active wakerefs.
*/
struct intel_runtime_pm_debug {
spinlock_t lock;

depot_stack_handle_t last_acquire;
depot_stack_handle_t last_release;

depot_stack_handle_t *owners;
unsigned long count;
} debug;
#endif
};

enum intel_pipe_crc_source {
Expand Down
44 changes: 29 additions & 15 deletions drivers/gpu/drm/i915/intel_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
#include <drm/drm_atomic.h>
#include <media/cec-notifier.h>

struct drm_printer;

/**
* __wait_for - magic wait macro
*
Expand Down Expand Up @@ -2084,6 +2086,7 @@ bool intel_psr_enabled(struct intel_dp *intel_dp);
void intel_init_quirks(struct drm_i915_private *dev_priv);

/* intel_runtime_pm.c */
void intel_runtime_pm_init_early(struct drm_i915_private *dev_priv);
int intel_power_domains_init(struct drm_i915_private *);
void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
Expand All @@ -2106,6 +2109,7 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
void intel_runtime_pm_disable(struct drm_i915_private *dev_priv);
void intel_runtime_pm_cleanup(struct drm_i915_private *dev_priv);
const char *
intel_display_power_domain_str(enum intel_display_power_domain domain);

Expand All @@ -2123,23 +2127,23 @@ void icl_dbuf_slices_update(struct drm_i915_private *dev_priv,
u8 req_slices);

static inline void
assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
assert_rpm_device_not_suspended(struct drm_i915_private *i915)
{
WARN_ONCE(dev_priv->runtime_pm.suspended,
WARN_ONCE(i915->runtime_pm.suspended,
"Device suspended during HW access\n");
}

static inline void
assert_rpm_wakelock_held(struct drm_i915_private *dev_priv)
assert_rpm_wakelock_held(struct drm_i915_private *i915)
{
assert_rpm_device_not_suspended(dev_priv);
WARN_ONCE(!atomic_read(&dev_priv->runtime_pm.wakeref_count),
assert_rpm_device_not_suspended(i915);
WARN_ONCE(!atomic_read(&i915->runtime_pm.wakeref_count),
"RPM wakelock ref not held during HW access");
}

/**
* disable_rpm_wakeref_asserts - disable the RPM assert checks
* @dev_priv: i915 device instance
* @i915: i915 device instance
*
* This function disable asserts that check if we hold an RPM wakelock
* reference, while keeping the device-not-suspended checks still enabled.
Expand All @@ -2156,14 +2160,14 @@ assert_rpm_wakelock_held(struct drm_i915_private *dev_priv)
* enable_rpm_wakeref_asserts().
*/
static inline void
disable_rpm_wakeref_asserts(struct drm_i915_private *dev_priv)
disable_rpm_wakeref_asserts(struct drm_i915_private *i915)
{
atomic_inc(&dev_priv->runtime_pm.wakeref_count);
atomic_inc(&i915->runtime_pm.wakeref_count);
}

/**
* enable_rpm_wakeref_asserts - re-enable the RPM assert checks
* @dev_priv: i915 device instance
* @i915: i915 device instance
*
* This function re-enables the RPM assert checks after disabling them with
* disable_rpm_wakeref_asserts. It's meant to be used only in special
Expand All @@ -2173,15 +2177,25 @@ disable_rpm_wakeref_asserts(struct drm_i915_private *dev_priv)
* disable_rpm_wakeref_asserts().
*/
static inline void
enable_rpm_wakeref_asserts(struct drm_i915_private *dev_priv)
enable_rpm_wakeref_asserts(struct drm_i915_private *i915)
{
atomic_dec(&dev_priv->runtime_pm.wakeref_count);
atomic_dec(&i915->runtime_pm.wakeref_count);
}

void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv);
void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv);
void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
void intel_runtime_pm_get(struct drm_i915_private *i915);
bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *i915);
void intel_runtime_pm_get_noresume(struct drm_i915_private *i915);
void intel_runtime_pm_put(struct drm_i915_private *i915);

#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
void print_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
struct drm_printer *p);
#else
static inline void print_intel_runtime_pm_wakeref(struct drm_i915_private *i915,
struct drm_printer *p)
{
}
#endif

void chv_phy_powergate_lanes(struct intel_encoder *encoder,
bool override, unsigned int mask);
Expand Down
Loading

0 comments on commit bd780f3

Please sign in to comment.