[Intel-xe] [PATCH 1/6] drm/xe/mocs: Drop unwanted TGL table

[Lucas De Marchi]

On Thu, Feb 16, 2023 at 03:17:19PM -0800, Matt Roper wrote:
>TGL/RKL/ADLS/ADLP are all supposed to use the same MOCS table, with
>values defined in the bspec.  Any entries listed in the bspec as
>reserved/error/undefined should always be initialized to the most cached
>and least coherent setting possible so that any userspace accidentally
>referencing those undefined entries will only experience an increase in
>coherency if spec updates down the road start defining real values.
>
>The TGL and gen12 tables that exist in the driver today are identical
>except that the TGL includes one additional (incorrect) setting for PAT
>index 1.  This is a holdover from i915 where the platform was enabled
>with an incorrect setting and by the time we noticed, it was too late to
>fix the table without breaking ABI compatibility (and on TGL we did
>indeed have some buggy userspace that was referencing the 'reserved'
>entry 1).  Since the Xe driver starts fresh with a clean slate on ABI,
>there's no need to repeat the mistakes of i915 here.
>
>Bspec: 45101
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>---
> drivers/gpu/drm/xe/xe_mocs.c | 46 ------------------------------------
> 1 file changed, 46 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_mocs.c b/drivers/gpu/drm/xe/xe_mocs.c
>index 86b966fffbe5..09adb69d1545 100644
>--- a/drivers/gpu/drm/xe/xe_mocs.c
>+++ b/drivers/gpu/drm/xe/xe_mocs.c
>@@ -247,47 +247,6 @@ struct xe_mocs_info {
> 		LE_3_WB | LE_TC_1_LLC | LE_LRUM(3), \
> 		L3_1_UC)
>
>-static const struct xe_mocs_entry tgl_mocs_desc[] = {
>-	/*
>-	 * NOTE:
>-	 * Reserved and unspecified MOCS indices have been set to (L3 + LCC).
>-	 * 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. We are programming index XE_MOCS_PTE(1)
>-	 * only, __init_mocs_table() take care to program unused index with
>-	 * this entry.
>-	 */
>-	MOCS_ENTRY(XE_MOCS_PTE,
>-		   LE_0_PAGETABLE | LE_TC_0_PAGETABLE,
>-		   L3_1_UC),
>-	GEN11_MOCS_ENTRIES,
>-
>-	/* Implicitly enable L1 - HDC:L1 + L3 + LLC */
>-	MOCS_ENTRY(48,
>-		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3),
>-		   L3_3_WB),
>-	/* Implicitly enable L1 - HDC:L1 + L3 */
>-	MOCS_ENTRY(49,
>-		   LE_1_UC | LE_TC_1_LLC,
>-		   L3_3_WB),
>-	/* Implicitly enable L1 - HDC:L1 + LLC */
>-	MOCS_ENTRY(50,
>-		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3),
>-		   L3_1_UC),
>-	/* Implicitly enable L1 - HDC:L1 */
>-	MOCS_ENTRY(51,
>-		   LE_1_UC | LE_TC_1_LLC,
>-		   L3_1_UC),
>-	/* HW Special Case (CCS) */
>-	MOCS_ENTRY(60,
>-		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3),
>-		   L3_1_UC),
>-	/* HW Special Case (Displayable) */
>-	MOCS_ENTRY(61,
>-		   LE_1_UC | LE_TC_1_LLC,
>-		   L3_3_WB),
>-};
>-
> static const struct xe_mocs_entry dg1_mocs_desc[] = {
> 	/* UC */
> 	MOCS_ENTRY(1, 0, L3_1_UC),
>@@ -422,11 +381,6 @@ static unsigned int get_mocs_settings(struct xe_device *xe,
> 		info->unused_entries_index = 5;
> 		break;
> 	case XE_TIGERLAKE:
>-		info->size  = ARRAY_SIZE(tgl_mocs_desc);
>-		info->table = tgl_mocs_desc;
>-		info->n_entries = GEN9_NUM_MOCS_ENTRIES;
>-		info->uc_index = 3;

there is another difference here: it's not only the index 1 that is
different, but any uninitialized entry.

that unused entries would be programmed to XE_MOCS_PTE (1):
LE_0_PAGETABLE | LE_TC_0_PAGETABLE | L3_1_UC. After this patch
they are programmed to
to index 3: LE_1_UC | LE_TC_1_LLC | L3_1_UC

 From the commit message I was understanding you said it was only 1 entry
with wrong value, which as per above is not the case. Maybe reword the
commit message a little bit?

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

Lucas De Marchi

>-		break;
> 	case XE_ALDERLAKE_S:
> 	case XE_ALDERLAKE_P:
> 		info->size  = ARRAY_SIZE(gen12_mocs_desc);
>-- 
>2.39.1
>