Am 22.08.2018 um 11:10 schrieb zhoucm1:
>
>
>
> On 2018å¹´08æ??22æ?¥ 17:05, Christian König wrote:
>> Am 22.08.2018 um 11:02 schrieb zhoucm1:
>>>
>>>
>>> On 2018å¹´08æ??22æ?¥ 16:52, Christian König wrote:
>>>> Am 22.08.2018 um 10:38 schrieb Chunming Zhou:
>>>>> stub fence will be used by timeline syncobj as well.
>>>>>
>>>>> Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285
>>>>> Signed-off-by: Chunming Zhou <david1.zhou at amd.com>
>>>>> Cc: Jason Ekstrand <jason at jlekstrand.net>
>>>>> ---
>>>>> Â drivers/gpu/drm/drm_syncobj.c | 28 ++--------------------------
>>>>>  include/drm/drm_syncobj.h    | 24 ++++++++++++++++++++++++
>>>>> Â 2 files changed, 26 insertions(+), 26 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_syncobj.c
>>>>> b/drivers/gpu/drm/drm_syncobj.c
>>>>> index d4f4ce484529..70af32d0def1 100644
>>>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>>>> @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct
>>>>> drm_syncobj *syncobj,
>>>>> Â }
>>>>> Â EXPORT_SYMBOL(drm_syncobj_replace_fence);
>>>>> Â -struct drm_syncobj_null_fence {
>>>>> -Â Â Â struct dma_fence base;
>>>>> -Â Â Â spinlock_t lock;
>>>>> -};
>>>>> -
>>>>> -static const char *drm_syncobj_null_fence_get_name(struct
>>>>> dma_fence *fence)
>>>>> -{
>>>>> -Â Â Â Â Â Â Â return "syncobjnull";
>>>>> -}
>>>>> -
>>>>> -static bool drm_syncobj_null_fence_enable_signaling(struct
>>>>> dma_fence *fence)
>>>>> -{
>>>>> -Â Â Â dma_fence_enable_sw_signaling(fence);
>>>>> -Â Â Â return !dma_fence_is_signaled(fence);
>>>>> -}
>>>>> -
>>>>> -static const struct dma_fence_ops drm_syncobj_null_fence_ops = {
>>>>> -Â Â Â .get_driver_name = drm_syncobj_null_fence_get_name,
>>>>> -Â Â Â .get_timeline_name = drm_syncobj_null_fence_get_name,
>>>>> -Â Â Â .enable_signaling = drm_syncobj_null_fence_enable_signaling,
>>>>> -Â Â Â .wait = dma_fence_default_wait,
>>>>> -Â Â Â .release = NULL,
>>>>> -};
>>>>> -
>>>>> Â static int drm_syncobj_assign_null_handle(struct drm_syncobj
>>>>> *syncobj)
>>>>> Â {
>>>>> -Â Â Â struct drm_syncobj_null_fence *fence;
>>>>> +Â Â Â struct drm_syncobj_stub_fence *fence;
>>>>> Â Â Â Â Â fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>>>>> Â Â Â Â Â if (fence == NULL)
>>>>> Â Â Â Â Â Â Â Â Â return -ENOMEM;
>>>>> Â Â Â Â Â Â spin_lock_init(&fence->lock);
>>>>> -Â Â Â dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops,
>>>>> +Â Â Â dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops,
>>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &fence->lock, 0, 0);
>>>>> Â Â Â Â Â dma_fence_signal(&fence->base);
>>>>> Â diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
>>>>> index 3980602472c0..b04c492ddbb5 100644
>>>>> --- a/include/drm/drm_syncobj.h
>>>>> +++ b/include/drm/drm_syncobj.h
>>>>> @@ -30,6 +30,30 @@
>>>>> Â Â struct drm_syncobj_cb;
>>>>> Â +struct drm_syncobj_stub_fence {
>>>>> +Â Â Â struct dma_fence base;
>>>>> +Â Â Â spinlock_t lock;
>>>>> +};
>>>>> +
>>>>> +const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence)
>>>>> +{
>>>>> +Â Â Â Â Â Â Â return "syncobjstub";
>>>>> +}
>>>>> +
>>>>> +bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence
>>>>> *fence)
>>>>> +{
>>>>> +Â Â Â dma_fence_enable_sw_signaling(fence);
>>>>
>>>> Copy&pasted from the old implementation, but that is certainly
>>>> totally nonsense.
>>>>
>>>> dma_fence_enable_sw_signaling() is the function who is calling this
>>>> callback.
>>> indeed, will fix.
>>>>
>>>>> +Â Â Â return !dma_fence_is_signaled(fence);
>>>>> +}
>>>>> +
>>>>> +const struct dma_fence_ops drm_syncobj_stub_fence_ops = {
>>>>> +Â Â Â .get_driver_name = drm_syncobj_stub_fence_get_name,
>>>>> +Â Â Â .get_timeline_name = drm_syncobj_stub_fence_get_name,
>>>>> +Â Â Â .enable_signaling = drm_syncobj_stub_fence_enable_signaling,
>>>>> +Â Â Â .wait = dma_fence_default_wait,
>>>>
>>>> The .wait callback should be dropped.
>>> why?
>>>
>>> fence->ops->wait(fence, intr, timeout) is called by
>>> dma_fence_wait(). If dropped, how does dma_fence_wait() work?
>>
>> You are working on an older code base, fence->ops->wait is optional
>> by now.
> Sorry, I still don't get it. My code is synced today from
> amd-staging-drm-next, and it's 4.18-rc1.
That is outdated, when working on common drm stuff you need to work on
drm-next or drm-misc-next.
> I still see the dma_fence_wait_timeout is :
> signed long
> dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long
> timeout)
> {
> Â Â Â Â Â Â Â signed long ret;
>
> Â Â Â Â Â Â Â if (WARN_ON(timeout < 0))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EINVAL;
>
> Â Â Â Â Â Â Â trace_dma_fence_wait_start(fence);
> Â Â Â Â Â Â Â ret = fence->ops->wait(fence, intr, timeout);
> Â Â Â Â Â Â Â trace_dma_fence_wait_end(fence);
> Â Â Â Â Â Â Â return ret;
> }
>
> .wait callback seems still a must have?
See drm-misc-next:
> Â Â Â Â Â Â Â trace_dma_fence_wait_start(fence);
> Â Â Â Â Â Â Â if (fence->ops->wait)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = fence->ops->wait(fence, intr, timeout);
> Â Â Â Â Â Â Â else
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = dma_fence_default_wait(fence, intr, timeout);
> Â Â Â Â Â Â Â trace_dma_fence_wait_end(fence);
Regards,
Christian.
>
> Thanks,
> David Zhou
>
>
>>
>> Christian.
>>
>>>
>>> Thanks,
>>> David
>>>>
>>>> Apart from that looks good to me.
>>>>
>>>> Christian.
>>>>
>>>>> +Â Â Â .release = NULL,
>>>>> +};
>>>>> +
>>>>> Â /**
>>>>> Â Â * struct drm_syncobj - sync object.
>>>>> Â Â *
>>>>
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180822/1564c984/attachment.html>
