Re: [PATCH] drm/i915/tgl: MOCS table fixes

[Lucas De Marchi <lucas.demarchi@intel.com>]

On Mon, Nov 11, 2019 at 11:07:21AM -0800, Matt Roper wrote:
>The bspec was just updated with a couple corrections to the TGL MOCS
>table.  Entries 16 and 17 are marked as reserved (overriding the value
>we inherit from GEN11_MOCS_ENTRIES) and entry 61 shouldn't have the
>LE_SCF bit applied.
>
>Note that since we're intentionally/explicitly overriding table entries
>from GEN11_MOCS_ENTRIES we should suppress the 'override-init' compiler
>warnings for this file.
>
>Bspec: 45101
>Fixes: 2ddf992179c4 ("drm/i915/tgl: Define MOCS entries for Tigerlake")
>Cc: Tomasz Lis <tomasz.lis@intel.com>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
> drivers/gpu/drm/i915/Makefile        | 1 +
> drivers/gpu/drm/i915/gt/intel_mocs.c | 6 +++++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>index e0fd10c0cfb8..8c6b5fa43473 100644
>--- a/drivers/gpu/drm/i915/Makefile
>+++ b/drivers/gpu/drm/i915/Makefile
>@@ -27,6 +27,7 @@ subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
> # Fine grained warnings disable
> CFLAGS_i915_pci.o = $(call cc-disable-warning, override-init)
> CFLAGS_display/intel_fbdev.o = $(call cc-disable-warning, override-init)
>+CFLAGS_gt/intel_mocs.o = $(call cc-disable-warning, override-init)

I'm pretty sure at some point I had a pragma push/pop to ignore
this warning just around the tables and nothing else. Probably it was
dropped in a patch revision, because git log doesn't show it.

It looks like we even define special macros __diag_push(), __diag_pop(),
__diag_ignore() for that, but it doesn't see much use in the kernel, not
sure why.

>
> subdir-ccflags-y += \
> 	$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
>diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
>index 6e881c735b20..cd72235553aa 100644
>--- a/drivers/gpu/drm/i915/gt/intel_mocs.c
>+++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
>@@ -249,6 +249,10 @@ static const struct drm_i915_mocs_entry tigerlake_mocs_table[] = {
>
> 	GEN11_MOCS_ENTRIES,
>
>+	/* Reserved (overrides values from GEN11_MOCS_ENTRIES) */
>+	MOCS_ENTRY(16, 0x0, 0x0),
>+	MOCS_ENTRY(17, 0x0, 0x0),

MOCS_ENTRY implicitly define the entry to used. What I think we need is
a way to override the used field, so it's more inline with what the
bspec is telling us to do...

probably something like

#define MOCS_ENTRY_SET_UNUSED(__idx) \
	[__idx] = { \
		.used = 0, \
	}

Difference is that in tests that read back the value and check if they
make sense, they won't test values that are marked as not used. IMO what
we should be doing for these fields.

Lucas De Marchi

>+
> 	/* Implicitly enable L1 - HDC:L1 + L3 + LLC */
> 	MOCS_ENTRY(48,
> 		   LE_3_WB | LE_TC_1_LLC | LE_LRUM(3),
>@@ -271,7 +275,7 @@ static const struct drm_i915_mocs_entry tigerlake_mocs_table[] = {
> 		   L3_1_UC),
> 	/* HW Special Case (Displayable) */
> 	MOCS_ENTRY(61,
>-		   LE_1_UC | LE_TC_1_LLC | LE_SCF(1),
>+		   LE_1_UC | LE_TC_1_LLC,
> 		   L3_3_WB),
> };
>
>-- 
>2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx