> Date: Thu, 15 Aug 2024 13:54:00 +1000
> From: Jonathan Gray <j...@jsg.id.au>
> 
> for in_s0ix to be set the activate function needs to
> call the pmops wrappers
> 
> that may help
> 
> amdgpu_pm_ops contains other function pointers that may
> also be of interest
> 
> Index: sys/dev/pci/drm/amd/amdgpu/amdgpu_drv.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/drm/amd/amdgpu/amdgpu_drv.c,v
> diff -u -p -r1.44 amdgpu_drv.c
> --- sys/dev/pci/drm/amd/amdgpu/amdgpu_drv.c   14 May 2024 04:55:42 -0000      
> 1.44
> +++ sys/dev/pci/drm/amd/amdgpu/amdgpu_drv.c   15 Aug 2024 03:45:44 -0000
> @@ -2412,9 +2412,10 @@ static void amdgpu_pmops_complete(struct
>       /* nothing to do */
>  }
>  
> -static int amdgpu_pmops_suspend(struct device *dev)
> +#endif /* notyet */
> +
> +static int amdgpu_pmops_suspend(struct drm_device *drm_dev)
>  {
> -     struct drm_device *drm_dev = dev_get_drvdata(dev);
>       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>  
>       adev->suspend_complete = false;
> @@ -2427,9 +2428,8 @@ static int amdgpu_pmops_suspend(struct d
>       return amdgpu_device_suspend(drm_dev, true);
>  }
>  
> -static int amdgpu_pmops_suspend_noirq(struct device *dev)
> +static int amdgpu_pmops_suspend_noirq(struct drm_device *drm_dev)
>  {
> -     struct drm_device *drm_dev = dev_get_drvdata(dev);
>       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>  
>       adev->suspend_complete = true;
> @@ -2439,18 +2439,19 @@ static int amdgpu_pmops_suspend_noirq(st
>       return 0;
>  }
>  
> -static int amdgpu_pmops_resume(struct device *dev)
> +static int amdgpu_pmops_resume(struct drm_device *drm_dev)
>  {
> -     struct drm_device *drm_dev = dev_get_drvdata(dev);
>       struct amdgpu_device *adev = drm_to_adev(drm_dev);
>       int r;
>  
>       if (!adev->in_s0ix && !adev->in_s3)
>               return 0;
>  
> +#ifdef notyet
>       /* Avoids registers access if device is physically gone */
>       if (!pci_device_is_present(adev->pdev))
>               adev->no_hw_access = true;
> +#endif
>  
>       r = amdgpu_device_resume(drm_dev, true);
>       if (amdgpu_acpi_is_s0ix_active(adev))
> @@ -2460,6 +2461,8 @@ static int amdgpu_pmops_resume(struct de
>       return r;
>  }
>  
> +#ifdef notyet
> +
>  static int amdgpu_pmops_freeze(struct device *dev)
>  {
>       struct drm_device *drm_dev = dev_get_drvdata(dev);
> @@ -3672,14 +3675,15 @@ amdgpu_activate(struct device *self, int
>       case DVACT_QUIESCE:
>               rv = config_activate_children(self, act);
>               amdgpu_device_prepare(dev);
> -             amdgpu_device_suspend(dev, true);
> +             amdgpu_pmops_suspend(dev);
>               break;
>       case DVACT_SUSPEND:
> +             amdgpu_pmops_suspend_noirq(dev);
>               break;
>       case DVACT_RESUME:
>               break;
>       case DVACT_WAKEUP:
> -             amdgpu_device_resume(dev, true);
> +             amdgpu_pmops_resume(dev);
>               rv = config_activate_children(self, act);
>               break;
>       }

That diff skips amdgpu_pmops_prepare().  Here is a better diff.  And
interestingly enough this fixes S3 suspend/resume on my m715q mini
desktop with:

amdgpu0: RAVEN GC 9.1.0 8 CU rev 0x01

I had to backport the following diff that's not in linux-stable to get
rid of a warning:

commit 3a9626c816db901def438dc2513622e281186d39
Author: Mario Limonciello <mario.limoncie...@amd.com>
Date:   Wed Feb 7 23:52:55 2024 -0600

    drm/amd: Stop evicting resources on APUs in suspend
    
    commit 5095d5418193 ("drm/amd: Evict resources during PM ops prepare()
    callback") intentionally moved the eviction of resources to earlier in
    the suspend process, but this introduced a subtle change that it occurs
    before adev->in_s0ix or adev->in_s3 are set. This meant that APUs
    actually started to evict resources at suspend time as well.
    
    Explicitly set s0ix or s3 in the prepare() stage, and unset them if the
    prepare() stage failed.
    
    v2: squash in warning fix from Stephen Rothwell
    
    Reported-by: J<C3><BC>rg Billeter <j...@bitron.ch>
    Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3132#note_2271038
    Fixes: 5095d5418193 ("drm/amd: Evict resources during PM ops prepare() 
callback")
    Acked-by: Alex Deucher <alexander.deuc...@amd.com>
    Signed-off-by: Mario Limonciello <mario.limoncie...@amd.com>
    Signed-off-by: Alex Deucher <alexander.deuc...@amd.com>


The diff also defines CONFIG_AMD_PMC and #ifdefs out the FADT check.
With that suspend-to-idle also works on that m715q.  I'm still not
entirely sure what to do there.  But I'll probably propose to commit
those separately such that we can easily back those bits out.

Thoughts?  Tests?  ok?


Index: dev/pci/drm/amd/amdgpu/amdgpu.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/amd/amdgpu/amdgpu.h,v
diff -u -p -r1.24 amdgpu.h
--- dev/pci/drm/amd/amdgpu/amdgpu.h     11 Apr 2024 03:24:40 -0000      1.24
+++ dev/pci/drm/amd/amdgpu/amdgpu.h     15 Aug 2024 22:34:06 -0000
@@ -1487,9 +1487,11 @@ static inline int amdgpu_acpi_smart_shif
 #if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND)
 bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev);
 bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev);
+void amdgpu_choose_low_power_state(struct amdgpu_device *adev);
 #else
 static inline bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) { 
return false; }
 static inline bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev) { 
return false; }
+static inline void amdgpu_choose_low_power_state(struct amdgpu_device *adev) { 
}
 #endif
 
 #if defined(CONFIG_DRM_AMD_DC)
Index: dev/pci/drm/amd/amdgpu/amdgpu_acpi.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/amd/amdgpu/amdgpu_acpi.c,v
diff -u -p -r1.14 amdgpu_acpi.c
--- dev/pci/drm/amd/amdgpu/amdgpu_acpi.c        26 Jan 2024 11:36:26 -0000      
1.14
+++ dev/pci/drm/amd/amdgpu/amdgpu_acpi.c        15 Aug 2024 22:34:06 -0000
@@ -1519,6 +1519,7 @@ bool amdgpu_acpi_is_s0ix_active(struct a
        if (adev->asic_type < CHIP_RAVEN)
                return false;
 
+#ifdef __linux__
        /*
         * If ACPI_FADT_LOW_POWER_S0 is not set in the FADT, it is generally
         * risky to do any special firmware-related preparations for entering
@@ -1531,6 +1532,7 @@ bool amdgpu_acpi_is_s0ix_active(struct a
                              "To use suspend-to-idle change the sleep mode in 
BIOS setup.\n");
                return false;
        }
+#endif
 
 #if !IS_ENABLED(CONFIG_AMD_PMC)
        dev_err_once(adev->dev,
@@ -1539,6 +1541,21 @@ bool amdgpu_acpi_is_s0ix_active(struct a
 #else
        return true;
 #endif /* CONFIG_AMD_PMC */
+}
+
+/**
+ * amdgpu_choose_low_power_state
+ *
+ * @adev: amdgpu_device_pointer
+ *
+ * Choose the target low power state for the GPU
+ */
+void amdgpu_choose_low_power_state(struct amdgpu_device *adev)
+{
+       if (amdgpu_acpi_is_s0ix_active(adev))
+               adev->in_s0ix = true;
+       else if (amdgpu_acpi_is_s3_active(adev))
+               adev->in_s3 = true;
 }
 
 #endif /* CONFIG_SUSPEND */
Index: dev/pci/drm/amd/amdgpu/amdgpu_device.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/amd/amdgpu/amdgpu_device.c,v
diff -u -p -r1.66 amdgpu_device.c
--- dev/pci/drm/amd/amdgpu/amdgpu_device.c      15 Aug 2024 03:04:18 -0000      
1.66
+++ dev/pci/drm/amd/amdgpu/amdgpu_device.c      15 Aug 2024 22:34:06 -0000
@@ -4221,13 +4221,15 @@ int amdgpu_device_prepare(struct drm_dev
        struct amdgpu_device *adev = drm_to_adev(dev);
        int i, r;
 
+       amdgpu_choose_low_power_state(adev);
+
        if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
                return 0;
 
        /* Evict the majority of BOs before starting suspend sequence */
        r = amdgpu_device_evict_resources(adev);
        if (r)
-               return r;
+               goto unprepare;
 
        flush_delayed_work(&adev->gfx.gfx_off_delay_work);
 
@@ -4238,10 +4240,15 @@ int amdgpu_device_prepare(struct drm_dev
                        continue;
                r = adev->ip_blocks[i].version->funcs->prepare_suspend((void 
*)adev);
                if (r)
-                       return r;
+                       goto unprepare;
        }
 
        return 0;
+
+unprepare:
+       adev->in_s0ix = adev->in_s3 = false;
+
+       return r;
 }
 
 /**
Index: dev/pci/drm/amd/amdgpu/amdgpu_drv.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/amd/amdgpu/amdgpu_drv.c,v
diff -u -p -r1.44 amdgpu_drv.c
--- dev/pci/drm/amd/amdgpu/amdgpu_drv.c 14 May 2024 04:55:42 -0000      1.44
+++ dev/pci/drm/amd/amdgpu/amdgpu_drv.c 15 Aug 2024 22:34:06 -0000
@@ -2383,8 +2383,6 @@ static void amdgpu_drv_delayed_reset_wor
        }
 }
 
-#ifdef notyet
-
 static int amdgpu_pmops_prepare(struct device *dev)
 {
        struct drm_device *drm_dev = dev_get_drvdata(dev);
@@ -2460,6 +2458,8 @@ static int amdgpu_pmops_resume(struct de
        return r;
 }
 
+#ifdef notyet
+
 static int amdgpu_pmops_freeze(struct device *dev)
 {
        struct drm_device *drm_dev = dev_get_drvdata(dev);
@@ -3496,6 +3496,8 @@ amdgpu_attachhook(struct device *self)
        struct drm_gem_object *obj;
        struct amdgpu_bo *rbo;
 
+       dev_set_drvdata(self, dev);
+
        r = amdgpu_driver_load_kms(adev, adev->flags);
        if (r)
                goto out;
@@ -3671,15 +3673,16 @@ amdgpu_activate(struct device *self, int
        switch (act) {
        case DVACT_QUIESCE:
                rv = config_activate_children(self, act);
-               amdgpu_device_prepare(dev);
-               amdgpu_device_suspend(dev, true);
+               amdgpu_pmops_prepare(self);
+               amdgpu_pmops_suspend(self);
                break;
        case DVACT_SUSPEND:
+               amdgpu_pmops_suspend_noirq(self);
                break;
        case DVACT_RESUME:
                break;
        case DVACT_WAKEUP:
-               amdgpu_device_resume(dev, true);
+               amdgpu_pmops_resume(self);
                rv = config_activate_children(self, act);
                break;
        }
Index: dev/pci/drm/include/generated/autoconf.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/include/generated/autoconf.h,v
diff -u -p -r1.15 autoconf.h
--- dev/pci/drm/include/generated/autoconf.h    13 Jul 2024 15:38:21 -0000      
1.15
+++ dev/pci/drm/include/generated/autoconf.h    15 Aug 2024 22:34:07 -0000
@@ -36,6 +36,7 @@
 #if NACPI > 0
 #define CONFIG_ACPI                            1
 #define CONFIG_ACPI_SLEEP                      1
+#define CONFIG_AMD_PMC                         1
 #endif
 #endif
 
Index: dev/pci/drm/include/linux/pci.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/include/linux/pci.h,v
diff -u -p -r1.16 pci.h
--- dev/pci/drm/include/linux/pci.h     16 Jan 2024 23:38:13 -0000      1.16
+++ dev/pci/drm/include/linux/pci.h     15 Aug 2024 22:34:07 -0000
@@ -524,4 +524,10 @@ pci_match_id(const struct pci_device_id 
 #define PCI_CLASS_ACCELERATOR_PROCESSING \
     (PCI_CLASS_ACCELERATOR << 8)
 
+static inline int
+pci_device_is_present(struct pci_dev *pdev)
+{
+       return 1;
+}
+
 #endif /* _LINUX_PCI_H_ */
Index: dev/pci/drm/include/linux/pm_runtime.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/include/linux/pm_runtime.h,v
diff -u -p -r1.4 pm_runtime.h
--- dev/pci/drm/include/linux/pm_runtime.h      1 Mar 2022 04:08:04 -0000       
1.4
+++ dev/pci/drm/include/linux/pm_runtime.h      15 Aug 2024 22:34:07 -0000
@@ -80,4 +80,10 @@ pm_runtime_get_if_active(struct device *
        return -EINVAL;
 }
 
+static inline int
+pm_runtime_suspended(struct device *dev)
+{
+       return 0;
+}
+
 #endif

Reply via email to