All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

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

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

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

* ✗ 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 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, 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, 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

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

* 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; +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

* 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

* 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

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.