Hello,
On Friday, 25 May 2018 19:39:23 EEST Laurent Pinchart wrote:
> The DRM device struct_mutex is used to protect against concurrent GEM
> object operations that deal with memory allocation and pinning. All
> those operations are local to a GEM object and don't need to be
> serialized across different GEM objects. Replace the struct_mutex with
> a local omap_obj.lock or drop it altogether where not needed.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> ---
> Changes since v1:
>
> - Use lockdep_assert_held() to verify locking correctness
> ---
> drivers/gpu/drm/omapdrm/omap_debugfs.c | 7 --
> drivers/gpu/drm/omapdrm/omap_fbdev.c | 8 +--
> drivers/gpu/drm/omapdrm/omap_gem.c | 122 ++++++++++++++++++++----------
> 3 files changed, 81 insertions(+), 56 deletions(-)
[snip]
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
> b/drivers/gpu/drm/omapdrm/omap_gem.c index 0a9ba1f711e6..39cbc7273b29
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
[snip]
> -/** release backing pages */
> +/* Release backing pages. Must be called with the omap_obj.lock held. */
> static void omap_gem_detach_pages(struct drm_gem_object *obj)
> {
> struct omap_gem_object *omap_obj = to_omap_bo(obj);
> unsigned int npages = obj->size >> PAGE_SHIFT;
> unsigned int i;
>
> + lockdep_assert_held(&omap_obj->lock);
> +
> for (i = 0; i < npages; i++) {
> if (omap_obj->dma_addrs[i])
> dma_unmap_page(obj->dev->dev, omap_obj->dma_addrs[i],
[snip]
> @@ -1056,15 +1091,16 @@ void omap_gem_free_object(struct drm_gem_object
> *obj)
>
> omap_gem_evict(obj);
>
> - WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> -
> spin_lock(&priv->list_lock);
> list_del(&omap_obj->mm_list);
> spin_unlock(&priv->list_lock);
>
> - /* this means the object is still pinned.. which really should
> - * not happen. I think..
> + /*
> + * No need to take the omap_obj.lock as at this point we own the sole
> + * reference to the object.
> */
And this of course causes a lockdep warning, as the function calls
omap_gem_detach_pages(), and the lockdep_assert_held() call there doesn't know
we're the sole owner. I've sent a v2.1 to fix this.
> +
> + /* The object should not be pinned. */
> WARN_ON(omap_obj->dma_addr_cnt > 0);
>
> if (omap_obj->pages) {
[snip]
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
