diff mbox series

[libdrm,v2,4/5] intel: make gen9 use generic gen macro

Message ID 20180829003532.22878-5-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series intel: rework how we add PCI IDs | expand

Commit Message

Lucas De Marchi Aug. 29, 2018, 12:35 a.m. UTC 
The 2 PCI IDs that are used for the command line overrid mechanism
were left defined. The rest can be gone and then we just use the kernel
defines.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 intel/intel_chipset.c |   5 ++
 intel/intel_chipset.h | 187 +-----------------------------------------
 2 files changed, 6 insertions(+), 186 deletions(-)

Comments

Chris Wilson Aug. 29, 2018, 10:32 a.m. UTC | #1 
Quoting Lucas De Marchi (2018-08-29 01:35:31)
> The 2 PCI IDs that are used for the command line overrid mechanism
> were left defined.

What makes them so special? Why not just match on the override devid?
-Chris
Lucas De Marchi Aug. 29, 2018, 4:01 p.m. UTC | #2 
On Wed, Aug 29, 2018 at 11:32:35AM +0100, Chris Wilson wrote:
> Quoting Lucas De Marchi (2018-08-29 01:35:31)
> > The 2 PCI IDs that are used for the command line overrid mechanism
> > were left defined.
> 
> What makes them so special? Why not just match on the override devid?

because it's a name -> id mapping? It maps a short string like "skl" to
a single specific PCI ID... how useful is that and if we should retain
its behavior, I have dunno. But
i915_pciids.h doesn't have defines for individual PCI IDs, but groups of
them.

I would either have to create an accessor/iter for gen x in
intel_chipset.c or do some macros to extract the first id from the
i915_pciids.h, just to get an ID that is set in stone and change the
current id used :-/

Lucas De Marchi
Chris Wilson Aug. 31, 2018, 8:20 a.m. UTC | #3 
Quoting Lucas De Marchi (2018-08-29 17:01:11)
> On Wed, Aug 29, 2018 at 11:32:35AM +0100, Chris Wilson wrote:
> > Quoting Lucas De Marchi (2018-08-29 01:35:31)
> > > The 2 PCI IDs that are used for the command line overrid mechanism
> > > were left defined.
> > 
> > What makes them so special? Why not just match on the override devid?
> 
> because it's a name -> id mapping? It maps a short string like "skl" to
> a single specific PCI ID... how useful is that and if we should retain
> its behavior, I have dunno. But
> i915_pciids.h doesn't have defines for individual PCI IDs, but groups of
> them.

My bad, I've always used pci-id overrides as a pci-id!

> I would either have to create an accessor/iter for gen x in
> intel_chipset.c or do some macros to extract the first id from the
> i915_pciids.h, just to get an ID that is set in stone and change the
> current id used :-/

Having the i915_pciids.h contain the codename (and /rough/ marketing
name) was pencilled in to my plans now that it no longer appears to be a
freak out. Once we have configurable macros, the extra parameters just
disappear when unwanted.
-Chris
Chris Wilson Aug. 31, 2018, 8:21 a.m. UTC | #4 
Quoting Lucas De Marchi (2018-08-29 01:35:31)
> The 2 PCI IDs that are used for the command line overrid mechanism
> were left defined. The rest can be gone and then we just use the kernel
> defines.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  intel/intel_chipset.c |   5 ++
>  intel/intel_chipset.h | 187 +-----------------------------------------
>  2 files changed, 6 insertions(+), 186 deletions(-)
> 
> diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
> index 0c2ba884..c984d8ac 100644
> --- a/intel/intel_chipset.c
> +++ b/intel/intel_chipset.c
> @@ -36,6 +36,11 @@ static const struct pci_device {
>  } pciids[] = {
>         INTEL_ICL_11_IDS(11),
>         INTEL_CNL_IDS(10),
> +       INTEL_CFL_IDS(9),
> +       INTEL_GLK_IDS(9),
> +       INTEL_KBL_IDS(9),
> +       INTEL_BXT_IDS(9),
> +       INTEL_SKL_IDS(9),

The gradual conversion lgtm. But why stop here? :)
-Chris
Lucas De Marchi Aug. 31, 2018, 4:14 p.m. UTC | #5 
On Fri, Aug 31, 2018 at 09:21:32AM +0100, Chris Wilson wrote:
> Quoting Lucas De Marchi (2018-08-29 01:35:31)
> > The 2 PCI IDs that are used for the command line overrid mechanism
> > were left defined. The rest can be gone and then we just use the kernel
> > defines.
> > 
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > ---
> >  intel/intel_chipset.c |   5 ++
> >  intel/intel_chipset.h | 187 +-----------------------------------------
> >  2 files changed, 6 insertions(+), 186 deletions(-)
> > 
> > diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
> > index 0c2ba884..c984d8ac 100644
> > --- a/intel/intel_chipset.c
> > +++ b/intel/intel_chipset.c
> > @@ -36,6 +36,11 @@ static const struct pci_device {
> >  } pciids[] = {
> >         INTEL_ICL_11_IDS(11),
> >         INTEL_CNL_IDS(10),
> > +       INTEL_CFL_IDS(9),
> > +       INTEL_GLK_IDS(9),
> > +       INTEL_KBL_IDS(9),
> > +       INTEL_BXT_IDS(9),
> > +       INTEL_SKL_IDS(9),
> 
> The gradual conversion lgtm. But why stop here? :)

From cover letter:

	Initially my plan was to convert all gens, back to gen2, but
	that proved slightly difficult since there are some corner cases
	to cover and I didn't want to block the important part, i.e.:
	for recent gens, there's no risk of missing a PCI ID.

With the last approach moving the implementation to a .c file I think it
will be easier to implement for older gens, but there's no point in
doing the manual boring labor of converting all gens just to have to
change the approach in a v2, v3 of the patch set. Like I did for v1 ->
v2.  I can convert the rest if we agree the current approach is
okish

I'm even ok with letting older ones as is since I hope we won't add a
new pci id for e.g. gen3, so I won't have to touch that.

Lucas De Marchi

> -Chris
diff mbox series

Patch

diff --git a/intel/intel_chipset.c b/intel/intel_chipset.c
index 0c2ba884..c984d8ac 100644
--- a/intel/intel_chipset.c
+++ b/intel/intel_chipset.c
@@ -36,6 +36,11 @@  static const struct pci_device {
 } pciids[] = {
 	INTEL_ICL_11_IDS(11),
 	INTEL_CNL_IDS(10),
+	INTEL_CFL_IDS(9),
+	INTEL_GLK_IDS(9),
+	INTEL_KBL_IDS(9),
+	INTEL_BXT_IDS(9),
+	INTEL_SKL_IDS(9),
 };
 
 bool intel_is_genx(unsigned int devid, int gen)
diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
index 6790f728..19263e19 100644
--- a/intel/intel_chipset.h
+++ b/intel/intel_chipset.h
@@ -165,86 +165,8 @@ 
 #define PCI_CHIP_CHERRYVIEW_2		0x22b2
 #define PCI_CHIP_CHERRYVIEW_3		0x22b3
 
-#define PCI_CHIP_SKYLAKE_DT_GT1		0x1902
-#define PCI_CHIP_SKYLAKE_ULT_GT1	0x1906
-#define PCI_CHIP_SKYLAKE_SRV_GT1	0x190A /* Reserved */
-#define PCI_CHIP_SKYLAKE_H_GT1		0x190B
-#define PCI_CHIP_SKYLAKE_ULX_GT1	0x190E /* Reserved */
 #define PCI_CHIP_SKYLAKE_DT_GT2		0x1912
-#define PCI_CHIP_SKYLAKE_FUSED0_GT2	0x1913 /* Reserved */
-#define PCI_CHIP_SKYLAKE_FUSED1_GT2	0x1915 /* Reserved */
-#define PCI_CHIP_SKYLAKE_ULT_GT2	0x1916
-#define PCI_CHIP_SKYLAKE_FUSED2_GT2	0x1917 /* Reserved */
-#define PCI_CHIP_SKYLAKE_SRV_GT2	0x191A /* Reserved */
-#define PCI_CHIP_SKYLAKE_HALO_GT2	0x191B
-#define PCI_CHIP_SKYLAKE_WKS_GT2 	0x191D
-#define PCI_CHIP_SKYLAKE_ULX_GT2	0x191E
-#define PCI_CHIP_SKYLAKE_MOBILE_GT2	0x1921 /* Reserved */
-#define PCI_CHIP_SKYLAKE_ULT_GT3_0	0x1923
-#define PCI_CHIP_SKYLAKE_ULT_GT3_1	0x1926
-#define PCI_CHIP_SKYLAKE_ULT_GT3_2	0x1927
-#define PCI_CHIP_SKYLAKE_SRV_GT4	0x192A
-#define PCI_CHIP_SKYLAKE_HALO_GT3	0x192B /* Reserved */
-#define PCI_CHIP_SKYLAKE_SRV_GT3	0x192D
-#define PCI_CHIP_SKYLAKE_DT_GT4		0x1932
-#define PCI_CHIP_SKYLAKE_SRV_GT4X	0x193A
-#define PCI_CHIP_SKYLAKE_H_GT4		0x193B
-#define PCI_CHIP_SKYLAKE_WKS_GT4	0x193D
-
-#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_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
-#define PCI_CHIP_KABYLAKE_ULX_GT2_0	0x591E
 #define PCI_CHIP_KABYLAKE_DT_GT2	0x5912
-#define PCI_CHIP_KABYLAKE_M_GT2		0x5917
-#define PCI_CHIP_KABYLAKE_DT_GT1	0x5902
-#define PCI_CHIP_KABYLAKE_HALO_GT2	0x591B
-#define PCI_CHIP_KABYLAKE_HALO_GT4	0x593B
-#define PCI_CHIP_KABYLAKE_HALO_GT1_0	0x5908
-#define PCI_CHIP_KABYLAKE_HALO_GT1_1	0x590B
-#define PCI_CHIP_KABYLAKE_SRV_GT2	0x591A
-#define PCI_CHIP_KABYLAKE_SRV_GT1	0x590A
-#define PCI_CHIP_KABYLAKE_WKS_GT2	0x591D
-
-#define PCI_CHIP_AMBERLAKE_ULX_GT2_1	0x591C
-#define PCI_CHIP_AMBERLAKE_ULX_GT2_2	0x87C0
-
-#define PCI_CHIP_BROXTON_0		0x0A84
-#define PCI_CHIP_BROXTON_1		0x1A84
-#define PCI_CHIP_BROXTON_2		0x5A84
-#define PCI_CHIP_BROXTON_3		0x1A85
-#define PCI_CHIP_BROXTON_4		0x5A85
-
-#define PCI_CHIP_GLK			0x3184
-#define PCI_CHIP_GLK_2X6		0x3185
-
-#define PCI_CHIP_COFFEELAKE_S_GT1_1     0x3E90
-#define PCI_CHIP_COFFEELAKE_S_GT1_2     0x3E93
-#define PCI_CHIP_COFFEELAKE_S_GT1_3     0x3E99
-#define PCI_CHIP_COFFEELAKE_S_GT2_1     0x3E91
-#define PCI_CHIP_COFFEELAKE_S_GT2_2     0x3E92
-#define PCI_CHIP_COFFEELAKE_S_GT2_3     0x3E96
-#define PCI_CHIP_COFFEELAKE_S_GT2_4     0x3E98
-#define PCI_CHIP_COFFEELAKE_S_GT2_5     0x3E9A
-#define PCI_CHIP_COFFEELAKE_H_GT2_1     0x3E9B
-#define PCI_CHIP_COFFEELAKE_H_GT2_2     0x3E94
-#define PCI_CHIP_COFFEELAKE_U_GT2_1     0x3EA9
-#define PCI_CHIP_COFFEELAKE_U_GT3_1     0x3EA5
-#define PCI_CHIP_COFFEELAKE_U_GT3_2     0x3EA6
-#define PCI_CHIP_COFFEELAKE_U_GT3_3     0x3EA7
-#define PCI_CHIP_COFFEELAKE_U_GT3_4     0x3EA8
-
-#define PCI_CHIP_WHISKEYLAKE_U_GT1_1     0x3EA1
-#define PCI_CHIP_WHISKEYLAKE_U_GT2_1     0x3EA0
-#define PCI_CHIP_WHISKEYLAKE_U_GT3_1     0x3EA2
-#define PCI_CHIP_WHISKEYLAKE_U_GT3_2     0x3EA3
-#define PCI_CHIP_WHISKEYLAKE_U_GT3_3     0x3EA4
 
 #define IS_MOBILE(devid)	((devid) == PCI_CHIP_I855_GM || \
 				 (devid) == PCI_CHIP_I915_GM || \
@@ -405,119 +327,13 @@ 
 #define IS_GEN8(devid)		(IS_BROADWELL(devid) || \
 				 IS_CHERRYVIEW(devid))
 
-#define IS_SKL_GT1(devid)	((devid) == PCI_CHIP_SKYLAKE_DT_GT1	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_ULT_GT1	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_SRV_GT1	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_H_GT1	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_ULX_GT1)
-
-#define IS_SKL_GT2(devid)	((devid) == PCI_CHIP_SKYLAKE_DT_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_FUSED0_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_FUSED1_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_ULT_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_FUSED2_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_SRV_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_HALO_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_WKS_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_ULX_GT2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_MOBILE_GT2)
-
-#define IS_SKL_GT3(devid)	((devid) == PCI_CHIP_SKYLAKE_ULT_GT3_0	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_ULT_GT3_1	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_ULT_GT3_2	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_HALO_GT3	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_SRV_GT3)
-
-#define IS_SKL_GT4(devid)	((devid) == PCI_CHIP_SKYLAKE_SRV_GT4	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_DT_GT4	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_SRV_GT4X	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_H_GT4	|| \
-				 (devid) == PCI_CHIP_SKYLAKE_WKS_GT4)
-
-#define IS_KBL_GT1(devid)	((devid) == PCI_CHIP_KABYLAKE_ULT_GT1_5	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_ULX_GT1_5	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_ULT_GT1	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_ULX_GT1	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_DT_GT1	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_HALO_GT1_0 || \
-				 (devid) == PCI_CHIP_KABYLAKE_HALO_GT1_1 || \
-				 (devid) == PCI_CHIP_KABYLAKE_SRV_GT1)
-
-#define IS_KBL_GT2(devid)	((devid) == PCI_CHIP_KABYLAKE_ULT_GT2	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_ULT_GT2F	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_ULX_GT2_0	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_DT_GT2	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_M_GT2	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_HALO_GT2	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_SRV_GT2	|| \
-				 (devid) == PCI_CHIP_KABYLAKE_WKS_GT2 || \
-				 (devid) == PCI_CHIP_AMBERLAKE_ULX_GT2_1	|| \
-				 (devid) == PCI_CHIP_AMBERLAKE_ULX_GT2_2)
-
-#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)
-
-#define IS_KBL_GT4(devid)	((devid) == PCI_CHIP_KABYLAKE_HALO_GT4)
-
-#define IS_KABYLAKE(devid)	(IS_KBL_GT1(devid) || \
-				 IS_KBL_GT2(devid) || \
-				 IS_KBL_GT3(devid) || \
-				 IS_KBL_GT4(devid))
-
-#define IS_SKYLAKE(devid)	(IS_SKL_GT1(devid) || \
-				 IS_SKL_GT2(devid) || \
-				 IS_SKL_GT3(devid) || \
-				 IS_SKL_GT4(devid))
-
-#define IS_BROXTON(devid)	((devid) == PCI_CHIP_BROXTON_0	|| \
-				 (devid) == PCI_CHIP_BROXTON_1	|| \
-				 (devid) == PCI_CHIP_BROXTON_2	|| \
-				 (devid) == PCI_CHIP_BROXTON_3	|| \
-				 (devid) == PCI_CHIP_BROXTON_4)
-
-#define IS_GEMINILAKE(devid)	((devid) == PCI_CHIP_GLK || \
-				 (devid) == PCI_CHIP_GLK_2X6)
-
-#define IS_CFL_S(devid)         ((devid) == PCI_CHIP_COFFEELAKE_S_GT1_1 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_S_GT1_2 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_S_GT1_3 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_S_GT2_1 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_S_GT2_2 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_S_GT2_3 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_S_GT2_4 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_S_GT2_5)
-
-#define IS_CFL_H(devid)         ((devid) == PCI_CHIP_COFFEELAKE_H_GT2_1 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_H_GT2_2)
-
-#define IS_CFL_U(devid)         ((devid) == PCI_CHIP_COFFEELAKE_U_GT2_1 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_U_GT3_1 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_U_GT3_2 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_U_GT3_3 || \
-                                 (devid) == PCI_CHIP_COFFEELAKE_U_GT3_4 || \
-                                 (devid) == PCI_CHIP_WHISKEYLAKE_U_GT1_1 || \
-                                 (devid) == PCI_CHIP_WHISKEYLAKE_U_GT2_1 || \
-                                 (devid) == PCI_CHIP_WHISKEYLAKE_U_GT3_1 || \
-                                 (devid) == PCI_CHIP_WHISKEYLAKE_U_GT3_2 || \
-                                 (devid) == PCI_CHIP_WHISKEYLAKE_U_GT3_3)
-
-#define IS_COFFEELAKE(devid)   (IS_CFL_S(devid) || \
-				IS_CFL_H(devid) || \
-				IS_CFL_U(devid))
-
-#define IS_GEN9(devid)		(IS_SKYLAKE(devid)  || \
-				 IS_BROXTON(devid)  || \
-				 IS_KABYLAKE(devid) || \
-				 IS_GEMINILAKE(devid) || \
-				 IS_COFFEELAKE(devid))
-
 /* New platforms use kernel pci ids */
 #include <stdbool.h>
 
 bool intel_is_genx(unsigned int devid, int gen);
 bool intel_get_genx(unsigned int devid, int *gen);
 
+#define IS_GEN9(devid) intel_is_genx(devid, 9)
 #define IS_GEN10(devid) intel_is_genx(devid, 10)
 #define IS_GEN11(devid) intel_is_genx(devid, 11)
 
@@ -528,7 +344,6 @@  bool intel_get_genx(unsigned int devid, int *gen);
 				 IS_GEN6(dev) || \
 				 IS_GEN7(dev) || \
 				 IS_GEN8(dev) || \
-				 IS_GEN9(dev) || \
 				 intel_get_genx(dev, NULL))
 
 #endif /* _INTEL_CHIPSET_H */