Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover

Ville Syrjälä
2015-04-14T09:00:21Z

On Mon, Apr 13, 2015 at 04:03:03PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> intel_user_framebuffer_destroy() requires the struct_mutex for its
> object bookkeeping, so this means that all calls to
> drm_framebuffer_unreference must be held without that lock.
>
> This is a simplified version of the identically named patch by Chris Wilson.
>
> Regression from commit ab8d66752a9c28cd6c94fa173feacdfc1554aa03
> Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Date: Mon Feb 2 15:44:15 2015 +0000
>
> drm/i915: Track old framebuffer instead of object
>
> v2: Bikeshedding.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
> drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 97922fb..5fb11bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14739,6 +14739,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *c;
> struct drm_i915_gem_object *obj;
> + int ret;
>
> mutex_lock(&dev->struct_mutex);
> intel_init_gt_powersave(dev);
> @@ -14763,16 +14764,18 @@ void intel_modeset_gem_init(struct drm_device *dev)
> * pinned & fenced. When we do the allocation it's too early
> * for this.
> */
> - mutex_lock(&dev->struct_mutex);
> for_each_crtc(dev, c) {
> obj = intel_fb_obj(c->primary->fb);
> if (obj == NULL)
> continue;
>
> - if (intel_pin_and_fence_fb_obj(c->primary,
> - c->primary->fb,
> - c->primary->state,
> - NULL)) {
> + mutex_lock(&dev->struct_mutex);
> + ret = intel_pin_and_fence_fb_obj(c->primary,
> + c->primary->fb,
> + c->primary->state,
> + NULL);
> + mutex_unlock(&dev->struct_mutex);
> + if (ret) {
> DRM_ERROR("failed to pin boot fb on pipe %d\n",
> to_intel_crtc(c)->pipe);
> drm_framebuffer_unreference(c->primary->fb);
> @@ -14780,7 +14783,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
> update_state_fb(c->primary);
> }
> }
> - mutex_unlock(&dev->struct_mutex);
>
> intel_backlight_register(dev);
> }
> --
> 2.3.5

--
Ville Syrjälä
Intel OTC