From patchwork Wed Nov 26 17:23:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 5386951 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 53F6C9F2F5 for ; Wed, 26 Nov 2014 17:23:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 64CEB201F4 for ; Wed, 26 Nov 2014 17:23:41 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id EF07820149 for ; Wed, 26 Nov 2014 17:23:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A8BEA6E10D; Wed, 26 Nov 2014 09:23:38 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wg0-f42.google.com (mail-wg0-f42.google.com [74.125.82.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 7E6126E12A for ; Wed, 26 Nov 2014 09:23:37 -0800 (PST) Received: by mail-wg0-f42.google.com with SMTP id z12so4366687wgg.1 for ; Wed, 26 Nov 2014 09:23:36 -0800 (PST) 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=DBn+trtqcH/tz5pQUOH/6tH39Vwnw4kErKxriIP/9CQ=; b=WYGcTxaA8TYFMKeLz7gfPbZyD6lSTQP3bMGb8KTaAsVpmujUZyG2LCbtjJnkpqtjJC A9zHzRBILPI82bMWB7cqcfBcU0fbtoAfcKUOUDqEyj4TRp90L2W67cyw6IVgz1PLd5NL UrMk6XErzR8PiRtbPy8R5PyaRVnbLLqW5DpAw= 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=DBn+trtqcH/tz5pQUOH/6tH39Vwnw4kErKxriIP/9CQ=; b=a7bOiAf5ekxyWgj8bUYzghFjO/Nqf0NGVMjqnepYSXFW12+vKIjOt4bkI3MQbSk4Hx 5EZPs0iVeW2qTVkLm3I696slh1YpdyQnAIhXAxPuntNpJzFGmWR3rZ5j8+hDpfN+iQu5 yZa1fEgethiueFfSB4XrroEDHe4fK5GPCwDlTI6oWl89GBf6k0aDNf492fLNXchf38fP UVXMp73ITwfuf+OPkznStviRieJdYb72d3C35p/2oYQH8jI/oISW/+CIsD8MI9CWk132 meIx/swmJ7rYfvMJQgVvIwJMjz1BVB94giPvZWtzuYG3raDg0ddBKVs34PqW3oFDFfEe cgjQ== X-Gm-Message-State: ALoCoQkywFIn1saHWeE63AINIVtaK608oSfEvI2Sgtbhyoja5JlJPa5T5MXbnUyPFWp4Wgq4rWWf X-Received: by 10.194.23.10 with SMTP id i10mr49817827wjf.11.1417022616827; Wed, 26 Nov 2014 09:23:36 -0800 (PST) Received: from phenom.ffwll.local (84-73-67-144.dclient.hispeed.ch. [84.73.67.144]) by mx.google.com with ESMTPSA id ef6sm7872607wic.1.2014.11.26.09.23.35 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 26 Nov 2014 09:23:35 -0800 (PST) From: Daniel Vetter To: DRI Development Subject: [PATCH 2/2] drm/atomic-helper: Again check modeset *before* plane states Date: Wed, 26 Nov 2014 18:23:57 +0100 Message-Id: <1417022637-20550-2-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.1.1 In-Reply-To: <1417022637-20550-1-git-send-email-daniel.vetter@ffwll.ch> References: <1417022637-20550-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development , Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This essentially reverts commit 934ce1c23624526d9d784e0499190bb48113e6f4 Author: Rob Clark Date: Wed Nov 19 16:41:33 2014 -0500 drm/atomic: check mode_changed *after* atomic_check Depending upon the driver both orders (or maybe even interleaving) is required: - If ->atomic_check updates ->mode_changed then helper_check_modeset must be run afters. - If ->atomic_check depends upon accurate adjusted dotclock values for e.g. watermarks, then helper_check_modeset must be run first. The failure mode in the first case is usually a totally angry hw because the pixel format switching doesn't happen. The failure mode in the later case is usually nothing, since in most cases the old adjusted mode from the previous modeset wont be too far off to be a problem. So just underruns and perhaps even just suboptimal (from a power consumption) watermarks. Furthermore in the transitional helpers we only call ->atomic_check after the new modeset state has been fully set up (and hence computed). Given that asymmetry in expected failure modes I think it's safer to go back to the older order. So do that and give msm a special check function to compensate. Also update kerneldoc to explain this a bit. v2: Actually add the missing hunk Rob spotted. v3: Move msm_atomic_check into msm_atomic.c, requested by Rob. Cc: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic_helper.c | 10 ++++++++-- drivers/gpu/drm/msm/msm_atomic.c | 20 ++++++++++++++++++++ drivers/gpu/drm/msm/msm_drv.c | 2 +- drivers/gpu/drm/msm/msm_drv.h | 2 ++ 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ed58c4c06d5e..6c0c6918bc12 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -508,6 +508,12 @@ EXPORT_SYMBOL(drm_atomic_helper_check_planes); * Drivers without such needs can directly use this as their ->atomic_check() * callback. * + * This just wraps the two parts of the state checking for planes and modeset + * state in the default order: First it calls drm_atomic_helper_check_modeset() + * and then drm_atomic_helper_check_planes(). The assumption is that the + * ->atomic_check functions depend upon an updated adjusted_mode.clock to + * e.g. properly compute watermarks. + * * RETURNS * Zero for success or -errno */ @@ -516,11 +522,11 @@ int drm_atomic_helper_check(struct drm_device *dev, { int ret; - ret = drm_atomic_helper_check_planes(dev, state); + ret = drm_atomic_helper_check_modeset(dev, state); if (ret) return ret; - ret = drm_atomic_helper_check_modeset(dev, state); + ret = drm_atomic_helper_check_planes(dev, state); if (ret) return ret; diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index f0de412e13dc..f8f18e882f97 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -81,6 +81,26 @@ static void add_fb(struct msm_commit *c, struct drm_framebuffer *fb) } +int msm_atomic_check(struct drm_device *dev, + struct drm_atomic_state *state) +{ + int ret; + + /* + * msm ->atomic_check can update ->mode_changed for pixel format + * changes, hence must be run before we check the modeset changes. + */ + ret = drm_atomic_helper_check_planes(dev, state); + if (ret) + return ret; + + ret = drm_atomic_helper_check_modeset(dev, state); + if (ret) + return ret; + + return ret; +} + /** * drm_atomic_helper_commit - commit validated state object * @dev: DRM device diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index d3b791b7ddef..473e4d6051e9 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -29,7 +29,7 @@ static void msm_fb_output_poll_changed(struct drm_device *dev) static const struct drm_mode_config_funcs mode_config_funcs = { .fb_create = msm_framebuffer_create, .output_poll_changed = msm_fb_output_poll_changed, - .atomic_check = drm_atomic_helper_check, + .atomic_check = msm_atomic_check, .atomic_commit = msm_atomic_commit, }; diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 136303818436..d408e02e4ee5 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -144,6 +144,8 @@ void __msm_fence_worker(struct work_struct *work); (_cb)->func = _func; \ } while (0) +int msm_atomic_check(struct drm_device *dev, + struct drm_atomic_state *state); int msm_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state, bool async);