From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: clarify IS_GEN vs IS_ usage Date: Thu, 05 May 2011 23:57:56 +0100 Message-ID: <0d30dc$m4ic2q@orsmga001.jf.intel.com> References: <1304633805-7505-1-git-send-email-jbarnes@virtuousgeek.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 00A899E730 for ; Thu, 5 May 2011 15:58:04 -0700 (PDT) In-Reply-To: <1304633805-7505-1-git-send-email-jbarnes@virtuousgeek.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jesse Barnes , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 5 May 2011 15:16:45 -0700, Jesse Barnes wrote: > We generally use the gen number to indicate the generation of the render > portion of the chip. In some cases this isn't the same as the display > generation (as in the case of G33 and GMA500). So codify the de facto > usage by converting some IS_GEN checks into product specific checks for > display related differences. (Note this makes me wonder about our G33 > watermark handling; shouldn't it be like 965 not 945? I don't have one > to test with...). As far as I've been able to tell, the current code works... So it can't be too far wrong, and I don't recall any mention of deviations in the gen3 docs. Whilst you are in the vicinity, does it make sense to rename info->gen to info->render (or info->render_gen)? > @@ -230,6 +231,7 @@ struct intel_device_info { > u8 is_broadwater : 1; > u8 is_crestline : 1; > u8 is_ivybridge : 1; > + u8 is_sandybridge : 1; > u8 has_fbc : 1; > u8 has_pipe_cxsr : 1; > u8 has_hotplug : 1; What's the sort order here? ;-) > @@ -915,10 +917,13 @@ enum intel_chip_family { > #define IS_845G(dev) ((dev)->pci_device == 0x2562) > #define IS_I85X(dev) (INTEL_INFO(dev)->is_i85x) > #define IS_I865G(dev) ((dev)->pci_device == 0x2572) > +#define IS_I8XX(dev) (INTEL_INFO(dev)->is_i8xx) > #define IS_I915G(dev) (INTEL_INFO(dev)->is_i915g) > #define IS_I915GM(dev) ((dev)->pci_device == 0x2592) > +#define IS_I915(dev) (IS_I915G(dev) || IS_I915GM(dev)) > #define IS_I945G(dev) ((dev)->pci_device == 0x2772) > #define IS_I945GM(dev) (INTEL_INFO(dev)->is_i945gm) > +#define IS_I945(dev) (IS_I945G(dev) || IS_I945GM(dev)) > #define IS_BROADWATER(dev) (INTEL_INFO(dev)->is_broadwater) > #define IS_CRESTLINE(dev) (INTEL_INFO(dev)->is_crestline) > #define IS_GM45(dev) ((dev)->pci_device == 0x2A42) > @@ -927,8 +932,10 @@ enum intel_chip_family { > #define IS_PINEVIEW_M(dev) ((dev)->pci_device == 0xa011) > #define IS_PINEVIEW(dev) (INTEL_INFO(dev)->is_pineview) > #define IS_G33(dev) (INTEL_INFO(dev)->is_g33) > +#define IS_I915_DISPLAY(dev) (IS_I915(dev) || IS_I945(dev) || IS_G33(dev)) Not more nested predicates that mix capability bits and devids - just add a new bit for the display engine! IS_I915_DISPLAY() is a good name, and makes me want IS_I830_DISPLAY rather than IS_I8XX. And I think you've just set a new precedent to distinguish the display engines from the render engine from the chipsets. Don't let me stop you from completing the job ;) -Chris -- Chris Wilson, Intel Open Source Technology Centre