This fixes a bunch of GPU hangs introduced in some CTS
tests like
dEQP-VK.memory.pipeline_barrier.host_write_uniform_buffer.65536
It works around an issue seen in the LLVM backend, but
also makes the radv code work more like the radeonsi stack.
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This is ported from GLSL and converts
if (cond)
discard;
into
discard_if(cond);
This removes a block, but also is needed by radv
to workaround a bug in the LLVM backend.
v2: handle if (a) discard_if(b) (nha)
cleanup and drop pointless loop (Matt)
make sure there are no dependent phis (Eric)
v3: make sure only one instruction in the then block.
v4: remove sneaky tabs, add cursor init (Eric)
Reviewed-by: Eric Anholt <eric@anholt.net>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
We are going to start lowering to this in NIR code,
so prepare radv for it.
v2: handle conversion to kilp properly (nha)
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Since our surface state buffer is shared by all batches, the kernel does a
full stall and sync with the CPU between batches every time we call
execbuf2 because it refuses to do relocations on an active buffer. Doing
them in userspace and passing the NO_RELOC flag to the kernel allows us to
perform the relocations without stalling.
This improves the performance of Dota 2 by around 30% on a Sky Lake GT2.
v2 (Jason Ekstrand):
- Better comments (Chris Wilson)
- Fixed write_reloc for correct canonical form (Chris Wilson)
v3 (Jason Ekstrand):
- Skip relocations which aren't needed
- Provide an environment variable to always use the kernel
- More comments about correctness (Chris Wilson)
v4 (Jason Ekstrand):
- More comments (Chris Wilson)
v5 (Jason Ekstrand):
- Rebase on top of moving execbuf2 setup go QueueSubmit
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
Ever since the early days of the Vulkan driver, we've been setting up the
lists of relocations at EndCommandBuffer time. The idea behind this was to
move some of the CPU load out of QueueSubmit which the client is required
to lock around and into command buffer building which could be done in
parallel. Then QueueSubmit basically just becomes a bunch of execbuf2
calls.
Technically, this works. However, when you start to do more in QueueSubmit
than just execbuf2, you start to run into problems. In particular, if a
block pool is resized between EndCommandBuffer and QueueSubmit, the list of
anv_bo's and the execbuf2 object list can get out of sync. This can cause
problems if, for instance, you wanted to do relocations in userspace.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
The original reason for putting it in the batch_bo was to allow primaries
to share it across secondaries or something like that. However, the
relocation lists in secondary command buffers are are always left alone and
copied into the primary command buffer's relocation list. This means that
the offset really applies at the command buffer level and putting it in the
batch_bo doesn't make sense. This fixes a couple of potential bugs around
re-submission of command buffers that are not likely to be hit but are bugs
none the less.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
This commit adds a little helper struct for storing everything we use to
build an execbuf2 call. Since the add_bo function really has nothing to do
with a command buffer, it makes sense to break it out a bit. This also
reduces some of the churn in the next commit.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
The old version wasn't properly handling large addresses where we have to
sign-extend to get it into the "canonical form" expected by the hardware.
Also, the new version is capable of doing a clflush of the newly written
reloc if requested.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
Since -1 is an invalid GPU address, this lets us know whether or not we
have a valid address for a buffer. We don't get a valid address until the
first time that buffer is used in an execbuf2 ioctl.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
The previous implementation was being overly clever and using the
anv_bo::size field as its mutex. Scratch pool allocations don't happen
often, will happen at most a fixed number of times, and never happen in the
critical path (they only happen in shader compilation). We can make this
much simpler by just using the device mutex. This also means that we can
start using anv_bo_init_new directly on the bo and avoid setting fields
one-at-a-time.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
This ensures that we're always setting all of the fields in anv_bo
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
Because our relocation processing happens at EndCommandBuffer time and
because RENDER_SURFACE_STATE objects may be shared by batches, we really
have no clue whatsoever what address is actually written to the relocation
offset in the BO. We need to stop making such claims to the kernel and
just let it relocate for us.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
This puts the actual execbuf2 call in anv_batch_chain.c along with the
other relocation stuff.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
This wrapper ensures that we always update all anv_bo::offset fields based
on the offsets returned by the kernel.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
When you fire up Dota2 on Haswell you get spammed with thousands of
"Implement Gen7 HZ ops" finishme's. The point of anv_finishme is to act as
a reminder that there is something left to implement. Printing it once
should be sufficient.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Trivial. There's some regressions internally, related to overflow
behavior. I'll have to look at it at another time, some interactions
with vsplit/vcache are actually mind-blowing.
This reverts commit 3fa10ffb49.
This patch should have been the part of commit e592f7df.
In a situation when there are multiple render targets with alpha testing
enabled, if fragment shader doesn't write to draw buffer zero, it causes
the GPU hang on SKL. No GPU hang is seen on HSW. Simulator gives a
warning for all gen6+ h/w:
"Illegal render target write message length 0xa expected 0xc"
This patch fixes the GPU hang as well as the simulator warning with
new piglit test fbo-mrt-alphatest-no-buffer-zero-write:
https://patchwork.freedesktop.org/patch/118212
No regressions in Jenkins CI system.
Cc: "12.0 13.0" <mesa-stable@lists.freedesktop.org>
Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
I was getting a random GPU hang in the renderpass simple tests,
it turns out sometimes radv emitted the wrong thing "last".
This fixes the logic to emit Z/stencil last if they occur,
and not mark a color output as last. Also this relies on the
Z/STENCIL being the first two fragment outputs, which they are
so yay.
Fixes: dEQP-VK.renderpass.simple.color_depth (random hangs)
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
Signed-off-by: Dave Airlie <airlied@redhat.com>
The first IF statement disabled the second one.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98599
Reviewed-by: Brian Paul <brianp@vmware.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Some of the details of this function are very confusing and have a long
history. We should document that history and this seems like the best
place to do it.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
At least on Sky Lake, after emitting 3DSTATE_CONSTANT_*, you are required
to re-emit the 3DSTATE_BINDING_TABLE_POINTERS packet for the corresponding
stage. If you don't, double-buffering may fail and you may get the wrong
constants. It turns out that you need to do this even if you have no push
constants to speak of or else the next 3DSTATE_CONSTANT packet you emit for
that stage may not work correctly.
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Cc: "13.0" <mesa-stable@lists.freedesktop.org>
For gen9+ this will indicate when we should allow hiz based sampling
during rendering.
Improves performance in :
- Synmark's OglDeferred by 2.2% (n=20)
- Synmark's OglShMapPcf by 0.44% (n=20)
v2 by Ben: Add spec reference, and make it fix with some of the changes made on
the previous patches
Change the check from mt->aux_buf to mt->num_samples. The presence of an aux_buf
isn't enough to determine there isn't a HiZ buffer to use.
v3: It seems all depth surface end up with num_samples = 0 by default,
so allow sampling from depth HiZ if num_samples <= 1. (Lionel)
Allow sampling from HiZ only if all LOD are available from the HiZ
buffer. (Lionel)
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> (v1)
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com> (v2)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v3)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
The original functionality this patch introduces was authored by a patch from
Ken (the commit subject was the same). Since I ended up changing so many patches
in the code before this one, I had some non-trivial decisions to make, and I
didn't feel it was appropriate to keeps Ken's name as author (mostly because he
might not like what I've done). Ken's original patch was like 2 LOC :-)
In either case, some credit needs to go to Ken, and to Jordan for a few small
other changes in that original patch.
v2: Back to a smaller diff now that ISL handles most of the actual
programming (Lionel)
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com> (v1)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Currently it indicates that this is never supported, but soon it will
be supported for gen8+^w gen9+
v2 by Ben:
- Explicitly disable aux_hiz for gen < 9 (with comment)
- squashed in next patch to avoid unused and useless functions
i965: Support sampling with hiz during rendering
For gen8, we can sample from depth while using the hiz buffer. This
allows us to sample depth without resolving from hiz to the depth
texture.
To do this we must resolve to hiz before drawing so we can use the hiz
buffer to sample while rendering. Hopefully the hiz buffer will
already be resolved in most cases because it was previously rendered,
meaning the hiz resolve is a no-op.
Note that this is still controlled by the
intel_miptree_sample_with_hiz function, and we will enable hiz
sampling for gen8 in a separate patch.
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> (v1)
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com> (v2)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
This seems counter to the goal of consolidating hiz, mcs, and later ccs buffers.
Unfortunately, hiz on gen6 is a thing the code supports, and this wart will be
helpful to achieve that. Overall, I believe it does help unify AUX buffers on
gen7+.
I updated the size field which I introduced in the previous patch, even though
we have no use for it.
XXX: As I mentioned in the last patch, the height given to the MCS buffer
allocation in intel_miptree_alloc_mcs() looks wrong, but I don't claim to fully
understand how the MCS buffer is laid out.
v2: rebase on master (Lionel)
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com> (v1)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
This patch will preserve the BO & offset, and not the miptree for the
aux_mcs buffer. Eventually it might make sense to pull put the sizing
function in miptree creation, but for now this should be sufficient
and not too hideous.
v2: Save BO's offset too (Lionel)
v3: Squash previous patch storing the size of the allocated aux buffer
(Lionel)
Fix memory leak with mcs_buf->bo (Lionel)
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com> (v1)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
The next patch will change the map type, and this will make sure there are no
regressions as a result of the other stuff. Since the miptree is newly created,
I believe it is always safe to just map.
It is possible to CPU map this buffer on LLC platforms (it additionally requires
rounding up to tile size). I did experiment with that patch, and found no
performance gains to be had.
I've added in error handling while here. Generally GTT mapping is an operation
which is highly unlikely to fail, but we may as well handle it when it does.
v2: rebase on master (Lionel)
v3: print out error if gtt mapping fails (Topi)
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com> (v1)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v2)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
This will allow us to treat HiZ and MCS the same when using as an
auxiliary surface buffer.
v2: (Ben) Minor rebase conflict resolution.
Rename mcs_buf to aux_buf to address upcoming change for hiz specific buffers.
That second thing is essentially a squash of:
i965/gen8: Use intel_miptree_aux_buffer for auxiliary buffer - which didn't need
to be separate in my opinion.
v3: rebase on master (Lionel)
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com> (v1)
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>a (v2)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> (v3)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
This patch does two things:
1. It separates the host-CPU code generation from the generic code
generation. This guards against accidently breaking things for
radeonsi in the future.
2. It makes sure we actually use both arguments and don't just compute
a square :-p
Fixes a regression introduced by commit 29279f44b3
Cc: Roland Scheidegger <sroland@vmware.com>
Reviewed-by: Roland Scheidegger <sroland@vmware.com>
Instead of doing all the math with scalars, use vectors. This means the
overflow math needs to be done manually, albeit that's only really
problematic for the stride/index mul, the rest has been pretty much
moved outside the shader loop (albeit the mul could actually be optimized
away too), where things are still scalar. Because llvm is complete fail
with the zero-extend widening mul, roll our own even...
To eliminate control flow in the main shader loop fetch, provide fake
buffers (so index 0 is always valid to fetch).
Still uses aos fetch though in the end - mostly because some more code
would be needed to handle unaligned fetches in that path, and because for
most formats it won't make a difference anyway (we generate some truly
horrendous code for things like R16G16_something for instance).
Instanced fetch however stays roughly the same as before, except that
no longer the same element is fetched multiple times (I've seen a reduction
of ~3 times in main shader loop size due to apparently llvm not being able
to deduce it's really all the same with a couple instanced elements).
Also, for elts gathering, use vectorized code as well - provide a fake
elt buffer if there's no valid one bound.
The generated shaders are smaller and faster to compile (not entirely sure
about execution speed, but generally unless there's just single vertices
to handle I would expect it to be faster - there's more opportunities
for future improvements by using soa fetch).
No piglit change.
Reviewed-by: Jose Fonseca <jfonseca@vmware.com>
This is used by shader umul_hi/imul_hi functions (and soon by draw).
It's actually useful separating this out on its own, however the real
reason for doing it is because we're using an optimized sse2 version,
since the code llvm generates is atrocious (since there's no widening
mul in llvm, and it does not recognize the widening mul pattern, so
it generates code for real 64x64->64bit mul, which the cpu can't do
natively, in contrast to 32x32->64bit mul which it could do).
Reviewed-by: Jose Fonseca <jfonseca@vmware.com>
In the event that multiple threads attempt to install a graph
concurrently, protect the shared list.
Signed-off-by: Steven Toth <stoth@kernellabs.com>
Reviewed-by: Brian Paul <brianp@vmware.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
We're missing the closedir() to the matching opendir().
Signed-off-by: Steven Toth <stoth@kernellabs.com>
Reviewed-by: Brian Paul <brianp@vmware.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Instead of trying to maintain a reference counted list of valid HUD
objects, and freeing them accordingly, creating race conditions
between unanticipated multiple threads, simply accept they're
allocated once and never released until the process terminates.
They're a shared resource between multiple threads, so accept
they're always available for use.
Signed-off-by: Steven Toth <stoth@kernellabs.com>
Reviewed-by: Brian Paul <brianp@vmware.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
This fixes a problem seen with gallium drivers vs android wallpaper.
Basically, what happens is:
EGLSurface tmpSurface = mEgl.eglCreatePbufferSurface(mEglDisplay, mEglConfig, attribs);
mEgl.eglMakeCurrent(mEglDisplay, tmpSurface, tmpSurface, mEglContext);
int[] maxSize = new int[1];
Rect frame = surfaceHolder.getSurfaceFrame();
glGetIntegerv(GL_MAX_TEXTURE_SIZE, maxSize, 0);
mEgl.eglMakeCurrent(mEglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
mEgl.eglDestroySurface(mEglDisplay, tmpSurface);
... check maxSize vs frame size and bail if needed ...
mEglSurface = mEgl.eglCreateWindowSurface(mEglDisplay, mEglConfig, surfaceHolder, null);
... error checking ...
mEgl.eglMakeCurrent(mEglDisplay, mEglSurface, mEglSurface, mEglContext);
When the window-surface is created, it ends up with the same ptr address
as the recently freed tmpSurface pbuffer surface. Which after many
levels of indirection, results in st_framebuffer_validate() ending up with
the same/old framebuffer object, and in the end never calling the
DRIimageLoaderExtension::getBuffers(). Then in droid_swap_buffers(), the
dri2_surf is still the old pbuffer surface (with dri2_surf->buffer being
NULL, obviously, so when wallpaper app calls eglSwapBuffers() nothing
gets enqueued to the compositor). Resulting in a black/blank background
layer.
Note that at the EGL layer, when the context is unbound, EGL drops it's
references to the draw and read buffer as well.
Signed-off-by: Rob Clark <robdclark@gmail.com>
Tested-by: Robert Foss <robert.foss@collabora.com>
Acked-by: Tapani Pälli <tapani.palli@intel.com>
v3 [Francisco Jerez]: Loosely based on Serge's v1 of this patch in
order to avoid CL-specific enums in the clover module binary
format. In addition to other changes made in v2: Represent the CL
program binary type as the section type instead of adding a CL
API-specific enum, check that the binary types of the input objects
are valid during clLinkProgram(), pass section type as argument to
build_module_library() instead of using separate function.
Reviewed-by: Francisco Jerez <currojerez@riseup.net>
