Re: Re: [PATCH 2/2] drm/xe: drop display/ subdir from include directories

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 23 Jan 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Mon, Jan 22, 2024 at 07:39:56PM +0200, Jani Nikula wrote:
>>On Mon, 22 Jan 2024, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>>> the downside of patch 1 is now that core xe code can include any of the
>>> display/ headers, but only xe_display.h is acceptable to keep the
>>> interface sane.
>>
>>I'd say xe_display.h remains the only interface towards xe core, I don't
>>think this patch changes that, and its location doesn't really give any
>>guarantees. It's been a matter of sticking display/ in the include
>>anyway, and enforcing that is a matter of maintainer vigilance.
>
> I just thought the previous split was slightly better: No code in xe
> should include display/ and should rather use the xe_display.[hc]
> interface.
>
> Now with #include "display/xe_display.h" spread throughout the code,
> this could serve as example for people to start including stuff they
> shouldn't.
>
> I'm not entirely opposed, so if you and others agree, please go ahead.

Well, at the moment you can include *anything* from under display/
*without* the prefix, because it's all in the include path. (Although
that will fail for DRM_XE_DISPLAY=n, but does CI even build that combo
regularly?)

I'm fine with dropping this too. I just think it makes it easier to
check that nothing outside of display/ does any displayish things.

Your call.


BR,
Jani.

>
> Lucas De Mrachi
>
>>
>>> Or are you thinking about changing the interface?
>>
>>I agree the interface should be in one file only, but changing the
>>interface is an orthogonal matter (I have no plans atm).
>>
>>BR,
>>Jani.
>>
>>
>>-- 
>>Jani Nikula, Intel

-- 
Jani Nikula, Intel