[Mesa-dev] [PATCH v2 0/5] i965: ASTC5x5 workaround

Tapani Pälli tapani.palli at intel.com
Wed Feb 14 07:58:19 UTC 2018 


On 14.02.2018 09:54, Tapani Pälli wrote:
> 
> 
> On 14.02.2018 09:38, Rogovin, Kevin wrote:
>> 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
> 
> Now it happens via intel_miptree_prepare_texture (in 
> intel_miptree_texture_aux_usage).

No it does not, sorry for that :) Yeah I believe I need to restore that.


> 
>>     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).
> 
> IMO it feels more centralized .. but no strong opinions here. I haven't 
> spotted any regressions because of this.
> 
> 
>> -Kevin
>>
>>
>> -----Original Message-----
>> From: Palli, Tapani
>> Sent: Monday, February 12, 2018 10:14 AM
>> To: Rogovin, Kevin <kevin.rogovin at intel.com>; Jason Ekstrand 
>> <jason at jlekstrand.net>
>> Cc: ML mesa-dev <mesa-dev at 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:jason at jlekstrand.net]
>>>> *Sent:* Thursday, February 8, 2018 2:47 AM
>>>> *To:* Rogovin, Kevin <kevin.rogovin at intel.com>
>>>> *Cc:* ML mesa-dev <mesa-dev at 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.rogovin at intel.com
>>>> <mailto:kevin.rogovin at intel.com>> wrote:
>>>>
>>>>      From: Kevin Rogovin <kevin.rogovin at intel.com
>>>>      <mailto:kevin.rogovin at 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 at lists.freedesktop.org
>>>> <mailto:mesa-dev at lists.freedesktop.org>
>>>>      https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

More information about the mesa-dev mailing list