Re: [Intel-gfx] [PATCH v2 10/12] drm/i915/xelpmp: Expose media as another GT

["Iddamsetty, Aravind" <aravind.iddamsetty@intel.com>]

On 03-09-2022 05:02, Matt Roper wrote:
> Xe_LPM+ platforms have "standalone media."  I.e., the media unit is
> designed as an additional GT with its own engine list, GuC, forcewake,
> etc.  Let's allow platforms to include media GTs in their device info.
> 
> v2:
>  - Simplify GSI register handling and split it out to a separate patch
>    for ease of review.  (Daniele)
> 
> Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile            |  1 +
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h  |  8 +++++
>  drivers/gpu/drm/i915/gt/intel_sa_media.c | 39 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_sa_media.h | 15 +++++++++
>  drivers/gpu/drm/i915/i915_pci.c          | 15 +++++++++
>  drivers/gpu/drm/i915/intel_device_info.h |  1 +
>  drivers/gpu/drm/i915/intel_uncore.c      |  4 +++
>  7 files changed, 83 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/gt/intel_sa_media.c
>  create mode 100644 drivers/gpu/drm/i915/gt/intel_sa_media.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 522ef9b4aff3..e83e4cd46968 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -123,6 +123,7 @@ gt-y += \
>  	gt/intel_ring.o \
>  	gt/intel_ring_submission.o \
>  	gt/intel_rps.o \
> +	gt/intel_sa_media.o \
>  	gt/intel_sseu.o \
>  	gt/intel_sseu_debugfs.o \
>  	gt/intel_timeline.o \
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index d414785003cc..fb2c56777480 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1578,4 +1578,12 @@
>  
>  #define GEN12_SFC_DONE(n)			_MMIO(0x1cc000 + (n) * 0x1000)
>  
> +/*
> + * Standalone Media's non-engine GT registers are located at their regular GT
> + * offsets plus 0x380000.  This extra offset is stored inside the intel_uncore
> + * structure so that the existing code can be used for both GTs without
> + * modification.
> + */
> +#define MTL_MEDIA_GSI_BASE			0x380000
> +
>  #endif /* __INTEL_GT_REGS__ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_sa_media.c b/drivers/gpu/drm/i915/gt/intel_sa_media.c
> new file mode 100644
> index 000000000000..8c5c519457cc
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/intel_sa_media.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#include <drm/drm_managed.h>
> +
> +#include "i915_drv.h"
> +#include "gt/intel_gt.h"
> +#include "gt/intel_sa_media.h"
> +
> +int intel_sa_mediagt_setup(struct intel_gt *gt, phys_addr_t phys_addr,
> +			   u32 gsi_offset)
> +{
> +	struct drm_i915_private *i915 = gt->i915;
> +	struct intel_uncore *uncore;
> +
> +	uncore = drmm_kzalloc(&i915->drm, sizeof(*uncore), GFP_KERNEL);
> +	if (!uncore)
> +		return -ENOMEM;
> +
> +	uncore->gsi_offset = gsi_offset;
> +
> +	intel_gt_common_init_early(gt);
> +	intel_uncore_init_early(uncore, gt);
> +
> +	/*
> +	 * Standalone media shares the general MMIO space with the primary
> +	 * GT.  We'll re-use the primary GT's mapping.
> +	 */
> +	uncore->regs = i915->uncore.regs;
> +	if (drm_WARN_ON(&i915->drm, uncore->regs == NULL))
> +		return -EIO;
> +
> +	gt->uncore = uncore;
> +	gt->phys_addr = phys_addr;
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/gt/intel_sa_media.h b/drivers/gpu/drm/i915/gt/intel_sa_media.h
> new file mode 100644
> index 000000000000..3afb310de932
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/intel_sa_media.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +#ifndef __INTEL_SA_MEDIA__
> +#define __INTEL_SA_MEDIA__
> +
> +#include <linux/types.h>
> +
> +struct intel_gt;
> +
> +int intel_sa_mediagt_setup(struct intel_gt *gt, phys_addr_t phys_addr,
> +			   u32 gsi_offset);
> +
> +#endif /* __INTEL_SA_MEDIA_H__ */
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 26b25d9434d6..18d3722331e4 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -26,6 +26,9 @@
>  #include <drm/drm_drv.h>
>  #include <drm/i915_pciids.h>
>  
> +#include "gt/intel_gt_regs.h"
> +#include "gt/intel_sa_media.h"
> +
>  #include "i915_driver.h"
>  #include "i915_drv.h"
>  #include "i915_pci.h"
> @@ -1115,6 +1118,17 @@ static const struct intel_device_info pvc_info = {
>  	.display.has_cdclk_crawl = 1, \
>  	.__runtime.fbc_mask = BIT(INTEL_FBC_A) | BIT(INTEL_FBC_B)
>  
> +static const struct intel_gt_definition xelpmp_extra_gt[] = {
> +	{
> +		.type = GT_MEDIA,
> +		.name = "Standalone Media GT",
> +		.setup = intel_sa_mediagt_setup,
> +		.gsi_offset = MTL_MEDIA_GSI_BASE,
> +		.engine_mask = BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
> +	},
> +	{}
> +};
> +
>  __maybe_unused
>  static const struct intel_device_info mtl_info = {
>  	XE_HP_FEATURES,
> @@ -1128,6 +1142,7 @@ static const struct intel_device_info mtl_info = {
>  	.media.ver = 13,
>  	PLATFORM(INTEL_METEORLAKE),
>  	.display.has_modular_fia = 1,
> +	.extra_gt_list = xelpmp_extra_gt,
>  	.has_flat_ccs = 0,
>  	.has_snoop = 1,
>  	.__runtime.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM,
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 85e0ef0e91b1..7b6d5341b34b 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -248,6 +248,7 @@ struct intel_runtime_info {
>  enum intel_gt_type {
>  	GT_PRIMARY,
>  	GT_TILE,
> +	GT_MEDIA,
>  };
>  
>  struct intel_gt_definition {
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index ecb02421502d..94fbc8cd986a 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -2271,6 +2271,10 @@ int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr)
>  
>  void intel_uncore_cleanup_mmio(struct intel_uncore *uncore)
>  {
> +	/* The media GT re-uses the primary GT's register mapping */
> +	if (uncore->gt->type == GT_MEDIA)
> +		return;
> +
>  	iounmap(uncore->regs);
>  }
>  
LGTM.
Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>

Thanks,
Aravind.