Re: [PATCH libdrm] amdgpu: Allow amdgpu device creation without merging fds.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 06.01.19 um 21:29 schrieb Bas Nieuwenhuizen:
On Sun, Jan 6, 2019 at 9:23 PM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux