Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
On Mon, Apr 13, 2015 at 04:03:03PM +0100, 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.
Regression from commit ab8d66752a9c28cd6c94fa173feacdfc1554aa03
Author: Tvrtko Ursulin tvrtko.ursu...@intel.com
Date: Mon Feb 2 15:44:15 2015 +
drm/i915: Track old framebuffer instead of object
v2: Bikeshedding.
References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
On Tue, 14 Apr 2015, Ville Syrjälä ville.syrj...@linux.intel.com wrote:
On Mon, Apr 13, 2015 at 04:03:03PM +0100, 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.
Regression from commit ab8d66752a9c28cd6c94fa173feacdfc1554aa03
Author: Tvrtko Ursulin tvrtko.ursu...@intel.com
Date: Mon Feb 2 15:44:15 2015 +
drm/i915: Track old framebuffer instead of object
v2: Bikeshedding.
References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6186 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -4 276/276 272/276 ILK 301/301 301/301 SNB -22 316/316 294/316 IVB -1 328/328 327/328 BYT 285/285 285/285 HSW 394/394 394/394 BDW 321/321 321/321 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt@gem_userptr_blits@coherency-unsync CRASH(2)PASS(4) CRASH(1)PASS(1) PNV igt@gen3_render_linear_blits FAIL(4)PASS(7) FAIL(1)PASS(1) PNV igt@gen3_render_mixed_blits FAIL(5)PASS(6) FAIL(1)PASS(1) PNV igt@gen3_render_tiledx_blits FAIL(5)PASS(7) FAIL(1)PASS(1) SNB igt@kms_cursor_crc@cursor-size-change NSPT(1)PASS(1) NSPT(2) SNB igt@kms_flip_event_leak NSPT(1)PASS(1) NSPT(2) SNB igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip NSPT(2)PASS(1) NSPT(2) SNB igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip NSPT(2)PASS(1) NSPT(2) SNB igt@kms_rotation_crc@primary-rotation NSPT(2)PASS(1) NSPT(2) SNB igt@kms_rotation_crc@sprite-rotation NSPT(2)PASS(3) NSPT(2) SNB igt@pm_rpm@cursor NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@cursor-dpms NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@dpms-mode-unset-non-lpsp NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@dpms-non-lpsp NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@drm-resources-equal NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@fences NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@fences-dpms NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@gem-execbuf NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@gem-mmap-cpu NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@gem-mmap-gtt NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@gem-pread NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@i2c NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@modeset-non-lpsp NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@modeset-non-lpsp-stress-no-wait NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@pci-d3-state NSPT(2)PASS(1) NSPT(2) SNB igt@pm_rpm@rte NSPT(2)PASS(1) NSPT(2) IVB igt@gem_pwrite_pread@uncached-copy-performance DMESG_WARN(1)PASS(7) DMESG_WARN(1)PASS(1) (dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
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 +, 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
Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
On Mon, Apr 13, 2015 at 02:37:41PM +0100, Tvrtko Ursulin wrote:
On 04/13/2015 01:09 PM, Jani Nikula wrote:
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 +, 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?
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.
I don't see what's unclear about locking only around the call that needs
the lock.
If we want to spend this much time on this that is.
Regards,
Tvrtko
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
On 04/13/2015 01:09 PM, Jani Nikula wrote:
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 +, 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?
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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
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.
Regression from commit ab8d66752a9c28cd6c94fa173feacdfc1554aa03
Author: Tvrtko Ursulin tvrtko.ursu...@intel.com
Date: Mon Feb 2 15:44:15 2015 +
drm/i915: Track old framebuffer instead of object
v2: Bikeshedding.
References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
On Thu, Mar 26, 2015 at 12:39:40PM +, 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.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
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);
--
2.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
On 03/26/2015 01:30 PM, Ville Syrjälä wrote:
On Thu, Mar 26, 2015 at 12:39:40PM +, 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.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6061 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -1 276/276 275/276 ILK 303/303 303/303 SNB 304/304 304/304 IVB 339/339 339/339 BYT 287/287 287/287 HSW 362/362 362/362 BDW 310/310 310/310 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt@gem_userptr_blits@minor-unsync-interruptible PASS(2) DMESG_WARN(2) (dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/i915_gem_evict.c:#i915_gem_evict_vm[i915]()@WARNING:.* at .* i915_gem_evict_vm+0x Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
