From 3471229c2a19a8228612cba91f38ad690d9f8fe6 Mon Sep 17 00:00:00 2001 From: Than Ngo Date: Fri, 3 Nov 2023 13:19:32 +0100 Subject: [PATCH] backport upstream patch, workaround for buggy Nvidia drivers --- ...a-use-separate-bo-to-verify-modifier.patch | 164 ++++++++++++++++++ chromium.spec | 4 + 2 files changed, 168 insertions(+) create mode 100644 chromium-119-nvidia-use-separate-bo-to-verify-modifier.patch diff --git a/chromium-119-nvidia-use-separate-bo-to-verify-modifier.patch b/chromium-119-nvidia-use-separate-bo-to-verify-modifier.patch new file mode 100644 index 00000000..717d8974 --- /dev/null +++ b/chromium-119-nvidia-use-separate-bo-to-verify-modifier.patch @@ -0,0 +1,164 @@ +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( diff --git a/chromium.spec b/chromium.spec index 0c9c34e0..af97fd45 100644 --- a/chromium.spec +++ b/chromium.spec @@ -413,6 +413,9 @@ Patch352: chromium-117-workaround_for_crash_on_BTI_capable_system.patch Patch400: chromium-119-dont-redefine-ATSPI-version-macros.patch # fix build error, nullptr_t without namespace std:: Patch401: chromium-119-nullptr_t-without-namespace-std.patch +# workaround for buggy Nvidia drivers fail to return FDs for planes +# of a BO which had already an imported BO destroyed before. +Patch402: chromium-119-nvidia-use-separate-bo-to-verify-modifier.patch # Use chromium-latest.py to generate clean tarball from released build tarballs, found here: # http://build.chromium.org/buildbot/official/ @@ -1014,6 +1017,7 @@ udev. %patch -P400 -p1 -R -b .revert-dont-redefine-ATSPI-version-macros.patch %patch -P401 -p1 -b .nullptr_t-without-namespace-std +%patch -P402 -p1 -b .nvidia-use-separate-bo-to-verify-modifiers # Change shebang in all relevant files in this directory and all subdirectories # See `man find` for how the `-exec command {} +` syntax works