Re: [PATCH 03/17] drm/xe/oa/uapi: Add OA data formats

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

On Thu, Jun 13, 2024 at 05:39:48PM +0200, Michal Wajdeczko wrote:
> 
> 
> On 12.06.2024 04:03, Dixit, Ashutosh wrote:
> > On Sat, 08 Jun 2024 03:54:16 -0700, Michal Wajdeczko wrote:
> 
> ...
> 
> >>> diff --git a/drivers/gpu/drm/xe/xe_oa.h b/drivers/gpu/drm/xe/xe_oa.h
> >>> new file mode 100644
> >>> index 000000000000..a2f301e2be57
> >>> --- /dev/null
> >>> +++ b/drivers/gpu/drm/xe/xe_oa.h
> >>> @@ -0,0 +1,16 @@
> >>> +/* SPDX-License-Identifier: MIT */
> >>> +/*
> >>> + * Copyright © 2023 Intel Corporation
> >>> + */
> >>> +
> >>> +#ifndef _XE_OA_H_
> >>> +#define _XE_OA_H_
> >>> +
> >>> +#include "xe_oa_types.h"
> >>
> >> don't include full header if you already have required forward decl
> > 
> > This one is unrelated. xe_oa.h is included in other files such as
> > xe_device.c and those files need declarations in xe_oa_types.h. So I have
> > left this as is. This is the same pattern as followed e.g. in
> > xe_bb.h/xe_bb_types.h, xe_bo.h/xe_bo_types.h, xe_device.h/xe_device_types.h
> > etc.
> > 
> 
> I didn't know that we have that pattern defined as BKM, usually extra
> includes are defined only if you provide some inline helpers that would
> need them, so I would ask Lucas for opinion here

Well, I don't have a very strong preference here.
In general we shouldn't include header from header, but I don't see
an issue with the header including its own 'types' header.

Perhaps for consistency we should avoid it and then come back later aand fix
these mentioned precedences here. But since we have these cases I would also
not block if everything is ready to get merged. Then we can work on a follow
up fixing this and the older cases all together.

> 
> Michal
> 
> 
> >
> >> +
> >> +struct xe_device;
> >> +
> >> +int xe_oa_init(struct xe_device *xe);
> >> +void xe_oa_fini(struct xe_device *xe);
> >> +
> >> +#endif