Chromium's minigbm uses incorrect path for loading radeonsi_dri driver.

[Bram Stolk]

[NOTE] This is for the HWACC build of chromium, from snap channel latest/candidate/hwacc

$ snap info chromium | grep installed
installed: 107.0.5304.68-hwacc (2301) 172MB -

This hwacc branch uses minigbm over the Ubuntu-supplied libgbm from Mesa origin.
(Unlike the Mesa variant, the minigbm bundled by chromium can do YUV buffers, like NV12.)
Regular builds of the chromium snap will not use minigbm, and thus not exhibit this behaviour!

Something goes wrong with the expansion of the macro DRI_DRIVER_DIR

$ strings /snap/chromium/current/usr/lib/chromium-browser/chrome | grep radeonsi_
/usr/lib64/dri/radeonsi_dri.so
DRI_DRIVER_DIR/radeonsi_dri.so
dlopen(radeonsi_dri.so) failed with error:

The DRI_DRIVER_DIR was not properly expanded to /usr/lib/x86_64-linux-gnu here.

I've also built current HEAD of chromium source code manually, and did the same test on that, and that version is unaffected:

$ strings out/Default/chrome | grep radeonsi_
dlopen(radeonsi_dri.so) failed with error:
/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so
/usr/lib64/dri/radeonsi_dri.so

A partner found this change to third_party/minigbm/src/amdgpu.c:27 to be working:

//#define DRI_PATH STRINGIZE(DRI_DRIVER_DIR/radeonsi_dri.so)
#define DRI_PATH "/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so"

But it is better to understand why this expansion failed.
And it is possibly fixed in later chromium releases, if not, we should look how our snap building differs from the official Linux building instructions for chromium.

In any case, we should track this.

OS: Ubuntu 22.10
Package: chromium
Snap version: 107.0.5304.68-hwacc
Channel: latest/candidate/hwacc

Expectations: correctly loading driver from correct location.
Actual: using incorrect location.

I will follow up by testing this on a machine with an AMD GPU.

[Nathan Teodosio (nteodosio)]

DRI_DRIVER_DIR is supposed to be defined in build/config/linux/BUILD.gn, which relies on pkg-config.py output.

The commits 5e90d57 and ac530f7 in amdgpu branch[1] were meant to inspect this more closely by turning debugging statements on. The log[2] says dridriverdir is found:

  line>dridriverdir=/usr/lib/x86_64-linux-gnu/dri
 Variable declaration, 'dridriverdir' has value '/usr/lib/x86_64-linux-gnu/dri'
dridriverdir: /usr/lib/x86_64-linux-gnu/dri

Then I made small changes (which include a runtime DRI_PATH log statement) that shouldn't affect that output and built again, but they affected it as that output mysteriously vanishes.

[1] https://code.launchpad.net/~nteodosio/chromium-browser/+git/chromium-browser/+ref/amdgpu
[2] https://launchpadlibrarian.net/649525665/buildlog_snap_ubuntu_focal_amd64_chromium-amdgpu_BUILDING.txt.gz
[3] https://launchpadlibrarian.net/649584762/buildlog_snap_ubuntu_focal_amd64_chromium-amdgpu_BUILDING.txt.gz

[Bram Stolk (b-stolk)]

Thanks for testing. So, your first log contains this error, though:
```
/build/chromium/parts/chromium/build
ERROR at //build/config/linux/dri/BUILD.gn:11:20: Script returned non-zero exit code.
  dri_driver_dir = exec_script(pkg_config_script,
                   ^----------
Current dir: /build/chromium/parts/chromium/build/out/Release/
Command: python3 /build/chromium/parts/chromium/build/build/config/linux/pkg-config.py -p /snap/gnome-3-38-2004/current/usr/bin/pkg-config --dridriverdir dri
Returned 1 and printed out:

Error from pkg-config.

stderr:
```

[Bram Stolk (b-stolk)]

I confirmed that DRI_DRIVER_DIR never makes it to the compiler flags.

I added an error pragma to catch an empty definition and no definition in attached patch.

The result when building w that patch (on 107 chromium):

FAILED: obj/third_party/minigbm/minigbm/amdgpu.o
../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF obj/third_party/minigbm/minigbm/amdgpu.o.d -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_OZONE=1 -DOFFICIAL_BUILD -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DNO_UNWIND_TABLES -D_GNU_SOURCE -DCR_CLANG_REVISION=\"llvmorg-16-init-4609-g025a5b22-2\" -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS -DCR_LIBCXX_REVISION=e2f63a1a48a3cdcacbfc212236050ca5deeacc30 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DDRV_AMDGPU -DDRV_I915 -I../.. -Igen -I../../buildtools/third_party/libc++ -I../../third_party/minigbm/src -fno-delete-null-pointer-checks -fno-ident -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -fno-unwind-tables -fno-asynchronous-unwind-tables -fPIC -pthread -fcolor-diagnostics -fmerge-all-constants -fcrash-diagnostics-dir=../../tools/clang/crashreports -mllvm -instcombine-lower-dbg-declare=0 -ffp-contract=off -flto=thin -fsplit-lto-unit -fwhole-program-vtables -fcomplete-member-pointers -m64 -msse3 -ffile-compilation-dir=. -no-canonical-prefixes -ftrivial-auto-var-init=pattern -O2 -fdata-sections -ffunction-sections -fno-unique-section-names -fno-omit-frame-pointer -g0 -fprofile-use=../../chrome/build/pgo_profiles/chrome-linux-5304-1666369430-50bfc04797db1acc0805fe1425e83d1bbe3b92f6.profdata -Wno-profile-instr-unprofiled -Wno-profile-instr-out-of-date -Wno-backend-plugin -fvisibility=hidden -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang raw-ref-template-as-trivial-member -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wall -Wno-unused-variable -Wno-c++11-narrowing -Wno-unused-but-set-variable -Wno-misleading-indentation -Wno-missing-field-initializers -Wno-unused-parameter -Wloop-analysis -Wno-unneeded-internal-declaration -Wenum-compare-conditional -Wno-psabi -Wno-ignored-pragma-optimize -Wno-deprecated-builtins -Wno-bitfield-constant-conversion -I/usr/include/libdrm -std=c11 -c ../../third_party/minigbm/src/amdgpu.c -o obj/third_party/minigbm/minigbm/amdgpu.o
../../third_party/minigbm/src/amdgpu.c:27:3: error: "DRI_DRIVER_DIR is not defined."
# error "DRI_DRIVER_DIR is not defined."
        ^
1 error generated.
[1511/54251] CXX obj/third_party/protobuf/protoc_lib/csharp_message.o
ninja: build stopped: subcommand failed.
Failed to run 'override-build': Exit code was 1.

If will probably make sense to add a default path for that if the symbol has not been defined.
I'll work on that.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "amdtest:check-for-empty-define.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Nathan Teodosio (nteodosio)]

> your first log contains this error

Indeed, I forced a error out with sys.exit[1] when it reached that step just so as to not waste time building the whole thing.

Since you'll be working on that, I'm reassigning.

[1] https://git.launchpad.net/~nteodosio/chromium-browser/commit/?id=5e90d5716f7214ce6e2818572c218ef95ae3e798

[Bram Stolk (b-stolk)]

This part is missing from the compiler command line -DDRI_DRIVER_DIR=\"/usr/lib/x86_64-linux-gnu/dri\"

I've verified that this can be fixed by adding a configuration to minigbm's BUILD.gn I will create a patch that addresses this.

UPDATE: Adding the config reference causes the definition to be included, unfortunately with quotationmarks that end up in the binary:

$ strings /snap/chromium/current/usr/lib/chromium-browser/chrome | grep radeonsi
/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so
"/usr/lib/x86_64-linux-gnu/dri"/radeonsi_dri.so
radeonsi
dlopen(radeonsi_dri.so) failed with error:

[Bram Stolk (b-stolk)]

I've sent an up-stream fix to Gerrit:
https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/4263398

Fix to snap-from-source will follow.

[Bram Stolk (b-stolk)]

MR for snap-from-source is here:
https://code.launchpad.net/~b-stolk/chromium-browser/+git/snap-from-source/+merge/437529

[Bram Stolk (b-stolk)]

Note that when logging into build VM after building the chromium-hwacc snap:

multipass shell snapcraft-chromium

I see:
# pkg-config --variable=dridriverdir dri
/usr/lib/x86_64-linux-gnu/dri

And not the expected
/snap/gnome-3-38-2004/current/usr/lib/x86_64-linux-gnu/dri

this value is from here, btw:
/usr/lib/x86_64-linux-gnu/pkgconfig/dri.pc

Maybe we should just create a symbolic link, and also fix the escaped quotes?
(Or fix amdgpu.c to not stringify)