Re: [PATCH 2/2] drm/i915: split out i915_gtt_view_types.h from i915_vma_types.h

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Feb 24, 2025 at 06:00:49PM +0200, Jani Nikula wrote:
> In the interest of limiting the display dependencies on i915 core
> headers, split out i915_gtt_view_types.h from i915_vma_types.h, and only
> include the new header from intel_display_types.h.
> 
> Reuse the new header from xe compat code too, failing build if partial
> view is used in display code.
> 
> Side note: Why would we ever have set enum i915_gtt_view_type values to
> size of each type?! What an insane hack.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |  2 +-
>  drivers/gpu/drm/i915/i915_gtt_view_types.h    | 59 +++++++++++++++
>  drivers/gpu/drm/i915/i915_vma_types.h         | 52 +------------
>  .../compat-i915-headers/i915_gtt_view_types.h |  7 ++
>  .../xe/compat-i915-headers/i915_vma_types.h   | 74 -------------------
>  5 files changed, 69 insertions(+), 125 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_gtt_view_types.h
>  create mode 100644 drivers/gpu/drm/xe/compat-i915-headers/i915_gtt_view_types.h
>  delete mode 100644 drivers/gpu/drm/xe/compat-i915-headers/i915_vma_types.h
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 12723ba13eb6..a856cbcf102f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -42,7 +42,7 @@
>  #include <drm/intel/i915_hdcp_interface.h>
>  #include <uapi/drm/i915_drm.h>
>  
> -#include "i915_vma_types.h"
> +#include "i915_gtt_view_types.h"
>  #include "intel_bios.h"
>  #include "intel_display.h"
>  #include "intel_display_conversion.h"
> diff --git a/drivers/gpu/drm/i915/i915_gtt_view_types.h b/drivers/gpu/drm/i915/i915_gtt_view_types.h
> new file mode 100644
> index 000000000000..c084f67bc880
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_gtt_view_types.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: MIT */
> +/* Copyright © 2025 Intel Corporation */
> +
> +#ifndef __I915_GTT_VIEW_TYPES_H__
> +#define __I915_GTT_VIEW_TYPES_H__
> +
> +#include <linux/types.h>
> +
> +struct intel_remapped_plane_info {
> +	/* in gtt pages */
> +	u32 offset:31;
> +	u32 linear:1;
> +	union {
> +		/* in gtt pages for !linear */
> +		struct {
> +			u16 width;
> +			u16 height;
> +			u16 src_stride;
> +			u16 dst_stride;
> +		};
> +
> +		/* in gtt pages for linear */
> +		u32 size;
> +	};
> +} __packed;
> +
> +struct intel_rotation_info {
> +	struct intel_remapped_plane_info plane[2];
> +} __packed;
> +
> +struct intel_partial_info {
> +	u64 offset;
> +	unsigned int size;
> +} __packed;
> +
> +struct intel_remapped_info {
> +	struct intel_remapped_plane_info plane[4];
> +	/* in gtt pages */
> +	u32 plane_alignment;
> +} __packed;
> +
> +enum i915_gtt_view_type {
> +	I915_GTT_VIEW_NORMAL = 0,
> +	I915_GTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
> +	I915_GTT_VIEW_PARTIAL = sizeof(struct intel_partial_info),
> +	I915_GTT_VIEW_REMAPPED = sizeof(struct intel_remapped_info),
> +};
> +
> +struct i915_gtt_view {
> +	enum i915_gtt_view_type type;
> +	union {
> +		/* Members need to contain no holes/padding */
> +		struct intel_partial_info partial;
> +		struct intel_rotation_info rotated;
> +		struct intel_remapped_info remapped;
> +	};
> +};
> +
> +#endif /* __I915_GTT_VIEW_TYPES_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_vma_types.h b/drivers/gpu/drm/i915/i915_vma_types.h
> index 559de74d0b11..a499a3bea874 100644
> --- a/drivers/gpu/drm/i915/i915_vma_types.h
> +++ b/drivers/gpu/drm/i915/i915_vma_types.h
> @@ -32,6 +32,8 @@
>  
>  #include "gem/i915_gem_object_types.h"
>  
> +#include "i915_gtt_view_types.h"
> +
>  /**
>   * DOC: Global GTT views
>   *
> @@ -95,46 +97,6 @@
>  
>  struct i915_vma_resource;
>  
> -struct intel_remapped_plane_info {
> -	/* in gtt pages */
> -	u32 offset:31;
> -	u32 linear:1;
> -	union {
> -		/* in gtt pages for !linear */
> -		struct {
> -			u16 width;
> -			u16 height;
> -			u16 src_stride;
> -			u16 dst_stride;
> -		};
> -
> -		/* in gtt pages for linear */
> -		u32 size;
> -	};
> -} __packed;
> -
> -struct intel_remapped_info {
> -	struct intel_remapped_plane_info plane[4];
> -	/* in gtt pages */
> -	u32 plane_alignment;
> -} __packed;
> -
> -struct intel_rotation_info {
> -	struct intel_remapped_plane_info plane[2];
> -} __packed;
> -
> -struct intel_partial_info {
> -	u64 offset;
> -	unsigned int size;
> -} __packed;
> -
> -enum i915_gtt_view_type {
> -	I915_GTT_VIEW_NORMAL = 0,
> -	I915_GTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
> -	I915_GTT_VIEW_PARTIAL = sizeof(struct intel_partial_info),
> -	I915_GTT_VIEW_REMAPPED = sizeof(struct intel_remapped_info),
> -};
> -
>  static inline void assert_i915_gem_gtt_types(void)
>  {
>  	BUILD_BUG_ON(sizeof(struct intel_rotation_info) != 2 * sizeof(u32) + 8 * sizeof(u16));
> @@ -160,16 +122,6 @@ static inline void assert_i915_gem_gtt_types(void)
>  	}
>  }
>  
> -struct i915_gtt_view {
> -	enum i915_gtt_view_type type;
> -	union {
> -		/* Members need to contain no holes/padding */
> -		struct intel_partial_info partial;
> -		struct intel_rotation_info rotated;
> -		struct intel_remapped_info remapped;
> -	};
> -};
> -
>  /**
>   * DOC: Virtual Memory Address
>   *
> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_gtt_view_types.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_gtt_view_types.h
> new file mode 100644
> index 000000000000..b261910cd6f9
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_gtt_view_types.h
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: MIT */
> +/* Copyright © 2025 Intel Corporation */
> +
> +#include "../../i915/i915_gtt_view_types.h"
> +
> +/* Partial view not supported in xe, fail build if used. */
> +#define I915_GTT_VIEW_PARTIAL

it's unfortunate we cannot get rid of this as well, but just by the below
clean up it is already worth

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_vma_types.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_vma_types.h
> deleted file mode 100644
> index e7aaf50f5485..000000000000
> --- a/drivers/gpu/drm/xe/compat-i915-headers/i915_vma_types.h
> +++ /dev/null
> @@ -1,74 +0,0 @@
> -/* SPDX-License-Identifier: MIT */
> -/*
> - * Copyright © 2023 Intel Corporation
> - */
> -
> -#include <linux/types.h>
> -#include <linux/build_bug.h>
> -
> -/* XX: Figure out how to handle this vma mapping in xe */
> -struct intel_remapped_plane_info {
> -	/* in gtt pages */
> -	u32 offset:31;
> -	u32 linear:1;
> -	union {
> -		/* in gtt pages for !linear */
> -		struct {
> -			u16 width;
> -			u16 height;
> -			u16 src_stride;
> -			u16 dst_stride;
> -		};
> -
> -		/* in gtt pages for linear */
> -		u32 size;
> -	};
> -} __packed;
> -
> -struct intel_remapped_info {
> -	struct intel_remapped_plane_info plane[4];
> -	/* in gtt pages */
> -	u32 plane_alignment;
> -} __packed;
> -
> -struct intel_rotation_info {
> -	struct intel_remapped_plane_info plane[2];
> -} __packed;
> -
> -enum i915_gtt_view_type {
> -	I915_GTT_VIEW_NORMAL = 0,
> -	I915_GTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
> -	I915_GTT_VIEW_REMAPPED = sizeof(struct intel_remapped_info),
> -};
> -
> -static inline void assert_i915_gem_gtt_types(void)
> -{
> -	BUILD_BUG_ON(sizeof(struct intel_rotation_info) != 2 * sizeof(u32) + 8 * sizeof(u16));
> -	BUILD_BUG_ON(sizeof(struct intel_remapped_info) != 5 * sizeof(u32) + 16 * sizeof(u16));
> -
> -	/* Check that rotation/remapped shares offsets for simplicity */
> -	BUILD_BUG_ON(offsetof(struct intel_remapped_info, plane[0]) !=
> -		     offsetof(struct intel_rotation_info, plane[0]));
> -	BUILD_BUG_ON(offsetofend(struct intel_remapped_info, plane[1]) !=
> -		     offsetofend(struct intel_rotation_info, plane[1]));
> -
> -	/* As we encode the size of each branch inside the union into its type,
> -	 * we have to be careful that each branch has a unique size.
> -	 */
> -	switch ((enum i915_gtt_view_type)0) {
> -	case I915_GTT_VIEW_NORMAL:
> -	case I915_GTT_VIEW_ROTATED:
> -	case I915_GTT_VIEW_REMAPPED:
> -		/* gcc complains if these are identical cases */
> -		break;
> -	}
> -}
> -
> -struct i915_gtt_view {
> -	enum i915_gtt_view_type type;
> -	union {
> -		/* Members need to contain no holes/padding */
> -		struct intel_rotation_info rotated;
> -		struct intel_remapped_info remapped;
> -	};
> -};
> -- 
> 2.39.5
>