I don't see anything wrong with this patch but you might want wait for
Ben's approval as he might know better than me if this work-around is
right (he is the author of the kernel commit mentioned in this patch).
Reviewed-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
Sam
On 18/11/15 06:54, Jordan Justen wrote:
> From: Francisco Jerez <curroje...@riseup.net>
>
> 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.
> ---
> src/mesa/drivers/dri/i965/brw_state.h | 4 +++
> src/mesa/drivers/dri/i965/gen7_l3_state.c | 48
> +++++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 7 ++++
> src/mesa/drivers/dri/i965/intel_batchbuffer.h | 6 +++-
> 4 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> b/src/mesa/drivers/dri/i965/brw_state.h
> index 49f301a..b7c0039 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -380,6 +380,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 45bad02..84ab118 100644
> --- a/src/mesa/drivers/dri/i965/gen7_l3_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_l3_state.c
> @@ -495,3 +495,51 @@ 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_l3_weights w =
> + get_default_l3_weights(brw->intelScreen->devinfo, false, false);
> + const struct brw_l3_config *const cfg =
> + get_l3_config(brw->intelScreen->devinfo, w);
> +
> + 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 0363bd3..f778074 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 2b177d3..f473690 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;
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
