P4 to Git Change 1979444 by gandryey@gera-win10 on 2019/08/07 10:43:18

SWDEV-197488 - [Navi10][Navi14][19.30][CopySurfaceRegion] CopySurfaceRegion Failing Multiple Tests
	- Try to optimize the condition for image buffer workaround. The new logic will attempt to validate the custom pitch with the HW requirement and use the backing store only if it doesn't match

Affected files ...

... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palmemory.cpp#28 edit
... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palmemory.hpp#14 edit
... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palresource.cpp#76 edit
... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palresource.hpp#29 edit

rocclr/runtime/device/pal/palmemory.cpp

@@ -74,19 +74,19 @@ static HANDLE getSharedHandle(IUnknown* pIface) {
 }
 #endif  //_WIN32
 
-bool Memory::create(Resource::MemoryType memType, Resource::CreateParams* params) {
+bool Memory::create(Resource::MemoryType memType, Resource::CreateParams* params, bool forceLinear) {
   bool result;
   uint allocAttempt = 0;
   // Reset the flag in case we reallocate the heap in local/remote
   flags_ &= ~HostMemoryDirectAccess;
-  
+
   if (!ValidateMemory(memType)) {
     return false;
   }
 
   do {
-    // Create a resource in CAL
-    result = Resource::create(memType, params);
+    // Create a resource in PAL
+    result = Resource::create(memType, params, forceLinear);
     if (!result) {
       size_t freeMemory[2];
       // if requested memory is greater than available then exit the loop
@@ -1096,12 +1096,29 @@ bool Image::ValidateMemory(Resource::MemoryType memType) {
   if (dev().settings().imageBufferWar_ && (memType == ImageBuffer) && (owner() != nullptr) &&
       ((owner()->asImage()->getWidth() * owner()->asImage()->getImageFormat().getElementSize()) <
        owner()->asImage()->getRowPitch())) {
-    // Create a native image without pitch as a backing store
+    constexpr bool ForceLinear = true;
+    // Create a native image without pitch for validation
     copyImageBuffer_ = new pal::Image(dev(), size(), desc().width_, desc().height_, desc().depth_,
                                       desc().format_, desc().topology_, 0);
-    if ((copyImageBuffer_ == nullptr) || !copyImageBuffer_->create(Resource::Local)) {
+    if ((copyImageBuffer_ == nullptr) ||
+        !copyImageBuffer_->create(Resource::Local, nullptr, ForceLinear)) {
       return false;
     }
+    constexpr Pal::SubresId ImgSubresId = {Pal::ImageAspect::Color, 0, 0};
+    Pal::SubresLayout layout;
+    copyImageBuffer_->image()->GetSubresourceLayout(ImgSubresId, &layout);
+    // Destroy temporary linear image, since it was allocated for the pitch validation only
+    delete copyImageBuffer_;
+    copyImageBuffer_ = nullptr;
+    // If pitch doesn't match HW expectation, then create a backing store
+    if (owner()->asImage()->getRowPitch() != layout.rowPitch) {
+      // Create a native image without pitch as a backing store
+      copyImageBuffer_ = new pal::Image(dev(), size(), desc().width_, desc().height_, desc().depth_,
+                                        desc().format_, desc().topology_, 0);
+      if ((copyImageBuffer_ == nullptr) || !copyImageBuffer_->create(Resource::Local)) {
+        return false;
+      }
+    }
   }
   return true;
 }

rocclr/runtime/device/pal/palmemory.hpp

@@ -68,8 +68,9 @@ class Memory : public device::Memory, public Resource {
   bool createInterop();
 
   //! Overloads the resource create method
-  virtual bool create(Resource::MemoryType memType,          //!< Memory type
-                      Resource::CreateParams* params = NULL  //!< Prameters for create
+  virtual bool create(Resource::MemoryType memType,             //!< Memory type
+                      Resource::CreateParams* params = nullptr, //!< Prameters for create
+                      bool forceLinear = false                  //!< Forces linear tiling for images
   );
 
   //! Allocate memory for API-level maps

rocclr/runtime/device/pal/palresource.cpp

@@ -421,7 +421,7 @@ void Resource::memTypeToHeap(Pal::GpuMemoryCreateInfo* createInfo) {
 }
 
 // ================================================================================================
-bool Resource::CreateImage(CreateParams* params) {
+bool Resource::CreateImage(CreateParams* params, bool forceLinear) {
   Pal::Result result;
   Pal::SubresId ImgSubresId = {Pal::ImageAspect::Color, 0, 0};
   Pal::SubresRange ImgSubresRange = {ImgSubresId, 1, 1};
@@ -542,7 +542,7 @@ bool Resource::CreateImage(CreateParams* params) {
     imgCreateInfo.mipLevels = (desc_.mipLevels_) ? desc_.mipLevels_ : 1;
     imgCreateInfo.samples = 1;
     imgCreateInfo.fragments = 1;
-    Pal::ImageTiling tiling = Pal::ImageTiling::Optimal;
+    Pal::ImageTiling tiling = forceLinear ? Pal::ImageTiling::Linear : Pal::ImageTiling::Optimal;
     uint32_t rowPitch = 0;
 
     if (((memoryType() == Persistent) && dev().settings().linearPersistentImage_) ||
@@ -1056,7 +1056,7 @@ bool Resource::CreateSvm(CreateParams* params, Pal::gpusize svmPtr) {
 }
 
 // ================================================================================================
-bool Resource::create(MemoryType memType, CreateParams* params) {
+bool Resource::create(MemoryType memType, CreateParams* params, bool forceLinear) {
   bool imageCreateView = false;
   bool foundCalRef = false;
   bool viewDefined = false;
@@ -1142,7 +1142,7 @@ bool Resource::create(MemoryType memType, CreateParams* params) {
   }
 
   if (!desc_.buffer_) {
-    return CreateImage(params);
+    return CreateImage(params, forceLinear);
   }
 
   Pal::gpusize svmPtr = 0;

rocclr/runtime/device/pal/palresource.hpp

@@ -209,7 +209,8 @@ class Resource : public amd::HeapObject {
    *  \return True if we succesfully created a PAL resource
    */
   virtual bool create(MemoryType memType,       //!< memory type
-                      CreateParams* params = 0  //!< special parameters for resource allocation
+                      CreateParams* params = 0, //!< special parameters for resource allocation
+                      bool forceLinear = false  //!< Forces linear tiling for images
   );
 
   /*! \brief Copies a subregion of memory from one resource to another
@@ -395,7 +396,8 @@ class Resource : public amd::HeapObject {
    *
    *  \return True if we succesfully created a PAL resource
    */
-  bool CreateImage(CreateParams* params  //!< special parameters for resource allocation
+  bool CreateImage(CreateParams* params,  //!< special parameters for resource allocation
+                   bool          forceLinear = false  //!< forces linear tiling for images
   );
 
   /*! \brief Creates a PAL interop object, associated with the resource
@@ -428,6 +430,9 @@ class Resource : public amd::HeapObject {
 
   uint elementSize_;  //!< Size of a single element in bytes
 
+  //! Returns PAL image object
+  Pal::IImage* image() const { return image_; }
+
  private:
   //! Disable copy constructor
   Resource(const Resource&);