[AMD Official Use Only - AMD Internal Distribution Only] With the update to commit message, some thing small and precise like Christian recommended sounds good. With that Reviewed-by: Sunil Khatri <sunil.kha...@amd.com>
-----Original Message----- From: Koenig, Christian <christian.koe...@amd.com> Sent: Friday, May 2, 2025 1:35 PM To: Yadav, Arvind <arvind.ya...@amd.com>; Deucher, Alexander <alexander.deuc...@amd.com>; Khatri, Sunil <sunil.kha...@amd.com>; Paneer Selvam, Arunpravin <arunpravin.paneersel...@amd.com> Cc: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH v4 2/2] drm/amdgpu: only keep most recent fence for each context On 4/30/25 18:05, Arvind Yadav wrote: > Mesa passes shared bo, fence syncobj to userq_ioctl. > There can be duplicates here or some fences that are old. > This patch is remove duplicates fence and only keep the most recent > fence for each context. > > v2: - Export this code from dma-fence-unwrap.c(by Christian). > v3: - To split this in a dma_buf patch and amd userq patch(by Sunil). > - No need to add a new function just re-use existing(by Christian). > v4: Export dma_fence_dedub_array function and used it(by Christian). > > Cc: Alex Deucher <alexander.deuc...@amd.com> > Cc: Christian Koenig <christian.koe...@amd.com> > Cc: Sunil Khatri <sunil.kha...@amd.com> > Cc: Arunpravin Paneer Selvam <arunpravin.paneersel...@amd.com> > Signed-off-by: Arvind Yadav <arvind.ya...@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > index 3288c2ff692e..e3e4aeee1356 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > @@ -851,6 +851,12 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void > *data, > fences[num_fences++] = fence; > } > > + /* > + * Remove duplicates fence and only keep the most recent fence > for > + * each context. > + */ Drop or at least change the comment. You should never document what the code does, that should be obvious by reading the code. Only document why the code does something, e.g. in this case here something like "Keep only the latest fences to reduce the number of values given back to userspace" would be appropriate. With that done the patch is Reviewed-by: Christian König <christian.koe...@amd.com>. Regards, Christian. > + num_fences = dma_fence_dedup_array(fences, num_fences); > + > waitq = idr_find(&userq_mgr->userq_idr, wait_info->waitq_id); > if (!waitq) > goto free_fences;
