[PATCH 00/11] i915: Introduce Ponte Vecchio

[PATCH 00/11] i915: Introduce Ponte Vecchio

[Matt Roper]

Ponte Vecchio (PVC) is a new GPU based on the Xe_HPC architecture.  As a
compute-focused platform, PVC has compute engines and enhanced copy
engines, but no render engine (there is no geometry pipeline) and no
display.

This is just a handful of early enablement patches, including some
initial support for the new copy engines (although we're not yet adding
those to the platform's engine list or exposing them to userspace just
yet).


Ayaz A Siddiqui (1):
  drm/i915/pvc: Define MOCS table for PVC

John Harrison (1):
  drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter
    engine

Lucas De Marchi (2):
  drm/i915/pvc: skip all copy engines from aux table invalidate
  drm/i915/pvc: read fuses for link copy engines

Matt Roper (5):
  drm/i915/pvc: Add forcewake support
  drm/i915/pvc: Read correct RP_STATE_CAP register
  drm/i915/pvc: Engines definitions for new copy engines
  drm/i915/pvc: Interrupt support for new copy engines
  drm/i915/pvc: Reset support for new copy engines

Stuart Summers (2):
  drm/i915/pvc: add initial Ponte Vecchio definitions
  drm/i915/pvc: Remove additional 3D flags from PIPE_CONTROL

 drivers/gpu/drm/i915/gt/gen8_engine_cs.c      |  20 ++-
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  92 +++++++++++
 drivers/gpu/drm/i915/gt/intel_engine_types.h  |  10 +-
 drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  12 +-
 drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  16 ++
 drivers/gpu/drm/i915/gt/intel_gt_regs.h       |  56 ++++---
 drivers/gpu/drm/i915/gt/intel_gt_types.h      |   1 +
 drivers/gpu/drm/i915/gt/intel_mocs.c          |  24 ++-
 drivers/gpu/drm/i915/gt/intel_rps.c           |   4 +-
 drivers/gpu/drm/i915/gt/intel_workarounds.c   |  13 +-
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |   9 +-
 drivers/gpu/drm/i915/gvt/cmd_parser.c         |   2 +-
 drivers/gpu/drm/i915/i915_drv.h               |   6 +
 drivers/gpu/drm/i915/i915_pci.c               |  23 +++
 drivers/gpu/drm/i915/i915_reg.h               |   9 ++
 drivers/gpu/drm/i915/intel_device_info.c      |   1 +
 drivers/gpu/drm/i915/intel_device_info.h      |   5 +-
 drivers/gpu/drm/i915/intel_uncore.c           | 150 +++++++++++++++++-
 drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +
 19 files changed, 417 insertions(+), 38 deletions(-)

-- 
2.35.1

[PATCH 01/11] drm/i915/pvc: add initial Ponte Vecchio definitions

[Matt Roper]

From: Stuart Summers <stuart.summers@intel.com>

Additional blitter and media engines will be enabled later.

Bspec: 44481, 44482
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  2 ++
 drivers/gpu/drm/i915/i915_pci.c          | 21 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_device_info.c |  1 +
 drivers/gpu/drm/i915/intel_device_info.h |  1 +
 4 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 24111bf42ce0..2dddc27a1b0e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1062,6 +1062,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define IS_ALDERLAKE_P(dev_priv) IS_PLATFORM(dev_priv, INTEL_ALDERLAKE_P)
 #define IS_XEHPSDV(dev_priv) IS_PLATFORM(dev_priv, INTEL_XEHPSDV)
 #define IS_DG2(dev_priv)	IS_PLATFORM(dev_priv, INTEL_DG2)
+#define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO)
+
 #define IS_DG2_G10(dev_priv) \
 	IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10)
 #define IS_DG2_G11(dev_priv) \
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 7739d6c33481..498708b33924 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1074,6 +1074,27 @@ static const struct intel_device_info ats_m_info = {
 	.require_force_probe = 1,
 };
 
+#define XE_HPC_FEATURES \
+	XE_HP_FEATURES, \
+	.dma_mask_size = 52
+
+__maybe_unused
+static const struct intel_device_info pvc_info = {
+	XE_HPC_FEATURES,
+	XE_HPM_FEATURES,
+	DGFX_FEATURES,
+	.graphics.rel = 60,
+	.media.rel = 60,
+	PLATFORM(INTEL_PONTEVECCHIO),
+	.display = { 0 },
+	.has_flat_ccs = 0,
+	.platform_engine_mask =
+		BIT(BCS0) |
+		BIT(VCS0) |
+		BIT(CCS0) | BIT(CCS1) | BIT(CCS2) | BIT(CCS3),
+	.require_force_probe = 1,
+};
+
 #undef PLATFORM
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 63e05cd15a90..f0bf23726ed8 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -72,6 +72,7 @@ static const char * const platform_names[] = {
 	PLATFORM_NAME(ALDERLAKE_P),
 	PLATFORM_NAME(XEHPSDV),
 	PLATFORM_NAME(DG2),
+	PLATFORM_NAME(PONTEVECCHIO),
 };
 #undef PLATFORM_NAME
 
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 20c351c8d5bd..e7d2cf7d65c8 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -88,6 +88,7 @@ enum intel_platform {
 	INTEL_ALDERLAKE_P,
 	INTEL_XEHPSDV,
 	INTEL_DG2,
+	INTEL_PONTEVECCHIO,
 	INTEL_MAX_PLATFORMS
 };
 
-- 
2.35.1

Re: [PATCH 01/11] drm/i915/pvc: add initial Ponte Vecchio definitions

[Lucas De Marchi]

On Mon, May 02, 2022 at 09:34:07AM -0700, Matt Roper wrote:
>From: Stuart Summers <stuart.summers@intel.com>
>
>Additional blitter and media engines will be enabled later.
>
>Bspec: 44481, 44482
>Signed-off-by: Stuart Summers <stuart.summers@intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

[PATCH 02/11] drm/i915/pvc: Add forcewake support

[Matt Roper]

Add PVC's forcewake ranges.

Bspec: 67609
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c           | 150 +++++++++++++++++-
 drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +
 2 files changed, 151 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 83517a703eb6..3352065635e8 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1080,6 +1080,45 @@ static const struct i915_range dg2_shadowed_regs[] = {
 	{ .start = 0x1F8510, .end = 0x1F8550 },
 };
 
+static const struct i915_range pvc_shadowed_regs[] = {
+	{ .start =   0x2030, .end =   0x2030 },
+	{ .start =   0x2510, .end =   0x2550 },
+	{ .start =   0xA008, .end =   0xA00C },
+	{ .start =   0xA188, .end =   0xA188 },
+	{ .start =   0xA278, .end =   0xA278 },
+	{ .start =   0xA540, .end =   0xA56C },
+	{ .start =   0xC4C8, .end =   0xC4C8 },
+	{ .start =   0xC4E0, .end =   0xC4E0 },
+	{ .start =   0xC600, .end =   0xC600 },
+	{ .start =   0xC658, .end =   0xC658 },
+	{ .start =  0x22030, .end =  0x22030 },
+	{ .start =  0x22510, .end =  0x22550 },
+	{ .start = 0x1C0030, .end = 0x1C0030 },
+	{ .start = 0x1C0510, .end = 0x1C0550 },
+	{ .start = 0x1C4030, .end = 0x1C4030 },
+	{ .start = 0x1C4510, .end = 0x1C4550 },
+	{ .start = 0x1C8030, .end = 0x1C8030 },
+	{ .start = 0x1C8510, .end = 0x1C8550 },
+	{ .start = 0x1D0030, .end = 0x1D0030 },
+	{ .start = 0x1D0510, .end = 0x1D0550 },
+	{ .start = 0x1D4030, .end = 0x1D4030 },
+	{ .start = 0x1D4510, .end = 0x1D4550 },
+	{ .start = 0x1D8030, .end = 0x1D8030 },
+	{ .start = 0x1D8510, .end = 0x1D8550 },
+	{ .start = 0x1E0030, .end = 0x1E0030 },
+	{ .start = 0x1E0510, .end = 0x1E0550 },
+	{ .start = 0x1E4030, .end = 0x1E4030 },
+	{ .start = 0x1E4510, .end = 0x1E4550 },
+	{ .start = 0x1E8030, .end = 0x1E8030 },
+	{ .start = 0x1E8510, .end = 0x1E8550 },
+	{ .start = 0x1F0030, .end = 0x1F0030 },
+	{ .start = 0x1F0510, .end = 0x1F0550 },
+	{ .start = 0x1F4030, .end = 0x1F4030 },
+	{ .start = 0x1F4510, .end = 0x1F4550 },
+	{ .start = 0x1F8030, .end = 0x1F8030 },
+	{ .start = 0x1F8510, .end = 0x1F8550 },
+};
+
 static int mmio_range_cmp(u32 key, const struct i915_range *range)
 {
 	if (key < range->start)
@@ -1490,6 +1529,111 @@ static const struct intel_forcewake_range __dg2_fw_ranges[] = {
 	XEHP_FWRANGES(FORCEWAKE_RENDER)
 };
 
+/*
+ * *Must* be sorted by offset ranges! See intel_fw_table_check().
+ *
+ * Note that the spec lists several reserved/unused ranges that don't actually
+ * contain any registers.  In the table below we'll combine those reserved
+ * ranges with either the preceding or following range to keep the table small
+ * and lookups fast.
+ */
+static const struct intel_forcewake_range __pvc_fw_ranges[] = {
+	GEN_FW_RANGE(0x0, 0xaff, 0),
+	GEN_FW_RANGE(0xb00, 0xbff, FORCEWAKE_GT),
+	GEN_FW_RANGE(0xc00, 0xfff, 0),
+	GEN_FW_RANGE(0x1000, 0x1fff, FORCEWAKE_GT),
+	GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_GT),
+	GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x4000, 0x813f, FORCEWAKE_GT), /*
+		0x4000 - 0x4aff: gt
+		0x4b00 - 0x4fff: reserved
+		0x5000 - 0x51ff: gt
+		0x5200 - 0x52ff: reserved
+		0x5300 - 0x53ff: gt
+		0x5400 - 0x7fff: reserved
+		0x8000 - 0x813f: gt */
+	GEN_FW_RANGE(0x8140, 0x817f, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x8180, 0x81ff, 0),
+	GEN_FW_RANGE(0x8200, 0x94cf, FORCEWAKE_GT), /*
+		0x8200 - 0x82ff: gt
+		0x8300 - 0x84ff: reserved
+		0x8500 - 0x887f: gt
+		0x8880 - 0x8a7f: reserved
+		0x8a80 - 0x8aff: gt
+		0x8b00 - 0x8fff: reserved
+		0x9000 - 0x947f: gt
+		0x9480 - 0x94cf: reserved */
+	GEN_FW_RANGE(0x94d0, 0x955f, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0x9560, 0x967f, 0), /*
+		0x9560 - 0x95ff: always on
+		0x9600 - 0x967f: reserved */
+	GEN_FW_RANGE(0x9680, 0x97ff, FORCEWAKE_RENDER), /*
+		0x9680 - 0x96ff: render
+		0x9700 - 0x97ff: reserved */
+	GEN_FW_RANGE(0x9800, 0xcfff, FORCEWAKE_GT), /*
+		0x9800 - 0xb4ff: gt
+		0xb500 - 0xbfff: reserved
+		0xc000 - 0xcfff: gt */
+	GEN_FW_RANGE(0xd000, 0xd3ff, 0),
+	GEN_FW_RANGE(0xd400, 0xdbff, FORCEWAKE_GT),
+	GEN_FW_RANGE(0xdc00, 0xdcff, FORCEWAKE_RENDER),
+	GEN_FW_RANGE(0xdd00, 0xde7f, FORCEWAKE_GT), /*
+		0xdd00 - 0xddff: gt
+		0xde00 - 0xde7f: reserved */
+	GEN_FW_RANGE(0xde80, 0xe8ff, FORCEWAKE_RENDER), /*
+		0xde80 - 0xdeff: render
+		0xdf00 - 0xe1ff: reserved
+		0xe200 - 0xe7ff: render
+		0xe800 - 0xe8ff: reserved */
+	GEN_FW_RANGE(0xe900, 0x11fff, FORCEWAKE_GT), /*
+		 0xe900 -  0xe9ff: gt
+		 0xea00 -  0xebff: reserved
+		 0xec00 -  0xffff: gt
+		0x10000 - 0x11fff: reserved */
+	GEN_FW_RANGE(0x12000, 0x12fff, 0), /*
+		0x12000 - 0x127ff: always on
+		0x12800 - 0x12fff: reserved */
+	GEN_FW_RANGE(0x13000, 0x23fff, FORCEWAKE_GT), /*
+		0x13000 - 0x135ff: gt
+		0x13600 - 0x147ff: reserved
+		0x14800 - 0x153ff: gt
+		0x15400 - 0x19fff: reserved
+		0x1a000 - 0x1ffff: gt
+		0x20000 - 0x21fff: reserved
+		0x22000 - 0x23fff: gt */
+	GEN_FW_RANGE(0x24000, 0x2417f, 0), /*
+		24000 - 0x2407f: always on
+		24080 - 0x2417f: reserved */
+	GEN_FW_RANGE(0x24180, 0x3ffff, FORCEWAKE_GT), /*
+		0x24180 - 0x241ff: gt
+		0x24200 - 0x251ff: reserved
+		0x25200 - 0x252ff: gt
+		0x25300 - 0x25fff: reserved
+		0x26000 - 0x27fff: gt
+		0x28000 - 0x2ffff: reserved
+		0x30000 - 0x3ffff: gt */
+	GEN_FW_RANGE(0x40000, 0x1bffff, 0),
+	GEN_FW_RANGE(0x1c0000, 0x1c3fff, FORCEWAKE_MEDIA_VDBOX0), /*
+		0x1c0000 - 0x1c2bff: VD0
+		0x1c2c00 - 0x1c2cff: reserved
+		0x1c2d00 - 0x1c2dff: VD0
+		0x1c2e00 - 0x1c3eff: reserved
+		0x1c3f00 - 0x1c3fff: VD0 */
+	GEN_FW_RANGE(0x1c4000, 0x1cffff, FORCEWAKE_MEDIA_VDBOX1), /*
+		0x1c4000 - 0x1c6aff: VD1
+		0x1c6b00 - 0x1c7eff: reserved
+		0x1c7f00 - 0x1c7fff: VD1
+		0x1c8000 - 0x1cffff: reserved */
+	GEN_FW_RANGE(0x1d0000, 0x23ffff, FORCEWAKE_MEDIA_VDBOX2), /*
+		0x1d0000 - 0x1d2aff: VD2
+		0x1d2b00 - 0x1d3eff: reserved
+		0x1d3f00 - 0x1d3fff: VD2
+		0x1d4000 - 0x23ffff: reserved */
+	GEN_FW_RANGE(0x240000, 0x3dffff, 0),
+	GEN_FW_RANGE(0x3e0000, 0x3effff, FORCEWAKE_GT),
+};
+
 static void
 ilk_dummy_write(struct intel_uncore *uncore)
 {
@@ -2125,7 +2269,11 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
 
 	ASSIGN_READ_MMIO_VFUNCS(uncore, fwtable);
 
-	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55)) {
+	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 60)) {
+		ASSIGN_FW_DOMAINS_TABLE(uncore, __pvc_fw_ranges);
+		ASSIGN_SHADOW_TABLE(uncore, pvc_shadowed_regs);
+		ASSIGN_WRITE_MMIO_VFUNCS(uncore, fwtable);
+	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55)) {
 		ASSIGN_FW_DOMAINS_TABLE(uncore, __dg2_fw_ranges);
 		ASSIGN_SHADOW_TABLE(uncore, dg2_shadowed_regs);
 		ASSIGN_WRITE_MMIO_VFUNCS(uncore, fwtable);
diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c b/drivers/gpu/drm/i915/selftests/intel_uncore.c
index cdd196783535..fda9bb79c049 100644
--- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
+++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
@@ -69,6 +69,7 @@ static int intel_shadow_table_check(void)
 		{ gen11_shadowed_regs, ARRAY_SIZE(gen11_shadowed_regs) },
 		{ gen12_shadowed_regs, ARRAY_SIZE(gen12_shadowed_regs) },
 		{ dg2_shadowed_regs, ARRAY_SIZE(dg2_shadowed_regs) },
+		{ pvc_shadowed_regs, ARRAY_SIZE(pvc_shadowed_regs) },
 	};
 	const struct i915_range *range;
 	unsigned int i, j;
@@ -115,6 +116,7 @@ int intel_uncore_mock_selftests(void)
 		{ __gen11_fw_ranges, ARRAY_SIZE(__gen11_fw_ranges), true },
 		{ __gen12_fw_ranges, ARRAY_SIZE(__gen12_fw_ranges), true },
 		{ __xehp_fw_ranges, ARRAY_SIZE(__xehp_fw_ranges), true },
+		{ __pvc_fw_ranges, ARRAY_SIZE(__pvc_fw_ranges), true },
 	};
 	int err, i;
 
-- 
2.35.1

Re: [PATCH 02/11] drm/i915/pvc: Add forcewake support

[Summers, Stuart]

On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> Add PVC's forcewake ranges.
> 
> Bspec: 67609
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c           | 150
> +++++++++++++++++-
>  drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +
>  2 files changed, 151 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 83517a703eb6..3352065635e8 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1080,6 +1080,45 @@ static const struct i915_range
> dg2_shadowed_regs[] = {
>  	{ .start = 0x1F8510, .end = 0x1F8550 },
>  };
>  
> +static const struct i915_range pvc_shadowed_regs[] = {
> +	{ .start =   0x2030, .end =   0x2030 },
> +	{ .start =   0x2510, .end =   0x2550 },
> +	{ .start =   0xA008, .end =   0xA00C },
> +	{ .start =   0xA188, .end =   0xA188 },
> +	{ .start =   0xA278, .end =   0xA278 },
> +	{ .start =   0xA540, .end =   0xA56C },
> +	{ .start =   0xC4C8, .end =   0xC4C8 },
> +	{ .start =   0xC4E0, .end =   0xC4E0 },
> +	{ .start =   0xC600, .end =   0xC600 },
> +	{ .start =   0xC658, .end =   0xC658 },
> +	{ .start =  0x22030, .end =  0x22030 },
> +	{ .start =  0x22510, .end =  0x22550 },
> +	{ .start = 0x1C0030, .end = 0x1C0030 },
> +	{ .start = 0x1C0510, .end = 0x1C0550 },
> +	{ .start = 0x1C4030, .end = 0x1C4030 },
> +	{ .start = 0x1C4510, .end = 0x1C4550 },
> +	{ .start = 0x1C8030, .end = 0x1C8030 },
> +	{ .start = 0x1C8510, .end = 0x1C8550 },
> +	{ .start = 0x1D0030, .end = 0x1D0030 },
> +	{ .start = 0x1D0510, .end = 0x1D0550 },
> +	{ .start = 0x1D4030, .end = 0x1D4030 },
> +	{ .start = 0x1D4510, .end = 0x1D4550 },
> +	{ .start = 0x1D8030, .end = 0x1D8030 },
> +	{ .start = 0x1D8510, .end = 0x1D8550 },
> +	{ .start = 0x1E0030, .end = 0x1E0030 },
> +	{ .start = 0x1E0510, .end = 0x1E0550 },
> +	{ .start = 0x1E4030, .end = 0x1E4030 },
> +	{ .start = 0x1E4510, .end = 0x1E4550 },
> +	{ .start = 0x1E8030, .end = 0x1E8030 },
> +	{ .start = 0x1E8510, .end = 0x1E8550 },
> +	{ .start = 0x1F0030, .end = 0x1F0030 },
> +	{ .start = 0x1F0510, .end = 0x1F0550 },
> +	{ .start = 0x1F4030, .end = 0x1F4030 },
> +	{ .start = 0x1F4510, .end = 0x1F4550 },
> +	{ .start = 0x1F8030, .end = 0x1F8030 },
> +	{ .start = 0x1F8510, .end = 0x1F8550 },
> +};
> +
>  static int mmio_range_cmp(u32 key, const struct i915_range *range)
>  {
>  	if (key < range->start)
> @@ -1490,6 +1529,111 @@ static const struct intel_forcewake_range
> __dg2_fw_ranges[] = {
>  	XEHP_FWRANGES(FORCEWAKE_RENDER)
>  };
>  
> +/*
> + * *Must* be sorted by offset ranges! See intel_fw_table_check().
> + *
> + * Note that the spec lists several reserved/unused ranges that
> don't actually
> + * contain any registers.  In the table below we'll combine those
> reserved
> + * ranges with either the preceding or following range to keep the
> table small

Looks like not just the reserved ranges are being used here. Maybe add
"combine all ranges with preceding or following range with similar FW
unit" or something similar.

Thanks,
Stuart

> + * and lookups fast.
> + */
> +static const struct intel_forcewake_range __pvc_fw_ranges[] = {
> +	GEN_FW_RANGE(0x0, 0xaff, 0),
> +	GEN_FW_RANGE(0xb00, 0xbff, FORCEWAKE_GT),
> +	GEN_FW_RANGE(0xc00, 0xfff, 0),
> +	GEN_FW_RANGE(0x1000, 0x1fff, FORCEWAKE_GT),
> +	GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_GT),
> +	GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x4000, 0x813f, FORCEWAKE_GT), /*
> +		0x4000 - 0x4aff: gt
> +		0x4b00 - 0x4fff: reserved
> +		0x5000 - 0x51ff: gt
> +		0x5200 - 0x52ff: reserved
> +		0x5300 - 0x53ff: gt
> +		0x5400 - 0x7fff: reserved
> +		0x8000 - 0x813f: gt */
> +	GEN_FW_RANGE(0x8140, 0x817f, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x8180, 0x81ff, 0),
> +	GEN_FW_RANGE(0x8200, 0x94cf, FORCEWAKE_GT), /*
> +		0x8200 - 0x82ff: gt
> +		0x8300 - 0x84ff: reserved
> +		0x8500 - 0x887f: gt
> +		0x8880 - 0x8a7f: reserved
> +		0x8a80 - 0x8aff: gt
> +		0x8b00 - 0x8fff: reserved
> +		0x9000 - 0x947f: gt
> +		0x9480 - 0x94cf: reserved */
> +	GEN_FW_RANGE(0x94d0, 0x955f, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0x9560, 0x967f, 0), /*
> +		0x9560 - 0x95ff: always on
> +		0x9600 - 0x967f: reserved */
> +	GEN_FW_RANGE(0x9680, 0x97ff, FORCEWAKE_RENDER), /*
> +		0x9680 - 0x96ff: render
> +		0x9700 - 0x97ff: reserved */
> +	GEN_FW_RANGE(0x9800, 0xcfff, FORCEWAKE_GT), /*
> +		0x9800 - 0xb4ff: gt
> +		0xb500 - 0xbfff: reserved
> +		0xc000 - 0xcfff: gt */
> +	GEN_FW_RANGE(0xd000, 0xd3ff, 0),
> +	GEN_FW_RANGE(0xd400, 0xdbff, FORCEWAKE_GT),
> +	GEN_FW_RANGE(0xdc00, 0xdcff, FORCEWAKE_RENDER),
> +	GEN_FW_RANGE(0xdd00, 0xde7f, FORCEWAKE_GT), /*
> +		0xdd00 - 0xddff: gt
> +		0xde00 - 0xde7f: reserved */
> +	GEN_FW_RANGE(0xde80, 0xe8ff, FORCEWAKE_RENDER), /*
> +		0xde80 - 0xdeff: render
> +		0xdf00 - 0xe1ff: reserved
> +		0xe200 - 0xe7ff: render
> +		0xe800 - 0xe8ff: reserved */
> +	GEN_FW_RANGE(0xe900, 0x11fff, FORCEWAKE_GT), /*
> +		 0xe900 -  0xe9ff: gt
> +		 0xea00 -  0xebff: reserved
> +		 0xec00 -  0xffff: gt
> +		0x10000 - 0x11fff: reserved */
> +	GEN_FW_RANGE(0x12000, 0x12fff, 0), /*
> +		0x12000 - 0x127ff: always on
> +		0x12800 - 0x12fff: reserved */
> +	GEN_FW_RANGE(0x13000, 0x23fff, FORCEWAKE_GT), /*
> +		0x13000 - 0x135ff: gt
> +		0x13600 - 0x147ff: reserved
> +		0x14800 - 0x153ff: gt
> +		0x15400 - 0x19fff: reserved
> +		0x1a000 - 0x1ffff: gt
> +		0x20000 - 0x21fff: reserved
> +		0x22000 - 0x23fff: gt */
> +	GEN_FW_RANGE(0x24000, 0x2417f, 0), /*
> +		24000 - 0x2407f: always on
> +		24080 - 0x2417f: reserved */
> +	GEN_FW_RANGE(0x24180, 0x3ffff, FORCEWAKE_GT), /*
> +		0x24180 - 0x241ff: gt
> +		0x24200 - 0x251ff: reserved
> +		0x25200 - 0x252ff: gt
> +		0x25300 - 0x25fff: reserved
> +		0x26000 - 0x27fff: gt
> +		0x28000 - 0x2ffff: reserved
> +		0x30000 - 0x3ffff: gt */
> +	GEN_FW_RANGE(0x40000, 0x1bffff, 0),
> +	GEN_FW_RANGE(0x1c0000, 0x1c3fff, FORCEWAKE_MEDIA_VDBOX0), /*
> +		0x1c0000 - 0x1c2bff: VD0
> +		0x1c2c00 - 0x1c2cff: reserved
> +		0x1c2d00 - 0x1c2dff: VD0
> +		0x1c2e00 - 0x1c3eff: reserved
> +		0x1c3f00 - 0x1c3fff: VD0 */
> +	GEN_FW_RANGE(0x1c4000, 0x1cffff, FORCEWAKE_MEDIA_VDBOX1), /*
> +		0x1c4000 - 0x1c6aff: VD1
> +		0x1c6b00 - 0x1c7eff: reserved
> +		0x1c7f00 - 0x1c7fff: VD1
> +		0x1c8000 - 0x1cffff: reserved */
> +	GEN_FW_RANGE(0x1d0000, 0x23ffff, FORCEWAKE_MEDIA_VDBOX2), /*
> +		0x1d0000 - 0x1d2aff: VD2
> +		0x1d2b00 - 0x1d3eff: reserved
> +		0x1d3f00 - 0x1d3fff: VD2
> +		0x1d4000 - 0x23ffff: reserved */
> +	GEN_FW_RANGE(0x240000, 0x3dffff, 0),
> +	GEN_FW_RANGE(0x3e0000, 0x3effff, FORCEWAKE_GT),
> +};
> +
>  static void
>  ilk_dummy_write(struct intel_uncore *uncore)
>  {
> @@ -2125,7 +2269,11 @@ static int uncore_forcewake_init(struct
> intel_uncore *uncore)
>  
>  	ASSIGN_READ_MMIO_VFUNCS(uncore, fwtable);
>  
> -	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55)) {
> +	if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 60)) {
> +		ASSIGN_FW_DOMAINS_TABLE(uncore, __pvc_fw_ranges);
> +		ASSIGN_SHADOW_TABLE(uncore, pvc_shadowed_regs);
> +		ASSIGN_WRITE_MMIO_VFUNCS(uncore, fwtable);
> +	} else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55)) {
>  		ASSIGN_FW_DOMAINS_TABLE(uncore, __dg2_fw_ranges);
>  		ASSIGN_SHADOW_TABLE(uncore, dg2_shadowed_regs);
>  		ASSIGN_WRITE_MMIO_VFUNCS(uncore, fwtable);
> diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> index cdd196783535..fda9bb79c049 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> @@ -69,6 +69,7 @@ static int intel_shadow_table_check(void)
>  		{ gen11_shadowed_regs, ARRAY_SIZE(gen11_shadowed_regs)
> },
>  		{ gen12_shadowed_regs, ARRAY_SIZE(gen12_shadowed_regs)
> },
>  		{ dg2_shadowed_regs, ARRAY_SIZE(dg2_shadowed_regs) },
> +		{ pvc_shadowed_regs, ARRAY_SIZE(pvc_shadowed_regs) },
>  	};
>  	const struct i915_range *range;
>  	unsigned int i, j;
> @@ -115,6 +116,7 @@ int intel_uncore_mock_selftests(void)
>  		{ __gen11_fw_ranges, ARRAY_SIZE(__gen11_fw_ranges),
> true },
>  		{ __gen12_fw_ranges, ARRAY_SIZE(__gen12_fw_ranges),
> true },
>  		{ __xehp_fw_ranges, ARRAY_SIZE(__xehp_fw_ranges), true
> },
> +		{ __pvc_fw_ranges, ARRAY_SIZE(__pvc_fw_ranges), true },
>  	};
>  	int err, i;
>  

Re: [PATCH 02/11] drm/i915/pvc: Add forcewake support

[Matt Roper]

On Mon, May 02, 2022 at 03:33:53PM -0700, Summers, Stuart wrote:
> On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> > Add PVC's forcewake ranges.
> >
> > Bspec: 67609
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_uncore.c           | 150
> > +++++++++++++++++-
> >  drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +
> >  2 files changed, 151 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> > b/drivers/gpu/drm/i915/intel_uncore.c
> > index 83517a703eb6..3352065635e8 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -1080,6 +1080,45 @@ static const struct i915_range
> > dg2_shadowed_regs[] = {
> >       { .start = 0x1F8510, .end = 0x1F8550 },
> >  };
> >
> > +static const struct i915_range pvc_shadowed_regs[] = {
> > +     { .start =   0x2030, .end =   0x2030 },
> > +     { .start =   0x2510, .end =   0x2550 },
> > +     { .start =   0xA008, .end =   0xA00C },
> > +     { .start =   0xA188, .end =   0xA188 },
> > +     { .start =   0xA278, .end =   0xA278 },
> > +     { .start =   0xA540, .end =   0xA56C },
> > +     { .start =   0xC4C8, .end =   0xC4C8 },
> > +     { .start =   0xC4E0, .end =   0xC4E0 },
> > +     { .start =   0xC600, .end =   0xC600 },
> > +     { .start =   0xC658, .end =   0xC658 },
> > +     { .start =  0x22030, .end =  0x22030 },
> > +     { .start =  0x22510, .end =  0x22550 },
> > +     { .start = 0x1C0030, .end = 0x1C0030 },
> > +     { .start = 0x1C0510, .end = 0x1C0550 },
> > +     { .start = 0x1C4030, .end = 0x1C4030 },
> > +     { .start = 0x1C4510, .end = 0x1C4550 },
> > +     { .start = 0x1C8030, .end = 0x1C8030 },
> > +     { .start = 0x1C8510, .end = 0x1C8550 },
> > +     { .start = 0x1D0030, .end = 0x1D0030 },
> > +     { .start = 0x1D0510, .end = 0x1D0550 },
> > +     { .start = 0x1D4030, .end = 0x1D4030 },
> > +     { .start = 0x1D4510, .end = 0x1D4550 },
> > +     { .start = 0x1D8030, .end = 0x1D8030 },
> > +     { .start = 0x1D8510, .end = 0x1D8550 },
> > +     { .start = 0x1E0030, .end = 0x1E0030 },
> > +     { .start = 0x1E0510, .end = 0x1E0550 },
> > +     { .start = 0x1E4030, .end = 0x1E4030 },
> > +     { .start = 0x1E4510, .end = 0x1E4550 },
> > +     { .start = 0x1E8030, .end = 0x1E8030 },
> > +     { .start = 0x1E8510, .end = 0x1E8550 },
> > +     { .start = 0x1F0030, .end = 0x1F0030 },
> > +     { .start = 0x1F0510, .end = 0x1F0550 },
> > +     { .start = 0x1F4030, .end = 0x1F4030 },
> > +     { .start = 0x1F4510, .end = 0x1F4550 },
> > +     { .start = 0x1F8030, .end = 0x1F8030 },
> > +     { .start = 0x1F8510, .end = 0x1F8550 },
> > +};
> > +
> >  static int mmio_range_cmp(u32 key, const struct i915_range *range)
> >  {
> >       if (key < range->start)
> > @@ -1490,6 +1529,111 @@ static const struct intel_forcewake_range
> > __dg2_fw_ranges[] = {
> >       XEHP_FWRANGES(FORCEWAKE_RENDER)
> >  };
> >
> > +/*
> > + * *Must* be sorted by offset ranges! See intel_fw_table_check().
> > + *
> > + * Note that the spec lists several reserved/unused ranges that
> > don't actually
> > + * contain any registers.  In the table below we'll combine those
> > reserved
> > + * ranges with either the preceding or following range to keep the
> > table small
> 
> Looks like not just the reserved ranges are being used here. Maybe add
> "combine all ranges with preceding or following range with similar FW
> unit" or something similar.

The comment here is a duplicate of the one on the TGL table.  We've been
copy/pasting it onto new forcewake tables, but we should probably just
delete all except one of those comments and clarify that it applies to
all tables now (and possibly tweak the wording if necessary).

By "similar FW unit" are you referring to where the bspec has table rows
like:

        0x1000 - 0x1fff:  GT
        0x2000 - 0x2fff:  GT

so the same domain shows up on consecutive rows?  The fact that those
even show up as two rows in the bspec is a relatively new change due to
the hardware people starting to document additional information in the
same table (e.g., multicast register characteristics); back when the
comment was written the extra information hadn't been added so there
really was only a single consolidated range in the spec for cases like
this.


Matt

> 
> Thanks,
> Stuart
> 
> > + * and lookups fast.
> > + */
> > +static const struct intel_forcewake_range __pvc_fw_ranges[] = {
> > +     GEN_FW_RANGE(0x0, 0xaff, 0),
> > +     GEN_FW_RANGE(0xb00, 0xbff, FORCEWAKE_GT),
> > +     GEN_FW_RANGE(0xc00, 0xfff, 0),
> > +     GEN_FW_RANGE(0x1000, 0x1fff, FORCEWAKE_GT),
> > +     GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
> > +     GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_GT),
> > +     GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
> > +     GEN_FW_RANGE(0x4000, 0x813f, FORCEWAKE_GT), /*
> > +             0x4000 - 0x4aff: gt
> > +             0x4b00 - 0x4fff: reserved
> > +             0x5000 - 0x51ff: gt
> > +             0x5200 - 0x52ff: reserved
> > +             0x5300 - 0x53ff: gt
> > +             0x5400 - 0x7fff: reserved
> > +             0x8000 - 0x813f: gt */
> > +     GEN_FW_RANGE(0x8140, 0x817f, FORCEWAKE_RENDER),
> > +     GEN_FW_RANGE(0x8180, 0x81ff, 0),
> > +     GEN_FW_RANGE(0x8200, 0x94cf, FORCEWAKE_GT), /*
> > +             0x8200 - 0x82ff: gt
> > +             0x8300 - 0x84ff: reserved
> > +             0x8500 - 0x887f: gt
> > +             0x8880 - 0x8a7f: reserved
> > +             0x8a80 - 0x8aff: gt
> > +             0x8b00 - 0x8fff: reserved
> > +             0x9000 - 0x947f: gt
> > +             0x9480 - 0x94cf: reserved */
> > +     GEN_FW_RANGE(0x94d0, 0x955f, FORCEWAKE_RENDER),
> > +     GEN_FW_RANGE(0x9560, 0x967f, 0), /*
> > +             0x9560 - 0x95ff: always on
> > +             0x9600 - 0x967f: reserved */
> > +     GEN_FW_RANGE(0x9680, 0x97ff, FORCEWAKE_RENDER), /*
> > +             0x9680 - 0x96ff: render
> > +             0x9700 - 0x97ff: reserved */
> > +     GEN_FW_RANGE(0x9800, 0xcfff, FORCEWAKE_GT), /*
> > +             0x9800 - 0xb4ff: gt
> > +             0xb500 - 0xbfff: reserved
> > +             0xc000 - 0xcfff: gt */
> > +     GEN_FW_RANGE(0xd000, 0xd3ff, 0),
> > +     GEN_FW_RANGE(0xd400, 0xdbff, FORCEWAKE_GT),
> > +     GEN_FW_RANGE(0xdc00, 0xdcff, FORCEWAKE_RENDER),
> > +     GEN_FW_RANGE(0xdd00, 0xde7f, FORCEWAKE_GT), /*
> > +             0xdd00 - 0xddff: gt
> > +             0xde00 - 0xde7f: reserved */
> > +     GEN_FW_RANGE(0xde80, 0xe8ff, FORCEWAKE_RENDER), /*
> > +             0xde80 - 0xdeff: render
> > +             0xdf00 - 0xe1ff: reserved
> > +             0xe200 - 0xe7ff: render
> > +             0xe800 - 0xe8ff: reserved */
> > +     GEN_FW_RANGE(0xe900, 0x11fff, FORCEWAKE_GT), /*
> > +              0xe900 -  0xe9ff: gt
> > +              0xea00 -  0xebff: reserved
> > +              0xec00 -  0xffff: gt
> > +             0x10000 - 0x11fff: reserved */
> > +     GEN_FW_RANGE(0x12000, 0x12fff, 0), /*
> > +             0x12000 - 0x127ff: always on
> > +             0x12800 - 0x12fff: reserved */
> > +     GEN_FW_RANGE(0x13000, 0x23fff, FORCEWAKE_GT), /*
> > +             0x13000 - 0x135ff: gt
> > +             0x13600 - 0x147ff: reserved
> > +             0x14800 - 0x153ff: gt
> > +             0x15400 - 0x19fff: reserved
> > +             0x1a000 - 0x1ffff: gt
> > +             0x20000 - 0x21fff: reserved
> > +             0x22000 - 0x23fff: gt */
> > +     GEN_FW_RANGE(0x24000, 0x2417f, 0), /*
> > +             24000 - 0x2407f: always on
> > +             24080 - 0x2417f: reserved */
> > +     GEN_FW_RANGE(0x24180, 0x3ffff, FORCEWAKE_GT), /*
> > +             0x24180 - 0x241ff: gt
> > +             0x24200 - 0x251ff: reserved
> > +             0x25200 - 0x252ff: gt
> > +             0x25300 - 0x25fff: reserved
> > +             0x26000 - 0x27fff: gt
> > +             0x28000 - 0x2ffff: reserved
> > +             0x30000 - 0x3ffff: gt */
> > +     GEN_FW_RANGE(0x40000, 0x1bffff, 0),
> > +     GEN_FW_RANGE(0x1c0000, 0x1c3fff, FORCEWAKE_MEDIA_VDBOX0), /*
> > +             0x1c0000 - 0x1c2bff: VD0
> > +             0x1c2c00 - 0x1c2cff: reserved
> > +             0x1c2d00 - 0x1c2dff: VD0
> > +             0x1c2e00 - 0x1c3eff: reserved
> > +             0x1c3f00 - 0x1c3fff: VD0 */
> > +     GEN_FW_RANGE(0x1c4000, 0x1cffff, FORCEWAKE_MEDIA_VDBOX1), /*
> > +             0x1c4000 - 0x1c6aff: VD1
> > +             0x1c6b00 - 0x1c7eff: reserved
> > +             0x1c7f00 - 0x1c7fff: VD1
> > +             0x1c8000 - 0x1cffff: reserved */
> > +     GEN_FW_RANGE(0x1d0000, 0x23ffff, FORCEWAKE_MEDIA_VDBOX2), /*
> > +             0x1d0000 - 0x1d2aff: VD2
> > +             0x1d2b00 - 0x1d3eff: reserved
> > +             0x1d3f00 - 0x1d3fff: VD2
> > +             0x1d4000 - 0x23ffff: reserved */
> > +     GEN_FW_RANGE(0x240000, 0x3dffff, 0),
> > +     GEN_FW_RANGE(0x3e0000, 0x3effff, FORCEWAKE_GT),
> > +};
> > +
> >  static void
> >  ilk_dummy_write(struct intel_uncore *uncore)
> >  {
> > @@ -2125,7 +2269,11 @@ static int uncore_forcewake_init(struct
> > intel_uncore *uncore)
> >
> >       ASSIGN_READ_MMIO_VFUNCS(uncore, fwtable);
> >
> > -     if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55)) {
> > +     if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 60)) {
> > +             ASSIGN_FW_DOMAINS_TABLE(uncore, __pvc_fw_ranges);
> > +             ASSIGN_SHADOW_TABLE(uncore, pvc_shadowed_regs);
> > +             ASSIGN_WRITE_MMIO_VFUNCS(uncore, fwtable);
> > +     } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 55)) {
> >               ASSIGN_FW_DOMAINS_TABLE(uncore, __dg2_fw_ranges);
> >               ASSIGN_SHADOW_TABLE(uncore, dg2_shadowed_regs);
> >               ASSIGN_WRITE_MMIO_VFUNCS(uncore, fwtable);
> > diff --git a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> > b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> > index cdd196783535..fda9bb79c049 100644
> > --- a/drivers/gpu/drm/i915/selftests/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/selftests/intel_uncore.c
> > @@ -69,6 +69,7 @@ static int intel_shadow_table_check(void)
> >               { gen11_shadowed_regs, ARRAY_SIZE(gen11_shadowed_regs)
> > },
> >               { gen12_shadowed_regs, ARRAY_SIZE(gen12_shadowed_regs)
> > },
> >               { dg2_shadowed_regs, ARRAY_SIZE(dg2_shadowed_regs) },
> > +             { pvc_shadowed_regs, ARRAY_SIZE(pvc_shadowed_regs) },
> >       };
> >       const struct i915_range *range;
> >       unsigned int i, j;
> > @@ -115,6 +116,7 @@ int intel_uncore_mock_selftests(void)
> >               { __gen11_fw_ranges, ARRAY_SIZE(__gen11_fw_ranges),
> > true },
> >               { __gen12_fw_ranges, ARRAY_SIZE(__gen12_fw_ranges),
> > true },
> >               { __xehp_fw_ranges, ARRAY_SIZE(__xehp_fw_ranges), true
> > },
> > +             { __pvc_fw_ranges, ARRAY_SIZE(__pvc_fw_ranges), true },
> >       };
> >       int err, i;
> >

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

[PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Matt Roper]

From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>

Bspec: 45101, 72161
Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
Signed-off-by: Fei Yang <fei.yang@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_types.h    |  1 +
 drivers/gpu/drm/i915/gt/intel_mocs.c        | 24 ++++++++++++++++++++-
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 ++++++++---
 drivers/gpu/drm/i915/i915_drv.h             |  2 ++
 drivers/gpu/drm/i915/i915_pci.c             |  3 ++-
 drivers/gpu/drm/i915/intel_device_info.h    |  1 +
 6 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index b06611c1d4ad..7853ea194ea6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -221,6 +221,7 @@ struct intel_gt {
 
 	struct {
 		u8 uc_index;
+		u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
 	} mocs;
 
 	struct intel_pxp pxp;
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
index c4c37585ae8c..265812589f87 100644
--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
@@ -23,6 +23,7 @@ struct drm_i915_mocs_table {
 	unsigned int n_entries;
 	const struct drm_i915_mocs_entry *table;
 	u8 uc_index;
+	u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
 	u8 unused_entries_index;
 };
 
@@ -47,6 +48,7 @@ struct drm_i915_mocs_table {
 
 /* Helper defines */
 #define GEN9_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
+#define PVC_NUM_MOCS_ENTRIES	3
 
 /* (e)LLC caching options */
 /*
@@ -394,6 +396,17 @@ static const struct drm_i915_mocs_entry dg2_mocs_table_g10_ax[] = {
 	MOCS_ENTRY(3, 0, L3_3_WB | L3_LKUP(1)),
 };
 
+static const struct drm_i915_mocs_entry pvc_mocs_table[] = {
+	/* Error */
+	MOCS_ENTRY(0, 0, L3_3_WB),
+
+	/* UC */
+	MOCS_ENTRY(1, 0, L3_1_UC),
+
+	/* WB */
+	MOCS_ENTRY(2, 0, L3_3_WB),
+};
+
 enum {
 	HAS_GLOBAL_MOCS = BIT(0),
 	HAS_ENGINE_MOCS = BIT(1),
@@ -423,7 +436,14 @@ static unsigned int get_mocs_settings(const struct drm_i915_private *i915,
 	memset(table, 0, sizeof(struct drm_i915_mocs_table));
 
 	table->unused_entries_index = I915_MOCS_PTE;
-	if (IS_DG2(i915)) {
+	if (IS_PONTEVECCHIO(i915)) {
+		table->size = ARRAY_SIZE(pvc_mocs_table);
+		table->table = pvc_mocs_table;
+		table->n_entries = PVC_NUM_MOCS_ENTRIES;
+		table->uc_index = 1;
+		table->wb_index = 2;
+		table->unused_entries_index = 2;
+	} else if (IS_DG2(i915)) {
 		if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) {
 			table->size = ARRAY_SIZE(dg2_mocs_table_g10_ax);
 			table->table = dg2_mocs_table_g10_ax;
@@ -622,6 +642,8 @@ void intel_set_mocs_index(struct intel_gt *gt)
 
 	get_mocs_settings(gt->i915, &table);
 	gt->mocs.uc_index = table.uc_index;
+	if (HAS_L3_CCS_READ(gt->i915))
+		gt->mocs.wb_index = table.wb_index;
 }
 
 void intel_mocs_init(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index a05c4b99b3fb..a656d9c2ca2b 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1994,7 +1994,7 @@ void intel_engine_apply_whitelist(struct intel_engine_cs *engine)
 static void
 engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 {
-	u8 mocs;
+	u8 mocs_w, mocs_r;
 
 	/*
 	 * RING_CMD_CCTL are need to be programed to un-cached
@@ -2002,11 +2002,18 @@ engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 	 * Streamers on Gen12 onward platforms.
 	 */
 	if (GRAPHICS_VER(engine->i915) >= 12) {
-		mocs = engine->gt->mocs.uc_index;
+		if (HAS_L3_CCS_READ(engine->i915) &&
+		    engine->class == COMPUTE_CLASS)
+			mocs_r = engine->gt->mocs.wb_index;
+		else
+			mocs_r = engine->gt->mocs.uc_index;
+
+		mocs_w = engine->gt->mocs.uc_index;
+
 		wa_masked_field_set(wal,
 				    RING_CMD_CCTL(engine->mmio_base),
 				    CMD_CCTL_MOCS_MASK,
-				    CMD_CCTL_MOCS_OVERRIDE(mocs, mocs));
+				    CMD_CCTL_MOCS_OVERRIDE(mocs_w, mocs_r));
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2dddc27a1b0e..8c8e7308502b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1369,6 +1369,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 
 #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
 
+#define HAS_L3_CCS_READ(i915) (INTEL_INFO(i915)->has_l3_ccs_read)
+
 /* DPF == dynamic parity feature */
 #define HAS_L3_DPF(dev_priv) (INTEL_INFO(dev_priv)->has_l3_dpf)
 #define NUM_L3_SLICES(dev_priv) (IS_HSW_GT3(dev_priv) ? \
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 498708b33924..07722cdf63ac 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1076,7 +1076,8 @@ static const struct intel_device_info ats_m_info = {
 
 #define XE_HPC_FEATURES \
 	XE_HP_FEATURES, \
-	.dma_mask_size = 52
+	.dma_mask_size = 52, \
+	.has_l3_ccs_read = 1
 
 __maybe_unused
 static const struct intel_device_info pvc_info = {
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index e7d2cf7d65c8..09e33296157a 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -150,6 +150,7 @@ enum intel_ppgtt_type {
 	func(has_heci_pxp); \
 	func(has_heci_gscfi); \
 	func(has_guc_deprivilege); \
+	func(has_l3_ccs_read); \
 	func(has_l3_dpf); \
 	func(has_llc); \
 	func(has_logical_ring_contexts); \
-- 
2.35.1

Re: [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Matt Roper]

On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> 
> Bspec: 45101, 72161
> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> Signed-off-by: Fei Yang <fei.yang@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_types.h    |  1 +
>  drivers/gpu/drm/i915/gt/intel_mocs.c        | 24 ++++++++++++++++++++-
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 ++++++++---
>  drivers/gpu/drm/i915/i915_drv.h             |  2 ++
>  drivers/gpu/drm/i915/i915_pci.c             |  3 ++-
>  drivers/gpu/drm/i915/intel_device_info.h    |  1 +
>  6 files changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index b06611c1d4ad..7853ea194ea6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -221,6 +221,7 @@ struct intel_gt {
>  
>  	struct {
>  		u8 uc_index;
> +		u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
>  	} mocs;
>  
>  	struct intel_pxp pxp;
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index c4c37585ae8c..265812589f87 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -23,6 +23,7 @@ struct drm_i915_mocs_table {
>  	unsigned int n_entries;
>  	const struct drm_i915_mocs_entry *table;
>  	u8 uc_index;
> +	u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
>  	u8 unused_entries_index;
>  };
>  
> @@ -47,6 +48,7 @@ struct drm_i915_mocs_table {
>  
>  /* Helper defines */
>  #define GEN9_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
> +#define PVC_NUM_MOCS_ENTRIES	3

Should this be 4?  The value here should reflect the number of entries
that can defined in hardware rather than the size of the table we're
asked to program.  Since there are two registers (each with a high and a
low entry), that would imply we should set 4 here to ensure that the
fourth entry is initialized according to unused_entries_index rather
than left at whatever the hardware defaults might be.


Matt

>  
>  /* (e)LLC caching options */
>  /*
> @@ -394,6 +396,17 @@ static const struct drm_i915_mocs_entry dg2_mocs_table_g10_ax[] = {
>  	MOCS_ENTRY(3, 0, L3_3_WB | L3_LKUP(1)),
>  };
>  
> +static const struct drm_i915_mocs_entry pvc_mocs_table[] = {
> +	/* Error */
> +	MOCS_ENTRY(0, 0, L3_3_WB),
> +
> +	/* UC */
> +	MOCS_ENTRY(1, 0, L3_1_UC),
> +
> +	/* WB */
> +	MOCS_ENTRY(2, 0, L3_3_WB),
> +};
> +
>  enum {
>  	HAS_GLOBAL_MOCS = BIT(0),
>  	HAS_ENGINE_MOCS = BIT(1),
> @@ -423,7 +436,14 @@ static unsigned int get_mocs_settings(const struct drm_i915_private *i915,
>  	memset(table, 0, sizeof(struct drm_i915_mocs_table));
>  
>  	table->unused_entries_index = I915_MOCS_PTE;
> -	if (IS_DG2(i915)) {
> +	if (IS_PONTEVECCHIO(i915)) {
> +		table->size = ARRAY_SIZE(pvc_mocs_table);
> +		table->table = pvc_mocs_table;
> +		table->n_entries = PVC_NUM_MOCS_ENTRIES;
> +		table->uc_index = 1;
> +		table->wb_index = 2;
> +		table->unused_entries_index = 2;
> +	} else if (IS_DG2(i915)) {
>  		if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) {
>  			table->size = ARRAY_SIZE(dg2_mocs_table_g10_ax);
>  			table->table = dg2_mocs_table_g10_ax;
> @@ -622,6 +642,8 @@ void intel_set_mocs_index(struct intel_gt *gt)
>  
>  	get_mocs_settings(gt->i915, &table);
>  	gt->mocs.uc_index = table.uc_index;
> +	if (HAS_L3_CCS_READ(gt->i915))
> +		gt->mocs.wb_index = table.wb_index;
>  }
>  
>  void intel_mocs_init(struct intel_gt *gt)
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index a05c4b99b3fb..a656d9c2ca2b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1994,7 +1994,7 @@ void intel_engine_apply_whitelist(struct intel_engine_cs *engine)
>  static void
>  engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>  {
> -	u8 mocs;
> +	u8 mocs_w, mocs_r;
>  
>  	/*
>  	 * RING_CMD_CCTL are need to be programed to un-cached
> @@ -2002,11 +2002,18 @@ engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>  	 * Streamers on Gen12 onward platforms.
>  	 */
>  	if (GRAPHICS_VER(engine->i915) >= 12) {
> -		mocs = engine->gt->mocs.uc_index;
> +		if (HAS_L3_CCS_READ(engine->i915) &&
> +		    engine->class == COMPUTE_CLASS)
> +			mocs_r = engine->gt->mocs.wb_index;
> +		else
> +			mocs_r = engine->gt->mocs.uc_index;
> +
> +		mocs_w = engine->gt->mocs.uc_index;
> +
>  		wa_masked_field_set(wal,
>  				    RING_CMD_CCTL(engine->mmio_base),
>  				    CMD_CCTL_MOCS_MASK,
> -				    CMD_CCTL_MOCS_OVERRIDE(mocs, mocs));
> +				    CMD_CCTL_MOCS_OVERRIDE(mocs_w, mocs_r));
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2dddc27a1b0e..8c8e7308502b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1369,6 +1369,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  
>  #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
>  
> +#define HAS_L3_CCS_READ(i915) (INTEL_INFO(i915)->has_l3_ccs_read)
> +
>  /* DPF == dynamic parity feature */
>  #define HAS_L3_DPF(dev_priv) (INTEL_INFO(dev_priv)->has_l3_dpf)
>  #define NUM_L3_SLICES(dev_priv) (IS_HSW_GT3(dev_priv) ? \
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 498708b33924..07722cdf63ac 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -1076,7 +1076,8 @@ static const struct intel_device_info ats_m_info = {
>  
>  #define XE_HPC_FEATURES \
>  	XE_HP_FEATURES, \
> -	.dma_mask_size = 52
> +	.dma_mask_size = 52, \
> +	.has_l3_ccs_read = 1
>  
>  __maybe_unused
>  static const struct intel_device_info pvc_info = {
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index e7d2cf7d65c8..09e33296157a 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -150,6 +150,7 @@ enum intel_ppgtt_type {
>  	func(has_heci_pxp); \
>  	func(has_heci_gscfi); \
>  	func(has_guc_deprivilege); \
> +	func(has_l3_ccs_read); \
>  	func(has_l3_dpf); \
>  	func(has_llc); \
>  	func(has_logical_ring_contexts); \
> -- 
> 2.35.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

Re: [Intel-gfx] [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Lucas De Marchi]

On Mon, May 02, 2022 at 09:50:23AM -0700, Matt Roper wrote:
>On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
>> From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>>
>> Bspec: 45101, 72161
>> Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>> Signed-off-by: Fei Yang <fei.yang@intel.com>
>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_gt_types.h    |  1 +
>>  drivers/gpu/drm/i915/gt/intel_mocs.c        | 24 ++++++++++++++++++++-
>>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 ++++++++---
>>  drivers/gpu/drm/i915/i915_drv.h             |  2 ++
>>  drivers/gpu/drm/i915/i915_pci.c             |  3 ++-
>>  drivers/gpu/drm/i915/intel_device_info.h    |  1 +
>>  6 files changed, 39 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> index b06611c1d4ad..7853ea194ea6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> @@ -221,6 +221,7 @@ struct intel_gt {
>>
>>  	struct {
>>  		u8 uc_index;
>> +		u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
>>  	} mocs;
>>
>>  	struct intel_pxp pxp;
>> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
>> index c4c37585ae8c..265812589f87 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
>> @@ -23,6 +23,7 @@ struct drm_i915_mocs_table {
>>  	unsigned int n_entries;
>>  	const struct drm_i915_mocs_entry *table;
>>  	u8 uc_index;
>> +	u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
>>  	u8 unused_entries_index;
>>  };
>>
>> @@ -47,6 +48,7 @@ struct drm_i915_mocs_table {
>>
>>  /* Helper defines */
>>  #define GEN9_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
>> +#define PVC_NUM_MOCS_ENTRIES	3
>
>Should this be 4?  The value here should reflect the number of entries
>that can defined in hardware rather than the size of the table we're
>asked to program.  Since there are two registers (each with a high and a
>low entry), that would imply we should set 4 here to ensure that the
>fourth entry is initialized according to unused_entries_index rather
>than left at whatever the hardware defaults might be.

not sure I understand what you mean here. The n_entries specifies, as
you said, the number of entries we can have. Bspec 45101 shows entries
for indexes 0, 1 and 2. As does the pvc_mocs_table below.

Also, from bspec 44509:
"For PVC, only 3 MOCS states are supported. The allowed index values are
in range [0, 2]..."

So, I don't think we want to program any fourth entry.

Lucas De Marchi

Re: [Intel-gfx] [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Matt Roper]

On Mon, May 02, 2022 at 11:39:48AM -0700, Lucas De Marchi wrote:
> On Mon, May 02, 2022 at 09:50:23AM -0700, Matt Roper wrote:
> > On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
> > > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > 
> > > Bspec: 45101, 72161
> > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > Signed-off-by: Fei Yang <fei.yang@intel.com>
> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gt/intel_gt_types.h    |  1 +
> > >  drivers/gpu/drm/i915/gt/intel_mocs.c        | 24 ++++++++++++++++++++-
> > >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 ++++++++---
> > >  drivers/gpu/drm/i915/i915_drv.h             |  2 ++
> > >  drivers/gpu/drm/i915/i915_pci.c             |  3 ++-
> > >  drivers/gpu/drm/i915/intel_device_info.h    |  1 +
> > >  6 files changed, 39 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > > index b06611c1d4ad..7853ea194ea6 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > > @@ -221,6 +221,7 @@ struct intel_gt {
> > > 
> > >  	struct {
> > >  		u8 uc_index;
> > > +		u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
> > >  	} mocs;
> > > 
> > >  	struct intel_pxp pxp;
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> > > index c4c37585ae8c..265812589f87 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> > > @@ -23,6 +23,7 @@ struct drm_i915_mocs_table {
> > >  	unsigned int n_entries;
> > >  	const struct drm_i915_mocs_entry *table;
> > >  	u8 uc_index;
> > > +	u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
> > >  	u8 unused_entries_index;
> > >  };
> > > 
> > > @@ -47,6 +48,7 @@ struct drm_i915_mocs_table {
> > > 
> > >  /* Helper defines */
> > >  #define GEN9_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
> > > +#define PVC_NUM_MOCS_ENTRIES	3
> > 
> > Should this be 4?  The value here should reflect the number of entries
> > that can defined in hardware rather than the size of the table we're
> > asked to program.  Since there are two registers (each with a high and a
> > low entry), that would imply we should set 4 here to ensure that the
> > fourth entry is initialized according to unused_entries_index rather
> > than left at whatever the hardware defaults might be.
> 
> not sure I understand what you mean here. The n_entries specifies, as
> you said, the number of entries we can have. Bspec 45101 shows entries
> for indexes 0, 1 and 2. As does the pvc_mocs_table below.
> 
> Also, from bspec 44509:
> "For PVC, only 3 MOCS states are supported. The allowed index values are
> in range [0, 2]..."
> 
> So, I don't think we want to program any fourth entry.

We don't have a choice; the fourth entry lives in the same register as
the third entry, so no matter what we're writing _something_ to those
bits.  The question is whether we should write all 0's or whether we
should treat it like other platforms and ensure it's initialized to the
unused entry values.  Entry #4 isn't supposed to be used, but if buggy
userspace tries to use it, we probably still want well-defined behavior,
just like it an invalid entry gets used on any other platform.


Matt

> 
> Lucas De Marchi

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

Re: [Intel-gfx] [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Lucas De Marchi]

On Mon, May 02, 2022 at 11:50:22AM -0700, Matt Roper wrote:
>On Mon, May 02, 2022 at 11:39:48AM -0700, Lucas De Marchi wrote:
>> On Mon, May 02, 2022 at 09:50:23AM -0700, Matt Roper wrote:
>> > On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
>> > > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>> > >
>> > > Bspec: 45101, 72161
>> > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>> > > Signed-off-by: Fei Yang <fei.yang@intel.com>
>> > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/gt/intel_gt_types.h    |  1 +
>> > >  drivers/gpu/drm/i915/gt/intel_mocs.c        | 24 ++++++++++++++++++++-
>> > >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 ++++++++---
>> > >  drivers/gpu/drm/i915/i915_drv.h             |  2 ++
>> > >  drivers/gpu/drm/i915/i915_pci.c             |  3 ++-
>> > >  drivers/gpu/drm/i915/intel_device_info.h    |  1 +
>> > >  6 files changed, 39 insertions(+), 5 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> > > index b06611c1d4ad..7853ea194ea6 100644
>> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> > > @@ -221,6 +221,7 @@ struct intel_gt {
>> > >
>> > >  	struct {
>> > >  		u8 uc_index;
>> > > +		u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
>> > >  	} mocs;
>> > >
>> > >  	struct intel_pxp pxp;
>> > > diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
>> > > index c4c37585ae8c..265812589f87 100644
>> > > --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
>> > > +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
>> > > @@ -23,6 +23,7 @@ struct drm_i915_mocs_table {
>> > >  	unsigned int n_entries;
>> > >  	const struct drm_i915_mocs_entry *table;
>> > >  	u8 uc_index;
>> > > +	u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
>> > >  	u8 unused_entries_index;
>> > >  };
>> > >
>> > > @@ -47,6 +48,7 @@ struct drm_i915_mocs_table {
>> > >
>> > >  /* Helper defines */
>> > >  #define GEN9_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
>> > > +#define PVC_NUM_MOCS_ENTRIES	3
>> >
>> > Should this be 4?  The value here should reflect the number of entries
>> > that can defined in hardware rather than the size of the table we're
>> > asked to program.  Since there are two registers (each with a high and a
>> > low entry), that would imply we should set 4 here to ensure that the
>> > fourth entry is initialized according to unused_entries_index rather
>> > than left at whatever the hardware defaults might be.
>>
>> not sure I understand what you mean here. The n_entries specifies, as
>> you said, the number of entries we can have. Bspec 45101 shows entries
>> for indexes 0, 1 and 2. As does the pvc_mocs_table below.
>>
>> Also, from bspec 44509:
>> "For PVC, only 3 MOCS states are supported. The allowed index values are
>> in range [0, 2]..."
>>
>> So, I don't think we want to program any fourth entry.
>
>We don't have a choice; the fourth entry lives in the same register as
>the third entry, so no matter what we're writing _something_ to those
>bits.  The question is whether we should write all 0's or whether we
>should treat it like other platforms and ensure it's initialized to the
>unused entry values.  Entry #4 isn't supposed to be used, but if buggy
>userspace tries to use it, we probably still want well-defined behavior,
>just like it an invalid entry gets used on any other platform.

Now I understand what you were talking about:  each register houses 2
entries. For PVC we have LNCFCMOCS0 and LNCFCMOCS1. Humn... looking at
for_each_l3cc(), that is actually handled and the rest of the register
is initialized with the value pointed by unused_entries_index.

Such situation would only happen for the last entry, which implies the
handling for odd size works for this as well.

Lucas De Marchi

>
>
>Matt
>
>>
>> Lucas De Marchi
>
>-- 
>Matt Roper
>Graphics Software Engineer
>VTT-OSGC Platform Enablement
>Intel Corporation
>(916) 356-2795

Re: [Intel-gfx] [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Matt Roper]

On Mon, May 02, 2022 at 12:27:29PM -0700, Lucas De Marchi wrote:
> On Mon, May 02, 2022 at 11:50:22AM -0700, Matt Roper wrote:
> > On Mon, May 02, 2022 at 11:39:48AM -0700, Lucas De Marchi wrote:
> > > On Mon, May 02, 2022 at 09:50:23AM -0700, Matt Roper wrote:
> > > > On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
> > > > > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > > >
> > > > > Bspec: 45101, 72161
> > > > > Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
> > > > > Signed-off-by: Fei Yang <fei.yang@intel.com>
> > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/gt/intel_gt_types.h    |  1 +
> > > > >  drivers/gpu/drm/i915/gt/intel_mocs.c        | 24 ++++++++++++++++++++-
> > > > >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 ++++++++---
> > > > >  drivers/gpu/drm/i915/i915_drv.h             |  2 ++
> > > > >  drivers/gpu/drm/i915/i915_pci.c             |  3 ++-
> > > > >  drivers/gpu/drm/i915/intel_device_info.h    |  1 +
> > > > >  6 files changed, 39 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > > > > index b06611c1d4ad..7853ea194ea6 100644
> > > > > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> > > > > @@ -221,6 +221,7 @@ struct intel_gt {
> > > > >
> > > > >  	struct {
> > > > >  		u8 uc_index;
> > > > > +		u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
> > > > >  	} mocs;
> > > > >
> > > > >  	struct intel_pxp pxp;
> > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> > > > > index c4c37585ae8c..265812589f87 100644
> > > > > --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> > > > > +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> > > > > @@ -23,6 +23,7 @@ struct drm_i915_mocs_table {
> > > > >  	unsigned int n_entries;
> > > > >  	const struct drm_i915_mocs_entry *table;
> > > > >  	u8 uc_index;
> > > > > +	u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
> > > > >  	u8 unused_entries_index;
> > > > >  };
> > > > >
> > > > > @@ -47,6 +48,7 @@ struct drm_i915_mocs_table {
> > > > >
> > > > >  /* Helper defines */
> > > > >  #define GEN9_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
> > > > > +#define PVC_NUM_MOCS_ENTRIES	3
> > > >
> > > > Should this be 4?  The value here should reflect the number of entries
> > > > that can defined in hardware rather than the size of the table we're
> > > > asked to program.  Since there are two registers (each with a high and a
> > > > low entry), that would imply we should set 4 here to ensure that the
> > > > fourth entry is initialized according to unused_entries_index rather
> > > > than left at whatever the hardware defaults might be.
> > > 
> > > not sure I understand what you mean here. The n_entries specifies, as
> > > you said, the number of entries we can have. Bspec 45101 shows entries
> > > for indexes 0, 1 and 2. As does the pvc_mocs_table below.
> > > 
> > > Also, from bspec 44509:
> > > "For PVC, only 3 MOCS states are supported. The allowed index values are
> > > in range [0, 2]..."
> > > 
> > > So, I don't think we want to program any fourth entry.
> > 
> > We don't have a choice; the fourth entry lives in the same register as
> > the third entry, so no matter what we're writing _something_ to those
> > bits.  The question is whether we should write all 0's or whether we
> > should treat it like other platforms and ensure it's initialized to the
> > unused entry values.  Entry #4 isn't supposed to be used, but if buggy
> > userspace tries to use it, we probably still want well-defined behavior,
> > just like it an invalid entry gets used on any other platform.
> 
> Now I understand what you were talking about:  each register houses 2
> entries. For PVC we have LNCFCMOCS0 and LNCFCMOCS1. Humn... looking at
> for_each_l3cc(), that is actually handled and the rest of the register
> is initialized with the value pointed by unused_entries_index.

Yep, you're right.  It looks like we still do a get_entry_l3cc() for the
upper entry of the final register, and that will return the unused_entry
value if it's out of bounds.  In that case I don't have any concerns
here.


Matt

> 
> Such situation would only happen for the last entry, which implies the
> handling for odd size works for this as well.
> 
> Lucas De Marchi
> 
> > 
> > 
> > Matt
> > 
> > > 
> > > Lucas De Marchi
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > VTT-OSGC Platform Enablement
> > Intel Corporation
> > (916) 356-2795

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

Re: [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Lucas De Marchi]

On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
>From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>
>Bspec: 45101, 72161
>Signed-off-by: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>Signed-off-by: Fei Yang <fei.yang@intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
> drivers/gpu/drm/i915/gt/intel_gt_types.h    |  1 +
> drivers/gpu/drm/i915/gt/intel_mocs.c        | 24 ++++++++++++++++++++-
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 ++++++++---
> drivers/gpu/drm/i915/i915_drv.h             |  2 ++
> drivers/gpu/drm/i915/i915_pci.c             |  3 ++-
> drivers/gpu/drm/i915/intel_device_info.h    |  1 +
> 6 files changed, 39 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>index b06611c1d4ad..7853ea194ea6 100644
>--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>@@ -221,6 +221,7 @@ struct intel_gt {
>
> 	struct {
> 		u8 uc_index;
>+		u8 wb_index; /* Only for platforms listed in Bspec: 72161 */

I don't like much writting the bspec in code like this. For commit
message it's acceptable/desired, but for code I think it's not great as
1) it's not something generally available and 2) it will likely get
outdated so one would have to rely on git log/blame to see when this was
actually valid.


> 	} mocs;
>
> 	struct intel_pxp pxp;
>diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
>index c4c37585ae8c..265812589f87 100644
>--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
>+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
>@@ -23,6 +23,7 @@ struct drm_i915_mocs_table {
> 	unsigned int n_entries;
> 	const struct drm_i915_mocs_entry *table;
> 	u8 uc_index;
>+	u8 wb_index; /* Only for platforms listed in Bspec: 72161 */
> 	u8 unused_entries_index;
> };
>
>@@ -47,6 +48,7 @@ struct drm_i915_mocs_table {
>
> /* Helper defines */
> #define GEN9_NUM_MOCS_ENTRIES	64  /* 63-64 are reserved, but configured. */
>+#define PVC_NUM_MOCS_ENTRIES	3
>
> /* (e)LLC caching options */
> /*
>@@ -394,6 +396,17 @@ static const struct drm_i915_mocs_entry dg2_mocs_table_g10_ax[] = {
> 	MOCS_ENTRY(3, 0, L3_3_WB | L3_LKUP(1)),
> };
>
>+static const struct drm_i915_mocs_entry pvc_mocs_table[] = {
>+	/* Error */
>+	MOCS_ENTRY(0, 0, L3_3_WB),
>+
>+	/* UC */
>+	MOCS_ENTRY(1, 0, L3_1_UC),
>+
>+	/* WB */
>+	MOCS_ENTRY(2, 0, L3_3_WB),
>+};
>+
> enum {
> 	HAS_GLOBAL_MOCS = BIT(0),
> 	HAS_ENGINE_MOCS = BIT(1),
>@@ -423,7 +436,14 @@ static unsigned int get_mocs_settings(const struct drm_i915_private *i915,
> 	memset(table, 0, sizeof(struct drm_i915_mocs_table));
>
> 	table->unused_entries_index = I915_MOCS_PTE;
>-	if (IS_DG2(i915)) {
>+	if (IS_PONTEVECCHIO(i915)) {
>+		table->size = ARRAY_SIZE(pvc_mocs_table);
>+		table->table = pvc_mocs_table;
>+		table->n_entries = PVC_NUM_MOCS_ENTRIES;
>+		table->uc_index = 1;
>+		table->wb_index = 2;
>+		table->unused_entries_index = 2;
>+	} else if (IS_DG2(i915)) {
> 		if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) {
> 			table->size = ARRAY_SIZE(dg2_mocs_table_g10_ax);
> 			table->table = dg2_mocs_table_g10_ax;
>@@ -622,6 +642,8 @@ void intel_set_mocs_index(struct intel_gt *gt)
>
> 	get_mocs_settings(gt->i915, &table);
> 	gt->mocs.uc_index = table.uc_index;
>+	if (HAS_L3_CCS_READ(gt->i915))
>+		gt->mocs.wb_index = table.wb_index;
> }
>
> void intel_mocs_init(struct intel_gt *gt)
>diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>index a05c4b99b3fb..a656d9c2ca2b 100644
>--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>@@ -1994,7 +1994,7 @@ void intel_engine_apply_whitelist(struct intel_engine_cs *engine)
> static void
> engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> {
>-	u8 mocs;
>+	u8 mocs_w, mocs_r;
>
> 	/*
> 	 * RING_CMD_CCTL are need to be programed to un-cached
>@@ -2002,11 +2002,18 @@ engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> 	 * Streamers on Gen12 onward platforms.
> 	 */
> 	if (GRAPHICS_VER(engine->i915) >= 12) {
>-		mocs = engine->gt->mocs.uc_index;
>+		if (HAS_L3_CCS_READ(engine->i915) &&
>+		    engine->class == COMPUTE_CLASS)
>+			mocs_r = engine->gt->mocs.wb_index;
>+		else
>+			mocs_r = engine->gt->mocs.uc_index;

shouldn't we add a warning in get_mocs_settings() if HAS_L3_CCS_READ(engine->i915)
and mocs.wb_index is 0 (since index 0 shouldn't really be used in latest
platforms)?

Lucas De Marchi

>+
>+		mocs_w = engine->gt->mocs.uc_index;
>+
> 		wa_masked_field_set(wal,
> 				    RING_CMD_CCTL(engine->mmio_base),
> 				    CMD_CCTL_MOCS_MASK,
>-				    CMD_CCTL_MOCS_OVERRIDE(mocs, mocs));
>+				    CMD_CCTL_MOCS_OVERRIDE(mocs_w, mocs_r));
> 	}
> }
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>index 2dddc27a1b0e..8c8e7308502b 100644
>--- a/drivers/gpu/drm/i915/i915_drv.h
>+++ b/drivers/gpu/drm/i915/i915_drv.h
>@@ -1369,6 +1369,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>
> #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
>
>+#define HAS_L3_CCS_READ(i915) (INTEL_INFO(i915)->has_l3_ccs_read)
>+
> /* DPF == dynamic parity feature */
> #define HAS_L3_DPF(dev_priv) (INTEL_INFO(dev_priv)->has_l3_dpf)
> #define NUM_L3_SLICES(dev_priv) (IS_HSW_GT3(dev_priv) ? \
>diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>index 498708b33924..07722cdf63ac 100644
>--- a/drivers/gpu/drm/i915/i915_pci.c
>+++ b/drivers/gpu/drm/i915/i915_pci.c
>@@ -1076,7 +1076,8 @@ static const struct intel_device_info ats_m_info = {
>
> #define XE_HPC_FEATURES \
> 	XE_HP_FEATURES, \
>-	.dma_mask_size = 52
>+	.dma_mask_size = 52, \
>+	.has_l3_ccs_read = 1
>
> __maybe_unused
> static const struct intel_device_info pvc_info = {
>diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
>index e7d2cf7d65c8..09e33296157a 100644
>--- a/drivers/gpu/drm/i915/intel_device_info.h
>+++ b/drivers/gpu/drm/i915/intel_device_info.h
>@@ -150,6 +150,7 @@ enum intel_ppgtt_type {
> 	func(has_heci_pxp); \
> 	func(has_heci_gscfi); \
> 	func(has_guc_deprivilege); \
>+	func(has_l3_ccs_read); \
> 	func(has_l3_dpf); \
> 	func(has_llc); \
> 	func(has_logical_ring_contexts); \
>-- 
>2.35.1
>

Re: [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Matt Roper]

On Mon, May 02, 2022 at 02:03:28PM -0700, Lucas De Marchi wrote:
> On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
> > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
...
> > @@ -2002,11 +2002,18 @@ engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> > 	 * Streamers on Gen12 onward platforms.
> > 	 */
> > 	if (GRAPHICS_VER(engine->i915) >= 12) {
> > -		mocs = engine->gt->mocs.uc_index;
> > +		if (HAS_L3_CCS_READ(engine->i915) &&
> > +		    engine->class == COMPUTE_CLASS)
> > +			mocs_r = engine->gt->mocs.wb_index;
> > +		else
> > +			mocs_r = engine->gt->mocs.uc_index;
> 
> shouldn't we add a warning in get_mocs_settings() if HAS_L3_CCS_READ(engine->i915)
> and mocs.wb_index is 0 (since index 0 shouldn't really be used in latest
> platforms)?

We should be careful about that assumption...index 0 is valid on DG2
today, although HAS_L3_CCS_READ() doesn't apply there.  And a couple
platforms in the future we're also going to have index 0 being valid on
a platform where HAS_L3_CCS_READ() is true (bspec 71582).  Index 0 would
still be the wrong entry to pick for WB behavior there, but it is a
legitimate entry in general.


Matt

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

Re: [PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC

[Lucas De Marchi]

On Mon, May 02, 2022 at 02:14:02PM -0700, Matt Roper wrote:
>On Mon, May 02, 2022 at 02:03:28PM -0700, Lucas De Marchi wrote:
>> On Mon, May 02, 2022 at 09:34:09AM -0700, Matt Roper wrote:
>> > From: Ayaz A Siddiqui <ayaz.siddiqui@intel.com>
>...
>> > @@ -2002,11 +2002,18 @@ engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
>> > 	 * Streamers on Gen12 onward platforms.
>> > 	 */
>> > 	if (GRAPHICS_VER(engine->i915) >= 12) {
>> > -		mocs = engine->gt->mocs.uc_index;
>> > +		if (HAS_L3_CCS_READ(engine->i915) &&
>> > +		    engine->class == COMPUTE_CLASS)
>> > +			mocs_r = engine->gt->mocs.wb_index;
>> > +		else
>> > +			mocs_r = engine->gt->mocs.uc_index;
>>
>> shouldn't we add a warning in get_mocs_settings() if HAS_L3_CCS_READ(engine->i915)
>> and mocs.wb_index is 0 (since index 0 shouldn't really be used in latest
>> platforms)?
>
>We should be careful about that assumption...index 0 is valid on DG2
>today, although HAS_L3_CCS_READ() doesn't apply there.  And a couple
>platforms in the future we're also going to have index 0 being valid on
>a platform where HAS_L3_CCS_READ() is true (bspec 71582).  Index 0 would
>still be the wrong entry to pick for WB behavior there, but it is a
>legitimate entry in general.

ok, but comment is more about "forgetting to initialize it in
get_mocs_settings() and then using it here". Using 0 as "it was not
initialized" may be an easy way to do that.

Lucas De Marchi

[PATCH 04/11] drm/i915/pvc: Read correct RP_STATE_CAP register

[Matt Roper]

The SoC registers, including RP_STATE_CAP, have moved to a new location
in GTTMMADR on Ponte Vecchio.  We need to update the register offset
accordingly.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_rps.c | 4 +++-
 drivers/gpu/drm/i915/i915_reg.h     | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 3476a11f294c..3bd8415a0f1b 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1075,7 +1075,9 @@ static u32 intel_rps_read_state_cap(struct intel_rps *rps)
 	struct drm_i915_private *i915 = rps_to_i915(rps);
 	struct intel_uncore *uncore = rps_to_uncore(rps);
 
-	if (IS_XEHPSDV(i915))
+	if (IS_PONTEVECCHIO(i915))
+		return intel_uncore_read(uncore, PVC_RP_STATE_CAP);
+	else if (IS_XEHPSDV(i915))
 		return intel_uncore_read(uncore, XEHPSDV_RP_STATE_CAP);
 	else if (IS_GEN9_LP(i915))
 		return intel_uncore_read(uncore, BXT_RP_STATE_CAP);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9ccb67eec1bd..4a3d7b96ef43 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1846,6 +1846,7 @@
 #define BXT_RP_STATE_CAP        _MMIO(0x138170)
 #define GEN9_RP_STATE_LIMITS	_MMIO(0x138148)
 #define XEHPSDV_RP_STATE_CAP	_MMIO(0x250014)
+#define PVC_RP_STATE_CAP	_MMIO(0x281014)
 
 #define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381a8)
 #define   GT0_PERF_LIMIT_REASONS_MASK	0xde3
-- 
2.35.1

Re: [PATCH 04/11] drm/i915/pvc: Read correct RP_STATE_CAP register

[Rodrigo Vivi]

On Mon, May 02, 2022 at 09:34:10AM -0700, Matt Roper wrote:
> The SoC registers, including RP_STATE_CAP, have moved to a new location
> in GTTMMADR on Ponte Vecchio.  We need to update the register offset
> accordingly.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

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

> ---
>  drivers/gpu/drm/i915/gt/intel_rps.c | 4 +++-
>  drivers/gpu/drm/i915/i915_reg.h     | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 3476a11f294c..3bd8415a0f1b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1075,7 +1075,9 @@ static u32 intel_rps_read_state_cap(struct intel_rps *rps)
>  	struct drm_i915_private *i915 = rps_to_i915(rps);
>  	struct intel_uncore *uncore = rps_to_uncore(rps);
>  
> -	if (IS_XEHPSDV(i915))
> +	if (IS_PONTEVECCHIO(i915))
> +		return intel_uncore_read(uncore, PVC_RP_STATE_CAP);
> +	else if (IS_XEHPSDV(i915))
>  		return intel_uncore_read(uncore, XEHPSDV_RP_STATE_CAP);
>  	else if (IS_GEN9_LP(i915))
>  		return intel_uncore_read(uncore, BXT_RP_STATE_CAP);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9ccb67eec1bd..4a3d7b96ef43 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1846,6 +1846,7 @@
>  #define BXT_RP_STATE_CAP        _MMIO(0x138170)
>  #define GEN9_RP_STATE_LIMITS	_MMIO(0x138148)
>  #define XEHPSDV_RP_STATE_CAP	_MMIO(0x250014)
> +#define PVC_RP_STATE_CAP	_MMIO(0x281014)
>  
>  #define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381a8)
>  #define   GT0_PERF_LIMIT_REASONS_MASK	0xde3
> -- 
> 2.35.1
> 

[PATCH 05/11] drm/i915/pvc: Remove additional 3D flags from PIPE_CONTROL

[Matt Roper]

From: Stuart Summers <stuart.summers@intel.com>

Although we already strip 3D-specific flags from PIPE_CONTROL
instructions when submitting to a compute engine, there are some
additional flags that need to be removed when the platform as a whole
lacks a 3D pipeline.  Add those restrictions here.

Bspec: 47112
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/gen8_engine_cs.c     | 18 ++++++++++++------
 drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 12 ++++++++++--
 drivers/gpu/drm/i915/i915_drv.h              |  2 ++
 drivers/gpu/drm/i915/i915_pci.c              |  3 ++-
 drivers/gpu/drm/i915/intel_device_info.h     |  3 ++-
 5 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 9529c5455bc3..0de17b568b41 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -196,8 +196,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
 
 		flags |= PIPE_CONTROL_CS_STALL;
 
-		if (engine->class == COMPUTE_CLASS)
-			flags &= ~PIPE_CONTROL_3D_FLAGS;
+		if (LACKS_3D_PIPELINE(engine->i915))
+			flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS;
+		else if (engine->class == COMPUTE_CLASS)
+			flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
 
 		cs = intel_ring_begin(rq, 6);
 		if (IS_ERR(cs))
@@ -226,8 +228,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
 
 		flags |= PIPE_CONTROL_CS_STALL;
 
-		if (engine->class == COMPUTE_CLASS)
-			flags &= ~PIPE_CONTROL_3D_FLAGS;
+		if (LACKS_3D_PIPELINE(engine->i915))
+			flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS;
+		else if (engine->class == COMPUTE_CLASS)
+			flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
 
 		if (!HAS_FLAT_CCS(rq->engine->i915))
 			count = 8 + 4;
@@ -662,8 +666,10 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
 		/* Wa_1409600907 */
 		flags |= PIPE_CONTROL_DEPTH_STALL;
 
-	if (rq->engine->class == COMPUTE_CLASS)
-		flags &= ~PIPE_CONTROL_3D_FLAGS;
+	if (LACKS_3D_PIPELINE(rq->engine->i915))
+		flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS;
+	else if (rq->engine->class == COMPUTE_CLASS)
+		flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
 
 	cs = gen12_emit_ggtt_write_rcs(cs,
 				       rq->fence.seqno,
diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
index e52718a87f14..5eaf54e72635 100644
--- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
+++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
@@ -286,8 +286,8 @@
 #define   PIPE_CONTROL_DEPTH_CACHE_FLUSH		(1<<0)
 #define   PIPE_CONTROL_GLOBAL_GTT (1<<2) /* in addr dword */
 
-/* 3D-related flags can't be set on compute engine */
-#define PIPE_CONTROL_3D_FLAGS (\
+/* 3D-related flags that can't be set on _engines_ that lack a 3D pipeline */
+#define PIPE_CONTROL_3D_ENGINE_FLAGS (\
 		PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH | \
 		PIPE_CONTROL_DEPTH_CACHE_FLUSH | \
 		PIPE_CONTROL_TILE_CACHE_FLUSH | \
@@ -298,6 +298,14 @@
 		PIPE_CONTROL_VF_CACHE_INVALIDATE | \
 		PIPE_CONTROL_GLOBAL_SNAPSHOT_RESET)
 
+/* 3D-related flags that can't be set on _platforms_ that lack a 3D pipeline */
+#define PIPE_CONTROL_3D_ARCH_FLAGS ( \
+		PIPE_CONTROL_3D_ENGINE_FLAGS | \
+		PIPE_CONTROL_INDIRECT_STATE_DISABLE | \
+		PIPE_CONTROL_FLUSH_ENABLE | \
+		PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE | \
+		PIPE_CONTROL_DC_FLUSH_ENABLE)
+
 #define MI_MATH(x)			MI_INSTR(0x1a, (x) - 1)
 #define MI_MATH_INSTR(opcode, op1, op2) ((opcode) << 20 | (op1) << 10 | (op2))
 /* Opcodes for MI_MATH_INSTR */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8c8e7308502b..55c6b9c4e476 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1402,6 +1402,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 
 #define HAS_MBUS_JOINING(i915) (IS_ALDERLAKE_P(i915))
 
+#define LACKS_3D_PIPELINE(i915)	(INTEL_INFO(i915)->lacks_3d_pipeline)
+
 /* i915_gem.c */
 void i915_gem_init_early(struct drm_i915_private *dev_priv);
 void i915_gem_cleanup_early(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 07722cdf63ac..14e0e8225324 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1077,7 +1077,8 @@ static const struct intel_device_info ats_m_info = {
 #define XE_HPC_FEATURES \
 	XE_HP_FEATURES, \
 	.dma_mask_size = 52, \
-	.has_l3_ccs_read = 1
+	.has_l3_ccs_read = 1, \
+	.lacks_3d_pipeline = 1
 
 __maybe_unused
 static const struct intel_device_info pvc_info = {
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 09e33296157a..972084676984 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -165,7 +165,8 @@ enum intel_ppgtt_type {
 	func(has_snoop); \
 	func(has_coherent_ggtt); \
 	func(unfenced_needs_alignment); \
-	func(hws_needs_physical);
+	func(hws_needs_physical); \
+	func(lacks_3d_pipeline);
 
 #define DEV_INFO_DISPLAY_FOR_EACH_FLAG(func) \
 	/* Keep in alphabetical order */ \
-- 
2.35.1

[PATCH 06/11] drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter engine

[Matt Roper]

From: John Harrison <John.C.Harrison@Intel.com>

PVC adds extra blitter engines (in the following patch). The reset
selftest has a local array on the stack which is sized by the number
of engines. The increase pushes the size of this array to the point
where it trips the 'stack too large' compile warning. This patch takes
the allocation of the stack and makes it dynamic instead.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 83ff4c2e57c5..3b9d82276db2 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -979,6 +979,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
 	enum intel_engine_id id, tmp;
 	struct hang h;
 	int err = 0;
+	struct active_engine *threads;
 
 	/* Check that issuing a reset on one engine does not interfere
 	 * with any other engine.
@@ -996,8 +997,11 @@ static int __igt_reset_engines(struct intel_gt *gt,
 			h.ctx->sched.priority = 1024;
 	}
 
+	threads = kzalloc(sizeof(*threads) * I915_NUM_ENGINES, GFP_KERNEL);
+	if (!threads)
+		return -ENOMEM;
+
 	for_each_engine(engine, gt, id) {
-		struct active_engine threads[I915_NUM_ENGINES] = {};
 		unsigned long device = i915_reset_count(global);
 		unsigned long count = 0, reported;
 		bool using_guc = intel_engine_uses_guc(engine);
@@ -1016,7 +1020,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
 			break;
 		}
 
-		memset(threads, 0, sizeof(threads));
+		memset(threads, 0, sizeof(*threads) * I915_NUM_ENGINES);
 		for_each_engine(other, gt, tmp) {
 			struct task_struct *tsk;
 
@@ -1236,6 +1240,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
 			break;
 		}
 	}
+	kfree(threads);
 
 	if (intel_gt_is_wedged(gt))
 		err = -EIO;
-- 
2.35.1

Re: [PATCH 06/11] drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter engine

[Souza, Jose]

On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> PVC adds extra blitter engines (in the following patch). The reset
> selftest has a local array on the stack which is sized by the number
> of engines. The increase pushes the size of this array to the point
> where it trips the 'stack too large' compile warning. This patch takes
> the allocation of the stack and makes it dynamic instead.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index 83ff4c2e57c5..3b9d82276db2 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -979,6 +979,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
>  	enum intel_engine_id id, tmp;
>  	struct hang h;
>  	int err = 0;
> +	struct active_engine *threads;
>  
>  	/* Check that issuing a reset on one engine does not interfere
>  	 * with any other engine.
> @@ -996,8 +997,11 @@ static int __igt_reset_engines(struct intel_gt *gt,
>  			h.ctx->sched.priority = 1024;
>  	}
>  
> +	threads = kzalloc(sizeof(*threads) * I915_NUM_ENGINES, GFP_KERNEL);
> +	if (!threads)
> +		return -ENOMEM;
> +
>  	for_each_engine(engine, gt, id) {
> -		struct active_engine threads[I915_NUM_ENGINES] = {};
>  		unsigned long device = i915_reset_count(global);
>  		unsigned long count = 0, reported;
>  		bool using_guc = intel_engine_uses_guc(engine);
> @@ -1016,7 +1020,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
>  			break;
>  		}
>  
> -		memset(threads, 0, sizeof(threads));
> +		memset(threads, 0, sizeof(*threads) * I915_NUM_ENGINES);
>  		for_each_engine(other, gt, tmp) {
>  			struct task_struct *tsk;
>  
> @@ -1236,6 +1240,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
>  			break;
>  		}
>  	}
> +	kfree(threads);
>  
>  	if (intel_gt_is_wedged(gt))
>  		err = -EIO;

Re: [Intel-gfx] [PATCH 06/11] drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter engine

[Tvrtko Ursulin]

On 02/05/2022 17:34, Matt Roper wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> PVC adds extra blitter engines (in the following patch). The reset
> selftest has a local array on the stack which is sized by the number
> of engines. The increase pushes the size of this array to the point
> where it trips the 'stack too large' compile warning. This patch takes
> the allocation of the stack and makes it dynamic instead.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index 83ff4c2e57c5..3b9d82276db2 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -979,6 +979,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
>   	enum intel_engine_id id, tmp;
>   	struct hang h;
>   	int err = 0;
> +	struct active_engine *threads;

Drive by nits - sticks out like a sore thumb a bit here - I'd put it 
above "id, tmp" so it's all nicely sorted by width.
>   
>   	/* Check that issuing a reset on one engine does not interfere
>   	 * with any other engine.
> @@ -996,8 +997,11 @@ static int __igt_reset_engines(struct intel_gt *gt,
>   			h.ctx->sched.priority = 1024;
>   	}
>   
> +	threads = kzalloc(sizeof(*threads) * I915_NUM_ENGINES, GFP_KERNEL);

And this could be kcalloc (or kmalloc_array since zeroing is not needed) 
if that's any better. Seems selftests use that pattern anyway.

Both comments are optional really.

Regards,

Tvrtko

> +	if (!threads)
> +		return -ENOMEM;
> +
>   	for_each_engine(engine, gt, id) {
> -		struct active_engine threads[I915_NUM_ENGINES] = {};
>   		unsigned long device = i915_reset_count(global);
>   		unsigned long count = 0, reported;
>   		bool using_guc = intel_engine_uses_guc(engine);
> @@ -1016,7 +1020,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
>   			break;
>   		}
>   
> -		memset(threads, 0, sizeof(threads));
> +		memset(threads, 0, sizeof(*threads) * I915_NUM_ENGINES);
>   		for_each_engine(other, gt, tmp) {
>   			struct task_struct *tsk;
>   
> @@ -1236,6 +1240,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
>   			break;
>   		}
>   	}
> +	kfree(threads);
>   
>   	if (intel_gt_is_wedged(gt))
>   		err = -EIO;

[PATCH 07/11] drm/i915/pvc: Engines definitions for new copy engines

[Matt Roper]

This patch adds the basic definitions needed to support
new copy engines. Also updating the cmd_info to accommodate
new engines, as the engine id's of legacy engines have been
changed.

Original-author: CQ Tang
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 56 ++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_engine_types.h | 10 +++-
 drivers/gpu/drm/i915/gt/intel_gt_regs.h      |  8 +++
 drivers/gpu/drm/i915/gvt/cmd_parser.c        |  2 +-
 drivers/gpu/drm/i915/i915_reg.h              |  8 +++
 5 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 14c6ddbbfde8..4532c3ea9ace 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -71,6 +71,62 @@ static const struct engine_info intel_engines[] = {
 			{ .graphics_ver = 6, .base = BLT_RING_BASE }
 		},
 	},
+	[BCS1] = {
+		.class = COPY_ENGINE_CLASS,
+		.instance = 1,
+		.mmio_bases = {
+			{ .graphics_ver = 12, .base = XEHPC_BCS1_RING_BASE }
+		},
+	},
+	[BCS2] = {
+		.class = COPY_ENGINE_CLASS,
+		.instance = 2,
+		.mmio_bases = {
+			{ .graphics_ver = 12, .base = XEHPC_BCS2_RING_BASE }
+		},
+	},
+	[BCS3] = {
+		.class = COPY_ENGINE_CLASS,
+		.instance = 3,
+		.mmio_bases = {
+			{ .graphics_ver = 12, .base = XEHPC_BCS3_RING_BASE }
+		},
+	},
+	[BCS4] = {
+		.class = COPY_ENGINE_CLASS,
+		.instance = 4,
+		.mmio_bases = {
+			{ .graphics_ver = 12, .base = XEHPC_BCS4_RING_BASE }
+		},
+	},
+	[BCS5] = {
+		.class = COPY_ENGINE_CLASS,
+		.instance = 5,
+		.mmio_bases = {
+			{ .graphics_ver = 12, .base = XEHPC_BCS5_RING_BASE }
+		},
+	},
+	[BCS6] = {
+		.class = COPY_ENGINE_CLASS,
+		.instance = 6,
+		.mmio_bases = {
+			{ .graphics_ver = 12, .base = XEHPC_BCS6_RING_BASE }
+		},
+	},
+	[BCS7] = {
+		.class = COPY_ENGINE_CLASS,
+		.instance = 7,
+		.mmio_bases = {
+			{ .graphics_ver = 12, .base = XEHPC_BCS7_RING_BASE }
+		},
+	},
+	[BCS8] = {
+		.class = COPY_ENGINE_CLASS,
+		.instance = 8,
+		.mmio_bases = {
+			{ .graphics_ver = 12, .base = XEHPC_BCS8_RING_BASE }
+		},
+	},
 	[VCS0] = {
 		.class = VIDEO_DECODE_CLASS,
 		.instance = 0,
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
index 298f2cc7a879..356c15cdccf0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
@@ -35,7 +35,7 @@
 #define OTHER_CLASS		4
 #define COMPUTE_CLASS		5
 #define MAX_ENGINE_CLASS	5
-#define MAX_ENGINE_INSTANCE	7
+#define MAX_ENGINE_INSTANCE	8
 
 #define I915_MAX_SLICES	3
 #define I915_MAX_SUBSLICES 8
@@ -107,6 +107,14 @@ struct i915_ctx_workarounds {
 enum intel_engine_id {
 	RCS0 = 0,
 	BCS0,
+	BCS1,
+	BCS2,
+	BCS3,
+	BCS4,
+	BCS5,
+	BCS6,
+	BCS7,
+	BCS8,
 	VCS0,
 	VCS1,
 	VCS2,
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index a0a49c16babd..aa2c0974b02c 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1476,6 +1476,14 @@
 #define   GEN11_KCR				(19)
 #define   GEN11_GTPM				(16)
 #define   GEN11_BCS				(15)
+#define   XEHPC_BCS1				(14)
+#define   XEHPC_BCS2				(13)
+#define   XEHPC_BCS3				(12)
+#define   XEHPC_BCS4				(11)
+#define   XEHPC_BCS5				(10)
+#define   XEHPC_BCS6				(9)
+#define   XEHPC_BCS7				(8)
+#define   XEHPC_BCS8				(23)
 #define   GEN12_CCS3				(7)
 #define   GEN12_CCS2				(6)
 #define   GEN12_CCS1				(5)
diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
index b9eb75a2b400..0ba2a3455d99 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -428,7 +428,7 @@ struct cmd_info {
 #define R_VECS	BIT(VECS0)
 #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)
 	/* rings that support this cmd: BLT/RCS/VCS/VECS */
-	u16 rings;
+	intel_engine_mask_t rings;
 
 	/* devices that support this cmd: SNB/IVB/HSW/... */
 	u16 devices;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4a3d7b96ef43..ab64ab4317b3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -976,6 +976,14 @@
 #define GEN12_COMPUTE2_RING_BASE	0x1e000
 #define GEN12_COMPUTE3_RING_BASE	0x26000
 #define BLT_RING_BASE		0x22000
+#define XEHPC_BCS1_RING_BASE	0x3e0000
+#define XEHPC_BCS2_RING_BASE	0x3e2000
+#define XEHPC_BCS3_RING_BASE	0x3e4000
+#define XEHPC_BCS4_RING_BASE	0x3e6000
+#define XEHPC_BCS5_RING_BASE	0x3e8000
+#define XEHPC_BCS6_RING_BASE	0x3ea000
+#define XEHPC_BCS7_RING_BASE	0x3ec000
+#define XEHPC_BCS8_RING_BASE	0x3ee000
 #define DG1_GSC_HECI1_BASE	0x00258000
 #define DG1_GSC_HECI2_BASE	0x00259000
 #define DG2_GSC_HECI1_BASE	0x00373000
-- 
2.35.1

Re: [Intel-gfx] [PATCH 07/11] drm/i915/pvc: Engines definitions for new copy engines

[Souza, Jose]

On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> This patch adds the basic definitions needed to support
> new copy engines. Also updating the cmd_info to accommodate
> new engines, as the engine id's of legacy engines have been
> changed.


Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Original-author: CQ Tang
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 56 ++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_engine_types.h | 10 +++-
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h      |  8 +++
>  drivers/gpu/drm/i915/gvt/cmd_parser.c        |  2 +-
>  drivers/gpu/drm/i915/i915_reg.h              |  8 +++
>  5 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 14c6ddbbfde8..4532c3ea9ace 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -71,6 +71,62 @@ static const struct engine_info intel_engines[] = {
>  			{ .graphics_ver = 6, .base = BLT_RING_BASE }
>  		},
>  	},
> +	[BCS1] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 1,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS1_RING_BASE }
> +		},
> +	},
> +	[BCS2] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 2,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS2_RING_BASE }
> +		},
> +	},
> +	[BCS3] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 3,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS3_RING_BASE }
> +		},
> +	},
> +	[BCS4] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 4,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS4_RING_BASE }
> +		},
> +	},
> +	[BCS5] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 5,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS5_RING_BASE }
> +		},
> +	},
> +	[BCS6] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 6,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS6_RING_BASE }
> +		},
> +	},
> +	[BCS7] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 7,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS7_RING_BASE }
> +		},
> +	},
> +	[BCS8] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 8,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS8_RING_BASE }
> +		},
> +	},
>  	[VCS0] = {
>  		.class = VIDEO_DECODE_CLASS,
>  		.instance = 0,
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 298f2cc7a879..356c15cdccf0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -35,7 +35,7 @@
>  #define OTHER_CLASS		4
>  #define COMPUTE_CLASS		5
>  #define MAX_ENGINE_CLASS	5
> -#define MAX_ENGINE_INSTANCE	7
> +#define MAX_ENGINE_INSTANCE	8
>  
>  #define I915_MAX_SLICES	3
>  #define I915_MAX_SUBSLICES 8
> @@ -107,6 +107,14 @@ struct i915_ctx_workarounds {
>  enum intel_engine_id {
>  	RCS0 = 0,
>  	BCS0,
> +	BCS1,
> +	BCS2,
> +	BCS3,
> +	BCS4,
> +	BCS5,
> +	BCS6,
> +	BCS7,
> +	BCS8,
>  	VCS0,
>  	VCS1,
>  	VCS2,
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index a0a49c16babd..aa2c0974b02c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1476,6 +1476,14 @@
>  #define   GEN11_KCR				(19)
>  #define   GEN11_GTPM				(16)
>  #define   GEN11_BCS				(15)
> +#define   XEHPC_BCS1				(14)
> +#define   XEHPC_BCS2				(13)
> +#define   XEHPC_BCS3				(12)
> +#define   XEHPC_BCS4				(11)
> +#define   XEHPC_BCS5				(10)
> +#define   XEHPC_BCS6				(9)
> +#define   XEHPC_BCS7				(8)
> +#define   XEHPC_BCS8				(23)
>  #define   GEN12_CCS3				(7)
>  #define   GEN12_CCS2				(6)
>  #define   GEN12_CCS1				(5)
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index b9eb75a2b400..0ba2a3455d99 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -428,7 +428,7 @@ struct cmd_info {
>  #define R_VECS	BIT(VECS0)
>  #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)
>  	/* rings that support this cmd: BLT/RCS/VCS/VECS */
> -	u16 rings;
> +	intel_engine_mask_t rings;
>  
>  	/* devices that support this cmd: SNB/IVB/HSW/... */
>  	u16 devices;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4a3d7b96ef43..ab64ab4317b3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -976,6 +976,14 @@
>  #define GEN12_COMPUTE2_RING_BASE	0x1e000
>  #define GEN12_COMPUTE3_RING_BASE	0x26000
>  #define BLT_RING_BASE		0x22000
> +#define XEHPC_BCS1_RING_BASE	0x3e0000
> +#define XEHPC_BCS2_RING_BASE	0x3e2000
> +#define XEHPC_BCS3_RING_BASE	0x3e4000
> +#define XEHPC_BCS4_RING_BASE	0x3e6000
> +#define XEHPC_BCS5_RING_BASE	0x3e8000
> +#define XEHPC_BCS6_RING_BASE	0x3ea000
> +#define XEHPC_BCS7_RING_BASE	0x3ec000
> +#define XEHPC_BCS8_RING_BASE	0x3ee000
>  #define DG1_GSC_HECI1_BASE	0x00258000
>  #define DG1_GSC_HECI2_BASE	0x00259000
>  #define DG2_GSC_HECI1_BASE	0x00373000

Re: [Intel-gfx] [PATCH 07/11] drm/i915/pvc: Engines definitions for new copy engines

[Tvrtko Ursulin]

On 02/05/2022 17:34, Matt Roper wrote:
> This patch adds the basic definitions needed to support
> new copy engines. Also updating the cmd_info to accommodate
> new engines, as the engine id's of legacy engines have been
> changed.
> 
> Original-author: CQ Tang
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 56 ++++++++++++++++++++
>   drivers/gpu/drm/i915/gt/intel_engine_types.h | 10 +++-
>   drivers/gpu/drm/i915/gt/intel_gt_regs.h      |  8 +++
>   drivers/gpu/drm/i915/gvt/cmd_parser.c        |  2 +-
>   drivers/gpu/drm/i915/i915_reg.h              |  8 +++
>   5 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 14c6ddbbfde8..4532c3ea9ace 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -71,6 +71,62 @@ static const struct engine_info intel_engines[] = {
>   			{ .graphics_ver = 6, .base = BLT_RING_BASE }
>   		},
>   	},
> +	[BCS1] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 1,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS1_RING_BASE }
> +		},
> +	},
> +	[BCS2] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 2,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS2_RING_BASE }
> +		},
> +	},
> +	[BCS3] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 3,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS3_RING_BASE }
> +		},
> +	},
> +	[BCS4] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 4,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS4_RING_BASE }
> +		},
> +	},
> +	[BCS5] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 5,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS5_RING_BASE }
> +		},
> +	},
> +	[BCS6] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 6,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS6_RING_BASE }
> +		},
> +	},
> +	[BCS7] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 7,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS7_RING_BASE }
> +		},
> +	},
> +	[BCS8] = {
> +		.class = COPY_ENGINE_CLASS,
> +		.instance = 8,
> +		.mmio_bases = {
> +			{ .graphics_ver = 12, .base = XEHPC_BCS8_RING_BASE }
> +		},
> +	},
>   	[VCS0] = {
>   		.class = VIDEO_DECODE_CLASS,
>   		.instance = 0,
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 298f2cc7a879..356c15cdccf0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -35,7 +35,7 @@
>   #define OTHER_CLASS		4
>   #define COMPUTE_CLASS		5
>   #define MAX_ENGINE_CLASS	5
> -#define MAX_ENGINE_INSTANCE	7
> +#define MAX_ENGINE_INSTANCE	8
>   
>   #define I915_MAX_SLICES	3
>   #define I915_MAX_SUBSLICES 8
> @@ -107,6 +107,14 @@ struct i915_ctx_workarounds {
>   enum intel_engine_id {
>   	RCS0 = 0,
>   	BCS0,
> +	BCS1,
> +	BCS2,
> +	BCS3,
> +	BCS4,
> +	BCS5,
> +	BCS6,
> +	BCS7,
> +	BCS8,

_BCS(n) macro will not be required?

>   	VCS0,
>   	VCS1,
>   	VCS2,
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index a0a49c16babd..aa2c0974b02c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1476,6 +1476,14 @@
>   #define   GEN11_KCR				(19)
>   #define   GEN11_GTPM				(16)
>   #define   GEN11_BCS				(15)
> +#define   XEHPC_BCS1				(14)
> +#define   XEHPC_BCS2				(13)
> +#define   XEHPC_BCS3				(12)
> +#define   XEHPC_BCS4				(11)
> +#define   XEHPC_BCS5				(10)
> +#define   XEHPC_BCS6				(9)
> +#define   XEHPC_BCS7				(8)
> +#define   XEHPC_BCS8				(23)
>   #define   GEN12_CCS3				(7)
>   #define   GEN12_CCS2				(6)
>   #define   GEN12_CCS1				(5)
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index b9eb75a2b400..0ba2a3455d99 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -428,7 +428,7 @@ struct cmd_info {
>   #define R_VECS	BIT(VECS0)
>   #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)
>   	/* rings that support this cmd: BLT/RCS/VCS/VECS */
> -	u16 rings;
> +	intel_engine_mask_t rings;

Looks like mask already overflows u16 even without the blitter engines. 
(When CCS were added.) Meaning maybe there should be a separate patch to 
fix it.

But good question though is GVT supporting CCS and should it be part of 
R_ALL? Or should this patch even be touching GVT since CCS enablement 
did not? Adding Zhi to comment.

Regards,

Tvrtko

>   
>   	/* devices that support this cmd: SNB/IVB/HSW/... */
>   	u16 devices;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4a3d7b96ef43..ab64ab4317b3 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -976,6 +976,14 @@
>   #define GEN12_COMPUTE2_RING_BASE	0x1e000
>   #define GEN12_COMPUTE3_RING_BASE	0x26000
>   #define BLT_RING_BASE		0x22000
> +#define XEHPC_BCS1_RING_BASE	0x3e0000
> +#define XEHPC_BCS2_RING_BASE	0x3e2000
> +#define XEHPC_BCS3_RING_BASE	0x3e4000
> +#define XEHPC_BCS4_RING_BASE	0x3e6000
> +#define XEHPC_BCS5_RING_BASE	0x3e8000
> +#define XEHPC_BCS6_RING_BASE	0x3ea000
> +#define XEHPC_BCS7_RING_BASE	0x3ec000
> +#define XEHPC_BCS8_RING_BASE	0x3ee000
>   #define DG1_GSC_HECI1_BASE	0x00258000
>   #define DG1_GSC_HECI2_BASE	0x00259000
>   #define DG2_GSC_HECI1_BASE	0x00373000

Re: [Intel-gfx] [PATCH 07/11] drm/i915/pvc: Engines definitions for new copy engines

[Matt Roper]

On Tue, May 03, 2022 at 09:05:43AM +0100, Tvrtko Ursulin wrote:
> 
> On 02/05/2022 17:34, Matt Roper wrote:
> > This patch adds the basic definitions needed to support
> > new copy engines. Also updating the cmd_info to accommodate
> > new engines, as the engine id's of legacy engines have been
> > changed.
> > 
> > Original-author: CQ Tang
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 56 ++++++++++++++++++++
> >   drivers/gpu/drm/i915/gt/intel_engine_types.h | 10 +++-
> >   drivers/gpu/drm/i915/gt/intel_gt_regs.h      |  8 +++
> >   drivers/gpu/drm/i915/gvt/cmd_parser.c        |  2 +-
> >   drivers/gpu/drm/i915/i915_reg.h              |  8 +++
> >   5 files changed, 82 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index 14c6ddbbfde8..4532c3ea9ace 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -71,6 +71,62 @@ static const struct engine_info intel_engines[] = {
> >   			{ .graphics_ver = 6, .base = BLT_RING_BASE }
> >   		},
> >   	},
> > +	[BCS1] = {
> > +		.class = COPY_ENGINE_CLASS,
> > +		.instance = 1,
> > +		.mmio_bases = {
> > +			{ .graphics_ver = 12, .base = XEHPC_BCS1_RING_BASE }
> > +		},
> > +	},
> > +	[BCS2] = {
> > +		.class = COPY_ENGINE_CLASS,
> > +		.instance = 2,
> > +		.mmio_bases = {
> > +			{ .graphics_ver = 12, .base = XEHPC_BCS2_RING_BASE }
> > +		},
> > +	},
> > +	[BCS3] = {
> > +		.class = COPY_ENGINE_CLASS,
> > +		.instance = 3,
> > +		.mmio_bases = {
> > +			{ .graphics_ver = 12, .base = XEHPC_BCS3_RING_BASE }
> > +		},
> > +	},
> > +	[BCS4] = {
> > +		.class = COPY_ENGINE_CLASS,
> > +		.instance = 4,
> > +		.mmio_bases = {
> > +			{ .graphics_ver = 12, .base = XEHPC_BCS4_RING_BASE }
> > +		},
> > +	},
> > +	[BCS5] = {
> > +		.class = COPY_ENGINE_CLASS,
> > +		.instance = 5,
> > +		.mmio_bases = {
> > +			{ .graphics_ver = 12, .base = XEHPC_BCS5_RING_BASE }
> > +		},
> > +	},
> > +	[BCS6] = {
> > +		.class = COPY_ENGINE_CLASS,
> > +		.instance = 6,
> > +		.mmio_bases = {
> > +			{ .graphics_ver = 12, .base = XEHPC_BCS6_RING_BASE }
> > +		},
> > +	},
> > +	[BCS7] = {
> > +		.class = COPY_ENGINE_CLASS,
> > +		.instance = 7,
> > +		.mmio_bases = {
> > +			{ .graphics_ver = 12, .base = XEHPC_BCS7_RING_BASE }
> > +		},
> > +	},
> > +	[BCS8] = {
> > +		.class = COPY_ENGINE_CLASS,
> > +		.instance = 8,
> > +		.mmio_bases = {
> > +			{ .graphics_ver = 12, .base = XEHPC_BCS8_RING_BASE }
> > +		},
> > +	},
> >   	[VCS0] = {
> >   		.class = VIDEO_DECODE_CLASS,
> >   		.instance = 0,
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > index 298f2cc7a879..356c15cdccf0 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > @@ -35,7 +35,7 @@
> >   #define OTHER_CLASS		4
> >   #define COMPUTE_CLASS		5
> >   #define MAX_ENGINE_CLASS	5
> > -#define MAX_ENGINE_INSTANCE	7
> > +#define MAX_ENGINE_INSTANCE	8
> >   #define I915_MAX_SLICES	3
> >   #define I915_MAX_SUBSLICES 8
> > @@ -107,6 +107,14 @@ struct i915_ctx_workarounds {
> >   enum intel_engine_id {
> >   	RCS0 = 0,
> >   	BCS0,
> > +	BCS1,
> > +	BCS2,
> > +	BCS3,
> > +	BCS4,
> > +	BCS5,
> > +	BCS6,
> > +	BCS7,
> > +	BCS8,
> 
> _BCS(n) macro will not be required?
> 
> >   	VCS0,
> >   	VCS1,
> >   	VCS2,
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index a0a49c16babd..aa2c0974b02c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1476,6 +1476,14 @@
> >   #define   GEN11_KCR				(19)
> >   #define   GEN11_GTPM				(16)
> >   #define   GEN11_BCS				(15)
> > +#define   XEHPC_BCS1				(14)
> > +#define   XEHPC_BCS2				(13)
> > +#define   XEHPC_BCS3				(12)
> > +#define   XEHPC_BCS4				(11)
> > +#define   XEHPC_BCS5				(10)
> > +#define   XEHPC_BCS6				(9)
> > +#define   XEHPC_BCS7				(8)
> > +#define   XEHPC_BCS8				(23)
> >   #define   GEN12_CCS3				(7)
> >   #define   GEN12_CCS2				(6)
> >   #define   GEN12_CCS1				(5)
> > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > index b9eb75a2b400..0ba2a3455d99 100644
> > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > @@ -428,7 +428,7 @@ struct cmd_info {
> >   #define R_VECS	BIT(VECS0)
> >   #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)
> >   	/* rings that support this cmd: BLT/RCS/VCS/VECS */
> > -	u16 rings;
> > +	intel_engine_mask_t rings;
> 
> Looks like mask already overflows u16 even without the blitter engines.
> (When CCS were added.) Meaning maybe there should be a separate patch to fix
> it.

Adding the CCS engines didn't cause a problem because GVT only includes
the gen11 set of engines in R_ALL.  Since the CCS engines (and even the
higher instances of VCS/VECS introduced by Xe_HP) are never used
anywhere in GVT, there's no overflow possible...the highest bit they
ever use anywhere is VECS0; before this patch, that was bit(10) and fit
within a u16 comfortably.  But since the new BCS engines added by this
patch get inserted at lower values within the engine_id enum, the
location of VECS0 moves up to bit(18), which falls outside the u16
definition and triggers a build failure:

drivers/gpu/drm/i915/gvt/cmd_parser.c:429:15: error: conversion from ‘long unsigned int’ to ‘short unsigned int’ changes value from ‘265219’ to ‘3075’ [-Werror=overflow]
  429 | #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)


Matt

> 
> But good question though is GVT supporting CCS and should it be part of
> R_ALL? Or should this patch even be touching GVT since CCS enablement did
> not? Adding Zhi to comment.
> 
> Regards,
> 
> Tvrtko
> 
> >   	/* devices that support this cmd: SNB/IVB/HSW/... */
> >   	u16 devices;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 4a3d7b96ef43..ab64ab4317b3 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -976,6 +976,14 @@
> >   #define GEN12_COMPUTE2_RING_BASE	0x1e000
> >   #define GEN12_COMPUTE3_RING_BASE	0x26000
> >   #define BLT_RING_BASE		0x22000
> > +#define XEHPC_BCS1_RING_BASE	0x3e0000
> > +#define XEHPC_BCS2_RING_BASE	0x3e2000
> > +#define XEHPC_BCS3_RING_BASE	0x3e4000
> > +#define XEHPC_BCS4_RING_BASE	0x3e6000
> > +#define XEHPC_BCS5_RING_BASE	0x3e8000
> > +#define XEHPC_BCS6_RING_BASE	0x3ea000
> > +#define XEHPC_BCS7_RING_BASE	0x3ec000
> > +#define XEHPC_BCS8_RING_BASE	0x3ee000
> >   #define DG1_GSC_HECI1_BASE	0x00258000
> >   #define DG1_GSC_HECI2_BASE	0x00259000
> >   #define DG2_GSC_HECI1_BASE	0x00373000

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

Re: [Intel-gfx] [PATCH 07/11] drm/i915/pvc: Engines definitions for new copy engines

[Tvrtko Ursulin]

On 05/05/2022 21:59, Matt Roper wrote:
> On Tue, May 03, 2022 at 09:05:43AM +0100, Tvrtko Ursulin wrote:
>>
>> On 02/05/2022 17:34, Matt Roper wrote:
>>> This patch adds the basic definitions needed to support
>>> new copy engines. Also updating the cmd_info to accommodate
>>> new engines, as the engine id's of legacy engines have been
>>> changed.
>>>
>>> Original-author: CQ Tang
>>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 56 ++++++++++++++++++++
>>>    drivers/gpu/drm/i915/gt/intel_engine_types.h | 10 +++-
>>>    drivers/gpu/drm/i915/gt/intel_gt_regs.h      |  8 +++
>>>    drivers/gpu/drm/i915/gvt/cmd_parser.c        |  2 +-
>>>    drivers/gpu/drm/i915/i915_reg.h              |  8 +++
>>>    5 files changed, 82 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> index 14c6ddbbfde8..4532c3ea9ace 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> @@ -71,6 +71,62 @@ static const struct engine_info intel_engines[] = {
>>>    			{ .graphics_ver = 6, .base = BLT_RING_BASE }
>>>    		},
>>>    	},
>>> +	[BCS1] = {
>>> +		.class = COPY_ENGINE_CLASS,
>>> +		.instance = 1,
>>> +		.mmio_bases = {
>>> +			{ .graphics_ver = 12, .base = XEHPC_BCS1_RING_BASE }
>>> +		},
>>> +	},
>>> +	[BCS2] = {
>>> +		.class = COPY_ENGINE_CLASS,
>>> +		.instance = 2,
>>> +		.mmio_bases = {
>>> +			{ .graphics_ver = 12, .base = XEHPC_BCS2_RING_BASE }
>>> +		},
>>> +	},
>>> +	[BCS3] = {
>>> +		.class = COPY_ENGINE_CLASS,
>>> +		.instance = 3,
>>> +		.mmio_bases = {
>>> +			{ .graphics_ver = 12, .base = XEHPC_BCS3_RING_BASE }
>>> +		},
>>> +	},
>>> +	[BCS4] = {
>>> +		.class = COPY_ENGINE_CLASS,
>>> +		.instance = 4,
>>> +		.mmio_bases = {
>>> +			{ .graphics_ver = 12, .base = XEHPC_BCS4_RING_BASE }
>>> +		},
>>> +	},
>>> +	[BCS5] = {
>>> +		.class = COPY_ENGINE_CLASS,
>>> +		.instance = 5,
>>> +		.mmio_bases = {
>>> +			{ .graphics_ver = 12, .base = XEHPC_BCS5_RING_BASE }
>>> +		},
>>> +	},
>>> +	[BCS6] = {
>>> +		.class = COPY_ENGINE_CLASS,
>>> +		.instance = 6,
>>> +		.mmio_bases = {
>>> +			{ .graphics_ver = 12, .base = XEHPC_BCS6_RING_BASE }
>>> +		},
>>> +	},
>>> +	[BCS7] = {
>>> +		.class = COPY_ENGINE_CLASS,
>>> +		.instance = 7,
>>> +		.mmio_bases = {
>>> +			{ .graphics_ver = 12, .base = XEHPC_BCS7_RING_BASE }
>>> +		},
>>> +	},
>>> +	[BCS8] = {
>>> +		.class = COPY_ENGINE_CLASS,
>>> +		.instance = 8,
>>> +		.mmio_bases = {
>>> +			{ .graphics_ver = 12, .base = XEHPC_BCS8_RING_BASE }
>>> +		},
>>> +	},
>>>    	[VCS0] = {
>>>    		.class = VIDEO_DECODE_CLASS,
>>>    		.instance = 0,
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> index 298f2cc7a879..356c15cdccf0 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
>>> @@ -35,7 +35,7 @@
>>>    #define OTHER_CLASS		4
>>>    #define COMPUTE_CLASS		5
>>>    #define MAX_ENGINE_CLASS	5
>>> -#define MAX_ENGINE_INSTANCE	7
>>> +#define MAX_ENGINE_INSTANCE	8
>>>    #define I915_MAX_SLICES	3
>>>    #define I915_MAX_SUBSLICES 8
>>> @@ -107,6 +107,14 @@ struct i915_ctx_workarounds {
>>>    enum intel_engine_id {
>>>    	RCS0 = 0,
>>>    	BCS0,
>>> +	BCS1,
>>> +	BCS2,
>>> +	BCS3,
>>> +	BCS4,
>>> +	BCS5,
>>> +	BCS6,
>>> +	BCS7,
>>> +	BCS8,
>>
>> _BCS(n) macro will not be required?
>>
>>>    	VCS0,
>>>    	VCS1,
>>>    	VCS2,
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> index a0a49c16babd..aa2c0974b02c 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
>>> @@ -1476,6 +1476,14 @@
>>>    #define   GEN11_KCR				(19)
>>>    #define   GEN11_GTPM				(16)
>>>    #define   GEN11_BCS				(15)
>>> +#define   XEHPC_BCS1				(14)
>>> +#define   XEHPC_BCS2				(13)
>>> +#define   XEHPC_BCS3				(12)
>>> +#define   XEHPC_BCS4				(11)
>>> +#define   XEHPC_BCS5				(10)
>>> +#define   XEHPC_BCS6				(9)
>>> +#define   XEHPC_BCS7				(8)
>>> +#define   XEHPC_BCS8				(23)
>>>    #define   GEN12_CCS3				(7)
>>>    #define   GEN12_CCS2				(6)
>>>    #define   GEN12_CCS1				(5)
>>> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
>>> index b9eb75a2b400..0ba2a3455d99 100644
>>> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
>>> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
>>> @@ -428,7 +428,7 @@ struct cmd_info {
>>>    #define R_VECS	BIT(VECS0)
>>>    #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)
>>>    	/* rings that support this cmd: BLT/RCS/VCS/VECS */
>>> -	u16 rings;
>>> +	intel_engine_mask_t rings;
>>
>> Looks like mask already overflows u16 even without the blitter engines.
>> (When CCS were added.) Meaning maybe there should be a separate patch to fix
>> it.
> 
> Adding the CCS engines didn't cause a problem because GVT only includes
> the gen11 set of engines in R_ALL.  Since the CCS engines (and even the
> higher instances of VCS/VECS introduced by Xe_HP) are never used
> anywhere in GVT, there's no overflow possible...the highest bit they
> ever use anywhere is VECS0; before this patch, that was bit(10) and fit
> within a u16 comfortably.  But since the new BCS engines added by this
> patch get inserted at lower values within the engine_id enum, the
> location of VECS0 moves up to bit(18), which falls outside the u16
> definition and triggers a build failure:
> 
> drivers/gpu/drm/i915/gvt/cmd_parser.c:429:15: error: conversion from ‘long unsigned int’ to ‘short unsigned int’ changes value from ‘265219’ to ‘3075’ [-Werror=overflow]
>    429 | #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)

Well I did not know why GVT does not support CCS, or when it will start 
supporting it. If that support would happen in a kernel with CCS 
support, but not this PVC patch then it would need the above hunk. So 
fundamentally it sounded the hunk belongs to a separate patch. But 
unless Zhi can comment I guess you are okay to proceed with what you have.

Regards,

Tvrtko

>>
>> But good question though is GVT supporting CCS and should it be part of
>> R_ALL? Or should this patch even be touching GVT since CCS enablement did
>> not? Adding Zhi to comment.
>>
>> Regards,
>>
>> Tvrtko
>>
>>>    	/* devices that support this cmd: SNB/IVB/HSW/... */
>>>    	u16 devices;
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 4a3d7b96ef43..ab64ab4317b3 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -976,6 +976,14 @@
>>>    #define GEN12_COMPUTE2_RING_BASE	0x1e000
>>>    #define GEN12_COMPUTE3_RING_BASE	0x26000
>>>    #define BLT_RING_BASE		0x22000
>>> +#define XEHPC_BCS1_RING_BASE	0x3e0000
>>> +#define XEHPC_BCS2_RING_BASE	0x3e2000
>>> +#define XEHPC_BCS3_RING_BASE	0x3e4000
>>> +#define XEHPC_BCS4_RING_BASE	0x3e6000
>>> +#define XEHPC_BCS5_RING_BASE	0x3e8000
>>> +#define XEHPC_BCS6_RING_BASE	0x3ea000
>>> +#define XEHPC_BCS7_RING_BASE	0x3ec000
>>> +#define XEHPC_BCS8_RING_BASE	0x3ee000
>>>    #define DG1_GSC_HECI1_BASE	0x00258000
>>>    #define DG1_GSC_HECI2_BASE	0x00259000
>>>    #define DG2_GSC_HECI1_BASE	0x00373000
> 

Re: [Intel-gfx] [PATCH 07/11] drm/i915/pvc: Engines definitions for new copy engines

[Matt Roper]

On Fri, May 06, 2022 at 08:21:46AM +0100, Tvrtko Ursulin wrote:
> 
> On 05/05/2022 21:59, Matt Roper wrote:
> > On Tue, May 03, 2022 at 09:05:43AM +0100, Tvrtko Ursulin wrote:
> > > 
> > > On 02/05/2022 17:34, Matt Roper wrote:
> > > > This patch adds the basic definitions needed to support
> > > > new copy engines. Also updating the cmd_info to accommodate
> > > > new engines, as the engine id's of legacy engines have been
> > > > changed.
> > > > 
> > > > Original-author: CQ Tang
> > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > > > ---
> > > >    drivers/gpu/drm/i915/gt/intel_engine_cs.c    | 56 ++++++++++++++++++++
> > > >    drivers/gpu/drm/i915/gt/intel_engine_types.h | 10 +++-
> > > >    drivers/gpu/drm/i915/gt/intel_gt_regs.h      |  8 +++
> > > >    drivers/gpu/drm/i915/gvt/cmd_parser.c        |  2 +-
> > > >    drivers/gpu/drm/i915/i915_reg.h              |  8 +++
> > > >    5 files changed, 82 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > index 14c6ddbbfde8..4532c3ea9ace 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > > > @@ -71,6 +71,62 @@ static const struct engine_info intel_engines[] = {
> > > >    			{ .graphics_ver = 6, .base = BLT_RING_BASE }
> > > >    		},
> > > >    	},
> > > > +	[BCS1] = {
> > > > +		.class = COPY_ENGINE_CLASS,
> > > > +		.instance = 1,
> > > > +		.mmio_bases = {
> > > > +			{ .graphics_ver = 12, .base = XEHPC_BCS1_RING_BASE }
> > > > +		},
> > > > +	},
> > > > +	[BCS2] = {
> > > > +		.class = COPY_ENGINE_CLASS,
> > > > +		.instance = 2,
> > > > +		.mmio_bases = {
> > > > +			{ .graphics_ver = 12, .base = XEHPC_BCS2_RING_BASE }
> > > > +		},
> > > > +	},
> > > > +	[BCS3] = {
> > > > +		.class = COPY_ENGINE_CLASS,
> > > > +		.instance = 3,
> > > > +		.mmio_bases = {
> > > > +			{ .graphics_ver = 12, .base = XEHPC_BCS3_RING_BASE }
> > > > +		},
> > > > +	},
> > > > +	[BCS4] = {
> > > > +		.class = COPY_ENGINE_CLASS,
> > > > +		.instance = 4,
> > > > +		.mmio_bases = {
> > > > +			{ .graphics_ver = 12, .base = XEHPC_BCS4_RING_BASE }
> > > > +		},
> > > > +	},
> > > > +	[BCS5] = {
> > > > +		.class = COPY_ENGINE_CLASS,
> > > > +		.instance = 5,
> > > > +		.mmio_bases = {
> > > > +			{ .graphics_ver = 12, .base = XEHPC_BCS5_RING_BASE }
> > > > +		},
> > > > +	},
> > > > +	[BCS6] = {
> > > > +		.class = COPY_ENGINE_CLASS,
> > > > +		.instance = 6,
> > > > +		.mmio_bases = {
> > > > +			{ .graphics_ver = 12, .base = XEHPC_BCS6_RING_BASE }
> > > > +		},
> > > > +	},
> > > > +	[BCS7] = {
> > > > +		.class = COPY_ENGINE_CLASS,
> > > > +		.instance = 7,
> > > > +		.mmio_bases = {
> > > > +			{ .graphics_ver = 12, .base = XEHPC_BCS7_RING_BASE }
> > > > +		},
> > > > +	},
> > > > +	[BCS8] = {
> > > > +		.class = COPY_ENGINE_CLASS,
> > > > +		.instance = 8,
> > > > +		.mmio_bases = {
> > > > +			{ .graphics_ver = 12, .base = XEHPC_BCS8_RING_BASE }
> > > > +		},
> > > > +	},
> > > >    	[VCS0] = {
> > > >    		.class = VIDEO_DECODE_CLASS,
> > > >    		.instance = 0,
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > > index 298f2cc7a879..356c15cdccf0 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > > > @@ -35,7 +35,7 @@
> > > >    #define OTHER_CLASS		4
> > > >    #define COMPUTE_CLASS		5
> > > >    #define MAX_ENGINE_CLASS	5
> > > > -#define MAX_ENGINE_INSTANCE	7
> > > > +#define MAX_ENGINE_INSTANCE	8
> > > >    #define I915_MAX_SLICES	3
> > > >    #define I915_MAX_SUBSLICES 8
> > > > @@ -107,6 +107,14 @@ struct i915_ctx_workarounds {
> > > >    enum intel_engine_id {
> > > >    	RCS0 = 0,
> > > >    	BCS0,
> > > > +	BCS1,
> > > > +	BCS2,
> > > > +	BCS3,
> > > > +	BCS4,
> > > > +	BCS5,
> > > > +	BCS6,
> > > > +	BCS7,
> > > > +	BCS8,
> > > 
> > > _BCS(n) macro will not be required?
> > > 
> > > >    	VCS0,
> > > >    	VCS1,
> > > >    	VCS2,
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > index a0a49c16babd..aa2c0974b02c 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > > @@ -1476,6 +1476,14 @@
> > > >    #define   GEN11_KCR				(19)
> > > >    #define   GEN11_GTPM				(16)
> > > >    #define   GEN11_BCS				(15)
> > > > +#define   XEHPC_BCS1				(14)
> > > > +#define   XEHPC_BCS2				(13)
> > > > +#define   XEHPC_BCS3				(12)
> > > > +#define   XEHPC_BCS4				(11)
> > > > +#define   XEHPC_BCS5				(10)
> > > > +#define   XEHPC_BCS6				(9)
> > > > +#define   XEHPC_BCS7				(8)
> > > > +#define   XEHPC_BCS8				(23)
> > > >    #define   GEN12_CCS3				(7)
> > > >    #define   GEN12_CCS2				(6)
> > > >    #define   GEN12_CCS1				(5)
> > > > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > > index b9eb75a2b400..0ba2a3455d99 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > > @@ -428,7 +428,7 @@ struct cmd_info {
> > > >    #define R_VECS	BIT(VECS0)
> > > >    #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)
> > > >    	/* rings that support this cmd: BLT/RCS/VCS/VECS */
> > > > -	u16 rings;
> > > > +	intel_engine_mask_t rings;
> > > 
> > > Looks like mask already overflows u16 even without the blitter engines.
> > > (When CCS were added.) Meaning maybe there should be a separate patch to fix
> > > it.
> > 
> > Adding the CCS engines didn't cause a problem because GVT only includes
> > the gen11 set of engines in R_ALL.  Since the CCS engines (and even the
> > higher instances of VCS/VECS introduced by Xe_HP) are never used
> > anywhere in GVT, there's no overflow possible...the highest bit they
> > ever use anywhere is VECS0; before this patch, that was bit(10) and fit
> > within a u16 comfortably.  But since the new BCS engines added by this
> > patch get inserted at lower values within the engine_id enum, the
> > location of VECS0 moves up to bit(18), which falls outside the u16
> > definition and triggers a build failure:
> > 
> > drivers/gpu/drm/i915/gvt/cmd_parser.c:429:15: error: conversion from ‘long unsigned int’ to ‘short unsigned int’ changes value from ‘265219’ to ‘3075’ [-Werror=overflow]
> >    429 | #define R_ALL (R_RCS | R_VCS | R_BCS | R_VECS)
> 
> Well I did not know why GVT does not support CCS, or when it will start
> supporting it. If that support would happen in a kernel with CCS support,

I believe GVT only supports up through gen11 (even gen12 support is
missing today if I understand correctly).  Since the CCS engines only
arrive with Xe_HP they aren't really relevant to GVT right now.


Matt

> but not this PVC patch then it would need the above hunk. So fundamentally
> it sounded the hunk belongs to a separate patch. But unless Zhi can comment
> I guess you are okay to proceed with what you have.
> 
> Regards,
> 
> Tvrtko
> 
> > > 
> > > But good question though is GVT supporting CCS and should it be part of
> > > R_ALL? Or should this patch even be touching GVT since CCS enablement did
> > > not? Adding Zhi to comment.
> > > 
> > > Regards,
> > > 
> > > Tvrtko
> > > 
> > > >    	/* devices that support this cmd: SNB/IVB/HSW/... */
> > > >    	u16 devices;
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 4a3d7b96ef43..ab64ab4317b3 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -976,6 +976,14 @@
> > > >    #define GEN12_COMPUTE2_RING_BASE	0x1e000
> > > >    #define GEN12_COMPUTE3_RING_BASE	0x26000
> > > >    #define BLT_RING_BASE		0x22000
> > > > +#define XEHPC_BCS1_RING_BASE	0x3e0000
> > > > +#define XEHPC_BCS2_RING_BASE	0x3e2000
> > > > +#define XEHPC_BCS3_RING_BASE	0x3e4000
> > > > +#define XEHPC_BCS4_RING_BASE	0x3e6000
> > > > +#define XEHPC_BCS5_RING_BASE	0x3e8000
> > > > +#define XEHPC_BCS6_RING_BASE	0x3ea000
> > > > +#define XEHPC_BCS7_RING_BASE	0x3ec000
> > > > +#define XEHPC_BCS8_RING_BASE	0x3ee000
> > > >    #define DG1_GSC_HECI1_BASE	0x00258000
> > > >    #define DG1_GSC_HECI2_BASE	0x00259000
> > > >    #define DG2_GSC_HECI1_BASE	0x00373000
> > 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

[PATCH 08/11] drm/i915/pvc: Interrupt support for new copy engines

[Matt Roper]

This patch adds the interrupt handler support for
new copy engines.

Bspec: 54030
Original-author: CQ Tang
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_irq.c  | 16 ++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_regs.h |  4 ++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 88b4becfcb17..3a72d4fd0214 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -193,6 +193,14 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
 	/* Restore masks irqs on RCS, BCS, VCS and VECS engines. */
 	intel_uncore_write(uncore, GEN11_RCS0_RSVD_INTR_MASK,	~0);
 	intel_uncore_write(uncore, GEN11_BCS_RSVD_INTR_MASK,	~0);
+	if (HAS_ENGINE(gt, BCS1) || HAS_ENGINE(gt, BCS2))
+		intel_uncore_write(uncore, XEHPC_BCS1_BCS2_INTR_MASK, ~0);
+	if (HAS_ENGINE(gt, BCS3) || HAS_ENGINE(gt, BCS4))
+		intel_uncore_write(uncore, XEHPC_BCS3_BCS4_INTR_MASK, ~0);
+	if (HAS_ENGINE(gt, BCS5) || HAS_ENGINE(gt, BCS6))
+		intel_uncore_write(uncore, XEHPC_BCS5_BCS6_INTR_MASK, ~0);
+	if (HAS_ENGINE(gt, BCS7) || HAS_ENGINE(gt, BCS8))
+		intel_uncore_write(uncore, XEHPC_BCS7_BCS8_INTR_MASK, ~0);
 	intel_uncore_write(uncore, GEN11_VCS0_VCS1_INTR_MASK,	~0);
 	intel_uncore_write(uncore, GEN11_VCS2_VCS3_INTR_MASK,	~0);
 	if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
@@ -248,6 +256,14 @@ void gen11_gt_irq_postinstall(struct intel_gt *gt)
 	/* Unmask irqs on RCS, BCS, VCS and VECS engines. */
 	intel_uncore_write(uncore, GEN11_RCS0_RSVD_INTR_MASK, ~smask);
 	intel_uncore_write(uncore, GEN11_BCS_RSVD_INTR_MASK, ~smask);
+	if (HAS_ENGINE(gt, BCS1) || HAS_ENGINE(gt, BCS2))
+		intel_uncore_write(uncore, XEHPC_BCS1_BCS2_INTR_MASK, ~dmask);
+	if (HAS_ENGINE(gt, BCS3) || HAS_ENGINE(gt, BCS4))
+		intel_uncore_write(uncore, XEHPC_BCS3_BCS4_INTR_MASK, ~dmask);
+	if (HAS_ENGINE(gt, BCS5) || HAS_ENGINE(gt, BCS6))
+		intel_uncore_write(uncore, XEHPC_BCS5_BCS6_INTR_MASK, ~dmask);
+	if (HAS_ENGINE(gt, BCS7) || HAS_ENGINE(gt, BCS8))
+		intel_uncore_write(uncore, XEHPC_BCS7_BCS8_INTR_MASK, ~dmask);
 	intel_uncore_write(uncore, GEN11_VCS0_VCS1_INTR_MASK, ~dmask);
 	intel_uncore_write(uncore, GEN11_VCS2_VCS3_INTR_MASK, ~dmask);
 	if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index aa2c0974b02c..fe09288a3145 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1529,6 +1529,10 @@
 #define GEN11_GUNIT_CSME_INTR_MASK		_MMIO(0x1900f4)
 #define GEN12_CCS0_CCS1_INTR_MASK		_MMIO(0x190100)
 #define GEN12_CCS2_CCS3_INTR_MASK		_MMIO(0x190104)
+#define XEHPC_BCS1_BCS2_INTR_MASK		_MMIO(0x190110)
+#define XEHPC_BCS3_BCS4_INTR_MASK		_MMIO(0x190114)
+#define XEHPC_BCS5_BCS6_INTR_MASK		_MMIO(0x190118)
+#define XEHPC_BCS7_BCS8_INTR_MASK		_MMIO(0x19011c)
 
 #define GEN12_SFC_DONE(n)			_MMIO(0x1cc000 + (n) * 0x1000)
 
-- 
2.35.1

Re: [PATCH 08/11] drm/i915/pvc: Interrupt support for new copy engines

[Summers, Stuart]

On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> This patch adds the interrupt handler support for

Imperative: Add the interrupt support for...

Otherwise:
Reviewed-by: Stuart Summers <stuart.summers@intel.com>

> new copy engines.
> 
> Bspec: 54030
> Original-author: CQ Tang
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_irq.c  | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h |  4 ++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> index 88b4becfcb17..3a72d4fd0214 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> @@ -193,6 +193,14 @@ void gen11_gt_irq_reset(struct intel_gt *gt)
>  	/* Restore masks irqs on RCS, BCS, VCS and VECS engines. */
>  	intel_uncore_write(uncore, GEN11_RCS0_RSVD_INTR_MASK,	~0);
>  	intel_uncore_write(uncore, GEN11_BCS_RSVD_INTR_MASK,	~0);
> +	if (HAS_ENGINE(gt, BCS1) || HAS_ENGINE(gt, BCS2))
> +		intel_uncore_write(uncore, XEHPC_BCS1_BCS2_INTR_MASK,
> ~0);
> +	if (HAS_ENGINE(gt, BCS3) || HAS_ENGINE(gt, BCS4))
> +		intel_uncore_write(uncore, XEHPC_BCS3_BCS4_INTR_MASK,
> ~0);
> +	if (HAS_ENGINE(gt, BCS5) || HAS_ENGINE(gt, BCS6))
> +		intel_uncore_write(uncore, XEHPC_BCS5_BCS6_INTR_MASK,
> ~0);
> +	if (HAS_ENGINE(gt, BCS7) || HAS_ENGINE(gt, BCS8))
> +		intel_uncore_write(uncore, XEHPC_BCS7_BCS8_INTR_MASK,
> ~0);
>  	intel_uncore_write(uncore, GEN11_VCS0_VCS1_INTR_MASK,	~0);
>  	intel_uncore_write(uncore, GEN11_VCS2_VCS3_INTR_MASK,	~0);
>  	if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
> @@ -248,6 +256,14 @@ void gen11_gt_irq_postinstall(struct intel_gt
> *gt)
>  	/* Unmask irqs on RCS, BCS, VCS and VECS engines. */
>  	intel_uncore_write(uncore, GEN11_RCS0_RSVD_INTR_MASK, ~smask);
>  	intel_uncore_write(uncore, GEN11_BCS_RSVD_INTR_MASK, ~smask);
> +	if (HAS_ENGINE(gt, BCS1) || HAS_ENGINE(gt, BCS2))
> +		intel_uncore_write(uncore, XEHPC_BCS1_BCS2_INTR_MASK,
> ~dmask);
> +	if (HAS_ENGINE(gt, BCS3) || HAS_ENGINE(gt, BCS4))
> +		intel_uncore_write(uncore, XEHPC_BCS3_BCS4_INTR_MASK,
> ~dmask);
> +	if (HAS_ENGINE(gt, BCS5) || HAS_ENGINE(gt, BCS6))
> +		intel_uncore_write(uncore, XEHPC_BCS5_BCS6_INTR_MASK,
> ~dmask);
> +	if (HAS_ENGINE(gt, BCS7) || HAS_ENGINE(gt, BCS8))
> +		intel_uncore_write(uncore, XEHPC_BCS7_BCS8_INTR_MASK,
> ~dmask);
>  	intel_uncore_write(uncore, GEN11_VCS0_VCS1_INTR_MASK, ~dmask);
>  	intel_uncore_write(uncore, GEN11_VCS2_VCS3_INTR_MASK, ~dmask);
>  	if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5))
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index aa2c0974b02c..fe09288a3145 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1529,6 +1529,10 @@
>  #define GEN11_GUNIT_CSME_INTR_MASK		_MMIO(0x1900f4)
>  #define GEN12_CCS0_CCS1_INTR_MASK		_MMIO(0x190100)
>  #define GEN12_CCS2_CCS3_INTR_MASK		_MMIO(0x190104)
> +#define XEHPC_BCS1_BCS2_INTR_MASK		_MMIO(0x190110)
> +#define XEHPC_BCS3_BCS4_INTR_MASK		_MMIO(0x190114)
> +#define XEHPC_BCS5_BCS6_INTR_MASK		_MMIO(0x190118)
> +#define XEHPC_BCS7_BCS8_INTR_MASK		_MMIO(0x19011c)
>  
>  #define GEN12_SFC_DONE(n)			_MMIO(0x1cc000 + (n) *
> 0x1000)
>  

[PATCH 09/11] drm/i915/pvc: Reset support for new copy engines

[Matt Roper]

This patch adds the reset support for new copy engines
in PVC.

Bspec: 52549
Original-author: CQ Tang
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c |  8 +++++
 drivers/gpu/drm/i915/gt/intel_gt_regs.h   | 44 +++++++++++++----------
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 4532c3ea9ace..c6e93db134b1 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -390,6 +390,14 @@ static u32 get_reset_domain(u8 ver, enum intel_engine_id id)
 		static const u32 engine_reset_domains[] = {
 			[RCS0]  = GEN11_GRDOM_RENDER,
 			[BCS0]  = GEN11_GRDOM_BLT,
+			[BCS1]  = XEHPC_GRDOM_BLT1,
+			[BCS2]  = XEHPC_GRDOM_BLT2,
+			[BCS3]  = XEHPC_GRDOM_BLT3,
+			[BCS4]  = XEHPC_GRDOM_BLT4,
+			[BCS5]  = XEHPC_GRDOM_BLT5,
+			[BCS6]  = XEHPC_GRDOM_BLT6,
+			[BCS7]  = XEHPC_GRDOM_BLT7,
+			[BCS8]  = XEHPC_GRDOM_BLT8,
 			[VCS0]  = GEN11_GRDOM_MEDIA,
 			[VCS1]  = GEN11_GRDOM_MEDIA2,
 			[VCS2]  = GEN11_GRDOM_MEDIA3,
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index fe09288a3145..98ede9c93f00 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -597,24 +597,32 @@
 /* GEN11 changed all bit defs except for FULL & RENDER */
 #define   GEN11_GRDOM_FULL			GEN6_GRDOM_FULL
 #define   GEN11_GRDOM_RENDER			GEN6_GRDOM_RENDER
-#define   GEN11_GRDOM_BLT			(1 << 2)
-#define   GEN11_GRDOM_GUC			(1 << 3)
-#define   GEN11_GRDOM_MEDIA			(1 << 5)
-#define   GEN11_GRDOM_MEDIA2			(1 << 6)
-#define   GEN11_GRDOM_MEDIA3			(1 << 7)
-#define   GEN11_GRDOM_MEDIA4			(1 << 8)
-#define   GEN11_GRDOM_MEDIA5			(1 << 9)
-#define   GEN11_GRDOM_MEDIA6			(1 << 10)
-#define   GEN11_GRDOM_MEDIA7			(1 << 11)
-#define   GEN11_GRDOM_MEDIA8			(1 << 12)
-#define   GEN11_GRDOM_VECS			(1 << 13)
-#define   GEN11_GRDOM_VECS2			(1 << 14)
-#define   GEN11_GRDOM_VECS3			(1 << 15)
-#define   GEN11_GRDOM_VECS4			(1 << 16)
-#define   GEN11_GRDOM_SFC0			(1 << 17)
-#define   GEN11_GRDOM_SFC1			(1 << 18)
-#define   GEN11_GRDOM_SFC2			(1 << 19)
-#define   GEN11_GRDOM_SFC3			(1 << 20)
+#define   XEHPC_GRDOM_BLT8			REG_BIT(31)
+#define   XEHPC_GRDOM_BLT7			REG_BIT(30)
+#define   XEHPC_GRDOM_BLT6			REG_BIT(29)
+#define   XEHPC_GRDOM_BLT5			REG_BIT(28)
+#define   XEHPC_GRDOM_BLT4			REG_BIT(27)
+#define   XEHPC_GRDOM_BLT3			REG_BIT(26)
+#define   XEHPC_GRDOM_BLT2			REG_BIT(25)
+#define   XEHPC_GRDOM_BLT1			REG_BIT(24)
+#define   GEN11_GRDOM_SFC3			REG_BIT(20)
+#define   GEN11_GRDOM_SFC2			REG_BIT(19)
+#define   GEN11_GRDOM_SFC1			REG_BIT(18)
+#define   GEN11_GRDOM_SFC0			REG_BIT(17)
+#define   GEN11_GRDOM_VECS4			REG_BIT(16)
+#define   GEN11_GRDOM_VECS3			REG_BIT(15)
+#define   GEN11_GRDOM_VECS2			REG_BIT(14)
+#define   GEN11_GRDOM_VECS			REG_BIT(13)
+#define   GEN11_GRDOM_MEDIA8			REG_BIT(12)
+#define   GEN11_GRDOM_MEDIA7			REG_BIT(11)
+#define   GEN11_GRDOM_MEDIA6			REG_BIT(10)
+#define   GEN11_GRDOM_MEDIA5			REG_BIT(9)
+#define   GEN11_GRDOM_MEDIA4			REG_BIT(8)
+#define   GEN11_GRDOM_MEDIA3			REG_BIT(7)
+#define   GEN11_GRDOM_MEDIA2			REG_BIT(6)
+#define   GEN11_GRDOM_MEDIA			REG_BIT(5)
+#define   GEN11_GRDOM_GUC			REG_BIT(3)
+#define   GEN11_GRDOM_BLT			REG_BIT(2)
 #define   GEN11_VCS_SFC_RESET_BIT(instance)	(GEN11_GRDOM_SFC0 << ((instance) >> 1))
 #define   GEN11_VECS_SFC_RESET_BIT(instance)	(GEN11_GRDOM_SFC0 << (instance))
 
-- 
2.35.1

Re: [Intel-gfx] [PATCH 09/11] drm/i915/pvc: Reset support for new copy engines

[Souza, Jose]

On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> This patch adds the reset support for new copy engines
> in PVC.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Bspec: 52549
> Original-author: CQ Tang
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c |  8 +++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h   | 44 +++++++++++++----------
>  2 files changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 4532c3ea9ace..c6e93db134b1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -390,6 +390,14 @@ static u32 get_reset_domain(u8 ver, enum intel_engine_id id)
>  		static const u32 engine_reset_domains[] = {
>  			[RCS0]  = GEN11_GRDOM_RENDER,
>  			[BCS0]  = GEN11_GRDOM_BLT,
> +			[BCS1]  = XEHPC_GRDOM_BLT1,
> +			[BCS2]  = XEHPC_GRDOM_BLT2,
> +			[BCS3]  = XEHPC_GRDOM_BLT3,
> +			[BCS4]  = XEHPC_GRDOM_BLT4,
> +			[BCS5]  = XEHPC_GRDOM_BLT5,
> +			[BCS6]  = XEHPC_GRDOM_BLT6,
> +			[BCS7]  = XEHPC_GRDOM_BLT7,
> +			[BCS8]  = XEHPC_GRDOM_BLT8,
>  			[VCS0]  = GEN11_GRDOM_MEDIA,
>  			[VCS1]  = GEN11_GRDOM_MEDIA2,
>  			[VCS2]  = GEN11_GRDOM_MEDIA3,
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index fe09288a3145..98ede9c93f00 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -597,24 +597,32 @@
>  /* GEN11 changed all bit defs except for FULL & RENDER */
>  #define   GEN11_GRDOM_FULL			GEN6_GRDOM_FULL
>  #define   GEN11_GRDOM_RENDER			GEN6_GRDOM_RENDER
> -#define   GEN11_GRDOM_BLT			(1 << 2)
> -#define   GEN11_GRDOM_GUC			(1 << 3)
> -#define   GEN11_GRDOM_MEDIA			(1 << 5)
> -#define   GEN11_GRDOM_MEDIA2			(1 << 6)
> -#define   GEN11_GRDOM_MEDIA3			(1 << 7)
> -#define   GEN11_GRDOM_MEDIA4			(1 << 8)
> -#define   GEN11_GRDOM_MEDIA5			(1 << 9)
> -#define   GEN11_GRDOM_MEDIA6			(1 << 10)
> -#define   GEN11_GRDOM_MEDIA7			(1 << 11)
> -#define   GEN11_GRDOM_MEDIA8			(1 << 12)
> -#define   GEN11_GRDOM_VECS			(1 << 13)
> -#define   GEN11_GRDOM_VECS2			(1 << 14)
> -#define   GEN11_GRDOM_VECS3			(1 << 15)
> -#define   GEN11_GRDOM_VECS4			(1 << 16)
> -#define   GEN11_GRDOM_SFC0			(1 << 17)
> -#define   GEN11_GRDOM_SFC1			(1 << 18)
> -#define   GEN11_GRDOM_SFC2			(1 << 19)
> -#define   GEN11_GRDOM_SFC3			(1 << 20)
> +#define   XEHPC_GRDOM_BLT8			REG_BIT(31)
> +#define   XEHPC_GRDOM_BLT7			REG_BIT(30)
> +#define   XEHPC_GRDOM_BLT6			REG_BIT(29)
> +#define   XEHPC_GRDOM_BLT5			REG_BIT(28)
> +#define   XEHPC_GRDOM_BLT4			REG_BIT(27)
> +#define   XEHPC_GRDOM_BLT3			REG_BIT(26)
> +#define   XEHPC_GRDOM_BLT2			REG_BIT(25)
> +#define   XEHPC_GRDOM_BLT1			REG_BIT(24)
> +#define   GEN11_GRDOM_SFC3			REG_BIT(20)
> +#define   GEN11_GRDOM_SFC2			REG_BIT(19)
> +#define   GEN11_GRDOM_SFC1			REG_BIT(18)
> +#define   GEN11_GRDOM_SFC0			REG_BIT(17)
> +#define   GEN11_GRDOM_VECS4			REG_BIT(16)
> +#define   GEN11_GRDOM_VECS3			REG_BIT(15)
> +#define   GEN11_GRDOM_VECS2			REG_BIT(14)
> +#define   GEN11_GRDOM_VECS			REG_BIT(13)
> +#define   GEN11_GRDOM_MEDIA8			REG_BIT(12)
> +#define   GEN11_GRDOM_MEDIA7			REG_BIT(11)
> +#define   GEN11_GRDOM_MEDIA6			REG_BIT(10)
> +#define   GEN11_GRDOM_MEDIA5			REG_BIT(9)
> +#define   GEN11_GRDOM_MEDIA4			REG_BIT(8)
> +#define   GEN11_GRDOM_MEDIA3			REG_BIT(7)
> +#define   GEN11_GRDOM_MEDIA2			REG_BIT(6)
> +#define   GEN11_GRDOM_MEDIA			REG_BIT(5)
> +#define   GEN11_GRDOM_GUC			REG_BIT(3)
> +#define   GEN11_GRDOM_BLT			REG_BIT(2)
>  #define   GEN11_VCS_SFC_RESET_BIT(instance)	(GEN11_GRDOM_SFC0 << ((instance) >> 1))
>  #define   GEN11_VECS_SFC_RESET_BIT(instance)	(GEN11_GRDOM_SFC0 << (instance))
>  

Re: [Intel-gfx] [PATCH 09/11] drm/i915/pvc: Reset support for new copy engines

[Summers, Stuart]

On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> This patch adds the reset support for new copy engines
> in PVC.
> 
> Bspec: 52549
> Original-author: CQ Tang
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Reviewed-by: Stuart Summers <stuart.summers@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c |  8 +++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h   | 44 +++++++++++++------
> ----
>  2 files changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 4532c3ea9ace..c6e93db134b1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -390,6 +390,14 @@ static u32 get_reset_domain(u8 ver, enum
> intel_engine_id id)
>  		static const u32 engine_reset_domains[] = {
>  			[RCS0]  = GEN11_GRDOM_RENDER,
>  			[BCS0]  = GEN11_GRDOM_BLT,
> +			[BCS1]  = XEHPC_GRDOM_BLT1,
> +			[BCS2]  = XEHPC_GRDOM_BLT2,
> +			[BCS3]  = XEHPC_GRDOM_BLT3,
> +			[BCS4]  = XEHPC_GRDOM_BLT4,
> +			[BCS5]  = XEHPC_GRDOM_BLT5,
> +			[BCS6]  = XEHPC_GRDOM_BLT6,
> +			[BCS7]  = XEHPC_GRDOM_BLT7,
> +			[BCS8]  = XEHPC_GRDOM_BLT8,
>  			[VCS0]  = GEN11_GRDOM_MEDIA,
>  			[VCS1]  = GEN11_GRDOM_MEDIA2,
>  			[VCS2]  = GEN11_GRDOM_MEDIA3,
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index fe09288a3145..98ede9c93f00 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -597,24 +597,32 @@
>  /* GEN11 changed all bit defs except for FULL & RENDER */
>  #define   GEN11_GRDOM_FULL			GEN6_GRDOM_FULL
>  #define   GEN11_GRDOM_RENDER			GEN6_GRDOM_RENDER
> -#define   GEN11_GRDOM_BLT			(1 << 2)
> -#define   GEN11_GRDOM_GUC			(1 << 3)
> -#define   GEN11_GRDOM_MEDIA			(1 << 5)
> -#define   GEN11_GRDOM_MEDIA2			(1 << 6)
> -#define   GEN11_GRDOM_MEDIA3			(1 << 7)
> -#define   GEN11_GRDOM_MEDIA4			(1 << 8)
> -#define   GEN11_GRDOM_MEDIA5			(1 << 9)
> -#define   GEN11_GRDOM_MEDIA6			(1 << 10)
> -#define   GEN11_GRDOM_MEDIA7			(1 << 11)
> -#define   GEN11_GRDOM_MEDIA8			(1 << 12)
> -#define   GEN11_GRDOM_VECS			(1 << 13)
> -#define   GEN11_GRDOM_VECS2			(1 << 14)
> -#define   GEN11_GRDOM_VECS3			(1 << 15)
> -#define   GEN11_GRDOM_VECS4			(1 << 16)
> -#define   GEN11_GRDOM_SFC0			(1 << 17)
> -#define   GEN11_GRDOM_SFC1			(1 << 18)
> -#define   GEN11_GRDOM_SFC2			(1 << 19)
> -#define   GEN11_GRDOM_SFC3			(1 << 20)
> +#define   XEHPC_GRDOM_BLT8			REG_BIT(31)
> +#define   XEHPC_GRDOM_BLT7			REG_BIT(30)
> +#define   XEHPC_GRDOM_BLT6			REG_BIT(29)
> +#define   XEHPC_GRDOM_BLT5			REG_BIT(28)
> +#define   XEHPC_GRDOM_BLT4			REG_BIT(27)
> +#define   XEHPC_GRDOM_BLT3			REG_BIT(26)
> +#define   XEHPC_GRDOM_BLT2			REG_BIT(25)
> +#define   XEHPC_GRDOM_BLT1			REG_BIT(24)
> +#define   GEN11_GRDOM_SFC3			REG_BIT(20)
> +#define   GEN11_GRDOM_SFC2			REG_BIT(19)
> +#define   GEN11_GRDOM_SFC1			REG_BIT(18)
> +#define   GEN11_GRDOM_SFC0			REG_BIT(17)
> +#define   GEN11_GRDOM_VECS4			REG_BIT(16)
> +#define   GEN11_GRDOM_VECS3			REG_BIT(15)
> +#define   GEN11_GRDOM_VECS2			REG_BIT(14)
> +#define   GEN11_GRDOM_VECS			REG_BIT(13)
> +#define   GEN11_GRDOM_MEDIA8			REG_BIT(12)
> +#define   GEN11_GRDOM_MEDIA7			REG_BIT(11)
> +#define   GEN11_GRDOM_MEDIA6			REG_BIT(10)
> +#define   GEN11_GRDOM_MEDIA5			REG_BIT(9)
> +#define   GEN11_GRDOM_MEDIA4			REG_BIT(8)
> +#define   GEN11_GRDOM_MEDIA3			REG_BIT(7)
> +#define   GEN11_GRDOM_MEDIA2			REG_BIT(6)
> +#define   GEN11_GRDOM_MEDIA			REG_BIT(5)
> +#define   GEN11_GRDOM_GUC			REG_BIT(3)
> +#define   GEN11_GRDOM_BLT			REG_BIT(2)
>  #define   GEN11_VCS_SFC_RESET_BIT(instance)	(GEN11_GRDOM_SFC0 <<
> ((instance) >> 1))
>  #define   GEN11_VECS_SFC_RESET_BIT(instance)	(GEN11_GRDOM_SFC0 <<
> (instance))
>  

[PATCH 10/11] drm/i915/pvc: skip all copy engines from aux table invalidate

[Matt Roper]

From: Lucas De Marchi <lucas.demarchi@intel.com>

As we have more copy engines now, mask all of them from aux table
invalidate.

Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index 0de17b568b41..f262aed94ef3 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -275,7 +275,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode)
 		if (!HAS_FLAT_CCS(rq->engine->i915) &&
 		    (rq->engine->class == VIDEO_DECODE_CLASS ||
 		     rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) {
-			aux_inv = rq->engine->mask & ~BIT(BCS0);
+			aux_inv = rq->engine->mask & ~GENMASK(BCS8, BCS0);
 			if (aux_inv)
 				cmd += 4;
 		}
-- 
2.35.1

Re: [Intel-gfx] [PATCH 10/11] drm/i915/pvc: skip all copy engines from aux table invalidate

[Souza, Jose]

On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> From: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> As we have more copy engines now, mask all of them from aux table
> invalidate.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 0de17b568b41..f262aed94ef3 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -275,7 +275,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode)
>  		if (!HAS_FLAT_CCS(rq->engine->i915) &&
>  		    (rq->engine->class == VIDEO_DECODE_CLASS ||
>  		     rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) {
> -			aux_inv = rq->engine->mask & ~BIT(BCS0);
> +			aux_inv = rq->engine->mask & ~GENMASK(BCS8, BCS0);
>  			if (aux_inv)
>  				cmd += 4;
>  		}

Re: [PATCH 10/11] drm/i915/pvc: skip all copy engines from aux table invalidate

[Kumar Valsan, Prathap]

On Mon, May 02, 2022 at 09:34:16AM -0700, Matt Roper wrote:
> From: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> As we have more copy engines now, mask all of them from aux table
> invalidate.
> 
> Cc: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 0de17b568b41..f262aed94ef3 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -275,7 +275,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode)
>  		if (!HAS_FLAT_CCS(rq->engine->i915) &&
>  		    (rq->engine->class == VIDEO_DECODE_CLASS ||
>  		     rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) {
> -			aux_inv = rq->engine->mask & ~BIT(BCS0);
> +			aux_inv = rq->engine->mask & ~GENMASK(BCS8, BCS0);
If we had defined I915_MAX_BCS earlier.
We use ~GENMASK(BCS0 + I915_MAX_BCS - 1, BCS0), so we don't need to
change this with the number of instances.

Otherwise looks good to me.

Reviewed-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
>  			if (aux_inv)
>  				cmd += 4;
>  		}
> -- 
> 2.35.1
> 

[PATCH 11/11] drm/i915/pvc: read fuses for link copy engines

[Matt Roper]

From: Lucas De Marchi <lucas.demarchi@intel.com>

The new Link Copy engines in PVC may be fused off according to the
mslice_mask. Each bit of the MEML3_EN_MASK we read from the
GEN10_MIRROR_FUSE3 register disables a pair of link copy engines.

Bspec: 44483
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 28 +++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index c6e93db134b1..d10cdeff5072 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -686,6 +686,33 @@ static void engine_mask_apply_compute_fuses(struct intel_gt *gt)
 	}
 }
 
+static void engine_mask_apply_copy_fuses(struct intel_gt *gt)
+{
+	struct drm_i915_private *i915 = gt->i915;
+	struct intel_gt_info *info = &gt->info;
+	unsigned long meml3_mask;
+	u8 quad;
+
+	meml3_mask = intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3);
+	meml3_mask = REG_FIELD_GET(GEN12_MEML3_EN_MASK, meml3_mask);
+
+	/*
+	 * Link Copy engines may be fused off according to meml3_mask. Each
+	 * bit is a quad that houses 2 Link Copy and two Sub Copy engines.
+	 */
+	for_each_clear_bit(quad, &meml3_mask, GEN12_MAX_MSLICES) {
+		intel_engine_mask_t mask = GENMASK(BCS1 + quad * 2 + 1,
+						   BCS1 + quad * 2);
+
+		if (mask & info->engine_mask) {
+			drm_dbg(&i915->drm, "bcs%u fused off\n", quad * 2 + 1);
+			drm_dbg(&i915->drm, "bcs%u fused off\n", quad * 2 + 2);
+
+			info->engine_mask &= ~mask;
+		}
+	}
+}
+
 /*
  * Determine which engines are fused off in our particular hardware.
  * Note that we have a catch-22 situation where we need to be able to access
@@ -768,6 +795,7 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt)
 	GEM_BUG_ON(vebox_mask != VEBOX_MASK(gt));
 
 	engine_mask_apply_compute_fuses(gt);
+	engine_mask_apply_copy_fuses(gt);
 
 	return info->engine_mask;
 }
-- 
2.35.1

Re: [PATCH 11/11] drm/i915/pvc: read fuses for link copy engines

[Souza, Jose]

On Mon, 2022-05-02 at 09:34 -0700, Matt Roper wrote:
> From: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> The new Link Copy engines in PVC may be fused off according to the
> mslice_mask. Each bit of the MEML3_EN_MASK we read from the
> GEN10_MIRROR_FUSE3 register disables a pair of link copy engines.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> Bspec: 44483
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c | 28 +++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index c6e93db134b1..d10cdeff5072 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -686,6 +686,33 @@ static void engine_mask_apply_compute_fuses(struct intel_gt *gt)
>  	}
>  }
>  
> +static void engine_mask_apply_copy_fuses(struct intel_gt *gt)
> +{
> +	struct drm_i915_private *i915 = gt->i915;
> +	struct intel_gt_info *info = &gt->info;
> +	unsigned long meml3_mask;
> +	u8 quad;
> +
> +	meml3_mask = intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3);
> +	meml3_mask = REG_FIELD_GET(GEN12_MEML3_EN_MASK, meml3_mask);
> +
> +	/*
> +	 * Link Copy engines may be fused off according to meml3_mask. Each
> +	 * bit is a quad that houses 2 Link Copy and two Sub Copy engines.
> +	 */
> +	for_each_clear_bit(quad, &meml3_mask, GEN12_MAX_MSLICES) {
> +		intel_engine_mask_t mask = GENMASK(BCS1 + quad * 2 + 1,
> +						   BCS1 + quad * 2);
> +
> +		if (mask & info->engine_mask) {
> +			drm_dbg(&i915->drm, "bcs%u fused off\n", quad * 2 + 1);
> +			drm_dbg(&i915->drm, "bcs%u fused off\n", quad * 2 + 2);
> +
> +			info->engine_mask &= ~mask;
> +		}
> +	}
> +}
> +
>  /*
>   * Determine which engines are fused off in our particular hardware.
>   * Note that we have a catch-22 situation where we need to be able to access
> @@ -768,6 +795,7 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt)
>  	GEM_BUG_ON(vebox_mask != VEBOX_MASK(gt));
>  
>  	engine_mask_apply_compute_fuses(gt);
> +	engine_mask_apply_copy_fuses(gt);
>  
>  	return info->engine_mask;
>  }

Re: [Intel-gfx] [PATCH 11/11] drm/i915/pvc: read fuses for link copy engines

[Tvrtko Ursulin]

On 02/05/2022 17:34, Matt Roper wrote:
> From: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> The new Link Copy engines in PVC may be fused off according to the
> mslice_mask. Each bit of the MEML3_EN_MASK we read from the
> GEN10_MIRROR_FUSE3 register disables a pair of link copy engines.
> 
> Bspec: 44483
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c | 28 +++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index c6e93db134b1..d10cdeff5072 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -686,6 +686,33 @@ static void engine_mask_apply_compute_fuses(struct intel_gt *gt)
>   	}
>   }
>   
> +static void engine_mask_apply_copy_fuses(struct intel_gt *gt)
> +{
> +	struct drm_i915_private *i915 = gt->i915;
> +	struct intel_gt_info *info = &gt->info;
> +	unsigned long meml3_mask;
> +	u8 quad;

Any hidden reason u8 is the right type here and not unsigned long like bitops expect? (Yes I did notice GEN12_MAX_MSLICES only goes to 4 but generally u8 sucks.)

> +
> +	meml3_mask = intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3);
> +	meml3_mask = REG_FIELD_GET(GEN12_MEML3_EN_MASK, meml3_mask);
> +
> +	/*
> +	 * Link Copy engines may be fused off according to meml3_mask. Each
> +	 * bit is a quad that houses 2 Link Copy and two Sub Copy engines.
> +	 */
> +	for_each_clear_bit(quad, &meml3_mask, GEN12_MAX_MSLICES) {
> +		intel_engine_mask_t mask = GENMASK(BCS1 + quad * 2 + 1,
> +						   BCS1 + quad * 2);

So internally we will be sure BCS1 to BCS9 are link copy engines? I mean enum names hardcoded/fixed to function. Should we have a comment to that effect somewhere? In intel_engine_types.h maybe?

> +
> +		if (mask & info->engine_mask) {
> +			drm_dbg(&i915->drm, "bcs%u fused off\n", quad * 2 + 1);
> +			drm_dbg(&i915->drm, "bcs%u fused off\n", quad * 2 + 2);

Bikeshed - I'd be tempted to decrease the amount of "quad * 2 + 1" by having a local variable.

   unsigned int instance = quad * 2 + 1;
   intel_engine_mask_t mask = GENMASK(_BCS(instance + 1), _BCS(instance));

Etc.

Regards,

Tvrtko

> +
> +			info->engine_mask &= ~mask;
> +		}
> +	}
> +}
> +
>   /*
>    * Determine which engines are fused off in our particular hardware.
>    * Note that we have a catch-22 situation where we need to be able to access
> @@ -768,6 +795,7 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt)
>   	GEM_BUG_ON(vebox_mask != VEBOX_MASK(gt));
>   
>   	engine_mask_apply_compute_fuses(gt);
> +	engine_mask_apply_copy_fuses(gt);
>   
>   	return info->engine_mask;
>   }

Re: [Intel-gfx] [PATCH 00/11] i915: Introduce Ponte Vecchio

[Tvrtko Ursulin]

On 02/05/2022 17:34, Matt Roper wrote:
> Ponte Vecchio (PVC) is a new GPU based on the Xe_HPC architecture.  As a
> compute-focused platform, PVC has compute engines and enhanced copy
> engines, but no render engine (there is no geometry pipeline) and no
> display.
> 
> This is just a handful of early enablement patches, including some
> initial support for the new copy engines (although we're not yet adding
> those to the platform's engine list or exposing them to userspace just
> yet).

IMO lets hold off merging this until next week if that sounds 
acceptable? This week I need to do a final pull for final bits of DG2 
and I would like to keep it as small as possible.

Regards,

Tvrtko

> 
> Ayaz A Siddiqui (1):
>    drm/i915/pvc: Define MOCS table for PVC
> 
> John Harrison (1):
>    drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter
>      engine
> 
> Lucas De Marchi (2):
>    drm/i915/pvc: skip all copy engines from aux table invalidate
>    drm/i915/pvc: read fuses for link copy engines
> 
> Matt Roper (5):
>    drm/i915/pvc: Add forcewake support
>    drm/i915/pvc: Read correct RP_STATE_CAP register
>    drm/i915/pvc: Engines definitions for new copy engines
>    drm/i915/pvc: Interrupt support for new copy engines
>    drm/i915/pvc: Reset support for new copy engines
> 
> Stuart Summers (2):
>    drm/i915/pvc: add initial Ponte Vecchio definitions
>    drm/i915/pvc: Remove additional 3D flags from PIPE_CONTROL
> 
>   drivers/gpu/drm/i915/gt/gen8_engine_cs.c      |  20 ++-
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  92 +++++++++++
>   drivers/gpu/drm/i915/gt/intel_engine_types.h  |  10 +-
>   drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  12 +-
>   drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  16 ++
>   drivers/gpu/drm/i915/gt/intel_gt_regs.h       |  56 ++++---
>   drivers/gpu/drm/i915/gt/intel_gt_types.h      |   1 +
>   drivers/gpu/drm/i915/gt/intel_mocs.c          |  24 ++-
>   drivers/gpu/drm/i915/gt/intel_rps.c           |   4 +-
>   drivers/gpu/drm/i915/gt/intel_workarounds.c   |  13 +-
>   drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |   9 +-
>   drivers/gpu/drm/i915/gvt/cmd_parser.c         |   2 +-
>   drivers/gpu/drm/i915/i915_drv.h               |   6 +
>   drivers/gpu/drm/i915/i915_pci.c               |  23 +++
>   drivers/gpu/drm/i915/i915_reg.h               |   9 ++
>   drivers/gpu/drm/i915/intel_device_info.c      |   1 +
>   drivers/gpu/drm/i915/intel_device_info.h      |   5 +-
>   drivers/gpu/drm/i915/intel_uncore.c           | 150 +++++++++++++++++-
>   drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +
>   19 files changed, 417 insertions(+), 38 deletions(-)
> 

Re: [Intel-gfx] [PATCH 00/11] i915: Introduce Ponte Vecchio

[Matt Roper]

On Tue, May 03, 2022 at 09:21:04AM +0100, Tvrtko Ursulin wrote:
> 
> On 02/05/2022 17:34, Matt Roper wrote:
> > Ponte Vecchio (PVC) is a new GPU based on the Xe_HPC architecture.  As a
> > compute-focused platform, PVC has compute engines and enhanced copy
> > engines, but no render engine (there is no geometry pipeline) and no
> > display.
> > 
> > This is just a handful of early enablement patches, including some
> > initial support for the new copy engines (although we're not yet adding
> > those to the platform's engine list or exposing them to userspace just
> > yet).
> 
> IMO lets hold off merging this until next week if that sounds acceptable?
> This week I need to do a final pull for final bits of DG2 and I would like
> to keep it as small as possible.

I was wondering if we should try to apply patch #1 quickly, just to get
the IS_PONTEVECCHIO definition into the tree early and minimize the
cross-tree merge hassles down the road?  Although I guess PVC might not
be as big a problem as some platforms since no display means that we
won't have a huge split of patches between -next and -gt-next that both
need the basic defines present.


Matt

> 
> Regards,
> 
> Tvrtko
> 
> > 
> > Ayaz A Siddiqui (1):
> >    drm/i915/pvc: Define MOCS table for PVC
> > 
> > John Harrison (1):
> >    drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter
> >      engine
> > 
> > Lucas De Marchi (2):
> >    drm/i915/pvc: skip all copy engines from aux table invalidate
> >    drm/i915/pvc: read fuses for link copy engines
> > 
> > Matt Roper (5):
> >    drm/i915/pvc: Add forcewake support
> >    drm/i915/pvc: Read correct RP_STATE_CAP register
> >    drm/i915/pvc: Engines definitions for new copy engines
> >    drm/i915/pvc: Interrupt support for new copy engines
> >    drm/i915/pvc: Reset support for new copy engines
> > 
> > Stuart Summers (2):
> >    drm/i915/pvc: add initial Ponte Vecchio definitions
> >    drm/i915/pvc: Remove additional 3D flags from PIPE_CONTROL
> > 
> >   drivers/gpu/drm/i915/gt/gen8_engine_cs.c      |  20 ++-
> >   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  92 +++++++++++
> >   drivers/gpu/drm/i915/gt/intel_engine_types.h  |  10 +-
> >   drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  12 +-
> >   drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  16 ++
> >   drivers/gpu/drm/i915/gt/intel_gt_regs.h       |  56 ++++---
> >   drivers/gpu/drm/i915/gt/intel_gt_types.h      |   1 +
> >   drivers/gpu/drm/i915/gt/intel_mocs.c          |  24 ++-
> >   drivers/gpu/drm/i915/gt/intel_rps.c           |   4 +-
> >   drivers/gpu/drm/i915/gt/intel_workarounds.c   |  13 +-
> >   drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |   9 +-
> >   drivers/gpu/drm/i915/gvt/cmd_parser.c         |   2 +-
> >   drivers/gpu/drm/i915/i915_drv.h               |   6 +
> >   drivers/gpu/drm/i915/i915_pci.c               |  23 +++
> >   drivers/gpu/drm/i915/i915_reg.h               |   9 ++
> >   drivers/gpu/drm/i915/intel_device_info.c      |   1 +
> >   drivers/gpu/drm/i915/intel_device_info.h      |   5 +-
> >   drivers/gpu/drm/i915/intel_uncore.c           | 150 +++++++++++++++++-
> >   drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +
> >   19 files changed, 417 insertions(+), 38 deletions(-)
> > 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

Re: [Intel-gfx] [PATCH 00/11] i915: Introduce Ponte Vecchio

[Tvrtko Ursulin]

On 03/05/2022 15:56, Matt Roper wrote:
> On Tue, May 03, 2022 at 09:21:04AM +0100, Tvrtko Ursulin wrote:
>>
>> On 02/05/2022 17:34, Matt Roper wrote:
>>> Ponte Vecchio (PVC) is a new GPU based on the Xe_HPC architecture.  As a
>>> compute-focused platform, PVC has compute engines and enhanced copy
>>> engines, but no render engine (there is no geometry pipeline) and no
>>> display.
>>>
>>> This is just a handful of early enablement patches, including some
>>> initial support for the new copy engines (although we're not yet adding
>>> those to the platform's engine list or exposing them to userspace just
>>> yet).
>>
>> IMO lets hold off merging this until next week if that sounds acceptable?
>> This week I need to do a final pull for final bits of DG2 and I would like
>> to keep it as small as possible.
> 
> I was wondering if we should try to apply patch #1 quickly, just to get
> the IS_PONTEVECCHIO definition into the tree early and minimize the
> cross-tree merge hassles down the road?  Although I guess PVC might not
> be as big a problem as some platforms since no display means that we
> won't have a huge split of patches between -next and -gt-next that both
> need the basic defines present.

Okay, that sounds fair.

Regards,

Tvrtko

> 
> 
> Matt
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>>
>>> Ayaz A Siddiqui (1):
>>>     drm/i915/pvc: Define MOCS table for PVC
>>>
>>> John Harrison (1):
>>>     drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter
>>>       engine
>>>
>>> Lucas De Marchi (2):
>>>     drm/i915/pvc: skip all copy engines from aux table invalidate
>>>     drm/i915/pvc: read fuses for link copy engines
>>>
>>> Matt Roper (5):
>>>     drm/i915/pvc: Add forcewake support
>>>     drm/i915/pvc: Read correct RP_STATE_CAP register
>>>     drm/i915/pvc: Engines definitions for new copy engines
>>>     drm/i915/pvc: Interrupt support for new copy engines
>>>     drm/i915/pvc: Reset support for new copy engines
>>>
>>> Stuart Summers (2):
>>>     drm/i915/pvc: add initial Ponte Vecchio definitions
>>>     drm/i915/pvc: Remove additional 3D flags from PIPE_CONTROL
>>>
>>>    drivers/gpu/drm/i915/gt/gen8_engine_cs.c      |  20 ++-
>>>    drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  92 +++++++++++
>>>    drivers/gpu/drm/i915/gt/intel_engine_types.h  |  10 +-
>>>    drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  12 +-
>>>    drivers/gpu/drm/i915/gt/intel_gt_irq.c        |  16 ++
>>>    drivers/gpu/drm/i915/gt/intel_gt_regs.h       |  56 ++++---
>>>    drivers/gpu/drm/i915/gt/intel_gt_types.h      |   1 +
>>>    drivers/gpu/drm/i915/gt/intel_mocs.c          |  24 ++-
>>>    drivers/gpu/drm/i915/gt/intel_rps.c           |   4 +-
>>>    drivers/gpu/drm/i915/gt/intel_workarounds.c   |  13 +-
>>>    drivers/gpu/drm/i915/gt/selftest_hangcheck.c  |   9 +-
>>>    drivers/gpu/drm/i915/gvt/cmd_parser.c         |   2 +-
>>>    drivers/gpu/drm/i915/i915_drv.h               |   6 +
>>>    drivers/gpu/drm/i915/i915_pci.c               |  23 +++
>>>    drivers/gpu/drm/i915/i915_reg.h               |   9 ++
>>>    drivers/gpu/drm/i915/intel_device_info.c      |   1 +
>>>    drivers/gpu/drm/i915/intel_device_info.h      |   5 +-
>>>    drivers/gpu/drm/i915/intel_uncore.c           | 150 +++++++++++++++++-
>>>    drivers/gpu/drm/i915/selftests/intel_uncore.c |   2 +
>>>    19 files changed, 417 insertions(+), 38 deletions(-)
>>>
>