diff mbox

[14/16] drm/i915: Use PCI-ID to identify Broadwater and Crestline

Message ID 1305235044-9159-15-git-send-email-chris@chris-wilson.co.uk
State New, archived
Headers show

Commit Message

Chris Wilson May 12, 2011, 9:17 p.m. UTC
... as they only had a single PCI-ID each, and so using the pci-id is
easier than using a capability bit.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c |    2 --
 drivers/gpu/drm/i915/i915_drv.c     |    9 +++------
 drivers/gpu/drm/i915/i915_drv.h     |    6 ++----
 3 files changed, 5 insertions(+), 12 deletions(-)

Comments

Keith Packard May 13, 2011, 1:16 a.m. UTC | #1
On Thu, 12 May 2011 22:17:22 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:

> ... as they only had a single PCI-ID each, and so using the pci-id is
> easier than using a capability bit.

This doesn't seem useful to me; it only saves a couple of bits in the
struct and replaces that with compares. Meh.

Nacked-by: Keith Packard <keithp@keithp.com>
Chris Wilson May 13, 2011, 9:39 a.m. UTC | #2
On Thu, 12 May 2011 18:16:00 -0700, Keith Packard <keithp@keithp.com> wrote:
> On Thu, 12 May 2011 22:17:22 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > ... as they only had a single PCI-ID each, and so using the pci-id is
> > easier than using a capability bit.
> 
> This doesn't seem useful to me; it only saves a couple of bits in the
> struct and replaces that with compares. Meh.

The switch is from a comparatively expensive compare to a cheaper compare
*consistent* with the other individual chipset identifiers.
-Chris
Daniel Vetter May 15, 2011, 8:49 p.m. UTC | #3
On Thu, May 12, 2011 at 06:16:00PM -0700, Keith Packard wrote:
> On Thu, 12 May 2011 22:17:22 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > ... as they only had a single PCI-ID each, and so using the pci-id is
> > easier than using a capability bit.
> 
> This doesn't seem useful to me; it only saves a couple of bits in the
> struct and replaces that with compares. Meh.
> 
> Nacked-by: Keith Packard <keithp@keithp.com>

I actually like this: I saves one needless indirection when reading
codepaths and trying to find out what code is run for a given pci id.
Also, these two bits seem to be the only ones that are used in only _one_
device type, which is a bit confusing.
-Daniel
Keith Packard May 15, 2011, 10:01 p.m. UTC | #4
On Sun, 15 May 2011 22:49:02 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:

> I actually like this: I saves one needless indirection when reading
> codepaths and trying to find out what code is run for a given pci id.
> Also, these two bits seem to be the only ones that are used in only _one_
> device type, which is a bit confusing.

Yeah, we've got a bunch of single-bits per display generation; should we
create one of those for broadwater+crestline? That combination is used a
couple of times in the driver.

Thanks for your comments; I'll take this patch and we can consider
whether to add a bit for crestline+broadwater later.
Keith Packard June 3, 2011, 8:59 p.m. UTC | #5
On Thu, 12 May 2011 22:17:22 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:

> ... as they only had a single PCI-ID each, and so using the pci-id is
> easier than using a capability bit.

This patch no longer applies; do you want to update it?
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 48f7e257..6612a61 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -70,8 +70,6 @@  static int i915_capabilities(struct seq_file *m, void *data)
 	B(need_gfx_hws);
 	B(is_g4x);
 	B(is_pineview);
-	B(is_broadwater);
-	B(is_crestline);
 	B(has_fbc);
 	B(has_pipe_cxsr);
 	B(has_hotplug);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 50c6065..9f42a57 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -122,15 +122,12 @@  static const struct intel_device_info intel_i945gm_info = {
 };
 
 static const struct intel_device_info intel_i965g_info = {
-	.gen = 4, .is_broadwater = 1,
-	.has_hotplug = 1,
-	.has_overlay = 1,
+	.gen = 4, .has_hotplug = 1, .has_overlay = 1,
 };
 
 static const struct intel_device_info intel_i965gm_info = {
-	.gen = 4, .is_crestline = 1,
-	.is_mobile = 1, .has_fbc = 1, .has_hotplug = 1,
-	.has_overlay = 1,
+	.gen = 4, .has_hotplug = 1, .has_overlay = 1,
+	.is_mobile = 1, .has_fbc = 1,
 	.supports_tv = 1,
 };
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ca11444..ae1ab1b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -227,8 +227,6 @@  struct intel_device_info {
 	u8 need_gfx_hws : 1;
 	u8 is_g4x : 1;
 	u8 is_pineview : 1;
-	u8 is_broadwater : 1;
-	u8 is_crestline : 1;
 	u8 has_fbc : 1;
 	u8 has_pipe_cxsr : 1;
 	u8 has_hotplug : 1;
@@ -917,8 +915,8 @@  struct drm_i915_file_private {
 #define IS_I915GM(dev)		((dev)->pci_device == 0x2592)
 #define IS_I945G(dev)		((dev)->pci_device == 0x2772)
 #define IS_I945GM(dev)		(INTEL_INFO(dev)->is_i945gm)
-#define IS_BROADWATER(dev)	(INTEL_INFO(dev)->is_broadwater)
-#define IS_CRESTLINE(dev)	(INTEL_INFO(dev)->is_crestline)
+#define IS_BROADWATER(dev)	((dev)->pci_device == 0x2972)
+#define IS_CRESTLINE(dev)	((dev)->pci_device == 0x2a02)
 #define IS_GM45(dev)		((dev)->pci_device == 0x2A42)
 #define IS_G4X(dev)		(INTEL_INFO(dev)->is_g4x)
 #define IS_PINEVIEW_G(dev)	((dev)->pci_device == 0xa001)