[AMD Official Use Only - AMD Internal Distribution Only]
Sure. Will change to AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT.
-----Original Message-----
From: Koenig, Christian <christian.koe...@amd.com>
Sent: Wednesday, August 20, 2025 5:38 PM
To: Zhang, Yifan <yifan1.zh...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <alexander.deuc...@amd.com>
Subject: Re: [PATCH] drm/amdgpu: replace AMDGPU_HAS_VRAM with is_app_apu
On 20.08.25 11:36, Zhang, Yifan wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> RE
> "The only reason I could come up with why we would need it is to have error
> handling if not enough VRAM is available for the TMR, but falling back to GTT
> is then probably still a good idea."
>
> TMR allocation only happens in psp_hw_init, suppose there is enough VRAM for
> TMR at that moment. Right ?
Yeah, that's exactly my thinking as well. There is most likely enough VRAM in
that situation.
Regards,
Christian.
>
>
> -----Original Message-----
> From: Koenig, Christian <christian.koe...@amd.com>
> Sent: Wednesday, August 20, 2025 4:14 PM
> To: Zhang, Yifan <yifan1.zh...@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <alexander.deuc...@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: replace AMDGPU_HAS_VRAM with
> is_app_apu
>
> On 20.08.25 09:31, Yifan Zhang wrote:
>> AMDGPU_HAS_VRAM is redundant with is_app_apu, as both refer to APUs
>> with no carve-out. Since AMDGPU_HAS_VRAM only occurs once, replace
>> AMDGPU_HAS_VRAM with is_app_apu.
>>
>> Signed-off-by: Yifan Zhang <yifan1.zh...@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 6 ------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 6 +++---
>> 2 files changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index ddd472e56f69..01f53700694b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -946,12 +946,6 @@ enum amdgpu_enforce_isolation_mode {
>> AMDGPU_ENFORCE_ISOLATION_NO_CLEANER_SHADER = 3, };
>>
>> -
>> -/*
>> - * Non-zero (true) if the GPU has VRAM. Zero (false) otherwise.
>> - */
>> -#define AMDGPU_HAS_VRAM(_adev) ((_adev)->gmc.real_vram_size)
>> -
>> struct amdgpu_device {
>> struct device *dev;
>> struct pci_dev *pdev;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> index fa3e55700ad6..4125e73a0647 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> @@ -880,9 +880,9 @@ static int psp_tmr_init(struct psp_context *psp)
>> pptr = amdgpu_sriov_vf(psp->adev) ? &tmr_buf : NULL;
>> ret = amdgpu_bo_create_kernel(psp->adev, tmr_size,
>> PSP_TMR_ALIGNMENT,
>> - AMDGPU_HAS_VRAM(psp->adev) ?
>> - AMDGPU_GEM_DOMAIN_VRAM :
>> - AMDGPU_GEM_DOMAIN_GTT,
>> + psp->adev->gmc.is_app_apu ?
>> + AMDGPU_GEM_DOMAIN_GTT :
>> + AMDGPU_GEM_DOMAIN_VRAM,
>
> Mhm the logic here is actually completely unnecessary. You can just specify
> AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT and get VRAM if available and
> GTT otherwise.
>
> The only reason I could come up with why we would need it is to have error
> handling if not enough VRAM is available for the TMR, but falling back to GTT
> is then probably still a good idea.
>
> Regards,
> Christian.
>
>
>> &psp->tmr_bo, &psp->tmr_mc_addr,
>> pptr);
>> }
>
