Re: [Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-25 Thread Chris Wilson 
Quoting Ville Syrjälä (2018-05-25 11:51:13)
> On Fri, May 25, 2018 at 10:26:29AM +0100, Chris Wilson wrote:
> > In order to prepare the GPU for sleeping, we may want to submit commands
> > to it. This is a complicated process that may even require some swapping
> > in from shmemfs, if the GPU was in the wrong state. As such, we need to
> > do this preparation step synchronously before the rest of the system has
> > started to turn off (e.g. swapin fails if scsi is suspended).
> > Fortunately, we are provided with a such a hook, pm_ops.prepare().
> > 
> > v2: Compile cleanup
> > v3: Fewer asserts, fewer problems?
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106640
> > Testcase: igt/drv_suspend after igt/gem_tiled_swapping
> > Signed-off-by: Chris Wilson 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 41 +
> >  1 file changed, 31 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 9c449b8d8eab..9d6ac7f44812 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1553,12 +1553,24 @@ static bool suspend_to_idle(struct drm_i915_private 
> > *dev_priv)
> >   return false;
> >  }
> >  
> > +static int i915_drm_prepare(struct drm_device *dev)
> > +{
> > + struct drm_i915_private *i915 = to_i915(dev);
> > + int err;
> > +
> > + err = i915_gem_suspend(i915);
> > + if (err)
> > + dev_err(>drm.pdev->dev,
> > + "GEM idle failed, suspend/resume might fail\n");
> > +
> > + return err;
> > +}
> > +
> >  static int i915_drm_suspend(struct drm_device *dev)
> >  {
> >   struct drm_i915_private *dev_priv = to_i915(dev);
> >   struct pci_dev *pdev = dev_priv->drm.pdev;
> >   pci_power_t opregion_target_state;
> > - int error;
> >  
> >   /* ignore lid events during suspend */
> >   mutex_lock(_priv->modeset_restore_lock);
> > @@ -1575,13 +1587,6 @@ static int i915_drm_suspend(struct drm_device *dev)
> >  
> >   pci_save_state(pdev);
> >  
> > - error = i915_gem_suspend(dev_priv);
> > - if (error) {
> > - dev_err(>dev,
> > - "GEM idle failed, resume might fail\n");
> > - goto out;
> > - }
> > -
> >   intel_display_suspend(dev);
> >  
> >   intel_dp_mst_suspend(dev);
> > @@ -1609,10 +1614,9 @@ static int i915_drm_suspend(struct drm_device *dev)
> >  
> >   intel_csr_ucode_suspend(dev_priv);
> >  
> > -out:
> >   enable_rpm_wakeref_asserts(dev_priv);
> >  
> > - return error;
> > + return 0;
> >  }
> >  
> >  static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
> > @@ -2081,6 +2085,22 @@ int i915_reset_engine(struct intel_engine_cs 
> > *engine, const char *msg)
> >   return ret;
> >  }
> >  
> > +static int i915_pm_prepare(struct device *kdev)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(kdev);
> > + struct drm_device *dev = pci_get_drvdata(pdev);
> > +
> > + if (!dev) {
> > + dev_err(kdev, "DRM not initialized, aborting suspend.\n");
> > + return -ENODEV;
> > + }
> 
> How can this happen?
> 
> IIRC I actually wrote a patch once to move the gem suspend to happen
> after display suspend. The idea being that shutting down the display(s)
> may require gem services (MI_OVERLAY_OFF being the prime example I
> had in mind at the time). Just wondering if we can split the gem suspend
> somehow to allow that, or would we need to just move display suspend
> earlier as well?

Ville accepted that this didn't really change the status quo (on irc)
and so was ok with postponing such fixes until later. I added
+   /*
+* NB intel_display_suspend() may issue new requests after we've
+* ostensibly marked the GPU as ready-to-sleep here. We need to
+* split out that work and pull it forward so that after point,
+* the GPU is not woken again.
+*/
to record the issue so that hopefully we might fix it before any one
notices.

I pulled in Mika's review from a later thread and pushed so I can close
the bug.

Thanks for the review,
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-25 Thread Chris Wilson 
Quoting Ville Syrjälä (2018-05-25 11:51:13)
> On Fri, May 25, 2018 at 10:26:29AM +0100, Chris Wilson wrote:
> > In order to prepare the GPU for sleeping, we may want to submit commands
> > to it. This is a complicated process that may even require some swapping
> > in from shmemfs, if the GPU was in the wrong state. As such, we need to
> > do this preparation step synchronously before the rest of the system has
> > started to turn off (e.g. swapin fails if scsi is suspended).
> > Fortunately, we are provided with a such a hook, pm_ops.prepare().
> > 
> > v2: Compile cleanup
> > v3: Fewer asserts, fewer problems?
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106640
> > Testcase: igt/drv_suspend after igt/gem_tiled_swapping
> > Signed-off-by: Chris Wilson 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 41 +
> >  1 file changed, 31 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 9c449b8d8eab..9d6ac7f44812 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1553,12 +1553,24 @@ static bool suspend_to_idle(struct drm_i915_private 
> > *dev_priv)
> >   return false;
> >  }
> >  
> > +static int i915_drm_prepare(struct drm_device *dev)
> > +{
> > + struct drm_i915_private *i915 = to_i915(dev);
> > + int err;
> > +
> > + err = i915_gem_suspend(i915);
> > + if (err)
> > + dev_err(>drm.pdev->dev,
> > + "GEM idle failed, suspend/resume might fail\n");
> > +
> > + return err;
> > +}
> > +
> >  static int i915_drm_suspend(struct drm_device *dev)
> >  {
> >   struct drm_i915_private *dev_priv = to_i915(dev);
> >   struct pci_dev *pdev = dev_priv->drm.pdev;
> >   pci_power_t opregion_target_state;
> > - int error;
> >  
> >   /* ignore lid events during suspend */
> >   mutex_lock(_priv->modeset_restore_lock);
> > @@ -1575,13 +1587,6 @@ static int i915_drm_suspend(struct drm_device *dev)
> >  
> >   pci_save_state(pdev);
> >  
> > - error = i915_gem_suspend(dev_priv);
> > - if (error) {
> > - dev_err(>dev,
> > - "GEM idle failed, resume might fail\n");
> > - goto out;
> > - }
> > -
> >   intel_display_suspend(dev);
> >  
> >   intel_dp_mst_suspend(dev);
> > @@ -1609,10 +1614,9 @@ static int i915_drm_suspend(struct drm_device *dev)
> >  
> >   intel_csr_ucode_suspend(dev_priv);
> >  
> > -out:
> >   enable_rpm_wakeref_asserts(dev_priv);
> >  
> > - return error;
> > + return 0;
> >  }
> >  
> >  static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
> > @@ -2081,6 +2085,22 @@ int i915_reset_engine(struct intel_engine_cs 
> > *engine, const char *msg)
> >   return ret;
> >  }
> >  
> > +static int i915_pm_prepare(struct device *kdev)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(kdev);
> > + struct drm_device *dev = pci_get_drvdata(pdev);
> > +
> > + if (!dev) {
> > + dev_err(kdev, "DRM not initialized, aborting suspend.\n");
> > + return -ENODEV;
> > + }
> 
> How can this happen?

Just copypaste, and not wanting to have to answer too many questions.
 
> IIRC I actually wrote a patch once to move the gem suspend to happen
> after display suspend. The idea being that shutting down the display(s)
> may require gem services (MI_OVERLAY_OFF being the prime example I
> had in mind at the time). Just wondering if we can split the gem suspend
> somehow to allow that, or would we need to just move display suspend
> earlier as well?

That would require doing that portion of display shutdown earlier. The
problem is in starting GPU activity from within the async suspend is
error prone. For overlay, it's not much of an issue, we could just
submit the command and wait on it synchronously, as those systems don't
have the fiddly things like contexts and rc6 to contend with ;)

I can't think of a different way of slicing gem suspend, as it's core
operation is to switch the GPU to the kernel context, and that involves
submitting a GPU command. After gem suspend, we shouldn't tolerate any
more requests.

I am splitting the reset at the end of gem suspend into suspend_late
though (mainly so that we don't do it for SUSPEND_TEST_DEVICES as it
conflicts nastily with reset failure testing).
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-25 Thread Ville Syrjälä 
On Fri, May 25, 2018 at 10:26:29AM +0100, Chris Wilson wrote:
> In order to prepare the GPU for sleeping, we may want to submit commands
> to it. This is a complicated process that may even require some swapping
> in from shmemfs, if the GPU was in the wrong state. As such, we need to
> do this preparation step synchronously before the rest of the system has
> started to turn off (e.g. swapin fails if scsi is suspended).
> Fortunately, we are provided with a such a hook, pm_ops.prepare().
> 
> v2: Compile cleanup
> v3: Fewer asserts, fewer problems?
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106640
> Testcase: igt/drv_suspend after igt/gem_tiled_swapping
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 41 +
>  1 file changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9c449b8d8eab..9d6ac7f44812 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1553,12 +1553,24 @@ static bool suspend_to_idle(struct drm_i915_private 
> *dev_priv)
>   return false;
>  }
>  
> +static int i915_drm_prepare(struct drm_device *dev)
> +{
> + struct drm_i915_private *i915 = to_i915(dev);
> + int err;
> +
> + err = i915_gem_suspend(i915);
> + if (err)
> + dev_err(>drm.pdev->dev,
> + "GEM idle failed, suspend/resume might fail\n");
> +
> + return err;
> +}
> +
>  static int i915_drm_suspend(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = to_i915(dev);
>   struct pci_dev *pdev = dev_priv->drm.pdev;
>   pci_power_t opregion_target_state;
> - int error;
>  
>   /* ignore lid events during suspend */
>   mutex_lock(_priv->modeset_restore_lock);
> @@ -1575,13 +1587,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>  
>   pci_save_state(pdev);
>  
> - error = i915_gem_suspend(dev_priv);
> - if (error) {
> - dev_err(>dev,
> - "GEM idle failed, resume might fail\n");
> - goto out;
> - }
> -
>   intel_display_suspend(dev);
>  
>   intel_dp_mst_suspend(dev);
> @@ -1609,10 +1614,9 @@ static int i915_drm_suspend(struct drm_device *dev)
>  
>   intel_csr_ucode_suspend(dev_priv);
>  
> -out:
>   enable_rpm_wakeref_asserts(dev_priv);
>  
> - return error;
> + return 0;
>  }
>  
>  static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
> @@ -2081,6 +2085,22 @@ int i915_reset_engine(struct intel_engine_cs *engine, 
> const char *msg)
>   return ret;
>  }
>  
> +static int i915_pm_prepare(struct device *kdev)
> +{
> + struct pci_dev *pdev = to_pci_dev(kdev);
> + struct drm_device *dev = pci_get_drvdata(pdev);
> +
> + if (!dev) {
> + dev_err(kdev, "DRM not initialized, aborting suspend.\n");
> + return -ENODEV;
> + }

How can this happen?

IIRC I actually wrote a patch once to move the gem suspend to happen
after display suspend. The idea being that shutting down the display(s)
may require gem services (MI_OVERLAY_OFF being the prime example I
had in mind at the time). Just wondering if we can split the gem suspend
somehow to allow that, or would we need to just move display suspend
earlier as well?

> +
> + if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> + return 0;
> +
> + return i915_drm_prepare(dev);
> +}
> +
>  static int i915_pm_suspend(struct device *kdev)
>  {
>   struct pci_dev *pdev = to_pci_dev(kdev);
> @@ -2731,6 +2751,7 @@ const struct dev_pm_ops i915_pm_ops = {
>* S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
>* PMSG_RESUME]
>*/
> + .prepare = i915_pm_prepare,
>   .suspend = i915_pm_suspend,
>   .suspend_late = i915_pm_suspend_late,
>   .resume_early = i915_pm_resume_early,
> -- 
> 2.17.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-25 Thread Chris Wilson 
In order to prepare the GPU for sleeping, we may want to submit commands
to it. This is a complicated process that may even require some swapping
in from shmemfs, if the GPU was in the wrong state. As such, we need to
do this preparation step synchronously before the rest of the system has
started to turn off (e.g. swapin fails if scsi is suspended).
Fortunately, we are provided with a such a hook, pm_ops.prepare().

v2: Compile cleanup
v3: Fewer asserts, fewer problems?

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106640
Testcase: igt/drv_suspend after igt/gem_tiled_swapping
Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.c | 41 +
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9c449b8d8eab..9d6ac7f44812 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1553,12 +1553,24 @@ static bool suspend_to_idle(struct drm_i915_private 
*dev_priv)
return false;
 }
 
+static int i915_drm_prepare(struct drm_device *dev)
+{
+   struct drm_i915_private *i915 = to_i915(dev);
+   int err;
+
+   err = i915_gem_suspend(i915);
+   if (err)
+   dev_err(>drm.pdev->dev,
+   "GEM idle failed, suspend/resume might fail\n");
+
+   return err;
+}
+
 static int i915_drm_suspend(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = to_i915(dev);
struct pci_dev *pdev = dev_priv->drm.pdev;
pci_power_t opregion_target_state;
-   int error;
 
/* ignore lid events during suspend */
mutex_lock(_priv->modeset_restore_lock);
@@ -1575,13 +1587,6 @@ static int i915_drm_suspend(struct drm_device *dev)
 
pci_save_state(pdev);
 
-   error = i915_gem_suspend(dev_priv);
-   if (error) {
-   dev_err(>dev,
-   "GEM idle failed, resume might fail\n");
-   goto out;
-   }
-
intel_display_suspend(dev);
 
intel_dp_mst_suspend(dev);
@@ -1609,10 +1614,9 @@ static int i915_drm_suspend(struct drm_device *dev)
 
intel_csr_ucode_suspend(dev_priv);
 
-out:
enable_rpm_wakeref_asserts(dev_priv);
 
-   return error;
+   return 0;
 }
 
 static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
@@ -2081,6 +2085,22 @@ int i915_reset_engine(struct intel_engine_cs *engine, 
const char *msg)
return ret;
 }
 
+static int i915_pm_prepare(struct device *kdev)
+{
+   struct pci_dev *pdev = to_pci_dev(kdev);
+   struct drm_device *dev = pci_get_drvdata(pdev);
+
+   if (!dev) {
+   dev_err(kdev, "DRM not initialized, aborting suspend.\n");
+   return -ENODEV;
+   }
+
+   if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+   return 0;
+
+   return i915_drm_prepare(dev);
+}
+
 static int i915_pm_suspend(struct device *kdev)
 {
struct pci_dev *pdev = to_pci_dev(kdev);
@@ -2731,6 +2751,7 @@ const struct dev_pm_ops i915_pm_ops = {
 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
 * PMSG_RESUME]
 */
+   .prepare = i915_pm_prepare,
.suspend = i915_pm_suspend,
.suspend_late = i915_pm_suspend_late,
.resume_early = i915_pm_resume_early,
-- 
2.17.0

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

[Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-25 Thread Chris Wilson 
In order to prepare the GPU for sleeping, we may want to submit commands
to it. This is a complicated process that may even require some swapping
in from shmemfs, if the GPU was in the wrong state. As such, we need to
do this preparation step synchronously before the rest of the system has
started to turn off (e.g. swapin fails if scsi is suspended).
Fortunately, we are provided with a such a hook, pm_ops.prepare().

v2: Compile cleanup

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106640
Testcase: igt/drv_suspend after igt/gem_tiled_swapping
Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.c | 47 ++---
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9c449b8d8eab..a0beabd49f62 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1553,12 +1553,30 @@ static bool suspend_to_idle(struct drm_i915_private 
*dev_priv)
return false;
 }
 
+static int i915_drm_prepare(struct drm_device *dev)
+{
+   struct drm_i915_private *i915 = to_i915(dev);
+   int err;
+
+   disable_rpm_wakeref_asserts(i915);
+
+   err = i915_gem_suspend(i915);
+   if (err) {
+   dev_err(>drm.pdev->dev,
+   "GEM idle failed, suspend/resume might fail\n");
+   goto out;
+   }
+out:
+   enable_rpm_wakeref_asserts(i915);
+
+   return err;
+}
+
 static int i915_drm_suspend(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = to_i915(dev);
struct pci_dev *pdev = dev_priv->drm.pdev;
pci_power_t opregion_target_state;
-   int error;
 
/* ignore lid events during suspend */
mutex_lock(_priv->modeset_restore_lock);
@@ -1575,13 +1593,6 @@ static int i915_drm_suspend(struct drm_device *dev)
 
pci_save_state(pdev);
 
-   error = i915_gem_suspend(dev_priv);
-   if (error) {
-   dev_err(>dev,
-   "GEM idle failed, resume might fail\n");
-   goto out;
-   }
-
intel_display_suspend(dev);
 
intel_dp_mst_suspend(dev);
@@ -1609,10 +1620,9 @@ static int i915_drm_suspend(struct drm_device *dev)
 
intel_csr_ucode_suspend(dev_priv);
 
-out:
enable_rpm_wakeref_asserts(dev_priv);
 
-   return error;
+   return 0;
 }
 
 static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
@@ -2081,6 +2091,22 @@ int i915_reset_engine(struct intel_engine_cs *engine, 
const char *msg)
return ret;
 }
 
+static int i915_pm_prepare(struct device *kdev)
+{
+   struct pci_dev *pdev = to_pci_dev(kdev);
+   struct drm_device *dev = pci_get_drvdata(pdev);
+
+   if (!dev) {
+   dev_err(kdev, "DRM not initialized, aborting suspend.\n");
+   return -ENODEV;
+   }
+
+   if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+   return 0;
+
+   return i915_drm_prepare(dev);
+}
+
 static int i915_pm_suspend(struct device *kdev)
 {
struct pci_dev *pdev = to_pci_dev(kdev);
@@ -2731,6 +2757,7 @@ const struct dev_pm_ops i915_pm_ops = {
 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
 * PMSG_RESUME]
 */
+   .prepare = i915_pm_prepare,
.suspend = i915_pm_suspend,
.suspend_late = i915_pm_suspend_late,
.resume_early = i915_pm_resume_early,
-- 
2.17.0

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

Re: [Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-24 Thread kbuild test robot 
Hi Chris,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.17-rc6 next-20180517]
[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/Chris-Wilson/drm-i915-Prepare-GEM-for-suspend-earlier/20180524-231951
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x000-201820 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/i915/i915_drv.c: In function 'i915_drm_suspend':
>> drivers/gpu//drm/i915/i915_drv.c:1624:1: error: label 'out' defined but not 
>> used [-Werror=unused-label]
out:
^~~
   Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size
   Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read
   Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write
   Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:clear_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
   Cyclomatic Complexity 1 arch/x86/include/asm/arch_hweight.h:__arch_hweight32
   Cyclomatic Complexity 1 arch/x86/include/asm/arch_hweight.h:__arch_hweight8
   Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32
   Cyclomatic Complexity 1 include/linux/list.h:list_empty
   Cyclomatic Complexity 1 arch/x86/include/asm/mem_encrypt.h:sme_active
   Cyclomatic Complexity 1 include/linux/mem_encrypt.h:sme_get_me_mask
   Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_read
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_inc
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_dec
   Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_read
   Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_inc
   Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_dec
   Cyclomatic Complexity 1 include/linux/err.h:ERR_PTR
   Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
   Cyclomatic Complexity 3 include/linux/err.h:IS_ERR_OR_NULL
   Cyclomatic Complexity 1 include/linux/lockdep.h:lock_is_held
   Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_lock_irq
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock_irq
   Cyclomatic Complexity 1 include/linux/jiffies.h:_msecs_to_jiffies
   Cyclomatic Complexity 3 include/linux/jiffies.h:msecs_to_jiffies
   Cyclomatic Complexity 3 include/linux/ktime.h:ktime_compare
   Cyclomatic Complexity 1 include/linux/ktime.h:ktime_after
   Cyclomatic Complexity 1 include/linux/workqueue.h:queue_delayed_work
   Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large
   Cyclomatic Complexity 3 include/linux/slab.h:kmalloc
   Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
   Cyclomatic Complexity 1 include/linux/device.h:dev_pm_set_driver_flags
   Cyclomatic Complexity 1 include/linux/pci.h:pci_disable_msi
   Cyclomatic Complexity 1 include/linux/pci.h:pci_enable_msi
   Cyclomatic Complexity 1 arch/x86/include/asm/pci.h:pci_domain_nr
   Cyclomatic Complexity 1 include/linux/pci.h:pci_get_drvdata
   Cyclomatic Complexity 1 include/linux/pci.h:pci_set_drvdata
   Cyclomatic Complexity 1 arch/x86/include/asm/dma-mapping.h:get_arch_dma_ops
   Cyclomatic Complexity 4 include/linux/dma-mapping.h:get_dma_ops
   Cyclomatic Complexity 3 include/linux/dma-mapping.h:dma_check_mask
   Cyclomatic Complexity 4 include/linux/dma-mapping.h:dma_supported
   Cyclomatic Complexity 2 include/linux/dma-mapping.h:dma_set_coherent_mask
   Cyclomatic Complexity 1 include/linux/vgaarb.h:vga_client_register
   Cyclomatic Complexity 2 include/linux/fb.h:alloc_apertures
   Cyclomatic Complexity 1 
include/linux/vga_switcheroo.h:vga_switcheroo_unregister_client
   Cyclomatic Complexity 1 
include/linux/vga_switcheroo.h:vga_switcheroo_register_client
   Cyclomatic Complexity 1 
include/linux/vga_switcheroo.h:vga_switcheroo_process_delayed_switch
   Cyclomatic Complexity 1 include/drm/drm_print.h:drm_debug_printer
   Cyclomatic Complexity 2 drivers/gpu//drm/i915/i915_utils.h:onoff
   Cyclomatic Complexity 3 
drivers/gpu//drm/i915/intel_device_info.h:sseu_subslice_total
   Cyclomatic Complexity 1 
drivers/gpu//drm/i915/intel_ringbuffer.h:intel_engine_flag
   Cyclomatic Complexity 1 
drivers/gpu//drm/i915/intel_uncore.h:intel_wait_for_register
   Cyclomatic Complexity 1 
drivers/gpu//drm/i915/i915_gpu_error.h:i915_reset_error_state
   Cyclomatic

Re: [Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-24 Thread kbuild test robot 
Hi Chris,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.17-rc6 next-20180517]
[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/Chris-Wilson/drm-i915-Prepare-GEM-for-suspend-earlier/20180524-231951
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-x015-201820 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/gpu//drm/i915/i915_drv.c: In function 'i915_drm_suspend':
   drivers/gpu//drm/i915/i915_drv.c:1624:1: warning: label 'out' defined but 
not used [-Wunused-label]
out:
^~~
>> drivers/gpu//drm/i915/i915_drv.c:1627:9: warning: 'error' is used 
>> uninitialized in this function [-Wuninitialized]
 return error;
^

vim +/error +1627 drivers/gpu//drm/i915/i915_drv.c

156987f3e drivers/gpu/drm/i915/i915_drv.c Chris Wilson  2018-05-22  1574  
5e365c391 drivers/gpu/drm/i915/i915_drv.c Imre Deak 2014-10-23  1575  
static int i915_drm_suspend(struct drm_device *dev)
ba8bbcf6f drivers/char/drm/i915_drv.c Jesse Barnes  2007-11-22  1576  {
fac5e23e3 drivers/gpu/drm/i915/i915_drv.c Chris Wilson  2016-07-04  1577
struct drm_i915_private *dev_priv = to_i915(dev);
52a05c302 drivers/gpu/drm/i915/i915_drv.c David Weinehall   2016-08-22  1578
struct pci_dev *pdev = dev_priv->drm.pdev;
e5747e3ad drivers/gpu/drm/i915/i915_drv.c Jesse Barnes  2014-06-12  1579
pci_power_t opregion_target_state;
d58189385 drivers/gpu/drm/i915/i915_drv.c Daniel Vetter 2015-02-23  1580
int error;
61caf87cb drivers/gpu/drm/i915/i915_drv.c Rafael J. Wysocki 2010-02-18  1581  
b8efb17b3 drivers/gpu/drm/i915/i915_drv.c Zhang Rui 2013-02-05  1582
/* ignore lid events during suspend */
b8efb17b3 drivers/gpu/drm/i915/i915_drv.c Zhang Rui 2013-02-05  1583
mutex_lock(_priv->modeset_restore_lock);
b8efb17b3 drivers/gpu/drm/i915/i915_drv.c Zhang Rui 2013-02-05  1584
dev_priv->modeset_restore = MODESET_SUSPENDED;
b8efb17b3 drivers/gpu/drm/i915/i915_drv.c Zhang Rui 2013-02-05  1585
mutex_unlock(_priv->modeset_restore_lock);
b8efb17b3 drivers/gpu/drm/i915/i915_drv.c Zhang Rui 2013-02-05  1586  
1f814daca drivers/gpu/drm/i915/i915_drv.c Imre Deak 2015-12-16  1587
disable_rpm_wakeref_asserts(dev_priv);
1f814daca drivers/gpu/drm/i915/i915_drv.c Imre Deak 2015-12-16  1588  
c67a470b1 drivers/gpu/drm/i915/i915_drv.c Paulo Zanoni  2013-08-19  1589
/* We do a lot of poking in a lot of registers, make sure they work
c67a470b1 drivers/gpu/drm/i915/i915_drv.c Paulo Zanoni  2013-08-19  1590
 * properly. */
da7e29bd5 drivers/gpu/drm/i915/i915_drv.c Imre Deak 2014-02-18  1591
intel_display_set_init_power(dev_priv, true);
cb10799c1 drivers/gpu/drm/i915/i915_drv.c Paulo Zanoni  2013-01-25  1592  
5bcf719b7 drivers/gpu/drm/i915/i915_drv.c Dave Airlie   2010-12-07  1593
drm_kms_helper_poll_disable(dev);
5bcf719b7 drivers/gpu/drm/i915/i915_drv.c Dave Airlie   2010-12-07  1594  
52a05c302 drivers/gpu/drm/i915/i915_drv.c David Weinehall   2016-08-22  1595
pci_save_state(pdev);
ba8bbcf6f drivers/char/drm/i915_drv.c Jesse Barnes  2007-11-22  1596  
6b72d4862 drivers/gpu/drm/i915/i915_drv.c Maarten Lankhorst 2015-06-01  1597
intel_display_suspend(dev);
7d708ee40 drivers/gpu/drm/i915/i915_drv.c Imre Deak 2013-04-17  1598  
0e32b39ce drivers/gpu/drm/i915/i915_drv.c Dave Airlie   2014-05-02  1599
intel_dp_mst_suspend(dev);
09b64267c drivers/gpu/drm/i915/i915_drv.c Dave Airlie   2014-07-23  1600  
b963291cf drivers/gpu/drm/i915/i915_drv.c Daniel Vetter 2014-09-30  1601
intel_runtime_pm_disable_interrupts(dev_priv);
1d0d343ab drivers/gpu/drm/i915/i915_drv.c Imre Deak 2014-08-18  1602
intel_hpd_cancel_work(dev_priv);
0e32b39ce drivers/gpu/drm/i915/i915_drv.c Dave Airlie   2014-05-02  1603  
07f9cd0b3 drivers/gpu/drm/i915/i915_drv.c Imre Deak 2014-08-18  1604
intel_suspend_encoders(dev_priv);
07f9cd0b3 drivers/gpu/drm/i915/i915_drv.c Imre Deak 2014-08-18  1605  
712bf3644 drivers/gpu/drm/i915/i915_drv.c Ville Syrjälä 2016-10-31  1606
intel_suspend_hw(dev_priv);
5669fcacc drivers/gpu/drm/i915/i915_drv.c Jesse Barnes  2009-02-17  1607  
275a991c0 drivers/gpu/drm/i915/i915_drv.c Tvrtko Ursulin2016-11-16  1608
i915_gem_suspend_gtt_mappings(dev_priv);
828c79087 drivers/gpu/drm/i915/i915_drv.c Ben Widawsky  2013-10-16  1609  
af6dc7425 drivers/gpu/drm/i915/i915_drv.c Tvrtko Ursulin2016-12-01  1610
i915_save_state(dev_priv);
9e06dd39f drivers/gpu/drm/i915/i915_drv.c Jesse Barnes  2009-06-22  1611

Re: [Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-22 Thread Chris Wilson 
Quoting Chris Wilson (2018-05-22 16:49:02)
> In order to prepare the GPU for sleeping, we may want to submit commands
> to it. This is a complicated process that may even require some swapping
> in from shmemfs, if the GPU was in the wrong state. As such, we need to
> do this preparation step synchronously before the rest of the system has
> started to turn off (e.g. swapin fails if scsi is suspended).
> Fortunately, we are provided with a such a hook, pm_ops.prepare().
> 
> v2: Compile cleanup
> v3: Add manual calls to i915_drm_prepare() before i915_drm_suspend().

Revision 2 passed, but revision 3 fails. I don't understand... :(
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-22 Thread Chris Wilson 
In order to prepare the GPU for sleeping, we may want to submit commands
to it. This is a complicated process that may even require some swapping
in from shmemfs, if the GPU was in the wrong state. As such, we need to
do this preparation step synchronously before the rest of the system has
started to turn off (e.g. swapin fails if scsi is suspended).
Fortunately, we are provided with a such a hook, pm_ops.prepare().

v2: Compile cleanup
v3: Add manual calls to i915_drm_prepare() before i915_drm_suspend().

Testcase: igt/drv_suspend after igt/gem_tiled_swapping
Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.c | 55 +++--
 1 file changed, 45 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9c449b8d8eab..8642f9a1aff2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1553,12 +1553,30 @@ static bool suspend_to_idle(struct drm_i915_private 
*dev_priv)
return false;
 }
 
+static int i915_drm_prepare(struct drm_device *dev)
+{
+   struct drm_i915_private *i915 = to_i915(dev);
+   int err;
+
+   disable_rpm_wakeref_asserts(i915);
+
+   err = i915_gem_suspend(i915);
+   if (err) {
+   dev_err(>drm.pdev->dev,
+   "GEM idle failed, suspend/resume might fail\n");
+   goto out;
+   }
+out:
+   enable_rpm_wakeref_asserts(i915);
+
+   return err;
+}
+
 static int i915_drm_suspend(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = to_i915(dev);
struct pci_dev *pdev = dev_priv->drm.pdev;
pci_power_t opregion_target_state;
-   int error;
 
/* ignore lid events during suspend */
mutex_lock(_priv->modeset_restore_lock);
@@ -1575,13 +1593,6 @@ static int i915_drm_suspend(struct drm_device *dev)
 
pci_save_state(pdev);
 
-   error = i915_gem_suspend(dev_priv);
-   if (error) {
-   dev_err(>dev,
-   "GEM idle failed, resume might fail\n");
-   goto out;
-   }
-
intel_display_suspend(dev);
 
intel_dp_mst_suspend(dev);
@@ -1609,10 +1620,9 @@ static int i915_drm_suspend(struct drm_device *dev)
 
intel_csr_ucode_suspend(dev_priv);
 
-out:
enable_rpm_wakeref_asserts(dev_priv);
 
-   return error;
+   return 0;
 }
 
 static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
@@ -1695,6 +1705,10 @@ static int i915_suspend_switcheroo(struct drm_device 
*dev, pm_message_t state)
if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
return 0;
 
+   error = i915_drm_prepare(dev);
+   if (error)
+   return error;
+
error = i915_drm_suspend(dev);
if (error)
return error;
@@ -2081,6 +2095,22 @@ int i915_reset_engine(struct intel_engine_cs *engine, 
const char *msg)
return ret;
 }
 
+static int i915_pm_prepare(struct device *kdev)
+{
+   struct pci_dev *pdev = to_pci_dev(kdev);
+   struct drm_device *dev = pci_get_drvdata(pdev);
+
+   if (!dev) {
+   dev_err(kdev, "DRM not initialized, aborting suspend.\n");
+   return -ENODEV;
+   }
+
+   if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+   return 0;
+
+   return i915_drm_prepare(dev);
+}
+
 static int i915_pm_suspend(struct device *kdev)
 {
struct pci_dev *pdev = to_pci_dev(kdev);
@@ -2153,6 +2183,10 @@ static int i915_pm_freeze(struct device *kdev)
int ret;
 
if (dev->switch_power_state != DRM_SWITCH_POWER_OFF) {
+   ret = i915_drm_prepare(dev);
+   if (ret)
+   return ret;
+
ret = i915_drm_suspend(dev);
if (ret)
return ret;
@@ -2731,6 +2765,7 @@ const struct dev_pm_ops i915_pm_ops = {
 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
 * PMSG_RESUME]
 */
+   .prepare = i915_pm_prepare,
.suspend = i915_pm_suspend,
.suspend_late = i915_pm_suspend_late,
.resume_early = i915_pm_resume_early,
-- 
2.17.0

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

[Intel-gfx] [PATCH] drm/i915: Prepare GEM for suspend earlier

2018-05-22 Thread Chris Wilson 
In order to prepare the GPU for sleeping, we may want to submit commands
to it. This is a complicated process that may even require some swapping
in from shmemfs, if the GPU was in the wrong state. As such, we need to
do this preparation step synchronously before the rest of the system has
started to turn off (e.g. swapin fails if scsi is suspended).
Fortunately, we are provided with a such a hook, pm_ops.prepare().

Testcase: igt/drv_suspend after igt/gem_tiled_swapping
Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.c | 43 +++--
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9c449b8d8eab..256642fb8315 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1553,6 +1553,25 @@ static bool suspend_to_idle(struct drm_i915_private 
*dev_priv)
return false;
 }
 
+static int i915_drm_prepare(struct drm_device *dev)
+{
+   struct drm_i915_private *i915 = to_i915(dev);
+   int err;
+
+   disable_rpm_wakeref_asserts(i915);
+
+   err = i915_gem_suspend(i915);
+   if (err) {
+   dev_err(>drm.pdev->dev,
+   "GEM idle failed, suspend/resume might fail\n");
+   goto out;
+   }
+out:
+   enable_rpm_wakeref_asserts(i915);
+
+   return err;
+}
+
 static int i915_drm_suspend(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -1575,13 +1594,6 @@ static int i915_drm_suspend(struct drm_device *dev)
 
pci_save_state(pdev);
 
-   error = i915_gem_suspend(dev_priv);
-   if (error) {
-   dev_err(>dev,
-   "GEM idle failed, resume might fail\n");
-   goto out;
-   }
-
intel_display_suspend(dev);
 
intel_dp_mst_suspend(dev);
@@ -2081,6 +2093,22 @@ int i915_reset_engine(struct intel_engine_cs *engine, 
const char *msg)
return ret;
 }
 
+static int i915_pm_prepare(struct device *kdev)
+{
+   struct pci_dev *pdev = to_pci_dev(kdev);
+   struct drm_device *dev = pci_get_drvdata(pdev);
+
+   if (!dev) {
+   dev_err(kdev, "DRM not initialized, aborting suspend.\n");
+   return -ENODEV;
+   }
+
+   if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+   return 0;
+
+   return i915_drm_prepare(dev);
+}
+
 static int i915_pm_suspend(struct device *kdev)
 {
struct pci_dev *pdev = to_pci_dev(kdev);
@@ -2731,6 +2759,7 @@ const struct dev_pm_ops i915_pm_ops = {
 * S0ix (via system suspend) and S3 event handlers [PMSG_SUSPEND,
 * PMSG_RESUME]
 */
+   .prepare = i915_pm_prepare,
.suspend = i915_pm_suspend,
.suspend_late = i915_pm_suspend_late,
.resume_early = i915_pm_resume_early,
-- 
2.17.0

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