diff mbox

[1/2] drm/i915: Add missing locking to primary plane handlers

Message ID 1404948131-12448-1-git-send-email-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matt Roper July 9, 2014, 11:22 p.m. UTC
intel_primary_plane_{setplane,disable} were lacking struct_mutex locking
around their GEM operations.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Daniel Vetter July 10, 2014, 6:52 a.m. UTC | #1
On Wed, Jul 09, 2014 at 04:22:10PM -0700, Matt Roper wrote:
> intel_primary_plane_{setplane,disable} were lacking struct_mutex locking
> around their GEM operations.
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Both merged with a reported-by: Damien added to this one here. Btw have
you checked that your primary plane tests hit this with the new WARN_ONs?
I'm missing the Testcase: tag ;-)
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5a50ff9..6b25068 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11389,9 +11389,11 @@ intel_primary_plane_disable(struct drm_plane *plane)
>  	intel_disable_primary_hw_plane(dev_priv, intel_plane->plane,
>  				       intel_plane->pipe);
>  disable_unpin:
> +	mutex_lock(&dev->struct_mutex);
>  	i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
>  			  INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
>  	intel_unpin_fb_obj(intel_fb_obj(plane->fb));
> +	mutex_unlock(&dev->struct_mutex);
>  	plane->fb = NULL;
>  
>  	return 0;
> @@ -11448,6 +11450,8 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	 * turn on the display with all planes setup as desired.
>  	 */
>  	if (!crtc->enabled) {
> +		mutex_lock(&dev->struct_mutex);
> +
>  		/*
>  		 * If we already called setplane while the crtc was disabled,
>  		 * we may have an fb pinned; unpin it.
> @@ -11459,7 +11463,10 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  				  INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
>  
>  		/* Pin and return without programming hardware */
> -		return intel_pin_and_fence_fb_obj(dev, obj, NULL);
> +		ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
> +		mutex_unlock(&dev->struct_mutex);
> +
> +		return ret;
>  	}
>  
>  	intel_crtc_wait_for_pending_flips(crtc);
> @@ -11471,14 +11478,18 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	 * because plane->fb still gets set and pinned.
>  	 */
>  	if (!visible) {
> +		mutex_lock(&dev->struct_mutex);
> +
>  		/*
>  		 * Try to pin the new fb first so that we can bail out if we
>  		 * fail.
>  		 */
>  		if (plane->fb != fb) {
>  			ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
> -			if (ret)
> +			if (ret) {
> +				mutex_unlock(&dev->struct_mutex);
>  				return ret;
> +			}
>  		}
>  
>  		i915_gem_track_fb(old_obj, obj,
> @@ -11494,6 +11505,8 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>  			if (plane->fb)
>  				intel_unpin_fb_obj(old_obj);
>  
> +		mutex_unlock(&dev->struct_mutex);
> +
>  		return 0;
>  	}
>  
> -- 
> 1.8.5.1
> 
> _______________________________________________
> 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 5a50ff9..6b25068 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11389,9 +11389,11 @@  intel_primary_plane_disable(struct drm_plane *plane)
 	intel_disable_primary_hw_plane(dev_priv, intel_plane->plane,
 				       intel_plane->pipe);
 disable_unpin:
+	mutex_lock(&dev->struct_mutex);
 	i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
 			  INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
 	intel_unpin_fb_obj(intel_fb_obj(plane->fb));
+	mutex_unlock(&dev->struct_mutex);
 	plane->fb = NULL;
 
 	return 0;
@@ -11448,6 +11450,8 @@  intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 * turn on the display with all planes setup as desired.
 	 */
 	if (!crtc->enabled) {
+		mutex_lock(&dev->struct_mutex);
+
 		/*
 		 * If we already called setplane while the crtc was disabled,
 		 * we may have an fb pinned; unpin it.
@@ -11459,7 +11463,10 @@  intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
 				  INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
 
 		/* Pin and return without programming hardware */
-		return intel_pin_and_fence_fb_obj(dev, obj, NULL);
+		ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
+		mutex_unlock(&dev->struct_mutex);
+
+		return ret;
 	}
 
 	intel_crtc_wait_for_pending_flips(crtc);
@@ -11471,14 +11478,18 @@  intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 * because plane->fb still gets set and pinned.
 	 */
 	if (!visible) {
+		mutex_lock(&dev->struct_mutex);
+
 		/*
 		 * Try to pin the new fb first so that we can bail out if we
 		 * fail.
 		 */
 		if (plane->fb != fb) {
 			ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
-			if (ret)
+			if (ret) {
+				mutex_unlock(&dev->struct_mutex);
 				return ret;
+			}
 		}
 
 		i915_gem_track_fb(old_obj, obj,
@@ -11494,6 +11505,8 @@  intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
 			if (plane->fb)
 				intel_unpin_fb_obj(old_obj);
 
+		mutex_unlock(&dev->struct_mutex);
+
 		return 0;
 	}