| Message ID | 20161118100603.14284-4-david.weinehall@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 18, 2016 at 12:06:03PM +0200, David Weinehall wrote: > Benchmarking shows that on resume we spend quite a bit of time > just taking and dropping these references, leaving us two options; > either rewriting the code not to take these references more than > once, which would be a rather invasive change since the involved > functions are used from other places, or to optimise > intel_runtime_pm_{get,put}(). This patch does the latter. > Initial benchmarking indicate improvements of a couple > of milliseconds on resume. > > Original patch by Chris, with slight fixes by me. > > Signed-off-by: David Weinehall <david.weinehall@linux.intel.com> > CC: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 356c662ad453..4bf279023b39 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -2632,6 +2632,9 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv) > struct pci_dev *pdev = dev_priv->drm.pdev; > struct device *kdev = &pdev->dev; > > + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) > + return; > + > pm_runtime_get_sync(kdev); > > atomic_inc(&dev_priv->pm.wakeref_count); > @@ -2653,6 +2656,9 @@ bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv) > struct pci_dev *pdev = dev_priv->drm.pdev; > struct device *kdev = &pdev->dev; > > + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) > + return; > + > if (IS_ENABLED(CONFIG_PM)) { > int ret = pm_runtime_get_if_in_use(kdev); > > @@ -2695,6 +2701,9 @@ void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv) > struct pci_dev *pdev = dev_priv->drm.pdev; > struct device *kdev = &pdev->dev; > > + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) > + return; > + > assert_rpm_wakelock_held(dev_priv); > pm_runtime_get_noresume(kdev); > > @@ -2714,6 +2723,9 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv) > struct pci_dev *pdev = dev_priv->drm.pdev; > struct device *kdev = &pdev->dev; > > + if (!atomic_dec_and_test(&dev_priv->pm.wakeref_count)) > + return; > + > assert_rpm_wakelock_held(dev_priv); > atomic_dec(&dev_priv->pm.wakeref_count); Have to remove this dec. Time to retest ;-) -Chris
On Fri, Nov 18, 2016 at 10:37:30AM +0000, Chris Wilson wrote: > On Fri, Nov 18, 2016 at 12:06:03PM +0200, David Weinehall wrote: > > Benchmarking shows that on resume we spend quite a bit of time > > just taking and dropping these references, leaving us two options; > > either rewriting the code not to take these references more than > > once, which would be a rather invasive change since the involved > > functions are used from other places, or to optimise > > intel_runtime_pm_{get,put}(). This patch does the latter. > > Initial benchmarking indicate improvements of a couple > > of milliseconds on resume. > > > > Original patch by Chris, with slight fixes by me. > > > > Signed-off-by: David Weinehall <david.weinehall@linux.intel.com> > > CC: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 356c662ad453..4bf279023b39 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -2632,6 +2632,9 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv) > > struct pci_dev *pdev = dev_priv->drm.pdev; > > struct device *kdev = &pdev->dev; > > > > + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) > > + return; > > + > > pm_runtime_get_sync(kdev); > > > > atomic_inc(&dev_priv->pm.wakeref_count); > > @@ -2653,6 +2656,9 @@ bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv) > > struct pci_dev *pdev = dev_priv->drm.pdev; > > struct device *kdev = &pdev->dev; > > > > + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) > > + return; > > + > > if (IS_ENABLED(CONFIG_PM)) { > > int ret = pm_runtime_get_if_in_use(kdev); > > > > @@ -2695,6 +2701,9 @@ void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv) > > struct pci_dev *pdev = dev_priv->drm.pdev; > > struct device *kdev = &pdev->dev; > > > > + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) > > + return; > > + > > assert_rpm_wakelock_held(dev_priv); > > pm_runtime_get_noresume(kdev); > > > > @@ -2714,6 +2723,9 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv) > > struct pci_dev *pdev = dev_priv->drm.pdev; > > struct device *kdev = &pdev->dev; > > > > + if (!atomic_dec_and_test(&dev_priv->pm.wakeref_count)) > > + return; > > + > > assert_rpm_wakelock_held(dev_priv); > > atomic_dec(&dev_priv->pm.wakeref_count); > > Have to remove this dec. Time to retest ;-) Will retest with that (and fix the return-type error). Kind regards, David
Hi David,
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20161117]
[cannot apply to v4.9-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/David-Weinehall/Resume-time-optimisation/20161118-181301
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x009-201646 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/intel_runtime_pm.c: In function 'intel_runtime_pm_get_if_in_use':
>> drivers/gpu/drm/i915/intel_runtime_pm.c:2660:3: warning: 'return' with no value, in function returning non-void [-Wreturn-type]
return;
^~~~~~
drivers/gpu/drm/i915/intel_runtime_pm.c:2654:6: note: declared here
bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/return +2660 drivers/gpu/drm/i915/intel_runtime_pm.c
2644 /**
2645 * intel_runtime_pm_get_if_in_use - grab a runtime pm reference if device in use
2646 * @dev_priv: i915 device instance
2647 *
2648 * This function grabs a device-level runtime pm reference if the device is
2649 * already in use and ensures that it is powered up.
2650 *
2651 * Any runtime pm reference obtained by this function must have a symmetric
2652 * call to intel_runtime_pm_put() to release the reference again.
2653 */
2654 bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv)
2655 {
2656 struct pci_dev *pdev = dev_priv->drm.pdev;
2657 struct device *kdev = &pdev->dev;
2658
2659 if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count))
> 2660 return;
2661
2662 if (IS_ENABLED(CONFIG_PM)) {
2663 int ret = pm_runtime_get_if_in_use(kdev);
2664
2665 /*
2666 * In cases runtime PM is disabled by the RPM core and we get
2667 * an -EINVAL return value we are not supposed to call this
2668 * function, since the power state is undefined. This applies
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 356c662ad453..4bf279023b39 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -2632,6 +2632,9 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv) struct pci_dev *pdev = dev_priv->drm.pdev; struct device *kdev = &pdev->dev; + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) + return; + pm_runtime_get_sync(kdev); atomic_inc(&dev_priv->pm.wakeref_count); @@ -2653,6 +2656,9 @@ bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv) struct pci_dev *pdev = dev_priv->drm.pdev; struct device *kdev = &pdev->dev; + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) + return; + if (IS_ENABLED(CONFIG_PM)) { int ret = pm_runtime_get_if_in_use(kdev); @@ -2695,6 +2701,9 @@ void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv) struct pci_dev *pdev = dev_priv->drm.pdev; struct device *kdev = &pdev->dev; + if (atomic_inc_not_zero(&dev_priv->pm.wakeref_count)) + return; + assert_rpm_wakelock_held(dev_priv); pm_runtime_get_noresume(kdev); @@ -2714,6 +2723,9 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv) struct pci_dev *pdev = dev_priv->drm.pdev; struct device *kdev = &pdev->dev; + if (!atomic_dec_and_test(&dev_priv->pm.wakeref_count)) + return; + assert_rpm_wakelock_held(dev_priv); atomic_dec(&dev_priv->pm.wakeref_count);
Benchmarking shows that on resume we spend quite a bit of time just taking and dropping these references, leaving us two options; either rewriting the code not to take these references more than once, which would be a rather invasive change since the involved functions are used from other places, or to optimise intel_runtime_pm_{get,put}(). This patch does the latter. Initial benchmarking indicate improvements of a couple of milliseconds on resume. Original patch by Chris, with slight fixes by me. Signed-off-by: David Weinehall <david.weinehall@linux.intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)