Re: [PATCH v1 1/5] drm/pagemap: Use struct drm_pagemap_device_addr in mapping and copy functions

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Thu, 2025-07-17 at 21:03 -0700, Matthew Brost wrote:
> On Thu, Jul 17, 2025 at 03:38:23PM +0200, Francois Dugast wrote:
> > This struct embeds more information than just the DMA address. This
> > will help
> > later to support folio orders greater than zero. At this point,
> > there is no
> > functional change as the only struct member used is addr.
> > 
> 
> This patch alone will break the build. You'll need to combine it with
> the next patch to avoid build breakage.
> 
> > Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > ---
> >  drivers/gpu/drm/drm_pagemap.c | 58 +++++++++++++++++--------------
> > ----
> >  include/drm/drm_pagemap.h     |  8 ++---
> >  2 files changed, 33 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_pagemap.c
> > b/drivers/gpu/drm/drm_pagemap.c
> > index 1da55322af12..0ed66aaade14 100644
> > --- a/drivers/gpu/drm/drm_pagemap.c
> > +++ b/drivers/gpu/drm/drm_pagemap.c
> > @@ -202,7 +202,7 @@ static void drm_pagemap_get_devmem_page(struct
> > page *page,
> >  /**
> >   * drm_pagemap_migrate_map_pages() - Map migration pages for GPU
> > SVM migration
> >   * @dev: The device for which the pages are being mapped
> > - * @dma_addr: Array to store DMA addresses corresponding to mapped
> > pages
> > + * @device_addr: Array to store DMA information corresponding to
> > mapped pages
> >   * @migrate_pfn: Array of migrate page frame numbers to map
> >   * @npages: Number of pages to map
> >   * @dir: Direction of data transfer (e.g., DMA_BIDIRECTIONAL)
> > @@ -215,7 +215,7 @@ static void drm_pagemap_get_devmem_page(struct
> > page *page,
> >   * Returns: 0 on success, -EFAULT if an error occurs during
> > mapping.
> >   */
> >  static int drm_pagemap_migrate_map_pages(struct device *dev,
> > -					 dma_addr_t *dma_addr,
> > +					 struct
> > drm_pagemap_device_addr *device_addr,
> 
> I like the change to drm_pagemap_device_addr—I think it fits with the
> patch—but it's not actually a device address. It's a DMA mapping of
> CPU
> memory. Originally, drm_pagemap_device_addr was intended to represent
> a
> device memory address shared between devices. That said, I think it
> still works for our purpose here.
> 
> So, I suggest we rename it:
> s/drm_pagemap_device_addr/drm_pagemap_addr
> And for the variable:
> s/device_addr/pagemap_addr

IIRC regardless of referencing system pages or device pages, both are
actually dma mappings.

So would drm_pagemap_dma_addr be a better fit? FWIW I'm OK with both.

Thanks,
Thomas