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

[Alex Deucher]

On Tue, Apr 25, 2017 at 4:28 PM, Andres Rodriguez <andresx7 at gmail.com> wrote:
>
>
> 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.

Maybe make the range -1023 to 1023 for consistency with the similar
proposed interface on Intel?
https://lists.freedesktop.org/archives/intel-gfx/2017-April/126155.html

Alex


>
> 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;
>>>
>>
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx