| Message ID | 1433923936-20014-1-git-send-email-peter.antoine@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 10, 2015 at 09:12:16AM +0100, Peter Antoine wrote: > This change adds the programming of the MOCS registers to the gen 9+ > platforms. This change set programs the MOCS register values to a set > of values that are defined to be optimal. > > 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. > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 3 +- > drivers/gpu/drm/i915/i915_reg.h | 9 ++ > drivers/gpu/drm/i915/intel_lrc.c | 68 ++++++++++ > drivers/gpu/drm/i915/intel_mocs.c | 252 ++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_mocs.h | 119 ++++++++++++++++++ > 5 files changed, 450 insertions(+), 1 deletion(-) > 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 b7ddf48..cd7b910 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -36,7 +36,8 @@ i915-y += i915_cmd_parser.o \ > i915_trace_points.o \ > intel_lrc.o \ > intel_ringbuffer.o \ > - intel_uncore.o > + intel_uncore.o \ > + intel_mocs.o Please keep it alphabetical. > # autogenerated null render state > i915-y += intel_renderstate_gen6.o \ > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 7213224..3a435b5 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7829,4 +7829,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 9f5485d..c875569 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) > @@ -1370,6 +1371,67 @@ out: > return ret; > } > > +/* > + * i915_gem_program_mocs() - program the MOCS register. > + * > + * ring: The ring that the programming batch will be run in. > + * ctx: The intel_context to be used. > + * > + * This function will emit a batch buffer with the values required for > + * programming the MOCS register values for all the currenly supported > + * rings. > + * > + * Return: 0 on success, otherwise the error status. > + */ > +static int i915_gem_program_mocs(struct intel_engine_cs *ring, > + struct intel_context *ctx) Odd function name. gen8_init_mocs_tables() > +{ > + int ret = 0; > + > + struct drm_i915_mocs_table t; > + struct drm_device *dev = ring->dev; > + struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; > + > + if (get_mocs_settings(dev, &t)) { Worse, this function is being pulled in from another file so needs a proper function naem. Just return the table pointer. > + u32 table_size; > + > + /* > + * OK. For each supported ring: > + * table_size * 2 dwords for each control_value > + * plus table/2 dwords for l3cc values. > + * > + * Plus 1 for the load command and 1 for the NOOP per ring > + * and the l3cc programming. > + */ > + table_size = GEN9_NUM_MOCS_RINGS * ((2 * t.size) + 2) + > + t.size + 2; > + ret = intel_logical_ring_begin(ringbuf, ctx, table_size); > + if (ret) { > + DRM_ERROR("intel_logical_ring_begin failed %d\n", ret); Error? Does this help the user in any way, especially as not enabling mocs is not critical? > + return ret; > + } > + > + /* program the control registers */ > + emit_mocs_control_table(ringbuf, &t, GEN9_GFX_MOCS_0); > + emit_mocs_control_table(ringbuf, &t, GEN9_MFX0_MOCS_0); > + emit_mocs_control_table(ringbuf, &t, GEN9_MFX1_MOCS_0); > + emit_mocs_control_table(ringbuf, &t, GEN9_VEBOX_MOCS_0); > + emit_mocs_control_table(ringbuf, &t, GEN9_BLT_MOCS_0); So why are programming other rings on RCS without explicit inter-engine ordering? > + > + /* now program the l3cc registers */ > + emit_mocs_l3cc_table(ringbuf, &t); > + > + intel_logical_ring_advance(ringbuf); > + > + DRM_INFO("MOCS: Table set in Context\n"); > + } else { > + DRM_INFO("MOCS: Table Not supported on platform\n"); Info? Just debug, these are not helpful user messages. > + } > + > + return ret; > +} > + > + > static int gen8_init_rcs_context(struct intel_engine_cs *ring, > struct intel_context *ctx) > { > @@ -1379,6 +1441,12 @@ static int gen8_init_rcs_context(struct intel_engine_cs *ring, > if (ret) > return ret; > > + /* > + * Failing to program the MOCS is non-fatal.The system will not > + * run at peak performance. So generate a warning and carry on. > + */ > + WARN_ON(i915_gem_program_mocs(ring, ctx) != 0); Warn? Just NOTICE since it is not a useful user error message and a non-critical error to boot. > return intel_lr_context_render_state_init(ring, ctx); > } > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > new file mode 100644 > index 0000000..841932a > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_mocs.c > @@ -0,0 +1,252 @@ > +/* > + * 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. > + * > + * Authors: > + * Peter Antoine <peter.antoine@intel.com> We don't encourage Authors: since git does a better job. > + */ > + > +#include "intel_mocs.h" > +#include "intel_lrc.h" > +#include "intel_ringbuffer.h" > + > +/* > + * MOCS tables > + * > + * These are the MOCS tables that are programmed across all the rings. > + * The control value is programmed to all the rings that support the > + * MOCS registers. While the l3cc_values are only programmed to the > + * LNCFCMOCS0 - LNCFCMOCS32 registers. > + * > + * NOTE: These tables MUST start with being uncached {0,0} and the > + * the length MUST be less than 63 as the last two registers are > + * reserved by the hardware. > + */ > +struct drm_i915_mocs_entry skylake_mocs_table[] = { static const ... > + /* {0x00000009, 0x0010} */ > + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(LLC_ELLC) | > + MOCS_LRUM(0) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | > + MOC_PFM(0) | MOCS_SCF(0)), > + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, Whitespace and alignment should be treated as our friends and not enemies. #define SKYLAKE_MOCS(cache, ellc, lrum, l3) { \ (MOCS_CACHEABILITY(cache) | \ MOCS_TGT_CACHE(ellc) | \ MOCS_LRUM(lrum) | \ MOCS_AOM(0) | \ MOCS_LECC_ESC(0) | \ MOCS_SCC(0) | \ MOC_PFM(0) | \ MOCS_SCF(0))), \ (MOCS_ESC(0) | \ MOSC_SCC(0) | \ MOCS_L3_CACHEABILITY(l3)) \ } would help clarify these even further, or at the very least being consistent in layout of the tables would improve readiblity and being able to cross-check. > + > +/** > + * get_mocs_settings > + * > + * This function will return the values of the MOCS table that needs to > + * be programmed for the platform. It will return the values that need > + * to be programmed and if they need to be programmed. > + * > + * If the return values is false then the registers do not need programming. > + */ > +bool get_mocs_settings(struct drm_device *dev, > + struct drm_i915_mocs_table *table) { const struct drm_i915_mocs_table * gen8_lookup_mocs_table(struct drm_i915_private *i915); > +/** > + * emit_mocs_control_table() - emit the mocs control table > + * @ringbuf: DRM device. > + * @table: The values to program into the control regs. > + * @reg_base: The base for the Engine that needs to be programmed. > + * > + * This function simply emits a MI_LOAD_REGISTER_IMM command for the > + * given table starting at the given address. > + * > + * Return: Nothing. > + */ > +void emit_mocs_control_table(struct intel_ringbuffer *ringbuf, > + struct drm_i915_mocs_table *table, > + u32 reg_base) gen8_emit_mocs_table(...) etc > +/* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */ > +#define MOCS_CACHEABILITY(value) ((value & 0x03) << 0) value & mask? These macros should only be feed enums so the masking of the input is superfluous and indicative of a programming bug. -Chris
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6561
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 303/303 303/303
SNB 312/312 312/312
IVB 343/343 343/343
BYT 287/287 287/287
BDW 321/321 321/321
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
Note: You need to pay more attention to line start with '*'
On Wed, Jun 10, 2015 at 09:12:16AM +0100, Peter Antoine wrote: > This change adds the programming of the MOCS registers to the gen 9+ > platforms. This change set programs the MOCS register values to a set > of values that are defined to be optimal. > > 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. > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 3 +- > drivers/gpu/drm/i915/i915_reg.h | 9 ++ > drivers/gpu/drm/i915/intel_lrc.c | 68 ++++++++++ > drivers/gpu/drm/i915/intel_mocs.c | 252 ++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_mocs.h | 119 ++++++++++++++++++ > 5 files changed, 450 insertions(+), 1 deletion(-) > 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 b7ddf48..cd7b910 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -36,7 +36,8 @@ i915-y += i915_cmd_parser.o \ > i915_trace_points.o \ > intel_lrc.o \ > intel_ringbuffer.o \ > - intel_uncore.o > + intel_uncore.o \ > + intel_mocs.o > > # autogenerated null render state > i915-y += intel_renderstate_gen6.o \ > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 7213224..3a435b5 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7829,4 +7829,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 9f5485d..c875569 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) > @@ -1370,6 +1371,67 @@ out: > return ret; > } > > +/* > + * i915_gem_program_mocs() - program the MOCS register. > + * > + * ring: The ring that the programming batch will be run in. > + * ctx: The intel_context to be used. > + * > + * This function will emit a batch buffer with the values required for > + * programming the MOCS register values for all the currenly supported > + * rings. > + * > + * Return: 0 on success, otherwise the error status. > + */ > +static int i915_gem_program_mocs(struct intel_engine_cs *ring, > + struct intel_context *ctx) If you go with a separate source file then imo this would be the function to move there. As a rule of thumb if your new C file is full of non-static functions used somewhere else and you need plenty of header definitions to make it all work then you've split things in a useless way. Separate source files are only useful if you actually manage to encapsulate things a bit. If that encapsulation isn't given then bothering with all the kerneldoc isn't worth it eithe since due to the tight coupling it'll outdate extremely fast. -Daniel
On Mon, 2015-06-15 at 14:51 +0200, Daniel Vetter wrote: > On Wed, Jun 10, 2015 at 09:12:16AM +0100, Peter Antoine wrote: > > This change adds the programming of the MOCS registers to the gen 9+ > > platforms. This change set programs the MOCS register values to a set > > of values that are defined to be optimal. > > > > 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. > > > > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com> > > --- > > drivers/gpu/drm/i915/Makefile | 3 +- > > drivers/gpu/drm/i915/i915_reg.h | 9 ++ > > drivers/gpu/drm/i915/intel_lrc.c | 68 ++++++++++ > > drivers/gpu/drm/i915/intel_mocs.c | 252 ++++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_mocs.h | 119 ++++++++++++++++++ > > 5 files changed, 450 insertions(+), 1 deletion(-) > > 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 b7ddf48..cd7b910 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -36,7 +36,8 @@ i915-y += i915_cmd_parser.o \ > > i915_trace_points.o \ > > intel_lrc.o \ > > intel_ringbuffer.o \ > > - intel_uncore.o > > + intel_uncore.o \ > > + intel_mocs.o > > > > # autogenerated null render state > > i915-y += intel_renderstate_gen6.o \ > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 7213224..3a435b5 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7829,4 +7829,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 9f5485d..c875569 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) > > @@ -1370,6 +1371,67 @@ out: > > return ret; > > } > > > > +/* > > + * i915_gem_program_mocs() - program the MOCS register. > > + * > > + * ring: The ring that the programming batch will be run in. > > + * ctx: The intel_context to be used. > > + * > > + * This function will emit a batch buffer with the values required for > > + * programming the MOCS register values for all the currenly supported > > + * rings. > > + * > > + * Return: 0 on success, otherwise the error status. > > + */ > > +static int i915_gem_program_mocs(struct intel_engine_cs *ring, > > + struct intel_context *ctx) > > If you go with a separate source file then imo this would be the function > to move there. As a rule of thumb if your new C file is full of non-static > functions used somewhere else and you need plenty of header definitions to > make it all work then you've split things in a useless way. Separate > source files are only useful if you actually manage to encapsulate things > a bit. If that encapsulation isn't given then bothering with all the > kerneldoc isn't worth it eithe since due to the tight coupling it'll > outdate extremely fast. Yup. Bad chose during the merge, intel_logical_ring_begin() became static, and instead of reversing that change, I moved my function. reverted and reverted. > -Daniel
On Wed, 2015-06-10 at 11:37 +0100, Chris Wilson wrote: > On Wed, Jun 10, 2015 at 09:12:16AM +0100, Peter Antoine wrote: > > This change adds the programming of the MOCS registers to the gen 9+ > > platforms. This change set programs the MOCS register values to a set > > of values that are defined to be optimal. > > > > 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. > > > > > > Signed-off-by: Peter Antoine <peter.antoine@intel.com> > > --- > > drivers/gpu/drm/i915/Makefile | 3 +- > > drivers/gpu/drm/i915/i915_reg.h | 9 ++ > > drivers/gpu/drm/i915/intel_lrc.c | 68 ++++++++++ > > drivers/gpu/drm/i915/intel_mocs.c | 252 ++++++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_mocs.h | 119 ++++++++++++++++++ > > 5 files changed, 450 insertions(+), 1 deletion(-) > > 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 b7ddf48..cd7b910 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -36,7 +36,8 @@ i915-y += i915_cmd_parser.o \ > > i915_trace_points.o \ > > intel_lrc.o \ > > intel_ringbuffer.o \ > > - intel_uncore.o > > + intel_uncore.o \ > > + intel_mocs.o > > Please keep it alphabetical. done. > > > # autogenerated null render state > > i915-y += intel_renderstate_gen6.o \ > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 7213224..3a435b5 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -7829,4 +7829,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 9f5485d..c875569 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) > > @@ -1370,6 +1371,67 @@ out: > > return ret; > > } > > > > +/* > > + * i915_gem_program_mocs() - program the MOCS register. > > + * > > + * ring: The ring that the programming batch will be run in. > > + * ctx: The intel_context to be used. > > + * > > + * This function will emit a batch buffer with the values required for > > + * programming the MOCS register values for all the currenly supported > > + * rings. > > + * > > + * Return: 0 on success, otherwise the error status. > > + */ > > +static int i915_gem_program_mocs(struct intel_engine_cs *ring, > > + struct intel_context *ctx) > > Odd function name. gen8_init_mocs_tables() Name fixed. gen9_* > > > +{ > > + int ret = 0; > > + > > + struct drm_i915_mocs_table t; > > + struct drm_device *dev = ring->dev; > > + struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; > > + > > + if (get_mocs_settings(dev, &t)) { > > Worse, this function is being pulled in from another file so needs a > proper function naem. Just return the table pointer. > function calls back were they belong. > > + u32 table_size; > > + > > + /* > > + * OK. For each supported ring: > > + * table_size * 2 dwords for each control_value > > + * plus table/2 dwords for l3cc values. > > + * > > + * Plus 1 for the load command and 1 for the NOOP per ring > > + * and the l3cc programming. > > + */ > > + table_size = GEN9_NUM_MOCS_RINGS * ((2 * t.size) + 2) + > > + t.size + 2; > > + ret = intel_logical_ring_begin(ringbuf, ctx, table_size); > > + if (ret) { > > + DRM_ERROR("intel_logical_ring_begin failed %d\n", ret); > > Error? Does this help the user in any way, especially as not enabling > mocs is not critical? No. Changed to DEBUG. > > > + return ret; > > + } > > + > > + /* program the control registers */ > > + emit_mocs_control_table(ringbuf, &t, GEN9_GFX_MOCS_0); > > + emit_mocs_control_table(ringbuf, &t, GEN9_MFX0_MOCS_0); > > + emit_mocs_control_table(ringbuf, &t, GEN9_MFX1_MOCS_0); > > + emit_mocs_control_table(ringbuf, &t, GEN9_VEBOX_MOCS_0); > > + emit_mocs_control_table(ringbuf, &t, GEN9_BLT_MOCS_0); > > So why are programming other rings on RCS without explicit inter-engine > ordering? Basically the same as the workarounds (and the null context) set these up when the the RCS is loaded. As the other engines DO NOT have these registers in their context it is done here. ALSO, as the l3cc registers are indexed by the same index value (when using the MOCS) as part of the caching, they need to match so this is why they are done at the same time. I have changed the comments to make this a little clearer. > > > + > > + /* now program the l3cc registers */ > > + emit_mocs_l3cc_table(ringbuf, &t); > > + > > + intel_logical_ring_advance(ringbuf); > > + > > + DRM_INFO("MOCS: Table set in Context\n"); > > + } else { > > + DRM_INFO("MOCS: Table Not supported on platform\n"); > > Info? Just debug, these are not helpful user messages. Change to DEBUG. > > > + } > > + > > + return ret; > > +} > > + > > + > > static int gen8_init_rcs_context(struct intel_engine_cs *ring, > > struct intel_context *ctx) > > { > > @@ -1379,6 +1441,12 @@ static int gen8_init_rcs_context(struct intel_engine_cs *ring, > > if (ret) > > return ret; > > > > + /* > > + * Failing to program the MOCS is non-fatal.The system will not > > + * run at peak performance. So generate a warning and carry on. > > + */ > > + WARN_ON(i915_gem_program_mocs(ring, ctx) != 0); > > Warn? Just NOTICE since it is not a useful user error message and a > non-critical error to boot. Changed to DRM_ERROR on fail. > > > return intel_lr_context_render_state_init(ring, ctx); > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c > > new file mode 100644 > > index 0000000..841932a > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_mocs.c > > @@ -0,0 +1,252 @@ > > +/* > > + * 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. > > + * > > + * Authors: > > + * Peter Antoine <peter.antoine@intel.com> > > We don't encourage Authors: since git does a better job. Removed. Was in the file header that I looked at. > > > + */ > > + > > +#include "intel_mocs.h" > > +#include "intel_lrc.h" > > +#include "intel_ringbuffer.h" > > + > > +/* > > + * MOCS tables > > + * > > + * These are the MOCS tables that are programmed across all the rings. > > + * The control value is programmed to all the rings that support the > > + * MOCS registers. While the l3cc_values are only programmed to the > > + * LNCFCMOCS0 - LNCFCMOCS32 registers. > > + * > > + * NOTE: These tables MUST start with being uncached {0,0} and the > > + * the length MUST be less than 63 as the last two registers are > > + * reserved by the hardware. > > + */ > > +struct drm_i915_mocs_entry skylake_mocs_table[] = { > > static const ... done. > > > + /* {0x00000009, 0x0010} */ > > + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(LLC_ELLC) | > > + MOCS_LRUM(0) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | > > + MOC_PFM(0) | MOCS_SCF(0)), > > + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, > > Whitespace and alignment should be treated as our friends and not > enemies. > > #define SKYLAKE_MOCS(cache, ellc, lrum, l3) { \ > (MOCS_CACHEABILITY(cache) | \ > MOCS_TGT_CACHE(ellc) | \ > MOCS_LRUM(lrum) | \ > MOCS_AOM(0) | \ > MOCS_LECC_ESC(0) | \ > MOCS_SCC(0) | \ > MOC_PFM(0) | \ > MOCS_SCF(0))), \ > (MOCS_ESC(0) | \ > MOSC_SCC(0) | \ > MOCS_L3_CACHEABILITY(l3)) \ > } > > would help clarify these even further, or at the very least being > consistent in layout of the tables would improve readiblity and being > able to cross-check. Made table more consistent. > > > + > > +/** > > + * get_mocs_settings > > + * > > + * This function will return the values of the MOCS table that needs to > > + * be programmed for the platform. It will return the values that need > > + * to be programmed and if they need to be programmed. > > + * > > + * If the return values is false then the registers do not need programming. > > + */ > > +bool get_mocs_settings(struct drm_device *dev, > > + struct drm_i915_mocs_table *table) { > > const struct drm_i915_mocs_table * > gen8_lookup_mocs_table(struct drm_i915_private *i915); > > > +/** > > + * emit_mocs_control_table() - emit the mocs control table > > + * @ringbuf: DRM device. > > + * @table: The values to program into the control regs. > > + * @reg_base: The base for the Engine that needs to be programmed. > > + * > > + * This function simply emits a MI_LOAD_REGISTER_IMM command for the > > + * given table starting at the given address. > > + * > > + * Return: Nothing. > > + */ > > +void emit_mocs_control_table(struct intel_ringbuffer *ringbuf, > > + struct drm_i915_mocs_table *table, > > + u32 reg_base) > > gen8_emit_mocs_table(...) > etc functions internal again. > > > +/* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */ > > +#define MOCS_CACHEABILITY(value) ((value & 0x03) << 0) > > value & mask? These macros should only be feed enums so the masking of > the input is superfluous and indicative of a programming bug. Superfluous yes, but it lets the coder know the layout of the field, but may hide a programming bug but does not indicate one (other coding standards (for safe systems) that I have used require this as it reduces the impact of coding errors). I have removed them. > -Chris >
On Wed, Jun 17, 2015 at 03:02:31PM +0000, Antoine, Peter wrote: > On Wed, 2015-06-10 at 11:37 +0100, Chris Wilson wrote: > > > +/* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */ > > > +#define MOCS_CACHEABILITY(value) ((value & 0x03) << 0) > > > > value & mask? These macros should only be feed enums so the masking of > > the input is superfluous and indicative of a programming bug. > Superfluous yes, but it lets the coder know the layout of the field, but > may hide a programming bug but does not indicate one (other coding > standards (for safe systems) that I have used require this as it reduces > the impact of coding errors). I'm wary. Maybe not so much for the MOCS table, but it means that the value being programmed to hw does not match what the programmer intended. It's a bug either way, the hardware is being programmed to an invalid value - and that may be catastrophic to use either the original value of the transformed value. > I have removed them. I kind of liked it. With a bit of work we could make it catch programming bugs at compile time. I think we have idly discussed such in the past, something like a #define SET_FIELD(x, shift, width) ({ if (__builtin_constant_p(x)) { BUILD_BUG_ON(x & -(1<<width)); } x << shift; }) #define MOCS_CACHEABILITY(value) SET_FIELD(value, 0, 2) may do the trick. -Chris
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index b7ddf48..cd7b910 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -36,7 +36,8 @@ i915-y += i915_cmd_parser.o \ i915_trace_points.o \ intel_lrc.o \ intel_ringbuffer.o \ - intel_uncore.o + intel_uncore.o \ + intel_mocs.o # autogenerated null render state i915-y += intel_renderstate_gen6.o \ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 7213224..3a435b5 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7829,4 +7829,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 9f5485d..c875569 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) @@ -1370,6 +1371,67 @@ out: return ret; } +/* + * i915_gem_program_mocs() - program the MOCS register. + * + * ring: The ring that the programming batch will be run in. + * ctx: The intel_context to be used. + * + * This function will emit a batch buffer with the values required for + * programming the MOCS register values for all the currenly supported + * rings. + * + * Return: 0 on success, otherwise the error status. + */ +static int i915_gem_program_mocs(struct intel_engine_cs *ring, + struct intel_context *ctx) +{ + int ret = 0; + + struct drm_i915_mocs_table t; + struct drm_device *dev = ring->dev; + struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf; + + if (get_mocs_settings(dev, &t)) { + u32 table_size; + + /* + * OK. For each supported ring: + * table_size * 2 dwords for each control_value + * plus table/2 dwords for l3cc values. + * + * Plus 1 for the load command and 1 for the NOOP per ring + * and the l3cc programming. + */ + table_size = GEN9_NUM_MOCS_RINGS * ((2 * t.size) + 2) + + t.size + 2; + ret = intel_logical_ring_begin(ringbuf, ctx, table_size); + if (ret) { + DRM_ERROR("intel_logical_ring_begin failed %d\n", ret); + return ret; + } + + /* program the control registers */ + emit_mocs_control_table(ringbuf, &t, GEN9_GFX_MOCS_0); + emit_mocs_control_table(ringbuf, &t, GEN9_MFX0_MOCS_0); + emit_mocs_control_table(ringbuf, &t, GEN9_MFX1_MOCS_0); + emit_mocs_control_table(ringbuf, &t, GEN9_VEBOX_MOCS_0); + emit_mocs_control_table(ringbuf, &t, GEN9_BLT_MOCS_0); + + /* now program the l3cc registers */ + emit_mocs_l3cc_table(ringbuf, &t); + + intel_logical_ring_advance(ringbuf); + + DRM_INFO("MOCS: Table set in Context\n"); + } else { + DRM_INFO("MOCS: Table Not supported on platform\n"); + } + + return ret; +} + + static int gen8_init_rcs_context(struct intel_engine_cs *ring, struct intel_context *ctx) { @@ -1379,6 +1441,12 @@ static int gen8_init_rcs_context(struct intel_engine_cs *ring, if (ret) return ret; + /* + * Failing to program the MOCS is non-fatal.The system will not + * run at peak performance. So generate a warning and carry on. + */ + WARN_ON(i915_gem_program_mocs(ring, ctx) != 0); + return intel_lr_context_render_state_init(ring, ctx); } diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c new file mode 100644 index 0000000..841932a --- /dev/null +++ b/drivers/gpu/drm/i915/intel_mocs.c @@ -0,0 +1,252 @@ +/* + * 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. + * + * Authors: + * Peter Antoine <peter.antoine@intel.com> + */ + +#include "intel_mocs.h" +#include "intel_lrc.h" +#include "intel_ringbuffer.h" + +/* + * MOCS tables + * + * These are the MOCS tables that are programmed across all the rings. + * The control value is programmed to all the rings that support the + * MOCS registers. While the l3cc_values are only programmed to the + * LNCFCMOCS0 - LNCFCMOCS32 registers. + * + * NOTE: These tables MUST start with being uncached {0,0} and the + * the length MUST be less than 63 as the last two registers are + * reserved by the hardware. + */ +struct drm_i915_mocs_entry skylake_mocs_table[] = { + /* {0x00000009, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(LLC_ELLC) | + MOCS_LRUM(0) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | + MOC_PFM(0) | MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x0000003b, 0x0030} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC_ELLC) | + MOCS_LRUM(3) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | + MOC_PFM(0) | MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_WB))}, + /* {0x00000039, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(LLC_ELLC) | + MOCS_LRUM(3) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | + MOC_PFM(0) | MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x00000017, 0x0030} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(1) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_WB))}, + /* {0x00000017, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(1) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x00000019, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(LLC_ELLC) | + MOCS_LRUM(1) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | + MOC_PFM(0) | MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x00000037, 0x0030} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(3) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_WB))}, + /* {0x00000037, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(3) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x0000003b, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC_ELLC) | + MOCS_LRUM(3) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | + MOC_PFM(0) | MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, +}; + +struct drm_i915_mocs_entry broxton_mocs_table[] = { + /* {0x00000001, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(ELLC) | MOCS_LRUM(0) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x00000005, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(0) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x00000005, 0x0030} */ + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(0) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_WB))}, + /* {0x00000017, 0x0030} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(1) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_WB))}, + /* {0x00000017, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(1) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x00000019, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_UC) | MOCS_TGT_CACHE(LLC_ELLC) | + MOCS_LRUM(1) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | + MOC_PFM(0) | MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x00000037, 0x0030} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(3) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_WB))}, + /* {0x00000037, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC) | MOCS_LRUM(3) | + MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | MOC_PFM(0) | + MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, + /* {0x0000003b, 0x0010} */ + {(MOCS_CACHEABILITY(EDRAM_WB) | MOCS_TGT_CACHE(LLC_ELLC) | + MOCS_LRUM(3) | MOCS_AOM(0) | MOCS_LECC_ESC(0) | MOCS_SCC(0) | + MOC_PFM(0) | MOCS_SCF(0)), + (MOCS_ESC(0) | MOCS_SCC(0) | MOCS_L3_CACHEABILITY(L3_UC))}, +}; + + +/** + * get_mocs_settings + * + * This function will return the values of the MOCS table that needs to + * be programmed for the platform. It will return the values that need + * to be programmed and if they need to be programmed. + * + * If the return values is false then the registers do not need programming. + */ +bool get_mocs_settings(struct drm_device *dev, + struct drm_i915_mocs_table *table) { + bool result = false; + + if (IS_SKYLAKE(dev)) { + table->size = ARRAY_SIZE(skylake_mocs_table); + table->table = skylake_mocs_table; + result = true; + } else if (IS_BROXTON(dev)) { + table->size = ARRAY_SIZE(broxton_mocs_table); + table->table = broxton_mocs_table; + result = true; + } else { + /* Platform that should have a MOCS table does not */ + WARN_ON(INTEL_INFO(dev)->gen >= 9); + } + + return result; +} + +/** + * emit_mocs_control_table() - emit the mocs control table + * @ringbuf: DRM device. + * @table: The values to program into the control regs. + * @reg_base: The base for the Engine that needs to be programmed. + * + * This function simply emits a MI_LOAD_REGISTER_IMM command for the + * given table starting at the given address. + * + * Return: Nothing. + */ +void emit_mocs_control_table(struct intel_ringbuffer *ringbuf, + struct drm_i915_mocs_table *table, + u32 reg_base) +{ + unsigned int index; + + intel_logical_ring_emit(ringbuf, + MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES)); + + for (index = 0; index < table->size; index++) { + intel_logical_ring_emit(ringbuf, reg_base + (index * 4)); + intel_logical_ring_emit(ringbuf, + table->table[index].control_value); + } + + /* + * Ok, now set the unused entries to uncached. These entries are + * officially undefined and no contact is given for the contents and + * settings is given for these entries. + * + * Entry 0 in the table is uncached - so we are just written that + * value to all the used entries. + */ + for (; index < GEN9_NUM_MOCS_ENTRIES; index++) { + intel_logical_ring_emit(ringbuf, reg_base + (index * 4)); + intel_logical_ring_emit(ringbuf, table->table[0].control_value); + } + + intel_logical_ring_emit(ringbuf, MI_NOOP); +} + +/** + * emit_mocs_l3cc_table() - emit the mocs control table + * @ringbuf: DRM device. + * @table: The values to program into the control regs. + * + * This function simply emits a MI_LOAD_REGISTER_IMM command for the + * given table starting at the given address. This register set is programmed + * in pairs. + * + * Return: Nothing. + */ +void emit_mocs_l3cc_table(struct intel_ringbuffer *ringbuf, + struct drm_i915_mocs_table *table) { + unsigned int count; + unsigned int i; + + intel_logical_ring_emit(ringbuf, + MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES / 2)); + + for (i = 0, count = 0; i < table->size / 2; i++, count += 2) { + u32 value = (table->table[count].l3cc_value & 0xffff) | + ((table->table[count + 1].l3cc_value & 0xffff) << 16); + + intel_logical_ring_emit(ringbuf, GEN9_LNCFCMOCS0 + (i * 4)); + intel_logical_ring_emit(ringbuf, value); + } + + /* + * Now set the rest of the table to uncached - use entry 0 as this + * will be uncached. Leave the last pair initialised as reserved by + * the hardware. + */ + for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) { + u32 value = (table->table[0].l3cc_value & 0xffff) | + ((table->table[0].l3cc_value & 0xffff) << 16); + + intel_logical_ring_emit(ringbuf, GEN9_LNCFCMOCS0 + (i * 4)); + intel_logical_ring_emit(ringbuf, value); + } + + intel_logical_ring_emit(ringbuf, MI_NOOP); +} + diff --git a/drivers/gpu/drm/i915/intel_mocs.h b/drivers/gpu/drm/i915/intel_mocs.h new file mode 100644 index 0000000..136d0cf --- /dev/null +++ b/drivers/gpu/drm/i915/intel_mocs.h @@ -0,0 +1,119 @@ +/* + * 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. + * + * Authors: + * Peter Antoine <peter.antoine@intel.com> + */ + +#ifndef INTEL_MOCS_H +#define INTEL_MOCS_H + +/** + * DOC: Memory Objects Control State (MOCS) + * + * Motivation: + * In previous Gens the MOCS settings was a value that was set by user land as + * part of the batch. In Gen9 this has changed to be a single table (per ring) + * that all batches now reference by index instead of programming the MOCS + * directly. + * + * The one wrinkle in this is that only PART of the MOCS tables are included + * in context (The GFX_MOCS_0 - GFX_MOCS_64 and the LNCFCMOCS0 - LNCFCMOCS32 + * registers). The rest are not (the settings for the other rings). + * + * This table needs to be set at system start-up because the way the table + * interacts with the contexts and the GmmLib interface. + * + * + * Implementation: + * + * The table is programmed on a platform basis from a table that is generated + * from the one that has been agreed by the different responsible parties. This + * tables (one per supported platform) is defined in intel_mocs.c and is + * programmed in the first batch after the context is loaded (with the hardware + * workarounds). This will then let the usual context handling keep the MOCS in + * step. + */ + +#include <drm/drmP.h> +#include "i915_drv.h" + +/* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */ +#define MOCS_CACHEABILITY(value) ((value & 0x03) << 0) +#define MOCS_TGT_CACHE(value) ((value & 0x03) << 2) +#define MOCS_LRUM(value) ((value & 0x03) << 4) +#define MOCS_AOM(value) ((value & 0x01) << 6) +#define MOCS_LECC_ESC(value) ((value & 0x01) << 7) +#define MOCS_LECC_SCC(value) ((value & 0x07) << 8) +#define MOC_PFM(value) ((value & 0x07) << 11) +#define MOCS_SCF(value) ((value & 0x01) << 14) + +/* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per word */ +#define MOCS_ESC(value) ((value & 0x01) << 0) +#define MOCS_SCC(value) ((value & 0x07) << 1) +#define MOCS_L3_CACHEABILITY(value) ((value & 0x03) << 4) + +/* Helper defines */ +#define GEN9_NUM_MOCS_RINGS (5) /* Number of mocs engines to program */ +#define GEN9_NUM_MOCS_ENTRIES (63) /* 63 out of 64 - 64 is rsvrd */ + +/* EDRAM Caching options */ +#define EDRAM_PAGETABLE (0) +#define EDRAM_UC (1) +#define EDRAM_RESERVED (2) +#define EDRAM_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) +#define LLC_ELLC (3) + +/* 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; +}; + +void emit_mocs_l3cc_table(struct intel_ringbuffer *ringbuf, + struct drm_i915_mocs_table *table); + +void emit_mocs_control_table(struct intel_ringbuffer *ringbuf, + struct drm_i915_mocs_table *table, + u32 reg_base); + +bool get_mocs_settings(struct drm_device *dev, + struct drm_i915_mocs_table *table); + +#endif +
This change adds the programming of the MOCS registers to the gen 9+ platforms. This change set programs the MOCS register values to a set of values that are defined to be optimal. 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. Signed-off-by: Peter Antoine <peter.antoine@intel.com> --- drivers/gpu/drm/i915/Makefile | 3 +- drivers/gpu/drm/i915/i915_reg.h | 9 ++ drivers/gpu/drm/i915/intel_lrc.c | 68 ++++++++++ drivers/gpu/drm/i915/intel_mocs.c | 252 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_mocs.h | 119 ++++++++++++++++++ 5 files changed, 450 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_mocs.c create mode 100644 drivers/gpu/drm/i915/intel_mocs.h