Re: [PATCH v2 4/4] drm/{i915, xe}/stolen: make insert_node, area_address, area_size optional

["Govindapillai, Vinod" <vinod.govindapillai@intel.com>]

On Fri, 2025-12-05 at 12:30 +0200, Jani Nikula wrote:
> Since the stolen memory hooks are function pointers, make some of
> them
> optional instead of having to define them for xe.
> 
> insert_node, area_address, and area_size are only needed on platforms
> not supported by xe.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_parent.c  | 10 ++++++++
>  drivers/gpu/drm/xe/display/xe_stolen.c       | 27 ------------------
> --
>  include/drm/intel/display_parent_interface.h |  6 ++---
>  3 files changed, 13 insertions(+), 30 deletions(-)
> 

Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>

> diff --git a/drivers/gpu/drm/i915/display/intel_parent.c
> b/drivers/gpu/drm/i915/display/intel_parent.c
> index 49cb64ca8c4c..d16163007545 100644
> --- a/drivers/gpu/drm/i915/display/intel_parent.c
> +++ b/drivers/gpu/drm/i915/display/intel_parent.c
> @@ -17,6 +17,7 @@
>   * function pointer interface.
>   */
>  
> +#include <drm/drm_print.h>
>  #include <drm/intel/display_parent_interface.h>
>  
>  #include "intel_display_core.h"
> @@ -89,6 +90,9 @@ int intel_parent_stolen_insert_node_in_range(struct
> intel_display *display,
>  int intel_parent_stolen_insert_node(struct intel_display *display,
> struct intel_stolen_node *node, u64 size,
>  				    unsigned int align)
>  {
> +	if (drm_WARN_ON_ONCE(display->drm, !display->parent->stolen-
> >insert_node))
> +		return -ENODEV;
> +
>  	return display->parent->stolen->insert_node(node, size,
> align);
>  }
>  
> @@ -116,11 +120,17 @@ u32 intel_parent_stolen_node_offset(struct
> intel_display *display, struct intel_
>  
>  u64 intel_parent_stolen_area_address(struct intel_display *display)
>  {
> +	if (drm_WARN_ON_ONCE(display->drm, !display->parent->stolen-
> >area_address))
> +		return 0;
> +
>  	return display->parent->stolen->area_address(display->drm);
>  }
>  
>  u64 intel_parent_stolen_area_size(struct intel_display *display)
>  {
> +	if (drm_WARN_ON_ONCE(display->drm, !display->parent->stolen-
> >area_size))
> +		return 0;
> +
>  	return display->parent->stolen->area_size(display->drm);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/display/xe_stolen.c
> b/drivers/gpu/drm/xe/display/xe_stolen.c
> index cc7aec7db76c..12771709183a 100644
> --- a/drivers/gpu/drm/xe/display/xe_stolen.c
> +++ b/drivers/gpu/drm/xe/display/xe_stolen.c
> @@ -43,14 +43,6 @@ static int xe_stolen_insert_node_in_range(struct
> intel_stolen_node *node, u64 si
>  	return err;
>  }
>  
> -static int xe_stolen_insert_node(struct intel_stolen_node *node, u64
> size, unsigned int align)
> -{
> -	/* Not used on xe */
> -	WARN_ON(1);
> -
> -	return -ENODEV;
> -}
> -
>  static void xe_stolen_remove_node(struct intel_stolen_node *node)
>  {
>  	xe_bo_unpin_map_no_vm(node->bo);
> @@ -77,22 +69,6 @@ static u64 xe_stolen_node_offset(const struct
> intel_stolen_node *node)
>  	return res.start;
>  }
>  
> -/* Used for < gen4. These are not supported by Xe */
> -static u64 xe_stolen_area_address(struct drm_device *drm)
> -{
> -	WARN_ON(1);
> -
> -	return 0;
> -}
> -
> -/* Used for gen9 specific WA. Gen9 is not supported by Xe */
> -static u64 xe_stolen_area_size(struct drm_device *drm)
> -{
> -	WARN_ON(1);
> -
> -	return 0;
> -}
> -
>  static u64 xe_stolen_node_address(const struct intel_stolen_node
> *node)
>  {
>  	struct xe_device *xe = node->xe;
> @@ -126,13 +102,10 @@ static void xe_stolen_node_free(const struct
> intel_stolen_node *node)
>  
>  const struct intel_display_stolen_interface
> xe_display_stolen_interface = {
>  	.insert_node_in_range = xe_stolen_insert_node_in_range,
> -	.insert_node = xe_stolen_insert_node,
>  	.remove_node = xe_stolen_remove_node,
>  	.initialized = xe_stolen_initialized,
>  	.node_allocated = xe_stolen_node_allocated,
>  	.node_offset = xe_stolen_node_offset,
> -	.area_address = xe_stolen_area_address,
> -	.area_size = xe_stolen_area_size,
>  	.node_address = xe_stolen_node_address,
>  	.node_size = xe_stolen_node_size,
>  	.node_alloc = xe_stolen_node_alloc,
> diff --git a/include/drm/intel/display_parent_interface.h
> b/include/drm/intel/display_parent_interface.h
> index f590e846464d..cc13b2ce1324 100644
> --- a/include/drm/intel/display_parent_interface.h
> +++ b/include/drm/intel/display_parent_interface.h
> @@ -51,13 +51,13 @@ struct intel_display_rps_interface {
>  struct intel_display_stolen_interface {
>  	int (*insert_node_in_range)(struct intel_stolen_node *node,
> u64 size,
>  				    unsigned int align, u64 start,
> u64 end);
> -	int (*insert_node)(struct intel_stolen_node *node, u64 size,
> unsigned int align);
> +	int (*insert_node)(struct intel_stolen_node *node, u64 size,
> unsigned int align); /* Optional */
>  	void (*remove_node)(struct intel_stolen_node *node);
>  	bool (*initialized)(struct drm_device *drm);
>  	bool (*node_allocated)(const struct intel_stolen_node
> *node);
>  	u64 (*node_offset)(const struct intel_stolen_node *node);
> -	u64 (*area_address)(struct drm_device *drm);
> -	u64 (*area_size)(struct drm_device *drm);
> +	u64 (*area_address)(struct drm_device *drm); /* Optional */
> +	u64 (*area_size)(struct drm_device *drm); /* Optional */
>  	u64 (*node_address)(const struct intel_stolen_node *node);
>  	u64 (*node_size)(const struct intel_stolen_node *node);
>  	struct intel_stolen_node *(*node_alloc)(struct drm_device
> *drm);