Message-ID: <c1809502-e9b7-43f7-9d88-0e615bf1ff94@suse.de>
Date: Mon, 31 Mar 2025 09:13:59 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Adrián Larumbe <adrian.larumbe@...labora.com>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>,
 Boris Brezillon <boris.brezillon@...labora.com>,
 Rob Herring <robh@...nel.org>, Steven Price <steven.price@....com>,
 Liviu Dudau <liviu.dudau@....com>
Cc: kernel@...labora.com, linux-kernel@...r.kernel.org,
 dri-devel@...ts.freedesktop.org
Subject: Re: [RFC PATCH v2 0/6] Introduce sparse DRM shmem object allocations

Hi

Am 26.03.25 um 03:14 schrieb Adrián Larumbe:
> This patch series is a proposal for implementing sparse page allocations
> for shmem objects. It was initially motivated by a kind of BO managed by
> the Panfrost driver, the tiler heap, which grows on demand every time the
> GPU faults on a virtual address within its drm_mm-managed ranged.

I've looked at panfrost_gem.h and found that the driver's gem structure 
has grown quite a bit. It seems to have outgrown gem-shmem already.  I 
think you should consider pulling a copy of gem-shmem into the driver 
and building a dedicated memory manager on top.

Best regards
Thomas

>
> Because keeping a struct page pointer array that can describe the entire
> virtual range is wasteful when only a few backing pages have been
> allocated, at Collabora we thought a sparse allocation approach with
> xarrays was a more efficient choice.
>
> Since sparse and 'dense' DRM shmem objects must be managed slightly
> differently, the API is expanded to allow client drivers to create sparse
> objects and also to expand their page backing range, but everything else
> should remain as transparent as possible and be handled from within the DRM
> shmem system itself.
>
> Discussion of previus revision can be found here:
> https://lore.kernel.org/dri-devel/20250218232552.3450939-1-adrian.larumbe@collabora.com/
>
> Changelog:
>   v2:
>    - Removed patch with helper for non-blocking shmem page allocations.
>    - Moved page_array definitions away from scatterlist interface to hide
>    them from consumers.
>    - Refactored sg_alloc_append_table_from_pages() so that it now calls
>    sg_alloc_append_table_from_page_array() to avoid code duplication.
>    - Undid extension of __drm_gem_shmem_create() argument list so that a sparse
>    shmem object is now fully defined in a parent function.
>    - Moved check for absence of backing pages when putting an object into
>    drm_gem_shmem_put_pages()
>    - Added explanatory comments above DRM WARN'ings across yet unimplemented
>    shmem code paths, like kernel vmap's and UM mappings of sparse objects
>    - Created drm_gem helper for doing the actual sparse allocation, to keep
>    the interface aligned with the existing one with regular shmem objects.
>    - Split the body of drm_gem_shmem_get_sparse_pages_locked() into two separate
>    functions, one which performs the actual page allocation, and another
>    one that retrieves an sgtable.
>    - Expanded the argument list of drm_gem_shmem_get_sparse_pages() and its
>    children functions so that it takes an gfp mask, in the even that we would
>    want to do non-blocking allocations, for instance like when we wish to
>    avoid races with the shrinker memory reclaim path.
>    - Created shmem helper that returns whether an shmem object has any backing pages.
>
> TODO:
> The following items need to be worked on, and will be the subject of a v3 of this RFC:
>
>   - Handle the special case when some of the pages in a sparse allocation range are
>     already present, rather than bailing out immediately.
>   - Redefining panfrost_gem_object::sgts into an xarray or perhaps a sg_append_table
>     to avoid memory waste in allocating more sgtable pointers than we could need.
>   - Deciding on the rules for sparse shmem object's kmaps and UM maps.
>
> Adrián Larumbe (6):
>    lib/scatterlist.c: Support constructing sgt from page xarray
>    drm/shmem: Introduce the notion of sparse objects
>    drm/shmem: Implement sparse allocation of pages for shmem objects
>    drm/panfrost: Use shmem sparse allocation for heap BOs
>    drm/shmem: Add a helper to check object's page backing status
>    drm/panfrost/panthor: Take sparse objects into account for fdinfo
>
>   drivers/gpu/drm/drm_gem.c               | 117 +++++++++++
>   drivers/gpu/drm/drm_gem_shmem_helper.c  | 264 +++++++++++++++++++++++-
>   drivers/gpu/drm/panfrost/panfrost_gem.c |  14 +-
>   drivers/gpu/drm/panfrost/panfrost_gem.h |   2 +-
>   drivers/gpu/drm/panfrost/panfrost_mmu.c |  86 ++------
>   drivers/gpu/drm/panthor/panthor_gem.c   |   2 +-
>   include/drm/drm_gem.h                   |   6 +
>   include/drm/drm_gem_shmem_helper.h      |  29 ++-
>   include/linux/scatterlist.h             |  17 ++
>   lib/scatterlist.c                       | 175 +++++++++++-----
>   10 files changed, 579 insertions(+), 133 deletions(-)
>
>
> base-commit: 2f9d51740cc30e0d2c8a23a55b1e20cf2513c250
> --
> 2.48.1

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Powered by blists - more mailing lists