All of lore.kernel.org
 help / color / mirror / Atom feed
* [git pull] drm fixes for 5.19-rc7
@ 2022-07-15  3:36 ` Dave Airlie
  0 siblings, 0 replies; 12+ messages in thread
From: Dave Airlie @ 2022-07-15  3:36 UTC (permalink / raw)
  To: Linus Torvalds, Daniel Vetter; +Cc: dri-devel, LKML

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [git pull] drm fixes for 5.19-rc7
@ 2022-07-15  3:36 ` Dave Airlie
  0 siblings, 0 replies; 12+ messages in thread
From: Dave Airlie @ 2022-07-15  3:36 UTC (permalink / raw)
  To: Linus Torvalds, Daniel Vetter; +Cc: LKML, dri-devel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
  2022-07-15  3:36 ` Dave Airlie
@ 2022-07-15 17:45   ` pr-tracker-bot
  -1 siblings, 0 replies; 12+ messages in thread
From: pr-tracker-bot @ 2022-07-15 17:45 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Linus Torvalds, Daniel Vetter, LKML, dri-devel

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!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
@ 2022-07-15 17:45   ` pr-tracker-bot
  0 siblings, 0 replies; 12+ messages in thread
From: pr-tracker-bot @ 2022-07-15 17:45 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Daniel Vetter, Linus Torvalds, LKML, dri-devel

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!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
  2022-07-15  3:36 ` Dave Airlie
@ 2022-07-15 21:09   ` Nathan Chancellor
  -1 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2022-07-15 21:09 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Linus Torvalds, Daniel Vetter, dri-devel, LKML

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
@ 2022-07-15 21:09   ` Nathan Chancellor
  0 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2022-07-15 21:09 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Daniel Vetter, Linus Torvalds, LKML, dri-devel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
  2022-07-15 21:09   ` Nathan Chancellor
@ 2022-07-16 21:35     ` Linus Torvalds
  -1 siblings, 0 replies; 12+ messages in thread
From: Linus Torvalds @ 2022-07-16 21:35 UTC (permalink / raw)
  To: Nathan Chancellor, Matthew Auld, Nirmoy Das, Rodrigo Vivi,
	Thomas Hellström
  Cc: Dave Airlie, Daniel Vetter, dri-devel, LKML

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
@ 2022-07-16 21:35     ` Linus Torvalds
  0 siblings, 0 replies; 12+ messages in thread
From: Linus Torvalds @ 2022-07-16 21:35 UTC (permalink / raw)
  To: Nathan Chancellor, Matthew Auld, Nirmoy Das, Rodrigo Vivi,
	Thomas Hellström
  Cc: Daniel Vetter, LKML, dri-devel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
  2022-07-16 21:35     ` Linus Torvalds
@ 2022-07-16 22:08       ` Linus Torvalds
  -1 siblings, 0 replies; 12+ messages in thread
From: Linus Torvalds @ 2022-07-16 22:08 UTC (permalink / raw)
  To: Nathan Chancellor, Matthew Auld, Nirmoy Das, Rodrigo Vivi,
	Thomas Hellström
  Cc: Dave Airlie, Daniel Vetter, dri-devel, LKML

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

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

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1631 bytes --]

 drivers/gpu/drm/i915/i915_scatterlist.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_scatterlist.c b/drivers/gpu/drm/i915/i915_scatterlist.c
index f63b50b71e10..830dcd833d4e 100644
--- a/drivers/gpu/drm/i915/i915_scatterlist.c
+++ b/drivers/gpu/drm/i915/i915_scatterlist.c
@@ -81,7 +81,9 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct drm_mm_node *node,
 					      u64 region_start,
 					      u64 page_alignment)
 {
-	const u64 max_segment = round_down(UINT_MAX, page_alignment);
+	// Make sure max_segment (and thus segment_pages) is
+	// a power of two that fits in 32 bits.
+	const u64 max_segment = 1ul << 31;
 	u64 segment_pages = max_segment >> PAGE_SHIFT;
 	u64 block_size, offset, prev_end;
 	struct i915_refct_sgt *rsgt;
@@ -96,7 +98,7 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct drm_mm_node *node,
 
 	i915_refct_sgt_init(rsgt, node->size << PAGE_SHIFT);
 	st = &rsgt->table;
-	if (sg_alloc_table(st, DIV_ROUND_UP(node->size, segment_pages),
+	if (sg_alloc_table(st, round_up(node->size, segment_pages),
 			   GFP_KERNEL)) {
 		i915_refct_sgt_put(rsgt);
 		return ERR_PTR(-ENOMEM);
@@ -159,7 +161,7 @@ struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct ttm_resource *res,
 {
 	struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res);
 	const u64 size = res->num_pages << PAGE_SHIFT;
-	const u64 max_segment = round_down(UINT_MAX, page_alignment);
+	const u64 max_segment = 1u << 31;
 	struct drm_buddy *mm = bman_res->mm;
 	struct list_head *blocks = &bman_res->blocks;
 	struct drm_buddy_block *block;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
@ 2022-07-16 22:08       ` Linus Torvalds
  0 siblings, 0 replies; 12+ messages in thread
From: Linus Torvalds @ 2022-07-16 22:08 UTC (permalink / raw)
  To: Nathan Chancellor, Matthew Auld, Nirmoy Das, Rodrigo Vivi,
	Thomas Hellström
  Cc: Daniel Vetter, LKML, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

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

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1631 bytes --]

 drivers/gpu/drm/i915/i915_scatterlist.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_scatterlist.c b/drivers/gpu/drm/i915/i915_scatterlist.c
index f63b50b71e10..830dcd833d4e 100644
--- a/drivers/gpu/drm/i915/i915_scatterlist.c
+++ b/drivers/gpu/drm/i915/i915_scatterlist.c
@@ -81,7 +81,9 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct drm_mm_node *node,
 					      u64 region_start,
 					      u64 page_alignment)
 {
-	const u64 max_segment = round_down(UINT_MAX, page_alignment);
+	// Make sure max_segment (and thus segment_pages) is
+	// a power of two that fits in 32 bits.
+	const u64 max_segment = 1ul << 31;
 	u64 segment_pages = max_segment >> PAGE_SHIFT;
 	u64 block_size, offset, prev_end;
 	struct i915_refct_sgt *rsgt;
@@ -96,7 +98,7 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct drm_mm_node *node,
 
 	i915_refct_sgt_init(rsgt, node->size << PAGE_SHIFT);
 	st = &rsgt->table;
-	if (sg_alloc_table(st, DIV_ROUND_UP(node->size, segment_pages),
+	if (sg_alloc_table(st, round_up(node->size, segment_pages),
 			   GFP_KERNEL)) {
 		i915_refct_sgt_put(rsgt);
 		return ERR_PTR(-ENOMEM);
@@ -159,7 +161,7 @@ struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct ttm_resource *res,
 {
 	struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res);
 	const u64 size = res->num_pages << PAGE_SHIFT;
-	const u64 max_segment = round_down(UINT_MAX, page_alignment);
+	const u64 max_segment = 1u << 31;
 	struct drm_buddy *mm = bman_res->mm;
 	struct list_head *blocks = &bman_res->blocks;
 	struct drm_buddy_block *block;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
  2022-07-16 22:08       ` Linus Torvalds
@ 2022-07-17 18:48         ` Vivi, Rodrigo
  -1 siblings, 0 replies; 12+ messages in thread
From: Vivi, Rodrigo @ 2022-07-17 18:48 UTC (permalink / raw)
  To: Torvalds, Linus, nathan, Das, Nirmoy, Auld, Matthew, thomas.hellstrom
  Cc: dri-devel, daniel.vetter, airlied, linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [git pull] drm fixes for 5.19-rc7
@ 2022-07-17 18:48         ` Vivi, Rodrigo
  0 siblings, 0 replies; 12+ messages in thread
From: Vivi, Rodrigo @ 2022-07-17 18:48 UTC (permalink / raw)
  To: Torvalds, Linus, nathan, Das, Nirmoy, Auld, Matthew, thomas.hellstrom
  Cc: daniel.vetter, linux-kernel, dri-devel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-07-17 18:48 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-15  3:36 [git pull] drm fixes for 5.19-rc7 Dave Airlie
2022-07-15  3:36 ` Dave Airlie
2022-07-15 17:45 ` pr-tracker-bot
2022-07-15 17:45   ` pr-tracker-bot
2022-07-15 21:09 ` Nathan Chancellor
2022-07-15 21:09   ` Nathan Chancellor
2022-07-16 21:35   ` Linus Torvalds
2022-07-16 21:35     ` Linus Torvalds
2022-07-16 22:08     ` Linus Torvalds
2022-07-16 22:08       ` Linus Torvalds
2022-07-17 18:48       ` Vivi, Rodrigo
2022-07-17 18:48         ` Vivi, Rodrigo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.