diff mbox

[1/6] drm/i915: Store first production revid into device info

Message ID 20180608083906.6006-1-mika.kuoppala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kuoppala June 8, 2018, 8:39 a.m. UTC
Store first known production revid into the device info.

This enables us to easily see if we are running on
a preproduction hardware.

Uninitialized (zero) product revision id means that
there are no known preliminary hardware for this platform,
or that the platform is of gen that we don't care.
This is all pre gen9 platforms.

Unknown product revision maps to REVID_FOREVER on a
gen9+ platforms on default. When the platform
gets the first production revision and our testing
infra is cleaned from preproduction hardware, we can
set a first production revid. At that point we start
to complain about running driver on preliminary hardware.

v2: initialize GEN9_FEATURES too (CI)

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c          |  6 +++---
 drivers/gpu/drm/i915/i915_drv.h          |  7 +++++++
 drivers/gpu/drm/i915/i915_pci.c          | 10 ++++++++--
 drivers/gpu/drm/i915/intel_device_info.h | 11 +++++++++++
 4 files changed, 29 insertions(+), 5 deletions(-)

Comments

Chris Wilson June 8, 2018, 10:46 a.m. UTC | #1
Quoting Mika Kuoppala (2018-06-08 09:39:06)
> Store first known production revid into the device info.
> 
> This enables us to easily see if we are running on
> a preproduction hardware.
> 
> Uninitialized (zero) product revision id means that
> there are no known preliminary hardware for this platform,
> or that the platform is of gen that we don't care.
> This is all pre gen9 platforms.
> 
> Unknown product revision maps to REVID_FOREVER on a
> gen9+ platforms on default. When the platform
> gets the first production revision and our testing
> infra is cleaned from preproduction hardware, we can
> set a first production revid. At that point we start
> to complain about running driver on preliminary hardware.
> 
> v2: initialize GEN9_FEATURES too (CI)
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c          |  6 +++---
>  drivers/gpu/drm/i915/i915_drv.h          |  7 +++++++
>  drivers/gpu/drm/i915/i915_pci.c          | 10 ++++++++--
>  drivers/gpu/drm/i915/intel_device_info.h | 11 +++++++++++
>  4 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index be71fdf8d92e..92f244c12f1e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -855,10 +855,10 @@ static void intel_detect_preproduction_hw(struct drm_i915_private *dev_priv)
>         bool pre = false;
>  
>         pre |= IS_HSW_EARLY_SDV(dev_priv);
> -       pre |= IS_SKL_REVID(dev_priv, 0, SKL_REVID_F0);
> -       pre |= IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST);
> +       pre |= IS_PREPRODUCTION_HW(dev_priv);
>  
> -       if (pre) {
> +       if (pre && FIRST_PRODUCT_REVID(INTEL_INFO(dev_priv))
> +           != PRODUCT_REVID_UNKNOWN) {
>                 DRM_ERROR("This is a pre-production stepping. "
>                           "It may not be fully functional.\n");
>                 add_taint(TAINT_MACHINE_CHECK, LOCKDEP_STILL_OK);

I know this is changed later, but this is not very pleasing on the eye.

	const struct intel_info *info = INTEL_INFO(dev_priv);

	if (IS_PREPRODUCTION_HW(dev_priv) &&
	    FIRST_PRODUCT_REVID(info) != PRODUCT_REVID_UNKNOWN) {


> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 933e31669557..9ae9dc553192 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -146,6 +146,17 @@ struct intel_device_info {
>         u16 device_id;
>         u16 gen_mask;
>  
> +       u8 first_product_revid;
> +       /* Set to corresponding first production hardware revision or:

/*
 * Set to

That's the style we're meant to use. It helps when we mix in kerneldoc
comments, e.g.
/**
 * Foo:

> +        *
> +        * 0x00 == uninitialized == no known preliminary hw (legacy gens)
> +        * 0xff == PRODUCT_REVID_UNKNOWN == no known production hw yet
> +        *
> +        * Do not set first product revid unless you are certain
> +        * that testing infrastructure is already on top of production
> +        * revid machines.
> +        */

Description block before member or short description on the same line.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index be71fdf8d92e..92f244c12f1e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -855,10 +855,10 @@  static void intel_detect_preproduction_hw(struct drm_i915_private *dev_priv)
 	bool pre = false;
 
 	pre |= IS_HSW_EARLY_SDV(dev_priv);
-	pre |= IS_SKL_REVID(dev_priv, 0, SKL_REVID_F0);
-	pre |= IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST);
+	pre |= IS_PREPRODUCTION_HW(dev_priv);
 
-	if (pre) {
+	if (pre && FIRST_PRODUCT_REVID(INTEL_INFO(dev_priv))
+	    != PRODUCT_REVID_UNKNOWN) {
 		DRM_ERROR("This is a pre-production stepping. "
 			  "It may not be fully functional.\n");
 		add_taint(TAINT_MACHINE_CHECK, LOCKDEP_STILL_OK);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c4073666f1ca..abca814ad758 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2430,6 +2430,10 @@  intel_info(const struct drm_i915_private *dev_priv)
 
 #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
 
+#define PRODUCT_REVID_UNKNOWN	REVID_FOREVER
+#define FIRST_PRODUCT_REVID(intel_info) ((intel_info)->first_product_revid)
+#define IS_PREPRODUCTION_HW(dev_priv)   (INTEL_REVID(dev_priv) < FIRST_PRODUCT_REVID(INTEL_INFO(dev_priv)))
+
 #define SKL_REVID_A0		0x0
 #define SKL_REVID_B0		0x1
 #define SKL_REVID_C0		0x2
@@ -2461,6 +2465,7 @@  intel_info(const struct drm_i915_private *dev_priv)
 
 #define GLK_REVID_A0		0x0
 #define GLK_REVID_A1		0x1
+#define GLK_REVID_B0		0x3
 
 #define IS_GLK_REVID(dev_priv, since, until) \
 	(IS_GEMINILAKE(dev_priv) && IS_REVID(dev_priv, since, until))
@@ -2468,6 +2473,8 @@  intel_info(const struct drm_i915_private *dev_priv)
 #define CNL_REVID_A0		0x0
 #define CNL_REVID_B0		0x1
 #define CNL_REVID_C0		0x2
+#define CNL_REVID_D0		0x4
+#define CNL_REVID_G0		0x5
 
 #define IS_CNL_REVID(p, since, until) \
 	(IS_CANNONLAKE(p) && IS_REVID(p, since, until))
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 97a91e6af7e3..ae4a5a0b0c28 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -461,10 +461,12 @@  static const struct intel_device_info intel_cherryview_info = {
 	.has_csr = 1, \
 	.has_guc = 1, \
 	.has_ipc = 1, \
+	.first_product_revid = PRODUCT_REVID_UNKNOWN, \
 	.ddb_size = 896
 
 #define SKL_PLATFORM \
 	GEN9_FEATURES, \
+	.first_product_revid = SKL_REVID_G0, \
 	PLATFORM(INTEL_SKYLAKE)
 
 static const struct intel_device_info intel_skylake_gt1_info = {
@@ -518,6 +520,7 @@  static const struct intel_device_info intel_skylake_gt4_info = {
 	.has_reset_engine = 1, \
 	.has_snoop = true, \
 	.has_ipc = 1, \
+	.first_product_revid = PRODUCT_REVID_UNKNOWN, \
 	GEN9_DEFAULT_PAGE_SIZES, \
 	GEN_DEFAULT_PIPEOFFSETS, \
 	IVB_CURSOR_OFFSETS, \
@@ -526,6 +529,7 @@  static const struct intel_device_info intel_skylake_gt4_info = {
 static const struct intel_device_info intel_broxton_info = {
 	GEN9_LP_FEATURES,
 	PLATFORM(INTEL_BROXTON),
+	.first_product_revid = BXT_REVID_C0,
 	.ddb_size = 512,
 };
 
@@ -538,7 +542,8 @@  static const struct intel_device_info intel_geminilake_info = {
 
 #define KBL_PLATFORM \
 	GEN9_FEATURES, \
-	PLATFORM(INTEL_KABYLAKE)
+	PLATFORM(INTEL_KABYLAKE), \
+	.first_product_revid = KBL_REVID_B0
 
 static const struct intel_device_info intel_kabylake_gt1_info = {
 	KBL_PLATFORM,
@@ -558,7 +563,8 @@  static const struct intel_device_info intel_kabylake_gt3_info = {
 
 #define CFL_PLATFORM \
 	GEN9_FEATURES, \
-	PLATFORM(INTEL_COFFEELAKE)
+	PLATFORM(INTEL_COFFEELAKE), \
+	.first_product_revid = KBL_REVID_B0
 
 static const struct intel_device_info intel_coffeelake_gt1_info = {
 	CFL_PLATFORM,
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 933e31669557..9ae9dc553192 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -146,6 +146,17 @@  struct intel_device_info {
 	u16 device_id;
 	u16 gen_mask;
 
+	u8 first_product_revid;
+	/* Set to corresponding first production hardware revision or:
+	 *
+	 * 0x00 == uninitialized == no known preliminary hw (legacy gens)
+	 * 0xff == PRODUCT_REVID_UNKNOWN == no known production hw yet
+	 *
+	 * Do not set first product revid unless you are certain
+	 * that testing infrastructure is already on top of production
+	 * revid machines.
+	 */
+
 	u8 gen;
 	u8 gt; /* GT number, 0 if undefined */
 	u8 num_rings;