From e47385dc112bf1a3bf26b18786f74c480a1e8dab Mon Sep 17 00:00:00 2001 From: Filip Hazubski Date: Wed, 13 Apr 2022 17:20:59 +0000 Subject: [PATCH] Refactor ShaderChannelSelect logic for images using CL_LUMINANCE Signed-off-by: Filip Hazubski --- opencl/source/mem_obj/image.inl | 12 ++++++++++-- .../unit_test/mem_obj/image_set_arg_tests.cpp | 18 +++++++++++++----- .../os_interface/hw_info_config_tests.cpp | 5 +++++ shared/source/os_interface/hw_info_config.h | 2 ++ .../hw_info_config_bdw_and_later.inl | 5 +++++ .../hw_info_config_xehp_and_later.inl | 5 +++++ .../test/common/mocks/mock_hw_info_config.cpp | 5 +++++ .../test_macros/header/common_matchers.h | 1 + 8 files changed, 46 insertions(+), 7 deletions(-) diff --git a/opencl/source/mem_obj/image.inl b/opencl/source/mem_obj/image.inl index a9f3228ad2..7a0bd771c6 100644 --- a/opencl/source/mem_obj/image.inl +++ b/opencl/source/mem_obj/image.inl @@ -12,6 +12,7 @@ #include "shared/source/gmm_helper/resource_info.h" #include "shared/source/helpers/aligned_memory.h" #include "shared/source/helpers/populate_factory.h" +#include "shared/source/os_interface/hw_info_config.h" #include "opencl/source/helpers/surface_formats.h" #include "opencl/source/mem_obj/image.h" @@ -74,8 +75,15 @@ void ImageHw::setImageArg(void *memory, bool setAsMediaBlockImage, ui surfaceState->setShaderChannelSelectRed(static_cast(shaderChannelValue)); if (imgChannelOrder == CL_LUMINANCE) { - surfaceState->setShaderChannelSelectGreen(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED); - surfaceState->setShaderChannelSelectBlue(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED); + auto &hwInfoConfig = *HwInfoConfig::get(executionEnvironment->rootDeviceEnvironments[rootDeviceIndex]->getHardwareInfo()->platform.eProductFamily); + if (hwInfoConfig.useChannelRedForUnusedShaderChannels()) { + surfaceState->setShaderChannelSelectGreen(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED); + surfaceState->setShaderChannelSelectBlue(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED); + } else { + surfaceState->setShaderChannelSelectGreen(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_ZERO); + surfaceState->setShaderChannelSelectBlue(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_ZERO); + surfaceState->setShaderChannelSelectAlpha(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_ONE); + } } else { shaderChannelValue = ImageHw::getShaderChannelValue(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_GREEN, imgChannelOrder); surfaceState->setShaderChannelSelectGreen(static_cast(shaderChannelValue)); diff --git a/opencl/test/unit_test/mem_obj/image_set_arg_tests.cpp b/opencl/test/unit_test/mem_obj/image_set_arg_tests.cpp index 62529e6b3e..423a273aed 100644 --- a/opencl/test/unit_test/mem_obj/image_set_arg_tests.cpp +++ b/opencl/test/unit_test/mem_obj/image_set_arg_tests.cpp @@ -841,11 +841,19 @@ HWTEST_F(ImageSetArgTest, GivenImageWithClLuminanceFormatWhenSettingKernelArgThe auto surfaceState = reinterpret_cast( ptrOffset(pKernel->getSurfaceStateHeap(), pKernelInfo->argAsImg(0).bindful)); - //for CL_LUMINANCE format we override channels to RED to be spec complaint. - EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED, surfaceState->getShaderChannelSelectRed()); - EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED, surfaceState->getShaderChannelSelectGreen()); - EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED, surfaceState->getShaderChannelSelectBlue()); - EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_ALPHA, surfaceState->getShaderChannelSelectAlpha()); + auto &hwInfoConfig = *HwInfoConfig::get(defaultHwInfo->platform.eProductFamily); + //for CL_LUMINANCE format we override channels to RED to be spec compliant. + if (hwInfoConfig.useChannelRedForUnusedShaderChannels()) { + EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED, surfaceState->getShaderChannelSelectRed()); + EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED, surfaceState->getShaderChannelSelectGreen()); + EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED, surfaceState->getShaderChannelSelectBlue()); + EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_ALPHA, surfaceState->getShaderChannelSelectAlpha()); + } else { + EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_RED, surfaceState->getShaderChannelSelectRed()); + EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_ZERO, surfaceState->getShaderChannelSelectGreen()); + EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_ZERO, surfaceState->getShaderChannelSelectBlue()); + EXPECT_EQ(RENDER_SURFACE_STATE::SHADER_CHANNEL_SELECT_ONE, surfaceState->getShaderChannelSelectAlpha()); + } std::vector surfaces; pKernel->getResidency(surfaces); diff --git a/opencl/test/unit_test/os_interface/hw_info_config_tests.cpp b/opencl/test/unit_test/os_interface/hw_info_config_tests.cpp index b4f6aa0165..c443de0023 100644 --- a/opencl/test/unit_test/os_interface/hw_info_config_tests.cpp +++ b/opencl/test/unit_test/os_interface/hw_info_config_tests.cpp @@ -395,3 +395,8 @@ HWTEST_F(HwInfoConfigTest, givenHwInfoConfigWhenAskedIfPatIndexProgrammingSuppor const auto &hwInfoConfig = *HwInfoConfig::get(pInHwInfo.platform.eProductFamily); EXPECT_FALSE(hwInfoConfig.isVmBindPatIndexProgrammingSupported()); } + +HWTEST2_F(HwInfoConfigTest, givenHwInfoConfigWhenAskedIfUseChannelRedForUnusedShaderChannelsThenTrueIsReturned, IsAtMostXeHpcCore) { + const auto &hwInfoConfig = *HwInfoConfig::get(pInHwInfo.platform.eProductFamily); + EXPECT_TRUE(hwInfoConfig.useChannelRedForUnusedShaderChannels()); +} diff --git a/shared/source/os_interface/hw_info_config.h b/shared/source/os_interface/hw_info_config.h index 5ac110af8d..af3498129a 100644 --- a/shared/source/os_interface/hw_info_config.h +++ b/shared/source/os_interface/hw_info_config.h @@ -97,6 +97,7 @@ class HwInfoConfig { virtual bool isComputeDispatchAllWalkerEnableInCfeStateRequired(const HardwareInfo &hwInfo) const = 0; virtual bool isVmBindPatIndexProgrammingSupported() const = 0; virtual bool isBFloat16ConversionSupported(const HardwareInfo &hwInfo) const = 0; + virtual bool useChannelRedForUnusedShaderChannels() const = 0; MOCKABLE_VIRTUAL ~HwInfoConfig() = default; @@ -178,6 +179,7 @@ class HwInfoConfigHw : public HwInfoConfig { bool isComputeDispatchAllWalkerEnableInCfeStateRequired(const HardwareInfo &hwInfo) const override; bool isVmBindPatIndexProgrammingSupported() const override; bool isBFloat16ConversionSupported(const HardwareInfo &hwInfo) const override; + bool useChannelRedForUnusedShaderChannels() const override; protected: HwInfoConfigHw() = default; diff --git a/shared/source/os_interface/hw_info_config_bdw_and_later.inl b/shared/source/os_interface/hw_info_config_bdw_and_later.inl index d74b56b765..d8e53dc2eb 100644 --- a/shared/source/os_interface/hw_info_config_bdw_and_later.inl +++ b/shared/source/os_interface/hw_info_config_bdw_and_later.inl @@ -65,4 +65,9 @@ bool HwInfoConfigHw::isBFloat16ConversionSupported(const HardwareInf return false; } +template +bool HwInfoConfigHw::useChannelRedForUnusedShaderChannels() const { + return true; +} + } // namespace NEO diff --git a/shared/source/os_interface/hw_info_config_xehp_and_later.inl b/shared/source/os_interface/hw_info_config_xehp_and_later.inl index a2ba45b487..b2d34041fb 100644 --- a/shared/source/os_interface/hw_info_config_xehp_and_later.inl +++ b/shared/source/os_interface/hw_info_config_xehp_and_later.inl @@ -62,4 +62,9 @@ bool HwInfoConfigHw::isBFloat16ConversionSupported(const HardwareInf return true; } +template +bool HwInfoConfigHw::useChannelRedForUnusedShaderChannels() const { + return true; +} + } // namespace NEO diff --git a/shared/test/common/mocks/mock_hw_info_config.cpp b/shared/test/common/mocks/mock_hw_info_config.cpp index 6d41f84bb2..3d0b24d876 100644 --- a/shared/test/common/mocks/mock_hw_info_config.cpp +++ b/shared/test/common/mocks/mock_hw_info_config.cpp @@ -337,4 +337,9 @@ bool HwInfoConfigHw::isBFloat16ConversionSupported(const HardwareI return false; } +template <> +bool HwInfoConfigHw::useChannelRedForUnusedShaderChannels() const { + return false; +} + } //namespace NEO diff --git a/shared/test/common/test_macros/header/common_matchers.h b/shared/test/common/test_macros/header/common_matchers.h index 0cf2c7efb2..86c5913412 100644 --- a/shared/test/common/test_macros/header/common_matchers.h +++ b/shared/test/common/test_macros/header/common_matchers.h @@ -32,6 +32,7 @@ using IsAtLeastXeHpgCore = IsAtLeastGfxCore; using IsAtMostXeHpgCore = IsAtMostGfxCore; using IsAtLeastXeHpcCore = IsAtLeastGfxCore; +using IsAtMostXeHpcCore = IsAtMostGfxCore; using isXeHpOrXeHpcCore = IsAnyGfxCores; using isXeHpcOrXeHpgCore = IsAnyGfxCores;