[Mesa-dev] i965 CS state tracking

Wed Sep 3 11:34:33 PDT 2014

On 2014-09-03 10:14:34, Kenneth Graunke wrote:
> On Wednesday, September 03, 2014 09:22:12 AM Jordan Justen wrote:
> > > 15:22:24 Kayden | jljusten: so how does the compute shader state upload work?
> > > 15:22:35 Kayden | I know you have to issue PIPELINE_SELECT to pick the
> > >                 | GPGPU pipe instead of 3D
> > > 15:22:53 Kayden | but does the GPGPU pipe use a lot of the same commands?
> > > 15:22:57 Kayden | do you have to reprogram them when switching?
> > > 15:30:19 Kayden | I don't understand why we have to have separate
> > >                 | dirty bit sets for compute vs. 3D
> > > 15:30:52 Kayden | It looks like if you're called by glDispatchCompute,
> > >                 | we should PIPELINE_SELECT to GPGPU, and do whatever
> > >                 | state upload you need for compute
> > > 15:31:13 Kayden | and if you're called by glDrawWhatever(), we should
> > >                 | PIPELINE_SELECT to 3D and do the existing 3D state
> > >                 | upload
> > > 15:31:52 Kayden | unless things like 3DSTATE_BINDING_TABLE_POINTERS
> > >                 | only takes effect on one pipe or the other, I
> > >                 | guess...
> > > 15:32:54 Kayden | but if most of the commands are different between
> > >                 | the two pipes...
> > > 15:34:20 Kayden | based on reading Paul's notes I don't see why he did
> > >                 | that.
> > > 15:55:08    krh | Kayden: there's about 3-4 commands you need to set up
> > >                 | gpgpu
> > > 15:55:18    krh | none of which are shared with the 3d pipeline
> > > 15:55:36    krh | aside from the pipeline select, it's completely
> > >                 | orthogonal
> > > 15:56:29    krh | I guess I don't understand why we need to track all the
> > >                 | state for all the things either
> > > 15:57:05    krh | but we do need to make sure update_state doesn't emit
> > >                 | dirty 3d state when we're trying to do cs and vice verse
> > > 15:57:08    krh | versa
> > > 15:57:41    krh | it seems like a pipeline mask should be enough
> > > 15:58:11    krh | ie, mask the dirty flags against a
> > >                 | pipeline_3d_flag_mask when we're emitting state for 3d
> > 
> > I'm not sure of Paul's thoughts.
> > 
> > One thought I have is, what if we decide to upload some state
> > differently depending on the pipeline? Then when we switch, we'd need
> > to make sure we re-emit it again for the other pipeline.
> 
> That's easy to handle.  In brw_upload_state, we can check
> brw->state.current_pipeline != pipeline to know when we've changed
> pipelines.  We can even detect if we're going from 3D -> GPGPU or
> vice versa by checking the existing value of
> brw->state.current_pipeline.  When the pipeline changes, we just
> need to flag the dirty bits for any state that gets implicitly
> clobbered.

Maintaining the list of 'any state that gets implicitly clobbered'
separate from the atoms seems like a hassle.

What about 'flagging all overlapping' state bits when changing
pipelines? (We could scan the two state atom lists to find the
overlap.)

But, I think this might cause excess re-emittion in some
scenarios. For example:
* Change atomics from GL, mark atomics dirty
* Render, atomics state is emitted and clean
* Pipeline=>Compute, mark atomics dirty and re-emit
* Pipeline=>Render, mark atomics dirty and re-emit
  - Did we need to re-emit here?

And, how about this:
* atomics are not used
* Render
* Pipeline=>Compute, mark atomics dirty and emit
  - Did we need to emit here?

I guess these scenarios seem to indicate the 2 sets of dirty bits
might instead be used to prevent excess state emission.

> > Does this seem explanation seem plausible?
> 
> No, and I think we should revert the patch.  Having two state atom
> lists seems sensible, and passing an "enum brw_pipeline" parameter
> to brw_upload_state also seems sensible.  But having two sets of
> dirty bits doesn't yet make sense to me.

Can we discuss and implement an alternative approach rather than
reverting it? I don't think it is wrong, but maybe could (perhaps) be
done better.

I will own implementing whatever change we decide on.

-Jordan

> Now, if GPGPU and 3D had largely overlapping state packets, yet were
> on separate /rings/ that preserved their state independently of one
> another...then I think it would make sense.  But that doesn't seem
> to be the case.
> 
> I'm sorry I wasn't able to review this series within a week before
> it landed; I've been trying to go through a 23 patch series from
> Matt, a 13 patch series from Connor, and a 37 patch series from Iago
> and Sam, and still haven't gotten to your other 14 patch series, so
> I'm just a little behind these days...and with the holiday...
> 
> --Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140903/a2137843/attachment-0001.sig>