[PATCH] amdgpu: implement context priority for amdgpu_cs_ctx_create2 v2

Emil Velikov emil.l.velikov at gmail.com
Wed Jan 4 13:47:22 UTC 2017


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

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

>
> +/* 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.

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


More information about the amd-gfx mailing list