[Intel-xe] [PATCH v4 10/12] drm/xe: Add missing includes for i915_reg_defs.h and others

Matt Roper matthew.d.roper at intel.com
Sat Feb 25 00:48:36 UTC 2023


On Fri, Feb 24, 2023 at 04:15:46PM -0800, Lucas De Marchi wrote:
> Several places were missing the i915_reg_defs.h include. Any place using
> the i915_reg_t typedef or macros like _MMIO(), FIELD_PREP(), etc should
> include the register defining them explicitly.
> 
> This has gone unnoticed due to the way the display is integrated. By
> including the display headers the i915_reg_defs.h is indirectly
> included. Also, although the Makefile tries to add more cflags for the
> display part, it fails in doing so: those cflags are actually added to
> the entire Makefile (see subdir-ccflags documentation).
> 
> This change is obtained by temporarily removing display from xe and
> fixing the breakages.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device.c       | 6 ++++--
>  drivers/gpu/drm/xe/xe_gt_pagefault.c | 2 ++
>  drivers/gpu/drm/xe/xe_gt_topology.c  | 2 ++
>  drivers/gpu/drm/xe/xe_migrate.c      | 2 ++
>  drivers/gpu/drm/xe/xe_pcode_api.h    | 2 ++
>  drivers/gpu/drm/xe/xe_step.c         | 2 ++
>  6 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 45e3c740fda2..8f2b2acd4024 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -31,6 +31,10 @@
>  #include "xe_vm_madvise.h"
>  #include "xe_wait_user_fence.h"
>  
> +#include "../i915/i915_reg_defs.h"
> +
> +#define SOFTWARE_FLAGS_SPR33         _MMIO(0x4F084)
> +
>  static int xe_file_open(struct drm_device *dev, struct drm_file *file)
>  {
>  	struct xe_file *xef;
> @@ -405,8 +409,6 @@ static void device_kill_persitent_engines(struct xe_device *xe,
>  	mutex_unlock(&xe->persitent_engines.lock);
>  }
>  
> -#define SOFTWARE_FLAGS_SPR33         _MMIO(0x4F084)
> -
>  void xe_device_wmb(struct xe_device *xe)
>  {
>  	struct xe_gt *gt = xe_device_get_gt(xe, 0);
> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> index 0e7047b89a83..19403dbea9f5 100644
> --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> @@ -20,6 +20,8 @@
>  #include "xe_trace.h"
>  #include "xe_vm.h"
>  
> +#include "../i915/i915_reg_defs.h"
> +

Does this file actually use any i915/xe register stuff or is it just the
generic FIELD_GET() definitions causing a problem?  If the latter, then
maybe

        #include <linux/bitfield.h>

would be more appropriate?


>  struct pagefault {
>  	u64 page_addr;
>  	u32 asid;
> diff --git a/drivers/gpu/drm/xe/xe_gt_topology.c b/drivers/gpu/drm/xe/xe_gt_topology.c
> index c76aaea1887c..264d6429318b 100644
> --- a/drivers/gpu/drm/xe/xe_gt_topology.c
> +++ b/drivers/gpu/drm/xe/xe_gt_topology.c
> @@ -10,6 +10,8 @@
>  #include "xe_gt.h"
>  #include "xe_mmio.h"
>  
> +#include "../i915/i915_reg_defs.h"
> +

I think there are some registers defined at the top of this file from
before we figured out what we were going to do about register
definitions; those should probably migrate into the new register
headers.  And then we can just include the register header instead of
the reg_defs.

>  #define XE_MAX_DSS_FUSE_BITS (32 * XE_MAX_DSS_FUSE_REGS)
>  #define XE_MAX_EU_FUSE_BITS (32 * XE_MAX_EU_FUSE_REGS)
>  
> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 6c0033cb238f..d845f7cadbd4 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -28,6 +28,8 @@
>  #include "xe_trace.h"
>  #include "xe_vm.h"
>  
> +#include "../i915/i915_reg_defs.h"

Maybe another candidate for linux/bitfield.h?

> +
>  /**
>   * struct xe_migrate - migrate context.
>   */
> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
> index 0762c8a912c7..2103f684a8b5 100644
> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
> @@ -5,6 +5,8 @@
>  
>  /* Internal to xe_pcode */
>  
> +#include "../i915/i915_reg_defs.h"
> +
>  #define PCODE_MAILBOX			_MMIO(0x138124)
>  #define   PCODE_READY			REG_BIT(31)
>  #define   PCODE_MB_PARAM2		REG_GENMASK(23, 16)
> diff --git a/drivers/gpu/drm/xe/xe_step.c b/drivers/gpu/drm/xe/xe_step.c
> index ca77d0971529..e5e77cb9ea09 100644
> --- a/drivers/gpu/drm/xe/xe_step.c
> +++ b/drivers/gpu/drm/xe/xe_step.c
> @@ -8,6 +8,8 @@
>  #include "xe_device.h"
>  #include "xe_platform_types.h"
>  
> +#include "../i915/i915_reg_defs.h"

And another linux/bitfield.h here too?


Matt

> +
>  /*
>   * Provide mapping between PCI's revision ID to the individual GMD
>   * (Graphics/Media/Display) stepping values that can be compared numerically.
> -- 
> 2.39.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation


More information about the Intel-xe mailing list