drm/i915: make Pineview a platform of its own

Submitted by Jani Nikula on Dec. 7, 2016, 2:06 p.m.

Details

Message ID 1481119608-7146-1-git-send-email-jani.nikula@intel.com
State New
Headers show
Series "drm/i915: make Pineview a platform of its own" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Jani Nikula Dec. 7, 2016, 2:06 p.m.
Pineview deserves to use its own platform enum (which was already added,
unused, previously).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

The wrinkle here is that IS_G33() still has to match both G33 and
Pineview. I'd prefer it if G33 *only* matched G33 and *not* Pineview. To
fix this, the alternatives are to a) add an additional IS_FOO() that
matches both, or b) replaces all references to IS_G33() with IS_G33() ||
IS_PINEVIEW(). I don't like either, but I also don't like
.is_pineview. We should reserve those for feature-ish things. Or just
apply this and leave it at that. Opinions?
---
 drivers/gpu/drm/i915/i915_drv.h | 6 +++---
 drivers/gpu/drm/i915/i915_pci.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8daa4fb13b52..4236cdd25b32 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -688,7 +688,6 @@  struct intel_csr {
 
 #define DEV_INFO_FOR_EACH_FLAG(func) \
 	func(is_mobile); \
-	func(is_pineview); \
 	func(is_lp); \
 	func(is_alpha_support); \
 	/* Keep has_* in alphabetical order */ \
@@ -2530,8 +2529,9 @@  intel_info(const struct drm_i915_private *dev_priv)
 #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
 #define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
 #define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
-#define IS_PINEVIEW(dev_priv)	((dev_priv)->info.is_pineview)
-#define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
+#define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
+#define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33 || \
+				 IS_PINEVIEW(dev_priv))
 #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
 #define IS_IVYBRIDGE(dev_priv)	((dev_priv)->info.platform == INTEL_IVYBRIDGE)
 #define IS_IVB_GT1(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0156 || \
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f7ec6e944e09..93f50ef2a309 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -141,7 +141,7 @@  static const struct intel_device_info intel_g33_info = {
 
 static const struct intel_device_info intel_pineview_info = {
 	GEN3_FEATURES,
-	.platform = INTEL_G33, .is_pineview = 1, .is_mobile = 1,
+	.platform = INTEL_PINEVIEW, .is_mobile = 1,
 	.has_hotplug = 1,
 	.has_overlay = 1,
 };

Comments

On Wed, Dec 07, 2016 at 04:06:48PM +0200, Jani Nikula wrote:
> Pineview deserves to use its own platform enum (which was already added,
> unused, previously).
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> The wrinkle here is that IS_G33() still has to match both G33 and
> Pineview. I'd prefer it if G33 *only* matched G33 and *not* Pineview. To
> fix this, the alternatives are to a) add an additional IS_FOO() that
> matches both, or b) replaces all references to IS_G33() with IS_G33() ||
> IS_PINEVIEW().

This would be in line with what we did for VLV vs. CHV. So I think it
would be OK.

> I don't like either, but I also don't like
> .is_pineview. We should reserve those for feature-ish things. Or just
> apply this and leave it at that. Opinions?
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 6 +++---
>  drivers/gpu/drm/i915/i915_pci.c | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8daa4fb13b52..4236cdd25b32 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -688,7 +688,6 @@ struct intel_csr {
>  
>  #define DEV_INFO_FOR_EACH_FLAG(func) \
>  	func(is_mobile); \
> -	func(is_pineview); \
>  	func(is_lp); \
>  	func(is_alpha_support); \
>  	/* Keep has_* in alphabetical order */ \
> @@ -2530,8 +2529,9 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
>  #define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
>  #define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
> -#define IS_PINEVIEW(dev_priv)	((dev_priv)->info.is_pineview)
> -#define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
> +#define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
> +#define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33 || \
> +				 IS_PINEVIEW(dev_priv))
>  #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
>  #define IS_IVYBRIDGE(dev_priv)	((dev_priv)->info.platform == INTEL_IVYBRIDGE)
>  #define IS_IVB_GT1(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0156 || \
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index f7ec6e944e09..93f50ef2a309 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -141,7 +141,7 @@ static const struct intel_device_info intel_g33_info = {
>  
>  static const struct intel_device_info intel_pineview_info = {
>  	GEN3_FEATURES,
> -	.platform = INTEL_G33, .is_pineview = 1, .is_mobile = 1,
> +	.platform = INTEL_PINEVIEW, .is_mobile = 1,
>  	.has_hotplug = 1,
>  	.has_overlay = 1,
>  };
> -- 
> 2.1.4
> == Series Details ==

> 

> Series: drm/i915: make Pineview a platform of its own

> URL   : https://patchwork.freedesktop.org/series/16490/

> State : warning

> 

> == Summary ==

> 

> Series 16490v1 drm/i915: make Pineview a platform of its own

> https://patchwork.freedesktop.org/api/1.0/series/16490/revisions/1/mbox/

> 

> Test kms_force_connector_basic:

>         Subgroup force-connector-state:

>                 pass       -> DMESG-WARN (fi-snb-2520m)

*ERROR* EDID checksum is invalid
=> https://bugs.freedesktop.org/show_bug.cgi?id=98625

> fi-snb-2520m     total:247  pass:201  dwarn:1   dfail:0   fail:0   skip:45

> 

> a20a4a6107256124d136c8320fc3f7adef93bb8a drm-tip: 2016y-12m-07d-15h-

> 56m-01s UTC integration manifest 8b8c20d drm/i915: make Pineview a

> platform of its own

> 

> == Logs ==

> 

> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3218/



Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo