On Thu, Jul 27, 2023 at 2:14 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
>
> On 7/27/2023 13:02, Alex Deucher wrote:
> > On Thu, Jul 27, 2023 at 1:29 PM Mario Limonciello
> > <mario.limonciello@xxxxxxx> wrote:
> >>
> >> Users report a white flickering screen on multiple systems that
> >> is tied to having 64GB or more memory. When S/G is enabled pages
> >> will get pinned to both VRAM carve out and system RAM leading to
> >> this.
> >>
> >> Until it can be fixed properly, disable S/G when 64GB of memory or
> >> more is detected. This will force pages to be pinned into VRAM.
> >> This should fix white screen flickers but if VRAM pressure is
> >> encountered may lead to black screens. It's a trade-off for now.
> >>
> >> Fixes: 81d0bcf990093 ("drm/amdgpu: make display pinning more flexible (v2)")
> >> Cc: Hamza Mahfooz <Hamza.Mahfooz@xxxxxxx>
> >> Cc: Roman Li <roman.li@xxxxxxx>
> >> Cc: <stable@xxxxxxxxxxxxxxx> # 6.1.y: bf0207e172703 ("drm/amdgpu: add S/G display parameter")
> >> Cc: <stable@xxxxxxxxxxxxxxx> # 6.4.y
> >> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2735
> >> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2354
> >> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> >> ---
> >> v1->v2:
> >> * Fix updating mode_info as well
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 25 +++++++++++++++++++
> >> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++--
> >> 3 files changed, 28 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> index 93d0f4c7b560e..2e3c7c15cb8e3 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> >> @@ -1313,6 +1313,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
> >> void amdgpu_device_pci_config_reset(struct amdgpu_device *adev);
> >> int amdgpu_device_pci_reset(struct amdgpu_device *adev);
> >> bool amdgpu_device_need_post(struct amdgpu_device *adev);
> >> +bool amdgpu_sg_display_supported(struct amdgpu_device *adev);
> >> bool amdgpu_device_pcie_dynamic_switching_supported(void);
> >> bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
> >> bool amdgpu_device_aspm_support_quirk(void);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index dc0e5227119b1..a4e36b178d86c 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -1296,6 +1296,31 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev)
> >> return true;
> >> }
> >>
> >> +/*
> >> + * On APUs with >= 64GB white flickering has been observed w/ SG enabled.
> >> + * Disable S/G on such systems until we have a proper fix.
> >> + * https://gitlab.freedesktop.org/drm/amd/-/issues/2354
> >> + * https://gitlab.freedesktop.org/drm/amd/-/issues/2735
> >> + */
> >> +bool amdgpu_sg_display_supported(struct amdgpu_device *adev)
> >> +{
> >> + switch (amdgpu_sg_display) {
> >> + case -1:
> >> + break;
> >> + case 0:
> >> + return false;
> >> + case 1:
> >> + return true;
> >> + default:
> >> + return false;
> >> + }
> >> + if (totalram_pages() << (PAGE_SHIFT - 10) >= 64000000) {
> >
> > Does totalram_pages() return the amount of physical ram or the amount
> > of usable ram (i.e., minus carveout, firmware reservations, etc.)?
>
> It's a good question. I don't have a system w/ 64GB in front of me so I
> just double checked on a Phoenix system that has 16GB soldered down.
>
> It returns 15520248, so it's missing the VRAM carveout. amdgpu does
> know this already via adev->gmc.real_vram_size.
> So I think the logic should be:
>
> if ((totalram_pages() << (PAGE_SHIFT - 10)) + (adev->gmc.real_vram_size
> / 1024) >= 64000000)
>
> That should bring it to ~16GB.
Yeah, that's probably good enough.
Alex
>
> > Assuming it does the right thing here:
> > Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> >
> >
> >> + DRM_WARN("Disabling S/G due to >=64GB RAM\n");
> >> + return false;
> >> + }
> >> + return true;
> >> +}
> >> +
> >> /*
> >> * Intel hosts such as Raptor Lake and Sapphire Rapids don't support dynamic
> >> * speed switching. Until we have confirmation from Intel that a specific host
> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> index 77d970a2ee693..26c3eb7a9f882 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -1639,9 +1639,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
> >> }
> >> break;
> >> }
> >> - if (init_data.flags.gpu_vm_support &&
> >> - (amdgpu_sg_display == 0))
> >> - init_data.flags.gpu_vm_support = false;
> >> + if (init_data.flags.gpu_vm_support)
> >> + init_data.flags.gpu_vm_support = amdgpu_sg_display_supported(adev);
> >>
> >> if (init_data.flags.gpu_vm_support)
> >> adev->mode_info.gpu_vm_support = true;
> >> --
> >> 2.25.1
> >>
>
