Re: [PATCH libdrm 1/4] intel: add IS_GENX() generic macro

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Tue, Aug 28, 2018 at 09:38:59AM +0100, Chris Wilson wrote:
> Quoting Lucas De Marchi (2018-08-28 02:00:27)
> > On Sat, Aug 25, 2018 at 10:35:23AM +0100, Chris Wilson wrote:
> > > Quoting Lucas De Marchi (2018-08-25 00:56:46)
> > > > diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
> > > > index 4a34b7be..8a0e3e76 100644
> > > > --- a/intel/intel_chipset.h
> > > > +++ b/intel/intel_chipset.h
> > > > @@ -568,6 +568,26 @@
> > > >  
> > > >  #define IS_GEN11(devid)                (IS_ICELAKE_11(devid))
> > > >  
> > > > +/* New platforms use kernel pci ids */
> > > > +#include "i915_pciids.h"
> > > > +
> > > > +struct pci_device_id {
> > > 
> > > Don't call it pci_device_id, depending on caller that name may already
> > > be taken by libpciaccess.
> > > 
> > > > +       uint32_t unused0, device;
> > > > +       uint32_t unused1, unused2;
> > > > +       uint32_t unused3, unused4;
> > > These are all uint16_t.
> > 
> > more on this:
> > 
> > I can make the first 2 uint16_t, but not the rest due to the way they
> > are declared in INTEL_VGA_DEVICE: (~0 has int type by default), unused3
> > and unused4 are clearly not uint16_t
> 
> I had it in my mind that we did have one extra level of macro in there
> that would allow us to drop unused fields. We could redef

we don't have right now, that needs to be added. And for any extra level
of macro we need to redef INTEL_VGA_DEVICE nonetheless because it
expands to "{ ... }". I don't know any cpp trick to remove the extra
fields if it keeps the braces. 

> INTEL_VGA_DEVICE() and INTEL_QUANTA_VGA_DEVICE() but one extra level of
> macro would be easier for future.

So... I don't see a way out except to redef it. This works today for
libdrm (on top of my unsent version of this patch):


[ NO CI ] diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
[ NO CI ] index 79581819..8ea24194 100644
[ NO CI ] --- a/intel/intel_chipset.c
[ NO CI ] +++ b/intel/intel_chipset.c
@@ -27,11 +27,12 @@
 
 #include "i915_pciids.h"
 
+#undef INTEL_VGA_DEVICE
+#define INTEL_VGA_DEVICE(id, gen) { id, gen }
+
 static const struct pci_device {
-	uint16_t unused0, device;
-	uint32_t unused1, unused2;
-	uint32_t unused3, unused4;
-	int gen;
+	uint16_t device;
+	uint16_t gen;
 } pciids[] = {
 	INTEL_ICL_11_IDS(11),
 	INTEL_CNL_IDS(10),

> 
> And then while you are there, add the missing 'u' to ~0u
> -Chris.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx