Some fence cleanups for amdgpu

Some fence cleanups for amdgpu

[Christian König]

Hi guys,

the following patches cleanup quite a bunch of fence functionality in amdgpu.

The first two patches move functions into the common fence code. I think that
having those two here make sense because they are hardware independent helper
functions.

Because of that I'm CCing Maarten and Sumit as well, please review and comment.

Regards,
Christian.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/10] dma-buf/fence: add fence_wait_any_timeout function

[Christian König]

From: Christian König <christian.koenig@amd.com>

Waiting for the first fence in an array of fences to signal.

This is useful for device driver specific resource managers
and also Vulkan needs something similar.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/dma-buf/fence.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fence.h   |  3 +-
 2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
index 50ef8bd..218623f 100644
--- a/drivers/dma-buf/fence.c
+++ b/drivers/dma-buf/fence.c
@@ -397,6 +397,102 @@ out:
 }
 EXPORT_SYMBOL(fence_default_wait);
 
+static bool
+fence_test_signaled_any(struct fence **fences, uint32_t count)
+{
+	int i;
+
+	for (i = 0; i < count; ++i) {
+		struct fence *fence = fences[i];
+		if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+			return true;
+	}
+	return false;
+}
+
+/**
+ * fence_wait_any_timeout - sleep until any fence gets signaled
+ * or until timeout elapses
+ * @fences:	[in]	array of fences to wait on
+ * @count:	[in]	number of fences to wait on
+ * @intr:	[in]	if true, do an interruptible wait
+ * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
+ *
+ * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
+ * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
+ * on success.
+ *
+ * Synchronous waits for the first fence in the array to be signaled. The
+ * caller needs to hold a reference to all fences in the array, otherwise a
+ * fence might be freed before return, resulting in undefined behavior.
+ */
+signed long
+fence_wait_any_timeout(struct fence **fences, uint32_t count,
+		       bool intr, signed long timeout)
+{
+	struct default_wait_cb *cb;
+	signed long ret = timeout;
+	unsigned i;
+
+	if (WARN_ON(!fences))
+		return -EINVAL;
+
+	cb = kcalloc(count, sizeof(struct default_wait_cb), GFP_KERNEL);
+	if (cb == NULL) {
+		ret = -ENOMEM;
+		goto err_free_cb;
+	}
+
+	for (i = 0; i < count; ++i) {
+		struct fence *fence = fences[i];
+
+		if (fence == NULL)
+			continue;
+
+		if (fence->ops->wait != fence_default_wait) {
+			ret = -EINVAL;
+			goto fence_rm_cb;
+		}
+
+		cb[i].task = current;
+		if (fence_add_callback(fence, &cb[i].base,
+				       fence_default_wait_cb)) {
+			/* This fence is already signaled */
+			goto fence_rm_cb;
+		}
+	}
+
+	while (ret > 0) {
+		if (intr)
+			set_current_state(TASK_INTERRUPTIBLE);
+		else
+			set_current_state(TASK_UNINTERRUPTIBLE);
+
+		if (fence_test_signaled_any(fences, count))
+			break;
+
+		ret = schedule_timeout(ret);
+
+		if (ret > 0 && intr && signal_pending(current))
+			ret = -ERESTARTSYS;
+	}
+
+	__set_current_state(TASK_RUNNING);
+
+fence_rm_cb:
+	for (i = 0; i < count; ++i) {
+		struct fence *fence = fences[i];
+		if (fence && cb[i].base.func)
+			fence_remove_callback(fence, &cb[i].base);
+	}
+
+err_free_cb:
+	kfree(cb);
+
+	return ret;
+}
+EXPORT_SYMBOL(fence_wait_any_timeout);
+
 /**
  * fence_init - Initialize a custom fence.
  * @fence:	[in]	the fence to initialize
diff --git a/include/linux/fence.h b/include/linux/fence.h
index 39efee1..a4084d6 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -305,7 +305,8 @@ static inline struct fence *fence_later(struct fence *f1, struct fence *f2)
 }
 
 signed long fence_wait_timeout(struct fence *, bool intr, signed long timeout);
-
+signed long fence_wait_any_timeout(struct fence **fences, uint32_t count,
+				   bool intr, signed long timeout);
 
 /**
  * fence_wait - sleep until the fence gets signaled
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/10] dma-buf/fence: add fence_wait_any_timeout function

[Maarten Lankhorst]

Op 26-10-15 om 13:29 schreef Christian König:
> From: Christian König <christian.koenig@amd.com>
>
> Waiting for the first fence in an array of fences to signal.
>
> This is useful for device driver specific resource managers
> and also Vulkan needs something similar.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/dma-buf/fence.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/fence.h   |  3 +-
>  2 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
> index 50ef8bd..218623f 100644
> --- a/drivers/dma-buf/fence.c
> +++ b/drivers/dma-buf/fence.c
> @@ -397,6 +397,102 @@ out:
>  }
>  EXPORT_SYMBOL(fence_default_wait);
>  
> +static bool
> +fence_test_signaled_any(struct fence **fences, uint32_t count)
> +{
> +	int i;
> +
> +	for (i = 0; i < count; ++i) {
> +		struct fence *fence = fences[i];
> +		if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +			return true;
> +	}
> +	return false;
> +}
> +
> +/**
> + * fence_wait_any_timeout - sleep until any fence gets signaled
> + * or until timeout elapses
> + * @fences:	[in]	array of fences to wait on
> + * @count:	[in]	number of fences to wait on
> + * @intr:	[in]	if true, do an interruptible wait
> + * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> + *
> + * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
> + * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
> + * on success.
> + *
> + * Synchronous waits for the first fence in the array to be signaled. The
> + * caller needs to hold a reference to all fences in the array, otherwise a
> + * fence might be freed before return, resulting in undefined behavior.
> + */
> +signed long
> +fence_wait_any_timeout(struct fence **fences, uint32_t count,
> +		       bool intr, signed long timeout)
> +{
> +	struct default_wait_cb *cb;
> +	signed long ret = timeout;
> +	unsigned i;
> +
> +	if (WARN_ON(!fences))
> +		return -EINVAL;
This should probably have a check for count == 0 before this WARN_ON, so it doesn't wait an infinite amount of time when count == 0.
And it also needs to special case timeout == 0 even before that, so it still returns 1 in that case to be compatible with other wait functions.

> +	cb = kcalloc(count, sizeof(struct default_wait_cb), GFP_KERNEL);
> +	if (cb == NULL) {
> +		ret = -ENOMEM;
> +		goto err_free_cb;
> +	}
> +
> +	for (i = 0; i < count; ++i) {
> +		struct fence *fence = fences[i];
> +
> +		if (fence == NULL)
> +			continue;
Not sure that NULL fences should be handled here.. and in the amdgpu case surely if fence is null that means
you don't need to wait for any fence to be signaled?
> +
> +		if (fence->ops->wait != fence_default_wait) {
> +			ret = -EINVAL;
> +			goto fence_rm_cb;
> +		}
> +
> +		cb[i].task = current;
> +		if (fence_add_callback(fence, &cb[i].base,
> +				       fence_default_wait_cb)) {
> +			/* This fence is already signaled */
> +			goto fence_rm_cb;
> +		}
> +	}
> +
> +	while (ret > 0) {
> +		if (intr)
> +			set_current_state(TASK_INTERRUPTIBLE);
> +		else
> +			set_current_state(TASK_UNINTERRUPTIBLE);
> +
> +		if (fence_test_signaled_any(fences, count))
> +			break;
> +
> +		ret = schedule_timeout(ret);
> +
> +		if (ret > 0 && intr && signal_pending(current))
> +			ret = -ERESTARTSYS;
> +	}
> +
> +	__set_current_state(TASK_RUNNING);
> +
> +fence_rm_cb:
> +	for (i = 0; i < count; ++i) {
If i is not used elsewhere then the fence_remove_callback could be made
unconditional and the loop changed to
while (i-- > 0) {
> +		struct fence *fence = fences[i];
> +		if (fence && cb[i].base.func)
> +			fence_remove_callback(fence, &cb[i].base);
> +	}
> +
> +err_free_cb:
> +	kfree(cb);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(fence_wait_any_timeout);
> +
>  /**
>   * fence_init - Initialize a custom fence.
>   * @fence:	[in]	the fence to initialize
> diff --git a/include/linux/fence.h b/include/linux/fence.h
> index 39efee1..a4084d6 100644
> --- a/include/linux/fence.h
> +++ b/include/linux/fence.h
> @@ -305,7 +305,8 @@ static inline struct fence *fence_later(struct fence *f1, struct fence *f2)
>  }
>  
>  signed long fence_wait_timeout(struct fence *, bool intr, signed long timeout);
> -
> +signed long fence_wait_any_timeout(struct fence **fences, uint32_t count,
> +				   bool intr, signed long timeout);
>  
>  /**
>   * fence_wait - sleep until the fence gets signaled

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/10] dma-buf/fence: add fence_wait_any_timeout function

[Christian König]

On 26.10.2015 14:18, Maarten Lankhorst wrote:
> Op 26-10-15 om 13:29 schreef Christian König:
>> From: Christian König <christian.koenig@amd.com>
>>
>> Waiting for the first fence in an array of fences to signal.
>>
>> This is useful for device driver specific resource managers
>> and also Vulkan needs something similar.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   drivers/dma-buf/fence.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/fence.h   |  3 +-
>>   2 files changed, 98 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
>> index 50ef8bd..218623f 100644
>> --- a/drivers/dma-buf/fence.c
>> +++ b/drivers/dma-buf/fence.c
>> @@ -397,6 +397,102 @@ out:
>>   }
>>   EXPORT_SYMBOL(fence_default_wait);
>>   
>> +static bool
>> +fence_test_signaled_any(struct fence **fences, uint32_t count)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < count; ++i) {
>> +		struct fence *fence = fences[i];
>> +		if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>> +			return true;
>> +	}
>> +	return false;
>> +}
>> +
>> +/**
>> + * fence_wait_any_timeout - sleep until any fence gets signaled
>> + * or until timeout elapses
>> + * @fences:	[in]	array of fences to wait on
>> + * @count:	[in]	number of fences to wait on
>> + * @intr:	[in]	if true, do an interruptible wait
>> + * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
>> + *
>> + * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
>> + * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
>> + * on success.
>> + *
>> + * Synchronous waits for the first fence in the array to be signaled. The
>> + * caller needs to hold a reference to all fences in the array, otherwise a
>> + * fence might be freed before return, resulting in undefined behavior.
>> + */
>> +signed long
>> +fence_wait_any_timeout(struct fence **fences, uint32_t count,
>> +		       bool intr, signed long timeout)
>> +{
>> +	struct default_wait_cb *cb;
>> +	signed long ret = timeout;
>> +	unsigned i;
>> +
>> +	if (WARN_ON(!fences))
>> +		return -EINVAL;
> This should probably have a check for count == 0 before this WARN_ON, so it doesn't wait an infinite amount of time when count == 0.
> And it also needs to special case timeout == 0 even before that, so it still returns 1 in that case to be compatible with other wait functions.

Makes sense, going to add the extra checks.

>
>> +	cb = kcalloc(count, sizeof(struct default_wait_cb), GFP_KERNEL);
>> +	if (cb == NULL) {
>> +		ret = -ENOMEM;
>> +		goto err_free_cb;
>> +	}
>> +
>> +	for (i = 0; i < count; ++i) {
>> +		struct fence *fence = fences[i];
>> +
>> +		if (fence == NULL)
>> +			continue;
> Not sure that NULL fences should be handled here.. and in the amdgpu case surely if fence is null that means
> you don't need to wait for any fence to be signaled?

Good point.

The use case in amdgpu is that we know how many fences we will maximum 
have (one per engine). So we allocate the array on the stack and fill in 
each slot with the earliest fence we can find, some slots can obviously 
be unused and so NULL.

Thought about removing that as well by accumulating the fences before 
calling the function, but couldn't find a good argument for doing so. 
That we wait forever if all fences are NULL is obviously a good argument.

What do you prefer that we accumulate the fence in amdgpu before calling 
the function or an extra check here that return -EINVAL if all fences 
are NULL?

>> +
>> +		if (fence->ops->wait != fence_default_wait) {
>> +			ret = -EINVAL;
>> +			goto fence_rm_cb;
>> +		}
>> +
>> +		cb[i].task = current;
>> +		if (fence_add_callback(fence, &cb[i].base,
>> +				       fence_default_wait_cb)) {
>> +			/* This fence is already signaled */
>> +			goto fence_rm_cb;
>> +		}
>> +	}
>> +
>> +	while (ret > 0) {
>> +		if (intr)
>> +			set_current_state(TASK_INTERRUPTIBLE);
>> +		else
>> +			set_current_state(TASK_UNINTERRUPTIBLE);
>> +
>> +		if (fence_test_signaled_any(fences, count))
>> +			break;
>> +
>> +		ret = schedule_timeout(ret);
>> +
>> +		if (ret > 0 && intr && signal_pending(current))
>> +			ret = -ERESTARTSYS;
>> +	}
>> +
>> +	__set_current_state(TASK_RUNNING);
>> +
>> +fence_rm_cb:
>> +	for (i = 0; i < count; ++i) {
> If i is not used elsewhere then the fence_remove_callback could be made
> unconditional and the loop changed to
> while (i-- > 0) {

Yeah, going to change that as well.

Regards,
Christian.

>> +		struct fence *fence = fences[i];
>> +		if (fence && cb[i].base.func)
>> +			fence_remove_callback(fence, &cb[i].base);
>> +	}
>> +
>> +err_free_cb:
>> +	kfree(cb);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(fence_wait_any_timeout);
>> +
>>   /**
>>    * fence_init - Initialize a custom fence.
>>    * @fence:	[in]	the fence to initialize
>> diff --git a/include/linux/fence.h b/include/linux/fence.h
>> index 39efee1..a4084d6 100644
>> --- a/include/linux/fence.h
>> +++ b/include/linux/fence.h
>> @@ -305,7 +305,8 @@ static inline struct fence *fence_later(struct fence *f1, struct fence *f2)
>>   }
>>   
>>   signed long fence_wait_timeout(struct fence *, bool intr, signed long timeout);
>> -
>> +signed long fence_wait_any_timeout(struct fence **fences, uint32_t count,
>> +				   bool intr, signed long timeout);
>>   
>>   /**
>>    * fence_wait - sleep until the fence gets signaled

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/10] dma-buf/fence: add fence_wait_any_timeout function

[Christian König]

> What do you prefer that we accumulate the fence in amdgpu before 
> calling the function or an extra check here that return -EINVAL if all 
> fences are NULL?
Forget that question, I realized that the current check in amdgpu for 
this doesn't work any more anyway (even without this patch).

I've just send an updated version of the patch to you, please review and 
comment.

Thanks,
Christian.

On 26.10.2015 14:45, Christian König wrote:
> On 26.10.2015 14:18, Maarten Lankhorst wrote:
>> Op 26-10-15 om 13:29 schreef Christian König:
>>> From: Christian König <christian.koenig@amd.com>
>>>
>>> Waiting for the first fence in an array of fences to signal.
>>>
>>> This is useful for device driver specific resource managers
>>> and also Vulkan needs something similar.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>   drivers/dma-buf/fence.c | 96 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/fence.h   |  3 +-
>>>   2 files changed, 98 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
>>> index 50ef8bd..218623f 100644
>>> --- a/drivers/dma-buf/fence.c
>>> +++ b/drivers/dma-buf/fence.c
>>> @@ -397,6 +397,102 @@ out:
>>>   }
>>>   EXPORT_SYMBOL(fence_default_wait);
>>>   +static bool
>>> +fence_test_signaled_any(struct fence **fences, uint32_t count)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i < count; ++i) {
>>> +        struct fence *fence = fences[i];
>>> +        if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>>> +            return true;
>>> +    }
>>> +    return false;
>>> +}
>>> +
>>> +/**
>>> + * fence_wait_any_timeout - sleep until any fence gets signaled
>>> + * or until timeout elapses
>>> + * @fences:    [in]    array of fences to wait on
>>> + * @count:    [in]    number of fences to wait on
>>> + * @intr:    [in]    if true, do an interruptible wait
>>> + * @timeout:    [in]    timeout value in jiffies, or 
>>> MAX_SCHEDULE_TIMEOUT
>>> + *
>>> + * Returns -EINVAL on custom fence wait implementation, 
>>> -ERESTARTSYS if
>>> + * interrupted, 0 if the wait timed out, or the remaining timeout 
>>> in jiffies
>>> + * on success.
>>> + *
>>> + * Synchronous waits for the first fence in the array to be 
>>> signaled. The
>>> + * caller needs to hold a reference to all fences in the array, 
>>> otherwise a
>>> + * fence might be freed before return, resulting in undefined 
>>> behavior.
>>> + */
>>> +signed long
>>> +fence_wait_any_timeout(struct fence **fences, uint32_t count,
>>> +               bool intr, signed long timeout)
>>> +{
>>> +    struct default_wait_cb *cb;
>>> +    signed long ret = timeout;
>>> +    unsigned i;
>>> +
>>> +    if (WARN_ON(!fences))
>>> +        return -EINVAL;
>> This should probably have a check for count == 0 before this WARN_ON, 
>> so it doesn't wait an infinite amount of time when count == 0.
>> And it also needs to special case timeout == 0 even before that, so 
>> it still returns 1 in that case to be compatible with other wait 
>> functions.
>
> Makes sense, going to add the extra checks.
>
>>
>>> +    cb = kcalloc(count, sizeof(struct default_wait_cb), GFP_KERNEL);
>>> +    if (cb == NULL) {
>>> +        ret = -ENOMEM;
>>> +        goto err_free_cb;
>>> +    }
>>> +
>>> +    for (i = 0; i < count; ++i) {
>>> +        struct fence *fence = fences[i];
>>> +
>>> +        if (fence == NULL)
>>> +            continue;
>> Not sure that NULL fences should be handled here.. and in the amdgpu 
>> case surely if fence is null that means
>> you don't need to wait for any fence to be signaled?
>
> Good point.
>
> The use case in amdgpu is that we know how many fences we will maximum 
> have (one per engine). So we allocate the array on the stack and fill 
> in each slot with the earliest fence we can find, some slots can 
> obviously be unused and so NULL.
>
> Thought about removing that as well by accumulating the fences before 
> calling the function, but couldn't find a good argument for doing so. 
> That we wait forever if all fences are NULL is obviously a good argument.
>
> What do you prefer that we accumulate the fence in amdgpu before 
> calling the function or an extra check here that return -EINVAL if all 
> fences are NULL?
>
>>> +
>>> +        if (fence->ops->wait != fence_default_wait) {
>>> +            ret = -EINVAL;
>>> +            goto fence_rm_cb;
>>> +        }
>>> +
>>> +        cb[i].task = current;
>>> +        if (fence_add_callback(fence, &cb[i].base,
>>> +                       fence_default_wait_cb)) {
>>> +            /* This fence is already signaled */
>>> +            goto fence_rm_cb;
>>> +        }
>>> +    }
>>> +
>>> +    while (ret > 0) {
>>> +        if (intr)
>>> +            set_current_state(TASK_INTERRUPTIBLE);
>>> +        else
>>> +            set_current_state(TASK_UNINTERRUPTIBLE);
>>> +
>>> +        if (fence_test_signaled_any(fences, count))
>>> +            break;
>>> +
>>> +        ret = schedule_timeout(ret);
>>> +
>>> +        if (ret > 0 && intr && signal_pending(current))
>>> +            ret = -ERESTARTSYS;
>>> +    }
>>> +
>>> +    __set_current_state(TASK_RUNNING);
>>> +
>>> +fence_rm_cb:
>>> +    for (i = 0; i < count; ++i) {
>> If i is not used elsewhere then the fence_remove_callback could be made
>> unconditional and the loop changed to
>> while (i-- > 0) {
>
> Yeah, going to change that as well.
>
> Regards,
> Christian.
>
>>> +        struct fence *fence = fences[i];
>>> +        if (fence && cb[i].base.func)
>>> +            fence_remove_callback(fence, &cb[i].base);
>>> +    }
>>> +
>>> +err_free_cb:
>>> +    kfree(cb);
>>> +
>>> +    return ret;
>>> +}
>>> +EXPORT_SYMBOL(fence_wait_any_timeout);
>>> +
>>>   /**
>>>    * fence_init - Initialize a custom fence.
>>>    * @fence:    [in]    the fence to initialize
>>> diff --git a/include/linux/fence.h b/include/linux/fence.h
>>> index 39efee1..a4084d6 100644
>>> --- a/include/linux/fence.h
>>> +++ b/include/linux/fence.h
>>> @@ -305,7 +305,8 @@ static inline struct fence *fence_later(struct 
>>> fence *f1, struct fence *f2)
>>>   }
>>>     signed long fence_wait_timeout(struct fence *, bool intr, signed 
>>> long timeout);
>>> -
>>> +signed long fence_wait_any_timeout(struct fence **fences, uint32_t 
>>> count,
>>> +                   bool intr, signed long timeout);
>>>     /**
>>>    * fence_wait - sleep until the fence gets signaled
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/10] dma-buf/fence: add fence_is_later()

[Christian König]

From: Christian König <christian.koenig@amd.com>

Return true when fence 1 is later than fence 2 without
checking if any of them are signaled.

Useful for driver specific resource handling based on fences.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 include/linux/fence.h | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/include/linux/fence.h b/include/linux/fence.h
index a4084d6..bb52201 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -280,6 +280,22 @@ fence_is_signaled(struct fence *fence)
 }
 
 /**
+ * fence_is_later - return if f1 is chronologically later than f2
+ * @f1:	[in]	the first fence from the same context
+ * @f2:	[in]	the second fence from the same context
+ *
+ * Returns true if f1 is chronologically later than f2. Both fences must be
+ * from the same context, since a seqno is not re-used across contexts.
+ */
+static inline bool fence_is_later(struct fence *f1, struct fence *f2)
+{
+	if (WARN_ON(f1->context != f2->context))
+		return false;
+
+	return f1->seqno - f2->seqno < INT_MAX;
+}
+
+/**
  * fence_later - return the chronologically later fence
  * @f1:	[in]	the first fence from the same context
  * @f2:	[in]	the second fence from the same context
@@ -298,10 +314,10 @@ static inline struct fence *fence_later(struct fence *f1, struct fence *f2)
 	 * set if enable_signaling wasn't called, and enabling that here is
 	 * overkill.
 	 */
-	if (f2->seqno - f1->seqno <= INT_MAX)
-		return fence_is_signaled(f2) ? NULL : f2;
-	else
+	if (fence_is_later(f1, f2))
 		return fence_is_signaled(f1) ? NULL : f1;
+	else
+		return fence_is_signaled(f2) ? NULL : f2;
 }
 
 signed long fence_wait_timeout(struct fence *, bool intr, signed long timeout);
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/10] drm/amdgpu: remove unneeded fence functions

[Christian König]

From: Christian König <christian.koenig@amd.com>

amdgpu_fence_default_wait isn't needed any more the default wait does the same
thing and amdgpu_test_signaled is dead as well.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 003a219..663caa9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -822,11 +822,6 @@ static const char *amdgpu_fence_get_timeline_name(struct fence *f)
 	return (const char *)fence->ring->name;
 }
 
-static inline bool amdgpu_test_signaled(struct amdgpu_fence *fence)
-{
-	return test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->base.flags);
-}
-
 static bool amdgpu_test_signaled_any(struct fence **fences, uint32_t count)
 {
 	int idx;
@@ -854,12 +849,6 @@ static void amdgpu_fence_wait_cb(struct fence *fence, struct fence_cb *cb)
 	wake_up_process(wait->task);
 }
 
-static signed long amdgpu_fence_default_wait(struct fence *f, bool intr,
-					     signed long t)
-{
-	return amdgpu_fence_wait_any(&f, 1, intr, t);
-}
-
 /**
  * Wait the fence array with timeout
  *
@@ -936,6 +925,6 @@ const struct fence_ops amdgpu_fence_ops = {
 	.get_timeline_name = amdgpu_fence_get_timeline_name,
 	.enable_signaling = amdgpu_fence_enable_signaling,
 	.signaled = amdgpu_fence_is_signaled,
-	.wait = amdgpu_fence_default_wait,
+	.wait = fence_default_wait,
 	.release = NULL,
 };
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/10] drm/amdgpu: switch to common fence_wait_any_timeout

[Christian König]

From: Christian König <christian.koenig@amd.com>

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 98 -------------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c    |  4 +-
 3 files changed, 2 insertions(+), 104 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a9c0def..dd7d2ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -447,10 +447,6 @@ int amdgpu_fence_wait_next(struct amdgpu_ring *ring);
 int amdgpu_fence_wait_empty(struct amdgpu_ring *ring);
 unsigned amdgpu_fence_count_emitted(struct amdgpu_ring *ring);
 
-signed long amdgpu_fence_wait_any(struct fence **array,
-				  uint32_t count,
-				  bool intr,
-				  signed long t);
 struct amdgpu_fence *amdgpu_fence_ref(struct amdgpu_fence *fence);
 void amdgpu_fence_unref(struct amdgpu_fence **fence);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 663caa9..c4bb282 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -822,104 +822,6 @@ static const char *amdgpu_fence_get_timeline_name(struct fence *f)
 	return (const char *)fence->ring->name;
 }
 
-static bool amdgpu_test_signaled_any(struct fence **fences, uint32_t count)
-{
-	int idx;
-	struct fence *fence;
-
-	for (idx = 0; idx < count; ++idx) {
-		fence = fences[idx];
-		if (fence) {
-			if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
-				return true;
-		}
-	}
-	return false;
-}
-
-struct amdgpu_wait_cb {
-	struct fence_cb base;
-	struct task_struct *task;
-};
-
-static void amdgpu_fence_wait_cb(struct fence *fence, struct fence_cb *cb)
-{
-	struct amdgpu_wait_cb *wait =
-		container_of(cb, struct amdgpu_wait_cb, base);
-	wake_up_process(wait->task);
-}
-
-/**
- * Wait the fence array with timeout
- *
- * @array:    the fence array with amdgpu fence pointer
- * @count:    the number of the fence array
- * @intr:     when sleep, set the current task interruptable or not
- * @t:        timeout to wait
- *
- * It will return when any fence is signaled or timeout.
- */
-signed long amdgpu_fence_wait_any(struct fence **array, uint32_t count,
-				  bool intr, signed long t)
-{
-	struct amdgpu_wait_cb *cb;
-	struct fence *fence;
-	unsigned idx;
-
-	BUG_ON(!array);
-
-	cb = kcalloc(count, sizeof(struct amdgpu_wait_cb), GFP_KERNEL);
-	if (cb == NULL) {
-		t = -ENOMEM;
-		goto err_free_cb;
-	}
-
-	for (idx = 0; idx < count; ++idx) {
-		fence = array[idx];
-		if (fence) {
-			cb[idx].task = current;
-			if (fence_add_callback(fence,
-					&cb[idx].base, amdgpu_fence_wait_cb)) {
-				/* The fence is already signaled */
-				goto fence_rm_cb;
-			}
-		}
-	}
-
-	while (t > 0) {
-		if (intr)
-			set_current_state(TASK_INTERRUPTIBLE);
-		else
-			set_current_state(TASK_UNINTERRUPTIBLE);
-
-		/*
-		 * amdgpu_test_signaled_any must be called after
-		 * set_current_state to prevent a race with wake_up_process
-		 */
-		if (amdgpu_test_signaled_any(array, count))
-			break;
-
-		t = schedule_timeout(t);
-
-		if (t > 0 && intr && signal_pending(current))
-			t = -ERESTARTSYS;
-	}
-
-	__set_current_state(TASK_RUNNING);
-
-fence_rm_cb:
-	for (idx = 0; idx < count; ++idx) {
-		fence = array[idx];
-		if (fence && cb[idx].base.func)
-			fence_remove_callback(fence, &cb[idx].base);
-	}
-
-err_free_cb:
-	kfree(cb);
-
-	return t;
-}
-
 const struct fence_ops amdgpu_fence_ops = {
 	.get_driver_name = amdgpu_fence_get_driver_name,
 	.get_timeline_name = amdgpu_fence_get_timeline_name,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
index 5cb27d5..4566d52 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
@@ -372,8 +372,8 @@ int amdgpu_sa_bo_new(struct amdgpu_device *adev,
 		} while (amdgpu_sa_bo_next_hole(sa_manager, fences, tries));
 
 		spin_unlock(&sa_manager->wq.lock);
-		t = amdgpu_fence_wait_any(fences, AMDGPU_MAX_RINGS,
-					  false, MAX_SCHEDULE_TIMEOUT);
+		t = fence_wait_any_timeout(fences, AMDGPU_MAX_RINGS,
+					   false, MAX_SCHEDULE_TIMEOUT);
 		r = (t > 0) ? 0 : t;
 		spin_lock(&sa_manager->wq.lock);
 		/* if we have nothing to wait for block */
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/10] drm/amdgpu: move ring_from_fence to common code

[Christian König]

From: Christian König <christian.koenig@amd.com>

Going to need that elsewhere as well.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c   | 23 ++---------------------
 3 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index dd7d2ce..6d9c929 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1231,6 +1231,7 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
 		     struct amdgpu_irq_src *irq_src, unsigned irq_type,
 		     enum amdgpu_ring_type ring_type);
 void amdgpu_ring_fini(struct amdgpu_ring *ring);
+struct amdgpu_ring *amdgpu_ring_from_fence(struct fence *f);
 
 /*
  * CS.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index b2df348..78e9b0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -436,6 +436,30 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
 	}
 }
 
+/**
+ * amdgpu_ring_from_fence - get ring from fence
+ *
+ * @f: fence structure
+ *
+ * Extract the ring a fence belongs to. Handles both scheduler as
+ * well as hardware fences.
+ */
+struct amdgpu_ring *amdgpu_ring_from_fence(struct fence *f)
+{
+	struct amdgpu_fence *a_fence;
+	struct amd_sched_fence *s_fence;
+
+	s_fence = to_amd_sched_fence(f);
+	if (s_fence)
+		return container_of(s_fence->sched, struct amdgpu_ring, sched);
+
+	a_fence = to_amdgpu_fence(f);
+	if (a_fence)
+		return a_fence->ring;
+
+	return NULL;
+}
+
 /*
  * Debugfs info
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
index 4566d52..fb3c7c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
@@ -139,25 +139,6 @@ int amdgpu_sa_bo_manager_suspend(struct amdgpu_device *adev,
 	return r;
 }
 
-static uint32_t amdgpu_sa_get_ring_from_fence(struct fence *f)
-{
-	struct amdgpu_fence *a_fence;
-	struct amd_sched_fence *s_fence;
-
-	s_fence = to_amd_sched_fence(f);
-	if (s_fence) {
-		struct amdgpu_ring *ring;
-
-		ring = container_of(s_fence->sched, struct amdgpu_ring, sched);
-		return ring->idx;
-	}
-
-	a_fence = to_amdgpu_fence(f);
-	if (a_fence)
-		return a_fence->ring->idx;
-	return 0;
-}
-
 static void amdgpu_sa_bo_remove_locked(struct amdgpu_sa_bo *sa_bo)
 {
 	struct amdgpu_sa_manager *sa_manager = sa_bo->manager;
@@ -318,7 +299,7 @@ static bool amdgpu_sa_bo_next_hole(struct amdgpu_sa_manager *sa_manager,
 	}
 
 	if (best_bo) {
-		uint32_t idx = amdgpu_sa_get_ring_from_fence(best_bo->fence);
+		uint32_t idx = amdgpu_ring_from_fence(best_bo->fence)->idx;
 		++tries[idx];
 		sa_manager->hole = best_bo->olist.prev;
 
@@ -406,7 +387,7 @@ void amdgpu_sa_bo_free(struct amdgpu_device *adev, struct amdgpu_sa_bo **sa_bo,
 	if (fence && !fence_is_signaled(fence)) {
 		uint32_t idx;
 		(*sa_bo)->fence = fence_get(fence);
-		idx = amdgpu_sa_get_ring_from_fence(fence);
+		idx = amdgpu_ring_from_fence(fence)->idx;
 		list_add_tail(&(*sa_bo)->flist, &sa_manager->flist[idx]);
 	} else {
 		amdgpu_sa_bo_remove_locked(*sa_bo);
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/10] drm/amdgpu: use common fences for VMID management v2

[Christian König]

From: Christian König <christian.koenig@amd.com>

v2: add missing NULL check.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h    |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 27 +++++++++++++++------------
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  4 ++--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  4 ++--
 4 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 6d9c929..371994c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -970,7 +970,7 @@ struct amdgpu_vm_id {
 	/* last flushed PD/PT update */
 	struct fence	        *flushed_updates;
 	/* last use of vmid */
-	struct amdgpu_fence	*last_id_use;
+	struct fence		*last_id_use;
 };
 
 struct amdgpu_vm {
@@ -1003,7 +1003,7 @@ struct amdgpu_vm {
 };
 
 struct amdgpu_vm_manager {
-	struct amdgpu_fence		*active[AMDGPU_NUM_VM];
+	struct fence			*active[AMDGPU_NUM_VM];
 	uint32_t			max_pfn;
 	/* number of VMIDs */
 	unsigned			nvm;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 06e207f..a12c726 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -135,7 +135,7 @@ struct amdgpu_bo_list_entry *amdgpu_vm_get_bos(struct amdgpu_device *adev,
 int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
 		      struct amdgpu_sync *sync)
 {
-	struct amdgpu_fence *best[AMDGPU_MAX_RINGS] = {};
+	struct fence *best[AMDGPU_MAX_RINGS] = {};
 	struct amdgpu_vm_id *vm_id = &vm->ids[ring->idx];
 	struct amdgpu_device *adev = ring->adev;
 
@@ -154,7 +154,8 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
 
 	/* skip over VMID 0, since it is the system VM */
 	for (i = 1; i < adev->vm_manager.nvm; ++i) {
-		struct amdgpu_fence *fence = adev->vm_manager.active[i];
+		struct fence *fence = adev->vm_manager.active[i];
+		struct amdgpu_ring *fring;
 
 		if (fence == NULL) {
 			/* found a free one */
@@ -163,21 +164,23 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
 			return 0;
 		}
 
-		if (amdgpu_fence_is_earlier(fence, best[fence->ring->idx])) {
-			best[fence->ring->idx] = fence;
-			choices[fence->ring == ring ? 0 : 1] = i;
+		fring = amdgpu_ring_from_fence(fence);
+		if (best[fring->idx] == NULL ||
+		    fence_is_later(best[fring->idx], fence)) {
+			best[fring->idx] = fence;
+			choices[fring == ring ? 0 : 1] = i;
 		}
 	}
 
 	for (i = 0; i < 2; ++i) {
 		if (choices[i]) {
-			struct amdgpu_fence *fence;
+			struct fence *fence;
 
 			fence  = adev->vm_manager.active[choices[i]];
 			vm_id->id = choices[i];
 
 			trace_amdgpu_vm_grab_id(choices[i], ring->idx);
-			return amdgpu_sync_fence(ring->adev, sync, &fence->base);
+			return amdgpu_sync_fence(ring->adev, sync, fence);
 		}
 	}
 
@@ -246,11 +249,11 @@ void amdgpu_vm_fence(struct amdgpu_device *adev,
 	unsigned ridx = fence->ring->idx;
 	unsigned vm_id = vm->ids[ridx].id;
 
-	amdgpu_fence_unref(&adev->vm_manager.active[vm_id]);
-	adev->vm_manager.active[vm_id] = amdgpu_fence_ref(fence);
+	fence_put(adev->vm_manager.active[vm_id]);
+	adev->vm_manager.active[vm_id] = fence_get(&fence->base);
 
-	amdgpu_fence_unref(&vm->ids[ridx].last_id_use);
-	vm->ids[ridx].last_id_use = amdgpu_fence_ref(fence);
+	fence_put(vm->ids[ridx].last_id_use);
+	vm->ids[ridx].last_id_use = fence_get(&fence->base);
 }
 
 /**
@@ -1311,7 +1314,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 
 	for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
 		fence_put(vm->ids[i].flushed_updates);
-		amdgpu_fence_unref(&vm->ids[i].last_id_use);
+		fence_put(vm->ids[i].last_id_use);
 	}
 
 	mutex_destroy(&vm->mutex);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 4883482..85bbcdc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -965,7 +965,7 @@ static int gmc_v7_0_sw_fini(void *handle)
 
 	if (adev->vm_manager.enabled) {
 		for (i = 0; i < AMDGPU_NUM_VM; ++i)
-			amdgpu_fence_unref(&adev->vm_manager.active[i]);
+			fence_put(adev->vm_manager.active[i]);
 		gmc_v7_0_vm_fini(adev);
 		adev->vm_manager.enabled = false;
 	}
@@ -1015,7 +1015,7 @@ static int gmc_v7_0_suspend(void *handle)
 
 	if (adev->vm_manager.enabled) {
 		for (i = 0; i < AMDGPU_NUM_VM; ++i)
-			amdgpu_fence_unref(&adev->vm_manager.active[i]);
+			fence_put(adev->vm_manager.active[i]);
 		gmc_v7_0_vm_fini(adev);
 		adev->vm_manager.enabled = false;
 	}
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 72e977b..1bcc4e7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -984,7 +984,7 @@ static int gmc_v8_0_sw_fini(void *handle)
 
 	if (adev->vm_manager.enabled) {
 		for (i = 0; i < AMDGPU_NUM_VM; ++i)
-			amdgpu_fence_unref(&adev->vm_manager.active[i]);
+			fence_put(adev->vm_manager.active[i]);
 		gmc_v8_0_vm_fini(adev);
 		adev->vm_manager.enabled = false;
 	}
@@ -1036,7 +1036,7 @@ static int gmc_v8_0_suspend(void *handle)
 
 	if (adev->vm_manager.enabled) {
 		for (i = 0; i < AMDGPU_NUM_VM; ++i)
-			amdgpu_fence_unref(&adev->vm_manager.active[i]);
+			fence_put(adev->vm_manager.active[i]);
 		gmc_v8_0_vm_fini(adev);
 		adev->vm_manager.enabled = false;
 	}
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/10] drm/amdgpu: use the new fence_is_later

[Christian König]

From: Christian König <christian.koenig@amd.com>

Instead of coding the check ourself.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index 4921de1..0499d2b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -87,6 +87,15 @@ static bool amdgpu_sync_test_owner(struct fence *f, void *owner)
 	return false;
 }
 
+static void amdgpu_sync_keep_later(struct fence **keep, struct fence *fence)
+{
+	if (*keep && fence_is_later(*keep, fence))
+		return;
+
+	fence_put(*keep);
+	*keep = fence_get(fence);
+}
+
 /**
  * amdgpu_sync_fence - remember to sync to this fence
  *
@@ -100,34 +109,21 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, struct amdgpu_sync *sync,
 	struct amdgpu_sync_entry *e;
 	struct amdgpu_fence *fence;
 	struct amdgpu_fence *other;
-	struct fence *tmp, *later;
 
 	if (!f)
 		return 0;
 
 	if (amdgpu_sync_same_dev(adev, f) &&
-	    amdgpu_sync_test_owner(f, AMDGPU_FENCE_OWNER_VM)) {
-		if (sync->last_vm_update) {
-			tmp = sync->last_vm_update;
-			BUG_ON(f->context != tmp->context);
-			later = (f->seqno - tmp->seqno <= INT_MAX) ? f : tmp;
-			sync->last_vm_update = fence_get(later);
-			fence_put(tmp);
-		} else
-			sync->last_vm_update = fence_get(f);
-	}
+	    amdgpu_sync_test_owner(f, AMDGPU_FENCE_OWNER_VM))
+		amdgpu_sync_keep_later(&sync->last_vm_update, f);
 
 	fence = to_amdgpu_fence(f);
 	if (!fence || fence->ring->adev != adev) {
 		hash_for_each_possible(sync->fences, e, node, f->context) {
-			struct fence *new;
 			if (unlikely(e->fence->context != f->context))
 				continue;
-			new = fence_get(fence_later(e->fence, f));
-			if (new) {
-				fence_put(e->fence);
-				e->fence = new;
-			}
+
+			amdgpu_sync_keep_later(&e->fence, f);
 			return 0;
 		}
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/10] drm/amdgpu: use common fence for sync

[Christian König]

From: Christian König <christian.koenig@amd.com>

Stop using the driver internal functions.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 21 +++++++++++----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 371994c..b986ea1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -704,7 +704,7 @@ void amdgpu_semaphore_free(struct amdgpu_device *adev,
  */
 struct amdgpu_sync {
 	struct amdgpu_semaphore *semaphores[AMDGPU_NUM_SYNCS];
-	struct amdgpu_fence	*sync_to[AMDGPU_MAX_RINGS];
+	struct fence		*sync_to[AMDGPU_MAX_RINGS];
 	DECLARE_HASHTABLE(fences, 4);
 	struct fence	        *last_vm_update;
 };
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index 0499d2b..c6489b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -108,7 +108,6 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, struct amdgpu_sync *sync,
 {
 	struct amdgpu_sync_entry *e;
 	struct amdgpu_fence *fence;
-	struct amdgpu_fence *other;
 
 	if (!f)
 		return 0;
@@ -136,10 +135,7 @@ int amdgpu_sync_fence(struct amdgpu_device *adev, struct amdgpu_sync *sync,
 		return 0;
 	}
 
-	other = sync->sync_to[fence->ring->idx];
-	sync->sync_to[fence->ring->idx] = amdgpu_fence_ref(
-		amdgpu_fence_later(fence, other));
-	amdgpu_fence_unref(&other);
+	amdgpu_sync_keep_later(&sync->sync_to[fence->ring->idx], f);
 
 	return 0;
 }
@@ -258,11 +254,11 @@ int amdgpu_sync_wait(struct amdgpu_sync *sync)
 		return 0;
 
 	for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
-		struct amdgpu_fence *fence = sync->sync_to[i];
+		struct fence *fence = sync->sync_to[i];
 		if (!fence)
 			continue;
 
-		r = fence_wait(&fence->base, false);
+		r = fence_wait(fence, false);
 		if (r)
 			return r;
 	}
@@ -287,9 +283,14 @@ int amdgpu_sync_rings(struct amdgpu_sync *sync,
 	int i, r;
 
 	for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
-		struct amdgpu_fence *fence = sync->sync_to[i];
-		struct amdgpu_semaphore *semaphore;
 		struct amdgpu_ring *other = adev->rings[i];
+		struct amdgpu_semaphore *semaphore;
+		struct amdgpu_fence *fence;
+
+		if (!sync->sync_to[i])
+			continue;
+
+		fence = to_amdgpu_fence(sync->sync_to[i]);
 
 		/* check if we really need to sync */
 		if (!amdgpu_fence_need_sync(fence, ring))
@@ -374,7 +375,7 @@ void amdgpu_sync_free(struct amdgpu_device *adev,
 		amdgpu_semaphore_free(adev, &sync->semaphores[i], fence);
 
 	for (i = 0; i < AMDGPU_MAX_RINGS; ++i)
-		amdgpu_fence_unref(&sync->sync_to[i]);
+		fence_put(sync->sync_to[i]);
 
 	fence_put(sync->last_vm_update);
 }
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/10] drm/amdgpu: remove amdgpu_fence_ref/unref

[Christian König]

From: Christian König <christian.koenig@amd.com>

Just move the remaining users to fence_put/get.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  3 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 30 ------------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c    |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c |  3 ++-
 4 files changed, 4 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b986ea1..8305a6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -447,9 +447,6 @@ int amdgpu_fence_wait_next(struct amdgpu_ring *ring);
 int amdgpu_fence_wait_empty(struct amdgpu_ring *ring);
 unsigned amdgpu_fence_count_emitted(struct amdgpu_ring *ring);
 
-struct amdgpu_fence *amdgpu_fence_ref(struct amdgpu_fence *fence);
-void amdgpu_fence_unref(struct amdgpu_fence **fence);
-
 bool amdgpu_fence_need_sync(struct amdgpu_fence *fence,
 			    struct amdgpu_ring *ring);
 void amdgpu_fence_note_sync(struct amdgpu_fence *fence,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index c4bb282..b912539 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -417,36 +417,6 @@ int amdgpu_fence_wait_empty(struct amdgpu_ring *ring)
 }
 
 /**
- * amdgpu_fence_ref - take a ref on a fence
- *
- * @fence: amdgpu fence object
- *
- * Take a reference on a fence (all asics).
- * Returns the fence.
- */
-struct amdgpu_fence *amdgpu_fence_ref(struct amdgpu_fence *fence)
-{
-	fence_get(&fence->base);
-	return fence;
-}
-
-/**
- * amdgpu_fence_unref - remove a ref on a fence
- *
- * @fence: amdgpu fence object
- *
- * Remove a reference on a fence (all asics).
- */
-void amdgpu_fence_unref(struct amdgpu_fence **fence)
-{
-	struct amdgpu_fence *tmp = *fence;
-
-	*fence = NULL;
-	if (tmp)
-		fence_put(&tmp->base);
-}
-
-/**
  * amdgpu_fence_count_emitted - get the count of emitted fences
  *
  * @ring: ring the fence is associated with
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index aad4c1c..e659877 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -95,7 +95,8 @@ void amdgpu_ib_free(struct amdgpu_device *adev, struct amdgpu_ib *ib)
 {
 	amdgpu_sync_free(adev, &ib->sync, &ib->fence->base);
 	amdgpu_sa_bo_free(adev, &ib->sa_bo, &ib->fence->base);
-	amdgpu_fence_unref(&ib->fence);
+	if (ib->fence)
+		fence_put(&ib->fence->base);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
index 2e946b2..dcf4a8a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
@@ -54,7 +54,8 @@ static struct fence *amdgpu_sched_run_job(struct amd_sched_job *sched_job)
 		goto err;
 	}
 
-	fence = amdgpu_fence_ref(job->ibs[job->num_ibs - 1].fence);
+	fence = job->ibs[job->num_ibs - 1].fence;
+	fence_get(&fence->base);
 
 err:
 	if (job->free_job)
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/10] drm/amdgpu: remove now unused fence functions

[Christian König]

From: Christian König <christian.koenig@amd.com>

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 39 -------------------------------------
 1 file changed, 39 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 8305a6c..e0d52c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -452,45 +452,6 @@ bool amdgpu_fence_need_sync(struct amdgpu_fence *fence,
 void amdgpu_fence_note_sync(struct amdgpu_fence *fence,
 			    struct amdgpu_ring *ring);
 
-static inline struct amdgpu_fence *amdgpu_fence_later(struct amdgpu_fence *a,
-						      struct amdgpu_fence *b)
-{
-	if (!a) {
-		return b;
-	}
-
-	if (!b) {
-		return a;
-	}
-
-	BUG_ON(a->ring != b->ring);
-
-	if (a->seq > b->seq) {
-		return a;
-	} else {
-		return b;
-	}
-}
-
-static inline bool amdgpu_fence_is_earlier(struct amdgpu_fence *a,
-					   struct amdgpu_fence *b)
-{
-	if (!a) {
-		return false;
-	}
-
-	if (!b) {
-		return true;
-	}
-
-	BUG_ON(a->ring != b->ring);
-
-	return a->seq < b->seq;
-}
-
-int amdgpu_user_fence_emit(struct amdgpu_ring *ring, struct amdgpu_user_fence *user,
-			   void *owner, struct amdgpu_fence **fence);
-
 /*
  * TTM.
  */
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel