Re: [PATCH v4 04/33] drm/pagemap: Add DRM pagemap

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Feb 07, 2025 at 09:34:00AM +0100, Thomas Hellström wrote:
> On Wed, 2025-01-29 at 11:51 -0800, Matthew Brost wrote:
> > From: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > 
> > Introduce drm_pagemap ops to map and unmap dma to VRAM resources. In
> > the
> > local memory case it's a matter of merely providing an offset into
> > the
> > device's physical address. For future p2p the map and unmap functions
> > may
> > encode as needed.
> > 
> > Similar to how dma-buf works, let the memory provider (drm_pagemap)
> > provide
> > the mapping functionality.
> 

Trying to parse all of this. 

> It should be noted that the long term idea for dma mapping is to have
> that done by the client instead of by the memory provider, which Jason

- Client here is the device mapping the memory.
- Memory provider is the device where the memory is located?

Did I get this correct?

> reminded me of in a discussion on dri-devel. The dma-mapping here is
> modeled after how it's done for dma-buf, where the exporter maps dma.
> 
> So following that, it might be that we should move these dma-mapping
> ops to the drm_gpusvm().
> 

So we move ops to the local client (gpusvm) rather than remote device,
right?

> The situation I can think of, where this might be a problem is that if
> the device-private struct page to dma address mapping is not known to
> the client.
>

I'm not following this but I agree if dma mapping at the client we need
the remote device structure given how the dma mapping API works.

So to wrap it up - what, if anything, do you think we need to do to this
individual patch as part of this series?

Matt

> /Thomas
> 
> 
> 
> 
> 
> > 
> > v3:
> >  - Move to drm level include
> > v4:
> >  - Fix kernel doc (G.G.)
> > 
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > ---
> >  include/drm/drm_pagemap.h | 105
> > ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 105 insertions(+)
> >  create mode 100644 include/drm/drm_pagemap.h
> > 
> > diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> > new file mode 100644
> > index 000000000000..2b610ccf7e30
> > --- /dev/null
> > +++ b/include/drm/drm_pagemap.h
> > @@ -0,0 +1,105 @@
> > +/* SPDX-License-Identifier: MIT */
> > +#ifndef _DRM_PAGEMAP_H_
> > +#define _DRM_PAGEMAP_H_
> > +
> > +#include <linux/dma-direction.h>
> > +#include <linux/hmm.h>
> > +#include <linux/types.h>
> > +
> > +struct drm_pagemap;
> > +struct device;
> > +
> > +/**
> > + * enum drm_interconnect_protocol - Used to identify an interconnect
> > protocol.
> > + */
> > +enum drm_interconnect_protocol {
> > +	DRM_INTERCONNECT_SYSTEM,    /* DMA map is system pages. */
> > +	DRM_INTERCONNECT_PCIE_P2P,  /* DMA map is PCIE P2P */
> > +	DRM_INTERCONNECT_DRIVER,    /* DMA map is driver defined */
> > +	/* A driver can add private values beyond
> > DRM_INTERCONNECT_DRIVER */
> > +};
> > +
> > +/**
> > + * struct drm_pagemap_dma_addr - DMA address representation.
> > + * @addr: The dma address or driver-defined address for driver
> > private interconnects.
> > + * @proto: The interconnect protocol.
> > + * @order: The page order of the dma mapping. (Size is PAGE_SIZE <<
> > order).
> > + * @dir: The DMA direction.
> > + *
> > + * Note: There is room for improvement here. We should be able to
> > pack into
> > + * 64 bits.
> > + */
> > +struct drm_pagemap_dma_addr {
> > +	dma_addr_t addr;
> > +	u64 proto : 54;
> > +	u64 order : 8;
> > +	u64 dir : 2;
> > +};
> > +
> > +/**
> > + * drm_pagemap_dma_addr_encode() - Encode a dma address with
> > metadata
> > + * @addr: The dma address or driver-defined address for driver
> > private interconnects.
> > + * @proto: The interconnect protocol.
> > + * @order: The page order of the dma mapping. (Size is PAGE_SIZE <<
> > order).
> > + * @dir: The DMA direction.
> > + *
> > + * Return: A struct drm_pagemap_dma_addr encoding the above
> > information.
> > + */
> > +static inline struct drm_pagemap_dma_addr
> > +drm_pagemap_dma_addr_encode(dma_addr_t addr,
> > +			    enum drm_interconnect_protocol proto,
> > +			    unsigned int order,
> > +			    enum dma_data_direction dir)
> > +{
> > +	return (struct drm_pagemap_dma_addr) {
> > +		.addr = addr,
> > +		.proto = proto,
> > +		.order = order,
> > +		.dir = dir,
> > +	};
> > +}
> > +
> > +/**
> > + * struct drm_pagemap_ops: Ops for a drm-pagemap.
> > + */
> > +struct drm_pagemap_ops {
> > +	/**
> > +	 * @map_dma: Map for dma access or provide a virtual address
> > suitable for
> > +	 *
> > +	 * @dpagemap: The struct drm_pagemap for the page.
> > +	 * @dev: The dma mapper.
> > +	 * @page: The page to map.
> > +	 * @order: The page order of the dma mapping. (Size is
> > PAGE_SIZE << order).
> > +	 * @dir: The transfer direction.
> > +	 */
> > +	struct drm_pagemap_dma_addr (*map_dma)(struct drm_pagemap
> > *dpagemap,
> > +					       struct device *dev,
> > +					       struct page *page,
> > +					       unsigned int order,
> > +					       enum
> > dma_data_direction dir);
> > +
> > +	/**
> > +	 * @unmap_dma: Unmap a dma address previously obtained using
> > @map_dma.
> > +	 *
> > +	 * @dpagemap: The struct drm_pagemap for the mapping.
> > +	 * @dev: The dma unmapper.
> > +	 * @addr: The dma address obtained when mapping.
> > +	 */
> > +	void (*unmap_dma)(struct drm_pagemap *dpagemap,
> > +			  struct device *dev,
> > +			  struct drm_pagemap_dma_addr addr);
> > +
> > +};
> > +
> > +/**
> > + * struct drm_pagemap: Additional information for a struct
> > dev_pagemap
> > + * used for device p2p handshaking.
> > + * @ops: The struct drm_pagemap_ops.
> > + * @dev: The struct drevice owning the device-private memory.
> > + */
> > +struct drm_pagemap {
> > +	const struct drm_pagemap_ops *ops;
> > +	struct device *dev;
> > +};
> > +
> > +#endif
>