On 04/01/16 14:03, Marek Olšák wrote:
On Mon, Jan 4, 2016 at 2:09 PM, Jose Fonseca <jfons...@vmware.com> wrote:
On 04/01/16 12:25, Jose Fonseca wrote:
On 21/12/15 22:35, Marek Olšák wrote:
Hi,
This patch series adds more flexibility to u_upload_mgr. First, it
adds the ability to specify the alignment per suballocation. The idea
is that several users can use the same upload buffer, but each may
need a different alignment. Finally, it allows specifying PIPE_USAGE,
which usually affects memory placement.
Please review.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Hi Marek,
This patch series, or the commit
http://cgit.freedesktop.org/mesa/mesa/commit/?id=36c93a6fae275614b6004ec5ab085774d527e1bc
I bisected and it turns out that 36c93a6fae275614b6004ec5ab085774d527e1bc is
the bad commit, and not the u_upload_mgr series.
In particular the issue steams from the hunk:
+
+ /* We uploaded modified constants, need to invalidate them. */
+ st->dirty.mesa |= _NEW_PROGRAM_CONSTANTS;
I suspect that drawing the text via glBitmap suddently became a huge hotspot
becuase of this. If one disables the help text (pressing h) there's no
performance regression (frame rate is the same).
The need for the constants is explained above:
/* As an optimization, Mesa's fragment programs will sometimes get the
* primary color from a statevar/constant rather than a varying
variable.
* when that's the case, we need to ensure that we use the 'color'
* parameter and not the current attribute color (which may have
changed
* through glRasterPos and state validation.
* So, we force the proper color here. Not elegant, but it works.
*/
{
GLfloat colorSave[4];
COPY_4V(colorSave, ctx->Current.Attrib[VERT_ATTRIB_COLOR0]);
COPY_4V(ctx->Current.Attrib[VERT_ATTRIB_COLOR0], color);
st_upload_constants(st, st->fp->Base.Base.Parameters,
PIPE_SHADER_FRAGMENT);
COPY_4V(ctx->Current.Attrib[VERT_ATTRIB_COLOR0], colorSave);
}
I wonder if putting colors in constants as opposed to vertex buffer with
zero stride or an INT_MAX instancing divisor is really a good idea.
It's a good idea from the GPU perspective.
Using a vertex buffer is:
- one load instruction per thread in vertex shaders (64 per thread
group, though the cache should hide 63 of them in theory)
- the vertex shader output must be written (64 per thread group), so
on-chip memory must be allocated, which reduces parallelism just like
register usage reduces parallelism
- the pixel shader must load and interpolate the input (interpolation
= 3 on-chip vec4 loads per thread, 2 MAD instructions per thread, all
multiplied by 64)
Using a constant buffer is:
- 1 load instruction per entire pixel shader thread group on GCN,
because the constant load executes on a separate scalar unit and not
on SIMD units. This is best for performance and low power.
Ok. So maybe the solution is to avoid touching the constants unless
absolutely necessary (ie, they actually changed and/or will be actually
needed.)
Anyway, if this only affects llvmpipe, no biggie, but if this change makes
text rendering via glBitmap much slower for all HW drivers, then we should
probably revisit this.
IIRC, ipers is CPU-bound, so any GPU improvement won't be seen here,
but any additional overhead in st/mesa will hurt.
Note the issue seems to be isolated to drawing text. So maybe if we
disable all but the text it might better allow to zoom in on this issue.
(That is, I think I noticed this only on ipers because it is one of the
few demos I bench continously that has a help dialog enabled by default.)
> Also, do we have
know any real apps using glBitmap?
Yes, Brian Paul came across real world apps that use glBitmap for text
drawing or thin wrappers around them like wglUseFontBitmaps/glXUseXFont,
and IIRC performance did matter.
Jose
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
