On Mon, 2018-07-02 at 08:23 -0500, Jason Ekstrand wrote:
> On July 2, 2018 01:09:38 Iago Toral <ito...@igalia.com> wrote:
>
> > On Sun, 2018-07-01 at 18:30 -0500, Jason Ekstrand wrote:
> > > On June 29, 2018 03:11:00 Iago Toral Quiroga <ito...@igalia.com>
> > > wrote:
> > >
> > > > ---
> > > > src/intel/vulkan/anv_private.h | 5 +++++
> > > > src/intel/vulkan/genX_cmd_buffer.c | 12 +++++++-----
> > > > 2 files changed, 12 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/src/intel/vulkan/anv_private.h
> > > > b/src/intel/vulkan/anv_private.h
> > > > index 510471da602..1a9ab7013f2 100644
> > > > --- a/src/intel/vulkan/anv_private.h
> > > > +++ b/src/intel/vulkan/anv_private.h
> > > > @@ -1989,6 +1989,11 @@ struct anv_cmd_state {
> > > > * is one of the states in render_pass_states.
> > > > */
> > > > struct
> > > > anv_state null_surface_state;
> > > > +
> > > > + /**
> > > > + * Current state base address.
> > > > + */
> > > > + struct
> > > > anv_address base_state_address;
> > > > };
> > > >
> > > > struct anv_cmd_pool {
> > > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > > index 611311904e6..2847e0b30c9 100644
> > > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > > @@ -67,6 +67,12 @@
> > > > genX(cmd_buffer_emit_state_base_address)(struct
> > > > anv_cmd_buffer *cmd_buffer)
> > > > {
> > > > struct anv_device *device = cmd_buffer->device;
> > > >
> > > > + struct anv_address new_base_address =
> > > > + anv_cmd_buffer_surface_base_address(cmd_buffer);
> > > > + if (new_base_address.bo == cmd_buffer-
> > > > > state.base_state_address.bo &&
> > > >
> > > > + new_base_address.offset == cmd_buffer-
> > > > > state.base_state_address.offset)
> > > >
> > > > + return;
> > > > +
> > > > /* If we are emitting a new state base address we probably need
> > > > to re-emit
> > > > * binding tables.
> > > > */
> > > > @@ -90,8 +96,7 @@
> > > > genX(cmd_buffer_emit_state_base_address)(struct
> > > > anv_cmd_buffer *cmd_buffer)
> > > > sba.GeneralStateMemoryObjectControlState = GENX(MOCS);
> > > > sba.GeneralStateBaseAddressModifyEnable = true;
> > > >
> > > > - sba.SurfaceStateBaseAddress =
> > > > - anv_cmd_buffer_surface_base_address(cmd_buffer);
> > > > + sba.SurfaceStateBaseAddress = new_base_address;
> > > > sba.SurfaceStateMemoryObjectControlState = GENX(MOCS);
> > > > sba.SurfaceStateBaseAddressModifyEnable = true;
> > > >
> > > > @@ -1521,9 +1526,6 @@ genX(CmdExecuteCommands)(
> > > > /* Each of the secondary command buffers will use its own state
> > > > base
> > > > * address. We need to re-emit state base address for the
> > > > primary after
> > > > * all of the secondaries are done.
> > > > - *
> > > > - * TODO: Maybe we want to make this a dirty bit to avoid
> > > > extra
> > > > state base
> > > > - * address calls?
> > >
> > > I don't think this is correct. When a secondary executes, we
> > > have
> > > to
> > > reemit STATE_BASE_ADDRESS because the secondary used it's own and
> > > we
> > > need
> > > to set it back for the primary. The comment above was saying that
> > > we
> > > can
> > > probably avoid it if we have a bunch of ExecuteCommands calls
> > > back to
> > > back
> > > or if the last thing in the batch is a call out to a secondary.
> > > As is, I
> > > think this patch will cause problems in the case where the client
> > > uses a
> > > secondary followed by rendering in the primary. Have I missed
> > > something?
> >
> > I shouldn't remove the comment since this patche doesn't address
> > that
> > TODO, we still emit the state base address for the primary below,
> > the
> > only change in here is that if the base state address of the
> > primary is
> > the same as the one for the secondaries we won't actually emit the
> > state packet, but that should be fine. Maybe you thought I was
> > removing
> > the line below?
>
> The problem is that it never will be the same. The secondary always
> allocate a new binding table pool and re-emit STATE_BASE_ADDRESS so I
> don't
> think we're actually saving anything.
Ok, in that case I agree it won't help anything, thanks for clarifying.
Let's drop this patch then.
Iago
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
