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, 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
