* [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry
@ 2016-07-01 13:40 Imre Deak
2016-07-01 13:40 ` [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions Imre Deak
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Imre Deak @ 2016-07-01 13:40 UTC (permalink / raw)
To: intel-gfx; +Cc: Eero Tamminen
This is v3 of [1] addressing Ville's and Chris' comments. On Daniel's
request I also discussed about these changes with Rong R Yang from the
Beignet and Yakui Zhao from the Libva team, they are CC'd.
Rong, Yakui please add your Acked-by/Tested-by if you are ok with the
changes.
I suggest merging these patches for 4.7, via drm-intel-fixes.
[1] https://lists.freedesktop.org/archives/intel-gfx/2016-April/094056.html
CC: Rong R Yang <rong.r.yang@intel.com>
CC: Yakui Zhao <yakui.zhao@intel.com>
CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Imre Deak (3):
drm/i915/gen9: Clean up MOCS table definitions
drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS
config
drm/i915: Give proper names to MOCS entries
drivers/gpu/drm/i915/intel_mocs.c | 89 +++++++++++++++++++++++++++------------
include/uapi/drm/i915_drm.h | 24 +++++++++++
2 files changed, 86 insertions(+), 27 deletions(-)
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions 2016-07-01 13:40 [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Imre Deak @ 2016-07-01 13:40 ` Imre Deak 2016-07-01 21:23 ` Bish, Jim ` (2 more replies) 2016-07-01 13:40 ` [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config Imre Deak ` (4 subsequent siblings) 5 siblings, 3 replies; 20+ messages in thread From: Imre Deak @ 2016-07-01 13:40 UTC (permalink / raw) To: intel-gfx; +Cc: Eero Tamminen Use named struct initializers for clarity. Also fix the target cache definition to reflect its role in GEN9 onwards. On GEN8 a TC value of 0 meant ELLC but on GEN9+ it means the TC and LRU controls are taken from the PTE. No functional change, igt/gem_mocs_settings still passing after this change. v2: (Chris) - Add back the hexa literals for the entries. Add note that igt/gem_mocs_settings still passes. CC: Rong R Yang <rong.r.yang@intel.com> CC: Yakui Zhao <yakui.zhao@intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_mocs.c | 88 +++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c index 3c1482b..d36e609 100644 --- a/drivers/gpu/drm/i915/intel_mocs.c +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -66,9 +66,10 @@ struct drm_i915_mocs_table { #define L3_WB 3 /* Target cache */ -#define ELLC 0 -#define LLC 1 -#define LLC_ELLC 2 +#define LE_TC_PAGETABLE 0 +#define LE_TC_LLC 1 +#define LE_TC_LLC_ELLC 2 +#define LE_TC_LLC_ELLC_ALT 3 /* * MOCS tables @@ -96,34 +97,67 @@ struct drm_i915_mocs_table { * end. */ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { - /* { 0x00000009, 0x0010 } */ - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, - /* { 0x00000038, 0x0030 } */ - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, - /* { 0x0000003b, 0x0030 } */ - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } + { /* 0x00000009 */ + .control_value = LE_CACHEABILITY(LE_UC) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + + /* 0x0010 */ + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), + }, + { + /* 0x00000038 */ + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + /* 0x0030 */ + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), + }, + { + /* 0x0000003b */ + .control_value = LE_CACHEABILITY(LE_WB) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + /* 0x0030 */ + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), + }, }; /* NOTE: the LE_TGT_CACHE is not used on Broxton */ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { - /* { 0x00000009, 0x0010 } */ - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, - /* { 0x00000038, 0x0030 } */ - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, - /* { 0x0000003b, 0x0030 } */ - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } + { + /* 0x00000009 */ + .control_value = LE_CACHEABILITY(LE_UC) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + + /* 0x0010 */ + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), + }, + { + /* 0x00000038 */ + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + + /* 0x0030 */ + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), + }, + { + /* 0x0000003b */ + .control_value = LE_CACHEABILITY(LE_WB) | + LE_TGT_CACHE(LE_TC_LLC_ELLC) | + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | + LE_PFM(0) | LE_SCF(0), + + /* 0x0030 */ + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), + }, }; /** -- 2.5.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions 2016-07-01 13:40 ` [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions Imre Deak @ 2016-07-01 21:23 ` Bish, Jim 2016-07-01 21:47 ` Francisco Jerez 2016-07-13 2:10 ` Zhao Yakui 2 siblings, 0 replies; 20+ messages in thread From: Bish, Jim @ 2016-07-01 21:23 UTC (permalink / raw) To: intel-gfx@lists.freedesktop.org, Deak, Imre; +Cc: Tamminen, Eero T like the cleanup. Perhaps someday we will add more entries and have the user space consume them :) Jim On Fri, 2016-07-01 at 16:40 +0300, Imre Deak wrote: > Use named struct initializers for clarity. Also fix the target cache > definition to reflect its role in GEN9 onwards. On GEN8 a TC value of > 0 > meant ELLC but on GEN9+ it means the TC and LRU controls are taken > from > the PTE. > > No functional change, igt/gem_mocs_settings still passing after this > change. > > v2: (Chris) > - Add back the hexa literals for the entries. > Add note that igt/gem_mocs_settings still passes. > > CC: Rong R Yang <rong.r.yang@intel.com> > CC: Yakui Zhao <yakui.zhao@intel.com> > CC: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_mocs.c | 88 +++++++++++++++++++++++++++ > ------------ > 1 file changed, 61 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c > b/drivers/gpu/drm/i915/intel_mocs.c > index 3c1482b..d36e609 100644 > --- a/drivers/gpu/drm/i915/intel_mocs.c > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -66,9 +66,10 @@ struct drm_i915_mocs_table { > #define L3_WB 3 > > /* Target cache */ > -#define ELLC 0 > -#define LLC 1 > -#define LLC_ELLC 2 > +#define LE_TC_PAGETABLE 0 > +#define LE_TC_LLC 1 > +#define LE_TC_LLC_ELLC 2 > +#define LE_TC_LLC_ELLC_ALT 3 > > /* > * MOCS tables > @@ -96,34 +97,67 @@ struct drm_i915_mocs_table { > * end. > */ > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > - /* { 0x00000009, 0x0010 } */ > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | > LE_LRUM(0) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > - /* { 0x00000038, 0x0030 } */ > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | > LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > - /* { 0x0000003b, 0x0030 } */ > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | > LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > + { /* 0x00000009 */ > + .control_value = LE_CACHEABILITY(LE_UC) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | > LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0010 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > L3_CACHEABILITY(L3_UC), > + }, > + { > + /* 0x00000038 */ > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > L3_CACHEABILITY(L3_WB), > + }, > + { > + /* 0x0000003b */ > + .control_value = LE_CACHEABILITY(LE_WB) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > L3_CACHEABILITY(L3_WB), > + }, > }; > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > - /* { 0x00000009, 0x0010 } */ > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | > LE_LRUM(0) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > - /* { 0x00000038, 0x0030 } */ > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | > LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > - /* { 0x0000003b, 0x0030 } */ > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | > LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > + { > + /* 0x00000009 */ > + .control_value = LE_CACHEABILITY(LE_UC) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | > LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0010 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > L3_CACHEABILITY(L3_UC), > + }, > + { > + /* 0x00000038 */ > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > L3_CACHEABILITY(L3_WB), > + }, > + { > + /* 0x0000003b */ > + .control_value = LE_CACHEABILITY(LE_WB) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > L3_CACHEABILITY(L3_WB), > + }, > }; > > /** _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions 2016-07-01 13:40 ` [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions Imre Deak 2016-07-01 21:23 ` Bish, Jim @ 2016-07-01 21:47 ` Francisco Jerez 2016-07-12 11:08 ` Imre Deak 2016-07-13 2:10 ` Zhao Yakui 2 siblings, 1 reply; 20+ messages in thread From: Francisco Jerez @ 2016-07-01 21:47 UTC (permalink / raw) To: Imre Deak, intel-gfx; +Cc: Eero Tamminen [-- Attachment #1.1.1: Type: text/plain, Size: 5559 bytes --] Hi Imre, Imre Deak <imre.deak@intel.com> writes: > Use named struct initializers for clarity. Also fix the target cache > definition to reflect its role in GEN9 onwards. On GEN8 a TC value of 0 > meant ELLC but on GEN9+ it means the TC and LRU controls are taken from > the PTE. > I vaguely recall bringing up this apparent inconsistency with the hardware spec during review of the original series of patches programming the MOCS tables. IIRC TC value 0 behaved as ELLC-only on the simulator, but the hardware docs claimed it would take the target cache from the PTE controls, not sure what the real hardware actually does. Maybe Peter can confirm whether this was intentional? > No functional change, igt/gem_mocs_settings still passing after this > change. > > v2: (Chris) > - Add back the hexa literals for the entries. > Add note that igt/gem_mocs_settings still passes. > > CC: Rong R Yang <rong.r.yang@intel.com> > CC: Yakui Zhao <yakui.zhao@intel.com> > CC: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_mocs.c | 88 +++++++++++++++++++++++++++------------ > 1 file changed, 61 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > index 3c1482b..d36e609 100644 > --- a/drivers/gpu/drm/i915/intel_mocs.c > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -66,9 +66,10 @@ struct drm_i915_mocs_table { > #define L3_WB 3 > > /* Target cache */ > -#define ELLC 0 > -#define LLC 1 > -#define LLC_ELLC 2 > +#define LE_TC_PAGETABLE 0 > +#define LE_TC_LLC 1 > +#define LE_TC_LLC_ELLC 2 > +#define LE_TC_LLC_ELLC_ALT 3 > > /* > * MOCS tables > @@ -96,34 +97,67 @@ struct drm_i915_mocs_table { > * end. > */ > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > - /* { 0x00000009, 0x0010 } */ > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > - /* { 0x00000038, 0x0030 } */ > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > - /* { 0x0000003b, 0x0030 } */ > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > + { /* 0x00000009 */ > + .control_value = LE_CACHEABILITY(LE_UC) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0010 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > + }, > + { > + /* 0x00000038 */ > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > + }, > + { > + /* 0x0000003b */ > + .control_value = LE_CACHEABILITY(LE_WB) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > + }, > }; > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > - /* { 0x00000009, 0x0010 } */ > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > - /* { 0x00000038, 0x0030 } */ > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > - /* { 0x0000003b, 0x0030 } */ > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > + { > + /* 0x00000009 */ > + .control_value = LE_CACHEABILITY(LE_UC) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0010 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > + }, > + { > + /* 0x00000038 */ > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > + }, > + { > + /* 0x0000003b */ > + .control_value = LE_CACHEABILITY(LE_WB) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > + }, > }; > > /** > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 212 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions 2016-07-01 21:47 ` Francisco Jerez @ 2016-07-12 11:08 ` Imre Deak 0 siblings, 0 replies; 20+ messages in thread From: Imre Deak @ 2016-07-12 11:08 UTC (permalink / raw) To: Francisco Jerez, intel-gfx; +Cc: Eero Tamminen On pe, 2016-07-01 at 14:47 -0700, Francisco Jerez wrote: > Hi Imre, > > Imre Deak <imre.deak@intel.com> writes: > > > Use named struct initializers for clarity. Also fix the target > > cache > > definition to reflect its role in GEN9 onwards. On GEN8 a TC value > > of 0 > > meant ELLC but on GEN9+ it means the TC and LRU controls are taken > > from > > the PTE. > > > I vaguely recall bringing up this apparent inconsistency with the > hardware spec during review of the original series of patches > programming the MOCS tables. IIRC TC value 0 behaved as ELLC-only on > the simulator, but the hardware docs claimed it would take the target > cache from the PTE controls, not sure what the real hardware actually > does. At least the documentation seems to be quite consistent on this. It's true that you have to set the MOCS TC to 0 for an eLLC-only mapping, but that depends on the (default) 0 of the PTE [3:2] field (override to eLLC-only). So maybe during simulation PTE was at the default eLLC- only, hence your observed behavior. In any case we don't use TC PTE passthrough for any MOCS index atm, so I would suggest keeping the code in line with the documentation for now. --Imre > Maybe Peter can confirm whether this was intentional? > > > No functional change, igt/gem_mocs_settings still passing after > > this > > change. > > > > v2: (Chris) > > - Add back the hexa literals for the entries. > > Add note that igt/gem_mocs_settings still passes. > > > > CC: Rong R Yang <rong.r.yang@intel.com> > > CC: Yakui Zhao <yakui.zhao@intel.com> > > CC: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/intel_mocs.c | 88 > > +++++++++++++++++++++++++++------------ > > 1 file changed, 61 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c > > b/drivers/gpu/drm/i915/intel_mocs.c > > index 3c1482b..d36e609 100644 > > --- a/drivers/gpu/drm/i915/intel_mocs.c > > +++ b/drivers/gpu/drm/i915/intel_mocs.c > > @@ -66,9 +66,10 @@ struct drm_i915_mocs_table { > > #define L3_WB 3 > > > > /* Target cache */ > > -#define ELLC 0 > > -#define LLC 1 > > -#define LLC_ELLC 2 > > +#define LE_TC_PAGETABLE 0 > > +#define LE_TC_LLC 1 > > +#define LE_TC_LLC_ELLC 2 > > +#define LE_TC_LLC_ELLC_ALT 3 > > > > /* > > * MOCS tables > > @@ -96,34 +97,67 @@ struct drm_i915_mocs_table { > > * end. > > */ > > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > - /* { 0x00000009, 0x0010 } */ > > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | > > LE_LRUM(0) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > > - /* { 0x00000038, 0x0030 } */ > > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) > > | LE_LRUM(3) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > > - /* { 0x0000003b, 0x0030 } */ > > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | > > LE_LRUM(3) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > > + { /* 0x00000009 */ > > + .control_value = LE_CACHEABILITY(LE_UC) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + > > + /* 0x0010 */ > > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_UC), > > + }, > > + { > > + /* 0x00000038 */ > > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + /* 0x0030 */ > > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_WB), > > + }, > > + { > > + /* 0x0000003b */ > > + .control_value = LE_CACHEABILITY(LE_WB) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + /* 0x0030 */ > > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_WB), > > + }, > > }; > > > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > > - /* { 0x00000009, 0x0010 } */ > > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | > > LE_LRUM(0) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > > - /* { 0x00000038, 0x0030 } */ > > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) > > | LE_LRUM(3) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > > - /* { 0x0000003b, 0x0030 } */ > > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | > > LE_LRUM(3) | > > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | > > LE_SCF(0)), > > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > > + { > > + /* 0x00000009 */ > > + .control_value = LE_CACHEABILITY(LE_UC) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + > > + /* 0x0010 */ > > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_UC), > > + }, > > + { > > + /* 0x00000038 */ > > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + > > + /* 0x0030 */ > > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_WB), > > + }, > > + { > > + /* 0x0000003b */ > > + .control_value = LE_CACHEABILITY(LE_WB) | > > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | > > LE_SCC(0) | > > + LE_PFM(0) | LE_SCF(0), > > + > > + /* 0x0030 */ > > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | > > L3_CACHEABILITY(L3_WB), > > + }, > > }; > > > > /** _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions 2016-07-01 13:40 ` [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions Imre Deak 2016-07-01 21:23 ` Bish, Jim 2016-07-01 21:47 ` Francisco Jerez @ 2016-07-13 2:10 ` Zhao Yakui 2 siblings, 0 replies; 20+ messages in thread From: Zhao Yakui @ 2016-07-13 2:10 UTC (permalink / raw) To: Deak, Imre; +Cc: intel-gfx@lists.freedesktop.org, Tamminen, Eero T On 07/01/2016 09:40 PM, Deak, Imre wrote: > Use named struct initializers for clarity. Also fix the target cache > definition to reflect its role in GEN9 onwards. On GEN8 a TC value of 0 > meant ELLC but on GEN9+ it means the TC and LRU controls are taken from > the PTE. > > No functional change, igt/gem_mocs_settings still passing after this > change. > > v2: (Chris) > - Add back the hexa literals for the entries. > Add note that igt/gem_mocs_settings still passes. > > CC: Rong R Yang<rong.r.yang@intel.com> > CC: Yakui Zhao<yakui.zhao@intel.com> > CC: Chris Wilson<chris@chris-wilson.co.uk> > Signed-off-by: Imre Deak<imre.deak@intel.com> It is helpful to understand the MOCS table definition after cleaning up. Add: Acked-by: Zhao Yakui <yakui.zhao@intel.com> Thanks Yakui > --- > drivers/gpu/drm/i915/intel_mocs.c | 88 +++++++++++++++++++++++++++------------ > 1 file changed, 61 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > index 3c1482b..d36e609 100644 > --- a/drivers/gpu/drm/i915/intel_mocs.c > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -66,9 +66,10 @@ struct drm_i915_mocs_table { > #define L3_WB 3 > > /* Target cache */ > -#define ELLC 0 > -#define LLC 1 > -#define LLC_ELLC 2 > +#define LE_TC_PAGETABLE 0 > +#define LE_TC_LLC 1 > +#define LE_TC_LLC_ELLC 2 > +#define LE_TC_LLC_ELLC_ALT 3 > > /* > * MOCS tables > @@ -96,34 +97,67 @@ struct drm_i915_mocs_table { > * end. > */ > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > - /* { 0x00000009, 0x0010 } */ > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > - /* { 0x00000038, 0x0030 } */ > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > - /* { 0x0000003b, 0x0030 } */ > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > + { /* 0x00000009 */ > + .control_value = LE_CACHEABILITY(LE_UC) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0010 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > + }, > + { > + /* 0x00000038 */ > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > + }, > + { > + /* 0x0000003b */ > + .control_value = LE_CACHEABILITY(LE_WB) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > + }, > }; > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > - /* { 0x00000009, 0x0010 } */ > - { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, > - /* { 0x00000038, 0x0030 } */ > - { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, > - /* { 0x0000003b, 0x0030 } */ > - { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | > - LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), > - (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } > + { > + /* 0x00000009 */ > + .control_value = LE_CACHEABILITY(LE_UC) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0010 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > + }, > + { > + /* 0x00000038 */ > + .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > + }, > + { > + /* 0x0000003b */ > + .control_value = LE_CACHEABILITY(LE_WB) | > + LE_TGT_CACHE(LE_TC_LLC_ELLC) | > + LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > + LE_PFM(0) | LE_SCF(0), > + > + /* 0x0030 */ > + .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > + }, > }; > > /** _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config 2016-07-01 13:40 [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Imre Deak 2016-07-01 13:40 ` [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions Imre Deak @ 2016-07-01 13:40 ` Imre Deak 2016-07-13 2:32 ` Zhao Yakui 2016-07-14 8:33 ` Yang, Rong R 2016-07-01 13:40 ` [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries Imre Deak ` (3 subsequent siblings) 5 siblings, 2 replies; 20+ messages in thread From: Imre Deak @ 2016-07-01 13:40 UTC (permalink / raw) To: intel-gfx; +Cc: Eero Tamminen Setting a write-back cache policy in the MOCS entry definition also implies snooping, which has a considerable overhead. This is unexpected for a few reasons: - From user-space's point of view since it didn't want a coherent surface (it didn't set the buffer as such via the set caching IOCTL). - There is a separate MOCS entry field for snooping (which we never set). - This MOCS table is about caching in (e)LLC and there is no (e)LLC on BXT. There is a separate table for L3 cache control. Considering the above the current behavior of snooping looks like an unintentional side-effect of the WB setting. Changing it to be LLC-UC gets rid of the snooping without any ill-effects. For a coherent surface the application would use a separate MOCS entry at index 1 and call the set caching IOCTL to setup the PTE entries for the corresponding buffer to be snooped. In the future we could also add a new MOCS entry for coherent surfaces. This resulted in 70% improvement in synthetic texturing benchmarks. Kudos to Valtteri Rantala, Eero Tamminen and Michael T Frederick and Ville who helped to narrow the source of problem to the kernel and to the snooping behaviour in particular. With a follow-up change to adjust the 3rd entry value igt/gem_mocs_settings is passing after this change. v2: - Rebase on v2 of patch 1/2. v3: - Set the entry as LLC uncached instead of PTE-passthrough. This way we also keep snooping disabled, but we also make the cacheability/ coherency setting indepent of the PTE which is managed by the kernel. (Chris) CC: Rong R Yang <rong.r.yang@intel.com> CC: Yakui Zhao <yakui.zhao@intel.com> CC: Valtteri Rantala <valtteri.rantala@intel.com> CC: Eero Tamminen <eero.t.tamminen@intel.com> CC: Michael T Frederick <michael.t.frederick@intel.com> CC: Ville Syrjälä <ville.syrjala@linux.intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_mocs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c index d36e609..927825f 100644 --- a/drivers/gpu/drm/i915/intel_mocs.c +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -149,8 +149,8 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), }, { - /* 0x0000003b */ - .control_value = LE_CACHEABILITY(LE_WB) | + /* 0x00000039 */ + .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0), -- 2.5.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config 2016-07-01 13:40 ` [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config Imre Deak @ 2016-07-13 2:32 ` Zhao Yakui 2016-07-14 8:33 ` Yang, Rong R 1 sibling, 0 replies; 20+ messages in thread From: Zhao Yakui @ 2016-07-13 2:32 UTC (permalink / raw) To: Deak, Imre; +Cc: intel-gfx@lists.freedesktop.org, Tamminen, Eero T On 07/01/2016 09:40 PM, Deak, Imre wrote: > Setting a write-back cache policy in the MOCS entry definition also > implies snooping, which has a considerable overhead. This is > unexpected for a few reasons: > - From user-space's point of view since it didn't want a coherent > surface (it didn't set the buffer as such via the set caching IOCTL). > - There is a separate MOCS entry field for snooping (which we never > set). > - This MOCS table is about caching in (e)LLC and there is no (e)LLC on > BXT. There is a separate table for L3 cache control. > > Considering the above the current behavior of snooping looks like an > unintentional side-effect of the WB setting. Changing it to be LLC-UC > gets rid of the snooping without any ill-effects. For a coherent > surface the application would use a separate MOCS entry at index 1 and > call the set caching IOCTL to setup the PTE entries for the > corresponding buffer to be snooped. In the future we could also add a > new MOCS entry for coherent surfaces. > > This resulted in 70% improvement in synthetic texturing benchmarks. > > Kudos to Valtteri Rantala, Eero Tamminen and Michael T Frederick and > Ville who helped to narrow the source of problem to the kernel and to > the snooping behaviour in particular. > > With a follow-up change to adjust the 3rd entry value > igt/gem_mocs_settings is passing after this change. > > v2: > - Rebase on v2 of patch 1/2. > v3: > - Set the entry as LLC uncached instead of PTE-passthrough. This way > we also keep snooping disabled, but we also make the cacheability/ > coherency setting indepent of the PTE which is managed by the > kernel. (Chris) > > CC: Rong R Yang<rong.r.yang@intel.com> > CC: Yakui Zhao<yakui.zhao@intel.com> > CC: Valtteri Rantala<valtteri.rantala@intel.com> > CC: Eero Tamminen<eero.t.tamminen@intel.com> > CC: Michael T Frederick<michael.t.frederick@intel.com> > CC: Ville Syrjälä<ville.syrjala@linux.intel.com> > CC: Chris Wilson<chris@chris-wilson.co.uk> > Signed-off-by: Imre Deak<imre.deak@intel.com> As the BXT has no LLC, setting the WB-policy will add the extra overhead. In such case the patch looks more reasonable for BXT. Add: Acked-by: Zhao Yakui <yakui.zhao@intel.com> > --- > drivers/gpu/drm/i915/intel_mocs.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > index d36e609..927825f 100644 > --- a/drivers/gpu/drm/i915/intel_mocs.c > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -149,8 +149,8 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > }, > { > - /* 0x0000003b */ > - .control_value = LE_CACHEABILITY(LE_WB) | > + /* 0x00000039 */ > + .control_value = LE_CACHEABILITY(LE_UC) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > LE_PFM(0) | LE_SCF(0), _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config 2016-07-01 13:40 ` [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config Imre Deak 2016-07-13 2:32 ` Zhao Yakui @ 2016-07-14 8:33 ` Yang, Rong R 1 sibling, 0 replies; 20+ messages in thread From: Yang, Rong R @ 2016-07-14 8:33 UTC (permalink / raw) To: Deak, Imre, intel-gfx@lists.freedesktop.org; +Cc: Tamminen, Eero T > -----Original Message----- > From: Deak, Imre > Sent: Friday, July 1, 2016 21:40 > To: intel-gfx@lists.freedesktop.org > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; Chris Wilson <chris@chris- > wilson.co.uk>; Yang, Rong R <rong.r.yang@intel.com>; Zhao, Yakui > <yakui.zhao@intel.com>; Tamminen, Eero T <eero.t.tamminen@intel.com> > Subject: [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to > incorrect MOCS config > > Setting a write-back cache policy in the MOCS entry definition also implies > snooping, which has a considerable overhead. This is unexpected for a few > reasons: > - From user-space's point of view since it didn't want a coherent > surface (it didn't set the buffer as such via the set caching IOCTL). > - There is a separate MOCS entry field for snooping (which we never > set). > - This MOCS table is about caching in (e)LLC and there is no (e)LLC on > BXT. There is a separate table for L3 cache control. > > Considering the above the current behavior of snooping looks like an > unintentional side-effect of the WB setting. Changing it to be LLC-UC gets rid > of the snooping without any ill-effects. For a coherent surface the application > would use a separate MOCS entry at index 1 and call the set caching IOCTL to > setup the PTE entries for the corresponding buffer to be snooped. In the > future we could also add a new MOCS entry for coherent surfaces. > > This resulted in 70% improvement in synthetic texturing benchmarks. > > Kudos to Valtteri Rantala, Eero Tamminen and Michael T Frederick and Ville > who helped to narrow the source of problem to the kernel and to the > snooping behaviour in particular. > > With a follow-up change to adjust the 3rd entry value > igt/gem_mocs_settings is passing after this change. > > v2: > - Rebase on v2 of patch 1/2. > v3: > - Set the entry as LLC uncached instead of PTE-passthrough. This way > we also keep snooping disabled, but we also make the cacheability/ > coherency setting indepent of the PTE which is managed by the > kernel. (Chris) About 20% improvement in OpenCL benchmark luxmark. Add: Tested-by: Rong R Yang <rong.r.yang@intel.com> > CC: Rong R Yang <rong.r.yang@intel.com> > CC: Yakui Zhao <yakui.zhao@intel.com> > CC: Valtteri Rantala <valtteri.rantala@intel.com> > CC: Eero Tamminen <eero.t.tamminen@intel.com> > CC: Michael T Frederick <michael.t.frederick@intel.com> > CC: Ville Syrjälä <ville.syrjala@linux.intel.com> > CC: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_mocs.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c > b/drivers/gpu/drm/i915/intel_mocs.c > index d36e609..927825f 100644 > --- a/drivers/gpu/drm/i915/intel_mocs.c > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -149,8 +149,8 @@ static const struct drm_i915_mocs_entry > broxton_mocs_table[] = { > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > }, > { > - /* 0x0000003b */ > - .control_value = LE_CACHEABILITY(LE_WB) | > + /* 0x00000039 */ > + .control_value = LE_CACHEABILITY(LE_UC) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > LE_LRUM(3) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > LE_PFM(0) | LE_SCF(0), > -- > 2.5.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries 2016-07-01 13:40 [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Imre Deak 2016-07-01 13:40 ` [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions Imre Deak 2016-07-01 13:40 ` [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config Imre Deak @ 2016-07-01 13:40 ` Imre Deak 2016-07-01 13:49 ` Chris Wilson ` (2 more replies) 2016-07-01 14:54 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry Patchwork ` (2 subsequent siblings) 5 siblings, 3 replies; 20+ messages in thread From: Imre Deak @ 2016-07-01 13:40 UTC (permalink / raw) To: intel-gfx; +Cc: Eero Tamminen The purpose for each MOCS entry isn't well defined atm. Defining these is important to remove any uncertainty about the use of these entries for example in terms of performance and GPU/CPU coherency. Suggested by Ville. CC: Rong R Yang <rong.r.yang@intel.com> CC: Yakui Zhao <yakui.zhao@intel.com> CC: Ville Syrjälä <ville.syrjala@linux.intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_mocs.c | 13 +++++++------ include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c index 927825f..86adc11 100644 --- a/drivers/gpu/drm/i915/intel_mocs.c +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -97,7 +97,8 @@ struct drm_i915_mocs_table { * end. */ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { - { /* 0x00000009 */ + [I915_MOCS_UNCACHED] = { + /* 0x00000009 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | @@ -106,7 +107,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* 0x0010 */ .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), }, - { + [I915_MOCS_AUTO] = { /* 0x00000038 */ .control_value = LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -115,7 +116,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* 0x0030 */ .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), }, - { + [I915_MOCS_CACHED] = { /* 0x0000003b */ .control_value = LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -128,7 +129,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* NOTE: the LE_TGT_CACHE is not used on Broxton */ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { - { + [I915_MOCS_UNCACHED] = { /* 0x00000009 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -138,7 +139,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { /* 0x0010 */ .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), }, - { + [I915_MOCS_AUTO] = { /* 0x00000038 */ .control_value = LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -148,7 +149,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { /* 0x0030 */ .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), }, - { + [I915_MOCS_CACHED] = { /* 0x00000039 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index c17d63d..a5d116f 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -62,6 +62,30 @@ extern "C" { #define I915_ERROR_UEVENT "ERROR" #define I915_RESET_UEVENT "RESET" +/* + * MOCS indexes used for GPU surfaces, defining the cacheability of the + * surface data and the coherency for this data wrt. CPU vs. GPU accesses. + */ +enum i915_mocs_table_index { + /* + * Not cached anywhere, coherency between CPU and GPU accesses is + * guaranteed. + */ + I915_MOCS_UNCACHED, + /* + * Cacheability and coherency controlled by the kernel automatically + * based on the DRM_I915_GEM_SET_CACHING IOCTL setting and the current + * usage of the surface (used for display scanout or not). + */ + I915_MOCS_AUTO, + /* + * Cached in all GPU caches available on the platform. + * Coherency between CPU and GPU accesses to the surface is not + * guaranteed without extra synchronization. + */ + I915_MOCS_CACHED, +}; + /* Each region is a minimum of 16k, and there are at most 255 of them. */ #define I915_NR_TEX_REGIONS 255 /* table size 2k - maximum due to use -- 2.5.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries 2016-07-01 13:40 ` [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries Imre Deak @ 2016-07-01 13:49 ` Chris Wilson 2016-07-01 13:56 ` Imre Deak 2016-07-01 14:32 ` [PATCH v4 " Imre Deak 2016-07-13 2:22 ` [PATCH v3 " Zhao Yakui 2 siblings, 1 reply; 20+ messages in thread From: Chris Wilson @ 2016-07-01 13:49 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx, Eero Tamminen On Fri, Jul 01, 2016 at 04:40:06PM +0300, Imre Deak wrote: > The purpose for each MOCS entry isn't well defined atm. Defining these > is important to remove any uncertainty about the use of these entries > for example in terms of performance and GPU/CPU coherency. > > Suggested by Ville. > > CC: Rong R Yang <rong.r.yang@intel.com> > CC: Yakui Zhao <yakui.zhao@intel.com> > CC: Ville Syrjälä <ville.syrjala@linux.intel.com> > CC: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_mocs.c | 13 +++++++------ > include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++++ > 2 files changed, 31 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > index 927825f..86adc11 100644 > --- a/drivers/gpu/drm/i915/intel_mocs.c > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -97,7 +97,8 @@ struct drm_i915_mocs_table { > * end. > */ > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > - { /* 0x00000009 */ > + [I915_MOCS_UNCACHED] = { > + /* 0x00000009 */ > .control_value = LE_CACHEABILITY(LE_UC) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > @@ -106,7 +107,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > /* 0x0010 */ > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > }, > - { > + [I915_MOCS_AUTO] = { > /* 0x00000038 */ > .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > @@ -115,7 +116,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > /* 0x0030 */ > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > }, > - { > + [I915_MOCS_CACHED] = { > /* 0x0000003b */ > .control_value = LE_CACHEABILITY(LE_WB) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > @@ -128,7 +129,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > - { > + [I915_MOCS_UNCACHED] = { > /* 0x00000009 */ > .control_value = LE_CACHEABILITY(LE_UC) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > @@ -138,7 +139,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > /* 0x0010 */ > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > }, > - { > + [I915_MOCS_AUTO] = { > /* 0x00000038 */ > .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > @@ -148,7 +149,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > /* 0x0030 */ > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > }, > - { > + [I915_MOCS_CACHED] = { > /* 0x00000039 */ > .control_value = LE_CACHEABILITY(LE_UC) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index c17d63d..a5d116f 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -62,6 +62,30 @@ extern "C" { > #define I915_ERROR_UEVENT "ERROR" > #define I915_RESET_UEVENT "RESET" > > +/* > + * MOCS indexes used for GPU surfaces, defining the cacheability of the > + * surface data and the coherency for this data wrt. CPU vs. GPU accesses. > + */ > +enum i915_mocs_table_index { > + /* > + * Not cached anywhere, coherency between CPU and GPU accesses is > + * guaranteed. > + */ > + I915_MOCS_UNCACHED, > + /* > + * Cacheability and coherency controlled by the kernel automatically > + * based on the DRM_I915_GEM_SET_CACHING IOCTL setting and the current > + * usage of the surface (used for display scanout or not). > + */ > + I915_MOCS_AUTO, So PTE. > + /* > + * Cached in all GPU caches available on the platform. > + * Coherency between CPU and GPU accesses to the surface is not > + * guaranteed without extra synchronization. > + */ > + I915_MOCS_CACHED, So pretty useless for its current usage then. -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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries 2016-07-01 13:49 ` Chris Wilson @ 2016-07-01 13:56 ` Imre Deak 0 siblings, 0 replies; 20+ messages in thread From: Imre Deak @ 2016-07-01 13:56 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx, Eero Tamminen On pe, 2016-07-01 at 14:49 +0100, Chris Wilson wrote: > On Fri, Jul 01, 2016 at 04:40:06PM +0300, Imre Deak wrote: > > The purpose for each MOCS entry isn't well defined atm. Defining these > > is important to remove any uncertainty about the use of these entries > > for example in terms of performance and GPU/CPU coherency. > > > > Suggested by Ville. > > > > CC: Rong R Yang <rong.r.yang@intel.com> > > CC: Yakui Zhao <yakui.zhao@intel.com> > > CC: Ville Syrjälä <ville.syrjala@linux.intel.com> > > CC: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/intel_mocs.c | 13 +++++++------ > > include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++++ > > 2 files changed, 31 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > > index 927825f..86adc11 100644 > > --- a/drivers/gpu/drm/i915/intel_mocs.c > > +++ b/drivers/gpu/drm/i915/intel_mocs.c > > @@ -97,7 +97,8 @@ struct drm_i915_mocs_table { > > * end. > > */ > > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > - { /* 0x00000009 */ > > + [I915_MOCS_UNCACHED] = { > > + /* 0x00000009 */ > > .control_value = LE_CACHEABILITY(LE_UC) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > > @@ -106,7 +107,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > /* 0x0010 */ > > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > > }, > > - { > > + [I915_MOCS_AUTO] = { > > /* 0x00000038 */ > > .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > @@ -115,7 +116,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > /* 0x0030 */ > > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > > }, > > - { > > + [I915_MOCS_CACHED] = { > > /* 0x0000003b */ > > .control_value = LE_CACHEABILITY(LE_WB) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > @@ -128,7 +129,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > > - { > > + [I915_MOCS_UNCACHED] = { > > /* 0x00000009 */ > > .control_value = LE_CACHEABILITY(LE_UC) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > @@ -138,7 +139,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > > /* 0x0010 */ > > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > > }, > > - { > > + [I915_MOCS_AUTO] = { > > /* 0x00000038 */ > > .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > @@ -148,7 +149,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > > /* 0x0030 */ > > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > > }, > > - { > > + [I915_MOCS_CACHED] = { > > /* 0x00000039 */ > > .control_value = LE_CACHEABILITY(LE_UC) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > > index c17d63d..a5d116f 100644 > > --- a/include/uapi/drm/i915_drm.h > > +++ b/include/uapi/drm/i915_drm.h > > @@ -62,6 +62,30 @@ extern "C" { > > #define I915_ERROR_UEVENT "ERROR" > > #define I915_RESET_UEVENT "RESET" > > > > +/* > > + * MOCS indexes used for GPU surfaces, defining the cacheability of the > > + * surface data and the coherency for this data wrt. CPU vs. GPU accesses. > > + */ > > +enum i915_mocs_table_index { > > + /* > > + * Not cached anywhere, coherency between CPU and GPU accesses is > > + * guaranteed. > > + */ > > + I915_MOCS_UNCACHED, > > + /* > > + * Cacheability and coherency controlled by the kernel automatically > > + * based on the DRM_I915_GEM_SET_CACHING IOCTL setting and the current > > + * usage of the surface (used for display scanout or not). > > + */ > > + I915_MOCS_AUTO, > > So PTE. Can change it. > > + /* > > + * Cached in all GPU caches available on the platform. > > + * Coherency between CPU and GPU accesses to the surface is not > > + * guaranteed without extra synchronization. > > + */ > > + I915_MOCS_CACHED, > > So pretty useless for its current usage then. This is how it's used in Mesa atm where there is no need for coherency. Beignet and Libva don't use this entry atm. --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 3/3] drm/i915: Give proper names to MOCS entries 2016-07-01 13:40 ` [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries Imre Deak 2016-07-01 13:49 ` Chris Wilson @ 2016-07-01 14:32 ` Imre Deak 2016-07-13 2:22 ` [PATCH v3 " Zhao Yakui 2 siblings, 0 replies; 20+ messages in thread From: Imre Deak @ 2016-07-01 14:32 UTC (permalink / raw) To: intel-gfx The purpose for each MOCS entry isn't well defined atm. Defining these is important to remove any uncertainty about the use of these entries for example in terms of performance and GPU/CPU coherency. Suggested by Ville. v4: - Rename I915_MOCS_AUTO to I915_MOCS_PTE. (Chris) CC: Rong R Yang <rong.r.yang@intel.com> CC: Yakui Zhao <yakui.zhao@intel.com> CC: Ville Syrjälä <ville.syrjala@linux.intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_mocs.c | 13 +++++++------ include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c index 927825f..2280c32 100644 --- a/drivers/gpu/drm/i915/intel_mocs.c +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -97,7 +97,8 @@ struct drm_i915_mocs_table { * end. */ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { - { /* 0x00000009 */ + [I915_MOCS_UNCACHED] = { + /* 0x00000009 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | @@ -106,7 +107,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* 0x0010 */ .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), }, - { + [I915_MOCS_PTE] = { /* 0x00000038 */ .control_value = LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -115,7 +116,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* 0x0030 */ .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), }, - { + [I915_MOCS_CACHED] = { /* 0x0000003b */ .control_value = LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -128,7 +129,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { /* NOTE: the LE_TGT_CACHE is not used on Broxton */ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { - { + [I915_MOCS_UNCACHED] = { /* 0x00000009 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -138,7 +139,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { /* 0x0010 */ .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), }, - { + [I915_MOCS_PTE] = { /* 0x00000038 */ .control_value = LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | @@ -148,7 +149,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { /* 0x0030 */ .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), }, - { + [I915_MOCS_CACHED] = { /* 0x00000039 */ .control_value = LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LE_TC_LLC_ELLC) | diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index c17d63d..e2a6969 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -62,6 +62,30 @@ extern "C" { #define I915_ERROR_UEVENT "ERROR" #define I915_RESET_UEVENT "RESET" +/* + * MOCS indexes used for GPU surfaces, defining the cacheability of the + * surface data and the coherency for this data wrt. CPU vs. GPU accesses. + */ +enum i915_mocs_table_index { + /* + * Not cached anywhere, coherency between CPU and GPU accesses is + * guaranteed. + */ + I915_MOCS_UNCACHED, + /* + * Cacheability and coherency controlled by the kernel automatically + * based on the DRM_I915_GEM_SET_CACHING IOCTL setting and the current + * usage of the surface (used for display scanout or not). + */ + I915_MOCS_PTE, + /* + * Cached in all GPU caches available on the platform. + * Coherency between CPU and GPU accesses to the surface is not + * guaranteed without extra synchronization. + */ + I915_MOCS_CACHED, +}; + /* Each region is a minimum of 16k, and there are at most 255 of them. */ #define I915_NR_TEX_REGIONS 255 /* table size 2k - maximum due to use -- 2.5.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries 2016-07-01 13:40 ` [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries Imre Deak 2016-07-01 13:49 ` Chris Wilson 2016-07-01 14:32 ` [PATCH v4 " Imre Deak @ 2016-07-13 2:22 ` Zhao Yakui 2016-07-13 10:04 ` Imre Deak 2 siblings, 1 reply; 20+ messages in thread From: Zhao Yakui @ 2016-07-13 2:22 UTC (permalink / raw) To: Deak, Imre; +Cc: intel-gfx@lists.freedesktop.org, Tamminen, Eero T On 07/01/2016 09:40 PM, Deak, Imre wrote: > The purpose for each MOCS entry isn't well defined atm. Defining these > is important to remove any uncertainty about the use of these entries > for example in terms of performance and GPU/CPU coherency. > > Suggested by Ville. > > CC: Rong R Yang<rong.r.yang@intel.com> > CC: Yakui Zhao<yakui.zhao@intel.com> > CC: Ville Syrjälä<ville.syrjala@linux.intel.com> > CC: Chris Wilson<chris@chris-wilson.co.uk> > Signed-off-by: Imre Deak<imre.deak@intel.com> This looks readable and meaningful after giving proper names to MOCS entry index. But not sure whether the comment of I915_MOCS_CACHE has one typo? > --- > drivers/gpu/drm/i915/intel_mocs.c | 13 +++++++------ > include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++++ > 2 files changed, 31 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > index 927825f..86adc11 100644 > --- a/drivers/gpu/drm/i915/intel_mocs.c > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -97,7 +97,8 @@ struct drm_i915_mocs_table { > * end. > */ > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > - { /* 0x00000009 */ > + [I915_MOCS_UNCACHED] = { > + /* 0x00000009 */ > .control_value = LE_CACHEABILITY(LE_UC) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > @@ -106,7 +107,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > /* 0x0010 */ > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > }, > - { > + [I915_MOCS_AUTO] = { > /* 0x00000038 */ > .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > @@ -115,7 +116,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > /* 0x0030 */ > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > }, > - { > + [I915_MOCS_CACHED] = { > /* 0x0000003b */ > .control_value = LE_CACHEABILITY(LE_WB) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > @@ -128,7 +129,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > - { > + [I915_MOCS_UNCACHED] = { > /* 0x00000009 */ > .control_value = LE_CACHEABILITY(LE_UC) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > @@ -138,7 +139,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > /* 0x0010 */ > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > }, > - { > + [I915_MOCS_AUTO] = { > /* 0x00000038 */ > .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > @@ -148,7 +149,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > /* 0x0030 */ > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > }, > - { > + [I915_MOCS_CACHED] = { > /* 0x00000039 */ > .control_value = LE_CACHEABILITY(LE_UC) | > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index c17d63d..a5d116f 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -62,6 +62,30 @@ extern "C" { > #define I915_ERROR_UEVENT "ERROR" > #define I915_RESET_UEVENT "RESET" > > +/* > + * MOCS indexes used for GPU surfaces, defining the cacheability of the > + * surface data and the coherency for this data wrt. CPU vs. GPU accesses. > + */ > +enum i915_mocs_table_index { > + /* > + * Not cached anywhere, coherency between CPU and GPU accesses is > + * guaranteed. > + */ > + I915_MOCS_UNCACHED, > + /* > + * Cacheability and coherency controlled by the kernel automatically > + * based on the DRM_I915_GEM_SET_CACHING IOCTL setting and the current > + * usage of the surface (used for display scanout or not). > + */ > + I915_MOCS_AUTO, > + /* > + * Cached in all GPU caches available on the platform. > + * Coherency between CPU and GPU accesses to the surface is not > + * guaranteed without extra synchronization. > + */ IMO the coherency is guaranteed without extra synchronization for the MOCS_CACHED. > + I915_MOCS_CACHED, > +}; > + > /* Each region is a minimum of 16k, and there are at most 255 of them. > */ > #define I915_NR_TEX_REGIONS 255 /* table size 2k - maximum due to use _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries 2016-07-13 2:22 ` [PATCH v3 " Zhao Yakui @ 2016-07-13 10:04 ` Imre Deak 2016-07-14 1:38 ` Zhao Yakui 0 siblings, 1 reply; 20+ messages in thread From: Imre Deak @ 2016-07-13 10:04 UTC (permalink / raw) To: Zhao Yakui; +Cc: intel-gfx@lists.freedesktop.org, Tamminen, Eero T Hi Yakui, thanks for taking a look at these, see my comment below. On ke, 2016-07-13 at 10:22 +0800, Zhao Yakui wrote: > On 07/01/2016 09:40 PM, Deak, Imre wrote: > > The purpose for each MOCS entry isn't well defined atm. Defining these > > is important to remove any uncertainty about the use of these entries > > for example in terms of performance and GPU/CPU coherency. > > > > Suggested by Ville. > > > > CC: Rong R Yang<rong.r.yang@intel.com> > > CC: Yakui Zhao<yakui.zhao@intel.com> > > CC: Ville Syrjälä<ville.syrjala@linux.intel.com> > > CC: Chris Wilson<chris@chris-wilson.co.uk> > > Signed-off-by: Imre Deak<imre.deak@intel.com> > > This looks readable and meaningful after giving proper names to MOCS > entry index. > > But not sure whether the comment of I915_MOCS_CACHE has one typo? > > > --- > > drivers/gpu/drm/i915/intel_mocs.c | 13 +++++++------ > > include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++++ > > 2 files changed, 31 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > > index 927825f..86adc11 100644 > > --- a/drivers/gpu/drm/i915/intel_mocs.c > > +++ b/drivers/gpu/drm/i915/intel_mocs.c > > @@ -97,7 +97,8 @@ struct drm_i915_mocs_table { > > * end. > > */ > > static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > - { /* 0x00000009 */ > > + [I915_MOCS_UNCACHED] = { > > + /* 0x00000009 */ > > .control_value = LE_CACHEABILITY(LE_UC) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | > > @@ -106,7 +107,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > /* 0x0010 */ > > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > > }, > > - { > > + [I915_MOCS_AUTO] = { > > /* 0x00000038 */ > > .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > @@ -115,7 +116,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > /* 0x0030 */ > > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > > }, > > - { > > + [I915_MOCS_CACHED] = { > > /* 0x0000003b */ > > .control_value = LE_CACHEABILITY(LE_WB) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > @@ -128,7 +129,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { > > > > /* NOTE: the LE_TGT_CACHE is not used on Broxton */ > > static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > > - { > > + [I915_MOCS_UNCACHED] = { > > /* 0x00000009 */ > > .control_value = LE_CACHEABILITY(LE_UC) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > @@ -138,7 +139,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > > /* 0x0010 */ > > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), > > }, > > - { > > + [I915_MOCS_AUTO] = { > > /* 0x00000038 */ > > .control_value = LE_CACHEABILITY(LE_PAGETABLE) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > @@ -148,7 +149,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { > > /* 0x0030 */ > > .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), > > }, > > - { > > + [I915_MOCS_CACHED] = { > > /* 0x00000039 */ > > .control_value = LE_CACHEABILITY(LE_UC) | > > LE_TGT_CACHE(LE_TC_LLC_ELLC) | > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > > index c17d63d..a5d116f 100644 > > --- a/include/uapi/drm/i915_drm.h > > +++ b/include/uapi/drm/i915_drm.h > > @@ -62,6 +62,30 @@ extern "C" { > > #define I915_ERROR_UEVENT "ERROR" > > #define I915_RESET_UEVENT "RESET" > > > > +/* > > + * MOCS indexes used for GPU surfaces, defining the cacheability of the > > + * surface data and the coherency for this data wrt. CPU vs. GPU accesses. > > + */ > > +enum i915_mocs_table_index { > > + /* > > + * Not cached anywhere, coherency between CPU and GPU accesses is > > + * guaranteed. > > + */ > > + I915_MOCS_UNCACHED, > > + /* > > + * Cacheability and coherency controlled by the kernel automatically > > + * based on the DRM_I915_GEM_SET_CACHING IOCTL setting and the current > > + * usage of the surface (used for display scanout or not). > > + */ > > + I915_MOCS_AUTO, > > + /* > > + * Cached in all GPU caches available on the platform. > > + * Coherency between CPU and GPU accesses to the surface is not > > + * guaranteed without extra synchronization. > > + */ > > IMO the coherency is guaranteed without extra synchronization for the > MOCS_CACHED. No. On BXT it will make the data cached in GPU caches but will not keep the data coherent between GPU and CPU without extra synchronization. For that we would need to enable snooping, but that has considerable overhead, so we turn that off in patch 2/3. On SKL using this entry happens to give you a coherent mapping, but that's just because the HW doesn't allow us to turn off snooping on that platform (supposedly because there snooping doesn't have a considerable overhead thanks to LLC). --Imre > > > + I915_MOCS_CACHED, > > +}; > > + > > /* Each region is a minimum of 16k, and there are at most 255 of them. > > */ > > #define I915_NR_TEX_REGIONS 255 /* table size 2k - maximum due to use > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries 2016-07-13 10:04 ` Imre Deak @ 2016-07-14 1:38 ` Zhao Yakui 0 siblings, 0 replies; 20+ messages in thread From: Zhao Yakui @ 2016-07-14 1:38 UTC (permalink / raw) To: Deak, Imre; +Cc: intel-gfx@lists.freedesktop.org, Tamminen, Eero T On 07/13/2016 06:04 PM, Deak, Imre wrote: > Hi Yakui, > > thanks for taking a look at these, see my comment below. > > On ke, 2016-07-13 at 10:22 +0800, Zhao Yakui wrote: >> On 07/01/2016 09:40 PM, Deak, Imre wrote: >>> The purpose for each MOCS entry isn't well defined atm. Defining these >>> is important to remove any uncertainty about the use of these entries >>> for example in terms of performance and GPU/CPU coherency. >>> >>> Suggested by Ville. >>> >>> CC: Rong R Yang<rong.r.yang@intel.com> >>> CC: Yakui Zhao<yakui.zhao@intel.com> >>> CC: Ville Syrjälä<ville.syrjala@linux.intel.com> >>> CC: Chris Wilson<chris@chris-wilson.co.uk> >>> Signed-off-by: Imre Deak<imre.deak@intel.com> >> >> This looks readable and meaningful after giving proper names to MOCS >> entry index. >> >> But not sure whether the comment of I915_MOCS_CACHE has one typo? >> >>> --- >>> drivers/gpu/drm/i915/intel_mocs.c | 13 +++++++------ >>> include/uapi/drm/i915_drm.h | 24 ++++++++++++++++++++++++ >>> 2 files changed, 31 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c >>> index 927825f..86adc11 100644 >>> --- a/drivers/gpu/drm/i915/intel_mocs.c >>> +++ b/drivers/gpu/drm/i915/intel_mocs.c >>> @@ -97,7 +97,8 @@ struct drm_i915_mocs_table { >>> * end. >>> */ >>> static const struct drm_i915_mocs_entry skylake_mocs_table[] = { >>> - { /* 0x00000009 */ >>> + [I915_MOCS_UNCACHED] = { >>> + /* 0x00000009 */ >>> .control_value = LE_CACHEABILITY(LE_UC) | >>> LE_TGT_CACHE(LE_TC_LLC_ELLC) | >>> LE_LRUM(0) | LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | >>> @@ -106,7 +107,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { >>> /* 0x0010 */ >>> .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), >>> }, >>> - { >>> + [I915_MOCS_AUTO] = { >>> /* 0x00000038 */ >>> .control_value = LE_CACHEABILITY(LE_PAGETABLE) | >>> LE_TGT_CACHE(LE_TC_LLC_ELLC) | >>> @@ -115,7 +116,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { >>> /* 0x0030 */ >>> .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), >>> }, >>> - { >>> + [I915_MOCS_CACHED] = { >>> /* 0x0000003b */ >>> .control_value = LE_CACHEABILITY(LE_WB) | >>> LE_TGT_CACHE(LE_TC_LLC_ELLC) | >>> @@ -128,7 +129,7 @@ static const struct drm_i915_mocs_entry skylake_mocs_table[] = { >>> >>> /* NOTE: the LE_TGT_CACHE is not used on Broxton */ >>> static const struct drm_i915_mocs_entry broxton_mocs_table[] = { >>> - { >>> + [I915_MOCS_UNCACHED] = { >>> /* 0x00000009 */ >>> .control_value = LE_CACHEABILITY(LE_UC) | >>> LE_TGT_CACHE(LE_TC_LLC_ELLC) | >>> @@ -138,7 +139,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { >>> /* 0x0010 */ >>> .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC), >>> }, >>> - { >>> + [I915_MOCS_AUTO] = { >>> /* 0x00000038 */ >>> .control_value = LE_CACHEABILITY(LE_PAGETABLE) | >>> LE_TGT_CACHE(LE_TC_LLC_ELLC) | >>> @@ -148,7 +149,7 @@ static const struct drm_i915_mocs_entry broxton_mocs_table[] = { >>> /* 0x0030 */ >>> .l3cc_value = L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB), >>> }, >>> - { >>> + [I915_MOCS_CACHED] = { >>> /* 0x00000039 */ >>> .control_value = LE_CACHEABILITY(LE_UC) | >>> LE_TGT_CACHE(LE_TC_LLC_ELLC) | >>> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h >>> index c17d63d..a5d116f 100644 >>> --- a/include/uapi/drm/i915_drm.h >>> +++ b/include/uapi/drm/i915_drm.h >>> @@ -62,6 +62,30 @@ extern "C" { >>> #define I915_ERROR_UEVENT "ERROR" >>> #define I915_RESET_UEVENT "RESET" >>> >>> +/* >>> + * MOCS indexes used for GPU surfaces, defining the cacheability of the >>> + * surface data and the coherency for this data wrt. CPU vs. GPU accesses. >>> + */ >>> +enum i915_mocs_table_index { >>> + /* >>> + * Not cached anywhere, coherency between CPU and GPU accesses is >>> + * guaranteed. >>> + */ >>> + I915_MOCS_UNCACHED, >>> + /* >>> + * Cacheability and coherency controlled by the kernel automatically >>> + * based on the DRM_I915_GEM_SET_CACHING IOCTL setting and the current >>> + * usage of the surface (used for display scanout or not). >>> + */ >>> + I915_MOCS_AUTO, >>> + /* >>> + * Cached in all GPU caches available on the platform. >>> + * Coherency between CPU and GPU accesses to the surface is not >>> + * guaranteed without extra synchronization. >>> + */ >> >> IMO the coherency is guaranteed without extra synchronization for the >> MOCS_CACHED. > > No. On BXT it will make the data cached in GPU caches but will not keep > the data coherent between GPU and CPU without extra synchronization. > For that we would need to enable snooping, but that has considerable > overhead, so we turn that off in patch 2/3. On SKL using this entry > happens to give you a coherent mapping, but that's just because the HW > doesn't allow us to turn off snooping on that platform (supposedly > because there snooping doesn't have a considerable overhead thanks to > LLC). thanks for the detailed explanation. Now it is clear to me. Thanks Yakui > > --Imre > >> >>> + I915_MOCS_CACHED, >>> +}; >>> + >>> /* Each region is a minimum of 16k, and there are at most 255 of them. >>> */ >>> #define I915_NR_TEX_REGIONS 255 /* table size 2k - maximum due to use >> _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry 2016-07-01 13:40 [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Imre Deak ` (2 preceding siblings ...) 2016-07-01 13:40 ` [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries Imre Deak @ 2016-07-01 14:54 ` Patchwork 2016-07-19 18:15 ` Imre Deak 2016-07-01 15:15 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry (rev2) Patchwork 2016-07-18 14:28 ` [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Ville Syrjälä 5 siblings, 1 reply; 20+ messages in thread From: Patchwork @ 2016-07-01 14:54 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx == Series Details == Series: drm/i915/bxt: Fix performance due to bogus MOCS entry URL : https://patchwork.freedesktop.org/series/9377/ State : warning == Summary == Series 9377v1 drm/i915/bxt: Fix performance due to bogus MOCS entry http://patchwork.freedesktop.org/api/1.0/series/9377/revisions/1/mbox Test gem_exec_flush: Subgroup basic-batch-kernel-default-cmd: fail -> PASS (ro-byt-n2820) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-a: skip -> DMESG-WARN (ro-bdw-i5-5250u) Subgroup suspend-read-crc-pipe-c: skip -> PASS (fi-skl-i5-6260u) fi-kbl-qkkr total:229 pass:160 dwarn:29 dfail:0 fail:0 skip:40 fi-skl-i5-6260u total:229 pass:204 dwarn:0 dfail:0 fail:0 skip:25 fi-skl-i7-6700k total:229 pass:190 dwarn:0 dfail:0 fail:0 skip:39 ro-bdw-i5-5250u total:229 pass:204 dwarn:4 dfail:1 fail:0 skip:20 ro-bdw-i7-5557U total:229 pass:204 dwarn:1 dfail:1 fail:0 skip:23 ro-bdw-i7-5600u total:229 pass:190 dwarn:0 dfail:1 fail:0 skip:38 ro-bsw-n3050 total:229 pass:177 dwarn:0 dfail:1 fail:2 skip:49 ro-byt-n2820 total:229 pass:181 dwarn:0 dfail:1 fail:2 skip:45 ro-hsw-i3-4010u total:229 pass:197 dwarn:0 dfail:1 fail:0 skip:31 ro-hsw-i7-4770r total:229 pass:197 dwarn:0 dfail:1 fail:0 skip:31 ro-ilk-i7-620lm total:229 pass:157 dwarn:0 dfail:1 fail:1 skip:70 ro-ilk1-i5-650 total:224 pass:157 dwarn:0 dfail:1 fail:1 skip:65 ro-ivb-i7-3770 total:229 pass:188 dwarn:0 dfail:1 fail:0 skip:40 ro-skl3-i5-6260u total:229 pass:208 dwarn:1 dfail:1 fail:0 skip:19 ro-snb-i7-2620M total:229 pass:179 dwarn:0 dfail:1 fail:1 skip:48 fi-hsw-i7-4770k failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_1361/ a755d6c drm-intel-nightly: 2016y-07m-01d-13h-54m-24s UTC integration manifest dedf573 drm/i915: Give proper names to MOCS entries ab3eabf drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config c73e12a drm/i915/gen9: Clean up MOCS table definitions _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry 2016-07-01 14:54 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry Patchwork @ 2016-07-19 18:15 ` Imre Deak 0 siblings, 0 replies; 20+ messages in thread From: Imre Deak @ 2016-07-19 18:15 UTC (permalink / raw) To: intel-gfx, Ville Syrjälä, chris Wilson, Tamminen, Eero T, Zhao Yakui, Yang, Rong R, Daniel Vetter, Francisco Jerez On pe, 2016-07-01 at 14:54 +0000, Patchwork wrote: > == Series Details == > > Series: drm/i915/bxt: Fix performance due to bogus MOCS entry > URL : https://patchwork.freedesktop.org/series/9377/ > State : warning > > == Summary == > > Series 9377v1 drm/i915/bxt: Fix performance due to bogus MOCS entry > http://patchwork.freedesktop.org/api/1.0/series/9377/revisions/1/mbox > > Test gem_exec_flush: > Subgroup basic-batch-kernel-default-cmd: > fail -> PASS (ro-byt-n2820) > Test kms_pipe_crc_basic: > Subgroup suspend-read-crc-pipe-a: > skip -> DMESG-WARN (ro-bdw-i5-5250u) Unrelated platform. I pushed the patches to drm-intel-next-queued, thanks for the reviews and tests. --Imre > Subgroup suspend-read-crc-pipe-c: > skip -> PASS (fi-skl-i5-6260u) > > fi-kbl- > qkkr total:229 pass:160 dwarn:29 dfail:0 fail:0 skip:40 > fi-skl-i5- > 6260u total:229 pass:204 dwarn:0 dfail:0 fail:0 skip:25 > fi-skl-i7- > 6700k total:229 pass:190 dwarn:0 dfail:0 fail:0 skip:39 > ro-bdw-i5- > 5250u total:229 pass:204 dwarn:4 dfail:1 fail:0 skip:20 > ro-bdw-i7- > 5557U total:229 pass:204 dwarn:1 dfail:1 fail:0 skip:23 > ro-bdw-i7- > 5600u total:229 pass:190 dwarn:0 dfail:1 fail:0 skip:38 > ro-bsw- > n3050 total:229 pass:177 dwarn:0 dfail:1 fail:2 skip:49 > ro-byt- > n2820 total:229 pass:181 dwarn:0 dfail:1 fail:2 skip:45 > ro-hsw-i3- > 4010u total:229 pass:197 dwarn:0 dfail:1 fail:0 skip:31 > ro-hsw-i7- > 4770r total:229 pass:197 dwarn:0 dfail:1 fail:0 skip:31 > ro-ilk-i7- > 620lm total:229 pass:157 dwarn:0 dfail:1 fail:1 skip:70 > ro-ilk1-i5- > 650 total:224 pass:157 dwarn:0 dfail:1 fail:1 skip:65 > ro-ivb-i7- > 3770 total:229 pass:188 dwarn:0 dfail:1 fail:0 skip:40 > ro-skl3-i5-6260u > total:229 pass:208 dwarn:1 dfail:1 fail:0 skip:19 > ro-snb-i7- > 2620M total:229 pass:179 dwarn:0 dfail:1 fail:1 skip:48 > fi-hsw-i7-4770k failed to connect after reboot > > Results at /archive/results/CI_IGT_test/RO_Patchwork_1361/ > > a755d6c drm-intel-nightly: 2016y-07m-01d-13h-54m-24s UTC integration > manifest > dedf573 drm/i915: Give proper names to MOCS entries > ab3eabf drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect > MOCS config > c73e12a drm/i915/gen9: Clean up MOCS table definitions > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry (rev2) 2016-07-01 13:40 [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Imre Deak ` (3 preceding siblings ...) 2016-07-01 14:54 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry Patchwork @ 2016-07-01 15:15 ` Patchwork 2016-07-18 14:28 ` [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Ville Syrjälä 5 siblings, 0 replies; 20+ messages in thread From: Patchwork @ 2016-07-01 15:15 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx == Series Details == Series: drm/i915/bxt: Fix performance due to bogus MOCS entry (rev2) URL : https://patchwork.freedesktop.org/series/9377/ State : warning == Summary == Series 9377v2 drm/i915/bxt: Fix performance due to bogus MOCS entry http://patchwork.freedesktop.org/api/1.0/series/9377/revisions/2/mbox Test gem_exec_flush: Subgroup basic-batch-kernel-default-cmd: fail -> PASS (ro-byt-n2820) Test kms_pipe_crc_basic: Subgroup nonblocking-crc-pipe-b: pass -> SKIP (fi-skl-i5-6260u) Subgroup read-crc-pipe-c-frame-sequence: pass -> SKIP (fi-skl-i5-6260u) Subgroup suspend-read-crc-pipe-b: dmesg-warn -> SKIP (ro-bdw-i5-5250u) Subgroup suspend-read-crc-pipe-c: skip -> PASS (fi-skl-i5-6260u) fi-hsw-i7-4770k total:229 pass:196 dwarn:0 dfail:0 fail:0 skip:33 fi-kbl-qkkr total:229 pass:160 dwarn:29 dfail:0 fail:0 skip:40 fi-skl-i5-6260u total:229 pass:202 dwarn:0 dfail:0 fail:0 skip:27 fi-skl-i7-6700k total:229 pass:190 dwarn:0 dfail:0 fail:0 skip:39 ro-bdw-i5-5250u total:229 pass:204 dwarn:2 dfail:1 fail:0 skip:22 ro-bdw-i7-5557U total:229 pass:204 dwarn:1 dfail:1 fail:0 skip:23 ro-bdw-i7-5600u total:229 pass:190 dwarn:0 dfail:1 fail:0 skip:38 ro-bsw-n3050 total:229 pass:177 dwarn:0 dfail:1 fail:2 skip:49 ro-byt-n2820 total:229 pass:181 dwarn:0 dfail:1 fail:2 skip:45 ro-hsw-i7-4770r total:229 pass:197 dwarn:0 dfail:1 fail:0 skip:31 ro-ilk-i7-620lm total:229 pass:157 dwarn:0 dfail:1 fail:1 skip:70 ro-ilk1-i5-650 total:224 pass:157 dwarn:0 dfail:1 fail:1 skip:65 ro-ivb-i7-3770 total:229 pass:188 dwarn:0 dfail:1 fail:0 skip:40 ro-skl3-i5-6260u total:229 pass:208 dwarn:1 dfail:1 fail:0 skip:19 ro-snb-i7-2620M total:229 pass:179 dwarn:0 dfail:1 fail:1 skip:48 ro-hsw-i3-4010u failed to connect after reboot Results at /archive/results/CI_IGT_test/RO_Patchwork_1362/ a755d6c drm-intel-nightly: 2016y-07m-01d-13h-54m-24s UTC integration manifest 5a7b0a5 drm/i915: Give proper names to MOCS entries 6a55933 drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config 9c5f6a5 drm/i915/gen9: Clean up MOCS table definitions _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry 2016-07-01 13:40 [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Imre Deak ` (4 preceding siblings ...) 2016-07-01 15:15 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry (rev2) Patchwork @ 2016-07-18 14:28 ` Ville Syrjälä 5 siblings, 0 replies; 20+ messages in thread From: Ville Syrjälä @ 2016-07-18 14:28 UTC (permalink / raw) To: Imre Deak; +Cc: intel-gfx, Eero Tamminen On Fri, Jul 01, 2016 at 04:40:03PM +0300, Imre Deak wrote: > This is v3 of [1] addressing Ville's and Chris' comments. On Daniel's > request I also discussed about these changes with Rong R Yang from the > Beignet and Yakui Zhao from the Libva team, they are CC'd. > > Rong, Yakui please add your Acked-by/Tested-by if you are ok with the > changes. > > I suggest merging these patches for 4.7, via drm-intel-fixes. > > [1] https://lists.freedesktop.org/archives/intel-gfx/2016-April/094056.html > > CC: Rong R Yang <rong.r.yang@intel.com> > CC: Yakui Zhao <yakui.zhao@intel.com> > CC: Ville Syrjälä <ville.syrjala@linux.intel.com> > CC: Chris Wilson <chris@chris-wilson.co.uk> > > Imre Deak (3): > drm/i915/gen9: Clean up MOCS table definitions > drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS > config > drm/i915: Give proper names to MOCS entries Patches lgtm Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > drivers/gpu/drm/i915/intel_mocs.c | 89 +++++++++++++++++++++++++++------------ > include/uapi/drm/i915_drm.h | 24 +++++++++++ > 2 files changed, 86 insertions(+), 27 deletions(-) > > -- > 2.5.0 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-07-19 18:15 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-07-01 13:40 [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Imre Deak 2016-07-01 13:40 ` [PATCH v3 1/3] drm/i915/gen9: Clean up MOCS table definitions Imre Deak 2016-07-01 21:23 ` Bish, Jim 2016-07-01 21:47 ` Francisco Jerez 2016-07-12 11:08 ` Imre Deak 2016-07-13 2:10 ` Zhao Yakui 2016-07-01 13:40 ` [PATCH v3 2/3] drm/i915/bxt: Fix inadvertent CPU snooping due to incorrect MOCS config Imre Deak 2016-07-13 2:32 ` Zhao Yakui 2016-07-14 8:33 ` Yang, Rong R 2016-07-01 13:40 ` [PATCH v3 3/3] drm/i915: Give proper names to MOCS entries Imre Deak 2016-07-01 13:49 ` Chris Wilson 2016-07-01 13:56 ` Imre Deak 2016-07-01 14:32 ` [PATCH v4 " Imre Deak 2016-07-13 2:22 ` [PATCH v3 " Zhao Yakui 2016-07-13 10:04 ` Imre Deak 2016-07-14 1:38 ` Zhao Yakui 2016-07-01 14:54 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry Patchwork 2016-07-19 18:15 ` Imre Deak 2016-07-01 15:15 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix performance due to bogus MOCS entry (rev2) Patchwork 2016-07-18 14:28 ` [PATCH v3 0/3] drm/i915/bxt: Fix performance due to bogus MOCS entry Ville Syrjälä
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox