On Tue, Feb 27, 2018 at 1:30 AM, <kevin.rogo...@intel.com> wrote:
> From: Kevin Rogovin <kevin.rogo...@intel.com>
>
> Gen9 GPU's suffer from a HW bug where the GPU will hang if
> the GPU accesses a texture with a an auxilary buffer and
> an ASTC5x5 texture without having a pipeline cs stall (and
> texture cache flush) between such accesses. This patch
> creates the infrastucture to track such potential texture
> accesses and to issue the required pipeline stalls.
>
> Signed-off-by: Kevin Rogovin <kevin.rogo...@intel.com>
> ---
> src/mesa/drivers/dri/i965/Makefile.sources | 1 +
> src/mesa/drivers/dri/i965/brw_context.c | 6 +++++
> src/mesa/drivers/dri/i965/brw_context.h | 24 ++++++++++++++++++
> src/mesa/drivers/dri/i965/gen9_astc5x5_wa.c | 36
> +++++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 1 +
> src/mesa/drivers/dri/i965/meson.build | 1 +
> 6 files changed, 69 insertions(+)
> create mode 100644 src/mesa/drivers/dri/i965/gen9_astc5x5_wa.c
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
> b/src/mesa/drivers/dri/i965/Makefile.sources
> index 2f349aa..20f7dee 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -78,6 +78,7 @@ i965_FILES = \
> gen7_urb.c \
> gen8_depth_state.c \
> gen8_multisample_state.c \
> + gen9_astc5x5_wa.c \
> hsw_queryobj.c \
> hsw_sol.c \
> intel_batchbuffer.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index ea1c78d..a6e95b2 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -1058,6 +1058,12 @@ brwCreateContext(gl_api api,
> if (ctx->Extensions.INTEL_performance_query)
> brw_init_performance_queries(brw);
>
> + brw->astc5x5_wa.required = (devinfo->gen == 9);
> + brw->astc5x5_wa.mode = BRW_ASTC5x5_WA_MODE_NONE;
> + brw->astc5x5_wa.texture_astc5x5_present = false;
> + brw->astc5x5_wa.texture_with_auxilary_present = false;
> + brw->astc5x5_wa.blorp_sampling_from_astc5x5 = false;
> +
> vbo_use_buffer_objects(ctx);
> vbo_always_unmap_buffers(ctx);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 050b656..5fe0b29 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -166,6 +166,12 @@ enum brw_cache_id {
> BRW_MAX_CACHE
> };
>
> +enum brw_astc5x5_wa_mode_t {
> + BRW_ASTC5x5_WA_MODE_NONE,
> + BRW_ASTC5x5_WA_MODE_HAS_ASTC5x5,
> + BRW_ASTC5x5_WA_MODE_HAS_AUX,
> +};
> +
> enum brw_state_id {
> /* brw_cache_ids must come first - see brw_program_cache.c */
> BRW_STATE_URB_FENCE = BRW_MAX_CACHE,
> @@ -1296,6 +1302,19 @@ struct brw_context
> */
> enum isl_aux_usage draw_aux_usage[MAX_DRAW_BUFFERS];
>
> + /* Certain GEN's have a hardware bug where the sampler hangs if it
> attempts
> + * to access auxilary buffers and an ASTC5x5 compressed buffer. The
> workaround
> + * is to make sure that the texture cache is cleared between such
> accesses
> + * and that such accesses have a command streamer stall between them.
> + */
> + struct {
> + bool required;
>
I would rather have a helper function in brw_draw near where we implement
the workaround than having a boolean in brw_context. That way the code to
detect the need for the workaround is near the implementation of the
workaround.
> + enum brw_astc5x5_wa_mode_t mode;
> + bool texture_astc5x5_present;
> + bool texture_with_auxilary_present;
> + bool blorp_sampling_from_astc5x5;
>
This is quite a bit more infastructure than we really need in brw_context.
All we reall need is the enum. The rest just enables us to spread the
workaround throughout the entire driver which is a bad thing. Most of my
comments will focus on this theme.
> + } astc5x5_wa;
> +
> __DRIcontext *driContext;
> struct intel_screen *screen;
> };
> @@ -1729,6 +1748,11 @@ void brw_query_internal_format(struct gl_context
> *ctx, GLenum target,
> GLenum internalFormat, GLenum pname,
> GLint *params);
>
> +/* gen9_astc5x5_wa.c */
> +void gen9_set_astc5x5_wa_mode(struct brw_context *brw,
> + enum brw_astc5x5_wa_mode_t mode);
> +void gen9_astc5x5_perform_wa(struct brw_context *brw);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/src/mesa/drivers/dri/i965/gen9_astc5x5_wa.c
> b/src/mesa/drivers/dri/i965/gen9_astc5x5_wa.c
> new file mode 100644
> index 0000000..d3efd05
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/gen9_astc5x5_wa.c
> @@ -0,0 +1,36 @@
> +#include "brw_context.h"
> +#include "brw_defines.h"
> +#include "intel_mipmap_tree.h"
> +
> +void
> +gen9_set_astc5x5_wa_mode(struct brw_context *brw,
> + enum brw_astc5x5_wa_mode_t mode)
> +{
> + if (!brw->astc5x5_wa.required ||
> + mode == BRW_ASTC5x5_WA_MODE_NONE ||
> + brw->astc5x5_wa.mode == mode) {
> + return;
> + }
> +
> + if (brw->astc5x5_wa.mode != BRW_ASTC5x5_WA_MODE_NONE) {
> + const uint32_t flags = PIPE_CONTROL_CS_STALL |
> + PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE;
> + brw_emit_pipe_control_flush(brw, flags);
> + }
> +
> + brw->astc5x5_wa.mode = mode;
> +}
> +
> +void
> +gen9_astc5x5_perform_wa(struct brw_context *brw)
> +{
> + if (!brw->astc5x5_wa.required) {
> + return;
> + }
> +
> + if (brw->astc5x5_wa.texture_astc5x5_present) {
> + gen9_set_astc5x5_wa_mode(brw, BRW_ASTC5x5_WA_MODE_HAS_ASTC5x5);
> + } else if (brw->astc5x5_wa.texture_with_auxilary_present) {
> + gen9_set_astc5x5_wa_mode(brw, BRW_ASTC5x5_WA_MODE_HAS_AUX);
> + }
> +}
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index d0999bb..4e56fa9 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -692,6 +692,7 @@ brw_new_batch(struct brw_context *brw)
>
> /* Create a new batchbuffer and reset the associated state: */
> intel_batchbuffer_reset_and_clear_render_cache(brw);
> + brw->astc5x5_wa.mode = BRW_ASTC5x5_WA_MODE_NONE;
>
> /* If the kernel supports hardware contexts, then most hardware state
> is
> * preserved between batches; we only need to re-emit state that is
> required
> diff --git a/src/mesa/drivers/dri/i965/meson.build
> b/src/mesa/drivers/dri/i965/meson.build
> index e686614..55a5f15 100644
> --- a/src/mesa/drivers/dri/i965/meson.build
> +++ b/src/mesa/drivers/dri/i965/meson.build
> @@ -97,6 +97,7 @@ files_i965 = files(
> 'gen7_urb.c',
> 'gen8_depth_state.c',
> 'gen8_multisample_state.c',
> + 'gen9_astc5x5_wa.c',
> 'hsw_queryobj.c',
> 'hsw_sol.c',
> 'intel_batchbuffer.c',
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
