diff mbox

drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover

Message ID 1428937383-5949-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin April 13, 2015, 3:03 p.m. UTC 
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>
---
 drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Shuang He April 13, 2015, 6:18 p.m. UTC | #1 
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@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  Test                                drm-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 '*'
Ville Syrjälä April 14, 2015, 9 a.m. UTC | #2 
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
Jani Nikula April 14, 2015, 1:19 p.m. UTC | #3 
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
diff mbox

Patch

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);
 }