From 1fc797e8e408522cfbd3fa9f81d4fb33acccb034 Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Thu, 3 Sep 2015 18:23:19 +0300 Subject: [PATCH] i965: Work around L3 state leaks during context switches. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This is going to require some rather intrusive kernel changes to fix properly, in the meantime (and forever on at least pre-v4.1 kernels) we'll have to restore the hardware defaults at the end of every batch in which the L3 configuration was changed to avoid interfering with the DDX and GL clients that use an older non-L3-aware version of Mesa. Reviewed-by: Samuel Iglesias Gonsálvez Reviewed-by: Kristian Høgsberg v2: Optimize look-up of the default configuration by assuming it's the first entry of the L3 config array in order to avoid an FPS regression in GpuTest Triangle and SynMark OglBatch2-7 on most affected platforms. Reviewed-by: Jordan Justen --- src/mesa/drivers/dri/i965/brw_state.h | 4 ++ src/mesa/drivers/dri/i965/gen7_l3_state.c | 61 +++++++++++++++++-- src/mesa/drivers/dri/i965/intel_batchbuffer.c | 7 +++ src/mesa/drivers/dri/i965/intel_batchbuffer.h | 6 +- 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h index 44537aac614..59acec990be 100644 --- a/src/mesa/drivers/dri/i965/brw_state.h +++ b/src/mesa/drivers/dri/i965/brw_state.h @@ -383,6 +383,10 @@ void gen7_update_binding_table_from_array(struct brw_context *brw, void gen7_disable_hw_binding_tables(struct brw_context *brw); void gen7_reset_hw_bt_pool_offsets(struct brw_context *brw); +/* gen7_l3_state.c */ +void +gen7_restore_default_l3_config(struct brw_context *brw); + #ifdef __cplusplus } #endif diff --git a/src/mesa/drivers/dri/i965/gen7_l3_state.c b/src/mesa/drivers/dri/i965/gen7_l3_state.c index 79569358914..b63e61ca8f0 100644 --- a/src/mesa/drivers/dri/i965/gen7_l3_state.c +++ b/src/mesa/drivers/dri/i965/gen7_l3_state.c @@ -59,7 +59,9 @@ struct brw_l3_config { }; /** - * IVB/HSW validated L3 configurations. + * IVB/HSW validated L3 configurations. The first entry will be used as + * default by gen7_restore_default_l3_config(), otherwise the ordering is + * unimportant. */ static const struct brw_l3_config ivb_l3_configs[] = { /* SLM URB ALL DC RO IS C T */ @@ -81,7 +83,7 @@ static const struct brw_l3_config ivb_l3_configs[] = { }; /** - * VLV validated L3 configurations. + * VLV validated L3 configurations. \sa ivb_l3_configs. */ static const struct brw_l3_config vlv_l3_configs[] = { /* SLM URB ALL DC RO IS C T */ @@ -97,7 +99,7 @@ static const struct brw_l3_config vlv_l3_configs[] = { }; /** - * BDW validated L3 configurations. + * BDW validated L3 configurations. \sa ivb_l3_configs. */ static const struct brw_l3_config bdw_l3_configs[] = { /* SLM URB ALL DC RO IS C T */ @@ -113,7 +115,7 @@ static const struct brw_l3_config bdw_l3_configs[] = { }; /** - * CHV/SKL validated L3 configurations. + * CHV/SKL validated L3 configurations. \sa ivb_l3_configs. */ static const struct brw_l3_config chv_l3_configs[] = { /* SLM URB ALL DC RO IS C T */ @@ -520,3 +522,54 @@ const struct brw_tracked_state gen7_l3_state = { }, .emit = emit_l3_state }; + +/** + * Hack to restore the default L3 configuration. + * + * This will be called at the end of every batch in order to reset the L3 + * configuration to the default values for the time being until the kernel is + * fixed. Until kernel commit 6702cf16e0ba8b0129f5aa1b6609d4e9c70bc13b + * (included in v4.1) we would set the MI_RESTORE_INHIBIT bit when submitting + * batch buffers for the default context used by the DDX, which meant that any + * context state changed by the GL would leak into the DDX, the assumption + * being that the DDX would initialize any state it cares about manually. The + * DDX is however not careful enough to program an L3 configuration + * explicitly, and it makes assumptions about it (URB size) which won't hold + * and cause it to misrender if we let our L3 set-up to leak into the DDX. + * + * Since v4.1 of the Linux kernel the default context is saved and restored + * normally, so it's far less likely for our L3 programming to interfere with + * other contexts -- In fact restoring the default L3 configuration at the end + * of the batch will be redundant most of the time. A kind of state leak is + * still possible though if the context making assumptions about L3 state is + * created immediately after our context was active (e.g. without the DDX + * default context being scheduled in between) because at present the DRM + * doesn't fully initialize the contents of newly created contexts and instead + * sets the MI_RESTORE_INHIBIT flag causing it to inherit the state from the + * last active context. + * + * It's possible to realize such a scenario if, say, an X server (or a GL + * application using an outdated non-L3-aware Mesa version) is started while + * another GL application is running and happens to have modified the L3 + * configuration, or if no X server is running at all and a GL application + * using a non-L3-aware Mesa version is started after another GL application + * ran and modified the L3 configuration -- The latter situation can actually + * be reproduced easily on IVB in our CI system. + */ +void +gen7_restore_default_l3_config(struct brw_context *brw) +{ + const struct brw_device_info *devinfo = brw->intelScreen->devinfo; + /* For efficiency assume that the first entry of the array matches the + * default configuration. + */ + const struct brw_l3_config *const cfg = get_l3_configs(devinfo); + assert(cfg == get_l3_config(devinfo, + get_default_l3_weights(devinfo, false, false))); + + if (cfg != brw->l3.config && brw->can_do_pipelined_register_writes) { + setup_l3_config(brw, cfg); + update_urb_size(brw, cfg); + brw->l3.config = cfg; + } +} diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c index 0363bd3789a..f77807472fd 100644 --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c @@ -208,6 +208,13 @@ brw_finish_batch(struct brw_context *brw) brw_emit_query_end(brw); if (brw->batch.ring == RENDER_RING) { + /* Work around L3 state leaks into contexts set MI_RESTORE_INHIBIT which + * assume that the L3 cache is configured according to the hardware + * defaults. + */ + if (brw->gen >= 7) + gen7_restore_default_l3_config(brw); + /* We may also need to snapshot and disable OA counters. */ brw_perf_monitor_finish_batch(brw); diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h b/src/mesa/drivers/dri/i965/intel_batchbuffer.h index 2b177d3a888..f47369029a0 100644 --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.h +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.h @@ -30,8 +30,12 @@ extern "C" { * - 5 dwords for initial mi_flush * - 2 dwords for CC state setup * - 5 dwords for the required pipe control at the end + * - Restoring L3 configuration: (24 dwords = 96 bytes) + * - 2*6 dwords for two PIPE_CONTROL flushes. + * - 7 dwords for L3 configuration set-up. + * - 5 dwords for L3 atomic set-up (on HSW). */ -#define BATCH_RESERVED 152 +#define BATCH_RESERVED 248 struct intel_batchbuffer; -- 2.30.2