[PATCH] drm/amdgpu: try allowed domain when pin framebuffer failed

[Zhang, Yifan]

[AMD Official Use Only - General]

Hi Alex,

We need to adjust the AMDGPU_SG_THRESHOLD as well since recent APUs are configured w/ 512MB VRAM. Pls check attached patch.

Best Regards,
Yifan

-----Original Message-----
From: Alex Deucher <alexdeucher at gmail.com> 
Sent: Thursday, December 8, 2022 12:21 AM
To: Christian König <ckoenig.leichtzumerken at gmail.com>
Cc: Zhang, Jesse(Jie) <Jesse.Zhang at amd.com>; Zhang, Yifan <Yifan1.Zhang at amd.com>; amd-gfx <amd-gfx-bounces at lists.freedesktop.org>; Paneer Selvam, Arunpravin <Arunpravin.PaneerSelvam at amd.com>; amd-gfx at lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>
Subject: Re: [PATCH] drm/amdgpu: try allowed domain when pin framebuffer failed

On Wed, Dec 7, 2022 at 11:10 AM Christian König <ckoenig.leichtzumerken at gmail.com> wrote:
>
>
>
> Am 07.12.22 um 17:08 schrieb Alex Deucher:
> > On Wed, Dec 7, 2022 at 10:52 AM Christian König 
> > <ckoenig.leichtzumerken at gmail.com> wrote:
> >> Am 07.12.22 um 16:38 schrieb Alex Deucher:
> >>> On Wed, Dec 7, 2022 at 10:23 AM Christian König 
> >>> <ckoenig.leichtzumerken at gmail.com> wrote:
> >>>> I would go a step further and just allow GTT domain on ASICs != 
> >>>> CARRIZO
> >>>> | STONEY.
> >>>>
> >>>> I can't see a good reason we should still have any limitation 
> >>>> here, VRAM doesn't have any advantage any more as far as I know.
> >>> Well, if VRAM is available we want to make sure someone uses it 
> >>> otherwise it's just wasted.
> >> Well it still gets used when it's free. So now problem at all here.
> >>
> >> We should just not force anything into VRAM or GTT any more if it's 
> >> technically not necessary.
> > So just this?
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > index 919bbea2e3ac..8e8f07fa7a93 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> > @@ -1506,7 +1506,7 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
> >   uint32_t amdgpu_bo_get_preferred_domain(struct amdgpu_device *adev,
> >                                              uint32_t domain)
> >   {
> > -       if (domain == (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT)) {
>
> We still need to keep this check to avoid trying to adjust VRAM only 
> allocations (the cursor still needs this IIRC).
>
> Apart from that I think that should work.

Attached.  Thanks,

Alex

>
> Christian.
>
> > +       if ((adev->asic_type == CHIP_CARRIZO) || (adev->asic_type ==
> > CHIP_STONEY)) {
> >                  domain = AMDGPU_GEM_DOMAIN_VRAM;
> >                  if (adev->gmc.real_vram_size <= AMDGPU_SG_THRESHOLD)
> >                          domain = AMDGPU_GEM_DOMAIN_GTT;
> >
> >
> >
> >> Christian.
> >>
> >>> Alex
> >>>
> >>>
> >>>> Christian.
> >>>>
> >>>> Am 07.12.22 um 16:10 schrieb Alex Deucher:
> >>>>> Does this patch fix the problem?
> >>>>>
> >>>>> Alex
> >>>>>
> >>>>> On Wed, Dec 7, 2022 at 2:27 AM Zhang, Jesse(Jie) <Jesse.Zhang at amd.com> wrote:
> >>>>>> [AMD Official Use Only - General]
> >>>>>>
> >>>>>>
> >>>>>>        drm/amdgpu: try allowed domain when pin framebuffer failed.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>        [WHY&HOW]
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>        in some scenarios, the allocate memory often failed. such as do hot plug or play games.
> >>>>>>
> >>>>>>        so we can try allowed domain, if the preferred domain cannot allocate memory.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>        Signed-off-by: jie1zhan jesse.zhang at amd.com
> >>>>>>
> >>>>>>        Change-Id: I4b62e2ff072d02c515f901000a5789339d481273
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> >>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >>>>>>
> >>>>>> index 1ae0c8723348..05fcaf7f9d92 100644
> >>>>>>
> >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >>>>>>
> >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >>>>>>
> >>>>>> @@ -39,6 +39,7 @@
> >>>>>>
> >>>>>> #include "amdgpu.h"
> >>>>>>
> >>>>>> #include "amdgpu_trace.h"
> >>>>>>
> >>>>>> #include "amdgpu_amdkfd.h"
> >>>>>>
> >>>>>> +#include "amdgpu_display.h"
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> /**
> >>>>>>
> >>>>>>      * DOC: amdgpu_object
> >>>>>>
> >>>>>> @@ -942,8 +943,14 @@ int amdgpu_bo_pin_restricted(struct 
> >>>>>> amdgpu_bo *bo, u32 domain,
> >>>>>>
> >>>>>>                            bo->placements[i].lpfn = lpfn;
> >>>>>>
> >>>>>>            }
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> +       retry:
> >>>>>>
> >>>>>>            r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> >>>>>>
> >>>>>>            if (unlikely(r)) {
> >>>>>>
> >>>>>> +               //try allowed domain when pin failed. just a workaround.
> >>>>>>
> >>>>>> +               if (unlikely(r == -ENOMEM) && domain != 
> >>>>>> + bo->allowed_domains) {
> >>>>>>
> >>>>>> +                       amdgpu_bo_placement_from_domain(bo, 
> >>>>>> + bo->allowed_domains);
> >>>>>>
> >>>>>> +                       goto retry;
> >>>>>>
> >>>>>> +               }
> >>>>>>
> >>>>>>                    dev_err(adev->dev, "%p pin failed\n", bo);
> >>>>>>
> >>>>>>                    goto error;
> >>>>>>
> >>>>>>            }
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-amdgpu-make-display-pinning-more-flexible-v3.patch
Type: application/octet-stream
Size: 2109 bytes
Desc: 0001-drm-amdgpu-make-display-pinning-more-flexible-v3.patch
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20221208/2d7c8371/attachment-0001.obj>