From patchwork Wed Apr 17 17:04:52 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?VmlsbGUgU3lyasOkbMOk?= X-Patchwork-Id: 2454791 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 25E693FC64 for ; Wed, 17 Apr 2013 17:10:27 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 04E47E610A for ; Wed, 17 Apr 2013 10:10:27 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id BC7B7E673E; Wed, 17 Apr 2013 10:05:22 -0700 (PDT) Received: from azsmga002.ch.intel.com ([10.2.17.35]) by azsmga101.ch.intel.com with ESMTP; 17 Apr 2013 10:05:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.87,494,1363158000"; d="scan'208";a="228436139" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.168]) by AZSMGA002.ch.intel.com with SMTP; 17 Apr 2013 10:04:53 -0700 Received: by stinkbox (sSMTP sendmail emulation); Wed, 17 Apr 2013 20:04:52 +0300 From: ville.syrjala@linux.intel.com To: dri-devel@lists.freedesktop.org Date: Wed, 17 Apr 2013 20:04:52 +0300 Message-Id: <1366218292-16565-1-git-send-email-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 1.8.1.5 MIME-Version: 1.0 Cc: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] [RFC][PATCH] drm: Insane but more fine grained locking for planes X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org From: Ville Syrjälä Instead of locking all modeset locks during plane updates, use just a single CRTC mutex. To make that work, track the CRTC that "owns" the plane currently. During enable/update that means the new CRTC, and during disable it means the old CRTC. Since the plane state is no longer protected by a single lock, we need to sprinkle some additional memory barriers when relinquishing ownership. Otherwise the next CRTC might observe some stale state even though the crtc_mutex already got updated. drm_framebuffer_remove() doesn't need extra barriers since it already holds all CRTC locks, and thus no-one can be poking around at the same time. On the read side cmpxchg() already should have the necessary memory barriers. This design implies that before a plane can be moved to another CRTC, it must be disabled first, even if the hardware would offer some kind of mechanism to move an active plane over directly. I believe everyone has agreed that this an acceptable compromise. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_crtc.c | 43 +++++++++++++++++++++++++++++++++++++++---- include/drm/drm_crtc.h | 3 +++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 957fb70..6f7385e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -576,6 +576,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) __drm_framebuffer_unreference(plane->fb); plane->fb = NULL; plane->crtc = NULL; + plane->crtc_mutex = NULL; } } drm_modeset_unlock_all(dev); @@ -1785,6 +1786,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data, int ret = 0; unsigned int fb_width, fb_height; int i; + struct mutex *old_crtc_mutex; if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -EINVAL; @@ -1804,12 +1806,33 @@ int drm_mode_setplane(struct drm_device *dev, void *data, /* No fb means shut it down */ if (!plane_req->fb_id) { - drm_modeset_lock_all(dev); + struct mutex *crtc_mutex; + + retry: + crtc_mutex = ACCESS_ONCE(plane->crtc_mutex); + + /* plane was already disabled? */ + if (!crtc_mutex) + return 0; + + mutex_lock(crtc_mutex); + + /* re-check that plane is still on the same crtc... */ + if (crtc_mutex != plane->crtc_mutex) { + mutex_unlock(crtc_mutex); + goto retry; + } + old_fb = plane->fb; plane->funcs->disable_plane(plane); plane->crtc = NULL; plane->fb = NULL; - drm_modeset_unlock_all(dev); + + smp_wmb(); + plane->crtc_mutex = NULL; + + mutex_unlock(crtc_mutex); + goto out; } @@ -1875,7 +1898,15 @@ int drm_mode_setplane(struct drm_device *dev, void *data, goto out; } - drm_modeset_lock_all(dev); + mutex_lock(&crtc->mutex); + + old_crtc_mutex = cmpxchg(&plane->crtc_mutex, NULL, &crtc->mutex); + if (old_crtc_mutex != NULL && old_crtc_mutex != &crtc->mutex) { + mutex_unlock(&crtc->mutex); + ret = -EBUSY; + goto out; + } + ret = plane->funcs->update_plane(plane, crtc, fb, plane_req->crtc_x, plane_req->crtc_y, plane_req->crtc_w, plane_req->crtc_h, @@ -1886,8 +1917,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data, plane->crtc = crtc; plane->fb = fb; fb = NULL; + } else { + smp_wmb(); + plane->crtc_mutex = old_crtc_mutex; } - drm_modeset_unlock_all(dev); + + mutex_unlock(&crtc->mutex); out: if (fb) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8c7846b..cc3779f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -651,6 +651,7 @@ struct drm_plane_funcs { * @dev: DRM device this plane belongs to * @head: for list management * @base: base mode object + * @crtc_mutex: points to the mutex of the current "owner" CRTC * @possible_crtcs: pipes this plane can be bound to * @format_types: array of formats supported by this plane * @format_count: number of formats supported @@ -669,6 +670,8 @@ struct drm_plane { struct drm_mode_object base; + struct mutex *crtc_mutex; + uint32_t possible_crtcs; uint32_t *format_types; uint32_t format_count;