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 a096883dda
("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 <lucas.demarchi@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
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
This commit is contained in:
@@ -107,7 +107,7 @@ intel_dsb_get(struct intel_crtc *crtc)
|
|||||||
if (!HAS_DSB(i915))
|
if (!HAS_DSB(i915))
|
||||||
return dsb;
|
return dsb;
|
||||||
|
|
||||||
if (atomic_add_return(1, &dsb->refcount) != 1)
|
if (dsb->refcount++ != 0)
|
||||||
return dsb;
|
return dsb;
|
||||||
|
|
||||||
dsb->id = DSB1;
|
dsb->id = DSB1;
|
||||||
@@ -123,7 +123,7 @@ intel_dsb_get(struct intel_crtc *crtc)
|
|||||||
if (IS_ERR(vma)) {
|
if (IS_ERR(vma)) {
|
||||||
DRM_ERROR("Vma creation failed\n");
|
DRM_ERROR("Vma creation failed\n");
|
||||||
i915_gem_object_put(obj);
|
i915_gem_object_put(obj);
|
||||||
atomic_dec(&dsb->refcount);
|
dsb->refcount--;
|
||||||
goto err;
|
goto err;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -132,7 +132,7 @@ intel_dsb_get(struct intel_crtc *crtc)
|
|||||||
DRM_ERROR("Command buffer creation failed\n");
|
DRM_ERROR("Command buffer creation failed\n");
|
||||||
i915_vma_unpin_and_release(&vma, 0);
|
i915_vma_unpin_and_release(&vma, 0);
|
||||||
dsb->cmd_buf = NULL;
|
dsb->cmd_buf = NULL;
|
||||||
atomic_dec(&dsb->refcount);
|
dsb->refcount--;
|
||||||
goto err;
|
goto err;
|
||||||
}
|
}
|
||||||
dsb->vma = vma;
|
dsb->vma = vma;
|
||||||
@@ -158,10 +158,10 @@ void intel_dsb_put(struct intel_dsb *dsb)
|
|||||||
if (!HAS_DSB(i915))
|
if (!HAS_DSB(i915))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
if (WARN_ON(atomic_read(&dsb->refcount) == 0))
|
if (WARN_ON(dsb->refcount == 0))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
if (atomic_dec_and_test(&dsb->refcount)) {
|
if (--dsb->refcount == 0) {
|
||||||
i915_vma_unpin_and_release(&dsb->vma, I915_VMA_RELEASE_MAP);
|
i915_vma_unpin_and_release(&dsb->vma, I915_VMA_RELEASE_MAP);
|
||||||
dsb->cmd_buf = NULL;
|
dsb->cmd_buf = NULL;
|
||||||
dsb->free_pos = 0;
|
dsb->free_pos = 0;
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ enum dsb_id {
|
|||||||
};
|
};
|
||||||
|
|
||||||
struct intel_dsb {
|
struct intel_dsb {
|
||||||
atomic_t refcount;
|
long refcount;
|
||||||
enum dsb_id id;
|
enum dsb_id id;
|
||||||
u32 *cmd_buf;
|
u32 *cmd_buf;
|
||||||
struct i915_vma *vma;
|
struct i915_vma *vma;
|
||||||
|
|||||||
Reference in New Issue
Block a user