On 2019-03-28 4:18 p.m., Christian König wrote: > Am 28.03.19 um 14:50 schrieb Lionel Landwerlin: >> On 25/03/2019 08:32, Chunming Zhou wrote: >>> From: Christian König <ckoenig.leichtzumer...@gmail.com> >>> >>> Use the dma_fence_chain object to create a timeline of fence objects >>> instead of just replacing the existing fence. >>> >>> v2: rebase and cleanup >>> v3: fix garbage collection parameters >>> v4: add unorder point check, print a warn calltrace >>> >>> Signed-off-by: Christian König <christian.koe...@amd.com> >>> Cc: Lionel Landwerlin <lionel.g.landwer...@intel.com> >>> --- >>> drivers/gpu/drm/drm_syncobj.c | 39 +++++++++++++++++++++++++++++++++++ >>> include/drm/drm_syncobj.h | 5 +++++ >>> 2 files changed, 44 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_syncobj.c >>> b/drivers/gpu/drm/drm_syncobj.c >>> index 5329e66598c6..19a9ce638119 100644 >>> --- a/drivers/gpu/drm/drm_syncobj.c >>> +++ b/drivers/gpu/drm/drm_syncobj.c >>> @@ -122,6 +122,45 @@ static void drm_syncobj_remove_wait(struct >>> drm_syncobj *syncobj, >>> spin_unlock(&syncobj->lock); >>> } >>> +/** >>> + * drm_syncobj_add_point - add new timeline point to the syncobj >>> + * @syncobj: sync object to add timeline point do >>> + * @chain: chain node to use to add the point >>> + * @fence: fence to encapsulate in the chain node >>> + * @point: sequence number to use for the point >>> + * >>> + * Add the chain node as new timeline point to the syncobj. >>> + */ >>> +void drm_syncobj_add_point(struct drm_syncobj *syncobj, >>> + struct dma_fence_chain *chain, >>> + struct dma_fence *fence, >>> + uint64_t point) >>> +{ >>> + struct syncobj_wait_entry *cur, *tmp; >>> + struct dma_fence *prev; >>> + >>> + dma_fence_get(fence); >>> + >>> + spin_lock(&syncobj->lock); >>> + >>> + prev = drm_syncobj_fence_get(syncobj); >>> + /* You are adding an unorder point to timeline, which could >>> cause payload returned from query_ioctl is 0! */ >>> + WARN_ON_ONCE(prev && prev->seqno >= point); >> >> >> I think the WARN/BUG macros should only fire when there is an issue >> with programming from within the kernel. >> >> But this particular warning can be triggered by an application. >> >> >> Probably best to just remove it? > > Yeah, that was also my argument against it. > > Key point here is that we still want to note somehow that userspace did > something wrong and returning an error is not an option. > > Maybe just use DRM_ERROR with a static variable to print the message > only once.
How about DRM_WARN_ONCE ? -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx