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

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

On Tue, 23 Jan 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> 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.

I've held off on pushing this. What's the verdict?

BR,
Jani.


-- 
Jani Nikula, Intel