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(+)