Re: [PATCH v3] drm/v3d: clean caches at the end of render jobs on request from user space

2019-09-19 Thread Eric Anholt
Iago Toral Quiroga  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.
>
> v2: if v3d_job_init() fails we need to kfree() the job instead of
> v3d_job_put() it (Eric Anholt).
>
> v3 (Eric Anholt):
>   - Drop _FLAG suffix from the new flag name.
>   - Add a new param so userspace can tell whether cache flushing is
> implemented in the kernel.

Appled to drm-misc-next.  Thanks!


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3] drm/v3d: clean caches at the end of render jobs on request from user space

2019-09-19 Thread Iago Toral Quiroga
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.

v2: if v3d_job_init() fails we need to kfree() the job instead of
v3d_job_put() it (Eric Anholt).

v3 (Eric Anholt):
  - Drop _FLAG suffix from the new flag name.
  - Add a new param so userspace can tell whether cache flushing is
implemented in the kernel.

Signed-off-by: Iago Toral Quiroga 
Reviewed-by: Eric Anholt 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20190912083516.13797-1-ito...@igalia.com
---
 drivers/gpu/drm/v3d/v3d_drv.c |  3 ++
 drivers/gpu/drm/v3d/v3d_gem.c | 54 +--
 include/uapi/drm/v3d_drm.h|  8 --
 3 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 3506ae2723ae..e94bf75368be 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -126,6 +126,9 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void 
*data,
case DRM_V3D_PARAM_SUPPORTS_CSD:
args->value = v3d_has_csd(v3d);
return 0;
+   case DRM_V3D_PARAM_SUPPORTS_CACHE_FLUSH:
+   args->value = 1;
+   return 0;
default:
DRM_DEBUG("Unknown parameter %d\n", args->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index fb32cda18ffe..4c4b59ae2c81 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) {
+   DRM_INFO("invalid flags: %d\n", args->flags);
return -EINVAL;
}
 
@@ -576,12 +579,31 @@ 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) {
+   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) {
+   kfree(clean_job);
+   clean_job = NULL;
+   goto fail;
+   }
+
+   last_job = clean_job;
+   } else {
+   last_job = &render->base;
+   }
+
+   ret = v3d_lookup_bos(dev, file_priv, last_job,
 args->bo_handles, args->bo_handle_count);
if (ret)
goto fail;
 
-   ret = v3d_lock_bo_reservations(&render->base, &acquire_ctx);
+   ret = v3d_lock_bo_reservations(last_job, &acquire_ctx);
if (ret)
goto fail;
 
@@ -600,28 +622,44 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
ret = v3d_push_job(v3d_priv, &render->base, V3D_RENDER);
if (ret)
goto fail_unreserve;
+
+   if (clean_job) {
+   struct dma_fence *render_fence =
+   dma_fence_get(render->base.done_fence);
+   ret = drm_gem_fence_array_add(&clean_job->deps, render_fence);
+   if (ret)
+   goto fail_unreserve;
+   ret = v3d_push_job(v3d_priv, clean_job, V3D_CACHE_CLEAN);
+   if (ret)
+   goto fail_unreserve;
+   }
+
mutex_unlock(&v3d->sched_lock);
 
v3d_attach_fences_and_unlock_reservation(file_priv,
-&render->base,
+last_job,
 &acquire_ctx,
 args->out_sync,
-render->base.done_fence);
+last_job->done_fence);
 
if (bin)
v3d_job_put(&bin->base);
v3d_job_put(&render->base);
+   if (clean_job)
+   v3d_job_put(clean_job);
 
return 0;
 
 fail_unreserve: