ozone: Cleanup code involved in Modeset

1. Use const and pass by reference when appropriate.
2. Convert NULL to nullptr in the modified files.
3. Fix some file formatting to match clang.
4. Use default and explicit constructors where they should be
5. Remove unused SetCrtc overload.

BUG=987274
TEST=HardwareDisplayControllerTest, HardwareDisplayPlaneManager*Test

Change-Id: Ic0f019bfa862a5160805ecbfd22dc391e6f9f418
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2031864
Commit-Queue: Mark Yacoub <markyacoub@google.com>
Auto-Submit: Mark Yacoub <markyacoub@google.com>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737024}

ui/ozone/platform/drm/gpu/crtc_controller.cc

@@ -45,9 +45,9 @@ CrtcController::~CrtcController() {
 }
 
 bool CrtcController::Modeset(const DrmOverlayPlane& plane,
-                             drmModeModeInfo mode) {
+                             const drmModeModeInfo& mode) {
   if (!drm_->SetCrtc(crtc_, plane.buffer->opaque_framebuffer_id(),
-                     std::vector<uint32_t>(1, connector_), &mode)) {
+                     std::vector<uint32_t>(1, connector_), mode)) {
     PLOG(ERROR) << "Failed to modeset: crtc=" << crtc_
                 << " connector=" << connector_
                 << " framebuffer_id=" << plane.buffer->opaque_framebuffer_id()

ui/ozone/platform/drm/gpu/crtc_controller.h

@@ -42,7 +42,7 @@ class CrtcController {
 
   // Perform the initial modesetting operation using |plane| as the buffer for
   // the primary plane. The CRTC configuration is specified by |mode|.
-  bool Modeset(const DrmOverlayPlane& plane, drmModeModeInfo mode);
+  bool Modeset(const DrmOverlayPlane& plane, const drmModeModeInfo& mode);
 
   // Disables the controller.
   bool Disable();

ui/ozone/platform/drm/gpu/drm_device.cc

@@ -146,7 +146,7 @@ DrmPropertyBlobMetadata::~DrmPropertyBlobMetadata() {
 class DrmDevice::PageFlipManager {
  public:
   PageFlipManager() : next_id_(0) {}
-  ~PageFlipManager() {}
+  ~PageFlipManager() = default;
 
   void OnPageFlip(uint32_t frame, base::TimeTicks timestamp, uint64_t id) {
     auto it =
@@ -248,7 +248,7 @@ DrmDevice::DrmDevice(const base::FilePath& device_path,
       is_primary_device_(is_primary_device),
       gbm_(std::move(gbm)) {}
 
-DrmDevice::~DrmDevice() {}
+DrmDevice::~DrmDevice() = default;
 
 bool DrmDevice::Initialize() {
   // Ignore devices that cannot perform modesetting.
@@ -302,36 +302,22 @@ ScopedDrmCrtcPtr DrmDevice::GetCrtc(uint32_t crtc_id) {
 bool DrmDevice::SetCrtc(uint32_t crtc_id,
                         uint32_t framebuffer,
                         std::vector<uint32_t> connectors,
-                        drmModeModeInfo* mode) {
+                        const drmModeModeInfo& mode) {
   DCHECK(file_.IsValid());
   DCHECK(!connectors.empty());
-  DCHECK(mode);
 
   TRACE_EVENT2("drm", "DrmDevice::SetCrtc", "crtc", crtc_id, "size",
-               gfx::Size(mode->hdisplay, mode->vdisplay).ToString());
+               gfx::Size(mode.hdisplay, mode.vdisplay).ToString());
   return !drmModeSetCrtc(file_.GetPlatformFile(), crtc_id, framebuffer, 0, 0,
-                         connectors.data(), connectors.size(), mode);
-}
-
-bool DrmDevice::SetCrtc(drmModeCrtc* crtc, std::vector<uint32_t> connectors) {
-  DCHECK(file_.IsValid());
-  // If there's no buffer then the CRTC was disabled.
-  if (!crtc->buffer_id)
-    return DisableCrtc(crtc->crtc_id);
-
-  DCHECK(!connectors.empty());
-
-  TRACE_EVENT1("drm", "DrmDevice::RestoreCrtc", "crtc", crtc->crtc_id);
-  return !drmModeSetCrtc(file_.GetPlatformFile(), crtc->crtc_id,
-                         crtc->buffer_id, crtc->x, crtc->y, connectors.data(),
-                         connectors.size(), &crtc->mode);
+                         connectors.data(), connectors.size(),
+                         const_cast<drmModeModeInfo*>(&mode));
 }
 
 bool DrmDevice::DisableCrtc(uint32_t crtc_id) {
   DCHECK(file_.IsValid());
   TRACE_EVENT1("drm", "DrmDevice::DisableCrtc", "crtc", crtc_id);
-  return !drmModeSetCrtc(file_.GetPlatformFile(), crtc_id, 0, 0, 0, NULL, 0,
-                         NULL);
+  return !drmModeSetCrtc(file_.GetPlatformFile(), crtc_id, 0, 0, 0, nullptr, 0,
+                         nullptr);
 }
 
 ScopedDrmConnectorPtr DrmDevice::GetConnector(uint32_t connector_id) {
@@ -432,7 +418,8 @@ bool DrmDevice::SetProperty(uint32_t connector_id,
                                       property_id, value);
 }
 
-ScopedDrmPropertyBlob DrmDevice::CreatePropertyBlob(void* blob, size_t size) {
+ScopedDrmPropertyBlob DrmDevice::CreatePropertyBlob(const void* blob,
+                                                    size_t size) {
   uint32_t id = 0;
   int ret = drmModeCreatePropertyBlob(file_.GetPlatformFile(), blob, size, &id);
   DCHECK(!ret && id);
@@ -526,7 +513,7 @@ bool DrmDevice::MapDumbBuffer(uint32_t handle, size_t size, void** pixels) {
     return false;
   }
 
-  *pixels = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED,
+  *pixels = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED,
                  file_.GetPlatformFile(), map_request.offset);
   if (*pixels == MAP_FAILED) {
     PLOG(ERROR) << "Cannot mmap dumb buffer";

ui/ozone/platform/drm/gpu/drm_device.h

@@ -106,12 +106,7 @@ class DrmDevice : public base::RefCountedThreadSafe<DrmDevice> {
   virtual bool SetCrtc(uint32_t crtc_id,
                        uint32_t framebuffer,
                        std::vector<uint32_t> connectors,
-                       drmModeModeInfo* mode);
-
-  // Used to set a specific configuration to the CRTC. Normally this function
-  // would be called with a CRTC saved state (from |GetCrtc|) to restore it to
-  // its original configuration.
-  virtual bool SetCrtc(drmModeCrtc* crtc, std::vector<uint32_t> connectors);
+                       const drmModeModeInfo& mode);
 
   virtual bool DisableCrtc(uint32_t crtc_id);
 
@@ -168,7 +163,8 @@ class DrmDevice : public base::RefCountedThreadSafe<DrmDevice> {
                            uint64_t value);
 
   // Creates a property blob with data |blob| of size |size|.
-  virtual ScopedDrmPropertyBlob CreatePropertyBlob(void* blob, size_t size);
+  virtual ScopedDrmPropertyBlob CreatePropertyBlob(const void* blob,
+                                                   size_t size);
 
   virtual void DestroyPropertyBlob(uint32_t id);
 

ui/ozone/platform/drm/gpu/drm_overlay_validator_unittest.cc

@@ -47,7 +47,7 @@ constexpr uint32_t kInFormatsPropId = 301;
 
 class DrmOverlayValidatorTest : public testing::Test {
  public:
-  DrmOverlayValidatorTest() {}
+  DrmOverlayValidatorTest() = default;
 
   void SetUp() override;
   void TearDown() override;

ui/ozone/platform/drm/gpu/hardware_display_controller.cc

@@ -52,7 +52,7 @@ void DrawCursor(DrmDumbBuffer* cursor, const SkBitmap& image) {
   // Clear to transparent in case |image| is smaller than the canvas.
   SkCanvas* canvas = cursor->GetCanvas();
   canvas->clear(SK_ColorTRANSPARENT);
-  canvas->drawBitmapRect(image, damage, NULL);
+  canvas->drawBitmapRect(image, damage, nullptr);
 }
 
 }  // namespace
@@ -65,11 +65,10 @@ HardwareDisplayController::HardwareDisplayController(
   AllocateCursorBuffers();
 }
 
-HardwareDisplayController::~HardwareDisplayController() {
-}
+HardwareDisplayController::~HardwareDisplayController() = default;
 
 bool HardwareDisplayController::Modeset(const DrmOverlayPlane& primary,
-                                        drmModeModeInfo mode) {
+                                        const drmModeModeInfo& mode) {
   TRACE_EVENT0("drm", "HDC::Modeset");
   DCHECK(primary.buffer.get());
   bool status = true;

ui/ozone/platform/drm/gpu/hardware_display_controller.h

@@ -95,7 +95,7 @@ class HardwareDisplayController {
 
   // Performs the initial CRTC configuration. If successful, it will display the
   // framebuffer for |primary| with |mode|.
-  bool Modeset(const DrmOverlayPlane& primary, drmModeModeInfo mode);
+  bool Modeset(const DrmOverlayPlane& primary, const drmModeModeInfo& mode);
 
   // Performs a CRTC configuration re-using the modes from the CRTCs.
   bool Enable(const DrmOverlayPlane& primary);

ui/ozone/platform/drm/gpu/hardware_display_plane_manager.cc

@@ -28,8 +28,7 @@ HardwareDisplayPlaneList::HardwareDisplayPlaneList() {
   atomic_property_set.reset(drmModeAtomicAlloc());
 }
 
-HardwareDisplayPlaneList::~HardwareDisplayPlaneList() {
-}
+HardwareDisplayPlaneList::~HardwareDisplayPlaneList() = default;
 
 HardwareDisplayPlaneList::PageFlipInfo::PageFlipInfo(uint32_t crtc_id,
                                                      uint32_t framebuffer)
@@ -38,8 +37,7 @@ HardwareDisplayPlaneList::PageFlipInfo::PageFlipInfo(uint32_t crtc_id,
 HardwareDisplayPlaneList::PageFlipInfo::PageFlipInfo(
     const PageFlipInfo& other) = default;
 
-HardwareDisplayPlaneList::PageFlipInfo::~PageFlipInfo() {
-}
+HardwareDisplayPlaneList::PageFlipInfo::~PageFlipInfo() = default;
 
 HardwareDisplayPlaneManager::CrtcState::CrtcState() = default;
 
@@ -50,8 +48,7 @@ HardwareDisplayPlaneManager::CrtcState::CrtcState(CrtcState&&) = default;
 HardwareDisplayPlaneManager::HardwareDisplayPlaneManager(DrmDevice* drm)
     : drm_(drm) {}
 
-HardwareDisplayPlaneManager::~HardwareDisplayPlaneManager() {
-}
+HardwareDisplayPlaneManager::~HardwareDisplayPlaneManager() = default;
 
 bool HardwareDisplayPlaneManager::Initialize() {
 // Try to get all of the planes if possible, so we don't have to try to
@@ -97,9 +94,10 @@ HardwareDisplayPlane* HardwareDisplayPlaneManager::FindNextUnusedPlane(
 }
 
 int HardwareDisplayPlaneManager::LookupCrtcIndex(uint32_t crtc_id) const {
-  for (size_t i = 0; i < crtc_state_.size(); ++i)
+  for (size_t i = 0; i < crtc_state_.size(); ++i) {
     if (crtc_state_[i].properties.id == crtc_id)
       return i;
+  }
   return -1;
 }
 
@@ -182,8 +180,9 @@ bool HardwareDisplayPlaneManager::AssignOverlayPlanes(
 
       // This returns a number in 16.16 fixed point, required by the DRM overlay
       // APIs.
-      auto to_fixed_point =
-          [](double v) -> uint32_t { return v * kFixedPointScaleValue; };
+      auto to_fixed_point = [](double v) -> uint32_t {
+        return v * kFixedPointScaleValue;
+      };
       fixed_point_rect = gfx::Rect(to_fixed_point(crop_rect.x()),
                                    to_fixed_point(crop_rect.y()),
                                    to_fixed_point(crop_rect.width()),

ui/ozone/platform/drm/gpu/hardware_display_plane_manager.h

@@ -56,7 +56,7 @@ struct HardwareDisplayPlaneList {
 
 class HardwareDisplayPlaneManager {
  public:
-  HardwareDisplayPlaneManager(DrmDevice* drm);
+  explicit HardwareDisplayPlaneManager(DrmDevice* drm);
   virtual ~HardwareDisplayPlaneManager();
 
   // This parses information from the drm driver, adding any new planes

ui/ozone/platform/drm/gpu/hardware_display_plane_manager_atomic.cc

@@ -57,8 +57,8 @@ HardwareDisplayPlaneManagerAtomic::HardwareDisplayPlaneManagerAtomic(
     DrmDevice* drm)
     : HardwareDisplayPlaneManager(drm) {}
 
-HardwareDisplayPlaneManagerAtomic::~HardwareDisplayPlaneManagerAtomic() {
-}
+HardwareDisplayPlaneManagerAtomic::~HardwareDisplayPlaneManagerAtomic() =
+    default;
 
 bool HardwareDisplayPlaneManagerAtomic::Commit(
     HardwareDisplayPlaneList* plane_list,

ui/ozone/platform/drm/gpu/hardware_display_plane_manager_atomic.h

@@ -15,7 +15,7 @@ namespace ui {
 
 class HardwareDisplayPlaneManagerAtomic : public HardwareDisplayPlaneManager {
  public:
-  HardwareDisplayPlaneManagerAtomic(DrmDevice* drm);
+  explicit HardwareDisplayPlaneManagerAtomic(DrmDevice* drm);
   ~HardwareDisplayPlaneManagerAtomic() override;
 
   // HardwareDisplayPlaneManager:

ui/ozone/platform/drm/gpu/hardware_display_plane_manager_legacy.cc

@@ -42,8 +42,8 @@ HardwareDisplayPlaneManagerLegacy::HardwareDisplayPlaneManagerLegacy(
     DrmDevice* drm)
     : HardwareDisplayPlaneManager(drm) {}
 
-HardwareDisplayPlaneManagerLegacy::~HardwareDisplayPlaneManagerLegacy() {
-}
+HardwareDisplayPlaneManagerLegacy::~HardwareDisplayPlaneManagerLegacy() =
+    default;
 
 bool HardwareDisplayPlaneManagerLegacy::Commit(
     HardwareDisplayPlaneList* plane_list,

ui/ozone/platform/drm/gpu/hardware_display_plane_manager_legacy.h

@@ -15,7 +15,7 @@ namespace ui {
 
 class HardwareDisplayPlaneManagerLegacy : public HardwareDisplayPlaneManager {
  public:
-  HardwareDisplayPlaneManagerLegacy(DrmDevice* device);
+  explicit HardwareDisplayPlaneManagerLegacy(DrmDevice* device);
   ~HardwareDisplayPlaneManagerLegacy() override;
 
   // HardwareDisplayPlaneManager:

ui/ozone/platform/drm/gpu/mock_drm_device.cc

@@ -82,7 +82,6 @@ MockDrmDevice::MockDrmDevice(std::unique_ptr<GbmDevice> gbm_device)
                 std::move(gbm_device)),
       get_crtc_call_count_(0),
       set_crtc_call_count_(0),
-      restore_crtc_call_count_(0),
       add_framebuffer_call_count_(0),
       remove_framebuffer_call_count_(0),
       page_flip_call_count_(0),
@@ -150,7 +149,7 @@ bool MockDrmDevice::InitializeStateWithResult(
   return plane_manager_->Initialize();
 }
 
-MockDrmDevice::~MockDrmDevice() {}
+MockDrmDevice::~MockDrmDevice() = default;
 
 ScopedDrmResourcesPtr MockDrmDevice::GetResources() {
   ScopedDrmResourcesPtr resources(DrmAllocator<drmModeRes>());
@@ -198,19 +197,13 @@ ScopedDrmCrtcPtr MockDrmDevice::GetCrtc(uint32_t crtc_id) {
 bool MockDrmDevice::SetCrtc(uint32_t crtc_id,
                             uint32_t framebuffer,
                             std::vector<uint32_t> connectors,
-                            drmModeModeInfo* mode) {
+                            const drmModeModeInfo& mode) {
   crtc_fb_[crtc_id] = framebuffer;
   current_framebuffer_ = framebuffer;
   set_crtc_call_count_++;
   return set_crtc_expectation_;
 }
 
-bool MockDrmDevice::SetCrtc(drmModeCrtc* crtc,
-                            std::vector<uint32_t> connectors) {
-  restore_crtc_call_count_++;
-  return true;
-}
-
 bool MockDrmDevice::DisableCrtc(uint32_t crtc_id) {
   current_framebuffer_ = 0;
   return true;
@@ -304,7 +297,7 @@ bool MockDrmDevice::SetProperty(uint32_t connector_id,
   return true;
 }
 
-ScopedDrmPropertyBlob MockDrmDevice::CreatePropertyBlob(void* blob,
+ScopedDrmPropertyBlob MockDrmDevice::CreatePropertyBlob(const void* blob,
                                                         size_t size) {
   uint32_t id = ++property_id_generator_;
   allocated_property_blobs_.insert(id);

ui/ozone/platform/drm/gpu/mock_drm_device.h

@@ -44,7 +44,7 @@ class MockDrmDevice : public DrmDevice {
     std::vector<DrmDevice::Property> properties;
   };
 
-  MockDrmDevice(std::unique_ptr<GbmDevice> gbm_device);
+  explicit MockDrmDevice(std::unique_ptr<GbmDevice> gbm_device);
 
   static ScopedDrmPropertyBlobPtr AllocateInFormatsBlob(
       uint32_t id,
@@ -53,7 +53,6 @@ class MockDrmDevice : public DrmDevice {
 
   int get_get_crtc_call_count() const { return get_crtc_call_count_; }
   int get_set_crtc_call_count() const { return set_crtc_call_count_; }
-  int get_restore_crtc_call_count() const { return restore_crtc_call_count_; }
   int get_add_framebuffer_call_count() const {
     return add_framebuffer_call_count_;
   }
@@ -114,8 +113,7 @@ class MockDrmDevice : public DrmDevice {
   bool SetCrtc(uint32_t crtc_id,
                uint32_t framebuffer,
                std::vector<uint32_t> connectors,
-               drmModeModeInfo* mode) override;
-  bool SetCrtc(drmModeCrtc* crtc, std::vector<uint32_t> connectors) override;
+               const drmModeModeInfo& mode) override;
   bool DisableCrtc(uint32_t crtc_id) override;
   ScopedDrmConnectorPtr GetConnector(uint32_t connector_id) override;
   bool AddFramebuffer2(uint32_t width,
@@ -139,7 +137,8 @@ class MockDrmDevice : public DrmDevice {
   bool SetProperty(uint32_t connector_id,
                    uint32_t property_id,
                    uint64_t value) override;
-  ScopedDrmPropertyBlob CreatePropertyBlob(void* blob, size_t size) override;
+  ScopedDrmPropertyBlob CreatePropertyBlob(const void* blob,
+                                           size_t size) override;
   void DestroyPropertyBlob(uint32_t id) override;
   bool GetCapability(uint64_t capability, uint64_t* value) override;
   ScopedDrmPropertyBlobPtr GetPropertyBlob(uint32_t property_id) override;
@@ -183,7 +182,6 @@ class MockDrmDevice : public DrmDevice {
 
   int get_crtc_call_count_;
   int set_crtc_call_count_;
-  int restore_crtc_call_count_;
   int add_framebuffer_call_count_;
   int remove_framebuffer_call_count_;
   int page_flip_call_count_;