[PATCH i-g-t,v1 0/4] L3 bank mask

[Zbigniew Kempczyński]

On Wed, Apr 17, 2024 at 11:07:54AM -0500, Lucas De Marchi wrote:

<cut>

> 
> include/drm-uapi/xe_drm.h should always and only be updated by copying
> from kernel from specific commits documented in the commit message.
> 
> WIP changes while the kernel side is not merged should be moved to
> another header. Traditionally we used LOCAL_* defines for that. But that
> comes with its own problems as it's more work to  go find them all and
> drop when we sync the headers.
> 
> What about doing this?
> 
> Add include/drm-uapi/local/ to -I in build system *before* the non-local
> one. Something like:
> 
> meson.build:
> 	inc = include_directories('include', 'include/drm-uapi/local' 'include/drm-uapi', 'include/linux-uapi', 'lib', 'lib/stubs/syscalls', '.')
> 
> Example content for
> include/drm-uapi/local/drm_xe.h:
> 	#pragma once
> 	#include_next <xe_drm.h>
> 
> 	/*
> 	 * Local definitions - delta with WIP UAPI while the upstream defines
> 	 * are not yet merged. These are supposed to conflict whenever the
> 	 * upstream header is updated and break the build. That is intended.
> 	 * Commit updating the upstream header should garbage collect these
> 	 * local definitions.
> 	 */
> 
> 	#ifdef DRM_XE_TOPO_L3_BANK
> 	#error "Already defined by upstream - remove"
> 	#define DRM_XE_TOPO_L3_BANK		3
> 	#endif
> 
> The ifdef + error could be optional as it's harmless (and allowed by
> compiler) to redefine it multiple times to the same value.

I like this solution and I vote for yes. As duplicates (definition
with same value) are allowed by the compiler, only drawback I see
is potentially have same definition across uapi headers - local
and global for some period. This is however not a problem, especially
same is with LOCAL_*.

--
Zbigniew

> 
> Thoughts?
> 
> If we don't want to decide on this and you only want to unblock this
> test for the kernel addition, then maybe just do the same LOCAL_
> approach currently being used. See e.g. lib/i915/i915_drm_local.h
> 
> Lucas De Marchi
> 
> >  lib/xe/xe_query: Add L3 bank mask test
> > 
> > include/drm-uapi/xe_drm.h | 27 +++-------------
> > tests/intel/xe_query.c    | 66 +++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 71 insertions(+), 22 deletions(-)
> > 
> > -- 
> > 2.34.1
> >