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

[Daniel Vetter <daniel@ffwll.ch>]

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch