mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] [v2] drm/pagemap: pass pagemap_addr by reference
@ 2026-02-16 13:46 Arnd Bergmann
  2026-02-16 13:59 ` Thomas Hellström
  0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2026-02-16 13:46 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Matthew Brost,
	Thomas Hellström, Rodrigo Vivi
  Cc: Arnd Bergmann, Himal Prasad Ghimiray, Lucas De Marchi,
	Matthew Auld, Francois Dugast, Andrew Morton, dri-devel,
	linux-kernel, intel-xe

From: Arnd Bergmann <arnd@arndb.de>

Passing a structure by value into a function is sometimes problematic,
for a number of reasons. Of of these is a warning from the 32-bit arm
compiler:

drivers/gpu/drm/drm_gpusvm.c: In function '__drm_gpusvm_unmap_pages':
drivers/gpu/drm/drm_gpusvm.c:1152:33: note: parameter passing for argument of type 'struct drm_pagemap_addr' changed in GCC 9.1
 1152 |                                 dpagemap->ops->device_unmap(dpagemap,
      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1153 |                                                             dev, *addr);
      |                                                             ~~~~~~~~~~~

This particular problem is harmless since we are not mixing compiler versions
inside of the compiler. However, passing this by reference avoids the warning
along with providing slightly better calling conventions as it avoids an
extra copy on the stack.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpu/drm/drm_gpusvm.c  | 2 +-
 drivers/gpu/drm/drm_pagemap.c | 2 +-
 drivers/gpu/drm/xe/xe_svm.c   | 8 ++++----
 include/drm/drm_pagemap.h     | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index c25f50cad6fe..81626b00b755 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1150,7 +1150,7 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
 					       addr->dir);
 			else if (dpagemap && dpagemap->ops->device_unmap)
 				dpagemap->ops->device_unmap(dpagemap,
-							    dev, *addr);
+							    dev, addr);
 			i += 1 << addr->order;
 		}
 
diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
index d0041c947a28..22579806c055 100644
--- a/drivers/gpu/drm/drm_pagemap.c
+++ b/drivers/gpu/drm/drm_pagemap.c
@@ -318,7 +318,7 @@ static void drm_pagemap_migrate_unmap_pages(struct device *dev,
 			struct drm_pagemap_zdd *zdd = page->zone_device_data;
 			struct drm_pagemap *dpagemap = zdd->dpagemap;
 
-			dpagemap->ops->device_unmap(dpagemap, dev, pagemap_addr[i]);
+			dpagemap->ops->device_unmap(dpagemap, dev, &pagemap_addr[i]);
 		} else {
 			dma_unmap_page(dev, pagemap_addr[i].addr,
 				       PAGE_SIZE << pagemap_addr[i].order, dir);
diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
index 213f0334518a..78f4b2c60670 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -1676,13 +1676,13 @@ xe_drm_pagemap_device_map(struct drm_pagemap *dpagemap,
 
 static void xe_drm_pagemap_device_unmap(struct drm_pagemap *dpagemap,
 					struct device *dev,
-					struct drm_pagemap_addr addr)
+					const struct drm_pagemap_addr *addr)
 {
-	if (addr.proto != XE_INTERCONNECT_P2P)
+	if (addr->proto != XE_INTERCONNECT_P2P)
 		return;
 
-	dma_unmap_resource(dev, addr.addr, PAGE_SIZE << addr.order,
-			   addr.dir, DMA_ATTR_SKIP_CPU_SYNC);
+	dma_unmap_resource(dev, addr->addr, PAGE_SIZE << addr->order,
+			   addr->dir, DMA_ATTR_SKIP_CPU_SYNC);
 }
 
 static void xe_pagemap_destroy_work(struct work_struct *work)
diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
index 2baf0861f78f..c848f578e3da 100644
--- a/include/drm/drm_pagemap.h
+++ b/include/drm/drm_pagemap.h
@@ -95,7 +95,7 @@ struct drm_pagemap_ops {
 	 */
 	void (*device_unmap)(struct drm_pagemap *dpagemap,
 			     struct device *dev,
-			     struct drm_pagemap_addr addr);
+			     const struct drm_pagemap_addr *addr);
 
 	/**
 	 * @populate_mm: Populate part of the mm with @dpagemap memory,
-- 
2.39.5


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] [v2] drm/pagemap: pass pagemap_addr by reference
  2026-02-16 13:46 [PATCH] [v2] drm/pagemap: pass pagemap_addr by reference Arnd Bergmann
@ 2026-02-16 13:59 ` Thomas Hellström
  2026-02-17 12:21   ` Thomas Hellström
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Hellström @ 2026-02-16 13:59 UTC (permalink / raw)
  To: Arnd Bergmann, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Matthew Brost,
	Rodrigo Vivi
  Cc: Arnd Bergmann, Himal Prasad Ghimiray, Lucas De Marchi,
	Matthew Auld, Francois Dugast, Andrew Morton, dri-devel,
	linux-kernel, intel-xe

On Mon, 2026-02-16 at 14:46 +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Passing a structure by value into a function is sometimes
> problematic,
> for a number of reasons. Of of these is a warning from the 32-bit arm
> compiler:
> 
> drivers/gpu/drm/drm_gpusvm.c: In function '__drm_gpusvm_unmap_pages':
> drivers/gpu/drm/drm_gpusvm.c:1152:33: note: parameter passing for
> argument of type 'struct drm_pagemap_addr' changed in GCC 9.1
>  1152 |                                 dpagemap->ops-
> >device_unmap(dpagemap,
>       |                                
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  1153 |                                                            
> dev, *addr);
>       |                                                            
> ~~~~~~~~~~~
> 
> This particular problem is harmless since we are not mixing compiler
> versions
> inside of the compiler. However, passing this by reference avoids the
> warning
> along with providing slightly better calling conventions as it avoids
> an
> extra copy on the stack.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks.

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

Will push to drm-misc-fixes once CI is complete.

/Thomas


> ---
>  drivers/gpu/drm/drm_gpusvm.c  | 2 +-
>  drivers/gpu/drm/drm_pagemap.c | 2 +-
>  drivers/gpu/drm/xe/xe_svm.c   | 8 ++++----
>  include/drm/drm_pagemap.h     | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpusvm.c
> b/drivers/gpu/drm/drm_gpusvm.c
> index c25f50cad6fe..81626b00b755 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1150,7 +1150,7 @@ static void __drm_gpusvm_unmap_pages(struct
> drm_gpusvm *gpusvm,
>  					       addr->dir);
>  			else if (dpagemap && dpagemap->ops-
> >device_unmap)
>  				dpagemap->ops-
> >device_unmap(dpagemap,
> -							    dev,
> *addr);
> +							    dev,
> addr);
>  			i += 1 << addr->order;
>  		}
>  
> diff --git a/drivers/gpu/drm/drm_pagemap.c
> b/drivers/gpu/drm/drm_pagemap.c
> index d0041c947a28..22579806c055 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
> @@ -318,7 +318,7 @@ static void
> drm_pagemap_migrate_unmap_pages(struct device *dev,
>  			struct drm_pagemap_zdd *zdd = page-
> >zone_device_data;
>  			struct drm_pagemap *dpagemap = zdd-
> >dpagemap;
>  
> -			dpagemap->ops->device_unmap(dpagemap, dev,
> pagemap_addr[i]);
> +			dpagemap->ops->device_unmap(dpagemap, dev,
> &pagemap_addr[i]);
>  		} else {
>  			dma_unmap_page(dev, pagemap_addr[i].addr,
>  				       PAGE_SIZE <<
> pagemap_addr[i].order, dir);
> diff --git a/drivers/gpu/drm/xe/xe_svm.c
> b/drivers/gpu/drm/xe/xe_svm.c
> index 213f0334518a..78f4b2c60670 100644
> --- a/drivers/gpu/drm/xe/xe_svm.c
> +++ b/drivers/gpu/drm/xe/xe_svm.c
> @@ -1676,13 +1676,13 @@ xe_drm_pagemap_device_map(struct drm_pagemap
> *dpagemap,
>  
>  static void xe_drm_pagemap_device_unmap(struct drm_pagemap
> *dpagemap,
>  					struct device *dev,
> -					struct drm_pagemap_addr
> addr)
> +					const struct
> drm_pagemap_addr *addr)
>  {
> -	if (addr.proto != XE_INTERCONNECT_P2P)
> +	if (addr->proto != XE_INTERCONNECT_P2P)
>  		return;
>  
> -	dma_unmap_resource(dev, addr.addr, PAGE_SIZE << addr.order,
> -			   addr.dir, DMA_ATTR_SKIP_CPU_SYNC);
> +	dma_unmap_resource(dev, addr->addr, PAGE_SIZE << addr-
> >order,
> +			   addr->dir, DMA_ATTR_SKIP_CPU_SYNC);
>  }
>  
>  static void xe_pagemap_destroy_work(struct work_struct *work)
> diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> index 2baf0861f78f..c848f578e3da 100644
> --- a/include/drm/drm_pagemap.h
> +++ b/include/drm/drm_pagemap.h
> @@ -95,7 +95,7 @@ struct drm_pagemap_ops {
>  	 */
>  	void (*device_unmap)(struct drm_pagemap *dpagemap,
>  			     struct device *dev,
> -			     struct drm_pagemap_addr addr);
> +			     const struct drm_pagemap_addr *addr);
>  
>  	/**
>  	 * @populate_mm: Populate part of the mm with @dpagemap
> memory,

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] [v2] drm/pagemap: pass pagemap_addr by reference
  2026-02-16 13:59 ` Thomas Hellström
@ 2026-02-17 12:21   ` Thomas Hellström
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Hellström @ 2026-02-17 12:21 UTC (permalink / raw)
  To: Arnd Bergmann, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Matthew Brost,
	Rodrigo Vivi
  Cc: Arnd Bergmann, Himal Prasad Ghimiray, Matthew Auld,
	Francois Dugast, Andrew Morton, dri-devel, linux-kernel,
	intel-xe

On Mon, 2026-02-16 at 14:59 +0100, Thomas Hellström wrote:
> On Mon, 2026-02-16 at 14:46 +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > Passing a structure by value into a function is sometimes
> > problematic,
> > for a number of reasons. Of of these is a warning from the 32-bit
> > arm
> > compiler:
> > 
> > drivers/gpu/drm/drm_gpusvm.c: In function
> > '__drm_gpusvm_unmap_pages':
> > drivers/gpu/drm/drm_gpusvm.c:1152:33: note: parameter passing for
> > argument of type 'struct drm_pagemap_addr' changed in GCC 9.1
> >  1152 |                                 dpagemap->ops-
> > > device_unmap(dpagemap,
> >       |                                
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  1153 |                                                            
> > dev, *addr);
> >       |                                                            
> > ~~~~~~~~~~~
> > 
> > This particular problem is harmless since we are not mixing
> > compiler
> > versions
> > inside of the compiler. However, passing this by reference avoids
> > the
> > warning
> > along with providing slightly better calling conventions as it
> > avoids
> > an
> > extra copy on the stack.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Thanks.
> 
> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> 
> Will push to drm-misc-fixes once CI is complete.

Merged to drm-xe-next. Will likely appear after the next drm-xe-fixes
PR in 7.0-rc2.

Thanks,
Thomas



> 
> /Thomas
> 
> 
> > ---
> >  drivers/gpu/drm/drm_gpusvm.c  | 2 +-
> >  drivers/gpu/drm/drm_pagemap.c | 2 +-
> >  drivers/gpu/drm/xe/xe_svm.c   | 8 ++++----
> >  include/drm/drm_pagemap.h     | 2 +-
> >  4 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c
> > b/drivers/gpu/drm/drm_gpusvm.c
> > index c25f50cad6fe..81626b00b755 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > @@ -1150,7 +1150,7 @@ static void __drm_gpusvm_unmap_pages(struct
> > drm_gpusvm *gpusvm,
> >  					       addr->dir);
> >  			else if (dpagemap && dpagemap->ops-
> > > device_unmap)
> >  				dpagemap->ops-
> > > device_unmap(dpagemap,
> > -							    dev,
> > *addr);
> > +							    dev,
> > addr);
> >  			i += 1 << addr->order;
> >  		}
> >  
> > diff --git a/drivers/gpu/drm/drm_pagemap.c
> > b/drivers/gpu/drm/drm_pagemap.c
> > index d0041c947a28..22579806c055 100644
> > --- a/drivers/gpu/drm/drm_pagemap.c
> > +++ b/drivers/gpu/drm/drm_pagemap.c
> > @@ -318,7 +318,7 @@ static void
> > drm_pagemap_migrate_unmap_pages(struct device *dev,
> >  			struct drm_pagemap_zdd *zdd = page-
> > > zone_device_data;
> >  			struct drm_pagemap *dpagemap = zdd-
> > > dpagemap;
> >  
> > -			dpagemap->ops->device_unmap(dpagemap, dev,
> > pagemap_addr[i]);
> > +			dpagemap->ops->device_unmap(dpagemap, dev,
> > &pagemap_addr[i]);
> >  		} else {
> >  			dma_unmap_page(dev, pagemap_addr[i].addr,
> >  				       PAGE_SIZE <<
> > pagemap_addr[i].order, dir);
> > diff --git a/drivers/gpu/drm/xe/xe_svm.c
> > b/drivers/gpu/drm/xe/xe_svm.c
> > index 213f0334518a..78f4b2c60670 100644
> > --- a/drivers/gpu/drm/xe/xe_svm.c
> > +++ b/drivers/gpu/drm/xe/xe_svm.c
> > @@ -1676,13 +1676,13 @@ xe_drm_pagemap_device_map(struct
> > drm_pagemap
> > *dpagemap,
> >  
> >  static void xe_drm_pagemap_device_unmap(struct drm_pagemap
> > *dpagemap,
> >  					struct device *dev,
> > -					struct drm_pagemap_addr
> > addr)
> > +					const struct
> > drm_pagemap_addr *addr)
> >  {
> > -	if (addr.proto != XE_INTERCONNECT_P2P)
> > +	if (addr->proto != XE_INTERCONNECT_P2P)
> >  		return;
> >  
> > -	dma_unmap_resource(dev, addr.addr, PAGE_SIZE <<
> > addr.order,
> > -			   addr.dir, DMA_ATTR_SKIP_CPU_SYNC);
> > +	dma_unmap_resource(dev, addr->addr, PAGE_SIZE << addr-
> > > order,
> > +			   addr->dir, DMA_ATTR_SKIP_CPU_SYNC);
> >  }
> >  
> >  static void xe_pagemap_destroy_work(struct work_struct *work)
> > diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> > index 2baf0861f78f..c848f578e3da 100644
> > --- a/include/drm/drm_pagemap.h
> > +++ b/include/drm/drm_pagemap.h
> > @@ -95,7 +95,7 @@ struct drm_pagemap_ops {
> >  	 */
> >  	void (*device_unmap)(struct drm_pagemap *dpagemap,
> >  			     struct device *dev,
> > -			     struct drm_pagemap_addr addr);
> > +			     const struct drm_pagemap_addr *addr);
> >  
> >  	/**
> >  	 * @populate_mm: Populate part of the mm with @dpagemap
> > memory,

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-02-17 12:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-16 13:46 [PATCH] [v2] drm/pagemap: pass pagemap_addr by reference Arnd Bergmann
2026-02-16 13:59 ` Thomas Hellström
2026-02-17 12:21   ` Thomas Hellström

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome