commit fe70c80d712417a7a76f0337ac44a5568ed4850b
author Drew Davenport <ddavenport@google.com> Thu Nov 07 13:02:21 2024 -0700
committer Drew Davenport <ddavenport@google.com> Thu Nov 14 09:47:25 2024 -0700
tree 0df4fe29aaf51212118753f9c9279ec2a55ff917
parent 9a2e2f6b95094f9d6eaaa97049d551ea584b4580

drm_hwcomposer: Simplify state tracking for staged configs

This helps to clarify and distinguish between the config that is
currently committed to the kernel and a pending config that will be
applied in a future update.

- Make staged_mode_config_id_ optional to indicate whether the config
  change is pending or has completed.
- Remove staged_mode_ since it can be derived from
  staged_mode_config_id_
- Add GetConfig() helper, and implement various methods to get a config
  in terms of it.

Change-Id: I1dad0fa8a52bc91d5210b3b096a43c21b2e842f5
Signed-off-by: Drew Davenport <ddavenport@google.com>

hwc2_device/HwcDisplay.cpp

diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index ffe6b11..7559835 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -96,20 +96,21 @@
   Deinit();
 };
 
-auto HwcDisplay::GetCurrentConfig() const -> const HwcDisplayConfig * {
-  auto config_iter = configs_.hwc_configs.find(configs_.active_config_id);
+auto HwcDisplay::GetConfig(hwc2_config_t config_id) const
+    -> const HwcDisplayConfig * {
+  auto config_iter = configs_.hwc_configs.find(config_id);
   if (config_iter == configs_.hwc_configs.end()) {
     return nullptr;
   }
   return &config_iter->second;
 }
 
+auto HwcDisplay::GetCurrentConfig() const -> const HwcDisplayConfig * {
+  return GetConfig(configs_.active_config_id);
+}
+
 auto HwcDisplay::GetLastRequestedConfig() const -> const HwcDisplayConfig * {
-  auto config_iter = configs_.hwc_configs.find(staged_mode_config_id_);
-  if (config_iter == configs_.hwc_configs.end()) {
-    return nullptr;
-  }
-  return &config_iter->second;
+  return GetConfig(staged_mode_config_id_.value_or(configs_.active_config_id));
 }
 
 auto HwcDisplay::QueueConfig(hwc2_config_t config, int64_t desired_time,
@@ -136,7 +137,6 @@
 
   // Queue the config change timing to be consistent with the requested
   // refresh time.
-  staged_mode_ = configs_.hwc_configs[config].mode;
   staged_mode_change_time_ = out_timing->refresh_time_ns;
   staged_mode_config_id_ = config;
 
@@ -296,10 +296,12 @@
 }
 
 HWC2::Error HwcDisplay::GetActiveConfig(hwc2_config_t *config) const {
-  if (configs_.hwc_configs.count(staged_mode_config_id_) == 0)
+  // If a config has been queued, it is considered the "active" config.
+  const HwcDisplayConfig *hwc_config = GetLastRequestedConfig();
+  if (hwc_config == nullptr)
     return HWC2::Error::BadConfig;
 
-  *config = staged_mode_config_id_;
+  *config = hwc_config->id;
   return HWC2::Error::None;
 }
 
@@ -532,17 +534,22 @@
   GetDisplayVsyncPeriod(&prev_vperiod_ns);
 
   auto mode_update_commited_ = false;
-  if (staged_mode_ &&
+  if (staged_mode_config_id_ &&
       staged_mode_change_time_ <= ResourceManager::GetTimeMonotonicNs()) {
+    const HwcDisplayConfig *staged_config = GetConfig(
+        staged_mode_config_id_.value());
+    if (staged_config == nullptr) {
+      return HWC2::Error::BadConfig;
+    }
     client_layer_.SetLayerDisplayFrame(
         (hwc_rect_t){.left = 0,
                      .top = 0,
-                     .right = int(staged_mode_->GetRawMode().hdisplay),
-                     .bottom = int(staged_mode_->GetRawMode().vdisplay)});
+                     .right = int(staged_config->mode.GetRawMode().hdisplay),
+                     .bottom = int(staged_config->mode.GetRawMode().vdisplay)});
 
-    configs_.active_config_id = staged_mode_config_id_;
+    configs_.active_config_id = staged_mode_config_id_.value();
 
-    a_args.display_mode = *staged_mode_;
+    a_args.display_mode = staged_config->mode;
     if (!a_args.test_only) {
       mode_update_commited_ = true;
     }
@@ -624,7 +631,7 @@
   }
 
   if (mode_update_commited_) {
-    staged_mode_.reset();
+    staged_mode_config_id_.reset();
     vsync_tracking_en_ = false;
     if (last_vsync_ts_ != 0) {
       hwc_->SendVsyncPeriodTimingChangedEventToClient(handle_,
@@ -679,7 +686,6 @@
     return HWC2::Error::BadConfig;
   }
 
-  staged_mode_ = configs_.hwc_configs[config].mode;
   staged_mode_change_time_ = change_time;
   staged_mode_config_id_ = config;
 

hwc2_device/HwcDisplay.h

diff --git a/hwc2_device/HwcDisplay.h b/hwc2_device/HwcDisplay.h
index 91e5df7..7cc1bee 100644
--- a/hwc2_device/HwcDisplay.h
+++ b/hwc2_device/HwcDisplay.h
@@ -78,6 +78,9 @@
   auto QueueConfig(hwc2_config_t config, int64_t desired_time, bool seamless,
                    QueuedConfigTiming *out_timing) -> ConfigError;
 
+  // Get the HwcDisplayConfig, or nullptor if none.
+  auto GetConfig(hwc2_config_t config_id) const -> const HwcDisplayConfig *;
+
   // HWC2 Hooks - these should not be used outside of the hwc2 device.
   HWC2::Error AcceptDisplayChanges();
   HWC2::Error CreateLayer(hwc2_layer_t *layer);
@@ -226,9 +229,8 @@
 
   SharedFd present_fence_;
 
-  std::optional<DrmMode> staged_mode_;
   int64_t staged_mode_change_time_{};
-  uint32_t staged_mode_config_id_{};
+  std::optional<uint32_t> staged_mode_config_id_{};
 
   std::shared_ptr<DrmDisplayPipeline> pipeline_;