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

Andres Rodriguez andresx7 at gmail.com
Fri Jun 9 19:07:52 UTC 2017



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 <andresx7 at 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) {
> 
> 


More information about the amd-gfx mailing list