On Wed, 11 Jan 2023 at 14:43, Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> Am 11.01.23 um 14:03 schrieb Matthew Auld:
> > On Wed, 11 Jan 2023 at 11:43, Christian König
> > <ckoenig.leichtzumerken@xxxxxxxxx> wrote:
> >> Instead of a list of separate busy placement add flags which indicate
> >> that a placement should only be used when there is room or if we need to
> >> evict.
> >>
> >> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 +-
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 +-
> >> drivers/gpu/drm/drm_gem_vram_helper.c | 2 -
> >> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 36 +++---
> >> drivers/gpu/drm/nouveau/nouveau_bo.c | 59 ++++------
> >> drivers/gpu/drm/nouveau/nouveau_bo.h | 1 -
> >> drivers/gpu/drm/qxl/qxl_object.c | 2 -
> >> drivers/gpu/drm/qxl/qxl_ttm.c | 2 -
> >> drivers/gpu/drm/radeon/radeon_object.c | 2 -
> >> drivers/gpu/drm/radeon/radeon_ttm.c | 8 +-
> >> drivers/gpu/drm/radeon/radeon_uvd.c | 1 -
> >> drivers/gpu/drm/ttm/ttm_bo.c | 21 ++--
> >> drivers/gpu/drm/ttm/ttm_resource.c | 73 +++----------
> >> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 2 -
> >> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 121 ++++++++++-----------
> >> include/drm/ttm/ttm_placement.h | 10 +-
> >> include/drm/ttm/ttm_resource.h | 8 +-
> >> 17 files changed, 134 insertions(+), 231 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> index 974e85d8b6cc..0995a2f41305 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >> @@ -201,9 +201,6 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
> >>
> >> placement->num_placement = c;
> >> placement->placement = places;
> >> -
> >> - placement->num_busy_placement = c;
> >> - placement->busy_placement = places;
> >> }
> >>
> >> /**
> >> @@ -1369,8 +1366,7 @@ vm_fault_t amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo)
> >> AMDGPU_GEM_DOMAIN_GTT);
> >>
> >> /* Avoid costly evictions; only set GTT as a busy placement */
> >> - abo->placement.num_busy_placement = 1;
> >> - abo->placement.busy_placement = &abo->placements[1];
> >> + abo->placements[0].flags |= TTM_PL_FLAG_IDLE;
> >>
> >> r = ttm_bo_validate(bo, &abo->placement, &ctx);
> >> if (unlikely(r == -EBUSY || r == -ERESTARTSYS))
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >> index 677cd7d91687..33cf6e835a68 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >> @@ -104,23 +104,19 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
> >> /* Don't handle scatter gather BOs */
> >> if (bo->type == ttm_bo_type_sg) {
> >> placement->num_placement = 0;
> >> - placement->num_busy_placement = 0;
> >> return;
> >> }
> >>
> >> /* Object isn't an AMDGPU object so ignore */
> >> if (!amdgpu_bo_is_amdgpu_bo(bo)) {
> >> placement->placement = &placements;
> >> - placement->busy_placement = &placements;
> >> placement->num_placement = 1;
> >> - placement->num_busy_placement = 1;
> >> return;
> >> }
> >>
> >> abo = ttm_to_amdgpu_bo(bo);
> >> if (abo->flags & AMDGPU_GEM_CREATE_DISCARDABLE) {
> >> placement->num_placement = 0;
> >> - placement->num_busy_placement = 0;
> >> return;
> >> }
> >>
> >> @@ -129,13 +125,13 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
> >> case AMDGPU_PL_GWS:
> >> case AMDGPU_PL_OA:
> >> placement->num_placement = 0;
> >> - placement->num_busy_placement = 0;
> >> return;
> >>
> >> case TTM_PL_VRAM:
> >> if (!adev->mman.buffer_funcs_enabled) {
> >> /* Move to system memory */
> >> amdgpu_bo_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_CPU);
> >> +
> >> } else if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
> >> !(abo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) &&
> >> amdgpu_bo_in_cpu_visible_vram(abo)) {
> >> @@ -150,8 +146,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
> >> AMDGPU_GEM_DOMAIN_CPU);
> >> abo->placements[0].fpfn = adev->gmc.visible_vram_size >> PAGE_SHIFT;
> >> abo->placements[0].lpfn = 0;
> >> - abo->placement.busy_placement = &abo->placements[1];
> >> - abo->placement.num_busy_placement = 1;
> >> + abo->placements[0].flags |= TTM_PL_FLAG_IDLE;
> >> } else {
> >> /* Move to GTT memory */
> >> amdgpu_bo_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_GTT |
> >> @@ -923,8 +918,6 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
> >> /* allocate GART space */
> >> placement.num_placement = 1;
> >> placement.placement = &placements;
> >> - placement.num_busy_placement = 1;
> >> - placement.busy_placement = &placements;
> >> placements.fpfn = 0;
> >> placements.lpfn = adev->gmc.gart_size >> PAGE_SHIFT;
> >> placements.mem_type = TTM_PL_TT;
> >> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> >> index d40b3edb52d0..f46792b757f9 100644
> >> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> >> @@ -147,7 +147,6 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
> >> invariant_flags = TTM_PL_FLAG_TOPDOWN;
> >>
> >> gbo->placement.placement = gbo->placements;
> >> - gbo->placement.busy_placement = gbo->placements;
> >>
> >> if (pl_flag & DRM_GEM_VRAM_PL_FLAG_VRAM) {
> >> gbo->placements[c].mem_type = TTM_PL_VRAM;
> >> @@ -160,7 +159,6 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
> >> }
> >>
> >> gbo->placement.num_placement = c;
> >> - gbo->placement.num_busy_placement = c;
> >>
> >> for (i = 0; i < c; ++i) {
> >> gbo->placements[i].fpfn = 0;
> >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> >> index d409a77449a3..dc483d601cf9 100644
> >> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> >> @@ -65,8 +65,6 @@ static const struct ttm_place sys_placement_flags = {
> >> static struct ttm_placement i915_sys_placement = {
> >> .num_placement = 1,
> >> .placement = &sys_placement_flags,
> >> - .num_busy_placement = 1,
> >> - .busy_placement = &sys_placement_flags,
> >> };
> >>
> >> /**
> >> @@ -154,32 +152,27 @@ i915_ttm_place_from_region(const struct intel_memory_region *mr,
> >>
> >> static void
> >> i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj,
> >> - struct ttm_place *requested,
> >> - struct ttm_place *busy,
> >> + struct ttm_place *places,
> >> struct ttm_placement *placement)
> >> {
> >> unsigned int num_allowed = obj->mm.n_placements;
> >> unsigned int flags = obj->flags;
> >> unsigned int i;
> >>
> >> - placement->num_placement = 1;
> >> i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] :
> >> - obj->mm.region, requested, obj->bo_offset,
> >> + obj->mm.region, &places[0], obj->bo_offset,
> >> obj->base.size, flags);
> > Do we also need places[0].flags |= TTM_PL_FLAG_IDLE somewhere here?
> >
> > Series doesn't seem to apply to drm-tip, so no intel-gfx CI. Would it
> > be possible to have a version that applies to drm-tip, just so we can
> > verify the i915 bits? We could send it to trybot[1] just to get some
> > CI results?
>
> Mpf, I was intentionally waiting with this for another cycle to be able
> to test it. Can you see why it doesn't apply?
Error: patch https://patchwork.freedesktop.org/api/1.0/series/112668/revisions/1/mbox/
not applied
Applying: drm/ttm: prevent moving of pinned BOs
Applying: drm/ttm: replace busy placement with flags
error: sha1 information is lacking or useless
(drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 drm/ttm: replace busy placement with flags
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
>
> Currently "dim rebuild-tip" doesn't work for me because of a conflict in
> i915_gem_execbuffer.c
Seems to be working now.
>
> Any idea how to fix this?
>
> Thanks,
> Christian.
>
>
> >
> > [1] https://patchwork.freedesktop.org/project/intel-gfx-trybot/series/?ordering=-last_updated
>
