[PATCH 0/3] Resume time optimisation

[PATCH 0/3] Resume time optimisation

[David Weinehall]

This patch series aims to reduce the time we spend in
i915_gem_restore_gtt_mappings() on resume.

Now that we're getting the resume times down a bit this function
started to stand out.

David Weinehall (3):
  drm/i915: Cleanup i915_gem_restore_gtt_mappings()
  drm/i915: Take runtime pm in i915_gem_resume()
  drm/i915: optimise intel_runtime_pm_{get,put}

 drivers/gpu/drm/i915/i915_gem.c         |  2 ++
 drivers/gpu/drm/i915/i915_gem_gtt.c     |  7 +------
 drivers/gpu/drm/i915/i915_vma.c         | 10 ++++++++++
 drivers/gpu/drm/i915/i915_vma.h         |  6 ++++--
 drivers/gpu/drm/i915/intel_runtime_pm.c | 12 ++++++++++++
 5 files changed, 29 insertions(+), 8 deletions(-)

-- 
2.10.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915: Cleanup i915_gem_restore_gtt_mappings()

[David Weinehall]

On resume we unbind+bind our VMA-mappings. This patch simplifies
this a bit by introducing a rebind() helper.  As a nice side-effect
this also makes the resume callgraph self-documenting.

Patch by Chris.

Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c |  7 +------
 drivers/gpu/drm/i915/i915_vma.c     | 10 ++++++++++
 drivers/gpu/drm/i915/i915_vma.h     |  6 ++++--
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b4bde1452f2a..76af33443fcd 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3299,12 +3299,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 			if (vma->vm != &ggtt->base)
 				continue;
 
-			if (!i915_vma_unbind(vma))
-				continue;
-
-			WARN_ON(i915_vma_bind(vma, obj->cache_level,
-					      PIN_UPDATE));
-			ggtt_bound = true;
+			ggtt_bound |= i915_vma_rebind(vma);
 		}
 
 		if (ggtt_bound)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 738ff3a5cd6e..1fc54686c2c3 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -648,3 +648,13 @@ int i915_vma_unbind(struct i915_vma *vma)
 	return 0;
 }
 
+bool i915_vma_rebind(struct i915_vma *vma)
+{
+	if (i915_vma_is_pinned(vma)) {
+		WARN_ON(i915_vma_bind(vma, vma->obj->cache_level, PIN_UPDATE));
+		return true;
+	} else {
+		WARN_ON(i915_vma_unbind(vma));
+		return false;
+	}
+}
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 2e49f5dd6107..16fa9cdc0dd2 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -200,8 +200,11 @@ i915_vma_compare(struct i915_vma *vma,
 		      sizeof(view->params));
 }
 
-int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
+int i915_vma_bind(struct i915_vma *vma,
+		  enum i915_cache_level cache_level,
 		  u32 flags);
+bool i915_vma_rebind(struct i915_vma *vma);
+
 bool i915_gem_valid_gtt_space(struct i915_vma *vma, unsigned long cache_level);
 bool
 i915_vma_misplaced(struct i915_vma *vma, u64 size, u64 alignment, u64 flags);
@@ -339,4 +342,3 @@ i915_vma_unpin_fence(struct i915_vma *vma)
 }
 
 #endif
-
-- 
2.10.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Take runtime pm in i915_gem_resume()

[David Weinehall]

In i915_gem_resume(), before calling i915_gem_restore_gtt_mappings(),
we want to take the runtime PM reference; this to allow for later
optimisation.

Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7b9f5b99b0f3..35c4ed17b898 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4229,6 +4229,7 @@ void i915_gem_resume(struct drm_device *dev)
 
 	WARN_ON(dev_priv->gt.awake);
 
+	intel_runtime_pm_get(dev_priv);
 	mutex_lock(&dev->struct_mutex);
 	i915_gem_restore_gtt_mappings(dev_priv);
 
@@ -4239,6 +4240,7 @@ void i915_gem_resume(struct drm_device *dev)
 	dev_priv->gt.resume(dev_priv);
 
 	mutex_unlock(&dev->struct_mutex);
+	intel_runtime_pm_put(dev_priv);
 }
 
 void i915_gem_init_swizzling(struct drm_i915_private *dev_priv)
-- 
2.10.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: optimise intel_runtime_pm_{get, put}

[David Weinehall]

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);
 
-- 
2.10.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Resume time optimisation

[Patchwork]

== Series Details ==

Series: Resume time optimisation
URL   : https://patchwork.freedesktop.org/series/15545/
State : failure

== Summary ==

  CC [M]  drivers/gpu/drm/i915/gvt/interrupt.o
  CC [M]  drivers/gpu/drm/i915/gvt/gtt.o
  CC [M]  drivers/gpu/drm/i915/gvt/opregion.o
  CC [M]  drivers/gpu/drm/i915/gvt/cfg_space.o
  LD      lib/raid6/raid6_pq.o
  CC [M]  drivers/gpu/drm/i915/gvt/edid.o
  CC [M]  drivers/gpu/drm/i915/gvt/display.o
  LD      drivers/pci/built-in.o
  CC [M]  drivers/gpu/drm/i915/gvt/mmio.o
  CC [M]  drivers/gpu/drm/i915/gvt/execlist.o
  CC [M]  drivers/gpu/drm/i915/gvt/scheduler.o
  CC [M]  drivers/gpu/drm/i915/gvt/sched_policy.o
  CC [M]  drivers/gpu/drm/i915/gvt/render.o
  CC [M]  drivers/gpu/drm/i915/gvt/cmd_parser.o
  LD      lib/raid6/built-in.o
  LD [M]  drivers/ssb/ssb.o
  LD      drivers/acpi/acpica/acpi.o
  LD [M]  drivers/net/usb/asix.o
  LD      drivers/md/dm-mod.o
  LD [M]  drivers/misc/mei/mei-me.o
  LD      net/ipv6/ipv6.o
  LD      drivers/misc/built-in.o
  LD      drivers/acpi/acpica/built-in.o
  LD      net/xfrm/built-in.o
  LD      net/ipv6/built-in.o
  LD [M]  drivers/net/ethernet/broadcom/genet/genet.o
  LD      drivers/acpi/built-in.o
  LD      drivers/gpu/drm/drm.o
  LD      drivers/usb/storage/usb-storage.o
  LD      drivers/scsi/scsi_mod.o
  LD      drivers/usb/storage/built-in.o
  LD      drivers/tty/serial/8250/8250.o
  LD      drivers/iommu/built-in.o
  LD      drivers/spi/built-in.o
  LD [M]  sound/pci/hda/snd-hda-codec-generic.o
  LD      sound/pci/built-in.o
  LD      drivers/video/fbdev/core/fb.o
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: error: ‘return’ with no value, in function returning non-void [-Werror=return-type]
   return;
   ^
  LD      drivers/video/fbdev/core/built-in.o
  LD      drivers/video/fbdev/built-in.o
  LD      sound/built-in.o
  LD      drivers/video/console/built-in.o
  LD [M]  drivers/usb/serial/usbserial.o
  LD      drivers/video/built-in.o
  LD      drivers/thermal/thermal_sys.o
  LD      drivers/thermal/built-in.o
  LD      drivers/usb/gadget/libcomposite.o
  LD      drivers/usb/gadget/udc/udc-core.o
  LD      drivers/usb/gadget/udc/built-in.o
  LD      drivers/usb/gadget/built-in.o
  LD      drivers/scsi/sd_mod.o
  LD      drivers/scsi/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
  LD      fs/btrfs/btrfs.o
  LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
  LD      net/ipv4/built-in.o
  AR      lib/lib.a
  LD      drivers/tty/serial/8250/8250_base.o
  LD      drivers/tty/serial/8250/built-in.o
  EXPORTS lib/lib-ksyms.o
  LD      drivers/tty/serial/built-in.o
  LD      fs/btrfs/built-in.o
  LD      lib/built-in.o
cc1: all warnings being treated as errors
scripts/Makefile.build:290: recipe for target 'drivers/gpu/drm/i915/intel_runtime_pm.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_runtime_pm.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  LD      drivers/usb/core/usbcore.o
  LD      drivers/usb/core/built-in.o
  CC      arch/x86/kernel/cpu/capflags.o
  LD      arch/x86/kernel/cpu/built-in.o
  LD      arch/x86/kernel/built-in.o
  LD      drivers/tty/vt/built-in.o
  LD      drivers/tty/built-in.o
  LD      arch/x86/built-in.o
  LD      drivers/md/md-mod.o
  LD      drivers/md/built-in.o
  LD      drivers/usb/host/xhci-hcd.o
  LD      net/core/built-in.o
  LD      net/built-in.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD      fs/ext4/ext4.o
  LD      fs/ext4/built-in.o
  LD      fs/built-in.o
  LD      drivers/usb/host/built-in.o
  LD      drivers/usb/built-in.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD      drivers/net/ethernet/built-in.o
  LD      drivers/net/built-in.o
scripts/Makefile.build:475: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:475: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:475: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:985: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Cleanup i915_gem_restore_gtt_mappings()

[Daniel Vetter]

On Fri, Nov 18, 2016 at 12:06:01PM +0200, David Weinehall wrote:
> On resume we unbind+bind our VMA-mappings. This patch simplifies
> this a bit by introducing a rebind() helper.  As a nice side-effect
> this also makes the resume callgraph self-documenting.
> 
> Patch by Chris.
> 
> Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
> CC: Chris Wilson <chris@chris-wilson.co.uk>

Not really sold all that much on this being more readable. Looks correct,
but needs some more acks form Tvrtko or Joonas. With that you can have my
r-b. Patches 2&3 look correct and are

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c |  7 +------
>  drivers/gpu/drm/i915/i915_vma.c     | 10 ++++++++++
>  drivers/gpu/drm/i915/i915_vma.h     |  6 ++++--
>  3 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b4bde1452f2a..76af33443fcd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3299,12 +3299,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
>  			if (vma->vm != &ggtt->base)
>  				continue;
>  
> -			if (!i915_vma_unbind(vma))
> -				continue;
> -
> -			WARN_ON(i915_vma_bind(vma, obj->cache_level,
> -					      PIN_UPDATE));
> -			ggtt_bound = true;
> +			ggtt_bound |= i915_vma_rebind(vma);
>  		}
>  
>  		if (ggtt_bound)
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 738ff3a5cd6e..1fc54686c2c3 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -648,3 +648,13 @@ int i915_vma_unbind(struct i915_vma *vma)
>  	return 0;
>  }
>  
> +bool i915_vma_rebind(struct i915_vma *vma)
> +{
> +	if (i915_vma_is_pinned(vma)) {
> +		WARN_ON(i915_vma_bind(vma, vma->obj->cache_level, PIN_UPDATE));
> +		return true;
> +	} else {
> +		WARN_ON(i915_vma_unbind(vma));
> +		return false;
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 2e49f5dd6107..16fa9cdc0dd2 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -200,8 +200,11 @@ i915_vma_compare(struct i915_vma *vma,
>  		      sizeof(view->params));
>  }
>  
> -int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
> +int i915_vma_bind(struct i915_vma *vma,
> +		  enum i915_cache_level cache_level,
>  		  u32 flags);
> +bool i915_vma_rebind(struct i915_vma *vma);
> +
>  bool i915_gem_valid_gtt_space(struct i915_vma *vma, unsigned long cache_level);
>  bool
>  i915_vma_misplaced(struct i915_vma *vma, u64 size, u64 alignment, u64 flags);
> @@ -339,4 +342,3 @@ i915_vma_unpin_fence(struct i915_vma *vma)
>  }
>  
>  #endif
> -
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: optimise intel_runtime_pm_{get, put}

[Chris Wilson]

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Cleanup i915_gem_restore_gtt_mappings()

[Chris Wilson]

On Fri, Nov 18, 2016 at 12:06:01PM +0200, David Weinehall wrote:
> On resume we unbind+bind our VMA-mappings. This patch simplifies
> this a bit by introducing a rebind() helper.  As a nice side-effect
> this also makes the resume callgraph self-documenting.
> 
> Patch by Chris.
> 
> Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c |  7 +------
>  drivers/gpu/drm/i915/i915_vma.c     | 10 ++++++++++
>  drivers/gpu/drm/i915/i915_vma.h     |  6 ++++--
>  3 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b4bde1452f2a..76af33443fcd 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3299,12 +3299,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
>  			if (vma->vm != &ggtt->base)
>  				continue;
>  
> -			if (!i915_vma_unbind(vma))
> -				continue;
> -
> -			WARN_ON(i915_vma_bind(vma, obj->cache_level,
> -					      PIN_UPDATE));
> -			ggtt_bound = true;
> +			ggtt_bound |= i915_vma_rebind(vma);
>  		}
>  
>  		if (ggtt_bound)
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 738ff3a5cd6e..1fc54686c2c3 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -648,3 +648,13 @@ int i915_vma_unbind(struct i915_vma *vma)
>  	return 0;
>  }
>  
> +bool i915_vma_rebind(struct i915_vma *vma)
> +{
> +	if (i915_vma_is_pinned(vma)) {
> +		WARN_ON(i915_vma_bind(vma, vma->obj->cache_level, PIN_UPDATE));
> +		return true;
> +	} else {
> +		WARN_ON(i915_vma_unbind(vma));
> +		return false;
> +	}
> +}

This function is a little warny for general consumption.

I think I would prefer this next to i915_gem_restore_gtt_mappings() as
perhaps static bool restore_vma(struct i915_vma *vma) {}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Take runtime pm in i915_gem_resume()

[Chris Wilson]

On Fri, Nov 18, 2016 at 12:06:02PM +0200, David Weinehall wrote:
> In i915_gem_resume(), before calling i915_gem_restore_gtt_mappings(),
> we want to take the runtime PM reference; this to allow for later
> optimisation.

...to hold the rpm wakelocks for the entire duration instead of many short
lived wakelocks around each access to the GGTT. We will take it in
i915_gem_resume() as any other GEM operation is likely to require the gT
powerwell as well.

> Signed-off-by: David Weinehall <david.weinehall@linux.intel.com>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: optimise intel_runtime_pm_{get, put}

[David Weinehall]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: optimise intel_runtime_pm_{get, put}

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2368 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30141 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx