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