[PATCH v2] drm/amd: Disable S/G for APUs when 64GB or more host memory

[Mario Limonciello]

On 7/27/2023 13:02, Alex Deucher wrote:
> On Thu, Jul 27, 2023 at 1:29 PM Mario Limonciello
> <mario.limonciello at amd.com> 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 at amd.com>
>> Cc: Roman Li <roman.li at amd.com>
>> Cc: <stable at vger.kernel.org> # 6.1.y: bf0207e172703 ("drm/amdgpu: add S/G display parameter")
>> Cc: <stable at vger.kernel.org> # 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 at amd.com>
>> ---
>> 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.

> Assuming it does the right thing here:
> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> 
> 
>> +               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
>>