From ac4eead3796579bdadd12e2fa99c4ddc920eda30 Mon Sep 17 00:00:00 2001 From: Lucas De Marchi Date: Fri, 15 Nov 2019 17:15:39 -0800 Subject: [PATCH] drm/i915/dsb: remove atomic operations The current dsb API is not really prepared to handle multithread access. I was debugging an issue that ended up fixed by commit a096883dda2c ("drm/i915/dsb: Remove PIN_MAPPABLE from the DSB object VMA") and was puzzled how these atomic operations were guaranteeing atomicity. if (atomic_add_return(1, &dsb->refcount) != 1) return dsb; Thread A could still be initializing dsb struct (and even fail in the middle) while thread B would take a reference and use it (even derefencing a NULL cmd_buf). I don't think the atomic operations here will help much if this were to support multithreaded scenario in future, so just remove them to avoid confusion. v2: Use refcount++ != 0 instead of ++refcount != 1 (from Ville) Signed-off-by: Lucas De Marchi Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/20191111205024.22853-2-lucas.demarchi@intel.com Link: https://patchwork.freedesktop.org/patch/msgid/20191116011539.18230-1-lucas.demarchi@intel.com --- drivers/gpu/drm/i915/display/intel_dsb.c | 10 +++++----- drivers/gpu/drm/i915/display/intel_dsb.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c index d8ad5fe1efef..50c4d98c0020 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.c +++ b/drivers/gpu/drm/i915/display/intel_dsb.c @@ -107,7 +107,7 @@ intel_dsb_get(struct intel_crtc *crtc) if (!HAS_DSB(i915)) return dsb; - if (atomic_add_return(1, &dsb->refcount) != 1) + if (dsb->refcount++ != 0) return dsb; dsb->id = DSB1; @@ -123,7 +123,7 @@ intel_dsb_get(struct intel_crtc *crtc) if (IS_ERR(vma)) { DRM_ERROR("Vma creation failed\n"); i915_gem_object_put(obj); - atomic_dec(&dsb->refcount); + dsb->refcount--; goto err; } @@ -132,7 +132,7 @@ intel_dsb_get(struct intel_crtc *crtc) DRM_ERROR("Command buffer creation failed\n"); i915_vma_unpin_and_release(&vma, 0); dsb->cmd_buf = NULL; - atomic_dec(&dsb->refcount); + dsb->refcount--; goto err; } dsb->vma = vma; @@ -158,10 +158,10 @@ void intel_dsb_put(struct intel_dsb *dsb) if (!HAS_DSB(i915)) return; - if (WARN_ON(atomic_read(&dsb->refcount) == 0)) + if (WARN_ON(dsb->refcount == 0)) return; - if (atomic_dec_and_test(&dsb->refcount)) { + if (--dsb->refcount == 0) { i915_vma_unpin_and_release(&dsb->vma, I915_VMA_RELEASE_MAP); dsb->cmd_buf = NULL; dsb->free_pos = 0; diff --git a/drivers/gpu/drm/i915/display/intel_dsb.h b/drivers/gpu/drm/i915/display/intel_dsb.h index 6f95c8e909e6..395ef9ce558e 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.h +++ b/drivers/gpu/drm/i915/display/intel_dsb.h @@ -22,7 +22,7 @@ enum dsb_id { }; struct intel_dsb { - atomic_t refcount; + long refcount; enum dsb_id id; u32 *cmd_buf; struct i915_vma *vma; -- 2.30.2