[RFC] Exclusive gpu access for SteamVR usecases

[RFC] Exclusive gpu access for SteamVR usecases

[Andres Rodriguez]

When multiple environments are running simultaneously on a system, e.g.
an X desktop + a SteamVR game session, it may be useful to sacrifice
performance in one environment in order to boost it on the other.

This series provides a mechanism for a DRM_MASTER to provide exclusive
gpu access to a group of processes.

Note: This series is built on the assumption that the drm lease patch series
will extend DRM_MASTER status to lesees.

The libdrm we intend to provide is as follows:

/**
 * Set the priority of all contexts in a process
 *
 * This function will change the priority of all contexts owned by
 * the process identified by fd.
 *
 * \param dev             - \c [in] device handle
 * \param fd              - \c [in] fd from target process
 * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
 *
 * \return  0 on success\n
 *         <0 - Negative POSIX error code
 *
 * \notes @fd can be *any* file descriptor from the target process.
 * \notes this function requires DRM_MASTER
 */
int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
				      int fd, int32_t priority);

/**
 * Request to raise the minimum required priority to schedule a gpu job
 *
 * Submit a request to increase the minimum required priority to schedule
 * a gpu job. Once this function returns, the gpu scheduler will no longer
 * consider jobs from contexts with priority lower than @priority.
 *
 * The minimum priority considered by the scheduler will be the highest from
 * all currently active requests.
 *
 * Requests are refcounted, and must be balanced using
 * amdgpu_sched_min_priority_put()
 *
 * \param dev             - \c [in] device handle
 * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
 *
 * \return  0 on success\n
 *         <0 - Negative POSIX error code
 *
 * \notes this function requires DRM_MASTER
 */
int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
				  int32_t priority);

/**
 * Drop a request to raise the minimum required scheduler priority
 *
 * This call balances amdgpu_sched_min_priority_get()
 *
 * If no other active requests exists for @priority, the minimum required
 * priority will decay to a lower level until one is reached with an active
 * request or the lowest priority is reached.
 *
 * \param dev             - \c [in] device handle
 * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
 *
 * \return  0 on success\n
 *         <0 - Negative POSIX error code
 *
 * \notes this function requires DRM_MASTER
 */
int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
				  int32_t priority);

Using this app, VRComposer can raise the priority of the VRapp and itself. Then
it can restrict the minimum scheduler priority in order to become exclusive gpu
clients.

One of the areas I'd like feedback is the following scenario. If a VRapp opens
a new fd and creates a new context after a call to set_priority, this specific
context will be lower priority than the rest. If the minimum required priority
is then raised, it is possible that this new context will be starved and
deadlock the VRapp.

One solution I had in mind to address this situation, is to make set_priority
also raise the priority of future contexts created by the VRapp. However, that
would require keeping track of the requested priority on a per-process data
structure. The current design appears to steer clean of keeping any process
specific data, and everything instead of stored on a per-file basis. Which is
why I did not pursue this approach. But if this is something you'd like me to
implement let me know.

One could also argue that preventing an application deadlock should be handled
between the VRComposer and the VRApp. It is not the kernel's responsibility to
babysit userspace applications and prevent themselves from shooting themselves
in the foot. The same could be achieved by improper usage of shared fences
between processes.

Thoughts/feedback/comments on this issue, or others, are appreciated.

Regards,
Andres
 

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

[PATCH 1/3] drm/amdgpu: add a new scheduler priority AMD_SCHED_PRIORITY_HIGH_SW

[Andres Rodriguez]

Add a new priority level to the gpu scheduler *_HIGH_SW. This level
intends to provide elevated entity priority at the sw scheduler level
without the negative side effects of an elevated HW priority.

Some of the negative effects of HW priorities can include stealing
resources from other queues.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c       |  8 +++++---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c         | 18 ++++++------------
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  3 ++-
 include/uapi/drm/amdgpu_drm.h                 |  3 ++-
 4 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index cc00110..48d0d1e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -35,7 +35,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
 	if (priority < 0 || priority >= AMD_SCHED_PRIORITY_MAX)
 		return -EINVAL;
 
-	if (priority >= AMD_SCHED_PRIORITY_HIGH && !capable(CAP_SYS_NICE))
+	if (priority > AMD_SCHED_PRIORITY_NORMAL && !capable(CAP_SYS_NICE))
 		return -EACCES;
 
 	memset(ctx, 0, sizeof(*ctx));
@@ -201,8 +201,10 @@ static int amdgpu_ctx_query(struct amdgpu_device *adev,
 static enum amd_sched_priority amdgpu_to_sched_priority(int amdgpu_priority)
 {
 	switch (amdgpu_priority) {
-	case AMDGPU_CTX_PRIORITY_HIGH:
-		return AMD_SCHED_PRIORITY_HIGH;
+	case AMDGPU_CTX_PRIORITY_HIGH_HW:
+		return AMD_SCHED_PRIORITY_HIGH_HW;
+	case AMDGPU_CTX_PRIORITY_HIGH_SW:
+		return AMD_SCHED_PRIORITY_HIGH_SW;
 	case AMDGPU_CTX_PRIORITY_NORMAL:
 		return AMD_SCHED_PRIORITY_NORMAL;
 	case AMDGPU_CTX_PRIORITY_LOW:
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 6147c94..396d3e4 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6749,19 +6749,12 @@ static void gfx_v8_0_hqd_set_priority(struct amdgpu_device *adev,
 	mutex_lock(&adev->srbm_mutex);
 	vi_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
 
-	switch (priority) {
-	case AMD_SCHED_PRIORITY_NORMAL:
-		WREG32(mmCP_HQD_PIPE_PRIORITY, 0x0);
-		WREG32(mmCP_HQD_QUEUE_PRIORITY, 0x0);
-		break;
-	case AMD_SCHED_PRIORITY_HIGH:
+	if (priority >=	AMD_SCHED_PRIORITY_HIGH_HW) {
 		WREG32(mmCP_HQD_PIPE_PRIORITY, 0x2);
 		WREG32(mmCP_HQD_QUEUE_PRIORITY, 0xf);
-		break;
-	default:
-		WARN(1, "Attempt to set invalid SPI priority:%d for ring:%d\n",
-				priority, ring->idx);
-		break;
+	} else {
+		WREG32(mmCP_HQD_PIPE_PRIORITY, 0x0);
+		WREG32(mmCP_HQD_QUEUE_PRIORITY, 0x0);
 	}
 
 	vi_srbm_select(adev, 0, 0, 0, 0);
@@ -6776,7 +6769,8 @@ static void gfx_v8_0_ring_set_priority_compute(struct amdgpu_ring *ring,
 		return;
 
 	gfx_v8_0_hqd_set_priority(adev, ring, priority);
-	gfx_v8_0_pipe_reserve_resources(adev, ring, priority >= AMD_SCHED_PRIORITY_HIGH);
+	gfx_v8_0_pipe_reserve_resources(adev, ring,
+					priority >= AMD_SCHED_PRIORITY_HIGH_HW);
 }
 
 static void gfx_v8_0_ring_emit_fence_compute(struct amdgpu_ring *ring,
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index 46c18424..dbcaa2e 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -117,7 +117,8 @@ enum amd_sched_priority {
 	AMD_SCHED_PRIORITY_MIN,
 	AMD_SCHED_PRIORITY_LOW = AMD_SCHED_PRIORITY_MIN,
 	AMD_SCHED_PRIORITY_NORMAL,
-	AMD_SCHED_PRIORITY_HIGH,
+	AMD_SCHED_PRIORITY_HIGH_SW,
+	AMD_SCHED_PRIORITY_HIGH_HW,
 	AMD_SCHED_PRIORITY_KERNEL,
 	AMD_SCHED_PRIORITY_MAX
 };
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 88b2a52..27d0a822 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -166,7 +166,8 @@ union drm_amdgpu_bo_list {
 #define AMDGPU_CTX_PRIORITY_LOW         -1023
 #define AMDGPU_CTX_PRIORITY_NORMAL      0
 /* Selecting a priority above NORMAL requires CAP_SYS_ADMIN */
-#define AMDGPU_CTX_PRIORITY_HIGH        1023
+#define AMDGPU_CTX_PRIORITY_HIGH_SW     512
+#define AMDGPU_CTX_PRIORITY_HIGH_HW     1023
 
 struct drm_amdgpu_ctx_in {
 	/** AMDGPU_CTX_OP_* */
-- 
2.9.3

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

[PATCH 2/3] drm/amdgpu: make amdgpu_to_sched_priority detect invalid parameters

[Andres Rodriguez]

Returning invalid priorities as _NORMAL is a backwards compatibility
quirk of amdgpu_ctx_ioctl(). Move this detail one layer up where it
belongs.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c       | 8 +++++---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 48d0d1e..43fe5ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -211,7 +211,7 @@ static enum amd_sched_priority amdgpu_to_sched_priority(int amdgpu_priority)
 		return AMD_SCHED_PRIORITY_LOW;
 	default:
 		WARN(1, "Invalid context priority %d\n", amdgpu_priority);
-		return AMD_SCHED_PRIORITY_NORMAL;
+		return AMD_SCHED_PRIORITY_INVALID;
 	}
 }
 
@@ -230,8 +230,10 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 	id = args->in.ctx_id;
 	priority = amdgpu_to_sched_priority(args->in.priority);
 
-	if (priority >= AMD_SCHED_PRIORITY_MAX)
-		return -EINVAL;
+	/* For backwards compatibility reasons, we need to accept
+	 * ioctls with garbage in the priority field */
+	if (priority == AMD_SCHED_PRIORITY_INVALID)
+		priority = AMD_SCHED_PRIORITY_NORMAL;
 
 	switch (args->in.op) {
 	case AMDGPU_CTX_OP_ALLOC_CTX:
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index dbcaa2e..da040bc 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -120,7 +120,8 @@ enum amd_sched_priority {
 	AMD_SCHED_PRIORITY_HIGH_SW,
 	AMD_SCHED_PRIORITY_HIGH_HW,
 	AMD_SCHED_PRIORITY_KERNEL,
-	AMD_SCHED_PRIORITY_MAX
+	AMD_SCHED_PRIORITY_MAX,
+	AMD_SCHED_PRIORITY_INVALID = -1
 };
 
 /**
-- 
2.9.3

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

[PATCH 3/3] drm/amdgpu: add a mechanism to acquire gpu exclusivity

[Andres Rodriguez]

A DRM_MASTER may wish to restrict gpu job submission to only a limited
set of clients. To enable this use case we provide the following new
IOCTL APIs:
  * A mechanism to change a process's ctx priorities
  * A mechanism to limit the minimum priority required for the gpu
    scheduler to queue a job to the HW

This functionality is useful in VR use cases, where two compositors are
operating simultaneously, e.g. X + SteamVRComposer.

In this case SteamVRComposer can limit gpu access to itself + the
relevant clients. Once critical work is complete, and if enough time is
available until the next HMD vblank, general access to the gpu can be
restored.

The operation is limited to DRM_MASTER since it may lead to starvation.
The implementation of drm leases is required to extend DRM_MASTER
status to the SteamVRComposer.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c       |  39 ++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c       |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c     | 131 ++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h     |  34 +++++++
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c |  81 ++++++++++++++--
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  14 ++-
 include/uapi/drm/amdgpu_drm.h                 |  26 +++++
 9 files changed, 306 insertions(+), 26 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index b62d9e9..e4d3b07 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -25,7 +25,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 	amdgpu_prime.o amdgpu_vm.o amdgpu_ib.o amdgpu_pll.o \
 	amdgpu_ucode.o amdgpu_bo_list.o amdgpu_ctx.o amdgpu_sync.o \
 	amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o amdgpu_atomfirmware.o \
-	amdgpu_queue_mgr.o
+	amdgpu_queue_mgr.o amdgpu_sched.o
 
 # add asic specific block
 amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o kv_smc.o kv_dpm.o \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 3722352..9681de7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -833,6 +833,9 @@ uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx, struct amdgpu_ring *ring,
 			      struct dma_fence *fence);
 struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
 				   struct amdgpu_ring *ring, uint64_t seq);
+void amdgpu_ctx_set_priority(struct amdgpu_device *adev,
+			     struct amdgpu_ctx *ctx,
+			     enum amd_sched_priority priority);
 
 int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 		     struct drm_file *filp);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 43fe5ae..996434f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -24,6 +24,7 @@
 
 #include <drm/drmP.h>
 #include "amdgpu.h"
+#include "amdgpu_sched.h"
 
 static int amdgpu_ctx_init(struct amdgpu_device *adev,
 			   enum amd_sched_priority priority,
@@ -198,23 +199,6 @@ static int amdgpu_ctx_query(struct amdgpu_device *adev,
 	return 0;
 }
 
-static enum amd_sched_priority amdgpu_to_sched_priority(int amdgpu_priority)
-{
-	switch (amdgpu_priority) {
-	case AMDGPU_CTX_PRIORITY_HIGH_HW:
-		return AMD_SCHED_PRIORITY_HIGH_HW;
-	case AMDGPU_CTX_PRIORITY_HIGH_SW:
-		return AMD_SCHED_PRIORITY_HIGH_SW;
-	case AMDGPU_CTX_PRIORITY_NORMAL:
-		return AMD_SCHED_PRIORITY_NORMAL;
-	case AMDGPU_CTX_PRIORITY_LOW:
-		return AMD_SCHED_PRIORITY_LOW;
-	default:
-		WARN(1, "Invalid context priority %d\n", amdgpu_priority);
-		return AMD_SCHED_PRIORITY_INVALID;
-	}
-}
-
 int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 		     struct drm_file *filp)
 {
@@ -337,6 +321,27 @@ struct dma_fence *amdgpu_ctx_get_fence(struct amdgpu_ctx *ctx,
 	return fence;
 }
 
+void amdgpu_ctx_set_priority(struct amdgpu_device *adev,
+			     struct amdgpu_ctx *ctx,
+			     enum amd_sched_priority priority)
+{
+	int i;
+	struct amd_sched_rq *rq;
+	struct amd_sched_entity *entity;
+	struct amdgpu_ring *ring;
+
+	spin_lock(&ctx->ring_lock);
+	for (i = 0; i < adev->num_rings; i++) {
+		ring = adev->rings[i];
+		entity = &ctx->rings[i].entity;
+		rq = &ring->sched.sched_rq[priority];
+
+		if (ring->funcs->type != AMDGPU_RING_TYPE_KIQ)
+			amd_sched_entity_set_rq(entity, rq);
+	}
+	spin_unlock(&ctx->ring_lock);
+}
+
 void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
 {
 	mutex_init(&mgr->lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 8c26ee1..376851d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -28,6 +28,7 @@
 #include <drm/drmP.h>
 #include "amdgpu.h"
 #include <drm/amdgpu_drm.h>
+#include "amdgpu_sched.h"
 #include "amdgpu_uvd.h"
 #include "amdgpu_vce.h"
 
@@ -1009,6 +1010,7 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
 	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(AMDGPU_VM, amdgpu_vm_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(AMDGPU_SCHED, amdgpu_sched_ioctl, DRM_MASTER),
 	DRM_IOCTL_DEF_DRV(AMDGPU_BO_LIST, amdgpu_bo_list_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
 	/* KMS */
 	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
new file mode 100644
index 0000000..04e2a51
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c
@@ -0,0 +1,131 @@
+/*
+ * Copyright 2017 Valve Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors: Andres Rodriguez <andresx7@gmail.com>
+ */
+
+#include <linux/fdtable.h>
+#include <linux/pid.h>
+#include <drm/amdgpu_drm.h>
+#include "amdgpu.h"
+
+#include "amdgpu_vm.h"
+
+enum amd_sched_priority amdgpu_to_sched_priority(int amdgpu_priority)
+{
+	switch (amdgpu_priority) {
+	case AMDGPU_CTX_PRIORITY_HIGH_HW:
+		return AMD_SCHED_PRIORITY_HIGH_HW;
+	case AMDGPU_CTX_PRIORITY_HIGH_SW:
+		return AMD_SCHED_PRIORITY_HIGH_SW;
+	case AMDGPU_CTX_PRIORITY_NORMAL:
+		return AMD_SCHED_PRIORITY_NORMAL;
+	case AMDGPU_CTX_PRIORITY_LOW:
+		return AMD_SCHED_PRIORITY_LOW;
+	default:
+		WARN(1, "Invalid context priority %d\n", amdgpu_priority);
+		return AMD_SCHED_PRIORITY_INVALID;
+	}
+}
+
+static int amdgpu_sched_process_priority_set(struct amdgpu_device *adev,
+					     int fd,
+					     enum amd_sched_priority priority)
+{
+	struct file *filp = fcheck(fd);
+	struct drm_file *file;
+	struct pid *pid;
+	struct amdgpu_fpriv *fpriv;
+	struct amdgpu_ctx *ctx;
+	uint32_t id;
+
+	if (!filp)
+		return -EINVAL;
+
+	pid = get_pid(((struct drm_file *)filp->private_data)->pid);
+
+	mutex_lock(&adev->ddev->filelist_mutex);
+	list_for_each_entry(file, &adev->ddev->filelist, lhead) {
+		if (file->pid != pid)
+			continue;
+
+		fpriv = file->driver_priv;
+		idr_for_each_entry(&fpriv->ctx_mgr.ctx_handles, ctx, id)
+			amdgpu_ctx_set_priority(adev, ctx, priority);
+	}
+	mutex_unlock(&adev->ddev->filelist_mutex);
+
+	put_pid(pid);
+
+	return 0;
+}
+
+static int amdgpu_sched_min_priority_get(struct amdgpu_device *adev,
+					 enum amd_sched_priority priority)
+{
+	int i;
+
+	for (i = 0; i < adev->num_rings; i++)
+		amd_sched_min_priority_get(&adev->rings[i]->sched, priority);
+
+	return 0;
+}
+
+static int amdgpu_sched_min_priority_put(struct amdgpu_device *adev,
+					 enum amd_sched_priority priority)
+{
+	int i;
+
+	for (i = 0; i < adev->num_rings; i++)
+		amd_sched_min_priority_put(&adev->rings[i]->sched, priority);
+
+	return 0;
+}
+
+int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
+		       struct drm_file *filp)
+{
+	union drm_amdgpu_sched *args = data;
+	struct amdgpu_device *adev = dev->dev_private;
+	enum amd_sched_priority priority;
+	int r;
+
+	priority = amdgpu_to_sched_priority(args->in.priority);
+	if (args->in.reserved || priority == AMD_SCHED_PRIORITY_INVALID)
+		return -EINVAL;
+
+	switch (args->in.op) {
+	case AMDGPU_SCHED_OP_PROCESS_PRIORITY_SET:
+		r = amdgpu_sched_process_priority_set(adev, args->in.fd, priority);
+		break;
+	case AMDGPU_SCHED_OP_MIN_PRIORITY_GET:
+		r = amdgpu_sched_min_priority_get(adev, priority);
+		break;
+	case AMDGPU_SCHED_OP_MIN_PRIORITY_PUT:
+		r = amdgpu_sched_min_priority_put(adev, priority);
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+
+	return r;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h
new file mode 100644
index 0000000..b28c067
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sched.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2017 Valve Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors: Andres Rodriguez <andresx7@gmail.com>
+ */
+
+#ifndef __AMDGPU_SCHED_H__
+#define __AMDGPU_SCHED_H__
+
+#include <drm/drmP.h>
+
+enum amd_sched_priority amdgpu_to_sched_priority(int amdgpu_priority);
+int amdgpu_sched_ioctl(struct drm_device *dev, void *data,
+		       struct drm_file *filp);
+
+#endif // __AMDGPU_SCHED_H__
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 38cea6f..4f2cbe9 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -133,6 +133,7 @@ int amd_sched_entity_init(struct amd_gpu_scheduler *sched,
 	entity->rq = rq;
 	entity->sched = sched;
 
+	spin_lock_init(&entity->rq_lock);
 	spin_lock_init(&entity->queue_lock);
 	r = kfifo_alloc(&entity->job_queue, jobs * sizeof(void *), GFP_KERNEL);
 	if (r)
@@ -204,8 +205,6 @@ static bool amd_sched_entity_is_ready(struct amd_sched_entity *entity)
 void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
 			   struct amd_sched_entity *entity)
 {
-	struct amd_sched_rq *rq = entity->rq;
-
 	if (!amd_sched_entity_is_initialized(sched, entity))
 		return;
 
@@ -215,7 +214,8 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
 	*/
 	wait_event(sched->job_scheduled, amd_sched_entity_is_idle(entity));
 
-	amd_sched_rq_remove_entity(rq, entity);
+	amd_sched_entity_set_rq(entity, NULL);
+
 	kfifo_free(&entity->job_queue);
 }
 
@@ -236,6 +236,24 @@ static void amd_sched_entity_clear_dep(struct dma_fence *f, struct dma_fence_cb
 	dma_fence_put(f);
 }
 
+void amd_sched_entity_set_rq(struct amd_sched_entity *entity,
+			     struct amd_sched_rq *rq)
+{
+	if (entity->rq == rq)
+		return;
+
+	spin_lock(&entity->rq_lock);
+
+	if (entity->rq)
+		amd_sched_rq_remove_entity(entity->rq, entity);
+
+	entity->rq = rq;
+	if (rq)
+		amd_sched_rq_add_entity(rq, entity);
+
+	spin_unlock(&entity->rq_lock);
+}
+
 bool amd_sched_dependency_optimized(struct dma_fence* fence,
 				    struct amd_sched_entity *entity)
 {
@@ -333,7 +351,9 @@ static bool amd_sched_entity_in(struct amd_sched_job *sched_job)
 	/* first job wakes up scheduler */
 	if (first) {
 		/* Add the entity to the run queue */
+		spin_lock(&entity->rq_lock);
 		amd_sched_rq_add_entity(entity->rq, entity);
+		spin_unlock(&entity->rq_lock);
 		amd_sched_wakeup(sched);
 	}
 	return added;
@@ -523,18 +543,22 @@ static void amd_sched_wakeup(struct amd_gpu_scheduler *sched)
 static struct amd_sched_entity *
 amd_sched_select_entity(struct amd_gpu_scheduler *sched)
 {
-	struct amd_sched_entity *entity;
-	int i;
+	struct amd_sched_entity *entity = NULL;
+	int i, min_required_prio;
 
 	if (!amd_sched_ready(sched))
 		return NULL;
 
+	spin_lock(&sched->min_required_prio_lock);
+	min_required_prio = sched->min_required_prio;
+
 	/* Kernel run queue has higher priority than normal run queue*/
-	for (i = AMD_SCHED_PRIORITY_MAX - 1; i >= AMD_SCHED_PRIORITY_MIN; i--) {
+	for (i = AMD_SCHED_PRIORITY_MAX - 1; i >= min_required_prio; i--) {
 		entity = amd_sched_rq_select_entity(&sched->sched_rq[i]);
 		if (entity)
 			break;
 	}
+	spin_unlock(&sched->min_required_prio_lock);
 
 	return entity;
 }
@@ -638,15 +662,19 @@ int amd_sched_init(struct amd_gpu_scheduler *sched,
 	sched->hw_submission_limit = hw_submission;
 	sched->name = name;
 	sched->timeout = timeout;
-	for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++)
+	for (i = AMD_SCHED_PRIORITY_MIN; i < AMD_SCHED_PRIORITY_MAX; i++) {
 		amd_sched_rq_init(&sched->sched_rq[i]);
+		atomic_set(&sched->prio_requests[i], 0);
+	}
 
 	init_waitqueue_head(&sched->wake_up_worker);
 	init_waitqueue_head(&sched->job_scheduled);
 	INIT_LIST_HEAD(&sched->ring_mirror_list);
 	spin_lock_init(&sched->job_list_lock);
+	spin_lock_init(&sched->min_required_prio_lock);
 	atomic_set(&sched->hw_rq_count, 0);
 	atomic64_set(&sched->job_id_count, 0);
+	sched->min_required_prio = AMD_SCHED_PRIORITY_MIN;
 
 	/* Each scheduler will run on a seperate kernel thread */
 	sched->thread = kthread_run(amd_sched_main, sched, sched->name);
@@ -668,3 +696,42 @@ void amd_sched_fini(struct amd_gpu_scheduler *sched)
 	if (sched->thread)
 		kthread_stop(sched->thread);
 }
+
+void amd_sched_min_priority_get(struct amd_gpu_scheduler *sched,
+				enum amd_sched_priority priority)
+{
+	atomic_inc(&sched->prio_requests[priority]);
+
+	spin_lock(&sched->min_required_prio_lock);
+	if (priority > sched->min_required_prio)
+		sched->min_required_prio = priority;
+	spin_unlock(&sched->min_required_prio_lock);
+}
+
+void amd_sched_min_priority_put(struct amd_gpu_scheduler *sched,
+				enum amd_sched_priority priority)
+{
+	int i;
+
+	if (atomic_dec_return(&sched->prio_requests[priority]) > 0)
+		return;
+
+	if (priority == AMD_SCHED_PRIORITY_MIN)
+		return;
+
+	spin_lock(&sched->min_required_prio_lock);
+	if (sched->min_required_prio > priority)
+		goto out_unlock;
+
+	for (i = priority; i >= AMD_SCHED_PRIORITY_MIN; i--) {
+		if (i == AMD_SCHED_PRIORITY_MIN
+				|| atomic_read(&sched->prio_requests[i])) {
+			sched->min_required_prio = i;
+			break;
+		}
+	}
+	amd_sched_wakeup(sched);
+
+out_unlock:
+	spin_unlock(&sched->min_required_prio_lock);
+}
diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
index da040bc..c489708 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
@@ -38,9 +38,11 @@ struct amd_sched_rq;
 */
 struct amd_sched_entity {
 	struct list_head		list;
-	struct amd_sched_rq		*rq;
 	struct amd_gpu_scheduler	*sched;
 
+	spinlock_t			rq_lock;
+	struct amd_sched_rq		*rq;
+
 	spinlock_t			queue_lock;
 	struct kfifo                    job_queue;
 
@@ -140,12 +142,20 @@ struct amd_gpu_scheduler {
 	struct task_struct		*thread;
 	struct list_head	ring_mirror_list;
 	spinlock_t			job_list_lock;
+	atomic_t			prio_requests[AMD_SCHED_PRIORITY_MAX];
+	/* minimum priority requrired for a job to be scheduled */
+	enum amd_sched_priority		min_required_prio;
+	spinlock_t			min_required_prio_lock;
 };
 
 int amd_sched_init(struct amd_gpu_scheduler *sched,
 		   const struct amd_sched_backend_ops *ops,
 		   uint32_t hw_submission, long timeout, const char *name);
 void amd_sched_fini(struct amd_gpu_scheduler *sched);
+void amd_sched_min_priority_get(struct amd_gpu_scheduler *sched,
+				enum amd_sched_priority priority);
+void amd_sched_min_priority_put(struct amd_gpu_scheduler *sched,
+				enum amd_sched_priority priority);
 
 int amd_sched_entity_init(struct amd_gpu_scheduler *sched,
 			  struct amd_sched_entity *entity,
@@ -154,6 +164,8 @@ int amd_sched_entity_init(struct amd_gpu_scheduler *sched,
 void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
 			   struct amd_sched_entity *entity);
 void amd_sched_entity_push_job(struct amd_sched_job *sched_job);
+void amd_sched_entity_set_rq(struct amd_sched_entity *entity,
+			     struct amd_sched_rq *rq);
 
 int amd_sched_fence_slab_init(void);
 void amd_sched_fence_slab_fini(void);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 27d0a822..5afb5a8 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -52,6 +52,7 @@ extern "C" {
 #define DRM_AMDGPU_GEM_USERPTR		0x11
 #define DRM_AMDGPU_WAIT_FENCES		0x12
 #define DRM_AMDGPU_VM			0x13
+#define DRM_AMDGPU_SCHED		0x14
 
 #define DRM_IOCTL_AMDGPU_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
 #define DRM_IOCTL_AMDGPU_GEM_MMAP	DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
@@ -67,6 +68,7 @@ extern "C" {
 #define DRM_IOCTL_AMDGPU_GEM_USERPTR	DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
 #define DRM_IOCTL_AMDGPU_WAIT_FENCES	DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
 #define DRM_IOCTL_AMDGPU_VM		DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_VM, union drm_amdgpu_vm)
+#define DRM_IOCTL_AMDGPU_SCHED		DRM_IOWR(DRM_COMMAND_BASE + DRM_AMDGPU_SCHED, union drm_amdgpu_sched)
 
 #define AMDGPU_GEM_DOMAIN_CPU		0x1
 #define AMDGPU_GEM_DOMAIN_GTT		0x2
@@ -219,6 +221,30 @@ union drm_amdgpu_vm {
 	struct drm_amdgpu_vm_out out;
 };
 
+/* sched ioctl */
+#define AMDGPU_SCHED_OP_PROCESS_PRIORITY_SET	1
+#define AMDGPU_SCHED_OP_MIN_PRIORITY_GET	2
+#define AMDGPU_SCHED_OP_MIN_PRIORITY_PUT	3
+
+struct drm_amdgpu_sched_in {
+	/* AMDGPU_SCHED_OP_* */
+	__u32	op;
+	__u32	fd;
+	__s32	priority;
+	/* For future use */
+	__u32	reserved;
+};
+
+struct drm_amdgpu_sched_out {
+	/* For future use */
+	__u64	reserved;
+};
+
+union drm_amdgpu_sched {
+	struct drm_amdgpu_sched_in in;
+	struct drm_amdgpu_sched_out out;
+};
+
 /*
  * This is not a reliable API and you should expect it to fail for any
  * number of reasons and have fallback path that do not use userptr to
-- 
2.9.3

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

Re: [RFC] Exclusive gpu access for SteamVR usecases

[Mao, David]

[-- Attachment #1.1: Type: text/plain, Size: 5000 bytes --]

Hi Andres,
Why the fd is needed for this interface?
Why not just using the dev->fd instead of fd?
IIRC, if there are more than one fds opened in the process upon the same device, they will share the same amdgpu_device_handle which is guaranteed by amdgpu_device_initialize.
In other word, we should not run into the case that user creates more contexts with newly opened fd after tuning the priority of existing context in the same process unless the previous fd is closed.

Thanks.
Best Regards,
David

On 25 May 2017, at 8:00 AM, Andres Rodriguez <andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:

When multiple environments are running simultaneously on a system, e.g.
an X desktop + a SteamVR game session, it may be useful to sacrifice
performance in one environment in order to boost it on the other.

This series provides a mechanism for a DRM_MASTER to provide exclusive
gpu access to a group of processes.

Note: This series is built on the assumption that the drm lease patch series
will extend DRM_MASTER status to lesees.

The libdrm we intend to provide is as follows:

/**
* Set the priority of all contexts in a process
*
* This function will change the priority of all contexts owned by
* the process identified by fd.
*
* \param dev             - \c [in] device handle
* \param fd              - \c [in] fd from target process
* \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
*         <0 - Negative POSIX error code
*
* \notes @fd can be *any* file descriptor from the target process.
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
     int fd, int32_t priority);

/**
* Request to raise the minimum required priority to schedule a gpu job
*
* Submit a request to increase the minimum required priority to schedule
* a gpu job. Once this function returns, the gpu scheduler will no longer
* consider jobs from contexts with priority lower than @priority.
*
* The minimum priority considered by the scheduler will be the highest from
* all currently active requests.
*
* Requests are refcounted, and must be balanced using
* amdgpu_sched_min_priority_put()
*
* \param dev             - \c [in] device handle
* \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
*         <0 - Negative POSIX error code
*
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
 int32_t priority);

/**
* Drop a request to raise the minimum required scheduler priority
*
* This call balances amdgpu_sched_min_priority_get()
*
* If no other active requests exists for @priority, the minimum required
* priority will decay to a lower level until one is reached with an active
* request or the lowest priority is reached.
*
* \param dev             - \c [in] device handle
* \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
*
* \return  0 on success\n
*         <0 - Negative POSIX error code
*
* \notes this function requires DRM_MASTER
*/
int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
 int32_t priority);

Using this app, VRComposer can raise the priority of the VRapp and itself. Then
it can restrict the minimum scheduler priority in order to become exclusive gpu
clients.

One of the areas I'd like feedback is the following scenario. If a VRapp opens
a new fd and creates a new context after a call to set_priority, this specific
context will be lower priority than the rest. If the minimum required priority
is then raised, it is possible that this new context will be starved and
deadlock the VRapp.

One solution I had in mind to address this situation, is to make set_priority
also raise the priority of future contexts created by the VRapp. However, that
would require keeping track of the requested priority on a per-process data
structure. The current design appears to steer clean of keeping any process
specific data, and everything instead of stored on a per-file basis. Which is
why I did not pursue this approach. But if this is something you'd like me to
implement let me know.

One could also argue that preventing an application deadlock should be handled
between the VRComposer and the VRApp. It is not the kernel's responsibility to
babysit userspace applications and prevent themselves from shooting themselves
in the foot. The same could be achieved by improper usage of shared fences
between processes.

Thoughts/feedback/comments on this issue, or others, are appreciated.

Regards,
Andres


_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 8883 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [RFC] Exclusive gpu access for SteamVR usecases

[Andres Rodriguez]

On 2017-05-26 05:02 AM, Mao, David wrote:
> Hi Andres,
> Why the fd is needed for this interface?

The fd is used to identify the process for which we wish to raise the 
priority. It can be any fd from the target process, it doesn't have to 
be a drm file descriptor at all.

The fd is used to retrieve the (struct pid*) of the target process on 
the kernel side. In effect, it is a replacement for passing a pid number 
across process boundaries.

For reference, amdgpu_sched_process_priority_set() in patch 3

> Why not just using the dev->fd instead of
> IIRC, if there are more than one fds opened in the process upon the same 
> device, they will share the same amdgpu_device_handle which is 
> guaranteed by amdgpu_device_initialize.

Thanks for pointing that out. I wasn't aware that the amdgpu drm layer 
would always perform all command submission through the same fd 
(dev->fd) for the same amdgpu_device.

Your suggestion actually makes it a lot simpler to deal with this issue 
at a file level instead of at a process level. Since only one fd per 
device is used for command submission.

For a multi-gpu setup we would still need to share multiple fds, across 
the process boundaries.

This also helped me realize that my current implementation doesn't deal 
with multi-gpu cases correctly. As I iterate over the fds belonging to
a single drm device.

> In other word, we should not run into the case that user creates more 
> contexts with newly opened fd after tuning the priority of existing 
> context in the same process unless the previous fd is closed.
> 
> Thanks.
> Best Regards,
> David
> 
>> On 25 May 2017, at 8:00 AM, Andres Rodriguez <andresx7@gmail.com 
>> <mailto:andresx7@gmail.com>> wrote:
>>
>> When multiple environments are running simultaneously on a system, e.g.
>> an X desktop + a SteamVR game session, it may be useful to sacrifice
>> performance in one environment in order to boost it on the other.
>>
>> This series provides a mechanism for a DRM_MASTER to provide exclusive
>> gpu access to a group of processes.
>>
>> Note: This series is built on the assumption that the drm lease patch 
>> series
>> will extend DRM_MASTER status to lesees.
>>
>> The libdrm we intend to provide is as follows:
>>
>> /**
>> * Set the priority of all contexts in a process
>> *
>> * This function will change the priority of all contexts owned by
>> * the process identified by fd.
>> *
>> * \param dev             - \c [in] device handle
>> * \param fd              - \c [in] fd from target process
>> * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>> *
>> * \return  0 on success\n
>> *         <0 - Negative POSIX error code
>> *
>> * \notes @fd can be *any* file descriptor from the target process.
>> * \notes this function requires DRM_MASTER
>> */
>> int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
>>      int fd, int32_t priority);
>>
>> /**
>> * Request to raise the minimum required priority to schedule a gpu job
>> *
>> * Submit a request to increase the minimum required priority to schedule
>> * a gpu job. Once this function returns, the gpu scheduler will no longer
>> * consider jobs from contexts with priority lower than @priority.
>> *
>> * The minimum priority considered by the scheduler will be the highest 
>> from
>> * all currently active requests.
>> *
>> * Requests are refcounted, and must be balanced using
>> * amdgpu_sched_min_priority_put()
>> *
>> * \param dev             - \c [in] device handle
>> * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>> *
>> * \return  0 on success\n
>> *         <0 - Negative POSIX error code
>> *
>> * \notes this function requires DRM_MASTER
>> */
>> int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
>>  int32_t priority);
>>
>> /**
>> * Drop a request to raise the minimum required scheduler priority
>> *
>> * This call balances amdgpu_sched_min_priority_get()
>> *
>> * If no other active requests exists for @priority, the minimum required
>> * priority will decay to a lower level until one is reached with an active
>> * request or the lowest priority is reached.
>> *
>> * \param dev             - \c [in] device handle
>> * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>> *
>> * \return  0 on success\n
>> *         <0 - Negative POSIX error code
>> *
>> * \notes this function requires DRM_MASTER
>> */
>> int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
>>  int32_t priority);
>>
>> Using this app, VRComposer can raise the priority of the VRapp and 
>> itself. Then
>> it can restrict the minimum scheduler priority in order to become 
>> exclusive gpu
>> clients.
>>
>> One of the areas I'd like feedback is the following scenario. If a 
>> VRapp opens
>> a new fd and creates a new context after a call to set_priority, this 
>> specific
>> context will be lower priority than the rest. If the minimum required 
>> priority
>> is then raised, it is possible that this new context will be starved and
>> deadlock the VRapp.
>>
>> One solution I had in mind to address this situation, is to make 
>> set_priority
>> also raise the priority of future contexts created by the VRapp. 
>> However, that
>> would require keeping track of the requested priority on a per-process 
>> data
>> structure. The current design appears to steer clean of keeping any 
>> process
>> specific data, and everything instead of stored on a per-file basis. 
>> Which is
>> why I did not pursue this approach. But if this is something you'd 
>> like me to
>> implement let me know.
>>
>> One could also argue that preventing an application deadlock should be 
>> handled
>> between the VRComposer and the VRApp. It is not the kernel's 
>> responsibility to
>> babysit userspace applications and prevent themselves from shooting 
>> themselves
>> in the foot. The same could be achieved by improper usage of shared fences
>> between processes.
>>
>> Thoughts/feedback/comments on this issue, or others, are appreciated.
>>
>> Regards,
>> Andres
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org <mailto: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: [RFC] Exclusive gpu access for SteamVR usecases

[Christian König]

[-- Attachment #1.1: Type: text/plain, Size: 5560 bytes --]

Hi David,

the idea is that the compositor (which is DRM master) can change the 
priority of it's clients.

So using dev->fd is pointless because that is the fd of the DRM master 
process.

Regards,
Christian.

Am 26.05.2017 um 11:02 schrieb Mao, David:
> Hi Andres,
> Why the fd is needed for this interface?
> Why not just using the dev->fd instead of fd?
> IIRC, if there are more than one fds opened in the process upon the 
> same device, they will share the same amdgpu_device_handle which is 
> guaranteed by amdgpu_device_initialize.
> In other word, we should not run into the case that user creates more 
> contexts with newly opened fd after tuning the priority of existing 
> context in the same process unless the previous fd is closed.
>
> Thanks.
> Best Regards,
> David
>
>> On 25 May 2017, at 8:00 AM, Andres Rodriguez <andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org 
>> <mailto:andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>> wrote:
>>
>> When multiple environments are running simultaneously on a system, e.g.
>> an X desktop + a SteamVR game session, it may be useful to sacrifice
>> performance in one environment in order to boost it on the other.
>>
>> This series provides a mechanism for a DRM_MASTER to provide exclusive
>> gpu access to a group of processes.
>>
>> Note: This series is built on the assumption that the drm lease patch 
>> series
>> will extend DRM_MASTER status to lesees.
>>
>> The libdrm we intend to provide is as follows:
>>
>> /**
>> * Set the priority of all contexts in a process
>> *
>> * This function will change the priority of all contexts owned by
>> * the process identified by fd.
>> *
>> * \param dev             - \c [in] device handle
>> * \param fd              - \c [in] fd from target process
>> * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>> *
>> * \return  0 on success\n
>> *         <0 - Negative POSIX error code
>> *
>> * \notes @fd can be *any* file descriptor from the target process.
>> * \notes this function requires DRM_MASTER
>> */
>> int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
>>      int fd, int32_t priority);
>>
>> /**
>> * Request to raise the minimum required priority to schedule a gpu job
>> *
>> * Submit a request to increase the minimum required priority to schedule
>> * a gpu job. Once this function returns, the gpu scheduler will no longer
>> * consider jobs from contexts with priority lower than @priority.
>> *
>> * The minimum priority considered by the scheduler will be the 
>> highest from
>> * all currently active requests.
>> *
>> * Requests are refcounted, and must be balanced using
>> * amdgpu_sched_min_priority_put()
>> *
>> * \param dev             - \c [in] device handle
>> * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>> *
>> * \return  0 on success\n
>> *         <0 - Negative POSIX error code
>> *
>> * \notes this function requires DRM_MASTER
>> */
>> int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
>>  int32_t priority);
>>
>> /**
>> * Drop a request to raise the minimum required scheduler priority
>> *
>> * This call balances amdgpu_sched_min_priority_get()
>> *
>> * If no other active requests exists for @priority, the minimum required
>> * priority will decay to a lower level until one is reached with an 
>> active
>> * request or the lowest priority is reached.
>> *
>> * \param dev             - \c [in] device handle
>> * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>> *
>> * \return  0 on success\n
>> *         <0 - Negative POSIX error code
>> *
>> * \notes this function requires DRM_MASTER
>> */
>> int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
>>  int32_t priority);
>>
>> Using this app, VRComposer can raise the priority of the VRapp and 
>> itself. Then
>> it can restrict the minimum scheduler priority in order to become 
>> exclusive gpu
>> clients.
>>
>> One of the areas I'd like feedback is the following scenario. If a 
>> VRapp opens
>> a new fd and creates a new context after a call to set_priority, this 
>> specific
>> context will be lower priority than the rest. If the minimum required 
>> priority
>> is then raised, it is possible that this new context will be starved and
>> deadlock the VRapp.
>>
>> One solution I had in mind to address this situation, is to make 
>> set_priority
>> also raise the priority of future contexts created by the VRapp. 
>> However, that
>> would require keeping track of the requested priority on a 
>> per-process data
>> structure. The current design appears to steer clean of keeping any 
>> process
>> specific data, and everything instead of stored on a per-file basis. 
>> Which is
>> why I did not pursue this approach. But if this is something you'd 
>> like me to
>> implement let me know.
>>
>> One could also argue that preventing an application deadlock should 
>> be handled
>> between the VRComposer and the VRApp. It is not the kernel's 
>> responsibility to
>> babysit userspace applications and prevent themselves from shooting 
>> themselves
>> in the foot. The same could be achieved by improper usage of shared 
>> fences
>> between processes.
>>
>> Thoughts/feedback/comments on this issue, or others, are appreciated.
>>
>> Regards,
>> Andres
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>


[-- Attachment #1.2: Type: text/html, Size: 11968 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [RFC] Exclusive gpu access for SteamVR usecases

[Christian König]

Looks like a good start, but a few notes in general:

1. Split the patches into two sets.

One for implementing changing the priorities and one for limiting the 
priorities.

2. How are the priorities from processes supposed to interact with the 
per context priority?

3. Thinking more about it we can't limit the minimum priority in the 
scheduler.

For example a low priority job might block resources the high priority 
job needs to run. E.g. VRAM memory.

We need something like blocking the submitter instead (bad) or detection 
of dependencies in the scheduler (good, but tricky to implement).

Otherwise we can easily run into a deadlock situation with that approach.

Regards,
Christian.

Am 25.05.2017 um 02:00 schrieb Andres Rodriguez:
> When multiple environments are running simultaneously on a system, e.g.
> an X desktop + a SteamVR game session, it may be useful to sacrifice
> performance in one environment in order to boost it on the other.
>
> This series provides a mechanism for a DRM_MASTER to provide exclusive
> gpu access to a group of processes.
>
> Note: This series is built on the assumption that the drm lease patch series
> will extend DRM_MASTER status to lesees.
>
> The libdrm we intend to provide is as follows:
>
> /**
>   * Set the priority of all contexts in a process
>   *
>   * This function will change the priority of all contexts owned by
>   * the process identified by fd.
>   *
>   * \param dev             - \c [in] device handle
>   * \param fd              - \c [in] fd from target process
>   * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>   *
>   * \return  0 on success\n
>   *         <0 - Negative POSIX error code
>   *
>   * \notes @fd can be *any* file descriptor from the target process.
>   * \notes this function requires DRM_MASTER
>   */
> int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
> 				      int fd, int32_t priority);
>
> /**
>   * Request to raise the minimum required priority to schedule a gpu job
>   *
>   * Submit a request to increase the minimum required priority to schedule
>   * a gpu job. Once this function returns, the gpu scheduler will no longer
>   * consider jobs from contexts with priority lower than @priority.
>   *
>   * The minimum priority considered by the scheduler will be the highest from
>   * all currently active requests.
>   *
>   * Requests are refcounted, and must be balanced using
>   * amdgpu_sched_min_priority_put()
>   *
>   * \param dev             - \c [in] device handle
>   * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>   *
>   * \return  0 on success\n
>   *         <0 - Negative POSIX error code
>   *
>   * \notes this function requires DRM_MASTER
>   */
> int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
> 				  int32_t priority);
>
> /**
>   * Drop a request to raise the minimum required scheduler priority
>   *
>   * This call balances amdgpu_sched_min_priority_get()
>   *
>   * If no other active requests exists for @priority, the minimum required
>   * priority will decay to a lower level until one is reached with an active
>   * request or the lowest priority is reached.
>   *
>   * \param dev             - \c [in] device handle
>   * \param priority        - \c [in] target priority AMDGPU_CTX_PRIORITY_*
>   *
>   * \return  0 on success\n
>   *         <0 - Negative POSIX error code
>   *
>   * \notes this function requires DRM_MASTER
>   */
> int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
> 				  int32_t priority);
>
> Using this app, VRComposer can raise the priority of the VRapp and itself. Then
> it can restrict the minimum scheduler priority in order to become exclusive gpu
> clients.
>
> One of the areas I'd like feedback is the following scenario. If a VRapp opens
> a new fd and creates a new context after a call to set_priority, this specific
> context will be lower priority than the rest. If the minimum required priority
> is then raised, it is possible that this new context will be starved and
> deadlock the VRapp.
>
> One solution I had in mind to address this situation, is to make set_priority
> also raise the priority of future contexts created by the VRapp. However, that
> would require keeping track of the requested priority on a per-process data
> structure. The current design appears to steer clean of keeping any process
> specific data, and everything instead of stored on a per-file basis. Which is
> why I did not pursue this approach. But if this is something you'd like me to
> implement let me know.
>
> One could also argue that preventing an application deadlock should be handled
> between the VRComposer and the VRApp. It is not the kernel's responsibility to
> babysit userspace applications and prevent themselves from shooting themselves
> in the foot. The same could be achieved by improper usage of shared fences
> between processes.
>
> Thoughts/feedback/comments on this issue, or others, are appreciated.
>
> Regards,
> Andres
>   
>

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

Re: [RFC] Exclusive gpu access for SteamVR usecases

[Andres Rodriguez]

On 2017-05-30 11:19 AM, Christian König wrote:
> Looks like a good start, but a few notes in general:
> 
> 1. Split the patches into two sets.
> 
> One for implementing changing the priorities and one for limiting the 
> priorities.
> 

No problem.

> 2. How are the priorities from processes supposed to interact with the 
> per context priority?

Do you mean process niceness?

There isn't any relationship between niceness and gpu priority.

Let me know if you meant something different here.

> 
> 3. Thinking more about it we can't limit the minimum priority in the 
> scheduler.
> 
> For example a low priority job might block resources the high priority 
> job needs to run. E.g. VRAM memory.
> 

We avoid deadlocks by making sure that all dependencies of an exclusive 
task are also elevated to the same priority as said task. Usermode (the 
DRM_MASTER) is responsible to maintain this guarantee. The kernel does 
provide an ioctl that makes this task simple, 
amdgpu_sched_process_priority_set().

Lets take a look at this issue through three different scenarios.

(i) Job dependencies are all process internal, i.e. multiple contexts in 
one process.

This is the trivial case. A call to amdgpu_sched_process_priority_set() 
will change the priority of all contexts belonging to a process in lockstep.

Once amdgpu_sched_process_priority_set() returns, it is safe to raise 
the minimum priority using amdgpu_sched_min_priority_get(). At this 
point we have a guarantee that all contexts belonging to the process 
will be in a runnable state, or all the contexts will be in a 
not-runnable state. There won't be a mix of runnable and non-runnable 
processes.

Getting into that mixed state is what could cause a deadlock, a runnable 
context depends on a non-runnable one.

Note: the current patchset needs a fix to provide this guarantee in 
multi-gpu systems.

(ii) Job dependencies between two processes.

This case is mentioned separately as it is probably the most common use 
case we will encounter for this feature. Most graphics applications 
enter producer/consumer relationship with the compositor process (window 
swapchain).

In this case the compositor should already have all the information 
required to avoid a deadlock. It knows:
   - Itself (as a process)
   - The application process
   - The dependencies between both processes

At this stage it is simple for the compositor to understand that if it 
wishes to perform an exclusive mode transition, all dependencies (which 
are known) should also be part of the exclusive group.

We should be able to implement this feature without modifying a 
game/application.

(iii) Job dependencies between multiple (3+) processes.

This scenario is very uncommon for games. For example, if a game or 
application is split into multiple processes. Process A interacts with 
the compositor. Process B does some physics/compute calculations and 
send the results to Process A.

To support this use case, we would require an interface for the 
application to communicate to the compositor its dependencies. I.e. 
Process A would say, "Also keep Process B's priority in sync with mine". 
This should be a simple bit of plumbing to allow Process A to share an 
fd from Process B with the compositor.

B --[pipe_send(fdB)]--> A --[compositor_ext_priority_group_add(fdB)]--> 
Compositor

Once the compositor is aware of all of A's dependencies, this can be 
handled in the same fashion as (ii).

A special extension would be required for compositor protocols to 
communicate the dependencies fd. Applications would also need to be 
updated to use this extension.

I think this case would be very uncommon. But it is something that we 
would be able to handle if the need would arise.

 > We need something like blocking the submitter instead (bad) or detection
 > of dependencies in the scheduler (good, but tricky to implement).
 >

I definitely agree that detecting dependencies is tricky. Which is why I 
prefer an approach where usermode defines the dependencies. It is simple 
for both the kernel and usermode to implement.

 > Otherwise we can easily run into a deadlock situation with that approach.
 >

The current API does allow you to deadlock yourself pretty easily if 
misused. But so do many other APIs, like having a thread trying to grab 
the same lock twice :)

Thanks for the comments,
Andres

> Regards,
> Christian.
> 
> Am 25.05.2017 um 02:00 schrieb Andres Rodriguez:
>> When multiple environments are running simultaneously on a system, e.g.
>> an X desktop + a SteamVR game session, it may be useful to sacrifice
>> performance in one environment in order to boost it on the other.
>>
>> This series provides a mechanism for a DRM_MASTER to provide exclusive
>> gpu access to a group of processes.
>>
>> Note: This series is built on the assumption that the drm lease patch 
>> series
>> will extend DRM_MASTER status to lesees.
>>
>> The libdrm we intend to provide is as follows:
>>
>> /**
>>   * Set the priority of all contexts in a process
>>   *
>>   * This function will change the priority of all contexts owned by
>>   * the process identified by fd.
>>   *
>>   * \param dev             - \c [in] device handle
>>   * \param fd              - \c [in] fd from target process
>>   * \param priority        - \c [in] target priority 
>> AMDGPU_CTX_PRIORITY_*
>>   *
>>   * \return  0 on success\n
>>   *         <0 - Negative POSIX error code
>>   *
>>   * \notes @fd can be *any* file descriptor from the target process.
>>   * \notes this function requires DRM_MASTER
>>   */
>> int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
>>                       int fd, int32_t priority);
>>
>> /**
>>   * Request to raise the minimum required priority to schedule a gpu job
>>   *
>>   * Submit a request to increase the minimum required priority to 
>> schedule
>>   * a gpu job. Once this function returns, the gpu scheduler will no 
>> longer
>>   * consider jobs from contexts with priority lower than @priority.
>>   *
>>   * The minimum priority considered by the scheduler will be the 
>> highest from
>>   * all currently active requests.
>>   *
>>   * Requests are refcounted, and must be balanced using
>>   * amdgpu_sched_min_priority_put()
>>   *
>>   * \param dev             - \c [in] device handle
>>   * \param priority        - \c [in] target priority 
>> AMDGPU_CTX_PRIORITY_*
>>   *
>>   * \return  0 on success\n
>>   *         <0 - Negative POSIX error code
>>   *
>>   * \notes this function requires DRM_MASTER
>>   */
>> int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
>>                   int32_t priority);
>>
>> /**
>>   * Drop a request to raise the minimum required scheduler priority
>>   *
>>   * This call balances amdgpu_sched_min_priority_get()
>>   *
>>   * If no other active requests exists for @priority, the minimum 
>> required
>>   * priority will decay to a lower level until one is reached with an 
>> active
>>   * request or the lowest priority is reached.
>>   *
>>   * \param dev             - \c [in] device handle
>>   * \param priority        - \c [in] target priority 
>> AMDGPU_CTX_PRIORITY_*
>>   *
>>   * \return  0 on success\n
>>   *         <0 - Negative POSIX error code
>>   *
>>   * \notes this function requires DRM_MASTER
>>   */
>> int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
>>                   int32_t priority);
>>
>> Using this app, VRComposer can raise the priority of the VRapp and 
>> itself. Then
>> it can restrict the minimum scheduler priority in order to become 
>> exclusive gpu
>> clients.
>>
>> One of the areas I'd like feedback is the following scenario. If a 
>> VRapp opens
>> a new fd and creates a new context after a call to set_priority, this 
>> specific
>> context will be lower priority than the rest. If the minimum required 
>> priority
>> is then raised, it is possible that this new context will be starved and
>> deadlock the VRapp.
>>
>> One solution I had in mind to address this situation, is to make 
>> set_priority
>> also raise the priority of future contexts created by the VRapp. 
>> However, that
>> would require keeping track of the requested priority on a per-process 
>> data
>> structure. The current design appears to steer clean of keeping any 
>> process
>> specific data, and everything instead of stored on a per-file basis. 
>> Which is
>> why I did not pursue this approach. But if this is something you'd 
>> like me to
>> implement let me know.
>>
>> One could also argue that preventing an application deadlock should be 
>> handled
>> between the VRComposer and the VRApp. It is not the kernel's 
>> responsibility to
>> babysit userspace applications and prevent themselves from shooting 
>> themselves
>> in the foot. The same could be achieved by improper usage of shared 
>> fences
>> between processes.
>>
>> Thoughts/feedback/comments on this issue, or others, are appreciated.
>>
>> Regards,
>> Andres
>>
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Exclusive gpu access for SteamVR usecases

[Christian König]

>
>> 2. How are the priorities from processes supposed to interact with 
>> the per context priority?
>
> Do you mean process niceness?
>
> There isn't any relationship between niceness and gpu priority.
>
> Let me know if you meant something different here. 
I meant something different.

The application controls the per context priority when creating the 
context and the compositor should control the per process priority.

Those two needs to be handled separately, otherwise we would could 
override the context priority from the compositor and so confuse the 
application.

I suggest to cleanly separate the two.

> (ii) Job dependencies between two processes.
>
> This case is mentioned separately as it is probably the most common 
> use case we will encounter for this feature. Most graphics 
> applications enter producer/consumer relationship with the compositor 
> process (window swapchain).
>
> In this case the compositor should already have all the information 
> required to avoid a deadlock.

That is nonsense. The kernel is the supervisor of resource management, 
so only the kernel knows how to avoid a deadlock.

Let's imagine the following example:
Process A is a low priority task (for example updating the clock bitmap) 
which needs a resource for it's command submission.
Process B is a high priority task (rendering of the VR) which needs a 
bunch of memory for it's command submission.

Now the kernel memory management decides that it needs to evict process 
A from VRAM to make room for the command submission of process B. To do 
this all command submissions of process A need to finish.

In this moment the compositor hands over exclusive access to process B 
and never gives process A a chance to run.

Now B depends on A, but A can never run because B has exclusive access 
-> deadlock.

We somehow need to handle this inside the kernel or this whole approach 
won't work.

Regards,
Christian.

Am 30.05.2017 um 23:38 schrieb Andres Rodriguez:
>
>
> On 2017-05-30 11:19 AM, Christian König wrote:
>> Looks like a good start, but a few notes in general:
>>
>> 1. Split the patches into two sets.
>>
>> One for implementing changing the priorities and one for limiting the 
>> priorities.
>>
>
> No problem.
>
>> 2. How are the priorities from processes supposed to interact with 
>> the per context priority?
>
> Do you mean process niceness?
>
> There isn't any relationship between niceness and gpu priority.
>
> Let me know if you meant something different here.
>
>>
>> 3. Thinking more about it we can't limit the minimum priority in the 
>> scheduler.
>>
>> For example a low priority job might block resources the high 
>> priority job needs to run. E.g. VRAM memory.
>>
>
> We avoid deadlocks by making sure that all dependencies of an 
> exclusive task are also elevated to the same priority as said task. 
> Usermode (the DRM_MASTER) is responsible to maintain this guarantee. 
> The kernel does provide an ioctl that makes this task simple, 
> amdgpu_sched_process_priority_set().
>
> Lets take a look at this issue through three different scenarios.
>
> (i) Job dependencies are all process internal, i.e. multiple contexts 
> in one process.
>
> This is the trivial case. A call to 
> amdgpu_sched_process_priority_set() will change the priority of all 
> contexts belonging to a process in lockstep.
>
> Once amdgpu_sched_process_priority_set() returns, it is safe to raise 
> the minimum priority using amdgpu_sched_min_priority_get(). At this 
> point we have a guarantee that all contexts belonging to the process 
> will be in a runnable state, or all the contexts will be in a 
> not-runnable state. There won't be a mix of runnable and non-runnable 
> processes.
>
> Getting into that mixed state is what could cause a deadlock, a 
> runnable context depends on a non-runnable one.
>
> Note: the current patchset needs a fix to provide this guarantee in 
> multi-gpu systems.
>
> (ii) Job dependencies between two processes.
>
> This case is mentioned separately as it is probably the most common 
> use case we will encounter for this feature. Most graphics 
> applications enter producer/consumer relationship with the compositor 
> process (window swapchain).
>
> In this case the compositor should already have all the information 
> required to avoid a deadlock. It knows:
>   - Itself (as a process)
>   - The application process
>   - The dependencies between both processes
>
> At this stage it is simple for the compositor to understand that if it 
> wishes to perform an exclusive mode transition, all dependencies 
> (which are known) should also be part of the exclusive group.
>
> We should be able to implement this feature without modifying a 
> game/application.
>
> (iii) Job dependencies between multiple (3+) processes.
>
> This scenario is very uncommon for games. For example, if a game or 
> application is split into multiple processes. Process A interacts with 
> the compositor. Process B does some physics/compute calculations and 
> send the results to Process A.
>
> To support this use case, we would require an interface for the 
> application to communicate to the compositor its dependencies. I.e. 
> Process A would say, "Also keep Process B's priority in sync with 
> mine". This should be a simple bit of plumbing to allow Process A to 
> share an fd from Process B with the compositor.
>
> B --[pipe_send(fdB)]--> A 
> --[compositor_ext_priority_group_add(fdB)]--> Compositor
>
> Once the compositor is aware of all of A's dependencies, this can be 
> handled in the same fashion as (ii).
>
> A special extension would be required for compositor protocols to 
> communicate the dependencies fd. Applications would also need to be 
> updated to use this extension.
>
> I think this case would be very uncommon. But it is something that we 
> would be able to handle if the need would arise.
>
> > We need something like blocking the submitter instead (bad) or 
> detection
> > of dependencies in the scheduler (good, but tricky to implement).
> >
>
> I definitely agree that detecting dependencies is tricky. Which is why 
> I prefer an approach where usermode defines the dependencies. It is 
> simple for both the kernel and usermode to implement.
>
> > Otherwise we can easily run into a deadlock situation with that 
> approach.
> >
>
> The current API does allow you to deadlock yourself pretty easily if 
> misused. But so do many other APIs, like having a thread trying to 
> grab the same lock twice :)
>
> Thanks for the comments,
> Andres
>
>> Regards,
>> Christian.
>>
>> Am 25.05.2017 um 02:00 schrieb Andres Rodriguez:
>>> When multiple environments are running simultaneously on a system, e.g.
>>> an X desktop + a SteamVR game session, it may be useful to sacrifice
>>> performance in one environment in order to boost it on the other.
>>>
>>> This series provides a mechanism for a DRM_MASTER to provide exclusive
>>> gpu access to a group of processes.
>>>
>>> Note: This series is built on the assumption that the drm lease 
>>> patch series
>>> will extend DRM_MASTER status to lesees.
>>>
>>> The libdrm we intend to provide is as follows:
>>>
>>> /**
>>>   * Set the priority of all contexts in a process
>>>   *
>>>   * This function will change the priority of all contexts owned by
>>>   * the process identified by fd.
>>>   *
>>>   * \param dev             - \c [in] device handle
>>>   * \param fd              - \c [in] fd from target process
>>>   * \param priority        - \c [in] target priority 
>>> AMDGPU_CTX_PRIORITY_*
>>>   *
>>>   * \return  0 on success\n
>>>   *         <0 - Negative POSIX error code
>>>   *
>>>   * \notes @fd can be *any* file descriptor from the target process.
>>>   * \notes this function requires DRM_MASTER
>>>   */
>>> int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
>>>                       int fd, int32_t priority);
>>>
>>> /**
>>>   * Request to raise the minimum required priority to schedule a gpu 
>>> job
>>>   *
>>>   * Submit a request to increase the minimum required priority to 
>>> schedule
>>>   * a gpu job. Once this function returns, the gpu scheduler will no 
>>> longer
>>>   * consider jobs from contexts with priority lower than @priority.
>>>   *
>>>   * The minimum priority considered by the scheduler will be the 
>>> highest from
>>>   * all currently active requests.
>>>   *
>>>   * Requests are refcounted, and must be balanced using
>>>   * amdgpu_sched_min_priority_put()
>>>   *
>>>   * \param dev             - \c [in] device handle
>>>   * \param priority        - \c [in] target priority 
>>> AMDGPU_CTX_PRIORITY_*
>>>   *
>>>   * \return  0 on success\n
>>>   *         <0 - Negative POSIX error code
>>>   *
>>>   * \notes this function requires DRM_MASTER
>>>   */
>>> int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
>>>                   int32_t priority);
>>>
>>> /**
>>>   * Drop a request to raise the minimum required scheduler priority
>>>   *
>>>   * This call balances amdgpu_sched_min_priority_get()
>>>   *
>>>   * If no other active requests exists for @priority, the minimum 
>>> required
>>>   * priority will decay to a lower level until one is reached with 
>>> an active
>>>   * request or the lowest priority is reached.
>>>   *
>>>   * \param dev             - \c [in] device handle
>>>   * \param priority        - \c [in] target priority 
>>> AMDGPU_CTX_PRIORITY_*
>>>   *
>>>   * \return  0 on success\n
>>>   *         <0 - Negative POSIX error code
>>>   *
>>>   * \notes this function requires DRM_MASTER
>>>   */
>>> int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
>>>                   int32_t priority);
>>>
>>> Using this app, VRComposer can raise the priority of the VRapp and 
>>> itself. Then
>>> it can restrict the minimum scheduler priority in order to become 
>>> exclusive gpu
>>> clients.
>>>
>>> One of the areas I'd like feedback is the following scenario. If a 
>>> VRapp opens
>>> a new fd and creates a new context after a call to set_priority, 
>>> this specific
>>> context will be lower priority than the rest. If the minimum 
>>> required priority
>>> is then raised, it is possible that this new context will be starved 
>>> and
>>> deadlock the VRapp.
>>>
>>> One solution I had in mind to address this situation, is to make 
>>> set_priority
>>> also raise the priority of future contexts created by the VRapp. 
>>> However, that
>>> would require keeping track of the requested priority on a 
>>> per-process data
>>> structure. The current design appears to steer clean of keeping any 
>>> process
>>> specific data, and everything instead of stored on a per-file basis. 
>>> Which is
>>> why I did not pursue this approach. But if this is something you'd 
>>> like me to
>>> implement let me know.
>>>
>>> One could also argue that preventing an application deadlock should 
>>> be handled
>>> between the VRComposer and the VRApp. It is not the kernel's 
>>> responsibility to
>>> babysit userspace applications and prevent themselves from shooting 
>>> themselves
>>> in the foot. The same could be achieved by improper usage of shared 
>>> fences
>>> between processes.
>>>
>>> Thoughts/feedback/comments on this issue, or others, are appreciated.
>>>
>>> Regards,
>>> Andres
>>>
>>
> _______________________________________________
> 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: [RFC] Exclusive gpu access for SteamVR usecases

[Andres Rodriguez]

On 2017-05-31 02:53 AM, Christian König wrote:
>>
>>> 2. How are the priorities from processes supposed to interact with 
>>> the per context priority?
>>
>> Do you mean process niceness?
>>
>> There isn't any relationship between niceness and gpu priority.
>>
>> Let me know if you meant something different here. 
> I meant something different.
> 
> The application controls the per context priority when creating the 
> context and the compositor should control the per process priority.
> 
> Those two needs to be handled separately, otherwise we would could 
> override the context priority from the compositor and so confuse the 
> application.
> 
> I suggest to cleanly separate the two.
> 

Would you then b be okay with having a list of amdgpu_process data 
structures stored in adev? The list would be searchable by struct pid.

However, I'm not 100% convinced on having two separate priorities. It 
duplicates the concept and we need to introduce relationship semantics 
between the two.

Alternatively, since the priority patches still aren't part of drm-next 
we still have a chance make some changes there. If we make the priority 
requests reference counted we should be able to remember any old requests.

>> (ii) Job dependencies between two processes.
>>
>> This case is mentioned separately as it is probably the most common 
>> use case we will encounter for this feature. Most graphics 
>> applications enter producer/consumer relationship with the compositor 
>> process (window swapchain).
>>
>> In this case the compositor should already have all the information 
>> required to avoid a deadlock.
> 
> That is nonsense. The kernel is the supervisor of resource management, 
> so only the kernel knows how to avoid a deadlock.

> Let's imagine the following example:
> Process A is a low priority task (for example updating the clock bitmap) 
> which needs a resource for it's command submission.
> Process B is a high priority task (rendering of the VR) which needs a 
> bunch of memory for it's command submission.
> 
> Now the kernel memory management decides that it needs to evict process 
> A from VRAM to make room for the command submission of process B. To do 
> this all command submissions of process A need to finish.
> 
> In this moment the compositor hands over exclusive access to process B 
> and never gives process A a chance to run.
> 
> Now B depends on A, but A can never run because B has exclusive access 
> -> deadlock.
> 
> We somehow need to handle this inside the kernel or this whole approach 
> won't work.
> 

Thanks for pointing that out. I thought cases like these would work okay 
since we always allow PRIORITY_KERNEL work to execute. But as you 
pointed out, I overlooked the dependency that is created once the 
command buffers have their final memory addresses attached.

Let me read and think about this a bit more.

Regards,
Andres

> Regards,
> Christian.
> 
> Am 30.05.2017 um 23:38 schrieb Andres Rodriguez:
>>
>>
>> On 2017-05-30 11:19 AM, Christian König wrote:
>>> Looks like a good start, but a few notes in general:
>>>
>>> 1. Split the patches into two sets.
>>>
>>> One for implementing changing the priorities and one for limiting the 
>>> priorities.
>>>
>>
>> No problem.
>>
>>> 2. How are the priorities from processes supposed to interact with 
>>> the per context priority?
>>
>> Do you mean process niceness?
>>
>> There isn't any relationship between niceness and gpu priority.
>>
>> Let me know if you meant something different here.
>>
>>>
>>> 3. Thinking more about it we can't limit the minimum priority in the 
>>> scheduler.
>>>
>>> For example a low priority job might block resources the high 
>>> priority job needs to run. E.g. VRAM memory.
>>>
>>
>> We avoid deadlocks by making sure that all dependencies of an 
>> exclusive task are also elevated to the same priority as said task. 
>> Usermode (the DRM_MASTER) is responsible to maintain this guarantee. 
>> The kernel does provide an ioctl that makes this task simple, 
>> amdgpu_sched_process_priority_set().
>>
>> Lets take a look at this issue through three different scenarios.
>>
>> (i) Job dependencies are all process internal, i.e. multiple contexts 
>> in one process.
>>
>> This is the trivial case. A call to 
>> amdgpu_sched_process_priority_set() will change the priority of all 
>> contexts belonging to a process in lockstep.
>>
>> Once amdgpu_sched_process_priority_set() returns, it is safe to raise 
>> the minimum priority using amdgpu_sched_min_priority_get(). At this 
>> point we have a guarantee that all contexts belonging to the process 
>> will be in a runnable state, or all the contexts will be in a 
>> not-runnable state. There won't be a mix of runnable and non-runnable 
>> processes.
>>
>> Getting into that mixed state is what could cause a deadlock, a 
>> runnable context depends on a non-runnable one.
>>
>> Note: the current patchset needs a fix to provide this guarantee in 
>> multi-gpu systems.
>>
>> (ii) Job dependencies between two processes.
>>
>> This case is mentioned separately as it is probably the most common 
>> use case we will encounter for this feature. Most graphics 
>> applications enter producer/consumer relationship with the compositor 
>> process (window swapchain).
>>
>> In this case the compositor should already have all the information 
>> required to avoid a deadlock. It knows:
>>   - Itself (as a process)
>>   - The application process
>>   - The dependencies between both processes
>>
>> At this stage it is simple for the compositor to understand that if it 
>> wishes to perform an exclusive mode transition, all dependencies 
>> (which are known) should also be part of the exclusive group.
>>
>> We should be able to implement this feature without modifying a 
>> game/application.
>>
>> (iii) Job dependencies between multiple (3+) processes.
>>
>> This scenario is very uncommon for games. For example, if a game or 
>> application is split into multiple processes. Process A interacts with 
>> the compositor. Process B does some physics/compute calculations and 
>> send the results to Process A.
>>
>> To support this use case, we would require an interface for the 
>> application to communicate to the compositor its dependencies. I.e. 
>> Process A would say, "Also keep Process B's priority in sync with 
>> mine". This should be a simple bit of plumbing to allow Process A to 
>> share an fd from Process B with the compositor.
>>
>> B --[pipe_send(fdB)]--> A 
>> --[compositor_ext_priority_group_add(fdB)]--> Compositor
>>
>> Once the compositor is aware of all of A's dependencies, this can be 
>> handled in the same fashion as (ii).
>>
>> A special extension would be required for compositor protocols to 
>> communicate the dependencies fd. Applications would also need to be 
>> updated to use this extension.
>>
>> I think this case would be very uncommon. But it is something that we 
>> would be able to handle if the need would arise.
>>
>> > We need something like blocking the submitter instead (bad) or 
>> detection
>> > of dependencies in the scheduler (good, but tricky to implement).
>> >
>>
>> I definitely agree that detecting dependencies is tricky. Which is why 
>> I prefer an approach where usermode defines the dependencies. It is 
>> simple for both the kernel and usermode to implement.
>>
>> > Otherwise we can easily run into a deadlock situation with that 
>> approach.
>> >
>>
>> The current API does allow you to deadlock yourself pretty easily if 
>> misused. But so do many other APIs, like having a thread trying to 
>> grab the same lock twice :)
>>
>> Thanks for the comments,
>> Andres
>>
>>> Regards,
>>> Christian.
>>>
>>> Am 25.05.2017 um 02:00 schrieb Andres Rodriguez:
>>>> When multiple environments are running simultaneously on a system, e.g.
>>>> an X desktop + a SteamVR game session, it may be useful to sacrifice
>>>> performance in one environment in order to boost it on the other.
>>>>
>>>> This series provides a mechanism for a DRM_MASTER to provide exclusive
>>>> gpu access to a group of processes.
>>>>
>>>> Note: This series is built on the assumption that the drm lease 
>>>> patch series
>>>> will extend DRM_MASTER status to lesees.
>>>>
>>>> The libdrm we intend to provide is as follows:
>>>>
>>>> /**
>>>>   * Set the priority of all contexts in a process
>>>>   *
>>>>   * This function will change the priority of all contexts owned by
>>>>   * the process identified by fd.
>>>>   *
>>>>   * \param dev             - \c [in] device handle
>>>>   * \param fd              - \c [in] fd from target process
>>>>   * \param priority        - \c [in] target priority 
>>>> AMDGPU_CTX_PRIORITY_*
>>>>   *
>>>>   * \return  0 on success\n
>>>>   *         <0 - Negative POSIX error code
>>>>   *
>>>>   * \notes @fd can be *any* file descriptor from the target process.
>>>>   * \notes this function requires DRM_MASTER
>>>>   */
>>>> int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
>>>>                       int fd, int32_t priority);
>>>>
>>>> /**
>>>>   * Request to raise the minimum required priority to schedule a gpu 
>>>> job
>>>>   *
>>>>   * Submit a request to increase the minimum required priority to 
>>>> schedule
>>>>   * a gpu job. Once this function returns, the gpu scheduler will no 
>>>> longer
>>>>   * consider jobs from contexts with priority lower than @priority.
>>>>   *
>>>>   * The minimum priority considered by the scheduler will be the 
>>>> highest from
>>>>   * all currently active requests.
>>>>   *
>>>>   * Requests are refcounted, and must be balanced using
>>>>   * amdgpu_sched_min_priority_put()
>>>>   *
>>>>   * \param dev             - \c [in] device handle
>>>>   * \param priority        - \c [in] target priority 
>>>> AMDGPU_CTX_PRIORITY_*
>>>>   *
>>>>   * \return  0 on success\n
>>>>   *         <0 - Negative POSIX error code
>>>>   *
>>>>   * \notes this function requires DRM_MASTER
>>>>   */
>>>> int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
>>>>                   int32_t priority);
>>>>
>>>> /**
>>>>   * Drop a request to raise the minimum required scheduler priority
>>>>   *
>>>>   * This call balances amdgpu_sched_min_priority_get()
>>>>   *
>>>>   * If no other active requests exists for @priority, the minimum 
>>>> required
>>>>   * priority will decay to a lower level until one is reached with 
>>>> an active
>>>>   * request or the lowest priority is reached.
>>>>   *
>>>>   * \param dev             - \c [in] device handle
>>>>   * \param priority        - \c [in] target priority 
>>>> AMDGPU_CTX_PRIORITY_*
>>>>   *
>>>>   * \return  0 on success\n
>>>>   *         <0 - Negative POSIX error code
>>>>   *
>>>>   * \notes this function requires DRM_MASTER
>>>>   */
>>>> int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
>>>>                   int32_t priority);
>>>>
>>>> Using this app, VRComposer can raise the priority of the VRapp and 
>>>> itself. Then
>>>> it can restrict the minimum scheduler priority in order to become 
>>>> exclusive gpu
>>>> clients.
>>>>
>>>> One of the areas I'd like feedback is the following scenario. If a 
>>>> VRapp opens
>>>> a new fd and creates a new context after a call to set_priority, 
>>>> this specific
>>>> context will be lower priority than the rest. If the minimum 
>>>> required priority
>>>> is then raised, it is possible that this new context will be starved 
>>>> and
>>>> deadlock the VRapp.
>>>>
>>>> One solution I had in mind to address this situation, is to make 
>>>> set_priority
>>>> also raise the priority of future contexts created by the VRapp. 
>>>> However, that
>>>> would require keeping track of the requested priority on a 
>>>> per-process data
>>>> structure. The current design appears to steer clean of keeping any 
>>>> process
>>>> specific data, and everything instead of stored on a per-file basis. 
>>>> Which is
>>>> why I did not pursue this approach. But if this is something you'd 
>>>> like me to
>>>> implement let me know.
>>>>
>>>> One could also argue that preventing an application deadlock should 
>>>> be handled
>>>> between the VRComposer and the VRApp. It is not the kernel's 
>>>> responsibility to
>>>> babysit userspace applications and prevent themselves from shooting 
>>>> themselves
>>>> in the foot. The same could be achieved by improper usage of shared 
>>>> fences
>>>> between processes.
>>>>
>>>> Thoughts/feedback/comments on this issue, or others, are appreciated.
>>>>
>>>> Regards,
>>>> Andres
>>>>
>>>
>> _______________________________________________
>> 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: [RFC] Exclusive gpu access for SteamVR usecases

[Christian König]

Am 31.05.2017 um 20:46 schrieb Andres Rodriguez:
>
>
> On 2017-05-31 02:53 AM, Christian König wrote:
>>>
>>>> 2. How are the priorities from processes supposed to interact with 
>>>> the per context priority?
>>>
>>> Do you mean process niceness?
>>>
>>> There isn't any relationship between niceness and gpu priority.
>>>
>>> Let me know if you meant something different here. 
>> I meant something different.
>>
>> The application controls the per context priority when creating the 
>> context and the compositor should control the per process priority.
>>
>> Those two needs to be handled separately, otherwise we would could 
>> override the context priority from the compositor and so confuse the 
>> application.
>>
>> I suggest to cleanly separate the two.
>>
>
> Would you then b be okay with having a list of amdgpu_process data 
> structures stored in adev? The list would be searchable by struct pid.
>
> However, I'm not 100% convinced on having two separate priorities. It 
> duplicates the concept and we need to introduce relationship semantics 
> between the two.

No, that wasn't what I meant. The kernel should internally work only 
with one priority, but this priority is defined by two parameters: 1. 
The context priority set by the application 2. The client priority set 
by the compositor.

When setting the client priority by the compositor we should still take 
the context priority into account.

In other words when we have 3 different context priorities and 3 
different client priorities we have 9 plus the kernel priority in total. 
That should still be handleable.

>
> Alternatively, since the priority patches still aren't part of 
> drm-next we still have a chance make some changes there. If we make 
> the priority requests reference counted we should be able to remember 
> any old requests.
>
>>> (ii) Job dependencies between two processes.
>>>
>>> This case is mentioned separately as it is probably the most common 
>>> use case we will encounter for this feature. Most graphics 
>>> applications enter producer/consumer relationship with the 
>>> compositor process (window swapchain).
>>>
>>> In this case the compositor should already have all the information 
>>> required to avoid a deadlock.
>>
>> That is nonsense. The kernel is the supervisor of resource 
>> management, so only the kernel knows how to avoid a deadlock.
>
>> Let's imagine the following example:
>> Process A is a low priority task (for example updating the clock 
>> bitmap) which needs a resource for it's command submission.
>> Process B is a high priority task (rendering of the VR) which needs a 
>> bunch of memory for it's command submission.
>>
>> Now the kernel memory management decides that it needs to evict 
>> process A from VRAM to make room for the command submission of 
>> process B. To do this all command submissions of process A need to 
>> finish.
>>
>> In this moment the compositor hands over exclusive access to process 
>> B and never gives process A a chance to run.
>>
>> Now B depends on A, but A can never run because B has exclusive 
>> access -> deadlock.
>>
>> We somehow need to handle this inside the kernel or this whole 
>> approach won't work.
>>
>
> Thanks for pointing that out. I thought cases like these would work 
> okay since we always allow PRIORITY_KERNEL work to execute. But as you 
> pointed out, I overlooked the dependency that is created once the 
> command buffers have their final memory addresses attached.
>
> Let me read and think about this a bit more.

Yeah, that is going to be really tricky to handle. Not sure how we 
should approach that either.

As a quick workaround for testing you could prevent the clients with a 
low priority from making submissions while the limit is in effect.

Regards,
Christian.

>
> Regards,
> Andres
>
>> Regards,
>> Christian.
>>
>> Am 30.05.2017 um 23:38 schrieb Andres Rodriguez:
>>>
>>>
>>> On 2017-05-30 11:19 AM, Christian König wrote:
>>>> Looks like a good start, but a few notes in general:
>>>>
>>>> 1. Split the patches into two sets.
>>>>
>>>> One for implementing changing the priorities and one for limiting 
>>>> the priorities.
>>>>
>>>
>>> No problem.
>>>
>>>> 2. How are the priorities from processes supposed to interact with 
>>>> the per context priority?
>>>
>>> Do you mean process niceness?
>>>
>>> There isn't any relationship between niceness and gpu priority.
>>>
>>> Let me know if you meant something different here.
>>>
>>>>
>>>> 3. Thinking more about it we can't limit the minimum priority in 
>>>> the scheduler.
>>>>
>>>> For example a low priority job might block resources the high 
>>>> priority job needs to run. E.g. VRAM memory.
>>>>
>>>
>>> We avoid deadlocks by making sure that all dependencies of an 
>>> exclusive task are also elevated to the same priority as said task. 
>>> Usermode (the DRM_MASTER) is responsible to maintain this guarantee. 
>>> The kernel does provide an ioctl that makes this task simple, 
>>> amdgpu_sched_process_priority_set().
>>>
>>> Lets take a look at this issue through three different scenarios.
>>>
>>> (i) Job dependencies are all process internal, i.e. multiple 
>>> contexts in one process.
>>>
>>> This is the trivial case. A call to 
>>> amdgpu_sched_process_priority_set() will change the priority of all 
>>> contexts belonging to a process in lockstep.
>>>
>>> Once amdgpu_sched_process_priority_set() returns, it is safe to 
>>> raise the minimum priority using amdgpu_sched_min_priority_get(). At 
>>> this point we have a guarantee that all contexts belonging to the 
>>> process will be in a runnable state, or all the contexts will be in 
>>> a not-runnable state. There won't be a mix of runnable and 
>>> non-runnable processes.
>>>
>>> Getting into that mixed state is what could cause a deadlock, a 
>>> runnable context depends on a non-runnable one.
>>>
>>> Note: the current patchset needs a fix to provide this guarantee in 
>>> multi-gpu systems.
>>>
>>> (ii) Job dependencies between two processes.
>>>
>>> This case is mentioned separately as it is probably the most common 
>>> use case we will encounter for this feature. Most graphics 
>>> applications enter producer/consumer relationship with the 
>>> compositor process (window swapchain).
>>>
>>> In this case the compositor should already have all the information 
>>> required to avoid a deadlock. It knows:
>>>   - Itself (as a process)
>>>   - The application process
>>>   - The dependencies between both processes
>>>
>>> At this stage it is simple for the compositor to understand that if 
>>> it wishes to perform an exclusive mode transition, all dependencies 
>>> (which are known) should also be part of the exclusive group.
>>>
>>> We should be able to implement this feature without modifying a 
>>> game/application.
>>>
>>> (iii) Job dependencies between multiple (3+) processes.
>>>
>>> This scenario is very uncommon for games. For example, if a game or 
>>> application is split into multiple processes. Process A interacts 
>>> with the compositor. Process B does some physics/compute 
>>> calculations and send the results to Process A.
>>>
>>> To support this use case, we would require an interface for the 
>>> application to communicate to the compositor its dependencies. I.e. 
>>> Process A would say, "Also keep Process B's priority in sync with 
>>> mine". This should be a simple bit of plumbing to allow Process A to 
>>> share an fd from Process B with the compositor.
>>>
>>> B --[pipe_send(fdB)]--> A 
>>> --[compositor_ext_priority_group_add(fdB)]--> Compositor
>>>
>>> Once the compositor is aware of all of A's dependencies, this can be 
>>> handled in the same fashion as (ii).
>>>
>>> A special extension would be required for compositor protocols to 
>>> communicate the dependencies fd. Applications would also need to be 
>>> updated to use this extension.
>>>
>>> I think this case would be very uncommon. But it is something that 
>>> we would be able to handle if the need would arise.
>>>
>>> > We need something like blocking the submitter instead (bad) or 
>>> detection
>>> > of dependencies in the scheduler (good, but tricky to implement).
>>> >
>>>
>>> I definitely agree that detecting dependencies is tricky. Which is 
>>> why I prefer an approach where usermode defines the dependencies. It 
>>> is simple for both the kernel and usermode to implement.
>>>
>>> > Otherwise we can easily run into a deadlock situation with that 
>>> approach.
>>> >
>>>
>>> The current API does allow you to deadlock yourself pretty easily if 
>>> misused. But so do many other APIs, like having a thread trying to 
>>> grab the same lock twice :)
>>>
>>> Thanks for the comments,
>>> Andres
>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>> Am 25.05.2017 um 02:00 schrieb Andres Rodriguez:
>>>>> When multiple environments are running simultaneously on a system, 
>>>>> e.g.
>>>>> an X desktop + a SteamVR game session, it may be useful to sacrifice
>>>>> performance in one environment in order to boost it on the other.
>>>>>
>>>>> This series provides a mechanism for a DRM_MASTER to provide 
>>>>> exclusive
>>>>> gpu access to a group of processes.
>>>>>
>>>>> Note: This series is built on the assumption that the drm lease 
>>>>> patch series
>>>>> will extend DRM_MASTER status to lesees.
>>>>>
>>>>> The libdrm we intend to provide is as follows:
>>>>>
>>>>> /**
>>>>>   * Set the priority of all contexts in a process
>>>>>   *
>>>>>   * This function will change the priority of all contexts owned by
>>>>>   * the process identified by fd.
>>>>>   *
>>>>>   * \param dev             - \c [in] device handle
>>>>>   * \param fd              - \c [in] fd from target process
>>>>>   * \param priority        - \c [in] target priority 
>>>>> AMDGPU_CTX_PRIORITY_*
>>>>>   *
>>>>>   * \return  0 on success\n
>>>>>   *         <0 - Negative POSIX error code
>>>>>   *
>>>>>   * \notes @fd can be *any* file descriptor from the target process.
>>>>>   * \notes this function requires DRM_MASTER
>>>>>   */
>>>>> int amdgpu_sched_process_priority_set(amdgpu_device_handle dev,
>>>>>                       int fd, int32_t priority);
>>>>>
>>>>> /**
>>>>>   * Request to raise the minimum required priority to schedule a 
>>>>> gpu job
>>>>>   *
>>>>>   * Submit a request to increase the minimum required priority to 
>>>>> schedule
>>>>>   * a gpu job. Once this function returns, the gpu scheduler will 
>>>>> no longer
>>>>>   * consider jobs from contexts with priority lower than @priority.
>>>>>   *
>>>>>   * The minimum priority considered by the scheduler will be the 
>>>>> highest from
>>>>>   * all currently active requests.
>>>>>   *
>>>>>   * Requests are refcounted, and must be balanced using
>>>>>   * amdgpu_sched_min_priority_put()
>>>>>   *
>>>>>   * \param dev             - \c [in] device handle
>>>>>   * \param priority        - \c [in] target priority 
>>>>> AMDGPU_CTX_PRIORITY_*
>>>>>   *
>>>>>   * \return  0 on success\n
>>>>>   *         <0 - Negative POSIX error code
>>>>>   *
>>>>>   * \notes this function requires DRM_MASTER
>>>>>   */
>>>>> int amdgpu_sched_min_priority_get(amdgpu_device_handle dev,
>>>>>                   int32_t priority);
>>>>>
>>>>> /**
>>>>>   * Drop a request to raise the minimum required scheduler priority
>>>>>   *
>>>>>   * This call balances amdgpu_sched_min_priority_get()
>>>>>   *
>>>>>   * If no other active requests exists for @priority, the minimum 
>>>>> required
>>>>>   * priority will decay to a lower level until one is reached with 
>>>>> an active
>>>>>   * request or the lowest priority is reached.
>>>>>   *
>>>>>   * \param dev             - \c [in] device handle
>>>>>   * \param priority        - \c [in] target priority 
>>>>> AMDGPU_CTX_PRIORITY_*
>>>>>   *
>>>>>   * \return  0 on success\n
>>>>>   *         <0 - Negative POSIX error code
>>>>>   *
>>>>>   * \notes this function requires DRM_MASTER
>>>>>   */
>>>>> int amdgpu_sched_min_priority_put(amdgpu_device_handle dev,
>>>>>                   int32_t priority);
>>>>>
>>>>> Using this app, VRComposer can raise the priority of the VRapp and 
>>>>> itself. Then
>>>>> it can restrict the minimum scheduler priority in order to become 
>>>>> exclusive gpu
>>>>> clients.
>>>>>
>>>>> One of the areas I'd like feedback is the following scenario. If a 
>>>>> VRapp opens
>>>>> a new fd and creates a new context after a call to set_priority, 
>>>>> this specific
>>>>> context will be lower priority than the rest. If the minimum 
>>>>> required priority
>>>>> is then raised, it is possible that this new context will be 
>>>>> starved and
>>>>> deadlock the VRapp.
>>>>>
>>>>> One solution I had in mind to address this situation, is to make 
>>>>> set_priority
>>>>> also raise the priority of future contexts created by the VRapp. 
>>>>> However, that
>>>>> would require keeping track of the requested priority on a 
>>>>> per-process data
>>>>> structure. The current design appears to steer clean of keeping 
>>>>> any process
>>>>> specific data, and everything instead of stored on a per-file 
>>>>> basis. Which is
>>>>> why I did not pursue this approach. But if this is something you'd 
>>>>> like me to
>>>>> implement let me know.
>>>>>
>>>>> One could also argue that preventing an application deadlock 
>>>>> should be handled
>>>>> between the VRComposer and the VRApp. It is not the kernel's 
>>>>> responsibility to
>>>>> babysit userspace applications and prevent themselves from 
>>>>> shooting themselves
>>>>> in the foot. The same could be achieved by improper usage of 
>>>>> shared fences
>>>>> between processes.
>>>>>
>>>>> Thoughts/feedback/comments on this issue, or others, are appreciated.
>>>>>
>>>>> Regards,
>>>>> Andres
>>>>>
>>>>
>>> _______________________________________________
>>> 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