[PATCH v4 04/33] drm/pagemap: Add DRM pagemap
Matthew Brost
matthew.brost at intel.com
Tue Feb 11 18:17:17 UTC 2025
On Tue, Feb 11, 2025 at 05:03:10PM +0100, Thomas Hellström wrote:
> On Mon, 2025-02-10 at 10:41 -0800, Matthew Brost wrote:
> > 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 at 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?
>
> I've been thinking a bit more about this, and I think a change we can
> do is to rename these methods to something along device_map() and
> device_unmap(). The purpose would be to emphasize that the resulting
> addresses are typically not meaningful outside of the driver, and not
> to be confused with standard dma-mapping.
>
Sure. I can rename this.
Matt
> /Thomas
>
>
> >
> > Matt
> >
> > > /Thomas
> > >
> > >
> > >
> > >
> > >
> > > >
> > > > v3:
> > > > - Move to drm level include
> > > > v4:
> > > > - Fix kernel doc (G.G.)
> > > >
> > > > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> > > > Signed-off-by: Thomas Hellström
> > > > <thomas.hellstrom at 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
> > >
>
More information about the dri-devel
mailing list