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@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxx> > > Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx> > > --- > > 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 >