fdo-mirrors/mesa
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/mesa.git synced 2026-01-31 17:50:35 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

panfrost: Fix fencing

Browse source 
Commit 64d6f56ad2 ("panfrost: Allocate syncobjs in panfrost_flush")
aimed at optimizing the fencing logic but it looks it also broke the
fence-based synchronization in subtle ways.

Indeed, now that the fence only waits on a single syncobj, we're not
guaranteed that all jobs queued in panfrost_flush_all_batches() will
be done when the fence is signaled, because jobs at the top level
(those stored in the batches hashmap) have not inter-dependencies.

Commit 9e397956b0 ("panfrost: signal syncobj if nothing is going to
be flushed") made this even more apparent by signaling the fence right
away if nothing was left to be drawn in the current context, thus
ignoring any of the batches left to flushed in the ->batches map.

If we want to keep relying the existing kernel APIs there's clearly no
ideal solution here. We can either go back to the original fencing
mechanism where each fence contained an array of syncobjs to be tested
or serialize jobs that have no explicit dependencies so we know the last
submitted job will also be the last one to return. The orginal approach
has proven to add quite a significant overhead (caused by the amount of
ioctls and the time spent in kernel space to gather dma fences attached
to those syncobjs and test them). So let's go for the simple solution
where we have a single syncobj bound to the context which we update to
point to the last job out_sync every time we submit a top-level job.

This approach implies reworking the way we create fences since we
need to capture the syncobj state at the time the fence is created.
Unfortunately, there's not SYNCOBJ_CLONE ioctl, which forces us to
export/create/import a fence so we have a new object that's not
subject to changes done to the context syncobj.

If we want to further optimize the logic, we should probably explore
some of those options:

1/ Adding array based SYNCOBJ ioctls (SYNCOBJ_{CREATE,DESTROY,CLONE}_ARRAY)
   so we can mitigate the cost of ioctls when we need to manipulate
   arrays of syncobjs
2/ Support synchronization jobs. That is, jobs that have a NULL job chain
   but an array of sync_in and a sync_out to allow creating
   synchronization points
3/ Add syncobj aggregators so we only have to wait on one syncobj from
   userspace. The syncobj aggregator would wait for all sub syncobjs to
   be signaled before signaling the top-level one.

Fixes: 64d6f56ad2 ("panfrost: Allocate syncobjs in panfrost_flush")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7831>
This commit is contained in:
Boris Brezillon 2020-11-30 11:07:33 +01:00 • committed by Marge Bot
parent 387221e4f2
commit 29f938a0ec
8 changed files with 76 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
.gitlab-ci/deqp-panfrost-t860-fails.txt
View file

@ -33,7 +33,6 @@ dEQP-GLES3.functional.fbo.msaa.4_samples.rg32f,Fail
dEQP-GLES3.functional.fbo.msaa.4_samples.rgba16f,Fail
dEQP-GLES3.functional.fbo.msaa.4_samples.rgba32f,Fail
dEQP-GLES3.functional.fbo.msaa.4_samples.stencil_index8,Fail
dEQP-GLES3.functional.fence_sync.client_wait_sync_finish,Fail
dEQP-GLES2.functional.shaders.indexing.tmp_array.float_const_write_dynamic_loop_read_fragment,Crash
dEQP-GLES2.functional.shaders.indexing.tmp_array.float_const_write_dynamic_loop_read_vertex,Crash
dEQP-GLES2.functional.shaders.indexing.tmp_array.float_const_write_dynamic_read_fragment,Crash

2
src/gallium/drivers/panfrost/pan_compute.c
View file

@ -158,7 +158,7 @@ panfrost_launch_grid(struct pipe_context *pipe,
panfrost_add_job(&batch->pool, &batch->scoreboard,
MALI_JOB_TYPE_COMPUTE, true, 0, &t, true);
panfrost_flush_all_batches(ctx, 0);
panfrost_flush_all_batches(ctx);
}
static void

19
src/gallium/drivers/panfrost/pan_context.c
View file

@ -160,16 +160,13 @@ panfrost_flush(
{
struct panfrost_context *ctx = pan_context(pipe);
struct panfrost_device *dev = pan_device(pipe->screen);
uint32_t syncobj = 0;
if (fence)
drmSyncobjCreate(dev->fd, 0, &syncobj);
/* Submit all pending jobs */
panfrost_flush_all_batches(ctx, syncobj);
panfrost_flush_all_batches(ctx);
if (fence) {
struct panfrost_fence *f = panfrost_fence_create(ctx, syncobj);
struct panfrost_fence *f = panfrost_fence_create(ctx);
pipe->screen->fence_reference(pipe->screen, fence, NULL);
*fence = (struct pipe_fence_handle *)f;
}
@ -182,7 +179,7 @@ static void
panfrost_texture_barrier(struct pipe_context *pipe, unsigned flags)
{
struct panfrost_context *ctx = pan_context(pipe);
panfrost_flush_all_batches(ctx, 0);
panfrost_flush_all_batches(ctx);
}
#define DEFINE_CASE(c) case PIPE_PRIM_##c: return MALI_DRAW_MODE_##c;
@ -1432,7 +1429,7 @@ panfrost_get_query_result(struct pipe_context *pipe,
case PIPE_QUERY_PRIMITIVES_GENERATED:
case PIPE_QUERY_PRIMITIVES_EMITTED:
panfrost_flush_all_batches(ctx, 0);
panfrost_flush_all_batches(ctx);
vresult->u64 = query->end - query->start;
break;
@ -1620,5 +1617,13 @@ panfrost_create_context(struct pipe_screen *screen, void *priv, unsigned flags)
ctx->sample_mask = ~0;
ctx->active_queries = true;
int ASSERTED ret;
/* Create a syncobj in a signaled state. Will be updated to point to the
* last queued job out_sync every time we submit a new job.
*/
ret = drmSyncobjCreate(dev->fd, DRM_SYNCOBJ_CREATE_SIGNALED, &ctx->syncobj);
assert(!ret && ctx->syncobj);
return gallium;
}

3
src/gallium/drivers/panfrost/pan_context.h
View file

@ -109,6 +109,9 @@ struct panfrost_context {
* size from the kernel is 4kb */
struct u_upload_mgr *state_uploader;
/* Sync obj used to keep track of in-flight jobs. */
uint32_t syncobj;
/* Bound job batch and map of panfrost_batch_key to job batches */
struct panfrost_batch *batch;
struct hash_table *batches;

49
src/gallium/drivers/panfrost/pan_job.c
View file

@ -938,6 +938,7 @@ static int
panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
mali_ptr first_job_desc,
uint32_t reqs,
uint32_t in_sync,
uint32_t out_sync)
{
struct panfrost_context *ctx = batch->ctx;
@ -952,16 +953,16 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
* after we're done but preventing double-frees if we were given a
* syncobj */
bool our_sync = false;
if (!out_sync && dev->debug & (PAN_DBG_TRACE | PAN_DBG_SYNC)) {
drmSyncobjCreate(dev->fd, 0, &out_sync);
our_sync = true;
}
if (!out_sync && dev->debug & (PAN_DBG_TRACE | PAN_DBG_SYNC))
out_sync = ctx->syncobj;
submit.out_sync = out_sync;
submit.jc = first_job_desc;
submit.requirements = reqs;
if (in_sync) {
submit.in_syncs = (u64)(uintptr_t)(&in_sync);
submit.in_sync_count = 1;
}
bo_handles = calloc(panfrost_pool_num_bos(&batch->pool) +
panfrost_pool_num_bos(&batch->invisible_pool) +
@ -989,9 +990,6 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
if (dev->debug & PAN_DBG_MSGS)
fprintf(stderr, "Error submitting: %m\n");
if (our_sync)
drmSyncobjDestroy(dev->fd, out_sync);
return errno;
}
@ -1006,10 +1004,6 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
pandecode_jc(submit.jc, dev->quirks & IS_BIFROST, dev->gpu_id, minimal);
}
/* Cleanup if we created the syncobj */
if (our_sync)
drmSyncobjDestroy(dev->fd, out_sync);
return 0;
}
@ -1018,7 +1012,7 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
* implicit dep between them) */
static int
panfrost_batch_submit_jobs(struct panfrost_batch *batch, uint32_t out_sync)
panfrost_batch_submit_jobs(struct panfrost_batch *batch, uint32_t in_sync, uint32_t out_sync)
{
bool has_draws = batch->scoreboard.first_job;
bool has_frag = batch->scoreboard.tiler_dep || batch->clear;
@ -1026,7 +1020,7 @@ panfrost_batch_submit_jobs(struct panfrost_batch *batch, uint32_t out_sync)
if (has_draws) {
ret = panfrost_batch_submit_ioctl(batch, batch->scoreboard.first_job,
0, has_frag ? 0 : out_sync);
0, in_sync, has_frag ? 0 : out_sync);
assert(!ret);
}
@ -1041,7 +1035,8 @@ panfrost_batch_submit_jobs(struct panfrost_batch *batch, uint32_t out_sync)
mali_ptr fragjob = panfrost_fragment_job(batch,
batch->scoreboard.tiler_dep != 0);
ret = panfrost_batch_submit_ioctl(batch, fragjob,
PANFROST_JD_REQ_FS, out_sync);
PANFROST_JD_REQ_FS, 0,
out_sync);
assert(!ret);
}
@ -1049,7 +1044,8 @@ panfrost_batch_submit_jobs(struct panfrost_batch *batch, uint32_t out_sync)
}
static void
panfrost_batch_submit(struct panfrost_batch *batch, uint32_t out_sync)
panfrost_batch_submit(struct panfrost_batch *batch,
uint32_t in_sync, uint32_t out_sync)
{
assert(batch);
struct panfrost_device *dev = pan_device(batch->ctx->base.screen);
@ -1059,17 +1055,14 @@ panfrost_batch_submit(struct panfrost_batch *batch, uint32_t out_sync)
util_dynarray_foreach(&batch->dependencies,
struct panfrost_batch_fence *, dep) {
if ((*dep)->batch)
panfrost_batch_submit((*dep)->batch, 0);
panfrost_batch_submit((*dep)->batch, 0, 0);
}
int ret;
/* Nothing to do! */
if (!batch->scoreboard.first_job && !batch->clear) {
if (out_sync)
drmSyncobjSignal(dev->fd, &out_sync, 1);
if (!batch->scoreboard.first_job && !batch->clear)
goto out;
}
panfrost_batch_draw_wallpaper(batch);
@ -1092,7 +1085,7 @@ panfrost_batch_submit(struct panfrost_batch *batch, uint32_t out_sync)
panfrost_scoreboard_initialize_tiler(&batch->pool, &batch->scoreboard, polygon_list);
ret = panfrost_batch_submit_jobs(batch, out_sync);
ret = panfrost_batch_submit_jobs(batch, in_sync, out_sync);
if (ret && dev->debug & PAN_DBG_MSGS)
fprintf(stderr, "panfrost_batch_submit failed: %d\n", ret);
@ -1122,16 +1115,16 @@ out:
/* Submit all batches, applying the out_sync to the currently bound batch */
void
panfrost_flush_all_batches(struct panfrost_context *ctx, uint32_t out_sync)
panfrost_flush_all_batches(struct panfrost_context *ctx)
{
struct panfrost_batch *batch = panfrost_get_batch_for_fbo(ctx);
panfrost_batch_submit(batch, out_sync);
panfrost_batch_submit(batch, ctx->syncobj, ctx->syncobj);
hash_table_foreach(ctx->batches, hentry) {
struct panfrost_batch *batch = hentry->data;
assert(batch);
panfrost_batch_submit(batch, 0);
panfrost_batch_submit(batch, ctx->syncobj, ctx->syncobj);
}
assert(!ctx->batches->entries);
@ -1180,7 +1173,7 @@ panfrost_flush_batches_accessing_bo(struct panfrost_context *ctx,
return;
if (access->writer && access->writer->batch)
panfrost_batch_submit(access->writer->batch, 0);
panfrost_batch_submit(access->writer->batch, ctx->syncobj, ctx->syncobj);
if (!flush_readers)
return;
@ -1188,7 +1181,7 @@ panfrost_flush_batches_accessing_bo(struct panfrost_context *ctx,
util_dynarray_foreach(&access->readers, struct panfrost_batch_fence *,
reader) {
if (*reader && (*reader)->batch)
panfrost_batch_submit((*reader)->batch, 0);
panfrost_batch_submit((*reader)->batch, ctx->syncobj, ctx->syncobj);
}
}

2
src/gallium/drivers/panfrost/pan_job.h
View file

@ -157,7 +157,7 @@ panfrost_batch_create_bo(struct panfrost_batch *batch, size_t size,
uint32_t create_flags, uint32_t access_flags);
void
panfrost_flush_all_batches(struct panfrost_context *ctx, uint32_t out_sync);
panfrost_flush_all_batches(struct panfrost_context *ctx);
bool
panfrost_pending_batches_access_bo(struct panfrost_context *ctx,

40
src/gallium/drivers/panfrost/pan_screen.c
View file

@ -702,17 +702,51 @@ panfrost_fence_finish(struct pipe_screen *pscreen,
}
struct panfrost_fence *
panfrost_fence_create(struct panfrost_context *ctx,
uint32_t syncobj)
panfrost_fence_create(struct panfrost_context *ctx)
{
struct panfrost_fence *f = calloc(1, sizeof(*f));
if (!f)
return NULL;
struct panfrost_device *dev = pan_device(ctx->base.screen);
int fd = -1, ret;
/* Snapshot the last rendering out fence. We'd rather have another
* syncobj instead of a sync file, but this is all we get.
* (HandleToFD/FDToHandle just gives you another syncobj ID for the
* same syncobj).
*/
ret = drmSyncobjExportSyncFile(dev->fd, ctx->syncobj, &fd);
if (ret || fd == -1) {
fprintf(stderr, "export failed\n");
goto err_free_fence;
}
ret = drmSyncobjCreate(dev->fd, 0, &f->syncobj);
if (ret) {
fprintf(stderr, "create syncobj failed\n");
goto err_close_fd;
}
ret = drmSyncobjImportSyncFile(dev->fd, f->syncobj, fd);
if (ret) {
fprintf(stderr, "create syncobj failed\n");
goto err_destroy_syncobj;
}
assert(f->syncobj != ctx->syncobj);
close(fd);
pipe_reference_init(&f->reference, 1);
f->syncobj = syncobj;
return f;
err_destroy_syncobj:
drmSyncobjDestroy(dev->fd, f->syncobj);
err_close_fd:
close(fd);
err_free_fence:
free(f);
return NULL;
}
static const void *

2
src/gallium/drivers/panfrost/pan_screen.h
View file

@ -62,6 +62,6 @@ pan_device(struct pipe_screen *p)
}
struct panfrost_fence *
panfrost_fence_create(struct panfrost_context *ctx, uint32_t syncobj);
panfrost_fence_create(struct panfrost_context *ctx);
#endif /* PAN_SCREEN_H */