* [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
@ 2019-01-06 9:46 Bas Nieuwenhuizen
2019-01-06 20:23 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Bas Nieuwenhuizen @ 2019-01-06 9:46 UTC (permalink / raw)
To: dri-devel
For radv we want to be able to pass in a master fd and be sure that
the created libdrm_amdgpu device also uses that master fd, so we can
use it for prioritized submission.
radv does all interaction with other APIs/processes with dma-bufs,
so we should not need the functionality in libdrm_amdgpu to only have
a single fd for a device in the process.
Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
amdgpu/amdgpu-symbol-check | 1 +
amdgpu/amdgpu.h | 37 ++++++++++++++++++++++++
amdgpu/amdgpu_device.c | 59 ++++++++++++++++++++++++--------------
3 files changed, 76 insertions(+), 21 deletions(-)
diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
index 6f5e0f95..bbf48985 100755
--- a/amdgpu/amdgpu-symbol-check
+++ b/amdgpu/amdgpu-symbol-check
@@ -56,6 +56,7 @@ amdgpu_cs_wait_fences
amdgpu_cs_wait_semaphore
amdgpu_device_deinitialize
amdgpu_device_initialize
+amdgpu_device_initialize2
amdgpu_find_bo_by_cpu_mapping
amdgpu_get_marketing_name
amdgpu_query_buffer_size_alignment
diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
index dc51659a..e5ed39bb 100644
--- a/amdgpu/amdgpu.h
+++ b/amdgpu/amdgpu.h
@@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip;
*/
#define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0)
+/**
+ * Uses in amdgpu_device_initialize2(), meaning that the passed in fd should
+ * not be deduplicated against other libdrm_amdgpu devices referring to the
+ * same kernel device.
+ */
+#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0)
+
/*--------------------------------------------------------------------------*/
/* ----------------------------- Enums ------------------------------------ */
/*--------------------------------------------------------------------------*/
@@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd,
uint32_t *minor_version,
amdgpu_device_handle *device_handle);
+/**
+ *
+ * \param fd - \c [in] File descriptor for AMD GPU device
+ * received previously as the result of
+ * e.g. drmOpen() call.
+ * For legacy fd type, the DRI2/DRI3
+ * authentication should be done before
+ * calling this function.
+ * \param flags - \c [in] Bitmask of flags for device creation.
+ * \param major_version - \c [out] Major version of library. It is assumed
+ * that adding new functionality will cause
+ * increase in major version
+ * \param minor_version - \c [out] Minor version of library
+ * \param device_handle - \c [out] Pointer to opaque context which should
+ * be passed as the first parameter on each
+ * API call
+ *
+ *
+ * \return 0 on success\n
+ * <0 - Negative POSIX Error code
+ *
+ *
+ * \sa amdgpu_device_deinitialize()
+*/
+int amdgpu_device_initialize2(int fd,
+ uint32_t flags,
+ uint32_t *major_version,
+ uint32_t *minor_version,
+ amdgpu_device_handle *device_handle);
+
/**
*
* When access to such library does not needed any more the special
diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index 362494b1..b4bf3f76 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -100,7 +100,8 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
pthread_mutex_lock(&fd_mutex);
while (*node != dev && (*node)->next)
node = &(*node)->next;
- *node = (*node)->next;
+ if (*node == dev)
+ *node = (*node)->next;
pthread_mutex_unlock(&fd_mutex);
close(dev->fd);
@@ -144,6 +145,16 @@ drm_public int amdgpu_device_initialize(int fd,
uint32_t *major_version,
uint32_t *minor_version,
amdgpu_device_handle *device_handle)
+{
+ return amdgpu_device_initialize2(fd, 0, major_version, minor_version,
+ device_handle);
+}
+
+drm_public int amdgpu_device_initialize2(int fd,
+ uint32_t flags,
+ uint32_t *major_version,
+ uint32_t *minor_version,
+ amdgpu_device_handle *device_handle)
{
struct amdgpu_device *dev;
drmVersionPtr version;
@@ -164,26 +175,28 @@ drm_public int amdgpu_device_initialize(int fd,
return r;
}
- for (dev = fd_list; dev; dev = dev->next)
- if (fd_compare(dev->fd, fd) == 0)
- break;
-
- if (dev) {
- r = amdgpu_get_auth(dev->fd, &flag_authexist);
- if (r) {
- fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
- __func__, r);
+ if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
+ for (dev = fd_list; dev; dev = dev->next)
+ if (fd_compare(dev->fd, fd) == 0)
+ break;
+
+ if (dev) {
+ r = amdgpu_get_auth(dev->fd, &flag_authexist);
+ if (r) {
+ fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
+ __func__, r);
+ pthread_mutex_unlock(&fd_mutex);
+ return r;
+ }
+ if ((flag_auth) && (!flag_authexist)) {
+ dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
+ }
+ *major_version = dev->major_version;
+ *minor_version = dev->minor_version;
+ amdgpu_device_reference(device_handle, dev);
pthread_mutex_unlock(&fd_mutex);
- return r;
- }
- if ((flag_auth) && (!flag_authexist)) {
- dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
+ return 0;
}
- *major_version = dev->major_version;
- *minor_version = dev->minor_version;
- amdgpu_device_reference(device_handle, dev);
- pthread_mutex_unlock(&fd_mutex);
- return 0;
}
dev = calloc(1, sizeof(struct amdgpu_device));
@@ -265,8 +278,12 @@ drm_public int amdgpu_device_initialize(int fd,
*major_version = dev->major_version;
*minor_version = dev->minor_version;
*device_handle = dev;
- dev->next = fd_list;
- fd_list = dev;
+
+ if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
+ dev->next = fd_list;
+ fd_list = dev;
+ }
+
pthread_mutex_unlock(&fd_mutex);
return 0;
--
2.19.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
2019-01-06 9:46 [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds Bas Nieuwenhuizen
@ 2019-01-06 20:23 ` Christian König
2019-01-06 20:29 ` Bas Nieuwenhuizen
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2019-01-06 20:23 UTC (permalink / raw)
To: Bas Nieuwenhuizen, dri-devel
Am 06.01.19 um 10:46 schrieb Bas Nieuwenhuizen:
> For radv we want to be able to pass in a master fd and be sure that
> the created libdrm_amdgpu device also uses that master fd, so we can
> use it for prioritized submission.
>
> radv does all interaction with other APIs/processes with dma-bufs,
> so we should not need the functionality in libdrm_amdgpu to only have
> a single fd for a device in the process.
>
> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Well NAK.
This breaks a couple of design assumptions we used for the kernel driver
and is strongly discouraged.
Instead radv should not use the master fd for command submission.
Regards,
Christian.
> ---
> amdgpu/amdgpu-symbol-check | 1 +
> amdgpu/amdgpu.h | 37 ++++++++++++++++++++++++
> amdgpu/amdgpu_device.c | 59 ++++++++++++++++++++++++--------------
> 3 files changed, 76 insertions(+), 21 deletions(-)
>
> diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
> index 6f5e0f95..bbf48985 100755
> --- a/amdgpu/amdgpu-symbol-check
> +++ b/amdgpu/amdgpu-symbol-check
> @@ -56,6 +56,7 @@ amdgpu_cs_wait_fences
> amdgpu_cs_wait_semaphore
> amdgpu_device_deinitialize
> amdgpu_device_initialize
> +amdgpu_device_initialize2
> amdgpu_find_bo_by_cpu_mapping
> amdgpu_get_marketing_name
> amdgpu_query_buffer_size_alignment
> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> index dc51659a..e5ed39bb 100644
> --- a/amdgpu/amdgpu.h
> +++ b/amdgpu/amdgpu.h
> @@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip;
> */
> #define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0)
>
> +/**
> + * Uses in amdgpu_device_initialize2(), meaning that the passed in fd should
> + * not be deduplicated against other libdrm_amdgpu devices referring to the
> + * same kernel device.
> + */
> +#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0)
> +
> /*--------------------------------------------------------------------------*/
> /* ----------------------------- Enums ------------------------------------ */
> /*--------------------------------------------------------------------------*/
> @@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd,
> uint32_t *minor_version,
> amdgpu_device_handle *device_handle);
>
> +/**
> + *
> + * \param fd - \c [in] File descriptor for AMD GPU device
> + * received previously as the result of
> + * e.g. drmOpen() call.
> + * For legacy fd type, the DRI2/DRI3
> + * authentication should be done before
> + * calling this function.
> + * \param flags - \c [in] Bitmask of flags for device creation.
> + * \param major_version - \c [out] Major version of library. It is assumed
> + * that adding new functionality will cause
> + * increase in major version
> + * \param minor_version - \c [out] Minor version of library
> + * \param device_handle - \c [out] Pointer to opaque context which should
> + * be passed as the first parameter on each
> + * API call
> + *
> + *
> + * \return 0 on success\n
> + * <0 - Negative POSIX Error code
> + *
> + *
> + * \sa amdgpu_device_deinitialize()
> +*/
> +int amdgpu_device_initialize2(int fd,
> + uint32_t flags,
> + uint32_t *major_version,
> + uint32_t *minor_version,
> + amdgpu_device_handle *device_handle);
> +
> /**
> *
> * When access to such library does not needed any more the special
> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
> index 362494b1..b4bf3f76 100644
> --- a/amdgpu/amdgpu_device.c
> +++ b/amdgpu/amdgpu_device.c
> @@ -100,7 +100,8 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
> pthread_mutex_lock(&fd_mutex);
> while (*node != dev && (*node)->next)
> node = &(*node)->next;
> - *node = (*node)->next;
> + if (*node == dev)
> + *node = (*node)->next;
> pthread_mutex_unlock(&fd_mutex);
>
> close(dev->fd);
> @@ -144,6 +145,16 @@ drm_public int amdgpu_device_initialize(int fd,
> uint32_t *major_version,
> uint32_t *minor_version,
> amdgpu_device_handle *device_handle)
> +{
> + return amdgpu_device_initialize2(fd, 0, major_version, minor_version,
> + device_handle);
> +}
> +
> +drm_public int amdgpu_device_initialize2(int fd,
> + uint32_t flags,
> + uint32_t *major_version,
> + uint32_t *minor_version,
> + amdgpu_device_handle *device_handle)
> {
> struct amdgpu_device *dev;
> drmVersionPtr version;
> @@ -164,26 +175,28 @@ drm_public int amdgpu_device_initialize(int fd,
> return r;
> }
>
> - for (dev = fd_list; dev; dev = dev->next)
> - if (fd_compare(dev->fd, fd) == 0)
> - break;
> -
> - if (dev) {
> - r = amdgpu_get_auth(dev->fd, &flag_authexist);
> - if (r) {
> - fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
> - __func__, r);
> + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
> + for (dev = fd_list; dev; dev = dev->next)
> + if (fd_compare(dev->fd, fd) == 0)
> + break;
> +
> + if (dev) {
> + r = amdgpu_get_auth(dev->fd, &flag_authexist);
> + if (r) {
> + fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
> + __func__, r);
> + pthread_mutex_unlock(&fd_mutex);
> + return r;
> + }
> + if ((flag_auth) && (!flag_authexist)) {
> + dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> + }
> + *major_version = dev->major_version;
> + *minor_version = dev->minor_version;
> + amdgpu_device_reference(device_handle, dev);
> pthread_mutex_unlock(&fd_mutex);
> - return r;
> - }
> - if ((flag_auth) && (!flag_authexist)) {
> - dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> + return 0;
> }
> - *major_version = dev->major_version;
> - *minor_version = dev->minor_version;
> - amdgpu_device_reference(device_handle, dev);
> - pthread_mutex_unlock(&fd_mutex);
> - return 0;
> }
>
> dev = calloc(1, sizeof(struct amdgpu_device));
> @@ -265,8 +278,12 @@ drm_public int amdgpu_device_initialize(int fd,
> *major_version = dev->major_version;
> *minor_version = dev->minor_version;
> *device_handle = dev;
> - dev->next = fd_list;
> - fd_list = dev;
> +
> + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
> + dev->next = fd_list;
> + fd_list = dev;
> + }
> +
> pthread_mutex_unlock(&fd_mutex);
>
> return 0;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
2019-01-06 20:23 ` Christian König
@ 2019-01-06 20:29 ` Bas Nieuwenhuizen
2019-01-07 12:23 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Bas Nieuwenhuizen @ 2019-01-06 20:29 UTC (permalink / raw)
To: christian.koenig; +Cc: ML dri-devel
On Sun, Jan 6, 2019 at 9:23 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 06.01.19 um 10:46 schrieb Bas Nieuwenhuizen:
> > For radv we want to be able to pass in a master fd and be sure that
> > the created libdrm_amdgpu device also uses that master fd, so we can
> > use it for prioritized submission.
> >
> > radv does all interaction with other APIs/processes with dma-bufs,
> > so we should not need the functionality in libdrm_amdgpu to only have
> > a single fd for a device in the process.
> >
> > Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>
> Well NAK.
>
> This breaks a couple of design assumptions we used for the kernel driver
> and is strongly discouraged.
What assumptions are these? As far as I can tell everything is per drm
fd, not process?
>
> Instead radv should not use the master fd for command submission.
High priority submission needs to be done through a master fd, so not
using a master fd is not an option ...
>
> Regards,
> Christian.
>
>
> > ---
> > amdgpu/amdgpu-symbol-check | 1 +
> > amdgpu/amdgpu.h | 37 ++++++++++++++++++++++++
> > amdgpu/amdgpu_device.c | 59 ++++++++++++++++++++++++--------------
> > 3 files changed, 76 insertions(+), 21 deletions(-)
> >
> > diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
> > index 6f5e0f95..bbf48985 100755
> > --- a/amdgpu/amdgpu-symbol-check
> > +++ b/amdgpu/amdgpu-symbol-check
> > @@ -56,6 +56,7 @@ amdgpu_cs_wait_fences
> > amdgpu_cs_wait_semaphore
> > amdgpu_device_deinitialize
> > amdgpu_device_initialize
> > +amdgpu_device_initialize2
> > amdgpu_find_bo_by_cpu_mapping
> > amdgpu_get_marketing_name
> > amdgpu_query_buffer_size_alignment
> > diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> > index dc51659a..e5ed39bb 100644
> > --- a/amdgpu/amdgpu.h
> > +++ b/amdgpu/amdgpu.h
> > @@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip;
> > */
> > #define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0)
> >
> > +/**
> > + * Uses in amdgpu_device_initialize2(), meaning that the passed in fd should
> > + * not be deduplicated against other libdrm_amdgpu devices referring to the
> > + * same kernel device.
> > + */
> > +#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0)
> > +
> > /*--------------------------------------------------------------------------*/
> > /* ----------------------------- Enums ------------------------------------ */
> > /*--------------------------------------------------------------------------*/
> > @@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd,
> > uint32_t *minor_version,
> > amdgpu_device_handle *device_handle);
> >
> > +/**
> > + *
> > + * \param fd - \c [in] File descriptor for AMD GPU device
> > + * received previously as the result of
> > + * e.g. drmOpen() call.
> > + * For legacy fd type, the DRI2/DRI3
> > + * authentication should be done before
> > + * calling this function.
> > + * \param flags - \c [in] Bitmask of flags for device creation.
> > + * \param major_version - \c [out] Major version of library. It is assumed
> > + * that adding new functionality will cause
> > + * increase in major version
> > + * \param minor_version - \c [out] Minor version of library
> > + * \param device_handle - \c [out] Pointer to opaque context which should
> > + * be passed as the first parameter on each
> > + * API call
> > + *
> > + *
> > + * \return 0 on success\n
> > + * <0 - Negative POSIX Error code
> > + *
> > + *
> > + * \sa amdgpu_device_deinitialize()
> > +*/
> > +int amdgpu_device_initialize2(int fd,
> > + uint32_t flags,
> > + uint32_t *major_version,
> > + uint32_t *minor_version,
> > + amdgpu_device_handle *device_handle);
> > +
> > /**
> > *
> > * When access to such library does not needed any more the special
> > diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
> > index 362494b1..b4bf3f76 100644
> > --- a/amdgpu/amdgpu_device.c
> > +++ b/amdgpu/amdgpu_device.c
> > @@ -100,7 +100,8 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
> > pthread_mutex_lock(&fd_mutex);
> > while (*node != dev && (*node)->next)
> > node = &(*node)->next;
> > - *node = (*node)->next;
> > + if (*node == dev)
> > + *node = (*node)->next;
> > pthread_mutex_unlock(&fd_mutex);
> >
> > close(dev->fd);
> > @@ -144,6 +145,16 @@ drm_public int amdgpu_device_initialize(int fd,
> > uint32_t *major_version,
> > uint32_t *minor_version,
> > amdgpu_device_handle *device_handle)
> > +{
> > + return amdgpu_device_initialize2(fd, 0, major_version, minor_version,
> > + device_handle);
> > +}
> > +
> > +drm_public int amdgpu_device_initialize2(int fd,
> > + uint32_t flags,
> > + uint32_t *major_version,
> > + uint32_t *minor_version,
> > + amdgpu_device_handle *device_handle)
> > {
> > struct amdgpu_device *dev;
> > drmVersionPtr version;
> > @@ -164,26 +175,28 @@ drm_public int amdgpu_device_initialize(int fd,
> > return r;
> > }
> >
> > - for (dev = fd_list; dev; dev = dev->next)
> > - if (fd_compare(dev->fd, fd) == 0)
> > - break;
> > -
> > - if (dev) {
> > - r = amdgpu_get_auth(dev->fd, &flag_authexist);
> > - if (r) {
> > - fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
> > - __func__, r);
> > + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
> > + for (dev = fd_list; dev; dev = dev->next)
> > + if (fd_compare(dev->fd, fd) == 0)
> > + break;
> > +
> > + if (dev) {
> > + r = amdgpu_get_auth(dev->fd, &flag_authexist);
> > + if (r) {
> > + fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
> > + __func__, r);
> > + pthread_mutex_unlock(&fd_mutex);
> > + return r;
> > + }
> > + if ((flag_auth) && (!flag_authexist)) {
> > + dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> > + }
> > + *major_version = dev->major_version;
> > + *minor_version = dev->minor_version;
> > + amdgpu_device_reference(device_handle, dev);
> > pthread_mutex_unlock(&fd_mutex);
> > - return r;
> > - }
> > - if ((flag_auth) && (!flag_authexist)) {
> > - dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> > + return 0;
> > }
> > - *major_version = dev->major_version;
> > - *minor_version = dev->minor_version;
> > - amdgpu_device_reference(device_handle, dev);
> > - pthread_mutex_unlock(&fd_mutex);
> > - return 0;
> > }
> >
> > dev = calloc(1, sizeof(struct amdgpu_device));
> > @@ -265,8 +278,12 @@ drm_public int amdgpu_device_initialize(int fd,
> > *major_version = dev->major_version;
> > *minor_version = dev->minor_version;
> > *device_handle = dev;
> > - dev->next = fd_list;
> > - fd_list = dev;
> > +
> > + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
> > + dev->next = fd_list;
> > + fd_list = dev;
> > + }
> > +
> > pthread_mutex_unlock(&fd_mutex);
> >
> > return 0;
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
2019-01-06 20:29 ` Bas Nieuwenhuizen
@ 2019-01-07 12:23 ` Christian König
2019-01-07 13:05 ` Bas Nieuwenhuizen
0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2019-01-07 12:23 UTC (permalink / raw)
To: Bas Nieuwenhuizen, christian.koenig; +Cc: ML dri-devel
Am 06.01.19 um 21:29 schrieb Bas Nieuwenhuizen:
> On Sun, Jan 6, 2019 at 9:23 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 06.01.19 um 10:46 schrieb Bas Nieuwenhuizen:
>>> For radv we want to be able to pass in a master fd and be sure that
>>> the created libdrm_amdgpu device also uses that master fd, so we can
>>> use it for prioritized submission.
>>>
>>> radv does all interaction with other APIs/processes with dma-bufs,
>>> so we should not need the functionality in libdrm_amdgpu to only have
>>> a single fd for a device in the process.
>>>
>>> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>> Well NAK.
>>
>> This breaks a couple of design assumptions we used for the kernel driver
>> and is strongly discouraged.
> What assumptions are these? As far as I can tell everything is per drm
> fd, not process?
>> Instead radv should not use the master fd for command submission.
> High priority submission needs to be done through a master fd
That assumption is incorrect. See file
drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c to answer both your questions
at the same time.
Additional to the scheduler priority we really don't want more than one
fd in a process because of SVM binding overhead.
So that whole approach is a really big NAK.
Regards,
Christian.
> , so not
> using a master fd is not an option ...
>
>> Regards,
>> Christian.
>>
>>
>>> ---
>>> amdgpu/amdgpu-symbol-check | 1 +
>>> amdgpu/amdgpu.h | 37 ++++++++++++++++++++++++
>>> amdgpu/amdgpu_device.c | 59 ++++++++++++++++++++++++--------------
>>> 3 files changed, 76 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
>>> index 6f5e0f95..bbf48985 100755
>>> --- a/amdgpu/amdgpu-symbol-check
>>> +++ b/amdgpu/amdgpu-symbol-check
>>> @@ -56,6 +56,7 @@ amdgpu_cs_wait_fences
>>> amdgpu_cs_wait_semaphore
>>> amdgpu_device_deinitialize
>>> amdgpu_device_initialize
>>> +amdgpu_device_initialize2
>>> amdgpu_find_bo_by_cpu_mapping
>>> amdgpu_get_marketing_name
>>> amdgpu_query_buffer_size_alignment
>>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>>> index dc51659a..e5ed39bb 100644
>>> --- a/amdgpu/amdgpu.h
>>> +++ b/amdgpu/amdgpu.h
>>> @@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip;
>>> */
>>> #define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0)
>>>
>>> +/**
>>> + * Uses in amdgpu_device_initialize2(), meaning that the passed in fd should
>>> + * not be deduplicated against other libdrm_amdgpu devices referring to the
>>> + * same kernel device.
>>> + */
>>> +#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0)
>>> +
>>> /*--------------------------------------------------------------------------*/
>>> /* ----------------------------- Enums ------------------------------------ */
>>> /*--------------------------------------------------------------------------*/
>>> @@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd,
>>> uint32_t *minor_version,
>>> amdgpu_device_handle *device_handle);
>>>
>>> +/**
>>> + *
>>> + * \param fd - \c [in] File descriptor for AMD GPU device
>>> + * received previously as the result of
>>> + * e.g. drmOpen() call.
>>> + * For legacy fd type, the DRI2/DRI3
>>> + * authentication should be done before
>>> + * calling this function.
>>> + * \param flags - \c [in] Bitmask of flags for device creation.
>>> + * \param major_version - \c [out] Major version of library. It is assumed
>>> + * that adding new functionality will cause
>>> + * increase in major version
>>> + * \param minor_version - \c [out] Minor version of library
>>> + * \param device_handle - \c [out] Pointer to opaque context which should
>>> + * be passed as the first parameter on each
>>> + * API call
>>> + *
>>> + *
>>> + * \return 0 on success\n
>>> + * <0 - Negative POSIX Error code
>>> + *
>>> + *
>>> + * \sa amdgpu_device_deinitialize()
>>> +*/
>>> +int amdgpu_device_initialize2(int fd,
>>> + uint32_t flags,
>>> + uint32_t *major_version,
>>> + uint32_t *minor_version,
>>> + amdgpu_device_handle *device_handle);
>>> +
>>> /**
>>> *
>>> * When access to such library does not needed any more the special
>>> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
>>> index 362494b1..b4bf3f76 100644
>>> --- a/amdgpu/amdgpu_device.c
>>> +++ b/amdgpu/amdgpu_device.c
>>> @@ -100,7 +100,8 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
>>> pthread_mutex_lock(&fd_mutex);
>>> while (*node != dev && (*node)->next)
>>> node = &(*node)->next;
>>> - *node = (*node)->next;
>>> + if (*node == dev)
>>> + *node = (*node)->next;
>>> pthread_mutex_unlock(&fd_mutex);
>>>
>>> close(dev->fd);
>>> @@ -144,6 +145,16 @@ drm_public int amdgpu_device_initialize(int fd,
>>> uint32_t *major_version,
>>> uint32_t *minor_version,
>>> amdgpu_device_handle *device_handle)
>>> +{
>>> + return amdgpu_device_initialize2(fd, 0, major_version, minor_version,
>>> + device_handle);
>>> +}
>>> +
>>> +drm_public int amdgpu_device_initialize2(int fd,
>>> + uint32_t flags,
>>> + uint32_t *major_version,
>>> + uint32_t *minor_version,
>>> + amdgpu_device_handle *device_handle)
>>> {
>>> struct amdgpu_device *dev;
>>> drmVersionPtr version;
>>> @@ -164,26 +175,28 @@ drm_public int amdgpu_device_initialize(int fd,
>>> return r;
>>> }
>>>
>>> - for (dev = fd_list; dev; dev = dev->next)
>>> - if (fd_compare(dev->fd, fd) == 0)
>>> - break;
>>> -
>>> - if (dev) {
>>> - r = amdgpu_get_auth(dev->fd, &flag_authexist);
>>> - if (r) {
>>> - fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
>>> - __func__, r);
>>> + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
>>> + for (dev = fd_list; dev; dev = dev->next)
>>> + if (fd_compare(dev->fd, fd) == 0)
>>> + break;
>>> +
>>> + if (dev) {
>>> + r = amdgpu_get_auth(dev->fd, &flag_authexist);
>>> + if (r) {
>>> + fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
>>> + __func__, r);
>>> + pthread_mutex_unlock(&fd_mutex);
>>> + return r;
>>> + }
>>> + if ((flag_auth) && (!flag_authexist)) {
>>> + dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
>>> + }
>>> + *major_version = dev->major_version;
>>> + *minor_version = dev->minor_version;
>>> + amdgpu_device_reference(device_handle, dev);
>>> pthread_mutex_unlock(&fd_mutex);
>>> - return r;
>>> - }
>>> - if ((flag_auth) && (!flag_authexist)) {
>>> - dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
>>> + return 0;
>>> }
>>> - *major_version = dev->major_version;
>>> - *minor_version = dev->minor_version;
>>> - amdgpu_device_reference(device_handle, dev);
>>> - pthread_mutex_unlock(&fd_mutex);
>>> - return 0;
>>> }
>>>
>>> dev = calloc(1, sizeof(struct amdgpu_device));
>>> @@ -265,8 +278,12 @@ drm_public int amdgpu_device_initialize(int fd,
>>> *major_version = dev->major_version;
>>> *minor_version = dev->minor_version;
>>> *device_handle = dev;
>>> - dev->next = fd_list;
>>> - fd_list = dev;
>>> +
>>> + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
>>> + dev->next = fd_list;
>>> + fd_list = dev;
>>> + }
>>> +
>>> pthread_mutex_unlock(&fd_mutex);
>>>
>>> return 0;
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
2019-01-07 12:23 ` Christian König
@ 2019-01-07 13:05 ` Bas Nieuwenhuizen
2019-01-07 13:08 ` Koenig, Christian
0 siblings, 1 reply; 6+ messages in thread
From: Bas Nieuwenhuizen @ 2019-01-07 13:05 UTC (permalink / raw)
To: christian.koenig; +Cc: ML dri-devel
On Mon, Jan 7, 2019 at 1:23 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 06.01.19 um 21:29 schrieb Bas Nieuwenhuizen:
> > On Sun, Jan 6, 2019 at 9:23 PM Christian König
> > <ckoenig.leichtzumerken@gmail.com> wrote:
> >> Am 06.01.19 um 10:46 schrieb Bas Nieuwenhuizen:
> >>> For radv we want to be able to pass in a master fd and be sure that
> >>> the created libdrm_amdgpu device also uses that master fd, so we can
> >>> use it for prioritized submission.
> >>>
> >>> radv does all interaction with other APIs/processes with dma-bufs,
> >>> so we should not need the functionality in libdrm_amdgpu to only have
> >>> a single fd for a device in the process.
> >>>
> >>> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> >> Well NAK.
> >>
> >> This breaks a couple of design assumptions we used for the kernel driver
> >> and is strongly discouraged.
> > What assumptions are these? As far as I can tell everything is per drm
> > fd, not process?
> >> Instead radv should not use the master fd for command submission.
> > High priority submission needs to be done through a master fd
>
> That assumption is incorrect. See file
> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c to answer both your questions
> at the same time.
Hmm, I did not know about the AMDGPU_SCHED ioctl. That would work with
the permissions. However as it stands we cannot use it, as it is for
the entire drm-fd, not per context.
Would you be open to a patch adding a context parameter to the struct
and a AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE?
>
> Additional to the scheduler priority we really don't want more than one
> fd in a process because of SVM binding overhead.
>
> So that whole approach is a really big NAK.
>
> Regards,
> Christian.
>
> > , so not
> > using a master fd is not an option ...
> >
> >> Regards,
> >> Christian.
> >>
> >>
> >>> ---
> >>> amdgpu/amdgpu-symbol-check | 1 +
> >>> amdgpu/amdgpu.h | 37 ++++++++++++++++++++++++
> >>> amdgpu/amdgpu_device.c | 59 ++++++++++++++++++++++++--------------
> >>> 3 files changed, 76 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
> >>> index 6f5e0f95..bbf48985 100755
> >>> --- a/amdgpu/amdgpu-symbol-check
> >>> +++ b/amdgpu/amdgpu-symbol-check
> >>> @@ -56,6 +56,7 @@ amdgpu_cs_wait_fences
> >>> amdgpu_cs_wait_semaphore
> >>> amdgpu_device_deinitialize
> >>> amdgpu_device_initialize
> >>> +amdgpu_device_initialize2
> >>> amdgpu_find_bo_by_cpu_mapping
> >>> amdgpu_get_marketing_name
> >>> amdgpu_query_buffer_size_alignment
> >>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
> >>> index dc51659a..e5ed39bb 100644
> >>> --- a/amdgpu/amdgpu.h
> >>> +++ b/amdgpu/amdgpu.h
> >>> @@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip;
> >>> */
> >>> #define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0)
> >>>
> >>> +/**
> >>> + * Uses in amdgpu_device_initialize2(), meaning that the passed in fd should
> >>> + * not be deduplicated against other libdrm_amdgpu devices referring to the
> >>> + * same kernel device.
> >>> + */
> >>> +#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0)
> >>> +
> >>> /*--------------------------------------------------------------------------*/
> >>> /* ----------------------------- Enums ------------------------------------ */
> >>> /*--------------------------------------------------------------------------*/
> >>> @@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd,
> >>> uint32_t *minor_version,
> >>> amdgpu_device_handle *device_handle);
> >>>
> >>> +/**
> >>> + *
> >>> + * \param fd - \c [in] File descriptor for AMD GPU device
> >>> + * received previously as the result of
> >>> + * e.g. drmOpen() call.
> >>> + * For legacy fd type, the DRI2/DRI3
> >>> + * authentication should be done before
> >>> + * calling this function.
> >>> + * \param flags - \c [in] Bitmask of flags for device creation.
> >>> + * \param major_version - \c [out] Major version of library. It is assumed
> >>> + * that adding new functionality will cause
> >>> + * increase in major version
> >>> + * \param minor_version - \c [out] Minor version of library
> >>> + * \param device_handle - \c [out] Pointer to opaque context which should
> >>> + * be passed as the first parameter on each
> >>> + * API call
> >>> + *
> >>> + *
> >>> + * \return 0 on success\n
> >>> + * <0 - Negative POSIX Error code
> >>> + *
> >>> + *
> >>> + * \sa amdgpu_device_deinitialize()
> >>> +*/
> >>> +int amdgpu_device_initialize2(int fd,
> >>> + uint32_t flags,
> >>> + uint32_t *major_version,
> >>> + uint32_t *minor_version,
> >>> + amdgpu_device_handle *device_handle);
> >>> +
> >>> /**
> >>> *
> >>> * When access to such library does not needed any more the special
> >>> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
> >>> index 362494b1..b4bf3f76 100644
> >>> --- a/amdgpu/amdgpu_device.c
> >>> +++ b/amdgpu/amdgpu_device.c
> >>> @@ -100,7 +100,8 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
> >>> pthread_mutex_lock(&fd_mutex);
> >>> while (*node != dev && (*node)->next)
> >>> node = &(*node)->next;
> >>> - *node = (*node)->next;
> >>> + if (*node == dev)
> >>> + *node = (*node)->next;
> >>> pthread_mutex_unlock(&fd_mutex);
> >>>
> >>> close(dev->fd);
> >>> @@ -144,6 +145,16 @@ drm_public int amdgpu_device_initialize(int fd,
> >>> uint32_t *major_version,
> >>> uint32_t *minor_version,
> >>> amdgpu_device_handle *device_handle)
> >>> +{
> >>> + return amdgpu_device_initialize2(fd, 0, major_version, minor_version,
> >>> + device_handle);
> >>> +}
> >>> +
> >>> +drm_public int amdgpu_device_initialize2(int fd,
> >>> + uint32_t flags,
> >>> + uint32_t *major_version,
> >>> + uint32_t *minor_version,
> >>> + amdgpu_device_handle *device_handle)
> >>> {
> >>> struct amdgpu_device *dev;
> >>> drmVersionPtr version;
> >>> @@ -164,26 +175,28 @@ drm_public int amdgpu_device_initialize(int fd,
> >>> return r;
> >>> }
> >>>
> >>> - for (dev = fd_list; dev; dev = dev->next)
> >>> - if (fd_compare(dev->fd, fd) == 0)
> >>> - break;
> >>> -
> >>> - if (dev) {
> >>> - r = amdgpu_get_auth(dev->fd, &flag_authexist);
> >>> - if (r) {
> >>> - fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
> >>> - __func__, r);
> >>> + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
> >>> + for (dev = fd_list; dev; dev = dev->next)
> >>> + if (fd_compare(dev->fd, fd) == 0)
> >>> + break;
> >>> +
> >>> + if (dev) {
> >>> + r = amdgpu_get_auth(dev->fd, &flag_authexist);
> >>> + if (r) {
> >>> + fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
> >>> + __func__, r);
> >>> + pthread_mutex_unlock(&fd_mutex);
> >>> + return r;
> >>> + }
> >>> + if ((flag_auth) && (!flag_authexist)) {
> >>> + dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> >>> + }
> >>> + *major_version = dev->major_version;
> >>> + *minor_version = dev->minor_version;
> >>> + amdgpu_device_reference(device_handle, dev);
> >>> pthread_mutex_unlock(&fd_mutex);
> >>> - return r;
> >>> - }
> >>> - if ((flag_auth) && (!flag_authexist)) {
> >>> - dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> >>> + return 0;
> >>> }
> >>> - *major_version = dev->major_version;
> >>> - *minor_version = dev->minor_version;
> >>> - amdgpu_device_reference(device_handle, dev);
> >>> - pthread_mutex_unlock(&fd_mutex);
> >>> - return 0;
> >>> }
> >>>
> >>> dev = calloc(1, sizeof(struct amdgpu_device));
> >>> @@ -265,8 +278,12 @@ drm_public int amdgpu_device_initialize(int fd,
> >>> *major_version = dev->major_version;
> >>> *minor_version = dev->minor_version;
> >>> *device_handle = dev;
> >>> - dev->next = fd_list;
> >>> - fd_list = dev;
> >>> +
> >>> + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
> >>> + dev->next = fd_list;
> >>> + fd_list = dev;
> >>> + }
> >>> +
> >>> pthread_mutex_unlock(&fd_mutex);
> >>>
> >>> return 0;
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.
2019-01-07 13:05 ` Bas Nieuwenhuizen
@ 2019-01-07 13:08 ` Koenig, Christian
0 siblings, 0 replies; 6+ messages in thread
From: Koenig, Christian @ 2019-01-07 13:08 UTC (permalink / raw)
To: Bas Nieuwenhuizen; +Cc: ML dri-devel
Am 07.01.19 um 14:05 schrieb Bas Nieuwenhuizen:
> On Mon, Jan 7, 2019 at 1:23 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 06.01.19 um 21:29 schrieb Bas Nieuwenhuizen:
>>> On Sun, Jan 6, 2019 at 9:23 PM Christian König
>>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>> Am 06.01.19 um 10:46 schrieb Bas Nieuwenhuizen:
>>>>> For radv we want to be able to pass in a master fd and be sure that
>>>>> the created libdrm_amdgpu device also uses that master fd, so we can
>>>>> use it for prioritized submission.
>>>>>
>>>>> radv does all interaction with other APIs/processes with dma-bufs,
>>>>> so we should not need the functionality in libdrm_amdgpu to only have
>>>>> a single fd for a device in the process.
>>>>>
>>>>> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>>>> Well NAK.
>>>>
>>>> This breaks a couple of design assumptions we used for the kernel driver
>>>> and is strongly discouraged.
>>> What assumptions are these? As far as I can tell everything is per drm
>>> fd, not process?
>>>> Instead radv should not use the master fd for command submission.
>>> High priority submission needs to be done through a master fd
>> That assumption is incorrect. See file
>> drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c to answer both your questions
>> at the same time.
> Hmm, I did not know about the AMDGPU_SCHED ioctl. That would work with
> the permissions. However as it stands we cannot use it, as it is for
> the entire drm-fd, not per context.
>
> Would you be open to a patch adding a context parameter to the struct
> and a AMDGPU_SCHED_OP_CONTEXT_PRIORITY_OVERRIDE?
Certainly yeah. Overriding the whole process priority was never my
favorite approach.
Regards,
Christian.
PS: I'm at the start of a bad cold / sinusitis, so sorry if my responses
are short and maybe delayed.
>
>> Additional to the scheduler priority we really don't want more than one
>> fd in a process because of SVM binding overhead.
>>
>> So that whole approach is a really big NAK.
>>
>> Regards,
>> Christian.
>>
>>> , so not
>>> using a master fd is not an option ...
>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>
>>>>> ---
>>>>> amdgpu/amdgpu-symbol-check | 1 +
>>>>> amdgpu/amdgpu.h | 37 ++++++++++++++++++++++++
>>>>> amdgpu/amdgpu_device.c | 59 ++++++++++++++++++++++++--------------
>>>>> 3 files changed, 76 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/amdgpu/amdgpu-symbol-check b/amdgpu/amdgpu-symbol-check
>>>>> index 6f5e0f95..bbf48985 100755
>>>>> --- a/amdgpu/amdgpu-symbol-check
>>>>> +++ b/amdgpu/amdgpu-symbol-check
>>>>> @@ -56,6 +56,7 @@ amdgpu_cs_wait_fences
>>>>> amdgpu_cs_wait_semaphore
>>>>> amdgpu_device_deinitialize
>>>>> amdgpu_device_initialize
>>>>> +amdgpu_device_initialize2
>>>>> amdgpu_find_bo_by_cpu_mapping
>>>>> amdgpu_get_marketing_name
>>>>> amdgpu_query_buffer_size_alignment
>>>>> diff --git a/amdgpu/amdgpu.h b/amdgpu/amdgpu.h
>>>>> index dc51659a..e5ed39bb 100644
>>>>> --- a/amdgpu/amdgpu.h
>>>>> +++ b/amdgpu/amdgpu.h
>>>>> @@ -66,6 +66,13 @@ struct drm_amdgpu_info_hw_ip;
>>>>> */
>>>>> #define AMDGPU_QUERY_FENCE_TIMEOUT_IS_ABSOLUTE (1 << 0)
>>>>>
>>>>> +/**
>>>>> + * Uses in amdgpu_device_initialize2(), meaning that the passed in fd should
>>>>> + * not be deduplicated against other libdrm_amdgpu devices referring to the
>>>>> + * same kernel device.
>>>>> + */
>>>>> +#define AMDGPU_DEVICE_DEDICATED_FD (1 << 0)
>>>>> +
>>>>> /*--------------------------------------------------------------------------*/
>>>>> /* ----------------------------- Enums ------------------------------------ */
>>>>> /*--------------------------------------------------------------------------*/
>>>>> @@ -526,6 +533,36 @@ int amdgpu_device_initialize(int fd,
>>>>> uint32_t *minor_version,
>>>>> amdgpu_device_handle *device_handle);
>>>>>
>>>>> +/**
>>>>> + *
>>>>> + * \param fd - \c [in] File descriptor for AMD GPU device
>>>>> + * received previously as the result of
>>>>> + * e.g. drmOpen() call.
>>>>> + * For legacy fd type, the DRI2/DRI3
>>>>> + * authentication should be done before
>>>>> + * calling this function.
>>>>> + * \param flags - \c [in] Bitmask of flags for device creation.
>>>>> + * \param major_version - \c [out] Major version of library. It is assumed
>>>>> + * that adding new functionality will cause
>>>>> + * increase in major version
>>>>> + * \param minor_version - \c [out] Minor version of library
>>>>> + * \param device_handle - \c [out] Pointer to opaque context which should
>>>>> + * be passed as the first parameter on each
>>>>> + * API call
>>>>> + *
>>>>> + *
>>>>> + * \return 0 on success\n
>>>>> + * <0 - Negative POSIX Error code
>>>>> + *
>>>>> + *
>>>>> + * \sa amdgpu_device_deinitialize()
>>>>> +*/
>>>>> +int amdgpu_device_initialize2(int fd,
>>>>> + uint32_t flags,
>>>>> + uint32_t *major_version,
>>>>> + uint32_t *minor_version,
>>>>> + amdgpu_device_handle *device_handle);
>>>>> +
>>>>> /**
>>>>> *
>>>>> * When access to such library does not needed any more the special
>>>>> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
>>>>> index 362494b1..b4bf3f76 100644
>>>>> --- a/amdgpu/amdgpu_device.c
>>>>> +++ b/amdgpu/amdgpu_device.c
>>>>> @@ -100,7 +100,8 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
>>>>> pthread_mutex_lock(&fd_mutex);
>>>>> while (*node != dev && (*node)->next)
>>>>> node = &(*node)->next;
>>>>> - *node = (*node)->next;
>>>>> + if (*node == dev)
>>>>> + *node = (*node)->next;
>>>>> pthread_mutex_unlock(&fd_mutex);
>>>>>
>>>>> close(dev->fd);
>>>>> @@ -144,6 +145,16 @@ drm_public int amdgpu_device_initialize(int fd,
>>>>> uint32_t *major_version,
>>>>> uint32_t *minor_version,
>>>>> amdgpu_device_handle *device_handle)
>>>>> +{
>>>>> + return amdgpu_device_initialize2(fd, 0, major_version, minor_version,
>>>>> + device_handle);
>>>>> +}
>>>>> +
>>>>> +drm_public int amdgpu_device_initialize2(int fd,
>>>>> + uint32_t flags,
>>>>> + uint32_t *major_version,
>>>>> + uint32_t *minor_version,
>>>>> + amdgpu_device_handle *device_handle)
>>>>> {
>>>>> struct amdgpu_device *dev;
>>>>> drmVersionPtr version;
>>>>> @@ -164,26 +175,28 @@ drm_public int amdgpu_device_initialize(int fd,
>>>>> return r;
>>>>> }
>>>>>
>>>>> - for (dev = fd_list; dev; dev = dev->next)
>>>>> - if (fd_compare(dev->fd, fd) == 0)
>>>>> - break;
>>>>> -
>>>>> - if (dev) {
>>>>> - r = amdgpu_get_auth(dev->fd, &flag_authexist);
>>>>> - if (r) {
>>>>> - fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
>>>>> - __func__, r);
>>>>> + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
>>>>> + for (dev = fd_list; dev; dev = dev->next)
>>>>> + if (fd_compare(dev->fd, fd) == 0)
>>>>> + break;
>>>>> +
>>>>> + if (dev) {
>>>>> + r = amdgpu_get_auth(dev->fd, &flag_authexist);
>>>>> + if (r) {
>>>>> + fprintf(stderr, "%s: amdgpu_get_auth (2) failed (%i)\n",
>>>>> + __func__, r);
>>>>> + pthread_mutex_unlock(&fd_mutex);
>>>>> + return r;
>>>>> + }
>>>>> + if ((flag_auth) && (!flag_authexist)) {
>>>>> + dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
>>>>> + }
>>>>> + *major_version = dev->major_version;
>>>>> + *minor_version = dev->minor_version;
>>>>> + amdgpu_device_reference(device_handle, dev);
>>>>> pthread_mutex_unlock(&fd_mutex);
>>>>> - return r;
>>>>> - }
>>>>> - if ((flag_auth) && (!flag_authexist)) {
>>>>> - dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
>>>>> + return 0;
>>>>> }
>>>>> - *major_version = dev->major_version;
>>>>> - *minor_version = dev->minor_version;
>>>>> - amdgpu_device_reference(device_handle, dev);
>>>>> - pthread_mutex_unlock(&fd_mutex);
>>>>> - return 0;
>>>>> }
>>>>>
>>>>> dev = calloc(1, sizeof(struct amdgpu_device));
>>>>> @@ -265,8 +278,12 @@ drm_public int amdgpu_device_initialize(int fd,
>>>>> *major_version = dev->major_version;
>>>>> *minor_version = dev->minor_version;
>>>>> *device_handle = dev;
>>>>> - dev->next = fd_list;
>>>>> - fd_list = dev;
>>>>> +
>>>>> + if (!(flags & AMDGPU_DEVICE_DEDICATED_FD)) {
>>>>> + dev->next = fd_list;
>>>>> + fd_list = dev;
>>>>> + }
>>>>> +
>>>>> pthread_mutex_unlock(&fd_mutex);
>>>>>
>>>>> return 0;
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-07 13:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-06 9:46 [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds Bas Nieuwenhuizen
2019-01-06 20:23 ` Christian König
2019-01-06 20:29 ` Bas Nieuwenhuizen
2019-01-07 12:23 ` Christian König
2019-01-07 13:05 ` Bas Nieuwenhuizen
2019-01-07 13:08 ` Koenig, Christian
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.