From patchwork Fri May 25 18:50:45 2018 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: 10428343 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7F01C602D8 for ; Fri, 25 May 2018 18:52:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 656A2297A3 for ; Fri, 25 May 2018 18:52:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 596FF297A5; Fri, 25 May 2018 18:52:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9A7AD297A3 for ; Fri, 25 May 2018 18:52:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1A1466EAA9; Fri, 25 May 2018 18:51:48 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 50EFA6EAEB; Fri, 25 May 2018 18:51:46 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 May 2018 11:51:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,440,1520924400"; d="scan'208";a="52218447" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by FMSMGA003.fm.intel.com with SMTP; 25 May 2018 11:51:43 -0700 Received: by stinkbox (sSMTP sendmail emulation); Fri, 25 May 2018 21:51:42 +0300 From: Ville Syrjala To: dri-devel@lists.freedesktop.org Date: Fri, 25 May 2018 21:50:45 +0300 Message-Id: <20180525185045.29689-14-ville.syrjala@linux.intel.com> X-Mailer: git-send-email 2.16.1 In-Reply-To: <20180525185045.29689-1-ville.syrjala@linux.intel.com> References: <20180525185045.29689-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v2 13/13] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP From: Ville Syrjälä Stop playing around with plane->crtc/fb/old_fb with atomic drivers. Make life a lot simpler when we don't have to do the magic old_fb vs. fb dance around plane updates. That way we can't risk plane->fb getting out of sync with plane->state->fb and we're less likely to leak any refcounts as well. Signed-off-by: Ville Syrjälä Reviewed-by: Maarten Lankhorst Acked-by: Harry Wentland --- drivers/gpu/drm/drm_atomic.c | 55 ++++--------------------------------- drivers/gpu/drm/drm_atomic_helper.c | 15 +--------- drivers/gpu/drm/drm_crtc.c | 8 ++++-- drivers/gpu/drm/drm_fb_helper.c | 7 ----- drivers/gpu/drm/drm_framebuffer.c | 5 ---- drivers/gpu/drm/drm_plane.c | 14 ++++++---- drivers/gpu/drm/drm_plane_helper.c | 4 ++- include/drm/drm_atomic.h | 3 -- 8 files changed, 24 insertions(+), 87 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 41334d0973a2..ee4b43b9404e 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -700,6 +700,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state, WARN_ON(!state->acquire_ctx); + /* the legacy pointers should never be set */ + WARN_ON(plane->fb); + WARN_ON(plane->old_fb); + WARN_ON(plane->crtc); + plane_state = drm_atomic_get_existing_plane_state(state, plane); if (plane_state) return plane_state; @@ -2051,45 +2056,6 @@ int drm_atomic_set_property(struct drm_atomic_state *state, return ret; } -/** - * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb pointers. - * - * @dev: drm device to check. - * @plane_mask: plane mask for planes that were updated. - * @ret: return value, can be -EDEADLK for a retry. - * - * Before doing an update &drm_plane.old_fb is set to &drm_plane.fb, but before - * dropping the locks old_fb needs to be set to NULL and plane->fb updated. This - * is a common operation for each atomic update, so this call is split off as a - * helper. - */ -void drm_atomic_clean_old_fb(struct drm_device *dev, - unsigned plane_mask, - int ret) -{ - struct drm_plane *plane; - - /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping - * locks (ie. while it is still safe to deref plane->state). We - * need to do this here because the driver entry points cannot - * distinguish between legacy and atomic ioctls. - */ - drm_for_each_plane_mask(plane, dev, plane_mask) { - if (ret == 0) { - struct drm_framebuffer *new_fb = plane->state->fb; - if (new_fb) - drm_framebuffer_get(new_fb); - plane->fb = new_fb; - plane->crtc = plane->state->crtc; - - if (plane->old_fb) - drm_framebuffer_put(plane->old_fb); - } - plane->old_fb = NULL; - } -} -EXPORT_SYMBOL(drm_atomic_clean_old_fb); - /** * DOC: explicit fencing properties * @@ -2310,9 +2276,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, unsigned int copied_objs, copied_props; struct drm_atomic_state *state; struct drm_modeset_acquire_ctx ctx; - struct drm_plane *plane; struct drm_out_fence_state *fence_state; - unsigned plane_mask; int ret = 0; unsigned int i, j, num_fences; @@ -2352,7 +2316,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET); retry: - plane_mask = 0; copied_objs = 0; copied_props = 0; fence_state = NULL; @@ -2423,12 +2386,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, copied_props++; } - if (obj->type == DRM_MODE_OBJECT_PLANE && count_props && - !(arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)) { - plane = obj_to_plane(obj); - plane_mask |= (1 << drm_plane_index(plane)); - plane->old_fb = plane->fb; - } drm_mode_object_put(obj); } @@ -2449,8 +2406,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, } out: - drm_atomic_clean_old_fb(dev, plane_mask, ret); - complete_crtc_signaling(dev, state, fence_state, num_fences, !ret); if (ret == -EDEADLK) { diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 130da5195f3b..232fa11a5e31 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2914,7 +2914,6 @@ static int __drm_atomic_helper_disable_all(struct drm_device *dev, struct drm_plane *plane; struct drm_crtc_state *crtc_state; struct drm_crtc *crtc; - unsigned plane_mask = 0; int ret, i; state = drm_atomic_state_alloc(dev); @@ -2957,17 +2956,10 @@ static int __drm_atomic_helper_disable_all(struct drm_device *dev, goto free; drm_atomic_set_fb_for_plane(plane_state, NULL); - - if (clean_old_fbs) { - plane->old_fb = plane->fb; - plane_mask |= BIT(drm_plane_index(plane)); - } } ret = drm_atomic_commit(state); free: - if (plane_mask) - drm_atomic_clean_old_fb(dev, plane_mask, ret); drm_atomic_state_put(state); return ret; } @@ -3129,13 +3121,8 @@ int drm_atomic_helper_commit_duplicated_state(struct drm_atomic_state *state, state->acquire_ctx = ctx; - for_each_new_plane_in_state(state, plane, new_plane_state, i) { - WARN_ON(plane->crtc != new_plane_state->crtc); - WARN_ON(plane->fb != new_plane_state->fb); - WARN_ON(plane->old_fb); - + for_each_new_plane_in_state(state, plane, new_plane_state, i) state->planes[i].old_state = plane->state; - } for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) state->crtcs[i].old_state = crtc->state; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 237bd34212db..53828fc8d911 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -479,8 +479,12 @@ static int __drm_mode_set_config_internal(struct drm_mode_set *set, ret = crtc->funcs->set_config(set, ctx); if (ret == 0) { - crtc->primary->crtc = fb ? crtc : NULL; - crtc->primary->fb = fb; + struct drm_plane *plane = crtc->primary; + + if (!plane->state) { + plane->crtc = fb ? crtc : NULL; + plane->fb = fb; + } } drm_for_each_crtc(tmp, crtc->dev) { diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 2ee1eaa66188..6964fe99f8b1 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -368,7 +368,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ struct drm_plane *plane; struct drm_atomic_state *state; int i, ret; - unsigned int plane_mask; struct drm_modeset_acquire_ctx ctx; drm_modeset_acquire_init(&ctx, 0); @@ -381,7 +380,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ state->acquire_ctx = &ctx; retry: - plane_mask = 0; drm_for_each_plane(plane, dev) { plane_state = drm_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) { @@ -391,9 +389,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ plane_state->rotation = DRM_MODE_ROTATE_0; - plane->old_fb = plane->fb; - plane_mask |= 1 << drm_plane_index(plane); - /* disable non-primary: */ if (plane->type == DRM_PLANE_TYPE_PRIMARY) continue; @@ -430,8 +425,6 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ ret = drm_atomic_commit(state); out_state: - drm_atomic_clean_old_fb(dev, plane_mask, ret); - if (ret == -EDEADLK) goto backoff; diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index bfedceff87bb..46b11e46edbd 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -836,8 +836,6 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) goto unlock; plane_mask |= BIT(drm_plane_index(plane)); - - plane->old_fb = plane->fb; } /* This list is only filled when disable_crtcs is set. */ @@ -852,9 +850,6 @@ static int atomic_remove_fb(struct drm_framebuffer *fb) ret = drm_atomic_commit(state); unlock: - if (plane_mask) - drm_atomic_clean_old_fb(dev, plane_mask, ret); - if (ret == -EDEADLK) { drm_atomic_state_clear(state); drm_modeset_backoff(&ctx); diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 035054455301..143041666096 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -650,9 +650,11 @@ static int __setplane_internal(struct drm_plane *plane, crtc_x, crtc_y, crtc_w, crtc_h, src_x, src_y, src_w, src_h, ctx); if (!ret) { - plane->crtc = crtc; - plane->fb = fb; - drm_framebuffer_get(plane->fb); + if (!plane->state) { + plane->crtc = crtc; + plane->fb = fb; + drm_framebuffer_get(plane->fb); + } } else { plane->old_fb = NULL; } @@ -1092,8 +1094,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, /* Keep the old fb, don't unref it. */ plane->old_fb = NULL; } else { - plane->fb = fb; - drm_framebuffer_get(fb); + if (!plane->state) { + plane->fb = fb; + drm_framebuffer_get(fb); + } } out: diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index f88f68161519..2010794943bc 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -502,6 +502,7 @@ EXPORT_SYMBOL(drm_plane_helper_update); int drm_plane_helper_disable(struct drm_plane *plane) { struct drm_plane_state *plane_state; + struct drm_framebuffer *old_fb; /* crtc helpers love to call disable functions for already disabled hw * functions. So cope with that. */ @@ -521,8 +522,9 @@ int drm_plane_helper_disable(struct drm_plane *plane) plane_state->plane = plane; plane_state->crtc = NULL; + old_fb = plane_state->fb; drm_atomic_set_fb_for_plane(plane_state, NULL); - return drm_plane_helper_commit(plane, plane_state, plane->fb); + return drm_plane_helper_commit(plane, plane_state, old_fb); } EXPORT_SYMBOL(drm_plane_helper_disable); diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index a57a8aa90ffb..ca461b6cf71f 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -601,9 +601,6 @@ int __must_check drm_atomic_add_affected_planes(struct drm_atomic_state *state, struct drm_crtc *crtc); -void -drm_atomic_clean_old_fb(struct drm_device *dev, unsigned plane_mask, int ret); - int __must_check drm_atomic_check_only(struct drm_atomic_state *state); int __must_check drm_atomic_commit(struct drm_atomic_state *state); int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state);