Re: [PATCH] amdgpu: implement context priority for amdgpu_cs_ctx_create2 v2

[Andres Rodriguez <andresx7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Thanks for the feedback Emil, it is very welcome :)

On 1/4/2017 8:47 AM, Emil Velikov wrote:
> Hi Andres,
>
> Pardon for jumping in uninvited, there's a few comments below that you
> might find useful.
>
> On 4 January 2017 at 01:56, Andres Rodriguez <andresx7@gmail.com> wrote:
>
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -20,7 +20,7 @@
>>
>>   AC_PREREQ([2.63])
>>   AC_INIT([libdrm],
>> -        [2.4.74],
>> +        [2.4.75],
> This part should be separate and done only as one rolls a release.
> Currently we have more than a dozen of people who can do that, with 3+
> from the AMD team.
I'll take this out of the patch.
>
>>           [https://bugs.freedesktop.org/enter_bug.cgi?product=DRI],
>>           [libdrm])
>>
>> diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
>> index d8f2497..01cf3c9 100644
>> --- a/include/drm/amdgpu_drm.h
>> +++ b/include/drm/amdgpu_drm.h
> Please sync this file as described in the README, in the same directory.
Thanks for pointing that out, I totally missed the README
>
>> +/* Context priority level */
>> +#define AMDGPU_CTX_PRIORITY_HIGH       0
>> +#define AMDGPU_CTX_PRIORITY_NORMAL     1
> You really want NORMAL to be 0 here, to preserve the behaviour. In
> other words, [sane] old userspace with new kernel will set HIGH prio,
> where NORMAL is used with older kernels.
Will fix.
>
>> +
>>   struct drm_amdgpu_ctx_in {
>>          /** AMDGPU_CTX_OP_* */
>>          uint32_t        op;
>>          /** For future use, no flags defined so far */
>>          uint32_t        flags;
>>          uint32_t        ctx_id;
>> -       uint32_t        _pad;
>> +       uint32_t        priority;
> This part is a bit of meh:
>
> Namely things can go crazy in either one of these scenarios:
>   - old kernel [validates _pad to be zero/other], new userspace feeds
> "invalid" value.
>   - new kernel. old [nasty] userspace which leaves the value uninitialised.
>
> Re-using _pad is ok, just make sure things don't explode all over the place.
>
> Regards,
> Emil
Regards,
Andres

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx