Re: [PATCH 4/7] drm/ttm: move the page_alignment into the BO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 14 Apr 2021 at 10:57, Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> Am 14.04.21 um 11:46 schrieb Matthew Auld:
> > On Tue, 13 Apr 2021 at 14:53, Christian König
> > <ckoenig.leichtzumerken@xxxxxxxxx> wrote:
> >> The alignment is a constant property and shouldn't change.
> >>
> >> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> > What is page alignment here? Is it just for HW restrictions, say if it
> > requires 64K pages with the same physical alignment for VRAM or
> > something? But then wouldn't it make more sense for that to remain as
> > a property of the resource, and not the object? Or am I
> > misunderstanding something?
>
> The page_alignment (bad name btw) is the physical base alignment of the
> allocation.
>
> I want to make resource allocation optional in the mid term, and this is
> the only information we currently don't have otherwise.
>
> The most sense would it make in the placement, since it is really an
> allocation restriction. But I would need to rework the placement
> handling in amdgpu for this to be consistent first.
>
> Regards,
> Christian.
>
> >
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c      |  2 +-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  |  2 +-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h   |  2 +-
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c |  5 +++--
> >>   drivers/gpu/drm/radeon/radeon_object.h       |  2 +-
> >>   drivers/gpu/drm/ttm/ttm_bo.c                 |  3 +--
> >>   drivers/gpu/drm/ttm/ttm_range_manager.c      |  5 ++---
> >>   drivers/gpu/drm/vmwgfx/vmwgfx_thp.c          | 15 ++++++++-------
> >>   include/drm/ttm/ttm_bo_api.h                 |  1 +
> >>   include/drm/ttm/ttm_resource.h               |  1 -
> >>   10 files changed, 19 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >> index b443907afcea..f1c397be383d 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >> @@ -763,7 +763,7 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
> >>                  void __user *out = u64_to_user_ptr(args->value);
> >>
> >>                  info.bo_size = robj->tbo.base.size;
> >> -               info.alignment = robj->tbo.mem.page_alignment << PAGE_SHIFT;
> >> +               info.alignment = robj->tbo.page_alignment << PAGE_SHIFT;
> >>                  info.domains = robj->preferred_domains;
> >>                  info.domain_flags = robj->flags;
> >>                  amdgpu_bo_unreserve(robj);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >> index 8860545344c7..c026972ca9a1 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> >> @@ -136,7 +136,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager *man,
> >>
> >>          spin_lock(&mgr->lock);
> >>          r = drm_mm_insert_node_in_range(&mgr->mm, &node->node, mem->num_pages,
> >> -                                       mem->page_alignment, 0, place->fpfn,
> >> +                                       tbo->page_alignment, 0, place->fpfn,
> >>                                          place->lpfn, DRM_MM_INSERT_BEST);
> >>          spin_unlock(&mgr->lock);
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> >> index 9ac37569823f..ae4a68db87c0 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> >> @@ -184,7 +184,7 @@ static inline unsigned amdgpu_bo_ngpu_pages(struct amdgpu_bo *bo)
> >>
> >>   static inline unsigned amdgpu_bo_gpu_page_alignment(struct amdgpu_bo *bo)
> >>   {
> >> -       return (bo->tbo.mem.page_alignment << PAGE_SHIFT) / AMDGPU_GPU_PAGE_SIZE;
> >> +       return (bo->tbo.page_alignment << PAGE_SHIFT) / AMDGPU_GPU_PAGE_SIZE;
> >>   }
> >>
> >>   /**
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> >> index 1fc7ec0b8915..38b1995d0d6c 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> >> @@ -392,7 +392,8 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> >>                  /* default to 2MB */
> >>                  pages_per_node = (2UL << (20UL - PAGE_SHIFT));
> >>   #endif
> >> -               pages_per_node = max((uint32_t)pages_per_node, mem->page_alignment);
> >> +               pages_per_node = max((uint32_t)pages_per_node,
> >> +                                    tbo->page_alignment);
> >>                  num_nodes = DIV_ROUND_UP(mem->num_pages, pages_per_node);
> >>          }
> >>
> >> @@ -431,7 +432,7 @@ static int amdgpu_vram_mgr_new(struct ttm_resource_manager *man,
> >>
> >>          for (; pages_left; ++i) {
> >>                  unsigned long pages = min(pages_left, pages_per_node);
> >> -               uint32_t alignment = mem->page_alignment;
> >> +               uint32_t alignment = tbo->page_alignment;
> >>
> >>                  if (pages == pages_per_node)
> >>                          alignment = pages_per_node;
> >> diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
> >> index 9896d8231fe5..fd4116bdde0f 100644
> >> --- a/drivers/gpu/drm/radeon/radeon_object.h
> >> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> >> @@ -119,7 +119,7 @@ static inline unsigned radeon_bo_ngpu_pages(struct radeon_bo *bo)
> >>
> >>   static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo)
> >>   {
> >> -       return (bo->tbo.mem.page_alignment << PAGE_SHIFT) / RADEON_GPU_PAGE_SIZE;
> >> +       return (bo->tbo.page_alignment << PAGE_SHIFT) / RADEON_GPU_PAGE_SIZE;
> >>   }
> >>
> >>   /**
> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> >> index cfd0b9292397..2efae620759a 100644
> >> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> >> @@ -903,7 +903,6 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo,
> >>          memset(&hop, 0, sizeof(hop));
> >>
> >>          mem.num_pages = PAGE_ALIGN(bo->base.size) >> PAGE_SHIFT;
> >> -       mem.page_alignment = bo->mem.page_alignment;
> >>          mem.bus.offset = 0;
> >>          mem.bus.addr = NULL;
> >>          mem.mm_node = NULL;
> >> @@ -1038,10 +1037,10 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
> >>          INIT_LIST_HEAD(&bo->ddestroy);
> >>          bo->bdev = bdev;
> >>          bo->type = type;
> >> +       bo->page_alignment = page_alignment;
> >>          bo->mem.mem_type = TTM_PL_SYSTEM;
> >>          bo->mem.num_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >>          bo->mem.mm_node = NULL;
> >> -       bo->mem.page_alignment = page_alignment;
> >>          bo->mem.bus.offset = 0;
> >>          bo->mem.bus.addr = NULL;
> >>          bo->moving = NULL;
> >> diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c b/drivers/gpu/drm/ttm/ttm_range_manager.c
> >> index b1e3f30f7e2d..b9d5da6e6a81 100644
> >> --- a/drivers/gpu/drm/ttm/ttm_range_manager.c
> >> +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
> >> @@ -79,9 +79,8 @@ static int ttm_range_man_alloc(struct ttm_resource_manager *man,
> >>                  mode = DRM_MM_INSERT_HIGH;
> >>
> >>          spin_lock(&rman->lock);
> >> -       ret = drm_mm_insert_node_in_range(mm, node,
> >> -                                         mem->num_pages,
> >> -                                         mem->page_alignment, 0,
> >> +       ret = drm_mm_insert_node_in_range(mm, node, mem->num_pages,
> >> +                                         bo->page_alignment, 0,
> >>                                            place->fpfn, lpfn, mode);
> >>          spin_unlock(&rman->lock);
> >>
> >> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
> >> index eb63cbe64909..5ccc35b3194c 100644
> >> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
> >> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c
> >> @@ -28,15 +28,16 @@ static struct vmw_thp_manager *to_thp_manager(struct ttm_resource_manager *man)
> >>
> >>   static const struct ttm_resource_manager_func vmw_thp_func;
> >>
> >> -static int vmw_thp_insert_aligned(struct drm_mm *mm, struct drm_mm_node *node,
> >> +static int vmw_thp_insert_aligned(struct ttm_buffer_object *bo,
> >> +                                 struct drm_mm *mm, struct drm_mm_node *node,
> >>                                    unsigned long align_pages,
> >>                                    const struct ttm_place *place,
> >>                                    struct ttm_resource *mem,
> >>                                    unsigned long lpfn,
> >>                                    enum drm_mm_insert_mode mode)
> >>   {
> >> -       if (align_pages >= mem->page_alignment &&
> >> -           (!mem->page_alignment || align_pages % mem->page_alignment == 0)) {
> >> +       if (align_pages >= bo->page_alignment &&
> >> +           (!bo->page_alignment || align_pages % bo->page_alignment == 0)) {
> >>                  return drm_mm_insert_node_in_range(mm, node,
> >>                                                     mem->num_pages,
> >>                                                     align_pages, 0,
> >> @@ -75,7 +76,7 @@ static int vmw_thp_get_node(struct ttm_resource_manager *man,
> >>          if (IS_ENABLED(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD)) {
> >>                  align_pages = (HPAGE_PUD_SIZE >> PAGE_SHIFT);
> >>                  if (mem->num_pages >= align_pages) {
> >> -                       ret = vmw_thp_insert_aligned(mm, node, align_pages,
> >> +                       ret = vmw_thp_insert_aligned(bo, mm, node, align_pages,
> >>                                                       place, mem, lpfn, mode);
> >>                          if (!ret)
> >>                                  goto found_unlock;
> >> @@ -84,14 +85,14 @@ static int vmw_thp_get_node(struct ttm_resource_manager *man,
> >>
> >>          align_pages = (HPAGE_PMD_SIZE >> PAGE_SHIFT);
> >>          if (mem->num_pages >= align_pages) {
> >> -               ret = vmw_thp_insert_aligned(mm, node, align_pages, place, mem,
> >> -                                            lpfn, mode);
> >> +               ret = vmw_thp_insert_aligned(bo, mm, node, align_pages, place,
> >> +                                            mem, lpfn, mode);
> >>                  if (!ret)
> >>                          goto found_unlock;
> >>          }
> >>
> >>          ret = drm_mm_insert_node_in_range(mm, node, mem->num_pages,
> >> -                                         mem->page_alignment, 0,
> >> +                                         bo->page_alignment, 0,
> >>                                            place->fpfn, lpfn, mode);
> >>   found_unlock:
> >>          spin_unlock(&rman->lock);
> >> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> >> index 3587f660e8f4..167c132ba1c2 100644
> >> --- a/include/drm/ttm/ttm_bo_api.h
> >> +++ b/include/drm/ttm/ttm_bo_api.h
> >> @@ -123,6 +123,7 @@ struct ttm_buffer_object {
> >>
> >>          struct ttm_device *bdev;
> >>          enum ttm_bo_type type;
> >> +       uint32_t page_alignment;
> >>          void (*destroy) (struct ttm_buffer_object *);
> >>
> >>          /**
> >> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> >> index 6164ccf4f308..3ff4a669641e 100644
> >> --- a/include/drm/ttm/ttm_resource.h
> >> +++ b/include/drm/ttm/ttm_resource.h
> >> @@ -172,7 +172,6 @@ struct ttm_resource {
> >>          void *mm_node;
> >>          unsigned long start;
> >>          unsigned long num_pages;
> >> -       uint32_t page_alignment;

The kernel doc for the @page_alignment should also be
removed/transplanted, although it does look like the kernel doc in
general is a bit stale anyway in this file.

Thanks for the above explanation,
Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>

> >>          uint32_t mem_type;
> >>          uint32_t placement;
> >>          struct ttm_bus_placement bus;
> >> --
> >> 2.25.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux