From dc84b163b51370ebb2fe8d352453adc6214792a5 Mon Sep 17 00:00:00 2001 From: Compute-Runtime-Validation Date: Tue, 3 Sep 2024 08:04:37 +0200 Subject: [PATCH] Revert "performance: Optimize heap handling when mitigate dc flush" This reverts commit 9249c5c65c23a4af26bcbd3cf748d20c2034700f. Signed-off-by: Compute-Runtime-Validation --- shared/source/command_container/cmdcontainer.cpp | 3 --- .../command_stream/command_stream_receiver.cpp | 4 ---- .../source/gmm_helper/cache_settings_helper.cpp | 2 +- shared/source/os_interface/product_helper.h | 4 +--- shared/source/os_interface/product_helper.inl | 16 +++------------- shared/source/os_interface/product_helper_hw.h | 4 +--- .../xe2_hpg_core/windows/product_helper_lnl.cpp | 6 +----- shared/test/common/mocks/mock_product_helper.cpp | 12 +----------- .../os_interface/product_helper_tests.cpp | 8 ++++---- .../lnl/product_helper_tests_lnl.cpp | 2 ++ .../windows/product_helper_tests_lnl_windows.cpp | 6 ------ 11 files changed, 14 insertions(+), 53 deletions(-) diff --git a/shared/source/command_container/cmdcontainer.cpp b/shared/source/command_container/cmdcontainer.cpp index 3814dad31c..0c4abc5171 100644 --- a/shared/source/command_container/cmdcontainer.cpp +++ b/shared/source/command_container/cmdcontainer.cpp @@ -570,9 +570,6 @@ void CommandContainer::storeAllocationAndFlushTagUpdate(GraphicsAllocation *allo } else { getHeapHelper()->storeHeapAllocation(allocation); } - if (device->getProductHelper().isDcFlushMitigated()) { - this->immediateCmdListCsr->registerDcFlushForDcMitigation(); - } this->immediateCmdListCsr->flushTagUpdate(); } diff --git a/shared/source/command_stream/command_stream_receiver.cpp b/shared/source/command_stream/command_stream_receiver.cpp index 53d486176a..5769412efb 100644 --- a/shared/source/command_stream/command_stream_receiver.cpp +++ b/shared/source/command_stream/command_stream_receiver.cpp @@ -696,10 +696,6 @@ IndirectHeap &CommandStreamReceiver::getIndirectHeap(IndirectHeap::Type heapType internalAllocationStorage->storeAllocation(std::unique_ptr(heapMemory), REUSABLE_ALLOCATION); heapMemory = nullptr; this->heapStorageRequiresRecyclingTag = true; - - if (this->peekRootDeviceEnvironment().getProductHelper().isDcFlushMitigated()) { - this->registerDcFlushForDcMitigation(); - } } if (!heapMemory) { diff --git a/shared/source/gmm_helper/cache_settings_helper.cpp b/shared/source/gmm_helper/cache_settings_helper.cpp index 41104030dc..0bd0efbe11 100644 --- a/shared/source/gmm_helper/cache_settings_helper.cpp +++ b/shared/source/gmm_helper/cache_settings_helper.cpp @@ -47,7 +47,7 @@ bool CacheSettingsHelper::preferNoCpuAccess(GMM_RESOURCE_USAGE_TYPE_ENUM gmmReso } GMM_RESOURCE_USAGE_TYPE_ENUM CacheSettingsHelper::getDefaultUsageTypeWithCachingEnabled(AllocationType allocationType, const ProductHelper &productHelper) { - if (productHelper.overrideUsageForDcFlushMitigation(allocationType)) { + if (productHelper.overridePatAndUsageForDcFlushMitigation(allocationType)) { return getDefaultUsageTypeWithCachingDisabled(allocationType, productHelper); } diff --git a/shared/source/os_interface/product_helper.h b/shared/source/os_interface/product_helper.h index 0f67b87a60..00dd82d8a5 100644 --- a/shared/source/os_interface/product_helper.h +++ b/shared/source/os_interface/product_helper.h @@ -135,9 +135,7 @@ class ProductHelper { virtual bool isDcFlushAllowed() const = 0; virtual bool isDcFlushMitigated() const = 0; virtual bool mitigateDcFlush() const = 0; - virtual bool overrideUsageForDcFlushMitigation(AllocationType allocationType) const = 0; - virtual bool overridePatToUCAndTwoWayCohForDcFlushMitigation(AllocationType allocationType) const = 0; - virtual bool overridePatToUCAndOneWayCohForDcFlushMitigation(AllocationType allocationType) const = 0; + virtual bool overridePatAndUsageForDcFlushMitigation(AllocationType allocationType) const = 0; virtual bool overrideCacheableForDcFlushMitigation(AllocationType allocationType) const = 0; virtual uint32_t computeMaxNeededSubSliceSpace(const HardwareInfo &hwInfo) const = 0; virtual bool getUuid(NEO::DriverModel *driverModel, const uint32_t subDeviceCount, const uint32_t deviceIndex, std::array &uuid) const = 0; diff --git a/shared/source/os_interface/product_helper.inl b/shared/source/os_interface/product_helper.inl index e04977ae57..6435fae1b5 100644 --- a/shared/source/os_interface/product_helper.inl +++ b/shared/source/os_interface/product_helper.inl @@ -416,12 +416,7 @@ bool ProductHelperHw::isDcFlushMitigated() const { } template -bool ProductHelperHw::overrideUsageForDcFlushMitigation(AllocationType allocationType) const { - return this->isDcFlushMitigated() && (this->overridePatToUCAndTwoWayCohForDcFlushMitigation(allocationType) || overridePatToUCAndOneWayCohForDcFlushMitigation(allocationType)); -} - -template -bool ProductHelperHw::overridePatToUCAndTwoWayCohForDcFlushMitigation(AllocationType allocationType) const { +bool ProductHelperHw::overridePatAndUsageForDcFlushMitigation(AllocationType allocationType) const { return this->isDcFlushMitigated() && (this->overrideCacheableForDcFlushMitigation(allocationType) || allocationType == AllocationType::timestampPacketTagBuffer || @@ -429,13 +424,6 @@ bool ProductHelperHw::overridePatToUCAndTwoWayCohForDcFlushMitigatio allocationType == AllocationType::gpuTimestampDeviceBuffer); } -template -bool ProductHelperHw::overridePatToUCAndOneWayCohForDcFlushMitigation(AllocationType allocationType) const { - return this->isDcFlushMitigated() && - (allocationType == AllocationType::internalHeap || - allocationType == AllocationType::linearStream); -} - template bool ProductHelperHw::overrideCacheableForDcFlushMitigation(AllocationType allocationType) const { return this->isDcFlushMitigated() && @@ -445,6 +433,8 @@ bool ProductHelperHw::overrideCacheableForDcFlushMitigation(Allocati allocationType == AllocationType::svmCpu || allocationType == AllocationType::svmZeroCopy || allocationType == AllocationType::internalHostMemory || + allocationType == AllocationType::internalHeap || + allocationType == AllocationType::linearStream || allocationType == AllocationType::printfSurface); } diff --git a/shared/source/os_interface/product_helper_hw.h b/shared/source/os_interface/product_helper_hw.h index 4dfabccd10..7b3260eb56 100644 --- a/shared/source/os_interface/product_helper_hw.h +++ b/shared/source/os_interface/product_helper_hw.h @@ -77,9 +77,7 @@ class ProductHelperHw : public ProductHelper { bool isDcFlushAllowed() const override; bool isDcFlushMitigated() const override; bool mitigateDcFlush() const override; - bool overrideUsageForDcFlushMitigation(AllocationType allocationType) const override; - bool overridePatToUCAndTwoWayCohForDcFlushMitigation(AllocationType allocationType) const override; - bool overridePatToUCAndOneWayCohForDcFlushMitigation(AllocationType allocationType) const override; + bool overridePatAndUsageForDcFlushMitigation(AllocationType allocationType) const override; bool overrideCacheableForDcFlushMitigation(AllocationType allocationType) const override; uint32_t computeMaxNeededSubSliceSpace(const HardwareInfo &hwInfo) const override; bool getUuid(NEO::DriverModel *driverModel, uint32_t subDeviceCount, uint32_t deviceIndex, std::array &uuid) const override; diff --git a/shared/source/xe2_hpg_core/windows/product_helper_lnl.cpp b/shared/source/xe2_hpg_core/windows/product_helper_lnl.cpp index 08c2c23c92..b9675e76f4 100644 --- a/shared/source/xe2_hpg_core/windows/product_helper_lnl.cpp +++ b/shared/source/xe2_hpg_core/windows/product_helper_lnl.cpp @@ -28,14 +28,10 @@ int ProductHelperHw::configureHardwareCustom(HardwareInfo *hwInfo, O template <> uint64_t ProductHelperHw::overridePatIndex(bool isUncachedType, uint64_t patIndex, AllocationType allocationType) const { - if (this->overridePatToUCAndTwoWayCohForDcFlushMitigation(allocationType)) { + if (this->overridePatAndUsageForDcFlushMitigation(allocationType)) { return 2; // L3: WB, L4: UC, 2-Way coh } - if (this->overridePatToUCAndOneWayCohForDcFlushMitigation(allocationType)) { - return 1; // L3: WB, L4: UC, 1-Way coh - } - return patIndex; } diff --git a/shared/test/common/mocks/mock_product_helper.cpp b/shared/test/common/mocks/mock_product_helper.cpp index 11b32ba923..82573105a1 100644 --- a/shared/test/common/mocks/mock_product_helper.cpp +++ b/shared/test/common/mocks/mock_product_helper.cpp @@ -292,17 +292,7 @@ bool ProductHelperHw::mitigateDcFlush() const { } template <> -bool ProductHelperHw::overridePatToUCAndTwoWayCohForDcFlushMitigation(AllocationType allocationType) const { - return false; -} - -template <> -bool ProductHelperHw::overrideUsageForDcFlushMitigation(AllocationType allocationType) const { - return false; -} - -template <> -bool ProductHelperHw::overridePatToUCAndOneWayCohForDcFlushMitigation(AllocationType allocationType) const { +bool ProductHelperHw::overridePatAndUsageForDcFlushMitigation(AllocationType allocationType) const { return false; } diff --git a/shared/test/unit_test/os_interface/product_helper_tests.cpp b/shared/test/unit_test/os_interface/product_helper_tests.cpp index c4600aaf47..ae40a94cde 100644 --- a/shared/test/unit_test/os_interface/product_helper_tests.cpp +++ b/shared/test/unit_test/os_interface/product_helper_tests.cpp @@ -311,12 +311,12 @@ HWTEST_F(ProductHelperTest, givenVariousValuesWhenGettingAubStreamSteppingFromHw EXPECT_EQ(AubMemDump::SteppingValues::A, mockProductHelper.getAubStreamSteppingFromHwRevId(pInHwInfo)); } -HWTEST_F(ProductHelperTest, givenDcFlushMitigationWhenOverridePatToUCAndTwoWayCohForDcFlushMitigationThenReturnCorrectValue) { +HWTEST_F(ProductHelperTest, givenDcFlushMitigationWhenOverridePatAndUsageForDcFlushMitigationThenReturnCorrectValue) { DebugManagerStateRestore restorer; if (!productHelper->isDcFlushMitigated()) { for (auto i = 0; i < static_cast(AllocationType::count); ++i) { auto allocationType = static_cast(i); - EXPECT_FALSE(productHelper->overridePatToUCAndTwoWayCohForDcFlushMitigation(allocationType)); + EXPECT_FALSE(productHelper->overridePatAndUsageForDcFlushMitigation(allocationType)); } } debugManager.flags.AllowDcFlush.set(0); @@ -334,9 +334,9 @@ HWTEST_F(ProductHelperTest, givenDcFlushMitigationWhenOverridePatToUCAndTwoWayCo allocationType == AllocationType::linearStream || allocationType == AllocationType::internalHeap || allocationType == AllocationType::printfSurface) { - EXPECT_EQ(productHelper->overrideUsageForDcFlushMitigation(allocationType), productHelper->isDcFlushMitigated()); + EXPECT_EQ(productHelper->overridePatAndUsageForDcFlushMitigation(allocationType), productHelper->isDcFlushMitigated()); } else { - EXPECT_FALSE(productHelper->overridePatToUCAndTwoWayCohForDcFlushMitigation(allocationType)); + EXPECT_FALSE(productHelper->overridePatAndUsageForDcFlushMitigation(allocationType)); } } } diff --git a/shared/test/unit_test/xe2_hpg_core/lnl/product_helper_tests_lnl.cpp b/shared/test/unit_test/xe2_hpg_core/lnl/product_helper_tests_lnl.cpp index 719c4cd22b..c82bdfa6f2 100644 --- a/shared/test/unit_test/xe2_hpg_core/lnl/product_helper_tests_lnl.cpp +++ b/shared/test/unit_test/xe2_hpg_core/lnl/product_helper_tests_lnl.cpp @@ -146,6 +146,8 @@ LNLTEST_F(LnlProductHelper, givenExternalHostPtrWhenMitigateDcFlushThenOverrideC allocationType == AllocationType::svmZeroCopy || allocationType == AllocationType::internalHostMemory || allocationType == AllocationType::commandBuffer || + allocationType == AllocationType::internalHeap || + allocationType == AllocationType::linearStream || allocationType == AllocationType::printfSurface) { EXPECT_TRUE(productHelper->overrideAllocationCacheable(allocationData)); } else { diff --git a/shared/test/unit_test/xe2_hpg_core/lnl/windows/product_helper_tests_lnl_windows.cpp b/shared/test/unit_test/xe2_hpg_core/lnl/windows/product_helper_tests_lnl_windows.cpp index d8633ab85e..2792e1e42a 100644 --- a/shared/test/unit_test/xe2_hpg_core/lnl/windows/product_helper_tests_lnl_windows.cpp +++ b/shared/test/unit_test/xe2_hpg_core/lnl/windows/product_helper_tests_lnl_windows.cpp @@ -40,8 +40,6 @@ LNLTEST_F(LnlProductHelperWindows, givenProductHelperWhenOverridePatIndexCalledT EXPECT_EQ(expectedPatIndex, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::internalHostMemory)); EXPECT_EQ(expectedPatIndex, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::timestampPacketTagBuffer)); EXPECT_EQ(expectedPatIndex, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::tagBuffer)); - EXPECT_EQ(expectedPatIndex, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::linearStream)); - EXPECT_EQ(expectedPatIndex, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::internalHeap)); debugManager.flags.AllowDcFlush.set(0); @@ -53,10 +51,6 @@ LNLTEST_F(LnlProductHelperWindows, givenProductHelperWhenOverridePatIndexCalledT EXPECT_EQ(expectedPatIndexOverride, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::internalHostMemory)); EXPECT_EQ(expectedPatIndexOverride, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::timestampPacketTagBuffer)); EXPECT_EQ(expectedPatIndexOverride, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::tagBuffer)); - - expectedPatIndexOverride = 1u; - EXPECT_EQ(expectedPatIndexOverride, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::linearStream)); - EXPECT_EQ(expectedPatIndexOverride, productHelper->overridePatIndex(0u, expectedPatIndex, AllocationType::internalHeap)); } LNLTEST_F(LnlProductHelperWindows, givenProductHelperWhenIsStagingBuffersEnabledThenTrueIsReturned) {