Skip to content

Commit

Permalink
drm/msm: fix potential deadlock in gpu init
Browse files Browse the repository at this point in the history
Somewhere along the way, the firmware loader sprouted another lock
dependency, resulting in possible deadlock scenario:

 &dev->struct_mutex --> &sb->s_type->i_mutex_key#2 --> &mm->mmap_sem

which is problematic vs things like gem mmap.

So introduce a separate mutex to synchronize gpu init.

Signed-off-by: Rob Clark <robdclark@gmail.com>
  • Loading branch information
Rob Clark committed Aug 4, 2014
1 parent 944fc36 commit a1ad352
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 8 deletions.
8 changes: 5 additions & 3 deletions drivers/gpu/drm/msm/adreno/adreno_gpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ int adreno_hw_init(struct msm_gpu *gpu)

DBG("%s", gpu->name);

ret = msm_gem_get_iova_locked(gpu->rb->bo, gpu->id, &gpu->rb_iova);
ret = msm_gem_get_iova(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);
Expand Down Expand Up @@ -370,22 +370,24 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
return ret;
}

mutex_lock(&drm->struct_mutex);
gpu->memptrs_bo = msm_gem_new(drm, sizeof(*gpu->memptrs),
MSM_BO_UNCACHED);
mutex_unlock(&drm->struct_mutex);
if (IS_ERR(gpu->memptrs_bo)) {
ret = PTR_ERR(gpu->memptrs_bo);
gpu->memptrs_bo = NULL;
dev_err(drm->dev, "could not allocate memptrs: %d\n", ret);
return ret;
}

gpu->memptrs = msm_gem_vaddr_locked(gpu->memptrs_bo);
gpu->memptrs = msm_gem_vaddr(gpu->memptrs_bo);
if (!gpu->memptrs) {
dev_err(drm->dev, "could not vmap memptrs\n");
return -ENOMEM;
}

ret = msm_gem_get_iova_locked(gpu->memptrs_bo, gpu->base.id,
ret = msm_gem_get_iova(gpu->memptrs_bo, gpu->base.id,
&gpu->memptrs_iova);
if (ret) {
dev_err(drm->dev, "could not map memptrs: %d\n", ret);
Expand Down
13 changes: 8 additions & 5 deletions drivers/gpu/drm/msm/msm_drv.c
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ static int msm_load(struct drm_device *dev, unsigned long flags)
struct msm_kms *kms;
int ret;


priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv) {
dev_err(dev->dev, "failed to allocate private data\n");
Expand Down Expand Up @@ -314,13 +313,15 @@ static int msm_load(struct drm_device *dev, unsigned long flags)

static void load_gpu(struct drm_device *dev)
{
static DEFINE_MUTEX(init_lock);
struct msm_drm_private *priv = dev->dev_private;
struct msm_gpu *gpu;

mutex_lock(&init_lock);

if (priv->gpu)
return;
goto out;

mutex_lock(&dev->struct_mutex);
gpu = a3xx_gpu_init(dev);
if (IS_ERR(gpu)) {
dev_warn(dev->dev, "failed to load a3xx gpu\n");
Expand All @@ -330,7 +331,9 @@ static void load_gpu(struct drm_device *dev)

if (gpu) {
int ret;
mutex_lock(&dev->struct_mutex);
gpu->funcs->pm_resume(gpu);
mutex_unlock(&dev->struct_mutex);
ret = gpu->funcs->hw_init(gpu);
if (ret) {
dev_err(dev->dev, "gpu hw init failed: %d\n", ret);
Expand All @@ -340,12 +343,12 @@ static void load_gpu(struct drm_device *dev)
/* give inactive pm a chance to kick in: */
msm_gpu_retire(gpu);
}

}

priv->gpu = gpu;

mutex_unlock(&dev->struct_mutex);
out:
mutex_unlock(&init_lock);
}

static int msm_open(struct drm_device *dev, struct drm_file *file)
Expand Down
3 changes: 3 additions & 0 deletions drivers/gpu/drm/msm/msm_gpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,11 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
}
gpu->id = msm_register_mmu(drm, gpu->mmu);


/* Create ringbuffer: */
mutex_lock(&drm->struct_mutex);
gpu->rb = msm_ringbuffer_new(gpu, ringsz);
mutex_unlock(&drm->struct_mutex);
if (IS_ERR(gpu->rb)) {
ret = PTR_ERR(gpu->rb);
gpu->rb = NULL;
Expand Down

0 comments on commit a1ad352

Please sign in to comment.