Here's a series of fixes for vc4 from writing igt testcases while debugging X window dragging.
Eric Anholt (5): drm/vc4: Validate that WAIT_BO padding is cleared. drm/vc4: Fix the clear color for the first tile rendered. drm/vc4: Return an ERR_PTR from BO creation instead of NULL. drm/vc4: Fix -ERESTARTSYS error return from BO waits. drm/vc4: Drop error message on seqno wait timeouts.
drivers/gpu/drm/vc4/vc4_bo.c | 16 ++++++++-------- drivers/gpu/drm/vc4/vc4_gem.c | 14 ++++++-------- drivers/gpu/drm/vc4/vc4_irq.c | 2 +- drivers/gpu/drm/vc4/vc4_render_cl.c | 22 +++++++++++----------- drivers/gpu/drm/vc4/vc4_validate.c | 4 ++-- 5 files changed, 28 insertions(+), 30 deletions(-)
This is ABI future-proofing if we ever want to extend the pad to mean something.
Signed-off-by: Eric Anholt eric@anholt.net --- drivers/gpu/drm/vc4/vc4_gem.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 48ce30a..cc89ffc 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -746,6 +746,9 @@ vc4_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_gem_object *gem_obj; struct vc4_bo *bo;
+ if (args->pad != 0) + return -EINVAL; + gem_obj = drm_gem_object_lookup(dev, file_priv, args->handle); if (!gem_obj) { DRM_ERROR("Failed to look up GEM BO %d\n", args->handle);
Apparently in hardware (as opposed to simulation), the clear colors need to be uploaded before the render config, otherwise they won't take effect. Fixes igt's vc4_wait_bo/used-bo-* subtests.
Signed-off-by: Eric Anholt eric@anholt.net --- drivers/gpu/drm/vc4/vc4_render_cl.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c index 8a2a312..dea97f4 100644 --- a/drivers/gpu/drm/vc4/vc4_render_cl.c +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c @@ -321,15 +321,6 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec, list_add_tail(&to_vc4_bo(&setup->rcl->base)->unref_head, &exec->unref_list);
- rcl_u8(setup, VC4_PACKET_TILE_RENDERING_MODE_CONFIG); - rcl_u32(setup, - (setup->color_write ? (setup->color_write->paddr + - args->color_write.offset) : - 0)); - rcl_u16(setup, args->width); - rcl_u16(setup, args->height); - rcl_u16(setup, args->color_write.bits); - /* The tile buffer gets cleared when the previous tile is stored. If * the clear values changed between frames, then the tile buffer has * stale clear values in it, so we have to do a store in None mode (no @@ -349,6 +340,15 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec, rcl_u32(setup, 0); /* no address, since we're in None mode */ }
+ rcl_u8(setup, VC4_PACKET_TILE_RENDERING_MODE_CONFIG); + rcl_u32(setup, + (setup->color_write ? (setup->color_write->paddr + + args->color_write.offset) : + 0)); + rcl_u16(setup, args->width); + rcl_u16(setup, args->height); + rcl_u16(setup, args->color_write.bits); + for (y = min_y_tile; y <= max_y_tile; y++) { for (x = min_x_tile; x <= max_x_tile; x++) { bool first = (x == min_x_tile && y == min_y_tile);
Fixes igt vc4_create_bo/create-bo-0 by returning -EINVAL from the ioctl instead of -ENOMEM.
Signed-off-by: Eric Anholt eric@anholt.net --- drivers/gpu/drm/vc4/vc4_bo.c | 16 ++++++++-------- drivers/gpu/drm/vc4/vc4_gem.c | 4 ++-- drivers/gpu/drm/vc4/vc4_irq.c | 2 +- drivers/gpu/drm/vc4/vc4_render_cl.c | 4 ++-- drivers/gpu/drm/vc4/vc4_validate.c | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c index 18dfe3e..22278bc 100644 --- a/drivers/gpu/drm/vc4/vc4_bo.c +++ b/drivers/gpu/drm/vc4/vc4_bo.c @@ -215,7 +215,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size, struct drm_gem_cma_object *cma_obj;
if (size == 0) - return NULL; + return ERR_PTR(-EINVAL);
/* First, try to get a vc4_bo from the kernel BO cache. */ if (from_cache) { @@ -237,7 +237,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, size_t unaligned_size, if (IS_ERR(cma_obj)) { DRM_ERROR("Failed to allocate from CMA:\n"); vc4_bo_stats_dump(vc4); - return NULL; + return ERR_PTR(-ENOMEM); } }
@@ -259,8 +259,8 @@ int vc4_dumb_create(struct drm_file *file_priv, args->size = args->pitch * args->height;
bo = vc4_bo_create(dev, args->size, false); - if (!bo) - return -ENOMEM; + if (IS_ERR(bo)) + return PTR_ERR(bo);
ret = drm_gem_handle_create(file_priv, &bo->base.base, &args->handle); drm_gem_object_unreference_unlocked(&bo->base.base); @@ -443,8 +443,8 @@ int vc4_create_bo_ioctl(struct drm_device *dev, void *data, * get zeroed, and that might leak data between users. */ bo = vc4_bo_create(dev, args->size, false); - if (!bo) - return -ENOMEM; + if (IS_ERR(bo)) + return PTR_ERR(bo);
ret = drm_gem_handle_create(file_priv, &bo->base.base, &args->handle); drm_gem_object_unreference_unlocked(&bo->base.base); @@ -496,8 +496,8 @@ vc4_create_shader_bo_ioctl(struct drm_device *dev, void *data, }
bo = vc4_bo_create(dev, args->size, true); - if (!bo) - return -ENOMEM; + if (IS_ERR(bo)) + return PTR_ERR(bo);
ret = copy_from_user(bo->base.vaddr, (void __user *)(uintptr_t)args->data, diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index cc89ffc..3bf679d 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -578,9 +578,9 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec) }
bo = vc4_bo_create(dev, exec_size, true); - if (!bo) { + if (IS_ERR(bo)) { DRM_ERROR("Couldn't allocate BO for binning\n"); - ret = -ENOMEM; + ret = PTR_ERR(bo); goto fail; } exec->exec_bo = &bo->base; diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c index b68060e..78a2135 100644 --- a/drivers/gpu/drm/vc4/vc4_irq.c +++ b/drivers/gpu/drm/vc4/vc4_irq.c @@ -57,7 +57,7 @@ vc4_overflow_mem_work(struct work_struct *work) struct vc4_bo *bo;
bo = vc4_bo_create(dev, 256 * 1024, true); - if (!bo) { + if (IS_ERR(bo)) { DRM_ERROR("Couldn't allocate binner overflow mem\n"); return; } diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c index dea97f4..0f12418 100644 --- a/drivers/gpu/drm/vc4/vc4_render_cl.c +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c @@ -316,8 +316,8 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec, size += xtiles * ytiles * loop_body_size;
setup->rcl = &vc4_bo_create(dev, size, true)->base; - if (!setup->rcl) - return -ENOMEM; + if (IS_ERR(setup->rcl)) + return PTR_ERR(setup->rcl); list_add_tail(&to_vc4_bo(&setup->rcl->base)->unref_head, &exec->unref_list);
diff --git a/drivers/gpu/drm/vc4/vc4_validate.c b/drivers/gpu/drm/vc4/vc4_validate.c index e26d9f6..24c2c74 100644 --- a/drivers/gpu/drm/vc4/vc4_validate.c +++ b/drivers/gpu/drm/vc4/vc4_validate.c @@ -401,8 +401,8 @@ validate_tile_binning_config(VALIDATE_ARGS) tile_bo = vc4_bo_create(dev, exec->tile_alloc_offset + tile_alloc_size, true); exec->tile_bo = &tile_bo->base; - if (!exec->tile_bo) - return -ENOMEM; + if (IS_ERR(exec->tile_bo)) + return PTR_ERR(exec->tile_bo); list_add_tail(&tile_bo->unref_head, &exec->unref_list);
/* tile alloc address. */
This caused the wait ioctls to claim that waiting had completed when we actually got interrupted by a signal before it was done. Fixes broken rendering throttling that produced serious lag in X window dragging.
Signed-off-by: Eric Anholt eric@anholt.net --- drivers/gpu/drm/vc4/vc4_gem.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 3bf679d..15619db 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -340,12 +340,10 @@ vc4_wait_for_seqno(struct drm_device *dev, uint64_t seqno, uint64_t timeout_ns, finish_wait(&vc4->job_wait_queue, &wait); trace_vc4_wait_for_seqno_end(dev, seqno);
- if (ret && ret != -ERESTARTSYS) { + if (ret && ret != -ERESTARTSYS) DRM_ERROR("timeout waiting for render thread idle\n"); - return ret; - }
- return 0; + return ret; }
static void
These ioctls end up getting exposed to fairly directly to GL users, and having normal user operations print DRM errors is obviously wrong. The message was originally to give us some idea of what happened when a hang occurred, but we have a DRM_INFO from reset for that.
Signed-off-by: Eric Anholt eric@anholt.net --- drivers/gpu/drm/vc4/vc4_gem.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 15619db..a9d020e 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -340,9 +340,6 @@ vc4_wait_for_seqno(struct drm_device *dev, uint64_t seqno, uint64_t timeout_ns, finish_wait(&vc4->job_wait_queue, &wait); trace_vc4_wait_for_seqno_end(dev, seqno);
- if (ret && ret != -ERESTARTSYS) - DRM_ERROR("timeout waiting for render thread idle\n"); - return ret; }
dri-devel@lists.freedesktop.org
-
Eric Anholt