[Mesa-dev] [PATCH 2/2] radeonsi: Add compute support v2

[Michel Dänzer]

On Die, 2013-03-12 at 17:42 -0400, Alex Deucher wrote: 
> On Tue, Mar 12, 2013 at 4:23 PM, Tom Stellard <tom at stellard.net> wrote:
> > From: Tom Stellard <thomas.stellard at amd.com>
> >
> > v2:
> >   - Only dump shaders when env variable is set.
> 
> A couple of comments below, other than that, looks good.
> 
> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

Likewise,

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


> > @@ -139,6 +140,11 @@ void si_pm4_inval_texture_cache(struct si_pm4_state *state)
> >         state->cp_coher_cntl |= S_0085F0_TC_ACTION_ENA(1);
> >  }
> >
> > +void si_pm4_inval_texture_l1_cache(struct si_pm4_state *state)
> > +{
> > +       state->cp_coher_cntl |= S_0085F0_TCL1_ACTION_ENA(1);
> > +}
> > +
> 
> Is there any value in keeping the L1 flush separate?

I don't think so: TC_ACTION_ENA should take care of L1 as well (search
for INVL2 in the register spec).

> Would it make more sense to just add it to si_pm4_inval_texture_cache()?

Yeah, for clarity's sake it might be a good idea to make the above
explicit by adding S_0085F0_TCL1_ACTION_ENA where S_0085F0_TC_ACTION_ENA
is used.


> On a somewhat related note, I'm also not sure it's worth having a
> separate si_pm4_inval_vertex_cache() since there is no VC anymore and
> the function is identical to si_pm4_inval_texture_cache().

Right, I think the main reason I kept it was in case it might help share
more code with r600g again.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer