On Thu, 26 Mar 2015, Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com> wrote:
> On 03/26/2015 01:30 PM, Ville Syrjälä wrote:
>> On Thu, Mar 26, 2015 at 12:39:40PM +0000, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursu...@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.
>>>
>>> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
>>> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index cb50854..0788507 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -14020,11 +14020,21 @@ void intel_modeset_gem_init(struct drm_device 
>>> *dev)
>>>                                            c->primary->fb,
>>>                                            c->primary->state,
>>>                                            NULL)) {
>>> +                   /*
>>> +                    * We must drop struct_mutex when calling
>>> +                    * drm_framebuffer_unreference and it is safe to do so
>>> +                    * because it is not needed at this point anyway.
>>> +                    * At this stage the driver is still single-threaded and
>>> +                    * we are taking it only to silence a warning in
>>> +                    * intel_pin_and_fence_fb_obj.
>>> +                    */
>>> +                   mutex_unlock(&dev->struct_mutex);
>>>                     DRM_ERROR("failed to pin boot fb on pipe %d\n",
>>>                               to_intel_crtc(c)->pipe);
>>>                     drm_framebuffer_unreference(c->primary->fb);
>>>                     c->primary->fb = NULL;
>>>                     update_state_fb(c->primary);
>>> +                   mutex_lock(&dev->struct_mutex);
>>>             }
>>>     }
>>>     mutex_unlock(&dev->struct_mutex);
>>
>> Just grab the mutex around the pin_and_fence inside the loop. It doesn't
>> protect anything else.
>
> Well the comment says so, but this way it only grabs and releases it 
> once if there are multiple active crtcs and nothing fails. So I was 
> hoping the comment was enough to explain the reality, even though the 
> other option would be more obvious code strictly speaking.

Tvrtko & Ville, can you reach a solution on this one? Or is there a
new patch that I may have missed?

BR,
Jani.


>
> Regards,
>
> Tvrtko
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to