From c9811d0fa55929b182f62e0ee49b71b0bea6a936 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 11 Oct 2017 11:36:56 +0000 Subject: drm/msm: fix return value check in _msm_gem_kernel_new() In case of error, the function msm_gem_get_vaddr() returns ERR_PTR() and never returns NULL. The NULL test in the return value check should be replaced with IS_ERR(). Fixes: 8223286d62e2 ("drm/msm: Add a helper function for in-kernel buffer allocations") Signed-off-by: Wei Yongjun Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gem.c') diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index f15821a0d900..0b338fbf97ce 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -1045,10 +1045,10 @@ static void *_msm_gem_kernel_new(struct drm_device *dev, uint32_t size, } vaddr = msm_gem_get_vaddr(obj); - if (!vaddr) { + if (IS_ERR(vaddr)) { msm_gem_put_iova(obj, aspace); drm_gem_object_unreference(obj); - return ERR_PTR(-ENOMEM); + return ERR_CAST(vaddr); } if (bo) -- cgit v1.2.3 From 06451a3d1d777141dedfa947649cbb0c594ac3af Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Tue, 12 Sep 2017 14:23:05 -0400 Subject: drm/msm: fix _NO_IMPLICIT fencing case We need to call reservation_object_reserve_shared() in both cases, but this wasn't happening in the _NO_IMPLICIT submit case. Fixes: f0a42bb ("drm/msm: submit support for in-fences") Reported-by: Jordan Crouse Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gem.c') diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 0b338fbf97ce..ea5bb0e1632c 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -610,17 +610,6 @@ int msm_gem_sync_object(struct drm_gem_object *obj, struct dma_fence *fence; int i, ret; - if (!exclusive) { - /* NOTE: _reserve_shared() must happen before _add_shared_fence(), - * which makes this a slightly strange place to call it. OTOH this - * is a convenient can-fail point to hook it in. (And similar to - * how etnaviv and nouveau handle this.) - */ - ret = reservation_object_reserve_shared(msm_obj->resv); - if (ret) - return ret; - } - fobj = reservation_object_get_list(msm_obj->resv); if (!fobj || (fobj->shared_count == 0)) { fence = reservation_object_get_excl(msm_obj->resv); -- cgit v1.2.3 From ad5149c4f65f7cb984ab85b366c6e7b573eaf48f Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Tue, 12 Sep 2017 14:23:05 -0400 Subject: drm/msm: fix _NO_IMPLICIT fencing case We need to call reservation_object_reserve_shared() in both cases, but this wasn't happening in the _NO_IMPLICIT submit case. Fixes: f0a42bb ("drm/msm: submit support for in-fences") Reported-by: Jordan Crouse Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gem.c') diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index f15821a0d900..a0ab6040c484 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -610,17 +610,6 @@ int msm_gem_sync_object(struct drm_gem_object *obj, struct dma_fence *fence; int i, ret; - if (!exclusive) { - /* NOTE: _reserve_shared() must happen before _add_shared_fence(), - * which makes this a slightly strange place to call it. OTOH this - * is a convenient can-fail point to hook it in. (And similar to - * how etnaviv and nouveau handle this.) - */ - ret = reservation_object_reserve_shared(msm_obj->resv); - if (ret) - return ret; - } - fobj = reservation_object_get_list(msm_obj->resv); if (!fobj || (fobj->shared_count == 0)) { fence = reservation_object_get_excl(msm_obj->resv); -- cgit v1.2.3 From fad33f4b1073a423a9ecd2a335de05e7a3ec1f37 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 15 Sep 2017 08:38:20 -0400 Subject: drm/msm: add special _get_vaddr_active() for cmdstream dumps Prep work for adding a debugfs file that dumps just submits which trigger hangs/faults. In this case the bo may already be in the MADV_DONTNEED state, but will be still on the active list (since the submit hasn't completed yet). So the normal check that the bo is in the WILLNEED state does not apply. (But of course the bo should definitely not be in the PURGED state!) Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gem.c') diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index a0ab6040c484..0776160a6924 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -470,14 +470,16 @@ fail: return ret; } -void *msm_gem_get_vaddr(struct drm_gem_object *obj) +static void *get_vaddr(struct drm_gem_object *obj, unsigned madv) { struct msm_gem_object *msm_obj = to_msm_bo(obj); int ret = 0; mutex_lock(&msm_obj->lock); - if (WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED)) { + if (WARN_ON(msm_obj->madv > madv)) { + dev_err(obj->dev->dev, "Invalid madv state: %u vs %u\n", + msm_obj->madv, madv); mutex_unlock(&msm_obj->lock); return ERR_PTR(-EBUSY); } @@ -513,6 +515,22 @@ fail: return ERR_PTR(ret); } +void *msm_gem_get_vaddr(struct drm_gem_object *obj) +{ + return get_vaddr(obj, MSM_MADV_WILLNEED); +} + +/* + * Don't use this! It is for the very special case of dumping + * submits from GPU hangs or faults, were the bo may already + * be MSM_MADV_DONTNEED, but we know the buffer is still on the + * active list. + */ +void *msm_gem_get_vaddr_active(struct drm_gem_object *obj) +{ + return get_vaddr(obj, __MSM_MADV_PURGED); +} + void msm_gem_put_vaddr(struct drm_gem_object *obj) { struct msm_gem_object *msm_obj = to_msm_bo(obj); -- cgit v1.2.3