Re: [Intel-gfx] [PATCHv8] drm/i915: Added Programming of the MOCS

2015-07-08 Thread Ville Syrjälä
On Wed, Jul 08, 2015 at 05:51:22PM +0300, Francisco Jerez wrote:

Just a few style nits, didn't look at the actual contents...

snip
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 2a29bcc..9b17260 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -7906,4 +7906,13 @@ enum skl_disp_power_wells {
  #define _PALETTE_A (dev_priv-info.display_mmio_offset + 0xa000)
  #define _PALETTE_B (dev_priv-info.display_mmio_offset + 0xa800)
  
 +/* MOCS (Memory Object Control State) registers */
 +#define GEN9_LNCFCMOCS0  (0xB020)/* L3 Cache Control 
 base */
 +
 +#define GEN9_GFX_MOCS_0  (0xc800)/* Graphics MOCS base 
 register*/
 +#define GEN9_MFX0_MOCS_0 (0xc900)/* Media 0 MOCS base register*/
 +#define GEN9_MFX1_MOCS_0 (0xcA00)/* Media 1 MOCS base register*/
 +#define GEN9_VEBOX_MOCS_0(0xcB00)/* Video MOCS base register*/
 +#define GEN9_BLT_MOCS_0  (0xcc00)/* Blitter MOCS base 
 register*/

No need for all the parens.

snip
 diff --git a/drivers/gpu/drm/i915/intel_lrc.h 
 b/drivers/gpu/drm/i915/intel_lrc.h
 index e0299fb..64f89f99 100644
 --- a/drivers/gpu/drm/i915/intel_lrc.h
 +++ b/drivers/gpu/drm/i915/intel_lrc.h
 @@ -42,6 +42,7 @@ int intel_logical_ring_reserve_space(struct 
 drm_i915_gem_request *request);
  void intel_logical_ring_stop(struct intel_engine_cs *ring);
  void intel_logical_ring_cleanup(struct intel_engine_cs *ring);
  int intel_logical_rings_init(struct drm_device *dev);
 +int intel_logical_ring_begin(struct drm_i915_gem_request *req, int 
 num_dwords);
  
  int logical_ring_flush_all_caches(struct drm_i915_gem_request *req);
  /**
 diff --git a/drivers/gpu/drm/i915/intel_mocs.c 
 b/drivers/gpu/drm/i915/intel_mocs.c
 new file mode 100644
 index 000..130bcbe
 --- /dev/null
 +++ b/drivers/gpu/drm/i915/intel_mocs.c
 @@ -0,0 +1,337 @@
 +/*
 + * Copyright (c) 2015 Intel Corporation
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions: *
 + * The above copyright notice and this permission notice (including the next
 + * paragraph) shall be included in all copies or substantial portions of the
 + * Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
 FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
 THE
 + * SOFTWARE.
 + */
 +
 +#include intel_mocs.h
 +#include intel_lrc.h
 +#include intel_ringbuffer.h
 +
 +/* structures required */
 +struct drm_i915_mocs_entry {
 + u32 control_value;
 + u16 l3cc_value;
 +};
 +
 +struct drm_i915_mocs_table {
 + u32 size;
 + const struct drm_i915_mocs_entry*table;
 +};
 +
 +/* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */
 +#define LE_CACHEABILITY(value)   (value  0)
 +#define LE_TGT_CACHE(value)  (value  2)
 +#define LE_LRUM(value)   (value  4)
 +#define LE_AOM(value)(value  6)
 +#define LE_RSC(value)(value  7)
 +#define LE_SCC(value)(value  8)
 +#define LE_PFM(value)(value  11)
 +#define LE_SCF(value)(value  14)
 +
 +/* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */
 +#define L3_ESC(value)(value  0)
 +#define L3_SCC(value)(value  1)
 +#define L3_CACHEABILITY(value)   (value  4)

These should have parens arond (value) to avoid suprises if someone
passes in something more complicated than a single number.

 +
 +/* Helper defines */
 +#define GEN9_NUM_MOCS_ENTRIES(62)  /* 62 out of 64 - 63  64 are 
 reserved. */
 +
 +/* (e)LLC caching options */
 +#define LE_PAGETABLE (0)
 +#define LE_UC(1)
 +#define LE_WT(2)
 +#define LE_WB(3)
 +
 +/* L3 caching options */
 +#define L3_DIRECT(0)
 +#define L3_UC(1)
 +#define L3_RESERVED  (2)
 +#define L3_WB(3)
 +
 +/* Target cache */
 +#define ELLC (0)
 +#define LLC  (1)
 +#define LLC_ELLC (2)

And these parens seem useless.

-- 
Ville Syrjälä
Intel 

[Intel-gfx] [PATCHv8] drm/i915: Added Programming of the MOCS

2015-07-08 Thread Francisco Jerez
From: Peter Antoine peter.anto...@intel.com

This change adds the programming of the MOCS registers to the gen 9+
platforms. The set of MOCS configuration entries introduced by this
patch is intended to be minimal but sufficient to cover the needs of
current userspace - i.e. a good set of defaults. It is expected to be
extended in the future to provide further default values or to allow
userspace to redefine its private MOCS tables based on its demand for
additional caching configurations. In this setup, userspace should
only utilize the first N entries, higher entries are reserved for
future use.

It creates a fixed register set that is programmed across the different
engines so that all engines have the same table. This is done as the
main RCS context only holds the registers for itself and the shared
L3 values. By trying to keep the registers consistent across the
different engines it should make the programming for the registers
consistent.

v2:
-'static const' for private data structures and style changes.(Matt Turner)
v3:
- Make the tables slightly more readable. (Damien Lespiau)
- Updated tables fix performance regression.
v4:
- Code formatting. (Chris Wilson)
- re-privatised mocs code. (Daniel Vetter)
v5:
- Changed the name of a function. (Chris Wilson)
v6:
- re-based
- Added Mesa table entry (skylake  broxton) (Francisco Jerez)
- Tidied up the readability defines (Francisco Jerez)
- NUMBER of entries defines wrong. (Jim Bish)
- Added comments to clear up the meaning of the tables (Jim Bish)

Signed-off-by: Peter Antoine peter.anto...@intel.com

v7 (Francisco Jerez):
- Don't write L3-specific MOCS_ESC/SCC values into the e/LLC control
  tables.  Prefix L3-specific defines consistently with L3_ and
  e/LLC-specific defines with LE_ to avoid this kind of confusion in
  the future.
- Change L3CC WT define back to RESERVED (matches my hardware
  documentation and the original patch, probably a misunderstanding
  of my own previous comment).
- Drop Android tables, define new minimal tables more suitable for the
  open source stack.
- Add comment that the MOCS tables are part of the kernel ABI.
- Move intel_logical_ring_begin() and _advance() calls one level down
  (Chris Wilson).
- Minor formatting and style fixes.

v8 (Francisco Jerez):
- Add table size sanity check to emit_mocs_control/l3cc_table() (Chris
  Wilson).
- Add comment about undefined entries being implicitly set to uncached
  for forwards compatibility.

Signed-off-by: Francisco Jerez curroje...@riseup.net
---
 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/i915_reg.h   |   9 +
 drivers/gpu/drm/i915/intel_lrc.c  |  11 +-
 drivers/gpu/drm/i915/intel_lrc.h  |   1 +
 drivers/gpu/drm/i915/intel_mocs.c | 337 ++
 drivers/gpu/drm/i915/intel_mocs.h |  57 +++
 6 files changed, 414 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_mocs.c
 create mode 100644 drivers/gpu/drm/i915/intel_mocs.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index de21965..e52e012 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -36,6 +36,7 @@ i915-y += i915_cmd_parser.o \
  i915_trace_points.o \
  intel_hotplug.o \
  intel_lrc.o \
+ intel_mocs.o \
  intel_ringbuffer.o \
  intel_uncore.o
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2a29bcc..9b17260 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7906,4 +7906,13 @@ enum skl_disp_power_wells {
 #define _PALETTE_A (dev_priv-info.display_mmio_offset + 0xa000)
 #define _PALETTE_B (dev_priv-info.display_mmio_offset + 0xa800)
 
+/* MOCS (Memory Object Control State) registers */
+#define GEN9_LNCFCMOCS0(0xB020)/* L3 Cache Control 
base */
+
+#define GEN9_GFX_MOCS_0(0xc800)/* Graphics MOCS base 
register*/
+#define GEN9_MFX0_MOCS_0   (0xc900)/* Media 0 MOCS base register*/
+#define GEN9_MFX1_MOCS_0   (0xcA00)/* Media 1 MOCS base register*/
+#define GEN9_VEBOX_MOCS_0  (0xcB00)/* Video MOCS base register*/
+#define GEN9_BLT_MOCS_0(0xcc00)/* Blitter MOCS base 
register*/
+
 #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d4f8b43..466d17c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -135,6 +135,7 @@
 #include drm/drmP.h
 #include drm/i915_drm.h
 #include i915_drv.h
+#include intel_mocs.h
 
 #define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE)
 #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
@@ -772,8 +773,7 @@ static int logical_ring_prepare(struct drm_i915_gem_request 
*req, int bytes)
  *
  * Return: non-zero if the ringbuffer is not ready to be written to.
  */
-static int intel_logical_ring_begin(struct drm_i915_gem_request *req,
-