| Message ID | CAPM=9twM75GDM9t+9-CSCPDZG3QdcEpQ-X+FzQ4CLUCM7cKLkw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [git,pull] drm fixes for 5.19-rc7 | expand |
The pull request you sent on Fri, 15 Jul 2022 13:36:17 +1000:
> git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-07-15
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/fcd1b2b9c7b085e9c200f73c079b322eb8c666f9
Thank you!
On Fri, Jul 15, 2022 at 01:36:17PM +1000, Dave Airlie wrote: > Matthew Auld (1): > drm/i915/ttm: fix sg_table construction This patch breaks i386_defconfig with both GCC and clang: ld: drivers/gpu/drm/i915/i915_scatterlist.o: in function `i915_rsgt_from_mm_node': i915_scatterlist.c:(.text+0x1a7): undefined reference to `__udivdi3' ld.lld: error: undefined symbol: __udivdi3 >>> referenced by i915_scatterlist.c >>> gpu/drm/i915/i915_scatterlist.o:(i915_rsgt_from_mm_node) in archive drivers/built-in.a It was reported by Stephen in -next [1] and there is a fix [2] that works for me but it doesn't appear to be applied yet (at least, it is not in drm-intel-fixes at the moment). It is a little disappointing to see new build errors being introduced before -rc7, especially when visible with a defconfig... [1]: https://lore.kernel.org/20220713221454.67bb20df@canb.auug.org.au/ [2]: https://lore.kernel.org/20220712174050.592550-1-matthew.auld@intel.com/ Cheers, Nathan
On Fri, Jul 15, 2022 at 2:09 PM Nathan Chancellor <nathan@kernel.org> wrote: > > On Fri, Jul 15, 2022 at 01:36:17PM +1000, Dave Airlie wrote: > > Matthew Auld (1): > > drm/i915/ttm: fix sg_table construction > > This patch breaks i386_defconfig with both GCC and clang: > > ld: drivers/gpu/drm/i915/i915_scatterlist.o: in function `i915_rsgt_from_mm_node': > i915_scatterlist.c:(.text+0x1a7): undefined reference to `__udivdi3' Yeah, we definitely don't want arbitrary 64x64 divides in the kernel, and the fact that we don't include libgcc.a once again caught this horrid issue. The offending code is if (sg_alloc_table(st, DIV_ROUND_UP(node->size, segment_pages), GFP_KERNEL)) { and I have to say that all of those games with "u64 page_alignment" that commit aff1e0b09b54 ("drm/i915/ttm: fix sg_table construction") introduces are absolutely disgusting. And they are just *pointlessly* disgusting. Why is that "page_alignment" a "u64"? And why is it a "size", instead of being a "number of bits"? The code literally does things like const u64 max_segment = round_down(UINT_MAX, page_alignment); which means that (a) page_alignment must be a power-of-two for this to work (round_down() only works in powers of two) (b) the result obviously must fit in an "unsigned int", since it's rounding down a UINT_MAX! So (a) makes it doubtful that "page_alignment" should have been a value (as opposed to mask), and (b) then questions why was that made an "u64" value when it cannot have a u64 range? And if max_segments cannot have a 64-bit range, then segment_pages here: u64 segment_pages = max_segment >> PAGE_SHIFT; sure cannot. Fixing those then uncovers other things: len = min(block_size, max_segment - sg->length); now complains about mixing types ("max_segment - sg->length" being u32), because 'block_size' is 64, bit, and that does seem to make some amount of sense: block_size = node->size << PAGE_SHIFT; with the 'node->size' being from drm_mm_node, and that size is a 'u64'. That I *could* see being more than 32 bits on a 64-bit architecture. Ok. But then that means that 'len' cannot be a 64-bit value either, and it should probably have been u32 len; .. len = min_t(u64, block_size, max_segment - sg->length); and that would just have been a lot nicer on 32-bit x86, avoiding a lot of pointlessly 64-bit things. That said, even those type simplifications do not fix the fundamental issue. That "DIV_ROUND_UP()" still ends up being a 64-bit divide, although now it's at least a "64-by-32" bit divide. Which needs to be handled by "do_div()" rather than the generic DIV_ROUND_UP() helper, because sadly, at least gcc still generates a full __udivdi3() even for the 64-by-32 divides. Can Intel GPU people please take a look? Linus
On Sat, Jul 16, 2022 at 2:35 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > That said, even those type simplifications do not fix the fundamental > issue. That "DIV_ROUND_UP()" still ends up being a 64-bit divide, > although now it's at least a "64-by-32" bit divide. Hmm. The "DIV_ROUND_UP()" issue could be solved by just making the rule be that the max_segment size is always a power of two. Then you don't need the (expensive!) DIV_ROUND_UP(), and can just use the regular "round_up()" that works on powers-of-two. And the simplest way to do that is to just make "max_segments" be 2GB. The whole "round_down(UINT_MAX, page_alignment)" seems entirely pointless. Do you really want segments that are some odd number just under the 4GB mark, and force expensive divides? For consistency, I used the same value in i915_rsgt_from_buddy_resource(). I have no idea if that makes sense. Anyway, the attached patch is COMPLETELY UNTESTED. But it at least seems to compile. Maybe. Linus
On Sat, 2022-07-16 at 15:08 -0700, Linus Torvalds wrote: > On Sat, Jul 16, 2022 at 2:35 PM Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > That said, even those type simplifications do not fix the > > fundamental > > issue. That "DIV_ROUND_UP()" still ends up being a 64-bit divide, > > although now it's at least a "64-by-32" bit divide. > > Hmm. The "DIV_ROUND_UP()" issue could be solved by just making the > rule be that the max_segment size is always a power of two. > > Then you don't need the (expensive!) DIV_ROUND_UP(), and can just use > the regular "round_up()" that works on powers-of-two. > > And the simplest way to do that is to just make "max_segments" be > 2GB. > > The whole "round_down(UINT_MAX, page_alignment)" seems entirely > pointless. Do you really want segments that are some odd number just > under the 4GB mark, and force expensive divides? I fully agree with you that if we have only things at 32bit we could use the round up and avoid the division. > > For consistency, I used the same value in > i915_rsgt_from_buddy_resource(). I have no idea if that makes sense. > > Anyway, the attached patch is COMPLETELY UNTESTED. But it at least > seems to compile. Maybe. Thanks. We should check this. Meanwhile I'd like to say that the team had worked already to fix the horrible 32 vs 64 bits inconsistency and the build breakage already. The fix [1] was merged Jul 13. [1] https://patchwork.freedesktop.org/patch/493637/?series=106260&rev=1 I'm the one to blame for not having propagated this along with the latest drm-intel-fixes round. Please accept my apologies. I will check right now why this was missed on my side and check how to propagate quickly. Sorry, Rodrigo. > > Linus
Hi Linus, This is the regular fixes pull for this week. This has a bunch of amdgpu fixes, major one reverts the buddy allocator until it can be tested more, otherwise just small ones, then i915 has a bunch of fixes. The outstanding firmware regressions reported by phoronix will hopefully be dealt with ASAP. Regards, Dave. drm-fixes-2022-07-15: drm fixes for 5.19-rc7 amdgpu: - revert buddy allocator support for now - DP MST blank screen fix for specific platforms - MEC firmware check fix for GC 10.3.7 - Deep color fix for DCE - Fix possible divide by 0 - Coverage blend mode fix - Fix cursor only commit timestamps i915: - Selftest fix - TTM fix sg_table construction - Error return fixes - Fix a performance regression related to waitboost - Fix GT resets The following changes since commit 3590b44b9434af1b9c81c3f40189087ed4fe3635: Merge tag 'drm-misc-fixes-2022-07-07-1' of ssh://git.freedesktop.org/git/drm/drm-misc into drm-fixes (2022-07-12 10:44:40 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-07-15 for you to fetch changes up to 093f8d8f10aa22935bc8bf7100700f714ebaba9c: Merge tag 'amd-drm-fixes-5.19-2022-07-13' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes (2022-07-15 11:26:20 +1000) ---------------------------------------------------------------- drm fixes for 5.19-rc7 amdgpu: - revert buddy allocator support for now - DP MST blank screen fix for specific platforms - MEC firmware check fix for GC 10.3.7 - Deep color fix for DCE - Fix possible divide by 0 - Coverage blend mode fix - Fix cursor only commit timestamps i915: - Selftest fix - TTM fix sg_table construction - Error return fixes - Fix a performance regression related to waitboost - Fix GT resets ---------------------------------------------------------------- Andrzej Hajda (1): drm/i915/selftests: fix subtraction overflow bug Arunpravin Paneer Selvam (1): Revert "drm/amdgpu: add drm buddy support to amdgpu" Chris Wilson (3): drm/i915/gt: Serialize GRDOM access between multiple engine resets drm/i915/gt: Serialize TLB invalidates with GT resets drm/i915/gem: Look for waitboosting across the whole object prior to individual waits Dan Carpenter (2): drm/i915/gvt: IS_ERR() vs NULL bug in intel_gvt_update_reg_whitelist() drm/i915/selftests: fix a couple IS_ERR() vs NULL tests Daniele Ceraolo Spurio (1): drm/i915/guc: ADL-N should use the same GuC FW as ADL-S Dave Airlie (3): Merge tag 'drm-misc-fixes-2022-07-14' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge tag 'drm-intel-fixes-2022-07-13' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'amd-drm-fixes-5.19-2022-07-13' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Fangzhi Zuo (1): drm/amd/display: Ignore First MST Sideband Message Return Error Hangyu Hua (1): drm/i915: fix a possible refcount leak in intel_dp_add_mst_connector() Mario Kleiner (1): drm/amd/display: Only use depth 36 bpp linebuffers on DCN display engines. Matthew Auld (1): drm/i915/ttm: fix sg_table construction Melissa Wen (1): drm/amd/display: correct check of coverage blend mode Michel Dänzer (1): drm/amd/display: Ensure valid event timestamp for cursor-only commits Prike Liang (1): drm/amdkfd: correct the MEC atomic support firmware checking for GC 10.3.7 Rodrigo Vivi (1): Merge tag 'gvt-fixes-2022-07-11' of https://github.com/intel/gvt-linux into drm-intel-fixes Thomas Hellström (1): drm/i915: Fix vm use-after-free in vma destruction Yefim Barashkin (1): drm/amd/pm: Prevent divide by zero drivers/gpu/drm/Kconfig | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 97 ++---- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 359 +++++++++------------ drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h | 89 ----- drivers/gpu/drm/amd/amdkfd/kfd_device.c | 2 + drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 84 ++++- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 8 + .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 17 + drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 11 +- drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 2 + drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 11 +- drivers/gpu/drm/i915/gem/i915_gem_wait.c | 34 ++ drivers/gpu/drm/i915/gt/intel_gt.c | 15 +- drivers/gpu/drm/i915/gt/intel_reset.c | 37 ++- drivers/gpu/drm/i915/gt/selftest_lrc.c | 8 +- drivers/gpu/drm/i915/gvt/cmd_parser.c | 6 +- drivers/gpu/drm/i915/i915_scatterlist.c | 19 +- drivers/gpu/drm/i915/i915_scatterlist.h | 6 +- drivers/gpu/drm/i915/intel_region_ttm.c | 10 +- drivers/gpu/drm/i915/intel_region_ttm.h | 3 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +- .../gpu/drm/i915/selftests/intel_memory_region.c | 21 +- drivers/gpu/drm/i915/selftests/mock_region.c | 3 +- 24 files changed, 433 insertions(+), 422 deletions(-) delete mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.h