On 04/13/2015 01:09 PM, Jani Nikula wrote:
On Thu, 26 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> 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.ursulin@xxxxxxxxx>
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 <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
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?
It was pretty much bike shedding - I am happy with this version since it
has a single lock/unlock on the normal path, compared to one pair per
active display with what Ville wanted.
Either approach makes for unclear code so needs a big comment anyway.
Which leaves only the exact placement of mutex_lock/unlock under discussion.
If we want to spend this much time on this that is.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
