mirror of
https://gitlab.freedesktop.org/mesa/mesa.git
synced 2026-05-25 16:58:10 +02:00
Huge thanks to Tapani Pälli for debugging this issue, figuring out
what was going wrong, proposing fixes, and walking me through where
things were going off the rails.
BLORP always disables tessellation and geometry shaders. Our handling
tried to look at ice->shaders.uncompiled[] to determine whether the next
draw needed those shaders. If not, we can leave BLORP's residual state
that disabled those stages in place, and skip looking at it.
Unfortunately, predicting the future is a bit fraught, in part due to
the uncompiled[] and prog[] arrays being slightly out of sync at times.
Consider the following case:
1. Draw with tessellation shaders in place
=> uncompiled[TES] and prog[TES] will both point at valid shaders.
2. Gallium calls pipe->bind_tes_state(NULL).
=> This makes uncompiled[TES] point at NULL, and flags
IRIS_STAGE_DIRTY_UNCOMPILED_TES.
Because iris_update_compiled_shaders() hasn't happened yet,
uncompiled[TES] is NULL but prog[TES] has the stale TES from
the previous draw still.
3. BLORP operations happen
=> BLORP sees uncompiled[TES] == NULL and decides that tessellation
is off for the upcoming draws. So it skips flagging tess state.
4. Gallium calls pipe->bind_tes_state(shader from step #1).
=> uncompiled[TES] points at the original shader.
IRIS_STAGE_DIRTY_UNCOMPILED_TES gets flagged again.
5. Draw again
=> This calls iris_update_compiled_shaders(), which sees that
a TES is bound, and calls iris_update_compiled_tes(). But
because the same shader was bound as before, the program it
comes up with is identical to the one already bound at
ice->shaders.prog[TES]. So, it thinks it doesn't have to
flag any tessellation state dirty because it was already
set up for the last draw.
This random unbind and rebind between draws leads to a situation
where, at step #3, BLORP thinks it can skip flagging tessellation
state (nothing is bound), and at step #5, normal state handling
thinks it can skip flagging tessellation state (nothing changed
since last time). So nobody does, and things break.
This unbind appears to be happening when st_release_variants()
decides it wants to free some shaders. Then a rebind happens to
put back the actual shader for the draw. So, it's not theoretical.
To fix this, we change BLORP to look at ice->shaders.prog[] rather
than uncompiled[]. This is equivalent to thinking about the previous
draw, rather than the next. If the last draw had tessellation off,
then BLORP's disabling was a no-op, and the GPU is still in the same
state as the previous draw. This is more reliable than predicting
the future.
Cc: mesa-stable
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/8308
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/9678
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24880>
|
||
|---|---|---|
| .. | ||
| auxiliary | ||
| drivers | ||
| frontends | ||
| include | ||
| targets | ||
| tests | ||
| tools | ||
| winsys | ||
| meson.build | ||
| README.portability | ||
CROSS-PLATFORM PORTABILITY GUIDELINES FOR GALLIUM3D
= General Considerations =
The frontend and winsys driver support a rather limited number of
platforms. However, the pipe drivers are meant to run in a wide number of
platforms. Hence the pipe drivers, the auxiliary modules, and all public
headers in general, should strictly follow these guidelines to ensure
= Compiler Support =
* Include the util/compiler.h.
* Cast explicitly when converting to integer types of smaller sizes.
* Cast explicitly when converting between float, double and integral types.
* Don't use named struct initializers.
* Don't use variable number of macro arguments. Use static inline functions
instead.
* Don't use C99 features.
= Standard Library =
* Avoid including standard library headers. Most standard library functions are
not available in Windows Kernel Mode. Use the appropriate p_*.h include.
== Memory Allocation ==
* Use MALLOC, CALLOC, FREE instead of the malloc, calloc, free functions.
* Use align_pointer() function defined in u_memory.h for aligning pointers
in a portable way.
== Debugging ==
* Use the functions/macros in p_debug.h.
* Don't include assert.h, call abort, printf, etc.
= Code Style =
== Inherantice in C ==
The main thing we do is mimic inheritance by structure containment.
Here's a silly made-up example:
/* base class */
struct buffer
{
int size;
void (*validate)(struct buffer *buf);
};
/* sub-class of bufffer */
struct texture_buffer
{
struct buffer base; /* the base class, MUST COME FIRST! */
int format;
int width, height;
};
Then, we'll typically have cast-wrapper functions to convert base-class
pointers to sub-class pointers where needed:
static inline struct vertex_buffer *vertex_buffer(struct buffer *buf)
{
return (struct vertex_buffer *) buf;
}
To create/init a sub-classed object:
struct buffer *create_texture_buffer(int w, int h, int format)
{
struct texture_buffer *t = malloc(sizeof(*t));
t->format = format;
t->width = w;
t->height = h;
t->base.size = w * h;
t->base.validate = tex_validate;
return &t->base;
}
Example sub-class method:
void tex_validate(struct buffer *buf)
{
struct texture_buffer *tb = texture_buffer(buf);
assert(tb->format);
assert(tb->width);
assert(tb->height);
}
Note that we typically do not use typedefs to make "class names"; we use
'struct whatever' everywhere.
Gallium's pipe_context and the subclassed psb_context, etc are prime examples
of this. There's also many examples in Mesa and the Mesa state tracker.