[PATCH v2] drm/msm: avoid an unmatched fbdev IOVA pin

[Michael McAllister]

Unbinding the display on the Moto G10 produces a pin-count warning when
the console framebuffer is freed. The fbdev probe takes an IOVA pin,
but normal teardown never drops it.

Reserve the address with msm_gem_get_iova() instead. Scanout pins the
buffer while it is in use, and the CPU mapping holds its own pin.

Keep a VMA reference for fbdev so deleting the temporary GEM handle does
not release the reserved address. Drop that reference on probe failure
or when the framebuffer is destroyed. This also balances the existing
error-path VMA put, which otherwise drops the handle's reference twice.

Get the CPU mapping before publishing the framebuffer in the helper, so
a mapping failure does not leave helper->fb and helper->buffer pointing
to freed memory.

Fixes: 7ad0e8cf6317 ("drm/msm: Count how many times iova memory is pinned")
Signed-off-by: Michael McAllister <me@xxxxxxxxxxxxxxxxxxxxx>
---
Hi,

Changes in v2:
- Keep the fbdev VMA reference until teardown and balance the error paths.
- Get the CPU mapping before publishing the helper pointers.

Thanks,
Michael

 drivers/gpu/drm/msm/msm_fbdev.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index dd6d6c507d..ed599884bf 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -48,6 +48,7 @@ static void msm_fbdev_fb_destroy(struct fb_info *info)
 	drm_fb_helper_fini(helper);
 
 	msm_gem_put_vaddr(bo);
+	msm_gem_vma_put(bo);
 
 	drm_client_buffer_delete(helper->buffer);
 	drm_client_release(&helper->client);
@@ -137,15 +138,18 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
 		goto err_drm_gem_handle_delete;
 	}
 
-	/*
-	 * NOTE: if we can be guaranteed to be able to map buffer
-	 * in panic (ie. lock-safe, etc) we could avoid pinning the
-	 * buffer now:
-	 */
-	ret = msm_gem_get_and_pin_iova(bo, priv->kms->vm, &paddr);
+	/* Keep the IOVA reserved after deleting the temporary GEM handle. */
+	msm_gem_vma_get(bo);
+	ret = msm_gem_get_iova(bo, priv->kms->vm, &paddr);
 	if (ret) {
 		drm_err(dev, "failed to get buffer obj iova: %d\n", ret);
-		goto err_drm_client_buffer_delete;
+		goto err_msm_gem_vma_put;
+	}
+
+	fbi->screen_buffer = msm_gem_get_vaddr(bo);
+	if (IS_ERR(fbi->screen_buffer)) {
+		ret = PTR_ERR(fbi->screen_buffer);
+		goto err_msm_gem_vma_put;
 	}
 
 	DBG("fbi=%p, dev=%p", fbi, dev);
@@ -159,11 +163,6 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
 
 	drm_fb_helper_fill_info(fbi, helper, sizes);
 
-	fbi->screen_buffer = msm_gem_get_vaddr(bo);
-	if (IS_ERR(fbi->screen_buffer)) {
-		ret = PTR_ERR(fbi->screen_buffer);
-		goto err_msm_gem_unpin;
-	}
 	fbi->screen_size = bo->size;
 	fbi->fix.smem_start = paddr;
 	fbi->fix.smem_len = bo->size;
@@ -179,10 +178,8 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
 
 	return 0;
 
-err_msm_gem_unpin:
-	msm_gem_unpin_iova(bo, priv->kms->vm);
+err_msm_gem_vma_put:
 	msm_gem_vma_put(bo);
-err_drm_client_buffer_delete:
 	drm_client_buffer_delete(buffer);
 err_drm_gem_handle_delete:
 	drm_gem_handle_delete(file, handle);

base-commit: 1e59855738454b14021b64115f0090eafc7526ae
-- 
2.39.5