From 3e4dd67f096ff0171953e089db7ad8b4075a59ca Mon Sep 17 00:00:00 2001 From: "Dunajski, Bartosz" Date: Fri, 6 Sep 2019 10:25:14 +0200 Subject: [PATCH] Refactor linear/tiled Images logic Change-Id: I1deac70e95c6953645e9f52fb75f103b62927066 Signed-off-by: Dunajski, Bartosz --- Jenkinsfile | 2 +- runtime/gmm_helper/gmm_helper.cpp | 8 -- runtime/gmm_helper/gmm_helper.h | 1 - runtime/helpers/hw_helper.h | 3 + runtime/helpers/hw_helper_base.inl | 13 ++++ runtime/mem_obj/image.cpp | 8 +- .../mem_obj/create_image_aub_tests.cpp | 3 +- unit_tests/gmm_helper/gmm_helper_tests.cpp | 75 +------------------ unit_tests/helpers/hw_helper_tests.cpp | 40 ++++++++++ unit_tests/helpers/unit_test_helper.h | 2 + unit_tests/helpers/unit_test_helper.inl | 4 + unit_tests/mem_obj/image_tests.cpp | 3 + .../memory_manager/memory_manager_tests.cpp | 3 +- .../linux/drm_memory_manager_tests.cpp | 1 + 14 files changed, 77 insertions(+), 89 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index c7241a7598..29de5ef1c7 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,5 +1,5 @@ #!groovy dependenciesRevision='2cb93cf2512d68900934c6aeccf1ddad5a927902-1308' strategy='EQUAL' -allowedCD=260 +allowedCD=259 allowedF=5 diff --git a/runtime/gmm_helper/gmm_helper.cpp b/runtime/gmm_helper/gmm_helper.cpp index faefd340a5..5ba8bee30d 100644 --- a/runtime/gmm_helper/gmm_helper.cpp +++ b/runtime/gmm_helper/gmm_helper.cpp @@ -81,14 +81,6 @@ void GmmHelper::queryImgFromBufferParams(ImageInfo &imgInfo, GraphicsAllocation imgInfo.qPitch = 0; } -bool GmmHelper::allowTiling(const cl_image_desc &imageDesc) { - auto imageType = imageDesc.image_type; - auto buffer = castToObject(imageDesc.buffer); - - return (!(DebugManager.flags.ForceLinearImages.get() || imageType == CL_MEM_OBJECT_IMAGE1D || - imageType == CL_MEM_OBJECT_IMAGE1D_ARRAY || imageType == CL_MEM_OBJECT_IMAGE1D_BUFFER || buffer)); -} - uint64_t GmmHelper::canonize(uint64_t address) { return ((int64_t)((address & 0xFFFFFFFFFFFF) << (64 - 48))) >> (64 - 48); } diff --git a/runtime/gmm_helper/gmm_helper.h b/runtime/gmm_helper/gmm_helper.h index d1052bde7d..e16d999403 100644 --- a/runtime/gmm_helper/gmm_helper.h +++ b/runtime/gmm_helper/gmm_helper.h @@ -46,7 +46,6 @@ class GmmHelper { static void queryImgFromBufferParams(ImageInfo &imgInfo, GraphicsAllocation *gfxAlloc); static GMM_CUBE_FACE_ENUM getCubeFaceIndex(uint32_t target); - static bool allowTiling(const cl_image_desc &imageDesc); static uint32_t getRenderMultisamplesCount(uint32_t numSamples); static GMM_YUV_PLANE convertPlane(OCLPlane oclPlane); diff --git a/runtime/helpers/hw_helper.h b/runtime/helpers/hw_helper.h index 7a7d2ba9ab..8efb0b967b 100644 --- a/runtime/helpers/hw_helper.h +++ b/runtime/helpers/hw_helper.h @@ -68,6 +68,7 @@ class HwHelper { virtual uint32_t getMetricsLibraryGenId() const = 0; virtual uint32_t getMocsIndex(GmmHelper &gmmHelper, bool l3enabled, bool l1enabled) const = 0; virtual bool requiresAuxResolves() const = 0; + virtual bool tilingAllowed(bool isSharedContext, const cl_image_desc &imgDesc, bool forceLinearStorage) = 0; static constexpr uint32_t lowPriorityGpgpuEngineIndex = 1; @@ -167,6 +168,8 @@ class HwHelperHw : public HwHelper { bool requiresAuxResolves() const override; + bool tilingAllowed(bool isSharedContext, const cl_image_desc &imgDesc, bool forceLinearStorage) override; + protected: HwHelperHw() = default; }; diff --git a/runtime/helpers/hw_helper_base.inl b/runtime/helpers/hw_helper_base.inl index 38e1f71903..6d542938e1 100644 --- a/runtime/helpers/hw_helper_base.inl +++ b/runtime/helpers/hw_helper_base.inl @@ -220,4 +220,17 @@ template inline bool HwHelperHw::requiresAuxResolves() const { return true; } + +template +bool HwHelperHw::tilingAllowed(bool isSharedContext, const cl_image_desc &imgDesc, bool forceLinearStorage) { + if (DebugManager.flags.ForceLinearImages.get() || forceLinearStorage || isSharedContext) { + return false; + } + + auto imageType = imgDesc.image_type; + auto buffer = castToObject(imgDesc.buffer); + + return !(imageType == CL_MEM_OBJECT_IMAGE1D || imageType == CL_MEM_OBJECT_IMAGE1D_ARRAY || + imageType == CL_MEM_OBJECT_IMAGE1D_BUFFER || buffer); +} } // namespace NEO diff --git a/runtime/mem_obj/image.cpp b/runtime/mem_obj/image.cpp index 2fb4df8cc1..2cffcf9505 100644 --- a/runtime/mem_obj/image.cpp +++ b/runtime/mem_obj/image.cpp @@ -119,6 +119,7 @@ Image *Image::create(Context *context, MemoryManager *memoryManager = context->getMemoryManager(); Buffer *parentBuffer = castToObject(imageDesc->mem_object); Image *parentImage = castToObject(imageDesc->mem_object); + auto &hwHelper = HwHelper::get(context->getDevice(0)->getHardwareInfo().platform.eRenderCoreFamily); do { size_t imageWidth = imageDesc->image_width; @@ -181,7 +182,7 @@ Image *Image::create(Context *context, auto hostPtrRowPitch = imageDesc->image_row_pitch ? imageDesc->image_row_pitch : imageWidth * surfaceFormat->ImageElementSizeInBytes; auto hostPtrSlicePitch = imageDesc->image_slice_pitch ? imageDesc->image_slice_pitch : hostPtrRowPitch * imageHeight; MemoryPropertiesFlags memoryProperties = MemoryPropertiesFlagsParser::createMemoryPropertiesFlags(properties); - imgInfo.linearStorage = context->isSharedContext || !GmmHelper::allowTiling(*imageDesc) || memoryProperties.flags.forceLinearStorage; + imgInfo.linearStorage = !hwHelper.tilingAllowed(context->isSharedContext, *imageDesc, memoryProperties.flags.forceLinearStorage); imgInfo.preferRenderCompression = MemObjHelper::isSuitableForRenderCompression(!imgInfo.linearStorage, memoryProperties, context->peekContextType(), true); @@ -215,7 +216,7 @@ Image *Image::create(Context *context, bool transferNeeded = false; if (((imageDesc->image_type == CL_MEM_OBJECT_IMAGE1D_BUFFER) || (imageDesc->image_type == CL_MEM_OBJECT_IMAGE2D)) && (parentBuffer != nullptr)) { - HwHelper::get(context->getDevice(0)->getHardwareInfo().platform.eRenderCoreFamily).checkResourceCompatibility(parentBuffer, errcodeRet); + hwHelper.checkResourceCompatibility(parentBuffer, errcodeRet); if (errcodeRet != CL_SUCCESS) { return nullptr; @@ -710,7 +711,6 @@ bool Image::isCopyRequired(ImageInfo &imgInfo, const void *hostPtr) { auto hostPtrRowPitch = imgInfo.imgDesc->image_row_pitch ? imgInfo.imgDesc->image_row_pitch : imageWidth * imgInfo.surfaceFormat->ImageElementSizeInBytes; auto hostPtrSlicePitch = imgInfo.imgDesc->image_slice_pitch ? imgInfo.imgDesc->image_slice_pitch : hostPtrRowPitch * imgInfo.imgDesc->image_height; - auto isTilingAllowed = GmmHelper::allowTiling(*imgInfo.imgDesc) && !imgInfo.linearStorage; size_t pointerPassedSize = hostPtrRowPitch * imageHeight * imageDepth * imageCount; auto alignedSizePassedPointer = alignSizeWholePage(const_cast(hostPtr), pointerPassedSize); @@ -721,7 +721,7 @@ bool Image::isCopyRequired(ImageInfo &imgInfo, const void *hostPtr) { (imgInfo.rowPitch != hostPtrRowPitch) | (imgInfo.slicePitch != hostPtrSlicePitch) | ((reinterpret_cast(hostPtr) & (MemoryConstants::cacheLineSize - 1)) != 0) | - isTilingAllowed; + !imgInfo.linearStorage; return copyRequired; } diff --git a/unit_tests/aub_tests/mem_obj/create_image_aub_tests.cpp b/unit_tests/aub_tests/mem_obj/create_image_aub_tests.cpp index 14d12a2119..555b1d47e0 100644 --- a/unit_tests/aub_tests/mem_obj/create_image_aub_tests.cpp +++ b/unit_tests/aub_tests/mem_obj/create_image_aub_tests.cpp @@ -84,6 +84,7 @@ INSTANTIATE_TEST_CASE_P( testing::ValuesIn(ImgArrayTypes)); HWTEST_P(AUBCreateImageArray, CheckArrayImages) { + auto &hwHelper = HwHelper::get(pDevice->getExecutionEnvironment()->getHardwareInfo()->platform.eRenderCoreFamily); imageDesc.image_type = GetParam(); if (imageDesc.image_type == CL_MEM_OBJECT_IMAGE1D_ARRAY) { imageDesc.image_height = 1; @@ -91,7 +92,7 @@ HWTEST_P(AUBCreateImageArray, CheckArrayImages) { cl_mem_flags flags = CL_MEM_COPY_HOST_PTR; auto surfaceFormat = Image::getSurfaceFormatFromTable(flags, &imageFormat); auto imgInfo = MockGmm::initImgInfo(imageDesc, 0, surfaceFormat); - imgInfo.linearStorage = !GmmHelper::allowTiling(imageDesc); + imgInfo.linearStorage = !hwHelper.tilingAllowed(false, imageDesc, false); auto queryGmm = MockGmm::queryImgParams(imgInfo); //allocate host_ptr diff --git a/unit_tests/gmm_helper/gmm_helper_tests.cpp b/unit_tests/gmm_helper/gmm_helper_tests.cpp index df1f451376..5df4954a5b 100644 --- a/unit_tests/gmm_helper/gmm_helper_tests.cpp +++ b/unit_tests/gmm_helper/gmm_helper_tests.cpp @@ -417,80 +417,8 @@ static const cl_mem_object_type imgTypes[6] = { CL_MEM_OBJECT_IMAGE2D, CL_MEM_OBJECT_IMAGE2D_ARRAY, CL_MEM_OBJECT_IMAGE3D}; - -static const cl_mem_object_type imgFromBufferTypes[2] = { - CL_MEM_OBJECT_IMAGE1D_BUFFER, - CL_MEM_OBJECT_IMAGE2D}; } // namespace GmmTestConst -class GmmTiling : public GmmTests, - public ::testing::WithParamInterface { - public: - void checkTiling(cl_image_desc &imgDesc, bool forceLinear) { - bool allowTiling = GmmHelper::allowTiling(imgDesc); - if (forceLinear) { - EXPECT_FALSE(allowTiling); - } else { - if (imgDesc.image_type == CL_MEM_OBJECT_IMAGE1D || - imgDesc.image_type == CL_MEM_OBJECT_IMAGE1D_ARRAY || - imgDesc.image_type == CL_MEM_OBJECT_IMAGE1D_BUFFER || - imgDesc.buffer != nullptr) { - EXPECT_FALSE(allowTiling); - } else { - EXPECT_TRUE(allowTiling); - } - } - }; -}; - -class GmmImgTilingTests : public GmmTiling {}; - -INSTANTIATE_TEST_CASE_P( - GmmTiledTests, - GmmImgTilingTests, - testing::ValuesIn(GmmTestConst::imgTypes)); - -TEST_P(GmmImgTilingTests, allowTiling) { - bool defaultTilingType = DebugManager.flags.ForceLinearImages.get(); - - cl_image_desc imgDesc = {}; - imgDesc.image_type = GetParam(); - - checkTiling(imgDesc, defaultTilingType); - - DebugManager.flags.ForceLinearImages.set(!defaultTilingType); - - checkTiling(imgDesc, !defaultTilingType); - - DebugManager.flags.ForceLinearImages.set(defaultTilingType); -} - -class GmmImgFromBufferTilingTests : public GmmTiling {}; -INSTANTIATE_TEST_CASE_P( - GmmTiledTests, - GmmImgFromBufferTilingTests, - testing::ValuesIn(GmmTestConst::imgFromBufferTypes)); - -TEST_P(GmmImgFromBufferTilingTests, disallowImgFromBufferTiling) { - bool defaultTilingType = DebugManager.flags.ForceLinearImages.get(); - if (defaultTilingType) { - DebugManager.flags.ForceLinearImages.set(false); - } - - cl_image_desc imgDesc = {}; - imgDesc.image_type = GetParam(); - - MockContext context; - cl_int retVal = CL_SUCCESS; - - auto buffer = std::unique_ptr(Buffer::create(&context, {}, 1, nullptr, retVal)); - imgDesc.buffer = buffer.get(); - - checkTiling(imgDesc, false); - - DebugManager.flags.ForceLinearImages.set(defaultTilingType); -} - TEST_F(GmmTests, converOclPlaneToGmmPlane) { std::vector> v = {{OCLPlane::NO_PLANE, GMM_YUV_PLANE::GMM_NO_PLANE}, {OCLPlane::PLANE_Y, GMM_YUV_PLANE::GMM_PLANE_Y}, @@ -503,7 +431,8 @@ TEST_F(GmmTests, converOclPlaneToGmmPlane) { } } -class GmmImgTest : public GmmTiling {}; +class GmmImgTest : public GmmTests, + public ::testing::WithParamInterface {}; INSTANTIATE_TEST_CASE_P( GmmImgTests, diff --git a/unit_tests/helpers/hw_helper_tests.cpp b/unit_tests/helpers/hw_helper_tests.cpp index c5d0b8a888..43944120a2 100644 --- a/unit_tests/helpers/hw_helper_tests.cpp +++ b/unit_tests/helpers/hw_helper_tests.cpp @@ -19,6 +19,7 @@ #include "runtime/os_interface/os_interface.h" #include "unit_tests/helpers/unit_test_helper.h" #include "unit_tests/helpers/variable_backup.h" +#include "unit_tests/mocks/mock_context.h" #include #include @@ -677,3 +678,42 @@ TEST_F(HwHelperTest, givenVariousCachesRequestProperMOCSIndexesAreBeingReturned) EXPECT_EQ(expectedMocsForL3andL1on, mocsIndex); } } + +HWTEST_F(HwHelperTest, givenHwHelperWhenAskingForTilingSupportThenReturnValidValue) { + bool tilingSupported = UnitTestHelper::tiledImagesSupported; + + const uint32_t numImageTypes = 6; + const cl_mem_object_type imgTypes[numImageTypes] = {CL_MEM_OBJECT_IMAGE1D, CL_MEM_OBJECT_IMAGE1D_ARRAY, CL_MEM_OBJECT_IMAGE1D_BUFFER, + CL_MEM_OBJECT_IMAGE2D, CL_MEM_OBJECT_IMAGE2D_ARRAY, CL_MEM_OBJECT_IMAGE3D}; + cl_image_desc imgDesc = {}; + MockContext context; + cl_int retVal = CL_SUCCESS; + auto buffer = std::unique_ptr(Buffer::create(&context, 0, 1, nullptr, retVal)); + + auto &helper = HwHelper::get(renderCoreFamily); + + for (uint32_t i = 0; i < numImageTypes; i++) { + imgDesc.image_type = imgTypes[i]; + imgDesc.buffer = nullptr; + + bool allowedType = imgTypes[i] == (CL_MEM_OBJECT_IMAGE2D) || (imgTypes[i] == CL_MEM_OBJECT_IMAGE3D) || + (imgTypes[i] == CL_MEM_OBJECT_IMAGE2D_ARRAY); + + // non shared context, dont force linear storage + EXPECT_EQ((tilingSupported & allowedType), helper.tilingAllowed(false, imgDesc, false)); + { + DebugManagerStateRestore restore; + DebugManager.flags.ForceLinearImages.set(true); + // non shared context, dont force linear storage + debug flag + EXPECT_FALSE(helper.tilingAllowed(false, imgDesc, false)); + } + // shared context, dont force linear storage + EXPECT_FALSE(helper.tilingAllowed(true, imgDesc, false)); + // non shared context, force linear storage + EXPECT_FALSE(helper.tilingAllowed(false, imgDesc, true)); + + // non shared context, dont force linear storage + create from buffer + imgDesc.buffer = buffer.get(); + EXPECT_FALSE(helper.tilingAllowed(false, imgDesc, false)); + } +} diff --git a/unit_tests/helpers/unit_test_helper.h b/unit_tests/helpers/unit_test_helper.h index 6f609950da..ba01b1d46e 100644 --- a/unit_tests/helpers/unit_test_helper.h +++ b/unit_tests/helpers/unit_test_helper.h @@ -33,5 +33,7 @@ struct UnitTestHelper { static bool isPipeControlWArequired(const HardwareInfo &hwInfo); static uint64_t getMemoryAddress(const typename GfxFamily::MI_ATOMIC &atomic); + + static const bool tiledImagesSupported; }; } // namespace NEO diff --git a/unit_tests/helpers/unit_test_helper.inl b/unit_tests/helpers/unit_test_helper.inl index cd7d0f4c23..1aa280a520 100644 --- a/unit_tests/helpers/unit_test_helper.inl +++ b/unit_tests/helpers/unit_test_helper.inl @@ -54,4 +54,8 @@ template inline uint64_t UnitTestHelper::getMemoryAddress(const typename GfxFamily::MI_ATOMIC &atomic) { return atomic.getMemoryAddress() | ((static_cast(atomic.getMemoryAddressHigh())) << 32); } + +template +const bool UnitTestHelper::tiledImagesSupported = true; + } // namespace NEO diff --git a/unit_tests/mem_obj/image_tests.cpp b/unit_tests/mem_obj/image_tests.cpp index 473d602b9d..0af7156d95 100644 --- a/unit_tests/mem_obj/image_tests.cpp +++ b/unit_tests/mem_obj/image_tests.cpp @@ -1264,6 +1264,7 @@ TEST(ImageTest, givenCachelineAlignedPointerAndProperDescriptorValuesWhenIsCopyR imgInfo.rowPitch = imageDesc.image_width * surfaceFormat->ImageElementSizeInBytes; imgInfo.slicePitch = imgInfo.rowPitch * imageDesc.image_height; imgInfo.size = imgInfo.slicePitch; + imgInfo.linearStorage = true; auto hostPtr = alignedMalloc(imgInfo.size, MemoryConstants::cacheLineSize); @@ -1274,6 +1275,7 @@ TEST(ImageTest, givenCachelineAlignedPointerAndProperDescriptorValuesWhenIsCopyR TEST(ImageTest, givenForcedLinearImages3DImageAndProperDescriptorValuesWhenIsCopyRequiredIsCalledThenFalseIsReturned) { DebugManagerStateRestore dbgRestorer; DebugManager.flags.ForceLinearImages.set(true); + auto &hwHelper = HwHelper::get(platformDevices[0]->platform.eRenderCoreFamily); ImageInfo imgInfo{}; @@ -1295,6 +1297,7 @@ TEST(ImageTest, givenForcedLinearImages3DImageAndProperDescriptorValuesWhenIsCop imgInfo.rowPitch = imageDesc.image_width * surfaceFormat->ImageElementSizeInBytes; imgInfo.slicePitch = imgInfo.rowPitch * imageDesc.image_height; imgInfo.size = imgInfo.slicePitch; + imgInfo.linearStorage = !hwHelper.tilingAllowed(false, *imgInfo.imgDesc, false); auto hostPtr = alignedMalloc(imgInfo.size, MemoryConstants::cacheLineSize); diff --git a/unit_tests/memory_manager/memory_manager_tests.cpp b/unit_tests/memory_manager/memory_manager_tests.cpp index 3b145ae799..8df2cf78ca 100644 --- a/unit_tests/memory_manager/memory_manager_tests.cpp +++ b/unit_tests/memory_manager/memory_manager_tests.cpp @@ -673,6 +673,7 @@ TEST(OsAgnosticMemoryManager, givenHostPointerNotRequiringCopyWhenAllocateGraphi imgInfo.rowPitch = imgDesc.image_width * 4; imgInfo.slicePitch = imgInfo.rowPitch * imgDesc.image_height; imgInfo.size = imgInfo.slicePitch; + imgInfo.linearStorage = true; auto hostPtr = alignedMalloc(imgDesc.image_width * imgDesc.image_height * 4, MemoryConstants::pageSize); @@ -1875,4 +1876,4 @@ TEST(MemoryManagerTest, givenMemoryManagerWhenGetReservedMemoryIsCalledManyTimes memoryManager.getReservedMemory(MemoryConstants::cacheLineSize, MemoryConstants::cacheLineSize); memoryManager.getReservedMemory(MemoryConstants::cacheLineSize, MemoryConstants::cacheLineSize); EXPECT_EQ(reservedMemory, memoryManager.reservedMemory); -} \ No newline at end of file +} diff --git a/unit_tests/os_interface/linux/drm_memory_manager_tests.cpp b/unit_tests/os_interface/linux/drm_memory_manager_tests.cpp index aa71f177b2..78f52cc03b 100644 --- a/unit_tests/os_interface/linux/drm_memory_manager_tests.cpp +++ b/unit_tests/os_interface/linux/drm_memory_manager_tests.cpp @@ -1476,6 +1476,7 @@ TEST_F(DrmMemoryManagerTest, givenHostPointerNotRequiringCopyWhenAllocateGraphic imgInfo.rowPitch = imgDesc.image_width * surfaceFormat->ImageElementSizeInBytes; imgInfo.slicePitch = imgInfo.rowPitch * imgDesc.image_height; imgInfo.size = imgInfo.slicePitch; + imgInfo.linearStorage = true; auto hostPtr = alignedMalloc(imgDesc.image_width * imgDesc.image_height * 4, MemoryConstants::pageSize); bool copyRequired = Image::isCopyRequired(imgInfo, hostPtr);