| Message ID | 1305235044-9159-15-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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
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
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.
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 --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)
... 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(-)