Revert "Revert "libhwc2.1: cleanup of BrightnessController code""

This reverts commit f87ab4e4b8b0f97e50ae34ada3a048ba08d5aeea.

Bug: 268991972
Change-Id: Iba217f322594360f29553aa693efdaaacafaa34c
Signed-off-by: Shiyong Li <shiyongli@google.com>

3 files changed, 62 insertions, 49 deletions

diff --git a/libhwc2.1/libdevice/BrightnessController.cpp b/libhwc2.1/libdevice/BrightnessController.cpp
index 5b91786..b4603c3 100644
--- a/libhwc2.1/libdevice/BrightnessController.cpp
+++ b/libhwc2.1/libdevice/BrightnessController.cpp
@@ -253,14 +253,15 @@ int BrightnessController::processDisplayBrightness(float brightness, const nsecs
     // path should check the sysfs content.
     if (mUncheckedGbhmRequest) {
         ATRACE_NAME("check_ghbm_mode");
-        checkSysfsStatus(kGlobalHbmModeFileNode,
+        checkSysfsStatus(GetPanelSysfileByIndex(kGlobalHbmModeFileNode),
                          {std::to_string(toUnderlying(mPendingGhbmStatus.load()))}, vsyncNs * 5);
         mUncheckedGbhmRequest = false;
     }
 
     if (mUncheckedLhbmRequest) {
         ATRACE_NAME("check_lhbm_mode");
-        checkSysfsStatus(kLocalHbmModeFileNode, {std::to_string(mPendingLhbmStatus)}, vsyncNs * 5);
+        checkSysfsStatus(GetPanelSysfileByIndex(kLocalHbmModeFileNode),
+                         {std::to_string(mPendingLhbmStatus)}, vsyncNs * 5);
         mUncheckedLhbmRequest = false;
     }
 
@@ -299,7 +300,8 @@ int BrightnessController::applyPendingChangeViaSysfs(const nsecs_t vsyncNs) {
 
     if (mUncheckedBlRequest) {
         ATRACE_NAME("check_bl_value");
-        checkSysfsStatus(BRIGHTNESS_SYSFS_NODE, {std::to_string(mPendingBl)}, vsyncNs * 5);
+        checkSysfsStatus(GetPanelSysfileByIndex(BRIGHTNESS_SYSFS_NODE),
+                         {std::to_string(mPendingBl)}, vsyncNs * 5);
         mUncheckedBlRequest = false;
     }
 
@@ -704,45 +706,39 @@ int BrightnessController::queryBrightness(float brightness, bool *ghbm, uint32_t
 }
 
 // Return immediately if it's already in the status. Otherwise poll the status
-int BrightnessController::checkSysfsStatus(const char* file,
+int BrightnessController::checkSysfsStatus(const std::string& file,
                                            const std::vector<std::string>& expectedValue,
                                            const nsecs_t timeoutNs) {
     ATRACE_CALL();
 
-    if (expectedValue.size() == 0) return false;
+    if (expectedValue.size() == 0) {
+      return -EINVAL;
+    }
 
     char buf[16];
-    String8 nodeName;
-    if (std::strstr(file, "%d")) {
-        nodeName.appendFormat(file, mPanelIndex);
-    } else if (std::strstr(file, "%s")) {
-        nodeName.appendFormat(file, mPanelIndex == 0 ? "primary"
-                                    : mPanelIndex == 1 ? "secondary" : "unknown");
-    } else {
-        nodeName = file;
-    }
-    UniqueFd fd = open(nodeName.string(), O_RDONLY);
+    UniqueFd fd = open(file.c_str(), O_RDONLY);
     if (fd.get() < 0) {
-        ALOGE("%s failed to open sysfs %s: %s", __func__, nodeName.c_str(), strerror(errno));
-        return false;
+        ALOGE("%s failed to open sysfs %s: %s", __func__, file.c_str(), strerror(errno));
+        return -ENOENT;
     }
 
     int size = read(fd.get(), buf, sizeof(buf));
     if (size <= 0) {
-        ALOGE("%s failed to read from %s: %s", __func__, nodeName.c_str(), strerror(errno));
-        return false;
+        ALOGE("%s failed to read from %s: %s", __func__, file.c_str(), strerror(errno));
+        return -EIO;
     }
 
     // '- 1' to remove trailing '\n'
     std::string val = std::string(buf, size - 1);
     if (std::find(expectedValue.begin(), expectedValue.end(), val) != expectedValue.end()) {
-        return true;
+        return OK;
     } else if (timeoutNs == 0) {
-        return false;
+        // not get the expected value and no intention to wait
+        return -EINVAL;
     }
 
     struct pollfd pfd;
-    int ret = EINVAL;
+    int ret = -EINVAL;
 
     auto startTime = systemTime(SYSTEM_TIME_MONOTONIC);
     pfd.fd = fd.get();
@@ -756,9 +752,9 @@ int BrightnessController::checkSysfsStatus(const char* file,
         }
         int pollRet = poll(&pfd, 1, ns2ms(remainTimeNs));
         if (pollRet == 0) {
-            ALOGW("%s poll %s timeout", __func__, nodeName.c_str());
+            ALOGW("%s poll %s timeout", __func__, file.c_str());
             // time out
-            ret = ETIMEDOUT;
+            ret = -ETIMEDOUT;
             break;
         } else if (pollRet > 0) {
             if (!(pfd.revents & POLLPRI)) {
@@ -781,12 +777,13 @@ int BrightnessController::checkSysfsStatus(const char* file,
                     if (values.size() > 0) {
                         values.resize(values.size() - 1);
                     }
-                    ALOGE("%s read %s expected %s after notified", __func__, val.c_str(),
-                          values.c_str());
+                    ALOGW("%s read %s expected %s after notified on file %s", __func__, val.c_str(),
+                          values.c_str(), file.c_str());
                 }
             } else {
-                ret = EIO;
-                ALOGE("%s failed to read after notified %d", __func__, errno);
+                ret = -EIO;
+                ALOGE("%s failed to read after notified %d on file %s", __func__, errno,
+                      file.c_str());
                 break;
             }
         } else {
@@ -794,13 +791,13 @@ int BrightnessController::checkSysfsStatus(const char* file,
                 continue;
             }
 
-            ALOGE("%s poll failed %d", __func__, errno);
-            ret = errno;
+            ALOGE("%s poll failed %d on file %s", __func__, errno, file.c_str());
+            ret = -errno;
             break;
         }
     };
 
-    return ret == NO_ERROR;
+    return ret;
 }
 
 void BrightnessController::resetLhbmState() {
diff --git a/libhwc2.1/libdevice/BrightnessController.h b/libhwc2.1/libdevice/BrightnessController.h
index 8ce7c4d..30cd899 100644
--- a/libhwc2.1/libdevice/BrightnessController.h
+++ b/libhwc2.1/libdevice/BrightnessController.h
@@ -114,7 +114,7 @@ public:
         std::lock_guard<std::recursive_mutex> lock(mBrightnessMutex);
         return mLhbm.get();
     }
-    int checkSysfsStatus(const char *file, const std::vector<std::string>& expectedValue,
+    int checkSysfsStatus(const std::string& file, const std::vector<std::string>& expectedValue,
                          const nsecs_t timeoutNs);
     void resetLhbmState();
 
@@ -143,6 +143,21 @@ public:
 
     int updateCabcMode();
 
+    const std::string GetPanelSysfileByIndex(const char *file_pattern) {
+        String8 nodeName;
+        nodeName.appendFormat(file_pattern, mPanelIndex);
+        return nodeName.c_str();
+    }
+
+    const std::string GetPanelRefreshRateSysfile() {
+        String8 nodeName;
+        nodeName.appendFormat(kRefreshrateFileNode,
+                              mPanelIndex == 0           ? "primary"
+                                      : mPanelIndex == 1 ? "secondary"
+                                                         : "unknown");
+        return nodeName.c_str();
+    }
+
     struct BrightnessTable {
         float mBriStart;
         float mBriEnd;
diff --git a/libhwc2.1/libmaindisplay/ExynosPrimaryDisplay.cpp b/libhwc2.1/libmaindisplay/ExynosPrimaryDisplay.cpp
index 619e55d..7078b83 100644
--- a/libhwc2.1/libmaindisplay/ExynosPrimaryDisplay.cpp
+++ b/libhwc2.1/libmaindisplay/ExynosPrimaryDisplay.cpp
@@ -528,6 +528,7 @@ bool ExynosPrimaryDisplay::isLhbmSupported() {
 // This function should be called by other threads (e.g. sensor HAL).
 // HWCService can call this function but it should be for test purpose only.
 int32_t ExynosPrimaryDisplay::setLhbmState(bool enabled) {
+    int ret;
     // NOTE: mLhbmOn could be set to false at any time by setPowerOff in another
     // thread. Make sure no side effect if that happens. Or add lock if we have
     // to when new code is added.
@@ -548,12 +549,12 @@ int32_t ExynosPrimaryDisplay::setLhbmState(bool enabled) {
 
     float peak_rr = getPeakRefreshRate();
     if (enabled && peak_rr > 0) {
-        ATRACE_NAME("wait for peak refresh rate");
-        bool succ = mBrightnessController->checkSysfsStatus(
-                                                BrightnessController::kRefreshrateFileNode,
+        ATRACE_NAME("wait for peak rr sent");
+        auto rrSysfs = mBrightnessController->GetPanelRefreshRateSysfile();
+        ret = mBrightnessController->checkSysfsStatus(rrSysfs,
                                                 {std::to_string(std::lround(peak_rr))},
                                                 ms2ns(kLhbmWaitForPeakRefreshRateMs));
-        if (!succ) {
+        if (ret != OK) {
             ALOGE("%s: check refresh rate sysfs node failed", __func__);
             return -EINVAL;
         }
@@ -563,12 +564,13 @@ int32_t ExynosPrimaryDisplay::setLhbmState(bool enabled) {
         setLHBMRefreshRateThrottle(kLhbmRefreshRateThrottleMs);
     }
 
-    bool wasDisabled =
-            mBrightnessController
-                    ->checkSysfsStatus(BrightnessController::kLocalHbmModeFileNode,
-                                       {std::to_string(static_cast<int>(
-                                               BrightnessController::LhbmMode::DISABLED))},
-                                       0);
+    auto lhbmSysfs = mBrightnessController->GetPanelSysfileByIndex(
+            BrightnessController::kLocalHbmModeFileNode);
+    ret = mBrightnessController->checkSysfsStatus(lhbmSysfs,
+                                         {std::to_string(static_cast<int>(
+                                                 BrightnessController::LhbmMode::DISABLED))},
+                                         0);
+    bool wasDisabled = ret == OK;
     if (!enabled && wasDisabled) {
         ALOGW("lhbm is at DISABLED state, skip disabling");
         return NO_ERROR;
@@ -589,10 +591,9 @@ int32_t ExynosPrimaryDisplay::setLhbmState(bool enabled) {
     requestLhbm(enabled);
     constexpr uint32_t kSysfsCheckTimeoutMs = 500;
     ALOGI("setLhbmState =%d", enabled);
-    bool succeed =
-            mBrightnessController->checkSysfsStatus(BrightnessController::kLocalHbmModeFileNode,
-                                                    checkingValue, ms2ns(kSysfsCheckTimeoutMs));
-    if (!succeed) {
+    ret = mBrightnessController->checkSysfsStatus(lhbmSysfs, checkingValue,
+                                                  ms2ns(kSysfsCheckTimeoutMs));
+    if (ret != OK) {
         ALOGE("failed to update lhbm mode");
         if (enabled) {
             setLHBMRefreshRateThrottle(0);
@@ -605,11 +606,11 @@ int32_t ExynosPrimaryDisplay::setLhbmState(bool enabled) {
         bool enablingStateSupported = !mFramesToReachLhbmPeakBrightness;
         if (enablingStateSupported) {
             ATRACE_NAME("lhbm_wait_peak_brightness");
-            if (!mBrightnessController
-                         ->checkSysfsStatus(BrightnessController::kLocalHbmModeFileNode,
+            ret = mBrightnessController->checkSysfsStatus(lhbmSysfs,
                                             {std::to_string(static_cast<int>(
                                                     BrightnessController::LhbmMode::ENABLED))},
-                                            ms2ns(kSysfsCheckTimeoutMs))) {
+                                            ms2ns(kSysfsCheckTimeoutMs));
+            if (ret != OK) {
                 ALOGE("failed to wait for lhbm becoming effective");
                 return -EIO;
             }