On Thu, 5 May 2011 15:16:45 -0700, Jesse Barnes <jbar...@virtuousgeek.org>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
