On Sat, Nov 19, 2011 at 04:41, Keith Packard <keithp at keithp.com> wrote: > > +static bool intel_enable_rc6(struct drm_device *dev) > +{ > + ? ? ? if (i915_enable_rc6 >= 0) > + ? ? ? ? ? ? ? return i915_enable_rc6; > + ? ? ? if (INTEL_INFO(dev)->gen >= 7) > + ? ? ? ? ? ? ? return 1; > +#ifdef CONFIG_INTEL_IOMMU > + ? ? ? /* > + ? ? ? ?* Only enable RC6 if all dma remapping is disabled, or if > + ? ? ? ?* there's no iommu present in the machine. > + ? ? ? ?*/ > + ? ? ? if (INTEL_INFO(dev)->gen == 6) > + ? ? ? ? ? ? ? return no_iommu || dmar_disabled; > +#endif > + ? ? ? return 0; > +} Just one question I caught on 2nd read. Shouldn't we have #else within this #ifdef block, to return 1? Otherwise, if CONFIG_INTEL_IOMMU is not defined, we'll always disable rc6. Or we just always intend to disable rc6 in case CONFIG_INTEL_IOMMU is not there? -- Eugeni Dodonov