[PATCH 1/3] drm/amdgpu: wait for all rings to drain before runtime suspending

[PATCH 1/3] drm/amdgpu: wait for all rings to drain before runtime suspending

[Alex Deucher]

Add a safety check to runtime suspend to make sure all outstanding
fences have signaled before we suspend.  Doesn't fix any known issue.

We already do this via the fence driver suspend function, but we
just force completion rather than bailing.  This bails on runtime
suspend so we can try again later once the fences are signaled to
avoid missing any outstanding work.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index d1e9946ac218..61dc26515c7e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1214,13 +1214,23 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
 	struct amdgpu_device *adev = drm_dev->dev_private;
-	int ret;
+	int ret, i;
 
 	if (!adev->runpm) {
 		pm_runtime_forbid(dev);
 		return -EBUSY;
 	}
 
+	/* wait for all rings to drain before suspending */
+	for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
+		struct amdgpu_ring *ring = adev->rings[i];
+		if (ring && ring->sched.ready) {
+			ret = amdgpu_fence_wait_empty(ring);
+			if (ret)
+				return -EBUSY;
+		}
+	}
+
 	if (amdgpu_device_supports_boco(drm_dev))
 		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 	drm_kms_helper_poll_disable(drm_dev);
-- 
2.23.0

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

[PATCH 2/3] drm/amdgpu/pm_runtime: update usage count in fence handling

[Alex Deucher]

Increment the usage count in emit fence, and decrement in
process fence to make sure the GPU is always considered in
use while there are fences outstanding.  We always wait for
the engines to drain in runtime suspend, but in practice
that only covers short lived jobs for gfx.  This should
cover us for longer lived fences.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 377fe20bce23..e9efee04ca23 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -34,6 +34,7 @@
 #include <linux/kref.h>
 #include <linux/slab.h>
 #include <linux/firmware.h>
+#include <linux/pm_runtime.h>
 
 #include <drm/drm_debugfs.h>
 
@@ -154,7 +155,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f,
 		       seq);
 	amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
 			       seq, flags | AMDGPU_FENCE_FLAG_INT);
-
+	pm_runtime_get_noresume(adev->ddev->dev);
 	ptr = &ring->fence_drv.fences[seq & ring->fence_drv.num_fences_mask];
 	if (unlikely(rcu_dereference_protected(*ptr, 1))) {
 		struct dma_fence *old;
@@ -234,6 +235,7 @@ static void amdgpu_fence_schedule_fallback(struct amdgpu_ring *ring)
 bool amdgpu_fence_process(struct amdgpu_ring *ring)
 {
 	struct amdgpu_fence_driver *drv = &ring->fence_drv;
+	struct amdgpu_device *adev = ring->adev;
 	uint32_t seq, last_seq;
 	int r;
 
@@ -274,6 +276,8 @@ bool amdgpu_fence_process(struct amdgpu_ring *ring)
 			BUG();
 
 		dma_fence_put(fence);
+		pm_runtime_mark_last_busy(adev->ddev->dev);
+		pm_runtime_put_autosuspend(adev->ddev->dev);
 	} while (last_seq != seq);
 
 	return true;
-- 
2.23.0

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

[PATCH 3/3] drm/amdgpu: Enter low power state if CRTC active.

[Alex Deucher]

From: Andrey Grodzovsky <andrey.grodzovsky@amd.com>

CRTC in DPMS state off calls for low power state entry.
Support both atomic mode setting and pre-atomic mode setting.

v2: move comment

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 45 +++++++++++++++++++++----
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 61dc26515c7e..e7f7463a0cbe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1296,24 +1296,55 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
 {
 	struct drm_device *drm_dev = dev_get_drvdata(dev);
 	struct amdgpu_device *adev = drm_dev->dev_private;
-	struct drm_crtc *crtc;
+	/* we don't want the main rpm_idle to call suspend - we want to autosuspend */
+	int ret = 1;
 
 	if (!adev->runpm) {
 		pm_runtime_forbid(dev);
 		return -EBUSY;
 	}
 
-	list_for_each_entry(crtc, &drm_dev->mode_config.crtc_list, head) {
-		if (crtc->enabled) {
-			DRM_DEBUG_DRIVER("failing to power off - crtc active\n");
-			return -EBUSY;
+	if (amdgpu_device_has_dc_support(adev)) {
+		struct drm_crtc *crtc;
+
+		drm_modeset_lock_all(drm_dev);
+
+		drm_for_each_crtc(crtc, drm_dev) {
+			if (crtc->state->active) {
+				ret = -EBUSY;
+				break;
+			}
 		}
+
+		drm_modeset_unlock_all(drm_dev);
+
+	} else {
+		struct drm_connector *list_connector;
+		struct drm_connector_list_iter iter;
+
+		mutex_lock(&drm_dev->mode_config.mutex);
+		drm_modeset_lock(&drm_dev->mode_config.connection_mutex, NULL);
+
+		drm_connector_list_iter_begin(drm_dev, &iter);
+		drm_for_each_connector_iter(list_connector, &iter) {
+			if (list_connector->dpms ==  DRM_MODE_DPMS_ON) {
+				ret = -EBUSY;
+				break;
+			}
+		}
+
+		drm_connector_list_iter_end(&iter);
+
+		drm_modeset_unlock(&drm_dev->mode_config.connection_mutex);
+		mutex_unlock(&drm_dev->mode_config.mutex);
 	}
 
+	if (ret == -EBUSY)
+		DRM_DEBUG_DRIVER("failing to power off - crtc active\n");
+
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_autosuspend(dev);
-	/* we don't want the main rpm_idle to call suspend - we want to autosuspend */
-	return 1;
+	return ret;
 }
 
 long amdgpu_drm_ioctl(struct file *filp,
-- 
2.23.0

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

Re: [PATCH 2/3] drm/amdgpu/pm_runtime: update usage count in fence handling

[Christian König]

Am 16.12.19 um 18:18 schrieb Alex Deucher:
> Increment the usage count in emit fence, and decrement in
> process fence to make sure the GPU is always considered in
> use while there are fences outstanding.  We always wait for
> the engines to drain in runtime suspend, but in practice
> that only covers short lived jobs for gfx.  This should
> cover us for longer lived fences.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 377fe20bce23..e9efee04ca23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -34,6 +34,7 @@
>   #include <linux/kref.h>
>   #include <linux/slab.h>
>   #include <linux/firmware.h>
> +#include <linux/pm_runtime.h>
>   
>   #include <drm/drm_debugfs.h>
>   
> @@ -154,7 +155,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f,
>   		       seq);
>   	amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
>   			       seq, flags | AMDGPU_FENCE_FLAG_INT);
> -
> +	pm_runtime_get_noresume(adev->ddev->dev);
>   	ptr = &ring->fence_drv.fences[seq & ring->fence_drv.num_fences_mask];
>   	if (unlikely(rcu_dereference_protected(*ptr, 1))) {
>   		struct dma_fence *old;
> @@ -234,6 +235,7 @@ static void amdgpu_fence_schedule_fallback(struct amdgpu_ring *ring)
>   bool amdgpu_fence_process(struct amdgpu_ring *ring)
>   {
>   	struct amdgpu_fence_driver *drv = &ring->fence_drv;
> +	struct amdgpu_device *adev = ring->adev;
>   	uint32_t seq, last_seq;
>   	int r;
>   
> @@ -274,6 +276,8 @@ bool amdgpu_fence_process(struct amdgpu_ring *ring)
>   			BUG();
>   
>   		dma_fence_put(fence);
> +		pm_runtime_mark_last_busy(adev->ddev->dev);
> +		pm_runtime_put_autosuspend(adev->ddev->dev);

Are you sure this is ok? Keep in mind that this function is called in 
interrupt context.

Christian.

>   	} while (last_seq != seq);
>   
>   	return true;

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

Re: [PATCH 2/3] drm/amdgpu/pm_runtime: update usage count in fence handling

[Alex Deucher]

On Mon, Dec 16, 2019 at 3:19 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 16.12.19 um 18:18 schrieb Alex Deucher:
> > Increment the usage count in emit fence, and decrement in
> > process fence to make sure the GPU is always considered in
> > use while there are fences outstanding.  We always wait for
> > the engines to drain in runtime suspend, but in practice
> > that only covers short lived jobs for gfx.  This should
> > cover us for longer lived fences.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > index 377fe20bce23..e9efee04ca23 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> > @@ -34,6 +34,7 @@
> >   #include <linux/kref.h>
> >   #include <linux/slab.h>
> >   #include <linux/firmware.h>
> > +#include <linux/pm_runtime.h>
> >
> >   #include <drm/drm_debugfs.h>
> >
> > @@ -154,7 +155,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f,
> >                      seq);
> >       amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
> >                              seq, flags | AMDGPU_FENCE_FLAG_INT);
> > -
> > +     pm_runtime_get_noresume(adev->ddev->dev);
> >       ptr = &ring->fence_drv.fences[seq & ring->fence_drv.num_fences_mask];
> >       if (unlikely(rcu_dereference_protected(*ptr, 1))) {
> >               struct dma_fence *old;
> > @@ -234,6 +235,7 @@ static void amdgpu_fence_schedule_fallback(struct amdgpu_ring *ring)
> >   bool amdgpu_fence_process(struct amdgpu_ring *ring)
> >   {
> >       struct amdgpu_fence_driver *drv = &ring->fence_drv;
> > +     struct amdgpu_device *adev = ring->adev;
> >       uint32_t seq, last_seq;
> >       int r;
> >
> > @@ -274,6 +276,8 @@ bool amdgpu_fence_process(struct amdgpu_ring *ring)
> >                       BUG();
> >
> >               dma_fence_put(fence);
> > +             pm_runtime_mark_last_busy(adev->ddev->dev);
> > +             pm_runtime_put_autosuspend(adev->ddev->dev);
>
> Are you sure this is ok? Keep in mind that this function is called in
> interrupt context.

According to:
https://www.kernel.org/doc/Documentation/power/runtime_pm.txt
it's ok to call those in an interrupt context.

Alex

>
> Christian.
>
> >       } while (last_seq != seq);
> >
> >       return true;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/amdgpu/pm_runtime: update usage count in fence handling

[Christian König]

Am 16.12.19 um 21:22 schrieb Alex Deucher:
> On Mon, Dec 16, 2019 at 3:19 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 16.12.19 um 18:18 schrieb Alex Deucher:
>>> Increment the usage count in emit fence, and decrement in
>>> process fence to make sure the GPU is always considered in
>>> use while there are fences outstanding.  We always wait for
>>> the engines to drain in runtime suspend, but in practice
>>> that only covers short lived jobs for gfx.  This should
>>> cover us for longer lived fences.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>> index 377fe20bce23..e9efee04ca23 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>> @@ -34,6 +34,7 @@
>>>    #include <linux/kref.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/firmware.h>
>>> +#include <linux/pm_runtime.h>
>>>
>>>    #include <drm/drm_debugfs.h>
>>>
>>> @@ -154,7 +155,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f,
>>>                       seq);
>>>        amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
>>>                               seq, flags | AMDGPU_FENCE_FLAG_INT);
>>> -
>>> +     pm_runtime_get_noresume(adev->ddev->dev);
>>>        ptr = &ring->fence_drv.fences[seq & ring->fence_drv.num_fences_mask];
>>>        if (unlikely(rcu_dereference_protected(*ptr, 1))) {
>>>                struct dma_fence *old;
>>> @@ -234,6 +235,7 @@ static void amdgpu_fence_schedule_fallback(struct amdgpu_ring *ring)
>>>    bool amdgpu_fence_process(struct amdgpu_ring *ring)
>>>    {
>>>        struct amdgpu_fence_driver *drv = &ring->fence_drv;
>>> +     struct amdgpu_device *adev = ring->adev;
>>>        uint32_t seq, last_seq;
>>>        int r;
>>>
>>> @@ -274,6 +276,8 @@ bool amdgpu_fence_process(struct amdgpu_ring *ring)
>>>                        BUG();
>>>
>>>                dma_fence_put(fence);
>>> +             pm_runtime_mark_last_busy(adev->ddev->dev);
>>> +             pm_runtime_put_autosuspend(adev->ddev->dev);
>> Are you sure this is ok? Keep in mind that this function is called in
>> interrupt context.
> According to:
> https://www.kernel.org/doc/Documentation/power/runtime_pm.txt
> it's ok to call those in an interrupt context.

In this case the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Christian.

>
> Alex
>
>> Christian.
>>
>>>        } while (last_seq != seq);
>>>
>>>        return true;
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

RE: [PATCH 2/3] drm/amdgpu/pm_runtime: update usage count in fence handling

[Zeng, Oak]

[AMD Official Use Only - Internal Distribution Only]



Regards,
Oak

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Christian König
Sent: Monday, December 16, 2019 3:25 PM
To: Alex Deucher <alexdeucher@gmail.com>; Koenig, Christian <Christian.Koenig@amd.com>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/3] drm/amdgpu/pm_runtime: update usage count in fence handling

Am 16.12.19 um 21:22 schrieb Alex Deucher:
> On Mon, Dec 16, 2019 at 3:19 PM Christian König 
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 16.12.19 um 18:18 schrieb Alex Deucher:
>>> Increment the usage count in emit fence, and decrement in process 
>>> fence to make sure the GPU is always considered in use while there 
>>> are fences outstanding.  We always wait for the engines to drain in 
>>> runtime suspend, but in practice that only covers short lived jobs 
>>> for gfx.  This should cover us for longer lived fences.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>> index 377fe20bce23..e9efee04ca23 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>> @@ -34,6 +34,7 @@
>>>    #include <linux/kref.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/firmware.h>
>>> +#include <linux/pm_runtime.h>
>>>
>>>    #include <drm/drm_debugfs.h>
>>>
>>> @@ -154,7 +155,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f,
>>>                       seq);
>>>        amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
>>>                               seq, flags | AMDGPU_FENCE_FLAG_INT);
>>> -
>>> +     pm_runtime_get_noresume(adev->ddev->dev);
>>>        ptr = &ring->fence_drv.fences[seq & ring->fence_drv.num_fences_mask];
>>>        if (unlikely(rcu_dereference_protected(*ptr, 1))) {
>>>                struct dma_fence *old; @@ -234,6 +235,7 @@ static 
>>> void amdgpu_fence_schedule_fallback(struct amdgpu_ring *ring)
>>>    bool amdgpu_fence_process(struct amdgpu_ring *ring)
>>>    {
>>>        struct amdgpu_fence_driver *drv = &ring->fence_drv;
>>> +     struct amdgpu_device *adev = ring->adev;
>>>        uint32_t seq, last_seq;
>>>        int r;
>>>
>>> @@ -274,6 +276,8 @@ bool amdgpu_fence_process(struct amdgpu_ring *ring)
>>>                        BUG();
>>>
>>>                dma_fence_put(fence);
>>> +             pm_runtime_mark_last_busy(adev->ddev->dev);
>>> +             pm_runtime_put_autosuspend(adev->ddev->dev);
>> Are you sure this is ok? Keep in mind that this function is called in 
>> interrupt context.
> According to:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> kernel.org%2Fdoc%2FDocumentation%2Fpower%2Fruntime_pm.txt&amp;data=02%
> 7C01%7Coak.zeng%40amd.com%7C7008cf5373094160552b08d78266032d%7C3dd8961
> fe4884e608e11a82d994e183d%7C0%7C0%7C637121247082078997&amp;sdata=kfUKc
> dw0PSXWspOBfXQT6BF4r4q3m64cPMbkX7xjOaA%3D&amp;reserved=0
> it's ok to call those in an interrupt context.

In this case the patch is Reviewed-by: Christian König <christian.koenig@amd.com>.

[Oak] Yes, pm_runtime_put_autosuspend calls __pm_runtime_suspend in the rpmflags set to RPM_ASYNC flag so it can be called in atomic context.

Christian.

>
> Alex
>
>> Christian.
>>
>>>        } while (last_seq != seq);
>>>
>>>        return true;
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Coa
> k.zeng%40amd.com%7C7008cf5373094160552b08d78266032d%7C3dd8961fe4884e60
> 8e11a82d994e183d%7C0%7C0%7C637121247082078997&amp;sdata=OIeDvbaRaR4lMJ
> 1munKIMkhGwQpSqxroNhIPJGmqx6c%3D&amp;reserved=0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Coak.zeng%40amd.com%7C7008cf5373094160552b08d78266032d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637121247082078997&amp;sdata=OIeDvbaRaR4lMJ1munKIMkhGwQpSqxroNhIPJGmqx6c%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] drm/amdgpu: wait for all rings to drain before runtime suspending

[Andrey Grodzovsky]

Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>

Andrey

On 12/16/19 12:18 PM, Alex Deucher wrote:
> Add a safety check to runtime suspend to make sure all outstanding
> fences have signaled before we suspend.  Doesn't fix any known issue.
>
> We already do this via the fence driver suspend function, but we
> just force completion rather than bailing.  This bails on runtime
> suspend so we can try again later once the fences are signaled to
> avoid missing any outstanding work.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index d1e9946ac218..61dc26515c7e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1214,13 +1214,23 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>   	struct pci_dev *pdev = to_pci_dev(dev);
>   	struct drm_device *drm_dev = pci_get_drvdata(pdev);
>   	struct amdgpu_device *adev = drm_dev->dev_private;
> -	int ret;
> +	int ret, i;
>   
>   	if (!adev->runpm) {
>   		pm_runtime_forbid(dev);
>   		return -EBUSY;
>   	}
>   
> +	/* wait for all rings to drain before suspending */
> +	for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
> +		struct amdgpu_ring *ring = adev->rings[i];
> +		if (ring && ring->sched.ready) {
> +			ret = amdgpu_fence_wait_empty(ring);
> +			if (ret)
> +				return -EBUSY;
> +		}
> +	}
> +
>   	if (amdgpu_device_supports_boco(drm_dev))
>   		drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>   	drm_kms_helper_poll_disable(drm_dev);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/amdgpu: Enter low power state if CRTC active.

[Alex Deucher]

On Mon, Dec 16, 2019 at 12:18 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> From: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>
> CRTC in DPMS state off calls for low power state entry.
> Support both atomic mode setting and pre-atomic mode setting.
>
> v2: move comment
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Ping?

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 45 +++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 61dc26515c7e..e7f7463a0cbe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1296,24 +1296,55 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
>  {
>         struct drm_device *drm_dev = dev_get_drvdata(dev);
>         struct amdgpu_device *adev = drm_dev->dev_private;
> -       struct drm_crtc *crtc;
> +       /* we don't want the main rpm_idle to call suspend - we want to autosuspend */
> +       int ret = 1;
>
>         if (!adev->runpm) {
>                 pm_runtime_forbid(dev);
>                 return -EBUSY;
>         }
>
> -       list_for_each_entry(crtc, &drm_dev->mode_config.crtc_list, head) {
> -               if (crtc->enabled) {
> -                       DRM_DEBUG_DRIVER("failing to power off - crtc active\n");
> -                       return -EBUSY;
> +       if (amdgpu_device_has_dc_support(adev)) {
> +               struct drm_crtc *crtc;
> +
> +               drm_modeset_lock_all(drm_dev);
> +
> +               drm_for_each_crtc(crtc, drm_dev) {
> +                       if (crtc->state->active) {
> +                               ret = -EBUSY;
> +                               break;
> +                       }
>                 }
> +
> +               drm_modeset_unlock_all(drm_dev);
> +
> +       } else {
> +               struct drm_connector *list_connector;
> +               struct drm_connector_list_iter iter;
> +
> +               mutex_lock(&drm_dev->mode_config.mutex);
> +               drm_modeset_lock(&drm_dev->mode_config.connection_mutex, NULL);
> +
> +               drm_connector_list_iter_begin(drm_dev, &iter);
> +               drm_for_each_connector_iter(list_connector, &iter) {
> +                       if (list_connector->dpms ==  DRM_MODE_DPMS_ON) {
> +                               ret = -EBUSY;
> +                               break;
> +                       }
> +               }
> +
> +               drm_connector_list_iter_end(&iter);
> +
> +               drm_modeset_unlock(&drm_dev->mode_config.connection_mutex);
> +               mutex_unlock(&drm_dev->mode_config.mutex);
>         }
>
> +       if (ret == -EBUSY)
> +               DRM_DEBUG_DRIVER("failing to power off - crtc active\n");
> +
>         pm_runtime_mark_last_busy(dev);
>         pm_runtime_autosuspend(dev);
> -       /* we don't want the main rpm_idle to call suspend - we want to autosuspend */
> -       return 1;
> +       return ret;
>  }
>
>  long amdgpu_drm_ioctl(struct file *filp,
> --
> 2.23.0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/amdgpu: Enter low power state if CRTC active.

[Alex Deucher]

Ping?

On Wed, Feb 19, 2020 at 11:25 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Mon, Dec 16, 2019 at 12:18 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > From: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> >
> > CRTC in DPMS state off calls for low power state entry.
> > Support both atomic mode setting and pre-atomic mode setting.
> >
> > v2: move comment
> >
> > Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>
> Ping?
>
> Alex
>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 45 +++++++++++++++++++++----
> >  1 file changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 61dc26515c7e..e7f7463a0cbe 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -1296,24 +1296,55 @@ static int amdgpu_pmops_runtime_idle(struct device *dev)
> >  {
> >         struct drm_device *drm_dev = dev_get_drvdata(dev);
> >         struct amdgpu_device *adev = drm_dev->dev_private;
> > -       struct drm_crtc *crtc;
> > +       /* we don't want the main rpm_idle to call suspend - we want to autosuspend */
> > +       int ret = 1;
> >
> >         if (!adev->runpm) {
> >                 pm_runtime_forbid(dev);
> >                 return -EBUSY;
> >         }
> >
> > -       list_for_each_entry(crtc, &drm_dev->mode_config.crtc_list, head) {
> > -               if (crtc->enabled) {
> > -                       DRM_DEBUG_DRIVER("failing to power off - crtc active\n");
> > -                       return -EBUSY;
> > +       if (amdgpu_device_has_dc_support(adev)) {
> > +               struct drm_crtc *crtc;
> > +
> > +               drm_modeset_lock_all(drm_dev);
> > +
> > +               drm_for_each_crtc(crtc, drm_dev) {
> > +                       if (crtc->state->active) {
> > +                               ret = -EBUSY;
> > +                               break;
> > +                       }
> >                 }
> > +
> > +               drm_modeset_unlock_all(drm_dev);
> > +
> > +       } else {
> > +               struct drm_connector *list_connector;
> > +               struct drm_connector_list_iter iter;
> > +
> > +               mutex_lock(&drm_dev->mode_config.mutex);
> > +               drm_modeset_lock(&drm_dev->mode_config.connection_mutex, NULL);
> > +
> > +               drm_connector_list_iter_begin(drm_dev, &iter);
> > +               drm_for_each_connector_iter(list_connector, &iter) {
> > +                       if (list_connector->dpms ==  DRM_MODE_DPMS_ON) {
> > +                               ret = -EBUSY;
> > +                               break;
> > +                       }
> > +               }
> > +
> > +               drm_connector_list_iter_end(&iter);
> > +
> > +               drm_modeset_unlock(&drm_dev->mode_config.connection_mutex);
> > +               mutex_unlock(&drm_dev->mode_config.mutex);
> >         }
> >
> > +       if (ret == -EBUSY)
> > +               DRM_DEBUG_DRIVER("failing to power off - crtc active\n");
> > +
> >         pm_runtime_mark_last_busy(dev);
> >         pm_runtime_autosuspend(dev);
> > -       /* we don't want the main rpm_idle to call suspend - we want to autosuspend */
> > -       return 1;
> > +       return ret;
> >  }
> >
> >  long amdgpu_drm_ioctl(struct file *filp,
> > --
> > 2.23.0
> >
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx