Move force_discrete GPU on older MacBookPro models to GpuBlacklist

Before we hardwired the forcing. Also, we refactor the SupportsDualGpus() code so it's only checked on browser process, mainly the part we check the availability of online/offline renderers. We suspect calling the renderer detection code at the same time in browser/gpu processes might cause GPU driver crashes on certain Mac systems (mostly 10.8 from crash reports). This refactoring hopefully will fix the issue. BUG=151741,131276 TEST=tree, manual on dual GPU systems (10.7 and 10.8) Review URL: https://codereview.chromium.org/10995002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@159579 0039d316-1c4b-4281-b951-d872f2087c98
author: zmo@google.com <zmo@google.com@0039d316-1c4b-4281-b951-d872f2087c98> 2012-10-01 23:06:04 +0000
committer: zmo@google.com <zmo@google.com@0039d316-1c4b-4281-b951-d872f2087c98> 2012-10-01 23:06:04 +0000
commit: 48fe7e4cbaf045f280a90c6e5f93867d49082ce6 (patch)
tree: 33c1511de1ee714d1eff15f18f9726ca939fcd45 /ui/gl
parent: 19be7a6da266c546a7a52a7170df789f1ab8b710 (diff)
download: chromium_src-48fe7e4cbaf045f280a90c6e5f93867d49082ce6.zip
chromium_src-48fe7e4cbaf045f280a90c6e5f93867d49082ce6.tar.gz
chromium_src-48fe7e4cbaf045f280a90c6e5f93867d49082ce6.tar.bz2
13 files changed, 173 insertions, 163 deletions
diff --git a/ui/gl/gl_context.h b/ui/gl/gl_context.h
index 7c0b669..27a1a78 100644
--- a/ui/gl/gl_context.h
+++ b/ui/gl/gl_context.h
@@ -71,8 +71,6 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
 
   static bool LosesAllContextsOnContextLost();
 
-  static bool SupportsDualGpus();
-
   static GLContext* GetCurrent();
 
   virtual bool WasAllocatedUsingRobustnessExtension();
diff --git a/ui/gl/gl_context_android.cc b/ui/gl/gl_context_android.cc
index 38d91e8..66ab2c6 100644
--- a/ui/gl/gl_context_android.cc
+++ b/ui/gl/gl_context_android.cc
@@ -27,8 +27,4 @@ scoped_refptr<GLContext> GLContext::CreateGLContext(
   return context;
 }
 
-bool GLContext::SupportsDualGpus() {
-  return false;
-}
-
 }
diff --git a/ui/gl/gl_context_cgl.cc b/ui/gl/gl_context_cgl.cc
index a2cf560..614ed20 100644
--- a/ui/gl/gl_context_cgl.cc
+++ b/ui/gl/gl_context_cgl.cc
@@ -37,7 +37,7 @@ bool GLContextCGL::Initialize(GLSurface* compatible_surface,
   std::vector<CGLPixelFormatAttribute> attribs;
   // If the system supports dual gpus then allow offline renderers for every
   // context, so that they can all be in the same share group.
-  if (SupportsDualGpus())
+  if (GpuSwitchingManager::GetInstance()->SupportsDualGpus())
     attribs.push_back(kCGLPFAAllowOfflineRenderers);
   if (GetGLImplementation() == kGLImplementationAppleGL) {
     attribs.push_back(kCGLPFARendererID);
@@ -61,7 +61,8 @@ bool GLContextCGL::Initialize(GLSurface* compatible_surface,
 
   // If using the discrete gpu, create a pixel format requiring it before we
   // create the context.
-  if (!SupportsDualGpus() || gpu_preference == PreferDiscreteGpu) {
+  if (!GpuSwitchingManager::GetInstance()->SupportsDualGpus() ||
+      gpu_preference == PreferDiscreteGpu) {
     std::vector<CGLPixelFormatAttribute> discrete_attribs;
     discrete_attribs.push_back((CGLPixelFormatAttribute) 0);
     GLint num_pixel_formats;
@@ -73,7 +74,6 @@ bool GLContextCGL::Initialize(GLSurface* compatible_surface,
     }
   }
 
-
   CGLError res = CGLCreateContext(
       format,
       share_context ?
@@ -224,15 +224,4 @@ void ScopedCGLDestroyRendererInfo::operator()(CGLRendererInfoObj x) const {
   CGLDestroyRendererInfo(x);
 }
 
-void GLContextCGL::ForceUseOfDiscreteGPU() {
-  static CGLPixelFormatObj format = NULL;
-  if (format)
-    return;
-  CGLPixelFormatAttribute attribs[1];
-  attribs[0] = static_cast<CGLPixelFormatAttribute>(0);
-  GLint num_pixel_formats = 0;
-  CGLChoosePixelFormat(attribs, &format, &num_pixel_formats);
-  // format is deliberately leaked.
-}
-
 }  // namespace gfx
diff --git a/ui/gl/gl_context_cgl.h b/ui/gl/gl_context_cgl.h
index 1c4431b..3e4e3c1 100644
--- a/ui/gl/gl_context_cgl.h
+++ b/ui/gl/gl_context_cgl.h
@@ -30,10 +30,6 @@ class GLContextCGL : public GLContext {
   virtual void SetSwapInterval(int interval) OVERRIDE;
   virtual bool GetTotalGpuMemory(size_t* bytes) OVERRIDE;
 
-  // Helper for dual-GPU support on systems where this is necessary
-  // for stability reasons.
-  static void ForceUseOfDiscreteGPU();
-
  protected:
   virtual ~GLContextCGL();
 
diff --git a/ui/gl/gl_context_linux.cc b/ui/gl/gl_context_linux.cc
index 667df48..33b5f52 100644
--- a/ui/gl/gl_context_linux.cc
+++ b/ui/gl/gl_context_linux.cc
@@ -58,8 +58,4 @@ scoped_refptr<GLContext> GLContext::CreateGLContext(
   }
 }
 
-bool GLContext::SupportsDualGpus() {
-  return false;
-}
-
 }  // namespace gfx
diff --git a/ui/gl/gl_context_mac.mm b/ui/gl/gl_context_mac.mm
index eb7ee4c..dbb0730 100644
--- a/ui/gl/gl_context_mac.mm
+++ b/ui/gl/gl_context_mac.mm
@@ -3,20 +3,16 @@
 // found in the LICENSE file.
 
 #include "base/basictypes.h"
-#include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
-#include "base/mac/mac_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "third_party/mesa/MesaLib/include/GL/osmesa.h"
-#include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_context_cgl.h"
 #include "ui/gl/gl_context_osmesa.h"
 #include "ui/gl/gl_context_stub.h"
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_surface.h"
 #include "ui/gl/gl_switches.h"
-#include "ui/gl/gpu_switching_manager.h"
 
 #if defined(USE_AURA)
 #include "ui/gl/gl_context_nsview.h"
@@ -63,108 +59,4 @@ scoped_refptr<GLContext> GLContext::CreateGLContext(
   }
 }
 
-bool GLContext::SupportsDualGpus() {
-  // We need to know the GL implementation in order to correctly
-  // answer whether dual GPUs are supported. This introduces an
-  // initialization cycle with GLSurface::InitializeOneOff() which we
-  // need to break.
-  static bool initialized = false;
-  static bool initializing = false;
-  static bool supports_dual_gpus = false;
-
-  if (initialized) {
-    return supports_dual_gpus;
-  } else {
-    if (!initializing) {
-      initializing = true;
-      if (!GLSurface::InitializeOneOff()) {
-        return false;
-      }
-    }
-    initialized = true;
-  }
-
-  if (!base::mac::IsOSLionOrLater()) {
-    return false;
-  }
-
-  if (GetGLImplementation() != kGLImplementationDesktopGL) {
-    return false;
-  }
-
-  // Enumerate all hardware-accelerated renderers. If we find one
-  // online and one offline, assume we're on a dual-GPU system.
-  GLuint display_mask = static_cast<GLuint>(-1);
-  CGLRendererInfoObj renderer_info = NULL;
-  GLint num_renderers = 0;
-
-  bool found_online = false;
-  bool found_offline = false;
-
-  if (CGLQueryRendererInfo(display_mask,
-                           &renderer_info,
-                           &num_renderers) != kCGLNoError) {
-    return false;
-  }
-
-  ScopedCGLRendererInfoObj scoper(renderer_info);
-
-  for (GLint i = 0; i < num_renderers; ++i) {
-    GLint accelerated = 0;
-    if (CGLDescribeRenderer(renderer_info,
-                            i,
-                            kCGLRPAccelerated,
-                            &accelerated) != kCGLNoError) {
-      return false;
-    }
-
-    if (!accelerated)
-      continue;
-
-    GLint online = 0;
-    if (CGLDescribeRenderer(renderer_info,
-                            i,
-                            kCGLRPOnline,
-                            &online) != kCGLNoError) {
-      return false;
-    }
-
-    if (online) {
-      found_online = true;
-    } else {
-      found_offline = true;
-    }
-  }
-
-  if (found_online && found_offline) {
-    // Only switch GPUs dynamically on recent MacBook Pro models.
-    // Otherwise, keep the system on the discrete GPU for the lifetime
-    // of the browser process, since switching back and forth causes
-    // system stability issues. http://crbug.com/113703
-    std::string model;
-    int32 model_major, model_minor;
-    if (!base::mac::ParseModelIdentifier(base::mac::GetModelIdentifier(),
-                                         &model, &model_major, &model_minor)) {
-      return false;
-    }
-
-    const int kMacBookProFirstDualAMDIntelGPUModel = 8;
-
-    // Do not overwrite commandline switches to honor a user's decision.
-    bool forcibly_disable =
-        ((model == "MacBookPro") &&
-         (model_major < kMacBookProFirstDualAMDIntelGPUModel)) &&
-        !CommandLine::ForCurrentProcess()->HasSwitch(switches::kGpuSwitching);
-
-    if (forcibly_disable) {
-      GpuSwitchingManager::GetInstance()->ForceUseOfDiscreteGpu();
-      return false;
-    }
-
-    supports_dual_gpus = true;
-  }
-
-  return supports_dual_gpus;
-}
-
 }  // namespace gfx
diff --git a/ui/gl/gl_context_win.cc b/ui/gl/gl_context_win.cc
index d2aaf85..1d81142 100644
--- a/ui/gl/gl_context_win.cc
+++ b/ui/gl/gl_context_win.cc
@@ -56,8 +56,4 @@ scoped_refptr<GLContext> GLContext::CreateGLContext(
   }
 }
 
-bool GLContext::SupportsDualGpus() {
-  return false;
-}
-
 }  // namespace gfx
diff --git a/ui/gl/gl_surface_cgl.cc b/ui/gl/gl_surface_cgl.cc
index 4f2fba2..6415d23 100644
--- a/ui/gl/gl_surface_cgl.cc
+++ b/ui/gl/gl_surface_cgl.cc
@@ -12,6 +12,7 @@
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_implementation.h"
+#include "ui/gl/gpu_switching_manager.h"
 
 namespace gfx {
 
@@ -30,7 +31,7 @@ bool GLSurfaceCGL::InitializeOneOff() {
   // GPU-related stuff is very slow without this, probably because
   // the sandbox prevents loading graphics drivers or some such.
   std::vector<CGLPixelFormatAttribute> attribs;
-  if (GLContext::SupportsDualGpus()) {
+  if (gfx::GpuSwitchingManager::GetInstance()->SupportsDualGpus()) {
     // Avoid switching to the discrete GPU just for this pixel
     // format selection.
     attribs.push_back(kCGLPFAAllowOfflineRenderers);
diff --git a/ui/gl/gl_switches.cc b/ui/gl/gl_switches.cc
index dddcd3f..c197cf7 100644
--- a/ui/gl/gl_switches.cc
+++ b/ui/gl/gl_switches.cc
@@ -43,6 +43,9 @@ const char kGpuNoContextLost[]              = "gpu-no-context-lost";
 // Simulates a slow GPU.
 const char kGpuSwapDelay[]                  = "gpu-swap-delay";
 
+// Indicates whether the dual GPU switching is supported or not.
+const char kSupportsDualGpus[]              = "supports-dual-gpus";
+
 // Overwrite the default GPU automatic switching behavior to force on
 // integrated GPU or discrete GPU.
 const char kGpuSwitching[]                  = "gpu-switching";
diff --git a/ui/gl/gl_switches.h b/ui/gl/gl_switches.h
index db042f1..4e73a15 100644
--- a/ui/gl/gl_switches.h
+++ b/ui/gl/gl_switches.h
@@ -29,6 +29,7 @@ GL_EXPORT extern const char kEnableGPUClientLogging[];
 GL_EXPORT extern const char kGpuNoContextLost[];
 GL_EXPORT extern const char kGpuSwapDelay[];
 
+GL_EXPORT extern const char kSupportsDualGpus[];
 GL_EXPORT extern const char kGpuSwitching[];
 
 // The GPU switching names that can be passed to --gpu-switching.
diff --git a/ui/gl/gpu_preference.h b/ui/gl/gpu_preference.h
index 867daa1..48e2903 100644
--- a/ui/gl/gpu_preference.h
+++ b/ui/gl/gpu_preference.h
@@ -9,7 +9,7 @@ namespace gfx {
 
 // On dual-GPU systems, expresses a preference for using the integrated
 // or discrete GPU. On systems that have dual-GPU support (see
-// GLContext::SupportsDualGpus), resource sharing only works between
+// GpuDataManagerImpl), resource sharing only works between
 // contexts that are created with the same GPU preference.
 //
 // This API will likely need to be adjusted as the functionality is
diff --git a/ui/gl/gpu_switching_manager.cc b/ui/gl/gpu_switching_manager.cc
index 94c57b6..9cabb24 100644
--- a/ui/gl/gpu_switching_manager.cc
+++ b/ui/gl/gpu_switching_manager.cc
@@ -4,7 +4,68 @@
 
 #include "ui/gl/gpu_switching_manager.h"
 
+#include "base/command_line.h"
 #include "base/logging.h"
+#include "ui/gl/gl_switches.h"
+
+#if defined(OS_MACOSX)
+#include "base/mac/mac_util.h"
+#include "ui/gl/gl_context_cgl.h"
+#endif  // OS_MACOSX
+
+namespace {
+
+#if defined(OS_MACOSX)
+bool SupportsOnlineAndOfflineRenderers() {
+  // Enumerate all hardware-accelerated renderers. If we find one
+  // online and one offline, assume we're on a dual-GPU system.
+  GLuint display_mask = static_cast<GLuint>(-1);
+  CGLRendererInfoObj renderer_info = NULL;
+  GLint num_renderers = 0;
+
+  bool found_online = false;
+  bool found_offline = false;
+
+  if (CGLQueryRendererInfo(display_mask,
+                           &renderer_info,
+                           &num_renderers) != kCGLNoError) {
+    return false;
+  }
+
+  gfx::ScopedCGLRendererInfoObj scoper(renderer_info);
+
+  for (GLint i = 0; i < num_renderers; ++i) {
+    GLint accelerated = 0;
+    if (CGLDescribeRenderer(renderer_info,
+                            i,
+                            kCGLRPAccelerated,
+                            &accelerated) != kCGLNoError) {
+      return false;
+    }
+
+    if (!accelerated)
+      continue;
+
+    GLint online = 0;
+    if (CGLDescribeRenderer(renderer_info,
+                            i,
+                            kCGLRPOnline,
+                            &online) != kCGLNoError) {
+      return false;
+    }
+
+    if (online) {
+      found_online = true;
+    } else {
+      found_offline = true;
+    }
+  }
+
+  return (found_online && found_offline);
+}
+#endif  // OS_MACOSX
+
+}  // namespace anonymous
 
 namespace gfx {
 
@@ -13,35 +74,100 @@ GpuSwitchingManager* GpuSwitchingManager::GetInstance() {
   return Singleton<GpuSwitchingManager>::get();
 }
 
-GpuSwitchingManager::GpuSwitchingManager() {
+GpuSwitchingManager::GpuSwitchingManager()
+    : gpu_switching_option_(PreferIntegratedGpu),
+      gpu_switching_option_set_(false),
+      supports_dual_gpus_(false),
+      supports_dual_gpus_set_(false) {
+#if defined(OS_MACOSX)
+  discrete_pixel_format_ = NULL;
+#endif  // OS_MACOSX
 }
 
 GpuSwitchingManager::~GpuSwitchingManager() {
+#if defined(OS_MACOSX)
+  if (discrete_pixel_format_)
+    CGLReleasePixelFormat(discrete_pixel_format_);
+#endif  // OS_MACOSX
 }
 
 void GpuSwitchingManager::ForceUseOfIntegratedGpu() {
-  gpu_switching_option_.clear();
-  gpu_switching_option_.push_back(PreferIntegratedGpu);
+  DCHECK(SupportsDualGpus());
+  if (gpu_switching_option_set_) {
+    DCHECK_EQ(gpu_switching_option_, PreferIntegratedGpu);
+  } else {
+    gpu_switching_option_ = PreferIntegratedGpu;
+    gpu_switching_option_set_ = true;
+  }
 }
 
 void GpuSwitchingManager::ForceUseOfDiscreteGpu() {
-  gpu_switching_option_.clear();
-  gpu_switching_option_.push_back(PreferDiscreteGpu);
+  DCHECK(SupportsDualGpus());
+  if (gpu_switching_option_set_) {
+    DCHECK_EQ(gpu_switching_option_, PreferDiscreteGpu);
+  } else {
+    gpu_switching_option_ = PreferDiscreteGpu;
+    gpu_switching_option_set_ = true;
+#if defined(OS_MACOSX)
+    // Create a pixel format that lasts the lifespan of Chrome, so Chrome
+    // stays on the discrete GPU.
+    SwitchToDiscreteGpuMac();
+#endif  // OS_MACOSX
+  }
+}
+
+bool GpuSwitchingManager::SupportsDualGpus() {
+  if (!supports_dual_gpus_set_) {
+    const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+    bool flag = false;
+    if (command_line.HasSwitch(switches::kSupportsDualGpus)) {
+      // GPU process, flag is passed down from browser process.
+      std::string flag_string = command_line.GetSwitchValueASCII(
+          switches::kSupportsDualGpus);
+      if (flag_string == "true") {
+        flag = true;
+      } else if (flag_string == "false") {
+        flag = false;
+      } else {
+        NOTIMPLEMENTED();
+      }
+    } else {
+      // Browser process.
+      // We only compute this flag in the browser process.
+#if defined(OS_MACOSX)
+      flag = SupportsOnlineAndOfflineRenderers();
+      if (flag && command_line.HasSwitch(switches::kUseGL) &&
+          command_line.GetSwitchValueASCII(switches::kUseGL) !=
+            gfx::kGLImplementationDesktopName)
+        flag = false;
+
+      if (flag && !base::mac::IsOSLionOrLater())
+        flag = false;
+#endif  // OS_MACOSX
+    }
+    supports_dual_gpus_ = flag;
+    supports_dual_gpus_set_ = true;
+  }
+  return supports_dual_gpus_;
 }
 
 GpuPreference GpuSwitchingManager::AdjustGpuPreference(
     GpuPreference gpu_preference) {
-  if (gpu_switching_option_.size() == 0)
+  if (!gpu_switching_option_set_)
     return gpu_preference;
-  DCHECK_EQ(gpu_switching_option_.size(), 1u);
+  return gpu_switching_option_;
+}
+
 #if defined(OS_MACOSX)
-  // Create a pixel format that lasts the lifespan of Chrome, so Chrome
-  // stays on the discrete GPU.
-  if (gpu_switching_option_[0] == PreferDiscreteGpu)
-    GLContextCGL::ForceUseOfDiscreteGPU();
-#endif  // OS_MACOSX
-  return gpu_switching_option_[0];
+void GpuSwitchingManager::SwitchToDiscreteGpuMac() {
+  if (discrete_pixel_format_)
+    return;
+  CGLPixelFormatAttribute attribs[1];
+  attribs[0] = static_cast<CGLPixelFormatAttribute>(0);
+  GLint num_pixel_formats = 0;
+  CGLChoosePixelFormat(attribs, &discrete_pixel_format_, &num_pixel_formats);
 }
+#endif  // OS_MACOSX
 
 }  // namespace gfx
 
diff --git a/ui/gl/gpu_switching_manager.h b/ui/gl/gpu_switching_manager.h
index 0534c98..5cf87e5 100644
--- a/ui/gl/gpu_switching_manager.h
+++ b/ui/gl/gpu_switching_manager.h
@@ -8,12 +8,13 @@
 #include <vector>
 
 #include "base/memory/singleton.h"
-#if defined(OS_MACOSX)
-#include "ui/gl/gl_context_cgl.h"
-#endif  // OS_MACOSX
 #include "ui/gl/gl_export.h"
 #include "ui/gl/gpu_preference.h"
 
+#if defined(OS_MACOSX)
+#include <OpenGL/OpenGL.h>
+#endif  // OS_MACOSX
+
 namespace gfx {
 
 class GL_EXPORT GpuSwitchingManager {
@@ -21,24 +22,39 @@ class GL_EXPORT GpuSwitchingManager {
   // Getter for the singleton. This will return NULL on failure.
   static GpuSwitchingManager* GetInstance();
 
-  // Set the switching option, but don't take any actions until later
-  // we access GPU.
+  // Set the switching option to PreferIntegratedGpu.
   void ForceUseOfIntegratedGpu();
+  // Set the switching option to PreferDiscreteGpu; switch to discrete GPU
+  // immediately on Mac where dual GPU switching is supported.
   void ForceUseOfDiscreteGpu();
 
   // If no GPU is forced, return the original GpuPreference; otherwise, return
   // the forced GPU.
-  // If forcing discrete GPU on Mac, we set up a pixel format, which keeps
-  // Chrome on the discrete GPU throughout the rest of Chrome's lifetime.
   GpuPreference AdjustGpuPreference(GpuPreference gpu_preference);
 
+  // In the browser process, the value for this flag is computed the first time
+  // this function is called.
+  // In the GPU process, the value is passed from the browser process using the
+  // --supports-dual-gpus commandline switch.
+  bool SupportsDualGpus();
+
  private:
   friend struct DefaultSingletonTraits<GpuSwitchingManager>;
 
   GpuSwitchingManager();
   virtual ~GpuSwitchingManager();
 
-  std::vector<GpuPreference> gpu_switching_option_;
+#if defined(OS_MACOSX)
+  void SwitchToDiscreteGpuMac();
+
+  CGLPixelFormatObj discrete_pixel_format_;
+#endif  // OS_MACOSX
+
+  GpuPreference gpu_switching_option_;
+  bool gpu_switching_option_set_;
+
+  bool supports_dual_gpus_;
+  bool supports_dual_gpus_set_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuSwitchingManager);
 };
author	zmo@google.com <zmo@google.com@0039d316-1c4b-4281-b951-d872f2087c98>	2012-10-01 23:06:04 +0000
committer	zmo@google.com <zmo@google.com@0039d316-1c4b-4281-b951-d872f2087c98>	2012-10-01 23:06:04 +0000
commit	48fe7e4cbaf045f280a90c6e5f93867d49082ce6 (patch)
tree	33c1511de1ee714d1eff15f18f9726ca939fcd45 /ui/gl
parent	19be7a6da266c546a7a52a7170df789f1ab8b710 (diff)
download	chromium_src-48fe7e4cbaf045f280a90c6e5f93867d49082ce6.zip chromium_src-48fe7e4cbaf045f280a90c6e5f93867d49082ce6.tar.gz chromium_src-48fe7e4cbaf045f280a90c6e5f93867d49082ce6.tar.bz2