From patchwork Wed Jun 1 22:06:52 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 9148535 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 B0CF860777 for ; Wed, 1 Jun 2016 22:08:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A40182040D for ; Wed, 1 Jun 2016 22:08:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 98CD6265F9; Wed, 1 Jun 2016 22:08:28 +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=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4ACD92040D for ; Wed, 1 Jun 2016 22:08:28 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2F04C6EAB7; Wed, 1 Jun 2016 22:07:53 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 13C096EAAF for ; Wed, 1 Jun 2016 22:07:49 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id e3so10583528wme.2 for ; Wed, 01 Jun 2016 15:07:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=XY2wVI5uFGWJq6c3hL16YPoMfnPCiOlpN9OUKJV9FwU=; b=bJ+YTygThKO1vzl8HizGt2MA7MizVowbYQryE7S7ASxn+mJiQrOUaiKsosB9UxB1ni EXVEEWz+0hdJ1zzF5aCMTVlYeGK5HkwfXV/8HG3KoK/do+rMi8fYXzuyhwfoq1r7aL/o wswQPv4+EEnrEZ5ZiL5XnfIBa3OPxqIyMlHAE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=XY2wVI5uFGWJq6c3hL16YPoMfnPCiOlpN9OUKJV9FwU=; b=YgmzfMZWK/Up7y/viMOiBcz0QZSogQjNR74S5D0PywRrtg+zaeCfK8H1A3rA160p8c mfJRdkidb76l79w6HdvGy89PcdCmUxQEYt2gTHAVoZw7Jfo9rG696TwC0derG0P6VtCG cNR9StTaibAdAg7BDDFmITIDbgdBMZ5zg41YK7GToEdmHpiR7aGaqDHAdcnmvnZBy6fZ m5iR+WKjw/+0n9mezghoKwTUeEBOV0vsy0GPiAxl1+iIjT2aaYXKFUbFMbNv3UQAvdHl xsDjx1ZiVRnx37NuVkx0gN9No9y+0PzMaI1ykeLJOtJ2XvLgsUS1sAz0ZHaYbax3crDk 2VXQ== X-Gm-Message-State: ALyK8tKZyEHVCuYh50DYyUfCzOUPnp0YyZNsZMqgj8eFeeFhaZQZpIKLQZNZUvWV47f2OQ== X-Received: by 10.28.44.87 with SMTP id s84mr5457959wms.61.1464818867094; Wed, 01 Jun 2016 15:07:47 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:56b5:0:ac27:b86c:7764:9429]) by smtp.gmail.com with ESMTPSA id lf7sm1571508wjb.23.2016.06.01.15.07.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Jun 2016 15:07:46 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Thu, 2 Jun 2016 00:06:52 +0200 Message-Id: <1464818821-5736-30-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.8.1 In-Reply-To: <1464818821-5736-1-git-send-email-daniel.vetter@ffwll.ch> References: <1464818821-5736-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development , Daniel Vetter Subject: [Intel-gfx] [PATCH 29/38] drm/i915: Move fb_bits updating later in atomic_commit X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Currently it's part of prepare_fb, still in the first phase of atomic_commit which might fail. Which means that we need to have some heuristics in cleanup_fb to figure out whether things failed, or whether we just clean up the old fbs. That's fragile, and worse, once we start pipelining commits gets confused: While the last commit is still getting cleanup up we already hammer in the new one, and fb_bits aren't refcounted, resulting in lost bits and WARN_ON galore. We could instead try to make cleanup_fb more clever, but a simpler fix is to postpone the fb_bits tracking past the point of no return, where we commit all the software state. That also makes conceptually more sense, since fb_bits must be updated synchronously from the ioctl (they track usage from userspace pov, not from the hw pov), right before we're fully committed to the updated. This fixes WARNING splats from track_fb with page_flip implemented through atomic_commit. Testcase: igt/kms_flip/flip-vs-rmfb Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 14fd10ec1bbf..adf80029ab64 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13882,6 +13882,25 @@ static void intel_atomic_commit_work(struct work_struct *work) intel_atomic_commit_tail(state); } +static void intel_atomic_track_fbs(struct drm_atomic_state *state) +{ + struct drm_plane_state *old_plane_state; + struct drm_plane *plane; + struct drm_i915_gem_object *obj, *old_obj; + struct intel_plane *intel_plane; + int i; + + mutex_lock(&state->dev->struct_mutex); + for_each_plane_in_state(state, plane, old_plane_state, i) { + obj = intel_fb_obj(plane->state->fb); + old_obj = intel_fb_obj(old_plane_state->fb); + intel_plane = to_intel_plane(plane); + + i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit); + } + mutex_unlock(&state->dev->struct_mutex); +} + /** * intel_atomic_commit - commit validated state object * @dev: DRM device @@ -13922,6 +13941,7 @@ static int intel_atomic_commit(struct drm_device *dev, dev_priv->wm.distrust_bios_wm = false; dev_priv->wm.skl_results = intel_state->wm_results; intel_shared_dpll_commit(state); + intel_atomic_track_fbs(state); if (nonblock) queue_work(system_unbound_wq, &state->commit_work); @@ -14001,7 +14021,6 @@ intel_prepare_plane_fb(struct drm_plane *plane, { struct drm_device *dev = plane->dev; struct drm_framebuffer *fb = new_state->fb; - struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb); int ret = 0; @@ -14058,16 +14077,12 @@ intel_prepare_plane_fb(struct drm_plane *plane, ret = intel_pin_and_fence_fb_obj(fb, new_state->rotation); } - if (ret == 0) { - if (obj) { - struct intel_plane_state *plane_state = - to_intel_plane_state(new_state); - - i915_gem_request_assign(&plane_state->wait_req, - obj->last_write_req); - } + if (ret == 0 && obj) { + struct intel_plane_state *plane_state = + to_intel_plane_state(new_state); - i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit); + i915_gem_request_assign(&plane_state->wait_req, + obj->last_write_req); } return ret; @@ -14087,7 +14102,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane, const struct drm_plane_state *old_state) { struct drm_device *dev = plane->dev; - struct intel_plane *intel_plane = to_intel_plane(plane); struct intel_plane_state *old_intel_state; struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb); struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb); @@ -14101,11 +14115,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane, !INTEL_INFO(dev)->cursor_needs_physical)) intel_unpin_fb_obj(old_state->fb, old_state->rotation); - /* prepare_fb aborted? */ - 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); - i915_gem_request_assign(&old_intel_state->wait_req, NULL); }