Re: [Intel-gfx] [PATCH V4 4/6] drm/i915/gt: Initialize unused MOCS entries with device specific values
On Fri, Sep 03, 2021 at 12:26:33AM +0530, Ayaz A Siddiqui wrote:
> Historically we've initialized all undefined/reserved entries in
> a platform's MOCS table to the contents of table entry #1 (i.e.,
> I915_MOCS_PTE).
> Going forward, we can't assume that table entry #1 will always
> contain suitable values to use for undefined/reserved table
> indices. We'll allow a platform-specific table index to be
> selected at table initialization time in these cases.
>
> This new mechanism to select L3 WB entry will be applicable for
> all the Gen12+ platforms except TGL and RKL.
>
> Since TGL and RLK are already in production so their mocs settings
> are intact to avoid ABI break.
>
> Cc: Matt Roper
> Signed-off-by: Ayaz A Siddiqui
> ---
> drivers/gpu/drm/i915/gt/intel_mocs.c | 46
> 1 file changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c
> b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index 7ccac15d9a331..0fdadefdabc29 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 unused_entries_index;
> };
>
> /* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */
> @@ -89,18 +90,25 @@ struct drm_i915_mocs_table {
> *
> * Entries not part of the following tables are undefined as far as
> * userspace is concerned and shouldn't be relied upon. For Gen < 12
> - * they will be initialized to PTE. Gen >= 12 onwards don't have a setting
> for
> - * PTE and will be initialized to an invalid value.
> + * they will be initialized to PTE. Gen >= 12 don't have a setting for
> + * PTE and those platforms except TGL/RKL will be initialized L3 WB to
> + * catch accidental use of reserved and unused mocs indexes.
> *
> * The last few entries are reserved by the hardware. For ICL+ they
> * should be initialized according to bspec and never used, for older
> * platforms they should never be written to.
> *
> - * NOTE: These tables are part of bspec and defined as part of hardware
> + * NOTE1: These tables are part of bspec and defined as part of hardware
> * interface for ICL+. For older platforms, they are part of kernel
> * ABI. It is expected that, for specific hardware platform, existing
> * entries will remain constant and the table will only be updated by
> * adding new entries, filling unused positions.
> + *
> + * NOTE2: For GEN >= 12 except TGL and RKL, reserved and unspecified MOCS
> + * indices have been set to L3 WB. These reserved entries should never
> + * be used, they may be changed to low performant variants with better
> + * coherency in the future if more entries are needed.
> + * For TGL/RKL, all the unspecified MOCS indexes are mapped to L3 UC.
> */
> #define GEN9_MOCS_ENTRIES \
> MOCS_ENTRY(I915_MOCS_UNCACHED, \
> @@ -283,17 +291,9 @@ static const struct drm_i915_mocs_entry icl_mocs_table[]
> = {
> };
>
> static const struct drm_i915_mocs_entry dg1_mocs_table[] = {
> - /* Error */
> - MOCS_ENTRY(0, 0, L3_0_DIRECT),
>
> /* UC */
> MOCS_ENTRY(1, 0, L3_1_UC),
> -
> - /* Reserved */
> - MOCS_ENTRY(2, 0, L3_0_DIRECT),
> - MOCS_ENTRY(3, 0, L3_0_DIRECT),
> - MOCS_ENTRY(4, 0, L3_0_DIRECT),
> -
> /* WB - L3 */
> MOCS_ENTRY(5, 0, L3_3_WB),
> /* WB - L3 50% */
> @@ -343,16 +343,22 @@ 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_DG1(i915)) {
> table->size = ARRAY_SIZE(dg1_mocs_table);
> table->table = dg1_mocs_table;
> table->uc_index = 1;
> table->n_entries = GEN9_NUM_MOCS_ENTRIES;
> + table->uc_index = 1;
> + table->unused_entries_index = 5;
> } else if (GRAPHICS_VER(i915) >= 12) {
> table->size = ARRAY_SIZE(tgl_mocs_table);
> table->table = tgl_mocs_table;
> table->n_entries = GEN9_NUM_MOCS_ENTRIES;
> table->uc_index = 3;
> + /* For TGL/RKL, Can't be changed now for ABI reasons */
> + if (!IS_TIGERLAKE(i915) || !IS_ROCKETLAKE(i915))
I think you meant '&&' here rather than '||' (otherwise this statement
will always evaluate to true).
With that fixed,
Reviewed-by: Matt Roper
> + table->unused_entries_index = 2;
> } else if (GRAPHICS_VER(i915) == 11) {
> table->size = ARRAY_SIZE(icl_mocs_table);
> table->table = icl_mocs_table;
> @@ -398,16 +404,16 @@ static unsigned int get_mocs_settings(const struct
> drm_i915_private *i915,
> }
>
> /*
> - * Get control_value from MOCS entry taking into account when it's
[Intel-gfx] [PATCH V4 4/6] drm/i915/gt: Initialize unused MOCS entries with device specific values
Historically we've initialized all undefined/reserved entries in
a platform's MOCS table to the contents of table entry #1 (i.e.,
I915_MOCS_PTE).
Going forward, we can't assume that table entry #1 will always
contain suitable values to use for undefined/reserved table
indices. We'll allow a platform-specific table index to be
selected at table initialization time in these cases.
This new mechanism to select L3 WB entry will be applicable for
all the Gen12+ platforms except TGL and RKL.
Since TGL and RLK are already in production so their mocs settings
are intact to avoid ABI break.
Cc: Matt Roper
Signed-off-by: Ayaz A Siddiqui
---
drivers/gpu/drm/i915/gt/intel_mocs.c | 46
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c
b/drivers/gpu/drm/i915/gt/intel_mocs.c
index 7ccac15d9a331..0fdadefdabc29 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 unused_entries_index;
};
/* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */
@@ -89,18 +90,25 @@ struct drm_i915_mocs_table {
*
* Entries not part of the following tables are undefined as far as
* userspace is concerned and shouldn't be relied upon. For Gen < 12
- * they will be initialized to PTE. Gen >= 12 onwards don't have a setting for
- * PTE and will be initialized to an invalid value.
+ * they will be initialized to PTE. Gen >= 12 don't have a setting for
+ * PTE and those platforms except TGL/RKL will be initialized L3 WB to
+ * catch accidental use of reserved and unused mocs indexes.
*
* The last few entries are reserved by the hardware. For ICL+ they
* should be initialized according to bspec and never used, for older
* platforms they should never be written to.
*
- * NOTE: These tables are part of bspec and defined as part of hardware
+ * NOTE1: These tables are part of bspec and defined as part of hardware
* interface for ICL+. For older platforms, they are part of kernel
* ABI. It is expected that, for specific hardware platform, existing
* entries will remain constant and the table will only be updated by
* adding new entries, filling unused positions.
+ *
+ * NOTE2: For GEN >= 12 except TGL and RKL, reserved and unspecified MOCS
+ * indices have been set to L3 WB. These reserved entries should never
+ * be used, they may be changed to low performant variants with better
+ * coherency in the future if more entries are needed.
+ * For TGL/RKL, all the unspecified MOCS indexes are mapped to L3 UC.
*/
#define GEN9_MOCS_ENTRIES \
MOCS_ENTRY(I915_MOCS_UNCACHED, \
@@ -283,17 +291,9 @@ static const struct drm_i915_mocs_entry icl_mocs_table[] =
{
};
static const struct drm_i915_mocs_entry dg1_mocs_table[] = {
- /* Error */
- MOCS_ENTRY(0, 0, L3_0_DIRECT),
/* UC */
MOCS_ENTRY(1, 0, L3_1_UC),
-
- /* Reserved */
- MOCS_ENTRY(2, 0, L3_0_DIRECT),
- MOCS_ENTRY(3, 0, L3_0_DIRECT),
- MOCS_ENTRY(4, 0, L3_0_DIRECT),
-
/* WB - L3 */
MOCS_ENTRY(5, 0, L3_3_WB),
/* WB - L3 50% */
@@ -343,16 +343,22 @@ 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_DG1(i915)) {
table->size = ARRAY_SIZE(dg1_mocs_table);
table->table = dg1_mocs_table;
table->uc_index = 1;
table->n_entries = GEN9_NUM_MOCS_ENTRIES;
+ table->uc_index = 1;
+ table->unused_entries_index = 5;
} else if (GRAPHICS_VER(i915) >= 12) {
table->size = ARRAY_SIZE(tgl_mocs_table);
table->table = tgl_mocs_table;
table->n_entries = GEN9_NUM_MOCS_ENTRIES;
table->uc_index = 3;
+ /* For TGL/RKL, Can't be changed now for ABI reasons */
+ if (!IS_TIGERLAKE(i915) || !IS_ROCKETLAKE(i915))
+ table->unused_entries_index = 2;
} else if (GRAPHICS_VER(i915) == 11) {
table->size = ARRAY_SIZE(icl_mocs_table);
table->table = icl_mocs_table;
@@ -398,16 +404,16 @@ static unsigned int get_mocs_settings(const struct
drm_i915_private *i915,
}
/*
- * Get control_value from MOCS entry taking into account when it's not used:
- * I915_MOCS_PTE's value is returned in this case.
+ * Get control_value from MOCS entry taking into account when it's not used
+ * then if unused_entries_index is non-zero then its value will be returned
+ * otherwise I915_MOCS_PTE's value is returned in this case.
*/
static u32 get_entry_control(const struct drm_i915_mocs_table *table,
