diff mbox

[04/16] drm/i915: add Ivy Bridge PCI IDs and flags

Message ID 1303861134-8762-5-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes April 26, 2011, 11:38 p.m. UTC
Check for IVB desktop, mobile and other SKUs and set flags
appropriately.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c |   19 +++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |    2 ++
 2 files changed, 21 insertions(+), 0 deletions(-)

Comments

Chris Wilson April 27, 2011, 6:59 a.m. UTC | #1
On Tue, 26 Apr 2011 16:38:42 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Check for IVB desktop, mobile and other SKUs and set flags
> appropriately.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2a41118..0b5e263 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -230,6 +230,7 @@ struct intel_device_info {
>  	u8 is_pineview : 1;
>  	u8 is_broadwater : 1;
>  	u8 is_crestline : 1;
> +	u8 is_ivybridge : 1;

Since ivybridge is synonymous with GEN7, we have been going with the
latter. I want to reserve the capability bits for instances where we
either have a workaround for a few chipsets (and so I have a patch to
strip out the is_broadwater and is_crestline since they are just a single
pci-id each) or otherwise describing a feature across chipsets. 

So I think we just want IS_GEN7() for IVB code.
-Chris
Chris Wilson April 27, 2011, 7:23 a.m. UTC | #2
On Tue, 26 Apr 2011 16:38:42 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Check for IVB desktop, mobile and other SKUs and set flags
> appropriately.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  static const struct pci_device_id pciidlist[] = {		/* aka */
>  	INTEL_VGA_DEVICE(0x3577, &intel_i830_info),		/* I830_M */
>  	INTEL_VGA_DEVICE(0x2562, &intel_845g_info),		/* 845_G */
> @@ -227,6 +241,11 @@ static const struct pci_device_id pciidlist[] = {		/* aka */
>  	INTEL_VGA_DEVICE(0x0116, &intel_sandybridge_m_info),
>  	INTEL_VGA_DEVICE(0x0126, &intel_sandybridge_m_info),
>  	INTEL_VGA_DEVICE(0x010A, &intel_sandybridge_d_info),
> +	INTEL_VGA_DEVICE(0x0156, &intel_ivybridge_m_info), /* GT1 mobile */
> +	INTEL_VGA_DEVICE(0x0166, &intel_ivybridge_m_info), /* GT2 mobile */
> +	INTEL_VGA_DEVICE(0x0152, &intel_ivybridge_d_info), /* GT1 desktop */
> +	INTEL_VGA_DEVICE(0x0162, &intel_ivybridge_d_info), /* GT2 desktop */
> +	INTEL_VGA_DEVICE(0x015a, &intel_ivybridge_d_info), /* GT1 server */

Shouldn't this chunk be the last patch in the series? (First for testing,
last for deployment.)
-Chris
Jesse Barnes April 27, 2011, 9:01 p.m. UTC | #3
On Wed, 27 Apr 2011 07:59:17 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, 26 Apr 2011 16:38:42 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > Check for IVB desktop, mobile and other SKUs and set flags
> > appropriately.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 2a41118..0b5e263 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -230,6 +230,7 @@ struct intel_device_info {
> >  	u8 is_pineview : 1;
> >  	u8 is_broadwater : 1;
> >  	u8 is_crestline : 1;
> > +	u8 is_ivybridge : 1;
> 
> Since ivybridge is synonymous with GEN7, we have been going with the
> latter. I want to reserve the capability bits for instances where we
> either have a workaround for a few chipsets (and so I have a patch to
> strip out the is_broadwater and is_crestline since they are just a single
> pci-id each) or otherwise describing a feature across chipsets. 
> 
> So I think we just want IS_GEN7() for IVB code.

I'd rather keep them separate since we know we'll have gen7 chips with
different display engines in the future.  I've been trying to use
IS_GEN for render related checks, IS_IVB for specific display checks,
and HAS_PCH for the ilk+ split (though for the most part that should go
away with proper function call backs).

But I'll take a look at them again and see what's most appropriate.

Jesse
Jesse Barnes April 27, 2011, 9:02 p.m. UTC | #4
On Wed, 27 Apr 2011 08:23:33 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue, 26 Apr 2011 16:38:42 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > Check for IVB desktop, mobile and other SKUs and set flags
> > appropriately.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  static const struct pci_device_id pciidlist[] = {		/* aka */
> >  	INTEL_VGA_DEVICE(0x3577, &intel_i830_info),		/* I830_M */
> >  	INTEL_VGA_DEVICE(0x2562, &intel_845g_info),		/* 845_G */
> > @@ -227,6 +241,11 @@ static const struct pci_device_id pciidlist[] = {		/* aka */
> >  	INTEL_VGA_DEVICE(0x0116, &intel_sandybridge_m_info),
> >  	INTEL_VGA_DEVICE(0x0126, &intel_sandybridge_m_info),
> >  	INTEL_VGA_DEVICE(0x010A, &intel_sandybridge_d_info),
> > +	INTEL_VGA_DEVICE(0x0156, &intel_ivybridge_m_info), /* GT1 mobile */
> > +	INTEL_VGA_DEVICE(0x0166, &intel_ivybridge_m_info), /* GT2 mobile */
> > +	INTEL_VGA_DEVICE(0x0152, &intel_ivybridge_d_info), /* GT1 desktop */
> > +	INTEL_VGA_DEVICE(0x0162, &intel_ivybridge_d_info), /* GT2 desktop */
> > +	INTEL_VGA_DEVICE(0x015a, &intel_ivybridge_d_info), /* GT1 server */
> 
> Shouldn't this chunk be the last patch in the series? (First for testing,
> last for deployment.)

Sure, this one's easy to move around.  Likewise with the AGP patch.

Jesse
Chris Wilson April 27, 2011, 9:28 p.m. UTC | #5
On Wed, 27 Apr 2011 14:01:50 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Wed, 27 Apr 2011 07:59:17 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > So I think we just want IS_GEN7() for IVB code.
> 
> I'd rather keep them separate since we know we'll have gen7 chips with
> different display engines in the future.  I've been trying to use
> IS_GEN for render related checks, IS_IVB for specific display checks,
> and HAS_PCH for the ilk+ split (though for the most part that should go
> away with proper function call backs).

If there is method to the madness, fine. I like the explanation and so
perhaps we should include that in the code somewhere. And we just need to
make sure we are consistent in our usage.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5d0d28c..016290e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -188,6 +188,20 @@  static const struct intel_device_info intel_sandybridge_m_info = {
 	.has_blt_ring = 1,
 };
 
+static const struct intel_device_info intel_ivybridge_d_info = {
+	.is_ivybridge = 1, .gen = 7,
+	.need_gfx_hws = 1, .has_hotplug = 1,
+	.has_bsd_ring = 1,
+	.has_blt_ring = 1,
+};
+
+static const struct intel_device_info intel_ivybridge_m_info = {
+	.is_ivybridge = 1, .gen = 7, .is_mobile = 1,
+	.need_gfx_hws = 1, .has_hotplug = 1,
+	.has_bsd_ring = 1,
+	.has_blt_ring = 1,
+};
+
 static const struct pci_device_id pciidlist[] = {		/* aka */
 	INTEL_VGA_DEVICE(0x3577, &intel_i830_info),		/* I830_M */
 	INTEL_VGA_DEVICE(0x2562, &intel_845g_info),		/* 845_G */
@@ -227,6 +241,11 @@  static const struct pci_device_id pciidlist[] = {		/* aka */
 	INTEL_VGA_DEVICE(0x0116, &intel_sandybridge_m_info),
 	INTEL_VGA_DEVICE(0x0126, &intel_sandybridge_m_info),
 	INTEL_VGA_DEVICE(0x010A, &intel_sandybridge_d_info),
+	INTEL_VGA_DEVICE(0x0156, &intel_ivybridge_m_info), /* GT1 mobile */
+	INTEL_VGA_DEVICE(0x0166, &intel_ivybridge_m_info), /* GT2 mobile */
+	INTEL_VGA_DEVICE(0x0152, &intel_ivybridge_d_info), /* GT1 desktop */
+	INTEL_VGA_DEVICE(0x0162, &intel_ivybridge_d_info), /* GT2 desktop */
+	INTEL_VGA_DEVICE(0x015a, &intel_ivybridge_d_info), /* GT1 server */
 	{0, 0, 0}
 };
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2a41118..0b5e263 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -230,6 +230,7 @@  struct intel_device_info {
 	u8 is_pineview : 1;
 	u8 is_broadwater : 1;
 	u8 is_crestline : 1;
+	u8 is_ivybridge : 1;
 	u8 has_fbc : 1;
 	u8 has_pipe_cxsr : 1;
 	u8 has_hotplug : 1;
@@ -929,6 +930,7 @@  enum intel_chip_family {
 #define IS_G33(dev)		(INTEL_INFO(dev)->is_g33)
 #define IS_IRONLAKE_D(dev)	((dev)->pci_device == 0x0042)
 #define IS_IRONLAKE_M(dev)	((dev)->pci_device == 0x0046)
+#define IS_IVYBRIDGE(dev)	(INTEL_INFO(dev)->is_ivybridge)
 #define IS_MOBILE(dev)		(INTEL_INFO(dev)->is_mobile)
 
 #define IS_GEN2(dev)	(INTEL_INFO(dev)->gen == 2)