| 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 |
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, };
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
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(-)