On 17.11.2014 23:30, Aaron Watry wrote:
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.
AFAICT it indeed needs to be handled in evergreen_set_compute_resources
and evergreen_set_global_binding. Does the attached patch work?
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
>From 2ee9c0c435405df7382363ae8f276cd3ce4f8c6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= <michel.daen...@amd.com>
Date: Tue, 18 Nov 2014 11:46:22 +0900
Subject: [PATCH] r600g/compute: Fix cbuf leaks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Michel Dänzer <michel.daen...@amd.com>
---
src/gallium/drivers/r600/evergreen_compute.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/gallium/drivers/r600/evergreen_compute.c b/src/gallium/drivers/r600/evergreen_compute.c
index 90fdd79..64c3cee 100644
--- a/src/gallium/drivers/r600/evergreen_compute.c
+++ b/src/gallium/drivers/r600/evergreen_compute.c
@@ -127,6 +127,7 @@ static void evergreen_set_rat(
rat_templ.u.tex.last_layer = 0;
/* Add the RAT the list of color buffers */
+ pipe_surface_reference(&pipe->ctx->framebuffer.state.cbufs[id], NULL);
pipe->ctx->framebuffer.state.cbufs[id] = pipe->ctx->b.b.create_surface(
(struct pipe_context *)pipe->ctx,
(struct pipe_resource *)bo, &rat_templ);
@@ -627,11 +628,16 @@ static void evergreen_set_compute_resources(struct pipe_context * ctx_,
{
struct r600_context *ctx = (struct r600_context *)ctx_;
struct r600_surface **resources = (struct r600_surface **)surfaces;
+ unsigned i;
COMPUTE_DBG(ctx->screen, "*** evergreen_set_compute_resources: start = %u count = %u\n",
start, count);
- for (unsigned i = 0; i < count; i++) {
+ /* Unreference unused surfaces */
+ for (i = count + 1; i < ctx->framebuffer.state.nr_cbufs; i++)
+ pipe_surface_reference(&ctx->framebuffer.state.cbufs[i], NULL);
+
+ for (i = 0; i < count; i++) {
/* The First two vertex buffers are reserved for parameters and
* global buffers. */
unsigned vtx_id = 2 + i;
@@ -651,6 +657,9 @@ static void evergreen_set_compute_resources(struct pipe_context * ctx_,
evergreen_cs_set_vertex_buffer(ctx, vtx_id,
buffer->chunk->start_in_dw * 4,
resources[i]->base.texture);
+ } else {
+ pipe_surface_reference(&ctx->framebuffer.state.cbufs[i+1],
+ NULL);
}
}
}
@@ -688,10 +697,11 @@ static void evergreen_set_global_binding(
COMPUTE_DBG(ctx->screen, "*** evergreen_set_global_binding first = %u n = %u\n",
first, n);
- if (!resources) {
- /* XXX: Unset */
+ /* Unreference previous resource */
+ pipe_surface_reference(&ctx->framebuffer.state.cbufs[0], NULL);
+
+ if (!resources)
return;
- }
/* We mark these items for promotion to the pool if they
* aren't already there */
@@ -702,10 +712,8 @@ static void evergreen_set_global_binding(
buffers[i]->chunk->status |= ITEM_FOR_PROMOTING;
}
- if (compute_memory_finalize_pending(pool, ctx_) == -1) {
- /* XXX: Unset */
+ if (compute_memory_finalize_pending(pool, ctx_) == -1)
return;
- }
for (i = first; i < first + n; i++)
{
--
2.1.3
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
