Hi, The 3rd patch, "i965: use ASTC5x5 workaround in intel_miptree_texture_aux_usage has issues: 1. Definitely: brw_draw lacks the call to gen9_astc5x5_perform_wa() which generates the needed flush between batchbuffers
2. Uneasy: I am nervous about hitting intel_miptree_texture_aux_usage() as it is used in lots of places directly and indirectly. The interaction with blorp resolve makes me very uneasy.... I would rather restore the bool argument disable_aux to intel_miptree_prepare_render() to keep the ASTC evil a little more localized (the function is used only in brw_draw.c for resolving inputs). -Kevin -----Original Message----- From: Palli, Tapani Sent: Monday, February 12, 2018 10:14 AM To: Rogovin, Kevin <kevin.rogo...@intel.com>; Jason Ekstrand <ja...@jlekstrand.net> Cc: ML mesa-dev <mesa-dev@lists.freedesktop.org> Subject: Re: [Mesa-dev] [PATCH v2 0/5] i965: ASTC5x5 workaround On 02/12/2018 09:44 AM, Tapani Pälli wrote: > Hi; > > On 02/08/2018 09:50 AM, Rogovin, Kevin wrote: >> Hi, >> >> I gave it a whirl of setting the .mocs field set to 0 passed to >> isl_surf_fill_state() ALWAYS. Sadly CarChase GLES continued to hang >> (where as the GL did not because it does not use ASTC). This makes >> sense since MOCS (atleast last time I looked at it) only really >> controls cache usage for L3 and eLLC (please anyone correct me if I >> am wrong in this) whereas the issue is that the samplers mess up how >> they deal with its own (private) cache. >> >> It really is nasty that it appears (as of now) that this complicated >> work around is needed and needs to somehow be re-implemented in anv >> as well. > > It seems surrounding code has changed so that these patches need some > changes. Kevin, are you planning to rebase/refactor these changes? FYI I've rebased the patches and did additional porting (because of commit df13588d21) here: https://cgit.freedesktop.org/~tpalli/mesa/log/?h=astc5x5 Let me know if this looks OK for you. > >> -Kevin >> >> *From:*Jason Ekstrand [mailto:ja...@jlekstrand.net] >> *Sent:* Thursday, February 8, 2018 2:47 AM >> *To:* Rogovin, Kevin <kevin.rogo...@intel.com> >> *Cc:* ML mesa-dev <mesa-dev@lists.freedesktop.org> >> *Subject:* Re: [Mesa-dev] [PATCH v2 0/5] i965: ASTC5x5 workaround >> >> Random thought: >> >> Nanley and I were talking about this just now and I was complaining >> about how much I hate the fact that this workaround exists because we >> can't implement it in Vulkan. Then I got an idea. What would happen >> if we just set MOCS to zero (uncached) for ASTC 5x5 textures? Does >> that make the hang go away? How bad is the car chase performance >> with that compared to this series? It's a bit of a big hammer but >> has the advantage of simplicity. If it causes performance to tank on >> anything then then the more complex solution is probably worth it but >> I thought it was worth a try. >> >> --Jason >> >> On Thu, Dec 14, 2017 at 9:39 AM, <kevin.rogo...@intel.com >> <mailto:kevin.rogo...@intel.com>> wrote: >> >> From: Kevin Rogovin <kevin.rogo...@intel.com >> <mailto:kevin.rogo...@intel.com>> >> >> This patch series implements a needed workaround for Gen9 for >> ASTC5x5 >> sampler reads. The crux of the work around is to make sure that >> the >> sampler does not read an ASTC5x5 texture and a surface with an >> auxilary >> buffer without having a texture cache invalidate and command >> streamer >> stall between such accesses. >> >> With this patch series applied to the (current) master branch of >> mesa, >> carchase works on my SKL GT4. >> >> v2: >> Rename workaround functions from brw_ to gen9_ >> (suggested/requested by Topi Pohjolainen). >> >> Place texture resolve to avoid using auxilary surface >> when ASTC5x5 is detected in brw_predraw_resolve_inputs() >> instead of another detected function; doing so allows >> one to avoid walking the textures again. >> (suggested/requested by Topi Pohjolainen). >> >> Emit command streamer stall in addition to texture >> invalidate. >> (original short-coming caught by Jason Ekstrand) >> >> Place workaround function in (new) dedicated file. >> >> Minor path re-ordering to accomodate changes. >> >> Kevin Rogovin (5): >> i965: define astx5x5 workaround infrastructure >> i965: set ASTC5x5 workaround texture type tracking on texture >> validate >> i965: use ASTC5x5 workaround in brw_draw >> i965: use ASTC5x5 workaround in brw_compute >> i965: ASTC5x5 workaround logic for blorp >> >> src/mesa/drivers/dri/i965/Makefile.sources | 1 + >> src/mesa/drivers/dri/i965/brw_compute.c | 6 ++++ >> src/mesa/drivers/dri/i965/brw_context.c | 6 ++++ >> src/mesa/drivers/dri/i965/brw_context.h | 24 >> ++++++++++++++++ >> src/mesa/drivers/dri/i965/brw_draw.c | 16 >> +++++++++-- >> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 5 ++++ >> src/mesa/drivers/dri/i965/gen9_astc5x5_wa.c | 36 >> ++++++++++++++++++++++++ >> src/mesa/drivers/dri/i965/genX_blorp_exec.c | 5 ++++ >> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 1 + >> src/mesa/drivers/dri/i965/intel_tex_image.c | 16 >> ++++++++--- >> src/mesa/drivers/dri/i965/intel_tex_validate.c | 13 +++++++++ >> src/mesa/drivers/dri/i965/meson.build | 1 + >> 12 files changed, 124 insertions(+), 6 deletions(-) >> create mode 100644 src/mesa/drivers/dri/i965/gen9_astc5x5_wa.c >> >> -- >> 2.7.4 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> <mailto: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 >> > _______________________________________________ > 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
