Re: [PATCH 1/2] intel: Add more Kabylake PCI IDs.

From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
To: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Cc: "mesa-dev@lists.freedesktop.org" <mesa-dev@lists.freedesktop.org>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] intel: Add more Kabylake PCI IDs.
Date: Fri, 24 Jun 2016 22:57:10 +0000	[thread overview]
Message-ID: <1466808970.1933.107.camel@intel.com> (raw)
In-Reply-To: <1466808963.27063.13.camel@dk-H97M-D3H>

On Fri, 2016-06-24 at 22:42 +0000, Pandiyan, Dhinakaran wrote:
> On Thu, 2016-06-23 at 14:50 -0700, Rodrigo Vivi wrote:
> > 
> > The spec has been updated adding new PCI IDs.
> > 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  intel/intel_chipset.h | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
> > index e2554c3..0c3ca82 100644
> > --- a/intel/intel_chipset.h
> > +++ b/intel/intel_chipset.h
> > @@ -194,7 +194,9 @@
> >  #define PCI_CHIP_KABYLAKE_ULT_GT2	0x5916
> >  #define PCI_CHIP_KABYLAKE_ULT_GT1_5	0x5913
> >  #define PCI_CHIP_KABYLAKE_ULT_GT1	0x5906
> > -#define PCI_CHIP_KABYLAKE_ULT_GT3	0x5926
> > +#define PCI_CHIP_KABYLAKE_ULT_GT3_0	0x5923
> > +#define PCI_CHIP_KABYLAKE_ULT_GT3_1	0x5926
> > +#define PCI_CHIP_KABYLAKE_ULT_GT3_2	0x5927
> >  #define PCI_CHIP_KABYLAKE_ULT_GT2F	0x5921
> >  #define PCI_CHIP_KABYLAKE_ULX_GT1_5	0x5915
> >  #define PCI_CHIP_KABYLAKE_ULX_GT1	0x590E
> > @@ -206,7 +208,8 @@
> >  #define PCI_CHIP_KABYLAKE_HALO_GT2	0x591B
> >  #define PCI_CHIP_KABYLAKE_HALO_GT4	0x593B
> >  #define PCI_CHIP_KABYLAKE_HALO_GT3	0x592B
> > -#define PCI_CHIP_KABYLAKE_HALO_GT1	0x590B
> > +#define PCI_CHIP_KABYLAKE_H_GT1_0	0x5908
> > +#define PCI_CHIP_KABYLAKE_H_GT1_1	0x590B
> Does H here mean Halo? Some of defines have the whole word "HALO" and
> some "H". Shouldn't we keep that uniform? 

I also like keeping it uniform, but was trying to keep the alignment...
otherwise I'd have to touch all lines and didn't want change all lines
that are not directly being affected....

Yeap H for Halo, like U for ULT and like Y for ULX :/
(KBL ULX are actually more known as KBL-Y. Nobody uses the terms ULT
and ULX after BDW...)

But you point is definitely good. I just don't know if I send one patch
to just fix the name convention and one to fix the ids or let HALO and
mess with alignments... suggestions?

> 
> > 
> >  #define PCI_CHIP_KABYLAKE_SRV_GT2	0x591A
> >  #define PCI_CHIP_KABYLAKE_SRV_GT3	0x592A
> >  #define PCI_CHIP_KABYLAKE_SRV_GT1	0x590A
> > @@ -414,7 +417,8 @@
> >  				 (devid) ==
> > PCI_CHIP_KABYLAKE_ULT_GT1	|| \
> >  				 (devid) ==
> > PCI_CHIP_KABYLAKE_ULX_GT1	|| \
> >  				 (devid) ==
> > PCI_CHIP_KABYLAKE_DT_GT1	|| \
> > -				 (devid) ==
> > PCI_CHIP_KABYLAKE_HALO_GT1	|| \
> > +				 (devid) ==
> > PCI_CHIP_KABYLAKE_H_GT1_0	|| \
> > +				 (devid) ==
> > PCI_CHIP_KABYLAKE_H_GT1_1	|| \
> >  				 (devid) ==
> > PCI_CHIP_KABYLAKE_SRV_GT1)
> >  
> >  #define IS_KBL_GT2(devid)	((devid) ==
> > PCI_CHIP_KABYLAKE_ULT_GT2	|| \
> > @@ -425,7 +429,9 @@
> >  				 (devid) ==
> > PCI_CHIP_KABYLAKE_SRV_GT2	|| \
> >  				 (devid) ==
> > PCI_CHIP_KABYLAKE_WKS_GT2)
> >  
> > -#define IS_KBL_GT3(devid)	((devid) ==
> > PCI_CHIP_KABYLAKE_ULT_GT3	|| \
> > +#define IS_KBL_GT3(devid)	((devid) ==
> > PCI_CHIP_KABYLAKE_ULT_GT3_0	|| \
> > +				 (devid) ==
> > PCI_CHIP_KABYLAKE_ULT_GT3_1	|| \
> > +				 (devid) ==
> > PCI_CHIP_KABYLAKE_ULT_GT3_2	|| \
> >  				 (devid) ==
> > PCI_CHIP_KABYLAKE_HALO_GT3	|| \
> >  				 (devid) ==
> > PCI_CHIP_KABYLAKE_SRV_GT3)
> >  
> I verified the PCI IDs against the Spec, looks good to me.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx