xe generated headers (was: Re: [PATCH] drm/xe/ptl: L3bank mask is not available on the media GT)

[Lucas De Marchi]

On Wed, Oct 09, 2024 at 02:55:59PM +0300, Jani Nikula wrote:
>On Fri, 04 Oct 2024, Matt Roper <matthew.d.roper at intel.com> wrote:
>> diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
>> index 651ba53623e5..df2042db7ee6 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_topology.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_topology.c
>> @@ -5,6 +5,7 @@
>>
>>  #include "xe_gt_topology.h"
>>
>> +#include <generated/xe_wa_oob.h>
>
>Hijacking the thread here a bit, sorry.
>
>This use of system include path <> instead of "" for xe generated
>headers keeps bugging me.
>
>There's a global directory for generated headers, and that's what
><generated/...> usually means. See:
>
>	git grep -h "#include <generated/" | sort | uniq
>
>Or look at top level include/generated in the build directory.
>
>Especially putting #include <generated/...> next to #include <linux/...>
>implies we're referring to the system generated headers, but we're not.
>
>I think we should be using #include "generated/xe_wa_oob.h" instead,

I'm ok with this. As long as we have
`subdir-ccflags-y += -I$(obj) -I$(src)` this should still work.

>possibly with a completely different subdirectory name to not have the
>name collision and confusion at all.

I think changing the name because of this is ugly and unnecesary.

>
>Another alternative is to do this in the Makefile:
>
>	subdir-ccflags-y += -I$(obj)/generated -I$(src)
>
>and just use #include "xe_wa_oob.h" directly. (FWIW this is what msm
>does.)

I like to keep the "generated" in the name since it's then clear
that file isn't in the source checkout, but one should look at build
directory instead. And even if someone mixes the build and source dir,
there's no possibility of a clash.

I view this differently than what you are writting above. There is no
"system/global" includes in the kernel. There are only the include paths
passed to the compiler and the only difference between " and <  is that
for the former the compiler will *first* try to find the file relative
to the compilation unit.

As for clashes, if someone adds a include/generated/xe_wa_oob.h  file,
with exact the same name, there's something really odd there. 

in summary:

#include "generated/xe_wa_oob.h"

has my ack.... which is where we may eventually put the regs/ too.

Lucas De Marchi

>
>
>BR,
>Jani.
>
>
>-- 
>Jani Nikula, Intel