drm/msm: fix locking inconsistency for gpu->hw_init()

Most, but not all, paths where calling the with struct_mutex held.  The
fast-path in msm_gem_get_iova() (plus some sub-code-paths that only run
the first time) was masking this issue.

So lets just always hold struct_mutex for hw_init().  And sprinkle some
WARN_ON()'s and might_lock() to avoid this sort of problem in the
future.

Signed-off-by: Rob Clark <robdclark@gmail.com>

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index c4b775e1f23bdcd2236f685aeee7e18d57128761..8d17f525c417a728d5339ec6916885062281a990 100644 (file)
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -297,31 +297,28 @@ static struct drm_gem_object *a5xx_ucode_load_bo(struct msm_gpu *gpu,
        struct drm_gem_object *bo;
        void *ptr;
 
-       mutex_lock(&drm->struct_mutex);
        bo = msm_gem_new(drm, fw->size - 4, MSM_BO_UNCACHED);
-       mutex_unlock(&drm->struct_mutex);
-
        if (IS_ERR(bo))
                return bo;
 
-       ptr = msm_gem_get_vaddr(bo);
+       ptr = msm_gem_get_vaddr_locked(bo);
        if (!ptr) {
-               drm_gem_object_unreference_unlocked(bo);
+               drm_gem_object_unreference(bo);
                return ERR_PTR(-ENOMEM);
        }
 
        if (iova) {
-               int ret = msm_gem_get_iova(bo, gpu->id, iova);
+               int ret = msm_gem_get_iova_locked(bo, gpu->id, iova);
 
                if (ret) {
-                       drm_gem_object_unreference_unlocked(bo);
+                       drm_gem_object_unreference(bo);
                        return ERR_PTR(ret);
                }
        }
 
        memcpy(ptr, &fw->data[4], fw->size - 4);
 
-       msm_gem_put_vaddr(bo);
+       msm_gem_put_vaddr_locked(bo);
        return bo;
 }
 

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c b/drivers/gpu/drm/msm/adreno/a5xx_power.c
index ed0802e6ca590ea43aba5f82b425ebcded8b5c98..f3274b827a491d38a0aa59ec401fa2f56b948502 100644 (file)
--- a/drivers/gpu/drm/msm/adreno/a5xx_power.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_power.c
@@ -294,17 +294,14 @@ void a5xx_gpmu_ucode_init(struct msm_gpu *gpu)
         */
        bosize = (cmds_size + (cmds_size / TYPE4_MAX_PAYLOAD) + 1) << 2;
 
-       mutex_lock(&drm->struct_mutex);
        a5xx_gpu->gpmu_bo = msm_gem_new(drm, bosize, MSM_BO_UNCACHED);
-       mutex_unlock(&drm->struct_mutex);
-
        if (IS_ERR(a5xx_gpu->gpmu_bo))
                goto err;
 
-       if (msm_gem_get_iova(a5xx_gpu->gpmu_bo, gpu->id, &a5xx_gpu->gpmu_iova))
+       if (msm_gem_get_iova_locked(a5xx_gpu->gpmu_bo, gpu->id, &a5xx_gpu->gpmu_iova))
                goto err;
 
-       ptr = msm_gem_get_vaddr(a5xx_gpu->gpmu_bo);
+       ptr = msm_gem_get_vaddr_locked(a5xx_gpu->gpmu_bo);
        if (!ptr)
                goto err;
 
@@ -323,7 +320,7 @@ void a5xx_gpmu_ucode_init(struct msm_gpu *gpu)
                cmds_size -= _size;
        }
 
-       msm_gem_put_vaddr(a5xx_gpu->gpmu_bo);
+       msm_gem_put_vaddr_locked(a5xx_gpu->gpmu_bo);
        a5xx_gpu->gpmu_dwords = dwords;
 
        goto out;
@@ -332,7 +329,7 @@ err:
        if (a5xx_gpu->gpmu_iova)
                msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->id);
        if (a5xx_gpu->gpmu_bo)
-               drm_gem_object_unreference_unlocked(a5xx_gpu->gpmu_bo);
+               drm_gem_object_unreference(a5xx_gpu->gpmu_bo);
 
        a5xx_gpu->gpmu_bo = NULL;
        a5xx_gpu->gpmu_iova = 0;

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index b7bd6d393215b349364a75a5463c9f7f22ce625b..c75c4df4bc3954e564b78d121495e63418401403 100644 (file)
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -159,7 +159,9 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
                int ret;
 
                pm_runtime_get_sync(&pdev->dev);
+               mutex_lock(&dev->struct_mutex);
                ret = msm_gpu_hw_init(gpu);
+               mutex_unlock(&dev->struct_mutex);
                pm_runtime_put_sync(&pdev->dev);
                if (ret) {
                        dev_err(dev->dev, "gpu hw init failed: %d\n", ret);

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index f8287fd727f12256cde9a2cc2e4ff4615972f95f..30a2096ac9a253378251a348e6814b7431b6f0af 100644 (file)
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -64,7 +64,7 @@ int adreno_hw_init(struct msm_gpu *gpu)
 
        DBG("%s", gpu->name);
 
-       ret = msm_gem_get_iova(gpu->rb->bo, gpu->id, &gpu->rb_iova);
+       ret = msm_gem_get_iova_locked(gpu->rb->bo, gpu->id, &gpu->rb_iova);
        if (ret) {
                gpu->rb_iova = 0;
                dev_err(gpu->dev->dev, "could not map ringbuffer: %d\n", ret);

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index be77a35a7a8e5c94f1c2de6c315a325a759abd20..38fbaadccfb7ee1acd90f4a1fc5158b64b684b82 100644 (file)
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -314,6 +314,8 @@ int msm_gem_get_iova_locked(struct drm_gem_object *obj, int id,
        struct msm_gem_object *msm_obj = to_msm_bo(obj);
        int ret = 0;
 
+       WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
+
        if (!msm_obj->domain[id].iova) {
                struct msm_drm_private *priv = obj->dev->dev_private;
                struct page **pages = get_pages(obj);
@@ -345,6 +347,7 @@ int msm_gem_get_iova(struct drm_gem_object *obj, int id, uint64_t *iova)
         * bo is deleted:
         */
        if (msm_obj->domain[id].iova) {
+               might_lock(&obj->dev->struct_mutex);
                *iova = msm_obj->domain[id].iova;
                return 0;
        }

diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 5b118e8ead18cc18fd96b7eaa2b8425e61570c74..ebbaed442e8ad72145d646cfc598cb21d721c2fa 100644 (file)
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -203,6 +203,8 @@ int msm_gpu_hw_init(struct msm_gpu *gpu)
 {
        int ret;
 
+       WARN_ON(!mutex_is_locked(&gpu->dev->struct_mutex));
+
        if (!gpu->needs_hw_init)
                return 0;