Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d92a9b15 authored by Greg Daniel's avatar Greg Daniel
Browse files

Fix deletion of VkSemaphores in VulkanManager. 

We were deleting the VkSemaphore objects too quickly when
importing/exporting the semaphores. Even though the semaphore payload
gets reset on these operations the VkSemaphore still needs to be
finished its use in Vulkan before being deleted.

Test: manual build and testing of vulkan apps and vulakn ImageConsumer
Bug: b/130643604
Change-Id: I7f03087e477d812c0174ede3a10f12dc1df72ee1
parent 9aa53da1

libs/hwui/renderthread/VulkanManager.cpp

+40 −88
Original line number Diff line number Diff line
@@ -58,10 +58,6 @@ static void free_features_extensions_structs(const VkPhysicalDeviceFeatures2& fe 
#define GET_DEV_PROC(F) m##F = (PFN_vk##F)vkGetDeviceProcAddr(mDevice, "vk" #F)



void VulkanManager::destroy() {

    // We don't need to explicitly free the command buffer since it automatically gets freed when we

    // delete the VkCommandPool below.

    mDummyCB = VK_NULL_HANDLE;



    if (VK_NULL_HANDLE != mCommandPool) {

        mDestroyCommandPool(mDevice, mCommandPool, nullptr);

        mCommandPool = VK_NULL_HANDLE;
@@ -376,12 +372,6 @@ void VulkanManager::initialize() { 
    }

    LOG_ALWAYS_FATAL_IF(mCommandPool == VK_NULL_HANDLE);



    if (!setupDummyCommandBuffer()) {

        this->destroy();

        // Pass through will crash on next line.

    }

    LOG_ALWAYS_FATAL_IF(mDummyCB == VK_NULL_HANDLE);



    mGetDeviceQueue(mDevice, mPresentQueueIndex, 0, &mPresentQueue);



    if (Properties::enablePartialUpdates && Properties::useBufferAge) {
@@ -488,6 +478,22 @@ Frame VulkanManager::dequeueNextBuffer(VulkanSurface* surface) { 
    return Frame(surface->logicalWidth(), surface->logicalHeight(), bufferAge);

}



struct DestroySemaphoreInfo {

    PFN_vkDestroySemaphore mDestroyFunction;

    VkDevice mDevice;

    VkSemaphore mSemaphore;



    DestroySemaphoreInfo(PFN_vkDestroySemaphore destroyFunction, VkDevice device,

            VkSemaphore semaphore)

            : mDestroyFunction(destroyFunction), mDevice(device), mSemaphore(semaphore) {}

};



static void destroy_semaphore(void* context) {

    DestroySemaphoreInfo* info = reinterpret_cast<DestroySemaphoreInfo*>(context);

    info->mDestroyFunction(info->mDevice, info->mSemaphore, nullptr);

    delete info;

}



void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect) {

    if (CC_UNLIKELY(Properties::waitForGpuCompletion)) {

        ATRACE_NAME("Finishing GPU work");
@@ -517,9 +523,12 @@ void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect) 
    backendSemaphore.initVulkan(semaphore);



    int fenceFd = -1;

    DestroySemaphoreInfo* destroyInfo = new DestroySemaphoreInfo(mDestroySemaphore, mDevice,

                                                                 semaphore);

    GrSemaphoresSubmitted submitted =

            bufferInfo->skSurface->flush(SkSurface::BackendSurfaceAccess::kPresent,

                                         SkSurface::kNone_FlushFlags, 1, &backendSemaphore);

                                         kNone_GrFlushFlags, 1, &backendSemaphore,

                                         destroy_semaphore, destroyInfo);

    if (submitted == GrSemaphoresSubmitted::kYes) {

        VkSemaphoreGetFdInfoKHR getFdInfo;

        getFdInfo.sType = VK_STRUCTURE_TYPE_SEMAPHORE_GET_FD_INFO_KHR;
@@ -535,12 +544,6 @@ void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect) 
    }



    surface->presentCurrentBuffer(dirtyRect, fenceFd);



    // Exporting a semaphore with copy transference via vkGetSemaphoreFdKHR, has the same effect of

    // destroying the semaphore and creating a new one with the same handle, and the payloads

    // ownership is move to the Fd we created. Thus the semaphore is in a state that we can delete

    // it and we don't need to wait on the command buffer we submitted to finish.

    mDestroySemaphore(mDevice, semaphore, nullptr);

}



void VulkanManager::destroySurface(VulkanSurface* surface) {
@@ -566,38 +569,7 @@ VulkanSurface* VulkanManager::createSurface(ANativeWindow* window, ColorMode col 
                                 *this, extraBuffers);

}



bool VulkanManager::setupDummyCommandBuffer() {

    if (mDummyCB != VK_NULL_HANDLE) {

        return true;

    }



    VkCommandBufferAllocateInfo commandBuffersInfo;

    memset(&commandBuffersInfo, 0, sizeof(VkCommandBufferAllocateInfo));

    commandBuffersInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO;

    commandBuffersInfo.pNext = nullptr;

    commandBuffersInfo.commandPool = mCommandPool;

    commandBuffersInfo.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY;

    commandBuffersInfo.commandBufferCount = 1;



    VkResult err = mAllocateCommandBuffers(mDevice, &commandBuffersInfo, &mDummyCB);

    if (err != VK_SUCCESS) {

        // It is probably unnecessary to set this back to VK_NULL_HANDLE, but we set it anyways to

        // make sure the driver didn't set a value and then return a failure.

        mDummyCB = VK_NULL_HANDLE;

        return false;

    }



    VkCommandBufferBeginInfo beginInfo;

    memset(&beginInfo, 0, sizeof(VkCommandBufferBeginInfo));

    beginInfo.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO;

    beginInfo.flags = VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT;



    mBeginCommandBuffer(mDummyCB, &beginInfo);

    mEndCommandBuffer(mDummyCB);

    return true;

}



status_t VulkanManager::fenceWait(sp<Fence>& fence) {

status_t VulkanManager::fenceWait(sp<Fence>& fence, GrContext* grContext) {

    if (!hasVkContext()) {

        ALOGE("VulkanManager::fenceWait: VkDevice not initialized");

        return INVALID_OPERATION;
@@ -630,36 +602,22 @@ status_t VulkanManager::fenceWait(sp<Fence>& fence) { 


    err = mImportSemaphoreFdKHR(mDevice, &importInfo);

    if (VK_SUCCESS != err) {

        mDestroySemaphore(mDevice, semaphore, nullptr);

        ALOGE("Failed to import semaphore, err: %d", err);

        return UNKNOWN_ERROR;

    }



    LOG_ALWAYS_FATAL_IF(mDummyCB == VK_NULL_HANDLE);

    GrBackendSemaphore beSemaphore;

    beSemaphore.initVulkan(semaphore);



    VkPipelineStageFlags waitDstStageFlags = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;

    // Skia takes ownership of the semaphore and will delete it once the wait has finished.

    grContext->wait(1, &beSemaphore);

    grContext->flush();



    VkSubmitInfo submitInfo;

    memset(&submitInfo, 0, sizeof(VkSubmitInfo));

    submitInfo.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;

    submitInfo.waitSemaphoreCount = 1;

    // Wait to make sure aquire semaphore set above has signaled.

    submitInfo.pWaitSemaphores = &semaphore;

    submitInfo.pWaitDstStageMask = &waitDstStageFlags;

    submitInfo.commandBufferCount = 1;

    submitInfo.pCommandBuffers = &mDummyCB;

    submitInfo.signalSemaphoreCount = 0;



    mQueueSubmit(mGraphicsQueue, 1, &submitInfo, VK_NULL_HANDLE);



    // On Android when we import a semaphore, it is imported using temporary permanence. That

    // means as soon as we queue the semaphore for a wait it reverts to its previous permanent

    // state before importing. This means it will now be in an idle state with no pending

    // signal or wait operations, so it is safe to immediately delete it.

    mDestroySemaphore(mDevice, semaphore, nullptr);

    return OK;

}



status_t VulkanManager::createReleaseFence(sp<Fence>& nativeFence) {

status_t VulkanManager::createReleaseFence(sp<Fence>& nativeFence, GrContext* grContext) {

    if (!hasVkContext()) {

        ALOGE("VulkanManager::createReleaseFence: VkDevice not initialized");

        return INVALID_OPERATION;
@@ -681,20 +639,20 @@ status_t VulkanManager::createReleaseFence(sp<Fence>& nativeFence) { 
        return INVALID_OPERATION;

    }



    LOG_ALWAYS_FATAL_IF(mDummyCB == VK_NULL_HANDLE);

    GrBackendSemaphore backendSemaphore;

    backendSemaphore.initVulkan(semaphore);



    VkSubmitInfo submitInfo;

    memset(&submitInfo, 0, sizeof(VkSubmitInfo));

    submitInfo.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;

    submitInfo.waitSemaphoreCount = 0;

    submitInfo.pWaitSemaphores = nullptr;

    submitInfo.pWaitDstStageMask = nullptr;

    submitInfo.commandBufferCount = 1;

    submitInfo.pCommandBuffers = &mDummyCB;

    submitInfo.signalSemaphoreCount = 1;

    submitInfo.pSignalSemaphores = &semaphore;

    DestroySemaphoreInfo* destroyInfo = new DestroySemaphoreInfo(mDestroySemaphore, mDevice,

                                                                 semaphore);

    GrSemaphoresSubmitted submitted =

            grContext->flush(kNone_GrFlushFlags, 1, &backendSemaphore,

                             destroy_semaphore, destroyInfo);



    mQueueSubmit(mGraphicsQueue, 1, &submitInfo, VK_NULL_HANDLE);

    if (submitted == GrSemaphoresSubmitted::kNo) {

        ALOGE("VulkanManager::createReleaseFence: Failed to submit semaphore");

        mDestroySemaphore(mDevice, semaphore, nullptr);

        return INVALID_OPERATION;

    }



    VkSemaphoreGetFdInfoKHR getFdInfo;

    getFdInfo.sType = VK_STRUCTURE_TYPE_SEMAPHORE_GET_FD_INFO_KHR;
@@ -711,12 +669,6 @@ status_t VulkanManager::createReleaseFence(sp<Fence>& nativeFence) { 
    }

    nativeFence = new Fence(fenceFd);



    // Exporting a semaphore with copy transference via vkGetSemahporeFdKHR, has the same effect of

    // destroying the semaphore and creating a new one with the same handle, and the payloads

    // ownership is move to the Fd we created. Thus the semahpore is in a state that we can delete

    // it and we don't need to wait on the command buffer we submitted to finish.

    mDestroySemaphore(mDevice, semaphore, nullptr);



    return OK;

}

libs/hwui/renderthread/VulkanManager.h

+4 −6
Original line number Diff line number Diff line
@@ -70,10 +70,11 @@ public: 
    void destroy();



    // Inserts a wait on fence command into the Vulkan command buffer.

    status_t fenceWait(sp<Fence>& fence);

    status_t fenceWait(sp<Fence>& fence, GrContext* grContext);



    // Creates a fence that is signaled, when all the pending Vulkan commands are flushed.

    status_t createReleaseFence(sp<Fence>& nativeFence);

    // Creates a fence that is signaled when all the pending Vulkan commands are finished on the

    // GPU.

    status_t createReleaseFence(sp<Fence>& nativeFence, GrContext* grContext);



    // Returned pointers are owned by VulkanManager.

    // An instance of VkFunctorInitParams returned from getVkFunctorInitParams refers to
@@ -89,7 +90,6 @@ private: 
    // Sets up the VkInstance and VkDevice objects. Also fills out the passed in

    // VkPhysicalDeviceFeatures struct.

    void setupDevice(GrVkExtensions&, VkPhysicalDeviceFeatures2&);

    bool setupDummyCommandBuffer();



    // simple wrapper class that exists only to initialize a pointer to NULL

    template <typename FNPTR_TYPE>
@@ -164,8 +164,6 @@ private: 
    VkQueue mPresentQueue = VK_NULL_HANDLE;

    VkCommandPool mCommandPool = VK_NULL_HANDLE;



    VkCommandBuffer mDummyCB = VK_NULL_HANDLE;



    // Variables saved to populate VkFunctorInitParams.

    static const uint32_t mAPIVersion = VK_MAKE_VERSION(1, 1, 0);

    std::vector<VkExtensionProperties> mInstanceExtensionsOwner;

libs/hwui/surfacetexture/ImageConsumer.cpp

+4 −2
Original line number Diff line number Diff line
@@ -212,7 +212,8 @@ sk_sp<SkImage> ImageConsumer::dequeueImage(bool* queueEmpty, SurfaceTexture& st, 
            uirenderer::RenderPipelineType::SkiaGL) {

            err = renderState.getRenderThread().eglManager().fenceWait(item.mFence);

        } else {

            err = renderState.getRenderThread().vulkanManager().fenceWait(item.mFence);

            err = renderState.getRenderThread().vulkanManager().fenceWait(

                    item.mFence, renderState.getRenderThread().getGrContext());

        }

        if (err != OK) {

            st.releaseBufferLocked(slot, st.mSlots[slot].mGraphicBuffer, EGL_NO_DISPLAY,
@@ -234,7 +235,8 @@ sk_sp<SkImage> ImageConsumer::dequeueImage(bool* queueEmpty, SurfaceTexture& st, 
            err = eglManager.createReleaseFence(st.mUseFenceSync, &mImageSlots[slot].eglFence(),

                                                releaseFence);

        } else {

            err = renderState.getRenderThread().vulkanManager().createReleaseFence(releaseFence);

            err = renderState.getRenderThread().vulkanManager().createReleaseFence(

                    releaseFence, renderState.getRenderThread().getGrContext());

        }

        if (OK != err) {

            st.releaseBufferLocked(slot, st.mSlots[slot].mGraphicBuffer, EGL_NO_DISPLAY,