[PATCH libdrm 1/4] amdgpu: add bo from user memory to handle table

[PATCH libdrm 1/4] amdgpu: add bo from user memory to handle table

[Junwei Zhang]

When create bo from user memory, add it to handle table
for future query.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
---
 amdgpu/amdgpu_bo.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 422c7c9..b24e698 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -556,7 +556,16 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
 	bo->alloc_size = size;
 	bo->handle = args.handle;
 
-	*buf_handle = bo;
+	pthread_mutex_lock(&bo->dev->bo_table_mutex);
+	r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
+	pthread_mutex_unlock(&bo->dev->bo_table_mutex);
+
+	pthread_mutex_init(&bo->cpu_access_mutex, NULL);
+
+	if (r)
+		amdgpu_bo_free(bo);
+	else
+		*buf_handle = bo;
 
 	return r;
 }
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 2/4] amdgpu: add count for handle table

[Junwei Zhang]

count indicats the total number of key in handle table
max_key becomes the max value of key

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
---
 amdgpu/handle_table.c | 18 +++++++++++-------
 amdgpu/handle_table.h |  1 +
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/amdgpu/handle_table.c b/amdgpu/handle_table.c
index 15cd476..34e8027 100644
--- a/amdgpu/handle_table.c
+++ b/amdgpu/handle_table.c
@@ -31,34 +31,37 @@
 drm_private int handle_table_insert(struct handle_table *table, uint32_t key,
 				    void *value)
 {
-	if (key >= table->max_key) {
+	if (key >= table->count) {
 		uint32_t alignment = sysconf(_SC_PAGESIZE) / sizeof(void*);
-		uint32_t max_key = ALIGN(key, alignment);
+		uint32_t count = ALIGN(key, alignment);
 		void **values;
 
-		values = realloc(table->values, max_key * sizeof(void *));
+		values = realloc(table->values, count * sizeof(void *));
 		if (!values)
 			return -ENOMEM;
 
-		memset(values + table->max_key, 0, (max_key - table->max_key) *
+		memset(values + table->count, 0, (count - table->count) *
 		       sizeof(void *));
 
-		table->max_key = max_key;
+		table->count = count;
 		table->values = values;
 	}
+	if (key > table->max_key)
+		table->max_key = key;
+
 	table->values[key] = value;
 	return 0;
 }
 
 drm_private void handle_table_remove(struct handle_table *table, uint32_t key)
 {
-	if (key < table->max_key)
+	if (key <= table->max_key)
 		table->values[key] = NULL;
 }
 
 drm_private void *handle_table_lookup(struct handle_table *table, uint32_t key)
 {
-	if (key < table->max_key)
+	if (key <= table->max_key)
 		return table->values[key];
 	else
 		return NULL;
@@ -68,5 +71,6 @@ drm_private void handle_table_fini(struct handle_table *table)
 {
 	free(table->values);
 	table->max_key = 0;
+	table->count = 0;
 	table->values = NULL;
 }
diff --git a/amdgpu/handle_table.h b/amdgpu/handle_table.h
index 461193f..007bb58 100644
--- a/amdgpu/handle_table.h
+++ b/amdgpu/handle_table.h
@@ -29,6 +29,7 @@
 
 struct handle_table {
 	uint32_t	max_key;
+	uint32_t	count;
 	void		**values;
 };
 
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Junwei Zhang]

Userspace needs to know if the user memory is from BO or malloc.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
---
 amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
 amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
index be83b45..a8c353c 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -678,6 +678,29 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
 				    amdgpu_bo_handle *buf_handle);
 
 /**
+ * Validate if the user memory comes from BO
+ *
+ * \param dev - [in] Device handle. See #amdgpu_device_initialize()
+ * \param cpu - [in] CPU address of user allocated memory which we
+ * want to map to GPU address space (make GPU accessible)
+ * (This address must be correctly aligned).
+ * \param size - [in] Size of allocation (must be correctly aligned)
+ * \param buf_handle - [out] Buffer handle for the userptr memory
+ * if the user memory is not from BO, the buf_handle will be NULL.
+ * \param offset_in_bo - [out] offset in this BO for this user memory
+ *
+ *
+ * \return   0 on success\n
+ *          <0 - Negative POSIX Error code
+ *
+*/
+int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
+				  void *cpu,
+				  uint64_t size,
+				  amdgpu_bo_handle *buf_handle,
+				  uint64_t *offset_in_bo);
+
+/**
  * Free previosuly allocated memory
  *
  * \param   dev	       - \c [in] Device handle. See #amdgpu_device_initialize()
diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index b24e698..a631050 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
 	}
 }
 
+int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
+				  void *cpu,
+				  uint64_t size,
+				  amdgpu_bo_handle *buf_handle,
+				  uint64_t *offset_in_bo)
+{
+	int i;
+	struct amdgpu_bo *bo;
+
+	if (cpu == NULL || size == 0)
+		return -EINVAL;
+
+	pthread_mutex_lock(&dev->bo_table_mutex);
+	for (i = 0; i <= dev->bo_handles.max_key; i++) {
+		bo = handle_table_lookup(&dev->bo_handles, i);
+		if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
+			continue;
+		if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + bo->alloc_size))
+			break;
+	}
+	pthread_mutex_unlock(&dev->bo_table_mutex);
+
+	if (i <= dev->bo_handles.max_key) {
+		atomic_inc(&bo->refcount);
+		*buf_handle = bo;
+		*offset_in_bo = cpu - bo->cpu_ptr;
+	} else {
+		*buf_handle = NULL;
+		*offset_in_bo = 0;
+	}
+
+	return 0;
+}
+
 int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
 				    void *cpu,
 				    uint64_t size,
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 4/4] tests/amdgpu: add test for finding bo by CPU mapping

[Junwei Zhang]

Add a test for API to query bo by CPU mapping

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 tests/amdgpu/bo_tests.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
index 9d4da4a..dc2de9b 100644
--- a/tests/amdgpu/bo_tests.c
+++ b/tests/amdgpu/bo_tests.c
@@ -27,6 +27,7 @@
 
 #include "amdgpu_test.h"
 #include "amdgpu_drm.h"
+#include "amdgpu_internal.h"
 
 #define BUFFER_SIZE (4*1024)
 #define BUFFER_ALIGN (4*1024)
@@ -44,6 +45,7 @@ static void amdgpu_bo_metadata(void);
 static void amdgpu_bo_map_unmap(void);
 static void amdgpu_memory_alloc(void);
 static void amdgpu_mem_fail_alloc(void);
+static void amdgpu_bo_find_by_cpu_mapping(void);
 
 CU_TestInfo bo_tests[] = {
 	{ "Export/Import",  amdgpu_bo_export_import },
@@ -51,6 +53,7 @@ CU_TestInfo bo_tests[] = {
 	{ "CPU map/unmap",  amdgpu_bo_map_unmap },
 	{ "Memory alloc Test",  amdgpu_memory_alloc },
 	{ "Memory fail alloc Test",  amdgpu_mem_fail_alloc },
+	{ "Find bo by CPU mapping",  amdgpu_bo_find_by_cpu_mapping },
 	CU_TEST_INFO_NULL,
 };
 
@@ -262,3 +265,33 @@ static void amdgpu_mem_fail_alloc(void)
 		CU_ASSERT_EQUAL(r, 0);
 	}
 }
+
+static void amdgpu_bo_find_by_cpu_mapping(void)
+{
+	amdgpu_bo_handle bo_handle, find_bo_handle;
+	amdgpu_va_handle va_handle;
+	void *bo_cpu;
+	uint64_t bo_mc_address;
+	uint64_t offset;
+	int r;
+
+	r = amdgpu_bo_alloc_and_map(device_handle, 4096, 4096,
+				    AMDGPU_GEM_DOMAIN_GTT, 0,
+				    &bo_handle, &bo_cpu,
+				    &bo_mc_address, &va_handle);
+	CU_ASSERT_EQUAL(r, 0);
+
+	r = amdgpu_find_bo_by_cpu_mapping(device_handle,
+					  bo_cpu,
+					  4096,
+					  &find_bo_handle,
+					  &offset);
+	CU_ASSERT_EQUAL(r, 0);
+	CU_ASSERT_EQUAL(offset, 0);
+	CU_ASSERT_EQUAL(bo_handle->handle, find_bo_handle->handle);
+
+	atomic_dec(&find_bo_handle->refcount, 1);
+	r = amdgpu_bo_unmap_and_free(bo_handle, va_handle,
+				     bo_mc_address, 4096);
+	CU_ASSERT_EQUAL(r, 0);
+}
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 1/4] amdgpu: add bo from user memory to handle table

[zhoucm1]

On 2018年08月07日 15:26, Junwei Zhang wrote:
> When create bo from user memory, add it to handle table
> for future query.
>
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> ---
>   amdgpu/amdgpu_bo.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index 422c7c9..b24e698 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -556,7 +556,16 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>   	bo->alloc_size = size;
>   	bo->handle = args.handle;
>   
> -	*buf_handle = bo;
> +	pthread_mutex_lock(&bo->dev->bo_table_mutex);
> +	r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
> +	pthread_mutex_unlock(&bo->dev->bo_table_mutex);
> +
> +	pthread_mutex_init(&bo->cpu_access_mutex, NULL);
This line is nothing with patch itself, please separate from it.

Regards,
David Zhou
> +
> +	if (r)
> +		amdgpu_bo_free(bo);
> +	else
> +		*buf_handle = bo;
>   
>   	return r;
>   }

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[zhoucm1]

On 2018年08月07日 15:26, Junwei Zhang wrote:
> Userspace needs to know if the user memory is from BO or malloc.
>
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> ---
>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>   2 files changed, 57 insertions(+)
>
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index be83b45..a8c353c 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -678,6 +678,29 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>   				    amdgpu_bo_handle *buf_handle);
>   
>   /**
> + * Validate if the user memory comes from BO
> + *
> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
> + * \param cpu - [in] CPU address of user allocated memory which we
> + * want to map to GPU address space (make GPU accessible)
> + * (This address must be correctly aligned).
> + * \param size - [in] Size of allocation (must be correctly aligned)
> + * \param buf_handle - [out] Buffer handle for the userptr memory
> + * if the user memory is not from BO, the buf_handle will be NULL.
> + * \param offset_in_bo - [out] offset in this BO for this user memory
> + *
> + *
> + * \return   0 on success\n
> + *          <0 - Negative POSIX Error code
> + *
> +*/
> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
> +				  void *cpu,
> +				  uint64_t size,
> +				  amdgpu_bo_handle *buf_handle,
> +				  uint64_t *offset_in_bo);
> +
> +/**
>    * Free previosuly allocated memory
>    *
>    * \param   dev	       - \c [in] Device handle. See #amdgpu_device_initialize()
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index b24e698..a631050 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>   	}
>   }
>   
> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
> +				  void *cpu,
> +				  uint64_t size,
> +				  amdgpu_bo_handle *buf_handle,
> +				  uint64_t *offset_in_bo)
> +{
> +	int i;
> +	struct amdgpu_bo *bo;
> +
> +	if (cpu == NULL || size == 0)
> +		return -EINVAL;
> +
> +	pthread_mutex_lock(&dev->bo_table_mutex);
> +	for (i = 0; i <= dev->bo_handles.max_key; i++) {
> +		bo = handle_table_lookup(&dev->bo_handles, i);
explicit cast is encouraged, like "

bo = (struct amdgpu_bo *)handle_table_lookup(&dev->bo_handles, i);

"

otherwise, the series looks good to me.

Regards,
David Zhou
> +		if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
> +			continue;
> +		if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + bo->alloc_size))
> +			break;
> +	}
> +	pthread_mutex_unlock(&dev->bo_table_mutex);
> +
> +	if (i <= dev->bo_handles.max_key) {
> +		atomic_inc(&bo->refcount);
> +		*buf_handle = bo;
> +		*offset_in_bo = cpu - bo->cpu_ptr;
> +	} else {
> +		*buf_handle = NULL;
> +		*offset_in_bo = 0;
> +	}
> +
> +	return 0;
> +}
> +
>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>   				    void *cpu,
>   				    uint64_t size,

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Christian König]

Well NAK, that wasn't the intention of putting all BOs into the handle 
table.

You should still use the kernel implementation.

Christian.

Am 07.08.2018 um 09:26 schrieb Junwei Zhang:
> Userspace needs to know if the user memory is from BO or malloc.
>
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> ---
>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>   2 files changed, 57 insertions(+)
>
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index be83b45..a8c353c 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -678,6 +678,29 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>   				    amdgpu_bo_handle *buf_handle);
>   
>   /**
> + * Validate if the user memory comes from BO
> + *
> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
> + * \param cpu - [in] CPU address of user allocated memory which we
> + * want to map to GPU address space (make GPU accessible)
> + * (This address must be correctly aligned).
> + * \param size - [in] Size of allocation (must be correctly aligned)
> + * \param buf_handle - [out] Buffer handle for the userptr memory
> + * if the user memory is not from BO, the buf_handle will be NULL.
> + * \param offset_in_bo - [out] offset in this BO for this user memory
> + *
> + *
> + * \return   0 on success\n
> + *          <0 - Negative POSIX Error code
> + *
> +*/
> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
> +				  void *cpu,
> +				  uint64_t size,
> +				  amdgpu_bo_handle *buf_handle,
> +				  uint64_t *offset_in_bo);
> +
> +/**
>    * Free previosuly allocated memory
>    *
>    * \param   dev	       - \c [in] Device handle. See #amdgpu_device_initialize()
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index b24e698..a631050 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>   	}
>   }
>   
> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
> +				  void *cpu,
> +				  uint64_t size,
> +				  amdgpu_bo_handle *buf_handle,
> +				  uint64_t *offset_in_bo)
> +{
> +	int i;
> +	struct amdgpu_bo *bo;
> +
> +	if (cpu == NULL || size == 0)
> +		return -EINVAL;
> +
> +	pthread_mutex_lock(&dev->bo_table_mutex);
> +	for (i = 0; i <= dev->bo_handles.max_key; i++) {
> +		bo = handle_table_lookup(&dev->bo_handles, i);
> +		if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
> +			continue;
> +		if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + bo->alloc_size))
> +			break;
> +	}
> +	pthread_mutex_unlock(&dev->bo_table_mutex);
> +
> +	if (i <= dev->bo_handles.max_key) {
> +		atomic_inc(&bo->refcount);
> +		*buf_handle = bo;
> +		*offset_in_bo = cpu - bo->cpu_ptr;
> +	} else {
> +		*buf_handle = NULL;
> +		*offset_in_bo = 0;
> +	}
> +
> +	return 0;
> +}
> +
>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>   				    void *cpu,
>   				    uint64_t size,

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Zhang, Jerry (Junwei)]

On 08/07/2018 04:20 PM, Christian König wrote:
> Well NAK, that wasn't the intention of putting all BOs into the handle table.
>
> You should still use the kernel implementation.

I thought we have discussed that in below mail thread. any gap?

[PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of bo (v3)
{{{
 >> Well we could just completely drop the kernel implementation and use
 >> an userspace implementation.
 >
 > Do you mean to implement finding bo by cpu address in libdrm completely?

Yes, exactly.
}}}

Regards,
Jerry

>
> Christian.
>
> Am 07.08.2018 um 09:26 schrieb Junwei Zhang:
>> Userspace needs to know if the user memory is from BO or malloc.
>>
>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>> ---
>>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 57 insertions(+)
>>
>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>> index be83b45..a8c353c 100644
>> --- a/amdgpu/amdgpu.h
>> +++ b/amdgpu/amdgpu.h
>> @@ -678,6 +678,29 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>                       amdgpu_bo_handle *buf_handle);
>>   /**
>> + * Validate if the user memory comes from BO
>> + *
>> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
>> + * \param cpu - [in] CPU address of user allocated memory which we
>> + * want to map to GPU address space (make GPU accessible)
>> + * (This address must be correctly aligned).
>> + * \param size - [in] Size of allocation (must be correctly aligned)
>> + * \param buf_handle - [out] Buffer handle for the userptr memory
>> + * if the user memory is not from BO, the buf_handle will be NULL.
>> + * \param offset_in_bo - [out] offset in this BO for this user memory
>> + *
>> + *
>> + * \return   0 on success\n
>> + *          <0 - Negative POSIX Error code
>> + *
>> +*/
>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>> +                  void *cpu,
>> +                  uint64_t size,
>> +                  amdgpu_bo_handle *buf_handle,
>> +                  uint64_t *offset_in_bo);
>> +
>> +/**
>>    * Free previosuly allocated memory
>>    *
>>    * \param   dev           - \c [in] Device handle. See #amdgpu_device_initialize()
>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>> index b24e698..a631050 100644
>> --- a/amdgpu/amdgpu_bo.c
>> +++ b/amdgpu/amdgpu_bo.c
>> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>>       }
>>   }
>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>> +                  void *cpu,
>> +                  uint64_t size,
>> +                  amdgpu_bo_handle *buf_handle,
>> +                  uint64_t *offset_in_bo)
>> +{
>> +    int i;
>> +    struct amdgpu_bo *bo;
>> +
>> +    if (cpu == NULL || size == 0)
>> +        return -EINVAL;
>> +
>> +    pthread_mutex_lock(&dev->bo_table_mutex);
>> +    for (i = 0; i <= dev->bo_handles.max_key; i++) {
>> +        bo = handle_table_lookup(&dev->bo_handles, i);
>> +        if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
>> +            continue;
>> +        if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + bo->alloc_size))
>> +            break;
>> +    }
>> +    pthread_mutex_unlock(&dev->bo_table_mutex);
>> +
>> +    if (i <= dev->bo_handles.max_key) {
>> +        atomic_inc(&bo->refcount);
>> +        *buf_handle = bo;
>> +        *offset_in_bo = cpu - bo->cpu_ptr;
>> +    } else {
>> +        *buf_handle = NULL;
>> +        *offset_in_bo = 0;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>                       void *cpu,
>>                       uint64_t size,
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Christian König]

Am 07.08.2018 um 09:55 schrieb zhoucm1:
>
>
> On 2018年08月07日 15:26, Junwei Zhang wrote:
>> Userspace needs to know if the user memory is from BO or malloc.
>>
>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>> ---
>>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 57 insertions(+)
>>
>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>> index be83b45..a8c353c 100644
>> --- a/amdgpu/amdgpu.h
>> +++ b/amdgpu/amdgpu.h
>> @@ -678,6 +678,29 @@ int 
>> amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>                       amdgpu_bo_handle *buf_handle);
>>     /**
>> + * Validate if the user memory comes from BO
>> + *
>> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
>> + * \param cpu - [in] CPU address of user allocated memory which we
>> + * want to map to GPU address space (make GPU accessible)
>> + * (This address must be correctly aligned).
>> + * \param size - [in] Size of allocation (must be correctly aligned)
>> + * \param buf_handle - [out] Buffer handle for the userptr memory
>> + * if the user memory is not from BO, the buf_handle will be NULL.
>> + * \param offset_in_bo - [out] offset in this BO for this user memory
>> + *
>> + *
>> + * \return   0 on success\n
>> + *          <0 - Negative POSIX Error code
>> + *
>> +*/
>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>> +                  void *cpu,
>> +                  uint64_t size,
>> +                  amdgpu_bo_handle *buf_handle,
>> +                  uint64_t *offset_in_bo);
>> +
>> +/**
>>    * Free previosuly allocated memory
>>    *
>>    * \param   dev           - \c [in] Device handle. See 
>> #amdgpu_device_initialize()
>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>> index b24e698..a631050 100644
>> --- a/amdgpu/amdgpu_bo.c
>> +++ b/amdgpu/amdgpu_bo.c
>> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>>       }
>>   }
>>   +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>> +                  void *cpu,
>> +                  uint64_t size,
>> +                  amdgpu_bo_handle *buf_handle,
>> +                  uint64_t *offset_in_bo)
>> +{
>> +    int i;
>> +    struct amdgpu_bo *bo;
>> +
>> +    if (cpu == NULL || size == 0)
>> +        return -EINVAL;
>> +
>> +    pthread_mutex_lock(&dev->bo_table_mutex);
>> +    for (i = 0; i <= dev->bo_handles.max_key; i++) {
>> +        bo = handle_table_lookup(&dev->bo_handles, i);
> explicit cast is encouraged, like "
>
> bo = (struct amdgpu_bo *)handle_table_lookup(&dev->bo_handles, i);

Actually it isn't. We use kernel coding style here, so explicit casts 
from "void*" should be avoided:
> Casting the return value which is a void pointer is redundant. The 
> conversion from void pointer to any other pointer type is guaranteed 
> by the C programming language.

I already had to remove quite a bunch of explicit casts because of this, 
so please stop adding new ones.

Regards,
Christian.


>
> "
>
> otherwise, the series looks good to me.
>
> Regards,
> David Zhou
>> +        if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
>> +            continue;
>> +        if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + bo->alloc_size))
>> +            break;
>> +    }
>> +    pthread_mutex_unlock(&dev->bo_table_mutex);
>> +
>> +    if (i <= dev->bo_handles.max_key) {
>> +        atomic_inc(&bo->refcount);
>> +        *buf_handle = bo;
>> +        *offset_in_bo = cpu - bo->cpu_ptr;
>> +    } else {
>> +        *buf_handle = NULL;
>> +        *offset_in_bo = 0;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>                       void *cpu,
>>                       uint64_t size,
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Christian König]

Am 07.08.2018 um 10:28 schrieb Zhang, Jerry (Junwei):
> On 08/07/2018 04:20 PM, Christian König wrote:
>> Well NAK, that wasn't the intention of putting all BOs into the 
>> handle table.
>>
>> You should still use the kernel implementation.
>
> I thought we have discussed that in below mail thread. any gap?
>
> [PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of bo 
> (v3)
> {{{
> >> Well we could just completely drop the kernel implementation and use
> >> an userspace implementation.
> >
> > Do you mean to implement finding bo by cpu address in libdrm 
> completely?
>
> Yes, exactly.
> }}}

Ok, well that is a misunderstanding. I was just speculating about that.

On the other hand if searching all BOs for the right one is not to much 
overhead for that workaround I'm fine with it.

Just drop patch #2 in this series. All unused entries should be 
initialized to zero and walking the extra 512 isn't that much overhead.

Regards,
Christian.

>
> Regards,
> Jerry
>
>>
>> Christian.
>>
>> Am 07.08.2018 um 09:26 schrieb Junwei Zhang:
>>> Userspace needs to know if the user memory is from BO or malloc.
>>>
>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>> ---
>>>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>>>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>>>   2 files changed, 57 insertions(+)
>>>
>>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>>> index be83b45..a8c353c 100644
>>> --- a/amdgpu/amdgpu.h
>>> +++ b/amdgpu/amdgpu.h
>>> @@ -678,6 +678,29 @@ int 
>>> amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>                       amdgpu_bo_handle *buf_handle);
>>>   /**
>>> + * Validate if the user memory comes from BO
>>> + *
>>> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
>>> + * \param cpu - [in] CPU address of user allocated memory which we
>>> + * want to map to GPU address space (make GPU accessible)
>>> + * (This address must be correctly aligned).
>>> + * \param size - [in] Size of allocation (must be correctly aligned)
>>> + * \param buf_handle - [out] Buffer handle for the userptr memory
>>> + * if the user memory is not from BO, the buf_handle will be NULL.
>>> + * \param offset_in_bo - [out] offset in this BO for this user memory
>>> + *
>>> + *
>>> + * \return   0 on success\n
>>> + *          <0 - Negative POSIX Error code
>>> + *
>>> +*/
>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>> +                  void *cpu,
>>> +                  uint64_t size,
>>> +                  amdgpu_bo_handle *buf_handle,
>>> +                  uint64_t *offset_in_bo);
>>> +
>>> +/**
>>>    * Free previosuly allocated memory
>>>    *
>>>    * \param   dev           - \c [in] Device handle. See 
>>> #amdgpu_device_initialize()
>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>> index b24e698..a631050 100644
>>> --- a/amdgpu/amdgpu_bo.c
>>> +++ b/amdgpu/amdgpu_bo.c
>>> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>>>       }
>>>   }
>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>> +                  void *cpu,
>>> +                  uint64_t size,
>>> +                  amdgpu_bo_handle *buf_handle,
>>> +                  uint64_t *offset_in_bo)
>>> +{
>>> +    int i;
>>> +    struct amdgpu_bo *bo;
>>> +
>>> +    if (cpu == NULL || size == 0)
>>> +        return -EINVAL;
>>> +
>>> +    pthread_mutex_lock(&dev->bo_table_mutex);
>>> +    for (i = 0; i <= dev->bo_handles.max_key; i++) {
>>> +        bo = handle_table_lookup(&dev->bo_handles, i);
>>> +        if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
>>> +            continue;
>>> +        if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + 
>>> bo->alloc_size))
>>> +            break;
>>> +    }
>>> +    pthread_mutex_unlock(&dev->bo_table_mutex);
>>> +
>>> +    if (i <= dev->bo_handles.max_key) {
>>> +        atomic_inc(&bo->refcount);
>>> +        *buf_handle = bo;
>>> +        *offset_in_bo = cpu - bo->cpu_ptr;
>>> +    } else {
>>> +        *buf_handle = NULL;
>>> +        *offset_in_bo = 0;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>                       void *cpu,
>>>                       uint64_t size,
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Zhang, Jerry (Junwei)]

On 08/07/2018 05:33 PM, Christian König wrote:
> Am 07.08.2018 um 10:28 schrieb Zhang, Jerry (Junwei):
>> On 08/07/2018 04:20 PM, Christian König wrote:
>>> Well NAK, that wasn't the intention of putting all BOs into the handle table.
>>>
>>> You should still use the kernel implementation.
>>
>> I thought we have discussed that in below mail thread. any gap?
>>
>> [PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of bo (v3)
>> {{{
>> >> Well we could just completely drop the kernel implementation and use
>> >> an userspace implementation.
>> >
>> > Do you mean to implement finding bo by cpu address in libdrm completely?
>>
>> Yes, exactly.
>> }}}
>
> Ok, well that is a misunderstanding. I was just speculating about that.
>
> On the other hand if searching all BOs for the right one is not to much overhead for that workaround I'm fine with it.
>
> Just drop patch #2 in this series. All unused entries should be initialized to zero and walking the extra 512 isn't that much overhead.

Yes, reserve it locally for now.
May I get your RB?

Regards,
Jerry

>
> Regards,
> Christian.
>
>>
>> Regards,
>> Jerry
>>
>>>
>>> Christian.
>>>
>>> Am 07.08.2018 um 09:26 schrieb Junwei Zhang:
>>>> Userspace needs to know if the user memory is from BO or malloc.
>>>>
>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>> ---
>>>>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>>>>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>>>>   2 files changed, 57 insertions(+)
>>>>
>>>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>>>> index be83b45..a8c353c 100644
>>>> --- a/amdgpu/amdgpu.h
>>>> +++ b/amdgpu/amdgpu.h
>>>> @@ -678,6 +678,29 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>>                       amdgpu_bo_handle *buf_handle);
>>>>   /**
>>>> + * Validate if the user memory comes from BO
>>>> + *
>>>> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
>>>> + * \param cpu - [in] CPU address of user allocated memory which we
>>>> + * want to map to GPU address space (make GPU accessible)
>>>> + * (This address must be correctly aligned).
>>>> + * \param size - [in] Size of allocation (must be correctly aligned)
>>>> + * \param buf_handle - [out] Buffer handle for the userptr memory
>>>> + * if the user memory is not from BO, the buf_handle will be NULL.
>>>> + * \param offset_in_bo - [out] offset in this BO for this user memory
>>>> + *
>>>> + *
>>>> + * \return   0 on success\n
>>>> + *          <0 - Negative POSIX Error code
>>>> + *
>>>> +*/
>>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>>> +                  void *cpu,
>>>> +                  uint64_t size,
>>>> +                  amdgpu_bo_handle *buf_handle,
>>>> +                  uint64_t *offset_in_bo);
>>>> +
>>>> +/**
>>>>    * Free previosuly allocated memory
>>>>    *
>>>>    * \param   dev           - \c [in] Device handle. See #amdgpu_device_initialize()
>>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>>> index b24e698..a631050 100644
>>>> --- a/amdgpu/amdgpu_bo.c
>>>> +++ b/amdgpu/amdgpu_bo.c
>>>> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>>>>       }
>>>>   }
>>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>>> +                  void *cpu,
>>>> +                  uint64_t size,
>>>> +                  amdgpu_bo_handle *buf_handle,
>>>> +                  uint64_t *offset_in_bo)
>>>> +{
>>>> +    int i;
>>>> +    struct amdgpu_bo *bo;
>>>> +
>>>> +    if (cpu == NULL || size == 0)
>>>> +        return -EINVAL;
>>>> +
>>>> +    pthread_mutex_lock(&dev->bo_table_mutex);
>>>> +    for (i = 0; i <= dev->bo_handles.max_key; i++) {
>>>> +        bo = handle_table_lookup(&dev->bo_handles, i);
>>>> +        if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
>>>> +            continue;
>>>> +        if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + bo->alloc_size))
>>>> +            break;
>>>> +    }
>>>> +    pthread_mutex_unlock(&dev->bo_table_mutex);
>>>> +
>>>> +    if (i <= dev->bo_handles.max_key) {
>>>> +        atomic_inc(&bo->refcount);
>>>> +        *buf_handle = bo;
>>>> +        *offset_in_bo = cpu - bo->cpu_ptr;
>>>> +    } else {
>>>> +        *buf_handle = NULL;
>>>> +        *offset_in_bo = 0;
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>>                       void *cpu,
>>>>                       uint64_t size,
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 1/4] amdgpu: add bo from user memory to handle table

[Zhang, Jerry (Junwei)]

On 08/07/2018 03:51 PM, zhoucm1 wrote:
>
>
> On 2018年08月07日 15:26, Junwei Zhang wrote:
>> When create bo from user memory, add it to handle table
>> for future query.
>>
>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>> ---
>>   amdgpu/amdgpu_bo.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>> index 422c7c9..b24e698 100644
>> --- a/amdgpu/amdgpu_bo.c
>> +++ b/amdgpu/amdgpu_bo.c
>> @@ -556,7 +556,16 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>       bo->alloc_size = size;
>>       bo->handle = args.handle;
>> -    *buf_handle = bo;
>> +    pthread_mutex_lock(&bo->dev->bo_table_mutex);
>> +    r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
>> +    pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>> +
>> +    pthread_mutex_init(&bo->cpu_access_mutex, NULL);
> This line is nothing with patch itself, please separate from it.

We may not add it for user mem bo, not sure if user would access that bo by cpu mapping.
so add that at the same time.

Do you mean add it in another patch?

Jerry

>
> Regards,
> David Zhou
>> +
>> +    if (r)
>> +        amdgpu_bo_free(bo);
>> +    else
>> +        *buf_handle = bo;
>>       return r;
>>   }
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Christian König]

Am 07.08.2018 um 11:52 schrieb Zhang, Jerry (Junwei):
> On 08/07/2018 05:33 PM, Christian König wrote:
>> Am 07.08.2018 um 10:28 schrieb Zhang, Jerry (Junwei):
>>> On 08/07/2018 04:20 PM, Christian König wrote:
>>>> Well NAK, that wasn't the intention of putting all BOs into the 
>>>> handle table.
>>>>
>>>> You should still use the kernel implementation.
>>>
>>> I thought we have discussed that in below mail thread. any gap?
>>>
>>> [PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of 
>>> bo (v3)
>>> {{{
>>> >> Well we could just completely drop the kernel implementation and use
>>> >> an userspace implementation.
>>> >
>>> > Do you mean to implement finding bo by cpu address in libdrm 
>>> completely?
>>>
>>> Yes, exactly.
>>> }}}
>>
>> Ok, well that is a misunderstanding. I was just speculating about that.
>>
>> On the other hand if searching all BOs for the right one is not to 
>> much overhead for that workaround I'm fine with it.
>>
>> Just drop patch #2 in this series. All unused entries should be 
>> initialized to zero and walking the extra 512 isn't that much overhead.
>
> Yes, reserve it locally for now.
> May I get your RB?

I've got a few minor comments on the last patch as well.

Give me a moment to figure out how to push to libdrm master repostory 
now (they migrated to github and my old account doesn't work any more).

After that I can comment on your patches.

Thanks,
Christian.

>
> Regards,
> Jerry
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>> Jerry
>>>
>>>>
>>>> Christian.
>>>>
>>>> Am 07.08.2018 um 09:26 schrieb Junwei Zhang:
>>>>> Userspace needs to know if the user memory is from BO or malloc.
>>>>>
>>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>>> ---
>>>>>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>>>>>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>   2 files changed, 57 insertions(+)
>>>>>
>>>>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>>>>> index be83b45..a8c353c 100644
>>>>> --- a/amdgpu/amdgpu.h
>>>>> +++ b/amdgpu/amdgpu.h
>>>>> @@ -678,6 +678,29 @@ int 
>>>>> amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>>>                       amdgpu_bo_handle *buf_handle);
>>>>>   /**
>>>>> + * Validate if the user memory comes from BO
>>>>> + *
>>>>> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
>>>>> + * \param cpu - [in] CPU address of user allocated memory which we
>>>>> + * want to map to GPU address space (make GPU accessible)
>>>>> + * (This address must be correctly aligned).
>>>>> + * \param size - [in] Size of allocation (must be correctly aligned)
>>>>> + * \param buf_handle - [out] Buffer handle for the userptr memory
>>>>> + * if the user memory is not from BO, the buf_handle will be NULL.
>>>>> + * \param offset_in_bo - [out] offset in this BO for this user 
>>>>> memory
>>>>> + *
>>>>> + *
>>>>> + * \return   0 on success\n
>>>>> + *          <0 - Negative POSIX Error code
>>>>> + *
>>>>> +*/
>>>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>>>> +                  void *cpu,
>>>>> +                  uint64_t size,
>>>>> +                  amdgpu_bo_handle *buf_handle,
>>>>> +                  uint64_t *offset_in_bo);
>>>>> +
>>>>> +/**
>>>>>    * Free previosuly allocated memory
>>>>>    *
>>>>>    * \param   dev           - \c [in] Device handle. See 
>>>>> #amdgpu_device_initialize()
>>>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>>>> index b24e698..a631050 100644
>>>>> --- a/amdgpu/amdgpu_bo.c
>>>>> +++ b/amdgpu/amdgpu_bo.c
>>>>> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>>>>>       }
>>>>>   }
>>>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>>>> +                  void *cpu,
>>>>> +                  uint64_t size,
>>>>> +                  amdgpu_bo_handle *buf_handle,
>>>>> +                  uint64_t *offset_in_bo)
>>>>> +{
>>>>> +    int i;
>>>>> +    struct amdgpu_bo *bo;
>>>>> +
>>>>> +    if (cpu == NULL || size == 0)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    pthread_mutex_lock(&dev->bo_table_mutex);
>>>>> +    for (i = 0; i <= dev->bo_handles.max_key; i++) {
>>>>> +        bo = handle_table_lookup(&dev->bo_handles, i);
>>>>> +        if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
>>>>> +            continue;
>>>>> +        if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + 
>>>>> bo->alloc_size))
>>>>> +            break;
>>>>> +    }
>>>>> +    pthread_mutex_unlock(&dev->bo_table_mutex);
>>>>> +
>>>>> +    if (i <= dev->bo_handles.max_key) {
>>>>> +        atomic_inc(&bo->refcount);
>>>>> +        *buf_handle = bo;
>>>>> +        *offset_in_bo = cpu - bo->cpu_ptr;
>>>>> +    } else {
>>>>> +        *buf_handle = NULL;
>>>>> +        *offset_in_bo = 0;
>>>>> +    }
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>>>                       void *cpu,
>>>>>                       uint64_t size,
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 1/4] amdgpu: add bo from user memory to handle table

[Christian König]

Am 07.08.2018 um 11:54 schrieb Zhang, Jerry (Junwei):
> On 08/07/2018 03:51 PM, zhoucm1 wrote:
>>
>>
>> On 2018年08月07日 15:26, Junwei Zhang wrote:
>>> When create bo from user memory, add it to handle table
>>> for future query.
>>>
>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>> ---
>>>   amdgpu/amdgpu_bo.c | 11 ++++++++++-
>>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>> index 422c7c9..b24e698 100644
>>> --- a/amdgpu/amdgpu_bo.c
>>> +++ b/amdgpu/amdgpu_bo.c
>>> @@ -556,7 +556,16 @@ int 
>>> amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>       bo->alloc_size = size;
>>>       bo->handle = args.handle;
>>> -    *buf_handle = bo;
>>> +    pthread_mutex_lock(&bo->dev->bo_table_mutex);
>>> +    r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
>>> +    pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>>> +
>>> +    pthread_mutex_init(&bo->cpu_access_mutex, NULL);
>> This line is nothing with patch itself, please separate from it.
>
> We may not add it for user mem bo, not sure if user would access that 
> bo by cpu mapping.
> so add that at the same time.
>
> Do you mean add it in another patch?

I'm ok with adding it in this patch, cause it is a necessary fix.

But we might have another patch to move common BO field initialization 
in a separate function, e.g. to avoid duplicating that stuff to often.

Anyway this patch is Reviewed-by: Christian König 
<christian.koenig@amd.com> for now.

Christian.

>
> Jerry
>
>>
>> Regards,
>> David Zhou
>>> +
>>> +    if (r)
>>> +        amdgpu_bo_free(bo);
>>> +    else
>>> +        *buf_handle = bo;
>>>       return r;
>>>   }
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[zhoucm1]

On 2018年08月07日 17:30, Christian König wrote:
> Am 07.08.2018 um 09:55 schrieb zhoucm1:
>>
>>
>> On 2018年08月07日 15:26, Junwei Zhang wrote:
>>> Userspace needs to know if the user memory is from BO or malloc.
>>>
>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>> ---
>>>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>>>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>>>   2 files changed, 57 insertions(+)
>>>
>>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>>> index be83b45..a8c353c 100644
>>> --- a/amdgpu/amdgpu.h
>>> +++ b/amdgpu/amdgpu.h
>>> @@ -678,6 +678,29 @@ int 
>>> amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>                       amdgpu_bo_handle *buf_handle);
>>>     /**
>>> + * Validate if the user memory comes from BO
>>> + *
>>> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
>>> + * \param cpu - [in] CPU address of user allocated memory which we
>>> + * want to map to GPU address space (make GPU accessible)
>>> + * (This address must be correctly aligned).
>>> + * \param size - [in] Size of allocation (must be correctly aligned)
>>> + * \param buf_handle - [out] Buffer handle for the userptr memory
>>> + * if the user memory is not from BO, the buf_handle will be NULL.
>>> + * \param offset_in_bo - [out] offset in this BO for this user memory
>>> + *
>>> + *
>>> + * \return   0 on success\n
>>> + *          <0 - Negative POSIX Error code
>>> + *
>>> +*/
>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>> +                  void *cpu,
>>> +                  uint64_t size,
>>> +                  amdgpu_bo_handle *buf_handle,
>>> +                  uint64_t *offset_in_bo);
>>> +
>>> +/**
>>>    * Free previosuly allocated memory
>>>    *
>>>    * \param   dev           - \c [in] Device handle. See 
>>> #amdgpu_device_initialize()
>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>> index b24e698..a631050 100644
>>> --- a/amdgpu/amdgpu_bo.c
>>> +++ b/amdgpu/amdgpu_bo.c
>>> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>>>       }
>>>   }
>>>   +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>> +                  void *cpu,
>>> +                  uint64_t size,
>>> +                  amdgpu_bo_handle *buf_handle,
>>> +                  uint64_t *offset_in_bo)
>>> +{
>>> +    int i;
>>> +    struct amdgpu_bo *bo;
>>> +
>>> +    if (cpu == NULL || size == 0)
>>> +        return -EINVAL;
>>> +
>>> +    pthread_mutex_lock(&dev->bo_table_mutex);
>>> +    for (i = 0; i <= dev->bo_handles.max_key; i++) {
>>> +        bo = handle_table_lookup(&dev->bo_handles, i);
>> explicit cast is encouraged, like "
>>
>> bo = (struct amdgpu_bo *)handle_table_lookup(&dev->bo_handles, i);
>
> Actually it isn't. We use kernel coding style here, so explicit casts 
> from "void*" should be avoided:
>> Casting the return value which is a void pointer is redundant. The 
>> conversion from void pointer to any other pointer type is guaranteed 
>> by the C programming language.
understand, personally, I still like explicit cast, which read easily.

David
>
> I already had to remove quite a bunch of explicit casts because of 
> this, so please stop adding new ones.
>
> Regards,
> Christian.
>
>
>>
>> "
>>
>> otherwise, the series looks good to me.
>>
>> Regards,
>> David Zhou
>>> +        if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
>>> +            continue;
>>> +        if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + 
>>> bo->alloc_size))
>>> +            break;
>>> +    }
>>> +    pthread_mutex_unlock(&dev->bo_table_mutex);
>>> +
>>> +    if (i <= dev->bo_handles.max_key) {
>>> +        atomic_inc(&bo->refcount);
>>> +        *buf_handle = bo;
>>> +        *offset_in_bo = cpu - bo->cpu_ptr;
>>> +    } else {
>>> +        *buf_handle = NULL;
>>> +        *offset_in_bo = 0;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>                       void *cpu,
>>>                       uint64_t size,
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Zhang, Jerry (Junwei)]

On 08/07/2018 05:59 PM, Christian König wrote:
> Am 07.08.2018 um 11:52 schrieb Zhang, Jerry (Junwei):
>> On 08/07/2018 05:33 PM, Christian König wrote:
>>> Am 07.08.2018 um 10:28 schrieb Zhang, Jerry (Junwei):
>>>> On 08/07/2018 04:20 PM, Christian König wrote:
>>>>> Well NAK, that wasn't the intention of putting all BOs into the handle table.
>>>>>
>>>>> You should still use the kernel implementation.
>>>>
>>>> I thought we have discussed that in below mail thread. any gap?
>>>>
>>>> [PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of bo (v3)
>>>> {{{
>>>> >> Well we could just completely drop the kernel implementation and use
>>>> >> an userspace implementation.
>>>> >
>>>> > Do you mean to implement finding bo by cpu address in libdrm completely?
>>>>
>>>> Yes, exactly.
>>>> }}}
>>>
>>> Ok, well that is a misunderstanding. I was just speculating about that.
>>>
>>> On the other hand if searching all BOs for the right one is not to much overhead for that workaround I'm fine with it.
>>>
>>> Just drop patch #2 in this series. All unused entries should be initialized to zero and walking the extra 512 isn't that much overhead.
>>
>> Yes, reserve it locally for now.
>> May I get your RB?
>
> I've got a few minor comments on the last patch as well.
>
> Give me a moment to figure out how to push to libdrm master repostory now (they migrated to github and my old account doesn't work any more).
>
> After that I can comment on your patches.

Got that, thanks.

BTW, where is the libdrm master github repo?
I can only find it from freedesktop.

Jerry

>
> Thanks,
> Christian.
>
>>
>> Regards,
>> Jerry
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Regards,
>>>> Jerry
>>>>
>>>>>
>>>>> Christian.
>>>>>
>>>>> Am 07.08.2018 um 09:26 schrieb Junwei Zhang:
>>>>>> Userspace needs to know if the user memory is from BO or malloc.
>>>>>>
>>>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>>>> ---
>>>>>>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>>>>>>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>   2 files changed, 57 insertions(+)
>>>>>>
>>>>>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>>>>>> index be83b45..a8c353c 100644
>>>>>> --- a/amdgpu/amdgpu.h
>>>>>> +++ b/amdgpu/amdgpu.h
>>>>>> @@ -678,6 +678,29 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>>>>                       amdgpu_bo_handle *buf_handle);
>>>>>>   /**
>>>>>> + * Validate if the user memory comes from BO
>>>>>> + *
>>>>>> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
>>>>>> + * \param cpu - [in] CPU address of user allocated memory which we
>>>>>> + * want to map to GPU address space (make GPU accessible)
>>>>>> + * (This address must be correctly aligned).
>>>>>> + * \param size - [in] Size of allocation (must be correctly aligned)
>>>>>> + * \param buf_handle - [out] Buffer handle for the userptr memory
>>>>>> + * if the user memory is not from BO, the buf_handle will be NULL.
>>>>>> + * \param offset_in_bo - [out] offset in this BO for this user memory
>>>>>> + *
>>>>>> + *
>>>>>> + * \return   0 on success\n
>>>>>> + *          <0 - Negative POSIX Error code
>>>>>> + *
>>>>>> +*/
>>>>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>>>>> +                  void *cpu,
>>>>>> +                  uint64_t size,
>>>>>> +                  amdgpu_bo_handle *buf_handle,
>>>>>> +                  uint64_t *offset_in_bo);
>>>>>> +
>>>>>> +/**
>>>>>>    * Free previosuly allocated memory
>>>>>>    *
>>>>>>    * \param   dev           - \c [in] Device handle. See #amdgpu_device_initialize()
>>>>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>>>>> index b24e698..a631050 100644
>>>>>> --- a/amdgpu/amdgpu_bo.c
>>>>>> +++ b/amdgpu/amdgpu_bo.c
>>>>>> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>>>>>>       }
>>>>>>   }
>>>>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>>>>> +                  void *cpu,
>>>>>> +                  uint64_t size,
>>>>>> +                  amdgpu_bo_handle *buf_handle,
>>>>>> +                  uint64_t *offset_in_bo)
>>>>>> +{
>>>>>> +    int i;
>>>>>> +    struct amdgpu_bo *bo;
>>>>>> +
>>>>>> +    if (cpu == NULL || size == 0)
>>>>>> +        return -EINVAL;
>>>>>> +
>>>>>> +    pthread_mutex_lock(&dev->bo_table_mutex);
>>>>>> +    for (i = 0; i <= dev->bo_handles.max_key; i++) {
>>>>>> +        bo = handle_table_lookup(&dev->bo_handles, i);
>>>>>> +        if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
>>>>>> +            continue;
>>>>>> +        if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + bo->alloc_size))
>>>>>> +            break;
>>>>>> +    }
>>>>>> +    pthread_mutex_unlock(&dev->bo_table_mutex);
>>>>>> +
>>>>>> +    if (i <= dev->bo_handles.max_key) {
>>>>>> +        atomic_inc(&bo->refcount);
>>>>>> +        *buf_handle = bo;
>>>>>> +        *offset_in_bo = cpu - bo->cpu_ptr;
>>>>>> +    } else {
>>>>>> +        *buf_handle = NULL;
>>>>>> +        *offset_in_bo = 0;
>>>>>> +    }
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>>> +
>>>>>>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>>>>                       void *cpu,
>>>>>>                       uint64_t size,
>>>>>
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Christian König]

Am 07.08.2018 um 12:23 schrieb Zhang, Jerry (Junwei):
> On 08/07/2018 05:59 PM, Christian König wrote:
>> Am 07.08.2018 um 11:52 schrieb Zhang, Jerry (Junwei):
>>> On 08/07/2018 05:33 PM, Christian König wrote:
>>>> Am 07.08.2018 um 10:28 schrieb Zhang, Jerry (Junwei):
>>>>> On 08/07/2018 04:20 PM, Christian König wrote:
>>>>>> Well NAK, that wasn't the intention of putting all BOs into the 
>>>>>> handle table.
>>>>>>
>>>>>> You should still use the kernel implementation.
>>>>>
>>>>> I thought we have discussed that in below mail thread. any gap?
>>>>>
>>>>> [PATCH 1/2] drm/amdgpu: return bo itself if userptr is cpu addr of 
>>>>> bo (v3)
>>>>> {{{
>>>>> >> Well we could just completely drop the kernel implementation 
>>>>> and use
>>>>> >> an userspace implementation.
>>>>> >
>>>>> > Do you mean to implement finding bo by cpu address in libdrm 
>>>>> completely?
>>>>>
>>>>> Yes, exactly.
>>>>> }}}
>>>>
>>>> Ok, well that is a misunderstanding. I was just speculating about 
>>>> that.
>>>>
>>>> On the other hand if searching all BOs for the right one is not to 
>>>> much overhead for that workaround I'm fine with it.
>>>>
>>>> Just drop patch #2 in this series. All unused entries should be 
>>>> initialized to zero and walking the extra 512 isn't that much 
>>>> overhead.
>>>
>>> Yes, reserve it locally for now.
>>> May I get your RB?
>>
>> I've got a few minor comments on the last patch as well.
>>
>> Give me a moment to figure out how to push to libdrm master repostory 
>> now (they migrated to github and my old account doesn't work any more).
>>
>> After that I can comment on your patches.
>
> Got that, thanks.
>
> BTW, where is the libdrm master github repo?

Yeah, well exactly that was the problem the address had changed.

The new one is ssh://git@gitlab.freedesktop.org:mesa/drm.git

Regards,
Christian.

> I can only find it from freedesktop.
>
> Jerry
>
>>
>> Thanks,
>> Christian.
>>
>>>
>>> Regards,
>>> Jerry
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Regards,
>>>>> Jerry
>>>>>
>>>>>>
>>>>>> Christian.
>>>>>>
>>>>>> Am 07.08.2018 um 09:26 schrieb Junwei Zhang:
>>>>>>> Userspace needs to know if the user memory is from BO or malloc.
>>>>>>>
>>>>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>>>>> ---
>>>>>>>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>>>>>>>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>>   2 files changed, 57 insertions(+)
>>>>>>>
>>>>>>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>>>>>>> index be83b45..a8c353c 100644
>>>>>>> --- a/amdgpu/amdgpu.h
>>>>>>> +++ b/amdgpu/amdgpu.h
>>>>>>> @@ -678,6 +678,29 @@ int 
>>>>>>> amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>>>>>                       amdgpu_bo_handle *buf_handle);
>>>>>>>   /**
>>>>>>> + * Validate if the user memory comes from BO
>>>>>>> + *
>>>>>>> + * \param dev - [in] Device handle. See 
>>>>>>> #amdgpu_device_initialize()
>>>>>>> + * \param cpu - [in] CPU address of user allocated memory which we
>>>>>>> + * want to map to GPU address space (make GPU accessible)
>>>>>>> + * (This address must be correctly aligned).
>>>>>>> + * \param size - [in] Size of allocation (must be correctly 
>>>>>>> aligned)
>>>>>>> + * \param buf_handle - [out] Buffer handle for the userptr memory
>>>>>>> + * if the user memory is not from BO, the buf_handle will be NULL.
>>>>>>> + * \param offset_in_bo - [out] offset in this BO for this user 
>>>>>>> memory
>>>>>>> + *
>>>>>>> + *
>>>>>>> + * \return   0 on success\n
>>>>>>> + *          <0 - Negative POSIX Error code
>>>>>>> + *
>>>>>>> +*/
>>>>>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>>>>>> +                  void *cpu,
>>>>>>> +                  uint64_t size,
>>>>>>> +                  amdgpu_bo_handle *buf_handle,
>>>>>>> +                  uint64_t *offset_in_bo);
>>>>>>> +
>>>>>>> +/**
>>>>>>>    * Free previosuly allocated memory
>>>>>>>    *
>>>>>>>    * \param   dev           - \c [in] Device handle. See 
>>>>>>> #amdgpu_device_initialize()
>>>>>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>>>>>> index b24e698..a631050 100644
>>>>>>> --- a/amdgpu/amdgpu_bo.c
>>>>>>> +++ b/amdgpu/amdgpu_bo.c
>>>>>>> @@ -529,6 +529,40 @@ int 
>>>>>>> amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>>>>>>>       }
>>>>>>>   }
>>>>>>> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
>>>>>>> +                  void *cpu,
>>>>>>> +                  uint64_t size,
>>>>>>> +                  amdgpu_bo_handle *buf_handle,
>>>>>>> +                  uint64_t *offset_in_bo)
>>>>>>> +{
>>>>>>> +    int i;
>>>>>>> +    struct amdgpu_bo *bo;
>>>>>>> +
>>>>>>> +    if (cpu == NULL || size == 0)
>>>>>>> +        return -EINVAL;
>>>>>>> +
>>>>>>> +    pthread_mutex_lock(&dev->bo_table_mutex);
>>>>>>> +    for (i = 0; i <= dev->bo_handles.max_key; i++) {
>>>>>>> +        bo = handle_table_lookup(&dev->bo_handles, i);
>>>>>>> +        if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
>>>>>>> +            continue;
>>>>>>> +        if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + 
>>>>>>> bo->alloc_size))
>>>>>>> +            break;
>>>>>>> +    }
>>>>>>> + pthread_mutex_unlock(&dev->bo_table_mutex);
>>>>>>> +
>>>>>>> +    if (i <= dev->bo_handles.max_key) {
>>>>>>> +        atomic_inc(&bo->refcount);
>>>>>>> +        *buf_handle = bo;
>>>>>>> +        *offset_in_bo = cpu - bo->cpu_ptr;
>>>>>>> +    } else {
>>>>>>> +        *buf_handle = NULL;
>>>>>>> +        *offset_in_bo = 0;
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>>>>>>>                       void *cpu,
>>>>>>>                       uint64_t size,
>>>>>>
>>>>>> _______________________________________________
>>>>>> amd-gfx mailing list
>>>>>> amd-gfx@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 3/4] amdgpu: add a function to find bo by cpu mapping

[Christian König]

Am 07.08.2018 um 09:26 schrieb Junwei Zhang:
> Userspace needs to know if the user memory is from BO or malloc.
>
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> ---
>   amdgpu/amdgpu.h    | 23 +++++++++++++++++++++++
>   amdgpu/amdgpu_bo.c | 34 ++++++++++++++++++++++++++++++++++
>   2 files changed, 57 insertions(+)
>
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index be83b45..a8c353c 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -678,6 +678,29 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>   				    amdgpu_bo_handle *buf_handle);
>   
>   /**
> + * Validate if the user memory comes from BO
> + *
> + * \param dev - [in] Device handle. See #amdgpu_device_initialize()
> + * \param cpu - [in] CPU address of user allocated memory which we
> + * want to map to GPU address space (make GPU accessible)
> + * (This address must be correctly aligned).
> + * \param size - [in] Size of allocation (must be correctly aligned)
> + * \param buf_handle - [out] Buffer handle for the userptr memory
> + * if the user memory is not from BO, the buf_handle will be NULL.
> + * \param offset_in_bo - [out] offset in this BO for this user memory
> + *
> + *
> + * \return   0 on success\n
> + *          <0 - Negative POSIX Error code
> + *
> +*/
> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
> +				  void *cpu,
> +				  uint64_t size,
> +				  amdgpu_bo_handle *buf_handle,
> +				  uint64_t *offset_in_bo);
> +
> +/**
>    * Free previosuly allocated memory
>    *
>    * \param   dev	       - \c [in] Device handle. See #amdgpu_device_initialize()
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index b24e698..a631050 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -529,6 +529,40 @@ int amdgpu_bo_wait_for_idle(amdgpu_bo_handle bo,
>   	}
>   }
>   
> +int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle dev,
> +				  void *cpu,
> +				  uint64_t size,
> +				  amdgpu_bo_handle *buf_handle,
> +				  uint64_t *offset_in_bo)
> +{
> +	int i;
> +	struct amdgpu_bo *bo;
> +
> +	if (cpu == NULL || size == 0)
> +		return -EINVAL;
> +
> +	pthread_mutex_lock(&dev->bo_table_mutex);
> +	for (i = 0; i <= dev->bo_handles.max_key; i++) {
> +		bo = handle_table_lookup(&dev->bo_handles, i);
> +		if (!bo || !bo->cpu_ptr || size > bo->alloc_size)
> +			continue;
> +		if (cpu >= bo->cpu_ptr && cpu < (bo->cpu_ptr + bo->alloc_size))
> +			break;
> +	}
> +	pthread_mutex_unlock(&dev->bo_table_mutex);
> +
> +	if (i <= dev->bo_handles.max_key) {

This check needs to be inside the lock or otherwise max_key could change 
at the same time as you do the check.

Apart from that this looks good to me if you can live with the extra 
overhead of scanning all BOs.

Regards,
Christian.

> +		atomic_inc(&bo->refcount);
> +		*buf_handle = bo;
> +		*offset_in_bo = cpu - bo->cpu_ptr;
> +	} else {
> +		*buf_handle = NULL;
> +		*offset_in_bo = 0;
> +	}
> +
> +	return 0;
> +}
> +
>   int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
>   				    void *cpu,
>   				    uint64_t size,

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 1/4] amdgpu: add bo from user memory to handle table

[Junwei Zhang]

When create bo from user memory, add it to handle table
for future query.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 amdgpu/amdgpu_bo.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 422c7c9..b24e698 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -556,7 +556,16 @@ int amdgpu_create_bo_from_user_mem(amdgpu_device_handle dev,
 	bo->alloc_size = size;
 	bo->handle = args.handle;
 
-	*buf_handle = bo;
+	pthread_mutex_lock(&bo->dev->bo_table_mutex);
+	r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
+	pthread_mutex_unlock(&bo->dev->bo_table_mutex);
+
+	pthread_mutex_init(&bo->cpu_access_mutex, NULL);
+
+	if (r)
+		amdgpu_bo_free(bo);
+	else
+		*buf_handle = bo;
 
 	return r;
 }
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx