Re: [igt-dev] [PATCH i-g-t 1/8] tests/i915/gem_pxp: Add LOCAL_ UAPI defines

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Oct 06, 2021 at 01:25:28PM -0400, Rodrigo Vivi wrote:
> On Wed, Oct 06, 2021 at 10:17:41AM -0700, Dixit, Ashutosh wrote:
> > On Wed, 06 Oct 2021 09:14:37 -0700, Rodrigo Vivi wrote:
> > >
> > > While the UAPI changes don't propagate to drm-next we should
> > > have that as LOCAL_ ones.
> > 
> > Please add these in lib/i915/i915_drm_local.h as follows:
> 
> Could we move this file to the include directory and document it
> in the README.md along with the uapi sync mention?
> 
> > 
> > /*
> >  * It is necessary on occasion to add uapi declarations to IGT before they
> >  * appear in imported kernel uapi headers. This header is provided for this
> >  * purpose.
> > 
> >  * Early uapi declarations should be added here exactly as they are
> >  * expected to appear in the kernel uapi headers, i.e. without the LOCAL_
> >  * or local_ prefix and without any #ifndef's. Attempt should be made to
> >  * clean these up when kernel uapi headers are sync'd.
> >  */

I'd like to highlight that I have a concern with this approach without
the local_ prefix.

This approach would force anyone that is syncing the header to solve
everyone's else updates.

It gets even worse and uglier when the api in here diverged from the
actually merged upstream. Okay, one can say this is really rare.

I'm okay with a centralized place for the locals... I believe we even
had that in the past. But I don't like the idea of the lack of prefix.

> > 
> > >
> > > That was my mistake during review and merge since I had
> > > ignored the rules documented at README.md and considered
> > > drm-intel branches would be enough. I'm sorry.
> > >
> > > Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>