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