All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	stable@vger.kernel.org, Alex Deucher <alexdeucher@gmail.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Lucas Stach <l.stach@pengutronix.de>
Subject: Re: [Intel-gfx] [PATCH] drm: Wrap direct calls to driver->gem_free_object from CMA
Date: Tue, 31 May 2016 23:37:17 +0200	[thread overview]
Message-ID: <20160531213717.GP7231@phenom.ffwll.local> (raw)
In-Reply-To: <1464729952-13582-1-git-send-email-chris@chris-wilson.co.uk>

On Tue, May 31, 2016 at 10:25:52PM +0100, Chris Wilson wrote:
> Since the introduction of (struct_mutex) lockless GEM bo freeing, there
> are a pair of driver vfuncs for freeing the GEM bo, of which the driver
> may choose to only implement driver->gem_object_free_unlocked (and so
> avoid taking the struct_mutex along the free path). However, the CMA GEM
> helpers were still calling driver->gem_free_object directly, now NULL,
> and promptly dying on the fancy new lockless drivers. Oops.
> 
> Robert Foss bisected this to b82caafcf2303 (drm/vc4: Use lockless gem BO
> free callback) on his vc4 device, but that just serves as an enabler for
> 9f0ba539d13ae (drm/gem: support BO freeing without dev->struct_mutex).
> 
> Reported-by: Robert Foss <robert.foss@collabora.com>
> Fixes: 9f0ba539d13ae (drm/gem: support BO freeing without dev->struct_mutex)
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Robert Foss <robert.foss@collabora.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Alex Deucher <alexdeucher@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: stable@vger.kernel.org

The original offending commit is only in 4.7, so no need for cc: stable.
It won't hurt either though.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c  |  2 +-
>  drivers/gpu/drm/drm_gem_cma_helper.c | 12 +++---------
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 172cafe11c71..5075fae3c4e2 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -445,7 +445,7 @@ err_cma_destroy:
>  err_fb_info_destroy:
>  	drm_fb_helper_release_fbi(helper);
>  err_gem_free_object:
> -	dev->driver->gem_free_object(&obj->base);
> +	drm_gem_object_unreference_unlocked(&obj->base);
>  	return ret;
>  }
>  EXPORT_SYMBOL(drm_fbdev_cma_create_with_funcs);
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index e1ab008b3f08..1d6c335584ec 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -121,7 +121,7 @@ struct drm_gem_cma_object *drm_gem_cma_create(struct drm_device *drm,
>  	return cma_obj;
>  
>  error:
> -	drm->driver->gem_free_object(&cma_obj->base);
> +	drm_gem_object_unreference_unlocked(&cma_obj->base);
>  	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_cma_create);
> @@ -162,18 +162,12 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv,
>  	 * and handle has the id what user can see.
>  	 */
>  	ret = drm_gem_handle_create(file_priv, gem_obj, handle);
> -	if (ret)
> -		goto err_handle_create;
> -
>  	/* drop reference from allocate - handle holds it now. */
>  	drm_gem_object_unreference_unlocked(gem_obj);
> +	if (ret)
> +		return ERR_PTR(ret);
>  
>  	return cma_obj;
> -
> -err_handle_create:
> -	drm->driver->gem_free_object(gem_obj);
> -
> -	return ERR_PTR(ret);
>  }
>  
>  /**
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

  reply	other threads:[~2016-05-31 21:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-31 21:25 [PATCH] drm: Wrap direct calls to driver->gem_free_object from CMA Chris Wilson
2016-05-31 21:25 ` Chris Wilson
2016-05-31 21:37 ` Daniel Vetter [this message]
2016-05-31 21:51 ` Robert Foss
2016-05-31 21:51   ` Robert Foss
2016-06-01  5:58 ` ✗ Ro.CI.BAT: warning for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160531213717.GP7231@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=alexdeucher@gmail.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=l.stach@pengutronix.de \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.