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

[Andres Rodriguez]

On 2017-04-25 06:21 PM, Alex Deucher wrote:
> 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
>
>

Sure, that gives us a range to add new priority leves.

Andres

>>
>> 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