[PATCH 0/6] drm/msm/gem: Drop use of pages for imported dma-bufs

[PATCH 0/6] drm/msm/gem: Drop use of pages for imported dma-bufs

[Rob Clark]

The use of msm_obj->pages _mostly_ served to mark that the obj is
resident in memory.  Mapping to the GPU, etc, all works in terms of
the sgt.

The exception was the fault path.  Resolve this by using dma_buf_mmap()
for mmap'ing imported buffers, like we are supposed to.  With that and
a bit more cleanup we can drop the use of drm_prime_sg_to_page_array().

This doesn't solve the issues with DMABUF_DEBUG, mapping to the GPU,
either via iommu_map_sgtable() (display, and TTBR1 for GPU, aka the
kernel managed mappings), or msm_iommu_pagetable_map() (userspace
managed mappings), still requires pages under the hood.  That will
be harder to solve.  But this is a step along the way.

Lightly tested.. in that I don't actually have a good way to test
importing buffers that weren't first exported by drm/msm.  And I'm
headed to XDC soon.

Rob Clark (6):
  drm/msm/gem: Add helper to check if resident
  drm/msm/gem: Mmap imported dma-bufs via the exporter
  drm/msm/gem: Use get_pages() directly in vmap path
  drm/msm/gem: Stop exposing a page array
  drm/msm/gem: Stop calling get_pages() for imported dma-bufs
  drm/msm/gem: Drop use of drm_prime_sg_to_page_array()

 drivers/gpu/drm/msm/msm_gem.c       | 107 +++++++++++++++++-----------
 drivers/gpu/drm/msm/msm_gem.h       |  10 ++-
 drivers/gpu/drm/msm/msm_gem_prime.c |   5 +-
 drivers/gpu/drm/msm/msm_gem_vma.c   |   8 +--
 4 files changed, 78 insertions(+), 52 deletions(-)

-- 
2.55.0

[PATCH 2/6] drm/msm/gem: Mmap imported dma-bufs via the exporter

[Rob Clark]

This was always the way it was supposed to work, and when we drop the
page array for imported dma-bufs our fault handling path will no longer
work.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_gem.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index e8390ebd5dd5..c90336b3b231 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -23,6 +23,8 @@
 #include "msm_gpu.h"
 #include "msm_kms.h"
 
+MODULE_IMPORT_NS("DMA_BUF");
+
 static void update_device_mem(struct msm_drm_private *priv, ssize_t size)
 {
 	uint64_t total_mem = atomic64_add_return(size, &priv->total_mem);
@@ -338,6 +340,9 @@ static vm_fault_t msm_gem_fault(struct vm_fault *vmf)
 	int err;
 	vm_fault_t ret;
 
+	if (drm_WARN_ON_ONCE(obj->dev, drm_gem_is_imported(obj)))
+		return VM_FAULT_SIGBUS;
+
 	/*
 	 * vm_ops.open/drm_gem_mmap_obj and close get and put
 	 * a reference on obj. So, we dont need to hold one here.
@@ -1126,6 +1131,25 @@ static int msm_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 
+	if (drm_gem_is_imported(obj)) {
+		int ret;
+
+		/* Reset both vm_ops and vm_private_data, so we don't end up with
+		 * vm_ops pointing to our implementation if the dma-buf backend
+		 * doesn't set those fields.
+		 */
+		vma->vm_private_data = NULL;
+		vma->vm_ops = NULL;
+
+		ret = dma_buf_mmap(obj->dma_buf, vma, 0);
+
+		/* Drop the reference drm_gem_mmap_obj() acquired.*/
+		if (!ret)
+			drm_gem_object_put(obj);
+
+		return ret;
+	}
+
 	vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
 	vma->vm_page_prot = msm_gem_pgprot(msm_obj, vma_get_page_prot(vma));
 
-- 
2.55.0

Re: [PATCH 2/6] drm/msm/gem: Mmap imported dma-bufs via the exporter

[Christian König]

On 9/26/26 20:30, Rob Clark wrote:
> This was always the way it was supposed to work, and when we drop the
> page array for imported dma-bufs our fault handling path will no longer
> work.
> 
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>

Nice to see that finally happening.

Reviewed-by: Christian König <christian.koenig@amd.com> for this patch here, Acked-by: Christian König <christian.koenig@amd.com> for the rest of the series.

Regards,
Christian.

> ---
>  drivers/gpu/drm/msm/msm_gem.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index e8390ebd5dd5..c90336b3b231 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -23,6 +23,8 @@
>  #include "msm_gpu.h"
>  #include "msm_kms.h"
>  
> +MODULE_IMPORT_NS("DMA_BUF");
> +
>  static void update_device_mem(struct msm_drm_private *priv, ssize_t size)
>  {
>  	uint64_t total_mem = atomic64_add_return(size, &priv->total_mem);
> @@ -338,6 +340,9 @@ static vm_fault_t msm_gem_fault(struct vm_fault *vmf)
>  	int err;
>  	vm_fault_t ret;
>  
> +	if (drm_WARN_ON_ONCE(obj->dev, drm_gem_is_imported(obj)))
> +		return VM_FAULT_SIGBUS;
> +
>  	/*
>  	 * vm_ops.open/drm_gem_mmap_obj and close get and put
>  	 * a reference on obj. So, we dont need to hold one here.
> @@ -1126,6 +1131,25 @@ static int msm_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct
>  {
>  	struct msm_gem_object *msm_obj = to_msm_bo(obj);
>  
> +	if (drm_gem_is_imported(obj)) {
> +		int ret;
> +
> +		/* Reset both vm_ops and vm_private_data, so we don't end up with
> +		 * vm_ops pointing to our implementation if the dma-buf backend
> +		 * doesn't set those fields.
> +		 */
> +		vma->vm_private_data = NULL;
> +		vma->vm_ops = NULL;
> +
> +		ret = dma_buf_mmap(obj->dma_buf, vma, 0);
> +
> +		/* Drop the reference drm_gem_mmap_obj() acquired.*/
> +		if (!ret)
> +			drm_gem_object_put(obj);
> +
> +		return ret;
> +	}
> +
>  	vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
>  	vma->vm_page_prot = msm_gem_pgprot(msm_obj, vma_get_page_prot(vma));
>