commit bdcc23e0a5e7e220660d3f54c97262f9a4c31606 Author: Nick Diego Yamane Date: Thu Nov 2 17:26:25 2023 +0000 gbm: nvidia: use separate bo to verify modifiers Buggy Nvidia drivers fail to return FDs for planes of a BO which had already an imported BO destroyed before. This is a workaround for that issue, which consists of creating/destroying a separate 1x1 BO for validating the modifiers before actually creating the final requested BO, which for now is limited to IS_LINUX builds. The Nvidia driver bug is being tracked under internal bug 4315529. There seems to be other issues when running under Wayland with Nvidia, which will be tracked and addressed in separate patches. R=dcastagna, msisov@igalia.com with ozone/wayland backend and verify GPU acceleration is not broken. Test: In a single Nvidia GPU setup, with proprietary driver, run Chrome Bug: 1273758, 1478684, 1463851 Change-Id: I9f322bcf40b460bcd4ead02f05dd2e9a8d271cea Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4989782 Reviewed-by: Maksim Sisov Commit-Queue: Nick Yamane Cr-Commit-Position: refs/heads/main@{#1218924} diff --git a/ui/gfx/linux/gbm_wrapper.cc b/ui/gfx/linux/gbm_wrapper.cc index bf90b76605f68..14918c19c0ab0 100644 --- a/ui/gfx/linux/gbm_wrapper.cc +++ b/ui/gfx/linux/gbm_wrapper.cc @@ -11,6 +11,7 @@ #include "base/logging.h" #include "base/memory/raw_ptr.h" #include "base/memory/raw_ptr_exclusion.h" +#include "base/numerics/safe_conversions.h" #include "base/posix/eintr_wrapper.h" #include "skia/ext/legacy_display_globals.h" #include "third_party/skia/include/core/SkSurface.h" @@ -71,6 +72,7 @@ base::ScopedFD GetPlaneFdForBo(gbm_bo* bo, size_t plane) { int ret; // Use DRM_RDWR to allow the fd to be mappable in another process. ret = drmPrimeHandleToFD(dev_fd, plane_handle, DRM_CLOEXEC | DRM_RDWR, &fd); + PLOG_IF(ERROR, ret != 0) << "Failed to get fd for plane."; // Older DRM implementations blocked DRM_RDWR, but gave a read/write mapping // anyways @@ -301,58 +303,82 @@ class Device final : public ui::GbmDevice { std::unique_ptr CreateBufferWithModifiers( uint32_t format, - const gfx::Size& size, + const gfx::Size& requested_size, uint32_t flags, const std::vector& modifiers) override { - if (modifiers.empty()) - return CreateBuffer(format, size, flags); - - std::vector filtered_modifiers = - GetFilteredModifiers(format, flags, modifiers); - struct gbm_bo* bo = nullptr; - while (filtered_modifiers.size() > 0) { - bo = gbm_bo_create_with_modifiers(device_, size.width(), size.height(), - format, filtered_modifiers.data(), - filtered_modifiers.size()); - if (!bo) { + if (modifiers.empty()) { + return CreateBuffer(format, requested_size, flags); + } + + // Buggy drivers prevent us from getting plane FDs from a BO which had its + // previously imported BO destroyed. E.g: Nvidia. Thus, on Linux Desktop, we + // do the create/import modifiers validation loop below using a separate set + // of 1x1 BOs which are destroyed before creating the final BO creation used + // to instantiate the returned GbmBuffer. + gfx::Size size = +#if BUILDFLAG(IS_LINUX) + gfx::Size(1, 1); +#else + requested_size; +#endif + auto filtered_modifiers = GetFilteredModifiers(format, flags, modifiers); + struct gbm_bo* created_bo = nullptr; + bool valid_modifiers = false; + + while (!valid_modifiers && !filtered_modifiers.empty()) { + created_bo = gbm_bo_create_with_modifiers( + device_, size.width(), size.height(), format, + filtered_modifiers.data(), filtered_modifiers.size()); + if (!created_bo) { return nullptr; } - struct gbm_import_fd_modifier_data fd_data; - fd_data.width = size.width(); - fd_data.height = size.height(); - fd_data.format = format; - fd_data.num_fds = gbm_bo_get_plane_count(bo); - fd_data.modifier = gbm_bo_get_modifier(bo); - - // Store fds in the vector of base::ScopedFDs. Will be released - // automatically. + const int planes_count = gbm_bo_get_plane_count(created_bo); + struct gbm_import_fd_modifier_data fd_data = { + .width = base::checked_cast(size.width()), + .height = base::checked_cast(size.height()), + .format = format, + .num_fds = base::checked_cast(planes_count), + .modifier = gbm_bo_get_modifier(created_bo)}; + // Store fds in a base::ScopedFDs vector. Will be released automatically. std::vector fds; for (size_t i = 0; i < static_cast(fd_data.num_fds); ++i) { - fds.emplace_back(GetPlaneFdForBo(bo, i)); + fds.emplace_back(GetPlaneFdForBo(created_bo, i)); fd_data.fds[i] = fds.back().get(); - fd_data.strides[i] = gbm_bo_get_stride_for_plane(bo, i); - fd_data.offsets[i] = gbm_bo_get_offset(bo, i); + fd_data.strides[i] = gbm_bo_get_stride_for_plane(created_bo, i); + fd_data.offsets[i] = gbm_bo_get_offset(created_bo, i); } - struct gbm_bo* bo_import = + struct gbm_bo* imported_bo = gbm_bo_import(device_, GBM_BO_IMPORT_FD_MODIFIER, &fd_data, flags); - if (bo_import) { - gbm_bo_destroy(bo_import); - break; + + if (imported_bo) { + valid_modifiers = true; + gbm_bo_destroy(imported_bo); } else { - gbm_bo_destroy(bo); - bo = nullptr; AddModifierToBlocklist(format, flags, fd_data.modifier); filtered_modifiers = GetFilteredModifiers(format, flags, filtered_modifiers); } + + if (!valid_modifiers || size != requested_size) { + gbm_bo_destroy(created_bo); + created_bo = nullptr; + } } - if (!bo) { - return nullptr; + + // If modifiers were successfully verified though `created_bo` is null here, + // it it means that the buffer created for verification could not be reused, + // ie: different size, so create it now with the `requested_size`. + if (valid_modifiers && !created_bo) { + created_bo = gbm_bo_create_with_modifiers( + device_, requested_size.width(), requested_size.height(), format, + filtered_modifiers.data(), filtered_modifiers.size()); + PLOG_IF(ERROR, !created_bo) << "Failed to create BO with modifiers."; } - return CreateBufferForBO(bo, format, size, flags); + return created_bo ? CreateBufferForBO(created_bo, format, size, flags) + : nullptr; } std::unique_ptr CreateBufferFromHandle(