mbox series

[v2,0/7] drm/vram-helper: Lock GEM BOs while they are mapped

Message ID 20201203140259.26580-1-tzimmermann@suse.de (mailing list archive)
Headers show
Series drm/vram-helper: Lock GEM BOs while they are mapped | expand

Message

Thomas Zimmermann Dec. 3, 2020, 2:02 p.m. UTC
GEM VRAM helpers used to pin the BO in their implementation of vmap, so
that they could not be relocated. In a recent discussion, [1] it became
clear that this is incorrect and that vmap should rather repend on the
reservation lock to prevent relocation. This patchset addresses the issue.

Version 2 of the patchset is a significant rework. In particular, the
helper function drm_gem_vram_vmap_unlocked() is gone and importers now
acquire the reservation locks by themselves. I took Christian's A-bs only
for patches that were not affected.

Patches 1 and 2 prepare the ast cursor code for the later changes.

Patch 3 pushes the BO locking into callers of vmap and vunmap. Besides
the VRAM helpers, this affects ast, vboxvideo and the generic fbdev
emulation.

Patches 4 to 6 clean up afterwards. BO pinning is not required any longer
and the VRAM-internal helpers are not needed.

Patch 7 adds documentation to vmap and vunmap in struct dma_buf_ops. It
hopefully reflects the discussion on the patchset's version 1.

Tested on ast with GEM VRAM and also on mgag200 to verify that the fbdev
change does not interfere with GEM SHMEM.

v2:
	* make importers acquire resv locks by themselves
	* document dma-buf vamp/vunmap ops

Thomas Zimmermann (7):
  drm/ast: Don't pin cursor source BO explicitly during update
  drm/ast: Only map cursor BOs during updates
  drm/vram-helper: Move BO locking from vmap code into callers
  drm/vram-helper: Remove pinning from drm_gem_vram_{vmap,vunmap}()
  drm/vram-helper: Remove vmap reference counting
  drm/vram-helper: Simplify vmap implementation
  dma-buf: Write down some rules for vmap usage

 Documentation/gpu/todo.rst            | 15 +++++
 drivers/gpu/drm/ast/ast_cursor.c      | 70 +++++++++++++---------
 drivers/gpu/drm/ast/ast_drv.h         |  2 -
 drivers/gpu/drm/drm_client.c          | 31 ++++++++++
 drivers/gpu/drm/drm_fb_helper.c       | 10 +++-
 drivers/gpu/drm/drm_gem_vram_helper.c | 85 ++++++---------------------
 drivers/gpu/drm/vboxvideo/vbox_mode.c | 11 ++--
 include/drm/drm_client.h              |  2 +
 include/drm/drm_gem.h                 |  4 ++
 include/drm/drm_gem_vram_helper.h     | 17 +-----
 include/linux/dma-buf.h               | 45 ++++++++++++++
 11 files changed, 175 insertions(+), 117 deletions(-)

--
2.29.2

Comments

Thomas Zimmermann Dec. 3, 2020, 2:05 p.m. UTC | #1
Am 03.12.20 um 15:02 schrieb Thomas Zimmermann:
> GEM VRAM helpers used to pin the BO in their implementation of vmap, so
> that they could not be relocated. In a recent discussion, [1] it became

Grrr, [1] was again supposed to point to the discussion at

   https://patchwork.freedesktop.org/patch/400054/?series=83765&rev=1

> clear that this is incorrect and that vmap should rather repend on the
> reservation lock to prevent relocation. This patchset addresses the issue.
> 
> Version 2 of the patchset is a significant rework. In particular, the
> helper function drm_gem_vram_vmap_unlocked() is gone and importers now
> acquire the reservation locks by themselves. I took Christian's A-bs only
> for patches that were not affected.
> 
> Patches 1 and 2 prepare the ast cursor code for the later changes.
> 
> Patch 3 pushes the BO locking into callers of vmap and vunmap. Besides
> the VRAM helpers, this affects ast, vboxvideo and the generic fbdev
> emulation.
> 
> Patches 4 to 6 clean up afterwards. BO pinning is not required any longer
> and the VRAM-internal helpers are not needed.
> 
> Patch 7 adds documentation to vmap and vunmap in struct dma_buf_ops. It
> hopefully reflects the discussion on the patchset's version 1.
> 
> Tested on ast with GEM VRAM and also on mgag200 to verify that the fbdev
> change does not interfere with GEM SHMEM.
> 
> v2:
> 	* make importers acquire resv locks by themselves
> 	* document dma-buf vamp/vunmap ops
> 
> Thomas Zimmermann (7):
>    drm/ast: Don't pin cursor source BO explicitly during update
>    drm/ast: Only map cursor BOs during updates
>    drm/vram-helper: Move BO locking from vmap code into callers
>    drm/vram-helper: Remove pinning from drm_gem_vram_{vmap,vunmap}()
>    drm/vram-helper: Remove vmap reference counting
>    drm/vram-helper: Simplify vmap implementation
>    dma-buf: Write down some rules for vmap usage
> 
>   Documentation/gpu/todo.rst            | 15 +++++
>   drivers/gpu/drm/ast/ast_cursor.c      | 70 +++++++++++++---------
>   drivers/gpu/drm/ast/ast_drv.h         |  2 -
>   drivers/gpu/drm/drm_client.c          | 31 ++++++++++
>   drivers/gpu/drm/drm_fb_helper.c       | 10 +++-
>   drivers/gpu/drm/drm_gem_vram_helper.c | 85 ++++++---------------------
>   drivers/gpu/drm/vboxvideo/vbox_mode.c | 11 ++--
>   include/drm/drm_client.h              |  2 +
>   include/drm/drm_gem.h                 |  4 ++
>   include/drm/drm_gem_vram_helper.h     | 17 +-----
>   include/linux/dma-buf.h               | 45 ++++++++++++++
>   11 files changed, 175 insertions(+), 117 deletions(-)
> 
> --
> 2.29.2
>