Am 07.01.19 um 14:05 schrieb Bas Nieuwenhuizen:
> On Mon, Jan 7, 2019 at 1:23 PM Christian König
> <ckoenig.leichtzumerken@xxxxxxxxx> 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@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.
> 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@xxxxxxxxxxxxxxxxxxxxx
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
