panfrost: Fix a polygon list corruption in the multi-context case

The polygon list is written by tiler jobs and read by fragment ones,
and nothing should re-use the heap until the fragment job is done.
4fec6c9448 ("panfrost: Add the tiler heap to fragment jobs") fixed
this for the !multi-context case by adding the heap BO to fragment job.
But the tiler heap is shared accross contexts, and vertex/tiler+fragment
job submission is done through 2 separate ioctls, meaning that
vertex/tiler and fragment jobs from 2 different context might be
interleaved.

Add a lock at the device level to ensure tiler/vertex+fragment jobs are
submitted sequentially, with no other jobs using the same tiler heap
in-between.

Cc: mesa-stable
Fixes: d8deb1eb6a ("panfrost: Share tiler_heap across batches/contexts")
Reported-by: Icecream95 <ixn@disroot.org>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Tested-by: Icecream95 <ixn@disroot.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8822>
This commit is contained in:
Boris Brezillon 2021-02-02 09:32:41 +01:00
parent 0c3fe06421
commit 66125c429f
3 changed files with 25 additions and 3 deletions

View file

@ -1021,10 +1021,19 @@ panfrost_batch_submit_ioctl(struct panfrost_batch *batch,
static int
panfrost_batch_submit_jobs(struct panfrost_batch *batch, uint32_t in_sync, uint32_t out_sync)
{
struct panfrost_device *dev = pan_device(batch->ctx->base.screen);
bool has_draws = batch->scoreboard.first_job;
bool has_frag = batch->scoreboard.tiler_dep || batch->clear;
bool has_tiler = batch->scoreboard.first_tiler;
bool has_frag = has_tiler || batch->clear;
int ret = 0;
/* Take the submit lock to make sure no tiler jobs from other context
* are inserted between our tiler and fragment jobs, failing to do that
* might result in tiler heap corruption.
*/
if (has_tiler)
pthread_mutex_lock(&dev->submit_lock);
if (has_draws) {
ret = panfrost_batch_submit_ioctl(batch, batch->scoreboard.first_job,
0, in_sync, has_frag ? 0 : out_sync);
@ -1039,14 +1048,16 @@ panfrost_batch_submit_jobs(struct panfrost_batch *batch, uint32_t in_sync, uint3
* *only* clears, since otherwise the tiler structures will be
* uninitialized leading to faults (or state leaks) */
mali_ptr fragjob = panfrost_fragment_job(batch,
batch->scoreboard.tiler_dep != 0);
mali_ptr fragjob = panfrost_fragment_job(batch, has_tiler);
ret = panfrost_batch_submit_ioctl(batch, fragjob,
PANFROST_JD_REQ_FS, 0,
out_sync);
assert(!ret);
}
if (has_tiler)
pthread_mutex_unlock(&dev->submit_lock);
return ret;
}

View file

@ -151,6 +151,14 @@ struct panfrost_device {
* costly per-context allocation. */
struct panfrost_bo *tiler_heap;
/* The tiler heap is shared by all contexts, and is written by tiler
* jobs and read by fragment job. We need to ensure that a
* vertex/tiler job chain from one context is not inserted between
* the vertex/tiler and fragment job of another context, otherwise
* we end up with tiler heap corruption.
*/
pthread_mutex_t submit_lock;
};
void

View file

@ -253,11 +253,14 @@ panfrost_open_device(void *memctx, int fd, struct panfrost_device *dev)
dev->tiler_heap = panfrost_bo_create(dev, 4096 * 4096,
PAN_BO_INVISIBLE | PAN_BO_GROWABLE);
pthread_mutex_init(&dev->submit_lock, NULL);
}
void
panfrost_close_device(struct panfrost_device *dev)
{
pthread_mutex_destroy(&dev->submit_lock);
panfrost_bo_unreference(dev->blit_shaders.bo);
panfrost_bo_unreference(dev->tiler_heap);
panfrost_bo_cache_evict_all(dev);