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

Jani Nikula
2015-04-14T12:18:03Z

On Tue, 14 Apr 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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>

Pushed to drm-intel-next-fixes, thanks for the patch and review.

BR,
Jani.

>
>> ---
>> 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Jani Nikula, Intel Open Source Technology Center