diff options
author | Rob Clark <robdclark@gmail.com> | 2017-06-13 09:15:36 -0400 |
---|---|---|
committer | Rob Clark <robdclark@gmail.com> | 2017-06-16 11:16:01 -0400 |
commit | cb1e38181a0728777057fb03fc4cddb29b7fb24d (patch) | |
tree | 27fbe164012963de61373865d74a73a289663752 | |
parent | 42a105e9cfaf0a0c74fdac5ba4ff17d6c0b024cd (diff) |
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>
-rw-r--r-- | drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 13 | ||||
-rw-r--r-- | drivers/gpu/drm/msm/adreno/a5xx_power.c | 11 | ||||
-rw-r--r-- | drivers/gpu/drm/msm/adreno/adreno_device.c | 2 | ||||
-rw-r--r-- | drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 | ||||
-rw-r--r-- | drivers/gpu/drm/msm/msm_gem.c | 3 | ||||
-rw-r--r-- | drivers/gpu/drm/msm/msm_gpu.c | 2 |
6 files changed, 17 insertions, 16 deletions
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c index c4b775e1f23b..8d17f525c417 100644 --- 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, | |||
297 | struct drm_gem_object *bo; | 297 | struct drm_gem_object *bo; |
298 | void *ptr; | 298 | void *ptr; |
299 | 299 | ||
300 | mutex_lock(&drm->struct_mutex); | ||
301 | bo = msm_gem_new(drm, fw->size - 4, MSM_BO_UNCACHED); | 300 | bo = msm_gem_new(drm, fw->size - 4, MSM_BO_UNCACHED); |
302 | mutex_unlock(&drm->struct_mutex); | ||
303 | |||
304 | if (IS_ERR(bo)) | 301 | if (IS_ERR(bo)) |
305 | return bo; | 302 | return bo; |
306 | 303 | ||
307 | ptr = msm_gem_get_vaddr(bo); | 304 | ptr = msm_gem_get_vaddr_locked(bo); |
308 | if (!ptr) { | 305 | if (!ptr) { |
309 | drm_gem_object_unreference_unlocked(bo); | 306 | drm_gem_object_unreference(bo); |
310 | return ERR_PTR(-ENOMEM); | 307 | return ERR_PTR(-ENOMEM); |
311 | } | 308 | } |
312 | 309 | ||
313 | if (iova) { | 310 | if (iova) { |
314 | int ret = msm_gem_get_iova(bo, gpu->id, iova); | 311 | int ret = msm_gem_get_iova_locked(bo, gpu->id, iova); |
315 | 312 | ||
316 | if (ret) { | 313 | if (ret) { |
317 | drm_gem_object_unreference_unlocked(bo); | 314 | drm_gem_object_unreference(bo); |
318 | return ERR_PTR(ret); | 315 | return ERR_PTR(ret); |
319 | } | 316 | } |
320 | } | 317 | } |
321 | 318 | ||
322 | memcpy(ptr, &fw->data[4], fw->size - 4); | 319 | memcpy(ptr, &fw->data[4], fw->size - 4); |
323 | 320 | ||
324 | msm_gem_put_vaddr(bo); | 321 | msm_gem_put_vaddr_locked(bo); |
325 | return bo; | 322 | return bo; |
326 | } | 323 | } |
327 | 324 | ||
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_power.c b/drivers/gpu/drm/msm/adreno/a5xx_power.c index ed0802e6ca59..f3274b827a49 100644 --- 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) | |||
294 | */ | 294 | */ |
295 | bosize = (cmds_size + (cmds_size / TYPE4_MAX_PAYLOAD) + 1) << 2; | 295 | bosize = (cmds_size + (cmds_size / TYPE4_MAX_PAYLOAD) + 1) << 2; |
296 | 296 | ||
297 | mutex_lock(&drm->struct_mutex); | ||
298 | a5xx_gpu->gpmu_bo = msm_gem_new(drm, bosize, MSM_BO_UNCACHED); | 297 | a5xx_gpu->gpmu_bo = msm_gem_new(drm, bosize, MSM_BO_UNCACHED); |
299 | mutex_unlock(&drm->struct_mutex); | ||
300 | |||
301 | if (IS_ERR(a5xx_gpu->gpmu_bo)) | 298 | if (IS_ERR(a5xx_gpu->gpmu_bo)) |
302 | goto err; | 299 | goto err; |
303 | 300 | ||
304 | if (msm_gem_get_iova(a5xx_gpu->gpmu_bo, gpu->id, &a5xx_gpu->gpmu_iova)) | 301 | if (msm_gem_get_iova_locked(a5xx_gpu->gpmu_bo, gpu->id, &a5xx_gpu->gpmu_iova)) |
305 | goto err; | 302 | goto err; |
306 | 303 | ||
307 | ptr = msm_gem_get_vaddr(a5xx_gpu->gpmu_bo); | 304 | ptr = msm_gem_get_vaddr_locked(a5xx_gpu->gpmu_bo); |
308 | if (!ptr) | 305 | if (!ptr) |
309 | goto err; | 306 | goto err; |
310 | 307 | ||
@@ -323,7 +320,7 @@ void a5xx_gpmu_ucode_init(struct msm_gpu *gpu) | |||
323 | cmds_size -= _size; | 320 | cmds_size -= _size; |
324 | } | 321 | } |
325 | 322 | ||
326 | msm_gem_put_vaddr(a5xx_gpu->gpmu_bo); | 323 | msm_gem_put_vaddr_locked(a5xx_gpu->gpmu_bo); |
327 | a5xx_gpu->gpmu_dwords = dwords; | 324 | a5xx_gpu->gpmu_dwords = dwords; |
328 | 325 | ||
329 | goto out; | 326 | goto out; |
@@ -332,7 +329,7 @@ err: | |||
332 | if (a5xx_gpu->gpmu_iova) | 329 | if (a5xx_gpu->gpmu_iova) |
333 | msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->id); | 330 | msm_gem_put_iova(a5xx_gpu->gpmu_bo, gpu->id); |
334 | if (a5xx_gpu->gpmu_bo) | 331 | if (a5xx_gpu->gpmu_bo) |
335 | drm_gem_object_unreference_unlocked(a5xx_gpu->gpmu_bo); | 332 | drm_gem_object_unreference(a5xx_gpu->gpmu_bo); |
336 | 333 | ||
337 | a5xx_gpu->gpmu_bo = NULL; | 334 | a5xx_gpu->gpmu_bo = NULL; |
338 | a5xx_gpu->gpmu_iova = 0; | 335 | 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 b7bd6d393215..c75c4df4bc39 100644 --- 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) | |||
159 | int ret; | 159 | int ret; |
160 | 160 | ||
161 | pm_runtime_get_sync(&pdev->dev); | 161 | pm_runtime_get_sync(&pdev->dev); |
162 | mutex_lock(&dev->struct_mutex); | ||
162 | ret = msm_gpu_hw_init(gpu); | 163 | ret = msm_gpu_hw_init(gpu); |
164 | mutex_unlock(&dev->struct_mutex); | ||
163 | pm_runtime_put_sync(&pdev->dev); | 165 | pm_runtime_put_sync(&pdev->dev); |
164 | if (ret) { | 166 | if (ret) { |
165 | dev_err(dev->dev, "gpu hw init failed: %d\n", ret); | 167 | 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 f8287fd727f1..30a2096ac9a2 100644 --- 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) | |||
64 | 64 | ||
65 | DBG("%s", gpu->name); | 65 | DBG("%s", gpu->name); |
66 | 66 | ||
67 | ret = msm_gem_get_iova(gpu->rb->bo, gpu->id, &gpu->rb_iova); | 67 | ret = msm_gem_get_iova_locked(gpu->rb->bo, gpu->id, &gpu->rb_iova); |
68 | if (ret) { | 68 | if (ret) { |
69 | gpu->rb_iova = 0; | 69 | gpu->rb_iova = 0; |
70 | dev_err(gpu->dev->dev, "could not map ringbuffer: %d\n", ret); | 70 | 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 be77a35a7a8e..38fbaadccfb7 100644 --- 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, | |||
314 | struct msm_gem_object *msm_obj = to_msm_bo(obj); | 314 | struct msm_gem_object *msm_obj = to_msm_bo(obj); |
315 | int ret = 0; | 315 | int ret = 0; |
316 | 316 | ||
317 | WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex)); | ||
318 | |||
317 | if (!msm_obj->domain[id].iova) { | 319 | if (!msm_obj->domain[id].iova) { |
318 | struct msm_drm_private *priv = obj->dev->dev_private; | 320 | struct msm_drm_private *priv = obj->dev->dev_private; |
319 | struct page **pages = get_pages(obj); | 321 | 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) | |||
345 | * bo is deleted: | 347 | * bo is deleted: |
346 | */ | 348 | */ |
347 | if (msm_obj->domain[id].iova) { | 349 | if (msm_obj->domain[id].iova) { |
350 | might_lock(&obj->dev->struct_mutex); | ||
348 | *iova = msm_obj->domain[id].iova; | 351 | *iova = msm_obj->domain[id].iova; |
349 | return 0; | 352 | return 0; |
350 | } | 353 | } |
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 5b118e8ead18..ebbaed442e8a 100644 --- 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) | |||
203 | { | 203 | { |
204 | int ret; | 204 | int ret; |
205 | 205 | ||
206 | WARN_ON(!mutex_is_locked(&gpu->dev->struct_mutex)); | ||
207 | |||
206 | if (!gpu->needs_hw_init) | 208 | if (!gpu->needs_hw_init) |
207 | return 0; | 209 | return 0; |
208 | 210 | ||