[PATCH v2 0/4] Implement Wa_14022698537

[PATCH v2 0/4] Implement Wa_14022698537

[Raag Jadav]

This series implements Wa_14022698537 along with its prerequisites.
Detailed description in commit message.

v2: Introduce DG2_WA subplatform for workaround (Jani)
    Fix Wa_ID and include it in subject (Badal)
    Rephrase commit message (Jani)
    Move CPU whitelist to intel_wa_cpu.c
v1: https://patchwork.freedesktop.org/series/139628/

Raag Jadav (4):
  drm/i915/pciids: Refactor DG2 PCI IDs into workaround ranges
  drm/i915/dg2: Introduce DG2_WA subplatform
  drm/i915/wa: Introduce intel_wa_cpu.c for CPU specific workarounds
  drm/i915/dg2: Implement Wa_14022698537

 drivers/gpu/drm/i915/Makefile               |  1 +
 drivers/gpu/drm/i915/gt/intel_wa_cpu.c      | 34 +++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 +++++++++++
 drivers/gpu/drm/i915/gt/intel_workarounds.h |  2 ++
 drivers/gpu/drm/i915/i915_drv.h             |  2 ++
 drivers/gpu/drm/i915/i915_reg.h             |  1 +
 drivers/gpu/drm/i915/intel_device_info.c    | 34 +++++++++++++++------
 drivers/gpu/drm/i915/intel_device_info.h    |  5 ++-
 include/drm/intel/i915_pciids.h             | 34 +++++++++++++++------
 9 files changed, 111 insertions(+), 20 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c

-- 
2.34.1

[PATCH v2 1/4] drm/i915/pciids: Refactor DG2 PCI IDs into workaround ranges

[Raag Jadav]

Refactor DG2 PCI IDs into device ranges that will be used in a workaround.

Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
 include/drm/intel/i915_pciids.h | 34 +++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/include/drm/intel/i915_pciids.h b/include/drm/intel/i915_pciids.h
index 2bf03ebfcf73..82f960f625c7 100644
--- a/include/drm/intel/i915_pciids.h
+++ b/include/drm/intel/i915_pciids.h
@@ -724,37 +724,51 @@
 	MACRO__(0xA7AB, ## __VA_ARGS__)
 
 /* DG2 */
+#define INTEL_DG2_G10_WA_IDS(MACRO__, ...) \
+	MACRO__(0x56A0, ## __VA_ARGS__), \
+	MACRO__(0x56A1, ## __VA_ARGS__), \
+	MACRO__(0x56A2, ## __VA_ARGS__)
+
 #define INTEL_DG2_G10_IDS(MACRO__, ...) \
+	INTEL_DG2_G10_WA_IDS(MACRO__, ## __VA_ARGS__), \
 	MACRO__(0x5690, ## __VA_ARGS__), \
 	MACRO__(0x5691, ## __VA_ARGS__), \
 	MACRO__(0x5692, ## __VA_ARGS__), \
-	MACRO__(0x56A0, ## __VA_ARGS__), \
-	MACRO__(0x56A1, ## __VA_ARGS__), \
-	MACRO__(0x56A2, ## __VA_ARGS__), \
 	MACRO__(0x56BE, ## __VA_ARGS__), \
 	MACRO__(0x56BF, ## __VA_ARGS__)
 
+#define INTEL_DG2_G11_WA_IDS(MACRO__, ...) \
+	MACRO__(0x56A5, ## __VA_ARGS__), \
+	MACRO__(0x56A6, ## __VA_ARGS__), \
+	MACRO__(0x56B0, ## __VA_ARGS__), \
+	MACRO__(0x56B1, ## __VA_ARGS__)
+
 #define INTEL_DG2_G11_IDS(MACRO__, ...) \
+	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
 	MACRO__(0x5693, ## __VA_ARGS__), \
 	MACRO__(0x5694, ## __VA_ARGS__), \
 	MACRO__(0x5695, ## __VA_ARGS__), \
-	MACRO__(0x56A5, ## __VA_ARGS__), \
-	MACRO__(0x56A6, ## __VA_ARGS__), \
-	MACRO__(0x56B0, ## __VA_ARGS__), \
-	MACRO__(0x56B1, ## __VA_ARGS__), \
 	MACRO__(0x56BA, ## __VA_ARGS__), \
 	MACRO__(0x56BB, ## __VA_ARGS__), \
 	MACRO__(0x56BC, ## __VA_ARGS__), \
 	MACRO__(0x56BD, ## __VA_ARGS__)
 
-#define INTEL_DG2_G12_IDS(MACRO__, ...) \
-	MACRO__(0x5696, ## __VA_ARGS__), \
-	MACRO__(0x5697, ## __VA_ARGS__), \
+#define INTEL_DG2_G12_WA_IDS(MACRO__, ...) \
 	MACRO__(0x56A3, ## __VA_ARGS__), \
 	MACRO__(0x56A4, ## __VA_ARGS__), \
 	MACRO__(0x56B2, ## __VA_ARGS__), \
 	MACRO__(0x56B3, ## __VA_ARGS__)
 
+#define INTEL_DG2_G12_IDS(MACRO__, ...) \
+	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	MACRO__(0x5696, ## __VA_ARGS__), \
+	MACRO__(0x5697, ## __VA_ARGS__)
+
+#define INTEL_DG2_WA_IDS(MACRO__, ...) \
+	INTEL_DG2_G10_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	INTEL_DG2_G12_WA_IDS(MACRO__, ## __VA_ARGS__)
+
 #define INTEL_DG2_IDS(MACRO__, ...) \
 	INTEL_DG2_G10_IDS(MACRO__, ## __VA_ARGS__), \
 	INTEL_DG2_G11_IDS(MACRO__, ## __VA_ARGS__), \
-- 
2.34.1

[PATCH v2 2/4] drm/i915/dg2: Introduce DG2_WA subplatform

[Raag Jadav]

Introduce DG2_WA subplatform for the devices that will be used in a
workaround and span across multiple DG2 subplatforms.

Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  2 ++
 drivers/gpu/drm/i915/intel_device_info.c | 34 +++++++++++++++++-------
 drivers/gpu/drm/i915/intel_device_info.h |  5 +++-
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 39f6614a0a99..0a68cd9379e8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -548,6 +548,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_G11)
 #define IS_DG2_G12(i915) \
 	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_G12)
+#define IS_DG2_WA(i915) \
+	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_WA)
 #define IS_RAPTORLAKE_S(i915) \
 	IS_SUBPLATFORM(i915, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL)
 #define IS_ALDERLAKE_P_N(i915) \
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 3c47c625993e..674ab2a4d75e 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -200,6 +200,15 @@ static const u16 subplatform_g12_ids[] = {
 	INTEL_DG2_G12_IDS(ID),
 };
 
+static const u16 subplatform_dg2_wa_ids[] = {
+	INTEL_DG2_WA_IDS(ID),
+};
+
+static const u16 subplatform_dg2_ids[] = {
+	INTEL_DG2_IDS(ID),
+	INTEL_ATS_M_IDS(ID),
+};
+
 static const u16 subplatform_arl_ids[] = {
 	INTEL_ARL_IDS(ID),
 };
@@ -252,15 +261,22 @@ static void intel_device_info_subplatform_init(struct drm_i915_private *i915)
 		if (find_devid(devid, subplatform_rplu_ids,
 			       ARRAY_SIZE(subplatform_rplu_ids)))
 			mask |= BIT(INTEL_SUBPLATFORM_RPLU);
-	} else if (find_devid(devid, subplatform_g10_ids,
-			      ARRAY_SIZE(subplatform_g10_ids))) {
-		mask = BIT(INTEL_SUBPLATFORM_G10);
-	} else if (find_devid(devid, subplatform_g11_ids,
-			      ARRAY_SIZE(subplatform_g11_ids))) {
-		mask = BIT(INTEL_SUBPLATFORM_G11);
-	} else if (find_devid(devid, subplatform_g12_ids,
-			      ARRAY_SIZE(subplatform_g12_ids))) {
-		mask = BIT(INTEL_SUBPLATFORM_G12);
+	} else if (find_devid(devid, subplatform_dg2_ids,
+			      ARRAY_SIZE(subplatform_dg2_ids))) {
+		if (find_devid(devid, subplatform_g10_ids,
+			       ARRAY_SIZE(subplatform_g10_ids)))
+			mask = BIT(INTEL_SUBPLATFORM_G10);
+		else if (find_devid(devid, subplatform_g11_ids,
+				    ARRAY_SIZE(subplatform_g11_ids)))
+			mask = BIT(INTEL_SUBPLATFORM_G11);
+		else if (find_devid(devid, subplatform_g12_ids,
+				    ARRAY_SIZE(subplatform_g12_ids)))
+			mask = BIT(INTEL_SUBPLATFORM_G12);
+
+		/* DG2 WA ids span across multiple subplatforms */
+		if (find_devid(devid, subplatform_dg2_wa_ids,
+			       ARRAY_SIZE(subplatform_dg2_wa_ids)))
+			mask |= BIT(INTEL_SUBPLATFORM_WA);
 	} else if (find_devid(devid, subplatform_arl_ids,
 			      ARRAY_SIZE(subplatform_arl_ids))) {
 		mask = BIT(INTEL_SUBPLATFORM_ARL);
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 643ff1bf74ee..c3623e859c78 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -95,9 +95,11 @@ enum intel_platform {
 /*
  * Subplatform bits share the same namespace per parent platform. In other words
  * it is fine for the same bit to be used on multiple parent platforms.
+ * Devices can belong to multiple subplatforms if needed, so it's possible to set
+ * multiple bits for same device.
  */
 
-#define INTEL_SUBPLATFORM_BITS (3)
+#define INTEL_SUBPLATFORM_BITS (4)
 #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1)
 
 /* HSW/BDW/SKL/KBL/CFL */
@@ -114,6 +116,7 @@ enum intel_platform {
 #define INTEL_SUBPLATFORM_G10	0
 #define INTEL_SUBPLATFORM_G11	1
 #define INTEL_SUBPLATFORM_G12	2
+#define INTEL_SUBPLATFORM_WA	3
 
 /* ADL */
 #define INTEL_SUBPLATFORM_RPL	0
-- 
2.34.1

[PATCH v2 3/4] drm/i915/wa: Introduce intel_wa_cpu.c for CPU specific workarounds

[Raag Jadav]

Having similar naming convention in intel-family.h and intel_device_info.h
results in redefinition of a few platforms. Define CPU IDs in its own file
to avoid this.

Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
 drivers/gpu/drm/i915/Makefile               |  1 +
 drivers/gpu/drm/i915/gt/intel_wa_cpu.c      | 34 +++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_workarounds.h |  2 ++
 3 files changed, 37 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c63fa2133ccb..1f9b503ab976 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -121,6 +121,7 @@ gt-y += \
 	gt/intel_timeline.o \
 	gt/intel_tlb.o \
 	gt/intel_wopcm.o \
+	gt/intel_wa_cpu.o \
 	gt/intel_workarounds.o \
 	gt/shmem_utils.o \
 	gt/sysfs_engines.o
diff --git a/drivers/gpu/drm/i915/gt/intel_wa_cpu.c b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c
new file mode 100644
index 000000000000..cbdab13e9db6
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2024 Intel Corporation
+ *
+ * This file is introduced to avoid platform redefinition from
+ * intel_device_info.h :(
+ */
+
+#include "intel_workarounds.h"
+
+#ifdef CONFIG_X86
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+
+static const struct x86_cpu_id wa_cpu_ids[] = {
+	X86_MATCH_VFM(INTEL_ALDERLAKE,		NULL),
+	X86_MATCH_VFM(INTEL_ALDERLAKE_L,	NULL),
+	X86_MATCH_VFM(INTEL_COMETLAKE,		NULL),
+	X86_MATCH_VFM(INTEL_KABYLAKE,		NULL),
+	X86_MATCH_VFM(INTEL_KABYLAKE_L,		NULL),
+	X86_MATCH_VFM(INTEL_RAPTORLAKE,		NULL),
+	X86_MATCH_VFM(INTEL_RAPTORLAKE_P,	NULL),
+	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,	NULL),
+	X86_MATCH_VFM(INTEL_ROCKETLAKE,		NULL),
+	{}
+};
+
+bool intel_match_wa_cpu(void)
+{
+	return x86_match_cpu(wa_cpu_ids);
+}
+#else
+bool intel_match_wa_cpu(void) { return false; }
+#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h
index 9beaab77c7f0..12f24fb31363 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.h
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h
@@ -21,6 +21,8 @@ static inline void intel_wa_list_free(struct i915_wa_list *wal)
 	memset(wal, 0, sizeof(*wal));
 }
 
+bool intel_match_wa_cpu(void);
+
 void intel_engine_init_ctx_wa(struct intel_engine_cs *engine);
 int intel_engine_emit_ctx_wa(struct i915_request *rq);
 
-- 
2.34.1

[PATCH v2 4/4] drm/i915/dg2: Implement Wa_14022698537

[Raag Jadav]

G8 power state entry is disabled due to a limitation on DG2, so we
enable it from driver with Wa_14022698537. Fow now we enable it for
all DG2 devices with the exception of a few, for which, we enable
only when paired with whitelisted CPU models.

v2: Fix Wa_ID and include it in subject (Badal)
    Rephrase commit message (Jani)

Signed-off-by: Raag Jadav <raag.jadav@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h             |  1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index e539a656cfc3..bcd7630c1631 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -14,6 +14,7 @@
 #include "intel_gt_mcr.h"
 #include "intel_gt_print.h"
 #include "intel_gt_regs.h"
+#include "intel_pcode.h"
 #include "intel_ring.h"
 #include "intel_workarounds.h"
 
@@ -1770,9 +1771,26 @@ static void wa_list_apply(const struct i915_wa_list *wal)
 	intel_gt_mcr_unlock(gt, flags);
 }
 
+/* Wa_14022698537:dg2 */
+static void intel_enable_g8(struct intel_uncore *uncore)
+{
+	struct drm_i915_private *i915 = uncore->i915;
+
+	if (IS_DG2(i915)) {
+		if (IS_DG2_WA(i915) && !intel_match_wa_cpu())
+			return;
+
+		snb_pcode_write_p(uncore, PCODE_POWER_SETUP,
+				  POWER_SETUP_SUBCOMMAND_G8_ENABLE, 0, 0);
+	}
+}
+
 void intel_gt_apply_workarounds(struct intel_gt *gt)
 {
 	wa_list_apply(&gt->wa_list);
+
+	/* Special case for pcode mailbox which can't be on wa_list */
+	intel_enable_g8(gt->uncore);
 }
 
 static bool wa_list_verify(struct intel_gt *gt,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 41f4350a7c6c..e948b194550c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3568,6 +3568,7 @@
 #define   PCODE_POWER_SETUP			0x7C
 #define     POWER_SETUP_SUBCOMMAND_READ_I1	0x4
 #define     POWER_SETUP_SUBCOMMAND_WRITE_I1	0x5
+#define     POWER_SETUP_SUBCOMMAND_G8_ENABLE	0x6
 #define	    POWER_SETUP_I1_WATTS		REG_BIT(31)
 #define	    POWER_SETUP_I1_SHIFT		6	/* 10.6 fixed point format */
 #define	    POWER_SETUP_I1_DATA_MASK		REG_GENMASK(15, 0)
-- 
2.34.1

Re: [PATCH v2 1/4] drm/i915/pciids: Refactor DG2 PCI IDs into workaround ranges

[Jani Nikula]

On Fri, 11 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> Refactor DG2 PCI IDs into device ranges that will be used in a workaround.

Give the PCI ID ranges a name other than "WA". What are they?

BR,
Jani.

>
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>  include/drm/intel/i915_pciids.h | 34 +++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/include/drm/intel/i915_pciids.h b/include/drm/intel/i915_pciids.h
> index 2bf03ebfcf73..82f960f625c7 100644
> --- a/include/drm/intel/i915_pciids.h
> +++ b/include/drm/intel/i915_pciids.h
> @@ -724,37 +724,51 @@
>  	MACRO__(0xA7AB, ## __VA_ARGS__)
>  
>  /* DG2 */
> +#define INTEL_DG2_G10_WA_IDS(MACRO__, ...) \
> +	MACRO__(0x56A0, ## __VA_ARGS__), \
> +	MACRO__(0x56A1, ## __VA_ARGS__), \
> +	MACRO__(0x56A2, ## __VA_ARGS__)
> +
>  #define INTEL_DG2_G10_IDS(MACRO__, ...) \
> +	INTEL_DG2_G10_WA_IDS(MACRO__, ## __VA_ARGS__), \
>  	MACRO__(0x5690, ## __VA_ARGS__), \
>  	MACRO__(0x5691, ## __VA_ARGS__), \
>  	MACRO__(0x5692, ## __VA_ARGS__), \
> -	MACRO__(0x56A0, ## __VA_ARGS__), \
> -	MACRO__(0x56A1, ## __VA_ARGS__), \
> -	MACRO__(0x56A2, ## __VA_ARGS__), \
>  	MACRO__(0x56BE, ## __VA_ARGS__), \
>  	MACRO__(0x56BF, ## __VA_ARGS__)
>  
> +#define INTEL_DG2_G11_WA_IDS(MACRO__, ...) \
> +	MACRO__(0x56A5, ## __VA_ARGS__), \
> +	MACRO__(0x56A6, ## __VA_ARGS__), \
> +	MACRO__(0x56B0, ## __VA_ARGS__), \
> +	MACRO__(0x56B1, ## __VA_ARGS__)
> +
>  #define INTEL_DG2_G11_IDS(MACRO__, ...) \
> +	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
>  	MACRO__(0x5693, ## __VA_ARGS__), \
>  	MACRO__(0x5694, ## __VA_ARGS__), \
>  	MACRO__(0x5695, ## __VA_ARGS__), \
> -	MACRO__(0x56A5, ## __VA_ARGS__), \
> -	MACRO__(0x56A6, ## __VA_ARGS__), \
> -	MACRO__(0x56B0, ## __VA_ARGS__), \
> -	MACRO__(0x56B1, ## __VA_ARGS__), \
>  	MACRO__(0x56BA, ## __VA_ARGS__), \
>  	MACRO__(0x56BB, ## __VA_ARGS__), \
>  	MACRO__(0x56BC, ## __VA_ARGS__), \
>  	MACRO__(0x56BD, ## __VA_ARGS__)
>  
> -#define INTEL_DG2_G12_IDS(MACRO__, ...) \
> -	MACRO__(0x5696, ## __VA_ARGS__), \
> -	MACRO__(0x5697, ## __VA_ARGS__), \
> +#define INTEL_DG2_G12_WA_IDS(MACRO__, ...) \
>  	MACRO__(0x56A3, ## __VA_ARGS__), \
>  	MACRO__(0x56A4, ## __VA_ARGS__), \
>  	MACRO__(0x56B2, ## __VA_ARGS__), \
>  	MACRO__(0x56B3, ## __VA_ARGS__)
>  
> +#define INTEL_DG2_G12_IDS(MACRO__, ...) \
> +	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
> +	MACRO__(0x5696, ## __VA_ARGS__), \
> +	MACRO__(0x5697, ## __VA_ARGS__)
> +
> +#define INTEL_DG2_WA_IDS(MACRO__, ...) \
> +	INTEL_DG2_G10_WA_IDS(MACRO__, ## __VA_ARGS__), \
> +	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
> +	INTEL_DG2_G12_WA_IDS(MACRO__, ## __VA_ARGS__)
> +
>  #define INTEL_DG2_IDS(MACRO__, ...) \
>  	INTEL_DG2_G10_IDS(MACRO__, ## __VA_ARGS__), \
>  	INTEL_DG2_G11_IDS(MACRO__, ## __VA_ARGS__), \

-- 
Jani Nikula, Intel

Re: [PATCH v2 2/4] drm/i915/dg2: Introduce DG2_WA subplatform

[Jani Nikula]

On Fri, 11 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> Introduce DG2_WA subplatform for the devices that will be used in a
> workaround and span across multiple DG2 subplatforms.

Ditto, give the subplatform a name other than "WA". Look up the specs,
what's in common?

> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  2 ++
>  drivers/gpu/drm/i915/intel_device_info.c | 34 +++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_device_info.h |  5 +++-
>  3 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 39f6614a0a99..0a68cd9379e8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -548,6 +548,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_G11)
>  #define IS_DG2_G12(i915) \
>  	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_G12)
> +#define IS_DG2_WA(i915) \
> +	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_WA)
>  #define IS_RAPTORLAKE_S(i915) \
>  	IS_SUBPLATFORM(i915, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL)
>  #define IS_ALDERLAKE_P_N(i915) \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 3c47c625993e..674ab2a4d75e 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -200,6 +200,15 @@ static const u16 subplatform_g12_ids[] = {
>  	INTEL_DG2_G12_IDS(ID),
>  };
>  
> +static const u16 subplatform_dg2_wa_ids[] = {
> +	INTEL_DG2_WA_IDS(ID),
> +};
> +
> +static const u16 subplatform_dg2_ids[] = {
> +	INTEL_DG2_IDS(ID),
> +	INTEL_ATS_M_IDS(ID),
> +};

Nope, you don't need this.

> +
>  static const u16 subplatform_arl_ids[] = {
>  	INTEL_ARL_IDS(ID),
>  };
> @@ -252,15 +261,22 @@ static void intel_device_info_subplatform_init(struct drm_i915_private *i915)
>  		if (find_devid(devid, subplatform_rplu_ids,
>  			       ARRAY_SIZE(subplatform_rplu_ids)))
>  			mask |= BIT(INTEL_SUBPLATFORM_RPLU);
> -	} else if (find_devid(devid, subplatform_g10_ids,
> -			      ARRAY_SIZE(subplatform_g10_ids))) {
> -		mask = BIT(INTEL_SUBPLATFORM_G10);
> -	} else if (find_devid(devid, subplatform_g11_ids,
> -			      ARRAY_SIZE(subplatform_g11_ids))) {
> -		mask = BIT(INTEL_SUBPLATFORM_G11);
> -	} else if (find_devid(devid, subplatform_g12_ids,
> -			      ARRAY_SIZE(subplatform_g12_ids))) {
> -		mask = BIT(INTEL_SUBPLATFORM_G12);
> +	} else if (find_devid(devid, subplatform_dg2_ids,
> +			      ARRAY_SIZE(subplatform_dg2_ids))) {

Nope. Just hoist the below checks to higher level.

> +		if (find_devid(devid, subplatform_g10_ids,
> +			       ARRAY_SIZE(subplatform_g10_ids)))
> +			mask = BIT(INTEL_SUBPLATFORM_G10);
> +		else if (find_devid(devid, subplatform_g11_ids,
> +				    ARRAY_SIZE(subplatform_g11_ids)))
> +			mask = BIT(INTEL_SUBPLATFORM_G11);
> +		else if (find_devid(devid, subplatform_g12_ids,
> +				    ARRAY_SIZE(subplatform_g12_ids)))
> +			mask = BIT(INTEL_SUBPLATFORM_G12);
> +
> +		/* DG2 WA ids span across multiple subplatforms */
> +		if (find_devid(devid, subplatform_dg2_wa_ids,
> +			       ARRAY_SIZE(subplatform_dg2_wa_ids)))
> +			mask |= BIT(INTEL_SUBPLATFORM_WA);
>  	} else if (find_devid(devid, subplatform_arl_ids,
>  			      ARRAY_SIZE(subplatform_arl_ids))) {
>  		mask = BIT(INTEL_SUBPLATFORM_ARL);
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 643ff1bf74ee..c3623e859c78 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -95,9 +95,11 @@ enum intel_platform {
>  /*
>   * Subplatform bits share the same namespace per parent platform. In other words
>   * it is fine for the same bit to be used on multiple parent platforms.
> + * Devices can belong to multiple subplatforms if needed, so it's possible to set
> + * multiple bits for same device.
>   */
>  
> -#define INTEL_SUBPLATFORM_BITS (3)
> +#define INTEL_SUBPLATFORM_BITS (4)
>  #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1)
>  
>  /* HSW/BDW/SKL/KBL/CFL */
> @@ -114,6 +116,7 @@ enum intel_platform {
>  #define INTEL_SUBPLATFORM_G10	0
>  #define INTEL_SUBPLATFORM_G11	1
>  #define INTEL_SUBPLATFORM_G12	2
> +#define INTEL_SUBPLATFORM_WA	3
>  
>  /* ADL */
>  #define INTEL_SUBPLATFORM_RPL	0

-- 
Jani Nikula, Intel

Re: [PATCH v2 3/4] drm/i915/wa: Introduce intel_wa_cpu.c for CPU specific workarounds

[Jani Nikula]

On Fri, 11 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> Having similar naming convention in intel-family.h and intel_device_info.h
> results in redefinition of a few platforms. Define CPU IDs in its own file
> to avoid this.
>
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile               |  1 +
>  drivers/gpu/drm/i915/gt/intel_wa_cpu.c      | 34 +++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_workarounds.h |  2 ++
>  3 files changed, 37 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index c63fa2133ccb..1f9b503ab976 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -121,6 +121,7 @@ gt-y += \
>  	gt/intel_timeline.o \
>  	gt/intel_tlb.o \
>  	gt/intel_wopcm.o \
> +	gt/intel_wa_cpu.o \
>  	gt/intel_workarounds.o \
>  	gt/shmem_utils.o \
>  	gt/sysfs_engines.o
> diff --git a/drivers/gpu/drm/i915/gt/intel_wa_cpu.c b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c
> new file mode 100644
> index 000000000000..cbdab13e9db6
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + *
> + * This file is introduced to avoid platform redefinition from
> + * intel_device_info.h :(

I think the comment is good to have, but please be more specific and
direct, and leave out the emoticons.

"Avoid INTEL_<PLATFORM> name collisions between asm/intel-family.h and
intel_device_info.h by having a separate file."

Or something like that. Spells out exactly what the problem is, instead
of leaving the reader guessing.

> + */
> +
> +#include "intel_workarounds.h"
> +
> +#ifdef CONFIG_X86
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> +
> +static const struct x86_cpu_id wa_cpu_ids[] = {
> +	X86_MATCH_VFM(INTEL_ALDERLAKE,		NULL),
> +	X86_MATCH_VFM(INTEL_ALDERLAKE_L,	NULL),
> +	X86_MATCH_VFM(INTEL_COMETLAKE,		NULL),
> +	X86_MATCH_VFM(INTEL_KABYLAKE,		NULL),
> +	X86_MATCH_VFM(INTEL_KABYLAKE_L,		NULL),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE,		NULL),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE_P,	NULL),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,	NULL),
> +	X86_MATCH_VFM(INTEL_ROCKETLAKE,		NULL),
> +	{}
> +};
> +
> +bool intel_match_wa_cpu(void)

IMO the name's too generic.

> +{
> +	return x86_match_cpu(wa_cpu_ids);
> +}
> +#else
> +bool intel_match_wa_cpu(void) { return false; }
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h
> index 9beaab77c7f0..12f24fb31363 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h
> @@ -21,6 +21,8 @@ static inline void intel_wa_list_free(struct i915_wa_list *wal)
>  	memset(wal, 0, sizeof(*wal));
>  }
>  
> +bool intel_match_wa_cpu(void);
> +
>  void intel_engine_init_ctx_wa(struct intel_engine_cs *engine);
>  int intel_engine_emit_ctx_wa(struct i915_request *rq);

-- 
Jani Nikula, Intel

Re: [PATCH v2 0/4] Implement Wa_14022698537

[Jani Nikula]

On Fri, 11 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> This series implements Wa_14022698537 along with its prerequisites.
> Detailed description in commit message.

The same workaround is also needed for the xe driver, right?

Please include that in the same series, so that we can unify the
solution in a way that suits everyone. Especially the PCI ID macros need
to match between i915_pciids.h and xe_pciids.h, because my plan is to
merge those two files together soonish.

BR,
Jani.

>
> v2: Introduce DG2_WA subplatform for workaround (Jani)
>     Fix Wa_ID and include it in subject (Badal)
>     Rephrase commit message (Jani)
>     Move CPU whitelist to intel_wa_cpu.c
> v1: https://patchwork.freedesktop.org/series/139628/
>
> Raag Jadav (4):
>   drm/i915/pciids: Refactor DG2 PCI IDs into workaround ranges
>   drm/i915/dg2: Introduce DG2_WA subplatform
>   drm/i915/wa: Introduce intel_wa_cpu.c for CPU specific workarounds
>   drm/i915/dg2: Implement Wa_14022698537
>
>  drivers/gpu/drm/i915/Makefile               |  1 +
>  drivers/gpu/drm/i915/gt/intel_wa_cpu.c      | 34 +++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 +++++++++++
>  drivers/gpu/drm/i915/gt/intel_workarounds.h |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h             |  2 ++
>  drivers/gpu/drm/i915/i915_reg.h             |  1 +
>  drivers/gpu/drm/i915/intel_device_info.c    | 34 +++++++++++++++------
>  drivers/gpu/drm/i915/intel_device_info.h    |  5 ++-
>  include/drm/intel/i915_pciids.h             | 34 +++++++++++++++------
>  9 files changed, 111 insertions(+), 20 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c

-- 
Jani Nikula, Intel

✗ Fi.CI.CHECKPATCH: warning for Implement Wa_14022698537

[Patchwork]

== Series Details ==

Series: Implement Wa_14022698537
URL   : https://patchwork.freedesktop.org/series/139883/
State : warning

== Summary ==

Error: dim checkpatch failed
132c044fd5e1 drm/i915/pciids: Refactor DG2 PCI IDs into workaround ranges
-:18: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#18: FILE: include/drm/intel/i915_pciids.h:720:
+#define INTEL_DG2_G10_WA_IDS(MACRO__, ...) \
+	MACRO__(0x56A0, ## __VA_ARGS__), \
+	MACRO__(0x56A1, ## __VA_ARGS__), \
+	MACRO__(0x56A2, ## __VA_ARGS__)

-:18: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'MACRO__' - possible side-effects?
#18: FILE: include/drm/intel/i915_pciids.h:720:
+#define INTEL_DG2_G10_WA_IDS(MACRO__, ...) \
+	MACRO__(0x56A0, ## __VA_ARGS__), \
+	MACRO__(0x56A1, ## __VA_ARGS__), \
+	MACRO__(0x56A2, ## __VA_ARGS__)

-:34: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#34: FILE: include/drm/intel/i915_pciids.h:733:
+#define INTEL_DG2_G11_WA_IDS(MACRO__, ...) \
+	MACRO__(0x56A5, ## __VA_ARGS__), \
+	MACRO__(0x56A6, ## __VA_ARGS__), \
+	MACRO__(0x56B0, ## __VA_ARGS__), \
+	MACRO__(0x56B1, ## __VA_ARGS__)

-:34: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'MACRO__' - possible side-effects?
#34: FILE: include/drm/intel/i915_pciids.h:733:
+#define INTEL_DG2_G11_WA_IDS(MACRO__, ...) \
+	MACRO__(0x56A5, ## __VA_ARGS__), \
+	MACRO__(0x56A6, ## __VA_ARGS__), \
+	MACRO__(0x56B0, ## __VA_ARGS__), \
+	MACRO__(0x56B1, ## __VA_ARGS__)

-:57: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#57: FILE: include/drm/intel/i915_pciids.h:749:
+#define INTEL_DG2_G12_WA_IDS(MACRO__, ...) \
 	MACRO__(0x56A3, ## __VA_ARGS__), \
 	MACRO__(0x56A4, ## __VA_ARGS__), \
 	MACRO__(0x56B2, ## __VA_ARGS__), \
 	MACRO__(0x56B3, ## __VA_ARGS__)

-:57: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'MACRO__' - possible side-effects?
#57: FILE: include/drm/intel/i915_pciids.h:749:
+#define INTEL_DG2_G12_WA_IDS(MACRO__, ...) \
 	MACRO__(0x56A3, ## __VA_ARGS__), \
 	MACRO__(0x56A4, ## __VA_ARGS__), \
 	MACRO__(0x56B2, ## __VA_ARGS__), \
 	MACRO__(0x56B3, ## __VA_ARGS__)

-:63: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#63: FILE: include/drm/intel/i915_pciids.h:755:
+#define INTEL_DG2_G12_IDS(MACRO__, ...) \
+	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	MACRO__(0x5696, ## __VA_ARGS__), \
+	MACRO__(0x5697, ## __VA_ARGS__)

-:63: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'MACRO__' - possible side-effects?
#63: FILE: include/drm/intel/i915_pciids.h:755:
+#define INTEL_DG2_G12_IDS(MACRO__, ...) \
+	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	MACRO__(0x5696, ## __VA_ARGS__), \
+	MACRO__(0x5697, ## __VA_ARGS__)

-:68: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#68: FILE: include/drm/intel/i915_pciids.h:760:
+#define INTEL_DG2_WA_IDS(MACRO__, ...) \
+	INTEL_DG2_G10_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	INTEL_DG2_G12_WA_IDS(MACRO__, ## __VA_ARGS__)

-:68: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'MACRO__' - possible side-effects?
#68: FILE: include/drm/intel/i915_pciids.h:760:
+#define INTEL_DG2_WA_IDS(MACRO__, ...) \
+	INTEL_DG2_G10_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	INTEL_DG2_G11_WA_IDS(MACRO__, ## __VA_ARGS__), \
+	INTEL_DG2_G12_WA_IDS(MACRO__, ## __VA_ARGS__)

total: 5 errors, 0 warnings, 5 checks, 61 lines checked
a91046b9601b drm/i915/dg2: Introduce DG2_WA subplatform
8de9d66be810 drm/i915/wa: Introduce intel_wa_cpu.c for CPU specific workarounds
-:26: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 49 lines checked
88cd723cc2f7 drm/i915/dg2: Implement Wa_14022698537

✗ Fi.CI.SPARSE: warning for Implement Wa_14022698537

[Patchwork]

== Series Details ==

Series: Implement Wa_14022698537
URL   : https://patchwork.freedesktop.org/series/139883/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for Implement Wa_14022698537

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 15662 bytes --]

== Series Details ==

Series: Implement Wa_14022698537
URL   : https://patchwork.freedesktop.org/series/139883/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15514 -> Patchwork_139883v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_139883v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_139883v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/index.html

Participating hosts (45 -> 44)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_139883v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - bat-dg2-13:         [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-13/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-13/igt@i915_module_load@load.html

  * igt@i915_pm_rpm@module-reload:
    - bat-dg2-14:         [PASS][3] -> [SKIP][4] +7 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@requests:
    - bat-dg2-14:         [PASS][5] -> [FAIL][6] +39 other tests fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@i915_selftest@live@requests.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@i915_selftest@live@requests.html

  
Known issues
------------

  Here are the changes found in Patchwork_139883v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - bat-arlh-2:         [PASS][7] -> [ABORT][8] ([i915#12133])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-arlh-2/igt@i915_selftest@live.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-arlh-2/igt@i915_selftest@live.html
    - bat-dg2-11:         [PASS][9] -> [ABORT][10] ([i915#12133])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-11/igt@i915_selftest@live.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-11/igt@i915_selftest@live.html

  * igt@i915_selftest@live@active:
    - bat-dg2-11:         [PASS][11] -> [ABORT][12] ([i915#12305])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-11/igt@i915_selftest@live@active.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-11/igt@i915_selftest@live@active.html

  * igt@i915_selftest@live@late_gt_pm:
    - bat-atsm-1:         [PASS][13] -> [ABORT][14] ([i915#12133]) +1 other test abort
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-atsm-1/igt@i915_selftest@live@late_gt_pm.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-atsm-1/igt@i915_selftest@live@late_gt_pm.html

  * igt@i915_selftest@live@workarounds:
    - bat-arlh-2:         [PASS][15] -> [ABORT][16] ([i915#12061])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-arlh-2/igt@i915_selftest@live@workarounds.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-arlh-2/igt@i915_selftest@live@workarounds.html

  * igt@kms_addfb_basic@addfb25-yf-tiled-legacy:
    - bat-dg2-14:         [PASS][17] -> [SKIP][18] ([i915#2575] / [i915#5190]) +1 other test skip
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_addfb_basic@addfb25-yf-tiled-legacy.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_addfb_basic@addfb25-yf-tiled-legacy.html

  * igt@kms_addfb_basic@unused-offsets:
    - bat-dg2-14:         [PASS][19] -> [SKIP][20] ([i915#2575]) +96 other tests skip
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_addfb_basic@unused-offsets.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_addfb_basic@unused-offsets.html

  * igt@kms_chamelium_hpd@dp-hpd-fast:
    - bat-dg2-13:         [PASS][21] -> [SKIP][22] ([i915#2575]) +6 other tests skip
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-13/igt@kms_chamelium_hpd@dp-hpd-fast.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-13/igt@kms_chamelium_hpd@dp-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-dg2-14:         [PASS][23] -> [SKIP][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_frontbuffer_tracking@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-6:         [ABORT][25] ([i915#12216]) -> [PASS][26] +1 other test pass
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-mtlp-6/igt@i915_selftest@live@workarounds.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-mtlp-6/igt@i915_selftest@live@workarounds.html

  
#### Warnings ####

  * igt@gem_mmap@basic:
    - bat-dg2-14:         [SKIP][27] ([i915#4083]) -> [SKIP][28] ([i915#2575])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@gem_mmap@basic.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@gem_mmap@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-dg2-14:         [SKIP][29] ([i915#4079]) -> [SKIP][30] ([i915#2575]) +1 other test skip
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@gem_render_tiled_blits@basic.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-dg2-14:         [SKIP][31] ([i915#4077]) -> [SKIP][32] ([i915#2575]) +2 other tests skip
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@gem_tiled_fence_blits@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@gem_tiled_fence_blits@basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg2-14:         [SKIP][33] ([i915#11681] / [i915#6621]) -> [SKIP][34] ([i915#2575])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@i915_pm_rps@basic-api.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live:
    - bat-arls-5:         [DMESG-WARN][35] ([i915#10341] / [i915#12133]) -> [ABORT][36] ([i915#12061] / [i915#12133])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-arls-5/igt@i915_selftest@live.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-arls-5/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [DMESG-WARN][37] ([i915#10341] / [i915#11637]) -> [ABORT][38] ([i915#12061])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-arls-5/igt@i915_selftest@live@workarounds.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-dg2-14:         [SKIP][39] ([i915#5190]) -> [SKIP][40] ([i915#2575] / [i915#5190])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-dg2-14:         [SKIP][41] ([i915#4215] / [i915#5190]) -> [SKIP][42] ([i915#2575] / [i915#5190])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_addfb_basic@basic-y-tiled-legacy.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@tile-pitch-mismatch:
    - bat-dg2-14:         [SKIP][43] ([i915#4212]) -> [SKIP][44] ([i915#2575]) +7 other tests skip
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_addfb_basic@tile-pitch-mismatch.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_addfb_basic@tile-pitch-mismatch.html

  * igt@kms_chamelium_edid@vga-edid-read:
    - bat-dg2-13:         [SKIP][45] ([Intel XE#484] / [i915#4550]) -> [SKIP][46] ([i915#2575]) +1 other test skip
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-13/igt@kms_chamelium_edid@vga-edid-read.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-13/igt@kms_chamelium_edid@vga-edid-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-dg2-14:         [SKIP][47] ([i915#4103] / [i915#4213]) -> [SKIP][48] ([i915#2575]) +1 other test skip
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-dg2-14:         [SKIP][49] ([i915#3555] / [i915#3840]) -> [SKIP][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_dsc@dsc-basic.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-dg2-14:         [SKIP][51] -> [SKIP][52] ([i915#2575])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_force_connector_basic@force-load-detect.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-dg2-14:         [SKIP][53] ([i915#5274]) -> [SKIP][54] ([i915#2575])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_force_connector_basic@prune-stale-modes.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-dg2-14:         [SKIP][55] ([i915#5354]) -> [SKIP][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_pm_backlight@basic-brightness.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_pm_backlight@basic-brightness.html

  * igt@kms_psr@psr-sprite-plane-onoff:
    - bat-dg2-14:         [SKIP][57] ([i915#1072] / [i915#9732]) -> [SKIP][58] +3 other tests skip
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_psr@psr-sprite-plane-onoff.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_psr@psr-sprite-plane-onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg2-14:         [SKIP][59] ([i915#3555]) -> [SKIP][60] ([i915#2575])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@kms_setmode@basic-clone-single-crtc.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-dg2-14:         [SKIP][61] ([i915#3708]) -> [SKIP][62] ([i915#2575])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@prime_vgem@basic-fence-flip.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-gtt:
    - bat-dg2-14:         [SKIP][63] ([i915#3708] / [i915#4077]) -> [SKIP][64] ([i915#2575]) +1 other test skip
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@prime_vgem@basic-gtt.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@prime_vgem@basic-gtt.html

  * igt@prime_vgem@basic-write:
    - bat-dg2-14:         [SKIP][65] ([i915#3291] / [i915#3708]) -> [SKIP][66] ([i915#2575]) +2 other tests skip
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15514/bat-dg2-14/igt@prime_vgem@basic-write.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/bat-dg2-14/igt@prime_vgem@basic-write.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [Intel XE#484]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/484
  [i915#10341]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10341
  [i915#1072]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1072
  [i915#11637]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11637
  [i915#11681]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11681
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12133]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12133
  [i915#12216]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12216
  [i915#12305]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12305
  [i915#2575]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/2575
  [i915#3291]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3291
  [i915#3555]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3708
  [i915#3840]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/3840
  [i915#4077]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4215
  [i915#4550]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4550
  [i915#5190]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/6621
  [i915#9732]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9732


Build changes
-------------

  * Linux: CI_DRM_15514 -> Patchwork_139883v1

  CI-20190529: 20190529
  CI_DRM_15514: f3354eab2a45420cf38a67f6d7735d5dff53eb6f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8063: 8063
  Patchwork_139883v1: f3354eab2a45420cf38a67f6d7735d5dff53eb6f @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139883v1/index.html

[-- Attachment #2: Type: text/html, Size: 19429 bytes --]

Re: [PATCH v2 2/4] drm/i915/dg2: Introduce DG2_WA subplatform

[Raag Jadav]

On Fri, Oct 11, 2024 at 01:44:01PM +0300, Jani Nikula wrote:
> On Fri, 11 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> > Introduce DG2_WA subplatform for the devices that will be used in a
> > workaround and span across multiple DG2 subplatforms.
> 
> Ditto, give the subplatform a name other than "WA". Look up the specs,
> what's in common?

This WA is basically for G8 power state which reduces idle power.

I initially opted for _G8 but it didn't sit quite well with existing G10, G11 macros
which I'm guessing means something else?

Perhaps we can use _PM, but it would be a bit misleading as well, since it has nothing
to do with Linux PM.

Open to suggestions.

> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h          |  2 ++
> >  drivers/gpu/drm/i915/intel_device_info.c | 34 +++++++++++++++++-------
> >  drivers/gpu/drm/i915/intel_device_info.h |  5 +++-
> >  3 files changed, 31 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 39f6614a0a99..0a68cd9379e8 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -548,6 +548,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
> >  	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_G11)
> >  #define IS_DG2_G12(i915) \
> >  	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_G12)
> > +#define IS_DG2_WA(i915) \
> > +	IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_WA)
> >  #define IS_RAPTORLAKE_S(i915) \
> >  	IS_SUBPLATFORM(i915, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL)
> >  #define IS_ALDERLAKE_P_N(i915) \
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > index 3c47c625993e..674ab2a4d75e 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -200,6 +200,15 @@ static const u16 subplatform_g12_ids[] = {
> >  	INTEL_DG2_G12_IDS(ID),
> >  };
> >  
> > +static const u16 subplatform_dg2_wa_ids[] = {
> > +	INTEL_DG2_WA_IDS(ID),
> > +};
> > +
> > +static const u16 subplatform_dg2_ids[] = {
> > +	INTEL_DG2_IDS(ID),
> > +	INTEL_ATS_M_IDS(ID),
> > +};
> 
> Nope, you don't need this.
> 
> > +
> >  static const u16 subplatform_arl_ids[] = {
> >  	INTEL_ARL_IDS(ID),
> >  };
> > @@ -252,15 +261,22 @@ static void intel_device_info_subplatform_init(struct drm_i915_private *i915)
> >  		if (find_devid(devid, subplatform_rplu_ids,
> >  			       ARRAY_SIZE(subplatform_rplu_ids)))
> >  			mask |= BIT(INTEL_SUBPLATFORM_RPLU);
> > -	} else if (find_devid(devid, subplatform_g10_ids,
> > -			      ARRAY_SIZE(subplatform_g10_ids))) {
> > -		mask = BIT(INTEL_SUBPLATFORM_G10);
> > -	} else if (find_devid(devid, subplatform_g11_ids,
> > -			      ARRAY_SIZE(subplatform_g11_ids))) {
> > -		mask = BIT(INTEL_SUBPLATFORM_G11);
> > -	} else if (find_devid(devid, subplatform_g12_ids,
> > -			      ARRAY_SIZE(subplatform_g12_ids))) {
> > -		mask = BIT(INTEL_SUBPLATFORM_G12);
> > +	} else if (find_devid(devid, subplatform_dg2_ids,
> > +			      ARRAY_SIZE(subplatform_dg2_ids))) {
> 
> Nope. Just hoist the below checks to higher level.

With that, won't we need to duplicate wa_ids check for each?

> > +		if (find_devid(devid, subplatform_g10_ids,
> > +			       ARRAY_SIZE(subplatform_g10_ids)))
> > +			mask = BIT(INTEL_SUBPLATFORM_G10);
> > +		else if (find_devid(devid, subplatform_g11_ids,
> > +				    ARRAY_SIZE(subplatform_g11_ids)))
> > +			mask = BIT(INTEL_SUBPLATFORM_G11);
> > +		else if (find_devid(devid, subplatform_g12_ids,
> > +				    ARRAY_SIZE(subplatform_g12_ids)))
> > +			mask = BIT(INTEL_SUBPLATFORM_G12);
> > +
> > +		/* DG2 WA ids span across multiple subplatforms */
> > +		if (find_devid(devid, subplatform_dg2_wa_ids,
> > +			       ARRAY_SIZE(subplatform_dg2_wa_ids)))
> > +			mask |= BIT(INTEL_SUBPLATFORM_WA);
> >  	} else if (find_devid(devid, subplatform_arl_ids,
> >  			      ARRAY_SIZE(subplatform_arl_ids))) {
> >  		mask = BIT(INTEL_SUBPLATFORM_ARL);
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> > index 643ff1bf74ee..c3623e859c78 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -95,9 +95,11 @@ enum intel_platform {
> >  /*
> >   * Subplatform bits share the same namespace per parent platform. In other words
> >   * it is fine for the same bit to be used on multiple parent platforms.
> > + * Devices can belong to multiple subplatforms if needed, so it's possible to set
> > + * multiple bits for same device.
> >   */
> >  
> > -#define INTEL_SUBPLATFORM_BITS (3)
> > +#define INTEL_SUBPLATFORM_BITS (4)
> >  #define INTEL_SUBPLATFORM_MASK (BIT(INTEL_SUBPLATFORM_BITS) - 1)
> >  
> >  /* HSW/BDW/SKL/KBL/CFL */
> > @@ -114,6 +116,7 @@ enum intel_platform {
> >  #define INTEL_SUBPLATFORM_G10	0
> >  #define INTEL_SUBPLATFORM_G11	1
> >  #define INTEL_SUBPLATFORM_G12	2
> > +#define INTEL_SUBPLATFORM_WA	3
> >  
> >  /* ADL */
> >  #define INTEL_SUBPLATFORM_RPL	0
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH v2 4/4] drm/i915/dg2: Implement Wa_14022698537

[Riana Tauro]

Hi Raag

On 10/11/2024 4:02 PM, Raag Jadav wrote:
> G8 power state entry is disabled due to a limitation on DG2, so we
> enable it from driver with Wa_14022698537. Fow now we enable it for
typo
> all DG2 devices with the exception of a few, for which, we enable
> only when paired with whitelisted CPU models.
> 
> v2: Fix Wa_ID and include it in subject (Badal)
>      Rephrase commit message (Jani)
> 
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 ++++++++++++++++++
>   drivers/gpu/drm/i915/i915_reg.h             |  1 +
>   2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index e539a656cfc3..bcd7630c1631 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -14,6 +14,7 @@
>   #include "intel_gt_mcr.h"
>   #include "intel_gt_print.h"
>   #include "intel_gt_regs.h"
> +#include "intel_pcode.h"
>   #include "intel_ring.h"
>   #include "intel_workarounds.h"
>   
> @@ -1770,9 +1771,26 @@ static void wa_list_apply(const struct i915_wa_list *wal)
>   	intel_gt_mcr_unlock(gt, flags);
>   }
>   
> +/* Wa_14022698537:dg2 */
> +static void intel_enable_g8(struct intel_uncore *uncore)
> +{
> +	struct drm_i915_private *i915 = uncore->i915;
> +
> +	if (IS_DG2(i915)) {
> +		if (IS_DG2_WA(i915) && !intel_match_wa_cpu())
> +			return;
> +
> +		snb_pcode_write_p(uncore, PCODE_POWER_SETUP,
> +				  POWER_SETUP_SUBCOMMAND_G8_ENABLE, 0, 0);
> +	}
> +}
> +
>   void intel_gt_apply_workarounds(struct intel_gt *gt)
>   {
>   	wa_list_apply(&gt->wa_list);
> +
> +	/* Special case for pcode mailbox which can't be on wa_list */
> +	intel_enable_g8(gt->uncore);
>   }
>   
>   static bool wa_list_verify(struct intel_gt *gt,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 41f4350a7c6c..e948b194550c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3568,6 +3568,7 @@
>   #define   PCODE_POWER_SETUP			0x7C
>   #define     POWER_SETUP_SUBCOMMAND_READ_I1	0x4
>   #define     POWER_SETUP_SUBCOMMAND_WRITE_I1	0x5
> +#define     POWER_SETUP_SUBCOMMAND_G8_ENABLE	0x6
Add this below the I1 bits

Thanks,
Riana
>   #define	    POWER_SETUP_I1_WATTS		REG_BIT(31)
>   #define	    POWER_SETUP_I1_SHIFT		6	/* 10.6 fixed point format */
>   #define	    POWER_SETUP_I1_DATA_MASK		REG_GENMASK(15, 0)

Re: [PATCH v2 3/4] drm/i915/wa: Introduce intel_wa_cpu.c for CPU specific workarounds

[Riana Tauro]

Hi Raag

On 10/11/2024 4:02 PM, Raag Jadav wrote:
> Having similar naming convention in intel-family.h and intel_device_info.h
> results in redefinition of a few platforms. Define CPU IDs in its own file
> to avoid this.
> 
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile               |  1 +
>   drivers/gpu/drm/i915/gt/intel_wa_cpu.c      | 34 +++++++++++++++++++++
>   drivers/gpu/drm/i915/gt/intel_workarounds.h |  2 ++
>   3 files changed, 37 insertions(+)
>   create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index c63fa2133ccb..1f9b503ab976 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -121,6 +121,7 @@ gt-y += \
>   	gt/intel_timeline.o \
>   	gt/intel_tlb.o \
>   	gt/intel_wopcm.o \
> +	gt/intel_wa_cpu.o \
Why is this file under gt/ ? Doesn't seem to be gt specific
>   	gt/intel_workarounds.o \
>   	gt/shmem_utils.o \
>   	gt/sysfs_engines.o
> diff --git a/drivers/gpu/drm/i915/gt/intel_wa_cpu.c b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c
> new file mode 100644
> index 000000000000..cbdab13e9db6
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/intel_wa_cpu.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2024 Intel Corporation
> + *
> + * This file is introduced to avoid platform redefinition from
> + * intel_device_info.h :(
> + */
> +
> +#include "intel_workarounds.h"
> +
> +#ifdef CONFIG_X86
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> +
> +static const struct x86_cpu_id wa_cpu_ids[] = {
> +	X86_MATCH_VFM(INTEL_ALDERLAKE,		NULL),
> +	X86_MATCH_VFM(INTEL_ALDERLAKE_L,	NULL),
> +	X86_MATCH_VFM(INTEL_COMETLAKE,		NULL),
> +	X86_MATCH_VFM(INTEL_KABYLAKE,		NULL),
> +	X86_MATCH_VFM(INTEL_KABYLAKE_L,		NULL),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE,		NULL),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE_P,	NULL),
> +	X86_MATCH_VFM(INTEL_RAPTORLAKE_S,	NULL),
> +	X86_MATCH_VFM(INTEL_ROCKETLAKE,		NULL),
> +	{}
> +};
> +
Add a doc

Thanks,
Riana
> +bool intel_match_wa_cpu(void)
> +{
> +	return x86_match_cpu(wa_cpu_ids);
> +}
> +#else
> +bool intel_match_wa_cpu(void) { return false; }
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.h b/drivers/gpu/drm/i915/gt/intel_workarounds.h
> index 9beaab77c7f0..12f24fb31363 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.h
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.h
> @@ -21,6 +21,8 @@ static inline void intel_wa_list_free(struct i915_wa_list *wal)
>   	memset(wal, 0, sizeof(*wal));
>   }
>   
> +bool intel_match_wa_cpu(void);
> +
>   void intel_engine_init_ctx_wa(struct intel_engine_cs *engine);
>   int intel_engine_emit_ctx_wa(struct i915_request *rq);
>   

Re: [PATCH v2 3/4] drm/i915/wa: Introduce intel_wa_cpu.c for CPU specific workarounds

[Raag Jadav]

On Tue, Oct 15, 2024 at 03:56:10PM +0530, Riana Tauro wrote:
> Hi Raag
> 
> On 10/11/2024 4:02 PM, Raag Jadav wrote:
> > Having similar naming convention in intel-family.h and intel_device_info.h
> > results in redefinition of a few platforms. Define CPU IDs in its own file
> > to avoid this.
> > 
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > ---
> >   drivers/gpu/drm/i915/Makefile               |  1 +
> >   drivers/gpu/drm/i915/gt/intel_wa_cpu.c      | 34 +++++++++++++++++++++
> >   drivers/gpu/drm/i915/gt/intel_workarounds.h |  2 ++
> >   3 files changed, 37 insertions(+)
> >   create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c
> > 
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index c63fa2133ccb..1f9b503ab976 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -121,6 +121,7 @@ gt-y += \
> >   	gt/intel_timeline.o \
> >   	gt/intel_tlb.o \
> >   	gt/intel_wopcm.o \
> > +	gt/intel_wa_cpu.o \
> Why is this file under gt/ ? Doesn't seem to be gt specific

The idea is to couple it with intel_workarounds.c
Any other place it'd rather be?

> >   	gt/intel_workarounds.o \
> >   	gt/shmem_utils.o \
> >   	gt/sysfs_engines.o

Raag

Re: [PATCH v2 3/4] drm/i915/wa: Introduce intel_wa_cpu.c for CPU specific workarounds

[Riana Tauro]

Hi Raag

On 10/16/2024 2:31 PM, Raag Jadav wrote:
> On Tue, Oct 15, 2024 at 03:56:10PM +0530, Riana Tauro wrote:
>> Hi Raag
>>
>> On 10/11/2024 4:02 PM, Raag Jadav wrote:
>>> Having similar naming convention in intel-family.h and intel_device_info.h
>>> results in redefinition of a few platforms. Define CPU IDs in its own file
>>> to avoid this.
>>>
>>> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/Makefile               |  1 +
>>>    drivers/gpu/drm/i915/gt/intel_wa_cpu.c      | 34 +++++++++++++++++++++
>>>    drivers/gpu/drm/i915/gt/intel_workarounds.h |  2 ++
>>>    3 files changed, 37 insertions(+)
>>>    create mode 100644 drivers/gpu/drm/i915/gt/intel_wa_cpu.c
>>>
>>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>>> index c63fa2133ccb..1f9b503ab976 100644
>>> --- a/drivers/gpu/drm/i915/Makefile
>>> +++ b/drivers/gpu/drm/i915/Makefile
>>> @@ -121,6 +121,7 @@ gt-y += \
>>>    	gt/intel_timeline.o \
>>>    	gt/intel_tlb.o \
>>>    	gt/intel_wopcm.o \
>>> +	gt/intel_wa_cpu.o \
>> Why is this file under gt/ ? Doesn't seem to be gt specific
> 
> The idea is to couple it with intel_workarounds.c
This doesn't use any gt specific data. IMO should be out of gt, but upto you
> Any other place it'd rather be?
> 
>>>    	gt/intel_workarounds.o \
>>>    	gt/shmem_utils.o \
>>>    	gt/sysfs_engines.o
> 
> Raag

Re: [PATCH v2 4/4] drm/i915/dg2: Implement Wa_14022698537

[Nilawar, Badal]

On 11-10-2024 16:02, Raag Jadav wrote:
> G8 power state entry is disabled due to a limitation on DG2, so we
> enable it from driver with Wa_14022698537. Fow now we enable it for
> all DG2 devices with the exception of a few, for which, we enable
> only when paired with whitelisted CPU models.
> 
> v2: Fix Wa_ID and include it in subject (Badal)
>      Rephrase commit message (Jani)
> 
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 ++++++++++++++++++
>   drivers/gpu/drm/i915/i915_reg.h             |  1 +
>   2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index e539a656cfc3..bcd7630c1631 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -14,6 +14,7 @@
>   #include "intel_gt_mcr.h"
>   #include "intel_gt_print.h"
>   #include "intel_gt_regs.h"
> +#include "intel_pcode.h"
>   #include "intel_ring.h"
>   #include "intel_workarounds.h"
>   
> @@ -1770,9 +1771,26 @@ static void wa_list_apply(const struct i915_wa_list *wal)
>   	intel_gt_mcr_unlock(gt, flags);
>   }
>   
> +/* Wa_14022698537:dg2 */
> +static void intel_enable_g8(struct intel_uncore *uncore)
> +{
> +	struct drm_i915_private *i915 = uncore->i915;
> +
> +	if (IS_DG2(i915)) {
> +		if (IS_DG2_WA(i915) && !intel_match_wa_cpu())
> +			return;
> +
> +		snb_pcode_write_p(uncore, PCODE_POWER_SETUP,
> +				  POWER_SETUP_SUBCOMMAND_G8_ENABLE, 0, 0);
> +	}
> +}
> +
>   void intel_gt_apply_workarounds(struct intel_gt *gt)
>   {
>   	wa_list_apply(&gt->wa_list);
> +
> +	/* Special case for pcode mailbox which can't be on wa_list */
> +	intel_enable_g8(gt->uncore);

This workaround is not specific to GT; G8 is a state specific to the 
SoC. Therefore, move this workaround above the GT layer and pass 
argument i915->uncore instead of gt->uncore

Regards,
Badal

>   }
>   
>   static bool wa_list_verify(struct intel_gt *gt,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 41f4350a7c6c..e948b194550c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3568,6 +3568,7 @@
>   #define   PCODE_POWER_SETUP			0x7C
>   #define     POWER_SETUP_SUBCOMMAND_READ_I1	0x4
>   #define     POWER_SETUP_SUBCOMMAND_WRITE_I1	0x5
> +#define     POWER_SETUP_SUBCOMMAND_G8_ENABLE	0x6
>   #define	    POWER_SETUP_I1_WATTS		REG_BIT(31)
>   #define	    POWER_SETUP_I1_SHIFT		6	/* 10.6 fixed point format */
>   #define	    POWER_SETUP_I1_DATA_MASK		REG_GENMASK(15, 0)

Re: [PATCH v2 4/4] drm/i915/dg2: Implement Wa_14022698537

[Raag Jadav]

On Tue, Oct 22, 2024 at 06:41:57PM +0530, Nilawar, Badal wrote:
> On 11-10-2024 16:02, Raag Jadav wrote:
> > G8 power state entry is disabled due to a limitation on DG2, so we
> > enable it from driver with Wa_14022698537. Fow now we enable it for
> > all DG2 devices with the exception of a few, for which, we enable
> > only when paired with whitelisted CPU models.
> > 
> > v2: Fix Wa_ID and include it in subject (Badal)
> >      Rephrase commit message (Jani)
> > 
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 ++++++++++++++++++
> >   drivers/gpu/drm/i915/i915_reg.h             |  1 +
> >   2 files changed, 19 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index e539a656cfc3..bcd7630c1631 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -14,6 +14,7 @@
> >   #include "intel_gt_mcr.h"
> >   #include "intel_gt_print.h"
> >   #include "intel_gt_regs.h"
> > +#include "intel_pcode.h"
> >   #include "intel_ring.h"
> >   #include "intel_workarounds.h"
> > @@ -1770,9 +1771,26 @@ static void wa_list_apply(const struct i915_wa_list *wal)
> >   	intel_gt_mcr_unlock(gt, flags);
> >   }
> > +/* Wa_14022698537:dg2 */
> > +static void intel_enable_g8(struct intel_uncore *uncore)
> > +{
> > +	struct drm_i915_private *i915 = uncore->i915;
> > +
> > +	if (IS_DG2(i915)) {
> > +		if (IS_DG2_WA(i915) && !intel_match_wa_cpu())
> > +			return;
> > +
> > +		snb_pcode_write_p(uncore, PCODE_POWER_SETUP,
> > +				  POWER_SETUP_SUBCOMMAND_G8_ENABLE, 0, 0);
> > +	}
> > +}
> > +
> >   void intel_gt_apply_workarounds(struct intel_gt *gt)
> >   {
> >   	wa_list_apply(&gt->wa_list);
> > +
> > +	/* Special case for pcode mailbox which can't be on wa_list */
> > +	intel_enable_g8(gt->uncore);
> 
> This workaround is not specific to GT; G8 is a state specific to the SoC.
> Therefore, move this workaround above the GT layer and pass argument
> i915->uncore instead of gt->uncore

Since this WA needs to be applied on suspend/resume/reset cycles, I found
it to be more suitable here, atleast according to the documentation.

 * - GT workarounds: the list of these WAs is applied whenever these registers
 *   revert to their default values: on GPU reset, suspend/resume [1]_, etc.

We can either limit this to primary gt (using gt->info.id) here or move
this to i915_pcode_init() instead, whichever is the better option.

Raag

RE: [PATCH v2 4/4] drm/i915/dg2: Implement Wa_14022698537

[Gupta, Anshuman]

> -----Original Message-----
> From: Jadav, Raag <raag.jadav@intel.com>
> Sent: Wednesday, October 23, 2024 12:40 PM
> To: Nilawar, Badal <badal.nilawar@intel.com>
> Cc: jani.nikula@linux.intel.com; joonas.lahtinen@linux.intel.com; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com>;
> andi.shyti@linux.intel.com; intel-gfx@lists.freedesktop.org; Gupta, Anshuman
> <anshuman.gupta@intel.com>; Tauro, Riana <riana.tauro@intel.com>
> Subject: Re: [PATCH v2 4/4] drm/i915/dg2: Implement Wa_14022698537
> 
> On Tue, Oct 22, 2024 at 06:41:57PM +0530, Nilawar, Badal wrote:
> > On 11-10-2024 16:02, Raag Jadav wrote:
> > > G8 power state entry is disabled due to a limitation on DG2, so we
> > > enable it from driver with Wa_14022698537. Fow now we enable it for
> > > all DG2 devices with the exception of a few, for which, we enable
> > > only when paired with whitelisted CPU models.
> > >
> > > v2: Fix Wa_ID and include it in subject (Badal)
> > >      Rephrase commit message (Jani)
> > >
> > > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 ++++++++++++++++++
> > >   drivers/gpu/drm/i915/i915_reg.h             |  1 +
> > >   2 files changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > index e539a656cfc3..bcd7630c1631 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > @@ -14,6 +14,7 @@
> > >   #include "intel_gt_mcr.h"
> > >   #include "intel_gt_print.h"
> > >   #include "intel_gt_regs.h"
> > > +#include "intel_pcode.h"
> > >   #include "intel_ring.h"
> > >   #include "intel_workarounds.h"
> > > @@ -1770,9 +1771,26 @@ static void wa_list_apply(const struct
> i915_wa_list *wal)
> > >   	intel_gt_mcr_unlock(gt, flags);
> > >   }
> > > +/* Wa_14022698537:dg2 */
> > > +static void intel_enable_g8(struct intel_uncore *uncore) {
> > > +	struct drm_i915_private *i915 = uncore->i915;
> > > +
> > > +	if (IS_DG2(i915)) {
> > > +		if (IS_DG2_WA(i915) && !intel_match_wa_cpu())
> > > +			return;
> > > +
> > > +		snb_pcode_write_p(uncore, PCODE_POWER_SETUP,
> > > +				  POWER_SETUP_SUBCOMMAND_G8_ENABLE,
> 0, 0);
> > > +	}
> > > +}
> > > +
> > >   void intel_gt_apply_workarounds(struct intel_gt *gt)
> > >   {
> > >   	wa_list_apply(&gt->wa_list);
> > > +
> > > +	/* Special case for pcode mailbox which can't be on wa_list */
> > > +	intel_enable_g8(gt->uncore);
My earlier comment was to include this WA in intel_gt_apply_workarounds with  an intention to
Add to wa_list_apply() but seems it is not feasible to keep in that list.
Therefore it is better to keep call this WA function from i915_pcode_init to wrap all pcode related
Functionality.

Thanks,
Anshuman Gupta.  
> >
> > This workaround is not specific to GT; G8 is a state specific to the SoC.
> > Therefore, move this workaround above the GT layer and pass argument
> > i915->uncore instead of gt->uncore
> 
> Since this WA needs to be applied on suspend/resume/reset cycles, I found it to
> be more suitable here, atleast according to the documentation.
> 
>  * - GT workarounds: the list of these WAs is applied whenever these registers
>  *   revert to their default values: on GPU reset, suspend/resume [1]_, etc.
> 
> We can either limit this to primary gt (using gt->info.id) here or move this to
> i915_pcode_init() instead, whichever is the better option.
> 
> Raag