Re: [PATCH 18/22] drm/amdgpu: add flag for high priority contexts v4

[zhoucm1 <david1.zhou-5C7GfCeVMHo@public.gmane.org>]

On 2017年03月01日 14:52, zhoucm1 wrote:
>
>
> On 2017年03月01日 06:14, Andres Rodriguez wrote:
>> Add a new context creation parameter to express a global context 
>> priority.
>>
>> Contexts allocated with AMDGPU_CTX_PRIORITY_HIGH will receive higher
>> priority to scheduler their work than AMDGPU_CTX_PRIORITY_NORMAL
>> (default) contexts.
>>
>> v2: Instead of using flags, repurpose __pad
>> v3: Swap enum values of _NORMAL _HIGH for backwards compatibility
>> v4: Validate usermode priority and store it
>>
>> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c       | 41 
>> +++++++++++++++++++++++----
>>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.h |  1 +
>>   include/uapi/drm/amdgpu_drm.h                 |  7 ++++-
>>   4 files changed, 44 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index e30c47e..366f6d3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -664,20 +664,21 @@ struct amdgpu_ctx_ring {
>>       struct amd_sched_entity    entity;
>>   };
>>     struct amdgpu_ctx {
>>       struct kref        refcount;
>>       struct amdgpu_device    *adev;
>>       unsigned        reset_counter;
>>       spinlock_t        ring_lock;
>>       struct dma_fence    **fences;
>>       struct amdgpu_ctx_ring    rings[AMDGPU_MAX_RINGS];
>> +    int            priority;
>>       bool preamble_presented;
>>   };
>>     struct amdgpu_ctx_mgr {
>>       struct amdgpu_device    *adev;
>>       struct mutex        lock;
>>       /* protected by lock */
>>       struct idr        ctx_handles;
>>   };
>>   diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index 400c66b..22a15d6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -18,47 +18,75 @@
>>    * 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: monk liu <monk.liu@amd.com>
>>    */
>>     #include <drm/drmP.h>
>>   #include "amdgpu.h"
>>   -static int amdgpu_ctx_init(struct amdgpu_device *adev, struct 
>> amdgpu_ctx *ctx)
>> +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_NORMAL:
>> +        return AMD_SCHED_PRIORITY_NORMAL;
>> +    default:
>> +        WARN(1, "Invalid context priority %d\n", amdgpu_priority);
>> +        return AMD_SCHED_PRIORITY_NORMAL;
>> +    }
>> +}
>> +
>> +static int amdgpu_ctx_init(struct amdgpu_device *adev,
>> +                uint32_t priority,
>> +                struct amdgpu_ctx *ctx)
>>   {
>>       unsigned i, j;
>>       int r;
>> +    enum amd_sched_priority sched_priority;
>> +
>> +    sched_priority = amdgpu_to_sched_priority(priority);
>> +
>> +    if (priority >= AMDGPU_CTX_PRIORITY_NUM)
>> +        return -EINVAL;
>> +
>> +    if (sched_priority < 0 || sched_priority >= AMD_SCHED_MAX_PRIORITY)
>> +        return -EINVAL;
>> +
>> +    if (sched_priority == AMD_SCHED_PRIORITY_HIGH && 
>> !capable(CAP_SYS_ADMIN))
>> +        return -EACCES;
>>         memset(ctx, 0, sizeof(*ctx));
>>       ctx->adev = adev;
>> +    ctx->priority = priority;
> seems not used.
I see ctx->priority is used in following patches, so pls remove it there.

Regards,
David Zhou
>
> Regards,
> David Zhou
>>       kref_init(&ctx->refcount);
>>       spin_lock_init(&ctx->ring_lock);
>>       ctx->fences = kcalloc(amdgpu_sched_jobs * AMDGPU_MAX_RINGS,
>>                     sizeof(struct dma_fence*), GFP_KERNEL);
>>       if (!ctx->fences)
>>           return -ENOMEM;
>>         for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>>           ctx->rings[i].sequence = 1;
>>           ctx->rings[i].fences = &ctx->fences[amdgpu_sched_jobs * i];
>>       }
>>         ctx->reset_counter = atomic_read(&adev->gpu_reset_counter);
>>         /* create context entity for each ring */
>>       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[AMD_SCHED_PRIORITY_NORMAL];
>> +        rq = &ring->sched.sched_rq[sched_priority];
>>           r = amd_sched_entity_init(&ring->sched, &ctx->rings[i].entity,
>>                         rq, amdgpu_sched_jobs);
>>           if (r)
>>               goto failed;
>>       }
>>         return 0;
>>     failed:
>>       for (j = 0; j < i; j++)
>> @@ -83,39 +111,41 @@ static void amdgpu_ctx_fini(struct amdgpu_ctx *ctx)
>>       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);
>>   }
>>     static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
>>                   struct amdgpu_fpriv *fpriv,
>> +                uint32_t priority,
>>                   uint32_t *id)
>>   {
>>       struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
>>       struct amdgpu_ctx *ctx;
>>       int r;
>>         ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
>>       if (!ctx)
>>           return -ENOMEM;
>>         mutex_lock(&mgr->lock);
>>       r = idr_alloc(&mgr->ctx_handles, ctx, 1, 0, GFP_KERNEL);
>>       if (r < 0) {
>>           mutex_unlock(&mgr->lock);
>>           kfree(ctx);
>>           return r;
>>       }
>> +
>>       *id = (uint32_t)r;
>> -    r = amdgpu_ctx_init(adev, ctx);
>> +    r = amdgpu_ctx_init(adev, priority, ctx);
>>       if (r) {
>>           idr_remove(&mgr->ctx_handles, *id);
>>           *id = 0;
>>           kfree(ctx);
>>       }
>>       mutex_unlock(&mgr->lock);
>>       return r;
>>   }
>>     static void amdgpu_ctx_do_release(struct kref *ref)
>> @@ -179,32 +209,33 @@ static int amdgpu_ctx_query(struct 
>> amdgpu_device *adev,
>>       ctx->reset_counter = reset_counter;
>>         mutex_unlock(&mgr->lock);
>>       return 0;
>>   }
>>     int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
>>                struct drm_file *filp)
>>   {
>>       int r;
>> -    uint32_t id;
>> +    uint32_t id, priority;
>>         union drm_amdgpu_ctx *args = data;
>>       struct amdgpu_device *adev = dev->dev_private;
>>       struct amdgpu_fpriv *fpriv = filp->driver_priv;
>>         r = 0;
>>       id = args->in.ctx_id;
>> +    priority = args->in.priority;
>>         switch (args->in.op) {
>>       case AMDGPU_CTX_OP_ALLOC_CTX:
>> -        r = amdgpu_ctx_alloc(adev, fpriv, &id);
>> +        r = amdgpu_ctx_alloc(adev, fpriv, priority, &id);
>>           args->out.alloc.ctx_id = id;
>>           break;
>>       case AMDGPU_CTX_OP_FREE_CTX:
>>           r = amdgpu_ctx_free(fpriv, id);
>>           break;
>>       case AMDGPU_CTX_OP_QUERY_STATE:
>>           r = amdgpu_ctx_query(adev, fpriv, id, &args->out);
>>           break;
>>       default:
>>           return -EINVAL;
>> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h 
>> b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> index d8dc681..2e458de 100644
>> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.h
>> @@ -101,20 +101,21 @@ static inline struct amd_sched_fence 
>> *to_amd_sched_fence(struct dma_fence *f)
>>   */
>>   struct amd_sched_backend_ops {
>>       struct dma_fence *(*dependency)(struct amd_sched_job *sched_job);
>>       struct dma_fence *(*run_job)(struct amd_sched_job *sched_job);
>>       void (*timedout_job)(struct amd_sched_job *sched_job);
>>       void (*free_job)(struct amd_sched_job *sched_job);
>>   };
>>     enum amd_sched_priority {
>>       AMD_SCHED_PRIORITY_KERNEL = 0,
>> +    AMD_SCHED_PRIORITY_HIGH,
>>       AMD_SCHED_PRIORITY_NORMAL,
>>       AMD_SCHED_MAX_PRIORITY
>>   };
>>     /**
>>    * One scheduler is implemented for each hardware ring
>>   */
>>   struct amd_gpu_scheduler {
>>       const struct amd_sched_backend_ops    *ops;
>>       uint32_t            hw_submission_limit;
>> diff --git a/include/uapi/drm/amdgpu_drm.h 
>> b/include/uapi/drm/amdgpu_drm.h
>> index b5ae774..b756599 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -153,27 +153,32 @@ union drm_amdgpu_bo_list {
>>     /* GPU reset status */
>>   #define AMDGPU_CTX_NO_RESET        0
>>   /* this the context caused it */
>>   #define AMDGPU_CTX_GUILTY_RESET        1
>>   /* some other context caused it */
>>   #define AMDGPU_CTX_INNOCENT_RESET    2
>>   /* unknown cause */
>>   #define AMDGPU_CTX_UNKNOWN_RESET    3
>>   +/* Context priority level */
>> +#define AMDGPU_CTX_PRIORITY_NORMAL    0
>> +#define AMDGPU_CTX_PRIORITY_HIGH    1
>> +#define AMDGPU_CTX_PRIORITY_NUM        2
>> +
>>   struct drm_amdgpu_ctx_in {
>>       /** AMDGPU_CTX_OP_* */
>>       __u32    op;
>>       /** For future use, no flags defined so far */
>>       __u32    flags;
>>       __u32    ctx_id;
>> -    __u32    _pad;
>> +    __u32    priority;
>>   };
>>     union drm_amdgpu_ctx_out {
>>           struct {
>>               __u32    ctx_id;
>>               __u32    _pad;
>>           } alloc;
>>             struct {
>>               /** For future use, no flags defined so far */
>
> _______________________________________________
> 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