prepare_count, iova[] and the pins they describe are updated locklessly,
but drm_atomic_helper_commit() prepares a new state in parallel with the
completion of the previous one -- stall_checks() only stalls on the second
previous commit. Commit N+1's ->prepare_fb() thus runs while commit N is
in ->cleanup_fb(), and for a double-buffered flip that is the same
framebuffer:
cleanup: prepare_count 1 -> 0
prepare: prepare_count 0 -> 1, pins, stores iova[]
cleanup: memset(iova, 0)
leaving the plane programmed with a NULL base address:
arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402,
iova=0x00000100, fsynr=0x3e0023, cbfrsynra=0x1c00, cb=11
The opposite order is broken too since commit 8ac37c88f991 ("drm/msm:
Refcount framebuffer pins"): a prepare which finds the count non-zero
returns at once, assuming iova[] is populated.
Both callbacks may sleep, so a mutex will do. drm_framebuffer_init()
adds the framebuffer to the object idr, from where userspace can reach it
before msm_framebuffer_init() returns, so take the private state out of
its way and initialise it first.
Fixes: 8ac37c88f991 ("drm/msm: Refcount framebuffer pins")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/msm_fb.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c
index 60c108d35d2a..a38e5b3a1d1e 100644
--- a/drivers/gpu/drm/msm/msm_fb.c
+++ b/drivers/gpu/drm/msm/msm_fb.c
@@ -22,9 +22,12 @@ struct msm_framebuffer {
/* Count of # of attached planes which need dirtyfb: */
refcount_t dirtyfb;
+ /* Protects the pin state below: */
+ struct mutex lock;
+
/* Framebuffer per-plane address, if pinned, else zero: */
uint64_t iova[DRM_FORMAT_MAX_PLANES];
- atomic_t prepare_count;
+ unsigned int prepare_count;
};
#define to_msm_framebuffer(x) container_of(x, struct msm_framebuffer, base)
@@ -45,9 +48,17 @@ static int msm_framebuffer_dirtyfb(struct drm_framebuffer
*fb,
clips, num_clips);
}
+static void msm_framebuffer_destroy(struct drm_framebuffer *fb)
+{
+ struct msm_framebuffer *msm_fb = to_msm_framebuffer(fb);
+
+ mutex_destroy(&msm_fb->lock);
+ drm_gem_fb_destroy(fb);
+}
+
static const struct drm_framebuffer_funcs msm_framebuffer_funcs = {
.create_handle = drm_gem_fb_create_handle,
- .destroy = drm_gem_fb_destroy,
+ .destroy = msm_framebuffer_destroy,
.dirty = msm_framebuffer_dirtyfb,
};
@@ -81,7 +92,9 @@ int msm_framebuffer_prepare(struct drm_framebuffer *fb, bool
needs_dirtyfb)
if (needs_dirtyfb)
refcount_inc(&msm_fb->dirtyfb);
- if (atomic_inc_return(&msm_fb->prepare_count) > 1)
+ guard(mutex)(&msm_fb->lock);
+
+ if (msm_fb->prepare_count++)
return 0;
for (i = 0; i < n; i++) {
@@ -106,7 +119,9 @@ void msm_framebuffer_cleanup(struct drm_framebuffer *fb,
bool needed_dirtyfb)
if (needed_dirtyfb)
refcount_dec(&msm_fb->dirtyfb);
- if (atomic_dec_return(&msm_fb->prepare_count))
+ guard(mutex)(&msm_fb->lock);
+
+ if (--msm_fb->prepare_count)
return;
memset(msm_fb->iova, 0, sizeof(msm_fb->iova));
@@ -199,14 +214,15 @@ msm_framebuffer_init(struct drm_device *dev, const struct
drm_format_info *info,
drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
+ refcount_set(&msm_fb->dirtyfb, 1);
+ mutex_init(&msm_fb->lock);
+
ret = drm_framebuffer_init(dev, fb, &msm_framebuffer_funcs);
if (ret) {
DRM_DEV_ERROR(dev->dev, "framebuffer init failed: %d\n", ret);
goto fail;
}
- refcount_set(&msm_fb->dirtyfb, 1);
-
drm_dbg_state(dev, "create: FB ID: %d (%p)\n", fb->base.id, fb);
return fb;
--
2.47.3
