| Message ID | 20231127145437.15060-3-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/ttm: replace busy placement with flags v3 | expand |
On 11/27/23 09:54, Christian König wrote: > Try to fill up VRAM as well by setting the busy flag on GTT allocations. > > This fixes the issue that when VRAM was evacuated for suspend it's never > filled up again unless the application is restarted. > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2893 > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index aa0dd6dad068..ddc8fb4db678 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain) > abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ? > AMDGPU_PL_PREEMPT : TTM_PL_TT; > places[c].flags = 0; > + /* > + * When GTT is just an alternative to VRAM make sure that we > + * only use it as fallback and still try to fill up VRAM first. > + */ > + if (domain & AMDGPU_GEM_DOMAIN_VRAM) > + places[c].flags |= TTM_PL_FLAG_BUSY; > c++; > } >
[AMD Official Use Only - General] -----Original Message----- From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Hamza Mahfooz Sent: Monday, November 27, 2023 10:53 AM To: Christian König <ckoenig.leichtzumerken@gmail.com>; jani.nikula@linux.intel.com; kherbst@redhat.com; dakr@redhat.com; zackr@vmware.com; Olsak, Marek <Marek.Olsak@amd.com>; linux-graphics-maintainer@vmware.com; amd-gfx@lists.freedesktop.org; nouveau@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; virtualization@lists.linux.dev; spice-devel@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH 2/2] drm/amdgpu: use GTT only as fallback for VRAM|GTT On 11/27/23 09:54, Christian König wrote: > Try to fill up VRAM as well by setting the busy flag on GTT allocations. > > This fixes the issue that when VRAM was evacuated for suspend it's > never filled up again unless the application is restarted. I found the subject description a bit misleading. Maybe use a Fixes tag describing it is a fix for suspend resume regression other than that, looks good to me. Acked-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com> > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2893 > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index aa0dd6dad068..ddc8fb4db678 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain) > abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ? > AMDGPU_PL_PREEMPT : TTM_PL_TT; > places[c].flags = 0; > + /* > + * When GTT is just an alternative to VRAM make sure that we > + * only use it as fallback and still try to fill up VRAM first. > + */ > + if (domain & AMDGPU_GEM_DOMAIN_VRAM) > + places[c].flags |= TTM_PL_FLAG_BUSY; > c++; > } > -- Hamza
Am 27.11.23 um 17:47 schrieb Bhardwaj, Rajneesh: > [AMD Official Use Only - General] > > -----Original Message----- > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Hamza Mahfooz > Sent: Monday, November 27, 2023 10:53 AM > To: Christian König <ckoenig.leichtzumerken@gmail.com>; jani.nikula@linux.intel.com; kherbst@redhat.com; dakr@redhat.com; zackr@vmware.com; Olsak, Marek <Marek.Olsak@amd.com>; linux-graphics-maintainer@vmware.com; amd-gfx@lists.freedesktop.org; nouveau@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; virtualization@lists.linux.dev; spice-devel@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Subject: Re: [PATCH 2/2] drm/amdgpu: use GTT only as fallback for VRAM|GTT > > On 11/27/23 09:54, Christian König wrote: >> Try to fill up VRAM as well by setting the busy flag on GTT allocations. >> >> This fixes the issue that when VRAM was evacuated for suspend it's >> never filled up again unless the application is restarted. > I found the subject description a bit misleading. Maybe use a Fixes tag describing it is a fix for suspend resume regression other than that, looks good to me. Well exactly that's the problem, this isn't really a fix and we also don't want to backport it. Basically the previous behavior was working as design, it's just that it was never intended to be used like this. > > Acked-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@amd.com> > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2893 Thanks, Christian. > >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> index aa0dd6dad068..ddc8fb4db678 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >> @@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain) >> abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ? >> AMDGPU_PL_PREEMPT : TTM_PL_TT; >> places[c].flags = 0; >> + /* >> + * When GTT is just an alternative to VRAM make sure that we >> + * only use it as fallback and still try to fill up VRAM first. >> + */ >> + if (domain & AMDGPU_GEM_DOMAIN_VRAM) >> + places[c].flags |= TTM_PL_FLAG_BUSY; >> c++; >> } >> > -- > Hamza >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index aa0dd6dad068..ddc8fb4db678 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain) abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ? AMDGPU_PL_PREEMPT : TTM_PL_TT; places[c].flags = 0; + /* + * When GTT is just an alternative to VRAM make sure that we + * only use it as fallback and still try to fill up VRAM first. + */ + if (domain & AMDGPU_GEM_DOMAIN_VRAM) + places[c].flags |= TTM_PL_FLAG_BUSY; c++; }
Try to fill up VRAM as well by setting the busy flag on GTT allocations. This fixes the issue that when VRAM was evacuated for suspend it's never filled up again unless the application is restarted. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++++++ 1 file changed, 6 insertions(+)