[PATCH i-g-t 1/3] lib/i915/perf: include i915_pciid_local.h

[Borah, Chaitanya Kumar]

Hello Kamil,

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Sent: Tuesday, January 9, 2024 6:07 PM
> To: igt-dev at lists.freedesktop.org
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>;
> juhapekka.heikkila at gmail.com; Landwerlin, Lionel G
> <lionel.g.landwerlin at intel.com>; Vishwanathapura, Niranjana
> <niranjana.vishwanathapura at intel.com>; Roper, Matthew D
> <matthew.d.roper at intel.com>
> Subject: Re: [PATCH i-g-t 1/3] lib/i915/perf: include i915_pciid_local.h
> 
> Hi Chaitanya,
> On 2024-01-09 at 11:39:41 +0530, Chaitanya Kumar Borah wrote:
> > perf.c uses PCI ID macros not found in kernel header i915_pciid.h.
> > Therefore include i915_pciid_local.h instead of i915_pciid.h so that
> -------------------------------------- ^^^^^^^^^^ This is somewhat misleading
> because _local.h will include i915_pciids.h imho s/instead of/and/
> 
> Or other way to put it:
> 
> Therefore include i915_pciid_local.h (which will include i915_pciid.h) so that
> 
> > we can move these macros to the local file. That way we don't lose
> > them when i915_pciid.h is synced with kernel.
> >
> > Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>
> > Cc: Matt Roper <matthew.d.roper at intel.com>
> > Cc: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> > Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> > Cc: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
> >
> > ---
> >  lib/i915/perf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/i915/perf.c b/lib/i915/perf.c index
> > ddadb53b6..2006dd4ad 100644
> > --- a/lib/i915/perf.c
> > +++ b/lib/i915/perf.c
> > @@ -37,7 +37,7 @@
> >
> >  #include <i915_drm.h>
> >
> > -#include "i915_pciids.h"
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Consider adding here comment like:
> /* i915_pciids.h will be included by i915_pciids_local.h */
> 
> or maybe even just leave that include here.
> 

I decided to go with this as there is already a precedence in lib/intel_device_info.c

Thank you for your feedback. A new version is available now.

Regards
Chaitanya

> Regards,
> Kamil
> 
> > +#include "i915_pciids_local.h"
> >
> >  #include "intel_chipset.h"
> >  #include "perf.h"
> > --
> > 2.25.1
> >