On Mon, Dec 04, 2017 at 09:01:44AM +0000, Rogovin, Kevin wrote:
> Hi,
> 
> > 1) do extra tex cache flush when needed and specifically only when needed
> > 2) resolve surfaces when undesired combination is about to be sampled
> 
> >Latter case could be addressed also with on-the-fly check in 
> >brw_predraw_resolve_inputs(). There one goes thru all the active textures 
> >for the
> > next draw call and resolves when necessary. One could check for the 
> > undesired combination of textures there. I understand we would need to
> > walk the textures twice, first to check for any occurrences of one type and 
> > then for the other. This, however, would fit to the way we resolve
> > other texture types and prevent from adding more things to check into the 
> > context.
> 
> This patch series does handle the second case, as follows:
>  a) Checking if there are astc5x5 textures and/or textures with auxiliary is 
> done in brw_validate_textures(); this was a really nice place because
>       the function loops over all textures anyways
>  b) the resolve operation is handled by the added function 
> brw_atsc_perform_wa(); this also sets the mode correctly too after the 
> resolve.
> 
> I chose to make a dedicated function that does the mode setting and resolve.  
> Putting the resolve logic into brw_predraw_resolve_inputs()
> is not a bad idea and I am open to it; the interior of the loop would then 
> look ickier as it would had the check on each texture unit of if
> the workaround is necessary and if there is an astc5x5 sampler handing 
> around. It would also kill off a member from the astc5x5_wa
> struct that tracks if there are auxiliary textures.

I wanted to see how it would look using brw_predraw_resolve_inputs(). See the
patch I sent as reply and let me know how you feel about doing something of
that sort.

Writing that reminded me of something I meant to ask: do we hit the sampler
bug with both directions: sampling astc5x5 first and then aux, and vice versa?
Or is only one direction problematic?

> 
> At any rate, I'd appreciate some review for the code so it can land in some 
> form shortly.
> 
> -Kevin
> 
> > 
> > 
> > Kevin Rogovin (5):
> >   i965: define astc5x5 workaround infrastructure
> >   i965: ASTC5x5 workaround logic for blorp
> >   i965: set ASTC5x5 workaround texture type tracking on texture validate
> >   i965: use ASTC5x5 workaround in brw_draw
> >   i965: use ASTC5x5 workaround in brw_compute
> > 
> >  src/mesa/drivers/dri/i965/brw_compute.c          |  6 +++
> >  src/mesa/drivers/dri/i965/brw_context.c          | 63 
> > ++++++++++++++++++++++++
> >  src/mesa/drivers/dri/i965/brw_context.h          | 23 +++++++++
> >  src/mesa/drivers/dri/i965/brw_draw.c             |  6 +++
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  5 ++
> >  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 +++++
> >  9 files changed, 134 insertions(+), 4 deletions(-)
> > 
> > --
> > 2.14.2
> > 
> > _______________________________________________
> > 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

Reply via email to