[PATCH] drm/amdgpu: Do not evict VRAM on APUs with disabled HIBERNATE

[Huang Rui]

On Wed, Aug 01, 2018 at 02:59:13PM +0800, Christian König wrote:
> Am 01.08.2018 um 09:03 schrieb Huang Rui:
> > On Tue, Jul 31, 2018 at 06:46:04PM +0200, Paul Menzel wrote:
> >> From: Paul Menzel <pmenzel at molgen.mpg.de>
> >> Date: Wed, 25 Jul 2018 12:54:19 +0200
> >>
> >> Improve commit d796d844 (drm/radeon/kms: make hibernate work on IGPs) to
> >> only migrate VRAM objects if the Linux kernel is actually built with
> >> support for hibernation (suspend to disk).
> >>
> >> The better solution is to get the information, if this is suspend or
> >> hibernate, from `amdgpu_device_suspend()`, but that’s more involved, so
> >> apply the simple solution first.
> >>
> >> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107277
> >> Cc: stable at vger.kernel.org
> >> Signed-off-by: Paul Menzel <pmenzel at molgen.mpg.de>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> index 3526efa8960e..a27a62980bb7 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> @@ -811,10 +811,12 @@ int amdgpu_bo_unpin(struct amdgpu_bo *bo)
> >>   int amdgpu_bo_evict_vram(struct amdgpu_device *adev)
> >>   {
> >>   	/* late 2.6.33 fix IGP hibernate - we need pm ops to do this correct */
> >> -	if (0 && (adev->flags & AMD_IS_APU)) {
> >> +#ifndef CONFIG_HIBERNATION
> >> +	if (adev->flags & AMD_IS_APU) {
> >>   		/* Useless to evict on IGP chips */
> >>   		return 0;
> >>   	}
> >> +#endif
> > Actually, when system enters into suspend, the vram will be cleared
> > (because of poweroff), so we need to evict them into system memory before
> > that. However, in APU, the vram is actually carved out from system memory,
> > so we needn't evict them in theory.
> >
> > Whatever hibernation or suspend, we actually follow the process. I don't
> > see the usage that you added CONFIG_HIBERNATION here. Alex, what do you
> > think?
> 
> The problem is even on APUs we need to safe VRAM on hibernation because 
> the OS doesn't know that it is just stolen system memory.

OK, I think I understand it. Even the vram is stolen system memory, but OS
won't treat it as the system memory, and actually do not save the vram to
disk like the system memory that OS identified in hibernation. So we have
to evict it in hibernation.

If yes, I think this patch should be OK.

> 
> The problem is Paul doesn't address the real underlying issue, but 
> instead just mitigating it by stopping VRAM eviction when hibernation 
> isn't compiled into the kernel.

Yes. I also think this is probably not the root cause.

> 
> The correct way would be to detect if we are suspending or hibernating 
> and evict VRAM based on that information.
> 

Agree, that make sense.

Thanks,
Ray