Am 05.10.2017 um 23:56 schrieb Kasiviswanathan, Harish: > -----Original Message----- > From: Deucher, Alexander > Sent: Thursday, October 05, 2017 3:10 PM > To: Kasiviswanathan, Harish <Harish.Kasiviswanathan at amd.com>; amd-gfx at lists.freedesktop.org > Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan at amd.com> > Subject: RE: [PATCH] drm/amdgpu: Refactor amdgpu_move_blit > >> -----Original Message----- >> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf >> Of Harish Kasiviswanathan >> Sent: Thursday, October 05, 2017 2:30 PM >> To: amd-gfx at lists.freedesktop.org >> Cc: Kasiviswanathan, Harish >> Subject: [PATCH] drm/amdgpu: Refactor amdgpu_move_blit >> >> Add more generic function amdgpu_copy_ttm_mem_to_mem() that supports >> arbitrary copy size, offsets and two BOs (source & dest.). >> >> This is useful for KFD Cross Memory Attach feature where data needs to >> be copied from BOs from different processes >> >> Change-Id: I848d541a84a1c2d12827d9dcf6d9054d854b4159 >> Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan at amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 159 >> ++++++++++++++++++++------------ >> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 7 ++ >> 2 files changed, 107 insertions(+), 59 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> index 1086f03..e5415fc 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >> @@ -289,97 +289,138 @@ static uint64_t amdgpu_mm_node_addr(struct >> ttm_buffer_object *bo, >> return addr; >> } >> >> -static int amdgpu_move_blit(struct ttm_buffer_object *bo, >> - bool evict, bool no_wait_gpu, >> - struct ttm_mem_reg *new_mem, >> - struct ttm_mem_reg *old_mem) >> +/** >> + * amdgpu_copy_ttm_mem_to_mem - Helper function for copy >> + * >> + * @bo, @mem and @offset: All are array of 2 items. The function >> +copies >> @size >> + * bytes from {mem[0] + offset[0]} to {mem[1] + offset[1]}. bo[0] and >> +bo[1] >> + * could be same BO for a move and different for a BO to BO copy. >> + * >> + * @f: Returns the last fence if multiple jobs are submitted. >> + */ >> +int amdgpu_copy_ttm_mem_to_mem(struct amdgpu_device *adev, >> + struct ttm_buffer_object *bo[2], >> + struct ttm_mem_reg *mem[2], >> + uint64_t offset[2], >> + uint64_t size, >> + struct reservation_object *resv, >> + struct dma_fence **f) > Please document which is src and which is dst. I think from a readability standpoint, it would be better to pass explicit src and dst parameters rather than an array. > > Alex > > [HK]: I will add the comment. Yes I thought of using src & dst but the number of parameters will increase (already a large number) from 7 to 10. Yes agree completely, this array thing is really ugly. You could add a structure describing source and destination to decrease the number of parameters. >> + if (mem[i]->mem_type == TTM_PL_TT && >> + !amdgpu_gtt_mgr_is_allocated(mem[i])) { BTW: The coding style here looks incorrect. The second line should be indented a bit less. Regards, Christian.
