From: Rafael J. Wysocki <[email protected]>
I have recently noticed a 55 sec. delay during the "device freeze"
phase of hibernation on my test-bed HP nx6325. Due to the 100%
reproducibility of it I was able to narrow it down to
radeon_suspend_kms() and then it turned out that the delay occured
somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
necessary or even very useful to me to evict VRAM at this particular
point, because we're going to create an image and bring the device
back to the fully functional state in a little while. Thus, I think
the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
which makes the delay go away.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/gpu/drm/radeon/radeon_device.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/gpu/drm/radeon/radeon_device.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/radeon/radeon_device.c
+++ linux-2.6/drivers/gpu/drm/radeon/radeon_device.c
@@ -750,8 +750,11 @@ int radeon_suspend_kms(struct drm_device
}
}
}
- /* evict vram memory */
- radeon_bo_evict_vram(rdev);
+
+ if (state.event != PM_EVENT_FREEZE) {
+ /* evict vram memory */
+ radeon_bo_evict_vram(rdev);
+ }
/* wait for gpu to finish processing current batch */
radeon_fence_wait_last(rdev);
2010/6/18 Rafael J. Wysocki <[email protected]>:
> From: Rafael J. Wysocki <[email protected]>
Thanks for you recent involvement in radeon driver! I can see you used
way shorter commit title this time ;) Just as a minor note, maybe you
could follow prefix-style we used to see around? Like simple
"drm/radeon/pm"? Or eventually something different instead of "pm" to
match difference between s/r and pm as engine/memory/voltage ops.
--
Greetings,
Rafał
On Fri, 2010-06-18 at 22:21 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> I have recently noticed a 55 sec. delay during the "device freeze"
> phase of hibernation on my test-bed HP nx6325. Due to the 100%
> reproducibility of it I was able to narrow it down to
> radeon_suspend_kms() and then it turned out that the delay occured
> somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
> necessary or even very useful to me to evict VRAM at this particular
> point, because we're going to create an image and bring the device
> back to the fully functional state in a little while. Thus, I think
> the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
> which makes the delay go away.
I'm not 100% sure of the hibernate sequencing and its early in the
morning, but we want to evict VRAM before image building so we can have
the contents of VRAM in the image so we can restore them on resume. Does
this just avoid evicting them a second time after we created the image?
Dave.
On Friday, June 18, 2010, Rafał Miłecki wrote:
> 2010/6/18 Rafael J. Wysocki <[email protected]>:
> > From: Rafael J. Wysocki <[email protected]>
>
> Thanks for you recent involvement in radeon driver!
Well, I'm just trying to address the issues I see on my test boxes.
> I can see you used way shorter commit title this time ;) Just as a minor
> note, maybe you could follow prefix-style we used to see around? Like simple
> "drm/radeon/pm"? Or eventually something different instead of "pm" to
> match difference between s/r and pm as engine/memory/voltage ops.
Sure, I can use whatever is suitable to you guys.
Thanks,
Rafael
On Friday, June 18, 2010, Dave Airlie wrote:
> On Fri, 2010-06-18 at 22:21 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > I have recently noticed a 55 sec. delay during the "device freeze"
> > phase of hibernation on my test-bed HP nx6325. Due to the 100%
> > reproducibility of it I was able to narrow it down to
> > radeon_suspend_kms() and then it turned out that the delay occured
> > somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
> > necessary or even very useful to me to evict VRAM at this particular
> > point, because we're going to create an image and bring the device
> > back to the fully functional state in a little while. Thus, I think
> > the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
> > which makes the delay go away.
>
> I'm not 100% sure of the hibernate sequencing and its early in the
> morning, but we want to evict VRAM before image building so we can have
> the contents of VRAM in the image so we can restore them on resume. Does
> this just avoid evicting them a second time after we created the image?
No, it's the first time, before creating the image, but I didn't seen any
difference on resume with and without the patch, so I thought it was a good
idea. :-)
If you don't like it, though, I guess it will be necessary to find the root
cause of the delay.
Rafael
On Sat, 2010-06-19 at 01:23 +0200, Rafael J. Wysocki wrote:
> On Friday, June 18, 2010, Dave Airlie wrote:
> > On Fri, 2010-06-18 at 22:21 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <[email protected]>
> > >
> > > I have recently noticed a 55 sec. delay during the "device freeze"
> > > phase of hibernation on my test-bed HP nx6325. Due to the 100%
> > > reproducibility of it I was able to narrow it down to
> > > radeon_suspend_kms() and then it turned out that the delay occured
> > > somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
> > > necessary or even very useful to me to evict VRAM at this particular
> > > point, because we're going to create an image and bring the device
> > > back to the fully functional state in a little while. Thus, I think
> > > the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
> > > which makes the delay go away.
> >
> > I'm not 100% sure of the hibernate sequencing and its early in the
> > morning, but we want to evict VRAM before image building so we can have
> > the contents of VRAM in the image so we can restore them on resume. Does
> > this just avoid evicting them a second time after we created the image?
>
> No, it's the first time, before creating the image, but I didn't seen any
> difference on resume with and without the patch, so I thought it was a good
> idea. :-)
On the machine you have its most likely not going to show up unless you
are running a 3D app or something across suspend, since currently X
re-exposes most apps on VT switch, so they just redraw.
Was it always this slow? you can see how many objects are in vram using
debugfs (/sys/kernel/debug/dri/0/radeon_vram_mm), it sounds like the TTM
eviction process is blocking on something, we shouldn't be using any
UC/WC memory on that machine so I can't imagine the new pool allocator
stuff would get in the way. Maybe its the lack of GFP_USER, (Jerome
posted a patch).
Dave.
On Saturday, June 19, 2010, Dave Airlie wrote:
> On Sat, 2010-06-19 at 01:23 +0200, Rafael J. Wysocki wrote:
> > On Friday, June 18, 2010, Dave Airlie wrote:
> > > On Fri, 2010-06-18 at 22:21 +0200, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <[email protected]>
> > > >
> > > > I have recently noticed a 55 sec. delay during the "device freeze"
> > > > phase of hibernation on my test-bed HP nx6325. Due to the 100%
> > > > reproducibility of it I was able to narrow it down to
> > > > radeon_suspend_kms() and then it turned out that the delay occured
> > > > somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
> > > > necessary or even very useful to me to evict VRAM at this particular
> > > > point, because we're going to create an image and bring the device
> > > > back to the fully functional state in a little while. Thus, I think
> > > > the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
> > > > which makes the delay go away.
> > >
> > > I'm not 100% sure of the hibernate sequencing and its early in the
> > > morning, but we want to evict VRAM before image building so we can have
> > > the contents of VRAM in the image so we can restore them on resume. Does
> > > this just avoid evicting them a second time after we created the image?
> >
> > No, it's the first time, before creating the image, but I didn't seen any
> > difference on resume with and without the patch, so I thought it was a good
> > idea. :-)
>
> On the machine you have its most likely not going to show up unless you
> are running a 3D app or something across suspend, since currently X
> re-exposes most apps on VT switch, so they just redraw.
Yes. Moreover, hibernation is always done after a VT switch. That's why
I said I thought the eviction wasn't necessary in the changelog.
BTW, I have three different test boxes with radeon hardware and the
$subject patch is not a problem on any of them.
> Was it always this slow?
Nope. It definitely is a regression, although I'm not sure what's the last
good kernel.
> you can see how many objects are in vram using
> debugfs (/sys/kernel/debug/dri/0/radeon_vram_mm), it sounds like the TTM
> eviction process is blocking on something,
Yup.
> we shouldn't be using any UC/WC memory on that machine so I can't imagine
> the new pool allocator stuff would get in the way. Maybe its the lack of
> GFP_USER, (Jerome posted a patch).
Can you give me a pointer to that patch, please?
Rafael
On Saturday, June 19, 2010, Dave Airlie wrote:
> On Sat, 2010-06-19 at 01:23 +0200, Rafael J. Wysocki wrote:
> > On Friday, June 18, 2010, Dave Airlie wrote:
> > > On Fri, 2010-06-18 at 22:21 +0200, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <[email protected]>
> > > >
> > > > I have recently noticed a 55 sec. delay during the "device freeze"
> > > > phase of hibernation on my test-bed HP nx6325. Due to the 100%
> > > > reproducibility of it I was able to narrow it down to
> > > > radeon_suspend_kms() and then it turned out that the delay occured
> > > > somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
> > > > necessary or even very useful to me to evict VRAM at this particular
> > > > point, because we're going to create an image and bring the device
> > > > back to the fully functional state in a little while. Thus, I think
> > > > the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
> > > > which makes the delay go away.
> > >
> > > I'm not 100% sure of the hibernate sequencing and its early in the
> > > morning, but we want to evict VRAM before image building so we can have
> > > the contents of VRAM in the image so we can restore them on resume. Does
> > > this just avoid evicting them a second time after we created the image?
> >
> > No, it's the first time, before creating the image, but I didn't seen any
> > difference on resume with and without the patch, so I thought it was a good
> > idea. :-)
>
> On the machine you have its most likely not going to show up unless you
> are running a 3D app or something across suspend, since currently X
> re-exposes most apps on VT switch, so they just redraw.
>
> Was it always this slow? you can see how many objects are in vram using
> debugfs (/sys/kernel/debug/dri/0/radeon_vram_mm),
It says there are 32768 objects and about 9000 of them are in use.
> it sounds like the TTM eviction process is blocking on something,
ttm_bo_force_list_clean() blocks for more than 5 seconds in some iterations.
Rafael
On Saturday, June 19, 2010, Rafael J. Wysocki wrote:
> On Saturday, June 19, 2010, Dave Airlie wrote:
> > On Sat, 2010-06-19 at 01:23 +0200, Rafael J. Wysocki wrote:
> > > On Friday, June 18, 2010, Dave Airlie wrote:
> > > > On Fri, 2010-06-18 at 22:21 +0200, Rafael J. Wysocki wrote:
> > > > > From: Rafael J. Wysocki <[email protected]>
> > > > >
> > > > > I have recently noticed a 55 sec. delay during the "device freeze"
> > > > > phase of hibernation on my test-bed HP nx6325. Due to the 100%
> > > > > reproducibility of it I was able to narrow it down to
> > > > > radeon_suspend_kms() and then it turned out that the delay occured
> > > > > somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
> > > > > necessary or even very useful to me to evict VRAM at this particular
> > > > > point, because we're going to create an image and bring the device
> > > > > back to the fully functional state in a little while. Thus, I think
> > > > > the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
> > > > > which makes the delay go away.
> > > >
> > > > I'm not 100% sure of the hibernate sequencing and its early in the
> > > > morning, but we want to evict VRAM before image building so we can have
> > > > the contents of VRAM in the image so we can restore them on resume. Does
> > > > this just avoid evicting them a second time after we created the image?
> > >
> > > No, it's the first time, before creating the image, but I didn't seen any
> > > difference on resume with and without the patch, so I thought it was a good
> > > idea. :-)
> >
> > On the machine you have its most likely not going to show up unless you
> > are running a 3D app or something across suspend, since currently X
> > re-exposes most apps on VT switch, so they just redraw.
>
> Yes. Moreover, hibernation is always done after a VT switch. That's why
> I said I thought the eviction wasn't necessary in the changelog.
>
> BTW, I have three different test boxes with radeon hardware and the
> $subject patch is not a problem on any of them.
>
> > Was it always this slow?
>
> Nope. It definitely is a regression, although I'm not sure what's the last
> good kernel.
>
> > you can see how many objects are in vram using
> > debugfs (/sys/kernel/debug/dri/0/radeon_vram_mm), it sounds like the TTM
> > eviction process is blocking on something,
I did some more debug work (the _total_ lack of comments inside of the
relevant radeon and ttm code makes this a next-to-impossible task, though)
and found that all of the delays (up to 5 seconds) happen inside of
ttm_bo_move_accel_cleanup() called from radeon_move_blit(), where the "new"
memory type is TTL_PL_TT and the "old" one is TTL_PL_VRAM. The preceding
radeon_copy() always returns 0.
Please let me know if you need more information.
Thanks,
Rafael
On Sun, Jun 20, 2010 at 01:43:05AM +0200, Rafael J. Wysocki wrote:
> On Saturday, June 19, 2010, Rafael J. Wysocki wrote:
> > On Saturday, June 19, 2010, Dave Airlie wrote:
> > > On Sat, 2010-06-19 at 01:23 +0200, Rafael J. Wysocki wrote:
> > > > On Friday, June 18, 2010, Dave Airlie wrote:
> > > > > On Fri, 2010-06-18 at 22:21 +0200, Rafael J. Wysocki wrote:
> > > > > > From: Rafael J. Wysocki <[email protected]>
> > > > > >
> > > > > > I have recently noticed a 55 sec. delay during the "device freeze"
> > > > > > phase of hibernation on my test-bed HP nx6325. Due to the 100%
> > > > > > reproducibility of it I was able to narrow it down to
> > > > > > radeon_suspend_kms() and then it turned out that the delay occured
> > > > > > somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
> > > > > > necessary or even very useful to me to evict VRAM at this particular
> > > > > > point, because we're going to create an image and bring the device
> > > > > > back to the fully functional state in a little while. Thus, I think
> > > > > > the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
> > > > > > which makes the delay go away.
> > > > >
> > > > > I'm not 100% sure of the hibernate sequencing and its early in the
> > > > > morning, but we want to evict VRAM before image building so we can have
> > > > > the contents of VRAM in the image so we can restore them on resume. Does
> > > > > this just avoid evicting them a second time after we created the image?
> > > >
> > > > No, it's the first time, before creating the image, but I didn't seen any
> > > > difference on resume with and without the patch, so I thought it was a good
> > > > idea. :-)
> > >
> > > On the machine you have its most likely not going to show up unless you
> > > are running a 3D app or something across suspend, since currently X
> > > re-exposes most apps on VT switch, so they just redraw.
> >
> > Yes. Moreover, hibernation is always done after a VT switch. That's why
> > I said I thought the eviction wasn't necessary in the changelog.
> >
> > BTW, I have three different test boxes with radeon hardware and the
> > $subject patch is not a problem on any of them.
> >
> > > Was it always this slow?
> >
> > Nope. It definitely is a regression, although I'm not sure what's the last
> > good kernel.
> >
> > > you can see how many objects are in vram using
> > > debugfs (/sys/kernel/debug/dri/0/radeon_vram_mm), it sounds like the TTM
> > > eviction process is blocking on something,
>
> I did some more debug work (the _total_ lack of comments inside of the
> relevant radeon and ttm code makes this a next-to-impossible task, though)
> and found that all of the delays (up to 5 seconds) happen inside of
> ttm_bo_move_accel_cleanup() called from radeon_move_blit(), where the "new"
> memory type is TTL_PL_TT and the "old" one is TTL_PL_VRAM. The preceding
> radeon_copy() always returns 0.
>
> Please let me know if you need more information.
>
> Thanks,
> Rafael
Can you confirm that this is trigger by first radeon_bo_evict_vram in
radeon_suspend_kms() ? Also can you check if irq is enabled (put some
debug in the irq handler of your gpu). My guess is that irq are stop
(likely stop before radeon suspend callback) and that we endup waiting
that the fence timeout expire in radeon_fence_wait().
Cheers,
Jerome
On Monday, June 21, 2010, Jerome Glisse wrote:
> On Sun, Jun 20, 2010 at 01:43:05AM +0200, Rafael J. Wysocki wrote:
> > On Saturday, June 19, 2010, Rafael J. Wysocki wrote:
> > > On Saturday, June 19, 2010, Dave Airlie wrote:
> > > > On Sat, 2010-06-19 at 01:23 +0200, Rafael J. Wysocki wrote:
> > > > > On Friday, June 18, 2010, Dave Airlie wrote:
> > > > > > On Fri, 2010-06-18 at 22:21 +0200, Rafael J. Wysocki wrote:
> > > > > > > From: Rafael J. Wysocki <[email protected]>
> > > > > > >
> > > > > > > I have recently noticed a 55 sec. delay during the "device freeze"
> > > > > > > phase of hibernation on my test-bed HP nx6325. Due to the 100%
> > > > > > > reproducibility of it I was able to narrow it down to
> > > > > > > radeon_suspend_kms() and then it turned out that the delay occured
> > > > > > > somewhere in radeon_bo_evict_vram(). However, it doesn't seem really
> > > > > > > necessary or even very useful to me to evict VRAM at this particular
> > > > > > > point, because we're going to create an image and bring the device
> > > > > > > back to the fully functional state in a little while. Thus, I think
> > > > > > > the VRAM evicition can be skipped for state.event == PM_EVENT_FREEZE,
> > > > > > > which makes the delay go away.
> > > > > >
> > > > > > I'm not 100% sure of the hibernate sequencing and its early in the
> > > > > > morning, but we want to evict VRAM before image building so we can have
> > > > > > the contents of VRAM in the image so we can restore them on resume. Does
> > > > > > this just avoid evicting them a second time after we created the image?
> > > > >
> > > > > No, it's the first time, before creating the image, but I didn't seen any
> > > > > difference on resume with and without the patch, so I thought it was a good
> > > > > idea. :-)
> > > >
> > > > On the machine you have its most likely not going to show up unless you
> > > > are running a 3D app or something across suspend, since currently X
> > > > re-exposes most apps on VT switch, so they just redraw.
> > >
> > > Yes. Moreover, hibernation is always done after a VT switch. That's why
> > > I said I thought the eviction wasn't necessary in the changelog.
> > >
> > > BTW, I have three different test boxes with radeon hardware and the
> > > $subject patch is not a problem on any of them.
> > >
> > > > Was it always this slow?
> > >
> > > Nope. It definitely is a regression, although I'm not sure what's the last
> > > good kernel.
> > >
> > > > you can see how many objects are in vram using
> > > > debugfs (/sys/kernel/debug/dri/0/radeon_vram_mm), it sounds like the TTM
> > > > eviction process is blocking on something,
> >
> > I did some more debug work (the _total_ lack of comments inside of the
> > relevant radeon and ttm code makes this a next-to-impossible task, though)
> > and found that all of the delays (up to 5 seconds) happen inside of
> > ttm_bo_move_accel_cleanup() called from radeon_move_blit(), where the "new"
> > memory type is TTL_PL_TT and the "old" one is TTL_PL_VRAM. The preceding
> > radeon_copy() always returns 0.
> >
> > Please let me know if you need more information.
> >
> > Thanks,
> > Rafael
>
> Can you confirm that this is trigger by first radeon_bo_evict_vram in
> radeon_suspend_kms() ?
Not really.
I used the attached debug patch and I got the attached dmesg output from
a "core" hibernate test.
It looks like the first one is relatively sane (71 usecs), but things get worse
going forward.
> Also can you check if irq is enabled (put some
> debug in the irq handler of your gpu). My guess is that irq are stop
> (likely stop before radeon suspend callback)
No, interrupts are not switched off at this point yet. At least not
permanently.
> and that we endup waiting that the fence timeout expire in radeon_fence_wait().
I guess something like this happens, although I'm not sure about the root
cause.
It looks like it interferes with something happening in parallel with it.
I wonder, however, why it is a problem for hibernation and it's not a problem
for suspend to RAM and why the other machines are not affected.
Rafael