[5.4-lts] AMD DCN (Navi/Renoir) FPU fixes #45
Conversation
|
I pulled your changes over and tried them with the RX 5700 XT. The driver loads correctly, so we didn't miss any FPU enter and exit. I think it's a good idea to make these changes. The original patch was fragile. It looks like the upstream fix is correct and if this makes future merges easier, so much the better. Thank you for looking at this, @myfreeweb ! |
|
I can't test these changes as I currently do not have access to my navi
machine, sorry. NorwegianRockCat's testing should be applicable to my RX
5500XT though
|
|
I would prefer to have asm/fpu/api.h in base. |
Sure, I'll prepare a patch
Well, these functions aren't really machine-dependent for us (since they just call our own common FPU ctx interface). |
They not implemented for every arch.
machine/fpu.h is arch dependent too and resolv to sys//include/fpu.h |
|
Hey nice job on the patch I'll only be able to give it a test in a few weeks however unfortunately. Need to make time to try out FreeBSD over a weekend and a few weekdays again. |
|
Sadly, under Renoir (Thinkpad T14, Ryzen 7 Pro 4750U) it still won't start X when I use the amdgpu DDX driver. I've also tried Wayland/Sway, same thing. I still get the same errors as here. I have to add that patch with the "fictitious pages", otherwise I get a crash in Haven't tested yet with the 5.5 WIP, though. I will try the |
|
What can I say, with the |
|
Yeah, that means 5.4 really wasn't ready for Renoir (it was |
Summary: With newer AMD GPUs (>=Navi,Renoir) there is FPU context usage in the amdgpu driver. The `kernel_fpu_begin/end` implementations in drm did not even allow nested begin-end blocks. This patch merges a more complete implementation into base as requested by @manu. The counter algorithm was tested (with the variables still being static) at freebsd/drm-kmod#45 In this version, the variables are moved into the linuxkpi module, the context is allocated with sysinit once, to avoid silly reallocations all over the place. --- Adding the file into `conf/files.ARCH` is quite annoying, would it be better to just ifdef the whole `linux_fpu.c` instead of excluding it from the build for arches that don't have fpu_kern(9)? --- By the way the #powerpc crowd really should implement fpu_kern(9) already ;) Test Plan: Ideally, someone should retest base with this patch + drm at valpackett/drm-kmod@5c7c900 with the new amdgpus. Reviewers: manu, #linuxkpi, #contributor_reviews_base, hselasky Reviewed By: manu, #linuxkpi, hselasky Subscribers: daniel.piecebypiece_yahoo.com, andrew, manu, emaste Tags: #linuxkpi Differential Revision: https://reviews.freebsd.org/D28061
…ream fix This will be replaced with "amdgpu: Wrap FPU dependent functions in dc20" in 5.6, for now just make our fix match that one closely.
Use a nesting level counter to ensure it works correctly.
valpackett
force-pushed
the
5.4-lts-navi
branch
from
222ac08 to
b322701
Compare
|
Since 5.5 is stable now, closing this |
FPU fixes from #40 (tested on both Navi and Renoir at FreeBSDDesktop/kms-drm#255) cherry-picked onto 5.4-lts.
Namely, this changes the fpu_begin/end calls to match the upstream ones torvalds/linux@7a8a343 (i.e. will be easy to switch to fully upstream code in 5.6) and makes our fpu_begin/end sections nestable.
Anyone with Navi or Renoir, please test! /cc @bcran @NorwegianRockCat @aiden-leong @DarkKirb @jiixyj @tsujp