From 4d6169ee8b27df75e4ca374d369690cd9e87f137 Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Wed, 8 Jun 2022 12:43:51 +0000 Subject: [PATCH] Extract querying device id and revision to a dedicated method Related-To: NEO-6999 Signed-off-by: Mateusz Jablonski --- .../unit_test/linux/drm_null_device_tests.cpp | 10 +++---- opencl/test/unit_test/linux/drm_wrap.h | 11 ++++++-- .../test/unit_test/linux/main_linux_dll.cpp | 6 +--- shared/source/dll/linux/drm_neo_create.cpp | 14 ++-------- shared/source/os_interface/linux/drm_neo.cpp | 22 +++++++++------ shared/source/os_interface/linux/drm_neo.h | 4 +-- .../source/os_interface/linux/drm_version.cpp | 4 +++ shared/test/common/libult/linux/drm_mock.h | 6 ++-- .../os_interface/linux/drm_tests.cpp | 28 ++++--------------- 9 files changed, 46 insertions(+), 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 index 54fe90c31f..6580c69f01 100644 --- 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 drmNullDevice; + std::unique_ptr drmNullDevice; ExecutionEnvironment executionEnvironment; protected: @@ -48,10 +48,10 @@ class DrmNullDeviceTestsFixture { typedef Test 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 index 1143c08609..c6b4678ffb 100644 --- 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 createDrm(RootDeviceEnvironment &rootDeviceEnvironment) { + static std::unique_ptr createDrm(RootDeviceEnvironment &rootDeviceEnvironment) { auto hwDeviceIds = OSInterface::discoverDevices(rootDeviceEnvironment.executionEnvironment); if (!hwDeviceIds.empty()) { - return std::unique_ptr{NEO::Drm::create(std::unique_ptr(hwDeviceIds[0].release()->as()), rootDeviceEnvironment)}; + return std::unique_ptr{static_cast(NEO::Drm::create(std::unique_ptr(hwDeviceIds[0].release()->as()), 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 index 5c168ce9bd..8ce7830d38 100644 --- 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::max(); constexpr long long initialMax = std::numeric_limits::min(); auto executionEnvironment = std::make_unique(); executionEnvironment->prepareRootDeviceEnvironments(1); - auto drm = static_cast(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 index 7bb5ddabfa..29d0176e2f 100644 --- 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 &&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 &&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 index a3c31624f6..fc4dbe6aac 100644 --- 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 index d275017efc..dae30b6c6c 100644 --- 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 index 5a97d68c4f..8e21573a31 100644 --- 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 index bd09b0848b..5bbbdefcc7 100644 --- 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(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 index 008d56f3f2..bf77aded2d 100644 --- 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->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->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_EQ(pDrm->storedDeviceID, deviceID); - EXPECT_EQ(pDrm->storedDeviceRevID, revID); + EXPECT_TRUE(pDrm->queryDeviceIdAndRevision()); + + EXPECT_EQ(pDrm->storedDeviceID, pDrm->deviceId); + EXPECT_EQ(pDrm->storedDeviceRevID, pDrm->revisionId); delete pDrm; }