public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag
@ 2026-04-21  8:47 Laura Nao
  2026-04-21 13:01 ` Thomas Hellström
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Laura Nao @ 2026-04-21  8:47 UTC (permalink / raw)
  To: dakr, boris.brezillon, steven.price, liviu.dudau, matthew.brost,
	thomas.hellstrom, rodrigo.vivi
  Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, nouveau,
	intel-xe, kernel, Laura Nao

The DRIVER_GEM_GPUVA feature flag is currently only used to control two
behaviors within the DRM core:
- calling  drm_gem_gpuva_init() during
  GEM object initialization
- creating the "gpuvas" debugfs entry

drm_gem_gpuva_init() is a plain INIT_LIST_HEAD() and therefore is cheap
to run for every GEM object. The DRM_DEBUGFS_GPUVA_INFO macro is only
referenced by GPU-VA capable drivers, so clearing the feature bit does
not cause any unrelated drivers to get the "gpuvas" debugfs node. The
flag doesn't have any relevant purpose (e.g. gating ioctl handlers or MM
logic) and doesn't provide any practical benefit.

Remove the flag definition and drop it from all drivers that use it,
call drm_gem_gpuva_init() unconditionally and clear the driver features
bit in DRM_DEBUGFS_GPUVA_INFO.

Signed-off-by: Laura Nao <laura.nao@collabora.com>
---
 drivers/gpu/drm/drm_gem.c             | 3 +--
 drivers/gpu/drm/imagination/pvr_drv.c | 2 +-
 drivers/gpu/drm/msm/msm_drv.c         | 2 --
 drivers/gpu/drm/nouveau/nouveau_drm.c | 1 -
 drivers/gpu/drm/panthor/panthor_drv.c | 2 +-
 drivers/gpu/drm/xe/xe_device.c        | 2 +-
 include/drm/drm_debugfs.h             | 2 +-
 include/drm/drm_drv.h                 | 6 ------
 include/drm/drm_gem.h                 | 3 ---
 9 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 0377a5fd402d..cb703d0072aa 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -232,8 +232,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	if (!obj->resv)
 		obj->resv = &obj->_resv;
 
-	if (drm_core_check_feature(dev, DRIVER_GEM_GPUVA))
-		drm_gem_gpuva_init(obj);
+	drm_gem_gpuva_init(obj);
 
 	drm_vma_node_reset(&obj->vma_node);
 	INIT_LIST_HEAD(&obj->lru_node);
diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
index 268900464ab6..80033d4cb66a 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.c
+++ b/drivers/gpu/drm/imagination/pvr_drv.c
@@ -1375,7 +1375,7 @@ pvr_drm_driver_postclose(__always_unused struct drm_device *drm_dev,
 DEFINE_DRM_GEM_FOPS(pvr_drm_driver_fops);
 
 static struct drm_driver pvr_drm_driver = {
-	.driver_features = DRIVER_GEM | DRIVER_GEM_GPUVA | DRIVER_RENDER |
+	.driver_features = DRIVER_GEM | DRIVER_RENDER |
 			   DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE,
 	.open = pvr_drm_driver_open,
 	.postclose = pvr_drm_driver_postclose,
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 195f40e331e5..ee38c18715f0 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -825,7 +825,6 @@ static const struct file_operations fops = {
 
 #define DRIVER_FEATURES_GPU ( \
 		DRIVER_GEM | \
-		DRIVER_GEM_GPUVA | \
 		DRIVER_RENDER | \
 		DRIVER_SYNCOBJ | \
 		DRIVER_SYNCOBJ_TIMELINE | \
@@ -833,7 +832,6 @@ static const struct file_operations fops = {
 
 #define DRIVER_FEATURES_KMS ( \
 		DRIVER_GEM | \
-		DRIVER_GEM_GPUVA | \
 		DRIVER_ATOMIC | \
 		DRIVER_MODESET | \
 		0 )
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 5d8475e4895e..0f5f662e5429 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1364,7 +1364,6 @@ static struct drm_driver
 driver_stub = {
 	.driver_features = DRIVER_GEM |
 			   DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE |
-			   DRIVER_GEM_GPUVA |
 			   DRIVER_MODESET |
 			   DRIVER_RENDER,
 	.open = nouveau_drm_open,
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 87d27c3c1456..1a1d40a744db 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1805,7 +1805,7 @@ static void panthor_debugfs_init(struct drm_minor *minor)
  */
 static const struct drm_driver panthor_drm_driver = {
 	.driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ |
-			   DRIVER_SYNCOBJ_TIMELINE | DRIVER_GEM_GPUVA,
+			   DRIVER_SYNCOBJ_TIMELINE,
 	.open = panthor_open,
 	.postclose = panthor_postclose,
 	.show_fdinfo = panthor_show_fdinfo,
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index ffea4a453c01..b3acc202df76 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -393,7 +393,7 @@ static struct drm_driver driver = {
 	.driver_features =
 	    DRIVER_GEM |
 	    DRIVER_RENDER | DRIVER_SYNCOBJ |
-	    DRIVER_SYNCOBJ_TIMELINE | DRIVER_GEM_GPUVA,
+	    DRIVER_SYNCOBJ_TIMELINE,
 	.open = xe_file_open,
 	.postclose = xe_file_close,
 
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
index ea8cba94208a..eb93512b0f23 100644
--- a/include/drm/drm_debugfs.h
+++ b/include/drm/drm_debugfs.h
@@ -48,7 +48,7 @@
  * For each DRM GPU VA space drivers should call drm_debugfs_gpuva_info() from
  * their @show callback.
  */
-#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show, DRIVER_GEM_GPUVA, data}
+#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show, 0, data}
 
 /**
  * struct drm_info_list - debugfs info list entry
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 42fc085f986d..e09559495c5b 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -107,12 +107,6 @@ enum drm_driver_feature {
 	 * acceleration should be handled by two drivers that are connected using auxiliary bus.
 	 */
 	DRIVER_COMPUTE_ACCEL            = BIT(7),
-	/**
-	 * @DRIVER_GEM_GPUVA:
-	 *
-	 * Driver supports user defined GPU VA bindings for GEM objects.
-	 */
-	DRIVER_GEM_GPUVA		= BIT(8),
 	/**
 	 * @DRIVER_CURSOR_HOTSPOT:
 	 *
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 86f5846154f7..996aa418dce9 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -663,9 +663,6 @@ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj)
  *
  * This initializes the &drm_gem_object's &drm_gpuvm_bo list.
  *
- * Calling this function is only necessary for drivers intending to support the
- * &drm_driver_feature DRIVER_GEM_GPUVA.
- *
  * See also drm_gem_gpuva_set_lock().
  */
 static inline void drm_gem_gpuva_init(struct drm_gem_object *obj)
-- 
2.39.5


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

* Re: [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag
  2026-04-21  8:47 [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag Laura Nao
@ 2026-04-21 13:01 ` Thomas Hellström
  2026-04-21 15:32 ` Liviu Dudau
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Thomas Hellström @ 2026-04-21 13:01 UTC (permalink / raw)
  To: Laura Nao, dakr, boris.brezillon, steven.price, liviu.dudau,
	matthew.brost, rodrigo.vivi
  Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno, nouveau,
	intel-xe, kernel

On Tue, 2026-04-21 at 10:47 +0200, Laura Nao wrote:
> The DRIVER_GEM_GPUVA feature flag is currently only used to control
> two
> behaviors within the DRM core:
> - calling  drm_gem_gpuva_init() during
>   GEM object initialization
> - creating the "gpuvas" debugfs entry
> 
> drm_gem_gpuva_init() is a plain INIT_LIST_HEAD() and therefore is
> cheap
> to run for every GEM object. The DRM_DEBUGFS_GPUVA_INFO macro is only
> referenced by GPU-VA capable drivers, so clearing the feature bit
> does
> not cause any unrelated drivers to get the "gpuvas" debugfs node. The
> flag doesn't have any relevant purpose (e.g. gating ioctl handlers or
> MM
> logic) and doesn't provide any practical benefit.
> 
> Remove the flag definition and drop it from all drivers that use it,
> call drm_gem_gpuva_init() unconditionally and clear the driver
> features
> bit in DRM_DEBUGFS_GPUVA_INFO.
> 
> Signed-off-by: Laura Nao <laura.nao@collabora.com>

For the xe changes:
Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> ---
>  drivers/gpu/drm/drm_gem.c             | 3 +--
>  drivers/gpu/drm/imagination/pvr_drv.c | 2 +-
>  drivers/gpu/drm/msm/msm_drv.c         | 2 --
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 1 -
>  drivers/gpu/drm/panthor/panthor_drv.c | 2 +-
>  drivers/gpu/drm/xe/xe_device.c        | 2 +-
>  include/drm/drm_debugfs.h             | 2 +-
>  include/drm/drm_drv.h                 | 6 ------
>  include/drm/drm_gem.h                 | 3 ---
>  9 files changed, 5 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 0377a5fd402d..cb703d0072aa 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -232,8 +232,7 @@ void drm_gem_private_object_init(struct
> drm_device *dev,
>  	if (!obj->resv)
>  		obj->resv = &obj->_resv;
>  
> -	if (drm_core_check_feature(dev, DRIVER_GEM_GPUVA))
> -		drm_gem_gpuva_init(obj);
> +	drm_gem_gpuva_init(obj);
>  
>  	drm_vma_node_reset(&obj->vma_node);
>  	INIT_LIST_HEAD(&obj->lru_node);
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c
> b/drivers/gpu/drm/imagination/pvr_drv.c
> index 268900464ab6..80033d4cb66a 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1375,7 +1375,7 @@ pvr_drm_driver_postclose(__always_unused struct
> drm_device *drm_dev,
>  DEFINE_DRM_GEM_FOPS(pvr_drm_driver_fops);
>  
>  static struct drm_driver pvr_drm_driver = {
> -	.driver_features = DRIVER_GEM | DRIVER_GEM_GPUVA |
> DRIVER_RENDER |
> +	.driver_features = DRIVER_GEM | DRIVER_RENDER |
>  			   DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE,
>  	.open = pvr_drm_driver_open,
>  	.postclose = pvr_drm_driver_postclose,
> diff --git a/drivers/gpu/drm/msm/msm_drv.c
> b/drivers/gpu/drm/msm/msm_drv.c
> index 195f40e331e5..ee38c18715f0 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -825,7 +825,6 @@ static const struct file_operations fops = {
>  
>  #define DRIVER_FEATURES_GPU ( \
>  		DRIVER_GEM | \
> -		DRIVER_GEM_GPUVA | \
>  		DRIVER_RENDER | \
>  		DRIVER_SYNCOBJ | \
>  		DRIVER_SYNCOBJ_TIMELINE | \
> @@ -833,7 +832,6 @@ static const struct file_operations fops = {
>  
>  #define DRIVER_FEATURES_KMS ( \
>  		DRIVER_GEM | \
> -		DRIVER_GEM_GPUVA | \
>  		DRIVER_ATOMIC | \
>  		DRIVER_MODESET | \
>  		0 )
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 5d8475e4895e..0f5f662e5429 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1364,7 +1364,6 @@ static struct drm_driver
>  driver_stub = {
>  	.driver_features = DRIVER_GEM |
>  			   DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE
> |
> -			   DRIVER_GEM_GPUVA |
>  			   DRIVER_MODESET |
>  			   DRIVER_RENDER,
>  	.open = nouveau_drm_open,
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c
> b/drivers/gpu/drm/panthor/panthor_drv.c
> index 87d27c3c1456..1a1d40a744db 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1805,7 +1805,7 @@ static void panthor_debugfs_init(struct
> drm_minor *minor)
>   */
>  static const struct drm_driver panthor_drm_driver = {
>  	.driver_features = DRIVER_RENDER | DRIVER_GEM |
> DRIVER_SYNCOBJ |
> -			   DRIVER_SYNCOBJ_TIMELINE |
> DRIVER_GEM_GPUVA,
> +			   DRIVER_SYNCOBJ_TIMELINE,
>  	.open = panthor_open,
>  	.postclose = panthor_postclose,
>  	.show_fdinfo = panthor_show_fdinfo,
> diff --git a/drivers/gpu/drm/xe/xe_device.c
> b/drivers/gpu/drm/xe/xe_device.c
> index ffea4a453c01..b3acc202df76 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -393,7 +393,7 @@ static struct drm_driver driver = {
>  	.driver_features =
>  	    DRIVER_GEM |
>  	    DRIVER_RENDER | DRIVER_SYNCOBJ |
> -	    DRIVER_SYNCOBJ_TIMELINE | DRIVER_GEM_GPUVA,
> +	    DRIVER_SYNCOBJ_TIMELINE,
>  	.open = xe_file_open,
>  	.postclose = xe_file_close,
>  
> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> index ea8cba94208a..eb93512b0f23 100644
> --- a/include/drm/drm_debugfs.h
> +++ b/include/drm/drm_debugfs.h
> @@ -48,7 +48,7 @@
>   * For each DRM GPU VA space drivers should call
> drm_debugfs_gpuva_info() from
>   * their @show callback.
>   */
> -#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show,
> DRIVER_GEM_GPUVA, data}
> +#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show, 0, data}
>  
>  /**
>   * struct drm_info_list - debugfs info list entry
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 42fc085f986d..e09559495c5b 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -107,12 +107,6 @@ enum drm_driver_feature {
>  	 * acceleration should be handled by two drivers that are
> connected using auxiliary bus.
>  	 */
>  	DRIVER_COMPUTE_ACCEL            = BIT(7),
> -	/**
> -	 * @DRIVER_GEM_GPUVA:
> -	 *
> -	 * Driver supports user defined GPU VA bindings for GEM
> objects.
> -	 */
> -	DRIVER_GEM_GPUVA		= BIT(8),
>  	/**
>  	 * @DRIVER_CURSOR_HOTSPOT:
>  	 *
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 86f5846154f7..996aa418dce9 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -663,9 +663,6 @@ static inline bool drm_gem_is_imported(const
> struct drm_gem_object *obj)
>   *
>   * This initializes the &drm_gem_object's &drm_gpuvm_bo list.
>   *
> - * Calling this function is only necessary for drivers intending to
> support the
> - * &drm_driver_feature DRIVER_GEM_GPUVA.
> - *
>   * See also drm_gem_gpuva_set_lock().
>   */
>  static inline void drm_gem_gpuva_init(struct drm_gem_object *obj)

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

* Re: [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag
  2026-04-21  8:47 [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag Laura Nao
  2026-04-21 13:01 ` Thomas Hellström
@ 2026-04-21 15:32 ` Liviu Dudau
  2026-04-21 16:34 ` Rob Clark
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Liviu Dudau @ 2026-04-21 15:32 UTC (permalink / raw)
  To: Laura Nao
  Cc: dakr, boris.brezillon, steven.price, matthew.brost,
	thomas.hellstrom, rodrigo.vivi, dri-devel, linux-kernel,
	linux-arm-msm, freedreno, nouveau, intel-xe, kernel

On Tue, Apr 21, 2026 at 10:47:01AM +0200, Laura Nao wrote:
> The DRIVER_GEM_GPUVA feature flag is currently only used to control two
> behaviors within the DRM core:
> - calling  drm_gem_gpuva_init() during
>   GEM object initialization
> - creating the "gpuvas" debugfs entry
> 
> drm_gem_gpuva_init() is a plain INIT_LIST_HEAD() and therefore is cheap
> to run for every GEM object. The DRM_DEBUGFS_GPUVA_INFO macro is only
> referenced by GPU-VA capable drivers, so clearing the feature bit does
> not cause any unrelated drivers to get the "gpuvas" debugfs node. The
> flag doesn't have any relevant purpose (e.g. gating ioctl handlers or MM
> logic) and doesn't provide any practical benefit.
> 
> Remove the flag definition and drop it from all drivers that use it,
> call drm_gem_gpuva_init() unconditionally and clear the driver features
> bit in DRM_DEBUGFS_GPUVA_INFO.
> 
> Signed-off-by: Laura Nao <laura.nao@collabora.com>
> ---
>  drivers/gpu/drm/drm_gem.c             | 3 +--
>  drivers/gpu/drm/imagination/pvr_drv.c | 2 +-
>  drivers/gpu/drm/msm/msm_drv.c         | 2 --
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 1 -
>  drivers/gpu/drm/panthor/panthor_drv.c | 2 +-

For panthor changes:

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

>  drivers/gpu/drm/xe/xe_device.c        | 2 +-
>  include/drm/drm_debugfs.h             | 2 +-
>  include/drm/drm_drv.h                 | 6 ------
>  include/drm/drm_gem.h                 | 3 ---
>  9 files changed, 5 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 0377a5fd402d..cb703d0072aa 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -232,8 +232,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
>  	if (!obj->resv)
>  		obj->resv = &obj->_resv;
>  
> -	if (drm_core_check_feature(dev, DRIVER_GEM_GPUVA))
> -		drm_gem_gpuva_init(obj);
> +	drm_gem_gpuva_init(obj);
>  
>  	drm_vma_node_reset(&obj->vma_node);
>  	INIT_LIST_HEAD(&obj->lru_node);
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index 268900464ab6..80033d4cb66a 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1375,7 +1375,7 @@ pvr_drm_driver_postclose(__always_unused struct drm_device *drm_dev,
>  DEFINE_DRM_GEM_FOPS(pvr_drm_driver_fops);
>  
>  static struct drm_driver pvr_drm_driver = {
> -	.driver_features = DRIVER_GEM | DRIVER_GEM_GPUVA | DRIVER_RENDER |
> +	.driver_features = DRIVER_GEM | DRIVER_RENDER |
>  			   DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE,
>  	.open = pvr_drm_driver_open,
>  	.postclose = pvr_drm_driver_postclose,
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 195f40e331e5..ee38c18715f0 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -825,7 +825,6 @@ static const struct file_operations fops = {
>  
>  #define DRIVER_FEATURES_GPU ( \
>  		DRIVER_GEM | \
> -		DRIVER_GEM_GPUVA | \
>  		DRIVER_RENDER | \
>  		DRIVER_SYNCOBJ | \
>  		DRIVER_SYNCOBJ_TIMELINE | \
> @@ -833,7 +832,6 @@ static const struct file_operations fops = {
>  
>  #define DRIVER_FEATURES_KMS ( \
>  		DRIVER_GEM | \
> -		DRIVER_GEM_GPUVA | \
>  		DRIVER_ATOMIC | \
>  		DRIVER_MODESET | \
>  		0 )
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 5d8475e4895e..0f5f662e5429 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1364,7 +1364,6 @@ static struct drm_driver
>  driver_stub = {
>  	.driver_features = DRIVER_GEM |
>  			   DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE |
> -			   DRIVER_GEM_GPUVA |
>  			   DRIVER_MODESET |
>  			   DRIVER_RENDER,
>  	.open = nouveau_drm_open,
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 87d27c3c1456..1a1d40a744db 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1805,7 +1805,7 @@ static void panthor_debugfs_init(struct drm_minor *minor)
>   */
>  static const struct drm_driver panthor_drm_driver = {
>  	.driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ |
> -			   DRIVER_SYNCOBJ_TIMELINE | DRIVER_GEM_GPUVA,
> +			   DRIVER_SYNCOBJ_TIMELINE,
>  	.open = panthor_open,
>  	.postclose = panthor_postclose,
>  	.show_fdinfo = panthor_show_fdinfo,
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index ffea4a453c01..b3acc202df76 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -393,7 +393,7 @@ static struct drm_driver driver = {
>  	.driver_features =
>  	    DRIVER_GEM |
>  	    DRIVER_RENDER | DRIVER_SYNCOBJ |
> -	    DRIVER_SYNCOBJ_TIMELINE | DRIVER_GEM_GPUVA,
> +	    DRIVER_SYNCOBJ_TIMELINE,
>  	.open = xe_file_open,
>  	.postclose = xe_file_close,
>  
> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> index ea8cba94208a..eb93512b0f23 100644
> --- a/include/drm/drm_debugfs.h
> +++ b/include/drm/drm_debugfs.h
> @@ -48,7 +48,7 @@
>   * For each DRM GPU VA space drivers should call drm_debugfs_gpuva_info() from
>   * their @show callback.
>   */
> -#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show, DRIVER_GEM_GPUVA, data}
> +#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show, 0, data}
>  
>  /**
>   * struct drm_info_list - debugfs info list entry
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 42fc085f986d..e09559495c5b 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -107,12 +107,6 @@ enum drm_driver_feature {
>  	 * acceleration should be handled by two drivers that are connected using auxiliary bus.
>  	 */
>  	DRIVER_COMPUTE_ACCEL            = BIT(7),
> -	/**
> -	 * @DRIVER_GEM_GPUVA:
> -	 *
> -	 * Driver supports user defined GPU VA bindings for GEM objects.
> -	 */
> -	DRIVER_GEM_GPUVA		= BIT(8),
>  	/**
>  	 * @DRIVER_CURSOR_HOTSPOT:
>  	 *
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 86f5846154f7..996aa418dce9 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -663,9 +663,6 @@ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj)
>   *
>   * This initializes the &drm_gem_object's &drm_gpuvm_bo list.
>   *
> - * Calling this function is only necessary for drivers intending to support the
> - * &drm_driver_feature DRIVER_GEM_GPUVA.
> - *
>   * See also drm_gem_gpuva_set_lock().
>   */
>  static inline void drm_gem_gpuva_init(struct drm_gem_object *obj)
> -- 
> 2.39.5
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

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

* Re: [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag
  2026-04-21  8:47 [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag Laura Nao
  2026-04-21 13:01 ` Thomas Hellström
  2026-04-21 15:32 ` Liviu Dudau
@ 2026-04-21 16:34 ` Rob Clark
  2026-04-22 22:39 ` Claude review: " Claude Code Review Bot
  2026-04-22 22:39 ` Claude Code Review Bot
  4 siblings, 0 replies; 6+ messages in thread
From: Rob Clark @ 2026-04-21 16:34 UTC (permalink / raw)
  To: Laura Nao
  Cc: dakr, boris.brezillon, steven.price, liviu.dudau, matthew.brost,
	thomas.hellstrom, rodrigo.vivi, dri-devel, linux-kernel,
	linux-arm-msm, freedreno, nouveau, intel-xe, kernel

On Tue, Apr 21, 2026 at 1:47 AM Laura Nao <laura.nao@collabora.com> wrote:
>
> The DRIVER_GEM_GPUVA feature flag is currently only used to control two
> behaviors within the DRM core:
> - calling  drm_gem_gpuva_init() during
>   GEM object initialization
> - creating the "gpuvas" debugfs entry
>
> drm_gem_gpuva_init() is a plain INIT_LIST_HEAD() and therefore is cheap
> to run for every GEM object. The DRM_DEBUGFS_GPUVA_INFO macro is only
> referenced by GPU-VA capable drivers, so clearing the feature bit does
> not cause any unrelated drivers to get the "gpuvas" debugfs node. The
> flag doesn't have any relevant purpose (e.g. gating ioctl handlers or MM
> logic) and doesn't provide any practical benefit.
>
> Remove the flag definition and drop it from all drivers that use it,
> call drm_gem_gpuva_init() unconditionally and clear the driver features
> bit in DRM_DEBUGFS_GPUVA_INFO.
>
> Signed-off-by: Laura Nao <laura.nao@collabora.com>

Acked-by: Rob Clark <rob.clark@oss.qualcomm.com>

> ---
>  drivers/gpu/drm/drm_gem.c             | 3 +--
>  drivers/gpu/drm/imagination/pvr_drv.c | 2 +-
>  drivers/gpu/drm/msm/msm_drv.c         | 2 --
>  drivers/gpu/drm/nouveau/nouveau_drm.c | 1 -
>  drivers/gpu/drm/panthor/panthor_drv.c | 2 +-
>  drivers/gpu/drm/xe/xe_device.c        | 2 +-
>  include/drm/drm_debugfs.h             | 2 +-
>  include/drm/drm_drv.h                 | 6 ------
>  include/drm/drm_gem.h                 | 3 ---
>  9 files changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 0377a5fd402d..cb703d0072aa 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -232,8 +232,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
>         if (!obj->resv)
>                 obj->resv = &obj->_resv;
>
> -       if (drm_core_check_feature(dev, DRIVER_GEM_GPUVA))
> -               drm_gem_gpuva_init(obj);
> +       drm_gem_gpuva_init(obj);
>
>         drm_vma_node_reset(&obj->vma_node);
>         INIT_LIST_HEAD(&obj->lru_node);
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index 268900464ab6..80033d4cb66a 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1375,7 +1375,7 @@ pvr_drm_driver_postclose(__always_unused struct drm_device *drm_dev,
>  DEFINE_DRM_GEM_FOPS(pvr_drm_driver_fops);
>
>  static struct drm_driver pvr_drm_driver = {
> -       .driver_features = DRIVER_GEM | DRIVER_GEM_GPUVA | DRIVER_RENDER |
> +       .driver_features = DRIVER_GEM | DRIVER_RENDER |
>                            DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE,
>         .open = pvr_drm_driver_open,
>         .postclose = pvr_drm_driver_postclose,
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 195f40e331e5..ee38c18715f0 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -825,7 +825,6 @@ static const struct file_operations fops = {
>
>  #define DRIVER_FEATURES_GPU ( \
>                 DRIVER_GEM | \
> -               DRIVER_GEM_GPUVA | \
>                 DRIVER_RENDER | \
>                 DRIVER_SYNCOBJ | \
>                 DRIVER_SYNCOBJ_TIMELINE | \
> @@ -833,7 +832,6 @@ static const struct file_operations fops = {
>
>  #define DRIVER_FEATURES_KMS ( \
>                 DRIVER_GEM | \
> -               DRIVER_GEM_GPUVA | \
>                 DRIVER_ATOMIC | \
>                 DRIVER_MODESET | \
>                 0 )
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 5d8475e4895e..0f5f662e5429 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1364,7 +1364,6 @@ static struct drm_driver
>  driver_stub = {
>         .driver_features = DRIVER_GEM |
>                            DRIVER_SYNCOBJ | DRIVER_SYNCOBJ_TIMELINE |
> -                          DRIVER_GEM_GPUVA |
>                            DRIVER_MODESET |
>                            DRIVER_RENDER,
>         .open = nouveau_drm_open,
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 87d27c3c1456..1a1d40a744db 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1805,7 +1805,7 @@ static void panthor_debugfs_init(struct drm_minor *minor)
>   */
>  static const struct drm_driver panthor_drm_driver = {
>         .driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ |
> -                          DRIVER_SYNCOBJ_TIMELINE | DRIVER_GEM_GPUVA,
> +                          DRIVER_SYNCOBJ_TIMELINE,
>         .open = panthor_open,
>         .postclose = panthor_postclose,
>         .show_fdinfo = panthor_show_fdinfo,
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index ffea4a453c01..b3acc202df76 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -393,7 +393,7 @@ static struct drm_driver driver = {
>         .driver_features =
>             DRIVER_GEM |
>             DRIVER_RENDER | DRIVER_SYNCOBJ |
> -           DRIVER_SYNCOBJ_TIMELINE | DRIVER_GEM_GPUVA,
> +           DRIVER_SYNCOBJ_TIMELINE,
>         .open = xe_file_open,
>         .postclose = xe_file_close,
>
> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> index ea8cba94208a..eb93512b0f23 100644
> --- a/include/drm/drm_debugfs.h
> +++ b/include/drm/drm_debugfs.h
> @@ -48,7 +48,7 @@
>   * For each DRM GPU VA space drivers should call drm_debugfs_gpuva_info() from
>   * their @show callback.
>   */
> -#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show, DRIVER_GEM_GPUVA, data}
> +#define DRM_DEBUGFS_GPUVA_INFO(show, data) {"gpuvas", show, 0, data}
>
>  /**
>   * struct drm_info_list - debugfs info list entry
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 42fc085f986d..e09559495c5b 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -107,12 +107,6 @@ enum drm_driver_feature {
>          * acceleration should be handled by two drivers that are connected using auxiliary bus.
>          */
>         DRIVER_COMPUTE_ACCEL            = BIT(7),
> -       /**
> -        * @DRIVER_GEM_GPUVA:
> -        *
> -        * Driver supports user defined GPU VA bindings for GEM objects.
> -        */
> -       DRIVER_GEM_GPUVA                = BIT(8),
>         /**
>          * @DRIVER_CURSOR_HOTSPOT:
>          *
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 86f5846154f7..996aa418dce9 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -663,9 +663,6 @@ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj)
>   *
>   * This initializes the &drm_gem_object's &drm_gpuvm_bo list.
>   *
> - * Calling this function is only necessary for drivers intending to support the
> - * &drm_driver_feature DRIVER_GEM_GPUVA.
> - *
>   * See also drm_gem_gpuva_set_lock().
>   */
>  static inline void drm_gem_gpuva_init(struct drm_gem_object *obj)
> --
> 2.39.5
>

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

* Claude review: drm: Remove DRIVER_GEM_GPUVA feature flag
  2026-04-21  8:47 [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag Laura Nao
                   ` (3 preceding siblings ...)
  2026-04-22 22:39 ` Claude review: " Claude Code Review Bot
@ 2026-04-22 22:39 ` Claude Code Review Bot
  4 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:39 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm: Remove DRIVER_GEM_GPUVA feature flag
Author: Laura Nao <laura.nao@collabora.com>
Patches: 4
Reviewed: 2026-04-23T08:39:45.525593

---

This is a clean, well-motivated single-patch cleanup that removes the `DRIVER_GEM_GPUVA` feature flag from the DRM subsystem. The commit message accurately describes the situation: the flag only gates an `INIT_LIST_HEAD()` call and a debugfs entry filter, neither of which justify a per-driver opt-in flag. The patch is straightforward and touches all the right files. There is one potential issue worth discussing regarding nouveau.

**Verdict:** Looks good overall with one minor point to verify.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: drm: Remove DRIVER_GEM_GPUVA feature flag
  2026-04-21  8:47 [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag Laura Nao
                   ` (2 preceding siblings ...)
  2026-04-21 16:34 ` Rob Clark
@ 2026-04-22 22:39 ` Claude Code Review Bot
  2026-04-22 22:39 ` Claude Code Review Bot
  4 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 22:39 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Core DRM changes (drm_gem.c, drm_drv.h, drm_gem.h, drm_debugfs.h):**

These are correct. The key observations:

1. `drm_gem_gpuva_init()` is just `INIT_LIST_HEAD(&obj->gpuva.list)` — calling it unconditionally for every GEM object is harmless. The `mutex_init(&obj->gpuva.lock)` at line 230 of `drm_gem.c` is already unconditional, so the list head init is the only thing that was gated.

2. The `DRM_DEBUGFS_GPUVA_INFO` change from `DRIVER_GEM_GPUVA` to `0` is correct. Looking at `drm_debugfs_create_files()`:
   ```c
   if (features && !drm_core_check_all_features(dev, features))
       continue;
   ```
   Passing `0` means `features` is falsy, so the condition short-circuits and the entry is always created — but only for drivers that actually use `DRM_DEBUGFS_GPUVA_INFO` in their debugfs info list (panthor and nouveau). Non-GPUVA drivers never reference this macro, so they don't get a spurious "gpuvas" debugfs entry. This is fine.

3. Removing the enum value and doc comment from `drm_drv.h` is clean. Note that `BIT(8)` is left as a gap — this is normal for DRM driver feature flags and avoids ABI issues with out-of-tree drivers.

4. The doc update in `drm_gem.h` removing the "only necessary for drivers intending to support DRIVER_GEM_GPUVA" comment is appropriate.

**Driver changes (pvr_drv.c, msm_drv.c, nouveau_drm.c, panthor_drv.c, xe_device.c):**

All driver removals of the flag from `.driver_features` look correct and mechanical.

**One point to note — nouveau double-init:**

In `nouveau_bo.c:394`, nouveau has an explicit call:
```c
drm_gem_gpuva_init(&nvbo->bo.base);
```
This exists because nouveau needs the list initialized *before* `ttm_bo_init_reserved()` (as the comment explains: "Subsequent bo_move() callbacks might already iterate the GEMs GPUVA list"). Previously, the core's `drm_gem_private_object_init()` would also call it (gated by the feature flag). With this patch, the core now calls it unconditionally, and then nouveau calls it again. Since it's just `INIT_LIST_HEAD()`, double-calling is harmless (re-initializing an empty list head is a no-op in effect). However, it would be tidier to also remove the now-redundant explicit call in `nouveau_bo.c` and its associated comment. This is a minor cleanup opportunity, not a correctness issue — but it would make the patch more complete.

**Summary:** The patch is correct and well-reasoned. The only suggestion is optionally removing the now-redundant `drm_gem_gpuva_init()` call in `drivers/gpu/drm/nouveau/nouveau_bo.c:394` (and its comment block at lines 391-393), since the core will now always handle this initialization.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-04-22 22:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21  8:47 [PATCH] drm: Remove DRIVER_GEM_GPUVA feature flag Laura Nao
2026-04-21 13:01 ` Thomas Hellström
2026-04-21 15:32 ` Liviu Dudau
2026-04-21 16:34 ` Rob Clark
2026-04-22 22:39 ` Claude review: " Claude Code Review Bot
2026-04-22 22:39 ` Claude Code Review Bot

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