Iago Toral itoral@igalia.com writes:
On Thu, 2019-09-12 at 10:25 -0700, Eric Anholt wrote:
Iago Toral Quiroga itoral@igalia.com writes:
Extends the user space ioctl for CL submissions so it can include a request to flush the cache once the CL execution has completed. Fixes memory write violation messages reported by the kernel in workloads involving shader memory writes (SSBOs, shader images, scratch, etc) which sometimes also lead to GPU resets during Piglit and CTS workloads.
Some context for any other reviewers: This patch is the interface change necessary to expose GLES 3.1 on V3D. It turns out the HW packets for flushing the caches were broken in multiple ways.
Signed-off-by: Iago Toral Quiroga itoral@igalia.com
drivers/gpu/drm/v3d/v3d_gem.c | 51 +++++++++++++++++++++++++++++
include/uapi/drm/v3d_drm.h | 7 ++--- 2 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 5d80507b539b..530fe9d9d5bd 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -530,13 +530,16 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, struct drm_v3d_submit_cl *args = data; struct v3d_bin_job *bin = NULL; struct v3d_render_job *render;
struct v3d_job *clean_job = NULL;
struct v3d_job *last_job; struct ww_acquire_ctx acquire_ctx; int ret = 0;
trace_v3d_submit_cl_ioctl(&v3d->drm, args->rcl_start, args-
rcl_end);
- if (args->pad != 0) {
DRM_INFO("pad must be zero: %d\n", args->pad);
- if (args->flags != 0 &&
args->flags != DRM_V3D_SUBMIT_CL_FLUSH_CACHE_FLAG) {DRM_INFO("invalid flags: %d\n", args->flags);@@ -575,12 +578,28 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, bin->render = render; }
- ret = v3d_lookup_bos(dev, file_priv, &render->base,
- if (args->flags & DRM_V3D_SUBMIT_CL_FLUSH_CACHE_FLAG) {
clean_job = kcalloc(1, sizeof(*clean_job), GFP_KERNEL);if (!clean_job) {ret = -ENOMEM;goto fail;}ret = v3d_job_init(v3d, file_priv, clean_job,v3d_job_free, 0);
if (ret)goto fail;Only issue I see: If v3d_job_init() fails, we need to not v3d_job_put() it. I'm fine with either kfree() it and NULL the ptr before jumping to fail, or open code the bin/render puts.
It seems we also call v3d_job_put() for the bin job when v3d_job_init() fails, which also returns immediately in that case instead of jumping to fail to v3d_job_put the render job, so I guess we need the same treatment there. Shall I fix that in this patch too or would you rather see a different patch sent separately for that?
I think you might be looking at the put of the (already-inited) render job when initing bin job fails?
Looks like we do leak bin in that case, though. Happy to see that as a fixup patch.