Support failing modeset

chromeos/display/output_configurator.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/display/output_configurator.h"
 
 #include <X11/Xlib.h>
 #include <X11/extensions/Xrandr.h>
 #include <X11/extensions/XInput2.h>
 
@@ skipping 925 matching lines @@
           output->transform = GetExtendedModeCTM(*output, width, height);
       }
       break;
     }
   }
 
   // Finally, apply the desired changes.
   DCHECK_EQ(cached_outputs_.size(), updated_outputs.size());
   if (!updated_outputs.empty()) {
     delegate_->CreateFrameBuffer(width, height, updated_outputs);
-    for (size_t i = 0; i < updated_outputs.size(); ++i) {
+    for (size_t i = 0; i < updated_outputs.size(); ) {

[Daniel Erat, 2013/12/21 14:25:54]

please structure this code in a more logical way:

for (size_t i = 0; i < updated_outputs.size(); ++i) {
  ...
  while (!delegate_->ConfigureCrtc(...)) {
    updated_outputs[i].current_mode = ...
    // break when you run out of modes to try
  }
}

       const OutputSnapshot& output = updated_outputs[i];
       if (delegate_->ConfigureCrtc(output.crtc, output.current_mode,
                                    output.output, output.x, output.y)) {
         if (output.touch_device_id)
           delegate_->ConfigureCTM(output.touch_device_id, output.transform);
         cached_outputs_[i] = updated_outputs[i];
       } else {
         LOG(WARNING) << "Unable to configure CRTC " << output.crtc << ":"
                      << " mode=" << output.current_mode
                      << " output=" << output.output
                      << " x=" << output.x
                      << " y=" << output.y;
+
+        int best_mode_pixels = 0;
+        const ModeInfo mode_info =
+               output.mode_infos.find(output.current_mode)->second;

[Daniel Erat, 2013/12/21 14:25:54]

indent 4 spaces, not 7

+        int pixels = mode_info.width * mode_info.height;

[Daniel Erat, 2013/12/21 14:25:54]

give this a better name like current_mode_pixels

+        for (ModeInfoMap::const_iterator it = output.mode_infos.begin();
+             it != output.mode_infos.end(); it++) {
+          int pixel_count = it->second.width * it->second.height;
+          if ((pixel_count < pixels) && (pixel_count > best_mode_pixels)) {
+            updated_outputs[i].current_mode = it->first;
+            best_mode_pixels = pixel_count;
+          }
+        }

[marcheu, 2013/12/21 01:56:15]

If we are in mirror mode, and modeset fails on one of the monitors, we will end
up with two mismatched modes. I'm not sure what the effect will be, but most
likely not what we want. We should probably fix that before landing this change.

To fix it, it might be sufficient to return false in this case.

+
+        if (best_mode_pixels != 0)
+        continue;

[Daniel Erat, 2013/12/21 14:25:54]

indent 2 spaces, not 0

       }
+      i++;
     }
   }
 
   output_state_ = output_state;
   power_state_ = power_state;
   return true;
 }
 
 OutputState OutputConfigurator::ChooseOutputState(
     DisplayPowerState power_state) const {
@@ skipping 115 matching lines @@
   float width_ratio = static_cast<float>(mirror_mode_info->width) /
       static_cast<float>(native_mode_info->width);
   float height_ratio = static_cast<float>(mirror_mode_info->height) /
       static_cast<float>(native_mode_info->height);
 
   area_ratio = width_ratio * height_ratio;
   return area_ratio;
 }
 
 }  // namespace chromeos