Extract querying device id and revision to a dedicated method

Related-To: NEO-6999 Signed-off-by: Mateusz Jablonski <[email protected]>
intel · Jun 8, 2022 · 4d6169e · 4d6169e
1 parent 39c1c4d
commit 4d6169e
Show file tree

Hide file tree

Showing 9 changed files with 46 additions and 59 deletions.
diff --git a/opencl/test/unit_test/linux/drm_null_device_tests.cpp b/opencl/test/unit_test/linux/drm_null_device_tests.cpp
@@ -38,7 +38,7 @@ class DrmNullDeviceTestsFixture {
     void TearDown() { // NOLINT(readability-identifier-naming)
     }
 
-    std::unique_ptr<Drm> drmNullDevice;
+    std::unique_ptr<DrmWrap> drmNullDevice;
     ExecutionEnvironment executionEnvironment;
 
   protected:
@@ -48,10 +48,10 @@ class DrmNullDeviceTestsFixture {
 typedef Test<DrmNullDeviceTestsFixture> DrmNullDeviceTests;
 
 TEST_F(DrmNullDeviceTests, GIVENdrmNullDeviceWHENcallGetDeviceIdTHENreturnProperDeviceId) {
-    int deviceIdQueried = 0;
-    int ret = drmNullDevice->getDeviceID(deviceIdQueried);
-    EXPECT_EQ(0, ret);
-    EXPECT_EQ(deviceId, deviceIdQueried);
+    int ret = drmNullDevice->queryDeviceIdAndRevision();
+    EXPECT_TRUE(ret);
+    EXPECT_EQ(deviceId, drmNullDevice->deviceId);
+    EXPECT_EQ(revisionId, drmNullDevice->revisionId);
 }
 
 TEST_F(DrmNullDeviceTests, GIVENdrmNullDeviceWHENcallIoctlTHENalwaysSuccess) {

diff --git a/opencl/test/unit_test/linux/drm_wrap.h b/opencl/test/unit_test/linux/drm_wrap.h
@@ -14,13 +14,18 @@
 
 class DrmWrap : public NEO::Drm {
   public:
+    using Drm::deviceId;
+    using Drm::ioctlStatistics;
+    using Drm::queryDeviceIdAndRevision;
+    using Drm::revisionId;
     using Drm::virtualMemoryIds;
-
-    static std::unique_ptr<NEO::Drm> createDrm(RootDeviceEnvironment &rootDeviceEnvironment) {
+    static std::unique_ptr<DrmWrap> createDrm(RootDeviceEnvironment &rootDeviceEnvironment) {
         auto hwDeviceIds = OSInterface::discoverDevices(rootDeviceEnvironment.executionEnvironment);
         if (!hwDeviceIds.empty()) {
-            return std::unique_ptr<Drm>{NEO::Drm::create(std::unique_ptr<HwDeviceIdDrm>(hwDeviceIds[0].release()->as<HwDeviceIdDrm>()), rootDeviceEnvironment)};
+            return std::unique_ptr<DrmWrap>{static_cast<DrmWrap *>(NEO::Drm::create(std::unique_ptr<HwDeviceIdDrm>(hwDeviceIds[0].release()->as<HwDeviceIdDrm>()), rootDeviceEnvironment))};
         }
         return nullptr;
     }
 };
+
+static_assert(sizeof(DrmWrap) == sizeof(NEO::Drm));
diff --git a/opencl/test/unit_test/linux/main_linux_dll.cpp b/opencl/test/unit_test/linux/main_linux_dll.cpp
@@ -254,16 +254,12 @@ TEST_F(DrmFailedIoctlTests, givenPrintIoctlEntriesWhenCallFailedIoctlThenExpecte
 }
 
 TEST_F(DrmSimpleTests, givenPrintIoctlTimesWhenCallIoctlThenStatisticsAreGathered) {
-    struct DrmMock : public Drm {
-        using Drm::ioctlStatistics;
-    };
-
     constexpr long long initialMin = std::numeric_limits<long long>::max();
     constexpr long long initialMax = std::numeric_limits<long long>::min();
 
     auto executionEnvironment = std::make_unique<ExecutionEnvironment>();
     executionEnvironment->prepareRootDeviceEnvironments(1);
-    auto drm = static_cast<DrmMock *>(DrmWrap::createDrm(*executionEnvironment->rootDeviceEnvironments[0]).release());
+    auto drm = DrmWrap::createDrm(*executionEnvironment->rootDeviceEnvironments[0]).release();
 
     DebugManagerStateRestore restorer;
     DebugManager.flags.PrintIoctlTimes.set(true);

diff --git a/shared/source/dll/linux/drm_neo_create.cpp b/shared/source/dll/linux/drm_neo_create.cpp
@@ -39,20 +39,11 @@ Drm *Drm::create(std::unique_ptr<HwDeviceIdDrm> &&hwDeviceId, RootDeviceEnvironm
         drmObject.reset(new Drm(std::move(hwDeviceId), rootDeviceEnvironment));
     }
 
-    // Get HW version (I915_drm.h)
-    int ret = drmObject->getDeviceID(drmObject->deviceId);
-    if (ret != 0) {
-        printDebugString(DebugManager.flags.PrintDebugMessages.get(), stderr, "%s", "FATAL: Cannot query device ID parameter!\n");
-        return nullptr;
-    }
-    if (!DeviceFactory::isAllowedDeviceId(drmObject->deviceId, DebugManager.flags.FilterDeviceId.get())) {
+    if (!drmObject->queryDeviceIdAndRevision()) {
         return nullptr;
     }
 
-    // Get HW Revision (I915_drm.h)
-    ret = drmObject->getDeviceRevID(drmObject->revisionId);
-    if (ret != 0) {
-        printDebugString(DebugManager.flags.PrintDebugMessages.get(), stderr, "%s", "FATAL: Cannot query device Rev ID parameter!\n");
+    if (!DeviceFactory::isAllowedDeviceId(drmObject->deviceId, DebugManager.flags.FilterDeviceId.get())) {
         return nullptr;
     }
 
@@ -65,6 +56,7 @@ Drm *Drm::create(std::unique_ptr<HwDeviceIdDrm> &&hwDeviceId, RootDeviceEnvironm
             break;
         }
     }
+    int ret = 0;
     if (device) {
         ret = drmObject->setupHardwareInfo(device, true);
         if (ret != 0) {

diff --git a/shared/source/os_interface/linux/drm_neo.cpp b/shared/source/os_interface/linux/drm_neo.cpp
@@ -127,18 +127,24 @@ int Drm::getParamIoctl(DrmParam param, int *dstValue) {
     return retVal;
 }
 
-int Drm::getDeviceID(int &devId) {
-    return getParamIoctl(DrmParam::ParamChipsetId, &devId);
-}
-
-int Drm::getDeviceRevID(int &revId) {
-    return getParamIoctl(DrmParam::ParamRevision, &revId);
-}
-
 int Drm::getExecSoftPin(int &execSoftPin) {
     return getParamIoctl(DrmParam::ParamHasExecSoftpin, &execSoftPin);
 }
 
+bool Drm::queryI915DeviceIdAndRevision() {
+    auto ret = getParamIoctl(DrmParam::ParamChipsetId, &this->deviceId);
+    if (ret != 0) {
+        printDebugString(DebugManager.flags.PrintDebugMessages.get(), stderr, "%s", "FATAL: Cannot query device ID parameter!\n");
+        return false;
+    }
+    ret = getParamIoctl(DrmParam::ParamRevision, &this->revisionId);
+    if (ret != 0) {
+        printDebugString(DebugManager.flags.PrintDebugMessages.get(), stderr, "%s", "FATAL: Cannot query device Rev ID parameter!\n");
+        return false;
+    }
+    return true;
+}
+
 int Drm::enableTurboBoost() {
     GemContextParam contextParam = {};
 

diff --git a/shared/source/os_interface/linux/drm_neo.h b/shared/source/os_interface/linux/drm_neo.h
@@ -82,12 +82,10 @@ class Drm : public DriverModel {
 
     virtual int ioctl(DrmIoctl request, void *arg);
 
-    int getDeviceID(int &devId);
     unsigned int getDeviceHandle() const override {
         return 0;
     }
     PhyicalDevicePciSpeedInfo getPciSpeedInfo() const override;
-    int getDeviceRevID(int &revId);
     int getExecSoftPin(int &execSoftPin);
     int enableTurboBoost();
     int getEuTotal(int &euTotal);
@@ -266,6 +264,8 @@ class Drm : public DriverModel {
     void setupIoctlHelper(const PRODUCT_FAMILY productFamily);
     void queryAndSetVmBindPatIndexProgrammingSupport();
     static std::string getDrmVersion(int fileDescriptor);
+    bool queryDeviceIdAndRevision();
+    bool queryI915DeviceIdAndRevision();
 
 #pragma pack(1)
     struct PCIConfig {

diff --git a/shared/source/os_interface/linux/drm_version.cpp b/shared/source/os_interface/linux/drm_version.cpp
@@ -13,4 +13,8 @@ bool Drm::isDrmSupported(int fileDescriptor) {
     return "i915" == drmVersion;
 }
 
+bool Drm::queryDeviceIdAndRevision() {
+    return queryI915DeviceIdAndRevision();
+}
+
 } // namespace NEO
diff --git a/shared/test/common/libult/linux/drm_mock.h b/shared/test/common/libult/linux/drm_mock.h
@@ -25,7 +25,6 @@
 
 using namespace NEO;
 
-// Mock DRM class that responds to DRM_IOCTL_I915_GETPARAMs
 class DrmMock : public Drm {
   public:
     using Drm::bindAvailable;
@@ -34,6 +33,7 @@ class DrmMock : public Drm {
     using Drm::classHandles;
     using Drm::completionFenceSupported;
     using Drm::contextDebugSupported;
+    using Drm::deviceId;
     using Drm::engineInfo;
     using Drm::fenceVal;
     using Drm::generateElfUUID;
@@ -47,7 +47,9 @@ class DrmMock : public Drm {
     using Drm::preemptionSupported;
     using Drm::query;
     using Drm::queryAndSetVmBindPatIndexProgrammingSupport;
+    using Drm::queryDeviceIdAndRevision;
     using Drm::requirePerContextVM;
+    using Drm::revisionId;
     using Drm::setupIoctlHelper;
     using Drm::sliceCountChangeSupported;
     using Drm::systemInfo;
@@ -110,8 +112,6 @@ class DrmMock : public Drm {
         hwDeviceId = std::make_unique<HwDeviceIdDrm>(getFileDescriptor(), pciPath);
     }
 
-    void setDeviceID(int deviceId) { this->deviceId = deviceId; }
-    void setDeviceRevID(int revisionId) { this->revisionId = revisionId; }
     void setBindAvailable() {
         this->bindAvailable = true;
     }

diff --git a/shared/test/unit_test/os_interface/linux/drm_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_tests.cpp
@@ -34,20 +34,6 @@ std::string getLinuxDevicesPath(const char *file) {
     return resultString;
 }
 
-TEST(DrmTest, WhenGettingDeviceIdThenCorrectIdReturned) {
-    auto executionEnvironment = std::make_unique<ExecutionEnvironment>();
-    executionEnvironment->prepareRootDeviceEnvironments(1);
-    DrmMock *pDrm = new DrmMock(*executionEnvironment->rootDeviceEnvironments[0]);
-    EXPECT_NE(nullptr, pDrm);
-
-    pDrm->storedDeviceID = 0x1234;
-    int deviceID = 0;
-    int ret = pDrm->getDeviceID(deviceID);
-    EXPECT_EQ(0, ret);
-    EXPECT_EQ(pDrm->storedDeviceID, deviceID);
-    delete pDrm;
-}
-
 TEST(DrmTest, GivenValidPciPathWhenGettingAdapterBdfThenCorrectValuesAreReturned) {
     auto executionEnvironment = std::make_unique<ExecutionEnvironment>();
     executionEnvironment->prepareRootDeviceEnvironments(1);
@@ -124,15 +110,13 @@ TEST(DrmTest, WhenGettingRevisionIdThenCorrectIdIsReturned) {
 
     pDrm->storedDeviceID = 0x1234;
     pDrm->storedDeviceRevID = 0xB;
-    int deviceID = 0;
-    int ret = pDrm->getDeviceID(deviceID);
-    EXPECT_EQ(0, ret);
-    int revID = 0;
-    ret = pDrm->getDeviceRevID(revID);
-    EXPECT_EQ(0, ret);
+    pDrm->deviceId = 0;
+    pDrm->revisionId = 0;
+
+    EXPECT_TRUE(pDrm->queryDeviceIdAndRevision());
 
-    EXPECT_EQ(pDrm->storedDeviceID, deviceID);
-    EXPECT_EQ(pDrm->storedDeviceRevID, revID);
+    EXPECT_EQ(pDrm->storedDeviceID, pDrm->deviceId);
+    EXPECT_EQ(pDrm->storedDeviceRevID, pDrm->revisionId);
 
     delete pDrm;
 }