Re: [PATCH 3/6] drm/i915/xelpmp: Expose media as another GT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 05 Sep 2022 02:11:16 -0700, Jani Nikula wrote:
>

Copying author, these patches are from a different series
(https://patchwork.freedesktop.org/series/107908/) as mentioned in the
cover letter.

> On Fri, 02 Sep 2022, Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> wrote:
> > From: Matt Roper <matthew.d.roper@xxxxxxxxx>
> >
> > 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.
> >
> > Cc: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxx>
> > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/Makefile            |  1 +
> >  drivers/gpu/drm/i915/gt/intel_gt.c       | 12 ++++++--
> >  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 |  5 ++-
> >  drivers/gpu/drm/i915/intel_uncore.c      | 16 ++++++++--
> >  drivers/gpu/drm/i915/intel_uncore.h      | 20 ++++++++++--
> >  9 files changed, 123 insertions(+), 8 deletions(-)
> >  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.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> > index 57a6488c0e14..bfe77d01f747 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> > @@ -776,10 +776,15 @@ void intel_gt_driver_late_release_all(struct drm_i915_private *i915)
> >	}
> >  }
> >
> > -static int intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
> > +static int intel_gt_tile_setup(struct intel_gt *gt,
> > +			       phys_addr_t phys_addr,
> > +			       u32 gsi_offset)
> >  {
> >	int ret;
> >
> > +	/* GSI offset is only applicable for media GTs */
> > +	drm_WARN_ON(&gt->i915->drm, gsi_offset);
> > +
> >	if (!gt_is_root(gt)) {
> >		struct intel_uncore_mmio_debug *mmio_debug;
> >		struct intel_uncore *uncore;
> > @@ -840,7 +845,7 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> >	gt->info.engine_mask = RUNTIME_INFO(i915)->platform_engine_mask;
> >
> >	drm_dbg(&i915->drm, "Setting up %s\n", gt->name);
> > -	ret = intel_gt_tile_setup(gt, phys_addr);
> > +	ret = intel_gt_tile_setup(gt, phys_addr, 0);
> >	if (ret)
> >		return ret;
> >
> > @@ -873,7 +878,8 @@ int intel_gt_probe_all(struct drm_i915_private *i915)
> >			goto err;
> >		}
> >
> > -		ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base);
> > +		ret = gtdef->setup(gt, phys_addr + gtdef->mapping_base,
> > +				   gtdef->gsi_offset);
> >		if (ret)
> >			goto err;
> >
> > 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),
> > +	},
>
> Whoa. This would be the first time that we'd be adding function pointers
> in device info to set up stuff. I don't think we should start now. Just
> add the flags needed, and use that to call the appropriate function
> within gt.
>
> > +	{}
> > +};
> > +
> >  __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 b408ce384cd7..7b6d5341b34b 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -248,14 +248,17 @@ struct intel_runtime_info {
> >  enum intel_gt_type {
> >	GT_PRIMARY,
> >	GT_TILE,
> > +	GT_MEDIA,
> >  };
> >
> >  struct intel_gt_definition {
> >	enum intel_gt_type type;
> >	char *name;
> >	int (*setup)(struct intel_gt *gt,
> > -		     phys_addr_t phys_addr);
> > +		     phys_addr_t phys_addr,
> > +		     u32 gsi_offset);
> >	u32 mapping_base;
> > +	u32 gsi_offset;
> >	intel_engine_mask_t engine_mask;
> >  };
>
> I think gt definition belongs in gt.
>
> BR,
> Jani.
>
>
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 9b81b2543ce2..faec6c1aad66 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -1771,10 +1771,15 @@ __gen2_read(64)
> >  #undef GEN2_READ_FOOTER
> >  #undef GEN2_READ_HEADER
> >
> > +#define IS_GSI_REG(reg) ((reg) < 0x40000)
> > +
> >  #define GEN6_READ_HEADER(x) \
> > -	u32 offset = i915_mmio_reg_offset(reg); \
> > +	u32 offset; \
> >	unsigned long irqflags; \
> >	u##x val = 0; \
> > +	if (IS_GSI_REG(reg.reg)) \
> > +		reg.reg += uncore->gsi_offset; \
> > +	offset = i915_mmio_reg_offset(reg); \
> >	assert_rpm_wakelock_held(uncore->rpm); \
> >	spin_lock_irqsave(&uncore->lock, irqflags); \
> >	unclaimed_reg_debug(uncore, reg, true, true)
> > @@ -1876,8 +1881,11 @@ __gen2_write(32)
> >  #undef GEN2_WRITE_HEADER
> >
> >  #define GEN6_WRITE_HEADER \
> > -	u32 offset = i915_mmio_reg_offset(reg); \
> > +	u32 offset; \
> >	unsigned long irqflags; \
> > +	if (IS_GSI_REG(reg.reg)) \
> > +		reg.reg += uncore->gsi_offset; \
> > +	offset = i915_mmio_reg_offset(reg); \
> >	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
> >	assert_rpm_wakelock_held(uncore->rpm); \
> >	spin_lock_irqsave(&uncore->lock, irqflags); \
> > @@ -2256,6 +2264,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);
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> > index b1fa912a65e7..b25efdd9560e 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.h
> > +++ b/drivers/gpu/drm/i915/intel_uncore.h
> > @@ -135,6 +135,16 @@ struct intel_uncore {
> >
> >	spinlock_t lock; /** lock is also taken in irq contexts. */
> >
> > +	/*
> > +	 * Do we need to apply an additional offset to reach the beginning
> > +	 * of the basic non-engine GT registers (referred to as "GSI" on
> > +	 * newer platforms, or "GT block" on older platforms)?  If so, we'll
> > +	 * track that here and apply it transparently to registers in the
> > +	 * appropriate range to maintain compatibility with our existing
> > +	 * register definitions and GT code.
> > +	 */
> > +	u32 gsi_offset;
> > +
> >	unsigned int flags;
> >  #define UNCORE_HAS_FORCEWAKE		BIT(0)
> >  #define UNCORE_HAS_FPGA_DBG_UNCLAIMED	BIT(1)
> > @@ -299,14 +309,20 @@ intel_wait_for_register_fw(struct intel_uncore *uncore,
> >  static inline u##x__ __raw_uncore_read##x__(const struct intel_uncore *uncore, \
> >					    i915_reg_t reg) \
> >  { \
> > -	return read##s__(uncore->regs + i915_mmio_reg_offset(reg)); \
> > +	u32 offset = i915_mmio_reg_offset(reg); \
> > +	if (offset < 0x40000) \
> > +		offset += uncore->gsi_offset; \
> > +	return read##s__(uncore->regs + offset); \
> >  }
> >
> >  #define __raw_write(x__, s__) \
> >  static inline void __raw_uncore_write##x__(const struct intel_uncore *uncore, \
> >					   i915_reg_t reg, u##x__ val) \
> >  { \
> > -	write##s__(val, uncore->regs + i915_mmio_reg_offset(reg)); \
> > +	u32 offset = i915_mmio_reg_offset(reg); \
> > +	if (offset < 0x40000) \
> > +		offset += uncore->gsi_offset; \
> > +	write##s__(val, uncore->regs + offset); \
> >  }
> >  __raw_read(8, b)
> >  __raw_read(16, w)
>
> --
> Jani Nikula, Intel Open Source Graphics Center




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux