gpu_info_util: address comments for CL 438940

BUILD.gn

@@ -171,7 +171,10 @@ static_library("angle_image_util") {
   sources = rebase_path(gles_gypi.libangle_image_util_sources, ".", "src")
 
   configs -= angle_undefine_configs
-  configs += [ ":internal_config" ]
+  configs += [
+    ":internal_config",
+    ":extra_warnings",
+  ]
 
   public_configs = [ ":angle_image_util_config" ]
 
@@ -189,7 +192,10 @@ config("angle_gpu_info_util_config") {
 
 static_library("angle_gpu_info_util") {
   configs -= angle_undefine_configs
-  configs += [ ":internal_config" ]
+  configs += [
+    ":internal_config",
+    ":extra_warnings",
+  ]
 
   public_configs = [ ":angle_gpu_info_util_config" ]
 
@@ -207,9 +213,8 @@ static_library("angle_gpu_info_util") {
         rebase_path(gles_gypi.libangle_gpu_info_util_linux_sources, ".", "src")
 
     if (use_x11) {
-      sources += rebase_path(gles_gypi.libangle_gpu_info_util_x11_sources,
-                             ".",
-                             "src")
+      sources +=
+          rebase_path(gles_gypi.libangle_gpu_info_util_x11_sources, ".", "src")
       deps += [ "src/third_party/libXNVCtrl:libXNVCtrl" ]
       defines += [ "GPU_INFO_USE_X11" ]
       libs += [
@@ -369,8 +374,8 @@ static_library("libANGLE") {
     ":angle_common",
   ]
   deps = [
-    ":angle_image_util",
     ":angle_gpu_info_util",
+    ":angle_image_util",
     ":commit_id",
     ":includes",
     ":translator",

src/angle.gyp

@@ -237,7 +237,8 @@
                     [
                         '<(angle_path)/src/third_party/libXNVCtrl/libXNVCtrl.gyp:libXNVCtrl',
                     ],
-                    'link_settings': {
+                    'link_settings':
+                    {
                         'ldflags':
                         [
                             '<!@(<(pkg-config) --libs-only-L --libs-only-other x11 xi xext)',
@@ -258,7 +259,8 @@
                     [
                         'GPU_INFO_USE_LIBPCI',
                     ],
-                    'link_settings': {
+                    'link_settings':
+                    {
                         'ldflags':
                         [
                             '<!@(<(pkg-config) --libs-only-L --libs-only-other libpci)',

src/gpu_info_util/SystemInfo.cpp

@@ -14,24 +14,24 @@
 namespace angle
 {
 
-bool IsAMD(uint32_t vendorId)
+bool IsAMD(VendorID vendorId)
 {
-    return vendorId == VENDOR_ID_AMD;
+    return vendorId == kVendorID_AMD;
 }
 
-bool IsIntel(uint32_t vendorId)
+bool IsIntel(VendorID vendorId)
 {
-    return vendorId == VENDOR_ID_INTEL;
+    return vendorId == kVendorID_Intel;
 }
 
-bool IsNvidia(uint32_t vendorId)
+bool IsNvidia(VendorID vendorId)
 {
-    return vendorId == VENDOR_ID_NVIDIA;
+    return vendorId == kVendorID_Nvidia;
 }
 
-bool IsQualcomm(uint32_t vendorId)
+bool IsQualcomm(VendorID vendorId)
 {
-    return vendorId == VENDOR_ID_QUALCOMM;
+    return vendorId == kVendorID_Qualcomm;
 }
 
 bool ParseAMDBrahmaDriverVersion(const std::string &content, std::string *version)

src/gpu_info_util/SystemInfo.h

@@ -16,10 +16,18 @@
 namespace angle
 {
 
+using VendorID = uint32_t;
+using DeviceID = uint32_t;
+
+constexpr VendorID kVendorID_AMD      = 0x1002;
+constexpr VendorID kVendorID_Intel    = 0x8086;
+constexpr VendorID kVendorID_Nvidia   = 0x10DE;
+constexpr VendorID kVendorID_Qualcomm = 0x5143;
+
 struct GPUDeviceInfo
 {
-    uint32_t vendorId;
-    uint32_t deviceId;
+    VendorID vendorId;
+    DeviceID deviceId;
 
     std::string driverVendor;
     std::string driverVersion;
@@ -39,21 +47,10 @@ struct SystemInfo
 
 bool GetSystemInfo(SystemInfo *info);
 
-enum VendorID : uint32_t
-{
-    VENDOR_ID_UNKNOWN = 0x0,
-    VENDOR_ID_AMD     = 0x1002,
-    VENDOR_ID_INTEL   = 0x8086,
-    VENDOR_ID_NVIDIA  = 0x10DE,
-    // This is Qualcomm PCI Vendor ID.
-    // Android doesn't have a PCI bus, but all we need is a unique id.
-    VENDOR_ID_QUALCOMM = 0x5143,
-};
-
-bool IsAMD(uint32_t vendorId);
-bool IsIntel(uint32_t vendorId);
-bool IsNvidia(uint32_t vendorId);
-bool IsQualcomm(uint32_t vendorId);
+bool IsAMD(VendorID vendorId);
+bool IsIntel(VendorID vendorId);
+bool IsNvidia(VendorID vendorId);
+bool IsQualcomm(VendorID vendorId);
 
 }  // namespace angle
 

src/gpu_info_util/SystemInfo_libpci.cpp

@@ -27,13 +27,13 @@ namespace
 
 struct LibPCI : angle::NonCopyable
 {
-    static bool IsSupported()
+    LibPCI()
     {
-        return access("/sys/bus/pci/", F_OK) == 0 || access("/sys/bs/pci_express/", F_OK) == 0;
-    }
+        if (access("/sys/bus/pci/", F_OK) != 0 && access("/sys/bs/pci_express/", F_OK) != 0)
+        {
+            return;
+        }
 
-    bool Load()
-    {
         mHandle = dlopen("libpci.so.3", RTLD_LAZY);
 
         if (mHandle == nullptr)
@@ -43,22 +43,24 @@ struct LibPCI : angle::NonCopyable
 
         if (mHandle == nullptr)
         {
-            return false;
+            return;
         }
 
-        return (Alloc = reinterpret_cast<decltype(Alloc)>(dlsym(mHandle, "pci_alloc"))) !=
-                   nullptr &&
-               (Init = reinterpret_cast<decltype(Init)>(dlsym(mHandle, "pci_init"))) != nullptr &&
-               (Cleanup = reinterpret_cast<decltype(Cleanup)>(dlsym(mHandle, "pci_cleanup"))) !=
-                   nullptr &&
-               (ScanBus = reinterpret_cast<decltype(ScanBus)>(dlsym(mHandle, "pci_scan_bus"))) !=
-                   nullptr &&
-               (FillInfo = reinterpret_cast<decltype(FillInfo)>(dlsym(mHandle, "pci_fill_info"))) !=
-                   nullptr &&
-               (LookupName = reinterpret_cast<decltype(LookupName)>(
-                    dlsym(mHandle, "pci_lookup_name"))) != nullptr;
+        mValid =
+            (Alloc = reinterpret_cast<decltype(Alloc)>(dlsym(mHandle, "pci_alloc"))) != nullptr &&
+            (Init = reinterpret_cast<decltype(Init)>(dlsym(mHandle, "pci_init"))) != nullptr &&
+            (Cleanup = reinterpret_cast<decltype(Cleanup)>(dlsym(mHandle, "pci_cleanup"))) !=
+                nullptr &&
+            (ScanBus = reinterpret_cast<decltype(ScanBus)>(dlsym(mHandle, "pci_scan_bus"))) !=
+                nullptr &&
+            (FillInfo = reinterpret_cast<decltype(FillInfo)>(dlsym(mHandle, "pci_fill_info"))) !=
+                nullptr &&
+            (LookupName = reinterpret_cast<decltype(LookupName)>(
+                 dlsym(mHandle, "pci_lookup_name"))) != nullptr;
     }
 
+    bool IsValid() const { return mValid; }
+
     ~LibPCI()
     {
         if (mHandle != nullptr)
@@ -76,6 +78,7 @@ struct LibPCI : angle::NonCopyable
 
   private:
     void *mHandle = nullptr;
+    bool mValid   = false;
 };
 
 }  // anonymous namespace
@@ -83,13 +86,8 @@ struct LibPCI : angle::NonCopyable
 // Adds an entry per PCI GPU found and fills the device and vendor ID.
 bool GetPCIDevicesWithLibPCI(std::vector<GPUDeviceInfo> *devices)
 {
-    if (!LibPCI::IsSupported())
-    {
-        return false;
-    }
-
     LibPCI pci;
-    if (!pci.Load())
+    if (!pci.IsValid())
     {
         return false;
     }

src/gpu_info_util/SystemInfo_linux.cpp

@@ -106,7 +106,7 @@ bool GetSystemInfo(SystemInfo *info)
         GPUDeviceInfo *gpu = &info->gpus[i];
 
         // New GPUs might be added inside this loop, don't query for their driver version again
-        if (gpu->driverVendor.empty())
+        if (!gpu->driverVendor.empty())
         {
             continue;
         }
@@ -116,12 +116,12 @@ bool GetSystemInfo(SystemInfo *info)
             std::string version;
             if (GetAMDBrahmaDriverVersion(&version))
             {
-                gpu->driverVendor  = "ATI / AMD (Brahma)";
+                gpu->driverVendor  = "AMD (Brahma)";
                 gpu->driverVersion = std::move(version);
             }
             else if (GetAMDCatalystDriverVersion(&version))
             {
-                gpu->driverVendor  = "ATI / AMD (Catalyst)";
+                gpu->driverVendor  = "AMD (Catalyst)";
                 gpu->driverVersion = std::move(version);
             }
         }
@@ -145,7 +145,7 @@ bool GetSystemInfo(SystemInfo *info)
             if (GetNvidiaDriverVersionWithXNVCtrl(&version))
             {
                 GPUDeviceInfo nvidiaInfo;
-                nvidiaInfo.vendorId = VENDOR_ID_NVIDIA;
+                nvidiaInfo.vendorId = kVendorID_Nvidia;
                 nvidiaInfo.deviceId = 0;
                 gpu->driverVendor   = "Nvidia";
                 gpu->driverVersion  = std::move(version);