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.leichtzumerken@xxxxxxxxx>
>>>
>>> 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.koenig@xxxxxxx>
>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
