[1/3] drm/syncobj: extract two helpers from drm_syncobj_create

Commit Message

Marek Olšák Sept. 12, 2017, 8:42 p.m. UTC

From: Marek Olšák <[e-mail]>

For amdgpu.

drm_syncobj_create is renamed to drm_syncobj_create_as_handle, and new
helpers drm_syncobj_create and drm_syncobj_get_handle are added.

Signed-off-by: Marek Olšák <[e-mail]>
---
 drivers/gpu/drm/drm_syncobj.c | 49 +++++++++++++++++++++++++++++++++++++++----
 include/drm/drm_syncobj.h     |  4 ++++
 2 files changed, 49 insertions(+), 4 deletions(-)

Comments

Emil Velikov Sept. 14, 2017, 7:56 a.m. UTC | #1

Hi Marek,

On 12 September 2017 at 21:42, Marek Olšák <[e-mail]> wrote:

>  include/drm/drm_syncobj.h     |  4 ++++
Please sync the header as described in
https://cgit.freedesktop.org/mesa/drm/tree/include/drm/README#n72

Tl;DR: cd .../linux; make headers_install; cp ... .../drm/include/drm;
cd .../drm; git commit -sm ".... $branch $sha1..."

Thank you
Emil

Emil Velikov Sept. 14, 2017, 8:01 a.m. UTC | #2

On 14 September 2017 at 08:56, Emil Velikov <[e-mail]> wrote:
> Hi Marek,
>
> On 12 September 2017 at 21:42, Marek Olšák <[e-mail]> wrote:
>
>>  include/drm/drm_syncobj.h     |  4 ++++
> Please sync the header as described in
> https://cgit.freedesktop.org/mesa/drm/tree/include/drm/README#n72
>
> Tl;DR: cd .../linux; make headers_install; cp ... .../drm/include/drm;
> cd .../drm; git commit -sm ".... $branch $sha1..."
>
Seems like I've replied to the wrong patch - silly me.
This was meant for the libdrm ones - suggestion still applies though.

-Emil

Marek Olšák Sept. 28, 2017, 10:06 p.m. UTC | #3

On Thu, Sep 14, 2017 at 10:01 AM, Emil Velikov <[e-mail]> wrote:
> On 14 September 2017 at 08:56, Emil Velikov <[e-mail]> wrote:
>> Hi Marek,
>>
>> On 12 September 2017 at 21:42, Marek Olšák <[e-mail]> wrote:
>>
>>>  include/drm/drm_syncobj.h     |  4 ++++
>> Please sync the header as described in
>> https://cgit.freedesktop.org/mesa/drm/tree/include/drm/README#n72
>>
>> Tl;DR: cd .../linux; make headers_install; cp ... .../drm/include/drm;
>> cd .../drm; git commit -sm ".... $branch $sha1..."
>>
> Seems like I've replied to the wrong patch - silly me.
> This was meant for the libdrm ones - suggestion still applies though.

I actually did that, but I'm on amd-staging-drm-next.

Marek

Chunming Zhou Sept. 29, 2017, 2:44 a.m. UTC | #4

On 2017年09月13日 04:42, Marek Olšák wrote:
> From: Marek Olšák <[e-mail]>
>
> For amdgpu.
>
> drm_syncobj_create is renamed to drm_syncobj_create_as_handle, and new
> helpers drm_syncobj_create and drm_syncobj_get_handle are added.
>
> Signed-off-by: Marek Olšák <[e-mail]>
> ---
>   drivers/gpu/drm/drm_syncobj.c | 49 +++++++++++++++++++++++++++++++++++++++----
>   include/drm/drm_syncobj.h     |  4 ++++
>   2 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 0422b8c..0bb1741 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -262,8 +262,14 @@ void drm_syncobj_free(struct kref *kref)
>   }
>   EXPORT_SYMBOL(drm_syncobj_free);
>   
> -static int drm_syncobj_create(struct drm_file *file_private,
> -			      u32 *handle, uint32_t flags)
You can add a new parameter for passing dma fence, then in patch3, you 
can directly use it for AMDGPU_FENCE_TO HANDLE_GET_SYNCOBJ.

otherwise the set looks good to me.

Regards,
David Zhou

> +/**
> + * drm_syncobj_create - create a new syncobj
> + * @out_syncobj: returned syncobj
> + * @flags: DRM_SYNCOBJ_* flags
> + * @fence: if non-NULL, the syncobj will represent this fence
> + */
> +int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
> +		       struct dma_fence *fence)
>   {
>   	int ret;
>   	struct drm_syncobj *syncobj;
> @@ -284,6 +290,25 @@ static int drm_syncobj_create(struct drm_file *file_private,
>   		}
>   	}
>   
> +	if (fence)
> +		drm_syncobj_replace_fence(syncobj, fence);
> +
> +	*out_syncobj = syncobj;
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_syncobj_create);
> +
> +/**
> + * drm_syncobj_get_handle - get a handle from a syncobj
> + */
> +int drm_syncobj_get_handle(struct drm_file *file_private,
> +			   struct drm_syncobj *syncobj, u32 *handle)
> +{
> +	int ret;
> +
> +	/* take a reference to put in the idr */
> +	drm_syncobj_get(syncobj);
> +
>   	idr_preload(GFP_KERNEL);
>   	spin_lock(&file_private->syncobj_table_lock);
>   	ret = idr_alloc(&file_private->syncobj_idr, syncobj, 1, 0, GFP_NOWAIT);
> @@ -299,6 +324,22 @@ static int drm_syncobj_create(struct drm_file *file_private,
>   	*handle = ret;
>   	return 0;
>   }
> +EXPORT_SYMBOL(drm_syncobj_get_handle);
> +
> +static int drm_syncobj_create_as_handle(struct drm_file *file_private,
> +					u32 *handle, uint32_t flags)
> +{
> +	int ret;
> +	struct drm_syncobj *syncobj;
> +
> +	ret = drm_syncobj_create(&syncobj, flags, NULL);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_syncobj_get_handle(file_private, syncobj, handle);
> +	drm_syncobj_put(syncobj);
> +	return ret;
> +}
>   
>   static int drm_syncobj_destroy(struct drm_file *file_private,
>   			       u32 handle)
> @@ -522,8 +563,8 @@ drm_syncobj_create_ioctl(struct drm_device *dev, void *data,
>   	if (args->flags & ~DRM_SYNCOBJ_CREATE_SIGNALED)
>   		return -EINVAL;
>   
> -	return drm_syncobj_create(file_private,
> -				  &args->handle, args->flags);
> +	return drm_syncobj_create_as_handle(file_private,
> +					    &args->handle, args->flags);
>   }
>   
>   int
> diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
> index c00fee5..e7f0035 100644
> --- a/include/drm/drm_syncobj.h
> +++ b/include/drm/drm_syncobj.h
> @@ -136,5 +136,9 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
>   			   u32 handle,
>   			   struct dma_fence **fence);
>   void drm_syncobj_free(struct kref *kref);
> +int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
> +		       struct dma_fence *fence);
> +int drm_syncobj_get_handle(struct drm_file *file_private,
> +			   struct drm_syncobj *syncobj, u32 *handle);
>   
>   #endif

Marek Olšák Sept. 29, 2017, 2:13 p.m. UTC | #5

On Fri, Sep 29, 2017 at 4:44 AM, Chunming Zhou <[e-mail]> wrote:
>
>
> On 2017年09月13日 04:42, Marek Olšák wrote:
>>
>> From: Marek Olšák <[e-mail]>
>>
>> For amdgpu.
>>
>> drm_syncobj_create is renamed to drm_syncobj_create_as_handle, and new
>> helpers drm_syncobj_create and drm_syncobj_get_handle are added.
>>
>> Signed-off-by: Marek Olšák <[e-mail]>
>> ---
>>   drivers/gpu/drm/drm_syncobj.c | 49
>> +++++++++++++++++++++++++++++++++++++++----
>>   include/drm/drm_syncobj.h     |  4 ++++
>>   2 files changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index 0422b8c..0bb1741 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -262,8 +262,14 @@ void drm_syncobj_free(struct kref *kref)
>>   }
>>   EXPORT_SYMBOL(drm_syncobj_free);
>>   -static int drm_syncobj_create(struct drm_file *file_private,
>> -                             u32 *handle, uint32_t flags)
>
> You can add a new parameter for passing dma fence, then in patch3, you can
> directly use it for AMDGPU_FENCE_TO HANDLE_GET_SYNCOBJ.
>
> otherwise the set looks good to me.

Sorry I just pushed this.

Marek

Marek Olšák Sept. 29, 2017, 2:15 p.m. UTC | #6

On Fri, Sep 29, 2017 at 4:13 PM, Marek Olšák <[e-mail]> wrote:
> On Fri, Sep 29, 2017 at 4:44 AM, Chunming Zhou <[e-mail]> wrote:
>>
>>
>> On 2017年09月13日 04:42, Marek Olšák wrote:
>>>
>>> From: Marek Olšák <[e-mail]>
>>>
>>> For amdgpu.
>>>
>>> drm_syncobj_create is renamed to drm_syncobj_create_as_handle, and new
>>> helpers drm_syncobj_create and drm_syncobj_get_handle are added.
>>>
>>> Signed-off-by: Marek Olšák <[e-mail]>
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 49
>>> +++++++++++++++++++++++++++++++++++++++----
>>>   include/drm/drm_syncobj.h     |  4 ++++
>>>   2 files changed, 49 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>> index 0422b8c..0bb1741 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -262,8 +262,14 @@ void drm_syncobj_free(struct kref *kref)
>>>   }
>>>   EXPORT_SYMBOL(drm_syncobj_free);
>>>   -static int drm_syncobj_create(struct drm_file *file_private,
>>> -                             u32 *handle, uint32_t flags)
>>
>> You can add a new parameter for passing dma fence, then in patch3, you can
>> directly use it for AMDGPU_FENCE_TO HANDLE_GET_SYNCOBJ.
>>
>> otherwise the set looks good to me.
>
> Sorry I just pushed this.

Actually, you commented on a deleted line. The function already has
dma_fence among the parameters.

Marek

Patch

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 0422b8c..0bb1741 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -262,8 +262,14 @@  void drm_syncobj_free(struct kref *kref)
 }
 EXPORT_SYMBOL(drm_syncobj_free);
 
-static int drm_syncobj_create(struct drm_file *file_private,
-			      u32 *handle, uint32_t flags)
+/**
+ * drm_syncobj_create - create a new syncobj
+ * @out_syncobj: returned syncobj
+ * @flags: DRM_SYNCOBJ_* flags
+ * @fence: if non-NULL, the syncobj will represent this fence
+ */
+int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
+		       struct dma_fence *fence)
 {
 	int ret;
 	struct drm_syncobj *syncobj;
@@ -284,6 +290,25 @@  static int drm_syncobj_create(struct drm_file *file_private,
 		}
 	}
 
+	if (fence)
+		drm_syncobj_replace_fence(syncobj, fence);
+
+	*out_syncobj = syncobj;
+	return 0;
+}
+EXPORT_SYMBOL(drm_syncobj_create);
+
+/**
+ * drm_syncobj_get_handle - get a handle from a syncobj
+ */
+int drm_syncobj_get_handle(struct drm_file *file_private,
+			   struct drm_syncobj *syncobj, u32 *handle)
+{
+	int ret;
+
+	/* take a reference to put in the idr */
+	drm_syncobj_get(syncobj);
+
 	idr_preload(GFP_KERNEL);
 	spin_lock(&file_private->syncobj_table_lock);
 	ret = idr_alloc(&file_private->syncobj_idr, syncobj, 1, 0, GFP_NOWAIT);
@@ -299,6 +324,22 @@  static int drm_syncobj_create(struct drm_file *file_private,
 	*handle = ret;
 	return 0;
 }
+EXPORT_SYMBOL(drm_syncobj_get_handle);
+
+static int drm_syncobj_create_as_handle(struct drm_file *file_private,
+					u32 *handle, uint32_t flags)
+{
+	int ret;
+	struct drm_syncobj *syncobj;
+
+	ret = drm_syncobj_create(&syncobj, flags, NULL);
+	if (ret)
+		return ret;
+
+	ret = drm_syncobj_get_handle(file_private, syncobj, handle);
+	drm_syncobj_put(syncobj);
+	return ret;
+}
 
 static int drm_syncobj_destroy(struct drm_file *file_private,
 			       u32 handle)
@@ -522,8 +563,8 @@  drm_syncobj_create_ioctl(struct drm_device *dev, void *data,
 	if (args->flags & ~DRM_SYNCOBJ_CREATE_SIGNALED)
 		return -EINVAL;
 
-	return drm_syncobj_create(file_private,
-				  &args->handle, args->flags);
+	return drm_syncobj_create_as_handle(file_private,
+					    &args->handle, args->flags);
 }
 
 int
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index c00fee5..e7f0035 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -136,5 +136,9 @@  int drm_syncobj_find_fence(struct drm_file *file_private,
 			   u32 handle,
 			   struct dma_fence **fence);
 void drm_syncobj_free(struct kref *kref);
+int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
+		       struct dma_fence *fence);
+int drm_syncobj_get_handle(struct drm_file *file_private,
+			   struct drm_syncobj *syncobj, u32 *handle);
 
 #endif