On Mon, Nov 17, 2014 at 1:45 AM, Michel Dänzer <mic...@daenzer.net> wrote: > On 14.11.2014 19:37, Marek Olšák wrote: >> surface_destroy should never be called directly, because surfaces have >> a reference counter. For unreferencing resources, use >> pipe_surface_reference(&pointer, NULL). It will call surface_destroy >> if needed. > > Indeed, if this was the right place for this, it could be done both > easier and more robustly: > > for (int i = 0; i < fb_state->nr_cbufs; i++) > pipe_surface_reference(&fb_state->cbufs[i], NULL); >
Yeah, you're right about that. After your (Michel/Marek) replies, I had come to the same conclusion about simplifying things. I'm having *fun* deciding where the proper place to put this in the evergreen code is. We end up calling evergreen_set_rat from multiple places, which is where the surfaces are created, and then we keep a list with a set count... and the individual surfaces themselves get freed as their reference counts change. In theory, they all get referenced/freed at the same point, but there's no guarantees that I can see. The first logical place that I saw to put the de-reference is in evergreen_set_global_binding and the matching create/delete_compute_state functions. This seems to only affect evergreen (See the XXX early return comment in the evergreen set global binding function, which implies someone knew this was an issue). SI seems to have all the right de-references in place, but I haven't verified that yet through actual run, just inspection. --Aaron > >> On Fri, Nov 14, 2014 at 12:43 AM, Aaron Watry <awa...@gmail.com> wrote: >>> Walk the array of cbufs backwards and free all of them. >>> >>> v3: Rebase on top of changes since Aug 2014 >>> >>> Signed-off-by: Aaron Watry <awa...@gmail.com> >>> --- >>> src/gallium/drivers/r600/evergreen_compute.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/src/gallium/drivers/r600/evergreen_compute.c >>> b/src/gallium/drivers/r600/evergreen_compute.c >>> index 90fdd79..4334743 100644 >>> --- a/src/gallium/drivers/r600/evergreen_compute.c >>> +++ b/src/gallium/drivers/r600/evergreen_compute.c >>> @@ -252,6 +252,15 @@ void evergreen_delete_compute_state(struct >>> pipe_context *ctx, void* state) >>> if (!shader) >>> return; >>> >>> + if (shader->ctx){ >>> + struct pipe_framebuffer_state *fb_state = >>> &shader->ctx->framebuffer.state; >>> + for (int i = fb_state->nr_cbufs - 1; fb_state->nr_cbufs > 0 >>> ; i--){ >>> + shader->ctx->b.b.surface_destroy(ctx, >>> fb_state->cbufs[i]); >>> + fb_state->cbufs[i] = NULL; >>> + fb_state->nr_cbufs--; >>> + } >>> + } >>> + >>> FREE(shader); >>> } >>> >>> -- >>> 2.1.0 >>> >>> _______________________________________________ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev