[Intel-gfx] [PATCH] drm/i915/tgl: MOCS table fixes

[Matt Roper]

On Mon, Nov 11, 2019 at 12:37:30PM -0800, Lucas De Marchi wrote:
> 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 at intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > Signed-off-by: Matt Roper <matthew.d.roper at 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.

Makes sense.  And we should probably switch the existing reserved
entries (0 and 1) over to that as well?


Matt

> 
> 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
> > 

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