[Mesa-dev] [PATCH 2/2] gallium: add flag PIPE_TRANSFER_MAP_PERMANENTLY

[Jose Fonseca]

----- Original Message -----
> On Tue, Jan 10, 2012 at 5:15 PM, Jose Fonseca <jfonseca at vmware.com>
> wrote:
> > ----- Original Message -----
> >> The flag is optional, it doesn't have to implemented by everybody.
> >> If
> >> we do the uploads in the state tracker, we will also do any
> >> required
> >> data transformation so that drivers don't have to do it at all.
> >
> > The same can be said for everything.
> >
> > I don't object adding yet another code path specific to a subset of
> > hardware, provided the benefits justify its existence.
> >
> > But honestly I'm not yet convinced of this, as there was no attempt
> > to backup with solid arguments why this matters.
> >
> > Furthermore this violates one of the principles gallium (and all 3D
> > apis) have of unmapping all resources when drawing.
> 
> If my idea violates one principle of all 3D APIs, this violates them
> all:
> http://www.opengl.org/registry/specs/AMD/pinned_memory.txt

Indeed.

> It allows reading user memory by hardware, and neither the driver nor
> the hardware is notified when the memory contents are changed by the
> user.
>
> BTW, I don't insist on this. I thought it would be a nice addition
> allowing user buffer uploads to be moved out of (especially radeon)
> drivers. If you really believe it's a bad optimization, feel free to
> revert. WIthout it, things wouldn't change for me at all...

Fair enough.

> >
> >> > It looks to me that state trackers and/or drivers are not
> >> > properly
> >> > using PIPE_USAGE_STREAM flag.  As all cases where
> >> > PIPE_TRANSFER_MAP_PERMANENTLY would be used, the right way to do
> >> > it would be for the state tracker to set PIPE_USAGE_STREAM, the
> >> > pipe driver to recognize PIPE_USAGE_STREAM, and keep the mapping
> >> > permanently internally, making mapping/unmapping of such buffers
> >> > mere no-ops.
> >>
> >> We were doing that already and it wasn't good enough. Avoiding
> >> create+map+unmap+destroy *calls* have brought higher frame rates
> >> in
> >> apps with lots of draw operations.
> >
> > I understand the number map/unmap call is reduced. But how does
> > this interface change affect in any way the number of
> > create/destroy calls?
> 
> create = get_transfer
> destroy = transfer_destroy

Ah. BTW, we should probably unify create+map and destroy+unmap to reduce overhead. I would probably helps here.

> > Also, please provide app name and performance figures w/ this
> > change.
> 
> OK. Torcs, the Forza track at the start.
> 
> With u_upload_unmap before drawing:
> 21.4 - 22.1 fps
> 
> Without u_upload_unmap:
> 22.7 - 23.1 fps


> The improvement is approximately 1.1 fps, which is probably too
> little
> for other people to care, but why throw it away?

This is roughly 5%. It is still significant.

But it surprises me it is so high.

Perhaps we should have a couple of fast entrypoints for transfering to/from buffers without the overhead of creating transfer objects.

Jose