-----Original Message----- From: Christian König [mailto:ckoenig.leichtzumerken@xxxxxxxxx] Sent: Monday, October 09, 2017 5:11 AM To: Kasiviswanathan, Harish <Harish.Kasiviswanathan at amd.com>; amd-gfx at lists.freedesktop.org Subject: Re: [PATCH v2 2/2] drm/amdgpu: Add amdgpu_find_mm_node() Am 07.10.2017 um 00:08 schrieb Harish Kasiviswanathan: > Change-Id: I12231e18bb60152843cd0e0213ddd0d0e04e7497 > Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan at amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 36 +++++++++++++++++++-------------- > 1 file changed, 21 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index dda4f08..e9bfe9d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -290,6 +290,23 @@ static uint64_t amdgpu_mm_node_addr(struct ttm_buffer_object *bo, > } > > /** > + * amdgpu_find_mm_node - Helper function finds the drm_mm_node > + * corresponding to @offset. It also modifies the offset to be > + * within the drm_mm_node returned > + */ > +static struct drm_mm_node *amdgpu_find_mm_node(struct ttm_mem_reg *mem, > + unsigned long *offset) > +{ > + struct drm_mm_node *mm_node = mem->mm_node; > + > + while (*offset >= (mm_node->size << PAGE_SHIFT)) { > + *offset -= (mm_node->size << PAGE_SHIFT); > + ++mm_node; > + } > + return mm_node; > +} Please take a look at amdgpu_ttm_io_mem_pfn(). There we assume that each node has the same size and do: > page_offset = do_div(offset, size); > mm += offset; Not 100% sure if that is a good idea, but when we add a common function for this we should use it everywhere. [HK]: Ok I missed this one, probably because it was used in a different way. I will change this function to call amdgpu_find_mm_node(). The optimization (assuming all nodes are same size) will work in the current setup but as you mentioned I am also not sure if it is a good idea. To be safe I will be use the while() loop for now. Regards, Christian. > + > +/** > * amdgpu_copy_ttm_mem_to_mem - Helper function for copy > * > * The function copies @size bytes from {src->mem + src->offset} to > @@ -319,21 +336,13 @@ int amdgpu_copy_ttm_mem_to_mem(struct amdgpu_device *adev, > return -EINVAL; > } > > - src_mm = src->mem->mm_node; > - while (src->offset >= (src_mm->size << PAGE_SHIFT)) { > - src->offset -= (src_mm->size << PAGE_SHIFT); > - ++src_mm; > - } > + src_mm = amdgpu_find_mm_node(src->mem, &src->offset); > src_node_start = amdgpu_mm_node_addr(src->bo, src_mm, src->mem) + > src->offset; > src_node_size = (src_mm->size << PAGE_SHIFT) - src->offset; > src_page_offset = src_node_start & (PAGE_SIZE - 1); > > - dst_mm = dst->mem->mm_node; > - while (dst->offset >= (dst_mm->size << PAGE_SHIFT)) { > - dst->offset -= (dst_mm->size << PAGE_SHIFT); > - ++dst_mm; > - } > + dst_mm = amdgpu_find_mm_node(dst->mem, &dst->offset); > dst_node_start = amdgpu_mm_node_addr(dst->bo, dst_mm, dst->mem) + > dst->offset; > dst_node_size = (dst_mm->size << PAGE_SHIFT) - dst->offset; > @@ -1216,7 +1225,7 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo, > { > struct amdgpu_bo *abo = container_of(bo, struct amdgpu_bo, tbo); > struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev); > - struct drm_mm_node *nodes = abo->tbo.mem.mm_node; > + struct drm_mm_node *nodes; > uint32_t value = 0; > int ret = 0; > uint64_t pos; > @@ -1225,10 +1234,7 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo, > if (bo->mem.mem_type != TTM_PL_VRAM) > return -EIO; > > - while (offset >= (nodes->size << PAGE_SHIFT)) { > - offset -= nodes->size << PAGE_SHIFT; > - ++nodes; > - } > + nodes = amdgpu_find_mm_node(&abo->tbo.mem, &offset); > pos = (nodes->start << PAGE_SHIFT) + offset; > > while (len && pos < adev->mc.mc_vram_size) {
