Re: [PATCH 7/8] drm/amdgpu: add plumbing for ctx priority changes

[Andres Rodriguez]

On 2017-06-09 07:11 AM, Christian König wrote:
Am 09.06.2017 um 00:06 schrieb Andres Rodriguez:
Introduce amdgpu_ctx_priority_get/put(). This a refcounted mechanism to
change a context's priority.

A context's priority will be set to the highest priority for which a
request exists.

If no active requests exist, the context will default to the priority
requested at context allocation time.

Note that the priority requested at allocation does not create a
request, therefore it can be overridden by a get() request for a lower
priority.

So you just use the maximum priority between the per process and the per 
context priority?

Not exactly. If we use the maximum of the two, then we won't be able to 
reduce the priority.

As long as we have a valid process priority, it will force override the 
context priority.

The actual priority of a context would be:
priority = (process_prio != 'INVALID') ? process_prio : context_prio

Where process_prio is the maximum level for which a DRM_MASTER has an 
active request.


Sounds logical, since the per context priority can only be used by the 
DRM master as well.


I think there is a small typo here (s/context/process). Based on your 
usage of the context priority in the next paragraph I think we are on 
the same page.

But you don't need the complicated amd_sched_priority_ctr handling for 
this.

Just add a min priority field into amdgpu_ctx_mgr and use that to 
override the context priority.

Agreed. My initial implementation was pretty much the one line of code 
above.

I just happened to have amd_sched_priority_ctr around for tracking the 
minimum scheduler priority, and it seems to fit this use case pretty well.

Regards,
Andres


Christian.


Signed-off-by: Andres Rodriguez <andre...@gmail.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  8 +++++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 43 
+++++++++++++++++++++++++++++++++
  2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 04ea1b9..b998f42 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -813,47 +813,53 @@ int amdgpu_queue_mgr_map(struct amdgpu_device 
*adev,
  struct amdgpu_ctx_ring {
      uint64_t        sequence;
      struct dma_fence    **fences;
      struct amd_sched_entity    entity;
  };
  struct amdgpu_ctx {
      struct kref        refcount;
      struct amdgpu_device    *adev;
      struct amdgpu_queue_mgr queue_mgr;
      unsigned        reset_counter;
      spinlock_t        ring_lock;
      struct dma_fence    **fences;
      struct amdgpu_ctx_ring    rings[AMDGPU_MAX_RINGS];
-    bool preamble_presented;
+    bool             preamble_presented;
+
+    struct amd_sched_priority_ctr    priority_ctr;
  };
  struct amdgpu_ctx_mgr {
      struct amdgpu_device    *adev;
      struct mutex        lock;
      /* protected by lock */
      struct idr        ctx_handles;
  };
  struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, 
uint32_t id);
  int amdgpu_ctx_put(struct amdgpu_ctx *ctx);
  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_priority_get(struct amdgpu_ctx *ctx,
+                 enum amd_sched_priority priority);
+void amdgpu_ctx_priority_put(struct amdgpu_ctx *ctx,
+                 enum amd_sched_priority priority);
  int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
               struct drm_file *filp);
  void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr);
  void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr);
  /*
   * file private structure
   */
  struct amdgpu_fpriv {
      struct amdgpu_vm    vm;
      struct amdgpu_bo_va    *prt_va;
      struct mutex        bo_list_lock;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 0285eef..cc15b7e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -29,30 +29,53 @@ static amdgpu_ctx_priority_permit(struct drm_file 
*filp,
                    enum amd_sched_priority priority)
  {
      /* NORMAL and below are accessible by everyone */
      if (priority <= AMD_SCHED_PRIORITY_NORMAL)
          return 0;
      if (capable(CAP_SYS_NICE))
          return 0;
      if (drm_is_current_master(filp))
          return 0;
      return -EACCES;
  }
+static void amdgpu_ctx_priority_set(struct amd_sched_priority_ctr *p,
+                    enum amd_sched_priority priority)
+{
+    int i;
+    struct amd_sched_rq *rq;
+    struct amd_sched_entity *entity;
+    struct amdgpu_ring *ring;
+    struct amdgpu_ctx *ctx = container_of(p, struct amdgpu_ctx,
+                          priority_ctr);
+    struct amdgpu_device *adev = ctx->adev;
+
+    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)
+            continue;
+
+        amd_sched_entity_set_rq(entity, rq);
+    }
+}
+
  static int amdgpu_ctx_init(struct amdgpu_device *adev,
                 enum amd_sched_priority priority,
                 struct drm_file *filp,
                 struct amdgpu_ctx *ctx)
  {
      unsigned i, j;
      int r;
      if (priority < 0 || priority >= AMD_SCHED_PRIORITY_MAX)
          return -EINVAL;
      r = amdgpu_ctx_priority_permit(filp, priority);
      if (r)
          return r;
@@ -76,53 +99,59 @@ static int amdgpu_ctx_init(struct amdgpu_device 
*adev,
      for (i = 0; i < adev->num_rings; i++) {
          struct amdgpu_ring *ring = adev->rings[i];
          struct amd_sched_rq *rq;
          rq = &ring->sched.sched_rq[priority];
          if (ring == &adev->gfx.kiq.ring)
              continue;
          r = amd_sched_entity_init(&ring->sched, &ctx->rings[i].entity,
                        rq, amdgpu_sched_jobs);
          if (r)
              goto failed;
      }
+    amd_sched_priority_ctr_init(&ctx->priority_ctr,
+                    priority,
+                    amdgpu_ctx_priority_set);
+
      r = amdgpu_queue_mgr_init(adev, &ctx->queue_mgr);
      if (r)
          goto failed;
      return 0;
  failed:
      for (j = 0; j < i; j++)
          amd_sched_entity_fini(&adev->rings[j]->sched,
                        &ctx->rings[j].entity);
      kfree(ctx->fences);
      ctx->fences = NULL;
      return r;
  }
  static void amdgpu_ctx_fini(struct amdgpu_ctx *ctx)
  {
      struct amdgpu_device *adev = ctx->adev;
      unsigned i, j;
      if (!adev)
          return;
+    amd_sched_priority_ctr_fini(&ctx->priority_ctr);
+
      for (i = 0; i < AMDGPU_MAX_RINGS; ++i)
          for (j = 0; j < amdgpu_sched_jobs; ++j)
              dma_fence_put(ctx->rings[i].fences[j]);
      kfree(ctx->fences);
      ctx->fences = NULL;
      for (i = 0; i < adev->num_rings; i++)
          amd_sched_entity_fini(&adev->rings[i]->sched,
                        &ctx->rings[i].entity);
      amdgpu_queue_mgr_fini(adev, &ctx->queue_mgr);
  }
  static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
                  struct amdgpu_fpriv *fpriv,
@@ -345,30 +374,44 @@ struct dma_fence *amdgpu_ctx_get_fence(struct 
amdgpu_ctx *ctx,
          return ERR_PTR(-EINVAL);
      }
      if (seq + amdgpu_sched_jobs < cring->sequence) {
          spin_unlock(&ctx->ring_lock);
          return NULL;
      }
      fence = dma_fence_get(cring->fences[seq & (amdgpu_sched_jobs - 
1)]);
      spin_unlock(&ctx->ring_lock);
      return fence;
  }
+void amdgpu_ctx_priority_get(struct amdgpu_ctx *ctx,
+                 enum amd_sched_priority priority)
+{
+    amd_sched_priority_ctr_get(&ctx->priority_ctr, priority);
+
+}
+
+void amdgpu_ctx_priority_put(struct amdgpu_ctx *ctx,
+                 enum amd_sched_priority priority)
+{
+    amd_sched_priority_ctr_put(&ctx->priority_ctr, priority);
+
+}
+
  void amdgpu_ctx_mgr_init(struct amdgpu_ctx_mgr *mgr)
  {
      mutex_init(&mgr->lock);
      idr_init(&mgr->ctx_handles);
  }
  void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr)
  {
      struct amdgpu_ctx *ctx;
      struct idr *idp;
      uint32_t id;
      idp = &mgr->ctx_handles;
      idr_for_each_entry(idp, ctx, id) {


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