On Wed, Sep 23, 2015 at 01:27:11PM +0200, Maarten Lankhorst wrote:
> Only acquire the struct_mutex once, and interruptibly.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Your headline/commit message seem a bit sparse here...you may want to
make it clear that this refers to framebuffer preparation/cleanup for
atomic commits. The i915 driver in general still has a bunch of of
struct_mutex usage in other places that isn't touched by this patch.
Effectively you're changing logic from:
loop {
grab mutex()
stuff
drop mutex()
}
to
grab mutex()
loop {
stuff
}
drop mutex()
The code change itself looks fine to me, so with an updated commit
message you can consider this
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
Matt
> ---
> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cd651ff6c15b..2f046134cc9a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13008,8 +13008,13 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
> return ret;
> }
>
> + ret = i915_mutex_lock_interruptible(dev);
> + if (ret)
> + return ret;
> +
> ret = drm_atomic_helper_prepare_planes(dev, state);
>
> + mutex_unlock(&dev->struct_mutex);
> return ret;
> }
>
> @@ -13108,7 +13113,10 @@ static int intel_atomic_commit(struct drm_device *dev,
> /* FIXME: add subpixel order */
>
> drm_atomic_helper_wait_for_vblanks(dev, state);
> +
> + mutex_lock(&dev->struct_mutex);
> drm_atomic_helper_cleanup_planes(dev, state);
> + mutex_unlock(&dev->struct_mutex);
>
> if (any_ms)
> intel_modeset_check_state(dev, state);
> @@ -13295,10 +13303,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> if (!obj && !old_obj)
> return 0;
>
> - ret = i915_mutex_lock_interruptible(dev);
> - if (ret)
> - return ret;
> -
> if (old_obj) {
> struct drm_crtc_state *crtc_state =
> drm_atomic_get_existing_crtc_state(new_state->state, plane->state->crtc);
> @@ -13319,7 +13323,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>
> /* Swallow -EIO errors to allow updates during hw lockup. */
> if (ret && ret != -EIO)
> - goto out;
> + return ret;
> }
>
> if (!obj) {
> @@ -13337,9 +13341,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> if (ret == 0)
> i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
>
> -out:
> - mutex_unlock(&dev->struct_mutex);
> -
> return ret;
> }
>
> @@ -13362,7 +13363,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
> if (!obj && !old_obj)
> return;
>
> - mutex_lock(&dev->struct_mutex);
> if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
> !INTEL_INFO(dev)->cursor_needs_physical))
> intel_unpin_fb_obj(old_state->fb, old_state);
> @@ -13371,7 +13371,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
> if ((old_obj && (old_obj->frontbuffer_bits & intel_plane->frontbuffer_bit)) ||
> (obj && !(obj->frontbuffer_bits & intel_plane->frontbuffer_bit)))
> i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
> - mutex_unlock(&dev->struct_mutex);
> }
>
> int
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
