[PATCH] drm/amdgpu: add parameter to allocate high priority contexts v8

[Andres Rodriguez]

On 2017-04-25 02:01 PM, Nicolai Hähnle wrote:
> On 24.04.2017 18:20, Andres Rodriguez wrote:
>> Add a new context creation parameter to express a global context
>> priority.
>>
>> The priority ranking in descending order is as follows:
>>  * AMDGPU_CTX_PRIORITY_HIGH
>>  * AMDGPU_CTX_PRIORITY_NORMAL
>>  * AMDGPU_CTX_PRIORITY_LOW
>>
>> The driver will attempt to schedule work to the hardware according to
>> the priorities. No latency or throughput guarantees are provided by
>> this patch.
>>
>> This interface intends to service the EGL_IMG_context_priority
>> extension, and vulkan equivalents.
>>
>> v2: Instead of using flags, repurpose __pad
>> v3: Swap enum values of _NORMAL _HIGH for backwards compatibility
>> v4: Validate usermode priority and store it
>> v5: Move priority validation into amdgpu_ctx_ioctl(), headline reword
>> v6: add UAPI note regarding priorities requiring CAP_SYS_ADMIN
>> v7: remove ctx->priority
>> v8: added AMDGPU_CTX_PRIORITY_LOW, s/CAP_SYS_ADMIN/CAP_SYS_NICE
>>
>> Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>
>> Reviewed-by: Christian König <christian.koenig at amd.com>
>> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com>
>
> I didn't follow all the discussion, so feel free to shut me up if this
> has already been discussed, but...
>
>
> [snip]
>> +/* Context priority level */
>> +#define AMDGPU_CTX_PRIORITY_NORMAL    0
>> +#define AMDGPU_CTX_PRIORITY_LOW        1
>> +/* Selecting a priority above NORMAL requires CAP_SYS_ADMIN */
>> +#define AMDGPU_CTX_PRIORITY_HIGH    2
>> +#define AMDGPU_CTX_PRIORITY_NUM        3
>
> I get that normal priority needs to be 0 for backwards compatibility,
> but having LOW between NORMAL and HIGH is still odd. Have you considered
> using signed integers as a way to fix that?

Thanks for the suggestion, that should make it a lot cleaner.

Regards,
Andres

>
> (AMDGPU_CTX_PRIORITY_NUM doesn't seem to be used anywhere...)
>
> Cheers,
> Nicolai
>
>
>> +
>>  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 */
>>              __u64    flags;
>>              /** Number of resets caused by this context so far. */
>>              __u32    hangs;
>>              /** Reset status since the last call of the ioctl. */
>>              __u32    reset_status;
>>          } state;
>>  };
>>
>>  union drm_amdgpu_ctx {
>>      struct drm_amdgpu_ctx_in in;
>>
>
>