Re: [Intel-gfx] [PATCH] drm/i915: Set all undefined MOCS entries to follow PTE
On Wed, Jun 28, 2017 at 11:19:21AM +0100, Chris Wilson wrote: > Quoting Francisco Jerez (2017-05-04 21:59:44) > > Chris Wilson writes: > > > > > On Thu, May 04, 2017 at 10:56:54AM -0700, Francisco Jerez wrote: > > >> David Weinehall writes: > > >> > > >> > On Thu, May 04, 2017 at 10:51:29AM +0100, Chris Wilson wrote: > > >> >> A good default for garbage entries from the user is to follow the > > >> >> default setting of the object (i.e. the PTE). Currently they use the > > >> >> uncached entry, and now the only way to accidentally hit uncached > > >> >> performance is via explicit use of the uncached MOCS or setting the > > >> >> object to uncached. Note that these entries are currently undefined in > > >> >> the ABI and we reserve the right to change them. We originally chose > > >> >> uncached to eliminate any problem with reducing the caching level in > > >> >> future, but the object is a much better definition of the minimum > > >> >> caching level. > > >> >> > > >> > > >> NAK. The reason for the default being UC is that it's the only setting > > >> that guarantees full forwards compatibility with any other entry that > > >> might be added in the future. If you default to PTE on (e)LLC and WB on > > >> L3, userspace will no longer be able to use any newly introduced entry > > >> with stricter coherency guarantees than that (e.g. any L3-uncached > > >> entry) in a backwards-compatible way. Attempting to do so may break > > >> memory coherency assumptions of the application and lead to misrendering > > >> when run on older kernel versions (which to my judgment is a scarier > > >> failure mode than reduced performance). Tbh, if you use a MOCS entry that's not set that's a userspace bug and needs to be fixed there. I don't think we should frob the settings of the unused entries. The rough plan is to have a GETPARAM to give you the version of the MOCS table, with the idea we'll only add new entries (on any giving pci id) at the end (and hope it'll never get full, but that seems to be the case looking at the bsepc recommended mocs entries). If userspace wants to use more than what's there at power on (or when we declare the i915.ko support stable and enable it by default), then it must consult the mocs table. Imo mocs entries shouldn't change once we've made that slot part of the uapi, that's just too much headaches. media-sdk userspace is in the process of getting fixed to work like that. Still a reason not to move forward with this patch, but a different one :-) > > > You can't use a weaker coherency model in mocs than that specified for > > > the object as you can't control other uses of the object (even just > > > memory pressure will break your assumptions). > > > > Exactly, but you can use a stronger coherency model than the application > > requested, which is why falling back to UC should generally work for > > unknown entries but falling back to PTE+WB isn't guaranteed to. > > Still wrong. GEM will write into the CPU cache believing the object is > coherent. The GPU will read from memory bypassing the CPU cache > following the UC mocs. The only safe option is for it to follow PTE. Userspace is already allowed to shoot it's feet off with corrupted cachelines when e.g. not calling set_domain when it should. Same applies if it plays games on the gpu side. The only thing we need to ensure from the kernel pov is that gpu caches are fully flushed (either to memory or cpu coherent caches) when the batch is over, to make sure the gpu doesn't scribble over memory after we've swapped it out. We might swap out garbage, but that's not the kernel's problem. Or do I miss something, and userspace can (ab)use this to pull the kernel over the table? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set all undefined MOCS entries to follow PTE
Chris Wilson writes: > Quoting Francisco Jerez (2017-05-04 21:59:44) >> Chris Wilson writes: >> >> > On Thu, May 04, 2017 at 10:56:54AM -0700, Francisco Jerez wrote: >> >> David Weinehall writes: >> >> >> >> > On Thu, May 04, 2017 at 10:51:29AM +0100, Chris Wilson wrote: >> >> >> A good default for garbage entries from the user is to follow the >> >> >> default setting of the object (i.e. the PTE). Currently they use the >> >> >> uncached entry, and now the only way to accidentally hit uncached >> >> >> performance is via explicit use of the uncached MOCS or setting the >> >> >> object to uncached. Note that these entries are currently undefined in >> >> >> the ABI and we reserve the right to change them. We originally chose >> >> >> uncached to eliminate any problem with reducing the caching level in >> >> >> future, but the object is a much better definition of the minimum >> >> >> caching level. >> >> >> >> >> >> >> NAK. The reason for the default being UC is that it's the only setting >> >> that guarantees full forwards compatibility with any other entry that >> >> might be added in the future. If you default to PTE on (e)LLC and WB on >> >> L3, userspace will no longer be able to use any newly introduced entry >> >> with stricter coherency guarantees than that (e.g. any L3-uncached >> >> entry) in a backwards-compatible way. Attempting to do so may break >> >> memory coherency assumptions of the application and lead to misrendering >> >> when run on older kernel versions (which to my judgment is a scarier >> >> failure mode than reduced performance). >> > >> > You can't use a weaker coherency model in mocs than that specified for >> > the object as you can't control other uses of the object (even just >> > memory pressure will break your assumptions). >> >> Exactly, but you can use a stronger coherency model than the application >> requested, which is why falling back to UC should generally work for >> unknown entries but falling back to PTE+WB isn't guaranteed to. > > Still wrong. GEM will write into the CPU cache believing the object is > coherent. The GPU will read from memory bypassing the CPU cache > following the UC mocs. I agree that this is a plausible scenario. > The only safe option is for it to follow PTE. Except you don't know whether the client reading or writing at the other end is the CPU, or whether the client at the other end is (set up to be) LLC-coherent. There's likely no 100% safe option on the LLC side of things. I could probably be convinced that in a number of scenarios PTE on LLC has somewhat better chances of success, but on the L3 side of things this patch enables WB which is AFAIA strictly more weakly coherent than UC, so it still gets my NAK. > -Chris signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set all undefined MOCS entries to follow PTE
Quoting Francisco Jerez (2017-05-04 21:59:44) > Chris Wilson writes: > > > On Thu, May 04, 2017 at 10:56:54AM -0700, Francisco Jerez wrote: > >> David Weinehall writes: > >> > >> > On Thu, May 04, 2017 at 10:51:29AM +0100, Chris Wilson wrote: > >> >> A good default for garbage entries from the user is to follow the > >> >> default setting of the object (i.e. the PTE). Currently they use the > >> >> uncached entry, and now the only way to accidentally hit uncached > >> >> performance is via explicit use of the uncached MOCS or setting the > >> >> object to uncached. Note that these entries are currently undefined in > >> >> the ABI and we reserve the right to change them. We originally chose > >> >> uncached to eliminate any problem with reducing the caching level in > >> >> future, but the object is a much better definition of the minimum > >> >> caching level. > >> >> > >> > >> NAK. The reason for the default being UC is that it's the only setting > >> that guarantees full forwards compatibility with any other entry that > >> might be added in the future. If you default to PTE on (e)LLC and WB on > >> L3, userspace will no longer be able to use any newly introduced entry > >> with stricter coherency guarantees than that (e.g. any L3-uncached > >> entry) in a backwards-compatible way. Attempting to do so may break > >> memory coherency assumptions of the application and lead to misrendering > >> when run on older kernel versions (which to my judgment is a scarier > >> failure mode than reduced performance). > > > > You can't use a weaker coherency model in mocs than that specified for > > the object as you can't control other uses of the object (even just > > memory pressure will break your assumptions). > > Exactly, but you can use a stronger coherency model than the application > requested, which is why falling back to UC should generally work for > unknown entries but falling back to PTE+WB isn't guaranteed to. Still wrong. GEM will write into the CPU cache believing the object is coherent. The GPU will read from memory bypassing the CPU cache following the UC mocs. The only safe option is for it to follow PTE. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set all undefined MOCS entries to follow PTE
Chris Wilson writes: > On Thu, May 04, 2017 at 10:56:54AM -0700, Francisco Jerez wrote: >> David Weinehall writes: >> >> > On Thu, May 04, 2017 at 10:51:29AM +0100, Chris Wilson wrote: >> >> A good default for garbage entries from the user is to follow the >> >> default setting of the object (i.e. the PTE). Currently they use the >> >> uncached entry, and now the only way to accidentally hit uncached >> >> performance is via explicit use of the uncached MOCS or setting the >> >> object to uncached. Note that these entries are currently undefined in >> >> the ABI and we reserve the right to change them. We originally chose >> >> uncached to eliminate any problem with reducing the caching level in >> >> future, but the object is a much better definition of the minimum >> >> caching level. >> >> >> >> NAK. The reason for the default being UC is that it's the only setting >> that guarantees full forwards compatibility with any other entry that >> might be added in the future. If you default to PTE on (e)LLC and WB on >> L3, userspace will no longer be able to use any newly introduced entry >> with stricter coherency guarantees than that (e.g. any L3-uncached >> entry) in a backwards-compatible way. Attempting to do so may break >> memory coherency assumptions of the application and lead to misrendering >> when run on older kernel versions (which to my judgment is a scarier >> failure mode than reduced performance). > > You can't use a weaker coherency model in mocs than that specified for > the object as you can't control other uses of the object (even just > memory pressure will break your assumptions). Exactly, but you can use a stronger coherency model than the application requested, which is why falling back to UC should generally work for unknown entries but falling back to PTE+WB isn't guaranteed to. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set all undefined MOCS entries to follow PTE
On Thu, May 04, 2017 at 10:56:54AM -0700, Francisco Jerez wrote: > David Weinehall writes: > > > On Thu, May 04, 2017 at 10:51:29AM +0100, Chris Wilson wrote: > >> A good default for garbage entries from the user is to follow the > >> default setting of the object (i.e. the PTE). Currently they use the > >> uncached entry, and now the only way to accidentally hit uncached > >> performance is via explicit use of the uncached MOCS or setting the > >> object to uncached. Note that these entries are currently undefined in > >> the ABI and we reserve the right to change them. We originally chose > >> uncached to eliminate any problem with reducing the caching level in > >> future, but the object is a much better definition of the minimum > >> caching level. > >> > > NAK. The reason for the default being UC is that it's the only setting > that guarantees full forwards compatibility with any other entry that > might be added in the future. If you default to PTE on (e)LLC and WB on > L3, userspace will no longer be able to use any newly introduced entry > with stricter coherency guarantees than that (e.g. any L3-uncached > entry) in a backwards-compatible way. Attempting to do so may break > memory coherency assumptions of the application and lead to misrendering > when run on older kernel versions (which to my judgment is a scarier > failure mode than reduced performance). You can't use a weaker coherency model in mocs than that specified for the object as you can't control other uses of the object (even just memory pressure will break your assumptions). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set all undefined MOCS entries to follow PTE
David Weinehall writes: > On Thu, May 04, 2017 at 10:51:29AM +0100, Chris Wilson wrote: >> A good default for garbage entries from the user is to follow the >> default setting of the object (i.e. the PTE). Currently they use the >> uncached entry, and now the only way to accidentally hit uncached >> performance is via explicit use of the uncached MOCS or setting the >> object to uncached. Note that these entries are currently undefined in >> the ABI and we reserve the right to change them. We originally chose >> uncached to eliminate any problem with reducing the caching level in >> future, but the object is a much better definition of the minimum >> caching level. >> NAK. The reason for the default being UC is that it's the only setting that guarantees full forwards compatibility with any other entry that might be added in the future. If you default to PTE on (e)LLC and WB on L3, userspace will no longer be able to use any newly introduced entry with stricter coherency guarantees than that (e.g. any L3-uncached entry) in a backwards-compatible way. Attempting to do so may break memory coherency assumptions of the application and lead to misrendering when run on older kernel versions (which to my judgment is a scarier failure mode than reduced performance). My other concern is that this change may make inadvertent use of undefined MOCS entries extremely difficult to detect in some cases -- UC gives userspace a pretty obvious (if functionally harmless) indicative that it's got its caching settings wrong, and is a strong motivation for userspace developers to contribute MOCS table changes to the kernel instead of blindly making assumptions about them (e.g. that they match the Android kernel as media-sdk was probably doing). With this change checked in, the performance drawback from using media-sdk on an upstream kernel may have been subtle enough that David would never have bothered to look into the issue. People may have started shipping copies of media-sdk making bogus MOCS table assumptions (with potential correctness implications), at which point you would have to deal with userspace regressions anytime the MOCS table is extended in the future. >> Fixes: 3bbaba0ceaa2 ("drm/i915: Added Programming of the MOCS") >> Signed-off-by: Chris Wilson >> Cc: David Weinehall >> Cc: Arkadiusz Hiler >> Cc: Tvrtko Ursulin >> Cc: sta...@vger.kernel.org > > LGTM, and passes our nightly msdk test case. > > Tested-by: David Weinehall > Reviewed-by: David Weinehall > >> --- >> drivers/gpu/drm/i915/intel_mocs.c | 39 >> +++ >> 1 file changed, 15 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_mocs.c >> b/drivers/gpu/drm/i915/intel_mocs.c >> index 92e461c68385..e7a7781ca457 100644 >> --- a/drivers/gpu/drm/i915/intel_mocs.c >> +++ b/drivers/gpu/drm/i915/intel_mocs.c >> @@ -85,10 +85,7 @@ 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 the time >> - * being they will be implicitly initialized to the strictest caching >> - * configuration (uncached) to guarantee forwards compatibility with >> - * userspace programs written against more recent kernels providing >> - * additional MOCS entries. >> + * being they will be implicitly initialized to follow the PTE. >> * >> * NOTE: These tables MUST start with being uncached and the length >> * MUST be less than 63 as the last two registers are reserved >> @@ -249,16 +246,13 @@ int intel_mocs_init_engine(struct intel_engine_cs >> *engine) >> table.table[index].control_value); >> >> /* >> - * Ok, now set the unused entries to uncached. These entries >> + * Ok, now set the unused entries to follow the PTE. These entries >> * are officially undefined and no contract for the contents >> * and settings is given for these entries. >> - * >> - * Entry 0 in the table is uncached - so we are just writing >> - * that value to all the used entries. >> */ >> for (; index < GEN9_NUM_MOCS_ENTRIES; index++) >> I915_WRITE(mocs_register(engine->id, index), >> - table.table[0].control_value); >> + table.table[I915_MOCS_PTE].control_value); >> >> return 0; >> } >> @@ -295,16 +289,13 @@ static int emit_mocs_control_table(struct >> drm_i915_gem_request *req, >> } >> >> /* >> - * Ok, now set the unused entries to uncached. These entries >> + * Ok, now set the unused entries to follow the PTE. These entries >> * are officially undefined and no contract for the contents >> * and settings is given for these entries. >> - * >> - * Entry 0 in the table is uncached - so we are just writing >> - * that value to all the used entries. >> */ >> for (; index < GEN9_NUM_MOCS_ENTRIES; index++) { >>
Re: [Intel-gfx] [PATCH] drm/i915: Set all undefined MOCS entries to follow PTE
On Thu, May 04, 2017 at 10:51:29AM +0100, Chris Wilson wrote: > A good default for garbage entries from the user is to follow the > default setting of the object (i.e. the PTE). Currently they use the > uncached entry, and now the only way to accidentally hit uncached > performance is via explicit use of the uncached MOCS or setting the > object to uncached. Note that these entries are currently undefined in > the ABI and we reserve the right to change them. We originally chose > uncached to eliminate any problem with reducing the caching level in > future, but the object is a much better definition of the minimum > caching level. > > Fixes: 3bbaba0ceaa2 ("drm/i915: Added Programming of the MOCS") > Signed-off-by: Chris Wilson > Cc: David Weinehall > Cc: Arkadiusz Hiler > Cc: Tvrtko Ursulin > Cc: sta...@vger.kernel.org LGTM, and passes our nightly msdk test case. Tested-by: David Weinehall Reviewed-by: David Weinehall > --- > drivers/gpu/drm/i915/intel_mocs.c | 39 > +++ > 1 file changed, 15 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c > b/drivers/gpu/drm/i915/intel_mocs.c > index 92e461c68385..e7a7781ca457 100644 > --- a/drivers/gpu/drm/i915/intel_mocs.c > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -85,10 +85,7 @@ 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 the time > - * being they will be implicitly initialized to the strictest caching > - * configuration (uncached) to guarantee forwards compatibility with > - * userspace programs written against more recent kernels providing > - * additional MOCS entries. > + * being they will be implicitly initialized to follow the PTE. > * > * NOTE: These tables MUST start with being uncached and the length > * MUST be less than 63 as the last two registers are reserved > @@ -249,16 +246,13 @@ int intel_mocs_init_engine(struct intel_engine_cs > *engine) > table.table[index].control_value); > > /* > - * Ok, now set the unused entries to uncached. These entries > + * Ok, now set the unused entries to follow the PTE. These entries >* are officially undefined and no contract for the contents >* and settings is given for these entries. > - * > - * Entry 0 in the table is uncached - so we are just writing > - * that value to all the used entries. >*/ > for (; index < GEN9_NUM_MOCS_ENTRIES; index++) > I915_WRITE(mocs_register(engine->id, index), > -table.table[0].control_value); > +table.table[I915_MOCS_PTE].control_value); > > return 0; > } > @@ -295,16 +289,13 @@ static int emit_mocs_control_table(struct > drm_i915_gem_request *req, > } > > /* > - * Ok, now set the unused entries to uncached. These entries > + * Ok, now set the unused entries to follow the PTE. These entries >* are officially undefined and no contract for the contents >* and settings is given for these entries. > - * > - * Entry 0 in the table is uncached - so we are just writing > - * that value to all the used entries. >*/ > for (; index < GEN9_NUM_MOCS_ENTRIES; index++) { > *cs++ = i915_mmio_reg_offset(mocs_register(engine, index)); > - *cs++ = table->table[0].control_value; > + *cs++ = table->table[I915_MOCS_PTE].control_value; > } > > *cs++ = MI_NOOP; > @@ -355,18 +346,17 @@ static int emit_mocs_l3cc_table(struct > drm_i915_gem_request *req, > if (table->size & 0x01) { > /* Odd table size - 1 left over */ > *cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i)); > - *cs++ = l3cc_combine(table, 2 * i, 0); > + *cs++ = l3cc_combine(table, 2 * i, I915_MOCS_PTE); > i++; > } > > /* > - * Now set the rest of the table to uncached - use entry 0 as > - * this will be uncached. Leave the last pair uninitialised as > - * they are reserved by the hardware. > + * Now set the rest of the table to follow the PTE. > + * Leave the last pair as they are reserved by the hardware. >*/ > for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) { > *cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i)); > - *cs++ = l3cc_combine(table, 0, 0); > + *cs++ = l3cc_combine(table, I915_MOCS_PTE, I915_MOCS_PTE); > } > > *cs++ = MI_NOOP; > @@ -402,17 +392,18 @@ void intel_mocs_init_l3cc_table(struct drm_i915_private > *dev_priv) > > /* Odd table size - 1 left over */ > if (table.size & 0x01) { > - I915_WRITE(GEN9_LNCFCMOCS(i), l3cc_combine(&table, 2*i, 0)); > + I915_WRITE(GEN9_LNCFCMOCS(i), > +l3cc_combine(&table, 2*i, I915
[Intel-gfx] [PATCH] drm/i915: Set all undefined MOCS entries to follow PTE
A good default for garbage entries from the user is to follow the default setting of the object (i.e. the PTE). Currently they use the uncached entry, and now the only way to accidentally hit uncached performance is via explicit use of the uncached MOCS or setting the object to uncached. Note that these entries are currently undefined in the ABI and we reserve the right to change them. We originally chose uncached to eliminate any problem with reducing the caching level in future, but the object is a much better definition of the minimum caching level. Fixes: 3bbaba0ceaa2 ("drm/i915: Added Programming of the MOCS") Signed-off-by: Chris Wilson Cc: David Weinehall Cc: Arkadiusz Hiler Cc: Tvrtko Ursulin Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/intel_mocs.c | 39 +++ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c index 92e461c68385..e7a7781ca457 100644 --- a/drivers/gpu/drm/i915/intel_mocs.c +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -85,10 +85,7 @@ 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 the time - * being they will be implicitly initialized to the strictest caching - * configuration (uncached) to guarantee forwards compatibility with - * userspace programs written against more recent kernels providing - * additional MOCS entries. + * being they will be implicitly initialized to follow the PTE. * * NOTE: These tables MUST start with being uncached and the length * MUST be less than 63 as the last two registers are reserved @@ -249,16 +246,13 @@ int intel_mocs_init_engine(struct intel_engine_cs *engine) table.table[index].control_value); /* -* Ok, now set the unused entries to uncached. These entries +* Ok, now set the unused entries to follow the PTE. These entries * are officially undefined and no contract for the contents * and settings is given for these entries. -* -* Entry 0 in the table is uncached - so we are just writing -* that value to all the used entries. */ for (; index < GEN9_NUM_MOCS_ENTRIES; index++) I915_WRITE(mocs_register(engine->id, index), - table.table[0].control_value); + table.table[I915_MOCS_PTE].control_value); return 0; } @@ -295,16 +289,13 @@ static int emit_mocs_control_table(struct drm_i915_gem_request *req, } /* -* Ok, now set the unused entries to uncached. These entries +* Ok, now set the unused entries to follow the PTE. These entries * are officially undefined and no contract for the contents * and settings is given for these entries. -* -* Entry 0 in the table is uncached - so we are just writing -* that value to all the used entries. */ for (; index < GEN9_NUM_MOCS_ENTRIES; index++) { *cs++ = i915_mmio_reg_offset(mocs_register(engine, index)); - *cs++ = table->table[0].control_value; + *cs++ = table->table[I915_MOCS_PTE].control_value; } *cs++ = MI_NOOP; @@ -355,18 +346,17 @@ static int emit_mocs_l3cc_table(struct drm_i915_gem_request *req, if (table->size & 0x01) { /* Odd table size - 1 left over */ *cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i)); - *cs++ = l3cc_combine(table, 2 * i, 0); + *cs++ = l3cc_combine(table, 2 * i, I915_MOCS_PTE); i++; } /* -* Now set the rest of the table to uncached - use entry 0 as -* this will be uncached. Leave the last pair uninitialised as -* they are reserved by the hardware. +* Now set the rest of the table to follow the PTE. +* Leave the last pair as they are reserved by the hardware. */ for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) { *cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(i)); - *cs++ = l3cc_combine(table, 0, 0); + *cs++ = l3cc_combine(table, I915_MOCS_PTE, I915_MOCS_PTE); } *cs++ = MI_NOOP; @@ -402,17 +392,18 @@ void intel_mocs_init_l3cc_table(struct drm_i915_private *dev_priv) /* Odd table size - 1 left over */ if (table.size & 0x01) { - I915_WRITE(GEN9_LNCFCMOCS(i), l3cc_combine(&table, 2*i, 0)); + I915_WRITE(GEN9_LNCFCMOCS(i), + l3cc_combine(&table, 2*i, I915_MOCS_PTE)); i++; } /* -* Now set the rest of the table to uncached - use entry 0 as -* this will be uncached. Leave the last pair as initialised as -* they are reserved by the hardware. +* Now set the rest of th